feat(compiler): SYN024 — warn on document.cookie access that bypasses the storage capability model (?bs 0.7+)#177
Open
marcelofarias wants to merge 5 commits into
Open
Conversation
… SYN021 (performance.now/timeOrigin time bypass) rebased onto main Ports SYN020 and SYN021 detection from botkowski/syn021-performance-time, inserting them in numeric order after SYN019 and before SYN022/SYN023 which were added to main since the original branch was opened. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… the storage capability model (?bs 0.7+)
document.cookie is persistent storage that is also transmitted with every matching
HTTP request, so it has implicit network-side effects. Unlike localStorage (SYN015)
or indexedDB (SYN016), cookies cross the storage/network boundary. Neither reads {}
/ writes {} labels nor uses { net } cover document.cookie — it is outside the
capability model entirely.
Detection: `document` not preceded by `.`/`?.`, followed by `.`/`?.`, member is
`cookie`. Covers both reads (const c = document.cookie) and writes
(document.cookie = '...'). Optional-chain form (document?.cookie) also detected.
obj.document.cookie (local binding) and fn declarations named `document` excluded.
unsafe {} and unsafe fn bodies suppressed.
Completes the browser storage trilogy: SYN015 (localStorage/sessionStorage),
SYN016 (indexedDB), SYN024 (document.cookie).
Co-Authored-By: Botkowski <noreply@botkowski.ai>
There was a problem hiding this comment.
Pull request overview
Adds new syn-check compiler warnings for ambient time and cookie access that bypass botscript’s capability model, and wires the new diagnostic codes through the compiler registry, MCP explanations, and tests.
Changes:
- Add
SYN020(Date time injection),SYN021(performance timing), andSYN024(document.cookie) detection topassSynCheckfor?bs 0.7+. - Register the new codes in
packages/compiler/src/error-codes.tsand add MCPexplainentries. - Add/extend test coverage for the new diagnostics and update MCP known-codes assertions.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compiler/src/passes/syn-check.ts | Implements SYN020/SYN021/SYN024 detection and emits warning diagnostics. |
| packages/compiler/src/error-codes.ts | Adds registry entries (rule/idiom/rewrite/example) for SYN020/SYN021/SYN024. |
| packages/compiler/tests/syn020-check.test.ts | Adds unit tests for Date-based ambient time detection. |
| packages/compiler/tests/syn021-check.test.ts | Adds unit tests for performance timing detection. |
| packages/compiler/tests/syn024-check.test.ts | Adds integration-style tests for document.cookie detection via transform(). |
| packages/compiler/tests/error-codes.test.ts | Extends the allowlist to include SYN020/SYN021/SYN024. |
| packages/mcp/src/explanations.ts | Adds long-form MCP explanations for SYN020/SYN021/SYN024. |
| packages/mcp/tests/server.test.ts | Updates MCP known-codes list to include SYN020/SYN021/SYN024. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+219
to
+222
| * Detection: `document` not preceded by `.`/`?.`, followed by `.`/`?.`, member is | ||
| * `cookie`. Both read access (`const c = document.cookie`) and assignment | ||
| * (`document.cookie = "k=v"`) are detected. The ternary-consequent `:` guard is applied | ||
| * before the member check to avoid false-positives from ternary expressions. |
Comment on lines
+297
to
+301
| const syn020 = getErrorCode("SYN020")!; | ||
| const syn021 = getErrorCode("SYN021")!; | ||
| const syn022 = getErrorCode("SYN022")!; | ||
| const syn023 = getErrorCode("SYN023")!; | ||
| const syn024 = getErrorCode("SYN024")!; |
Comment on lines
+297
to
299
| const syn020 = getErrorCode("SYN020")!; | ||
| const syn021 = getErrorCode("SYN021")!; | ||
| const syn022 = getErrorCode("SYN022")!; |
…ding SYN024 doc comment - AGENTS.md: add rows for SYN020 (Date ambient time), SYN021 (performance timing), and SYN024 (document.cookie) in numeric order - README.md: add SYN020, SYN021, SYN024 to the explain tool code list - syn-check.ts: remove false claim that a ternary-consequent ':' guard is applied before the SYN024 member check (no such suppression exists; document.cookie fires in ternary positions intentionally) Addresses Copilot review comments on PR #177. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+297
to
+301
| const syn020 = getErrorCode("SYN020")!; | ||
| const syn021 = getErrorCode("SYN021")!; | ||
| const syn022 = getErrorCode("SYN022")!; | ||
| const syn023 = getErrorCode("SYN023")!; | ||
| const syn024 = getErrorCode("SYN024")!; |
…yet implemented SYN015 (localStorage/sessionStorage) is not registered in the error-code registry, so references to (SYN015) in diagnostic text, MCP explanations, and AGENTS.md pointed users at a nonexistent code. Removed the (SYN015) parenthetical from all three locations; the prose still names localStorage as context so the semantic comparison is clear. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+1466
to
+1468
| "`document.cookie` is a persistent storage mechanism that is also transmitted with every " + | ||
| "matching HTTP request — unlike `localStorage` (SYN015) or `indexedDB` (SYN016), cookies " + | ||
| "have implicit network-side effects. Neither `reads {}` / `writes {}` labels nor `uses { net }` " + |
| | SYN018 | (0.7+, warning) A fn body calls `Math.random()`, `Math?.random()`, or `Math.random?.()`. `Math.random` generates a random float at runtime but is invisible to botscript's capability model: `uses { random }` covers `random.*` stdlib namespace calls, not the `Math` global. A fn that calls `Math.random()` has an undeclared randomness dependency — callers cannot see it, tests cannot mock or suppress it the way they can the `random` stdlib. Detection: `Math` ident not preceded by `.`/`?.`, followed by `.` or `?.`, member is `random`, followed by `(` or `?.(`. Bare `Math.random` references without a trailing `(` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Replace `Math.random()` with `random.next()` and add `uses { random }` to the fn header. If `Math.random` is required, wrap in `unsafe "uses Math.random for <reason>" { Math.random() }`. | | ||
| | SYN019 | (0.7+, warning) A fn body calls `crypto.getRandomValues(buf)` or `crypto.randomUUID()` (including optional-chain forms `crypto?.getRandomValues(buf)` and optional-call forms `crypto.getRandomValues?.(buf)`). These calls generate cryptographic randomness at runtime but are invisible to botscript's capability model: `uses { random }` covers `random.*` stdlib calls (`random.next()`, `random.int()`), not the `crypto` global. A fn that calls these methods has an undeclared randomness dependency — tests cannot control the output and callers cannot observe the dependency from the fn header. Detection: `crypto` ident not preceded by `.`/`?.`, followed by `.` or `?.`, followed by `getRandomValues` or `randomUUID`, followed by `(` or `?.(`. Member calls (`obj.crypto.getRandomValues(...)`), non-randomness members (e.g. `crypto.subtle.digest(...)`), bare references, and `fn`/`function` declarations named `crypto` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Use `random.next()` or `random.int(min, max)` from the `random` stdlib with `uses { random }` when general randomness is sufficient. If cryptographic randomness or UUIDs are genuinely required, wrap in `unsafe "uses crypto for <reason>" { crypto.getRandomValues(buf) }`. | | ||
| | SYN020 | (0.7+, warning) A fn body calls `Date.now()`, constructs `new Date()` / `new Date` (no-arg ambient time), or calls `Date()` / `Date?.()`. These inject the current wallclock time at runtime but are invisible to botscript's capability model: `uses { time }` covers `time.*` stdlib calls, not the `Date` global. A fn that reads these has an undeclared time dependency — callers cannot see it and tests cannot control the clock value observed. Detection: `Date` ident not preceded by `.`/`?.`, followed by `.` or `?.` then `now`, or preceded by `new` / bare call form, followed by `(` or `?.(` or `)` (no-arg). `obj.Date.*`, member calls on a `Date` instance, and `fn`/`function` declarations named `Date` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Pass `nowMs: number` as an explicit parameter so callers and tests can control the time value. If ambient time is required at an entry point, use `time.now()` from the `time` stdlib with `uses { time }`, or wrap in `unsafe "reads wallclock time for <reason>" { Date.now() }`. | | ||
| | SYN021 | (0.7+, warning) A fn body calls `performance.now()`, `performance?.now()`, `performance.now?.()`, or reads `performance.timeOrigin` / `performance?.timeOrigin`. These inject ambient timing information at runtime but are invisible to botscript's capability model: `uses { time }` covers `time.*` stdlib calls, not the `performance` global. Detection: `performance` ident not preceded by `.`/`?.`, followed by `.` or `?.`, member is `now` (with trailing `(` or `?.(`) or `timeOrigin`. `obj.performance.*` member calls, `fn`/`function` declarations named `performance`, and `unsafe {}` / `unsafe "reason" fn` bodies are suppressed. | Pass the start time as an explicit parameter. If monotonic ordering (not wallclock) is the goal, use `clock.sequence()` (no capability required). If `performance.now` is genuinely needed, wrap in `unsafe "uses performance.now for <reason>" { performance.now() }`. | |
Comment on lines
294
to
+301
| const syn016 = getErrorCode("SYN016")!; | ||
| const syn018 = getErrorCode("SYN018")!; | ||
| const syn019 = getErrorCode("SYN019")!; | ||
| const syn020 = getErrorCode("SYN020")!; | ||
| const syn021 = getErrorCode("SYN021")!; | ||
| const syn022 = getErrorCode("SYN022")!; | ||
| const syn023 = getErrorCode("SYN023")!; | ||
| const syn024 = getErrorCode("SYN024")!; |
8 tasks
… MCP explanation; drop clock.sequence forward ref - SYN021 performance.now() diagnostic end was set to end of `now` ident; changed to afterNow21.start+1 (the opening paren) to match other call-site SYN diagnostics. - SYN024 MCP explanation still referenced localStorage as (SYN015); SYN015 is not registered in this repo yet, so removed the code ref. - SYN021 AGENTS.md fix-hint referenced clock.sequence() which does not exist in main; replaced with a plain "pass the start time as a parameter" hint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
294
to
+301
| const syn016 = getErrorCode("SYN016")!; | ||
| const syn018 = getErrorCode("SYN018")!; | ||
| const syn019 = getErrorCode("SYN019")!; | ||
| const syn020 = getErrorCode("SYN020")!; | ||
| const syn021 = getErrorCode("SYN021")!; | ||
| const syn022 = getErrorCode("SYN022")!; | ||
| const syn023 = getErrorCode("SYN023")!; | ||
| const syn024 = getErrorCode("SYN024")!; |
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
This PR stacks on top of #176 (SYN020/SYN021). The diff vs
mainincludes SYN020/SYN021 from #176's branch — those are not new here. The sole addition in this PR is SYN024.SYN024to the compiler's syn-check pass: fires whendocument.cookieis accessed (read or write) inside a?bs 0.7+fn bodydocument.cookieis unique in the storage bypass family — unlikelocalStorage(SYN015, in PR feat(compiler): SYN015 — warn on localStorage/sessionStorage access that bypasses the storage capability model (?bs 0.7+) #162) orindexedDB(SYN016), cookies are also transmitted with every matching HTTP request, giving them implicit network-side effectsdocument.cookie,document?.cookie, and assignmentdocument.cookie = '...'; excludesobj.document.cookie(local binding), fn declarations nameddocument, andunsafe {}/unsafe fnbodiesexplain, tests, and docsCompletes the browser storage trilogy
localStorage/sessionStorage(PR #162)indexedDBdocument.cookie(+ implicit network-side effect)Test plan
document.cookieread, write, member call (.includes()), optional-chain?bs 0.7, insideunsafe,obj.document.cookie, baredocument,document.title,fn document()declarationdocument.cookie🤖 Generated with Claude Code