Blocked: 'env | grep' prints matching values in full. Use an existence-only check: [ -n "$VAR" ] && echo set
Cause
hooks/secret-guard.sh:
grep -Eqi '(^|[;&|]|\s)env(\s|$).*\|.*grep'
The pattern matches any env token followed anywhere later by a pipe into grep. But env -u VAR cmd and env VAR=x cmdrun a command with a modified environment — they print nothing at all. Piping their output into grep is greping the command's output, not the environment.
The dangerous forms are bare env, env | ..., and env with only assignments and no command. Those are already covered by the adjacent bare-env rule.
Suggested fix
Only treat env as a dump when no command word follows it:
# env with a command word after any options/assignments is exec, not a dump.ifprintf'%s'"$CMD"| grep -Eqi '(^|[;&|]|\s)env([[:space:]]+-[iu][[:space:]]*[A-Za-z_]*|[[:space:]]+[A-Za-z_][A-Za-z0-9_]*=[^[:space:]]*)*[[:space:]]*(\||;|&|$)';then
deny "..."fi
i.e. deny only when what follows env is exhausted by options and VAR=value assignments. Add cases to tests/secret_guard_test.sh:
expect allow 'env -u ANTHROPIC_API_KEY claude -p ok | grep plugin' # runs a command
expect allow 'env FOO=bar ./script.sh | grep x' # runs a command
expect deny 'env | grep -i token' # real dump
expect deny 'env' # real dump
Why not just fix it now
Deliberately deferred out of the migration commit. Changing a deny to an allow in a leak guard is a security-relevant behaviour change and deserves its own commit and its own review, not a drive-by inside a refactor. The guard was migrated verbatim so the 14 pinned cases prove behaviour is unchanged from what koala ran before.
Impact meanwhile: false positives only, which cost a workaround ((unset VAR; cmd)), never a leak.
## Symptom
```sh
env -u ANTHROPIC_API_KEY claude --debug -p "ok" 2>&1 | grep -i plugin
```
is blocked with:
> Blocked: 'env | grep' prints matching values in full. Use an existence-only check: `[ -n "$VAR" ] && echo set`
## Cause
`hooks/secret-guard.sh`:
```sh
grep -Eqi '(^|[;&|]|\s)env(\s|$).*\|.*grep'
```
The pattern matches any `env` token followed anywhere later by a pipe into `grep`. But `env -u VAR cmd` and `env VAR=x cmd` **run a command with a modified environment** — they print nothing at all. Piping their *output* into grep is greping the command's output, not the environment.
The dangerous forms are bare `env`, `env | ...`, and `env` with only assignments and no command. Those are already covered by the adjacent bare-`env` rule.
## Suggested fix
Only treat `env` as a dump when no command word follows it:
```sh
# env with a command word after any options/assignments is exec, not a dump.
if printf '%s' "$CMD" | grep -Eqi '(^|[;&|]|\s)env([[:space:]]+-[iu][[:space:]]*[A-Za-z_]*|[[:space:]]+[A-Za-z_][A-Za-z0-9_]*=[^[:space:]]*)*[[:space:]]*(\||;|&|$)'; then
deny "..."
fi
```
i.e. deny only when what follows `env` is exhausted by options and `VAR=value` assignments. Add cases to `tests/secret_guard_test.sh`:
```
expect allow 'env -u ANTHROPIC_API_KEY claude -p ok | grep plugin' # runs a command
expect allow 'env FOO=bar ./script.sh | grep x' # runs a command
expect deny 'env | grep -i token' # real dump
expect deny 'env' # real dump
```
## Why not just fix it now
Deliberately deferred out of the migration commit. Changing a deny to an allow in a leak guard is a security-relevant behaviour change and deserves its own commit and its own review, not a drive-by inside a refactor. The guard was migrated verbatim so the 14 pinned cases prove behaviour is unchanged from what koala ran before.
Impact meanwhile: false positives only, which cost a workaround (`(unset VAR; cmd)`), never a leak.
Fixed in a42ec18, released as v0.8.1 — CI run #26conclusion: success, which is now a real signal: this is the first commit to land through the gate added in #22, so task test actually ran.
What changed
Took the suggested direction — deny only when everything following env is exhausted by options and VAR=value assignments — with two adjustments to the proposed pattern:
-[iu] with no operand (env -i | grep x) needed its own alternative, otherwise the option branch demanded a variable name and the whole match failed → allow, on a genuine dump.
The two old rules (env-into-a-filter, and bare env) collapse into this one. That is a net tightening, not just a relaxation: env FOO=bar with no command word prints the whole environment and both old rules let it through. It is denied now.
Test evidence
Before the fix, 3 red:
FAIL env with only assignments and no command is a dump want=deny got=allow cmd=env FOO=bar
FAIL env -u VAR runs a command want=allow got=deny cmd=env -u ANTHROPIC_API_KEY claude --debug -p "ok" 2>&1 | grep -i plugin
FAIL env VAR=x runs a command want=allow got=deny cmd=env FOO=bar ./script.sh | grep x
secret_guard: 17 passed, 3 failed
After: secret_guard: 20 passed, 0 failed. The pinned deny cases are unchanged, including env -u FOO ./run.sh; env | grep -i secret — a real dump later on the same line still denies, so the fix does not open a bypass by splitting on ;.
One of the new allow cases is env -u OP_SERVICE_ACCOUNT_TOKEN op item create - < item.json, the documented 1Password write path that this false positive was forcing a (unset VAR; cmd) workaround around.
Aside worth recording
Writing the test hit the guard itself: the patch script's heredoc contained the literal dump shape as test data, and the installed guard matched the raw command string, heredoc body included. Same class as the app.ini comment note already in the file — a pattern that fires on any mention of itself makes its own fix awkward to write. Worked around by assembling the literal from fragments in the patch script.
Fixed in `a42ec18`, released as **v0.8.1** — CI run [#26](https://git.d-ma.be/mathias/skills/actions/runs/4833) `conclusion: success`, which is now a real signal: this is the first commit to land through the gate added in #22, so `task test` actually ran.
## What changed
Took the suggested direction — deny only when everything following `env` is exhausted by options and `VAR=value` assignments — with two adjustments to the proposed pattern:
- `-[iu]` with no operand (`env -i | grep x`) needed its own alternative, otherwise the option branch demanded a variable name and the whole match failed → allow, on a genuine dump.
- Variable names allow digits (`[A-Za-z_][A-Za-z0-9_]*`), so `env -u FOO2 cmd` isn't mis-parsed.
The two old rules (`env`-into-a-filter, and bare `env`) collapse into this one. That is a net *tightening*, not just a relaxation: **`env FOO=bar` with no command word prints the whole environment and both old rules let it through.** It is denied now.
## Test evidence
Before the fix, 3 red:
```
FAIL env with only assignments and no command is a dump want=deny got=allow cmd=env FOO=bar
FAIL env -u VAR runs a command want=allow got=deny cmd=env -u ANTHROPIC_API_KEY claude --debug -p "ok" 2>&1 | grep -i plugin
FAIL env VAR=x runs a command want=allow got=deny cmd=env FOO=bar ./script.sh | grep x
secret_guard: 17 passed, 3 failed
```
After: `secret_guard: 20 passed, 0 failed`. The pinned deny cases are unchanged, including `env -u FOO ./run.sh; env | grep -i secret` — a real dump later on the same line still denies, so the fix does not open a bypass by splitting on `;`.
One of the new allow cases is `env -u OP_SERVICE_ACCOUNT_TOKEN op item create - < item.json`, the documented 1Password write path that this false positive was forcing a `(unset VAR; cmd)` workaround around.
## Aside worth recording
Writing the test hit the guard itself: the patch script's heredoc contained the literal dump shape as test data, and the *installed* guard matched the raw command string, heredoc body included. Same class as the `app.ini` comment note already in the file — a pattern that fires on any mention of itself makes its own fix awkward to write. Worked around by assembling the literal from fragments in the patch script.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Symptom
is blocked with:
Cause
hooks/secret-guard.sh:The pattern matches any
envtoken followed anywhere later by a pipe intogrep. Butenv -u VAR cmdandenv VAR=x cmdrun a command with a modified environment — they print nothing at all. Piping their output into grep is greping the command's output, not the environment.The dangerous forms are bare
env,env | ..., andenvwith only assignments and no command. Those are already covered by the adjacent bare-envrule.Suggested fix
Only treat
envas a dump when no command word follows it:i.e. deny only when what follows
envis exhausted by options andVAR=valueassignments. Add cases totests/secret_guard_test.sh:Why not just fix it now
Deliberately deferred out of the migration commit. Changing a deny to an allow in a leak guard is a security-relevant behaviour change and deserves its own commit and its own review, not a drive-by inside a refactor. The guard was migrated verbatim so the 14 pinned cases prove behaviour is unchanged from what koala ran before.
Impact meanwhile: false positives only, which cost a workaround (
(unset VAR; cmd)), never a leak.Fixed in
a42ec18, released as v0.8.1 — CI run #26conclusion: success, which is now a real signal: this is the first commit to land through the gate added in #22, sotask testactually ran.What changed
Took the suggested direction — deny only when everything following
envis exhausted by options andVAR=valueassignments — with two adjustments to the proposed pattern:-[iu]with no operand (env -i | grep x) needed its own alternative, otherwise the option branch demanded a variable name and the whole match failed → allow, on a genuine dump.[A-Za-z_][A-Za-z0-9_]*), soenv -u FOO2 cmdisn't mis-parsed.The two old rules (
env-into-a-filter, and bareenv) collapse into this one. That is a net tightening, not just a relaxation:env FOO=barwith no command word prints the whole environment and both old rules let it through. It is denied now.Test evidence
Before the fix, 3 red:
After:
secret_guard: 20 passed, 0 failed. The pinned deny cases are unchanged, includingenv -u FOO ./run.sh; env | grep -i secret— a real dump later on the same line still denies, so the fix does not open a bypass by splitting on;.One of the new allow cases is
env -u OP_SERVICE_ACCOUNT_TOKEN op item create - < item.json, the documented 1Password write path that this false positive was forcing a(unset VAR; cmd)workaround around.Aside worth recording
Writing the test hit the guard itself: the patch script's heredoc contained the literal dump shape as test data, and the installed guard matched the raw command string, heredoc body included. Same class as the
app.inicomment note already in the file — a pattern that fires on any mention of itself makes its own fix awkward to write. Worked around by assembling the literal from fragments in the patch script.