[melodic-automation-lanes]

Closes #7333

Hold: a human merges this

Summary

Opened by the work-resolve-issue lane. The run's own summary:

The clean destructive guard now gates only while a per-session run-open marker (under CLAUDE_PLUGIN_DATA, keyed by session_id, one-hour idle timeout) is live; mutating tiers open it and clear it at the final outcome, scan and scan-batch never write it. On the Bash tool it matches each parsed segment's argv via hook::bash_parse_segments (including bash -c operands), so destructive text inside a quoted argument no longer blocks. Hold: a human merges this.

Not verified

• check-skill reported FAIL for clean only because of remove-path.test.sh cases 39-40, a sandbox git-config error in a file this change does not touch; other results were clean.
• Not verified live: that CLAUDE_PLUGIN_DATA reaches a skill-frontmatter hook process and that ${CLAUDE_SESSION_ID} equals the hook input's session_id. No web docs were read this run. If CLAUDE_PLUGIN_DATA is empty the guard stays armed (previous behavior); if the two ids differ the guard allows comma
• The stay-quiet test was not run against the pre-fix guard; it is a case the old raw-regex matching blocked by construction.
• The PowerShell tool keeps raw-text matching, so quoted destructive text there still blocks. The wider repo test suite and the --check-bump CI step were not run.
• review: ran as one fresh-context subagent (review-venue.txt absent); its shell -c operand regression was fixed and tested, command substitution inside double quotes is left inert as a documented best-effort limit.

Fix

• plugins/repo-hygiene/skills/clean/scripts/destructive-guard.sh: marker check (fail closed when data dir or session id unresolved), argv_destructive/is_destructive_bash for the Bash tool, header corrected (skill hooks last the session)
• plugins/repo-hygiene/skills/clean/scripts/clean-run-marker.sh: new open/close script, no marker for scan and scan-batch
• plugins/repo-hygiene/skills/clean/scripts/lib/run-marker.sh: shared marker path, write and state helpers
• plugins/repo-hygiene/skills/clean/scripts/hook-utils.sh: generated copy of lib/hook-utils.sh
• scripts/shared-copies.txt: one new line registering that copy
• plugins/repo-hygiene/skills/clean/scripts/destructive-guard.test.sh: stay-quiet cases, marker cases, parser call, existing block cases now run with a live marker
• plugins/repo-hygiene/skills/clean/SKILL.md, context/run-marker.md, context/git-tree-reset.md, README.md: document the marker, open/clear steps and word matching
• .changes/repo-hygiene/fix-clean-guard-run-marker-quoted-args-7333a1f4.md: patch fragment

Verification

The run reports:

• run-test-file destructive-guard.test.sh: all 274 checks passed
• run-test-file plugins/repo-hygiene/scripts/allowed-tools-pairing.test.sh: passed
• sync-shared-copies --check: all 494 registered copies match
• check-changelog-fragments: all 146 fragments valid
• cant-fail-scan --file destructive-guard.test.sh --check: no gating findings
• check-skill plugins/repo-hygiene/skills: clean's SKILL.md markdownlint and every script test passed except remove-path.test.sh cases 39-40 (git config write to /dev/null refused in this sandbox; file untouched by this change)
• The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/39106103357

Left out: the items under Not verified.

Related

#7333

[github-actions]

claude-security-review reviewed b97ad7b3235b27a02691ab89e5bc099469b57797: 0 finding(s), 0 as inline comments.

Security review (incremental, head b97ad7b)

No security findings. Scope: the four files changed since 163aac2, read against destructive-guard.sh and hook-utils.sh. I did not run the test script.

What changed

The run-marker refresh trigger moves from a raw-text regex match to a parsed check. invokes_clean_script walks the parsed segments and counts a call only when a clean script is the command word or the first operand of bash/sh. It skips leading VAR=val words and re-parses bash -c operands. A command that only names a script (git add …/clean-batch.sh, a gh issue comment body) no longer refreshes or keeps alive a stale marker. The marker-absent early exit and the sourcing of hook-utils.sh were reordered, with the same outcome for each marker state.

Why this is not an exploitable weakening

• Blocking path is unchanged. With a live marker, is_acknowledged and is_destructive_bash (and is_destructive for other tools) decide as before. The new code only decides whether to refresh a marker. It cannot turn a block into an allow while the marker is live.
• The stale-marker allow is not new. The old code deleted a stale marker and allowed the command on any ordinary call. The new code differs only for a script call that is not at command position, such as behind sudo, env, timeout, or a PowerShell & call. That call used to keep the stale marker, and now it removes the marker and is allowed. The test file documents this as intended. An hour of idle is needed first, and the attacker does not control it. A prompt-injected model gains nothing, because any ordinary call at that point already ended the run.
• Forging a refresh is no easier than before. Running bash clean-caches.sh against a nonexistent file refreshed the marker under the old regex too. The parsed check is stricter, so mentioning a script in a quoted argument can no longer keep a run open.
• No new injection surface. CLEAN_SCRIPT_NAMES is a fixed literal. The =~ match runs on a value taken from parsed words, and the pattern is built from that constant, not from input. Nothing from the command is evaluated or expanded.
• Parse failure is safe. If hook::bash_parse_segments cannot parse a command, the result is "not a script call", so the marker is not refreshed. The blocking path is unaffected.

Instruction-surface check

The doc and changelog edits narrow nothing that forbids an action. They describe the refresh trigger more precisely. I did not consult the instruction exception register. No guardrail text was removed, and the guard code still enforces the blocking behavior.

Operational note, not a finding

The old raw-text regex matched script names in PowerShell commands as well. The new parser reads Bash syntax. A PowerShell call that runs a clean script through a wrapper no longer refreshes the marker. On a run that sits idle for over an hour, the guard stops gating, which is the same fail-open window as before. I treat it as a documented limitation.

[github-actions]

claude-review reviewed b97ad7b3235b27a02691ab89e5bc099469b57797: 0 finding(s), 0 as inline comments.

No findings on the incremental change (since 163aac2).

What changed. The guard no longer refreshes the run marker when a command merely names a clean script. It now refreshes only when a parsed segment runs one. A segment runs a script when the script is the command word, or the first operand of a bash/sh at the command position. Leading VAR=val words are skipped, and bash -c operands are re-parsed. This closes the gap where git add …/clean-batch.sh or a gh … --body 'see clean-caches.sh' kept a dead marker alive.

What I checked by reading (I ran no tests, so the PR's "274 checks passed" is author-claimed and unverified by me):

• Control flow in destructive-guard.sh. The marker absent case still exits 0 before hook-utils.sh is sourced. An unresolved data dir or session id leaves MARKER_STATE empty, which skips the new block and keeps the guard armed, as before. A stale marker plus a script run is still refreshed to live and kept. A stale marker plus an ordinary call is still deleted and the command allowed.
• Invocation forms in SKILL.md. Every form it documents is ${CLAUDE_SKILL_DIR}/scripts/<name>.sh …, run directly or behind CLEAN_GUARD_ACK=1. The new matcher handles both. I found no documented sudo/env/timeout wrapper, so the "wrapper not resolved" limit (stated in the code comment and covered by a test) doesn't hit a documented flow.
• New tests. They cover:
  • mention-only commands (git add, grep, sudo bash …) leave the marker stamp unchanged;
  • bash -c 'bash scripts/…' refreshes it;
  • a stale marker plus a comment naming a script is removed and allowed.
• Docs. The run-marker.md, header-comment and changelog-fragment wording match the new behavior. MD013 is disabled in .markdownlint-cli2.jsonc, so the long line in run-marker.md is fine.

Minor limits, not worth blocking (judgment):

• script_segment parses every tool's command as bash, including PowerShell. An unquoted backslash path like .\clean-caches.sh loses its backslashes in bash parsing, so it would no longer count as running a script. The old raw regex matched it. This only matters for a PowerShell dry-run followed by more than an hour of idle before the apply. A quoted path, or the documented ${CLAUDE_SKILL_DIR}/scripts/… form, still matches.
• bash -o pipefail script.sh and shells other than bash/sh (zsh, dash) at the command position are not resolved as running a script, though hook::shell_c_operand recognizes them for -c. Neither form appears in the skill's documented flows.

[melodic-automation-lanes]

pr-unblock / address-feedback finished round 1 of 3 on aa9329b.

[melodic-automation-lanes]

pr-unblock / address-feedback finished round 2 of 3 on 724b6f3.

[melodic-automation-lanes]

pr-unblock / address-feedback finished round 3 of 3 on 724b6f3.

[melodic-automation-lanes]

The pr-unblock lane stopped on this pull request and needs a person: a review finding is uncertain or deferred, so a person decides it.

• Reason: feedback-uncertain
• Resume key: 724b6f36d8e2a659fe78656ec1c3a315f2063b01 (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=8168 and activity=address-feedback:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=8168 -f activity=address-feedback

[melodic-automation-lanes]

pr-unblock / address-feedback finished round 4 of 3 on 6a3eef2.

[melodic-automation-lanes]

The pr-unblock lane stopped on this pull request and needs a person: a review finding is uncertain or deferred, so a person decides it.

• Reason: feedback-uncertain
• Resume key: 6a3eef2d6370d12315a325e522a76d6ab4119c58 (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=8168 and activity=address-feedback:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=8168 -f activity=address-feedback