fix(coderd): add frame-ancestors CSP directive to prevent clickjacking#24474
fix(coderd): add frame-ancestors CSP directive to prevent clickjacking#24474jdomeracki-coder merged 3 commits intomainfrom
Conversation
Add frame-ancestors 'self' as a default CSP directive to prevent
clickjacking attacks against all Coder dashboard pages. Agent embed
routes (/agents/{id}/embed) use frame-ancestors * so the VS Code
extension iframe continues to work.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e7ae293ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for k, v := range additionalCSPHeaders { | ||
| embedCSPHeaders[k] = v | ||
| } | ||
| embedCSPHeaders[httpmw.CSPFrameAncestors] = []string{"*"} |
There was a problem hiding this comment.
This assignment unconditionally replaces any frame-ancestors value from CODER_ADDITIONAL_CSP_POLICY for /agents/{agentId}/embed*. In deployments that intentionally set a stricter policy (for example 'none' or a specific allowlist), the embed route is now force-opened to all origins, which weakens an explicit security configuration and contradicts the expected “operator override wins” behavior. The embed override should be a fallback only when no explicit frame-ancestors is configured.
Useful? React with 👍 / 👎.
Only default embed routes to frame-ancestors * when the operator has not explicitly set frame-ancestors via CODER_ADDITIONAL_CSP_POLICY. If they configured a stricter policy, it takes precedence.
|
Addressed in 8d6e2e1 — the embed route now only defaults to
|
2 participants
The Coder dashboard CSP headers did not include a
frame-ancestorsdirective, allowing any external site to embed dashboard pages in an iframe
for clickjacking attacks. Only the OAuth2 consent page had hardcoded
protection (
frame-ancestors 'none'+X-Frame-Options: DENY).This change adds
frame-ancestors 'self'as the default for all pagesserved through the CSP middleware, and registers agent embed routes
(
/agents/{id}/embed) withframe-ancestors *so the VS Code extensioniframe continues to work. Deployments that already set
frame-ancestorsvia
CODER_ADDITIONAL_CSP_POLICYkeep their custom value — the defaultonly applies when no override is present.
Implementation notes
coderd/httpmw/csp.go: After thestaticAdditionsloop, setframe-ancestors 'self'only if no entry already exists. This letsstaticAdditions(including the deployment-levelCODER_ADDITIONAL_CSP_POLICY) override the default.coderd/coderd.go: Extract proxy-hosts closure into a sharedcspProxyHostsvariable. Create a second CSP middleware instance forembed routes with
frame-ancestors *, and register/agents/{agentId}/embed+/agents/{agentId}/embed/*before ther.NotFoundcatch-all.coderd/httpmw/csp_test.go: Two new subtests — default'self'andoverride via
staticAdditions.