Skip to content

Reject bare wildcard env-forward patterns at load time#130

Merged
JAORMX merged 1 commit intomainfrom
sec/reject-wildcard-env-forward
Apr 17, 2026
Merged

Reject bare wildcard env-forward patterns at load time#130
JAORMX merged 1 commit intomainfrom
sec/reject-wildcard-env-forward

Conversation

@JAORMX
Copy link
Copy Markdown
Contributor

@JAORMX JAORMX commented Apr 17, 2026

Summary

Closes a HIGH security finding. A bare * in env_forward (global config, workspace .broodbox.yaml, or --env-forward CLI flag) matched every host environment variable because matchPattern trimmed the trailing * and then HasPrefix(key, "") was always true. Combined with mergeAgentOverride replacing the global value with the workspace-local value verbatim, a malicious .broodbox.yaml could ship agents.claude-code.env_forward: ["*"] and scoop up host secrets (SSH_AUTH_SOCK, GITHUB_TOKEN, AWS_*, VAULT_TOKEN, OKTA_*) into the guest agent's environment — from where any permitted egress host becomes an exfiltration channel.

The narrow fix preserves the feature (per-repo allowlists like ["GOFLAGS", "CARGO_*"] are legitimate and useful) while rejecting three classes of footgun:

  • Bare * and whitespace-trimmed variants
  • Empty / whitespace-only patterns
  • Leading-star (*_KEY) — silently useless today; surface a clear error so users learn the correct syntax

Validation coverage

Validation runs at four load points so no user-supplied source bypasses it:

  • NewLoader.Load — global ~/.config/broodbox/config.yaml
  • LoadFromPath — workspace-local .broodbox.yaml
  • run()--env-forward CLI flag
  • SandboxRunner.Prepare — SDK boundary (RunOpts.EnvForwardExtra)

Defense-in-depth: matchPattern also returns false for empty / bare-* patterns so a bypass of load-time validation cannot silently forward every env var.

UX change

Global config load errors are now fatal instead of warn-and-fall-back-to-defaults. Silently discarding every override in a user's own config because one line fails validation was worse UX than a clear error naming the file and the exact problem. Workspace-local config keeps its warn-and-skip behavior so a malicious repo cannot DOS a user's session.

Error messages

Every rejection pinpoints the location and suggests the fix:

  • loading global config /path/to/config.yaml: validating config file ...: agents.claude-code.env_forward[0]: bare "*" matches every host env var — specify an exact name or a prefix like "AWS_*"
  • invalid --env-forward: env_forward[0]: leading-star patterns are not supported ("*_API_KEY") — only trailing-star globs like "AWS_*"

Test plan

  • task fmt && task lint && task test — all green
  • Unit tests cover the validator directly (exact names, trailing globs, bare * variants, empty, whitespace, leading-star, mixed-with-valid patterns)
  • Integration tests exercise global Loader.Load, workspace LoadFromPath, and empty-string in config
  • Defensive matchPattern test: bad patterns ("*", "", " ", " * ") must not match any realistic env var name including SSH_AUTH_SOCK, GITHUB_TOKEN, AWS_ACCESS_KEY_ID
  • Headless end-to-end:
    • bbox claude-code --env-forward '*' → exit non-zero with clear error
    • bbox claude-code --env-forward '*_API_KEY' → exit non-zero with clear error
    • bbox claude-code --config /path/with-star.yaml → exit non-zero with file path in error
    • bbox claude-code --no-mcp --env-forward HOME --exec /bin/sh -- -c 'env | grep ^HOME=' → VM boots, HOME forwarded, AWS_*/GITHUB_TOKEN/ANTHROPIC_API_KEY NOT present

🤖 Generated with Claude Code

A bare `*` in env_forward matches every host environment variable
because matchPattern trimmed the trailing `*` and HasPrefix against
the empty string always returns true. Combined with mergeAgentOverride
replacing the global env_forward with the workspace-local value
verbatim, a malicious `.broodbox.yaml` could ship
`agents.claude-code.env_forward: ["*"]` and scoop up host secrets
(SSH_AUTH_SOCK, GITHUB_TOKEN, AWS_*, VAULT_TOKEN, OKTA_*) into the
guest agent's environment — from where any permitted egress host
becomes an exfiltration channel.

The narrow fix preserves the feature (per-repo env_forward allowlists
like `["GOFLAGS", "CARGO_*"]` are legitimate and useful) while
rejecting the footgun:

- Reject bare "*" and whitespace-trimmed variants
- Reject empty / whitespace-only patterns
- Reject leading-star patterns ("*_KEY") which are silently useless
  under matchPattern's semantics — surface as a clear error rather
  than let users believe their config works

Validation runs at four load points:

- NewLoader.Load — global ~/.config/broodbox/config.yaml
- LoadFromPath — workspace-local .broodbox.yaml
- run() — `--env-forward` CLI flag
- SandboxRunner.Prepare — SDK boundary (RunOpts.EnvForwardExtra)

Defense-in-depth: matchPattern also returns false for empty / bare-"*"
patterns so a bypass of load-time validation cannot silently forward
every env var.

UX change: global config load errors are now fatal instead of warn-
and-fall-back-to-defaults. Silently discarding every override in a
user's own config because one line fails validation was worse than
a clear error naming the file and the exact problem. Workspace-local
config keeps its warn-and-skip behavior so a malicious repo cannot
DOS a user's session.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JAORMX JAORMX merged commit b5d0280 into main Apr 17, 2026
8 checks passed
@JAORMX JAORMX deleted the sec/reject-wildcard-env-forward branch April 17, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant