ci(guard): run the guard corpus on ubuntu, windows and macos - #8
Merged
Merged
Conversation
A platform-specific crash in guard.py fails open silently: the Windows path bug disabled every path rule there and surfaced only when someone ran the hook on Windows. Run the corpus on ubuntu, windows and macos on every push and PR, and assert the Claude and Codex copies stay identical. Also deny rm -r of a UNC share root (//server/share), which the new drive-letter check did not match; paths inside a share stay allowed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #7.
Why
A crash inside
evaluate()makesmain()fail open (except Exception -> exit(0)), by design — failing closed would stall an unattended session. The cost showed up in #7: onere.PatternErroron Windows disabled every path rule on that platform, silently, until someone happened to run the hook there.That is a testing gap, not a rule gap. #7 already fixed the half that could be fixed in the corpus (pinning a POSIX host, simulating Windows). This adds the other half: actually running it on the platforms the hook is installed on.
What
.github/workflows/guard.yml— the corpus runs onubuntu-latest,windows-latestandmacos-lateston every push and PR (fail-fast: false, so one red leg does not hide the others). Plainpythonviaactions/setup-python, nouv: the corpus is pure stdlib, so CI does not need the extra link..claude/hooks/guard.pyand.codex/hooks/guard.pystay byte-identical. They are today, but only by the author's discipline.rm -rof a UNC share root (//server/share,\\server\share) is now denied asnetwork share. The drive-letter check added in fix(guard): normalize Windows paths so path rules work on Windows #7 does not match a//path, so it fell through to the POSIX ladder where no rule covers it. Paths inside a share (//server/share/build) stay allowed, so working off a network drive is unaffected.Verification
OK: 204 cases passed(201 from fix(guard): normalize Windows paths so path rules work on Windows #7 + 3 UNC cases).ALLOW -> DENY. The other 37 are identical. On POSIX, zero verdicts changed.main()on Linux (rm -rf ~,rm -rf /,git push --force, reading~/.ssh/id_rsa, archiving~/.ssh,disableAllHookswrite, ...): stderr and exit codes byte-identical to master.Not covered
/cygdrive/c/...is not normalized (the docs point at Git Bash, which uses/c/);to_nativehandsopen()a lowercased path, so on a case-sensitive-flagged NTFS directory a.pemholding a key reads as non-secret;rm -rf C:is denied, but by the home-parent rule rather than the drive rule, so the reason string is wrong; PowerShellRemove-Item -Recurseis not matched by any rm rule.