* 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>
271 lines
12 KiB
JavaScript
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 };
|