diff --git a/hooks/secret-guard.sh b/hooks/secret-guard.sh index 5f5cd1c..81348f0 100755 --- a/hooks/secret-guard.sh +++ b/hooks/secret-guard.sh @@ -50,11 +50,18 @@ if printf '%s' "$CMD" | grep -Eqi '(^|[;&|]|\s)op\s+document\s+get\b'; then fi # Raw env/secret-file dumps (infra#228/#229 root cause). -if printf '%s' "$CMD" | grep -Eqi '(^|[;&|]|\s)env(\s|$).*\|.*grep'; then - deny "Blocked: 'env | grep' prints matching values in full. Use an existence-only check: [ -n \"\$VAR\" ] && echo set" -fi -if printf '%s' "$CMD" | grep -Eq '(^|[;&|])[[:space:]]*env[[:space:]]*($|[;&|])'; then - deny "Blocked: bare 'env' prints every variable's value, including secrets. Use an existence-only check: [ -n \"\$VAR\" ] && echo set" +# +# `env` is a dump only when nothing but options and VAR=value assignments +# follow it. `env -u VAR cmd` and `env FOO=bar cmd` EXEC a command with a +# modified environment and print nothing themselves — piping those into grep +# filters the command's own output, not the environment (#25). The earlier +# pattern matched any `env` token followed anywhere later by a pipe into +# grep, which blocked the documented 1Password write path +# (`env -u OP_SERVICE_ACCOUNT_TOKEN op item create`) and cost a workaround. +# Denying only the "no command word follows" shape also catches `env FOO=bar` +# with no command, which the two previous rules let through. +if printf '%s' "$CMD" | grep -Eqi '(^|[;&|]|[[:space:]])env([[:space:]]+-[iu][[:space:]]*[A-Za-z_][A-Za-z0-9_]*|[[:space:]]+-[iu]|[[:space:]]+[A-Za-z_][A-Za-z0-9_]*=[^[:space:]]*)*[[:space:]]*($|[|;&])'; then + deny "Blocked: this 'env' prints every variable's value, including secrets (no command word follows it, so it dumps rather than execs). Use an existence-only check: [ -n \"\$VAR\" ] && echo set. Note 'env -u VAR cmd' and 'env FOO=bar cmd' are allowed — they run a command." fi if printf '%s' "$CMD" | grep -Eqi '(^|[;&|]|\s)printenv\b'; then deny "Blocked: printenv dumps all env values. Use: [ -n \"\$VAR\" ] && echo set" diff --git a/tests/secret_guard_test.sh b/tests/secret_guard_test.sh index d9e27da..e6715ae 100644 --- a/tests/secret_guard_test.sh +++ b/tests/secret_guard_test.sh @@ -61,6 +61,17 @@ expect deny 'op read "op://vault/item/field"' 'op read' expect deny 'env | grep -i gitea' 'env piped to grep' expect deny 'env' 'bare env' expect deny 'printenv' 'printenv' +expect deny 'env -i | grep -i token' 'env -i with no command is still a dump' +expect deny 'env FOO=bar' 'env with only assignments and no command is a dump' + +# --- allow: `env VAR=x cmd` / `env -u VAR cmd` RUN a command (#25) --- +# These print nothing themselves; a pipe into grep filters the command's own +# output, not the environment. Blocking them cost a documented workaround +# ((unset VAR; cmd)) on the 1Password write path, which is the shape below. +expect allow 'env -u ANTHROPIC_API_KEY claude --debug -p "ok" 2>&1 | grep -i plugin' 'env -u VAR runs a command' +expect allow 'env FOO=bar ./script.sh | grep x' 'env VAR=x runs a command' +expect allow 'env -u OP_SERVICE_ACCOUNT_TOKEN op item create - < item.json' 'env -u on the 1P write path' +expect deny 'env -u FOO ./run.sh; env | grep -i secret' 'a real dump later in the same line still denies' # --- deny: kubernetes secret material --- expect deny 'kubectl get secret brain-app -o yaml' 'kubectl get secret -o yaml'