feat(compiler): SYN028 — warn on window.open() and location.* navigation bypass (?bs 0.7+)#184
Open
marcelofarias wants to merge 4 commits into
Open
feat(compiler): SYN028 — warn on window.open() and location.* navigation bypass (?bs 0.7+)#184marcelofarias wants to merge 4 commits into
marcelofarias wants to merge 4 commits into
Conversation
…ion bypass (?bs 0.7+, closes #183) Adds SYN028 to the net-bypass family: - location.href = url — full-page navigation (GET-equivalent) - location.assign(url), location.replace(url), location.reload() — navigation calls - window.open(url), globalThis.open(url), self.open(url) — new browsing context + HTTP request All trigger real network activity invisible to the capability model (no uses { net } covers any of them). Completes the net-bypass series after SYN027 (postMessage). Security angle: prompt-injected content can redirect users to phishing pages via location.href or exfiltrate data as URL parameters via window.open — harder to intercept than fetch() (no ServiceWorker intercept, no CSP connect-src by default). Detection: - `location` ident (not preceded by `.`/`?.`), followed by `.`/`?.`, then: - `href` followed by `=` (lexer kind "eq", precise — excludes == and ===) - `assign`/`replace`/`reload` followed by `(` or `?.(` - `open` ident preceded by `.`/`?.` from ambient receiver (window/globalThis/self), where that receiver is not itself a member access Excluded: obj.location.* (non-ambient), obj.window.open (non-ambient receiver), fn/function/function* declarations, method shorthands, unsafe {}/unsafe fn bodies. Wired through: error-codes registry, MCP explain, error-codes.test.ts allowlist, MCP KNOWN_CODES contract, AGENTS.md diagnostic table, README.md explain list. 32 tests in packages/compiler/tests/syn028-check.test.ts. Full suite: 1512/1512. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new compiler syn-check warning (SYN028) to detect browser navigation APIs (location.* navigation and window/globalThis/self.open) that can trigger real network activity while bypassing the uses { net } capability surface, and wires the new code through the project’s diagnostic/documentation/MCP plumbing.
Changes:
- Implement
SYN028detection inpackages/compiler/src/passes/syn-check.tsforlocation.href =,location.assign/replace/reload(), and ambient-global*.open(...)calls (with unsafe suppression and non-ambient exclusions). - Register
SYN028in the compiler error-code registry and MCP explanations, and extend the MCP “known codes” assertion. - Add a dedicated compiler test suite for
SYN028and update the compiler error-code allowlist test; update README + AGENTS diagnostic tables.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds SYN028 to the documented explain-supported diagnostic codes list. |
| packages/mcp/tests/server.test.ts | Adds SYN028 to the MCP KNOWN_CODES contract test. |
| packages/mcp/src/explanations.ts | Adds the long-form MCP explanation entry for SYN028 with fails/passes examples. |
| packages/compiler/tests/syn028-check.test.ts | Introduces test coverage for SYN028 detection and non-detection cases. |
| packages/compiler/tests/error-codes.test.ts | Adds SYN028 to the compiler diagnostic allowlist. |
| packages/compiler/src/passes/syn-check.ts | Implements SYN028 detection logic and corresponding warning messages. |
| packages/compiler/src/error-codes.ts | Registers SYN028 rule/idiom/rewrite/example in the compiler registry. |
| AGENTS.md | Adds SYN028 to the diagnostic codes table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1720
to
+1741
| // Must be followed by `.` or `?.` | ||
| const dotIdx28loc = nextSignificant(tokens, i + 1); | ||
| const dotTok28loc = tokens[dotIdx28loc]; | ||
| const isDot28loc = dotTok28loc && dotTok28loc.kind === "punct" && dotTok28loc.text === "."; | ||
| const isOptChain28loc = dotTok28loc && dotTok28loc.kind === "questionDot"; | ||
| if (!isDot28loc && !isOptChain28loc) continue; | ||
|
|
||
| // Member must be href, assign, replace, or reload | ||
| const memberIdx28loc = nextSignificant(tokens, dotIdx28loc + 1); | ||
| const memberTok28loc = tokens[memberIdx28loc]; | ||
| if (!memberTok28loc || memberTok28loc.kind !== "ident") continue; | ||
| const memberName28loc = memberTok28loc.text; | ||
|
|
||
| const sep28loc = isOptChain28loc ? "?." : "."; | ||
|
|
||
| if (memberName28loc === "href") { | ||
| // Must be followed by `=` (assignment). The lexer emits `=` as kind "eq", | ||
| // while `==` and `===` are kind "operator" — so checking for "eq" is precise. | ||
| const afterHrefIdx = nextSignificant(tokens, memberIdx28loc + 1); | ||
| const afterHref = tokens[afterHrefIdx]; | ||
| if (!afterHref || afterHref.kind !== "eq") continue; | ||
|
|
Comment on lines
+1788
to
+1799
| start: tok.start, | ||
| end: afterMember28loc.start + 1, | ||
| message: | ||
| `fn '${decl.name}' calls location${sep28loc}${memberName28loc}${callSep28loc}() — ` + | ||
| `location.${memberName28loc} triggers navigation, invisible to the capability model; ` + | ||
| `no uses { net } declaration covers this network-equivalent operation; ` + | ||
| `pass a navigation callback as a parameter so callers can see the dependency, ` + | ||
| `or wrap in unsafe "navigates for <reason>" { location${sep28loc}${memberName28loc}${callSep28loc}(url) }`, | ||
| rule: syn028.rule, | ||
| idiom: syn028.idiom, | ||
| rewrite: syn028.rewrite, | ||
| }); |
Comment on lines
+32
to
+40
| it("fires on location?.href = url (optional-chain receiver)", () => { | ||
| const src = | ||
| "?bs 0.7\n" + | ||
| "fn redirectTo(url: string) -> void {\n" + | ||
| " location?.href = url\n" + | ||
| "}\n"; | ||
| const result = compile(src); | ||
| expect(result.warnings.some((w) => w.code === "SYN028")).toBe(true); | ||
| }); |
…href LHS, fix reload() message, use receiver name in open() message - location?.href = url is invalid JS/TS (optional chaining cannot appear on an assignment LHS); skip the `?.` path for the href form and flip the test to assert SYN028 does NOT fire - location.reload() takes no URL argument; the unsafe-wrap suggestion now emits `reload()` not `reload(url)`, using a per-member ternary - window.open() message hardcoded `window.open()` even when the receiver was `globalThis` or `self`; now uses `receiverName28` so the message matches the actual call form - Add tests: reload message must not contain `(url)`, globalThis.open message must say `globalThis` not `window.open()` Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+186
to
+188
| * - `window.open(url, ...)`, `globalThis.open(url, ...)`, `self.open(url, ...)` — | ||
| * opens a new browsing context and issues a real HTTP request; the URL is sent to | ||
| * the network without any `uses { net }` declaration in the fn header. |
Comment on lines
+32
to
+35
| it("does NOT fire on location?.href = url (invalid LHS — optional chaining cannot be an assignment target)", () => { | ||
| // `location?.href = url` is syntactically invalid JS/TS: optional chaining is not | ||
| // allowed on the left-hand side of an assignment. SYN028 must not fire on this form. | ||
| const src = |
…ility model even with uses { net }
`uses { net }` covers stdlib network calls, not navigation APIs; the old
wording implied adding the declaration would address the warning.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| " navigate(url)\n" + | ||
| "}\n\n" + | ||
| "// or use unsafe when navigation is genuinely required at an entry point\n" + | ||
| "fn redirectTo(url: string) -> void {\n" + |
…open() arg placeholder, fix duplicate fn in example
- Change "no uses { net } declaration covers this" → "uses { net } does not
cover navigation APIs even when declared" in all three SYN028 warning
messages (href, location methods, window.open). Copilot correctly flagged
that the old wording reads as if adding uses { net } would fix the issue.
- Change unsafe-wrap placeholder for window.open from (url) to (url, ...)
to avoid implying additional arguments are dropped (common usage includes
target and features, e.g. window.open(url, '_blank')).
- Rename second fn redirectTo → fn navigateToLogin in the MCP explanations
passes example to eliminate the duplicate function name, which would emit
invalid TypeScript if the snippet were copy-pasted into a real file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
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
Detected forms:
Not detected (invalid syntax): `location?.href = url` — optional chaining cannot appear on the LHS of an assignment in JS/TS, so this form is excluded from detection.
Security angle: In bot/agent contexts, prompt-injected content can redirect users to phishing pages via `location.href` or exfiltrate data as URL parameters via `window.open` — harder to intercept than `fetch()` (no ServiceWorker intercept by default, CSP `connect-src` doesn't cover navigation unless combined with `navigate-to`).
Excluded: `obj.location.` (non-ambient binding), `obj.window.open` (non-ambient receiver), fn/function/function declarations, object method shorthands, `unsafe {}` blocks, `unsafe "reason" fn` bodies.
Relation to existing checks
Implementation notes
The `href =` detection requires checking for the lexer's `"eq"` kind (not `"operator"`) since the lexer emits `=` as kind `"eq"`, while `==`/`===` are kind `"operator"`. This gives a precise assignment-only match with no false positives on comparisons.
The `window.open` detection follows the SYN027 (postMessage) pattern: match on the `open` ident, check that the preceding token is `.`/`?.` from an ambient global (`window`, `globalThis`, `self`), and verify that ambient global is not itself a member access.
Test plan
🤖 Generated with Claude Code