Remove deprecated headers and stale policy features - #1728
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe header configuration replaces ChangesSecurity header policy
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This PR makes a localized cleanup to deprecated headers and stale policy features in one configuration file; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
bf7934d to
22d099f
Compare
Deploying dev-site with
|
| Latest commit: |
2516be7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0a5a83e1.dev-site-4ua.pages.dev |
| Branch Preview URL: | https://kevin-policy-headers.dev-site-4ua.pages.dev |
Feature-Policy was superseded by Permissions-Policy. X-XSS-Protection is non-standard, deprecated, and can itself introduce XSS vulnerabilities in otherwise safe pages; our Content-Security-Policy covers what it was for. Also drop the Permissions-Policy features that no browser recognizes, verified against Chrome 149 and 150: ambient-light-sensor, battery, document-domain, execution-while-not-rendered, execution-while-out-of-viewport, and speaker-selection. Chrome reported each of these as an unrecognized feature, so they never had any effect.
sync-xhr was denied only via Feature-Policy and had no Permissions-Policy counterpart, so removing that header dropped the denial entirely. Chrome still enforces Feature-Policy, and sync-xhr defaults to an allowlist of *, so this was blocking synchronous XHR in the top document and in every cross-origin frame.
This brings the policy to the same 76 features mm_website denies. The list was 22 features short of mm_website, and a further 31 recognized features were denied on none of the three sites. clipboard-write allows self because the copy button in assets/js/copy-markdown.ts calls navigator.clipboard.writeText. Every other addition is a flat deny. static/_headers is generated, so run npm run build:headers to pick this up.
22d099f to
344762b
Compare
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
PatrickCroninMM
left a comment
PatrickCroninMM
left a comment
There was a problem hiding this comment.
Looks good. Same finding about the two features that may no longer be necessary.
| 'identity-credentials-get=()', | ||
| 'idle-detection=()', | ||
| 'interest-cohort=()', | ||
| 'join-ad-interest-group=()', |
There was a problem hiding this comment.
Same maybe about join-ad-interest-group and run-ad-auction. I don't think it hurts to keep them currently.
| // implication, deny them out of an abundance of caution to limit what | ||
| // injected or embedded content can reach. See | ||
| // https://github.com/w3c/webappsec-permissions-policy/blob/main/features.md | ||
| // for the features that exist. |
There was a problem hiding this comment.
Also would be useful to have a comment on the things we are allowing in these two PRs.
Chromium is removing join-ad-interest-group and run-ad-auction, so the denials become dead weight once the names go. This is not free today. Chrome 152 still recognizes both and raises no warning for them, so a page can call the Protected Audience API where the denial used to stop it. browsing-topics stays denied, which is the publisher-side feature of that family with a name that is not going away. Record why the policy allows what it allows, on both sides: the features listed with an allowlist, and the features left off the list entirely. Neither should read as an oversight. static/_headers is generated, so run npm run build:headers to pick this up.
Summary by CodeRabbit