Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rename_csp_helper_nonce_attribute ActionView configuration to avoid value exfiltration #51729

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

codergeek121
Copy link
Contributor

@codergeek121 codergeek121 commented May 3, 2024

Adds a configuration to rename the CSP helper attribute name to avoid value exfiltration as described in #51580 (comment)

It's disabled by default currently until the JS libraries are updated to the new attribute name and Rails can ship with a new default attribute name.

Fixes the Rails part #51580. See this PR for a potential fix in Turbo: https://github.com/hotwired/turbo/pull/1254/files

If this approach makes sense in general, I'd try to fix ujs as well 👍

@rails-bot rails-bot bot added the actionview label May 3, 2024
codergeek121 added a commit to codergeek121/turbo that referenced this pull request May 3, 2024
…ttribute

This PR allows Turbo to support rails/rails#51729 to allow using the `nonce` attribute as well as the `content` attribute.

As described in rails/rails#51580 (comment) this makes it harder to extract the nonce value.
@codergeek121
Copy link
Contributor Author

I added a commit to change UJS as well. It changes UJS to check for the nonce attribute first, and if absent falls back to the content attribute.

@skipkayhil
Copy link
Member

UJS is dead #50535, so those files should not be changed

@codergeek121 codergeek121 force-pushed the csp-meta-tag-content-to-nonce branch from c9ac6cf to 4294d40 Compare May 10, 2024 19:34
@codergeek121
Copy link
Contributor Author

I removed the UJS changes, thank you for the hint!

Since main now target Rails 8, I could also remove the configuration flag and default to the nonce attribute instead. This would be a breaking change. Note that Turbo & Trix would work with both attributes after hotwired/turbo#1254 & basecamp/trix#1151.

I'd be happy to adapt 👍

Adds a configuration to rename the csp helper attribute name.

It's disabled by default currently until the JS libraries are updated to the
new attribute name and Rails can ship with a new default attribute name.

Fixes rails#51580
@codergeek121 codergeek121 force-pushed the csp-meta-tag-content-to-nonce branch from 4294d40 to 68a83a2 Compare May 14, 2024 19:07
@zzak
Copy link
Member

zzak commented May 18, 2024

Since main now target Rails 8, I could also remove the configuration flag and default to the nonce attribute instead.

Even with a major version, we still have to follow the deprecation cycle.

@codergeek121
Copy link
Contributor Author

Thank you for the link/info @zzak!

Can we still deprecate this in 7.2, or should we deprecate this in 8.0?

There's no new_framework_defaults_8_1.rb.tt and corresponding case statement yet, I can take a look into creating it though, but maybe this should happen in a separate PR?

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.

None yet

3 participants