diff --git a/.changeset/happy-bears-gather.md b/.changeset/happy-bears-gather.md new file mode 100644 index 000000000..dd4170e4c --- /dev/null +++ b/.changeset/happy-bears-gather.md @@ -0,0 +1,5 @@ +--- +type: Security +pr: 2478 +--- +**Patched a transitive denial-of-service advisory in the production dependency tree** — `body-parser` reached GSD via the Claude Agent SDK's MCP dependency and, on versions through 2.2.2, silently stopped enforcing request size limits when given an invalid limit value (GHSA-v422-hmwv-36x6). Pinned to >=2.3.0. (#2470) diff --git a/.changeset/noble-newts-roam.md b/.changeset/noble-newts-roam.md new file mode 100644 index 000000000..8362aeccc --- /dev/null +++ b/.changeset/noble-newts-roam.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2478 +--- +**`/gsd` now registers on pi** — installing GSD for pi wrote its extension as `gsd.cjs`, a suffix pi's extension auto-discovery skips silently, so `/gsd` never appeared and nothing reported an error. The extension now installs as `gsd.js`, and upgrading removes the stale `gsd.cjs`. (#2470) diff --git a/.gitignore b/.gitignore index 98e345a05..53c018a18 100644 --- a/.gitignore +++ b/.gitignore @@ -152,6 +152,7 @@ build/ /gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs /gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs /gsd-core/bin/lib/installer-migrations/005-opencode-baseline-commands-dir.cjs +/gsd-core/bin/lib/installer-migrations/006-pi-extension-cjs-to-js.cjs /gsd-core/bin/lib/observability/logger.cjs /gsd-core/bin/lib/active-workstream-store.cjs /gsd-core/bin/lib/adr-parser.cjs diff --git a/bin/install.js b/bin/install.js index d219cefce..bb05b8a2d 100755 --- a/bin/install.js +++ b/bin/install.js @@ -10524,7 +10524,8 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } else if (_hostBehaviors(runtime).pluginOnlyInstall) { // pi (ADR-1239 / #2102 Stage 1): plugin-only install — pi's /gsd command is // registered programmatically by the native extension (pi/gsd.cjs → - // extensions/gsd.cjs, staged separately below) and dispatches in-process + // extensions/gsd.js, staged separately below; the dest suffix must be + // .ts/.js or pi's auto-discovery skips it silently — #2470) and dispatches in-process // through the embedded gsd-core command-routing hub. pi has no host-read // markdown surface (unlike Claude/OpenCode/etc., which scan commands/ or // command/ directories), so writing flat gsd-.md files here would be diff --git a/capabilities/pi/capability.json b/capabilities/pi/capability.json index a48edcfd8..045198a5e 100644 --- a/capabilities/pi/capability.json +++ b/capabilities/pi/capability.json @@ -3,7 +3,7 @@ "role": "runtime", "version": "1.7.0", "title": "pi", - "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.cjs; no shared-settings hook surface; tier-2 support.", + "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", "requires": [], "engines": { @@ -51,7 +51,7 @@ "hostBehaviors": { "nativePlugin": { "dir": "extensions", - "file": "gsd.cjs", + "file": "gsd.js", "source": "pi/gsd.cjs" }, "pluginOnlyInstall": true diff --git a/docs/how-to/install-on-your-runtime.md b/docs/how-to/install-on-your-runtime.md index 4b77719ed..830e37cf7 100644 --- a/docs/how-to/install-on-your-runtime.md +++ b/docs/how-to/install-on-your-runtime.md @@ -469,7 +469,9 @@ npx @opengsd/gsd-core@latest --pi --global [pi](https://pi.dev) is a bun-runtime programmatic CLI whose extensions implement pi's own `ExtensionAPI` (`registerCommand`/`registerTool`/`registerProvider`/`pi.on`) rather than a settings-file or slash-markdown surface. GSD ships a single native-extension file: -- **Extension** → `~/.pi/agent/extensions/gsd.cjs` (global) or `.pi/extensions/gsd.cjs` (local) +- **Extension** → `~/.pi/agent/extensions/gsd.js` (global) or `.pi/extensions/gsd.js` (local) + +The `.js` suffix is load-bearing: pi auto-discovers extensions by scanning that directory and keeping only names ending in `.ts` or `.js`, and it skips anything else **silently** — no error, no log line. GSD shipped the file as `gsd.cjs` through 1.7.0, which pi therefore never loaded, so `/gsd` never appeared ([#2470](https://github.com/open-gsd/gsd-core/issues/2470)). Upgrading removes the stale `gsd.cjs`; if you had added a manual `extensions` entry in `~/.pi/agent/settings.json` as a workaround, you can drop it. The extension registers a `/gsd` command and a `gsd_invoke` tool that dispatch GSD commands via a bounded subprocess call to `gsd-core/bin/gsd-tools.cjs` (no fully-populated in-process command-routing hub exists — see the matrix's Stage 2 note). This is a **plugin-only install**: pi has no shared-settings hook surface (`hooksSurface: none`) and, unlike Claude/OpenCode/Kilo, no host-read markdown surface at all — pi's `/gsd` command is registered programmatically by the extension, not discovered from files, so GSD installs the extension plus its universal `gsd-core/` engine payload and the shared `hooks/`/`hooks/lib/` bundle (spawned by the extension itself, not by any config-file hook bus), and does **not** write any `commands/`, `agents/`, or `skills/` directory for pi. The extension bridges GSD's `session_start`/`before_agent_start`/`session_before_compact`/`tool_call` lifecycle events to those staged `hooks/` scripts as bounded, fail-open subprocesses, and steers pi's active model (`modelMode: active`) to a tier-resolved bare anthropic id via `pi.on('before_provider_request', ...)`. See the [`## pi`](host-integration-capability-matrix.md#pi) section of the host-integration capability matrix for the negotiated axes and citations. diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 6fdae1b39..d15cb82e8 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -378,6 +378,7 @@ for the new shape before changing migration behavior. | Qwen Code | Claude-compatible skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; optional common hook/settings integration through GSD | Global `QWEN_CONFIG_DIR` or `~/.qwen`; local `./.qwen` | GSD owns generated skills/agents/hooks and GSD settings entries only | [Qwen commands and skills](https://qwenlm.github.io/qwen-code-docs/en/users/features/commands/); docs last updated 2026-05-06 | | Hermes Agent | Category skills under `skills/gsd/` with `DESCRIPTION.md` plus nested `gsd-*/SKILL.md`; agents in `agents/`; optional common hook/settings integration through GSD | Global `HERMES_HOME` or `~/.hermes`; local `./.hermes` | GSD owns generated `skills/gsd/` category content, generated agents, and GSD settings entries only | [Hermes configuration](https://hermes-agent.nousresearch.com/docs/user-guide/configuration), [Hermes skills](https://hermes-agent.nousresearch.com/docs/zh-Hans/user-guide/features/skills), [working with skills](https://hermes-agent.nousresearch.com/docs/guides/work-with-skills); docs checked 2026-05-11 | | CodeBuddy | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; optional common hook/settings integration through GSD | Global `CODEBUDDY_CONFIG_DIR` or `~/.codebuddy`; local `./.codebuddy` | GSD owns generated skills/agents/hooks and GSD settings entries only | [CodeBuddy CLI skills](https://www.codebuddy.ai/docs/cli/skills), [CodeBuddy IDE skills](https://www.codebuddy.ai/docs/ide/Features/Skills); docs checked 2026-05-11 | +| pi | A single native extension at `extensions/gsd.js` (registers `/gsd` + the `gsd_invoke` tool programmatically); the shared `hooks/` + `hooks/lib/` bundle the extension spawns as bounded subprocesses; the `gsd-core/` payload. No commands/agents/skills surface (`pluginOnlyInstall`) | Global `~/.pi/agent`; local `./.pi` | GSD owns only the generated extension file, the installed `hooks/`/`hooks/lib/` bundle, and the `gsd-core/` payload. GSD writes **no** pi config: `configFormat: "none"`, `hooksSurface: "none"`, `writesSharedSettings: false` — `~/.pi/agent/settings.json` is entirely user-owned and must never be rewritten, including its `extensions` array. Other users' extensions in `extensions/` are unknown files and are preserved | [pi extension loader](https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/loader.ts): `discoverExtensionsInDir()` scans `/extensions/` and keeps only names passing `isExtensionFile()` (`.ts`/`.js`); accepted files load through `jiti`, which handles CommonJS and ESM alike, so the suffix — not the module format — is what gates discovery. Explicit paths in `settings.json` bypass the filter. Source read 2026-07-20 against `@earendil-works/pi-coding-agent` 0.80.10 (#2470) | | Cline | Rule-based integration via `.clinerules` for current installer output | Global `CLINE_CONFIG_DIR` or `~/.cline`; local project root `.clinerules` | GSD owns the generated `.clinerules` file only when it created or manifest-tracked it; no hooks/statusline ownership | [Cline rules](https://docs.cline.bot/customization/cline-rules); docs prefer `.clinerules/` directory and still detect legacy rule files, checked 2026-05-11 | ### Registry Authoring Rules @@ -502,6 +503,7 @@ Each row corresponds to one migration record in `src/installer-migrations/`. | `2026-06-02-rename-get-shit-done-to-gsd-core` | `003-rename-get-shit-done-to-gsd-core.cts` | 1.2.0 | global, local | Yes | Removes managed files from the stale `get-shit-done/` runtime directory after the rename to `gsd-core/` (#604). User-added files are preserved; emptied directories may remain (framework limitation). | | `2026-06-09-prune-stale-pristine-get-shit-done` | `004-prune-stale-pristine-snapshots.cts` | 1.4.3 | global, local | Yes | Removes stale `gsd-pristine/get-shit-done/` snapshot files left behind by migration 003, which caused false `verify-reapply-patches` failures (#934). | `2026-07-17-opencode-baseline-commands-dir` | `005-opencode-baseline-commands-dir.cts` | 1.7.0 | global, local | No | Baselines pre-existing files under OpenCode's `commands/` (plural) directory during the first-time scan. #2329 moved OpenCode command materialization to `commands/`, but 000's `RUNTIME_SURFACES.opencode` is a shipped, immutable body that still only names the legacy `command/` alias, so this fix-forward migration widens the scanned surface. OpenCode only; Kilo is unaffected. | +| `2026-07-20-pi-extension-cjs-to-js` | `006-pi-extension-cjs-to-js.cts` | 1.7.1 | global, local | Yes | Removes the stale `extensions/gsd.cjs` left by pre-#2470 pi installs. pi's extension auto-discovery (`isExtensionFile()`) accepts only `.ts`/`.js`, so the `.cjs` file was never loaded and `/gsd` never registered; #2470 renamed the installed artifact to `extensions/gsd.js`, orphaning the old path. Locally modified copies are backed up rather than deleted; an unmanifested `gsd.cjs` is preserved as a user file. pi only. | ## Prior Art diff --git a/docs/reference/host-integration-capability-matrix.md b/docs/reference/host-integration-capability-matrix.md index 819e1fead..b5f8d0b3e 100644 --- a/docs/reference/host-integration-capability-matrix.md +++ b/docs/reference/host-integration-capability-matrix.md @@ -657,7 +657,7 @@ EoS migration status (#2101, ADR-1239): ZCode's install is fully dogfooded throu ## pi -> pi (pi.dev) is a bun-runtime Programmatic-CLI: it exposes an in-process TypeScript `ExtensionAPI` (`registerCommand`/`registerTool`/`registerProvider`/`pi.on`) rather than a settings-file or slash-markdown surface. GSD ships a single native-extension file (`pi/gsd.cjs`) installed to `~/.pi/agent/extensions/gsd.cjs` (global) or `.pi/extensions/gsd.cjs` (local) — the programmatic-CLI peer of the OpenCode/Kilo native-plugin binding. **Sourcing note:** the citations below are the pi.dev documentation pages named in ADR-1239 Stage 1 (#2102) as the source for each axis; this environment did not have live doc-fetch access at authoring time, so the Evidence column below is a paraphrase of pi's documented extension model rather than a verbatim excerpt — a maintainer with Context7/web access should verify the exact wording before treating this section as fully cited (flagged in the #2102 PR). +> pi (pi.dev) is a bun-runtime Programmatic-CLI: it exposes an in-process TypeScript `ExtensionAPI` (`registerCommand`/`registerTool`/`registerProvider`/`pi.on`) rather than a settings-file or slash-markdown surface. GSD ships a single native-extension file (`pi/gsd.cjs`) installed to `~/.pi/agent/extensions/gsd.js` (global) or `.pi/extensions/gsd.js` (local) — the programmatic-CLI peer of the OpenCode/Kilo native-plugin binding. **Sourcing note:** the citations below are the pi.dev documentation pages named in ADR-1239 Stage 1 (#2102) as the source for each axis; this environment did not have live doc-fetch access at authoring time, so the Evidence column below is a paraphrase of pi's documented extension model rather than a verbatim excerpt — a maintainer with Context7/web access should verify the exact wording before treating this section as fully cited (flagged in the #2102 PR). **Partially discharged (#2470, 2026-07-20):** pi's extension-loader contract specifically has now been read at source — `packages/coding-agent/src/core/extensions/loader.ts` in `earendil-works/pi` — confirming `discoverExtensionsInDir()` keeps only names passing `isExtensionFile()` (`.ts`/`.js`, everything else skipped silently), that accepted files load via `jiti` (CommonJS and ESM alike), and that explicit `settings.json` paths bypass the filter. The remaining axes below are still paraphrase. | Axis | Value | Source | Evidence | |---|---|---|---| @@ -685,11 +685,11 @@ Documentation gaps: - dispatch.maxDepth / dispatch.background / dispatch.backgroundDispatch — recorded as `0`/`false`/`false` (not `undocumented`) because the absence of any dispatch primitive is itself the documented ceiling, matching `shouldFlattenDispatch`'s fail-closed default. - This section's Evidence-column wording was authored without live Context7/web-fetch access (see the sourcing note above the table) — verify against the cited pi.dev pages before relying on it for a future capability upgrade. -EoS migration status (#2102 Stage 1, ADR-1239): pi lands as a NET-NEW installable runtime — pure additive descriptor + installer wiring, no prior `runtime === 'pi'` branches existed to fold. `artifactLayout` is declared empty (`global: []`, `local: []`) — pi has no skills/commands/agents layout, and installs as **PLUGIN-ONLY**: `hostBehaviors.pluginOnlyInstall: true` explicitly skips `bin/install.js`'s generic flat-commands-and-agents fallback (the legacy path Claude Code's LOCAL layout also uses), which would otherwise write inert `commands/gsd-.md` + `agents/gsd-.md` reference files no part of pi ever reads. pi's `/gsd` command and `gsd_invoke` tool are registered **programmatically** by the native extension (`pi/gsd.cjs` → `extensions/gsd.cjs`, mirroring OpenCode/Kilo's `nativePlugin` shape) — pi has no host-read markdown surface at all (unlike Claude/OpenCode/Kilo, which scan a `commands/`/`command/` directory), so a declarative artifact surface would be dead weight, not merely unused. `dispatch.subagentToolkit: "undocumented"` and `dispatch.backgroundDispatch: false` are both required by the capability validator's dispatch schema and reflect that pi has no documented named-dispatch primitive at all. (Stage 1 originally also set `hostBehaviors.skipSharedHooksInstall:true`, reasoning the staged `hooks/*.js` bundle would be dead weight for pi the way it genuinely is for Kilo/ZCode — **corrected in Stage 2 below**: pi's native extension DOES spawn them, so they are live, not dead, and the flag was removed.) +EoS migration status (#2102 Stage 1, ADR-1239): pi lands as a NET-NEW installable runtime — pure additive descriptor + installer wiring, no prior `runtime === 'pi'` branches existed to fold. `artifactLayout` is declared empty (`global: []`, `local: []`) — pi has no skills/commands/agents layout, and installs as **PLUGIN-ONLY**: `hostBehaviors.pluginOnlyInstall: true` explicitly skips `bin/install.js`'s generic flat-commands-and-agents fallback (the legacy path Claude Code's LOCAL layout also uses), which would otherwise write inert `commands/gsd-.md` + `agents/gsd-.md` reference files no part of pi ever reads. pi's `/gsd` command and `gsd_invoke` tool are registered **programmatically** by the native extension (`pi/gsd.cjs` → `extensions/gsd.js`, mirroring OpenCode/Kilo's `nativePlugin` shape) — pi has no host-read markdown surface at all (unlike Claude/OpenCode/Kilo, which scan a `commands/`/`command/` directory), so a declarative artifact surface would be dead weight, not merely unused. `dispatch.subagentToolkit: "undocumented"` and `dispatch.backgroundDispatch: false` are both required by the capability validator's dispatch schema and reflect that pi has no documented named-dispatch primitive at all. (Stage 1 originally also set `hostBehaviors.skipSharedHooksInstall:true`, reasoning the staged `hooks/*.js` bundle would be dead weight for pi the way it genuinely is for Kilo/ZCode — **corrected in Stage 2 below**: pi's native extension DOES spawn them, so they are live, not dead, and the flag was removed.) EoS migration status (#2102 Stage 2, ADR-1239): Stage 1's "in-process `gsd-core` command-routing hub" framing was aspirational and is corrected here — no fully-populated hub factory exists anywhere in gsd-core (every `createHub()` caller in the tree builds a single-family hub for its own narrow purpose), so `/gsd` and `gsd_invoke` instead dispatch via **SUBPROCESS REUSE**: `dispatchGsdCommand` (`src/shell-command-projection.cts`) spawns `gsd-core/bin/gsd-tools.cjs [subcommand] ... --cwd --raw --json-errors` bounded and non-throwing, mirroring the precedent already established for the OpenCode/Kilo hook bridge (`.opencode/plugins/gsd-core.js`'s "Architecture: SUBPROCESS REUSE" header). The companion MCP server's `gsd_invoke_command` tool dispatches through the SAME shared helper (it had the identical `createHub()`-with-no-args bug). `/gsd`'s command handler is `handler(args, ctx)` (pi's real ExtensionAPI shape — a raw args string, not `execute(ctx)`); `gsd_invoke`'s tool handler is the real 5-arg `execute(toolCallId, params, signal, onUpdate, ctx)`. The event surface (`EXTENSION_EVENT_SURFACES.pi`, `src/host-integration.cts`) now declares the full ~30-event pi ExtensionAPI vocabulary (was a placeholder `['tool_call']`), and `pi/gsd.cjs` binds `session_start` (→ `gsd-ensure-canonical-path.js`), `before_agent_start` (→ `gsd-workflow-guard.js`, a forward-compatible no-op today since that hook's triggers are tool-scoped), `session_before_compact` (→ `gsd-context-monitor.js`), and `tool_call`, each as a bounded fail-open `spawnSync` subprocess (mirroring `.opencode/plugins/gsd-core.js`'s `runHook`). `modelMode: active` is realized via `pi.on('before_provider_request', ...)`, which resolves a tier through the model-catalog's now-populated `runtimeTierDefaults.pi` entries (bare anthropic ids — `claude-opus-4-8`/`claude-sonnet-5`/`claude-haiku-4-5`, matching the `claude` runtime's own ids since pi talks the anthropic API) and returns a modified payload, or `undefined` (fail-open, pi's model left untouched) when resolution comes back null — **not** `registerProvider`, which would register a new model provider rather than steering pi's existing built-in anthropic models. -**Adversarial-review correction (#2102 Stage 2, post-review):** the event bridges above and the `/gsd` tokenizer's `hooks/lib/git-cmd.js` require were DEAD in a real install — Stage 1's `hostBehaviors.skipSharedHooksInstall:true` meant pi shipped NO `hooks/` directory at all, so `runHook('gsd-ensure-canonical-path.js', ...)` etc. always hit the "hook file absent → silent no-op" branch, and the tokenizer always fell back to plain whitespace-splitting. The tests masked this because they run against the dev tree, where `hooks/` genuinely exists. **Fix:** `capabilities/pi/capability.json` no longer sets `skipSharedHooksInstall` — pi is architecturally identical to OpenCode here (`hooksSurface: "none"` + a native extension that spawns the staged hooks), not to Kilo/ZCode (`hooksSurface: "none"` with NO plugin surface, where the same hooks genuinely are dead weight). pi now installs `hooks/` + `hooks/lib/` (27 entries: the same `INSTALLED_HOOK_FILES` set OpenCode gets) alongside `extensions/gsd.cjs`, verified end-to-end via a real `node bin/install.js --pi --global`/`--local` — `resolveEngineRoot`'s walk-up from the installed extension's own directory finds `ENGINE_ROOT/hooks/{gsd-ensure-canonical-path.js,gsd-workflow-guard.js,gsd-context-monitor.js,lib/git-cmd.js}`, and each bridge/`runHook` call exits 0 against the real installed files. `hooksSurface: "none"` + `configFormat: "none"` + `writesSharedSettings: false` are unaffected — no settings/hooks.json/config.toml is written for pi; the extension spawns hooks by absolute path, not via a config-file hook bus. `tests/fixtures/golden-install-parity/pi.json` grew from 292 → 320 entries (the 28 new `hooks/`/`hooks/lib/` files); `commands/`, `agents/`, `skills/` remain absent (`pluginOnlyInstall` is untouched — it only gates the declarative-markdown surfaces, not hooks). `tests/install-minimal-hooks.test.cjs`'s #1821 suite moved pi from the Kilo/ZCode (no-hooks) group into the OpenCode (ships-hooks) group accordingly. +**Adversarial-review correction (#2102 Stage 2, post-review):** the event bridges above and the `/gsd` tokenizer's `hooks/lib/git-cmd.js` require were DEAD in a real install — Stage 1's `hostBehaviors.skipSharedHooksInstall:true` meant pi shipped NO `hooks/` directory at all, so `runHook('gsd-ensure-canonical-path.js', ...)` etc. always hit the "hook file absent → silent no-op" branch, and the tokenizer always fell back to plain whitespace-splitting. The tests masked this because they run against the dev tree, where `hooks/` genuinely exists. **Fix:** `capabilities/pi/capability.json` no longer sets `skipSharedHooksInstall` — pi is architecturally identical to OpenCode here (`hooksSurface: "none"` + a native extension that spawns the staged hooks), not to Kilo/ZCode (`hooksSurface: "none"` with NO plugin surface, where the same hooks genuinely are dead weight). pi now installs `hooks/` + `hooks/lib/` (27 entries: the same `INSTALLED_HOOK_FILES` set OpenCode gets) alongside `extensions/gsd.js`, verified end-to-end via a real `node bin/install.js --pi --global`/`--local` — `resolveEngineRoot`'s walk-up from the installed extension's own directory finds `ENGINE_ROOT/hooks/{gsd-ensure-canonical-path.js,gsd-workflow-guard.js,gsd-context-monitor.js,lib/git-cmd.js}`, and each bridge/`runHook` call exits 0 against the real installed files. `hooksSurface: "none"` + `configFormat: "none"` + `writesSharedSettings: false` are unaffected — no settings/hooks.json/config.toml is written for pi; the extension spawns hooks by absolute path, not via a config-file hook bus. `tests/fixtures/golden-install-parity/pi.json` grew from 292 → 320 entries (the 28 new `hooks/`/`hooks/lib/` files); `commands/`, `agents/`, `skills/` remain absent (`pluginOnlyInstall` is untouched — it only gates the declarative-markdown surfaces, not hooks). `tests/install-minimal-hooks.test.cjs`'s #1821 suite moved pi from the Kilo/ZCode (no-hooks) group into the OpenCode (ships-hooks) group accordingly. ## vscode diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index d24f37c7d..1a40c9116 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -2130,7 +2130,7 @@ const capabilities = { "role": "runtime", "version": "1.7.0", "title": "pi", - "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.cjs; no shared-settings hook surface; tier-2 support.", + "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", "requires": [], "engines": { @@ -2178,7 +2178,7 @@ const capabilities = { "hostBehaviors": { "nativePlugin": { "dir": "extensions", - "file": "gsd.cjs", + "file": "gsd.js", "source": "pi/gsd.cjs" }, "pluginOnlyInstall": true @@ -5159,7 +5159,7 @@ const runtimes = { "role": "runtime", "version": "1.7.0", "title": "pi", - "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.cjs; no shared-settings hook surface; tier-2 support.", + "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", "requires": [], "engines": { @@ -5207,7 +5207,7 @@ const runtimes = { "hostBehaviors": { "nativePlugin": { "dir": "extensions", - "file": "gsd.cjs", + "file": "gsd.js", "source": "pi/gsd.cjs" }, "pluginOnlyInstall": true diff --git a/package.json b/package.json index 4aa9f1425..b50241829 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,8 @@ "typescript-eslint": "^8.60.0" }, "overrides": { - "qs": ">=6.15.2" + "qs": ">=6.15.2", + "body-parser": ">=2.3.0" }, "optionalDependencies": { "fallow": "^2.70.0" diff --git a/pi/gsd.cjs b/pi/gsd.cjs index 9953d35fe..b1ea2b6c1 100644 --- a/pi/gsd.cjs +++ b/pi/gsd.cjs @@ -9,8 +9,14 @@ * This extension binds GSD's command surface to pi via the imperative adapter * path — the programmatic-CLI peer of the OpenCode worked binding. * - * Installation: copy this file to ~/.pi/agent/extensions/gsd.cjs (pi loads - * extensions via jiti from that dir). The engine is resolved from the installed + * Installation: copy this file to ~/.pi/agent/extensions/gsd.js — note the + * `.js` DEST suffix, not `.cjs`. pi auto-discovers extensions/ entries through + * `isExtensionFile()`, which accepts only `.ts`/`.js` and skips anything else + * SILENTLY (no error, no log line), so a `.cjs` dest installs fine and is then + * never loaded (#2470). This source file keeps its `.cjs` suffix on purpose — + * tests `require()` it directly and `.cjs` is unambiguous CommonJS — and pi + * loads the copied file via jiti, which handles CommonJS and ESM alike, so the + * suffix gates discovery, not parsing. The engine is resolved from the installed * GSD tree (walk-up like the OpenCode plugin). pi's shared hooks/ bundle * (hooks/*.js + hooks/lib/git-cmd.js) is installed alongside the extension — * capabilities/pi/capability.json does NOT set diff --git a/src/install-engine.cts b/src/install-engine.cts index ee9387325..cf1ceab01 100644 --- a/src/install-engine.cts +++ b/src/install-engine.cts @@ -1106,9 +1106,20 @@ function _installNativePluginIfDeclared( if (np && np.source) { const pluginSrc = path.join(src, np.source); if (fs.existsSync(pluginSrc)) { - const destDir = runtimeArtifactInstallPlan.assertDestWithinConfigHome(configDir, np.dir); - fs.mkdirSync(destDir, { recursive: true }); - fs.copyFileSync(pluginSrc, path.join(destDir, np.file)); + // Confine the FULL dest path (dir + file), not just the dir. Previously + // only `np.dir` was validated and `np.file` was joined on unchecked, so a + // descriptor whose `file` carried `..`, an absolute path, or a NUL byte + // would have written outside configHome. Not reachable today — descriptors + // are first-party and compiled into the capability registry at build time — + // but `np.file` is exactly the field #2470 changes, and the guard costs + // nothing. For a well-formed descriptor this resolves identically to the + // previous mkdir(dir) + join(dir, file). + const destPath = runtimeArtifactInstallPlan.assertDestWithinConfigHome( + configDir, + path.join(np.dir, np.file), + ); + fs.mkdirSync(path.dirname(destPath), { recursive: true }); + fs.copyFileSync(pluginSrc, destPath); } } } diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts index 2232401f2..fd4e7e2f7 100644 --- a/src/installer-migrations.cts +++ b/src/installer-migrations.cts @@ -46,6 +46,38 @@ function sha256Text(value: string): string { return crypto.createHash('sha256').update(value).digest('hex'); } +/** + * Copy a managed path for the rollback snapshot or the user-facing backup, + * WITHOUT dereferencing a symlink. + * + * `fs.copyFileSync` follows symlinks, so a managed path that has been replaced + * by a link (tampering, or an unexpected user layout) would have had the + * LINK TARGET's bytes copied into `gsd-migration-journal/…-backups/` — e.g. a + * `gsd.cjs` symlinked at `~/.ssh/id_rsa` would land that key's contents in the + * backup tree. Nothing GSD installs is ever a symlink, so the faithful snapshot + * of a symlinked managed path is the link itself: recreating it preserves + * rollback fidelity (restore re-creates the same link) while never reading the + * referent. Deletion was already safe — `fs.rmSync` unlinks the link, never the + * target. + * + * Windows note: `fs.symlinkSync` can throw EPERM for unprivileged users. That + * surfaces as an apply failure and triggers the normal rollback path, which is + * the correct outcome — refusing to proceed beats silently copying referent + * bytes. + */ +function copyPreservingSymlink(srcPath: string, destPath: string): void { + if (fs.lstatSync(srcPath).isSymbolicLink()) { + // symlinkSync fails with EEXIST on an occupied path, so clear it first. + // Scoped to this branch on purpose: the regular-file path below keeps + // copyFileSync's overwrite-in-place, so a mid-restore failure cannot leave + // the destination destroyed. + fs.rmSync(destPath, { force: true }); + fs.symlinkSync(fs.readlinkSync(srcPath), destPath); + return; + } + fs.copyFileSync(srcPath, destPath); +} + function readJsonIfPresent(filePath: string, fallback: unknown): unknown { if (!fs.existsSync(filePath)) return fallback; try { @@ -627,9 +659,12 @@ function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollb const rollbackPath = path.join(configDir, action.rollbackRelPath as string); const dest = path.join(configDir, action.relPath as string); try { - if (fs.existsSync(rollbackPath)) { + // lstat-based existence check: a snapshot of a symlinked managed path is + // itself a link, and existsSync() follows it — a link whose target is + // gone would read as "missing" and silently skip the restore. + if (fs.lstatSync(rollbackPath, { throwIfNoEntry: false })) { fs.mkdirSync(path.dirname(dest), { recursive: true }); - fs.copyFileSync(rollbackPath, dest); + copyPreservingSymlink(rollbackPath, dest); } } catch (error) { failures.push({ relPath: action.relPath as string, error: (error as Error).message }); @@ -743,7 +778,7 @@ function applyInstallerMigrationPlan({ const rollbackPath = path.join(rollbackRoot, normalized); fs.mkdirSync(path.dirname(rollbackPath), { recursive: true }); - fs.copyFileSync(fullPath, rollbackPath); + copyPreservingSymlink(fullPath, rollbackPath); rollback.push({ relPath: normalized, rollbackPath }); if (action.type === 'rewrite-json') { @@ -765,7 +800,7 @@ function applyInstallerMigrationPlan({ const backupRelPath = action.backupRelPath || path.posix.join(backupRootRelPath, normalized); const backupPath = path.join(configDir, backupRelPath); fs.mkdirSync(path.dirname(backupPath), { recursive: true }); - fs.copyFileSync(fullPath, backupPath); + copyPreservingSymlink(fullPath, backupPath); journal.actions.push(journalAction(action, 'removed', { backupRelPath, rollbackRelPath: path.posix.join(rollbackRootRelPath, normalized), @@ -817,7 +852,12 @@ function applyInstallerMigrationPlan({ const dest = path.join(configDir, entry.relPath); try { fs.mkdirSync(path.dirname(dest), { recursive: true }); - fs.copyFileSync(entry.rollbackPath, dest); + // Symlink-preserving, same as the forward path: `entry.rollbackPath` is + // itself a link whenever the managed path was one, so a raw copy here + // would dereference it and write the referent's bytes back to the LIVE + // install path — a worse leak than the journal-tree one, since it is + // user-visible and at a predictable location. + copyPreservingSymlink(entry.rollbackPath, dest); } catch (rollbackError) { rollbackFailures.push({ relPath: entry.relPath, diff --git a/src/installer-migrations/006-pi-extension-cjs-to-js.cts b/src/installer-migrations/006-pi-extension-cjs-to-js.cts new file mode 100644 index 000000000..ad890757b --- /dev/null +++ b/src/installer-migrations/006-pi-extension-cjs-to-js.cts @@ -0,0 +1,129 @@ +/** + * Installer migration: retire pi's stale `extensions/gsd.cjs` after #2470 + * renamed the installed native extension to `extensions/gsd.js`. + * + * What old artifact is being retired? + * `extensions/gsd.cjs` — the pre-#2470 dest filename for pi's native + * extension. pi auto-discovers extensions by scanning `/extensions/` + * and keeping only names accepted by its own predicate + * (`isExtensionFile()` in @earendil-works/pi-coding-agent: + * `name.endsWith(".ts") || name.endsWith(".js")`). A `.cjs` file is skipped + * SILENTLY — no `/gsd` command, no error, no log line. The file is therefore + * permanently inert, not merely redundant. + * + * How do we prove it is GSD-owned? + * The installer records the native plugin in the install manifest as + * `/` (bin/install.js, the + * `_hostBehaviors(runtime).nativePlugin` manifest block), so a pre-#2470 pi + * install carries `extensions/gsd.cjs` as a manifest-managed entry. Only a + * manifest-managed classification produces an action here; an unmanifested + * `gsd.cjs` is treated as a user's own file and preserved. + * + * What happens if the user modified it? + * `backup-and-remove` instead of `remove-managed`, so a patched extension is + * recoverable from the backup rather than silently destroyed. + * + * What happens if it is missing? + * No actions — fresh (post-#2470) installs and already-migrated installs both + * plan empty, so the migration is idempotent. + * + * What runtime and scope does it affect? + * pi only, global and local. No other runtime ever installed this path: + * OpenCode and Kilo — the only other runtimes declaring + * `hostBehaviors.nativePlugin` — both ship `plugins/gsd-core.js`. + * + * Is the action safe in non-interactive install? + * Yes. Both emitted action types are non-interactive and journaled; neither + * requires a user choice, and unknown files never produce an action. + * + * Why not `move-managed`? The installer materializes the new `extensions/gsd.js` + * from the package payload in the same run, so moving the stale file onto that + * path would just be overwritten. Retiring the old path is the accurate + * description of the change. + * + * See docs/installer-migrations.md#shipped-migrations and the pi row of + * docs/installer-migrations.md#runtime-configuration-contract-registry. + */ + +type ArtifactClassification = string; + +interface ClassifiedArtifact { + classification: ArtifactClassification; + [key: string]: unknown; +} + +type ActionType = 'remove-managed' | 'backup-and-remove'; + +interface MigrationAction { + type: ActionType; + relPath: string; + reason: string; + ownershipEvidence: string; +} + +interface MigrationPlanContext { + classifyArtifact(relPath: string): ClassifiedArtifact; +} + +interface InstallerMigration { + id: string; + title: string; + description: string; + introducedIn: string; + runtimes: string[]; + scopes: string[]; + destructive: boolean; + plan: (ctx: MigrationPlanContext) => MigrationAction[]; +} + +/** Pre-#2470 dest filename for pi's native extension. */ +const STALE_PI_EXTENSION = 'extensions/gsd.cjs'; + +const OWNERSHIP_EVIDENCE = + 'pre-#2470 pi installs record the native extension at extensions/gsd.cjs in ' + + 'gsd-file-manifest.json (installer nativePlugin manifest entry)'; + +const REASON = + 'pi cannot auto-discover a .cjs extension (isExtensionFile accepts only .ts/.js), ' + + 'so this file is inert; superseded by extensions/gsd.js (#2470)'; + +const migration: InstallerMigration = { + id: '2026-07-20-pi-extension-cjs-to-js', + title: 'Retire pi\'s undiscoverable extensions/gsd.cjs', + description: + 'Remove the stale extensions/gsd.cjs left by pre-#2470 pi installs, superseded by ' + + 'extensions/gsd.js — the suffix pi\'s extension auto-discovery actually accepts.', + introducedIn: '1.7.1', + runtimes: ['pi'], + scopes: ['global', 'local'], + destructive: true, + plan: (ctx: MigrationPlanContext): MigrationAction[] => { + const artifact = ctx.classifyArtifact(STALE_PI_EXTENSION); + if (artifact.classification === 'managed-pristine') { + return [ + { + type: 'remove-managed', + relPath: STALE_PI_EXTENSION, + reason: REASON, + ownershipEvidence: OWNERSHIP_EVIDENCE, + }, + ]; + } + if (artifact.classification === 'managed-modified') { + return [ + { + type: 'backup-and-remove', + relPath: STALE_PI_EXTENSION, + reason: REASON, + ownershipEvidence: OWNERSHIP_EVIDENCE, + }, + ]; + } + // 'unknown' (never GSD-managed), 'missing', and 'managed-missing' all plan + // nothing: unknown files are preserved by policy, and an absent file needs + // no retirement. + return []; + }, +}; + +export = migration; diff --git a/tests/fixtures/golden-install-parity/pi.json b/tests/fixtures/golden-install-parity/pi.json index 80143d591..d6dcae10d 100644 --- a/tests/fixtures/golden-install-parity/pi.json +++ b/tests/fixtures/golden-install-parity/pi.json @@ -1,7 +1,7 @@ { ".gsd-profile": "0e716a5fef4e6dc1", ".gsd/defaults.json": "615261cfd3ae1c96", - "extensions/gsd.cjs": "619cec0af9cfdadf", + "extensions/gsd.js": "976c6546391caf8f", "gsd-core/.gsd-runtime": "94e95f0bb38f8e0f", "gsd-core/VERSION": "ef0deccd81a6723c", "gsd-core/bin/check-latest-version.cjs": "e4a224058c8f4d74", diff --git a/tests/fixtures/install-tree/pi.json b/tests/fixtures/install-tree/pi.json index 05793d57c..9c216cd10 100644 --- a/tests/fixtures/install-tree/pi.json +++ b/tests/fixtures/install-tree/pi.json @@ -1,7 +1,7 @@ [ ".gsd-profile", ".gsd/defaults.json", - "extensions/gsd.cjs", + "extensions/gsd.js", "gsd-core/.gsd-runtime", "gsd-core/VERSION", "gsd-core/bin/check-latest-version.cjs", diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index cccc896e0..7fae2a031 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -678,7 +678,7 @@ describe('#1755: .sh hooks are copied and executable after install', () => { // hooksSurface:'none', but each has a native plugin adapter that spawns the // staged hooks/*.js scripts as subprocesses (OpenCode's #1914 // plugins/gsd-core.js via OpenCode's event bus; pi's #2102 Stage 2 pi/gsd.cjs -// → extensions/gsd.cjs via pi.on(...) bridges; Kilo's plugins/gsd-core.js, +// → extensions/gsd.js via pi.on(...) bridges; Kilo's plugins/gsd-core.js, // byte-identical to OpenCode's) — so for all three, the hooks are LIVE and // must keep being copied. ZCode has no plugin surface at all, so its staged // hooks are genuinely dead: that is the case #1821's fix removes. (#1821 @@ -766,7 +766,7 @@ describe('#1821/#2305: ZCode receives no dead hook files; Kilo/OpenCode/Claude k }); // pi ALSO declares hooksSurface:'none', but — like OpenCode — it is NOT a - // dead-weight case: pi's native extension (pi/gsd.cjs → extensions/gsd.cjs) + // dead-weight case: pi's native extension (pi/gsd.cjs → extensions/gsd.js) // spawns the staged hooks/*.js scripts as bounded subprocesses (session_start // → gsd-ensure-canonical-path.js, before_agent_start → gsd-workflow-guard.js, // session_before_compact → gsd-context-monitor.js — #2102 Stage 2), and its @@ -775,8 +775,20 @@ describe('#1821/#2305: ZCode receives no dead hook files; Kilo/OpenCode/Claude k // pi (unlike Kilo/ZCode/Cursor/Cline/Trae/Copilot/Windsurf/Kimi) — pi is in // the OpenCode group, not the Kilo/ZCode group. test('pi --global install still copies hooks (spawned by the native extension) + hooks/lib/git-cmd.js + the extension itself', () => { + // #2470: derive the extension filename from pi's own descriptor rather than + // hardcoding it, and assert it satisfies pi's isExtensionFile() discovery + // filter (.ts/.js only) — a dest pi cannot discover installs "successfully" + // while /gsd never registers, which is exactly how this shipped broken. + const piNativePlugin = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'pi', 'capability.json'), 'utf8'), + ).runtime.hostBehaviors.nativePlugin; + assert.ok( + piNativePlugin.file.endsWith('.ts') || piNativePlugin.file.endsWith('.js'), + `pi's installed extension "${piNativePlugin.file}" must end in .ts or .js — pi silently ` + + 'skips any other suffix during extensions/ auto-discovery (#2470)', + ); const { hookFiles, hooksLibExists, gitCmdExists, pluginExists } = installAndCollect('pi', { - pluginRelPath: path.join('extensions', 'gsd.cjs'), + pluginRelPath: path.join(piNativePlugin.dir, piNativePlugin.file), }); const basenames = hookFiles.map((f) => path.basename(f)); for (const expected of ['gsd-ensure-canonical-path.js', 'gsd-workflow-guard.js', 'gsd-context-monitor.js']) { @@ -787,7 +799,10 @@ describe('#1821/#2305: ZCode receives no dead hook files; Kilo/OpenCode/Claude k } assert.ok(hooksLibExists, 'pi install must create hooks/lib/'); assert.ok(gitCmdExists, 'pi install must copy hooks/lib/git-cmd.js (the /gsd command tokenizer)'); - assert.ok(pluginExists, 'pi install must install extensions/gsd.cjs (the native-extension hook bridge)'); + assert.ok( + pluginExists, + `pi install must install ${piNativePlugin.dir}/${piNativePlugin.file} (the native-extension hook bridge)`, + ); }); // Positive control: guards against over-exclusion breaking runtimes that diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs index ac77e3b63..5a8345032 100644 --- a/tests/install-write-confinement.test.cjs +++ b/tests/install-write-confinement.test.cjs @@ -2964,3 +2964,104 @@ describe('#2393: GSD_ALLOW_SYMLINKED_DEST opt-in for intentional symlinked-dest } }); }); + +// --------------------------------------------------------------------------- +// _installNativePluginIfDeclared write-confinement (#2470) +// --------------------------------------------------------------------------- +// +// The native-plugin copy previously confined only `nativePlugin.dir`, then +// joined `nativePlugin.file` onto the validated directory unchecked. #2470 +// makes `file` a field we actively change (pi: gsd.cjs -> gsd.js), so the full +// dest path is now confined. Descriptors are first-party and compiled into the +// capability registry at build time, so this was never reachable in a shipped +// build — these tests keep it that way. + +describe('_installNativePluginIfDeclared write-confinement', () => { + const engine = require('../gsd-core/bin/lib/install-engine.cjs'); + + /** Build a src tree containing the declared plugin source. */ + function stageSource() { + const src = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-src-')); + const full = path.join(src, 'pi', 'gsd.cjs'); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, "'use strict';\n// plugin\n", 'utf8'); + return src; + } + + const behaviorsWith = (file) => ({ + nativePlugin: { dir: 'extensions', file, source: 'pi/gsd.cjs' }, + }); + + test('happy path: declared dir/file lands inside configDir', () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-cfg-')); + const src = stageSource(); + try { + engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith('gsd.js'), src); + assert.ok( + fs.existsSync(path.join(configDir, 'extensions', 'gsd.js')), + 'plugin should be copied to /extensions/gsd.js', + ); + } finally { + cleanup(configDir); + cleanup(src); + } + }); + + const ESCAPE_CASES = [ + ['traversal', '../../evil.js'], + ['deep traversal', '../../../../../../tmp/evil.js'], + ['NUL byte', 'gsd\u0000.js'], + ]; + + for (const [label, file] of ESCAPE_CASES) { + test(`escape rejected: nativePlugin.file with ${label} → throws`, () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-esc-')); + const src = stageSource(); + try { + assert.throws( + () => engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith(file), src), + /escap|strict subpath|configHome|NUL byte/i, + `nativePlugin.file=${JSON.stringify(file)} must be rejected, not joined onto the validated dir`, + ); + } finally { + cleanup(configDir); + cleanup(src); + } + }); + } + + test('nothing is written outside configDir when file tries to escape', () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-out-')); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-outside-')); + const src = stageSource(); + try { + const escapeFile = path.join('..', '..', path.basename(outside), 'pwned.js'); + assert.throws( + () => engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith(escapeFile), src), + /escap|strict subpath|configHome/i, + ); + assert.ok( + !fs.existsSync(path.join(outside, 'pwned.js')), + 'nothing may be written outside configDir', + ); + } finally { + cleanup(configDir); + cleanup(outside); + cleanup(src); + } + }); + + test('missing source is a silent no-op (unchanged behavior)', () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-nosrc-')); + const src = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-emptysrc-')); + try { + assert.doesNotThrow(() => + engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith('gsd.js'), src), + ); + assert.ok(!fs.existsSync(path.join(configDir, 'extensions', 'gsd.js'))); + } finally { + cleanup(configDir); + cleanup(src); + } + }); +}); diff --git a/tests/installer-migration-install.integration.test.cjs b/tests/installer-migration-install.integration.test.cjs index 57667c291..ddb5e3992 100644 --- a/tests/installer-migration-install.integration.test.cjs +++ b/tests/installer-migration-install.integration.test.cjs @@ -292,9 +292,19 @@ function assertFreshInstallContract(runtime, targetDir) { // adversarial-review fix — hooksSurface:'none' no longer implies // skipSharedHooksInstall for pi, mirroring OpenCode), so hooks/ + the // git-cmd.js tokenizer helper ARE part of the artifact surface now. + // #2470: the dest filename comes from pi's descriptor, and must satisfy + // pi's isExtensionFile() auto-discovery filter (.ts/.js only) — otherwise + // the file installs but pi never loads it and /gsd never registers. + const piNativePlugin = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'pi', 'capability.json'), 'utf8') + ).runtime.hostBehaviors.nativePlugin; assert.ok( - fs.existsSync(path.join(targetDir, 'extensions', 'gsd.cjs')), - `${runtime} should install the native extension file at extensions/gsd.cjs` + piNativePlugin.file.endsWith('.ts') || piNativePlugin.file.endsWith('.js'), + `${runtime}'s extension "${piNativePlugin.file}" must end in .ts or .js for pi to discover it (#2470)` + ); + assert.ok( + fs.existsSync(path.join(targetDir, piNativePlugin.dir, piNativePlugin.file)), + `${runtime} should install the native extension file at ${piNativePlugin.dir}/${piNativePlugin.file}` ); assert.ok( fs.existsSync(path.join(targetDir, 'hooks', 'gsd-ensure-canonical-path.js')), diff --git a/tests/installer-migration-pi-extension-ext.test.cjs b/tests/installer-migration-pi-extension-ext.test.cjs new file mode 100644 index 000000000..26b3c4214 --- /dev/null +++ b/tests/installer-migration-pi-extension-ext.test.cjs @@ -0,0 +1,231 @@ +'use strict'; + +/** + * TDD tests for installer migration 006: + * 2026-07-20-pi-extension-cjs-to-js (#2470) + * + * #2470 renamed pi's installed native extension from `extensions/gsd.cjs` to + * `extensions/gsd.js`, because pi's own auto-discovery filter + * (`isExtensionFile()` in @earendil-works/pi-coding-agent) accepts only `.ts` + * and `.js` and silently skips everything else. Installs made before that fix + * still carry the stale, permanently-inert `extensions/gsd.cjs`; the installer + * writes the new `.js` alongside it and would otherwise orphan the old file + * forever (it drops out of the manifest, so uninstall never removes it). + * + * This migration retires the stale copy. Coverage follows the matrix in + * docs/installer-migrations.md#authoring-workflow: + * 1. metadata / authoring-guard conformance + * 2. stale file absent -> empty plan (idempotent, fresh installs) + * 3. stale file managed-pristine -> remove-managed + * 4. stale file managed-modified -> backup-and-remove (never silent delete) + * 5. stale file unknown -> NO action (never remove unowned files) + * 6. the replacement gsd.js and neighbouring user files are never touched + * 7. runtime scoping: pi only + */ + +const { describe, test } = 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 migration = require('../gsd-core/bin/lib/installer-migrations/006-pi-extension-cjs-to-js.cjs'); + +const { + classifyArtifact: realClassifyArtifact, + readInstallManifest, +} = require('../gsd-core/bin/lib/installer-migrations.cjs'); + +const STALE_REL = 'extensions/gsd.cjs'; +const CURRENT_REL = 'extensions/gsd.js'; + +function createTempDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-migration-006-test-')); +} + +function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup in migration test; no helpers import available + fs.rmSync(dir, { recursive: true, force: true }); +} + +function writeFile(root, relPath, content) { + const fullPath = path.join(root, relPath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, content, 'utf8'); +} + +function writeManifest(root, files) { + fs.writeFileSync( + path.join(root, 'gsd-file-manifest.json'), + JSON.stringify( + { + version: '1.7.0', + timestamp: '2026-07-20T00:00:00.000Z', + mode: 'full', + files, + }, + null, + 2, + ), + 'utf8', + ); +} + +function makePlanCtx(configDir) { + const manifest = readInstallManifest(configDir); + return { + configDir, + classifyArtifact: (relPath) => realClassifyArtifact(configDir, relPath, manifest), + }; +} + +/** sha256 hash in the manifest's own format, so a file reads as pristine. */ +function hashOf(root, relPath) { + const crypto = require('node:crypto'); + return crypto + .createHash('sha256') + .update(fs.readFileSync(path.join(root, relPath))) + .digest('hex'); +} + +// --------------------------------------------------------------------------- +// 1. Metadata +// --------------------------------------------------------------------------- + +describe('migration 006 metadata', () => { + test('exports a single migration object with the required authoring fields', () => { + assert.equal(typeof migration, 'object'); + assert.equal(typeof migration.id, 'string'); + assert.ok(migration.id.length > 0, 'id must be non-empty'); + assert.equal(typeof migration.title, 'string'); + assert.equal(typeof migration.description, 'string'); + assert.equal(typeof migration.introducedIn, 'string'); + assert.ok(Array.isArray(migration.scopes), 'scopes must be an array'); + assert.ok(migration.scopes.includes('global'), 'scopes must include global'); + assert.ok(migration.scopes.includes('local'), 'scopes must include local'); + assert.strictEqual(migration.destructive, true); + assert.equal(typeof migration.plan, 'function'); + }); + + test('is scoped to pi only (no other runtime installs this artifact)', () => { + assert.ok(Array.isArray(migration.runtimes), 'runtimes must be an explicit array'); + assert.deepEqual(migration.runtimes, ['pi']); + }); + + test('id carries the expected date prefix and names the retired artifact', () => { + assert.ok( + migration.id.startsWith('2026-07-20-'), + `id should start with the date prefix, got: ${migration.id}`, + ); + assert.match(migration.id, /pi-extension/); + }); +}); + +// --------------------------------------------------------------------------- +// 2-5. plan() behaviour by classification +// --------------------------------------------------------------------------- + +describe('migration 006 plan()', () => { + test('emits no actions when the stale extension is absent (fresh install, idempotent)', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, CURRENT_REL, '// current extension\n'); + writeManifest(dir, { [CURRENT_REL]: hashOf(dir, CURRENT_REL) }); + + const actions = migration.plan(makePlanCtx(dir)); + assert.deepEqual(actions, [], 'no stale file -> no actions'); + }); + + test('emits remove-managed for a pristine manifest-managed stale extension', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale pre-#2470 extension\n'); + writeManifest(dir, { [STALE_REL]: hashOf(dir, STALE_REL) }); + + const actions = migration.plan(makePlanCtx(dir)); + assert.equal(actions.length, 1, `expected exactly one action, got ${JSON.stringify(actions)}`); + assert.equal(actions[0].type, 'remove-managed'); + assert.equal(actions[0].relPath, STALE_REL); + assert.ok( + typeof actions[0].ownershipEvidence === 'string' && actions[0].ownershipEvidence.length > 0, + 'destructive actions require ownershipEvidence (authoring guard)', + ); + assert.ok( + typeof actions[0].reason === 'string' && actions[0].reason.length > 0, + 'action must carry a human-readable reason for dry-run output', + ); + }); + + test('emits backup-and-remove when the user locally modified the stale extension', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale pre-#2470 extension\n'); + // Manifest records a DIFFERENT hash -> managed-modified. + writeManifest(dir, { [STALE_REL]: 'a'.repeat(64) }); + + const actions = migration.plan(makePlanCtx(dir)); + assert.equal(actions.length, 1); + assert.equal( + actions[0].type, + 'backup-and-remove', + 'a locally patched managed file must be backed up, never silently deleted', + ); + assert.equal(actions[0].relPath, STALE_REL); + assert.ok(actions[0].ownershipEvidence); + }); + + test('emits NO action for an unknown (non-manifest) gsd.cjs — never remove unowned files', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + // File present but absent from the manifest -> classification 'unknown'. + writeFile(dir, STALE_REL, '// hand-placed by the user\n'); + writeManifest(dir, {}); + + const actions = migration.plan(makePlanCtx(dir)); + assert.deepEqual( + actions, + [], + 'unknown files are preserved (docs/installer-migrations.md#ownership)', + ); + }); + + test('never targets the replacement extension or neighbouring user files', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale\n'); + writeFile(dir, CURRENT_REL, '// current\n'); + writeFile(dir, 'extensions/my-own-extension.js', '// user-authored\n'); + writeManifest(dir, { + [STALE_REL]: hashOf(dir, STALE_REL), + [CURRENT_REL]: hashOf(dir, CURRENT_REL), + }); + + const actions = migration.plan(makePlanCtx(dir)); + const targeted = actions.map((a) => a.relPath); + assert.deepEqual(targeted, [STALE_REL]); + assert.ok(!targeted.includes(CURRENT_REL), 'must not remove the replacement extension'); + assert.ok( + !targeted.includes('extensions/my-own-extension.js'), + "must not touch a user's own extension sitting in the same directory", + ); + }); + + test('plan() does not mutate disk (planning is pure)', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale\n'); + writeManifest(dir, { [STALE_REL]: hashOf(dir, STALE_REL) }); + + migration.plan(makePlanCtx(dir)); + assert.ok( + fs.existsSync(path.join(dir, STALE_REL)), + 'plan() must not remove anything — the executor owns mutation', + ); + }); +}); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index d1b30279f..337ec7a61 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -1665,6 +1665,14 @@ test('shipped installer-migration checksums are locked to a committed baseline ( // fix-forward migration widens the scanned surface without touching 000. '2026-07-17-opencode-baseline-commands-dir': 'sha256:0f6080b5f9b75fb5adbe9664a71152e23a5336813453b0a77e4df6fd483ad38e', + // Migration 006 (NEW, added here per this test's own sanctioned "adding a new + // migration" case — not a shipped-body edit): retire pi's stale + // extensions/gsd.cjs. #2470 renamed the installed extension to + // extensions/gsd.js because pi's isExtensionFile() auto-discovery accepts + // only .ts/.js and silently skips everything else; without this migration the + // old path drops out of the manifest and uninstall can never remove it. + '2026-07-20-pi-extension-cjs-to-js': + 'sha256:185fa926ae24d83cbdd95c31a9ad2cc8d123e176ad543669b3b0ed75e6ca6f4a', }; const { DEFAULT_MIGRATIONS_DIR, migrationChecksum: computeChecksum } = require('../gsd-core/bin/lib/installer-migrations.cjs'); @@ -2565,3 +2573,240 @@ describe('migration.plan()', () => { }); }); } + +// --------------------------------------------------------------------------- +// Symlinked managed path: backup must never dereference (#2470 security review) +// --------------------------------------------------------------------------- +// +// `fs.copyFileSync` follows symlinks. Before this hardening, a managed path +// replaced by a link would have had the LINK TARGET's bytes copied into the +// journal's backup tree — e.g. a `gsd.cjs` symlinked at a private key would +// land that key's contents under gsd-migration-journal/. Nothing GSD installs +// is ever a symlink, so the faithful snapshot is the link itself. + +{ + const { describe, test } = require('node:test'); + describe('symlinked managed path is snapshotted as a link, never dereferenced', () => { + const piExtensionMigration = require('../gsd-core/bin/lib/installer-migrations/006-pi-extension-cjs-to-js.cjs'); + const SECRET = 'TOP-SECRET-PRIVATE-KEY-MATERIAL\n'; + + test('backup-and-remove on a symlinked managed file copies the link, not the referent', (t) => { + const configDir = createTempInstall(); + const secretDir = createTempInstall(); + try { + const secretPath = path.join(secretDir, 'id_rsa'); + fs.writeFileSync(secretPath, SECRET, 'utf8'); + + const linkPath = path.join(configDir, 'extensions', 'gsd.cjs'); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + try { + fs.symlinkSync(secretPath, linkPath); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + + // Manifest records the path as managed with a hash that cannot match the + // referent -> classification 'managed-modified' -> backup-and-remove. + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + const result = runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }); + + const backupAction = result.plan.actions.find((a) => a.type === 'backup-and-remove'); + assert.ok(backupAction, 'expected a backup-and-remove action for the modified managed file'); + + // The referent is untouched and still holds its content. + assert.ok(fs.existsSync(secretPath), 'symlink target must survive'); + assert.equal(fs.readFileSync(secretPath, 'utf8'), SECRET, 'symlink target content must be unchanged'); + + // The link itself is gone from the install tree. + assert.equal( + fs.lstatSync(linkPath, { throwIfNoEntry: false }), + undefined, + 'the symlink at the managed path must be removed', + ); + + // Nothing anywhere under configDir may contain the referent's bytes. + const leaked = []; + const walk = (dir) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isSymbolicLink()) continue; // a link is fine; its content is not copied + if (entry.isDirectory()) { walk(full); continue; } + let body; + try { body = fs.readFileSync(full, 'utf8'); } catch { continue; } + if (body.includes('TOP-SECRET')) leaked.push(path.relative(configDir, full)); + } + }; + walk(configDir); + assert.deepEqual(leaked, [], `symlink referent content leaked into: ${leaked.join(', ')}`); + } finally { + cleanup(configDir); + cleanup(secretDir); + } + }); + + test('a regular managed file is still backed up by content (no behavior change)', (t) => { + const configDir = createTempInstall(); + try { + writeFile(configDir, 'extensions/gsd.cjs', 'locally patched extension\n'); + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + const result = runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }); + + const backupAction = result.plan.actions.find((a) => a.type === 'backup-and-remove'); + assert.ok(backupAction, 'expected backup-and-remove for the locally patched file'); + + // The PLAN carries backupRelPath: null — the concrete backup location is + // chosen during apply and recorded in the journal, so read it from there. + const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8')); + const journalled = journal.actions.find((a) => a.backupRelPath); + assert.ok(journalled, 'apply must record the backup path in the journal for the user'); + const backupPath = path.join(configDir, journalled.backupRelPath); + assert.equal( + fs.readFileSync(backupPath, 'utf8'), + 'locally patched extension\n', + 'a real file must still be backed up by content so the user can recover it', + ); + assert.ok(!fs.existsSync(path.join(configDir, 'extensions', 'gsd.cjs'))); + } finally { + cleanup(configDir); + } + }); + + // In-flight failure recovery: when a later step of the SAME apply() attempt + // throws, the catch block replays the rollback snapshots it already took. + // Those snapshots are themselves symlinks, so a raw copy there dereferences + // and writes the referent's bytes back to the LIVE install path — worse than + // the journal-tree leak, because it is user-visible at a predictable path. + // + // The failure is injected by monkeypatching fs.rmSync (restored in finally) + // rather than by chmod/permission tricks: deterministic, root- and + // OS-independent. The delete of the managed path is allowed to SUCCEED and + // then throws once, modelling a later step failing after the delete. That + // ordering is load-bearing: if the live path still existed, the pre-fix + // copyFileSync would hit a same-file collision and throw instead of leaking, + // and this test would pass against the very bug it exists to catch. + test('apply failure after a symlinked snapshot does not leak the referent into the live tree', (t) => { + const configDir = createTempInstall(); + const secretDir = createTempInstall(); + const realRmSync = fs.rmSync; + try { + const secretPath = path.join(secretDir, 'id_rsa'); + fs.writeFileSync(secretPath, SECRET, 'utf8'); + + const linkPath = path.join(configDir, 'extensions', 'gsd.cjs'); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + try { + fs.symlinkSync(secretPath, linkPath); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + let fired = false; + fs.rmSync = function patched(target, options) { + const result = realRmSync.call(fs, target, options); + if (!fired && path.resolve(String(target)) === path.resolve(linkPath)) { + fired = true; + throw new Error('injected post-delete failure'); + } + return result; + }; + + assert.throws(() => runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }), /injected post-delete failure/, 'the injected failure must propagate, not be swallowed'); + + fs.rmSync = realRmSync; + + // The referent is untouched... + assert.ok(fs.existsSync(secretPath)); + assert.equal(fs.readFileSync(secretPath, 'utf8'), SECRET); + + // ...the managed path is restored as a LINK, not a dereferenced copy... + const restored = fs.lstatSync(linkPath, { throwIfNoEntry: false }); + assert.ok(restored, 'failure recovery must restore the managed path'); + assert.ok( + restored.isSymbolicLink(), + 'restored path must be a symlink — a regular file here means the referent was dereferenced into the live tree', + ); + + // ...and its bytes appear nowhere under the install tree. + const leaked = []; + const walk = (dir) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isSymbolicLink()) continue; + if (entry.isDirectory()) { walk(full); continue; } + let body; + try { body = fs.readFileSync(full, 'utf8'); } catch { continue; } + if (body.includes('TOP-SECRET')) leaked.push(path.relative(configDir, full)); + } + }; + walk(configDir); + assert.deepEqual(leaked, [], `referent content leaked into: ${leaked.join(', ')}`); + } finally { + fs.rmSync = realRmSync; + cleanup(configDir); + cleanup(secretDir); + } + }); + + test('rollback() restores a symlinked managed path as a link, not a dereferenced copy', (t) => { + const configDir = createTempInstall(); + const secretDir = createTempInstall(); + try { + const targetPath = path.join(secretDir, 'id_rsa'); + fs.writeFileSync(targetPath, SECRET, 'utf8'); + + const linkPath = path.join(configDir, 'extensions', 'gsd.cjs'); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + try { + fs.symlinkSync(targetPath, linkPath); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + const result = runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }); + assert.equal(fs.lstatSync(linkPath, { throwIfNoEntry: false }), undefined, 'link removed by apply'); + + result.rollback(); + + const restored = fs.lstatSync(linkPath, { throwIfNoEntry: false }); + assert.ok(restored, 'rollback must restore the managed path'); + assert.ok(restored.isSymbolicLink(), 'restored path must be a symlink, not a dereferenced copy'); + assert.equal(fs.readlinkSync(linkPath), targetPath, 'restored link must point at the original target'); + assert.equal(fs.readFileSync(targetPath, 'utf8'), SECRET, 'target content must be untouched throughout'); + } finally { + cleanup(configDir); + cleanup(secretDir); + } + }); +}); +} diff --git a/tests/pi-upgrades.test.cjs b/tests/pi-upgrades.test.cjs index 8d1cbac94..1b705c8b8 100644 --- a/tests/pi-upgrades.test.cjs +++ b/tests/pi-upgrades.test.cjs @@ -170,3 +170,53 @@ test('pi axes negotiate modelMode:"active" (the active-model steering axis)', () const result = negotiateHostCapabilities(PI_AXES); assert.equal(result.effective.modelMode, 'active'); }); + +// -- native-extension auto-discovery contract (#2470) ------------------------- +// +// pi auto-discovers extensions by scanning /extensions/ and keeping +// only names its `isExtensionFile()` predicate accepts: +// +// function isExtensionFile(name) { +// return name.endsWith(".ts") || name.endsWith(".js"); +// } +// +// (@earendil-works/pi-coding-agent, packages/coding-agent/src/core/extensions/ +// loader.ts — verified upstream 2026-07-20.) A dest filename outside that set +// is skipped SILENTLY: no /gsd command, no error, no log line. pi loads the +// accepted file through jiti, which handles CommonJS and ESM alike, so the +// extension's module format is irrelevant to discovery — only the suffix is. +// +// These assertions deliberately encode pi's PREDICATE rather than the literal +// filename, so they keep protecting the contract if the extension is ever +// renamed again, and they state the reason the rename mattered. + +/** pi's upstream discovery predicate, mirrored verbatim. */ +function piIsExtensionFile(name) { + return name.endsWith('.ts') || name.endsWith('.js'); +} + +test('pi capability declares a native-extension dest filename pi will auto-discover (#2470)', () => { + const np = PI_CAP.runtime.hostBehaviors.nativePlugin; + assert.ok(np && np.file, 'pi must declare hostBehaviors.nativePlugin.file'); + assert.ok( + piIsExtensionFile(np.file), + `pi's installed extension "${np.file}" must end in .ts or .js — pi's isExtensionFile() ` + + 'auto-discovery filter silently skips every other suffix, so /gsd never registers (#2470)', + ); +}); + +test('pi native-extension source stays CommonJS-explicit while the dest satisfies pi (#2470)', () => { + const np = PI_CAP.runtime.hostBehaviors.nativePlugin; + // The in-repo source keeps its .cjs suffix on purpose: tests require() it + // directly and .cjs is unambiguous CommonJS regardless of any future + // package.json "type" flip. Only the INSTALLED name must satisfy pi, and + // jiti parses the copied file by content, not by suffix. + assert.ok( + np.source.endsWith('.cjs'), + `pi's in-repo extension source should stay .cjs (explicit CommonJS), got "${np.source}"`, + ); + assert.ok( + fs.existsSync(path.join(__dirname, '..', np.source)), + `pi's declared nativePlugin.source "${np.source}" must exist in the repo`, + ); +});