Skip to content

Commit 0f7c8be

Browse files
committed
fix(checkpoints): GW-17 could never have run
The checkpoint runner rejects the pattern GW-17 shipped with. Measured by sourcing is_safe_eval_command out of automated-assessment's run-checkpoints.sh and calling it: OLD: REJECTED — pattern contains command-chaining metacharacter (; && || ` $()) NEW: accepted Two independent reasons, both fatal: the allowlist rejects `$(`, `;`, `&&`, `||` and backticks outright, and a multi-line YAML scalar reaches the runner as an EMPTY pattern ("checkpoint likely uses multi-line YAML scalar; use single-line pattern"). A mechanical checkpoint that cannot execute is the same defect this branch exists to remove, one layer up. GW-17 is now single-line, HEAD-only, and free of chaining metacharacters. `sed -n -e '/^$/q' -e p` is the semicolon-free spelling of the header cut, so the message-body false positive stays closed — verified against a commit whose body starts with `gpgsig`. The sweep over several commits stays where a full shell is available: verify-git-workflow.sh (last 10) and signing-preflight.sh. Mirroring rather than reproducing is the arrangement skill-repo already documents for SR-37. The suite now asserts both allowlist constraints on the pattern, so the next edit cannot reintroduce either. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
1 parent 377ef58 commit 0f7c8be

2 files changed

Lines changed: 20 additions & 4 deletions

File tree

skills/git-workflow/checkpoints.yaml

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,20 @@ mechanical:
151151
# unsigned, which fails a fully signed repository. Whether a signature
152152
# verifies is the host's answer (GW-11 / the commits API), not this one's.
153153
# Kept in step with signing-preflight.sh by tests/test_signing_preflight.sh.
154+
#
155+
# HEAD only, single line, and no `$(`/`;`/`&&`: the checkpoint runner's
156+
# allowlist (is_safe_eval_command in automated-assessment's
157+
# run-checkpoints.sh) rejects command-chaining metacharacters outright, and a
158+
# multi-line YAML scalar reaches it as an EMPTY pattern. A loop over the last
159+
# five commits satisfies neither, so this mirrors the rule rather than
160+
# reproducing it — the same arrangement as SR-37 in skill-repo. The full
161+
# sweep lives in verify-git-workflow.sh (last 10) and signing-preflight.sh.
162+
# `sed -n -e '/^$/q' -e p` is the semicolon-free spelling of the header cut.
154163
- id: GW-17
155164
type: command
156-
pattern: |-
157-
test -z "$(git log -5 --format=%H | while read -r c; do git cat-file commit "$c" | sed -n '/^$/q;p' | grep -qE '^gpgsig(-sha256)? ' || echo "$c"; done)"
165+
pattern: "git cat-file commit HEAD | sed -n -e '/^$/q' -e p | grep -qE '^gpgsig(-sha256)? '"
158166
severity: info
159-
desc: "Recent commits should carry a GPG/SSH signature"
167+
desc: "HEAD should carry a GPG/SSH signature"
160168

161169
llm_reviews:
162170
# === CONVENTIONAL COMMITS ===

tests/test_signing_preflight.sh

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,15 @@ CHECKPOINTS="$REPO_ROOT/skills/git-workflow/checkpoints.yaml"
174174

175175
# Extracted with awk rather than a YAML parser: CI installs no PyYAML, and a
176176
# test that needs a dependency the runner lacks is a test that does not run.
177-
gw17=$(awk '/^ - id: GW-17$/{f=1} f && /^ test -z/{print; exit}' "$CHECKPOINTS")
177+
gw17=$(awk '/^ - id: GW-17$/{f=1} f && /^ pattern: /{sub(/^ pattern: "/, ""); sub(/"$/, ""); print; exit}' "$CHECKPOINTS")
178+
179+
# The checkpoint runner's allowlist (is_safe_eval_command) rejects `$(`, `;`,
180+
# `&&`, `||` and backticks outright, and a multi-line YAML scalar reaches it as
181+
# an empty pattern. A checkpoint that trips either never runs — the exact
182+
# failure this whole branch is about, one layer up.
183+
check "GW-17's pattern is a single line" 1 "$(wc -l <<<"$gw17")"
184+
check "GW-17's pattern has no chaining metacharacters" 0 \
185+
"$(grep -cE '[;`]|&&|\|\||\$\(' <<<"$gw17")"
178186

179187
# The load-bearing part is the header pattern. Every place that answers "is this
180188
# commit signed" must use the same rule, or one of them silently becomes a

0 commit comments

Comments
 (0)