Skip to content

Conversation

@omerfaruk-pseud
Copy link
Member

@omerfaruk-pseud omerfaruk-pseud commented Jan 30, 2026

Pull Request Checklist

Issue

https://otwarchive.atlassian.net/browse/AO3-7271

Purpose

When the instance uses Cloudflare, add Cloudflare's information - which will be available on the request headers- to akismet comment checks for better accuracy. It will not add anything otherwise.

Credit

ömer faruk (he/him)
marcus8448, for the tests

@omerfaruk-pseud
Copy link
Member Author

I couldn't figure out how to add response headers in tests, the current one just compares nil to nil. How can I properly test this?

Copy link
Member

@marcus8448 marcus8448 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a controller test should work (see comments/default_rails_actions_spec.rb)

Comment on lines 394 to 396
@comment.cloudflare_bot_score = headers["cf-bot-score"]
@comment.cloudflare_ja3_hash = headers["cf-ja3-hash"]
@comment.cloudflare_ja4 = headers["cf-ja4"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be request.headers (headers is for response headers).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, Jira ticket says "available in the response header" though?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried writing a test there too, the problem was even when I added cf info to response.headers it was resetting back to default headers when creating the comment and setting akismet_attributes from there.

Copy link
Member

@marcus8448 marcus8448 Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, Jira ticket says "available in the response header" though?

I think that might be a mistake in the ticket.

I've tried writing a test there too, the problem was even when I added cf info to response.headers it was resetting back to default headers when creating the comment and setting akismet_attributes from there.

I'm fairly certain setting request.headers should work? But the actual testing is a bit more complicated since the cloudflare attributes are short lived on the Comment. I'm not sure what the best practices are here so I'll open this up for someone else to review.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what we use else where is req.env["HTTP_X_UNICORNS"]

Like https://github.com/otwcode/otwarchive/blob/master/config/initializers/rack_attack.rb#L48

It's what we get in the request ( the information is inserted by cloudflare )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants