diff --git a/.changeset/proud-koalas-rest.md b/.changeset/proud-koalas-rest.md new file mode 100644 index 000000000..e74b80ac3 --- /dev/null +++ b/.changeset/proud-koalas-rest.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 2826 +--- +**Reviewer lanes are disclosed and consent-gated before install** — a capability that declares a reviewer lane now discloses what it will run and what it will be sent, and blocks on consent before any file is promoted. A spawned lane discloses its binary and its full arguments; an OpenAI-compatible lane discloses its destination host and the config key naming it, including a localhost destination. Both name the egress payload classes — plan text, requirements, research findings, and CONTEXT.md decisions. Changing a lane's binary, arguments, destination, prompt channel, or handler forces re-consent on update; a capability with no reviewer lane is unaffected and its consent record is unchanged. (#2796) diff --git a/docs/explanation/capability-trust-model.md b/docs/explanation/capability-trust-model.md index 2a44d0563..7f69249ee 100644 --- a/docs/explanation/capability-trust-model.md +++ b/docs/explanation/capability-trust-model.md @@ -15,8 +15,9 @@ GSD 1.6.0 opens the capability platform to third-party authors with **full artifact parity**: a third-party capability may ship the same executable -surfaces that GSD Core ships — hooks, MCP servers, command modules. This is a -deliberate product choice, and it carries real security weight. +surfaces that GSD Core ships — hooks, MCP servers, command modules, and +reviewer lanes. This is a deliberate product choice, and it carries real +security weight. Full parity means a third-party capability, once installed, can execute code the next time a relevant loop event fires. There is no "first use" gate. @@ -58,9 +59,9 @@ contains. GSD's response: auto-update is **off by default** for third-party capabilities. When it is enabled, a change to the *executable set* (the set of hooks, MCP -servers, or command modules the capability declares) triggers a re-consent -prompt before the update applies. Updating a non-executable capability -(documentation, agents, skills) does not require re-consent. +servers, command modules, or reviewer lanes the capability declares) triggers a +re-consent prompt before the update applies. Updating a non-executable +capability (documentation, agents, skills) does not require re-consent. VS Code also has no signature check on VSIX packages. GSD requires an `integrity` SHA-512 pin in the ledger, verified before extraction. @@ -75,11 +76,57 @@ recommendation for sensitive environments is `--ignore-scripts`. GSD takes a stronger position: **install never executes capability code**, full stop. Installation is a copy-only staging operation. There is no -`postinstall`-equivalent. A capability's hooks, MCP server, and command -modules are not invoked during install; they are first invoked when the loop -fires after install. This means a malicious payload in an executable surface -cannot be triggered by the act of downloading it — the user has a window -between install and first use to verify what they consented to. +`postinstall`-equivalent. A capability's hooks, MCP server, command +modules, and reviewer lanes are not invoked during install; they are first +invoked when the loop fires after install. This means a malicious payload in an +executable surface cannot be triggered by the act of downloading it — the user +has a window between install and first use to verify what they consented to. + +### The reviewer lane: the one surface that *receives* data + +Three of the four disclosure classes are about code the capability gets to +**run**. A reviewer lane — one external CLI or model endpoint that `/gsd:review` +hands a plan to — is different in kind, and the difference is the reason it is +disclosed at all. + +A lane is piped the plan text, the requirements, the research findings, and the +`CONTEXT.md` decisions, and its output is read back into `REVIEWS.md`. That is an +**egress channel for the most sensitive artifacts GSD produces**. Making lanes +pluggable without a disclosure class would have opened a data-exfiltration path +behind a manifest field, which is why the trust work gates the feature rather +than following it. + +What is disclosed depends on how the lane is reached: + +- A **spawned** lane discloses its binary **and its full declared arguments**, in + both rendered and raw form. Disclosing the binary alone would be insufficient, + and not hypothetically: a lane declaring `python3` with innocuous arguments + could later change them to `["-c", ""]` without the binary + changing at all. Arguments are therefore signature-bound, exactly as they are + for MCP servers. +- An **OpenAI-compatible HTTP** lane has no binary, so it discloses the + **destination host** and the config key that names it. Disclosing `curl` would + be technically true and practically meaningless; the destination is the + disclosure that matters. A `localhost` destination is still disclosed, and is + distinguished from a remote one — a lane pointed at a local port is an egress + channel too, and the port may not be what the user assumes. + +Both forms additionally name the **egress payload classes**, rather than an +unhelpful "sends data to the tool". + +**Stated honestly:** consent-at-install is a weaker gate for a *standing* egress +channel than it is for a hook. A user consents once; the lane thereafter receives +every plan on every review run. Disclosure makes the channel visible, pinned and +revocable — it does not make it safe. A per-run egress prompt was considered and +rejected as consent fatigue that trains users to approve blindly. + +One consequence is worth naming because it does not follow the pattern of the +other three classes. A lane's destination host lives in `.planning/config.json`, +which is user- and CI-editable at any time with no re-install and no integrity +check — unlike every other consent-bound value, all of which come from the +SHA-pinned manifest. Consent therefore binds the **resolved host**, not merely +the config key, so that a later edit redirecting a consented lane to a different +destination is detectable rather than silent. SLSA provenance (the `provenance` field in `capability.json`) provides a machine-checkable link from a capability bundle back to a specific commit in a diff --git a/src/capability-trust.cts b/src/capability-trust.cts index fddb28faa..16c1f66b5 100644 --- a/src/capability-trust.cts +++ b/src/capability-trust.cts @@ -1,5 +1,6 @@ /** - * Capability trust gate — ADR-1244 Phase 4 (Decision D5 + the compatibility half of D6). + * Capability trust gate — ADR-1244 Phase 4 (Decision D5 + the compatibility half of D6), extended + * by ADR-2782 Phase 3 (#2796) with a FOURTH executable-surface class: the reviewer lane. * * PURE module. It computes *what* a capability would do and *whether* policy allows it; it * never mutates the filesystem and never performs I/O beyond reading staged files to confirm @@ -9,15 +10,28 @@ * * LEAF MODULE — imports ONLY: node:fs, node:path, and ./semver-compare.cjs. * + * ADR-2782 D5 (#2796): a `reviewer` lane is piped the plan text, requirements, research findings + * and CONTEXT.md decisions, then its output is read back into REVIEWS.md — making it an executable + * surface exactly like a hook, command module, or MCP server, and it is disclosed and consent-bound + * the same way. `disclosureSignature` appends the lane element to its output ONLY when at least one + * lane is declared (D4.5) — a lane-free manifest's signature stays byte-identical to before this + * class existed, so no already-consented capability re-prompts on upgrade. The RESOLVED host (as + * opposed to the declared `hostConfigKey`) is disclosed to a human but deliberately EXCLUDED from + * the signature — the loader has no config resolver and must compute the same signature as the + * lifecycle (constraint 2, `.gsd/phase/chore-2796-reviewer-trust-disclosure/40-design.md`). + * * Exports: * RESERVED_NAMESPACES — id prefixes third parties may not claim - * discloseExecutableSurfaces(...) — enumerate hooks / command modules / mcpServers + * discloseExecutableSurfaces(...) — enumerate hooks / command modules / mcpServers / reviewer lanes + * collectReviewerLaneSurfaces(...) — the reviewer-lane collector, independently testable * checkReservedNamespace(id) — is this id in a reserved namespace? * evaluateSourceAllowed(parsed,...) — strictKnownRegistries enforcement * checkEngines(manifest, host) — engines.gsd hard gate + compatVersions downgrade * evaluateInstallTrust(args) — compose: source + namespace + engines + disclosure * executableSetChanged(old, new) — did the executable surface set change between versions? * summarizeDisclosure(disclosure) — human-readable consent-prompt lines + * UNRESOLVED_HOST_MARKER — the non-blank marker for an unresolved openai-http host + * EGRESS_PAYLOAD_CLASSES — the named data classes every reviewer lane receives */ import fs from 'node:fs'; @@ -40,6 +54,29 @@ const semverMod = require('./semver-compare.cjs') as { */ const RESERVED_NAMESPACES = ['gsd-', 'gsd-core-', 'anthropic-']; +/** + * ADR-2782 D5's gating requirement (#2796): every reviewer lane is piped the plan text, + * requirements, research findings and CONTEXT.md decisions. Named explicitly here so disclosure + * says exactly this — never the unhelpful "sends data to the tool" (design section B5). + */ +const EGRESS_PAYLOAD_CLASSES = ['plan text', 'requirements', 'research findings', 'CONTEXT.md decisions']; + +/** + * B3 (#2796 matrix): `resolvedHost` must never be a blank string — a blank reads as "no + * destination" rather than "not resolved". This marker is disclosed for an `openai-http` lane + * when no resolver was supplied to `collectReviewerLaneSurfaces`, or the supplied resolver could + * not resolve the declared `hostConfigKey`. Deliberately NOT part of `disclosureSignature`'s input + * (see the lane signature line) — only the human-facing surface carries it. + */ +const UNRESOLVED_HOST_MARKER = '(unresolved — no host resolver was supplied at disclosure time)'; + +/** + * Loopback hostnames recognized LITERALLY, never by substring (an evil host must not spoof this, + * e.g. `notlocalhost.example`). D5: localhost is not "safe by default" — it is disclosed and + * FLAGGED, never omitted (matrix B4). + */ +const LOOPBACK_HOSTNAMES = new Set(['localhost', '127.0.0.1', '::1', '[::1]', '0.0.0.0']); + // --------------------------------------------------------------------------- // Types // --------------------------------------------------------------------------- @@ -52,6 +89,8 @@ interface CapabilityManifest { hooks?: unknown; commands?: unknown; mcpServers?: unknown; + /** ADR-2782 (#2796): a single reviewer-lane body — never an array (Phase 2's own validator rejects that shape). */ + reviewer?: unknown; [k: string]: unknown; } @@ -124,6 +163,65 @@ interface McpServerSurface { rawConfig: Record; } +/** + * Resolves a reviewer lane's `hostConfigKey` (a dotted key into `.planning/config.json`) to its + * configured destination host, for HUMAN disclosure only. Optional — the loader has no config + * access and calls `discloseExecutableSurfaces`/`signatureForManifest` without one; the lifecycle + * MAY supply one so the consent prompt shows a real destination instead of `UNRESOLVED_HOST_MARKER`. + * MUST NOT throw is not required of the caller — `collectReviewerLaneSurfaces` treats any thrown + * error or non-string/empty return as "could not resolve" and falls back to the marker. + */ +type ReviewerHostResolver = (hostConfigKey: string) => string | undefined; + +/** + * ADR-2782 D5 (#2796): the reviewer-lane executable surface. A capability manifest carries AT MOST + * ONE `reviewer` body (Phase 2's validator rejects an array), so this collector returns 0 or 1 + * entries per manifest — the array return shape matches the other three collectors for a uniform + * `Disclosure` and lets `disclosureSignature` sort/fold it the same way. + */ +interface ReviewerLaneSurface { + /** The lane's declared identity (also its config/flag namer). Empty when undeclared/malformed. */ + slug: string; + /** `'spawn'` | `'openai-http'` as declared, or '' when absent/malformed — disclosure never validates. */ + transport: string; + /** spawn: the executable name/path as declared. Empty for an openai-http lane or when undeclared. */ + binary: string; + /** spawn: the declared args, RENDERED (string-filtered) for the human summary — mirrors MCP's argv. */ + args: string[]; + /** + * spawn: the FULL declared args array (may contain non-strings the host still receives) — folded + * into the signature so ANY member change forces re-consent (matrix A6, the #1459 bug class: + * `python3` with innocuous args later becoming `['-c', '']`). Empty when undeclared. + */ + rawArgs: unknown[]; + /** openai-http: the dotted config key naming the destination host. Empty for a spawn lane. */ + hostConfigKey: string; + /** + * openai-http: the resolved destination host when a resolver was supplied and could resolve + * `hostConfigKey`; `UNRESOLVED_HOST_MARKER` when a resolver was not supplied or could not resolve + * it. NEVER '' for an openai-http lane (matrix B3) — a blank reads as "no destination". '' for a + * spawn lane (no destination concept — mirrors McpServerSurface's empty-when-inapplicable + * convention). Deliberately EXCLUDED from `disclosureSignature`'s input (design constraint 2): the + * loader has no resolver and must compute the SAME signature as the lifecycle. + */ + resolvedHost: string; + /** + * True when `resolvedHost` is a loopback/local destination. D5: localhost is disclosed like any + * other destination, never treated as "safe by default" (matrix B4). Always false for a spawn + * lane and for an unresolved openai-http host. + */ + isLocalDestination: boolean; + /** How the review prompt reaches the lane (`'stdin'|'argv'|'argv-file-ref'|'none'`, or ''). */ + promptChannel: string; + /** The first-party handler module name that post-processes this lane's output, or '' when undeclared. */ + handler: string; + /** + * The data classes that egress to this lane on every run (`EGRESS_PAYLOAD_CLASSES`) — named + * honestly (Kerckhoffs's Principle) rather than disclosed as an unhelpful "sends data to the tool". + */ + egressPayloadClasses: string[]; +} + interface Disclosure { /** Hook scripts the capability registers (each runs as a runtime hook command). */ hooks: HookSurface[]; @@ -131,6 +229,12 @@ interface Disclosure { commandModules: CommandModuleSurface[]; /** MCP servers the capability declares (each spawned by the host runtime) — name AND command. */ mcpServers: McpServerSurface[]; + /** + * ADR-2782 (#2796): the reviewer lane this capability declares — 0 or 1 entries (a manifest + * carries at most one `reviewer` body). A fourth executable-surface class alongside the three + * above; a standing egress channel to an external reviewer. + */ + reviewerLanes: ReviewerLaneSurface[]; /** True when the capability ships ANY executable surface (=> consent required). */ hasExecutable: boolean; /** @@ -172,6 +276,12 @@ interface InstallTrustArgs { stagedDir?: string; strictKnownRegistries?: StrictKnownRegistries; hostVersion: string; + /** + * Optional (#2796): resolves a reviewer lane's `hostConfigKey` to its configured destination, for + * HUMAN disclosure at install time only — never folds into the consent signature (design + * constraint 2; see `ReviewerHostResolver`). + */ + resolveHost?: ReviewerHostResolver; } interface InstallTrustVerdict { @@ -194,25 +304,147 @@ function asString(v: unknown): string { } /** - * Enumerate every executable surface a capability manifest declares. - * - * Recognizes the three executable surface kinds a capability can ship: - * - `hooks`: [{ event, script }] — scripts run as runtime hook commands - * - `commands`:[{ family, module, router? }] — modules require()'d into the CLI process - * - `mcpServers`: { : {...} } | [{ name }] — servers spawned by the host runtime - * - * `mcpServers` is not a first-party capability.json field today, but a third-party manifest may - * declare it, so the trust gate discloses it whenever present (honest disclosure over the - * narrower first-party schema). Pure: when `stagedDir` is provided, declared script/module - * files are existence-checked and any missing ones reported, but nothing is mutated. + * Run `fn`, returning `fallback` instead of throwing. Makes each per-class collector total: a + * hostile manifest (a Proxy with a throwing trap, a throwing getter, or a non-object/null root) + * degrades ONE surface class to empty rather than crashing disclosure for the other three classes + * behind it in the same manifest (ADR-2782 #2796 — disclosure runs before validation and must never + * throw; matrix C5/E2). */ -function discloseExecutableSurfaces(manifest: CapabilityManifest, stagedDir?: string): Disclosure { - const hooks: HookSurface[] = []; - const commandModules: CommandModuleSurface[] = []; - const mcpServers: McpServerSurface[] = []; - const missingArtifacts: string[] = []; +function safeCollect(fn: () => T, fallback: T): T { + try { + return fn(); + } catch { + return fallback; + } +} - // hooks: [{ event, script }] +/** + * Recognize a loopback/local destination from a RESOLVED openai-http host value (matrix B4). Matches + * literally, never by substring — an evil host must not spoof `localhost` via e.g. + * `notlocalhost.example`. Falls back to a scheme-less leading-segment match so a bare config value + * like `localhost:1234` or `192.168.1.5:8080` (no `http://` prefix) is still recognized. + * + * The fallback triggers on EITHER `new URL()` throwing (a value that is not parseable as an absolute + * URL at all, e.g. `192.168.1.5:8080` — WHATWG scheme names cannot start with a digit) OR it + * succeeding with an EMPTY hostname: `new URL('localhost:1234')` does NOT throw — it mis-parses the + * scheme-less `host:port` shape as an opaque URL whose "scheme" IS the hostname text + * (`protocol: "localhost:"`, `hostname: ""`), which would otherwise silently fail to recognize a + * bare local config value as local. + */ +function isLocalHostValue(hostValue: string): boolean { + let hostname = ''; + try { + hostname = new URL(hostValue).hostname; + } catch { + hostname = ''; + } + if (!hostname) { + hostname = extractBareHost(hostValue); + } + // WHATWG returns an IPv6 hostname bracketed; a bare config value may not be. + const lower = hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, ''); + if (LOOPBACK_HOSTNAMES.has(lower)) return true; + if (isLoopbackIpv6(lower)) return true; + return isLoopbackIpv4(lower); +} + +/** + * Pull the host out of a value `new URL()` could not parse — a scheme-less + * `host:port`, or one carrying a path/query/fragment. + * + * IPv6 needs explicit handling: splitting on `:` mangles `[::1]:8080` to `[`, + * which then matches nothing and silently reports a loopback destination as + * remote. A bracketed literal is taken through its closing bracket; an unbracketed + * value with two or more colons is treated as a bare IPv6 address rather than + * `host:port`, since a host:port has exactly one. + */ +function extractBareHost(hostValue: string): string { + let s = String(hostValue).trim(); + const schemeEnd = s.indexOf('://'); + if (schemeEnd >= 0) s = s.slice(schemeEnd + 3); + s = s.split(/[/?#]/)[0] || ''; + if (s.startsWith('[')) { + const close = s.indexOf(']'); + return close > 0 ? s.slice(1, close) : s; + } + const colons = (s.match(/:/g) || []).length; + if (colons >= 2) return s; + return colons === 1 ? s.slice(0, s.indexOf(':')) : s; +} + +/** + * Render one declared argv member for the human consent prompt. + * + * A string prints as itself. Anything else prints in a form that makes its + * presence and shape visible rather than vanishing: an argv member the host + * still receives, but which the user was never shown, is a surface consented to + * unseen. Never throws — a circular or BigInt member must not break the prompt. + */ +function renderArgForHuman(arg: unknown): string { + if (typeof arg === 'string') return arg; + if (typeof arg === 'bigint') return `<${String(arg)}n>`; + try { + const json = JSON.stringify(arg); + return json === undefined ? `<${typeof arg}>` : `<${json}>`; + } catch { + return `<${typeof arg}>`; + } +} + +/** `::1`, its expanded forms, and IPv4-mapped loopback (`::ffff:127.0.0.1`). */ +function isLoopbackIpv6(host: string): boolean { + if (!host.includes(':')) return false; + if (host === '::1') return true; + const mapped = /^::ffff:(.+)$/i.exec(host); + if (mapped) return isLoopbackIpv4(mapped[1]); + const groups = host.split(':').filter((g) => g !== ''); + if (groups.length === 0) return false; + return groups.every((g, i) => (i === groups.length - 1 ? /^0*1$/.test(g) : /^0*$/.test(g))); +} + +/** + * 127.0.0.0/8 under inet_aton semantics, which is what a browser, curl and the + * OS resolver all accept. `127.1`, `2130706433`, `0x7f000001` and `0177.0.0.1` + * are every bit as loopback as `127.0.0.1`; a disclosure that flags only the + * dotted-quad form understates a local destination for the other four. + */ +function isLoopbackIpv4(host: string): boolean { + const parts = host.split('.'); + if (parts.length < 1 || parts.length > 4) return false; + const nums: number[] = []; + for (const part of parts) { + let n: number; + if (/^0[xX][0-9a-fA-F]+$/.test(part)) n = parseInt(part, 16); + else if (/^0[0-7]+$/.test(part)) n = parseInt(part, 8); + else if (/^\d+$/.test(part)) n = parseInt(part, 10); + else return false; + if (!Number.isFinite(n) || n < 0) return false; + nums.push(n); + } + // inet_aton: the final part absorbs every remaining octet. + let addr: number; + if (nums.length === 1) addr = nums[0]; + else if (nums.length === 2) addr = ((nums[0] & 0xff) * 0x1000000) + (nums[1] & 0xffffff); + else if (nums.length === 3) addr = ((nums[0] & 0xff) * 0x1000000) + ((nums[1] & 0xff) * 0x10000) + (nums[2] & 0xffff); + else addr = ((nums[0] & 0xff) * 0x1000000) + ((nums[1] & 0xff) * 0x10000) + ((nums[2] & 0xff) * 0x100) + (nums[3] & 0xff); + if (!Number.isFinite(addr) || addr < 0 || addr > 0xffffffff) return false; + return Math.floor(addr / 0x1000000) === 127; +} + +/** + * Collect the `hooks` executable-surface class: [{ event, script }] — scripts run as runtime hook + * commands. Extracted from the former monolithic `discloseExecutableSurfaces` (ADR-2782 #2796, + * cyclomatic 51 / cognitive 99 / 110 lines / `risk_level: critical`) — BEHAVIOR UNCHANGED, only + * isolated so it is independently testable and the orchestrator shrinks instead of growing a fourth + * class inline. `missingArtifacts` is a shared accumulator the orchestrator passes to every collector + * that can populate it. + */ +function collectHookSurfaces( + manifest: CapabilityManifest, + stagedDir: string | undefined, + missingArtifacts: string[], +): HookSurface[] { + const hooks: HookSurface[] = []; if (Array.isArray(manifest.hooks)) { for (const h of manifest.hooks) { if (typeof h !== 'object' || h === null) continue; @@ -227,8 +459,19 @@ function discloseExecutableSurfaces(manifest: CapabilityManifest, stagedDir?: st } } } + return hooks; +} - // commands: [{ family, module, router? }] +/** + * Collect the `commands` executable-surface class: [{ family, module, router? }] — modules + * require()'d into the GSD CLI process. Extracted, BEHAVIOR UNCHANGED — see `collectHookSurfaces`. + */ +function collectCommandSurfaces( + manifest: CapabilityManifest, + stagedDir: string | undefined, + missingArtifacts: string[], +): CommandModuleSurface[] { + const commandModules: CommandModuleSurface[] = []; if (Array.isArray(manifest.commands)) { for (const c of manifest.commands) { if (typeof c !== 'object' || c === null) continue; @@ -245,10 +488,21 @@ function discloseExecutableSurfaces(manifest: CapabilityManifest, stagedDir?: st } } } + return commandModules; +} - // mcpServers: object map { name: { command, args } } OR array [{ name, command, args }] - // (or array [{ name, config: { command, args } }]). Capture the COMMAND, not just the name — - // the command is the executable that actually runs, and consent must disclose it (Codex R1 H1). +/** + * Collect the `mcpServers` executable-surface class: object map { name: { command, args } } OR + * array [{ name, command, args }] (or array [{ name, config: { command, args } }]). Captures the + * COMMAND, not just the name — the command is the executable that actually runs, and consent must + * disclose it (Codex R1 H1). Extracted, BEHAVIOR UNCHANGED — see `collectHookSurfaces`. Unlike + * hooks/commands, an MCP server's command is never existence-checked against `stagedDir` (exactly + * like a reviewer lane's `binary` — see `collectReviewerLaneSurfaces` — it may be any PATH + * executable, not necessarily a bundle artifact), so this collector takes no `missingArtifacts` + * accumulator. + */ +function collectMcpSurfaces(manifest: CapabilityManifest): McpServerSurface[] { + const mcpServers: McpServerSurface[] = []; if (manifest.mcpServers && typeof manifest.mcpServers === 'object') { const pushServer = (name: string, config: unknown): void => { if (!name) return; @@ -313,9 +567,174 @@ function discloseExecutableSurfaces(manifest: CapabilityManifest, stagedDir?: st } } } + return mcpServers; +} - const hasExecutable = hooks.length > 0 || commandModules.length > 0 || mcpServers.length > 0; - return { hooks, commandModules, mcpServers, hasExecutable, missingArtifacts }; +/** + * Collect the reviewer-lane executable-surface class (ADR-2782 D5, #2796): 0 or 1 entries, since a + * capability manifest carries AT MOST ONE `reviewer` body (Phase 2's validator rejects an array + * shape outright — matrix C2b). The array return shape matches the other three collectors so + * `Disclosure`/`disclosureSignature` treat it uniformly (sort-then-fold), even though today it can + * never hold more than one entry. + * + * TOTAL and absent-safe (matrix C1–C5): no `reviewer` key, `reviewer: null`, a non-object body + * (array/boolean/number), a malformed `invoke`, non-array `flags`, or the whole manifest being a + * throwing Proxy/getter all degrade to "no lane" rather than throwing — disclosure runs BEFORE + * Phase 2's validation, on a manifest validation would reject outright. + * + * `resolveHost` is optional — supplied by the lifecycle (never the loader, which has no config + * access) to disclose the REAL destination of an `openai-http` lane to a human at install/upgrade + * time. Its return value is NEVER folded into `disclosureSignature` (design constraint 2: the + * signature must stay a pure function of the manifest, or the loader and lifecycle would compute + * different signatures for the same manifest and produce a permanent false re-consent loop). + */ +function collectReviewerLaneSurfaces( + manifest: CapabilityManifest, + resolveHost?: ReviewerHostResolver, +): ReviewerLaneSurface[] { + return safeCollect(() => { + const r = manifest.reviewer; + // C1 (no reviewer key) / C2a (null) / C2b (non-object: array, boolean, number) all disclose no + // lane — never an error at this layer. Validation of a malformed body is Phase 2's job. + if (typeof r !== 'object' || r === null || Array.isArray(r)) return []; + const rec = r as Record; + + const slug = asString(rec['slug']); + const transport = asString(rec['transport']); + const handler = asString(rec['handler']); + + // C3: `invoke` absent/malformed still discloses a lane, with empty binary/args/rawArgs rather + // than crashing — validating `invoke`'s shape is Phase 2's job, not disclosure's. + const invokeRaw = rec['invoke']; + const invoke = (typeof invokeRaw === 'object' && invokeRaw !== null && !Array.isArray(invokeRaw)) + ? (invokeRaw as Record) + : {}; + + const binary = asString(invoke['binary']); + // B1b: the RAW declared args (may contain non-strings the host still receives) is what the + // signature binds; `args` is the string-filtered RENDERED view for a human summary — the exact + // argv/rawArgs split MCP servers already use for the same reason (TRUST2-4, #1459). + const rawArgsDeclared = Array.isArray(invoke['args']) ? (invoke['args'] as unknown[]) : []; + const args = rawArgsDeclared.filter((a): a is string => typeof a === 'string'); + const hostConfigKey = asString(invoke['hostConfigKey']); + const promptChannel = asString(invoke['promptChannel']); + + // An EMPTY (or wholly unrecognised) reviewer body declares no lane and must + // not be treated as one. Without this, `reviewer: {}` alone flips + // hasExecutable true and perturbs the disclosure signature — producing a + // re-consent prompt whose only content is "(no binary declared)". That is a + // prompt carrying no security information, which is exactly the + // click-through-training harm this design refuses for reviewsSection and + // timeoutFloorMs; refusing it there and permitting it here would be + // inconsistent. + // + // The test is deliberately BROAD — any one recognised field with a value is + // enough. Requiring specifically a binary, or specifically a slug, would let + // a lane declaring only the other slip through unconsented, which is the far + // worse failure. + const declaresSomething = Boolean( + slug || transport || handler || binary || hostConfigKey || promptChannel + || rawArgsDeclared.length > 0, + ); + if (!declaresSomething) return []; + + // B2/B3/B4: resolvedHost/isLocalDestination are only meaningful for an openai-http lane — a + // spawn lane has no destination concept, so both stay at their inapplicable defaults ('' / + // false), mirroring McpServerSurface's existing empty-when-inapplicable convention (e.g. + // `url: ''` for a stdio server). For openai-http, resolvedHost never ends up '' — it is either a + // real resolved value or the explicit UNRESOLVED_HOST_MARKER (never a blank read as "no + // destination"). + // The shape test is deliberately WIDER than an exact transport match, and the + // human summary uses the same one. Disclosure runs BEFORE validation, so a + // mis-cased or unrecognised `transport` reaches here; keying only on the exact + // string would leave a lane that plainly declares a hostConfigKey with a BLANK + // destination, which reads as "no destination" — the precise thing B3 forbids. + const hasHttpShape = transport === 'openai-http' || (!binary && Boolean(hostConfigKey)); + + let resolvedHost = ''; + let isLocalDestination = false; + if (hasHttpShape) { + resolvedHost = UNRESOLVED_HOST_MARKER; + if (typeof resolveHost === 'function') { + let resolved: string | undefined; + try { + resolved = resolveHost(hostConfigKey); + } catch { + resolved = undefined; + } + if (typeof resolved === 'string' && resolved) resolvedHost = resolved; + } + if (resolvedHost !== UNRESOLVED_HOST_MARKER) { + isLocalDestination = isLocalHostValue(resolvedHost); + } + } + + const surface: ReviewerLaneSurface = { + slug, + transport, + binary, + args, + rawArgs: rawArgsDeclared, + hostConfigKey, + resolvedHost, + isLocalDestination, + promptChannel, + handler, + // B5: every lane receives the same named egress payload classes — a fresh copy per surface so + // no caller can mutate the shared constant through a returned surface. + egressPayloadClasses: [...EGRESS_PAYLOAD_CLASSES], + }; + return [surface]; + }, []); +} + +/** + * Enumerate every executable surface a capability manifest declares. + * + * Recognizes the FOUR executable surface kinds a capability can ship: + * - `hooks`: [{ event, script }] — scripts run as runtime hook commands + * - `commands`: [{ family, module, router? }] — modules require()'d into the CLI process + * - `mcpServers`: { : {...} } | [{ name }] — servers spawned by the host runtime + * - `reviewer`: { slug, transport, invoke, ... } — an external reviewer lane (ADR-2782 D5, #2796) + * + * `mcpServers` is not a first-party capability.json field today, but a third-party manifest may + * declare it, so the trust gate discloses it whenever present (honest disclosure over the + * narrower first-party schema). Pure: when `stagedDir` is provided, declared hook/command-module + * files are existence-checked and any missing ones reported, but nothing is mutated. A reviewer + * lane's `binary` is NEVER existence-checked against `stagedDir` (matrix C6) — like an MCP server's + * command, it is a PATH lookup on the user's machine, never a bundle artifact; existence-checking it + * would block every lane install. + * + * TOTAL: never throws, for any manifest shape — including a non-object manifest, a Proxy with + * throwing traps, or a property with a throwing getter (matrix C5, E2). Disclosure runs BEFORE + * Phase 2's validation, on a manifest validation would reject outright, so it must tolerate what + * validation does not. Each surface class is collected independently (`safeCollect`) so a hostile + * value in ONE class degrades only that class to empty rather than losing the other three. + * + * `resolveHost` (optional, #2796) is forwarded to `collectReviewerLaneSurfaces` so a caller with + * config access (the lifecycle, never the loader — see `signatureForManifest`) can disclose the REAL + * destination of an `openai-http` lane. It never affects the returned signature. + */ +function discloseExecutableSurfaces( + manifest: CapabilityManifest, + stagedDir?: string, + resolveHost?: ReviewerHostResolver, +): Disclosure { + const missingArtifacts: string[] = []; + const hooks = safeCollect(() => collectHookSurfaces(manifest, stagedDir, missingArtifacts), [] as HookSurface[]); + const commandModules = safeCollect( + () => collectCommandSurfaces(manifest, stagedDir, missingArtifacts), + [] as CommandModuleSurface[], + ); + const mcpServers = safeCollect(() => collectMcpSurfaces(manifest), [] as McpServerSurface[]); + const reviewerLanes = safeCollect( + () => collectReviewerLaneSurfaces(manifest, resolveHost), + [] as ReviewerLaneSurface[], + ); + + const hasExecutable = + hooks.length > 0 || commandModules.length > 0 || mcpServers.length > 0 || reviewerLanes.length > 0; + return { hooks, commandModules, mcpServers, reviewerLanes, hasExecutable, missingArtifacts }; } /** @@ -510,7 +929,7 @@ function checkEngines(manifest: CapabilityManifest, hostVersion: string): Engine * is defense-in-depth and lets callers surface a compatVersions downgrade hint. */ function evaluateInstallTrust(args: InstallTrustArgs): InstallTrustVerdict { - const { parsed, manifest, stagedDir, strictKnownRegistries, hostVersion } = args; + const { parsed, manifest, stagedDir, strictKnownRegistries, hostVersion, resolveHost } = args; const blockReasons: string[] = []; const src = evaluateSourceAllowed(parsed, strictKnownRegistries); @@ -534,7 +953,10 @@ function evaluateInstallTrust(args: InstallTrustArgs): InstallTrustVerdict { ); } - const disclosure = discloseExecutableSurfaces(manifest, stagedDir); + // #2796: resolveHost is optional and, when supplied, discloses the REAL destination of an + // openai-http reviewer lane to the human at install/upgrade time — it never affects the + // consent-binding signature (disclosureSignature never reads resolvedHost; design constraint 2). + const disclosure = discloseExecutableSurfaces(manifest, stagedDir, resolveHost); // A manifest that declares a hook script or command module NOT present in the staged bundle // (missing, or escaping the bundle via an absolute/`..` path) is rejected: such an artifact @@ -560,13 +982,42 @@ function evaluateInstallTrust(args: InstallTrustArgs): InstallTrustVerdict { * reordering. Used to fold an MCP server's `env` map into the disclosure signature: ADDING or * CHANGING any env entry changes the signature (forces re-consent), but merely REORDERING the keys * does NOT (no false re-prompt). TRUST-2 (#1459). + * + * TOTAL (#2796, matrix C5c/E2): a value declared inside an unvalidated manifest — e.g. a reviewer + * lane's `invoke.args` — may contain a BigInt (which `JSON.stringify` throws on) or a circular + * reference (which unguarded recursion stack-overflows on). Both are handled without throwing: + * a BigInt renders as its decimal string; a cycle (an object that is its OWN ancestor in the current + * recursion path — tracked via `seen`, added before recursing into children and removed once fully + * processed) renders as the literal string `"[Circular]"`. Neither case is reachable for the golden + * hooks/mods/mcp fixtures this phase's byte-identity tests pin down, so their output is unaffected. */ -function stableJson(value: unknown): string { - if (value === null || typeof value !== 'object') return JSON.stringify(value) ?? 'null'; - if (Array.isArray(value)) return `[${value.map(stableJson).join(',')}]`; - const obj = value as Record; - const keys = Object.keys(obj).sort(); - return `{${keys.map((k) => `${JSON.stringify(k)}:${stableJson(obj[k])}`).join(',')}}`; +function stableJson(value: unknown, seen?: Set): string { + if (typeof value === 'bigint') return JSON.stringify(`${value.toString()}n`); + if (value === null || typeof value !== 'object') { + try { + return JSON.stringify(value) ?? 'null'; + } catch { + // A non-object value whose serialization still throws (defensive; JSON.stringify does not + // throw for any other typeof today, but this keeps the contract TOTAL against future engines). + return 'null'; + } + } + const seenSet = seen ?? new Set(); + if (seenSet.has(value)) return '"[Circular]"'; + try { + seenSet.add(value); + if (Array.isArray(value)) { + return `[${value.map((v) => stableJson(v, seenSet)).join(',')}]`; + } + const obj = value as Record; + const keys = Object.keys(obj).sort(); + return `{${keys.map((k) => `${JSON.stringify(k)}:${stableJson(obj[k], seenSet)}`).join(',')}}`; + } catch { + // A Proxy with a throwing trap, or a getter that throws on read — never propagate (matrix C5). + return '"[unserializable]"'; + } finally { + seenSet.delete(value); + } } function disclosureSignature(d: Disclosure): string { @@ -607,7 +1058,25 @@ function disclosureSignature(d: Disclosure): string { ]), ) .sort(); - return JSON.stringify([hooks, mods, mcp]); + // ADR-2782 D5 (#2796): fold in slug/transport/binary/rawArgs/hostConfigKey/promptChannel/handler — + // every field that changes WHAT runs, WHERE it sends data, or WHAT CODE post-processes its output + // (matrix A3–A9). Deliberately ABSENT from this line: `reviewsSection` and `timeoutFloorMs` (matrix + // A10/A13 — cosmetic fields; folding them in would force a re-consent prompt that carries no + // security information, training users to click through) and the RESOLVED host (design constraint + // 2 — the loader has no config resolver and must compute the SAME signature as the lifecycle, or a + // resolver-bearing caller and a resolver-less caller would permanently disagree on one manifest's + // signature). + const lanes = d.reviewerLanes + .map((l) => + stableJson(['lane', l.slug, l.transport, l.binary, l.rawArgs || [], l.hostConfigKey, l.promptChannel, l.handler]), + ) + .sort(); + // D4.5 (the highest-consequence line in this phase): the lane element is appended ONLY when at + // least one lane is declared. A lane-free manifest's signature stays BYTE-IDENTICAL to before this + // class existed (matrix A1a/A1b/A1c) — appending unconditionally would change every already- + // installed capability's signature and re-prompt every user for every capability on next upgrade, + // whether or not they use any reviewer lane at all. + return lanes.length > 0 ? JSON.stringify([hooks, mods, mcp, lanes]) : JSON.stringify([hooks, mods, mcp]); } /** @@ -698,6 +1167,34 @@ function summarizeDisclosure(disclosure: Disclosure): string[] { if (s.cwd) lines.push(` cwd: ${s.cwd}`); } } + if (disclosure.reviewerLanes.length > 0) { + lines.push(` reviewer lane (${disclosure.reviewerLanes.length}): an external reviewer receives plan/review data on every run`); + for (const l of disclosure.reviewerLanes) { + // B1/B2/B3/B4: disclose binary+args for a spawn lane, or hostConfigKey+resolved destination + // (flagged local when applicable, never omitted as "safe") for an openai-http lane — never + // curl/the transport name alone, which would be true and useless (design B2). + // Branch on the DECLARED SHAPE, not on an exact transport string. A lane + // whose transport is mis-cased or unrecognised still has a hostConfigKey, + // and falling through to the spawn branch would print "(no binary + // declared)" for a lane that in fact egresses to a live remote host — + // understating the disclosure precisely when it matters. Disclosure runs + // BEFORE validation, so a non-canonical transport does reach this code. + if (l.transport === 'openai-http' || (!l.binary && l.hostConfigKey)) { + const localTag = l.isLocalDestination ? ' [local]' : ''; + lines.push(` - ${l.slug || '(slug?)'} -> [openai-http] ${l.hostConfigKey || '(hostConfigKey?)'} => ${l.resolvedHost}${localTag}`); + } else { + // Render the RAW declared args, not the string-filtered view. The raw + // array is what the host receives and what the consent signature binds, + // so a non-string member that is invisible here is a surface the user + // consented to without being shown — the opposite of the disclosure's + // whole purpose. + const cmd = [l.binary, ...l.rawArgs.map(renderArgForHuman)].filter(Boolean).join(' '); + lines.push(` - ${l.slug || '(slug?)'} -> ${cmd || '(no binary declared)'}`); + } + if (l.handler) lines.push(` handler: ${l.handler}`); + lines.push(` sends: ${l.egressPayloadClasses.join(', ')}`); + } + } if (disclosure.missingArtifacts.length > 0) { lines.push(' WARNING — declared artifacts not found in the staged bundle:'); for (const a of disclosure.missingArtifacts) { @@ -714,6 +1211,9 @@ function summarizeDisclosure(disclosure: Disclosure): string[] { export = { RESERVED_NAMESPACES, discloseExecutableSurfaces, + // #2796: the reviewer-lane collector, exported for independent testability (ADR-2782's own + // argument for extracting per-class collectors rather than growing the switch inline). + collectReviewerLaneSurfaces, checkReservedNamespace, evaluateSourceAllowed, checkEngines, @@ -723,4 +1223,8 @@ export = { // #1459: the consent-binding signature (single source of truth for loader + lifecycle consent). disclosureSignature, signatureForManifest, + // #2796: the non-blank unresolved-host marker and the named egress payload classes, exported so + // tests can assert exact equality rather than a loose substring match. + UNRESOLVED_HOST_MARKER, + EGRESS_PAYLOAD_CLASSES, }; diff --git a/tests/reviewer-trust-disclosure.test.cjs b/tests/reviewer-trust-disclosure.test.cjs new file mode 100644 index 000000000..7b3407fff --- /dev/null +++ b/tests/reviewer-trust-disclosure.test.cjs @@ -0,0 +1,1030 @@ +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * reviewer-trust-disclosure.test.cjs — behavioral tests for the reviewer lane as a FOURTH + * executable-surface class inside the capability trust gate (ADR-2782, chore #2796 Phase 3): + * `discloseExecutableSurfaces`, `collectReviewerLaneSurfaces`, `disclosureSignature`, + * `executableSetChanged`, `signatureForManifest`, and `evaluateInstallTrust`'s consent-gating rows. + * + * Implements every row in `.gsd/phase/chore-2796-reviewer-trust-disclosure/50-test-matrix.md` that + * carries a Test name (sections A-E). Test names are copied verbatim from the matrix. See + * `.gsd/phase/chore-2796-reviewer-trust-disclosure/40-design.md` for the behavior table the matrix + * derives from. + * + * Level choice: rows describing WHAT a single lane discloses (B1-B8, C1-C6) call + * `collectReviewerLaneSurfaces` or `discloseExecutableSurfaces` directly — the cheapest unit that + * proves the behavior, per `tests/reviewer-manifest-body.test.cjs`'s own established idiom. Rows + * about signature/re-consent behavior (A3-A10, A13) go through `discloseExecutableSurfaces` + + * `executableSetChanged`. A11/A12/E3 construct a `Disclosure` object DIRECTLY and call + * `disclosureSignature` on it: a real capability manifest carries AT MOST ONE `reviewer` body + * (Phase 2's own validator rejects an array — matrix C2b), so "two lanes" can only be exercised at + * the `disclosureSignature`-direct level, exactly as that exported function's own contract permits + * (it operates on a `Disclosure`, not a manifest). D1-D4 exercise `evaluateInstallTrust` + + * `executableSetChanged` — the real caller shape for the consent-gating rows. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const fc = require('fast-check'); + +const { cleanup } = require('./helpers.cjs'); + +const trust = require('../gsd-core/bin/lib/capability-trust.cjs'); + +// ─── Fixture builders ────────────────────────────────────────────────────── +// House convention (tests/reviewer-manifest-body.test.cjs): builder functions return a VALID +// fixture; an optional `mutator` callback is applied to the FRESH object before it is returned. +// Every call builds a brand-new object — no module-level mutable shared state. + +/** A valid `role:'reviewer'` capability manifest carrying a well-formed `spawn`-transport lane. */ +function spawnLaneManifest(mutator) { + const manifest = { + id: 'test-cap', + role: 'reviewer', + title: 'Test Capability', + description: 'A test capability for the reviewer trust-disclosure test suite.', + tier: 'standard', + requires: [], + version: '1.0.0', + reviewer: { + slug: 'my-lane', + flags: ['--my-lane'], + transport: 'spawn', + probe: { kind: 'command-exists', binary: 'my-lane' }, + invoke: { + binary: 'my-lane', + args: ['--json'], + promptChannel: 'stdin', + outputChannel: 'stdout', + modelArg: null, + effortChannel: 'none', + }, + timeoutFloorMs: 5000, + emptyOutput: 'stub-with-stderr', + reviewsSection: 'My Lane', + evidenceClass: 'source-grounded', + requiresBinaries: ['my-lane'], + promptBudgetKey: null, + handler: null, + }, + }; + if (mutator) mutator(manifest); + return manifest; +} + +/** A valid `role:'reviewer'` capability manifest carrying a well-formed `openai-http`-transport lane. */ +function httpLaneManifest(mutator) { + const manifest = { + id: 'test-cap-http', + role: 'reviewer', + title: 'Test Capability (HTTP)', + description: 'A test capability (openai-http lane) for the reviewer trust-disclosure test suite.', + tier: 'standard', + requires: [], + version: '1.0.0', + reviewer: { + slug: 'lm-studio-http', + flags: ['--lm-studio'], + transport: 'openai-http', + probe: { + kind: 'http-reachable', + hostConfigKey: 'lmStudio.baseUrl', + path: '/v1/models', + timeoutMs: 2000, + }, + invoke: { + hostConfigKey: 'lmStudio.baseUrl', + path: '/v1/chat/completions', + modelDiscovery: 'none', + effortChannel: 'none', + }, + timeoutFloorMs: 5000, + emptyOutput: 'stub-with-stderr', + reviewsSection: 'LM Studio', + evidenceClass: 'source-grounded', + requiresBinaries: [], + promptBudgetKey: null, + handler: null, + }, + }; + if (mutator) mutator(manifest); + return manifest; +} + +/** A lane-free manifest carrying one hook, one command module, and one mcpServer. */ +function laneFreeManifestWithSurfaces() { + return { + id: 'x', + hooks: [{ event: 'PostToolUse', script: 'hooks/x.js' }], + commands: [{ family: 'demo', module: 'demo.cjs', router: 'run' }], + mcpServers: { srv: { command: 'node', args: ['s.js'], env: { A: '1' } } }, + }; +} + +/** A hand-built `ReviewerLaneSurface`, mutated in place by `overrides` — used ONLY to exercise + * `disclosureSignature` directly for a synthetic multi-lane `Disclosure` (see A11/A12/E3). */ +function laneSurfaceFixture(overrides) { + return Object.assign( + { + slug: 'a', + transport: 'spawn', + binary: 'x', + args: [], + rawArgs: [], + hostConfigKey: '', + resolvedHost: '', + isLocalDestination: false, + promptChannel: 'stdin', + handler: '', + egressPayloadClasses: [...trust.EGRESS_PAYLOAD_CLASSES], + }, + overrides || {}, + ); +} + +/** A synthetic `Disclosure` carrying only the given `reviewerLanes` — the only way to exercise + * `disclosureSignature`'s reordering/sort behavior with 2+ lanes, since a real manifest is capped + * at one `reviewer` body. */ +function disclosureWithLanes(lanes) { + return { hooks: [], commandModules: [], mcpServers: [], reviewerLanes: lanes, hasExecutable: true, missingArtifacts: [] }; +} + +const LOCAL_SPEC = { kind: 'local', raw: '.', target: '.' }; + +// The measured PRE-#2796 goldens (task brief) — byte-for-byte, not recomputed. +const GOLDEN_BARE_SIGNATURE = '[[],[],[]]'; +const GOLDEN_HOOKS_MODS_MCP_SIGNATURE = + '[["[\\"hook\\",\\"PostToolUse\\",\\"hooks/x.js\\"]"],["[\\"mod\\",\\"demo\\",\\"demo.cjs\\",\\"run\\"]"],' + + '["[\\"mcp\\",\\"srv\\",\\"\\",\\"node\\",[\\"s.js\\"],\\"\\",{},{\\"A\\":\\"1\\"},\\"\\",' + + '{\\"args\\":[\\"s.js\\"],\\"command\\":\\"node\\",\\"env\\":{\\"A\\":\\"1\\"}}]"]]'; + +// ─── A. Signature stability — the absent-safe invariant (highest consequence) ────────────── + +describe('A. Signature stability — the absent-safe invariant', () => { + test('laneFreeManifestSignatureIsByteIdentical', () => { + // Regression tripwire: if the lane element were appended unconditionally, both assertions fail. + assert.equal( + trust.signatureForManifest({ id: 'bare', role: 'feature', version: '1.0.0' }), + GOLDEN_BARE_SIGNATURE, + ); + assert.equal(trust.signatureForManifest(laneFreeManifestWithSurfaces()), GOLDEN_HOOKS_MODS_MCP_SIGNATURE); + }); + + test('laneFreeSignatureIsIndependentOfStagedDir', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-trust-lane-a1b-')); + try { + // stagedDir is EMPTY — every declared hook/module artifact is "missing" — yet the signature is + // over the executable SET, not missingArtifacts (TV-09's own precedent), so it must be + // identical to the no-stagedDir call. + assert.equal( + trust.signatureForManifest(laneFreeManifestWithSurfaces(), dir), + GOLDEN_HOOKS_MODS_MCP_SIGNATURE, + ); + } finally { + cleanup(dir); + } + }); + + test('laneFreeSignatureKeepsThreeElements', () => { + const parsed = JSON.parse(trust.signatureForManifest(laneFreeManifestWithSurfaces())); + assert.equal(parsed.length, 3, `expected exactly 3 elements (no lane element), got ${parsed.length}`); + }); + + test('laneBearingManifestSignatureGainsLaneElement', () => { + const parsed = JSON.parse(trust.signatureForManifest(spawnLaneManifest())); + assert.equal(parsed.length, 4, `expected 4 elements once a lane is declared, got ${parsed.length}`); + assert.equal(parsed[3].length, 1); + }); + + test('addingALaneForcesReconsent', () => { + const before = trust.discloseExecutableSurfaces({ id: 'x' }); + const after = trust.discloseExecutableSurfaces(spawnLaneManifest()); + assert.equal(trust.executableSetChanged(before, after), true); + }); + + test('removingALaneForcesReconsent', () => { + const before = trust.discloseExecutableSurfaces(spawnLaneManifest()); + const after = trust.discloseExecutableSurfaces({ id: 'x' }); + assert.equal(trust.executableSetChanged(before, after), true); + }); + + test('changingLaneBinaryForcesReconsent', () => { + const before = trust.discloseExecutableSurfaces(spawnLaneManifest()); + const after = trust.discloseExecutableSurfaces( + spawnLaneManifest((m) => { + m.reviewer.invoke.binary = 'other-binary'; + }), + ); + assert.equal(trust.executableSetChanged(before, after), true); + }); + + test('changingLaneArgsForcesReconsentEvenWhenBinaryIsUnchanged', () => { + // #1459's bug class: `python3` with innocuous args later becoming `['-c', '']`. + const beforeManifest = spawnLaneManifest((m) => { + m.reviewer.invoke.binary = 'python3'; + m.reviewer.invoke.args = ['--version']; + }); + const afterManifest = spawnLaneManifest((m) => { + m.reviewer.invoke.binary = 'python3'; + m.reviewer.invoke.args = ['-c', '']; + }); + assert.equal(beforeManifest.reviewer.invoke.binary, afterManifest.reviewer.invoke.binary, 'binary must be identical — only args changed'); + const before = trust.discloseExecutableSurfaces(beforeManifest); + const after = trust.discloseExecutableSurfaces(afterManifest); + assert.equal(trust.executableSetChanged(before, after), true); + }); + + test('changingHostConfigKeyForcesReconsent', () => { + const before = trust.discloseExecutableSurfaces(httpLaneManifest()); + const after = trust.discloseExecutableSurfaces( + httpLaneManifest((m) => { + m.reviewer.invoke.hostConfigKey = 'otherHost.baseUrl'; + }), + ); + assert.equal(trust.executableSetChanged(before, after), true); + }); + + test('changingPromptChannelForcesReconsent', () => { + const before = trust.discloseExecutableSurfaces(spawnLaneManifest()); + const after = trust.discloseExecutableSurfaces( + spawnLaneManifest((m) => { + m.reviewer.invoke.promptChannel = 'argv'; + }), + ); + assert.equal(trust.executableSetChanged(before, after), true, 'promptChannel changes WHAT is sent to the lane'); + }); + + test('changingHandlerForcesReconsent', () => { + const before = trust.discloseExecutableSurfaces(spawnLaneManifest()); + const after = trust.discloseExecutableSurfaces( + spawnLaneManifest((m) => { + m.reviewer.handler = 'antigravity'; + }), + ); + assert.equal(trust.executableSetChanged(before, after), true, 'handler changes what CODE post-processes the output'); + }); + + test('changingReviewsSectionDoesNotForceReconsent', () => { + const before = trust.discloseExecutableSurfaces(spawnLaneManifest()); + const after = trust.discloseExecutableSurfaces( + spawnLaneManifest((m) => { + m.reviewer.reviewsSection = 'Totally Different Section'; + }), + ); + assert.equal( + trust.executableSetChanged(before, after), + false, + 'reviewsSection is cosmetic — folding it into the signature would train users to click through a false re-prompt', + ); + }); + + test('reorderingLanesDoesNotForceReconsent', () => { + // A real manifest carries at most ONE reviewer body (Phase 2 rejects an array), so "two lanes + // reordered" is exercised by calling disclosureSignature directly on a hand-built Disclosure — + // exactly the exported function's own documented unit (it operates on a Disclosure, not a + // manifest). + const laneA = laneSurfaceFixture({ slug: 'a', binary: 'binary-a' }); + const laneB = laneSurfaceFixture({ slug: 'b', binary: 'binary-b' }); + const forward = trust.disclosureSignature(disclosureWithLanes([laneA, laneB])); + const reversed = trust.disclosureSignature(disclosureWithLanes([laneB, laneA])); + assert.equal(forward, reversed, 'reviewerLanes must be sorted before folding, matching the existing hook/mod/mcp treatment'); + }); + + test('laneKeyReorderDoesNotForceReconsent', () => { + const before = spawnLaneManifest((m) => { + m.reviewer.invoke.args = [{ a: 1, b: 2 }]; + }); + const after = spawnLaneManifest((m) => { + m.reviewer.invoke.args = [{ b: 2, a: 1 }]; + }); + assert.equal( + trust.signatureForManifest(before), + trust.signatureForManifest(after), + 'stableJson recursively sorts keys — a pure key reorder inside a declared arg object must not force re-consent', + ); + }); + + test('changingTimeoutFloorDoesNotForceReconsent', () => { + const before = trust.discloseExecutableSurfaces(spawnLaneManifest()); + const after = trust.discloseExecutableSurfaces( + spawnLaneManifest((m) => { + m.reviewer.timeoutFloorMs = 999999; + }), + ); + assert.equal(trust.executableSetChanged(before, after), false, 'timeoutFloorMs is not an executable-surface property'); + }); +}); + +// ─── B. What is disclosed ─────────────────────────────────────────────────── + +describe('B. What is disclosed', () => { + test('spawnLaneDisclosesBinaryAndArgs', () => { + const manifest = spawnLaneManifest((m) => { + m.reviewer.invoke.args = ['--json', '--verbose']; + }); + const [surface] = trust.collectReviewerLaneSurfaces(manifest); + assert.equal(surface.binary, 'my-lane'); + assert.deepEqual(surface.args, ['--json', '--verbose']); + assert.deepEqual(surface.rawArgs, ['--json', '--verbose']); + }); + + test('spawnLaneDisclosesRawArgsIncludingNonStrings', () => { + const nested = { mode: 'fast' }; + const manifest = spawnLaneManifest((m) => { + m.reviewer.invoke.args = ['-p', 7, nested, true]; + }); + const [surface] = trust.collectReviewerLaneSurfaces(manifest); + assert.deepEqual( + surface.rawArgs, + ['-p', 7, nested, true], + 'the FULL declared array — the host receives every member, string or not', + ); + assert.deepEqual( + surface.args, + ['-p'], + 'the rendered (human-summary) form is string-filtered, mirroring MCP argv/rawArgs', + ); + }); + + test('httpLaneDisclosesResolvedHostAndConfigKey', () => { + const manifest = httpLaneManifest(); + const [surface] = trust.collectReviewerLaneSurfaces(manifest, (key) => + key === 'lmStudio.baseUrl' ? 'http://192.168.1.50:1234' : undefined, + ); + assert.equal(surface.hostConfigKey, 'lmStudio.baseUrl'); + assert.equal(surface.resolvedHost, 'http://192.168.1.50:1234'); + }); + + test('httpLaneWithoutResolverMarksHostUnresolved', () => { + const manifest = httpLaneManifest(); + const [surface] = trust.collectReviewerLaneSurfaces(manifest); + assert.equal(surface.hostConfigKey, 'lmStudio.baseUrl'); + assert.equal(surface.resolvedHost, trust.UNRESOLVED_HOST_MARKER); + assert.notEqual(surface.resolvedHost, '', 'a blank reads as "no destination" — must be the explicit marker instead'); + }); + + test('localhostDestinationIsStillDisclosed', () => { + const manifest = httpLaneManifest(); + const [localSurface] = trust.collectReviewerLaneSurfaces(manifest, () => 'http://localhost:8080'); + assert.equal(localSurface.resolvedHost, 'http://localhost:8080', 'localhost is disclosed like any other destination, never omitted'); + assert.equal(localSurface.isLocalDestination, true); + + const [remoteSurface] = trust.collectReviewerLaneSurfaces(manifest, () => 'https://api.example.com'); + assert.equal(remoteSurface.resolvedHost, 'https://api.example.com'); + assert.equal(remoteSurface.isLocalDestination, false, 'a remote destination must be distinguished from a local one'); + + // Regression: `new URL('localhost:1234')` does NOT throw — it mis-parses the scheme-less + // "host:port" shape as an opaque URL whose "scheme" IS the hostname text (protocol: + // "localhost:", hostname: ""), which would silently fail to flag a bare local config value + // (no `http://` prefix) as local. `192.168.1.5:8080` exercises the ordinary (throws-then-falls- + // back) path for a non-local scheme-less value, so it must stay false. + const [schemelessLocal] = trust.collectReviewerLaneSurfaces(manifest, () => 'localhost:1234'); + assert.equal(schemelessLocal.isLocalDestination, true, 'a scheme-less "localhost:port" config value must still be recognized as local'); + + const [schemelessRemote] = trust.collectReviewerLaneSurfaces(manifest, () => '192.168.1.5:8080'); + assert.equal(schemelessRemote.isLocalDestination, false, 'a scheme-less non-loopback host:port must not be misflagged as local'); + + const [loopbackIp] = trust.collectReviewerLaneSurfaces(manifest, () => '127.0.0.1:1234'); + assert.equal(loopbackIp.isLocalDestination, true, 'a scheme-less 127.x.x.x:port must be recognized as local'); + }); + + test('laneDisclosesEgressPayloadClasses', () => { + const [surface] = trust.collectReviewerLaneSurfaces(spawnLaneManifest()); + assert.deepEqual(surface.egressPayloadClasses, trust.EGRESS_PAYLOAD_CLASSES); + assert.notStrictEqual( + surface.egressPayloadClasses, + trust.EGRESS_PAYLOAD_CLASSES, + 'must be a fresh copy per surface, not the shared constant reference', + ); + for (const term of ['plan text', 'requirements', 'research findings', 'CONTEXT.md decisions']) { + assert.ok(surface.egressPayloadClasses.includes(term), `expected "${term}" among the disclosed egress classes`); + } + }); + + test('laneDisclosesHandlerName', () => { + const manifest = spawnLaneManifest((m) => { + m.reviewer.handler = 'antigravity'; + }); + const [surface] = trust.collectReviewerLaneSurfaces(manifest); + assert.equal(surface.handler, 'antigravity'); + }); + + test('laneOnlyCapabilityRequiresConsent', () => { + const manifest = spawnLaneManifest(); + const d = trust.discloseExecutableSurfaces(manifest); + assert.equal(d.hasExecutable, true); + const verdict = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest, hostVersion: '1.0.0' }); + assert.equal(verdict.requiresConsent, true); + }); + + test('httpLaneRequiresConsentDespiteSpawningNothing', () => { + const manifest = httpLaneManifest(); + assert.equal(manifest.reviewer.invoke.binary, undefined, 'an openai-http lane declares no binary at all'); + const d = trust.discloseExecutableSurfaces(manifest); + assert.equal(d.reviewerLanes[0].binary, '', 'nothing spawns for this lane'); + assert.equal(d.hasExecutable, true, 'a standing egress channel still requires consent even though no process starts'); + }); + + test('laneAndHookSurfacesCoexist', () => { + const manifest = spawnLaneManifest((m) => { + m.hooks = [{ event: 'PostToolUse', script: 'hooks/check.js' }]; + }); + const d = trust.discloseExecutableSurfaces(manifest); + assert.equal(d.hooks.length, 1); + assert.deepEqual(d.hooks[0], { event: 'PostToolUse', script: 'hooks/check.js' }); + assert.equal(d.reviewerLanes.length, 1); + assert.equal(d.reviewerLanes[0].slug, 'my-lane'); + assert.equal(d.hasExecutable, true); + }); +}); + +// ─── C. Totality / malformed — disclosure runs BEFORE validation ─────────── + +describe('C. Totality / malformed', () => { + test('absentReviewerBodyDisclosesNoLane', () => { + assert.doesNotThrow(() => trust.discloseExecutableSurfaces({ id: 'x', role: 'feature' })); + const d = trust.discloseExecutableSurfaces({ id: 'x', role: 'feature' }); + assert.deepEqual(d.reviewerLanes, []); + }); + + test('nullReviewerBodyDisclosesNothing', () => { + assert.doesNotThrow(() => trust.discloseExecutableSurfaces({ id: 'x', reviewer: null })); + const d = trust.discloseExecutableSurfaces({ id: 'x', reviewer: null }); + assert.deepEqual(d.reviewerLanes, []); + }); + + test('nonObjectReviewerBodyDisclosesNothing', () => { + for (const badReviewer of [[], true, 0]) { + assert.doesNotThrow(() => trust.discloseExecutableSurfaces({ id: 'x', reviewer: badReviewer })); + const d = trust.discloseExecutableSurfaces({ id: 'x', reviewer: badReviewer }); + assert.deepEqual(d.reviewerLanes, [], `reviewer=${JSON.stringify(badReviewer)} must disclose no lane`); + } + }); + + test('malformedInvokeStillDisclosesWithoutThrowing', () => { + for (const badInvoke of [undefined, null, 'garbage', 42, []]) { + const manifest = spawnLaneManifest((m) => { + if (badInvoke === undefined) delete m.reviewer.invoke; + else m.reviewer.invoke = badInvoke; + }); + assert.doesNotThrow(() => trust.discloseExecutableSurfaces(manifest)); + const d = trust.discloseExecutableSurfaces(manifest); + assert.equal(d.reviewerLanes.length, 1, `invoke=${JSON.stringify(badInvoke)} must still disclose the lane`); + assert.equal(d.reviewerLanes[0].binary, ''); + assert.deepEqual(d.reviewerLanes[0].args, []); + assert.deepEqual(d.reviewerLanes[0].rawArgs, []); + } + }); + + test('nonArrayFlagsAreIgnoredByDisclosure', () => { + const manifest = spawnLaneManifest((m) => { + m.reviewer.flags = 'not-an-array'; + }); + assert.doesNotThrow(() => trust.discloseExecutableSurfaces(manifest)); + const d = trust.discloseExecutableSurfaces(manifest); + assert.equal(d.reviewerLanes.length, 1); + assert.equal(d.reviewerLanes[0].slug, 'my-lane', 'a malformed flags field must not disturb the rest of the disclosed lane'); + }); + + test('proxyManifestDoesNotBreakDisclosure', () => { + const proxyManifest = new Proxy( + {}, + { + get() { + throw new Error('boom: get trap'); + }, + has() { + throw new Error('boom: has trap'); + }, + ownKeys() { + throw new Error('boom: ownKeys trap'); + }, + }, + ); + assert.doesNotThrow(() => trust.discloseExecutableSurfaces(proxyManifest)); + const d = trust.discloseExecutableSurfaces(proxyManifest); + assert.equal(d.hasExecutable, false); + assert.deepEqual(d.reviewerLanes, []); + }); + + test('throwingGetterDoesNotBreakDisclosure', () => { + const manifest = { id: 'x' }; + Object.defineProperty(manifest, 'reviewer', { + enumerable: true, + get() { + throw new Error('boom: throwing getter'); + }, + }); + assert.doesNotThrow(() => trust.discloseExecutableSurfaces(manifest)); + const d = trust.discloseExecutableSurfaces(manifest); + assert.deepEqual(d.reviewerLanes, []); + }); + + test('circularArgValueDoesNotBreakSignature', () => { + const circular = {}; + circular.self = circular; + const manifest = spawnLaneManifest((m) => { + m.reviewer.invoke.args = [circular]; + }); + assert.doesNotThrow(() => trust.signatureForManifest(manifest)); + const sig = trust.signatureForManifest(manifest); + assert.equal(typeof sig, 'string'); + assert.ok(sig.length > 0); + }); + + test('laneBinaryIsNotTreatedAsAMissingBundleArtifact', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-trust-lane-c6-')); + try { + const manifest = spawnLaneManifest(); + const d = trust.discloseExecutableSurfaces(manifest, dir); + assert.deepEqual( + d.missingArtifacts, + [], + 'a lane binary is a PATH tool, never a bundle artifact — existence-checking it would block every lane install', + ); + } finally { + cleanup(dir); + } + }); +}); + +// ─── D. Consent flow — evaluateInstallTrust ──────────────────────────────── + +describe('D. Consent flow — evaluateInstallTrust', () => { + test('laneBearingInstallRequiresConsentBeforePromotion', () => { + const manifest = spawnLaneManifest(); + const verdict = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest, hostVersion: '1.0.0' }); + assert.equal(verdict.allowed, true); + assert.equal( + verdict.requiresConsent, + true, + 'evaluateInstallTrust is the PURE verdict the (untouched) lifecycle gates promotion on — a ' + + 'true requiresConsent is what forces the consent prompt before any file is promoted', + ); + assert.equal(verdict.disclosure.reviewerLanes.length, 1); + const lines = trust.summarizeDisclosure(verdict.disclosure); + const joined = lines.join('\n'); + assert.match(joined, /my-lane/, 'the human-facing prompt names the lane/binary'); + assert.match(joined, /plan text/, 'the human-facing prompt names the egress payload classes'); + }); + + test('decliningLaneConsentWritesNothing', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-trust-lane-d2-')); + try { + fs.writeFileSync(path.join(dir, 'pre-existing.txt'), 'unchanged'); + const before = fs.readdirSync(dir, { recursive: true }).sort(); + const manifest = spawnLaneManifest(); + const verdict = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest, stagedDir: dir, hostVersion: '1.0.0' }); + assert.equal(verdict.requiresConsent, true); + // evaluateInstallTrust never writes, regardless of the (not-yet-made) consent decision — this + // purity is exactly what makes "decline -> nothing written" true upstream: nothing was ever + // written by computing the verdict in the first place, so a decline has nothing to undo. + const after = fs.readdirSync(dir, { recursive: true }).sort(); + assert.deepEqual(after, before, 'evaluateInstallTrust must perform no filesystem writes'); + } finally { + cleanup(dir); + } + }); + + test('upgradeWithChangedLaneRequiresReconsent', () => { + const v1 = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest: spawnLaneManifest(), hostVersion: '1.0.0' }); + const v2 = trust.evaluateInstallTrust({ + parsed: LOCAL_SPEC, + manifest: spawnLaneManifest((m) => { + m.reviewer.invoke.binary = 'my-lane-v2'; + }), + hostVersion: '1.0.0', + }); + assert.equal(trust.executableSetChanged(v1.disclosure, v2.disclosure), true); + }); + + test('upgradeWithUnchangedSurfacesDoesNotReprompt', () => { + const v1 = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest: spawnLaneManifest(), hostVersion: '1.0.0' }); + const v2 = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest: spawnLaneManifest(), hostVersion: '1.0.0' }); + assert.equal(trust.executableSetChanged(v1.disclosure, v2.disclosure), false); + }); +}); + +// ─── E. Property-based (fast-check) ──────────────────────────────────────── + +describe('E. Property-based (fast-check)', () => { + // A JS re-implementation of the PRE-#2796 discloseExecutableSurfaces (hooks/commands/mcpServers + // ONLY, no reviewer lanes) + disclosureSignature + stableJson, copied from src/capability-trust.cts + // as it stood before this phase. This is the "oracle" E1 checks the new signatureForManifest + // against for any lane-free manifest — the property-based generalization of the A1a golden-literal + // regression check. + function refAsString(v) { + return typeof v === 'string' ? v : ''; + } + + function refStableJson(value) { + if (value === null || typeof value !== 'object') return JSON.stringify(value) ?? 'null'; + if (Array.isArray(value)) return `[${value.map(refStableJson).join(',')}]`; + const keys = Object.keys(value).sort(); + return `{${keys.map((k) => `${JSON.stringify(k)}:${refStableJson(value[k])}`).join(',')}}`; + } + + function refDiscloseExecutableSurfaces(manifest) { + const hooks = []; + const commandModules = []; + const mcpServers = []; + + if (Array.isArray(manifest.hooks)) { + for (const h of manifest.hooks) { + if (typeof h !== 'object' || h === null) continue; + const script = refAsString(h['script']); + const event = refAsString(h['event']); + if (script) hooks.push({ event, script }); + } + } + + if (Array.isArray(manifest.commands)) { + for (const c of manifest.commands) { + if (typeof c !== 'object' || c === null) continue; + const moduleName = refAsString(c['module']); + const family = refAsString(c['family']); + const router = refAsString(c['router']); + if (moduleName) commandModules.push({ family, module: moduleName, router }); + } + } + + if (manifest.mcpServers && typeof manifest.mcpServers === 'object') { + const pushServer = (name, config) => { + if (!name) return; + const cfg = typeof config === 'object' && config !== null ? config : {}; + const command = refAsString(cfg['command']); + const rawArgs = Array.isArray(cfg['args']) ? cfg['args'] : []; + const argv = rawArgs.filter((a) => typeof a === 'string'); + const transport = refAsString(cfg['type']) || refAsString(cfg['transport']); + const url = refAsString(cfg['url']); + const headers = {}; + const rawHeaders = cfg['headers']; + if (rawHeaders && typeof rawHeaders === 'object' && !Array.isArray(rawHeaders)) { + for (const [k, v] of Object.entries(rawHeaders)) { + if (k === '__proto__' || k === 'constructor' || k === 'prototype') continue; + if (typeof v === 'string') headers[k] = v; + } + } + const env = {}; + const rawEnv = cfg['env']; + if (rawEnv && typeof rawEnv === 'object' && !Array.isArray(rawEnv)) { + for (const [k, v] of Object.entries(rawEnv)) { + if (k === '__proto__' || k === 'constructor' || k === 'prototype') continue; + if (typeof v === 'string') env[k] = v; + } + } + const cwd = refAsString(cfg['cwd']); + const rawConfig = {}; + for (const [k, v] of Object.entries(cfg)) { + if (k === '__proto__' || k === 'constructor' || k === 'prototype') continue; + rawConfig[k] = v; + } + const surface = { name, transport, command, argv, rawArgs, url, headers, env, rawConfig }; + if (cwd) surface.cwd = cwd; + mcpServers.push(surface); + }; + if (Array.isArray(manifest.mcpServers)) { + for (const s of manifest.mcpServers) { + if (typeof s === 'object' && s !== null) pushServer(refAsString(s['name']), s['config'] ?? s); + } + } else { + for (const [name, config] of Object.entries(manifest.mcpServers)) pushServer(name, config); + } + } + + return { hooks, commandModules, mcpServers }; + } + + function refDisclosureSignature(d) { + const hooks = d.hooks.map((h) => refStableJson(['hook', h.event, h.script])).sort(); + const mods = d.commandModules.map((m) => refStableJson(['mod', m.family, m.module, m.router || ''])).sort(); + const mcp = d.mcpServers + .map((s) => + refStableJson([ + 'mcp', + s.name, + s.transport || '', + s.command, + s.rawArgs || [], + s.url || '', + s.headers || {}, + s.env || {}, + s.cwd || '', + s.rawConfig || {}, + ]), + ) + .sort(); + return JSON.stringify([hooks, mods, mcp]); + } + + function referenceLaneFreeSignature(manifest) { + return refDisclosureSignature(refDiscloseExecutableSurfaces(manifest)); + } + + test('laneFreeSignatureIsUnchangedForArbitraryManifests', () => { + const stringArb = fc.string(); + const hookArb = fc.record({ event: stringArb, script: stringArb }, { requiredKeys: [] }); + const commandArb = fc.record({ family: stringArb, module: stringArb, router: stringArb }, { requiredKeys: [] }); + const mcpConfigArb = fc.record( + { + command: stringArb, + args: fc.array(fc.oneof(stringArb, fc.integer(), fc.boolean())), + transport: fc.constantFrom('', 'stdio', 'http', 'sse'), + url: stringArb, + headers: fc.dictionary(stringArb, stringArb), + env: fc.dictionary(stringArb, stringArb), + cwd: stringArb, + }, + { requiredKeys: [] }, + ); + const mcpArrayArb = fc.array(fc.record({ name: stringArb, config: mcpConfigArb }, { requiredKeys: [] })); + const mcpMapArb = fc.dictionary(stringArb, mcpConfigArb); + + // Field-targeted, not fc.anything() — no `reviewer` key is ever present, generalizing A1 across + // diverse hooks/commands/mcpServers shapes rather than testing a property no caller exercises. + const laneFreeManifestArb = fc.record( + { + id: stringArb, + hooks: fc.array(hookArb), + commands: fc.array(commandArb), + mcpServers: fc.oneof(mcpArrayArb, mcpMapArb), + }, + { requiredKeys: [] }, + ); + + fc.assert( + fc.property(laneFreeManifestArb, (manifest) => { + assert.equal(trust.signatureForManifest(manifest), referenceLaneFreeSignature(manifest)); + }), + ); + }); + + test('discloseExecutableSurfacesIsTotal', () => { + function makeCircularArgManifest() { + const circular = {}; + circular.self = circular; + return { id: 'x', reviewer: { slug: 'x', transport: 'spawn', invoke: { binary: 'x', args: [circular] } } }; + } + function makeThrowingGetterManifest() { + const m = { id: 'x' }; + Object.defineProperty(m, 'reviewer', { + enumerable: true, + get() { + throw new Error('boom: throwing getter'); + }, + }); + return m; + } + function makeThrowingProxyManifest() { + // Symbol-keyed access passes through to the real (empty) target: fast-check's OWN internal + // Value wrapper unconditionally probes every generated value via `fc.cloneMethod in value` + // (a Symbol-keyed `has` check) BEFORE this property's predicate ever runs — a `has` trap that + // throws for ANY key (including that internal probe) crashes fast-check's generation step + // itself, not the code under test. Only STRING-keyed access throws here, which is exactly + // what `discloseExecutableSurfaces` exercises (`manifest.hooks`, `manifest.reviewer`, ...) and + // matches the matrix's own wording for this row ("a Proxy with a throwing get"). The fuller + // get+has+ownKeys-all-throw Proxy is exercised directly (bypassing fast-check) in section C's + // `proxyManifestDoesNotBreakDisclosure`. + return new Proxy( + {}, + { + get(target, prop, receiver) { + if (typeof prop === 'symbol') return Reflect.get(target, prop, receiver); + throw new Error('boom: get trap'); + }, + }, + ); + } + function makeBigIntManifest() { + return { + id: 'x', + reviewer: { slug: 'x', transport: 'spawn', invoke: { binary: 'x', args: [10n, 'ok'] } }, + mcpServers: { srv: { command: 'node', args: [20n] } }, + }; + } + function makeNonObjectManifest(v) { + return v; + } + + // fast-check's default fc.anything() generator emits no BigInt, circular ref, getter, or Proxy — + // each hostile shape is enumerated explicitly and mixed with fc.anything() for baseline + // structural diversity (matrix's own stated limitation of the default generator). + const manifestArb = fc.oneof( + fc.anything(), + fc.constant(0).map(makeCircularArgManifest), + fc.constant(0).map(makeThrowingGetterManifest), + fc.constant(0).map(makeThrowingProxyManifest), + fc.constant(0).map(makeBigIntManifest), + fc.constantFrom(null, undefined, 42, 'not-an-object', true).map(makeNonObjectManifest), + ); + + fc.assert( + fc.property(manifestArb, (manifest) => { + assert.doesNotThrow(() => trust.discloseExecutableSurfaces(manifest)); + }), + ); + }); + + test('signatureIsInvariantUnderLaneAndKeyOrder', () => { + const stringArb = fc.string(); + const argObjectArb = fc.dictionary(fc.string({ minLength: 1, maxLength: 6 }), fc.oneof(stringArb, fc.integer()), { + maxKeys: 4, + }); + const laneArb = fc.record({ + slug: stringArb, + transport: fc.constantFrom('spawn', 'openai-http', ''), + binary: stringArb, + args: fc.array(stringArb), + rawArgs: fc.array(fc.oneof(stringArb, fc.integer(), argObjectArb)), + hostConfigKey: stringArb, + resolvedHost: stringArb, + isLocalDestination: fc.boolean(), + promptChannel: stringArb, + handler: stringArb, + egressPayloadClasses: fc.constant([]), + }); + const lanesArb = fc.array(laneArb, { maxLength: 5 }); + + fc.assert( + fc.property(lanesArb, (lanes) => { + const sigOriginal = trust.disclosureSignature(disclosureWithLanes(lanes)); + + const reordered = [...lanes].reverse(); + const sigReordered = trust.disclosureSignature(disclosureWithLanes(reordered)); + assert.equal(sigOriginal, sigReordered, 'lane order must not affect the signature'); + + const keyReordered = lanes.map((l) => ({ + ...l, + rawArgs: l.rawArgs.map((a) => + a && typeof a === 'object' && !Array.isArray(a) ? Object.fromEntries([...Object.entries(a)].reverse()) : a, + ), + })); + const sigKeyReordered = trust.disclosureSignature(disclosureWithLanes(keyReordered)); + assert.equal(sigOriginal, sigKeyReordered, 'key order within a rawArgs object must not affect the signature'); + }), + ); + }); +}); + +// ─── F. Isolated-security-review regressions (#2796) ───────────────────────── +// +// Every test below corresponds to a finding an independent adversarial reviewer +// REPRODUCED against the first cut of this phase. The 41 tests above all passed +// while these defects were live, which is the point: each row here exists +// because the matrix did not think to ask. +describe('F. Isolated-security-review regressions', () => { + const asText = (disclosure) => { + const summary = trust.summarizeDisclosure(disclosure); + return Array.isArray(summary) ? summary.join('\n') : String(summary); + }; + const httpLane = (extra) => ({ + id: 'x', + reviewer: Object.assign({ slug: 'l', transport: 'openai-http', invoke: { hostConfigKey: 'k' } }, extra || {}), + }); + + // Finding B (MEDIUM). Non-string argv members are folded into the consent + // SIGNATURE but were dropped from the human-facing text, because the summary + // rendered the string-filtered `args` rather than the raw declared array. The + // host still receives them — so the user consented to a surface never shown. + test('nonStringArgvMembersAreVisibleInTheConsentSummary', () => { + const manifest = { + id: 'x', + reviewer: { + slug: 'my-lane', + transport: 'spawn', + invoke: { + binary: 'my-lane', + args: ['--json', 7, { mode: 'exfiltrate-everything' }, true, '--safe-looking-flag'], + promptChannel: 'stdin', + }, + }, + }; + const text = asText(trust.discloseExecutableSurfaces(manifest)); + assert.ok(text.includes('exfiltrate-everything'), `object arg must be visible, got: ${text}`); + assert.ok(/\b7\b/.test(text), `numeric arg must be visible, got: ${text}`); + assert.ok(text.includes('true'), `boolean arg must be visible, got: ${text}`); + assert.ok(text.includes('--safe-looking-flag'), 'string args must still render'); + }); + + test('renderingHostileArgvMembersDoesNotThrow', () => { + const circular = {}; + circular.self = circular; + for (const hostile of [circular, 10n, Symbol('s'), () => {}, undefined, null]) { + const manifest = { + id: 'x', + reviewer: { slug: 'l', transport: 'spawn', invoke: { binary: 'b', args: [hostile] } }, + }; + let text; + assert.doesNotThrow(() => { text = asText(trust.discloseExecutableSurfaces(manifest)); }, + 'the consent summary must not throw for a hostile argv member'); + assert.ok(typeof text === 'string' && text.length > 0); + } + }); + + // Finding C (LOW). An empty reviewer body flipped hasExecutable true and + // perturbed the signature, producing a re-consent prompt whose only content was + // "(no binary declared)" — a prompt carrying no security information, which is + // the click-through-training harm this design refuses elsewhere. + test('emptyReviewerBodyIsNotALaneAndDoesNotPerturbTheSignature', () => { + const withEmpty = { id: 'x', role: 'reviewer', version: '1.0.0', reviewer: {} }; + const laneFree = { id: 'x', role: 'reviewer', version: '1.0.0' }; + const disclosure = trust.discloseExecutableSurfaces(withEmpty); + assert.deepEqual(disclosure.reviewerLanes, [], 'an empty body declares no lane'); + assert.equal(disclosure.hasExecutable, false, 'an empty body must not require consent'); + assert.equal( + trust.signatureForManifest(withEmpty), trust.signatureForManifest(laneFree), + 'an empty reviewer body must not change the consent signature', + ); + }); + + // The inverse of Finding C, and the more dangerous direction: the + // meaningfulness test must never let a real lane through unconsented. + test('aLaneDeclaringAnySingleFieldStillRequiresConsent', () => { + const singleFieldBodies = [ + { slug: 's' }, + { transport: 'spawn' }, + { handler: 'antigravity' }, + { invoke: { binary: 'b' } }, + { invoke: { hostConfigKey: 'review.x_host' } }, + { invoke: { promptChannel: 'stdin' } }, + { invoke: { args: ['--x'] } }, + ]; + for (const reviewer of singleFieldBodies) { + const disclosure = trust.discloseExecutableSurfaces({ id: 'x', reviewer }); + assert.equal( + disclosure.hasExecutable, true, + `a lane declaring ${JSON.stringify(reviewer)} must still require consent`, + ); + } + }); + + // Finding D (LOW-MEDIUM). Disclosure runs BEFORE validation, so a mis-cased or + // unrecognised transport reaches this code. Keying the summary on an exact + // string sent a lane that plainly declares a hostConfigKey down the spawn + // branch, printing "(no binary declared)" for a lane egressing to a live host. + test('nonCanonicalTransportStillDisclosesTheDestination', () => { + const text = asText(trust.discloseExecutableSurfaces( + httpLane({ transport: 'OpenAI-HTTP', invoke: { hostConfigKey: 'review.x_host' } }), + undefined, + () => 'http://remote.example', + )); + assert.ok(text.includes('review.x_host'), `hostConfigKey must be disclosed, got: ${text}`); + assert.ok(text.includes('remote.example'), `resolved destination must be disclosed, got: ${text}`); + assert.ok(!text.includes('no binary declared'), `must not claim there is no binary, got: ${text}`); + }); + + test('nonCanonicalTransportWithoutResolverShowsTheUnresolvedMarkerNotABlank', () => { + const lane = trust.discloseExecutableSurfaces( + httpLane({ transport: 'OpenAI-HTTP', invoke: { hostConfigKey: 'review.x_host' } }), + ).reviewerLanes[0]; + assert.notEqual(lane.resolvedHost, '', 'a blank destination reads as "no destination"'); + assert.equal(lane.resolvedHost, trust.UNRESOLVED_HOST_MARKER); + }); + + test('spawnLaneKeepsDestinationFieldsInapplicable', () => { + const lane = trust.discloseExecutableSurfaces({ + id: 'x', reviewer: { slug: 's', transport: 'spawn', invoke: { binary: 'b' } }, + }).reviewerLanes[0]; + assert.equal(lane.resolvedHost, '', 'a spawn lane has no destination concept'); + assert.equal(lane.isLocalDestination, false); + }); + + // Finding F (MEDIUM). The [local] flag is design-load-bearing (B4), and it was + // dropped for every loopback form except the dotted quad and the bare hostname: + // bracketed IPv6 was mangled by splitting on its own colons, and legacy IPv4 + // encodings were not recognised at all. A browser, curl and the OS resolver all + // accept every one of these as 127.0.0.0/8. + test('everyLoopbackEncodingIsFlaggedLocal', () => { + const loopbacks = [ + 'localhost', 'localhost:1234', 'http://localhost:8080', + '127.0.0.1', '127.0.0.1:1234', 'http://127.0.0.1:11434', + '127.1', '127.0.1', '2130706433', '0x7f000001', '0177.0.0.1', + '::1', '[::1]', '[::1]:8080', '::ffff:127.0.0.1', + ]; + for (const host of loopbacks) { + const lane = trust.discloseExecutableSurfaces(httpLane(), undefined, () => host).reviewerLanes[0]; + assert.equal(lane.isLocalDestination, true, `"${host}" is loopback and must be flagged local`); + } + }); + + // The dangerous direction: a REMOTE host must never be mislabelled local, which + // would understate the disclosure. Includes the userinfo-@ and fragment tricks + // that make a remote host superficially resemble a local one. + test('remoteHostsAreNeverMislabelledLocal', () => { + const remotes = [ + 'localhost.evil.com', 'notlocalhost.example', 'evil-localhost', + 'http://evil.com#localhost', 'http://evil.com?x=localhost', + 'http://user@localhost@evil.com', 'http://localhost@evil.com', + 'http://127.0.0.1.evil.com', '192.168.1.5:8080', '8.8.8.8', + '128.0.0.1', '126.255.255.255', + ]; + for (const host of remotes) { + const lane = trust.discloseExecutableSurfaces(httpLane(), undefined, () => host).reviewerLanes[0]; + assert.equal(lane.isLocalDestination, false, `"${host}" is REMOTE and must not be flagged local`); + } + }); +});