From 4e1c8e2de1e084b58da4e9935d16de9c1668da4e Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Wed, 30 Sep 2026 16:17:21 +0200 Subject: [PATCH] fix(agents): deny rm -rf and the host credential files in the sandbox An external report flagged the Bash(*) allow in the committed .claude/settings.json. Bash(*) stays: every command still runs inside the OS sandbox, and deny and ask take precedence over it. Reviewing the file against its own documentation turned up two real gaps instead. security.md and settings.md cite an rm -rf* deny rule as the guard against `rm -rf .` wiping the worktree and .git/, but the rule was lost in ee30f6d446 when the deny list was rewritten. It is back. denyRead covered ~/.aws, ~/.config/gcloud, ~/.gnupg and ~/.kube only. ~/.docker/config.json holds registry logins inline, ~/.git-credentials and ~/.netrc hold plaintext credentials, and the default ~/.ssh/id_* keys were readable because Assumption 10 kept ~/.ssh open for an ask-gated git push that has since moved to deny. All nine paths are denied now. The rest of ~/.ssh stays readable, so SSH fetches keep known_hosts and authenticate through the agent socket. Sandboxed docker calls pull anonymously and print a config warning on stderr. Thanks to Hanno Horrmeyer for the report. Reported-by: Hanno Horrmeyer Co-Authored-By: Claude Opus 5.5 (1M context) --- .claude/settings.json | 12 +++++++++++- .../tools/agents/claude/sandbox.md | 8 ++++++-- .../tools/agents/claude/security.md | 18 ++++++------------ .../tools/agents/claude/settings.md | 2 +- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 4648a56520..695f8bcda9 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -503,6 +503,7 @@ "Bash(git send-pack*)", "Bash(git-setup-remotes*)", "Bash(git-sign-push*)", + "Bash(rm -rf*)", "Bash(sudo *)", "Bash(until *)", "Bash(while *)" @@ -650,8 +651,17 @@ "denyRead": [ "~/.aws", "~/.config/gcloud", + "~/.docker/config.json", + "~/.git-credentials", "~/.gnupg", - "~/.kube" + "~/.kube", + "~/.netrc", + "~/.ssh/id_dsa", + "~/.ssh/id_ecdsa", + "~/.ssh/id_ecdsa_sk", + "~/.ssh/id_ed25519", + "~/.ssh/id_ed25519_sk", + "~/.ssh/id_rsa" ] } } diff --git a/docs/contributing/tools/agents/claude/sandbox.md b/docs/contributing/tools/agents/claude/sandbox.md index 98cd1d6a31..bb8f00e0f6 100644 --- a/docs/contributing/tools/agents/claude/sandbox.md +++ b/docs/contributing/tools/agents/claude/sandbox.md @@ -62,7 +62,7 @@ Adding entries here widens the agent's effective write scope. New paths SHOULD b ## Read Restrictions ๐Ÿšซ -The following directories are never readable, even if a task explicitly requests access: +The following paths are never readable, even if a task explicitly requests access: | Path | What it protects | |---|---| @@ -70,8 +70,12 @@ The following directories are never readable, even if a task explicitly requests | `~/.kube` | Kubernetes cluster credentials. | | `~/.aws` | AWS access keys and configuration. | | `~/.config/gcloud` | Google Cloud service account credentials. | +| `~/.docker/config.json` | Registry credentials that `docker login` stores inline. Docker CLI calls inside the sandbox pull anonymously and print `WARNING: Error loading config file` on stderr. | +| `~/.git-credentials` | HTTPS credentials written by git's `store` credential helper. | +| `~/.netrc` | Machine credentials read by `curl`, `git`, and `pip`. | +| `~/.ssh/id_dsa`, `~/.ssh/id_ecdsa`, `~/.ssh/id_ecdsa_sk`, `~/.ssh/id_ed25519`, `~/.ssh/id_ed25519_sk`, `~/.ssh/id_rsa` | The default SSH private keys. | -`~/.ssh` is intentionally **not** in this list. Denying it would make `git push` over SSH unreachable and defeat the `Bash(git push*)` ask-gate. The trade-off and its mitigations are documented in [security.md](security.md#assumption-10) (Assumption 10). +The rest of `~/.ssh` stays readable, because `ssh` needs `known_hosts` and `config` for `git fetch`; authentication goes through the SSH agent. See [security.md](security.md#assumption-10) (Assumption 10). ## Network ๐ŸŒ diff --git a/docs/contributing/tools/agents/claude/security.md b/docs/contributing/tools/agents/claude/security.md index f099cdb17c..9cb946dd76 100644 --- a/docs/contributing/tools/agents/claude/security.md +++ b/docs/contributing/tools/agents/claude/security.md @@ -42,7 +42,7 @@ The `Bash(*)` allow grants broad access to the `docker` CLI (alongside every oth Every rule in `permissions.ask` assumes the operator reads the prompt and confirms the action matches intent. Rules in `ask` are NOT a hard barrier. They are a speed-bump that converts automatic execution into a deliberate decision. -**Protected by this:** `git commit*`, `git push*`, `docker run*`, `docker build*`, `docker push*`, `docker login*`, every mutating `gh` verb, and every non-GET `gh api` call. Each would be destructive or externally visible if executed without review. +**Protected by this:** `git commit*`, `docker run*`, `docker build*`, `docker push*`, `docker login*`, every mutating `gh` verb, and every non-GET `gh api` call. Each would be destructive or externally visible if executed without review. **If violated:** Blind-approving `ask` prompts defeats the policy layer entirely. Operators MUST treat every `ask` prompt as a code-review checkpoint. @@ -80,23 +80,17 @@ The `Bash(*)` allow lets the agent run any Makefile target without prompting. Th ## Assumption 9: `denyRead` lists every credential directory ๐Ÿ” -The sandbox's `denyRead` list (`~/.gnupg`, `~/.kube`, `~/.aws`, `~/.config/gcloud`) is the primary mechanism that prevents the agent from reading host credentials. Every credential store an operator uses on the host is assumed to be covered, **with the documented exception of `~/.ssh`** (see Assumption 10). +The sandbox's `denyRead` list (`~/.aws`, `~/.config/gcloud`, `~/.docker/config.json`, `~/.git-credentials`, `~/.gnupg`, `~/.kube`, `~/.netrc`, and the default private keys under `~/.ssh`) is the primary mechanism that prevents the agent from reading host credentials. Every credential store an operator uses on the host is assumed to be covered, **with the documented exceptions of the rest of `~/.ssh`** (see Assumption 10) **and `~/.config/gh`**, which `gh` needs on every call. **If violated:** A credential directory not listed in `denyRead` (for example `~/.azure`, `~/.doctl`, `~/.config/op`) is readable by the agent, and its contents can leak into the transcript. Contributors adding a new credential store MUST extend `denyRead` in the same commit. -## Assumption 10: `~/.ssh` is intentionally readable to allow `git push` on `ask` ๐Ÿ”‘ +## Assumption 10: `~/.ssh` stays readable except for the default private keys ๐Ÿ”‘ -`git push`, `git fetch`, and `git clone` over SSH all require `ssh(1)` to read `~/.ssh/` (private keys, `known_hosts`, `config`). With `~/.ssh` in `denyRead`, `ssh` fails with `Host key verification failed` / `Could not read from remote repository`, which makes the `ask`-gated `Bash(git push*)` rule unreachable in practice. +`git fetch` and `git ls-remote` over SSH need `ssh(1)` to read `~/.ssh/known_hosts` and `~/.ssh/config`. With the whole directory in `denyRead`, `ssh` fails with `Host key verification failed`. `denyRead` therefore lists only the default private key files, and `ssh` authenticates through the agent socket named in `SSH_AUTH_SOCK`, which `allowAllUnixSockets: true` keeps reachable. `Bash(git push*)` is in `deny`, so pushes stay with the operator. -The chosen trade-off is to keep `~/.ssh` **out** of `denyRead` so that SSH auth works and `Bash(git push*)` in `ask` becomes the effective control point: every push pauses for operator confirmation. `allowUnsandboxedCommands: false` remains in place, so there is no per-call sandbox bypass. +**Cost:** The agent can authenticate with every key loaded into the SSH agent, although it cannot read the key itself. A private key stored under a non-default file name stays readable. -**Cost:** The agent has read access to all files under `~/.ssh/`, including private keys. A compromised or prompt-injected agent could load those keys into the transcript via read tools (which are NOT gated by `ask`). Mitigations relied upon: - -- Private keys SHOULD be passphrase-protected, so exfiltration yields an encrypted blob rather than a usable key. -- Operators MUST treat any unexplained read of `~/.ssh/id_*` in the transcript as a trust incident and rotate the affected keys. -- Hosts that store credentials to high-blast-radius targets (production servers, infrastructure root) SHOULD NOT rely on this assumption; instead use an SSH-agent socket with `~/.ssh` left denied, or switch the git remote to HTTPS plus a narrowly-scoped token. - -**If violated (i.e. the cost turns out to be unacceptable on a given host):** Re-add `~/.ssh` to `denyRead` in `.claude/settings.local.json` and handle git pushes from an external terminal, or adopt the SSH-agent-socket variant. Both options preserve the deny without breaking the global policy. +**If violated:** Without a running SSH agent, SSH fetches inside the sandbox fail with `Permission denied (publickey)`; load the key into an agent or fetch over HTTPS. A contributor whose key uses a non-default file name MUST add that path to `denyRead` in `.claude/settings.local.json`. ## Updating Assumptions ๐Ÿงน diff --git a/docs/contributing/tools/agents/claude/settings.md b/docs/contributing/tools/agents/claude/settings.md index eef3e9c076..dd641e1f93 100644 --- a/docs/contributing/tools/agents/claude/settings.md +++ b/docs/contributing/tools/agents/claude/settings.md @@ -39,7 +39,7 @@ Because Bash is allowed wholesale, the operational safety of shell invocations r 1. **`deny`** (precedence 1): structurally destructive shapes are blocked outright, even when sandboxed. Covers shell loops (`for`/`while`/`until`), cross-repo `git -C *`, force-push/reset/clean/amend, `rm -rf*`, `sudo*`, privileged `gh` verbs (`secret`/`ssh-key`/`gpg-key`/`variable`/`extension`/`workflow enable`/`workflow disable`/`repo delete`/`pr merge`/`issue transfer`), and mutating `gh api` verbs (`DELETE`/`PUT`, secrets/keys/collaborators/merge paths). 2. **`ask`** (precedence 2): mutating `gh`/`docker`/`git` verbs and `Skill(update-config*)` pause for operator review. Every image mutation (`docker run`/`build`/`push`/`login`), `gh api` with a non-GET verb or a body-flag or `graphql`, every known `gh` mutating verb (21 of them via `gh * *`), `git commit*`, and anything touching `.claude/settings.json` routes through here. -3. **Sandbox** (precedence 3): everything else runs under `allowWrite` / `denyRead` / `allowedDomains`. Writes outside `allowWrite` fail with `EROFS`; reads from `~/.ssh`, `~/.gnupg`, `~/.aws`, etc. are denied; outbound connections to anything not in `allowedDomains` fail. +3. **Sandbox** (precedence 3): everything else runs under `allowWrite` / `denyRead` / `allowedDomains`. Writes outside `allowWrite` fail with `EROFS`; reads from the default `~/.ssh` private keys, `~/.gnupg`, `~/.aws`, etc. are denied; outbound connections to anything not in `allowedDomains` fail. The `sandbox.autoAllowBashIfSandboxed: true` flag is retained as belt-and-suspenders. If `Bash(*)` were ever removed, the sandbox auto-allow would still cover the common shapes, but it is no longer the primary gate.