From d845a6f9389aa10b6f4f072921b029332146a26c Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 18 Jun 2026 07:42:06 -0300 Subject: [PATCH] =?UTF-8?q?feat(compiler):=20SYN015=20=E2=80=94=20warn=20o?= =?UTF-8?q?n=20localStorage/sessionStorage=20access=20that=20bypasses=20th?= =?UTF-8?q?e=20storage=20capability=20model=20(=3Fbs=200.7+)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit localStorage.* and sessionStorage.* accesses are synchronous same-origin Web Storage operations invisible to botscript's capability model: reads {} and writes {} labels cover declared resource identifiers, not the Web Storage API globals. A fn that accesses either store has undeclared persistent (localStorage) or session-scoped (sessionStorage) state dependencies that callers cannot observe or audit from the fn header. Detection: localStorage or sessionStorage ident not preceded by ./?. (member-of-another-object guard), followed by . or ?. (confirming global access, not a bare reference). fn/function/function* declarations named localStorage/sessionStorage are excluded. unsafe {} blocks and unsafe fn bodies are suppressed. Wired through: error-codes, MCP explain + server test, AGENTS.md diagnostic table, README.md explain row, and 21 tests covering both globals, optional chains, unsafe suppression, version guard, and negative cases. This closes the gap that multiple previous PRs (SYN016, SYN024) referenced as (SYN015) without the code existing in the registry. Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 3 +- README.md | 2 +- packages/compiler/src/error-codes.ts | 33 +++ packages/compiler/src/passes/syn-check.ts | 58 +++++ packages/compiler/tests/error-codes.test.ts | 2 +- packages/compiler/tests/syn015-check.test.ts | 225 +++++++++++++++++++ packages/mcp/src/explanations.ts | 46 ++++ packages/mcp/tests/server.test.ts | 1 + 8 files changed, 367 insertions(+), 3 deletions(-) create mode 100644 packages/compiler/tests/syn015-check.test.ts diff --git a/AGENTS.md b/AGENTS.md index c23bffd4..7ec68035 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -208,7 +208,8 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | SYN012 | (0.7+, warning) A fn body constructs an `EventSource` via `new EventSource(url)`, bare `EventSource(url)`, `EventSource?.(url)`, or TypeScript instantiation form `new EventSource(url)`. EventSource opens a persistent server-sent-events (SSE) connection at runtime but is invisible to CAP001 — the capability model only checks `http.*` member calls. Detection: `EventSource` not preceded by `.`/`?.`, followed by `(`, `?.(`, or `(` (generic scan gated on `new`). Member calls, `function`/`fn` declarations, object method shorthands, and TS method signatures are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Wrap in `unsafe "wraps EventSource for " { new EventSource(url) }` to make the escape hatch visible in the diff. | | SYN013 | (0.7+, warning) A fn body constructs a `Worker` or `SharedWorker` via `new Worker(scriptURL)`, bare `Worker(scriptURL)`, `Worker?.(scriptURL)`, `new SharedWorker(scriptURL)`, `SharedWorker?.(scriptURL)`, or TypeScript instantiation forms. Worker construction spawns a new JS execution context whose capability surface is unbounded — the worker script can make network requests, access storage, and perform any operation, none of which is visible in the spawning fn's `uses {}`, `reads {}`, or `writes {}` declarations. CAP001 cannot infer any capability from worker construction. Detection: `Worker` or `SharedWorker` not preceded by `.`/`?.`, followed by `(`, `?.(`, or `(` (generic scan gated on `new`). Member calls, `function`/`fn`/`function*` declarations, object method shorthands, and TS method signatures are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Wrap in `unsafe "" { new Worker(scriptURL) }` with a reason that documents what capabilities the worker script is expected to use. | | SYN014 | (0.7+, warning) A fn body calls `new BroadcastChannel(name)`, `BroadcastChannel(name)`, or TypeScript instantiation form `new BroadcastChannel(name)`. `BroadcastChannel` opens a cross-context message channel at runtime — any tab, window, or worker on the same origin can post to or receive from it — invisible to CAP001. A fn that constructs a BroadcastChannel has an undeclared cross-context messaging dependency. Detection: `BroadcastChannel` not preceded by `.`/`?.`, followed by `(` or `?.(` — or `(` when preceded by `new` (generic scan is gated on `new` to avoid `<`/`>` comparison false-positives). Member calls (`obj.BroadcastChannel(...)`), object method shorthands, TypeScript method signatures, and `fn`/`function` declarations named `BroadcastChannel` are excluded. The `:` exclusion is guarded against ternary consequents. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Wrap in `unsafe "" { new BroadcastChannel(name) }` (or matching call form) to make the cross-context messaging dependency visible in the diff. | -| SYN016 | (0.7+, warning) A fn body accesses `indexedDB.*` — any member access (`.`, `?.`) on the `indexedDB` global. `indexedDB` is same-origin persistent database storage invisible to botscript's capability model (`reads {}` / `writes {}` labels cover declared resource identifiers, not the Web Storage API). Unlike `localStorage`, `indexedDB` is asynchronous and has no practical size limit, making invisible access higher-impact. A fn that accesses `indexedDB` has undeclared persistent state dependencies invisible to callers and audit tooling. Detection: `indexedDB` ident not preceded by `.`/`?.`, followed by `.` or `?.`. Bare references and `fn`/`function` declarations named `indexedDB` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Pass an `IDBDatabase` handle as an explicit fn parameter so callers control what database is accessed and tests can inject a mock. If direct global access is required, wrap in `unsafe "reads/writes indexedDB for " { indexedDB.open(name) }`. | +| SYN015 | (0.7+, warning) A fn body accesses `localStorage.*` or `sessionStorage.*` — any member access (`.`, `?.`) on either Web Storage global. Both are synchronous same-origin key/value stores invisible to botscript's capability model (`reads {}` / `writes {}` labels cover declared resource identifiers, not the Web Storage API). A fn that accesses either store has undeclared persistent (`localStorage`) or session-scoped (`sessionStorage`) state dependencies invisible to callers and audit tooling. Detection: `localStorage` or `sessionStorage` ident not preceded by `.`/`?.`, followed by `.` or `?.`. Bare references and `fn`/`function`/`function*` declarations named `localStorage`/`sessionStorage` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Pass the storage value as an explicit fn parameter so callers control what is read and tests can inject a mock. If direct access is required, wrap in `unsafe "accesses localStorage for " { localStorage.getItem(key) }`. | +| SYN016 | (0.7+, warning) A fn body accesses `indexedDB.*` — any member access (`.`, `?.`) on the `indexedDB` global. `indexedDB` is same-origin persistent database storage invisible to botscript's capability model (`reads {}` / `writes {}` labels cover declared resource identifiers, not the Web Storage API). Unlike `localStorage` (SYN015), `indexedDB` is asynchronous and has no practical size limit, making invisible access higher-impact. A fn that accesses `indexedDB` has undeclared persistent state dependencies invisible to callers and audit tooling. Detection: `indexedDB` ident not preceded by `.`/`?.`, followed by `.` or `?.`. Bare references and `fn`/`function` declarations named `indexedDB` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Pass an `IDBDatabase` handle as an explicit fn parameter so callers control what database is accessed and tests can inject a mock. If direct global access is required, wrap in `unsafe "reads/writes indexedDB for " { indexedDB.open(name) }`. | | 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 " { 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 " { crypto.getRandomValues(buf) }`. | | SYN022 | (0.7+, warning) A fn body accesses `process.argv`, `process.cwd()`, `process.platform`, `process.arch`, `process.pid`, `process.ppid`, `process.version`, `process.versions`, `process.hrtime()`, `process.uptime()`, `process.memoryUsage()`, `process.cpuUsage()`, or `process.resourceUsage()`. These read ambient Node.js process state at runtime — OS identity, process tree, memory, CPU — invisible to botscript's capability model. Unlike `process.env` (SYN005) and `process.exit` (SYN006), which cover configuration and termination respectively, SYN022 targets the remaining ambient introspection surface. Detection: `process` ident not preceded by `.`/`?.`, followed by `.` or `?.`, member in the ambient-state set, optionally followed by `(` or `?.(`. Bare `process.*` references without a trailing `(` still fire. `obj.process.*` member calls, `unsafe {}` blocks, and `unsafe "reason" fn` bodies are excluded. `process.env` and `process.exit` are handled by SYN005/SYN006 and excluded here. | Pass the required ambient value as an explicit fn parameter so the dependency is visible in the call signature and tests can inject a mock. If direct `process.*` access is required at a bootstrap entry point, wrap in `unsafe "reads process state for " { process.argv }`. | diff --git a/README.md b/README.md index 00df835d..d3639494 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ claude mcp add botscript -- npx -y @mbfarias/botscript-mcp | ----------- | -------------------------------------- | --------------------------------------------------------------------------------------------------- | | `primer` | (no args) | The canonical language primer (same text the `?primer` directive emits). | | `transform` | `{ source: string, filename?: string }` | `{ ok: true, code, forms, version, warnings: [...] }` on success, or `{ ok: false, diagnostics: [...] }` on failure. `warnings` is an array of non-blocking diagnostics (e.g. CAP003). | -| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN004`, `SYN005`, `SYN006`, `SYN007`, `SYN008`, `SYN010`, `SYN011`, `SYN012`, `SYN013`, `SYN014`, `SYN016`, `SYN018`, `SYN019`, `SYN022`, `SYN023`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | +| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN004`, `SYN005`, `SYN006`, `SYN007`, `SYN008`, `SYN010`, `SYN011`, `SYN012`, `SYN013`, `SYN014`, `SYN015`, `SYN016`, `SYN018`, `SYN019`, `SYN022`, `SYN023`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | A bot's loop becomes deterministic: `transform` → if `ok=false`, read `diagnostics[0].code` → `explain(code)` → apply `rewrite` → `transform` again. diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 1bd18e33..c3ea1a32 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -876,6 +876,39 @@ const E: Record = { " return bc\n" + "}", }, + SYN015: { + code: "SYN015", + title: "localStorage / sessionStorage access bypasses the storage capability model", + rule: + "`localStorage.*` and `sessionStorage.*` accesses are synchronous same-origin Web Storage " + + "operations invisible to botscript's capability model: `reads {}` / `writes {}` labels cover " + + "declared resource identifiers, not the Web Storage API globals. A fn that accesses either " + + "store has undeclared persistent (`localStorage`) or session-scoped (`sessionStorage`) state " + + "dependencies — no header declaration covers the access, and callers cannot observe or audit " + + "the dependency from the fn's declared surface.", + idiom: + "pass the storage value as an explicit fn parameter so callers control what is read and tests " + + "can inject a mock; if direct Web Storage access is genuinely required, wrap in " + + "`unsafe \"accesses localStorage for \" { localStorage.getItem(key) }`", + rewrite: + "// before — localStorage access invisible to the capability model\n" + + "fn getTheme() -> string {\n" + + " return localStorage.getItem('theme') ?? 'light' // SYN015\n" + + "}\n\n" + + "// after — value passed as parameter; dependency visible in the signature\n" + + "fn getTheme(storedTheme: string | null) -> string {\n" + + " return storedTheme ?? 'light'\n" + + "}", + example: + "// SYN015: localStorage access invisible to capability model\n" + + "fn saveUser(user: User) -> void {\n" + + " localStorage.setItem('user', JSON.stringify(user)) // SYN015\n" + + "}\n\n" + + "// fix: wrap in unsafe with a justification\n" + + "fn saveUser(user: User) -> void {\n" + + " unsafe \"persists user to localStorage\" { localStorage.setItem('user', JSON.stringify(user)) }\n" + + "}", + }, SYN016: { code: "SYN016", title: "indexedDB access bypasses the storage capability model", diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index a9f192c6..24222eeb 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -104,6 +104,17 @@ * named `BroadcastChannel`, object/class method shorthands, and TypeScript method * signatures. The `:` exclusion is guarded against ternary consequents. * + * SYN015 A `localStorage.*` or `sessionStorage.*` access was detected in a fn body (?bs 0.7+). + * Both are same-origin synchronous key/value stores invisible to botscript's + * capability model: `reads {}` / `writes {}` labels cover declared resource + * identifiers, not the Web Storage API globals. A fn that reads or writes either + * store has undeclared persistent (`localStorage`) or session-scoped (`sessionStorage`) + * state dependencies that callers cannot observe or audit from the fn's declared surface. + * Detection: `localStorage` or `sessionStorage` ident not preceded by `.`/`?.`, + * followed by `.` or `?.`. `fn`/`function`/`function*` declarations named + * `localStorage`/`sessionStorage` and bare references are excluded. + * `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. + * * SYN016 An `indexedDB.*` access was detected in a fn body (?bs 0.7+). * `indexedDB` is same-origin persistent database storage invisible to botscript's * capability model: `reads {}` / `writes {}` labels cover declared resource @@ -239,6 +250,7 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const syn012 = getErrorCode("SYN012")!; const syn013 = getErrorCode("SYN013")!; const syn014 = getErrorCode("SYN014")!; + const syn015 = getErrorCode("SYN015")!; const syn016 = getErrorCode("SYN016")!; const syn018 = getErrorCode("SYN018")!; const syn019 = getErrorCode("SYN019")!; @@ -1284,6 +1296,52 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult break; } + // ── SYN015: localStorage.* / sessionStorage.* access ───────────────── + case "localStorage": + case "sessionStorage": { + // Exclude: `obj.localStorage` — preceded by `.` or `?.` + const prevIdx15 = prevSignificant(tokens, i - 1); + const prev15 = tokens[prevIdx15]; + if (prev15 && ((prev15.kind === "punct" && prev15.text === ".") || prev15.kind === "questionDot")) + continue; + + // Exclude: fn/function/function* declarations named localStorage or sessionStorage + if (prev15 && prev15.kind === "keyword" && prev15.text === "fn") continue; + if (prev15 && prev15.kind === "ident" && prev15.text === "function") continue; + if (isFunctionStarDecl(tokens, prevIdx15)) continue; + + // Must be followed by `.` or `?.` — bare references (e.g. passing `localStorage` as a value) are not flagged + const nextIdx15 = nextSignificant(tokens, i + 1); + const next15 = tokens[nextIdx15]; + const isDot15 = next15 && next15.kind === "punct" && next15.text === "."; + const isOptChain15 = next15 && next15.kind === "questionDot"; + if (!isDot15 && !isOptChain15) continue; + + if (isInsideRange(tok.start, unsafeRanges)) continue; + + const sep15 = isOptChain15 ? "?." : "."; + const loc15 = locationOf(src, tok.start); + const ns15 = tok.text; + warnings.push({ + code: "SYN015", + severity: "warning", + file: null, + line: loc15.line, + column: loc15.column, + start: tok.start, + end: next15!.end, + message: + `fn '${decl.name}' accesses ${ns15}${sep15} — ` + + `${ns15} is synchronous same-origin Web Storage invisible to the capability model; ` + + `no reads {} / writes {} label covers it; ` + + `pass the storage value as a parameter or wrap in unsafe "accesses ${ns15} for " { ${ns15}${sep15}getItem(key) }`, + rule: syn015.rule, + idiom: syn015.idiom, + rewrite: syn015.rewrite, + }); + break; + } + // ── SYN016: indexedDB.* access ─────────────────────────────────────── case "indexedDB": { // Exclude: `obj.indexedDB` — preceded by `.` or `?.` diff --git a/packages/compiler/tests/error-codes.test.ts b/packages/compiler/tests/error-codes.test.ts index 8c9bfdf2..a10335b1 100644 --- a/packages/compiler/tests/error-codes.test.ts +++ b/packages/compiler/tests/error-codes.test.ts @@ -15,7 +15,7 @@ describe("error-code registry", () => { "INT001", "INT002", "INT003", "INT004", "INT005", "MAT001", "MAT002", "MAT003", "MAT004", "RES001", "RES002", - "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", "SYN007", "SYN008", "SYN010", "SYN011", "SYN012", "SYN013", "SYN014", "SYN016", "SYN018", "SYN019", "SYN022", "SYN023", + "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", "SYN007", "SYN008", "SYN010", "SYN011", "SYN012", "SYN013", "SYN014", "SYN015", "SYN016", "SYN018", "SYN019", "SYN022", "SYN023", "THR001", "THR002", "THR003", "THR004", "UNS001", "UNS002", "UNS003", "UNS004", "UNS005", "VER001", "VER002", "VER003", diff --git a/packages/compiler/tests/syn015-check.test.ts b/packages/compiler/tests/syn015-check.test.ts new file mode 100644 index 00000000..5bd32aeb --- /dev/null +++ b/packages/compiler/tests/syn015-check.test.ts @@ -0,0 +1,225 @@ +import { describe, expect, it } from "vitest"; +import { passSynCheck } from "../src/passes/syn-check.js"; + +function compile(src: string) { + return passSynCheck(src, { resolved: "0.7", declared: "0.7" }); +} + +describe("SYN015: localStorage / sessionStorage access detection", () => { + it("fires on localStorage.getItem()", () => { + const src = + "?bs 0.7\n" + + "fn loadTheme() -> string {\n" + + " return localStorage.getItem('theme') ?? 'light'\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on localStorage.setItem()", () => { + const src = + "?bs 0.7\n" + + "fn saveTheme(theme: string) -> void {\n" + + " localStorage.setItem('theme', theme)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on localStorage.removeItem()", () => { + const src = + "?bs 0.7\n" + + "fn clearTheme() -> void {\n" + + " localStorage.removeItem('theme')\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on localStorage.clear()", () => { + const src = + "?bs 0.7\n" + + "fn clearAll() -> void {\n" + + " localStorage.clear()\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on localStorage.length (property access)", () => { + const src = + "?bs 0.7\n" + + "fn countKeys() -> number {\n" + + " return localStorage.length\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on sessionStorage.getItem()", () => { + const src = + "?bs 0.7\n" + + "fn getSession(key: string) -> string | null {\n" + + " return sessionStorage.getItem(key)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on sessionStorage.setItem()", () => { + const src = + "?bs 0.7\n" + + "fn setSession(key: string, val: string) -> void {\n" + + " sessionStorage.setItem(key, val)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on optional-chain localStorage?.getItem()", () => { + const src = + "?bs 0.7\n" + + "fn loadTheme() -> string | null {\n" + + " return localStorage?.getItem('theme')\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on optional-chain sessionStorage?.setItem()", () => { + const src = + "?bs 0.7\n" + + "fn saveSession(key: string, val: string) -> void {\n" + + " sessionStorage?.setItem(key, val)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("does NOT fire on obj.localStorage.getItem() — member of a local", () => { + const src = + "?bs 0.7\n" + + "fn loadTheme(ctx: any) -> string | null {\n" + + " return ctx.localStorage.getItem('theme')\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire on bare localStorage reference without member access", () => { + const src = + "?bs 0.7\n" + + "fn passStorage(fn: (s: Storage) => void) -> void {\n" + + " fn(localStorage)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire on fn localStorage() botscript declarations", () => { + const src = + "?bs 0.7\n" + + "fn localStorage(key: string) -> string | null {\n" + + " return null\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire on JS function localStorage() declaration inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn outer() -> void {\n" + + " function localStorage(key: string) { return null }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire on JS generator function* localStorage() declaration inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn outer() -> void {\n" + + " function* localStorage(key: string) { yield null }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("is suppressed inside unsafe {} blocks", () => { + const src = + "?bs 0.7\n" + + "fn loadTheme() -> string {\n" + + ' return unsafe "reads theme from localStorage" { localStorage.getItem(\'theme\') } ?? \'light\'\n' + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("is suppressed inside unsafe fn bodies", () => { + const src = + "?bs 0.7\n" + + 'unsafe "accesses localStorage directly" fn saveTheme(theme: string) -> void {\n' + + " localStorage.setItem('theme', theme)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire before ?bs 0.7", () => { + const src = + "?bs 0.6\n" + + "fn loadTheme() -> string {\n" + + " return localStorage.getItem('theme') ?? 'light'\n" + + "}\n"; + const result = passSynCheck(src, { resolved: "0.6", declared: "0.6" }); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("fires with warning severity", () => { + const src = + "?bs 0.7\n" + + "fn loadTheme() -> string {\n" + + " return localStorage.getItem('theme') ?? 'light'\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.severity).toBe("warning"); + }); + + it("counts multiple localStorage accesses independently", () => { + const src = + "?bs 0.7\n" + + "fn syncTheme() -> void {\n" + + " const theme = localStorage.getItem('theme')\n" + + " localStorage.setItem('theme-copy', theme ?? 'light')\n" + + "}\n"; + const result = compile(src); + const codes = result.warnings.filter((w) => w.code === "SYN015"); + expect(codes.length).toBe(2); + }); + + it("counts localStorage and sessionStorage accesses independently", () => { + const src = + "?bs 0.7\n" + + "fn syncStorage() -> void {\n" + + " const val = localStorage.getItem('key')\n" + + " sessionStorage.setItem('key', val ?? '')\n" + + "}\n"; + const result = compile(src); + const codes = result.warnings.filter((w) => w.code === "SYN015"); + expect(codes.length).toBe(2); + }); + + it("message includes the storage global name and member separator", () => { + const src = + "?bs 0.7\n" + + "fn loadTheme() -> string {\n" + + " return localStorage.getItem('theme') ?? 'light'\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.message).toContain("localStorage."); + expect(w?.message).toContain("capability model"); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index e6f5a6a0..53d92e1c 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -1107,6 +1107,52 @@ export const EXPLANATIONS: Readonly> = { "}\n", }, }, + SYN015: { + code: "SYN015", + title: "localStorage / sessionStorage access bypasses the storage capability model", + body: + "SYN015 fires when a fn body accesses `localStorage.*` or `sessionStorage.*` — any member " + + "access (`.getItem`, `.setItem`, `.removeItem`, `.clear`, `.length`, `.key`, etc.) on either " + + "Web Storage global.\n\n" + + "**Why it matters:** `reads {}` and `writes {}` labels in botscript cover declared resource " + + "identifiers (e.g. `reads { cache }`, `writes { prefs }`). Neither `localStorage` nor " + + "`sessionStorage` is part of the stdlib namespace system. A fn that calls " + + "`localStorage.setItem('key', val)` mutates persistent browser storage at runtime but declares " + + "nothing about it in its header. Callers cannot see the dependency, and no audit tool can " + + "observe it from the fn signature.\n\n" + + "**Detection:** the check looks for a `localStorage` or `sessionStorage` ident token not " + + "preceded by `.`/`?.` (which would make it a member of another object), followed by `.` or " + + "`?.` (confirming this is an access on the global, not a bare reference or a declaration). " + + "Fn/function declarations named `localStorage`/`sessionStorage` are excluded.\n\n" + + "**Fix (preferred):** read the value at the call site and pass it as an explicit fn parameter. " + + "This makes the dependency visible in the signature and tests can inject a mock:\n\n" + + "```\n" + + "// SYN015\n" + + "fn getTheme() -> string {\n" + + " return localStorage.getItem('theme') ?? 'light'\n" + + "}\n\n" + + "// fix — stored value is now an explicit parameter\n" + + "fn getTheme(storedTheme: string | null) -> string {\n" + + " return storedTheme ?? 'light'\n" + + "}\n" + + "```\n\n" + + "**Fix (escape hatch):** if direct access is genuinely required, wrap in an `unsafe` block:\n" + + "`unsafe \"reads theme from localStorage\" { localStorage.getItem('theme') }`\n\n" + + "SYN015 fires at `?bs 0.7+` as a non-blocking warning. " + + "Accesses inside `unsafe { }` blocks or `unsafe \"reason\" fn` bodies are suppressed.", + example: { + fails: + "?bs 0.7\n" + + "fn getTheme() -> string {\n" + + " return localStorage.getItem('theme') ?? 'light'\n" + + "}\n", + passes: + "?bs 0.7\n" + + "fn getTheme(storedTheme: string | null) -> string {\n" + + " return storedTheme ?? 'light'\n" + + "}\n", + }, + }, SYN016: { code: "SYN016", title: "indexedDB access bypasses the storage capability model", diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 780ef193..f8229132 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -85,6 +85,7 @@ describe("botscript-mcp explanations", () => { "SYN012", "SYN013", "SYN014", + "SYN015", "SYN016", "SYN018", "SYN019",