fix(docs-hygiene): gate the porcelain rename split and unescape \\ (0.19.1)

[kyle-sexton]

Closes #3143

Summary

audit-noise's git status --porcelain parse dropped files whose paths git
treats specially. Both defects are the silent-false-negative class: the file
did not error, it simply disappeared from the target list, so a run over a tree
containing one reported clean.

Fix

Rename split is gated on the status letter, not the path text. It previously
fired on any record whose path contained " -> ", so a file literally named
notes -> draft.md was reduced to draft.md — a name that resolves to nothing.
It now gates on [RC] in either column, which is narrower and still catches a
rename recorded in the index or the worktree.

\\ is now unescaped as well as \". Git C-quotes a path for an embedded
backslash too, so back\-slash.md stayed escaped and resolved to nothing. Both
escapes are undone, \" before \\.

#3140 landed on main while this branch was in flight. The gate and the
two-step unescape here are identical to the ones it gave
code-tidying/audit-comment-residue, so the two porcelain parsers now converge
rather than failing in opposite directions on renames — which is what #3143
asked for.

audit-noise's SKILL.md does not mirror this parse — it uses a plain
grep '\.md$' pipeline — so the conditional "mirrored parser" half of the issue
does not apply here.

docs-hygiene 0.19.1.

Verification

• detect.test.sh: all 66 checks pass, including 5 new cases.
• Discriminator check: reverting only detect.sh to the old parse fails
  exactly the two new defect cases (' -> ' path, backslash path) and nothing
  else — so neither case passes vacuously. The rename cases pass under both
  implementations by design; they are regression guards showing the new gate
  does not cost the old -> new handling it narrows.
• All 7 docs-hygiene suites pass, re-run after merging main; nothing outside
  this skill references audit-noise/scripts/detect.sh.
• shellcheck, shfmt -d, markdownlint-cli2, typos, editorconfig-checker: clean.
• check-fixture-git-isolation.sh --check, check-plugin-manifest-presence.sh,
  check-changelog-parity.sh --check / --check-order / --check-bump /
  --check-preserved against origin/main: pass.

The suite also picks up the unset GIT_DIR GIT_WORK_TREE GIT_CONFIG isolation
line that 0.18.3's sweep missed on this file.

Known residual, recorded at the parse site rather than left implicit: git's
octal escapes (\NNN) for control and non-ASCII bytes are still not decoded, so
those paths continue to miss. Converging on git status --porcelain -z would
close the class outright rather than extending the string parse a third time.

Related

• Refs fix(code-tidying): parse porcelain by slicing so spaced paths survive discovery #3140 — the sibling code-tidying/audit-comment-residue parse fix this
  issue was deferred out of; now landed, and this change converges with it.
• Refs fix(code-tidying): audit-comment-residue silently skips paths containing spaces, reporting a clean tree instead #3126 — the original audit-comment-residue defect report.
• Refs Drain scripts/fixture-git-isolation-baseline.txt (36 grandfathered suites) #2872 — the fixture git-isolation convention this suite now satisfies.
• Refs feat(docs-hygiene): bucket extract-ssot candidates by multiplicity, gate artifact creation not reporting (0.19.0) #3142 — bumped docs-hygiene to 0.19.0 mid-flight; merged in, so this
  PR ships 0.19.1.

[claude]

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

---

Security review

• Gather context (PR diff, changed files)
• Apply skip gate
• Review changed files for security-relevant issues
• Post findings / conclusion

No security issues found.

This PR touches plugins/docs-hygiene/skills/audit-noise/scripts/detect.sh (and its test), a read-only local audit script that parses git status --porcelain output from the current working repo. Reviewed the full diff against the security criteria (injection, trust-boundary, auth/access-control, GitHub Actions, secrets handling):

• No eval, bash -c with concatenated strings, or other dynamic code execution is introduced. The status-letter gate ([[ "${line:0:1}" == [RC] || "${line:1:1}" == [RC] ]]) and the added backslash-unescape (${local_path//\\\\/\\}) are both plain Bash parameter expansions/pattern matches — no shell metacharacter interpretation of the parsed path.
• All array/variable usages remain properly quoted ("$target", "${TARGETS[@]}", "$local_path") both before and after the change, so a maliciously-named file (e.g. containing `, $(), or ;) still can't reach command injection.
• The C-quote unescaping only undoes \" and \\; it doesn't decode octal escapes (\NNN), and the PR explicitly documents this as a known residual (comment at plugins/docs-hygiene/skills/audit-noise/scripts/detect.sh lines 139–142) rather than a silent gap — no path-traversal implication since / isn't part of git's C-quote escape set and porcelain paths stay repo-relative.
• detect.test.sh adds unset GIT_DIR GIT_WORK_TREE GIT_CONFIG (line 7) to prevent an inherited git env from redirecting fixture git init/git config calls into the CI runner's actual repo — a good isolation hardening, not a regression.
• plugin.json / CHANGELOG.md changes are metadata/version-bump only.

No GitHub Actions workflows, auth logic, credential handling, or network-facing input parsing are touched by this PR — skip-gate condition 2 (no security-relevant surface) applies.

[github-actions]

Last security-reviewed head: 22fb51108b7d9c54a47521e9c324e9c78d05e791. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

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

---

Code review complete

• Gather context (PR diff, changed files, CLAUDE.md)
• Run skip-gate checks
• Review detect.sh changes (rename-split gate, \\ unescape)
• Review detect.test.sh new cases
• Review CHANGELOG / plugin.json version bump
• Post findings

No blocking findings.

What I checked

• Rename-split gate (detect.sh#L128-L132): [[ "${line:0:1}" == [RC] || "${line:1:1}" == [RC] ]] correctly gates the " -> " split on the porcelain status letter rather than the path text. Traced through the untracked/plain, ordinary-modified-with-literal-" -> ", and genuine-rename cases by hand against the porcelain format — each resolves the way the PR describes.
• \\ unescape (detect.sh#L138-L141): ${local_path//\\\\/\\} is the same idiom already shipped in the sibling code-tidying/audit-comment-residue fix (commit 9a6c649, now on main and merged into this branch), applied in the same order (\" before \\). That sibling fix landed with its own passing regression suite, which gives strong independent corroboration that this exact construct behaves as intended — I did not have a working Bash execution path in this sandbox (script/test-suite invocations were denied approval) to run detect.test.sh myself, so I'm not claiming to have independently executed it; the "66 checks pass" and discriminator-revert results in the PR body are author-claimed, not something I verified by running the suite.
• Test coverage: the three new cases (literal " -> " in an untracked path, a genuine rename still resolving to the new path, and a backslash path) target the two defect classes precisely, and the backslash case is defensively skipped when the filesystem rejects the character. The unset GIT_DIR GIT_WORK_TREE GIT_CONFIG isolation line matches the convention from Drain scripts/fixture-git-isolation-baseline.txt (36 grandfathered suites) #2872.
• CHANGELOG.md / plugin.json: version bump (0.18.3 → 0.18.4) and changelog entry are consistent with the fix.
• Scope claim on SKILL.md: confirmed audit-noise/SKILL.md line 16 is a plain grep '\.md$' pipeline unrelated to the rename/quote parsing in detect.sh — the "mirrored parser" concern genuinely doesn't apply here, unlike the code-tidying sibling where the SKILL.md pre-computed-context line carried the identical parse and needed the same fix.

One non-blocking note: the PR body's "Related" section says #3140 has not landed, but that commit (9a6c649) is now present on this branch via the origin/main merge (22fb511) — purely a description-accuracy point, not a code issue, and doesn't affect the diff's correctness.

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[kyle-sexton]

Thanks — no blocking findings on either lane, and the hand-trace of the three porcelain cases matches what the suite asserts.

Two points in the review read a stale snapshot of this PR and are already resolved on the current head (22fb5110):

• "the PR body's Related section says #3140 has not landed" — it no longer does. That claim was true when the PR was opened; fix(code-tidying): parse porcelain by slicing so spaced paths survive discovery #3140 landed mid-flight, and the body was rewritten to say so ("fix(code-tidying): parse porcelain by slicing so spaced paths survive discovery #3140 landed on main while this branch was in flight… the gate and the two-step unescape here are identical") with the Related entry updated to match.
• "version bump (0.18.3 → 0.18.4)" — that was the pre-merge state. feat(docs-hygiene): bucket extract-ssot candidates by multiplicity, gate artifact creation not reporting (0.19.0) #3142 took docs-hygiene to 0.19.0 while this branch was open, so the merge resolved to 0.19.1; plugin.json and the changelog heading at this head both read 0.19.1.

On the execution caveat: correct that you couldn't run the suite, and treating "66 checks pass" as author-claimed is the right posture. For the record, the load-bearing evidence isn't the pass count — it's the discriminator: reverting only detect.sh to the old parse fails exactly the two new defect cases (' -> ' path, backslash path) and nothing else. The rename cases pass under both implementations by design, so they're regression guards rather than evidence the fix works.

---

Generated by Claude Code

[kyle-sexton]

No description provided.