diff --git a/AGENTS.md b/AGENTS.md index c3e501ba..1d69e2be 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -208,6 +208,7 @@ 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. | +| SYN015 | (0.7+, warning) A fn body accesses `localStorage.*`, `localStorage[key]`, `sessionStorage.*`, or `sessionStorage[key]` — any member or computed access (`.`, `?.`, `[`) on either Web Storage global. Both globals are same-origin storage invisible to botscript's capability model (`reads {}` / `writes {}` labels cover declared resource identifiers, not the Web Storage API). `localStorage` persists across browser sessions and is shared across all tabs on the same origin; `sessionStorage` is per-tab and cleared when the tab closes, but is equally invisible to the model. A fn that accesses either global has undeclared state dependencies invisible to callers and audit tooling. Detection: `localStorage`/`sessionStorage` ident not preceded by `.`/`?.`, followed by `.`, `?.`, or `[`. Fn/function/function* declarations named `localStorage`/`sessionStorage` are excluded. Fn parameters that shadow the global name are excluded (e.g. `fn f(localStorage: Storage)` does not warn); body-local `const`/`let`/`var` bindings are NOT excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Pass a `Storage`-compatible object as an explicit fn parameter so callers control what storage is accessed and tests can inject a mock. If direct global access is required, wrap in `unsafe "reads/writes 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`, `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() }`. | | 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 c4740a04..73f1ed15 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`, `SYN022`, `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`, `SYN022`, `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 643c4b11..c781944b 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -876,6 +876,41 @@ const E: Record = { " return bc\n" + "}", }, + SYN015: { + code: "SYN015", + title: "localStorage / sessionStorage access bypasses the storage capability model", + rule: + "`localStorage.*`, `localStorage[key]`, `sessionStorage.*`, and `sessionStorage[key]` accesses are same-origin storage operations " + + "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 `localStorage`/`sessionStorage` has " + + "undeclared state dependencies — no `reads {}` / `writes {}` declaration in the fn header " + + "covers the access, and callers cannot observe or audit the dependency from the fn's declared surface. " + + "(`localStorage` persists across browser sessions; `sessionStorage` is scoped to the current tab and cleared when the tab closes.)", + idiom: + "pass a storage abstraction (e.g. a `Storage` interface) as an explicit fn parameter so callers " + + "control what storage is accessed, the dependency is visible in the fn signature, and tests can inject a mock; " + + "if direct access is genuinely required, wrap in " + + "`unsafe \"reads/writes storage for \" { localStorage.getItem(key) }` " + + "(substitute `sessionStorage` and the actual method — e.g. `setItem(key, val)`, `removeItem(key)`, `clear()` — as appropriate)", + rewrite: + "// before — localStorage access invisible to the capability model\n" + + "fn getToken() -> string | null {\n" + + " return localStorage.getItem(\"auth_token\") // SYN015\n" + + "}\n\n" + + "// after — storage passed as a parameter; dependency visible in the signature\n" + + "fn getToken(storage: Storage) -> string | null {\n" + + " return storage.getItem(\"auth_token\")\n" + + "}", + example: + "// SYN015: localStorage access bypasses the storage capability model\n" + + "fn saveUser(user: User) -> void {\n" + + " localStorage.setItem(\"user\", JSON.stringify(user)) // SYN015\n" + + "}\n\n" + + "// fix: pass storage as a parameter\n" + + "fn saveUser(storage: Storage, user: User) -> void {\n" + + " storage.setItem(\"user\", JSON.stringify(user))\n" + + "}", + }, SYN016: { code: "SYN016", title: "indexedDB access bypasses the storage capability model", diff --git a/packages/compiler/src/passes/_callgraph.ts b/packages/compiler/src/passes/_callgraph.ts index e2cc4c05..8ffa1597 100644 --- a/packages/compiler/src/passes/_callgraph.ts +++ b/packages/compiler/src/passes/_callgraph.ts @@ -329,11 +329,17 @@ export function collectFnBodyLocalNames( tokens: Token[], fn: FnDecl, inner: FnDecl[], + opts?: { topLevelOnly?: boolean }, ): Set { const names = new Set(); const open: FnDecl[] = []; let nextInner = 0; const start = fn.bodyTokenStart ?? fn.tokenStart; + const topLevelOnly = opts?.topLevelOnly ?? false; + // blockDepth tracks brace nesting for non-function blocks. + // bodyTokenStart points to the function body's opening `{`, so the first `{` + // increments depth to 1; top-level bindings are at depth === 1. + let blockDepth = 0; for (let i = start; i < fn.tokenEnd; i++) { while (open.length > 0 && open[open.length - 1]!.tokenEnd <= i) open.pop(); @@ -344,8 +350,18 @@ export function collectFnBodyLocalNames( if (open.length > 0) continue; const tok = tokens[i]; - if (!tok || tok.kind !== "ident") continue; + if (!tok) continue; + + // Track brace depth for non-function blocks (inner function bodies are already skipped above). + if (tok.kind === "open" && tok.text === "{") { blockDepth++; continue; } + if (tok.kind === "close" && tok.text === "}") { blockDepth--; continue; } + + if (tok.kind !== "ident") continue; if (tok.text !== "const" && tok.text !== "let" && tok.text !== "var") continue; + // When topLevelOnly, skip const/let bindings inside nested blocks (if, for, etc.), + // but keep var — var is function-scoped in JS/TS and shadows the global name for the + // entire function body regardless of where the declaration appears. + if (topLevelOnly && blockDepth > 1 && tok.text !== "var") continue; const nameIdx = nextSignificant(tokens, i + 1); const nameTok = tokens[nameIdx]; diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 525dfc8b..363fe396 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -57,6 +57,21 @@ * `cond ? new WebSocket(url) : other`). Generic `` detection only when * preceded by `new` (avoids false-positives on `WebSocket < x > (y)` comparisons). * + * SYN015 A `localStorage.*` / `localStorage[key]` or `sessionStorage.*` / `sessionStorage[key]` + * access was detected in a fn body (?bs 0.7+). Both globals are same-origin storage + * whose reads and writes are invisible to botscript's capability model: `reads {}` / + * `writes {}` labels cover declared resource identifiers, not the Web Storage API globals. + * A fn that accesses `localStorage` or `sessionStorage` has undeclared state dependencies + * invisible to callers and audit tooling. (`localStorage` persists across browser sessions; + * `sessionStorage` is per-tab and cleared when the tab closes.) + * Detected forms: `localStorage.key`, `localStorage?.key`, `localStorage[key]`, + * `localStorage?.[key]` (and equivalents for `sessionStorage`). + * Excluded: member calls (`obj.localStorage.*`), `fn`/`function` declarations named + * `localStorage`/`sessionStorage`, fn parameters shadowing the global name, and object + * method shorthands. Body-local `const`/`let`/`var` bindings that shadow the name are + * NOT excluded — only parameter-name shadows are suppressed. + * The check fires on any member access (`.` or `?.`): both reads and writes. + * * SYN010 A `setTimeout(...)`, `setInterval(...)`, or `queueMicrotask(...)` * call was detected in a fn body (?bs 0.7+). These globals schedule * callbacks to run after the current fn returns — any effects inside @@ -148,7 +163,7 @@ import { getErrorCode } from "../error-codes.js"; import { parseProgram } from "../parser/parse.js"; import type { Token } from "../parser/lex.js"; import { locationOf } from "./_location.js"; -import { computeNesting, prevSignificant, nextSignificant } from "./_callgraph.js"; +import { computeNesting, prevSignificant, nextSignificant, collectTopLevelParamNames } from "./_callgraph.js"; import { atLeast, type VersionInfo } from "./version.js"; import { collectUnsafeBlockRanges, isInsideRange } from "./_unsafe-ranges.js"; @@ -199,6 +214,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 syn022 = getErrorCode("SYN022")!; @@ -226,6 +242,10 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const open: typeof inner = []; let nextInner = 0; + // Lazily computed on first localStorage/sessionStorage hit to avoid scanning + // every fn body unconditionally — see case "localStorage"/"sessionStorage" below. + let localBindings: Set | null = null; + const bodyStart = decl.bodyTokenStart ?? decl.tokenStart; // Single dispatch loop: nesting bookkeeping runs once per token position. @@ -1242,6 +1262,114 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult break; } + // ── SYN015: localStorage / sessionStorage access ───────────────────── + case "localStorage": + case "sessionStorage": { + const prevIdx15 = prevSignificant(tokens, i - 1); + const prev15 = tokens[prevIdx15]; + + // Exclude: `obj.localStorage.*` — preceded by `.` or `?.` + if (prev15 && ((prev15.kind === "punct" && prev15.text === ".") || prev15.kind === "questionDot")) + continue; + + // Exclude: `function localStorage(...)` / `function* localStorage(...)` / `fn localStorage(...)` declarations + if (prev15 && prev15.kind === "ident" && prev15.text === "function") continue; + if (prev15 && prev15.kind === "keyword" && prev15.text === "fn") continue; + if (prev15 && prev15.kind === "operator" && prev15.text === "*" && isFunctionStarDecl(tokens, prevIdx15)) continue; + + // Must be followed by `.`, `?.`, or `[` — confirming this is a property/method + // access on the storage object (not a bare reference or assignment target). + // `[` covers computed access: localStorage[key] / sessionStorage[key]. + // Check before localBindings to avoid an unnecessary body scan for bare references. + const nextIdx15 = nextSignificant(tokens, i + 1); + const next15 = tokens[nextIdx15]; + if (!next15 || !( + (next15.kind === "punct" && next15.text === ".") || + next15.kind === "questionDot" || + (next15.kind === "open" && next15.text === "[") + )) continue; + + // Exclude: fn parameters named localStorage / sessionStorage. + // Body-local binding shadowing is intentionally NOT tracked here — block-scope + // awareness would require a full scope walk, and the parameter case (a fn that + // accepts a Storage-compatible mock) is the only practically occurring one. + // This mirrors SYN016 which also does not track body-local shadows. + if (localBindings === null) { + localBindings = collectTopLevelParamNames(decl.args); + } + if (localBindings.has(tok.text)) continue; + + if (isInsideRange(tok.start, unsafeRanges)) continue; + + const storageName15 = tok.text; + const isDirect15 = next15.kind === "open" && next15.text === "["; + const sep15 = next15.kind === "questionDot" ? "?." : isDirect15 ? "" : "."; + const memberIdx15 = nextSignificant(tokens, nextIdx15 + 1); + const memberTok15 = tokens[memberIdx15]; + + // Computed member access: localStorage[key], localStorage?.[key], sessionStorage[key] + const isComputedViaOptChain15 = !!(memberTok15 && memberTok15.kind === "open" && memberTok15.text === "["); + const isComputed15 = isDirect15 || isComputedViaOptChain15; + let memberName15: string; + let rangeEnd15: number; + let afterMemberScanIdx15: number; + if (isComputed15) { + memberName15 = "[…]"; + // Start scanning from the `[` — which is at nextIdx15 for direct access + // (localStorage[key]) or at memberIdx15 for optional-chain (localStorage?.[key]). + const bracketIdx15 = isDirect15 ? nextIdx15 : memberIdx15; + let depth15 = 1; + let j15 = bracketIdx15 + 1; + while (j15 < decl.tokenEnd && depth15 > 0) { + const t15 = tokens[j15]; + if (!t15) break; + if (t15.kind === "open" && t15.text === "[") depth15++; + else if (t15.kind === "close" && t15.text === "]") depth15--; + j15++; + } + const closeBracket15 = tokens[j15 - 1]; + rangeEnd15 = closeBracket15 ? closeBracket15.end : next15!.end; + afterMemberScanIdx15 = nextSignificant(tokens, j15); + } else { + memberName15 = (memberTok15 && memberTok15.kind === "ident") ? memberTok15.text : ""; + rangeEnd15 = (memberTok15 && memberTok15.kind === "ident") ? memberTok15.end : next15!.end; + afterMemberScanIdx15 = nextSignificant(tokens, memberIdx15 + 1); + } + + // Determine if this is a call: `(` = direct call; `?.(` = optional call; `?.` alone = optional property access. + const afterMember15 = tokens[afterMemberScanIdx15]; + let isCall15 = false; + if (afterMember15 && afterMember15.kind === "open" && afterMember15.text === "(") { + isCall15 = true; + } else if (afterMember15 && afterMember15.kind === "questionDot") { + // Optional call `?.(` — check next token is `(` + const afterQD15 = tokens[nextSignificant(tokens, afterMemberScanIdx15 + 1)]; + isCall15 = !!(afterQD15 && afterQD15.kind === "open" && afterQD15.text === "("); + } + const unsafeExample15 = isCall15 + ? `${storageName15}${sep15}${memberName15}(...)` + : `${storageName15}${sep15}${memberName15}`; + const loc15 = locationOf(src, tok.start); + warnings.push({ + code: "SYN015", + severity: "warning", + file: null, + line: loc15.line, + column: loc15.column, + start: tok.start, + end: rangeEnd15, + message: + `fn '${decl.name}' accesses ${storageName15}${sep15}${memberName15} — ` + + `${storageName15} is same-origin storage invisible to the capability model; ` + + `no reads {} / writes {} label covers it; ` + + `pass a storage abstraction as a parameter or wrap in unsafe "accesses ${storageName15} for " { ${unsafeExample15} }`, + 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 607ed593..b7d49c89 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", "SYN022", + "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", "SYN007", "SYN008", "SYN010", "SYN011", "SYN012", "SYN013", "SYN014", "SYN015", "SYN016", "SYN018", "SYN022", "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..d2ea8206 --- /dev/null +++ b/packages/compiler/tests/syn015-check.test.ts @@ -0,0 +1,341 @@ +/** + * Tests for SYN015: localStorage / sessionStorage access detection (?bs 0.7+). + * + * SYN015 is a non-blocking warning — transform must not throw. + */ + +import { describe, it, expect } from "vitest"; +import { transform } from "../src/index.js"; + +function compile(src: string) { + return transform(src, {}); +} + +describe("SYN015: localStorage / sessionStorage access detection", () => { + it("fires on localStorage.getItem(...) inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn getToken() -> any {\n" + + " return localStorage.getItem(\"auth\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on localStorage.setItem(...) inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn saveToken(token: string) -> void {\n" + + " localStorage.setItem(\"auth\", token)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("fires on sessionStorage.getItem(...) inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn getSession(key: string) -> any {\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(...) inside a fn body", () => { + 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("fires on localStorage.removeItem(...)", () => { + const src = + "?bs 0.7\n" + + "fn clearToken() -> void {\n" + + " localStorage.removeItem(\"auth\")\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 sessionStorage?.getItem(...) optional chaining", () => { + const src = + "?bs 0.7\n" + + "fn getToken(key: string) -> any {\n" + + " return sessionStorage?.getItem(key)\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 storageSize() -> any {\n" + + " return localStorage.length\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("does NOT fire below ?bs 0.7", () => { + const src = + "?bs 0.6\n" + + "fn getToken() -> any {\n" + + " return localStorage.getItem(\"auth\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire inside unsafe {} block", () => { + const src = + "?bs 0.7\n" + + "fn getToken() -> any {\n" + + " return unsafe \"reads auth token from localStorage\" { localStorage.getItem(\"auth\") }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire inside unsafe fn body", () => { + const src = + "?bs 0.7\n" + + "unsafe \"accesses localStorage directly\" fn getToken() -> any {\n" + + " return localStorage.getItem(\"auth\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire on obj.localStorage.getItem(...) — member of a local", () => { + const src = + "?bs 0.7\n" + + "fn getToken(obj: any) -> any {\n" + + " return obj.localStorage.getItem(\"auth\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire on bare localStorage reference (not accessed)", () => { + const src = + "?bs 0.7\n" + + "fn getStorage() -> any {\n" + + " return 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 declaration", () => { + const src = + "?bs 0.7\n" + + "fn localStorage(key: string) -> any {\n" + + " return key\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("fires twice when both localStorage and sessionStorage are accessed", () => { + const src = + "?bs 0.7\n" + + "fn syncStorage(key: string) -> 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("severity is 'warning'", () => { + const src = + "?bs 0.7\n" + + "fn getToken() -> any {\n" + + " return localStorage.getItem(\"auth\")\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.severity).toBe("warning"); + }); + + it("message mentions localStorage and the capability model", () => { + const src = + "?bs 0.7\n" + + "fn getToken() -> any {\n" + + " return localStorage.getItem(\"auth\")\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"); + }); + + it("message mentions sessionStorage for sessionStorage access", () => { + const src = + "?bs 0.7\n" + + "fn getSession(key: string) -> any {\n" + + " return sessionStorage.getItem(key)\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.message).toContain("sessionStorage"); + }); + + it("does NOT fire when localStorage is a function parameter name", () => { + const src = + "?bs 0.7\n" + + "fn getToken(localStorage: Storage) -> any {\n" + + " return localStorage.getItem(\"auth\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire when sessionStorage is a function parameter name", () => { + const src = + "?bs 0.7\n" + + "fn getSession(sessionStorage: Storage) -> any {\n" + + " return sessionStorage.getItem(\"key\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("message includes the actual member name for non-getItem accesses", () => { + const src = + "?bs 0.7\n" + + "fn save(key: string, val: string) -> void {\n" + + " localStorage.setItem(key, val)\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.message).toContain("localStorage.setItem"); + }); + + it("DOES fire when localStorage is a local const binding (body-local shadow not tracked)", () => { + // SYN015 only suppresses parameter-name shadows, not body-local bindings. + // Tracking block-scope const/let/var rebindings would require a full scope walk; + // the parameter case is the only practically relevant shadow to suppress. + const src = + "?bs 0.7\n" + + "fn getToken() -> any {\n" + + " const localStorage = window.localStorage\n" + + " return localStorage.getItem(\"auth\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("does NOT fire on function localStorage(...) JS declaration", () => { + const src = + "?bs 0.7\n" + + "fn useStorage() -> void {\n" + + " function localStorage(key: string) { return key }\n" + + " localStorage(\"x\")\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("does NOT fire on function* localStorage(...) generator declaration", () => { + const src = + "?bs 0.7\n" + + "fn useStorage() -> void {\n" + + " function* localStorage(key: string) { yield key }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(false); + }); + + it("fires on localStorage?.[key] (optional computed member) without placeholder", () => { + const src = + "?bs 0.7\n" + + "fn getByKey(key: string) -> any {\n" + + " return localStorage?.[key]\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.message).not.toContain(""); + expect(w?.message).toContain("localStorage?.["); + }); + + it("fires on sessionStorage?.[key] (optional computed member) without placeholder", () => { + const src = + "?bs 0.7\n" + + "fn getByKey(key: string) -> any {\n" + + " return sessionStorage?.[key]\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.message).not.toContain(""); + expect(w?.message).toContain("sessionStorage?.["); + }); + + it("fires on localStorage[key] — non-optional computed member access", () => { + const src = + "?bs 0.7\n" + + "fn getByKey(key: string) -> any {\n" + + " return localStorage[key]\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.message).toContain("localStorage["); + }); + + it("fires on sessionStorage[key] — non-optional computed member access", () => { + const src = + "?bs 0.7\n" + + "fn getByKey(key: string) -> any {\n" + + " return sessionStorage[key]\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + const w = result.warnings.find((w) => w.code === "SYN015"); + expect(w?.message).toContain("sessionStorage["); + }); + + it("DOES fire when localStorage is shadowed only in an inner block — outer-scope access is real", () => { + // A block-scoped shadow inside `if` should not suppress SYN015 for outer-scope access. + const src = + "?bs 0.7\n" + + "fn mixed(cond: boolean) -> string {\n" + + " if (cond) { const localStorage = 'mock' }\n" + + " return localStorage.getItem('key') ?? ''\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); + + it("DOES fire when var localStorage is declared inside a nested block (body-local shadow not tracked)", () => { + // Even though `var` is function-scoped, SYN015 only suppresses parameter-name shadows. + // Body-local bindings (const/let/var) are not tracked to avoid a full scope walk. + const src = + "?bs 0.7\n" + + "fn check(cond: boolean) -> string {\n" + + " if (cond) { var localStorage = 'mock' }\n" + + " return localStorage.getItem('key') ?? ''\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN015")).toBe(true); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 964643a2..39b1aada 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -1107,6 +1107,58 @@ 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`, `.key`, `.length`, etc.) " + + "on either global storage object.\n\n" + + "**Why it matters:** `reads {}` and `writes {}` labels in botscript cover declared resource identifiers " + + "(e.g. `reads { db }`, `writes { cache }`). The Web Storage API globals — `localStorage` and " + + "`sessionStorage` — are not part of the stdlib namespace system. A fn that calls " + + "`localStorage.setItem(key, val)` writes same-origin state 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. The state outlives the fn invocation: a side effect written in one call " + + "is visible in a completely unrelated future call. `localStorage` is persistent across browser " + + "sessions and shared across all tabs on the same origin; `sessionStorage` is per-tab and cleared " + + "when the tab closes, but still invisible to the capability model.\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 a member or computed access on the storage global, not a bare reference or a declaration). " + + "Fn/function/function* declarations named `localStorage`/`sessionStorage` are excluded. " + + "Fn parameters that shadow the global name are excluded — e.g. `fn f(localStorage: Storage)` " + + "will not warn. Body-local `const`/`let`/`var` bindings are NOT excluded; only parameter shadows " + + "are suppressed.\n\n" + + "**Fix (preferred):** pass a `Storage`-compatible object as an explicit fn parameter. This makes " + + "the dependency visible in the fn signature and tests can inject a mock:\n\n" + + "```\n" + + "// SYN015\n" + + "fn getToken() -> string | null {\n" + + " return localStorage.getItem(\"auth_token\")\n" + + "}\n\n" + + "// fix — storage is now an explicit parameter\n" + + "fn getToken(storage: Storage) -> string | null {\n" + + " return storage.getItem(\"auth_token\")\n" + + "}\n" + + "```\n\n" + + "**Fix (escape hatch):** if direct access is genuinely required, wrap in an `unsafe` block:\n" + + "`unsafe \"reads auth token from localStorage\" { localStorage.getItem(\"auth_token\") }`\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 saveUser(user: User) -> void {\n" + + " localStorage.setItem(\"user\", JSON.stringify(user))\n" + + "}\n", + passes: + "?bs 0.7\n" + + "fn saveUser(storage: Storage, user: User) -> void {\n" + + " storage.setItem(\"user\", JSON.stringify(user))\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 ed60cab7..04da37d6 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", "SYN022",