diff --git a/tools/product_guardrail/canon_payload.py b/tools/product_guardrail/canon_payload.py index af49544..ea9d9e8 100644 --- a/tools/product_guardrail/canon_payload.py +++ b/tools/product_guardrail/canon_payload.py @@ -12,7 +12,7 @@ Emitted 2026-09-06. """ -CANON_ID = 'ca94e2b86495d009148024de6014f2a37c8076218f26ff501c4b7e4bd4808b4c' +CANON_ID = '785922bbdc3f88d382141b7728d2b8a561fe38b7bf8bf5ff23b76a8abf4c9993' # Approved names. Exact casing is part of the name. `near` matches the # near-misses case-insensitively; anything it catches that is not the target diff --git a/tools/product_guardrail/hook_posttooluse.py b/tools/product_guardrail/hook_posttooluse.py index 1df82af..85b4fe3 100644 --- a/tools/product_guardrail/hook_posttooluse.py +++ b/tools/product_guardrail/hook_posttooluse.py @@ -58,6 +58,7 @@ import ast import datetime as dt +import fcntl import hashlib import json import os @@ -133,6 +134,33 @@ def _append_grace(profile_path, entries): profile has been restructured; say so and change nothing rather than guessing where a list starts. """ + # One writer at a time. This is a read-modify-write of a file the checker + # also reads, and two edits in flight could interleave and lose a waiver or + # collide on the temp file. An exclusive lock beside the profile is enough: + # the only writers are these hooks on one machine. + lock = profile_path.with_suffix(".py.lock") + lock_fd = None + try: + lock_fd = os.open(str(lock), os.O_CREAT | os.O_RDWR) + fcntl.flock(lock_fd, fcntl.LOCK_EX) + except OSError: + if lock_fd is not None: + os.close(lock_fd) + lock_fd = None # locking is best-effort; never block the edit + + try: + return _append_grace_locked(profile_path, entries) + finally: + if lock_fd is not None: + try: + fcntl.flock(lock_fd, fcntl.LOCK_UN) + os.close(lock_fd) + lock.unlink(missing_ok=True) + except OSError: + pass + + +def _append_grace_locked(profile_path, entries): src = profile_path.read_text(encoding="utf-8") anchor = "GRACE = [" if src.count(anchor) != 1: @@ -157,7 +185,8 @@ def _append_grace(profile_path, entries): import importlib.util as _ilu _spec = _ilu.spec_from_file_location("_pg_check_validate", tmp_check) _chk = _ilu.module_from_spec(_spec); _spec.loader.exec_module(_chk) - _probe = profile_path.with_suffix(".py.probe") + # Unique per process: a shared name is a collision between two hooks. + _probe = profile_path.with_suffix(f".py.{os.getpid()}.probe") _probe.write_text(updated, encoding="utf-8") try: _chk._load_data(_probe) @@ -167,7 +196,7 @@ def _append_grace(profile_path, entries): # open, so an encoding error mid-write left profile.py at ZERO BYTES — # reproduced under a latin-1 locale, where the em dash in the placeholder # raised and the hook then swallowed the error and exited 0. - tmp = profile_path.with_suffix(".py.tmp") + tmp = profile_path.with_suffix(f".py.{os.getpid()}.tmp") tmp.write_text(updated, encoding="utf-8") os.replace(tmp, profile_path) return True @@ -204,7 +233,15 @@ def main(): # edit to the same file consumed it and recorded a grace entry for the # finding the owner had just refused. Reproduced end to end, and again # with a note aged thirty days. - if time.time() - float(note.get("at", 0)) > PENDING_TTL: + # The TTL exists so a DENIED note cannot be consumed by a later edit. + # It must not discard an APPROVAL: the owner may leave the dialog open + # for a while, and silently losing their answer is worse than the stale + # note it was written to prevent. The content hash below already proves + # this is the very edit that was asked about — that is the real guard, + # and it does not expire. An aged note without a content hash (only + # possible for a note written before this change) is still dropped. + aged = time.time() - float(note.get("at", 0)) > PENDING_TTL + if aged and not note.get("content_sha"): return 0 proposed = note.get("content_sha") if proposed: diff --git a/tools/product_guardrail/install-hooks.sh b/tools/product_guardrail/install-hooks.sh index 37050b7..2b67701 100755 --- a/tools/product_guardrail/install-hooks.sh +++ b/tools/product_guardrail/install-hooks.sh @@ -13,7 +13,15 @@ # git push --no-verify set -e cd "$(git rev-parse --show-toplevel)" -HOOK=.git/hooks/pre-push + +# The REAL hooks directory. `.git/hooks` is wrong in a git worktree, where +# `.git` is a FILE pointing elsewhere — the redirection failed with "Not a +# directory" and no hook was installed, silently, in exactly the checkouts this +# team works in. `--git-path hooks` resolves it correctly everywhere, and +# honours core.hooksPath if the repo sets one. +HOOKS_DIR="$(git rev-parse --git-path hooks)" +mkdir -p "$HOOKS_DIR" +HOOK="$HOOKS_DIR/pre-push" LINE='python3 tools/product_guardrail/check.py || exit 1' # NEVER clobber an existing hook. This repo already ships a pre-push hook that