fix(x-frame-options): ALLOW-FROM should score 0, not 8 — no current browser honors it#107
Open
dmchaledev wants to merge 1 commit into
Open
fix(x-frame-options): ALLOW-FROM should score 0, not 8 — no current browser honors it#107dmchaledev wants to merge 1 commit into
dmchaledev wants to merge 1 commit into
Conversation
…ore it X-Frame-Options: ALLOW-FROM is non-standard and has been stripped from every current browser engine (Chrome, Firefox, Safari, Edge) — the header value is silently ignored, not honored, so it provides exactly the same (zero) clickjacking protection as no header at all. The previous scoring gave it 8/15 alongside genuinely ambiguous/unknown values, materially overstating real protection for anyone shipping ALLOW-FROM today (a common copy-paste from outdated IE-era guidance). Distinguish ALLOW-FROM (score 0, with a finding explaining why) from other unrecognized values (unchanged at 8/15).
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
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.
Summary
checkXFrameOptionsinsrc/rules.tsgaveX-Frame-Options: ALLOW-FROM <url>a score of 8/15 with statuswarning— the same generic score used for any unrecognized value. ButALLOW-FROMisn't merely a weaker directive: it's been removed from every current browser engine (Chrome, Firefox, Safari, Edge) for years. A browser encountering it simply ignores the header, so a response with onlyX-Frame-Options: ALLOW-FROM https://trusted.exampleis exactly as embeddable/clickjackable in production as a response with noX-Frame-Optionsheader at all — yet the tool credited it with more than half marks and a reassuring "warning" (rather than effectively "missing") status.This is a real accuracy gap in the tool's core grading promise, and
ALLOW-FROMis a common copy-paste leftover from older IE-era security guidance, so it's not just a theoretical edge case.Confirmed via the repo's own open-issue/PR history that this isn't a duplicate of anything already filed or in flight — the existing coverage in this area (
frame-ancestors */bare-scheme handling) addresses a different code path and was already fixed.Changes
src/rules.ts(checkXFrameOptions):ALLOW-FROM(case-insensitive) now scores0/15with a finding explaining that it's ignored by all current browsers, distinct from other unrecognized values which keep the existing8/15(genuinely ambiguous/unknown input, not a known-defunct directive).test/analyzer.test.ts: updated the test that previously locked inALLOW-FROM→ score 8 (now asserts score 0 + the new finding text), added a case-insensitivity check, and added a separate test to confirm other unrecognized values are unaffected (still 8/15).Test plan
npm run typecheckpassesnpm test— 87/87 tests pass (was 85; added 2)ALLOW-FROM→ 8 behavior (grep -rn "ALLOW-FROM"across the repo only touches the two files changed here)Generated by Claude Code