From 66b3b15fb605e4dd74a4ea7fb2452f92a3c319d6 Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Thu, 18 Jun 2026 12:03:11 +0200 Subject: [PATCH] =?UTF-8?q?chore(voyage):=20W3=20hardening=20(S2)=20?= =?UTF-8?q?=E2=80=94=20exec-form=20hooks,=20enforced=20disallowed-tools,?= =?UTF-8?q?=20F2=20decision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S2 of the 2.1.181 upgrade. Schemas verified verbatim against the official slash-commands and hooks docs before editing (a first-pass camelCase 'disallowedTools' claim was caught and corrected to kebab-case against the doc). - CC-14 (SHIP): migrate all 7 hooks in hooks/hooks.json to exec-form {command:"node", args:["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/X.mjs"]}. Doc recommends exec-form whenever a hook references a path placeholder; protects consumers installing under a path with spaces. ${CLAUDE_PLUGIN_ROOT} interpolates inside args (verified). hooks-json-stop-wired test made form-agnostic (normalizes command+args to one invocation string). - CC-11 (SHIP): add `disallowed-tools: Agent, TeamCreate` to trekexecute frontmatter, enforcing its documented "No Agent tool, no TeamCreate" rule. allowed-tools grants auto-approval but does NOT remove tools from the pool, so the prior omission left Agent callable; disallowed-tools removes it. trekexecute is the only command with a documented exclusion. - CC-15 (DECIDE: keep universal): re-affirm F2 deferral. pre-bash/pre-write executors stay universal -- session-agnostic safety (rm -rf /, ~/.ssh, .env) that narrowing to execute-only would only weaken. Header comments corrected. - CC-10 (DECIDE: design note, no code): no blanket Agent(model:opus) deny rule -- would break balanced/economy profiles; any model-enforcement must be profile-aware, deferred into W2. Folded into open question #3. Matrix updated with S2 resolutions section. Tests 578 pass / 0 fail / 2 skip; claude plugin validate passes (only pre-existing root-CLAUDE.md warning). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01LqBYc8Ltrk7LipyJmGxXiB --- commands/trekexecute.md | 6 ++++- docs/cc-upgrade-2.1.181-decision-matrix.md | 17 +++++++++---- hooks/hooks.json | 21 ++++++++++------ hooks/scripts/pre-bash-executor.mjs | 6 ++++- hooks/scripts/pre-write-executor.mjs | 6 ++++- tests/hooks/hooks-json-stop-wired.test.mjs | 28 ++++++++++++++-------- 6 files changed, 60 insertions(+), 24 deletions(-) diff --git a/commands/trekexecute.md b/commands/trekexecute.md index d6b8115..01b4c6f 100644 --- a/commands/trekexecute.md +++ b/commands/trekexecute.md @@ -4,6 +4,7 @@ description: Disciplined plan executor — single-session or multi-session with argument-hint: "[--project ] [--fg | --resume | --dry-run | --validate | --step N | --session N] [plan.md]" model: opus allowed-tools: Read, Write, Edit, Bash, Glob, Grep, AskUserQuestion +disallowed-tools: Agent, TeamCreate --- # Ultraexecute Local @@ -1606,7 +1607,10 @@ code-path. 9. **Progress file is ground truth.** Resume uses the progress file, not git log. 10. **No sub-agents.** The executor reads and implements directly. - No Agent tool, no TeamCreate, no delegation. + No Agent tool, no TeamCreate, no delegation. Enforced (not just + documented) via `disallowed-tools: Agent, TeamCreate` in the frontmatter, + which removes both tools from the pool — `allowed-tools` alone does not, + since omission leaves a tool callable (CC 2.1.152). 11. **Worktree isolation is mandatory for parallel execution.** Every parallel `claude -p` session MUST run in its own git worktree. Never launch two or diff --git a/docs/cc-upgrade-2.1.181-decision-matrix.md b/docs/cc-upgrade-2.1.181-decision-matrix.md index ed5570c..1bf1f4d 100644 --- a/docs/cc-upgrade-2.1.181-decision-matrix.md +++ b/docs/cc-upgrade-2.1.181-decision-matrix.md @@ -58,17 +58,26 @@ | ID | Change (version) | Type | Voyage relevance | Decision | Rationale | |----|------------------|------|------------------|----------|-----------| -| **CC-10** | `Tool(param:value)` permission syntax, e.g. `Agent(model:opus)` (**2.1.178**, verified) | NEW | Voyage pins `opus` in 23 agent frontmatters *by convention*; this could enforce it as a hard permission rule | **EVALUATE** ⚠️ | **Trap:** a blanket `Agent(model:opus)`-style rule conflicts with the `balanced`/`economy` profiles, which deliberately spawn `sonnet`. Any enforcement must be **profile-aware** → tie to W2. Do NOT ship a blanket opus-lock. | -| **CC-11** | `disallowed-tools` in command/skill frontmatter (2.1.152, verified) | NEW | tighten per-command tool surfaces (e.g. ensure `trekexecute` can't spawn Agents — it already documents "no Agent tool") | **EVALUATE** (low) | Promote existing *documented* tool exclusions into *enforced* `disallowed-tools`. Small, defense-in-depth. Check interaction with `allowed-tools`. | +| **CC-10** | `Tool(param:value)` permission syntax, e.g. `Agent(model:opus)` (**2.1.178**, verified) | NEW | Voyage pins `opus` in 23 agent frontmatters *by convention*; this could enforce it as a hard permission rule | **DECIDED (design note)** ⚠️ | **Trap:** a blanket `Agent(model:opus)`-style rule conflicts with the `balanced`/`economy` profiles, which deliberately spawn `sonnet`. Any enforcement must be **profile-aware** → tie to W2. Do NOT ship a blanket opus-lock. **S2 resolution: no rule added; deferred into W2** — see §S2 resolutions. | +| **CC-11** | `disallowed-tools` in command/skill frontmatter (2.1.152, verified) | NEW | tighten per-command tool surfaces (e.g. ensure `trekexecute` can't spawn Agents — it already documents "no Agent tool") | **SHIPPED ✅** | Promote existing *documented* tool exclusions into *enforced* `disallowed-tools`. Small, defense-in-depth. **S2: added `disallowed-tools: Agent, TeamCreate` to `trekexecute` frontmatter** (the only command with a documented exclusion). Verified `allowed-tools` omission does NOT remove a tool from the pool — `disallowed-tools` does. | | **CC-12** | Stop/SubagentStop hooks can return `hookSpecificOutput.additionalContext` without being a hook error (2.1.163) | NEW | `post-compact-flush.mjs` already emits `additionalContext`; SubagentStop is a new surface | **DEFER** | Possible richer continuity injection. No current gap forces it. | | **CC-13** | Stop/SubagentStop hook input now includes `background_tasks` + `session_crons` (2.1.145) | NEW | `otel-export.mjs` (Stop hook) observability | **DEFER** | Could enrich exported telemetry. Adopt when next touching the exporter. | -| **CC-14** | Hook `args: string[]` exec-form — no shell, no quoting (2.1.139) | NEW | Voyage's 7 hooks invoke node scripts via shell form with `${CLAUDE_PLUGIN_ROOT}` paths | **EVALUATE** (low) | Exec-form removes a class of path-quoting bugs. Low-risk robustness upgrade across `hooks/hooks.json`; verify each hook's invocation. | -| **CC-15** | Hook `if:` conditions for Read/Edit/Write paths now match reliably (2.1.139/176); reopens deferred **F2** (scope pre-bash/pre-write executors to execute sessions) | FIX | `pre-bash-executor.mjs`, `pre-write-executor.mjs` are currently universal | **EVALUATE** | F2 was deferred with "universal protection wins." The `if:` mechanism now works, so the *option* is real again — but the original rationale (brief/plan sessions also benefit from the denylist) may still hold. Re-decide explicitly, don't auto-adopt. | +| **CC-14** | Hook `args: string[]` exec-form — no shell, no quoting (2.1.139) | NEW | Voyage's 7 hooks invoke node scripts via shell form with `${CLAUDE_PLUGIN_ROOT}` paths | **SHIPPED ✅** | Exec-form removes a class of path-quoting bugs. **S2: migrated all 7 hooks** in `hooks/hooks.json` to `{command:"node", args:["${CLAUDE_PLUGIN_ROOT}/…"]}`. Official hooks doc recommends exec-form *"whenever the hook references a path placeholder"* — protects consumers who install Voyage under a path containing spaces. `${CLAUDE_PLUGIN_ROOT}` interpolates in `args` (verified). | +| **CC-15** | Hook `if:` conditions for Read/Edit/Write paths now match reliably (2.1.139/176); reopens deferred **F2** (scope pre-bash/pre-write executors to execute sessions) | FIX | `pre-bash-executor.mjs`, `pre-write-executor.mjs` are currently universal | **DECIDED (keep universal)** | F2 was deferred with "universal protection wins." The `if:` mechanism now works, so the *option* is real again — but the original rationale holds. **S2 resolution: KEEP UNIVERSAL.** These guardrails (rm -rf /, fork bombs; writes to ~/.ssh, .env, .git/hooks) are session-agnostic safety; narrowing to execute-only would only weaken protection with no benefit and never interferes with brief/research/plan work. Header comments corrected to state the universal scope. | | **CC-16** | `SessionStart` `reloadSkills` + `sessionTitle` (2.1.152) | NEW | Voyage sets session title via `UserPromptSubmit` (`session-title.mjs`) | **SKIP** | Current mechanism works and is command-scoped (title reflects the invoked `/trek*` command). SessionStart-title would fire before the command is known. No gain. | | **CC-17** | Hook `terminalSequence` output — notifications/bells without a TTY (2.1.141); `continueOnBlock` for PostToolUse (2.1.139); `MessageDisplay` event (2.1.152); Stop block-cap 8 (2.1.143) | NEW | minor ergonomics; Voyage hooks are fail-open and non-interactive | **SKIP/DEFER** | No current need. `MessageDisplay`/`continueOnBlock` SKIP (no use case); `terminalSequence` DEFER (could notify on long headless waves). Block-cap is informational. | --- +## S2 resolutions (W3 hardening — 2026-06-18) + +S2 shipped the mechanical W3 items and recorded the two W3 decisions. Schemas verified verbatim against the official slash-commands and hooks docs before any edit (the casing of `disallowed-tools` and exec-form `${CLAUDE_PLUGIN_ROOT}` interpolation are both load-bearing; a first-pass assistant claim of `disallowedTools` camelCase was caught and corrected against the doc). + +- **CC-14 — SHIPPED.** All 7 hooks in `hooks/hooks.json` migrated to exec-form (`command:"node"`, `args:["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/X.mjs"]`). `tests/hooks/hooks-json-stop-wired.test.mjs` updated to be form-agnostic (normalizes `command`+`args` to one invocation string). 578/580 pass, 0 fail. +- **CC-11 — SHIPPED.** `disallowed-tools: Agent, TeamCreate` added to `commands/trekexecute.md`, promoting its documented "No Agent tool, no TeamCreate, no delegation" rule from prose to enforcement. Key fact: `allowed-tools` grants auto-approval but does **not** remove tools from the pool (doc, slash-commands.md), so the prior omission left Agent callable; `disallowed-tools` removes it. trekexecute is the only command with a documented exclusion (trekplan's "never spawn more agents than warranted" is an adaptive-count guideline, not an exclusion — trekplan legitimately uses Agent). +- **CC-15 — DECIDED (keep universal).** F2 stays deferred: `pre-bash-executor.mjs` / `pre-write-executor.mjs` remain universal. Rationale re-affirmed explicitly now that `if:` works — narrowing to execute-only sessions would weaken session-agnostic safety with no benefit. Header comments corrected to state the universal scope by design. +- **CC-10 — DECIDED (design note; no code).** Do **not** add a `permissions.deny` rule like `Agent(model:opus)`. Such rules live in `settings.json` permissions (colon separator inside parens, confirmed 2.1.178), not in frontmatter, and a blanket opus-lock would break the `balanced`/`economy` profiles that deliberately spawn `sonnet`. If model-enforcement is ever wanted, it must be **profile-aware** and emitted as part of the W2 model/effort work (S3/S4) — e.g. a profile could ship its own `availableModels`/deny set. Until then, the `model: opus` convention in the 23 agent frontmatters stays advisory. Folded into W2 open question #3. + ## W2 — Model & effort alignment (gates v5.4) | ID | Change (version) | Type | Voyage relevance | Decision | Rationale | diff --git a/hooks/hooks.json b/hooks/hooks.json index e403b20..b3ae7bd 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -6,7 +6,8 @@ "hooks": [ { "type": "command", - "command": "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-bash-executor.mjs" + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-bash-executor.mjs"] } ] }, @@ -15,7 +16,8 @@ "hooks": [ { "type": "command", - "command": "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-write-executor.mjs" + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-write-executor.mjs"] } ] } @@ -25,7 +27,8 @@ "hooks": [ { "type": "command", - "command": "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/session-title.mjs" + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/session-title.mjs"] } ] } @@ -36,7 +39,8 @@ "hooks": [ { "type": "command", - "command": "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/post-bash-stats.mjs" + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/post-bash-stats.mjs"] } ] } @@ -46,7 +50,8 @@ "hooks": [ { "type": "command", - "command": "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-compact-flush.mjs" + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/pre-compact-flush.mjs"] } ] } @@ -56,7 +61,8 @@ "hooks": [ { "type": "command", - "command": "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/post-compact-flush.mjs" + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/post-compact-flush.mjs"] } ] } @@ -66,7 +72,8 @@ "hooks": [ { "type": "command", - "command": "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/otel-export.mjs" + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/hooks/scripts/otel-export.mjs"] } ] } diff --git a/hooks/scripts/pre-bash-executor.mjs b/hooks/scripts/pre-bash-executor.mjs index 97fe6ab..68f4682 100644 --- a/hooks/scripts/pre-bash-executor.mjs +++ b/hooks/scripts/pre-bash-executor.mjs @@ -1,7 +1,11 @@ #!/usr/bin/env node // Hook: pre-bash-executor.mjs // Event: PreToolUse (Bash) -// Purpose: Block or warn about destructive shell commands during plan execution. +// Purpose: Block or warn about destructive shell commands. Wired universally +// (every Voyage session, not just execution) by deliberate decision — these +// are session-agnostic safety rails (rm -rf /, fork bombs, …). The CC if: +// path-scoping mechanism now works, but narrowing to execute-only would only +// weaken protection with no offsetting benefit. See cc-upgrade matrix CC-15/F2. // // Protocol: // - Read JSON from stdin: { tool_name, tool_input } diff --git a/hooks/scripts/pre-write-executor.mjs b/hooks/scripts/pre-write-executor.mjs index 7a65fee..2ff4031 100644 --- a/hooks/scripts/pre-write-executor.mjs +++ b/hooks/scripts/pre-write-executor.mjs @@ -1,7 +1,11 @@ #!/usr/bin/env node // Hook: pre-write-executor.mjs // Event: PreToolUse (Write) -// Purpose: Block writes to security-sensitive paths during plan execution. +// Purpose: Block writes to security-sensitive paths. Wired universally (every +// Voyage session, not just execution) by deliberate decision — protecting +// ~/.ssh, ~/.aws, .git/hooks, .env, shell configs benefits every session. +// The CC if: path-scoping mechanism now works, but narrowing to execute-only +// would only weaken protection with no benefit. See cc-upgrade matrix CC-15/F2. // // Protocol: // - Read JSON from stdin: { tool_name, tool_input } diff --git a/tests/hooks/hooks-json-stop-wired.test.mjs b/tests/hooks/hooks-json-stop-wired.test.mjs index dc9523d..e5cba16 100644 --- a/tests/hooks/hooks-json-stop-wired.test.mjs +++ b/tests/hooks/hooks-json-stop-wired.test.mjs @@ -16,6 +16,13 @@ function loadHooksJson() { return JSON.parse(raw); } +// Hooks may use shell form ({command: "node X.mjs"}) or exec form +// ({command: "node", args: ["X.mjs"]}, CC 2.1.139). Normalize both to a single +// invocation string so assertions are form-agnostic. +function invocationOf(h) { + return [h.command || '', ...(h.args || [])].join(' ').trim(); +} + test('hooks.json — Stop key exists with at least one entry', () => { const cfg = loadHooksJson(); assert.ok(cfg.hooks, 'hooks.json mangler top-level "hooks" object'); @@ -26,13 +33,13 @@ test('hooks.json — Stop key exists with at least one entry', () => { test('hooks.json — Stop entry refererer otel-export.mjs', () => { const cfg = loadHooksJson(); const stopEntries = cfg.hooks.Stop; - const allCommands = stopEntries.flatMap((entry) => - (entry.hooks || []).map((h) => h.command || ''), + const allInvocations = stopEntries.flatMap((entry) => + (entry.hooks || []).map(invocationOf), ); - const hasOtelExport = allCommands.some((cmd) => cmd.includes('otel-export.mjs')); + const hasOtelExport = allInvocations.some((cmd) => cmd.includes('otel-export.mjs')); assert.ok( hasOtelExport, - `ingen Stop-hook refererer otel-export.mjs. Funnet: ${JSON.stringify(allCommands)}`, + `ingen Stop-hook refererer otel-export.mjs. Funnet: ${JSON.stringify(allInvocations)}`, ); }); @@ -41,17 +48,18 @@ test('hooks.json — Stop entry bruker ${CLAUDE_PLUGIN_ROOT}-substitusjon', () = const stopEntries = cfg.hooks.Stop; const otelEntry = stopEntries .flatMap((entry) => entry.hooks || []) - .find((h) => (h.command || '').includes('otel-export.mjs')); + .find((h) => invocationOf(h).includes('otel-export.mjs')); assert.ok(otelEntry, 'fant ikke otel-export-entry i Stop'); + const otelInvocation = invocationOf(otelEntry); assert.match( - otelEntry.command, + otelInvocation, /\$\{CLAUDE_PLUGIN_ROOT\}/, - 'otel-export-command bruker ikke ${CLAUDE_PLUGIN_ROOT}-prefix — relative paths feiler i headless', + 'otel-export-invocation bruker ikke ${CLAUDE_PLUGIN_ROOT}-prefix — relative paths feiler i headless', ); assert.match( - otelEntry.command, + otelInvocation, /^node\s+/, - 'otel-export-command starter ikke med "node " — invocation-form ikke korrekt', + 'otel-export-invocation starter ikke med "node " — invocation-form ikke korrekt', ); }); @@ -60,6 +68,6 @@ test('hooks.json — Stop entry har "type": "command"', () => { const stopEntries = cfg.hooks.Stop; const otelHook = stopEntries .flatMap((entry) => entry.hooks || []) - .find((h) => (h.command || '').includes('otel-export.mjs')); + .find((h) => invocationOf(h).includes('otel-export.mjs')); assert.equal(otelHook.type, 'command', 'otel-export-hook mangler "type": "command"'); });