Skip to content

fix(guards): read the command from tool_input, not the payload root - #80

Merged
CybotTM merged 5 commits into
mainfrom
fix/retro-guard-payload-shape
Aug 10, 2026
Merged

fix(guards): read the command from tool_input, not the payload root#80
CybotTM merged 5 commits into
mainfrom
fix/retro-guard-payload-shape

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 10, 2026

Copy link
Copy Markdown
Member

Summary

Both PreToolUse guards this skill ships are no-ops in Claude Code. They read the command from the payload root; Claude Code nests it under tool_input. Installing them therefore changes nothing — no lightweight tag is blocked, no gh release create is blocked.

Came from

/retro session on 2026-08-09/10. Found while installing the guards after a release had been cut without them.

  • Symptom: After wiring both guards into ~/.claude/settings.json, git tag v0.28.0 (lightweight) and gh release create v9.9.9 were both allowed.

  • Cause: parse_command() does data.get("command", ""). The Claude Code PreToolUse payload is {"tool_name": "Bash", "tool_input": {"command": "..."}}, so the lookup misses and the guard exits 0 on every invocation. Measured, same guard, same command, two payload shapes:

    guard payload rc
    guard-lightweight-tag {"command": "git tag v0.28.0"} 2
    guard-lightweight-tag {"tool_input": {"command": "git tag v0.28.0"}} 0
    guard-gh-release {"command": "gh release create v9"} 2
    guard-gh-release {"tool_input": {"command": "gh release create v9"}} 0

    Every other hook in a standard install reads (payload.get("tool_input") or {}).get("command").

  • Required behavior: Prefer tool_input.command, keep the flat shape working for direct invocation and tests.

  • Verification: scripts/tests/guard-payload-shape.test.sh.

Change

parse_command() in both guards: read tool_input.command first, fall back to a top-level command, and return "" for a non-dict payload instead of raising. No change to what either guard blocks — only to whether it ever sees the command.

New scripts/tests/guard-payload-shape.test.sh — 13 assertions covering both payload shapes on both guards, the allow paths (git tag -s, git tag -a, gh release edit --notes-file, gh release view) and the malformed-input cases (empty stdin, non-JSON) which must never block.

Test plan

  • New test green against the fix (13/13)
  • New test red against the previous scripts, failing on exactly the two nested blocking cases
  • Allow paths unchanged: signed tag, annotated tag, --notes-file edit, read-only view all exit 0
  • pre-commit (ruff, shellcheck) green; commit signed and DCO signed-off
  • Reviewer: worth checking whether other skills in the org ship guards with the same root-level parse — this one went unnoticed because nothing tested the guards at all.

SonarCloud

Quality Gate rot, einzige verletzte Bedingung new_duplicated_lines_density 29.5 % > 3. Offene Issues: 0 (die 12 Code Smells der ersten Fassung sind mit dem Umbau des Tests auf eine Falltabelle weg, die Testdatei zählt jetzt 0 duplizierte Zeilen).

Die verbleibenden 26 duplizierten Zeilen sind das identische parse_command in beiden Guards, 13 Zeilen je Datei — also exakt der Fix, den dieser PR an derselben Stelle in zwei Dateien anbringt:

Datei new_lines new_duplicated_lines
guard-gh-release.py 13 13
guard-lightweight-tag.py 13 13
tests/guard-payload-shape.test.sh 62 0

Ein gemeinsames Modul wäre hier falsch: beide Guards werden einzeln nach ~/.claude/hooks/ kopiert (so steht es in der Installationsanweisung und so macht es settings.json). Ein Import würde jede Einzelkopie brechen.

Damit sind die drei Bedingungen aus git-workflow/references/merge-gate-watcher.md § "Sonar gate introspection" erfüllt: einzige rote Bedingung ist eine Zurechnungsmetrik auf berührten Zeilen, offene Issues sind 0, und die Begründung steht hier.

Both PreToolUse guards parse the hook payload with `data.get("command")`.
Claude Code delivers the command nested as `tool_input.command`, so every real
invocation fell through to exit 0: guard-lightweight-tag blocked no lightweight
tag and guard-gh-release blocked no `gh release create` in the harness they are
installed into. Every other hook in a standard install reads
`(payload.get("tool_input") or {}).get("command")`.

Found while installing both guards after a release was cut without them
(2026-08-09). Wiring them changed nothing, which is what exposed the parse.

parse_command now prefers tool_input.command and falls back to a top-level
`command`, so direct invocation and other harnesses keep working. A non-dict
payload yields "" instead of raising.

Adds tests/guard-payload-shape.test.sh: 13 assertions over both payload
shapes, both guards, the allow paths (signed tag, annotated tag, notes-file
edit, read-only view) and the malformed-input cases. Green against the fix,
red on exactly the two nested blocking cases against the previous scripts.

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Copilot AI lite review requested due to automatic review settings August 10, 2026 05:33
@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

SonarCloud flagged 12 code smells and 33.8% duplicated lines on the previous
shape: thirteen near-identical check() calls, each passing positional
parameters straight through. The assertions and their expected exit codes are
unchanged; they now come from a single case table read by one loop.

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Both PreToolUse guards are single-file scripts by design: the install recipe
copies one of them into ~/.claude/hooks/ and wires that path in settings.json,
so a shared helper module would break every single-file copy. The payload
parsing is therefore identical in both — 13 lines each, enough to fail
new_duplicated_lines_density on any PR touching them (29.5% on this one, with
0 open issues).

Scope is duplication detection on those two files only; every other rule keeps
applying, and the test file that carried the real smells was restructured
rather than excluded.

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
sonar-project.properties is scanner-only. This project runs SonarCloud
Automatic Analysis (sonar.autoscan.enabled=true, verified via the settings
API), which reads .sonarcloud.properties — the previous commit's file was
never picked up and the API confirmed no cpd exclusion was in effect.

projectKey/organization dropped: autoscan supplies both, and neither was read
from the file.

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
@sonarqubecloud

Copy link
Copy Markdown

@CybotTM

CybotTM commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

Bot-Review nicht verfügbar — von Hand geprüft und darauf entschieden.

Copilot antwortet mit der Quota-Meldung. Die fünf Pflicht-Checks sind grün; copilot-pull-request-reviewer steht laut isRequired(pullRequestNumber:80) auf keiner Pflichtliste. SonarCloud steht inzwischen auf OK (Duplikation 0.0), siehe unten.

Ich bin Autor dieses PRs — keine unabhängige Review.

Geprüft:

  • Der Defekt ist gemessen, nicht vermutet: gleicher Guard, gleicher Befehl, zwei Payload-Formen — flach rc=2, verschachtelt rc=0, für beide Guards.
  • Beim Messen selbst einen Fehler gemacht und korrigiert: eine Befehlssubstitution im printf-Argument überschrieb $?, wodurch kurz alle Aufrufe als rc=0 erschienen. Die belastbare Tabelle stammt aus dem Lauf, der rc sofort in eine Variable schreibt.
  • 13 Assertions grün gegen den Fix, rot gegen die unveränderten Guards in genau den zwei verschachtelten Blockierfällen — der Test misst den behobenen Defekt, nicht sich selbst.
  • Erlaubpfade unverändert: git tag -s, git tag -a, gh release edit --notes-file, gh release view; Fehlformate blockieren nie.

Zum Sonar-Verlauf, weil zwei Anläufe nötig waren: die erste Fassung des Tests brachte 12 Code Smells und 33,8 % Duplikation, beides aus dreizehn fast gleichen check-Aufrufen — behoben durch den Umbau auf eine Falltabelle, die Testdatei zählt jetzt 0 duplizierte Zeilen. Die verbliebene Duplikation war das identische parse_command in den zwei bewusst eigenständigen Guards. Die Ausnahme dafür landete zuerst in sonar-project.properties und wurde nicht gelesen: dieses Projekt läuft auf Automatic Analysis (sonar.autoscan.enabled=true, über die Settings-API geprüft), und die liest .sonarcloud.properties. Nach der Umbenennung steht das Gate auf OK.

@CybotTM
CybotTM merged commit 8b90f3a into main Aug 10, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants