fix(repo-hygiene): close the clean-guard gaps Security & Cost found on #8168 #8622

Activity

1. github-actions

   added

   needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

   on Oct 20, 2026
2. github-actions commented on Oct 20, 2026

   

   github-actionsBot

   on Oct 20, 2026 – with GitHub Actions

   Last edited by melodic-automation-lanes

   Contributor

   More actions

   Proposed work class: work-class: scoped
   Basis: automated intake triage; the lanes App applied this class.

   Size: Small
   Complexity: Medium
   Uncertainty: Known
   Flags: none
   Blocked by: none

   Decision recorded. The owner answered in the Refinement and Issue Triage sessions (2026-10-25), deciding the open items below on the facts:

   go with whatever the facts/data/metrics + /discovery:research says and ensure we're aligned with SOLID, DRY/SSOT principles

   [quote truncated; the full decision is at the link]

   #8622 (comment)

   The decision settles the open questions: all item-1 forms are covered, a mismatched CLAUDE_PLUGIN_DATA fails closed using the clean-batch.sh segment rule, and hook-utils.sh is sourced with || exit 2. No questions remain. Related: #9779 edits the same matchers, so the two changes may need coordination.

   _{Automated triage, applied by the lanes App.}
3. github-actions

   added

   area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.Security-relevant: vulnerability, hardening, or disclosure follow-up.

   priority: lowNice-to-have, cosmetic, or speculative; opportunistic.Nice-to-have, cosmetic, or speculative; opportunistic.

   on Oct 20, 2026
4. github-actions

   added the

   Bug

   issue type on Oct 20, 2026
5. github-actions commented on Oct 26, 2026

   

   github-actionsBot

   on Oct 26, 2026 – with GitHub Actions

   Contributor

   More actions

   The triage sweep dispatched intake triage for this issue (attempt 1 of 2).
6. github-actions

   added

   needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.

   and removed

   needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

   on Oct 26, 2026
7. melodic-automation-lanes commented on Oct 26, 2026

   

   melodic-automation-lanesBot

   on Oct 26, 2026

   Last edited by melodic-automation-lanes

   Contributor

   More actions

   The latest triage verdict holds nothing: the issue is at status: ready.
8. melodic-automation-lanes

   added

   status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.

   on Oct 26, 2026
9. kyle-sexton

   removed

   status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.

   needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.

   on Oct 26, 2026
10. kyle-sexton commented on Oct 26, 2026

    

    kyle-sexton

    on Oct 26, 2026

    MemberAuthor

    More actions

    Decision recorded. The owner answered in the Refinement and Issue Triage sessions (2026-10-25), deciding the open items below on the facts:

    go with whatever the facts/data/metrics + /discovery:research says and ensure we're aligned with SOLID, DRY/SSOT principles

    I should only ever be in the loop ONLY when necessary (intent/vision changed, something came up during implementation that impacts trajectory/alignment, OR a high security risk) - I've covered this all before

    No owner ledger (.work/*/interview-checklist.md, .work/gh-monitor-owner-decisions/) names this issue. The body rates all three findings P4 or P5, so this is no high security risk. Every change below hardens the guard; none narrows what it blocks.

    All three gaps are still open on main. A probe of the origin/main scripts (a live marker for session s1, CLAUDE_PLUGIN_DATA ending in repo-hygiene) exited 0, allowing the command, for every item-1 form: x="$(git stash drop)", "$(rm -rf x)", a backtick inside double quotes, $() in an unquoted heredoc, bash -c behind sudo, timeout, env and xargs, eval, ssh h "…", bash <<< "…", printf … | bash, and git --git-dir /r/.git reset --hard. Bare git stash drop and git reset --hard exited 2. A CLAUDE_PLUGIN_DATA naming another plugin exited 0. With hook-utils.sh removed, git stash drop exited 0 after hook::bash_parse_segments: command not found.

    1. Item 1: which forms, and how?
       Decided: Every form the body lists, since its Acceptance asks for a stay-loud test for each one. The body's fix line (re-parse a word holding an unescaped $( or backtick, and every eval operand) does not reach the wrapper, ssh, <<<, pipe-into-shell and --git-dir forms, so the fix covers those too. The script's threat-model note says these forms bypass the guard "by design" (plugins/repo-hygiene/skills/clean/scripts/destructive-guard.sh:20-29, and :60-62 for a global option with a separate value). That note changes with the fix. It still calls the guard a best-effort net, not a security boundary, and still names permissions.deny as the durable boundary. Precision holds: the existing stay-quiet cases for quoted text (destructive-guard.test.sh:68-79) and the benign bash -c operand (:88) still pass.
    2. Item 2: how does a mismatched CLAUDE_PLUGIN_DATA fail closed?
       Decided: The guard treats a CLAUDE_PLUGIN_DATA whose last path segment is not repo-hygiene or repo-hygiene-* as unresolved, using the same segment rule as clean-batch.sh:155-161. That reaches the existing fail-closed path: no marker path, so the guard stays armed (destructive-guard.sh:334-338). The test for an unset data dir (destructive-guard.test.sh:215-220) is already in place. The suite's own fixture sets CLAUDE_PLUGIN_DATA="$TEST_ROOT/data" (destructive-guard.test.sh:23), so it moves to a directory named repo-hygiene. The body's other suggestion, a probe command after open, is not added. The fail-closed check removes the cause, and a probe would add a step to every run to detect the same thing.
    3. Item 3.
       Decided: As the body says. destructive-guard.sh:341 sources hook-utils.sh with no || exit 2.

    Out of scope, per the body: the marker-deletion unlock, which CLEAN_GUARD_ACK=1 already equals.

    Acceptance criteria: (converted from the body's Acceptance list, wording unchanged except "test" made specific to destructive-guard.test.sh, and the precision and comment lines added from decisions 1 and 2)

    • destructive-guard.test.sh has a stay-loud test for each form in item 1, and each fails before the fix.
    • A test where a mismatched CLAUDE_PLUGIN_DATA blocks instead of allowing.
    • A missing hook-utils.sh exits 2.
    • The existing stay-quiet cases in destructive-guard.test.sh still pass.
    • The threat-model note in destructive-guard.sh no longer lists the item-1 forms as bypassed by design, and still calls the guard a best-effort net with permissions.deny as the durable boundary.
11. github-actions

    added

    needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

    priority: mediumReal value, no hard deadline; normal backlog flow.Real value, no hard deadline; normal backlog flow.

    and removed

    priority: lowNice-to-have, cosmetic, or speculative; opportunistic.Nice-to-have, cosmetic, or speculative; opportunistic.

    on Oct 26, 2026
12. melodic-automation-lanes

    added

    status: readyTriaged, unblocked, and fully specified; eligible to pick up.Triaged, unblocked, and fully specified; eligible to pick up.

    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.A briefed fix or small feature; blast radius bounded by the brief, tests exist.

    admission-deferred

    needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

    and removed

    needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

    on Oct 26, 2026
13. melodic-automation-lanes commented on Oct 26, 2026

    

    melodic-automation-lanesBot

    on Oct 26, 2026

    Contributor

    More actions

    Refinement. A read-only run explored this repository, searched its issues and pull requests, and read the allowed docs to draft what follows. On an issue a listed human filed, it counts as the issue's spec once posted, while unedited; it is still model text, so check it.

    Intended outcome: Harden plugins/repo-hygiene/skills/clean/scripts/destructive-guard.sh so nested and wrapped destructive forms are blocked, a CLAUDE_PLUGIN_DATA not naming repo-hygiene fails closed, and a missing hook-utils.sh exits 2.

    Acceptance criteria

    • destructive-guard.test.sh has a stay-loud test (exit 2 while a run marker is live) for each item-1 form: x="$(git stash drop)", "$(rm -rf x)", backticks inside double quotes, $() in an unquoted heredoc, bash -c behind sudo, timeout, env and xargs, eval, ssh h "...", bash <<< "...", printf ... | bash, and git --git-dir /r/.git reset --hard.
    • Each of those new tests fails (exits 0) against the pre-fix destructive-guard.sh and passes after the fix.
    • A test sets CLAUDE_PLUGIN_DATA to a directory whose last segment is not repo-hygiene or repo-hygiene-* and asserts exit 2 instead of 0.
    • The guard accepts a CLAUDE_PLUGIN_DATA whose last segment is repo-hygiene or repo-hygiene-*, using the same rule as clean-batch.sh:155-161, and treats any other value as unresolved so the guard stays armed.
    • The test fixture at destructive-guard.test.sh:23 uses a data dir named repo-hygiene so existing marker tests still pass.
    • A test runs the guard from a copy of the scripts without hook-utils.sh and asserts exit 2.
    • destructive-guard.sh:341 sources hook-utils.sh with || exit 2.
    • The existing stay-quiet tests in destructive-guard.test.sh (quoted text at :68-79 and the benign bash -c operand at :88) still pass.
    • The threat-model note in destructive-guard.sh (about :20-29 and :60-62) no longer lists the item-1 forms as bypassed by design, and still calls the guard a best-effort net with permissions.deny as the durable boundary.
    • bash plugins/repo-hygiene/skills/clean/scripts/destructive-guard.test.sh exits 0.

    Related work

    • fix(repo-hygiene): scope clean's destructive guard to an open run and match parsed words #8168 (related): The merged PR that introduced the parsed-word guard these gaps were found on.
    • guardrails, disk-hygiene: narrow three destructive-guard matchers to their measured false positives #9779 (related): Also edits destructive-guard matchers (to narrow false positives); the two changes may touch the same code and need coordination.

    Sources read

    • /home/runner/work/_temp/refine-data/context.json: the owner's Decision recorded comment settles all three items, the approach and the acceptance list
    • plugins/repo-hygiene/skills/clean/scripts/destructive-guard.sh:335-341: marker path is resolved from CLAUDE_PLUGIN_DATA, and hook-utils.sh is sourced with no || exit 2
    • plugins/repo-hygiene/skills/clean/scripts/clean-batch.sh:155-159: existing last-segment check on CLAUDE_PLUGIN_DATA to reuse
    • plugins/repo-hygiene/skills/clean/scripts/destructive-guard.test.sh:23: fixture sets CLAUDE_PLUGIN_DATA to $TEST_ROOT/data, which must be renamed
    • plugins/repo-hygiene/skills/clean/scripts/destructive-guard.test.sh:217-220: existing unset-data-dir fail-closed test

    _{Automated refinement by the lanes App. Intake triages the issue again with it.}
14. melodic-automation-lanes

    added

    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.Fully specified and briefed; eligible for autonomous pickup from the frontier.

    and removed

    needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

    admission-deferred

    on Oct 26, 2026
15. melodic-automation-lanes

    closed this as completedin #10412on Oct 27, 2026