Files
msd-core/scripts/build-hooks.js
Tom Boucher 8f75e27554 fix(#3045): fail closed when an executor dispatch drops its resolved isolation (#3069)
* feat(#3045): deny an executor dispatch that drops its isolation flag

Every isolation gate already resolved correctly. The resolved value then reached
the executor through a prose instruction telling the model to substitute it into
a call the model composes itself, and nothing verified the substitution. When it
was dropped, the executor edited and committed in the user's primary checkout
with no consent and no warning.

A prose backstop would be the same class of artifact as the defect, so this is a
shipped PreToolUse hook on the Agent tool. It fires at the instant of the call
rather than being read once at the top of a workflow, which is the only placement
the model cannot skip.

The guard is inert unless it can positively establish that this is a GSD project,
that the project resolves to harness isolation, and that the dispatch targets an
executor. A non-GSD repo has no invariant to enforce. Where it cannot read the
configuration at all, it denies rather than assuming, with its own reason -- a
guard that cannot verify must not answer safe. A malformed payload allows rather
than throwing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* feat(#3045): extend the isolation guard to Cursor

Cursor is the second of only two runtimes that resolve harness isolation, so
shipping the guard for Claude alone left half the exposed surface unguarded while
the changeset implied it was covered.

The two runtimes fail differently. On Claude the harness flag is a per-dispatch
kwarg the model must copy into a call it composes, and the defect is that it can
be dropped. On Cursor the flag is --worktree, which applies to the whole session,
and the subagent-start payload carries no isolation field at all. There is no
flag to check, so the guard verifies the effective state instead: whether the
workspace is genuinely running outside the user's primary checkout. That is a
stronger check than the Claude one because it tests reality rather than intent,
and it is commented so nobody later rewrites it into a flag check.

Isolation is established two ways, either sufficient: the workspace resolves to a
linked git worktree, or it sits under the worktree root Cursor manages. The
second matters because a directory Cursor placed there is a legitimate isolated
session even before it becomes a distinct git worktree, where linkage alone would
report no repository.

Detecting linkage required a new primitive rather than the existing context
resolver. That resolver short-circuits on finding a local .planning directory
before it ever compares the git directory to the common one -- and an isolation
worktree normally has its own checked-out .planning. Reusing it would have read a
correctly isolated session as unisolated and denied it, which is the failure
direction that gets a guard switched off. The comparison is now its own
shortcut-free function that the resolver delegates to after its own shortcut, so
existing behavior is unchanged, and the case that would have broken is pinned.

The subagent type is checked before any configuration is read, so an unreadable
config cannot deny a dispatch this guard would never have enforced against.

The input-schema comment on the Cursor hook documented only the fields common to
every event and omitted the ones specific to this one. That omission cost a
halt during this work; it now documents both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3045): enforce the resolved dispatch decision, not the host capability

The guard keyed on the registry's dispatch.isolation, which says only that a
runtime is CAPABLE of harness worktrees. The decision that actually governs a
dispatch is the one the workflow resolves after gating, and that legitimately
comes out as sequential in three documented cases: a project setting
use_worktrees false, a per-plan submodule intersection, and the base-check
auto-degrade. The workflow tells the model to omit the flag in exactly those
cases, and the guard was denying every one of them.

The third case matters most. The preceding fix made the base-check degrade on
git timeouts and a missing git binary, where it had previously answered "safe".
That correction is right, and it means a transient hang now degrades to
sequential far more often than before -- so the two changes composed into a trap
where the workflow behaved exactly as designed and the guard blocked it.

The workflow already resolves isolation in shell, deterministically, which is
what makes it a trustworthy source in a way the model-authored call is not. It
now records that resolved value through a dedicated verb, and both guards read
it first. A fresh record is authoritative, so sequential dispatches pass
untouched. Absent or stale, the guards fall back to the capability check
combined with the project's use_worktrees setting, which still covers the case
that never reaches the workflow.

Also widened the matcher to accept Task alongside Agent, since a host that names
the tool Task would otherwise leave the guard silently inert while implying
coverage; stopped assuming Claude when no runtime is declared, which is the
shipped default and would have demanded a Claude-only argument elsewhere; and
made a non-git project inert rather than denied, since advising a worktree
session is not actionable without a repository.

The original diagnosis never modeled sequential mode as legitimate. That
omission is what let this through, and it is now recorded there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3045): record at resolution and bind the record to its dispatch

Two independent reviews converged on the same failure: the guard was fail-open in
a default install, so it did not catch the defect it exists to catch. A shipped
project carries no runtime key, which made "runtime not confidently known" the
common case rather than a corner one. A record asserting that isolation was
required but carrying no flag then fell through to a capability lookup that
answered "none", and the dispatch was allowed. The flag itself only arrived from
a second shell block -- the same block a model dropping the argument would also
skip. A test had pinned that behavior as intended.

The record is now written by the resolver, as an unavoidable consequence of
asking for the value, rather than by a step the model is told in prose to go and
run. A guard against a prose-carried value cannot itself depend on prose. Mode,
flag and identifiers are written together and atomically, so the flagless window
is gone, and a record asserting isolation with no resolvable flag now denies
instead of degrading. Runtime is also resolved from the installer's own recorded
default, which makes confident resolution the normal case.

The per-plan submodule gate degrades after the phase-level decision and never
re-recorded, so a plan that legitimately ran sequentially was denied against a
still-fresh phase record. It now records its own, scoped to the plan.

A record also authorized any dispatch for four hours. One phase degrading to
sequential could silently license an unisolated dispatch in the next. Records
now carry phase and plan, the guards require them to match, and the window is
minutes rather than hours -- the resolver rewrites it before every dispatch, so
a long window bought nothing and only widened the hole.

The flag validator rejected any value beginning with two dashes, which is exactly
the form Cursor and Windsurf declare, so their real value could never have been
stored. Writer and reader also derived the record path differently and diverged
inside a linked worktree without local planning state.

The predictable path remains a way to silence the control without leaving a trace
in the diff. It grants no access an agent with shell does not already have, so it
is documented as accepted rather than redesigned around.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3045): correct the staleness boundary and unmask a vacuous parity test

The remote runner returned twenty failures. One was a real production defect the
boundary case existed to catch: a record whose age exactly equalled the staleness
window was treated as fresh, so it stayed authoritative for one tick past its own
expiry. Freshness is now strictly inside the window.

The parity test meant to stop the two guards' executor lists from drifting could
never have failed. Its project fixture was a bare directory rather than a
repository, so the non-git inert branch answered before the executor list was
ever consulted. It asserted agreement it never actually measured. The fixture is
now a real repository, like every sibling in the file.

A test also asserted that Windsurf declares the worktree flag. It does not --
Windsurf resolves to no isolation by design, having no named concurrent dispatch
to isolate. The test claimed a registry fact that was never true, and a comment
in the resolver repeated it. Both corrected, and the test now proves what it
should have all along: that the parser accepts any bare flag value, rather than
one runtime's supposed value.

The new guard was missing from the bundled-hook whitelist, which is the surface
that decides what actually ships, and the per-plan gate had gained calls to the
launcher without the preamble those calls require. The changeset carried
parenthetical product descriptions the purity rule forbids.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#3045): backfill changeset pr number

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3045): make the guard tests hold on Windows

Two tests redirect HOME to control where the installer-persisted runtime default
is read from. Node resolves the home directory from USERPROFILE on Windows and
never consults HOME, so both silently read the real runner profile, found no
recorded runtime, and asserted against a project the hook had not recognised. The
production code was already correct in asking the platform rather than the
variable; only the tests were wrong to assume one variable answers everywhere.
The helpers now mirror the override onto both.

The symlink spoofing test also created a directory symlink unconditionally, which
needs elevated privileges on Windows. It survived on this runner, but it would
fail on any host without them, so the creation is now attempted and the test
skips explicitly when it cannot be done -- a bare return would have counted as a
pass and hidden the gap.

Skipping alone would have left the platform uncovered, so the behaviour it proves
is now also driven in-process through an injected realpath, following the seam
already used for the clock. That case no longer depends on privileges at all, and
the end-to-end test keeps its original assertions wherever symlinks work.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 23:42:16 -04:00

271 lines
12 KiB
JavaScript

#!/usr/bin/env node
/**
* Copy GSD hooks to dist for installation.
* Validates JavaScript syntax before copying to prevent shipping broken hooks.
* See #1107, #1109, #1125, #1161 — a duplicate const declaration shipped
* in dist and caused PostToolUse hook errors for all users.
*/
const fs = require('fs');
const path = require('path');
const vm = require('vm');
const HOOKS_DIR = path.join(__dirname, '..', 'hooks');
const DIST_DIR = path.join(HOOKS_DIR, 'dist');
// Per-process staging directory for atomic writes. Using process.pid in the
// name eliminates all contention between concurrent builders: each process
// owns its own staging dir and never races with another builder's cleanup.
// Lives under hooks/ so it shares a filesystem with DIST_DIR (POSIX
// rename(2) is only atomic within the same filesystem) but is NOT inside
// DIST_DIR — so readers that readdirSync(DIST_DIR) (e.g. bin/install.js,
// install-hooks-copy tests) never observe a transient ".tmp" sibling.
// The parent pattern hooks/.dist-staging-*/ is gitignored.
const STAGE_DIR = path.join(HOOKS_DIR, `.dist-staging-${process.pid}`);
// Hooks to copy (pure Node.js, no bundling needed)
const HOOKS_TO_COPY = [
'gsd-check-update-worker.js',
'gsd-check-update.js',
// SessionStart canonical-path bootstrap (#997). In a Claude Code marketplace
// plugin install, ~/.claude/gsd-core is never created, so every
// `@~/.claude/gsd-core/...` include in agents/commands/templates resolves to
// nothing. This hook symlinks the canonical path's immutable subdirs to the
// plugin's bundled gsd-core/ tree; no-op in classic installs. Must ship to
// dist so the installer copies it into the target hooks/ dir.
'gsd-ensure-canonical-path.js',
// Required by gsd-check-update-worker.js at runtime — must ship alongside it
// so require('./managed-hooks-registry.cjs') resolves in the installed hooks/ dir.
'managed-hooks-registry.cjs',
'gsd-context-monitor.js',
// Cursor lifecycle hooks (#777 + ADR-1239/#2089): 6 managed events
'gsd-cursor-session-start.js',
'gsd-cursor-post-tool.js',
'gsd-cursor-pre-tool.js',
'gsd-cursor-stop.js',
'gsd-cursor-subagent-start.js',
'gsd-cursor-subagent-stop.js',
// Windsurf/Cascade lifecycle hooks (ADR-1239/#2100 Stage 2): 2 blocking events
'gsd-windsurf-pre-write.js',
'gsd-windsurf-pre-command.js',
// Claude Code FileChanged hook (#770) — hot-reloads gsd config when
// .planning/config.json changes mid-session. Must ship to dist so the
// installer can copy it to the target hooks/ dir and register FileChanged.
'gsd-config-reload.js',
// Agent-dispatch isolation guard (#3045): blocks an executor Agent()
// dispatch missing its harness isolation parameter when the project
// resolves to harness-worktree. Requires the sibling
// gsd-core/bin/lib/{runtime-name-policy,capability-registry}.cjs modules
// at runtime — those ship as part of the full gsd-core/ tree, not via this
// list.
'gsd-agent-isolation-guard.js',
'gsd-prompt-guard.js',
'gsd-read-guard.js',
'gsd-read-injection-scanner.js',
'gsd-statusline.js',
'gsd-update-banner.js',
'gsd-workflow-guard.js',
'gsd-worktree-path-guard.js',
// Catastrophic-shrink guard for curated .planning/ artifacts (#2255, fix 3 of #973)
'gsd-write-guard.js',
// Community hooks (bash, opt-in via .planning/config.json hooks.community)
'gsd-session-state.sh',
'gsd-validate-commit.sh',
'gsd-phase-boundary.sh',
// Graphify auto-update hook (#3347 / PR #3557 / #3579). Opt-in via
// .planning/config.json graphify.auto_update; off by default.
'gsd-graphify-update.sh'
];
// Subdirectories under hooks/ whose contents must also ship to dist. Each
// entry is copied as `hooks/<dir>/*` → `hooks/dist/<dir>/*` so detached
// helpers (e.g. hooks/lib/gsd-graphify-rebuild.sh) resolve from the hook's
// installed runtime path. See #3579.
const HOOKS_SUBDIRS_TO_COPY = ['lib'];
// Sync millisecond sleep using Atomics.wait on a throwaway SharedArrayBuffer.
// Used between Windows rename retries; this script is sync end-to-end so
// setTimeout would not work. Total worst-case backoff across MAX_ATTEMPTS
// is bounded (~400ms) — acceptable for a one-shot build script.
function sleepSync(ms) {
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
}
/**
* Atomic-replace via fs.renameSync, with Windows-only retry and fallback.
*
* POSIX rename(2) atomically replaces dest even when readers hold open
* handles on it. Windows MoveFileEx (which fs.renameSync uses with
* MOVEFILE_REPLACE_EXISTING) cannot — it throws EPERM/EBUSY when another
* process has the destination open. Concurrent install.js readers and
* antivirus scanners are the realistic triggers; both release handles
* within milliseconds, so a short backoff resolves the race. After
* retries are exhausted, fall back to copy-then-unlink (re-introduces
* the truncate-then-write race for this single file but keeps the build
* moving rather than crashing). If even copy fails because dest is hard-
* locked, log a non-fatal warning and leave the prior dest in place — a
* subsequent build invocation will retry from a fresh state.
*/
function renameAtomicWithRetry(stagedDest, dest, hook) {
if (process.platform !== 'win32') {
fs.renameSync(stagedDest, dest);
return;
}
const BACKOFFS_MS = [10, 30, 90, 270];
for (let attempt = 0; attempt <= BACKOFFS_MS.length; attempt++) {
try {
fs.renameSync(stagedDest, dest);
return;
} catch (e) {
const transient = e && (e.code === 'EPERM' || e.code === 'EBUSY');
if (!transient) throw e;
if (attempt < BACKOFFS_MS.length) {
sleepSync(BACKOFFS_MS[attempt]);
continue;
}
// Retries exhausted; fall back to copy-then-unlink.
try {
fs.copyFileSync(stagedDest, dest);
try { fs.unlinkSync(stagedDest); } catch (_) { /* tolerate */ }
console.warn(`\x1b[33m! ${hook}: rename failed (${e.code}) after ${BACKOFFS_MS.length} retries; used copy-fallback\x1b[0m`);
return;
} catch (fallbackErr) {
try { fs.unlinkSync(stagedDest); } catch (_) { /* tolerate */ }
console.warn(`\x1b[33m! ${hook}: rename + copy fallback both failed (${e.code} → ${fallbackErr.code || fallbackErr.message}); leaving prior dest in place\x1b[0m`);
return;
}
}
}
}
/**
* Validate JavaScript syntax without executing the file.
* Catches SyntaxError (duplicate const, missing brackets, etc.)
* before the hook gets shipped to users.
*/
function validateSyntax(filePath) {
const content = fs.readFileSync(filePath, 'utf8');
try {
// Use vm.compileFunction to check syntax without executing
new vm.Script(content, { filename: path.basename(filePath) });
return null; // No error
} catch (e) {
if (e instanceof SyntaxError) {
return e.message;
}
throw e;
}
}
function build() {
// Ensure dist and staging directories exist (staging is a sibling of dist
// used to make writes atomic — see STAGE_DIR comment above).
if (!fs.existsSync(DIST_DIR)) {
fs.mkdirSync(DIST_DIR, { recursive: true });
}
if (!fs.existsSync(STAGE_DIR)) {
fs.mkdirSync(STAGE_DIR, { recursive: true });
}
let hasErrors = false;
// Copy hooks to dist with syntax validation
for (const hook of HOOKS_TO_COPY) {
const src = path.join(HOOKS_DIR, hook);
const dest = path.join(DIST_DIR, hook);
if (!fs.existsSync(src)) {
console.warn(`Warning: ${hook} not found, skipping`);
continue;
}
// Validate JS syntax before copying (.sh files skip — not Node.js)
if (hook.endsWith('.js')) {
const syntaxError = validateSyntax(src);
if (syntaxError) {
console.error(`\x1b[31m✗ ${hook}: SyntaxError — ${syntaxError}\x1b[0m`);
hasErrors = true;
continue;
}
}
console.log(`\x1b[32m✓\x1b[0m Copying ${hook}...`);
// Atomic write: copy to a per-process staging file in the per-PID sibling
// STAGE_DIR (same filesystem as DIST_DIR so rename(2) is atomic), then
// rename into place. Multiple test files invoke this script concurrently
// from their before() hooks; fs.copyFileSync truncates then writes the
// destination — readers (install.js subprocesses spawned by parallel
// install tests) can observe the dest empty or partial mid-write,
// producing flaky failures such as bug-2136 part 4 where installed .sh
// hooks lacked their "# gsd-hook-version:" header. POSIX rename(2)
// makes the swap atomic so readers see either the old file or the new
// file. The staging file lives outside DIST_DIR so readdirSync(DIST_DIR)
// (in install.js and tests) never observes a transient ".tmp" sibling.
// Each process uses its own STAGE_DIR (keyed by PID) so concurrent
// builders never race on staging-dir creation or cleanup.
const stagedDest = path.join(STAGE_DIR, `${hook}.${Date.now()}`);
fs.copyFileSync(src, stagedDest);
// Preserve executable bit for shell scripts before rename so the
// installed file is executable from the very first observation.
if (hook.endsWith('.sh')) {
try { fs.chmodSync(stagedDest, 0o755); } catch (e) { /* Windows */ }
}
renameAtomicWithRetry(stagedDest, dest, hook);
}
// Copy whitelisted hook subdirectories (e.g. hooks/lib/) into dist so the
// installer's readdir-and-isFile loop in bin/install.js sees them and
// detached hook helpers resolve from the installed runtime path (#3579).
for (const subdir of HOOKS_SUBDIRS_TO_COPY) {
const srcDir = path.join(HOOKS_DIR, subdir);
if (!fs.existsSync(srcDir)) continue;
const destDir = path.join(DIST_DIR, subdir);
fs.mkdirSync(destDir, { recursive: true });
const entries = fs.readdirSync(srcDir, { withFileTypes: true });
for (const ent of entries) {
if (!ent.isFile()) continue;
const srcFile = path.join(srcDir, ent.name);
const destFile = path.join(destDir, ent.name);
if (ent.name.endsWith('.js')) {
const syntaxError = validateSyntax(srcFile);
if (syntaxError) {
console.error(`\x1b[31m✗ ${subdir}/${ent.name}: SyntaxError — ${syntaxError}\x1b[0m`);
hasErrors = true;
continue;
}
}
console.log(`\x1b[32m✓\x1b[0m Copying ${subdir}/${ent.name}...`);
const stagedDest = path.join(STAGE_DIR, `${subdir}__${ent.name}.${Date.now()}`);
fs.copyFileSync(srcFile, stagedDest);
if (ent.name.endsWith('.sh')) {
try { fs.chmodSync(stagedDest, 0o755); } catch (e) { /* Windows */ }
}
renameAtomicWithRetry(stagedDest, destFile, `${subdir}/${ent.name}`);
}
}
// Best-effort cleanup of this process's own staging dir. Since STAGE_DIR
// is per-PID (`.dist-staging-<pid>/`), no other builder touches it — so
// rmSync with recursive:true is safe and leaves no race window.
try {
fs.rmSync(STAGE_DIR, { recursive: true, force: true });
} catch (e) { /* tolerate ENOENT if the dir was never created (e.g. all hooks skipped) */ }
if (hasErrors) {
console.error('\n\x1b[31mBuild failed: fix syntax errors above before publishing.\x1b[0m');
process.exit(1);
}
console.log('\nBuild complete.');
}
// Export HOOKS_TO_COPY so tests can require() this file and assert against
// the typed value instead of regex-parsing the source text (retires
// pending-migration-to-typed-ir for orphaned-hooks.test.cjs, per #455).
// Guard the build() call so requiring this file as a module does not trigger
// a full build run (which copies files and writes to disk).
if (require.main === module) {
build();
}
module.exports = { HOOKS_TO_COPY, HOOKS_SUBDIRS_TO_COPY };