From acc9075b633b9ef668b8284f5c5626cb23521a6c Mon Sep 17 00:00:00 2001 From: DefaultPerson Date: Fri, 18 Sep 2026 12:00:31 +0000 Subject: [PATCH] ci(guard): run the guard corpus on all three platforms 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. --- .claude/hooks/guard.py | 16 ++++++++++------ .claude/hooks/test_guard.py | 3 +++ .codex/hooks/guard.py | 16 ++++++++++------ .github/workflows/guard.yml | 28 ++++++++++++++++++++++++++++ CHANGELOG.md | 2 ++ 5 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/guard.yml diff --git a/.claude/hooks/guard.py b/.claude/hooks/guard.py index bb2fb31..57210b2 100644 --- a/.claude/hooks/guard.py +++ b/.claude/hooks/guard.py @@ -360,9 +360,13 @@ def _is_protected_home_top(name: str) -> bool: or (IS_WINDOWS and name == 'appdata')) -def _windows_drive_verdict(parts: list[str]) -> str | None: - """Why a //... path is too broad for rm -r on Windows, else None.""" - if not (IS_WINDOWS and parts and len(parts[0]) == 1): +def _windows_drive_verdict(parts: list[str], base: str) -> str | None: + """Why a //... or //server/share path is too broad for rm -r on Windows.""" + if not (IS_WINDOWS and parts): + return None + if base.startswith('//'): # UNC \\server\share: only the share root is too broad + return 'network share' if len(parts) <= 2 else None + if len(parts[0]) != 1: return None if len(parts) == 1: return 'drive root' @@ -395,7 +399,7 @@ def classify_rm_target(t: str, cwd: str | None = None) -> tuple[int, str | None] return ALLOW, None parts = [x for x in base.split('/') if x] if (base == '/' or base == HOME or (parts and parts[0] in SYSTEM_TOP) - or _windows_drive_verdict(parts)): + or _windows_drive_verdict(parts, base)): return DENY, f'rm -r * in {base}: wildcard wipe of a sensitive directory' return ALLOW, None @@ -428,8 +432,8 @@ def classify_rm_target(t: str, cwd: str | None = None) -> tuple[int, str | None] parts = [x for x in base.split('/') if x] if not parts: return DENY, f'rm -r {t}: filesystem root' - if IS_WINDOWS and len(parts[0]) == 1: # //... - why = _windows_drive_verdict(parts) + if IS_WINDOWS and (len(parts[0]) == 1 or base.startswith('//')): # //... or UNC + why = _windows_drive_verdict(parts, base) return (DENY, f'rm -r {t}: {why}') if why else (ALLOW, None) top = parts[0] if top == 'tmp' or base.startswith('/var/tmp') or base.startswith('/dev/shm'): diff --git a/.claude/hooks/test_guard.py b/.claude/hooks/test_guard.py index 83039be..3609fb6 100644 --- a/.claude/hooks/test_guard.py +++ b/.claude/hooks/test_guard.py @@ -255,6 +255,9 @@ (D, 'Bash', {'command': 'rm -rf "/c/Program Files"'}), (D, 'Bash', {'command': 'rm -rf /c'}), (D, 'Bash', {'command': 'rm -rf C:/foo'}), + (D, 'Bash', {'command': 'rm -rf //fileserver/share'}), # UNC \\server\share + (D, 'Bash', {'command': 'rm -rf //fileserver'}), + (A, 'Bash', {'command': 'rm -rf //fileserver/share/build'}), (D, 'Bash', {'command': 'rm -rf ../../..'}), (D, 'Bash', {'command': 'find ~ -mindepth 1 -delete'}), (A, 'Bash', {'command': 'rm -rf ./build'}), diff --git a/.codex/hooks/guard.py b/.codex/hooks/guard.py index bb2fb31..57210b2 100644 --- a/.codex/hooks/guard.py +++ b/.codex/hooks/guard.py @@ -360,9 +360,13 @@ def _is_protected_home_top(name: str) -> bool: or (IS_WINDOWS and name == 'appdata')) -def _windows_drive_verdict(parts: list[str]) -> str | None: - """Why a //... path is too broad for rm -r on Windows, else None.""" - if not (IS_WINDOWS and parts and len(parts[0]) == 1): +def _windows_drive_verdict(parts: list[str], base: str) -> str | None: + """Why a //... or //server/share path is too broad for rm -r on Windows.""" + if not (IS_WINDOWS and parts): + return None + if base.startswith('//'): # UNC \\server\share: only the share root is too broad + return 'network share' if len(parts) <= 2 else None + if len(parts[0]) != 1: return None if len(parts) == 1: return 'drive root' @@ -395,7 +399,7 @@ def classify_rm_target(t: str, cwd: str | None = None) -> tuple[int, str | None] return ALLOW, None parts = [x for x in base.split('/') if x] if (base == '/' or base == HOME or (parts and parts[0] in SYSTEM_TOP) - or _windows_drive_verdict(parts)): + or _windows_drive_verdict(parts, base)): return DENY, f'rm -r * in {base}: wildcard wipe of a sensitive directory' return ALLOW, None @@ -428,8 +432,8 @@ def classify_rm_target(t: str, cwd: str | None = None) -> tuple[int, str | None] parts = [x for x in base.split('/') if x] if not parts: return DENY, f'rm -r {t}: filesystem root' - if IS_WINDOWS and len(parts[0]) == 1: # //... - why = _windows_drive_verdict(parts) + if IS_WINDOWS and (len(parts[0]) == 1 or base.startswith('//')): # //... or UNC + why = _windows_drive_verdict(parts, base) return (DENY, f'rm -r {t}: {why}') if why else (ALLOW, None) top = parts[0] if top == 'tmp' or base.startswith('/var/tmp') or base.startswith('/dev/shm'): diff --git a/.github/workflows/guard.yml b/.github/workflows/guard.yml new file mode 100644 index 0000000..57dd258 --- /dev/null +++ b/.github/workflows/guard.yml @@ -0,0 +1,28 @@ +name: guard + +on: + push: + branches: [master] + pull_request: + +jobs: + # The guard's path rules are platform-specific, so the corpus has to run on + # every platform the hook is installed on — a crash there fails open silently. + test: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-python@v7 + with: + python-version: '3.12' + - run: python .claude/hooks/test_guard.py + + sync: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - run: diff .claude/hooks/guard.py .codex/hooks/guard.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ed2c8c9..857f5c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Added +- **.github/workflows/guard.yml** — the guard corpus runs on ubuntu, windows and macos on every push and PR, plus a job asserting `.claude/hooks/guard.py` and `.codex/hooks/guard.py` are byte-identical. A platform-specific crash in the guard fails open silently, so it has to be caught by CI rather than by whoever happens to run that platform. - **guard.py** — `pkill -f` / `pgrep -f` self-match rule: a full-match pattern (`-f`, `--full`, `-ef`, `-9f`, also after `sudo`/`timeout`, inside `bash -c`, `$(…)`, backticks, pipes and `ssh host '…'`) that matches the command line it is part of is denied, because the Bash tool's own `bash -c ""` shell is in that pattern's way and dies with it (exit 144); `pgrep` is denied only when its output feeds a `kill`, and `[p]attern` or anchored patterns stay allowed. - **guard.py** — `GUARD_PROTECTED_UNITS` (comma/whitespace separated unit names, with or without `.service`): `systemctl stop|restart|disable|kill|mask|try-restart|reload-or-restart` on a listed unit is denied, while `status`, `start`, `show`, `cat`, `list-units` and `daemon-reload` stay allowed. - **.claude/skills/repo-context** — compact repository snapshot collected with `!` injections before the model runs; replaces `/prime`. @@ -57,6 +58,7 @@ All notable changes to this project will be documented in this file. ### Fixed +- **guard.py** — `rm -r` of a UNC share root on Windows (`//server/share`, `\\server\share`) was allowed: the drive-letter check did not match a `//` path, so it fell through to the POSIX ladder and no rule covered it. It is now denied as `network share`, the Windows counterpart of the existing `/mnt`, `/media` rule; paths inside a share (`//server/share/build`) stay allowed. - **statusline.py** — rate-limit segment disappeared entirely when the 5-hour window was absent (Claude Code drops a window once it resets, e.g. while idle); a missing window now renders as `-` (`-/76% (-/48h)`). - **guard.py** — path rules on Windows. `HOME` (`C:\Users\x`) was passed to `re.sub` as a replacement template, so every path check raised `bad escape \U` and the hook failed open: `rm -rf /`, reading `~/.ssh` keys and unhooking guard.py from settings were all allowed. Past that crash, `os.path.normpath` produced backslash paths the POSIX rules never matched, so every `rm -r` (even `./build`) was denied as a top-level directory, while `C:\…` tool paths skipped the `~/.ssh` directory and settings checks. Paths are now normalized to one lowercase form before any check (`C:\x`, `C:/x` and Git Bash `/c/x` all become `/c/x`), `$USERPROFILE` expands like `$HOME`, and on Windows `rm -r` of a drive root, `Windows`, `Program Files`, `ProgramData` or another profile under `Users` is denied and `~/AppData` is protected. POSIX behavior is unchanged. `test_guard.py` pins a `/home/def` POSIX host so the corpus gives the same verdicts on any machine, and adds 28 simulated Windows cases.