feat(csp): patch CSP meta tags and update PatchHeaders signature #478
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughRefactors CSP patching to operate on the full *http.Response, adds streaming rewrite support for , returns (nonce, error), surfaces patch errors, and updates injector callsites and tests to use the new API and error handling. No exported type names changed; PatchHeaders signature changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Injector
participant CSP as csp.PatchHeaders
participant Res as http.Response
participant Meta as meta tags
participant Headers as response headers
Injector->>CSP: PatchHeaders(Res, kind)
note right of CSP: generate nonce (if needed)
CSP->>Meta: scan & stream-rewrite <meta http-equiv="Content-Security-Policy">
alt meta patched
Meta-->>CSP: meta changed
else meta unchanged
Meta-->>CSP: no-op
end
CSP->>Headers: rewrite CSP header values via patchPolicies
alt any patch applied
Headers-->>CSP: headers changed
CSP-->>Injector: (nonce, nil)
Injector->>Injector: execute templates using nonce
else no patches
CSP-->>Injector: ("", nil)
Injector->>Injector: continue without nonce
end
opt error during patch
CSP-->>Injector: ("", error)
Injector-->>Injector: return wrapped error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus review on:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-09-05T11:40:45.127ZApplied to files:
📚 Learning: 2025-09-08T15:48:08.326ZApplied to files:
🧬 Code graph analysis (1)internal/csp/patch.go (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/extendedcss/injector.go (1)
97-100: Preserve error wrapping with%w.We’re standardizing on
%wso callers can unwrap the root cause. Please changefmt.Errorf("patch CSP headers: %v", err)to use%w, matching the other injectors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
internal/cosmetic/injector.go(1 hunks)internal/csp/patch.go(4 hunks)internal/csp/patch_test.go(5 hunks)internal/cssrule/injector.go(1 hunks)internal/extendedcss/injector.go(1 hunks)internal/scriptlet/injector.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
internal/csp/patch.go (1)
internal/httprewrite/rawbody.go (1)
StreamRewrite(27-46)
internal/extendedcss/injector.go (1)
internal/csp/patch.go (2)
PatchHeaders(31-51)InlineScript(24-24)
internal/cosmetic/injector.go (1)
internal/csp/patch.go (2)
PatchHeaders(31-51)InlineStyle(25-25)
internal/csp/patch_test.go (1)
internal/csp/patch.go (3)
PatchHeaders(31-51)InlineScript(24-24)InlineStyle(25-25)
internal/scriptlet/injector.go (1)
internal/csp/patch.go (2)
PatchHeaders(31-51)InlineScript(24-24)
internal/cssrule/injector.go (1)
internal/csp/patch.go (2)
PatchHeaders(31-51)InlineStyle(25-25)
anfragment
requested changes
Oct 21, 2025
Member
anfragment
left a comment
anfragment
left a comment
There was a problem hiding this comment.
Hello @kasyap1234, sorry for the late reply.
LGTM overall, just a few minor comments below.
Comment on lines
51
to
53
| // In case of multiple lines/policies, the browsers will select the most restrictive one. | ||
| // For this reason, we modify each independently so they all allow the inline tag. | ||
| // See more: https://content-security-policy.com/examples/multiple-csp-headers/. |
Member
There was a problem hiding this comment.
Can we preserve this, and the other comments within the function?
internal/csp/patch.go
Outdated
| var newValue string | ||
| if bestValue == "'none'" { | ||
| switch bestValue { | ||
| case "", "'none'": |
Member
There was a problem hiding this comment.
I believe the "" case will never be triggered:
Suggested change
| case "", "'none'": | |
| case "'none'": |
internal/csp/patch.go
Outdated
Comment on lines
128
to
136
| contentType := res.Header.Get("Content-Type") | ||
| if contentType == "" { | ||
| return false, nil | ||
| } | ||
|
|
||
| mediaType, _, err := mime.ParseMediaType(contentType) | ||
| if err != nil || !strings.EqualFold(mediaType, "text/html") { | ||
| return false, nil | ||
| } |
Member
There was a problem hiding this comment.
I believe this is somewhat redundant in practice due to this check a couple levels above:
https://github.com/ZenPrivacy/zen-desktop/blob/master/internal/filter/filter.go#L313
| defer src.Close() | ||
|
|
||
| z := html.NewTokenizer(src) | ||
| for { |
Member
There was a problem hiding this comment.
One potential performance optimization within the loop – since <meta http-equiv> can only be present in <head>, we can directly copy the remains of src to dst without the overhead of tokenization after we encounter </head> or <body>.
See an example here:
https://github.com/ZenPrivacy/zen-desktop/blob/master/internal/httprewrite/html.go#L56-L65
Member
|
Hey @kasyap1234! Sorry for not notifying you earlier – we've transferred most of the Go packages from this repository to |
Member
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR adds support for patching Content-Security-Policy (CSP) directives defined in HTML
<meta http-equiv="Content-Security-Policy">tags, in addition to those set via HTTP headers.Previously,
csp.PatchHeadersonly handled CSPs in response headers, which caused injection failures on websites that define their CSPs using meta tags (e.g., LinkedIn).To fix this, the
PatchHeadersfunction now accepts a*http.Responseinstead ofhttp.Header, enabling unified handling of both header-based and meta-based CSPs.BREAKING CHANGE:
PatchHeaders(h http.Header, kind inlineKind) (nonce string)→PatchHeaders(res *http.Response, kind inlineKind) (string, error)How did you verify your code works?
<meta http-equiv="Content-Security-Policy">tags.What are the relevant issues?
Closes #442