chore: remove stray .pr-body-*.md scratch files from repo root (#4710)
.pr-body-2100.md, .pr-body-3514.md and .pr-body-3515.md are scratch PR body files produced by `gh pr create --body-file`. They were accidentally committed alongside their own feature/fix commits (bd613566cb,6badb839a0,e57918a648) and have no place in the tree. They are unreferenced: no literal or glob reference anywhere in the repo, absent from docs/INVENTORY-MANIFEST.json, and outside the package.json `files:` surface, so they never shipped and no install-tree golden covers them. The security disclosures the #3514 and #3515 bodies describe are already documented in docs/explanation/capability-trust-model.md (lines 218 and 307-308), so nothing unique is lost. Add a `.pr-body-*.md` .gitignore rule so the same scratch files cannot be committed again. Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
3
.gitignore
vendored
3
.gitignore
vendored
@@ -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
|
||||
|
||||
@@ -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:{<event>:[{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.
|
||||
@@ -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:<commit>` 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).
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user