diff --git a/.gitignore b/.gitignore index 7b768feea..07d9cf3d9 100644 --- a/.gitignore +++ b/.gitignore @@ -363,3 +363,6 @@ reports/mutation/ # QA-walk report artifact (generated by tests/qa/run-report.cjs) qa-report.json + +# Scratch PR body files (gh pr create --body-file); never committed +.pr-body-*.md diff --git a/.pr-body-2100.md b/.pr-body-2100.md deleted file mode 100644 index 082c3cbe1..000000000 --- a/.pr-body-2100.md +++ /dev/null @@ -1,71 +0,0 @@ -## Linked Issue - -Closes #2100 - -The linked issue carries the `approved-feature` label. - ---- - -## Feature summary - -Migrates **Windsurf** onto the ADR-1239 Embeddable Orchestration System — the largest of the EoS migrations. Folds all 10 residual `isWindsurf` branches onto the capability descriptor **and** wires GSD's write/command safety guards into Windsurf/Cascade's native blocking hook bus. - -## What changed (highlights) - -| File | What changed | -|------|-------------| -| `bin/install.js` | Folded 10 `isWindsurf` sites onto `hostBehaviors` (skipSharedHooksInstall, legacyDevinSkillsCleanup, installsCommandBodiesForWorkflowDelegation [#1629], verificationStyle); dropped 2 dead destructures + the dead `else if (isWindsurf)` agent arm; wired the Cascade hook-bridge install/uninstall; corrected stale comments | -| `capabilities/windsurf/capability.json` | `hostBehaviors` block; `hooksSurface: "none"` → `"windsurf-hooks-json"` | -| `src/runtime-hooks-surface.cts` | `writeWindsurfHooksJson`/`reconcileWindsurfHooksJson`/`removeWindsurfHooksJson` (Cursor-templated, Cascade's flat `{hooks:{:[{command}]}}` shape) + event/script constants | -| `hooks/gsd-windsurf-pre-write.js`, `gsd-windsurf-pre-command.js` | **New** Cascade-native blocking guard scripts (stdin JSON, exit-code-2 blocking) | -| `gsd-core/bin/lib/capability-validator.cjs`, `src/runtime-config-adapter-registry.cts` | `windsurf-hooks-json` added to `VALID_HOOKS_SURFACES`, GATE A's `profile-marker-only` allowlist, and the `HooksSurface` union | -| managed-hooks-registry / build-hooks / INVENTORY | registered the 2 new guard scripts | -| tests / docs / changeset | `declarative-reference-windsurf` + `windsurf-hooks-bridge` (live blocking); matrix hookBus delta; changeset (`Changed`) | - -## Implementation notes - -- **Byte-parity concretely verified** (via the review): a real windsurf install rebuilt through the golden-parity harness → 327 files, 0 drift; only `windsurf.json` gains the 2 new script hashes. cursor/trae re-verified 0 drift. The load-bearing #1629 command-body copy (`.windsurf/gsd-core/commands/gsd/*.md` for local installs) is intact. -- **The hook-bridge is faithful, not padding.** Cascade's `hooks.json` genuinely supports blocking via exit code 2 (confirmed against docs.windsurf.com / docs.devin.ai). Only **2 of GSD's 6 guards** faithfully map — the worktree-path guard (→ `pre_write_code`) and a destructive-command guard (→ `pre_run_command`). The 4 advisory guards + `pre_mcp_tool_use` + the 5 `post_*` logging events are **deliberately not wired**: Cascade's hook bus has no context-injection channel to carry GSD's advisory reminders faithfully, and GSD has no MCP-tool policy — porting them would be non-functional padding. This is the same faithful-subset pattern used for codebuddy #2098 / copilot #2099, and it satisfies AC4's testable requirement ("a real blocking hook rejecting a disallowed write/command"). -- **Security (reviewed, clean).** The guard scripts parse untrusted stdin and spawn `git rev-parse` — command injection via `file_path` was **refuted** (argv array, no `shell:true`, PATH-resolved git). No traversal / prototype-pollution (frozen 2-event set, fixed script names). The pre-command guard was **hardened post-review**: a tokenize-based classifier (no catastrophic-backtracking regex — a 200k-char pathological input now completes in ~32ms via a 4096-char cap), catching prefixed `rm -rf` forms (`sudo`/`env`/`/bin/rm`) and refspec force-pushes (`HEAD:main`, `+main`), and a fail-closed false-positive fixed (a `feature/main-fix` branch or a trailing-`# ...main` comment no longer wrongly blocks a legit force-push). Guards fail-open (never wedge Cascade) by design. -- **Golden mechanics.** `.windsurf/hooks.json` is golden-excluded by basename (like settings.json); the 2 guard scripts under `hooks/` are windsurf-specific → only windsurf.json regenerates, additively. - -## Spec compliance (acceptance criteria) - -- [x] Golden parity: byte-identical for the folds across all 16 runtimes (windsurf.json regen is the additive hook-script delta only) -- [x] Driven through the descriptor — zero live `runtime==='windsurf'`/`isWindsurf` branches (AC2 guard over 4 files) -- [x] Every axis populated + `capability-validator`-clean (`runtime`/dispatch stay `undocumented` per the cited search trail) -- [x] UPGRADE implemented AND exercised by a test driving a real blocking hook (exit-2 on a disallowed write/command) -- [x] `negotiateHostCapabilities` fail-closes for windsurf (test) -- [x] `gsd-test` green (linux node22/24); no other-runtime regression (cursor.json byte-identical) -- [x] Docs (matrix hookBus delta) + changeset (`Changed`) - -## Testing - -- [x] macOS (real install byte-parity harness + live guard-script exit-2 probing + ReDoS timing) -- [x] Windows (backslash; Windows destructive-command forms handled) — GitHub CI -- [x] Linux (`gsd-test`) -- [x] Runtimes: Windsurf (primary) + all 16 golden fixtures (only windsurf's 2 new scripts) - ---- - -## Scope confirmation - -- [x] Windsurf only; other runtimes byte-identical. The hook-bridge's faithful 2-guard scope (vs. the AC's fuller event list) is disclosed above — the unbridged events have no faithful GSD logic / Cascade channel. -- [x] Cascade envelope/schema is best-effort per the official docs (guards fail-open if the live schema differs, never breaking Cascade); flagged for a live-Cascade schema confirmation follow-up. - -## Documentation - -- [x] matrix (## windsurf hookBus/hooksSurface delta + the not-ported-guards rationale); English - -## Checklist - -- [x] `Closes #2100`; issue has `approved-feature` -- [x] Acceptance criteria met (faithful hook-bridge scope disclosed) -- [x] `gsd-test` green -- [x] New tests cover the folds (AC2 guard) + the blocking hook bus (live exit-2) + fail-closed negotiation -- [x] `.changeset/` fragment (`Changed`) -- [x] No new dependencies - -## Breaking changes - -None at landing. New Windsurf install output is additive: 2 guard scripts + a `.windsurf/hooks.json` registering blocking pre-hooks. No skill, agent, workflow, or path is removed or altered; the guards fail-open. diff --git a/.pr-body-3514.md b/.pr-body-3514.md deleted file mode 100644 index 248e99da9..000000000 --- a/.pr-body-3514.md +++ /dev/null @@ -1,73 +0,0 @@ -## Fix PR - -> **Using the wrong template?** -> — Enhancement: use [enhancement.md](?template=enhancement.md) -> — Feature: use [feature.md](?template=feature.md) - ---- - -## Linked Issue - -> **Required.** This PR will be auto-closed if no valid issue link is found. - -Fixes #3514 - -> The linked issue must have the `confirmed-bug` label. If it doesn't, ask a maintainer to confirm the bug before continuing. - ---- - -## What was broken - -Three hardening gaps at the ADR-1244 D3/D5 edges (epic #1900, finding F21): the capability URL-import fetcher never validated the resolved host (a loopback or cloud-metadata **https** endpoint was fetched like any URL); an `http://` tarball spec failed with a raw `ERR_INVALID_PROTOCOL` instead of a named reason; and an install without an integrity pin produced a consent prompt that did not distinguish verified from unverified content. - -## What this fix does - -- **Pre-transport fetch gate** (`realHttpsGet` → `assertFetchableUrl`): loopback/link-local/metadata/unspecified hosts (`127/8`, `169.254/16` incl. `169.254.169.254`, `0/8`, `::1`, `fe80::/10`, `::`, IPv4-mapped spellings in both dotted and hex-normalized forms) and `localhost`/`*.localhost` names are denied with a named error **before any bytes leave** — the injected transport is provably never invoked (tests assert call-count 0). The WHATWG URL parser normalizes alternate IP spellings (decimal `2130706433`, hex `0x7f000001`, octal, short forms) to dotted-quad before the gate sees them — locked by tests. -- **Non-https, per the adopted split decision**: `parseSpec` still *classifies* `http://` tarball specs (internal-mirror flows are not broken at parse); the fetcher refuses with a clear https-only reason naming the mirror alternative. -- **Unverified-integrity disclosure**: `evaluateInstallTrust` accepts `integrityPinned` (a verified `--integrity` pin, or a git `#sha:` ref); the consent prompt renders `content: NO PINNED HASH — staged unverified` when no pin was supplied (and the pinned counterpart when one was). The status is **prompt-only** — deliberately excluded from `disclosureSignature` (consent's content binding is `bundleContentHash`, #1459; tests lock that it can never fire a spurious re-consent). Legacy callers see byte-identical output. - -## Root cause - -The D3 fetcher was built with a byte cap and a timeout but no host policy — the finding is an edge the pipeline's safe posture simply wasn't applied to. The integrity line was missing because the disclosure was built from the manifest alone; the pin fact lives on the resolve path, and nothing threaded it into the verdict. - -## Testing - -### How I verified the fix - -- Failing-first: `gsd-test` at the tests-only commit (holodeck, linux-node24) — verdict `failed` with exactly the 20 expected failures (12-case denylist matrix, https-reason, trust/lifecycle integrity suites), 34,343 green. -- GREEN: full `gsd-test` matrix at the final HEAD — verdict below. -- Controls lock the negative space: a public host and a private-range mirror still fetch through the gate; `parseSpec` still classifies `http://`; legacy `summarizeDisclosure` output is line-identical. - -### Regression test added? - -- [x] Yes — added a test that would have caught this bug - -### Platforms tested - -- [x] macOS -- [ ] Windows (including backslash path handling) -- [x] Linux - -### Runtimes tested - -- [ ] Claude Code -- [ ] Gemini CLI -- [ ] OpenCode -- [ ] Other: ___ -- [x] N/A (not runtime-specific — engine-internal source resolver + trust gate) - ---- - -## Checklist - -- [x] Issue linked above with `Fixes #3514` — **PR will be auto-closed if missing** -- [x] Linked issue has the `confirmed-bug` label -- [x] Fix is scoped to the reported bug — no unrelated changes included -- [x] Regression test added (or explained why not) -- [x] All existing tests pass (`npm test`) — full `gsd-test` matrix at final HEAD -- [x] `.changeset/` fragment added — `Security` type -- [x] No unnecessary dependencies added - -## Breaking changes - -None intended. `http://` tarball URLs already failed at the transport; they now fail with a better message. Denied hosts (loopback/link-local/metadata/localhost) are not legitimate install sources. **Known limits, documented** in `docs/explanation/capability-trust-model.md`: RFC1918 private ranges are deliberately *not* denied (internal mirrors); the check is on the URL's host literal — DNS rebinding is out of scope. Local/unpinned-git installs render the unverified line even though their trust basis is the path/commit (honest, slightly noisy). diff --git a/.pr-body-3515.md b/.pr-body-3515.md deleted file mode 100644 index 78e2339ce..000000000 --- a/.pr-body-3515.md +++ /dev/null @@ -1,71 +0,0 @@ -## Fix PR - -> **Using the wrong template?** -> — Enhancement: use [enhancement.md](?template=enhancement.md) -> — Feature: use [feature.md](?template=feature.md) - ---- - -## Linked Issue - -> **Required.** This PR will be auto-closed if no valid issue link is found. - -Fixes #3515 - -> The linked issue must have the `confirmed-bug` label. If it doesn't, ask a maintainer to confirm the bug before continuing. - ---- - -## What was broken - -The capability consent prompt never stated the hooks-vs-MCP confinement asymmetry: hook commands are confined to the capability bundle (ADR-1244 D5 rule 5), but an MCP server's `command`/`args`/`env`/`cwd` are written verbatim and may point anywhere on the machine. The asymmetry is intentional (per the maintainer decision on the epic — most real MCP servers legitimately resolve to global/npx installs, so confinement would break them), but unstated, so the consent was not informed about it. - -## What this fix does - -The document+disclose arm of the decision (no confinement machinery): the consent disclosure's MCP section now renders one explicit notice for every **spawned** (stdio) server — *"intentionally NOT confined to the bundle: a server's command, args, env, and cwd are written verbatim and may point anywhere on this machine — unlike hooks, which are confined to the capability bundle root"*. Remote-only (http/sse) servers render no notice (nothing local is spawned — the claim stays exact), decided by one shared `isRemoteMcpServer` predicate also used for the per-server rendering so the two cannot drift. The lifecycle's MCP write path documents the intentional asymmetry in code, cross-referencing the notice and the existing re-consent binding (`disclosureSignature` already folds command/args/env/cwd + full `rawConfig`, #1459 — any change forces re-consent). - -## Root cause - -The D5 hook confinement (rule 5) postdates the MCP write path; the asymmetry was deliberate but was never carried into the human disclosure, so the prompt showed MCP servers without saying their posture differs from the hooks listed right above them. - -## Testing - -### How I verified the fix - -- Failing-first: `gsd-test` at the tests-only commit — verdict below. -- GREEN: full `gsd-test` matrix at the final HEAD — verdict below. -- A GOLDEN signature test locks the consent signature's exact bytes for a spawned-server manifest — the notice provably introduces no new disclosure state, so no already-consented install can be spuriously re-prompted. - -### Regression test added? - -- [x] Yes — added a test that would have caught this bug - -### Platforms tested - -- [x] macOS -- [ ] Windows (including backslash path handling) -- [x] Linux - -### Runtimes tested - -- [ ] Claude Code -- [ ] Gemini CLI -- [ ] OpenCode -- [ ] Other: ___ -- [x] N/A (not runtime-specific — trust-gate prompt renderer) - ---- - -## Checklist - -- [x] Issue linked above with `Fixes #3515` — **PR will be auto-closed if missing** -- [x] Linked issue has the `confirmed-bug` label -- [x] Fix is scoped to the reported bug — no unrelated changes included -- [x] Regression test added (or explained why not) -- [x] All existing tests pass (`npm test`) — full `gsd-test` matrix at final HEAD -- [x] `.changeset/` fragment added — `Security` type -- [x] No unnecessary dependencies added - -## Breaking changes - -None — no behavior change anywhere; one added prompt line (spawned MCP servers only), comments, and documentation.