* test(#3248): failing-first suite for instruction-surface disclosure 28 matrix rows from 50-test-matrix.md. Rows requiring the new Disclosure.instructionSurfaces field fail today; rows 18-20/23-25 (the ADR-2363 D4 signature invariants) pass today by construction because the current code never reads skills/agents at all, and stand as regression guards for the implementation commit. Refs #3248 * feat(#3248): disclose capability skills and agents as an instruction surface ADR-2363 D5. A capability whose only contribution was skills disclosed nothing at install: summarizeDisclosure early-returned "ships no executable surfaces (declarative only)" because hasExecutable was false, while each SKILL.md body landed verbatim in the agent's instruction context. discloseExecutableSurfaces gains a fifth, NON-executable class, instructionSurfaces, collecting declared skills/agents stems through the same safeCollect wrapper as the four existing collectors, so a hostile value degrades only this class and the function stays total for any manifest shape. Nothing existing is edited: the collectors, hasExecutable, disclosureSignature and missingArtifacts are untouched. get_impact rates the symbol CRITICAL at 196 affected, which is why the design is strictly additive. D4 is implemented by omission and pinned rather than left incidental: adding, changing or removing skills/agents leaves disclosureSignature byte-identical, so no stored consent record is perturbed and no spurious re-consent fires. ADR-2782's conditional-append trick is deliberately NOT reused - it worked because no manifest could declare a reviewer body before that class existed, whereas skills predate this one, so a conditional append would re-sign every already-consented skill-bearing capability. The renderer is extracted as summarizeInstructionSurfaces and called from BOTH branches of summarizeDisclosure. Appending only at the end would never render for skill-only capabilities - the ones that need it - since those take the early return. That branch's "declarative only" claim is now conditional on there being no instruction surface either. The renderer iterates rather than spreading into push, so an unbounded stem count cannot throw RangeError, and tolerates the bare {} the CLI edge passes via `res.disclosure || {}`. Scope note: #3248's prose says "skill stems"; ADR-2363 D3 classifies instruction surfaces as "skills, agents". Shipping skills alone would leave an ADR deliverable owned by no phase, and the epic has no Phase 2. Agents are the same shape at no extra cost. Narrowing back is a two-line change. Ratifies ADR-2363 (Proposed -> Accepted) and adds the owed ADR-1244 back-link. Closes #3248 * fix(#3248): escape consent-prompt values and narrow disclosure to skills Two review findings, both of which made the previous commit wrong. BLOCKER (isolated adversarial review). Every manifest-supplied value interpolated into a consent-prompt line was rendered unescaped. Those lines are joined with \n and written RAW to stderr on the needs-consent path (capability-command-router -> cli-exit runMain), so a stem carrying a newline forged additional lines indistinguishable from genuine GSD disclosure text, and an ANSI escape could clear or rewrite lines already printed. That defeats the informed-consent guarantee this change exists to provide, and is a prompt-injection vector against any agent that reads the stderr text to decide whether to retry with --yes. The hole was not unique to the new class - hook event/script, command family/module/router, every MCP field, and every reviewer-lane field were equally unescaped. Fixing only the new one would have created the generative-fix divergence this repo tracks, so renderValueForPrompt is applied to all five classes through one helper, guarded by a parity test that fails if a future class skips it. Escaping is identity for ordinary names, so no well-formed manifest's output changes. The disclosure OBJECT stays verbatim - only the rendered LINE is escaped - because the signature and every consumer reasoning about identity depend on the declared value. NARROWED to skills only. The previous commit also collected agents, arguing ADR-2363 D3 classifies instruction surfaces as "skills, agents". Verified against staging: stageSkillsForRuntimeAsSkills takes a registry and unions third-party skills in via readInstalledCapabilitySkill, while stageAgentsForRuntimeWithConverter takes only a source directory and has no registry-aware path. Third-party agents are never staged into the instruction context, so disclosing them would have put a false claim in a security prompt - worse than the scope creep two reviewers flagged it as. D3's classification stands; D5 now records that Phase 1 implements the skills half and that whether agents should be staged at all is an open maintainer question. Also reverts the premature ADR-2363 ratification. The previous commit flipped it to Accepted and asserted "#3248 merged" while this branch IS #3248 and is unmerged. Status returns to Proposed, and the ADR-1244 back-link - owed only on ratification - is withdrawn. Adds the fast-check property suite CLAUDE.md requires and the direct precedent (reviewer-trust-disclosure) already had: totality, D4 signature invariance, D3 hasExecutable invariance, and renderer totality over adversarial manifests. Refs #3248 * chore(#3248): correct changeset scope claim and backfill pr number The fragment was written against the pre-narrowing commit and still advertised 'skills and agents'. 4d26887e narrowed disclosure to skills only - third-party agents are never staged into the instruction context - but did not touch the fragment, so the release notes would have carried a claim the code does not implement. Also backfills pr:0 -> 3253 and names the prompt-escaping fix, which is user-visible and was absent from the original body. Changeset-only; no code or test changed, so the gsd-test pass recorded for 4d26887e still describes this tree's behavior. Refs #3248 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/clever-eagles-wake.md
Normal file
5
.changeset/clever-eagles-wake.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Added
|
||||
pr: 3253
|
||||
---
|
||||
**Capability skills are now named at the install consent prompt** — installing a third-party capability whose only contribution was skills printed "ships no executable surfaces (declarative only)" and listed nothing, even though each `SKILL.md` body lands verbatim in your agent's instruction context. The pre-install disclosure now names every contributed skill in its own section and states plainly that the bodies are not content-scanned. Values interpolated into the prompt are escaped across every disclosed surface, so a crafted name can no longer forge additional lines of disclosure text. No stored consent is disturbed and no re-consent prompt fires. (#3248)
|
||||
@@ -274,7 +274,7 @@ Issue #1459 user-owned consent seam (`gsd-core/bin/lib/capability-consent.cjs`,
|
||||
Issue #1459 finding 4 shared cross-process lock primitive (`gsd-core/bin/lib/capability-lock.cjs`, generated from `src/capability-lock.cts`). Leaf module (`node:fs`/`node:path`/`node:os`/`node:crypto` + the ledger's bounded `readSmallRegularFile` + `shell-command-projection`'s `execTool` for the rare start-time shell-out). THE single hardened lockfile protocol shared by BOTH `capability-lifecycle` (the `.gsd/capabilities/.lock` mutation lock) and `capability-consent` (the consent-store `.consent.lock`) — extracted so the two locks cannot diverge (mirrors the shared-validator / shared bounded-reader lessons). Exports: `acquireLock(lockPath, opts?)` (O_EXCL create with a JSON `{token,pid,hostname,startTime,ts}` body; steal protocol binds age to the body's own `ts`, never stale-steals a VERIFIED-LIVE same-host holder — pid alive AND recorded start-time matches the pid's current start-time, defeating pid-reuse without ever stealing a live holder — and reclaims only a dead/unverifiable holder via the dead-pid fast path or the hard `LOCK_DEADMAN_MS` deadman; `opts.maxAttempts` raises the bounded retry budget and `opts.waitForFresh` makes a contended fresh/live holder be WAITED FOR rather than failed-fast so genuinely-racing consent writers serialize), `releaseLock(handle)` (token + inode owner-safe — never deletes a successor's lock), `getProcessStartTime`, and the `_setLockProbes`/`_resetLockProbes` test seams. Carries the #1462 lifecycle-lock invariants (process-start-time liveness, TOCTOU-safe pre-rename identity recheck, bounded iterative loop).
|
||||
|
||||
### Capability Trust Gate
|
||||
ADR-1244 Phase 4 (D5) PURE policy module (`gsd-core/bin/lib/capability-trust.cjs`). Computes *what* a capability would do and *whether* policy permits it; performs no mutation and no I/O beyond existence-checking declared artifacts. Exports: `discloseExecutableSurfaces(manifest, stagedDir?, resolveHost?)` (enumerates the four executable surfaces — `hooks`, command modules, `mcpServers`, and reviewer lanes (ADR-2782 D5) — and flags `hasExecutable`; a reviewer lane is the one class that *receives* data, so it discloses its binary + full args (spawn) or destination host + `hostConfigKey` (openai-http) together with the egress payload classes); `evaluateInstallTrust(args)` (composes source policy + reserved-namespace + engines gate + disclosure into `{ allowed, requiresConsent, disclosure, engines, blockReasons }`); `evaluateSourceAllowed(parsed, strictKnownRegistries)` enforcing `capabilities.strict_known_registries` (unset/null → permissive-with-consent; `[]` → block all external; non-empty → host-based allowlist, never substring); `checkEngines(manifest, hostVersion)` (engines.gsd hard gate via `semverSatisfies` + `compatVersions` graceful-downgrade picking the newest working version); `executableSetChanged(old, new)` (auto-update re-consent trigger); `checkReservedNamespace` (`gsd-`/`gsd-core-`/`anthropic-`). The MCP disclosure also captures each server's `env` (string→string, filtered) and `cwd` (#1459) — `disclosureSignature` folds them in as STABLE SORTED JSON so any env/cwd add/change forces re-consent while a key reorder does not; `signatureForManifest(manifest, stagedDir?)` is the single source of truth for that signature (consumed by the loader's consent check and the lifecycle's consent binding). #1459 finding 5: each MCP surface also carries `rawConfig` — the FULL declared server config the writer persists (`{...config}`), prototype-pollution-cleaned — folded into the signature as STABLE SORTED JSON so a change to ANY persisted field (not just the explicit whitelist — a future `envFile`/`workingDir`/launch option) forces re-consent, while a pure key reorder does not; the human summary stays readable via the key fields only. The barrier is consent + integrity + reversibility, NOT a sandbox — see `docs/explanation/capability-trust-model.md`.
|
||||
ADR-1244 Phase 4 (D5) PURE policy module (`gsd-core/bin/lib/capability-trust.cjs`). Computes *what* a capability would do and *whether* policy permits it; performs no mutation and no I/O beyond existence-checking declared artifacts. Exports: `discloseExecutableSurfaces(manifest, stagedDir?, resolveHost?)` (enumerates the four executable surfaces — `hooks`, command modules, `mcpServers`, and reviewer lanes (ADR-2782 D5) — plus a fifth, non-executable class, instruction surfaces (`skills` stems only, ADR-2363 D5, #3248), returned as `instructionSurfaces`; flags `hasExecutable` from the four executable classes only — instruction surfaces deliberately do NOT contribute to it; a reviewer lane is the one class that *receives* data, so it discloses its binary + full args (spawn) or destination host + `hostConfigKey` (openai-http) together with the egress payload classes); `evaluateInstallTrust(args)` (composes source policy + reserved-namespace + engines gate + disclosure into `{ allowed, requiresConsent, disclosure, engines, blockReasons }`); `evaluateSourceAllowed(parsed, strictKnownRegistries)` enforcing `capabilities.strict_known_registries` (unset/null → permissive-with-consent; `[]` → block all external; non-empty → host-based allowlist, never substring); `checkEngines(manifest, hostVersion)` (engines.gsd hard gate via `semverSatisfies` + `compatVersions` graceful-downgrade picking the newest working version); `executableSetChanged(old, new)` (auto-update re-consent trigger); `checkReservedNamespace` (`gsd-`/`gsd-core-`/`anthropic-`); `collectInstructionSurfaces(manifest)` (the instruction-surface collector — `skills` stems only, independently testable, same total/`safeCollect` contract as the four executable collectors; ADR-2363 D3 classifies `agents` as an instruction surface too, but a third-party capability's declared `agents[]` are never staged into the agent's instruction context — `stageAgentsForRuntimeWithConverter` (`src/install-profiles.cts`) has no registry-aware third-party path the way `readInstalledCapabilitySkill` gives skills — so disclosing them would name a surface that does not exist; agents stay unimplemented pending a maintainer decision, and are NOT thereby safe or inert, only undisclosed); `summarizeInstructionSurfaces(disclosure)` (renders the instruction-surface section of the consent summary; called from BOTH branches of `summarizeDisclosure` because a skill-only capability has `hasExecutable === false` and takes the early return, so a section appended only at the end would never render for exactly the capabilities that need it). The MCP disclosure also captures each server's `env` (string→string, filtered) and `cwd` (#1459) — `disclosureSignature` folds them in as STABLE SORTED JSON so any env/cwd add/change forces re-consent while a key reorder does not; `signatureForManifest(manifest, stagedDir?)` is the single source of truth for that signature (consumed by the loader's consent check and the lifecycle's consent binding). #1459 finding 5: each MCP surface also carries `rawConfig` — the FULL declared server config the writer persists (`{...config}`), prototype-pollution-cleaned — folded into the signature as STABLE SORTED JSON so a change to ANY persisted field (not just the explicit whitelist — a future `envFile`/`workingDir`/launch option) forces re-consent, while a pure key reorder does not; the human summary stays readable via the key fields only. Instruction surfaces are deliberately EXCLUDED from `disclosureSignature` (ADR-2363 D4, #3248) — a manifest gaining, losing, or changing `skills` produces a byte-identical signature and disturbs no stored consent record; any future binding arrives as a versioned v2, never an in-place re-encoding of v1. The barrier is consent + integrity + reversibility, NOT a sandbox — see `docs/explanation/capability-trust-model.md`.
|
||||
|
||||
### Capability Lifecycle
|
||||
ADR-1244 Phase 4 (D5+D6) orchestration seam (`gsd-core/bin/lib/capability-lifecycle.cjs`) composing the source resolver, ledger, and trust gate into the mutating operations. Exports: `installCapability` (pre-fetch source gate → resolve copy-only with `promote:false` → trust verdict → promote + apply marker-stamped shared edits → **ledger commit**; nothing written on block/abort), `upgradeCapability` (atomic stage-then-swap: old set aside, new swapped in, shared edits re-derived, **ledger committed**, backup dropped; re-prompts when the executable set changed), `removeCapability` (strip only `_gsdCapability`-marked shared-config entries — user hand-edits preserved — delete exactly the ledger-recorded files, then drop the entry; `CAPABILITY_DATA` preserved unless `removeData`), `reconcileCapabilities` (crash recovery driven by the ledger's `_pending {kind,backupName,sharedFiles}` INTENT — not a version comparison: roll an uncommitted upgrade back by restoring the backup, an uncommitted fresh install away entirely, and re-sync shared config from the winning bundle, guaranteeing no half-state), plus `applyCapabilitySharedEdits`/`stripCapabilitySharedEdits` (marker-isolated JSON edits, prototype-pollution-guarded). All four mutating ops + reconcile take a cross-process lock (`.gsd/capabilities/.lock`, atomic stale-steal) so a concurrent reconcile can't clear a live intent. Capability code never executes during any operation. The source resolver's `promote:false`/`skipEnginesGate` options are the seams that let this module own the swap/commit ordering and the engines gate (with `compatVersions` downgrade hint).
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# ADR-2363: A capability's skill body is an instruction surface — trusted, unscanned, and disclosed
|
||||
|
||||
- **Status:** Proposed — D1–D4 are decided; **D5's mechanism is unshipped** and is tracked by [#3248](https://github.com/open-gsd/gsd-core/issues/3248) (Phase 1). Per the corpus lifecycle rule, an ADR with an outstanding phase stays `Proposed`. Ratify when #3248 has merged and the consent summary renders the instruction surface.
|
||||
- **Status:** Proposed — D1–D4 are decided. D5's mechanism ships in [#3248](https://github.com/open-gsd/gsd-core/issues/3248), which is open and unmerged; this ADR ratifies to `Accepted` when that PR merges (see the ratification bar under Consequences).
|
||||
- **Date:** 2026-08-09
|
||||
- **Issue:** [#2363](https://github.com/open-gsd/gsd-core/issues/2363) (epic); Phase 0 tracked by [#3247](https://github.com/open-gsd/gsd-core/issues/3247), Phase 1 by [#3248](https://github.com/open-gsd/gsd-core/issues/3248)
|
||||
- **Amends (prospective — this ADR is `Proposed`, so no back-link is owed yet):** [ADR-1244](1244-capability-ecosystem.md) — D5's disclosure gains a **fifth** class, and it is the first that is *not* an executable surface. [ADR-2782](2782-reviewer-lane-capability-surface.md) added the fourth (reviewer lanes); this adds the first non-executable one, which is why it needs its own classification rather than a fifth entry in the same list.
|
||||
@@ -91,15 +91,17 @@ Point 3 is the part that makes this a decision rather than a punt: the door stay
|
||||
|
||||
**The residual gap, stated plainly — and it is smaller than it first looks.** At **project** scope there is no gap at all: `bundleContentHash` walks every entry under the bundle with no exclusions and hashes every regular file (failing closed on symlinks and non-regular files), so a single changed byte in a skill body already deactivates a project-scoped capability until re-consent. At **global** scope there is no consent record in the first place — a global install is trusted because it sits under the user's own home ([ADR-1244](1244-capability-ecosystem.md) D5) — so a skill-body change on upgrade is not consent-gated there, and would not have been even if instruction surfaces were signature-bound. **The gap global scope has is the one it already had for code, and D4 neither widens nor narrows it.** What D4 declines to add is a re-consent *prompt* on skill-body change during an interactive upgrade; the v2 path above is where that would land if it is ever wanted.
|
||||
|
||||
### D5 — The mechanism *(Phase 1, [#3248](https://github.com/open-gsd/gsd-core/issues/3248) — not shipped by this ADR)*
|
||||
### D5 — The mechanism *(Phase 1, implemented in [#3248](https://github.com/open-gsd/gsd-core/issues/3248), open and unmerged at time of writing — not shipped by this ADR itself)*
|
||||
|
||||
`discloseExecutableSurfaces` gains an `instructionSurfaces` collector, enumerating each skill stem the manifest contributes, collected through the same `safeCollect` wrapper as the existing four classes so a hostile value degrades only that class and the function stays total for any manifest shape. The pre-install consent summary names those skills as an instruction surface. The signature behavior implements D4 exactly, pinned by a test that asserts what happens to a pre-existing consent record rather than leaving it incidental.
|
||||
|
||||
The design is deliberately **additive** — a new independent collector and a new field, with no change to the four existing collectors and none to `hasExecutable` — because `get_impact` rates `discloseExecutableSurfaces` **CRITICAL** at 196 affected symbols.
|
||||
|
||||
**Implementation note (Phase 1 scope).** The mechanism above discloses `skills` only. D3's class table names `skills, agents` as instruction surfaces, and that classification stands unchanged. `agents[]` are not disclosed by this mechanism, because third-party `agents[]` are never actually staged into the agent's instruction context: `stageSkillsForRuntimeAsSkills` (`src/install-profiles.cts`) unions third-party skills in via `readInstalledCapabilitySkill`, whereas `stageAgentsForRuntimeWithConverter` (same file) takes only a source directory and has no registry-aware third-party path. Disclosing agents here would have named a surface that does not exist. Whether third-party `agents[]` should be staged at all — and, if so, whether D3's agents half becomes reachable — is an open question for the maintainer, not a silent omission. Agents remain classified as an instruction surface either way; they are simply not yet a staged one.
|
||||
|
||||
## Consequences
|
||||
|
||||
**What improves.** The boundary is written down on both sides: a capability author reading [`docs/how-to/develop-a-capability.md`](../how-to/develop-a-capability.md) learns their skill body ships verbatim and unscanned, and a user reading [`capability-trust-model.md`](../explanation/capability-trust-model.md) learns what installing a skill-bearing capability grants. After Phase 1, a skill-only capability — which today discloses nothing at all — names its instruction surface at the consent moment.
|
||||
**What improves.** The boundary is written down on both sides: a capability author reading [`docs/how-to/develop-a-capability.md`](../how-to/develop-a-capability.md) learns their skill body ships verbatim and unscanned, and a user reading [`capability-trust-model.md`](../explanation/capability-trust-model.md) learns what installing a skill-bearing capability grants. A skill-only capability — which used to disclose nothing at all — now names its instruction surface at the consent moment.
|
||||
|
||||
**What does not change.** No behavior changes in this ADR's phase. No stored consent record is perturbed and no spurious re-consent fires, now or under Phase 1 (D4). Skills remain the intended, low-friction contribution path; this is disclosure, not discouragement.
|
||||
|
||||
@@ -109,7 +111,7 @@ The design is deliberately **additive** — a new independent collector and a ne
|
||||
- **First-party skills are equally unscanned.** Their assurance is provenance — they are the shipped package, and the GSD Core release process is their control — not content inspection. No content control exists on the first-party side either, and no reader should infer one.
|
||||
- **Global-scope skill-body change on upgrade is not consent-gated.** That is true of a global install's code too, and D4 does not change it either way. See D4.
|
||||
|
||||
**Ratification bar.** This ADR flips to `Accepted` when #3248 has merged, the consent summary renders instruction surfaces, and the D4 signature behavior is pinned by a passing test. Until then it is `Proposed` for a recorded reason, not through neglect.
|
||||
**Ratification bar.** This ADR flips to `Accepted` when #3248 has merged, the consent summary renders instruction surfaces, and the D4 signature behavior is pinned by a passing test. #3248, while open and unmerged, already delivers the second and third of these: the pre-install consent summary renders skill instruction surfaces by name, and a passing test pins the D4 signature behavior — instruction surfaces do not perturb `disclosureSignature`. The first condition, #3248's merge, has not yet occurred.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
|
||||
@@ -236,12 +236,21 @@ So there are three classes, not two:
|
||||
| **Instruction surface** | skills, agents | Instructions that will reach the agent. Reach bounded only by what the agent will do when told. |
|
||||
| **Inert artifact** | everything else in the bundle | Note only. |
|
||||
|
||||
**Not yet itemized at the prompt.** Naming a capability's individual skills in the
|
||||
pre-install consent summary lands with
|
||||
[#3248](https://github.com/open-gsd/gsd-core/issues/3248); today a skill-bearing
|
||||
capability is covered by the bundle's integrity and by your consent to install it,
|
||||
but its skills are not listed for you one by one. The classification above is
|
||||
what GSD has decided; the itemized prompt is what it has not yet built.
|
||||
Both `skills` and `agents` are classified as instruction surfaces, but only
|
||||
`skills` are disclosed today. A third-party capability's declared `agents[]`
|
||||
are never staged into the agent's instruction context — the staging path that
|
||||
unions third-party skills into a runtime's skills directory has no equivalent
|
||||
for agents — so naming them at the consent prompt would claim a surface that
|
||||
does not exist. This is not a claim that agents are safe or inert: they are
|
||||
still classified as an instruction surface, they are simply not staged for
|
||||
third-party capabilities today, which is why they are not itemized below.
|
||||
|
||||
**Itemized at the prompt.** [#3248](https://github.com/open-gsd/gsd-core/issues/3248)
|
||||
made the pre-install consent summary name each contributed skill in its own
|
||||
section. A capability whose only contribution is skills — which used to
|
||||
disclose nothing at all beyond the bundle's integrity — is included: you see its
|
||||
skills listed before you consent to install it. The listing names the surface;
|
||||
it does not assert anything about what the surface contains.
|
||||
|
||||
Installing a capability that ships skills grants it **instruction reach**. That is
|
||||
a real grant, and it is the same bargain this document already describes for code:
|
||||
@@ -255,11 +264,11 @@ Two things worth stating so you do not infer them:
|
||||
- **First-party skills are equally unscanned.** Their assurance is provenance —
|
||||
they are the shipped package — not content inspection. There is no content
|
||||
control on either side.
|
||||
- **Correcting this document did not disturb any consent you have already
|
||||
given.** No re-consent prompt follows from it. ADR-2363 D4 keeps instruction
|
||||
surfaces out of the v1 disclosure signature precisely so that recording the
|
||||
boundary honestly does not fire a spurious re-consent prompt on every
|
||||
skill-bearing capability you have installed.
|
||||
- **Naming the instruction surface did not disturb any consent you have
|
||||
already given.** No re-consent prompt follows from it. ADR-2363 D4 keeps
|
||||
instruction surfaces out of the v1 disclosure signature precisely so that
|
||||
disclosing the boundary honestly does not fire a spurious re-consent prompt
|
||||
on every skill-bearing capability you have installed.
|
||||
|
||||
### Integrity pinning
|
||||
|
||||
|
||||
@@ -128,7 +128,7 @@ That makes a skill body an **instruction surface**, and it carries author respon
|
||||
- **Write instructions you would be comfortable defending.** Your reach is bounded only by what the agent will do when told. Consent, integrity pinning and reversibility are the user's protections; content review is not among them.
|
||||
- **Do not embed anything that tries to redirect the agent away from the user's task** — reframing its role, overriding host instructions, or persisting directives past the skill's own scope. That is indistinguishable from a prompt-injection payload, and the fact that it is not scanned is not permission.
|
||||
- **Treat anything your skill tells the agent to read as data, not instructions.** If your skill has the agent ingest a file, a URL, or tool output, say so explicitly in the body. See [the untrusted-input boundary](../adr/1577-untrusted-input-boundary-and-injection-blocking.md).
|
||||
- **Expect the surface to be disclosed.** Naming the skills your Capability contributes in the pre-install consent summary lands with [#3248](https://github.com/open-gsd/gsd-core/issues/3248) — it does not happen today. Write your skill body on the assumption that a user will see it listed before they accept.
|
||||
- **The surface is disclosed.** The skills your Capability contributes are named, by stem, in their own section of the pre-install consent summary ([#3248](https://github.com/open-gsd/gsd-core/issues/3248)). Write your skill body on the assumption that a user sees it listed before they accept. Declared `agents[]` are not named there today — a third-party capability's agents are never staged into the agent's instruction context, so there is nothing to disclose — but they remain classified as an instruction surface, not an inert or safe one.
|
||||
|
||||
The reasoning behind this posture — including why scanning was considered and rejected — is recorded in [ADR-2363](../adr/2363-capability-instruction-surface-trust.md), and the user-facing side is [the capability trust model](../explanation/capability-trust-model.md).
|
||||
|
||||
|
||||
@@ -45,6 +45,10 @@ Feature capabilities declare owned artefacts, lifecycle hooks, a federated confi
|
||||
| `skills` | string[] | Owned skill stems. Exactly one capability may own each stem across the entire merged registry (first-party ∪ overlay). |
|
||||
| `agents` | string[] | Owned agent stems. Same uniqueness constraint as skills. |
|
||||
|
||||
The `skills` stems declared here are disclosed by name as **instruction surfaces** in the pre-install consent summary ([ADR-2363](../adr/2363-capability-instruction-surface-trust.md)). Bodies are installed verbatim and are not content-scanned — see [the capability trust model](../explanation/capability-trust-model.md).
|
||||
|
||||
`agents` are classified as an instruction surface too (ADR-2363 D3), but the stems declared here are **not** disclosed at the prompt: a third-party capability's `agents[]` are never staged into the agent's instruction context — the staging path that unions third-party skills into a runtime's skills directory has no equivalent for agents — so naming them would claim a surface that does not exist. This does not make agents safe or inert; it means the mechanism does not yet reach them.
|
||||
|
||||
### `hooks`
|
||||
|
||||
Non-loop lifecycle hooks.
|
||||
|
||||
@@ -37,7 +37,7 @@ gsd capability install <spec> [--integrity sha512-<hash>] [--scope global|projec
|
||||
|
||||
**Behaviour**
|
||||
|
||||
Resolves `<spec>` to a versioned, staged capability bundle. The pipeline is: fetch → verify integrity or SHA pin → check `engines.gsd` against the installed GSD version → disclose executable surfaces (hooks, command modules, MCP servers) → obtain consent (a declarative capability needs none; an executable one requires `--yes`) → validate the incoming manifest against the trust invariants → extract to the scope root → write the ledger entry atomically.
|
||||
Resolves `<spec>` to a versioned, staged capability bundle. The pipeline is: fetch → verify integrity or SHA pin → check `engines.gsd` against the installed GSD version → disclose executable surfaces (hooks, command modules, MCP servers) and instruction surfaces (skills) → obtain consent (a declarative capability needs none; an executable one requires `--yes` — disclosed instruction surfaces do not by themselves trigger this requirement) → validate the incoming manifest against the trust invariants → extract to the scope root → write the ledger entry atomically.
|
||||
|
||||
An overlay whose `id` uses a reserved first-party prefix (`gsd-`, `gsd-core-`, `anthropic-`) is rejected before extraction. Install never executes capability code; staging is copy-only. A declined install (executable surface, no `--yes`) writes **nothing** — no bundle, no ledger entry, no shared-file edits.
|
||||
|
||||
@@ -304,6 +304,18 @@ An empty (or missing) ledger reports nothing: `--json` emits `[]`; the table not
|
||||
|
||||
---
|
||||
|
||||
### Instruction surfaces (skills)
|
||||
|
||||
A capability's declared `skills` stems are **instruction surfaces**: each stem's body — its `SKILL.md` — is copied verbatim into the runtime's instruction context, where it reaches the agent directly. `install` and `update` list every such stem by name in the disclosure printed before the pipeline proceeds, alongside the executable-surface disclosure.
|
||||
|
||||
ADR-2363 D3 classifies `agents` as an instruction surface too, but a third-party capability's declared `agents[]` are not disclosed here: they are never staged into the agent's instruction context — the staging path that unions third-party skills into a runtime's skills directory has no equivalent for agents — so naming them would name a surface that does not exist. This is not a claim that agents are safe or inert, only that they are not currently staged for third-party capabilities.
|
||||
|
||||
An instruction surface is disclosed but is **not** an executable surface: naming it does not set `hasExecutable` and it does not enter the `disclosureSignature`, so adding, removing, or changing skills between versions does not by itself require `--yes` or force re-consent on `update`. See [ADR-2363](../adr/2363-capability-instruction-surface-trust.md) D3 (the instruction-surface classification) and D4 (why it is excluded from the disclosure signature).
|
||||
|
||||
Bodies are not content-scanned, at install, update, or any later point — disclosure names the surface; it never inspects what it contains.
|
||||
|
||||
---
|
||||
|
||||
### `trust`
|
||||
|
||||
Manage the **user-owned consent store** (#1459) that gates project-scope third-party capability activation. The store lives at `${GSD_HOME||homedir()}/.gsd/consent.json` — **outside any repository** — and records, per `(realpath(projectRoot), capability id)`, the bundle integrity and disclosure signature you consented to **on this machine**. A project-scope overlay is inactive until such a record exists (so a forged or cloned in-repo project ledger activates nothing on its own); installing a project-scope capability through the lifecycle writes the record, and removing it revokes the record.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
/**
|
||||
* 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.
|
||||
* by ADR-2782 Phase 3 (#2796) with a FOURTH executable-surface class (the reviewer lane), and by
|
||||
* ADR-2363 Phase 1 (#3248) with a FIFTH, NON-executable class: the instruction surface.
|
||||
*
|
||||
* 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
|
||||
@@ -20,16 +21,33 @@
|
||||
* 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`).
|
||||
*
|
||||
* ADR-2363 D5 (#3248): a capability's declared `skills` are INSTRUCTION surfaces — their bodies are
|
||||
* copied verbatim into the user's agent instruction context, so their reach is bounded only by what
|
||||
* the agent will do when told. They are disclosed BY NAME and never content-scanned (D2 —
|
||||
* Kerckhoffs: a shipped rule set is readable by the adversary who installs it). Unlike the four
|
||||
* executable classes they never set `hasExecutable` (D3) and never enter `disclosureSignature` (D4):
|
||||
* folding them in would perturb the stored signature of every already-consented skill-bearing
|
||||
* capability and fire a spurious re-consent on its next upgrade — the harm ADR-2782 D4 rule 5
|
||||
* already forbids. Any future signature binding arrives as a versioned v2, never an in-place
|
||||
* re-encoding of v1. ADR-2363 D3's class table names "skills, agents", but third-party `agents[]`
|
||||
* are deliberately EXCLUDED here: `stageAgentsForRuntimeWithConverter` (`src/install-profiles.cts`)
|
||||
* takes only a source directory, with no registry-aware third-party staging path the way
|
||||
* `readInstalledCapabilitySkill` gives skills — so a declared agent is never actually staged into
|
||||
* the instruction context, and disclosing it would name a surface that does not exist. Agents stay
|
||||
* unimplemented pending a maintainer decision.
|
||||
*
|
||||
* Exports:
|
||||
* RESERVED_NAMESPACES — id prefixes third parties may not claim
|
||||
* discloseExecutableSurfaces(...) — enumerate hooks / command modules / mcpServers / reviewer lanes
|
||||
* discloseExecutableSurfaces(...) — enumerate the four executable classes + instruction surfaces
|
||||
* collectReviewerLaneSurfaces(...) — the reviewer-lane collector, independently testable
|
||||
* collectInstructionSurfaces(...) — the instruction-surface 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
|
||||
* summarizeInstructionSurfaces(d) — the instruction-surface section of the consent summary
|
||||
* UNRESOLVED_HOST_MARKER — the non-blank marker for an unresolved openai-http host
|
||||
* EGRESS_PAYLOAD_CLASSES — the named data classes every reviewer lane receives
|
||||
*/
|
||||
@@ -222,6 +240,25 @@ interface ReviewerLaneSurface {
|
||||
egressPayloadClasses: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* ADR-2363 D3 (#3248): an INSTRUCTION surface — an artifact whose body is copied verbatim into the
|
||||
* user's agent instruction context. Peer to the four executable-surface classes, and deliberately
|
||||
* NOT one of them: a skill body does not execute code, it instructs the thing that does.
|
||||
*
|
||||
* Disclosure NAMES the surface; it never inspects the body. Content scanning is rejected outright
|
||||
* by ADR-2363 D2 (Kerckhoffs — a shipped rule set is readable by the adversary who installs it).
|
||||
*/
|
||||
interface InstructionSurface {
|
||||
/**
|
||||
* Which declaration array the name came from — still a discriminator even with one member: a
|
||||
* future addition (see `INSTRUCTION_SURFACE_FIELDS`) is why this stays a field rather than being
|
||||
* dropped now.
|
||||
*/
|
||||
kind: 'skill';
|
||||
/** The declared stem/name, VERBATIM — never normalized, truncated, or deduped. */
|
||||
name: string;
|
||||
}
|
||||
|
||||
interface Disclosure {
|
||||
/** Hook scripts the capability registers (each runs as a runtime hook command). */
|
||||
hooks: HookSurface[];
|
||||
@@ -235,6 +272,14 @@ interface Disclosure {
|
||||
* above; a standing egress channel to an external reviewer.
|
||||
*/
|
||||
reviewerLanes: ReviewerLaneSurface[];
|
||||
/**
|
||||
* ADR-2363 D5 (#3248): the skills this capability contributes to the agent's instruction context
|
||||
* (agents are excluded — see the module header). A FIFTH disclosed class that is deliberately NOT
|
||||
* executable: it never contributes to `hasExecutable` (D3) and never enters `disclosureSignature`
|
||||
* (D4 — folding it in would perturb the stored signature of every already-consented skill-bearing
|
||||
* capability and fire a spurious re-consent on its next upgrade, which ADR-2782 D4 rule 5 forbids).
|
||||
*/
|
||||
instructionSurfaces: InstructionSurface[];
|
||||
/** True when the capability ships ANY executable surface (=> consent required). */
|
||||
hasExecutable: boolean;
|
||||
/**
|
||||
@@ -688,6 +733,54 @@ function collectReviewerLaneSurfaces(
|
||||
}, []);
|
||||
}
|
||||
|
||||
/**
|
||||
* The manifest fields whose declared names become instruction surfaces. Ordered data rather than a
|
||||
* hand-rolled loop per field, so a future second member of the class is one row, not a second copy
|
||||
* of the same filter. Deliberately a ONE-row table today: third-party `agents[]` are never staged
|
||||
* into the instruction context — `stageAgentsForRuntimeWithConverter` (`src/install-profiles.cts`)
|
||||
* takes only a source directory, with no registry-aware staging path the way
|
||||
* `readInstalledCapabilitySkill` gives skills — so disclosing them would name a surface that does
|
||||
* not exist. ADR-2363 D3's class table says "skills, agents"; the agents half is therefore
|
||||
* deliberately unimplemented pending a maintainer decision.
|
||||
*/
|
||||
const INSTRUCTION_SURFACE_FIELDS: ReadonlyArray<{ field: string; kind: InstructionSurface['kind'] }> = [
|
||||
{ field: 'skills', kind: 'skill' },
|
||||
];
|
||||
|
||||
/**
|
||||
* Collect the instruction surfaces a manifest declares (ADR-2363 D5, #3248) — peer to the four
|
||||
* executable-surface collectors, and invoked through the same `safeCollect` wrapper so a hostile
|
||||
* value here degrades ONLY this class to empty rather than losing the other four.
|
||||
*
|
||||
* Liberal in what it accepts, exactly like the existing collectors: a non-object manifest, an
|
||||
* absent field, a non-array field, and a non-string/blank member each degrade quietly. A non-array
|
||||
* `skills` is NOT a partial success — it yields nothing, because a scalar declares no set.
|
||||
*
|
||||
* Deliberately does NOT:
|
||||
* - dedup (a manifest declaring a stem twice discloses it twice — disclosure reports what the
|
||||
* manifest SAYS; collapsing would misreport it, and dedup is the registry's job);
|
||||
* - normalize or truncate a name (it is disclosed verbatim so the user sees what was declared);
|
||||
* - existence-check the stem against `stagedDir`. A stem is a REGISTRY name, not a bundle-relative
|
||||
* artifact path — checking it would repeat the reviewer-lane `binary` mistake (matrix C6) and
|
||||
* put registry names into `missingArtifacts`, which is for declared bundle FILES only.
|
||||
*/
|
||||
function collectInstructionSurfaces(manifest: CapabilityManifest): InstructionSurface[] {
|
||||
const surfaces: InstructionSurface[] = [];
|
||||
if (typeof manifest !== 'object' || manifest === null) return surfaces;
|
||||
for (const { field, kind } of INSTRUCTION_SURFACE_FIELDS) {
|
||||
const declared = (manifest as Record<string, unknown>)[field];
|
||||
if (!Array.isArray(declared)) continue;
|
||||
for (const entry of declared) {
|
||||
// A non-string or blank member is dropped INDIVIDUALLY — the valid siblings around it still
|
||||
// disclose, mirroring how collectHookSurfaces skips a malformed entry rather than the array.
|
||||
if (typeof entry !== 'string') continue;
|
||||
if (entry.trim() === '') continue;
|
||||
surfaces.push({ kind, name: entry });
|
||||
}
|
||||
}
|
||||
return surfaces;
|
||||
}
|
||||
|
||||
/**
|
||||
* Enumerate every executable surface a capability manifest declares.
|
||||
*
|
||||
@@ -697,6 +790,13 @@ function collectReviewerLaneSurfaces(
|
||||
* - `mcpServers`: { <name>: {...} } | [{ name }] — servers spawned by the host runtime
|
||||
* - `reviewer`: { slug, transport, invoke, ... } — an external reviewer lane (ADR-2782 D5, #2796)
|
||||
*
|
||||
* plus ONE non-executable class (ADR-2363 D5, #3248):
|
||||
* - `skills`: string[] of owned stems — INSTRUCTION surfaces, whose bodies land in the agent's
|
||||
* instruction context. Disclosed by name; they never set `hasExecutable` (D3) and never enter
|
||||
* `disclosureSignature` (D4). Stems are registry names, so they are never existence-checked
|
||||
* against `stagedDir` and never appear in `missingArtifacts`. `agents` is excluded — see the
|
||||
* module header.
|
||||
*
|
||||
* `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
|
||||
@@ -709,7 +809,7 @@ function collectReviewerLaneSurfaces(
|
||||
* 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.
|
||||
* value in ONE class degrades only that class to empty rather than losing the others.
|
||||
*
|
||||
* `resolveHost` (optional, #2796) is forwarded to `collectReviewerLaneSurfaces` so a caller with
|
||||
* config access (the lifecycle, never the loader — see `signatureForManifest`) can disclose the REAL
|
||||
@@ -731,10 +831,16 @@ function discloseExecutableSurfaces(
|
||||
() => collectReviewerLaneSurfaces(manifest, resolveHost),
|
||||
[] as ReviewerLaneSurface[],
|
||||
);
|
||||
const instructionSurfaces = safeCollect(
|
||||
() => collectInstructionSurfaces(manifest),
|
||||
[] as InstructionSurface[],
|
||||
);
|
||||
|
||||
// ADR-2363 D3: instruction surfaces are deliberately ABSENT from this expression. Adding them
|
||||
// would silently change `executableSetChanged` and the auto-update re-consent trigger.
|
||||
const hasExecutable =
|
||||
hooks.length > 0 || commandModules.length > 0 || mcpServers.length > 0 || reviewerLanes.length > 0;
|
||||
return { hooks, commandModules, mcpServers, reviewerLanes, hasExecutable, missingArtifacts };
|
||||
return { hooks, commandModules, mcpServers, reviewerLanes, instructionSurfaces, hasExecutable, missingArtifacts };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1126,21 +1232,127 @@ function truncateEnvValue(v: string): string {
|
||||
return v.length > ENV_VALUE_MAX ? `${v.slice(0, ENV_VALUE_MAX)}… (${v.length} chars)` : v;
|
||||
}
|
||||
|
||||
/**
|
||||
* Characters that must never reach the consent prompt unescaped. `summarizeDisclosure`'s lines are
|
||||
* joined with `\n` and written RAW to stderr on the needs-consent path, and every value in them is
|
||||
* attacker-controlled manifest data. A raw newline forges a line indistinguishable from genuine
|
||||
* disclosure text; a raw ESC lets a value rewrite or clear lines already printed; a bidi override
|
||||
* visually reorders one. C0, DEL, C1, the bidi/isolate controls, and the line/paragraph separators
|
||||
* are all escaped to a visible `\uXXXX`, so the value stays identifiable and cannot forge output.
|
||||
*/
|
||||
const UNSAFE_PROMPT_CHARS = /[\u0000-\u001f\u007f-\u009f\u200e\u200f\u2028\u2029\u202a-\u202e\u2066-\u2069]/g;
|
||||
|
||||
/** Max characters of any single manifest-supplied value rendered into the consent prompt. */
|
||||
const PROMPT_VALUE_MAX = 200;
|
||||
|
||||
/**
|
||||
* Render one manifest-supplied value safely into a consent-prompt line: escape every character that
|
||||
* could forge or rewrite output, then bound the length so one oversized value cannot flood the
|
||||
* prompt and push the rest off screen. Escaping is IDENTITY for ordinary names, so this changes no
|
||||
* existing rendered output for any well-formed manifest — only the disclosure OBJECT is verbatim;
|
||||
* the rendered LINE is always escaped.
|
||||
*/
|
||||
function renderValueForPrompt(v: unknown): string {
|
||||
// `String(v)` on an arbitrary `unknown` risks Object's default `[object Object]` stringification
|
||||
// (@typescript-eslint/no-base-to-string) for a non-primitive; every call site here passes a string
|
||||
// in practice, but the parameter stays `unknown` for the same total-collector discipline as
|
||||
// `renderArgForHuman`, so a non-primitive is JSON-stringified instead of coerced.
|
||||
let s: string;
|
||||
if (typeof v === 'string') {
|
||||
s = v;
|
||||
} else if (v === null || v === undefined) {
|
||||
s = '';
|
||||
} else if (typeof v === 'number' || typeof v === 'boolean' || typeof v === 'bigint') {
|
||||
s = String(v);
|
||||
} else {
|
||||
try {
|
||||
s = JSON.stringify(v) ?? '';
|
||||
} catch {
|
||||
s = '';
|
||||
}
|
||||
}
|
||||
const escaped = s.replace(UNSAFE_PROMPT_CHARS, (c) => `\\u${c.charCodeAt(0).toString(16).padStart(4, '0')}`);
|
||||
return escaped.length > PROMPT_VALUE_MAX
|
||||
? `${escaped.slice(0, PROMPT_VALUE_MAX)}… (${escaped.length} chars)`
|
||||
: escaped;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the instruction-surface section of a consent summary (ADR-2363 D3/D5, #3248).
|
||||
*
|
||||
* Extracted as its own exported function for two reasons. It is called from BOTH branches of
|
||||
* `summarizeDisclosure` — a skill-only capability has `hasExecutable === false` and takes the early
|
||||
* return, so a section appended only at the end would never render for exactly the capabilities
|
||||
* that need it. And it gives tests a typed surface to assert on, instead of regex-matching prose out
|
||||
* of `summarizeDisclosure` (CONTRIBUTING — "Prohibited: Raw Text Matching on Test Outputs").
|
||||
*
|
||||
* Returns `[]` when nothing is declared, so either caller can append unconditionally without
|
||||
* emitting an empty header.
|
||||
*
|
||||
* TOTAL for a partial disclosure object: the CLI edge calls `summarizeDisclosure(res.disclosure || {})`
|
||||
* (`capability-command-router.cjs`), so a bare `{}` — carrying no `instructionSurfaces` at all —
|
||||
* reaches this function whenever a lifecycle result has no disclosure.
|
||||
*
|
||||
* #3248: every manifest-supplied value rendered here (`kind`, `name`) goes through
|
||||
* `renderValueForPrompt` first — the CLI edge writes these lines RAW to stderr on the needs-consent
|
||||
* path, and an unescaped name could forge a line or rewrite output already printed (see that
|
||||
* function's comment).
|
||||
*/
|
||||
function summarizeInstructionSurfaces(disclosure: Disclosure): string[] {
|
||||
const declared = (disclosure as Partial<Disclosure> | null | undefined)?.instructionSurfaces;
|
||||
const surfaces = Array.isArray(declared) ? declared : [];
|
||||
if (surfaces.length === 0) return [];
|
||||
const lines: string[] = [
|
||||
` instruction surfaces (${surfaces.length}): installed into your agent's instruction context`,
|
||||
];
|
||||
for (const s of surfaces) {
|
||||
// #3248: kind/name are manifest-supplied — escape+bound before rendering (see `renderValueForPrompt`).
|
||||
const kind = s?.kind ? renderValueForPrompt(s.kind) : '(kind?)';
|
||||
const name = s?.name ? renderValueForPrompt(s.name) : '(name?)';
|
||||
lines.push(` - ${kind}: ${name}`);
|
||||
}
|
||||
// ADR-2363 D1/D2, and Kerckhoffs: say plainly that nothing inspected these bodies. A summary that
|
||||
// named the surface while implying review would be worse than silence — a "looks checked" line
|
||||
// displaces the judgement this prompt exists to provoke (Goodhart, D2).
|
||||
lines.push(' these bodies are installed verbatim and are NOT content-scanned');
|
||||
return lines;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render a disclosure as consent-prompt lines. Returned as an array so the CLI/runtime edge can
|
||||
* format it; the lib never writes to stdout.
|
||||
*
|
||||
* #3248: every manifest-supplied value interpolated into a line (hook event/script, command
|
||||
* family/module/router, MCP name/transport/url/command/argv/header-keys/env-keys+values/cwd,
|
||||
* reviewer-lane slug/hostConfigKey/resolvedHost/binary/rawArgs/handler, missingArtifacts entries)
|
||||
* goes through `renderValueForPrompt` first, which escapes forging/rewriting control characters and
|
||||
* bounds the length. These lines are joined with `\n` and written RAW to stderr on the
|
||||
* needs-consent path (`capability-command-router.cjs`), so an unescaped value could forge a line or
|
||||
* rewrite/clear output already printed — defeating the informed-consent guarantee this function
|
||||
* exists to provide. GSD-authored literals (fallback placeholders, headings, `<redacted>`) are never
|
||||
* escaped — only manifest-supplied data is.
|
||||
*/
|
||||
function summarizeDisclosure(disclosure: Disclosure): string[] {
|
||||
const lines: string[] = [];
|
||||
const instructionLines = summarizeInstructionSurfaces(disclosure);
|
||||
if (!disclosure.hasExecutable) {
|
||||
lines.push('This capability ships no executable surfaces (declarative only).');
|
||||
// ADR-2363 D3: "declarative only" is true ONLY when there is no instruction surface either.
|
||||
// Claiming it unconditionally told a user their capability contributes nothing to weigh while
|
||||
// it was contributing agent instructions — the exact category error ADR-2363 was written to end.
|
||||
if (instructionLines.length === 0) {
|
||||
lines.push('This capability ships no executable surfaces (declarative only).');
|
||||
return lines;
|
||||
}
|
||||
lines.push('This capability ships no executable surfaces, but contributes agent instructions:');
|
||||
for (const line of instructionLines) lines.push(line);
|
||||
return lines;
|
||||
}
|
||||
lines.push('This capability ships executable surfaces that will run in your agent runtime:');
|
||||
if (disclosure.hooks.length > 0) {
|
||||
lines.push(` hooks (${disclosure.hooks.length}): run as runtime hook commands`);
|
||||
for (const h of disclosure.hooks) {
|
||||
lines.push(` - ${h.event || '(event?)'} -> ${h.script}`);
|
||||
const event = h.event ? renderValueForPrompt(h.event) : '(event?)';
|
||||
lines.push(` - ${event} -> ${renderValueForPrompt(h.script)}`);
|
||||
}
|
||||
}
|
||||
if (disclosure.commandModules.length > 0) {
|
||||
@@ -1149,8 +1361,9 @@ function summarizeDisclosure(disclosure: Disclosure): string[] {
|
||||
);
|
||||
for (const m of disclosure.commandModules) {
|
||||
// TRUST2-3 (#1459): show the router (which exported fn runs) so the user consents to the exact entry point.
|
||||
const routerSuffix = m.router ? ` [router: ${m.router}]` : '';
|
||||
lines.push(` - ${m.family || '(family?)'} -> ${m.module}${routerSuffix}`);
|
||||
const routerSuffix = m.router ? ` [router: ${renderValueForPrompt(m.router)}]` : '';
|
||||
const family = m.family ? renderValueForPrompt(m.family) : '(family?)';
|
||||
lines.push(` - ${family} -> ${renderValueForPrompt(m.module)}${routerSuffix}`);
|
||||
}
|
||||
}
|
||||
if (disclosure.mcpServers.length > 0) {
|
||||
@@ -1159,26 +1372,36 @@ function summarizeDisclosure(disclosure: Disclosure): string[] {
|
||||
// TRUST2-2 (#1459): a non-stdio (http/sse) server connects to a URL; disclose the endpoint, not
|
||||
// a (nonexistent) command. A stdio server discloses command + args as before.
|
||||
const isRemote = (s.transport === 'http' || s.transport === 'sse') || (!s.command && !!s.url);
|
||||
const name = renderValueForPrompt(s.name);
|
||||
if (isRemote) {
|
||||
const t = s.transport || 'http';
|
||||
lines.push(` - ${s.name} -> [${t}] ${s.url || '(no url declared)'}`);
|
||||
const t = s.transport ? renderValueForPrompt(s.transport) : 'http';
|
||||
const url = s.url ? renderValueForPrompt(s.url) : '(no url declared)';
|
||||
lines.push(` - ${name} -> [${t}] ${url}`);
|
||||
// Header VALUES are redacted in the human summary (they may carry secrets); only the KEY set
|
||||
// is shown. The full values ARE in the signature, so a value change forces re-consent.
|
||||
const hdrKeys = s.headers ? Object.keys(s.headers) : [];
|
||||
if (hdrKeys.length > 0) {
|
||||
lines.push(` headers: ${hdrKeys.map((k) => `${k}=<redacted>`).join(', ')}`);
|
||||
lines.push(` headers: ${hdrKeys.map((k) => `${renderValueForPrompt(k)}=<redacted>`).join(', ')}`);
|
||||
}
|
||||
} else {
|
||||
const cmd = [s.command, ...s.argv].filter(Boolean).join(' ');
|
||||
lines.push(` - ${s.name} -> ${cmd || '(no command declared)'}`);
|
||||
// Command + args are each escaped individually (not the joined string) so a value that
|
||||
// embeds a newline cannot forge a line even when it lands mid-argv.
|
||||
const cmd = [s.command, ...s.argv].filter(Boolean).map(renderValueForPrompt).join(' ');
|
||||
lines.push(` - ${name} -> ${cmd || '(no command declared)'}`);
|
||||
}
|
||||
// TRUST-2 (#1459): env can change WHAT runs without touching the command, so show each env key
|
||||
// and its (truncated) value — the user is consenting to this exact environment.
|
||||
// and its (truncated) value — the user is consenting to this exact environment. Truncate first
|
||||
// (keeps the prompt readable at the existing 60-char bound), then escape the result (#3248) so
|
||||
// the truncated value can still not forge or rewrite output.
|
||||
const envKeys = s.env ? Object.keys(s.env) : [];
|
||||
if (envKeys.length > 0) {
|
||||
lines.push(` env: ${envKeys.map((k) => `${k}=${truncateEnvValue(s.env[k])}`).join(', ')}`);
|
||||
lines.push(
|
||||
` env: ${envKeys
|
||||
.map((k) => `${renderValueForPrompt(k)}=${renderValueForPrompt(truncateEnvValue(s.env[k]))}`)
|
||||
.join(', ')}`,
|
||||
);
|
||||
}
|
||||
if (s.cwd) lines.push(` cwd: ${s.cwd}`);
|
||||
if (s.cwd) lines.push(` cwd: ${renderValueForPrompt(s.cwd)}`);
|
||||
}
|
||||
}
|
||||
if (disclosure.reviewerLanes.length > 0) {
|
||||
@@ -1193,26 +1416,33 @@ function summarizeDisclosure(disclosure: Disclosure): string[] {
|
||||
// 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.
|
||||
const slug = l.slug ? renderValueForPrompt(l.slug) : '(slug?)';
|
||||
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}`);
|
||||
const hostConfigKey = l.hostConfigKey ? renderValueForPrompt(l.hostConfigKey) : '(hostConfigKey?)';
|
||||
lines.push(` - ${slug} -> [openai-http] ${hostConfigKey} => ${renderValueForPrompt(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)'}`);
|
||||
// whole purpose. `renderArgForHuman` stringifies a non-string member; that
|
||||
// string is equally attacker-controlled, so it is escaped too (#3248).
|
||||
const cmd = [l.binary, ...l.rawArgs.map(renderArgForHuman)]
|
||||
.filter(Boolean)
|
||||
.map(renderValueForPrompt)
|
||||
.join(' ');
|
||||
lines.push(` - ${slug} -> ${cmd || '(no binary declared)'}`);
|
||||
}
|
||||
if (l.handler) lines.push(` handler: ${l.handler}`);
|
||||
if (l.handler) lines.push(` handler: ${renderValueForPrompt(l.handler)}`);
|
||||
lines.push(` sends: ${l.egressPayloadClasses.join(', ')}`);
|
||||
}
|
||||
}
|
||||
for (const line of instructionLines) lines.push(line);
|
||||
if (disclosure.missingArtifacts.length > 0) {
|
||||
lines.push(' WARNING — declared artifacts not found in the staged bundle:');
|
||||
for (const a of disclosure.missingArtifacts) {
|
||||
lines.push(` - ${a}`);
|
||||
lines.push(` - ${renderValueForPrompt(a)}`);
|
||||
}
|
||||
}
|
||||
return lines;
|
||||
@@ -1228,12 +1458,21 @@ export = {
|
||||
// #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,
|
||||
// ADR-2363 D5 (#3248): the instruction-surface collector, exported for independent testability —
|
||||
// same rationale as `collectReviewerLaneSurfaces` above.
|
||||
collectInstructionSurfaces,
|
||||
checkReservedNamespace,
|
||||
evaluateSourceAllowed,
|
||||
checkEngines,
|
||||
evaluateInstallTrust,
|
||||
executableSetChanged,
|
||||
summarizeDisclosure,
|
||||
// ADR-2363 D3/D5 (#3248): the instruction-surface section of the consent summary, exported so
|
||||
// callers/tests can assert on it directly — see `summarizeInstructionSurfaces`'s own JSDoc.
|
||||
summarizeInstructionSurfaces,
|
||||
// #3248: the consent-prompt escaping/bounding helper, exported so tests can assert directly that
|
||||
// control characters (newline, ESC, bidi overrides, etc.) never reach a rendered prompt line.
|
||||
renderValueForPrompt,
|
||||
// #1459: the consent-binding signature (single source of truth for loader + lifecycle consent).
|
||||
disclosureSignature,
|
||||
signatureForManifest,
|
||||
|
||||
901
tests/instruction-surface-disclosure.security.test.cjs
Normal file
901
tests/instruction-surface-disclosure.security.test.cjs
Normal file
@@ -0,0 +1,901 @@
|
||||
'use strict';
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
|
||||
/**
|
||||
* instruction-surface-disclosure.security.test.cjs — behavioral tests for a FIFTH disclosed
|
||||
* class inside the capability trust gate (ADR-2363 D5, #3248): `instructionSurfaces` — the
|
||||
* skill stems a capability manifest declares — added to `discloseExecutableSurfaces`.
|
||||
*
|
||||
* Instruction surfaces are SKILLS ONLY (`InstructionSurface.kind` is the literal `'skill'`).
|
||||
* ADR-2363 D3's class table names "skills, agents", but a declared `agents[]` is deliberately
|
||||
* NOT collected: third-party `agents[]` are never staged into the agent's instruction context
|
||||
* (there is no registry-aware agent staging path, unlike `readInstalledCapabilitySkill` for
|
||||
* skills), so disclosing them would be a false claim in a consent prompt. Tests below that used
|
||||
* to assert agent disclosure now assert the NEGATIVE — that a declared `agents` array yields no
|
||||
* instruction surface at all.
|
||||
*
|
||||
* Implements every row carrying a Test name in
|
||||
* `.gsd/phase/feat-3248-disclose-instruction-surfaces/50-test-matrix.md`, derived from
|
||||
* `40-design.md`'s behavior table. Rows 18-20 and 23-25 are the load-bearing ones: they encode
|
||||
* ADR-2363 D4 — instruction surfaces must never perturb `disclosureSignature`/`hasExecutable`, and
|
||||
* no pre-existing consent record may be disturbed by a manifest gaining a `skills`/`agents` array.
|
||||
*
|
||||
* FAILING-FIRST: at the time this file was written, `Disclosure.instructionSurfaces` does not
|
||||
* exist. `discloseExecutableSurfaces` is called directly (the cheapest unit that proves the
|
||||
* behavior), matching `tests/reviewer-trust-disclosure.test.cjs`'s own established idiom.
|
||||
*
|
||||
* Suite: `security` (filename `.security.` infix) — this is a trust-gate surface.
|
||||
*/
|
||||
|
||||
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, tests/reviewer-trust-disclosure.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 shared mutable state.
|
||||
|
||||
/**
|
||||
* A minimal, valid capability manifest declaring both skills and agents. Kept declaring BOTH
|
||||
* deliberately — it is now valuable precisely because it proves `agents` is ignored: every
|
||||
* assertion against this fixture's `instructionSurfaces` must show the skills only, never the
|
||||
* declared agent name.
|
||||
*/
|
||||
function skillsAndAgentsManifest(mutator) {
|
||||
const manifest = {
|
||||
id: 'test-cap',
|
||||
role: 'feature',
|
||||
title: 'Test Capability',
|
||||
description: 'A test capability for the instruction-surface disclosure test suite.',
|
||||
tier: 'standard',
|
||||
requires: [],
|
||||
version: '1.0.0',
|
||||
skills: ['ui-phase', 'ui-review'],
|
||||
agents: ['gsd-ui-checker'],
|
||||
};
|
||||
if (mutator) mutator(manifest);
|
||||
return manifest;
|
||||
}
|
||||
|
||||
/** A manifest carrying one hook, one command module, and one mcpServer — no skills/agents/reviewer. */
|
||||
function executableSurfaceManifest(mutator) {
|
||||
const manifest = {
|
||||
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' } } },
|
||||
};
|
||||
if (mutator) mutator(manifest);
|
||||
return manifest;
|
||||
}
|
||||
|
||||
// ─── A. Happy path (rows 1-3) ───────────────────────────────────────────────
|
||||
|
||||
describe('A. Happy path', () => {
|
||||
test('discloses declared skill stems in order', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: ['a', 'b'] });
|
||||
assert.deepEqual(d.instructionSurfaces, [
|
||||
{ kind: 'skill', name: 'a' },
|
||||
{ kind: 'skill', name: 'b' },
|
||||
]);
|
||||
});
|
||||
|
||||
// A declared `agents` array is classified as an instruction surface by ADR-2363 D3's class
|
||||
// table, but is deliberately NOT staged into the instruction context for third-party
|
||||
// capabilities (no registry-aware agent staging path — see the module header on
|
||||
// src/capability-trust.cts). Disclosing it would name a surface that does not exist, so it
|
||||
// must yield NO instruction surfaces at all.
|
||||
test('declared agent names yield no instruction surfaces', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', agents: ['gsd-ui-checker'] });
|
||||
assert.deepEqual(d.instructionSurfaces, []);
|
||||
});
|
||||
|
||||
test('a manifest declaring both skills and agents discloses only its skills', () => {
|
||||
const d = trust.discloseExecutableSurfaces(skillsAndAgentsManifest());
|
||||
assert.deepEqual(d.instructionSurfaces, [
|
||||
{ kind: 'skill', name: 'ui-phase' },
|
||||
{ kind: 'skill', name: 'ui-review' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── B. Boundary (rows 4-7) ─────────────────────────────────────────────────
|
||||
|
||||
describe('B. Boundary', () => {
|
||||
test('absent skills yields an empty instruction surface list', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x' });
|
||||
assert.deepEqual(d.instructionSurfaces, []);
|
||||
});
|
||||
|
||||
test('empty skills array yields empty list', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: [] });
|
||||
assert.deepEqual(d.instructionSurfaces, []);
|
||||
});
|
||||
|
||||
test('single skill', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: ['only'] });
|
||||
assert.deepEqual(d.instructionSurfaces, [{ kind: 'skill', name: 'only' }]);
|
||||
});
|
||||
|
||||
test('many skills are not truncated', () => {
|
||||
const stems = Array.from({ length: 64 }, (_, i) => `skill-${i}`);
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: stems });
|
||||
assert.deepEqual(
|
||||
d.instructionSurfaces,
|
||||
stems.map((name) => ({ kind: 'skill', name })),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── C. Negative / malformed (rows 8-11, 15) ────────────────────────────────
|
||||
|
||||
describe('C. Negative / malformed', () => {
|
||||
test('non-array skills yields empty list', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: 'a' });
|
||||
assert.deepEqual(d.instructionSurfaces, []);
|
||||
});
|
||||
|
||||
test('object skills yields empty list', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: { 0: 'a' } });
|
||||
assert.deepEqual(d.instructionSurfaces, []);
|
||||
});
|
||||
|
||||
test('non-string and empty stems are dropped individually', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: ['ok', 42, null, {}, '', true] });
|
||||
assert.deepEqual(d.instructionSurfaces, [{ kind: 'skill', name: 'ok' }]);
|
||||
});
|
||||
|
||||
test('whitespace-only stem is dropped', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: [' '] });
|
||||
assert.deepEqual(d.instructionSurfaces, []);
|
||||
});
|
||||
|
||||
test('non-object manifest is total', () => {
|
||||
for (const manifest of [null, 42, []]) {
|
||||
assert.doesNotThrow(() => trust.discloseExecutableSurfaces(manifest));
|
||||
const d = trust.discloseExecutableSurfaces(manifest);
|
||||
assert.deepEqual(d.instructionSurfaces, [], `manifest=${JSON.stringify(manifest)} must disclose no instruction surfaces`);
|
||||
assert.deepEqual(d.hooks, []);
|
||||
assert.deepEqual(d.commandModules, []);
|
||||
assert.deepEqual(d.mcpServers, []);
|
||||
assert.deepEqual(d.reviewerLanes, []);
|
||||
assert.equal(d.hasExecutable, false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── D. Hostile (rows 12-14, 16) ─────────────────────────────────────────────
|
||||
|
||||
describe('D. Hostile', () => {
|
||||
test('a throwing skills getter degrades only its own class', () => {
|
||||
const manifest = executableSurfaceManifest();
|
||||
Object.defineProperty(manifest, 'skills', {
|
||||
enumerable: true,
|
||||
get() {
|
||||
throw new Error('boom: throwing skills getter');
|
||||
},
|
||||
});
|
||||
assert.doesNotThrow(() => trust.discloseExecutableSurfaces(manifest));
|
||||
const d = trust.discloseExecutableSurfaces(manifest);
|
||||
assert.deepEqual(d.instructionSurfaces, [], 'a throwing skills getter must degrade to no instruction surfaces');
|
||||
assert.deepEqual(d.hooks, [{ event: 'PostToolUse', script: 'hooks/x.js' }], 'hooks must still populate');
|
||||
assert.deepEqual(
|
||||
d.commandModules,
|
||||
[{ family: 'demo', module: 'demo.cjs', router: 'run' }],
|
||||
'command modules must still populate',
|
||||
);
|
||||
assert.equal(d.mcpServers.length, 1, 'mcp servers must still populate');
|
||||
assert.deepEqual(d.reviewerLanes, [], 'lane-free manifest still discloses no lane (unaffected either way)');
|
||||
assert.equal(d.hasExecutable, true, 'the other three classes still set hasExecutable');
|
||||
});
|
||||
|
||||
test('a hostile Proxy manifest never throws', () => {
|
||||
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.instructionSurfaces, []);
|
||||
assert.deepEqual(d.hooks, []);
|
||||
assert.deepEqual(d.commandModules, []);
|
||||
assert.deepEqual(d.mcpServers, []);
|
||||
assert.deepEqual(d.reviewerLanes, []);
|
||||
});
|
||||
|
||||
test('prototype-polluting stem names do not mutate Object.prototype', () => {
|
||||
const beforeProps = Object.getOwnPropertyNames(Object.prototype).sort();
|
||||
const d = trust.discloseExecutableSurfaces({
|
||||
id: 'x',
|
||||
skills: ['__proto__', 'constructor', 'prototype'],
|
||||
});
|
||||
assert.deepEqual(d.instructionSurfaces, [
|
||||
{ kind: 'skill', name: '__proto__' },
|
||||
{ kind: 'skill', name: 'constructor' },
|
||||
{ kind: 'skill', name: 'prototype' },
|
||||
], 'the literal names are disclosed, not interpreted as prototype keys');
|
||||
const afterProps = Object.getOwnPropertyNames(Object.prototype).sort();
|
||||
assert.deepEqual(afterProps, beforeProps, 'Object.prototype must be unchanged');
|
||||
assert.equal(({}).polluted, undefined, 'a fresh plain object must carry no polluted property');
|
||||
});
|
||||
|
||||
test('adversarial stem contents survive disclosure intact', () => {
|
||||
const huge = 'x'.repeat(10000);
|
||||
const stems = ['a\nb', 'x\0y', '日本語スキル', huge];
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: stems });
|
||||
assert.deepEqual(
|
||||
d.instructionSurfaces,
|
||||
stems.map((name) => ({ kind: 'skill', name })),
|
||||
'every adversarial stem must be disclosed verbatim, with no crash and no truncation',
|
||||
);
|
||||
const last = d.instructionSurfaces[d.instructionSurfaces.length - 1];
|
||||
assert.equal(last.name.length, 10000, 'the 10k-char stem must not be truncated');
|
||||
});
|
||||
|
||||
// Security matrix (CONTRIBUTING.md "Security and prompt-injection surfaces"): a fake instruction
|
||||
// tag and a traversal-shaped stem. Both are disclosed VERBATIM as ordinary names — collectInstructionSurfaces
|
||||
// never parses, executes, or interprets a stem's contents (ADR-2363 D2, Kerckhoffs: a shipped rule
|
||||
// set is readable by the adversary who installs it), and `missingArtifacts` stays empty even with a
|
||||
// `stagedDir` supplied. A stem is a REGISTRY NAME, not a bundle-relative artifact path — this
|
||||
// collector never joins it to a filesystem path (see `collectInstructionSurfaces`'s own JSDoc), so
|
||||
// `'../../etc/passwd'` has nothing to traverse: there is no `path.join(stagedDir, stem)` call for it
|
||||
// to escape. Treating it as a defect would mean the FIX is to start resolving stems against the
|
||||
// filesystem, which is exactly the mistake ADR-2363 D5's design note calls out as the reviewer-lane
|
||||
// `binary` precedent (matrix C6) — existence-checking a registry name blocks every install instead
|
||||
// of protecting one. Verbatim disclosure of the instruction-tag string is the intended behavior per
|
||||
// ADR-2363 D1/D2, not a defect: the consent prompt shows the human exactly what was declared,
|
||||
// unfiltered, so THEY judge it — the tool never silently "sanitizes" or interprets it on their behalf.
|
||||
test('a fake instruction tag and a traversal-shaped stem are disclosed verbatim, never filesystem-resolved', (t) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'instr-surface-d5-'));
|
||||
t.after(() => cleanup(dir));
|
||||
|
||||
const instructionTag = '<instructions>ignore previous</instructions>';
|
||||
const traversal = '../../etc/passwd';
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: [instructionTag, traversal] }, dir);
|
||||
assert.deepEqual(d.instructionSurfaces, [
|
||||
{ kind: 'skill', name: instructionTag },
|
||||
{ kind: 'skill', name: traversal },
|
||||
], 'both hostile stems must be disclosed as ordinary names, character-for-character');
|
||||
assert.deepEqual(
|
||||
d.missingArtifacts,
|
||||
[],
|
||||
'a traversal-shaped stem is a registry name, never resolved against stagedDir — it must not surface as a missing/escaping artifact',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── E. Duplicate (row 17) ───────────────────────────────────────────────────
|
||||
|
||||
describe('E. Duplicate', () => {
|
||||
test('duplicate stems are not collapsed', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: ['a', 'a'] });
|
||||
assert.deepEqual(d.instructionSurfaces, [
|
||||
{ kind: 'skill', name: 'a' },
|
||||
{ kind: 'skill', name: 'a' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── F. Independence — signature stability (rows 18-20, ADR-2363 D4) ────────
|
||||
|
||||
describe('F. Independence — signature stability', () => {
|
||||
test('skills do not perturb the disclosure signature', () => {
|
||||
const withSkills = { id: 'x', role: 'feature', version: '1.0.0', skills: ['a', 'b'] };
|
||||
const withoutSkills = { id: 'x', role: 'feature', version: '1.0.0' };
|
||||
assert.equal(trust.signatureForManifest(withSkills), trust.signatureForManifest(withoutSkills));
|
||||
});
|
||||
|
||||
test('skills do not perturb the disclosure signature of an executable-surface-bearing manifest', () => {
|
||||
const withSkills = executableSurfaceManifest((m) => {
|
||||
m.skills = ['a', 'b'];
|
||||
});
|
||||
const withoutSkills = executableSurfaceManifest();
|
||||
assert.equal(trust.signatureForManifest(withSkills), trust.signatureForManifest(withoutSkills));
|
||||
});
|
||||
|
||||
// These two `agents` signature tests still assert a true and useful property — agents never
|
||||
// perturb the signature — but are now TRIVIALLY true, since `agents` is not collected into
|
||||
// `instructionSurfaces` at all (it never reaches `collectInstructionSurfaces`'s per-field
|
||||
// loop). Kept so they guard the NARROWING (agents dropped entirely) rather than D4
|
||||
// specifically — a regression that made `agents` collected again would still need a separate
|
||||
// D4 test to catch a signature perturbation.
|
||||
test('agents do not perturb the disclosure signature', () => {
|
||||
const withAgents = { id: 'x', role: 'feature', version: '1.0.0', agents: ['gsd-ui-checker'] };
|
||||
const withoutAgents = { id: 'x', role: 'feature', version: '1.0.0' };
|
||||
assert.equal(trust.signatureForManifest(withAgents), trust.signatureForManifest(withoutAgents));
|
||||
});
|
||||
|
||||
test('agents do not perturb the disclosure signature of an executable-surface-bearing manifest', () => {
|
||||
const withAgents = executableSurfaceManifest((m) => {
|
||||
m.agents = ['gsd-ui-checker'];
|
||||
});
|
||||
const withoutAgents = executableSurfaceManifest();
|
||||
assert.equal(trust.signatureForManifest(withAgents), trust.signatureForManifest(withoutAgents));
|
||||
});
|
||||
|
||||
// A JS re-implementation of the PRE-#3248 (and pre-#2796-lane) discloseExecutableSurfaces
|
||||
// (hooks/commands/mcpServers ONLY) + disclosureSignature + stableJson — copied verbatim from
|
||||
// `tests/reviewer-trust-disclosure.test.cjs`'s own `refDiscloseExecutableSurfaces` /
|
||||
// `refStableJson` oracle (itself copied from src/capability-trust.cts as it stood before ADR-2782
|
||||
// Phase 3), which satisfies the fixture-provenance rule (#2371): it was written by a source that
|
||||
// does not know the `skills`/`agents`/`instructionSurfaces` class exists at all.
|
||||
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('signature matches the pre-change oracle for a skill-bearing manifest', () => {
|
||||
const manifestWithSkills = executableSurfaceManifest((m) => {
|
||||
m.skills = ['ui-phase', 'ui-review'];
|
||||
m.agents = ['gsd-ui-checker'];
|
||||
});
|
||||
// The oracle does not know `skills`/`agents`/`reviewer` exist at all — it only ever reads
|
||||
// hooks/commands/mcpServers — so its output for the skill-bearing manifest IS the reference
|
||||
// "sans skills" signature the matrix asks for.
|
||||
assert.equal(trust.signatureForManifest(manifestWithSkills), referenceLaneFreeSignature(manifestWithSkills));
|
||||
});
|
||||
});
|
||||
|
||||
// ─── G. Independence — hasExecutable (rows 21-22) ───────────────────────────
|
||||
|
||||
describe('G. Independence — hasExecutable', () => {
|
||||
test('an instruction surface alone does not set hasExecutable', () => {
|
||||
const d = trust.discloseExecutableSurfaces(skillsAndAgentsManifest());
|
||||
assert.equal(d.hasExecutable, false);
|
||||
});
|
||||
|
||||
test('hasExecutable still reflects executable surfaces only', () => {
|
||||
const manifest = skillsAndAgentsManifest((m) => {
|
||||
m.hooks = [{ event: 'PostToolUse', script: 'hooks/x.js' }];
|
||||
});
|
||||
const d = trust.discloseExecutableSurfaces(manifest);
|
||||
assert.equal(d.hasExecutable, true, 'the hook, not the instruction surfaces, sets hasExecutable');
|
||||
assert.equal(d.hooks.length, 1);
|
||||
// Skills-only count: `skillsAndAgentsManifest` declares 2 skills + 1 agent, but agents are
|
||||
// not collected (see the module-header comment above), so only the 2 skills disclose.
|
||||
assert.equal(d.instructionSurfaces.length, 2, 'instruction surfaces still disclosed alongside the hook');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── H. Independence — executableSetChanged (row 23) ────────────────────────
|
||||
|
||||
describe('H. Independence — executableSetChanged', () => {
|
||||
test('adding a skill is not an executable-set change', () => {
|
||||
const before = trust.discloseExecutableSurfaces({ id: 'x' });
|
||||
const after = trust.discloseExecutableSurfaces({ id: 'x', skills: ['a'] });
|
||||
assert.equal(trust.executableSetChanged(before, after), false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── I. Regression — pre-existing consent record (row 24) ───────────────────
|
||||
|
||||
describe('I. Regression — pre-existing consent record', () => {
|
||||
const LOCAL_SPEC = { kind: 'local', raw: '.', target: '.' };
|
||||
|
||||
test('a pre-existing consent record survives instruction-surface disclosure', () => {
|
||||
// Simulates a consent record written BEFORE this phase (a manifest with no skills/agents),
|
||||
// then the capability being upgraded to a version that adds a skill — the stored signature
|
||||
// must still match, so no re-consent prompt fires.
|
||||
const preChangeManifest = { id: 'x', role: 'feature', version: '1.0.0' };
|
||||
const v1 = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest: preChangeManifest, hostVersion: '1.0.0' });
|
||||
const storedSignature = trust.disclosureSignature(v1.disclosure);
|
||||
|
||||
const upgradedManifest = { id: 'x', role: 'feature', version: '1.1.0', skills: ['ui-phase'] };
|
||||
const v2 = trust.evaluateInstallTrust({ parsed: LOCAL_SPEC, manifest: upgradedManifest, hostVersion: '1.0.0' });
|
||||
const upgradedSignature = trust.disclosureSignature(v2.disclosure);
|
||||
|
||||
assert.equal(upgradedSignature, storedSignature, 'the stored consent signature must still match after the upgrade');
|
||||
assert.equal(
|
||||
trust.executableSetChanged(v1.disclosure, v2.disclosure),
|
||||
false,
|
||||
'gaining a skill must not force a re-consent prompt',
|
||||
);
|
||||
assert.equal(v2.requiresConsent, false, 'a skill-only capability requires no consent at all');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── J. Independence — missingArtifacts (row 25) ────────────────────────────
|
||||
|
||||
describe('J. Independence — missingArtifacts', () => {
|
||||
test('skill stems are never existence-checked against stagedDir', (t) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'instr-surface-j1-'));
|
||||
t.after(() => cleanup(dir));
|
||||
|
||||
const manifest = { id: 'x', skills: ['nonexistent-skill-stem', 'another-missing-one'] };
|
||||
const d = trust.discloseExecutableSurfaces(manifest, dir);
|
||||
assert.deepEqual(
|
||||
d.missingArtifacts,
|
||||
[],
|
||||
'skill stems are registry names, not bundle-relative artifact paths — they must never contribute to missingArtifacts',
|
||||
);
|
||||
assert.equal(d.instructionSurfaces.length, 2, 'the skills must still be disclosed');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── K. Consent prompt (rows 26-27) ──────────────────────────────────────────
|
||||
//
|
||||
// `summarizeInstructionSurfaces(disclosure)` is the typed surface the implementation added for
|
||||
// exactly this: CONTRIBUTING's "Prohibited: Raw Text Matching on Test Outputs" forbids regex-matching
|
||||
// `summarizeDisclosure`'s rendered prose, so these rows assert on that function's structured output
|
||||
// (its length against the declared surface count) and on ARRAY CONTAINMENT between the two
|
||||
// renderers — never on the wording of a line.
|
||||
|
||||
describe('K. Consent prompt', () => {
|
||||
test('consent prompt names instruction surfaces separately', () => {
|
||||
const manifest = { id: 'x', skills: ['ui-phase', 'ui-review'] };
|
||||
const d = trust.discloseExecutableSurfaces(manifest);
|
||||
assert.deepEqual(d.instructionSurfaces, [
|
||||
{ kind: 'skill', name: 'ui-phase' },
|
||||
{ kind: 'skill', name: 'ui-review' },
|
||||
]);
|
||||
assert.deepEqual(d.hooks, [], 'instruction surfaces must not be folded into an executable-surface class');
|
||||
assert.equal(d.hasExecutable, false, 'a skill-only manifest never requires consent from this data');
|
||||
|
||||
// One header line + one line per surface + one "not content-scanned" line.
|
||||
const section = trust.summarizeInstructionSurfaces(d);
|
||||
assert.equal(section.length, d.instructionSurfaces.length + 2, 'every declared surface gets its own line');
|
||||
});
|
||||
|
||||
test('consent prompt omits the section when there is nothing to disclose', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x' });
|
||||
assert.deepEqual(d.instructionSurfaces, []);
|
||||
assert.deepEqual(
|
||||
trust.summarizeInstructionSurfaces(d),
|
||||
[],
|
||||
'nothing declared => no lines at all, so no empty header can render',
|
||||
);
|
||||
});
|
||||
|
||||
// Row 26a — the defect this phase is most likely to ship silently. A skill-only capability has
|
||||
// hasExecutable === false and takes summarizeDisclosure's EARLY RETURN, so a section appended only
|
||||
// at the end of the function would never render for precisely the capabilities that need it.
|
||||
// Asserted as ARRAY CONTAINMENT of one renderer's output in the other's — a structural property,
|
||||
// not a prose match.
|
||||
test('a skill-only capability still renders its instruction surfaces in the consent summary', () => {
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: ['ui-phase'] });
|
||||
assert.equal(d.hasExecutable, false, 'precondition: this manifest takes the early-return branch');
|
||||
const section = trust.summarizeInstructionSurfaces(d);
|
||||
const summary = trust.summarizeDisclosure(d);
|
||||
assert.ok(section.length > 0, 'precondition: there is a section to render');
|
||||
for (const line of section) {
|
||||
assert.ok(summary.includes(line), 'every instruction-surface line must reach the rendered summary');
|
||||
}
|
||||
});
|
||||
|
||||
// Row 26b — the same containment property for a capability that ships BOTH, where the summary
|
||||
// takes the executable branch instead.
|
||||
test('a capability with both executable and instruction surfaces renders both', () => {
|
||||
const manifest = executableSurfaceManifest((m) => {
|
||||
m.skills = ['ui-phase'];
|
||||
m.agents = ['gsd-ui-checker'];
|
||||
});
|
||||
const d = trust.discloseExecutableSurfaces(manifest);
|
||||
assert.equal(d.hasExecutable, true, 'precondition: this manifest takes the executable branch');
|
||||
const section = trust.summarizeInstructionSurfaces(d);
|
||||
const summary = trust.summarizeDisclosure(d);
|
||||
// Skills-only: the declared `agents` entry is not collected, so this is 1 surface (the
|
||||
// skill), not 2 — header + 1 surface + the not-scanned line.
|
||||
assert.equal(section.length, 3, 'header + 1 surface + the not-scanned line');
|
||||
for (const line of section) {
|
||||
assert.ok(summary.includes(line), 'every instruction-surface line must reach the rendered summary');
|
||||
}
|
||||
});
|
||||
|
||||
// Row 26c — the CLI edge calls `summarizeDisclosure(res.disclosure || {})`
|
||||
// (gsd-core/bin/lib/capability-command-router.cjs), so a BARE `{}` carrying no arrays at all
|
||||
// reaches both renderers whenever a lifecycle result has no disclosure. Reading
|
||||
// `.instructionSurfaces.length` off that object unguarded would throw a TypeError at the consent
|
||||
// prompt — a crash on the exact path that is supposed to inform the user.
|
||||
test('a partial disclosure object from the CLI edge never throws', () => {
|
||||
assert.doesNotThrow(() => trust.summarizeInstructionSurfaces({}));
|
||||
assert.deepEqual(trust.summarizeInstructionSurfaces({}), []);
|
||||
assert.doesNotThrow(() => trust.summarizeDisclosure({}));
|
||||
assert.deepEqual(trust.summarizeDisclosure({}), ['This capability ships no executable surfaces (declarative only).']);
|
||||
});
|
||||
|
||||
// Row 26d — a manifest may declare an unbounded number of stems. `lines.push(...section)` would
|
||||
// exceed the engine's argument limit and throw RangeError here; the renderer must iterate. This
|
||||
// guards a "simplification" back to spread, which no smaller fixture can catch.
|
||||
test('an unbounded stem count does not break the renderer', () => {
|
||||
const stems = Array.from({ length: 200000 }, (_, i) => `s${i}`);
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: stems });
|
||||
assert.equal(d.instructionSurfaces.length, 200000);
|
||||
let summary;
|
||||
assert.doesNotThrow(() => {
|
||||
summary = trust.summarizeDisclosure(d);
|
||||
});
|
||||
assert.equal(summary.length, 200000 + 3, 'intro line + header + one line per stem + the not-scanned line');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── L. Cross-platform (row 28) ──────────────────────────────────────────────
|
||||
|
||||
describe('L. Cross-platform', () => {
|
||||
test('CRLF in a stem does not split the entry', () => {
|
||||
const stem = 'ui-phase\r\nwith-crlf';
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: [stem] });
|
||||
assert.deepEqual(
|
||||
d.instructionSurfaces,
|
||||
[{ kind: 'skill', name: stem }],
|
||||
'a CRLF inside a stem must be disclosed verbatim as ONE entry, never split into two',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── M. Property-based (fast-check) ─────────────────────────────────────────
|
||||
//
|
||||
// Generalizes the hand-written A-L fixtures with an ADVERSARIAL manifest arbitrary: valid string
|
||||
// stems mixed with non-strings, blanks, nested arrays, nested objects, nulls, and (via
|
||||
// `instructionFieldArb`'s low-weight branch) `skills`/`agents` occasionally replaced wholesale by a
|
||||
// non-array. `manifestArb` STILL GENERATES `agents` (good — hostile/adversarial input coverage),
|
||||
// but `agents` is never collected into `instructionSurfaces`: only `skills` feeds
|
||||
// `collectInstructionSurfaces`'s per-field loop (`INSTRUCTION_SURFACE_FIELDS` is a one-row table).
|
||||
// `manifestArb` always yields a plain object (never array/null/Proxy — those totality
|
||||
// cases are covered directly in section C/D) so P2/P3 can safely spread-and-delete `skills`/`agents`
|
||||
// off the SAME generated manifest, matching this file's `refDiscloseExecutableSurfaces`/section D's
|
||||
// established idiom of importing fast-check as `const fc = require('fast-check')` and calling
|
||||
// `fc.assert(fc.property(...))` with no per-call seed/numRuns override.
|
||||
|
||||
describe('M. Property-based (fast-check)', () => {
|
||||
const stringArb = fc.string();
|
||||
const blankArb = fc.constantFrom('', ' ', '\n', '\t');
|
||||
const nonStringStemArb = fc.oneof(
|
||||
fc.integer(),
|
||||
fc.boolean(),
|
||||
fc.constant(null),
|
||||
fc.constant(undefined),
|
||||
fc.array(stringArb, { maxLength: 3 }),
|
||||
fc.object({ maxDepth: 1 }),
|
||||
);
|
||||
const stemMemberArb = fc.oneof(
|
||||
{ weight: 3, arbitrary: stringArb },
|
||||
{ weight: 1, arbitrary: blankArb },
|
||||
{ weight: 1, arbitrary: nonStringStemArb },
|
||||
);
|
||||
const stemsArrayArb = fc.array(stemMemberArb, { maxLength: 6 });
|
||||
// Occasionally replace the whole field with a non-array (a scalar, null, or a plain object) —
|
||||
// exercises `collectInstructionSurfaces`'s "a non-array field declares nothing" branch.
|
||||
const instructionFieldArb = fc.oneof(
|
||||
{ weight: 5, arbitrary: stemsArrayArb },
|
||||
{ weight: 1, arbitrary: fc.oneof(stringArb, fc.integer(), fc.constant(null), fc.object({ maxDepth: 1 })) },
|
||||
);
|
||||
|
||||
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())) },
|
||||
{ requiredKeys: [] },
|
||||
);
|
||||
|
||||
// Always a plain object — P2/P3 rely on being able to spread it and delete skills/agents.
|
||||
const manifestArb = fc.record(
|
||||
{
|
||||
id: stringArb,
|
||||
hooks: fc.array(hookArb, { maxLength: 3 }),
|
||||
commands: fc.array(commandArb, { maxLength: 3 }),
|
||||
mcpServers: fc.dictionary(stringArb, mcpConfigArb),
|
||||
skills: instructionFieldArb,
|
||||
agents: instructionFieldArb,
|
||||
},
|
||||
{ requiredKeys: [] },
|
||||
);
|
||||
|
||||
/** `m` with `skills`/`agents` deleted — the D4 "sans instruction surfaces" comparison object. */
|
||||
function withoutInstructionFields(m) {
|
||||
const m2 = { ...m };
|
||||
delete m2.skills;
|
||||
delete m2.agents;
|
||||
return m2;
|
||||
}
|
||||
|
||||
test('P1: discloseExecutableSurfaces is total and every instruction surface is well-shaped', () => {
|
||||
fc.assert(
|
||||
fc.property(manifestArb, (manifest) => {
|
||||
let d;
|
||||
assert.doesNotThrow(() => {
|
||||
d = trust.discloseExecutableSurfaces(manifest);
|
||||
}, `discloseExecutableSurfaces threw for manifest=${JSON.stringify(manifest)}`);
|
||||
assert.ok(Array.isArray(d.instructionSurfaces), 'instructionSurfaces must always be an array');
|
||||
for (const surface of d.instructionSurfaces) {
|
||||
// Skills-only: `agents` is generated by the arbitrary but never collected, so every
|
||||
// disclosed instruction surface must be a skill.
|
||||
assert.equal(surface.kind, 'skill', `unexpected kind ${JSON.stringify(surface.kind)}`);
|
||||
assert.equal(typeof surface.name, 'string', `name must be a string, got ${typeof surface.name}`);
|
||||
assert.ok(surface.name.length > 0, 'name must be non-empty');
|
||||
}
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test('P2: ADR-2363 D4 — instruction surfaces never perturb the disclosure signature', () => {
|
||||
fc.assert(
|
||||
fc.property(manifestArb, (manifest) => {
|
||||
const m2 = withoutInstructionFields(manifest);
|
||||
assert.equal(
|
||||
trust.signatureForManifest(manifest),
|
||||
trust.signatureForManifest(m2),
|
||||
`signature diverged for manifest=${JSON.stringify(manifest)}`,
|
||||
);
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test('P3: ADR-2363 D3 — instruction surfaces never perturb hasExecutable', () => {
|
||||
fc.assert(
|
||||
fc.property(manifestArb, (manifest) => {
|
||||
const m2 = withoutInstructionFields(manifest);
|
||||
assert.equal(
|
||||
trust.discloseExecutableSurfaces(manifest).hasExecutable,
|
||||
trust.discloseExecutableSurfaces(m2).hasExecutable,
|
||||
`hasExecutable diverged for manifest=${JSON.stringify(manifest)}`,
|
||||
);
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test('P4: summarizeInstructionSurfaces is total and its length tracks instructionSurfaces.length', () => {
|
||||
fc.assert(
|
||||
fc.property(manifestArb, (manifest) => {
|
||||
const d = trust.discloseExecutableSurfaces(manifest);
|
||||
let section;
|
||||
assert.doesNotThrow(() => {
|
||||
section = trust.summarizeInstructionSurfaces(d);
|
||||
}, `summarizeInstructionSurfaces threw for manifest=${JSON.stringify(manifest)}`);
|
||||
if (d.instructionSurfaces.length === 0) {
|
||||
assert.deepEqual(section, []);
|
||||
} else {
|
||||
assert.equal(section.length, d.instructionSurfaces.length + 2);
|
||||
}
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── N. Consent-prompt injection safety ─────────────────────────────────────
|
||||
//
|
||||
// #3248 BLOCKER finding: `summarizeDisclosure`'s lines are joined with `\n` and written RAW to
|
||||
// stderr on the needs-consent path (`capability-command-router.cjs`). An unescaped newline in a
|
||||
// manifest-supplied value forged lines indistinguishable from genuine GSD disclosure text, and an
|
||||
// unescaped ANSI/control sequence could rewrite already-printed terminal lines. `renderValueForPrompt`
|
||||
// is the fix: every manifest-supplied value rendered into a consent-prompt line is escaped and
|
||||
// length-bounded first. The DISCLOSURE OBJECT itself still carries values VERBATIM (unchanged) —
|
||||
// only the RENDERED line is escaped.
|
||||
//
|
||||
// Assertions here are on TYPED values and STRUCTURAL properties only (array length, character-class
|
||||
// absence) — CONTRIBUTING.md forbids regex-matching rendered prose. Checking a rendered line for the
|
||||
// ABSENCE of specific control characters is a structural safety property, not a prose match.
|
||||
|
||||
describe('N. Consent-prompt injection safety', () => {
|
||||
// Every character that must never survive into a rendered consent-prompt line: C0, DEL, C1, the
|
||||
// bidi/isolate controls, and the line/paragraph separators. A raw newline forges a line that is
|
||||
// indistinguishable from genuine GSD disclosure text; a raw ESC lets a manifest value rewrite lines
|
||||
// already printed to the terminal. Defined independently of `src/capability-trust.cts`'s own
|
||||
// `UNSAFE_PROMPT_CHARS` (not imported) so this test does not just echo the implementation back at
|
||||
// itself — it is an independent restatement of the same forbidden-character contract.
|
||||
// eslint-disable-next-line no-control-regex -- deliberately matching C0/DEL/C1 control chars.
|
||||
const FORBIDDEN_IN_RENDERED_LINE = /[\u0000-\u001f\u007f-\u009f\u200e\u200f\u2028\u2029\u202a-\u202e\u2066-\u2069]/;
|
||||
|
||||
test('renderValueForPrompt is identity for an ordinary stem', () => {
|
||||
assert.equal(trust.renderValueForPrompt('ui-phase'), 'ui-phase');
|
||||
});
|
||||
|
||||
test('renderValueForPrompt escapes each hostile class', () => {
|
||||
const hostileInputs = [
|
||||
'\n', // C0 — line feed
|
||||
'\r\n', // C0 — CRLF
|
||||
'\x1b[2K', // C0 ESC — ANSI erase-line, can rewrite already-printed terminal output
|
||||
'
', // line/paragraph separator
|
||||
'', // bidi/isolate control — RIGHT-TO-LEFT OVERRIDE
|
||||
];
|
||||
for (const input of hostileInputs) {
|
||||
const result = trust.renderValueForPrompt(input);
|
||||
assert.equal(
|
||||
FORBIDDEN_IN_RENDERED_LINE.test(result),
|
||||
false,
|
||||
`renderValueForPrompt(${JSON.stringify(input)}) must contain no forbidden character, got ${JSON.stringify(result)}`,
|
||||
);
|
||||
}
|
||||
// The escaped form still contains the surrounding legible text, so the value stays
|
||||
// identifiable rather than vanishing.
|
||||
const escaped = trust.renderValueForPrompt('a\nb');
|
||||
assert.ok(escaped.includes('a'), 'escaped form must still contain the leading legible text');
|
||||
assert.ok(escaped.includes('b'), 'escaped form must still contain the trailing legible text');
|
||||
});
|
||||
|
||||
test('renderValueForPrompt bounds length', () => {
|
||||
const huge = 'x'.repeat(10000);
|
||||
const result = trust.renderValueForPrompt(huge);
|
||||
assert.ok(result.length < 10000, `expected a materially shorter result, got length ${result.length}`);
|
||||
});
|
||||
|
||||
test('a forged skill stem cannot inject a line', () => {
|
||||
const forged =
|
||||
'ok\n hooks (1): run as runtime hook commands\n - fake -> ok\nRe-run with --yes to grant consent.';
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: [forged] });
|
||||
const summary = trust.summarizeDisclosure(d);
|
||||
for (const line of summary) {
|
||||
assert.equal(
|
||||
FORBIDDEN_IN_RENDERED_LINE.test(line),
|
||||
false,
|
||||
`rendered line must contain no forbidden character: ${JSON.stringify(line)}`,
|
||||
);
|
||||
}
|
||||
// intro line + header + 1 surface + the not-scanned line — the forged text must not become
|
||||
// EXTRA array entries either (it stayed one skill, so it renders as exactly one surface line).
|
||||
assert.equal(summary.length, 4, 'intro + header + 1 surface + not-scanned line');
|
||||
});
|
||||
|
||||
// PARITY — the generative-fix-divergence guard (CLAUDE.md "Generative Fix Divergence"): the same
|
||||
// escaping guarantee must hold for every one of the five disclosed classes, not just skills. Every
|
||||
// rendered field of every class carries the same hostile payload; if a future class is added to the
|
||||
// renderer without routing its values through `renderValueForPrompt`, this test catches it instead
|
||||
// of that class silently shipping unescaped.
|
||||
test('PARITY — the same injection-safety guarantee holds for all five disclosed classes', () => {
|
||||
const payload = 'a\nb[2Kc';
|
||||
const manifest = {
|
||||
id: 'x',
|
||||
hooks: [{ event: payload, script: payload }],
|
||||
commands: [{ family: payload, module: payload, router: payload }],
|
||||
mcpServers: {
|
||||
[payload]: {
|
||||
command: payload,
|
||||
args: [payload],
|
||||
url: payload,
|
||||
cwd: payload,
|
||||
env: { [payload]: payload },
|
||||
},
|
||||
},
|
||||
reviewer: {
|
||||
slug: payload,
|
||||
transport: 'spawn',
|
||||
invoke: {
|
||||
binary: payload,
|
||||
args: [payload],
|
||||
hostConfigKey: payload,
|
||||
},
|
||||
handler: payload,
|
||||
},
|
||||
skills: [payload],
|
||||
};
|
||||
const d = trust.discloseExecutableSurfaces(manifest);
|
||||
const summary = trust.summarizeDisclosure(d);
|
||||
for (const line of summary) {
|
||||
assert.equal(
|
||||
FORBIDDEN_IN_RENDERED_LINE.test(line),
|
||||
false,
|
||||
`rendered line must contain no forbidden character: ${JSON.stringify(line)}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('the disclosure OBJECT stays verbatim', () => {
|
||||
// Escaping is a RENDERING concern only. The object must stay verbatim because
|
||||
// `disclosureSignature` and any consumer reasoning about identity depend on the declared
|
||||
// value, not the escaped-for-display one.
|
||||
const forged = 'ok\n hooks (1): run as runtime hook commands\nRe-run with --yes to grant consent.';
|
||||
const d = trust.discloseExecutableSurfaces({ id: 'x', skills: [forged] });
|
||||
assert.equal(d.instructionSurfaces[0].name, forged, 'the disclosure object must carry the exact unescaped value');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user