fix(repo-hygiene): validate clean-batch plan records against requested --tier (#1081)

[kyle-sexton]

Summary

clean-batch.sh --apply --batch-plan <path> dispatched purely on each plan line's
kind (REPO/GITDIR); the apply-time --tier flag was informational only (it set
the banner and whether the summary reports gitdirs=). A stale or swapped plan
therefore executed its full gated content while the banner named a narrower tier —
e.g. a --tier build dry-run plan applied with --tier caches removed both bin/
and .pytest_cache/ while printing Tier: caches. Bounded defect: every removed
path was still enumerated and confirmation-gated at plan creation, so nothing
un-gated is ever removed — the flaw is scope misrepresentation at apply time.

Fix

Per the triage decision on #1081 (validate, do not drop the flag), --apply now
pre-scans the whole plan before the banner and before touching any disk and
refuses it atomically (usage error, exit 2, nothing removed, no apply banner) when a
record the requested --tier does not authorize is present:

Record	Authorized under
REPO token caches	--tier caches
REPO token build	--tier build, --tier all
GITDIR	--tier git, --tier all

This rejects a build-class REPO record under --tier caches and gates the
GITDIR arm on a git-bearing tier, exactly as the triage resolved. Atomic refusal
(vs. per-record rejection) means the apply banner is never printed for a mismatched
plan, so the misrepresentation is structurally impossible rather than merely caught
after some records already ran. A malformed/unrecognized token is still handled as
before by the apply loop's per-record fail-closed guard (exit 1) — a distinct error
class (structural corruption) from a well-formed plan built for the wrong tier.

Authorization uses a dedicated tier_repo_token helper (empty for the git tier) —
deliberately not the existing manifest_child_token, which returns build for
git and would wrongly authorize a build REPO record under --tier git.

Header / usage / clean-batch.md exit-taxonomy and gated-set docs updated to match.

Verification

All commands run in the worktree on this branch.

Full test suite (57 assertions, incl. 3 new tier-authorization tests) + shellcheck:

$ shellcheck plugins/repo-hygiene/skills/clean/scripts/clean-batch.sh && echo CLEAN
CLEAN
$ bash plugins/repo-hygiene/skills/clean/scripts/clean-batch.test.sh | tail -8
PASS: [35] build plan under --tier caches is refused (exit 2)
PASS: [36] tier-mismatch refusal reported
PASS: [37] no apply banner printed on tier mismatch
PASS: [38] build dir NOT removed by mismatched apply
PASS: [39] cache NOT removed by mismatched apply
PASS: [40] git plan under --tier caches is refused (exit 2)
PASS: [41] GITDIR-under-non-git refusal reported
PASS: [42] all plan under --tier all applies (exit 0)
...
clean-batch.test.sh: all passed

Exact issue repro (a --tier build dry-run plan applied with --tier caches) —
both bin/ and .pytest_cache/ survive:

=== plan contents (REPO/build record folds caches) ===
REPO	<demo>	build	<tmp>/000-....manifest

=== 2. --tier caches --apply --batch-plan <build-plan>  (the bug scenario) ===
clean-batch.sh: plan record does not match --tier caches: a 'build' REPO record is not authorized (plan built for a different tier?). Re-run --dry-run --tier caches.
exit=2
=== 3. artifacts still present? ===
<demo>/.pytest_cache/x
<demo>/bin/b
BOTH SURVIVE -- scope misrepresentation prevented

Version: plugins/repo-hygiene bumped 0.7.1 → 0.7.2 (patch bugfix) with a matching
CHANGELOG [0.7.2] Fixed entry. main published its own doc-only 0.7.1 while
this branch was open, so this fix takes 0.7.2 — otherwise consumers already on the
published 0.7.1 would never see it. main's [0.7.1] entry is preserved verbatim.

Related

• repo-hygiene: no batch mode for caches/build/git tiers — fleet sweep forces hand-rolled orchestration #994 (fleet batch mode, PR feat(repo-hygiene): fleet batch mode for the caches/build/git/all tiers #1064) — where the plan-as-gated-set contract landed;
  this issue is the deferred Codex P2 finding from that review.
• The triage comment on repo-hygiene: clean-batch apply should validate plan records against the requested --tier #1081 resolved the validate-vs-drop-flag fork in favor of
  validation ("reject build-class REPO records under --tier caches; gate the
  GITDIR arm on a git-bearing tier — veto before merge"); this PR implements that
  direction exactly.

Closes #1081

Work-class: C3 (bug-fix-shaped) — attended triage 2026-07-23, operator-ratified. 🤖

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[kyle-sexton]

🔒 babysit-prs lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, safe tier). Will fix clear branch-owned findings and push; will not resolve threads or merge (safe tier).

[claude]

Claude finished @kyle-sexton's task in 2m 27s —— View job

---

PR Review — fix(repo-hygiene): validate clean-batch plan records against requested --tier (#1081)

• Gather context and understand PR scope
• Read changed files (clean-batch.sh, clean-batch.test.sh, clean-batch.md, plugin.json, CHANGELOG.md)
• Analyze correctness of tier-authorization logic
• Review test coverage
• Check documentation and version bump alignment
• Post review findings

---

Summary

The fix is correct and well-designed. The tier_repo_token helper correctly encodes the authorization matrix, the pre-scan is genuinely atomic (runs before the banner and before any disk operation), and fail_usage exits with the right code (2). The separation between tier-mismatch (exit 2, usage error) and structural corruption in the apply loop (exit 1) is clearly maintained. Three findings below, two minor and one cosmetic.

---

Findings

1. Test gap: caches plan applied under --tier build is not covered

The documentation at clean-batch.md:108 lists this as a refused case:

a caches record under build

The code correctly handles it — tier_repo_token() returns 'build' for --tier build, so a caches REPO record (b=caches) fails the comparison — but clean-batch.test.sh has no assertion for it. Tests 4d and 4e cover broader-under-narrower (build→caches, git→caches) and the correct round-trip (all→all), but the narrower-under-broader direction (caches→build) is untested.

The actual logic path: pre-scan reads kind=REPO, b=caches; tier_repo_token() returns build; caches != build → fail_usage. Straightforward to add a test alongside 4d/4e.

Fix this →

---

2. PR description version note is stale

The PR body says "Version: plugins/repo-hygiene bumped 0.7.0 → 0.7.1 (patch bugfix)", but plugin.json and CHANGELOG.md both show 0.7.2. The branch already carried a 0.7.1 bump (the test-helpers doc pointer), so this fix correctly lands at 0.7.2. The code is self-consistent; only the PR body text is wrong.

Cosmetic only — the changelog and manifest agree, so this does not affect release tooling.

---

3. Minor: pre-scan silently skips unrecognized REPO tokens

clean-batch.sh:203:

[[ "$b" == caches || "$b" == build ]] || continue

A REPO record with an unrecognized token (e.g. bogus) is silently skipped in the pre-scan and deferred to the apply loop, which correctly fails it closed (exit 1) as structural corruption. This is the documented design ("a different error class from a well-formed plan for the wrong tier"), and the per-record guard at line 233 handles it correctly. This is intentional — just confirming the logic is sound, not requesting a change.

---

What's working well

• tier_repo_token vs manifest_child_token distinction — the comment at lines 172–174 clearly explains why a separate helper is needed; manifest_child_token never returns empty, which would incorrectly authorize a build REPO record under --tier git. The new helper fails closed for the git tier by returning an empty string. ✓
• Atomic refusal — pre-scan runs entirely before printf 'Fleet Clean (apply)\n', making the banner/scope-misrepresentation impossible by construction, not just by detection. Test 4d verifies this with assert_not_contains. ✓
• --tier all round-trip — tier_repo_token() returns 'build' for all, matching what manifest_child_token writes into the plan, so the common case (all plan → --tier all) passes cleanly. Test 4f confirms it. ✓
• GITDIR pre-scan is correctly scoped — the GITDIR record has only 3 fields; c is empty but unused in the GITDIR case. IFS=$'\t' scoping to the read call is standard bash idiom. ✓

---

Finding 1 (test gap) is worth addressing before merge since the docs explicitly call out the caches-under-build refusal as expected behavior. Findings 2 and 3 are cosmetic/informational.

[claude]

Claude finished @kyle-sexton's task in 2m 46s —— View job

---

Security Review — PR #1088

• Fetch PR diff and read changed files
• Analyze clean-batch.sh for shell injection, path traversal, and logic flaws
• Analyze test file for security-relevant issues
• Review docs/manifest changes for surface changes
• Report findings

---

No security issues found.

The five changed files are: clean-batch.sh (45 lines added, 3 removed), clean-batch.test.sh (38 added), clean-batch.md (7 added), plugin.json (version bump), and CHANGELOG.md. Review focused on the shell script changes where the entire new attack surface lives.

Authorization logic (tier_repo_token + pre-scan)

The new tier_repo_token() helper and the pre-scan loop (clean-batch.sh lines 175–213) implement fail-closed semantics at every branch:

• tier_repo_token returns empty ('') for --tier git, so any recognized REPO token (caches or build) fails [[ "$b" != "" ]] → fail_usage. A tier with no REPO work cannot be coerced into authorizing a REPO record.
• GITDIR records are gated on tier_has_git (git or all only) — correctly refuses GITDIR under caches or build.
• Unrecognized/malformed tokens hit [[ "$b" == caches || "$b" == build ]] || continue → skipped in the pre-scan and then caught as a malformed plan record (exit 1) in the apply loop. Nothing is removed — fail-closed.
• --tier all returns build from tier_repo_token, correctly refusing a caches-tier plan applied as all.

Injection surfaces

All values sourced from the plan file ($a toplevel, $b token, $c manifest path) are double-quoted throughout — cd "$a", bash "$child" --apply --manifest "$c", batch_emit "$a" ... — ruling out word-splitting or glob injection. The error message in fail_usage embeds $b only after the guard [[ "$b" == caches || "$b" == build ]] || continue has already restricted it to one of two safe literals; no arbitrary plan-file content reaches stderr.

TOCTOU between pre-scan and apply loop

The plan file is read twice: once for the tier pre-scan, once for the apply loop. A local attacker who can overwrite the plan file between those two reads could bypass the authorization check. This is not a meaningful security boundary — a principal who can write the plan file already has the file-system access needed for direct impact. The existing single-read alternatives (reading into memory, then replaying) would not change the threat model for a local CLI tool.

Structural-corruption vs. tier-mismatch distinction

Exit 2 (tier mismatch, refused before banner) vs. exit 1 (malformed record, caught per-record in apply loop) is intentional and correct. The two error classes are orthogonal: tier mismatch is a policy violation; structural corruption is data integrity failure. The distinction is documented and tested.

Test surface

The new test cases (sections 4d–4f in clean-batch.test.sh) exercise the exact repro scenario from the issue, GITDIR refusal under a non-git tier, and the "not over-strict" all-plan-under-all` pass case. No test-helper changes introduce new injection surfaces.

[claude]

Claude finished @kyle-sexton's task in 1m 58s —— View job

---

PR Review — fix(repo-hygiene): validate clean-batch plan records against requested --tier (#1081)

• Gather context and understand PR scope
• Read changed files (clean-batch.sh, clean-batch.test.sh, clean-batch.md, plugin.json, CHANGELOG.md)
• Analyze correctness of tier-authorization logic
• Review test coverage for gaps
• Check documentation and version bump alignment
• Post review findings

---

Summary

The fix is correct and structurally sound. tier_repo_token() implements a clean authorization matrix, the pre-scan runs before the banner and before any disk operation (atomic refusal), and the fail_usage exit path (exit 2) is correctly distinct from the apply loop's per-record structural-corruption path (exit 1). One actionable test gap below; the rest are informational.

---

Finding 1 — Test gap: caches plan applied under --tier build is untested

clean-batch.md:108 explicitly documents this as a refused case:

a caches record under build

The logic is correct — tier_repo_token() returns build for --tier build, so a caches REPO record ($b=caches) hits caches != build → fail_usage. But test sections 4d–4f don't cover it. The tests cover broader-under-narrower (4d: build→caches, 4e: git→caches) and the round-trip (4f: all→all), but the narrower-under-broader direction (caches→build) — which the docs call out — has no assertion.

Worth adding before merge since the docs claim it and the assertion would lock in the behavior. Easy to add alongside 4d/4e.

Fix this →

---

Finding 2 (informational) — Pre-scan silently skips unrecognized REPO tokens

clean-batch.sh:203:

[[ "$b" == caches || "$b" == build ]] || continue

A REPO record with an unrecognized token is silently skipped in the pre-scan and deferred to the apply loop's per-record malformed-record guard (exit 1). This is the documented design — a different error class from tier mismatch — and the per-record guard at line 233 catches it correctly. No change needed; noting for completeness.

---

What's working well

• tier_repo_token vs manifest_child_token — the comment at lines 172–174 is precise about why a separate helper is necessary. manifest_child_token never returns empty and would wrongly authorize a build REPO record under --tier git; the new helper returns '' for git, causing any recognized REPO token (caches or build) to fail the != "" comparison and be refused. Fail-closed. ✓

• Atomic refusal — the pre-scan loop (lines 199–213) runs to completion before printf 'Fleet Clean (apply)\n' at line 215. Test 4d's assert_not_contains "no apply banner printed on tier mismatch" verifies this property directly. ✓

• GITDIR arm correctly scoped — tier_has_git (line 166) returns true only for git and all, so a GITDIR record in a plan applied under --tier caches or --tier build hits fail_usage at line 209 before any pruning. ✓

• $b in the error message is always a safe literal — by the time fail_usage embeds $b in the error string (line 205), the || continue guard at line 203 has already restricted it to one of caches or build. No arbitrary plan-file content reaches stderr. ✓

• Version and changelog are self-consistent — plugin.json and CHANGELOG.md both carry 0.7.2; the 0.7.1 doc-only entry from main is preserved verbatim. ✓

---

Finding 1 is the only item worth addressing before merge — it's a gap between the docs' stated guarantee and the test suite's coverage. Everything else is correct.

[claude]

Claude finished @kyle-sexton's task in 1m 52s —— View job

---

Security Review — PR #1088

• Get PR diff and identify changed files
• Analyze clean-batch.sh for injection, path traversal, authorization logic
• Analyze tier authorization matrix for bypass vectors
• Check plan file parsing for unsafe handling
• Review test file for security-relevant patterns
• Post security findings

---

No security issues found.

The five changed files are clean-batch.sh (+45/-3), clean-batch.test.sh (+38), clean-batch.md (+7), plugin.json (version bump), and CHANGELOG.md. The security-relevant surface is entirely in the shell script changes.

Tier-authorization logic (tier_repo_token + pre-scan)

The new pre-scan (lines 199–213) is fail-closed at every branch:

• tier_repo_token() returns '' for --tier git, causing any recognized REPO token to fail [[ "$b" != "$(tier_repo_token)" ]] (empty string vs. caches/build) → fail_usage. No REPO record can be coerced through a git-tier apply.
• GITDIR records are gated on tier_has_git (git or all only) — correctly refuses GITDIR under caches or build.
• --tier all returns build from tier_repo_token, which matches the token manifest_child_token writes into all-tier plans. The round-trip is correct.
• Unrecognized REPO tokens bypass the pre-scan via [[ "$b" == caches || "$b" == build ]] || continue and are caught as malformed plan record (exit 1) in the existing apply loop — distinct error class, still fail-closed.

Injection surfaces

Plan-file field $b in the error message (line 205): $b originates from plan-file content parsed by IFS=$'\t' read -r .... The || continue on line 203 guarantees that by the time $b appears in the fail_usage string it is exactly caches or build — two safe literals. The error path writes only to stderr, not to a command or eval. No injection risk.

$TIER in the GITDIR error message (line 209): $TIER comes from the CLI argument, validated at lines 151–155 to be one of caches|build|git|all. Safe.

read -r: The -r flag prevents backslash interpretation across all plan-file reads. All IFS assignments are scoped to the read call. No glob expansion inside [[ ]]. ✓

TOCTOU (plan file read twice)

The plan file is read once for the pre-scan and once for the apply loop — a local attacker with write access to the plan file could swap it between reads. However, write access to the plan file implies the same file-system privilege needed for direct impact. This is not a meaningful security boundary for a local CLI tool, and mitigating it (e.g., reading into memory) would not change the threat model. Previously documented, not new to this PR. No new risk introduced.

No new command execution surfaces

The new helper tier_repo_token() uses printf only. The pre-scan loop calls tier_repo_token() and fail_usage() — no eval, no source, no new child process execution on plan-controlled data.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f699fdb84f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude encountered an error after 2s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 2s —— View job

---

I'll analyze this and get back to you.

[github-actions]

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/30185335365
• Last SDK result: {"subtype":"success","is_error":true,"num_turns":1,"duration_ms":408,"total_cost_usd":0}

Re-running the job, or pushing a new commit, will retry the review.

[github-actions]

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/30185335372
• Last SDK result: {"subtype":"success","is_error":true,"num_turns":1,"duration_ms":434,"total_cost_usd":0}

Re-running the job, or pushing a new commit, will retry the review.

[claude]

Claude encountered an error after 2s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 2s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98fbe7064a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.