fix(secret-guard): treat env as a dump only when no command follows

`env -u VAR cmd` and `env FOO=bar cmd` exec a command with a modified
environment and print nothing themselves. The old rule matched any `env`
token followed anywhere later by a pipe into a filter, so it blocked the
documented 1Password write path and forced a `(unset VAR; cmd)` workaround.
False positives only — never a leak — but a guard people route around is a
guard on its way out.

The rule now denies only when everything after `env` is exhausted by options
and VAR=value assignments. That is strictly the dumping shape, and it also
closes a hole the two previous rules left open: `env FOO=bar` with no command
word prints the whole environment and was allowed.

Three new deny/allow pairs pin it, including the 1Password write shape that
motivated the report. 20 cases now, up from 14, all passing.

Closes #25
Bump-Type: patch

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "skills",
   "description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.",
-  "version": "0.7.0",
+  "version": "0.8.0",
   "author": {
     "name": "Mathias",
     "url": "https://git.d-ma.be/mathias"

.gitea/workflows/ci.yml

@@ -1,23 +1,52 @@
-name: release
+name: CI
 
-# Auto-tag every push to main with a semver bump derived from the latest
+# Two jobs, in order:
+#
+#   gate — `task check` (structure, frontmatter, manifest/tag parity) plus
+#          `task test` (hooks, doctor, release-skip). Runs on every push and
+#          every PR. Before this existed the suite ran nowhere: CI reported
+#          green having tested nothing (#22).
+#
+#   tag  — auto-tags a push to main with a semver bump derived from the latest
 #          commit footer:
 #            Bump-Type: major  → vX.0.0
 #            Bump-Type: minor  → v0.Y.0
 #            (default)         → v0.0.Z (patch)
-#
+#          Skips when the commit carries the `Skip-Release: true` trailer (the
-# Skips when the latest commit is itself the auto-tagger (commit message
+#          tagger's own manifest bump) to prevent recursion. Gated on `gate`,
-# contains "[skip-release]") to prevent recursion. Tag is pushed back
+#          so a red check cannot ship a tag.
-# to the repo using the GITEA_TOKEN provided by act_runner.
 
-on:
+"on":
   push:
-    branches:
+    branches: ["**"]
-      - main
+  pull_request:
+    branches: [main]
 
 jobs:
-  tag:
+  gate:
+    name: Check / Test
     runs-on: self-hosted
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # `task check` compares plugin.json's version to `git describe`;
+          # a shallow checkout has no tags and would skip that assertion.
+          fetch-depth: 0
+
+      - name: Verify toolchain
+        run: task --version
+
+      - name: Structure gate
+        run: task check
+
+      - name: Shell test suite
+        run: task test
+
+  tag:
+    name: Tag release
+    needs: gate
+    runs-on: self-hosted
+    if: github.event_name == 'push' && github.ref == 'refs/heads/main'
     steps:
       - name: checkout
         uses: actions/checkout@v4
@@ -72,16 +101,19 @@ jobs:
           manifest=.claude-plugin/plugin.json
           version="${TAG#v}"
           current=$(sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "$manifest" | head -1)
+          git tag -a "$TAG" -m "release $TAG"
+
           if [ "$current" != "$version" ]; then
             sed -i "s/\"version\"[[:space:]]*:[[:space:]]*\"$current\"/\"version\": \"$version\"/" "$manifest"
             git add "$manifest"
             # The trailer below stops this push from re-triggering the tagger.
             git commit -m "chore(release): plugin.json version $version" -m "Skip-Release: true"
-            git push origin HEAD:main
+            # Commit and tag go up in ONE push: the gate job runs `task check`
+            # on the bump commit, and that asserts manifest == latest tag. Two
+            # separate pushes leave a window where it legitimately fails.
+            git push origin HEAD:main "$TAG"
             echo "bumped $manifest to $version"
-          fi
+          else
-
-          git tag -a "$TAG" -m "release $TAG"
-          # gitea-actions checkout uses an x-access-token URL; reuse it.
             git push origin "$TAG"
+          fi
           echo "pushed $TAG"

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"
+# `env` is a dump only when nothing but options and VAR=value assignments
-fi
+# follow it. `env -u VAR cmd` and `env FOO=bar cmd` EXEC a command with a
-if printf '%s' "$CMD" | grep -Eq '(^|[;&|])[[:space:]]*env[[:space:]]*($|[;&|])'; then
+# modified environment and print nothing themselves — piping those into grep
-  deny "Blocked: bare 'env' prints every variable's value, including secrets. Use an existence-only check: [ -n \"\$VAR\" ] && echo set"
+# 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"

tests/ci_gate_test.sh

@@ -0,0 +1,57 @@
+#!/bin/sh
+# Tests for the CI gate workflow.
+#
+# The regression these exist for: this repo had a release workflow that
+# auto-tagged every push to main, and a `task check` / `task test` suite that
+# nothing ever ran. CI reported green having tested nothing — a gate that
+# fails *open*. These assertions pin the wiring so it cannot silently regress.
+
+set -u
+
+ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
+SUT="$ROOT/.gitea/workflows/ci.yml"
+
+pass=0
+fail=0
+
+ok() { pass=$((pass + 1)); printf '  ok   %s\n' "$1"; }
+no() { fail=$((fail + 1)); printf '  FAIL %s\n' "$1"; }
+
+if [ ! -f "$SUT" ]; then
+  printf 'FAIL: %s does not exist\n' "$SUT"
+  exit 1
+fi
+ok 'ci.yml exists'
+
+# A pull_request trigger is what gives tbd_ship a CI run to read on the head
+# commit of a branch; without one it fails closed and never merges (#22).
+grep -qE '^  pull_request:' "$SUT" \
+  && ok 'runs on pull_request' \
+  || no 'runs on pull_request'
+
+grep -qE '^ +push:' "$SUT" \
+  && ok 'runs on push' \
+  || no 'runs on push'
+
+grep -qE '^ +- *(run: *)?task check$|task check' "$SUT" \
+  && ok 'gate runs task check' \
+  || no 'gate runs task check'
+
+grep -qE 'task test' "$SUT" \
+  && ok 'gate runs task test' \
+  || no 'gate runs task test'
+
+# The point of the gate: tagging must not happen on a red check.
+tag_job=$(awk '/^  tag:/{f=1} f&&/^  [a-z][a-z_-]*:/&&!/^  tag:/{f=0} f' "$SUT")
+printf '%s' "$tag_job" | grep -qE '^ +needs: *gate' \
+  && ok 'tag job needs the gate' \
+  || no 'tag job needs the gate'
+
+# The old workflow file must be gone, or two workflows race on the same push
+# and the ungated one still tags.
+[ ! -f "$ROOT/.gitea/workflows/release.yml" ] \
+  && ok 'ungated release.yml removed' \
+  || no 'ungated release.yml removed'
+
+printf '\nci_gate: %d passed, %d failed\n' "$pass" "$fail"
+[ "$fail" -eq 0 ]

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'