fix(security): close the untrusted-repo execution boundary (#905)

[frankbria]

Closes #905.

Problem

Cloning an untrusted repository and running any cf command was equivalent to running its code, and a prompt-injected agent could read the credential store.

Four distinct doors:

1. Repo-committed hooks. run_hook executes hook strings with shell=True, and those strings come from files a repository can commit. cf init fires after_init immediately; cf work start fires before_task/after_task_*.
2. The walk-up CODEFRAME.md fallback could supply hooks: — and that file is located by walking up from the workspace, so even a parent directory could inject commands.
3. render_hook_command's shlex.quote was not the protection it claimed. Single quotes are inert inside a double-quoted template, so echo "{{ task_title }}" with a title of $(id) rendered to echo "'$(id)'" — and the shell ran the substitution.
4. run_command passed the operator's HOME. The [P0.10] Do not pass the operator's full secret environment into LLM-driven run_command #721 allowlist kept secrets out of the environment, but HOME is a pointer to them: ~/.codeframe holds the credential store, whose Fernet key is machine-id-derived unless CODEFRAME_CREDENTIAL_SECRET is set.

Fix

Trust gate (codeframe/core/hook_trust.py, new). A hook runs only if the operator recorded a decision for these exact commands in this workspace. The record lives in ~/.codeframe/trusted_hooks.json — outside the repo tree, so a repo cannot grant itself trust by committing the file — and is keyed by a sha256 of the commands, so editing a hook revokes the old approval instead of inheriting it. Enforced in execute_hook, the single point every hook execution passes through (CLI, runtime, batch, server). A refusal reuses the existing failure path, so each caller's abort/warn handling applies unchanged.

• cf hooks trust records the decision, printing the exact commands first.
• cf hooks show now reports the trust state — a configured-but-untrusted hook silently never running would be worse than the vulnerability.
• cf hooks set / clear record trust themselves: those are operator actions on this machine, and the fingerprint is command-keyed, so otherwise setting your own hook would immediately refuse to run it.
• cf init --allow-hooks / CODEFRAME_ALLOW_HOOKS=1 is the non-interactive opt-in.
• A corrupt store reads as "nothing is trusted", never as a blanket pass.

CODEFRAME.md no longer supplies hooks at all (core/config.py). It still supplies every other setting; a declared hooks: block is ignored with a warning.

Context values leave the command text (core/hooks.py). The template now renders "${CF_HOOK_TASK_TITLE}" references, and the values travel in the subprocess environment. A parameter expansion is safe quoted and unquoted, because the shell does not rescan an expansion's result for command substitution — so $(id) stays four characters either way.

Sandboxed HOME (core/agent_env.py, new leaf module). build_agent_env() is the single place the allowlist and the sandbox are built: HOME and XDG_* point at <workspace>/.codeframe/agent-home — a real, writable directory so npm/pip/cargo/git still work. If it cannot be created, all four are dropped rather than falling back to the operator's.

Three callers converge on it, because patching only the one the issue named would have left the siblings open (all three found in review):

Caller	Was
run_command (ReAct)	allowlisted env, operator's HOME
run_tests (ReAct)	no env= at all — npm test runs a repo-committed package.json script
Executor._execute_shell_command (legacy --engine plan)	no env= at all, on both the shell=True and argv branches

Defense in depth (core/dangerous_commands.py): commands naming .codeframe/credentials are refused, catching the absolute path an agent can still discover via ls /home.

Tests

tests/core/test_untrusted_repo_execution_905.py — 19 tests. The hook tests assert against a canary file, so they prove the command never ran rather than that we printed a refusal. Two end-to-end tests run cf init against a repo carrying a hostile hooks block (one via CODEFRAME.md, one via .codeframe/config.yaml) and assert the canary is absent.

Mutation-checked: reverting each of the four fixes independently fails 12 tests.

Known limitations

• Existing workspaces with configured hooks need a one-time cf hooks trust. This is the intended fail-closed behavior, but it is a behavior change for anyone already using hooks — including via the web UI, which has no trust UI yet (CLI/env only).
• The HOME sandbox is not containment. It closes ~, $HOME and the XDG_* paths; the credential-store pattern catches the obvious absolute path. A deliberately obfuscated path still reaches the store — only OS-level isolation (worktree/E2B/container) actually contains a hostile shell command.
• cf review / cf proof run gates still run repo-controlled scripts with the operator's environment (core/gates.py) — npm test, npm run build, type-check, plus test_command/lint_command from .codeframe/config.yaml. Same shape as the run_tests fix and now a one-line change each, but it alters a user-invoked command's environment (a project whose suite legitimately needs ~/.npmrc or ~/.aws would break), so it belongs in its own issue rather than riding along in a security fix. Filed as Gates and proof runs execute repo-controlled scripts with the operator's environment #995.
• Delegated agent adapters inherit the full operator environment (core/adapters/subprocess_adapter.py, adapters/codex.py). --engine claude-code / --engine codex spawn a CLI that legitimately needs provider credentials, so sanitizing them is a design question, not a one-liner. Filed as [P0.25] Delegated agent adapters inherit the operator's full environment and HOME #996.

[github-actions]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[github-actions]

Claude finished @frankbria's task in 7m 4s —— View job

---

Bug-hunting review complete

Scoped to concrete defects only (logic, security, data-loss, contracts); style/coverage left to the general review.

• Gather PR diff + existing comments
• Read each changed file in full context (hook_trust.py, hooks.py, tools.py, config.py, dangerous_commands.py, hooks_commands.py, app.py)
• Trace callers/callees to confirm suspected defects
• Post inline findings + summary

Result: 1 minor finding.

Severity	File:line	Finding
minor	codeframe/core/tools.py:859	run_command HOME-sandbox except OSError branch drops HOME but leaves XDG_* at the operator's real values, so the fail-closed / XDG-pinning guarantee holds only on the success path. An OSError (read-only/ENOSPC/symlinked .codeframe) plus an operator-set XDG_CONFIG_HOME lets a prompt-injected command read config-dir secrets (e.g. $XDG_CONFIG_HOME/gh/hosts.yml) that the .codeframe/credentials regex does not cover. One-line fix in the inline comment.

The four core fixes are sound and verified against callers: the trust gate is keyed on the resolved workspace + sha256 of the exact command set (edit revokes approval), the store lives outside the repo, CODEFRAME.md hooks are dropped entirely, the "${CF_HOOK_*}" templating defeats $(...) injection, and every hook path flows through the single gated execute_hook (run_hook has no other caller). No defects found in those.

Summary comment posted inline + via gh pr comment.

[github-actions]

GLM bug-hunting review

Scoped to concrete defects only (logic, security, data-loss, contracts). Style/coverage are covered by the general review.

Severity	File:line	Finding
minor	codeframe/core/tools.py:859	run_command HOME-sandbox except OSError branch drops HOME but leaves XDG_* at the operator's real values, so the "fail closed" / XDG-pinning guarantee only holds on the success path. An OSError (read-only/ENOSPC/symlinked .codeframe) plus an operator-set XDG_CONFIG_HOME lets a prompt-injected command read config-dir secrets (e.g. $XDG_CONFIG_HOME/gh/hosts.yml) that the .codeframe/credentials regex does not cover. See inline comment for a one-line fix.

Everything else checks out: the trust gate is keyed on resolve()d workspace + a sha256 of the exact command set (so any edit revokes approval), the store lives outside the repo, CODEFRAME.md hooks are fully dropped (not just walk-up-scoped), the "${CF_HOOK_*}" templating genuinely defeats command-substitution injection (the shell does not rescan an expansion's result), and every hook path flows through the single gated execute_hook (run_hook has no other caller). No defects found in those.

[frankbria]

Fixed the XDG finding in 013062c.

Confirmed real: XDG_CACHE_HOME/XDG_CONFIG_HOME/XDG_DATA_HOME are on _RUN_COMMAND_SAFE_ENV_VARS (tools.py:799), so the except OSError branch dropped HOME while leaving them pointing at the operator's real config dir — ~/.config/gh/hosts.yml is not covered by the credential-store pattern. The failure path was less closed than the success path.

The branch now pops all three alongside HOME. test_unbuildable_sandbox_drops_home_and_xdg forces the mkdir to raise and asserts the operator's home appears nowhere in the command's output; mutation-checked (reverting the pop fails it).

[github-actions]

Claude finished @frankbria's task in 3m 33s —— View job

---

GLM bug-hunting review

Scoped to concrete defects only (logic, security, data-loss, contracts). Style/coverage are covered by the general review.

• Gather PR diff + latest commit (013062c XDG fix)
• Verify the XDG fail-closed fix in tools.py
• Read core security files in full context (hook_trust.py, hooks.py, dangerous_commands.py, config.py)
• Trace callers/callees to confirm suspected defects
• Post inline findings + summary

✅ GLM review: no defects found (re-review of the synchronized code after 013062c).

The one finding from the prior pass — the run_command except OSError branch dropping HOME while leaving XDG_* at the operator's real values — is correctly resolved in 013062c. codeframe/core/tools.py:856-863 now pops HOME and all three of XDG_CACHE_HOME/XDG_CONFIG_HOME/XDG_DATA_HOME, so the fail-closed guarantee holds on both the success and failure paths.

Re-verified the four core fixes against their callers; nothing new surfaced:

• Trust gate (hook_trust.py) — is_trusted/record_trust both key on Path(workspace_path).resolve() and a sha256 over exactly the command strings (any edit revokes approval). Runtime (runtime.py:738) and cf hooks trust both pass workspace.repo_path, so the trust key is consistent across recording and execution — no silent trusted-but-never-runs mismatch. Corrupt/missing store reads as {} → fail-closed. Store lives under ~/.codeframe, outside any repo tree.
• CODEFRAME.md hooks dropped (config.py:440-445) — warns and does not map; only .codeframe/config.yaml supplies hooks, and even there the trust gate applies.
• Templating (hooks.py:95-114) — context values travel in the subprocess env as CF_HOOK_*; the template emits "${CF_HOOK_*}" references. Parameter-expansion results are not rescanned for command substitution, so $(...) in a value stays inert. run_hook is called only from the gated execute_hook.
• Credential-store guard (dangerous_commands.py:50) — \.codeframe/credentials is an unanchored substring, so it matches the real file credentials.encrypted (core/credentials.py:78); the defense-in-depth pattern is effective, not dead code.

No inline comments — nothing met the concrete-defect bar.

[github-actions]

Claude finished @frankbria's task in 5m 7s —— View job

---

Bug-hunting review (GLM, pass 3 — post-synchronize)

Scoped to concrete defects only (logic, security, data-loss, contracts). Style/coverage left to the general review.

• Gather PR diff + latest commit state
• Review prior passes — never repeat a posted finding
• Read each changed file in full context + trace callers/callees
• Post inline findings + summary

✅ GLM review: no defects found.

The prior finding (XDG fail-closed on the run_command except OSError branch) is correctly resolved in 013062c and now centralized in agent_env.py (build_agent_env pops HOME + all three XDG_* on the failure path). Re-verified the four core fixes and the defense-in-depth against real code — nothing new surfaced:

Area	Verdict
Trust gate unbypassable	run_hook has exactly one production caller — the gated execute_hook (hooks.py:229). Every hook path (CLI run, runtime before_task/after_task_*, cf init after_init) flows through it; the refusal reuses each caller's existing abort/warn path.
Trust-key consistency	is_trusted/record_trust both key on Path(...).resolve() of the workspace; set/clear/trust/run/show and runtime all resolve to workspace.repo_path, so recording and checking agree — no silent trusted-but-never-runs mismatch. A mismatch would fail closed anyway (both key + fingerprint must match).
No repo self-authorization	CODEFRAME_ALLOW_HOOKS is set only from the CLI --allow-hooks flag (app.py:205); config loading never mutates os.environ, so a cloned repo cannot flip the bypass or grant itself trust (store is ~/.codeframe, outside the tree).
Templating	Context values travel in the subprocess env as CF_HOOK_*; the template emits "${CF_HOOK_*}" refs. Shell does not rescan an expansion's result, so $(...) in a value stays inert — quoted or not. Edit/glob risk only, not injection.
CODEFRAME.md hooks dropped	Warned and never mapped (config.py); only .codeframe/config.yaml supplies hooks, and the trust gate still applies there.
Credential-store guard is live	Real file is credentials.encrypted (credentials.py:78); the unanchored \.codeframe/credentials substring matches it. Guard runs in both _execute_run_command (tools.py:813) and the legacy executor (executor.py:483) — the two sibling callers the env fix also covered.
Sandboxed HOME	build_agent_env is the single construction point for all three callers (ReAct run_command, run_tests, plan-engine executor); fail-closed drops HOME + XDG_* on OSError.
Fingerprint scope	Covers all 5 hook-command string fields; excludes hook_timeout (int) — keyed on the commands, so any edit revokes approval.

No inline comments — nothing met the concrete-defect bar.

[frankbria]

Cross-family adversarial review (codex) — 5 findings triaged

Fixed (2) — both were sibling callers of the bug I patched, the same shape as the original:

• run_tests passed no env= at all. npm test runs a repo-committed package.json script, so a hostile repo + the agent calling that tool was enough to read ~/.codeframe.
• The legacy --engine plan executor passed no env= on either branch (shell=True and shlex-split argv).

Both now go through a new leaf module core/agent_env.py:build_agent_env(), which is also where run_command's allowlist and sandbox moved. One convergence point rather than three call sites.

Worth recording: my first two tests for these were tautological and I caught it by mutation-checking. The plan-engine one used cat \$HOME/.codeframe/credentials.encrypted, which the dangerous-command regex blocks before execution — it proved the regex, not the sandbox. And \$HOME there never expands, because that command has no shell operator so it takes the argv branch. The rewritten tests use a non-matching path (~/.ssh/id_rsa), exercise both branches, and for run_tests assert on a \$HOME value the subprocess recorded into a file rather than on tool output (run_tests returns only a summary line when tests pass, which swallowed the leak). Deleting either fix now fails them.

Accepted as known, already documented (1)

• Obfuscated absolute paths still reach the store (python3 -c '...glob("cred*")...'). True, and stated in both the code comment and Known Limitations: the regex is defense-in-depth against a prompt-injected agent typing the obvious command, not a containment boundary. Only OS-level isolation contains a hostile command.

Deferred to their own issues (2) — both real, both out of scope for this PR:

• cf review / cf proof run gates (core/gates.py) run repo-controlled npm scripts with the operator's environment. Now a one-line change each given the shared helper, but it changes a user-invoked command's environment — a project whose suite legitimately needs ~/.npmrc or ~/.aws would break. That trade-off deserves its own review, not a ride-along in a security fix.
• Delegated agent adapters (subprocess_adapter.py, adapters/codex.py) inherit the full operator environment. Those CLIs need provider credentials to function, so sanitizing them is a design question rather than a one-liner.

Both are in Known Limitations in the PR body.