claudiomaradonna/claude-jail
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Activity Actions

fix: keep secrets out of the podman argv, add --dry-run #24

Merged
claudiomaradonna merged 9 commits from fix/secret-argv-and-dry-run into main 2026-07-27 14:39:44 +02:00
Conversation 0 Commits 9 Files changed 13 +1777 -23
claudiomaradonna commented 2026-07-27 14:39:20 +02:00
Owner
Copy link

Why

claude.sh built -e KEY=value arguments for every variable in .env.claude,
~/.claude-jail/env.default, and ANTHROPIC_API_KEY. Process arguments are
world-readable on Linux, so any user on the host could read the API key for the
whole life of the container:

ps aux | grep podman          # -e ANTHROPIC_API_KEY=sk-ant-...
cat /proc/<pid>/cmdline

Verified before the fix: with a two-line .env.claude, the key appeared in the
argv twice — once from the env-file loop, once from the explicit API-key line.

What changed

1. Env vars are forwarded by name only. -e KEY instead of -e KEY=value.
Podman reads each value from the environment it inherits from the wrapper, so
values never reach the command line. Wrapper-controlled variables whose container
value deliberately differs from the host's (SSH_AUTH_SOCK=/ssh-agent,
CLAUDE_CODE_WORKSPACE, TERM, COLUMNS, LINES) stay inline — name-only for
SSH_AUTH_SOCK would forward the host socket path and break agent forwarding.

A guard warns and skips when an env-file key collides with one of the wrapper's
own internal variable names, so a stray max_memory=… can't quietly leak the
wrapper's internals into the container.

2. --dry-run. Prints the exact podman run invocation and exits without
running it. The command is assembled into a single array that is either printed
or exec'd, so the printed command can't drift from the real one. No session
directory is created; diagnostics go to stderr, so stdout is exactly the command.

This composes with the change above: because argv now carries only variable
names, the printed command contains no secrets and needs no redaction.

3. Fixes a pre-existing silent-exit bug. In _load_env_file,
[[ -z "${!key:-}" ]] && export … was the last command in a while body. A
while loop returns its body's last status, so when the key was already set that
&& returned 1, the function returned 1, and set -euo pipefail killed the
script with no output at all.

Trigger: export a variable in your shell and list it last in .env.claude. A
trailing comment or blank line masks it, which is likely why it went unnoticed.

.env.claude, with ANTHROPIC_API_KEY exported in your shell exit
MY_VAR=ok / ANTHROPIC_API_KEY=sk-from-file 1, silent
same, plus a trailing # comment 0
ANTHROPIC_API_KEY=sk-from-file / MY_VAR=ok 0

4. Adds the project's first test suite — 26 tests, no new dependencies (no
bats, no shunit2). A stub podman on PATH records both the argv it received and
the environment it inherited, which is what makes the security property directly
assertable: the secret must be absent from argv and present in the environment.
Run with make test.

Behaviour changes worth knowing

  • Container env precedence. When a variable is set in both your shell and
    .env.claude, the container now receives the shell value. Previously the
    file's value leaked through even though the wrapper itself used the shell's.
    The new behaviour matches the precedence the README already documented.
  • Session: and .claudeignore diagnostics now go to stderr, so
    claude --dry-run . > cmd.sh yields a clean file.

Scope

Threat model is hiding secrets from other users on the host. Secrets remaining
in .env.claude plaintext on disk, and in /proc/<pid>/environ (readable only by
you and root), is unchanged and out of scope — as are at-rest encryption and
secret-manager integration.

Verification

$ make test
26 tests, 0 failures — ALL TESTS PASSED

End-to-end, against a real .env.claude:

check before after
secret value in podman argv 2 occurrences 0
secret reachable by podman yes yes (via inherited env)
SSH_AUTH_SOCK / TERM inline yes yes (unchanged)
--dry-run stdout n/a exactly 1 line, no secrets

Note on the plan document

docs/superpowers/plans/2026-07-27-secret-argv-and-dry-run.md is the design doc
this work was built from, committed for traceability. Drop those two docs(plan)
commits if you'd rather not carry it in the repo.

## Why `claude.sh` built `-e KEY=value` arguments for every variable in `.env.claude`, `~/.claude-jail/env.default`, and `ANTHROPIC_API_KEY`. Process arguments are world-readable on Linux, so any user on the host could read the API key for the whole life of the container: ```bash ps aux | grep podman # -e ANTHROPIC_API_KEY=sk-ant-... cat /proc/<pid>/cmdline ``` Verified before the fix: with a two-line `.env.claude`, the key appeared in the argv **twice** — once from the env-file loop, once from the explicit API-key line. ## What changed **1. Env vars are forwarded by name only.** `-e KEY` instead of `-e KEY=value`. Podman reads each value from the environment it inherits from the wrapper, so values never reach the command line. Wrapper-controlled variables whose container value deliberately differs from the host's (`SSH_AUTH_SOCK=/ssh-agent`, `CLAUDE_CODE_WORKSPACE`, `TERM`, `COLUMNS`, `LINES`) stay inline — name-only for `SSH_AUTH_SOCK` would forward the host socket path and break agent forwarding. A guard warns and skips when an env-file key collides with one of the wrapper's own internal variable names, so a stray `max_memory=…` can't quietly leak the wrapper's internals into the container. **2. `--dry-run`.** Prints the exact `podman run` invocation and exits without running it. The command is assembled into a single array that is either printed or `exec`'d, so the printed command can't drift from the real one. No session directory is created; diagnostics go to stderr, so stdout is exactly the command. This composes with the change above: because argv now carries only variable *names*, the printed command contains no secrets and needs no redaction. **3. Fixes a pre-existing silent-exit bug.** In `_load_env_file`, `[[ -z "${!key:-}" ]] && export …` was the last command in a `while` body. A `while` loop returns its body's last status, so when the key was already set that `&&` returned 1, the function returned 1, and `set -euo pipefail` killed the script **with no output at all**. Trigger: export a variable in your shell *and* list it last in `.env.claude`. A trailing comment or blank line masks it, which is likely why it went unnoticed. | `.env.claude`, with `ANTHROPIC_API_KEY` exported in your shell | exit | |---|---| | `MY_VAR=ok` / `ANTHROPIC_API_KEY=sk-from-file` | **1, silent** | | same, plus a trailing `# comment` | 0 | | `ANTHROPIC_API_KEY=sk-from-file` / `MY_VAR=ok` | 0 | **4. Adds the project's first test suite** — 26 tests, no new dependencies (no bats, no shunit2). A stub `podman` on `PATH` records both the argv it received and the environment it inherited, which is what makes the security property directly assertable: the secret must be *absent* from argv and *present* in the environment. Run with `make test`. ## Behaviour changes worth knowing - **Container env precedence.** When a variable is set in both your shell and `.env.claude`, the container now receives the **shell** value. Previously the file's value leaked through even though the wrapper itself used the shell's. The new behaviour matches the precedence the README already documented. - **`Session:` and `.claudeignore` diagnostics now go to stderr**, so `claude --dry-run . > cmd.sh` yields a clean file. ## Scope Threat model is **hiding secrets from other users on the host**. Secrets remaining in `.env.claude` plaintext on disk, and in `/proc/<pid>/environ` (readable only by you and root), is unchanged and out of scope — as are at-rest encryption and secret-manager integration. ## Verification ``` $ make test 26 tests, 0 failures — ALL TESTS PASSED ``` End-to-end, against a real `.env.claude`: | check | before | after | |---|---|---| | secret value in podman argv | 2 occurrences | **0** | | secret reachable by podman | yes | yes (via inherited env) | | `SSH_AUTH_SOCK` / `TERM` inline | yes | yes (unchanged) | | `--dry-run` stdout | n/a | exactly 1 line, no secrets | ## Note on the plan document `docs/superpowers/plans/2026-07-27-secret-argv-and-dry-run.md` is the design doc this work was built from, committed for traceability. Drop those two `docs(plan)` commits if you'd rather not carry it in the repo. 
Also ignore .claude/worktrees/ alongside the existing .claude
settings entry, so nested worktrees don't show up as untracked.

Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
Task 1 review flagged a self-contradiction: Global Constraints bound
all new shell files to _-prefixed internals, but the plan's own
tests/helpers.sh block uses conventional unprefixed harness names.
Human ruling: the rule describes claude.sh, not tests/.

Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
.env.claude values (including ANTHROPIC_API_KEY) were being passed as
`-e KEY=value` podman arguments, which are world-readable via `ps aux`
and /proc/<pid>/cmdline for the life of the container. Forward by name
only (`-e KEY`) so podman reads the value from its own environment,
inherited from the wrapper, instead.

A name is only forwardable if it is an actual exported shell variable;
script-local internals (e.g. `workspace`) are skipped with a warning
rather than silently forwarding nothing.

Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
_is_exported alone only detects a .env.claude key colliding with a
script-internal that already had a value before Phase 4 (e.g. workspace).
Internals assigned later in Phase 5 (network, max_memory, session_id,
image_name, container_workdir, with_ssh, ignore_file) are unset when
_load_env_file runs, so it exports the user's value; Phase 5 then
overwrites the variable but bash keeps the export attribute, so Phase 8
wrongly treated it as a legitimately forwardable value — leaking the
wrapper's own internal value into the container under the user's key
(e.g. `max_memory=999g` in .env.claude silently forwarded the real
container memory limit as `max_memory`).

Record each key's intended value at load time in _env_intended, and in
Phase 8 require the live value to still match before forwarding by name.
A mismatch means Phase 5 clobbered it, so warn and skip exactly as the
workspace-collision case already does.

Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
Assembles the invocation into a single podman_cmd array so the printed
command can never drift from what exec would run. Session-directory
creation is skipped and reported instead of performed, and the
existing Session/.claudeignore diagnostics move to stderr so that
--dry-run stdout is exactly the copy-pasteable command.

Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
bash's `cd ""` returns 0 and stays in the current directory, so if
`mktemp -d` failed, `TMP_ROOT="$(cd "$(mktemp -d)" && pwd)"` silently
became the directory the suite was invoked from — the repo root. The
old teardown's `[[ -n "$TMP_ROOT" ]]` guard then passed and
`rm -rf "$TMP_ROOT"` deleted the working tree.

setup() now checks mktemp before use, and teardown() refuses to remove
anything outside ${TMPDIR:-/tmp}. tests/test_helpers_safety.sh covers
the guard with a canary outside the temp root.

Found by the final whole-branch review.

Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
Consolidated fix wave from the whole-branch final review (Finding 1, the
mktemp/rm-rf teardown hazard, was already fixed and committed separately
as f368ac2):

- Finding 2: --dry-run's printed command is not a verbatim rerun. Added a
  stderr warning that pasting it would auto-create .claude.json as a
  directory on a fresh workspace, and a README callout that name-only
  `-e KEY` args resolve against the pasting shell's environment, not
  .env.claude.
- Finding 3: documented that name-only forwarding changed which value a
  container sees when a variable is set both in the host shell and in
  .env.claude (now the shell's, matching documented precedence). Added
  test_container_receives_host_value_not_file_value, which fails against
  the pre-fix argv-based forwarding and passes now.
- Finding 4: corrected the README's collision-guard description — internals
  are not "all lowercase"; JAIL_DIR and SESSIONS_DIR are uppercase and do
  collide. Dropped the misleading uppercase-naming advice.
- Finding 5: asserted COLUMNS/LINES stay inline as KEY=<number> in
  test_wrapper_controlled_vars_stay_inline (values are tput-derived and
  host-dependent, so shape is asserted, not an exact number).
- Finding 6: normalized tests/test_env_passthrough.sh to 100644, matching
  the other test_*.sh files (all run via `bash file`, none need +x).
- Finding 7: setup() now also unsets MY_VAR, so it can't leak between tests
  in the same file now that two tests use it.
- Finding 8: amended the plan doc's Task 3 section to record the shipped
  _env_intended mechanism (commit b64357c), which replaced the
  order-dependent _is_exported-only check.

Claude-Session: https://claude.ai/code/session_01PB8GPWDXX7pFnTFC4QZCYv
claudiomaradonna deleted branch fix/secret-argv-and-dry-run 2026-07-27 14:39:46 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
claudiomaradonna/claude-jail!24
No description provided.