Skip to content

HOLD: Enable Content Security Policy for AWBW#1491

Draft
lenikadali wants to merge 3 commits into
mainfrom
enable-content-security-policy
Draft

HOLD: Enable Content Security Policy for AWBW#1491
lenikadali wants to merge 3 commits into
mainfrom
enable-content-security-policy

Conversation

@lenikadali
Copy link
Copy Markdown
Collaborator

@lenikadali lenikadali commented Apr 24, 2026

Closes #261

What is the goal of this PR and why is this important?

Enables Content Security Policy for AWBW based on what we're already using in the codebase.
The code is mostly vanilla Rails with minimal to no JavaScript so much of the Rails defaults have been removed.
We're adding this so that AWBW is not vulnerable to Cross-site scripting (XSS) attacks.

How did you approach the change?

I reviewed the issue, then read up on how Rails handles CSP configuration and compared the different options available to what the AWBW code is using. Important notes:

  • <%= csp_meta_tag %> is used when considering using 'unsafe-inline' as per Rails guide here. Since we are not considering using 'unsafe-inline' and it is not already used in the codebase, it has been omitted from the initial configuration.
  • As far as I can tell, we are not using Vite and JavaScript in a way that requires configurations related to them, so they have been removed.

Anything else to add?

I could have left in the Vite/JavaScript lines but removing them keeps the file cleaner and easy to read for someone looking at it. Happy to restore the lines if there's a preference for preserving them 😄

Edit: Note on the PR itself

It seems enabling the Content Security Policy interferes with how end-to-end tests run. The currently commented out tests succeed when you follow the test steps in a development environment but fail in the GitHub workflow; for some reason Capybara no longer is able to find the banner/flash message with the appropriate message nor is it able to detect that the redirection has happened.

I think this may be related to the usage of Turbo Stream in the different paths under test (going by the logs) but I cannot say for sure. I tried some fixes I saw in the wild here and here, but the first two commented out tests continued to fail.

For now putting this on hold because the number of specs affected seems likely to grow and disabling them all doesn't seem like the right solution.

@lenikadali
Copy link
Copy Markdown
Collaborator Author

Some tests are broken; going to look into them and fix. For now, switching to draft.

@lenikadali lenikadali marked this pull request as draft April 24, 2026 19:17
@maebeale
Copy link
Copy Markdown
Collaborator

@lenikadali i think these are flaky but am going to rebase and see if we get a green light

@lenikadali lenikadali force-pushed the enable-content-security-policy branch from 7061e68 to c69ec03 Compare May 19, 2026 13:11
@lenikadali lenikadali self-assigned this May 19, 2026
@lenikadali
Copy link
Copy Markdown
Collaborator Author

@lenikadali i think these are flaky but am going to rebase and see if we get a green light

I rebased and it seems now the build is failing because there's a security vulnerability surfaced by bundler-audit's check.

@lenikadali
Copy link
Copy Markdown
Collaborator Author

@lenikadali i think these are flaky but am going to rebase and see if we get a green light

I rebased and it seems now the build is failing because there's a security vulnerability surfaced by bundler-audit's check.

Have updated to fix the security vulnerability and the failures are persisting. Will investigate 🧐

@lenikadali lenikadali force-pushed the enable-content-security-policy branch from 4781727 to f852455 Compare May 22, 2026 12:07
@lenikadali lenikadali marked this pull request as ready for review May 22, 2026 14:56
@lenikadali lenikadali requested a review from maebeale May 22, 2026 14:56
@lenikadali
Copy link
Copy Markdown
Collaborator Author

lenikadali commented May 22, 2026

@maebeale these tests seem to be failing because of something related to "Turbo Stream", going by the logs:

web-1  | Started PATCH "/stories/1" for 172.18.0.1 at 2026-05-22 10:23:13 +0000
web-1  | Processing by StoriesController#update as TURBO_STREAM
web-1  |   Parameters: {"authenticity_token" => "[FILTERED]", "story" => {"title" => "Healing Through Art: A Survivor's Journey", "published" => "1", "featured" => "1", "publicly_visible" => "0", "publicly_featured" => "0", "windows_type_id" => "3", "workshop_id" => "21", "external_workshop_title" => "", "organization_id" => "8", "rhino_body" => "<p>Natus sit eum. Necessitatibus voluptas ipsa. Assumenda tenetur temporibus. Rerum quo aut. Deserunt et facilis. Fugiat sit dolore. Tenetur est est. Nisi consequuntur qui. Voluptas id provident. Sed at sit.</p>", "sector_ids" => ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""], "category_ids" => ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""], "author_credit_preference" => "", "youtube_url" => "https://youtube.com/watch?v=abcd1234xyz", "website_url" => "", "created_by_id" => "1", "spotlighted_facilitator_id" => "", "story_idea_id" => "", "updated_by_id" => "1"}, "commit" => "Save changes", "id" => "1"}

Manually editing the story title as the user Umberto successfully shows the message "Story was successfully updated." on the page, so I can confirm that the feature still works as intended, without any errors in the logs.

I am not as familiar with Turbo, so it would take me a bit longer to figure out a solution for the failing tests.

Copy link
Copy Markdown
Collaborator

@jmilljr24 jmilljr24 left a comment

Choose a reason for hiding this comment

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

@lenikadali I'm happy with this! Thanks!!

Thank you for manually testing the story failure. I'm personally ok with you disabling that test and opening a new issue for it if you want to get this PR merged. I can work on the test/turbo at a later date.

Enables Content Security Policy for AWBW based on
what we're already using in the codebase (the code
is mostly vanilla Rails with minimal to no JavaScript)
so much of the Rails defaults have removed.
@lenikadali lenikadali force-pushed the enable-content-security-policy branch from f852455 to 93d7fc3 Compare May 26, 2026 18:29
@lenikadali
Copy link
Copy Markdown
Collaborator Author

Currently the test in spec/system/change_password_flow_spec.rb is the one that is failing. Going through the test case steps manually works. The logs are similar to the stories spec:

web-1                      | Started DELETE "/users/sign_out" for 172.18.0.1 at 2026-05-26 18:21:23 +0000
web-1                      | Processing by Devise::SessionsController#destroy as TURBO_STREAM
web-1                      |   Parameters: {"reset_password" => "[FILTERED]"}
web-1                      |   User Load (21.3ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 1 ORDER BY `users`.`id` ASC LIMIT 1
web-1                      |   Bookmark Load (3.6ms)  SELECT `bookmarks`.* FROM `bookmarks` WHERE `bookmarks`.`user_id` = 1
web-1                      |   ↳ app/controllers/application_controller.rb:61:in 'ApplicationController#preload_current_user_associations'
web-1                      |   Person Load (2.0ms)  SELECT `people`.* FROM `people` WHERE `people`.`id` = 1 LIMIT 1
web-1                      |   ↳ app/controllers/application_controller.rb:63:in 'ApplicationController#preload_current_user_associations'
web-1                      |   Affiliation Load (11.8ms)  SELECT `affiliations`.* FROM `affiliations` WHERE `affiliations`.`person_id` = 1 AND `affiliations`.`inactive` = FALSE AND (affiliations.end_date IS NULL OR affiliations.end_date >= '2026-05-26')
web-1                      |   ↳ app/controllers/application_controller.rb:68:in 'ApplicationController#preload_current_user_associations'
web-1                      |   Organization Load (1.2ms)  SELECT `organizations`.* FROM `organizations` WHERE `organizations`.`id` = 1
web-1                      |   ↳ app/controllers/application_controller.rb:68:in 'ApplicationController#preload_current_user_associations'
web-1                      | Redirected to http://localhost:3000/users/password/new
web-1                      | Completed 303 See Other in 357ms (ActiveRecord: 14.7ms (5 queries, 0 cached) | GC: 0.0ms)
web-1                      | 
web-1                      | 
web-1                      | user: root
web-1                      | DELETE /users/sign_out
web-1                      | AVOID eager loading detected
web-1                      |   Affiliation => [:organization]
web-1                      |   Remove from your query: .includes([:organization])
web-1                      | Call stack
web-1                      |   /app/app/controllers/application_controller.rb:68:in 'ApplicationController#preload_current_user_associations'
web-1                      | 
web-1                      | 
web-1                      | Started GET "/users/password/new" for 172.18.0.1 at 2026-05-26 18:21:24 +0000
web-1                      | Processing by PasswordsController#new as TURBO_STREAM
web-1                      |   Rendering layout layouts/application.html.erb
web-1                      |   Rendering devise/passwords/new.html.erb within layouts/application
web-1                      |   Rendered layouts/mailer/_logo.html.erb (Duration: 69.2ms | GC: 0.0ms)
web-1                      |   Rendered devise/shared/_error_messages.html.erb (Duration: 1.2ms | GC: 0.0ms)
web-1                      |   Rendered devise/shared/_links.html.erb (Duration: 9.5ms | GC: 0.0ms)
web-1                      |   Rendered devise/passwords/new.html.erb within layouts/application (Duration: 169.0ms | GC: 0.0ms)
web-1                      |   Rendered shared/_svg_symbols.html.erb (Duration: 0.3ms | GC: 0.0ms)
web-1                      |   Rendered shared/_navbar_menu.html.erb (Duration: 5.0ms | GC: 0.0ms)
web-1                      |   Rendered shared/_navbar_menu_mobile.html.erb (Duration: 0.9ms | GC: 0.0ms)
web-1                      |   Rendered shared/_navbar.html.erb (Duration: 9.8ms | GC: 0.0ms)
web-1                      |   Banner Load (2.1ms)  SELECT id, content FROM `banners` WHERE `banners`.`published` = TRUE
web-1                      |   ↳ app/helpers/application_helper.rb:85:in 'ApplicationHelper#display_banner'
web-1                      |   Rendered shared/_banner.html.erb (Duration: 4.8ms | GC: 0.0ms)
web-1                      |   Rendered shared/_flash_messages.html.erb (Duration: 1.9ms | GC: 0.0ms)
web-1                      |   Rendered shared/_footer.html.erb (Duration: 0.2ms | GC: 0.0ms)
web-1                      |   Rendered shared/_footer_nav.html.erb (Duration: 1.4ms | GC: 0.0ms)
web-1                      |   Rendered layout layouts/application.html.erb (Duration: 242.5ms | GC: 0.0ms)
web-1                      | Completed 200 OK in 266ms (Views: 253.8ms | ActiveRecord: 0.8ms (1 query, 0 cached) | GC: 0.0ms)

Have commented it out with a TODO added to the test itself. Will merge tomorrow but leaving it open for now in case there are any objections.

Commented out the "Log out and reset" and "Edit Story" tests
because they fail due to something about the tests' interaction
with Turbo Stream.

Manually, the test case is successful as the user is redirected
to the new_user_password path ("/users/password/new")
and the Story page does have the message "Story was successfully updated."
@lenikadali lenikadali force-pushed the enable-content-security-policy branch from 93d7fc3 to fa43459 Compare May 26, 2026 18:44
Disabled out the "does not allow login and shows locked message" test
because it fails due to something about the test's interaction
with Turbo Stream.
@lenikadali lenikadali changed the title Enable Content Security Policy for AWBW HOLD: Enable Content Security Policy for AWBW May 27, 2026
@lenikadali lenikadali marked this pull request as draft May 27, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update content security policy in layouts/application.html.erb

3 participants