Skip to content

Conversation

@lognaturel
Copy link
Member

@lognaturel lognaturel commented Dec 4, 2025

Closes #1130
Alternative to https://github.com/getodk/central/pull/1467/files

What has been done to verify that this works as intended?

Put on dev server, loaded All Question Types form as new submission, Public Link, and for submission edit. Looked in console for any violations.

I did NOT review reported violations extensively.

Why is this the best possible solution? Were any other approaches considered?

The most obvious alternative is to have the same policy for all of frontend. I don't think that would be so bad but the increased granularity is nice in some cases. The regex approach to matching routes feels a little unsatisfying/brittle but the stakes are low if there's a mistake because the policies are not very different.

One challenge for this behavior is where to define the Web Forms-specific headers which apply to several different routes. This PR uses a map to go from the request URI to the intended CSP headers. That feels like a clean, easy-to-read solution to me. I mostly took the regex from #1467 and trusted that the tests are complete enough.

I tried mapping on $uri to allow query parameters but realized that gets rewritten so mapped on $request_uri and explicitly allowed query params in the regex.

The directives are intended to be in alphabetical order with default-src: 'none' listed first for readability.

Two fetch directives are omitted: child-src and prefetch-src. I think that's ok because they're not likely to be the cause of violations. The more specific directives like script-elem-src are also omitted and I think that's ok too.

TODO:

  • remove 'self' from frame-src for Web Forms

I also reviewed each existing directive:

Directive Default Web Forms Explanation
default-src none none Want restrictive baseline
connect-src 'self' Google translate 'self' https: Web Forms may need to show images hosted on some arbitrary blob storage server. The blob storage URLs may not exactly match the domain specified in the config.
font-src 'self' 'self' data: data: is because Roboto gets downloaded (note: seeing it on test. but not dev.)
frame-src 'self' https://getodk.github.io/central/news.html 'none' At some point we'll need to figure out how we want to allow Web Forms to be used in frames but we don't need to do that now
img-src https: data: https: blob: I did see a data: image request from frontend but ⚠️ couldn't figure out what it is; https: for frontend is because users can link to arbitrary images in markdown. https: for Web Forms is currently for map tiles. We could pretty easily be more restrictive here I think.
manifest-src 'none' 'none' Listed just for reports
media-src 'none' 'none' Listed just for reports
object-src 'none' 'none' Listed just for reports
script-src 'self' 'self' 'wasm-unsafe-eval' WASM used by XPath evaluator
style-src 'self' 'self' 'unsafe-inline' PrimeVue uses inline styles for at least DatePicker
style-src-attr 'unsafe-inline' (not set) ⚠️ review why this is needed for frontend
worker-src blob: blob: Used by OpenLayers

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

The most important intended behavior change is that we start getting more meaningful / less noisy reports and that users don't see warnings in their consoles.

No immediate regression risk because we're still doing report only. We'd like to turn on actual protection as soon as possible, though, so if we made mistakes we run the risk of further delaying that.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No

Before submitting this PR, please make sure you have:

  • branched off and targeted the next branch OR only changed documentation/infrastructure (master is stable and used in production)
  • verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

@lognaturel lognaturel requested a review from alxndrsn December 4, 2025 00:44
@lognaturel lognaturel changed the title Wf csp Add CSP headers specifically for Web Forms Dec 4, 2025
@lognaturel
Copy link
Member Author

I'm happy with where this is. It's my PR so I can't approve it. @alxndrsn please squash and merge when you're happy with it!

@alxndrsn
Copy link
Contributor

alxndrsn commented Dec 5, 2025

TODO:

  • remove 'self' from frame-src for Web Forms

@lognaturel does this need to be done?

I haven't merged due to this - please feel free to merge, either with or without frame-src: 'self' set.

@alxndrsn alxndrsn mentioned this pull request Dec 5, 2025
2 tasks
@lognaturel lognaturel merged commit d210e58 into next Dec 5, 2025
14 of 15 checks passed
@lognaturel lognaturel deleted the wf-csp branch December 5, 2025 21:30
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.

3 participants