chrisleekr / chrisleekr/github-app

security(agent-sdk): PreToolUse hook scopes only Bash; Read can dump parent process env via /proc/<pid>/environ

Open
#250 0 comments 0 reactions 0 assignees View on GitHub
research
Dominant language
TypeScript
Stars
1
Forks
0
Avg merge
6h 39m
Merged PRs (30d)
27

Description

## Finding

The runtime destructive-action gate added in #222 wires a single PreToolUse hook scoped to `matcher: "Bash"` (`src/core/executor.ts:275`), and the hook itself early-returns when `tool_name !== "Bash"` (`src/core/hooks/forbidden-bash.ts:32`). The agent's filesystem tools (`Edit`, `MultiEdit`, `Glob`, `Grep`, `LS`, `Read`, `Write`) are unconditionally enabled in `resolveAllowedTools` (`src/core/prompt-builder.ts:801`-`src/core/prompt-builder.ts:820`) and are NOT covered by any hook. Together with `permissionMode: "bypassPermissions"` + `allowDangerouslySkipPermissions: true` (`src/core/executor.ts:260`-`src/core/executor.ts:261`), this means a prompt-injected agent can hand the `Read` tool any absolute path the subprocess UID can open, and the SDK applies no path scoping (`cwd: workDir` only sets the resolution base for relative paths, it does not chroot).

The concrete consequence is that the env-allowlist defense-in-depth added in #102 is bypassable. `buildProviderEnv` (`src/core/executor.ts:91`-`src/core/executor.ts:104`) explicitly strips `DAEMON_AUTH_TOKEN`, `GITHUB_PERSONAL_ACCESS_TOKEN`, `DATABASE_URL`, `VALKEY_URL`, `CONTEXT7_API_KEY`, the `GITHUB_APP_*` family, and the `GITHUB_WEBHOOK_*` family from the Claude CLI subprocess env so a successful injection cannot exfiltrate them from `/proc/self/environ`. However the parent daemon process has those same names populated at the Pod level via `envFrom: secretRef` against `daemon-secrets` (`src/k8s/ephemeral-daemon-spawner.ts:189`) and the Claude CLI subprocess runs as the same UID 1000 the daemon does (Dockerfile.daemon final `USER bun`; ephemeral Pod `runAsUser: 1000` at `src/k8s/ephemeral-daemon-spawner.ts:158`). On a stock Linux `/proc` mount (no `hidepid=2`), a same-UID process can read `/proc//environ`, so the agent can dump the daemon's full env, including the very keys the allowlist worked to keep out.

In addition to env exfiltration, the same gap lets `Read`/`Glob`/`LS` enumerate anything the UID can see outside `cwd` (the bot's own compiled source under `/app/dist`, mounted CA bundles, `/proc/*/cmdline`, `/run/*`). The hook is mechanically the right place to close this: the SDK's hook matcher accepts a tool-name regex (`matcher: "Read|Write|Edit|MultiEdit|Glob|Grep|LS|NotebookEdit"`) and the callback can inspect `tool_input.file_path` to deny anything whose `realpath` resolution falls outside `{workDir, artifactsDir}`. This is distinct from #240, which proposes a Write-side `.git/` carve-out to back the destructive-action invariant; the gap here is the unrestricted **Read** scope that bypasses the env allowlist's defense-in-depth.

## Diagram

```mermaid
flowchart TB
Pod[Daemon Pod
envFrom: daemon-secrets]:::ctx
Daemon[bun PID 1
process.env holds
DAEMON_AUTH_TOKEN
ANTHROPIC_API_KEY
GITHUB_PERSONAL_ACCESS_TOKEN
AWS_*]:::trusted
Filter[buildProviderEnv
ENV_DENY_KEYS strips
secrets from CLI env]:::gate
CLI[Claude CLI subprocess
filtered env only
same UID 1000 as daemon]:::sub
PR[Attacker PR/issue
prompt injection]:::attacker
Hook[PreToolUse hook
matcher: 'Bash'
denies force-push etc]:::gate
Read[Read tool
file_path = /proc/1/environ
matcher does NOT match]:::bypass
Leak[Daemon env bytes returned
secrets reach agent context]:::leak
Exfil[Exfil channel:
Bash curl wget egress OR
obfuscated tracking comment]:::leak

Pod --> Daemon
Daemon -->|spawn child| Filter
Filter --> CLI
PR -->|untrusted content| CLI
CLI -->|attempts tool call| Hook
Hook -.->|matcher Bash only| Read
Read --> Leak
Leak --> Exfil

classDef ctx fill:#1a5276;color:#ffffff
classDef trusted fill:#196f3d;color:#ffffff
classDef gate fill:#7d6608;color:#ffffff
classDef sub fill:#5b2c6f;color:#ffffff
classDef attacker fill:#922b21;color:#ffffff
classDef bypass fill:#a04000;color:#ffffff
classDef leak fill:#922b21;color:#ffffff
```

## Rationale

The env allowlist in `buildProviderEnv` was added (#102) as a defense-in-depth so a successful prompt injection cannot trivially exfiltrate Pod-level secrets even if the model has been jailbroken. That control assumes the agent subprocess's own env is the only source of secret bytes within reach. On a stock K8s node `/proc` is mounted without `hidepid`, so any same-UID process can read the parent's env via `/proc//environ`, which means the agent's `Read` tool re-opens the exfiltration path the allowlist closed. The blast radius is the full `daemon-secrets` set documented at `src/k8s/ephemeral-daemon-spawner.ts:167`-`src/k8s/ephemeral-daemon-spawner.ts:180`: `DAEMON_AUTH_TOKEN[_PREVIOUS]` (lets an attacker impersonate a daemon to the orchestrator over WebSocket), `ANTHROPIC_API_KEY` / `CLAUDE_CODE_OAUTH_TOKEN` (cost amplification + quota theft), the AWS Bedrock chain, and `GITHUB_PERSONAL_ACCESS_TOKEN` when PAT mode is enabled (full repo access on the single-tenant operator). Several of these (DAEMON_AUTH_TOKEN, AWS session tokens) are not covered by the regex secret-strip in `redactSecrets` (`src/utils/sanitize.ts:130`-`src/utils/sanitize.ts:175`), so an obfuscated dump into the tracking comment can survive the output guard; for the rest, the agent has `Bash(curl:*)` / `Bash(wget:*)` available whenever the daemon tool-discovery surfaces them (`src/daemon/tool-discovery.ts:52`-`src/daemon/tool-discovery.ts:53` and `src/core/prompt-builder.ts:835`-`src/core/prompt-builder.ts:840`), giving a direct network egress channel.

The mechanically right place to close this is the SDK's PreToolUse hook framework, the same primitive #222 already uses. The hook can match `Read|Write|Edit|MultiEdit|Glob|Grep|LS|NotebookEdit` and deny any `file_path` whose `path.resolve` + `fs.realpathSync` resolution falls outside `{workDir, artifactsDir}`. That closes both the `/proc//environ` exfil and the broader read-outside-cwd surface in one place, with the same observability shape (`event: "agent.hook.denied"`, `tool`, `rule` fields, no raw path logged) the existing `forbidden-bash` hook already emits. Path resolution must use `realpathSync` on both inputs (symlink-aware) since a `startsWith(workDir)` on raw strings is bypassable with `..` or symlinks per the SDK's own permission docs.

## References

**Internal:**
- `src/core/executor.ts:275` — PreToolUse hook wired only for `matcher: "Bash"`.
- `src/core/hooks/forbidden-bash.ts:32` — hook early-returns when `tool_name !== "Bash"`, confirming no path coverage.
- `src/core/executor.ts:91`-`src/core/executor.ts:104` — `ENV_DENY_KEYS` / `ENV_DENY_PREFIXES` env allowlist whose defense-in-depth this finding bypasses.
- `src/core/executor.ts:258`-`src/core/executor.ts:284` — full `queryOptions` showing `bypassPermissions`, `allowDangerouslySkipPermissions`, `cwd: workDir`, and `settingSources: []`.
- `src/core/prompt-builder.ts:801`-`src/core/prompt-builder.ts:820` — `Read` / `Write` / `Edit` / `MultiEdit` / `Glob` / `Grep` / `LS` unconditionally in `resolveAllowedTools`.
- `src/k8s/ephemeral-daemon-spawner.ts:167`-`src/k8s/ephemeral-daemon-spawner.ts:189` — `envFrom: daemon-secrets` populating the daemon-process env this finding reads back via `/proc`.
- `src/utils/sanitize.ts:130`-`src/utils/sanitize.ts:175` — secret-strip regex set; `DAEMON_AUTH_TOKEN` and AWS session tokens are out of scope.
- `src/daemon/tool-discovery.ts:52`-`src/daemon/tool-discovery.ts:53` — `curl` / `wget` in `CLI_TOOL_NAMES`, surfaced as `Bash(curl:*)` / `Bash(wget:*)` for the agent when functional on the image.
- `CLAUDE.md` security invariants 1 (subprocess env allowlist) and 5 (runtime destructive-Bash gate) — the two invariants whose seam this finding sits in.
- Issues #102 (env allowlist), #222 (runtime destructive-Bash hook), #240 (companion finding on the Write side).

**External:**
- [Claude Agent SDK — Configure permissions](https://docs.claude.com/en/docs/agent-sdk/permissions) — PreToolUse deny rules beat everything; `allowed_tools` does not restrict `bypassPermissions` mode.
- [Claude Code Hooks (2026): Block Claude Reading .env + 30 Hook Events](https://www.morphllm.com/claude-code-hooks) — pattern for blocking file-path-based reads via a PreToolUse hook (exit code 2 / `permissionDecision: "deny"`).
- [Claude Agent SDK — Intercept and control agent behavior with hooks](https://docs.claude.com/en/docs/agent-sdk/hooks) — matcher accepts a tool-name regex; callback inspects `tool_input.file_path`; path-jail must use `realpath` on both sides.
- [Sensitive Data Exposed Through Container Environment Variables](https://www.sourcery.ai/vulnerabilities/container-secrets-environment-variables) — secrets in `env`/`envFrom: secretRef` are readable through `/proc//environ` by any process in the container.
- [Stop passing secrets via environment variables](https://www.linkedin.com/pulse/stop-passing-secrets-via-environment-variables-your-huerta) — `/proc/PID/environ` exposure of parent-process secrets to child processes.
- [Container Isolation Misconceptions: PID Namespaces](https://tasnimzotder.com/posts/container-isolation-misconceptions) — same-PID-namespace processes share `/proc` visibility absent `hidepid`.

## Suggested Next Steps

1. Add a new hook module `src/core/hooks/scoped-paths.ts` exporting `createScopedPathsHook({ workDir, artifactsDir, log })` that:
- Returns `permissionDecision: "deny"` when `tool_input.file_path` (or `tool_input.path` for `LS`/`Glob`/`Grep`) resolves via `path.resolve(workDir, p)` + `fs.realpathSync.native` to anything outside `realpath(workDir)` or `realpath(artifactsDir)`, with the existing fail-safe of denying when realpath throws (the not-yet-existing target case for `Write` is handled by realpath'ing the deepest existing ancestor of the parent dir).
- Emits `event: "agent.hook.denied"`, `tool`, `rule: "path-outside-scope"` and never logs the raw path, mirroring the `forbidden-bash` pattern.
2. Wire the hook in `src/core/executor.ts:275` as a second `PreToolUse` entry: `{ matcher: "Read|Write|Edit|MultiEdit|Glob|Grep|LS|NotebookEdit", hooks: [createScopedPathsHook({ workDir, artifactsDir, log })] }`, alongside the existing Bash matcher.
3. Add unit coverage under `src/core/hooks/scoped-paths.test.ts` for: `/proc/1/environ` denied; `/etc/passwd` denied; symlink that escapes `workDir` denied (create the symlink in a tmp fixture); `cwd`-relative `Read("./src/foo.ts")` allowed; `Read(artifactsDir + "/IMPLEMENT.md")` allowed; `Write` to a not-yet-existing path inside `workDir` allowed (parent-dir realpath fallback).
4. Defense-in-depth at the Pod level (separate change, optional follow-up): evaluate a `procMount: "Unmasked"`-aware `securityContext` change or a sidecar/initContainer that bind-mounts `/proc` with `hidepid=invisible` on the ephemeral daemon container so even a hook-bypass attack cannot see the parent's env.
5. Document the new hook under CLAUDE.md security invariant #5 alongside the existing destructive-Bash gate, and extend the docs/operate/configuration.md "Subprocess env allowlist" section to note that the path-scope hook is the load-bearing backstop for the allowlist's defense-in-depth posture.

## Areas Evaluated

- `src/core/executor.ts` — full `queryOptions` shape, hooks wiring, env allowlist/denylist, `bypassPermissions` + `allowDangerouslySkipPermissions` posture.
- `src/core/hooks/forbidden-bash.ts` and `src/utils/forbidden-bash.ts` — existing PreToolUse hook scope, early-return behavior on non-Bash tool names.
- `src/core/prompt-builder.ts` `resolveAllowedTools` — the filesystem and Bash tool surface exposed to the agent, including daemon-capabilities expansion (`curl`, `wget`).
- `src/k8s/ephemeral-daemon-spawner.ts` — daemon Pod env source (`envFrom: secretRef daemon-secrets`), securityContext, `automountServiceAccountToken: false`.
- `src/utils/sanitize.ts` — output-side secret-strip regex set, gaps (`DAEMON_AUTH_TOKEN`, AWS session tokens).
- `src/daemon/tool-discovery.ts` — CLI tool detection feeding the dynamic `Bash(:*)` allowlist.
- Closed/open research issues on the agent-sdk area (#222, #240, #102, #198, #191, #196) to ensure non-duplication.

*Generated by the scheduled research action on 2026-06-24*

Contributor guide

Open the contributing guide

Research direction

Start with the existing hook wiring in src/core/executor.ts and the behavior in src/core/hooks/forbidden-bash.ts, then inspect the allowed filesystem tools in src/core/prompt-builder.ts. Use the proposed cases in src/core/hooks/scoped-paths.test.ts to verify paths outside workDir and artifactsDir are denied, while in-scope and new files are handled correctly; document the security invariant after the tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
kubernetes, typescript
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
57/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.