Skip to content

Commit 9c8c059

Browse files
authored
fix(hooks): examine sensitive files reached via Bash; scope fixture exceptions (S4) (#893)
check_sensitive_file ran only in the Write/Edit/Read tool branches, so the identical acts spelled as a shell command were never examined at all. Verified allowed before this change: `cat ~/.ssh/id_ed25519`, `cp ~/.env /var/www/html/`, `cat > ~/.env`, while the Write-tool equivalents were denied. Bash commands are now scanned per segment. The posture mirrors the tool branches rather than inventing a stricter one for Bash: copy/write verbs (cp, mv, scp, rsync, tee, dd, install, rclone) and output-redirect targets DENY, because they duplicate a secret to a new location or overwrite a credential file, which is exactly what Write/Edit deny; read verbs (cat, less, head, base64, ...) WARN only, matching the Read tool and the reason its docstring gives -- agents routinely read a project's own .env while debugging and blocking that breaks common legitimate work. _SENSITIVE_EXCEPTIONS matched `/fixtures/` anywhere in a path, so a real credential file at /home/feedgen/fixtures/.env was excused from every check. Directory exceptions (fixtures, testdata, __fixtures__, test_data) now apply only when the path resolves inside the repo worktree; suffix exceptions (.env.example and friends) still apply anywhere. Three existing tests asserted the unscoped behavior using out-of-repo paths; they now use in-repo paths and gained paired out-of-repo deny rows. Adds the credential stores the home CLAUDE.md names but the pattern list missed: .git-credentials, .netrc, ~/.config/gh/hosts.yml, .envrc. This extends the list S1 modified rather than replacing it. Adds 44 table-driven rows: every closed bypass, warn-vs-deny posture rows asserting the advisory actually fires, exception-scoping rows in both directions, and false-positive rows for ordinary work (cat README.md, cp README.md /tmp/r, echo hi > /tmp/out.txt, grep -r token .).
1 parent 9d02695 commit 9c8c059

3 files changed

Lines changed: 378 additions & 9 deletions

File tree

hooks/pretool-unified-gate.py

Lines changed: 179 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,12 +365,27 @@ def _force_push_protected(command: str) -> str | None:
365365
# THIS gate's dangerous/sensitive checks — writing one IS the disarm act, and
366366
# no agent flow legitimately writes them (owner-authored config only).
367367
(re.compile(r"/\.guard-(?:whitelist|patterns)$"), "guard-config", ".guard-* guard config"),
368+
# Credential stores the home CLAUDE.md names but the list above missed
369+
# (audit S4). Each holds a live credential in plaintext.
370+
(re.compile(r"/\.git-credentials$"), "credentials", ".git-credentials"),
371+
(re.compile(r"/\.netrc$"), "credentials", ".netrc"),
372+
(re.compile(r"/\.config/gh/hosts\.yml$"), "token", "gh CLI hosts.yml (auth token)"),
373+
(re.compile(r"/\.envrc$"), "env", ".envrc (direnv)"),
368374
]
369375

376+
# Suffix exceptions: safe ANYWHERE. These name a file that by convention holds
377+
# placeholders, not secrets, so their location does not change the risk.
370378
_SENSITIVE_EXCEPTIONS: list[re.Pattern[str]] = [
371379
re.compile(r"\.env\.example$"),
372380
re.compile(r"\.env\.sample$"),
373381
re.compile(r"\.env\.template$"),
382+
]
383+
384+
# Directory exceptions: safe ONLY inside the repo worktree (audit S4). These say
385+
# "this is test data", which is a claim about a project's own tree. Matched
386+
# anywhere, `/fixtures/` excused `/home/feedgen/fixtures/.env` — a real
387+
# credential file in the home directory — from every sensitive-file check.
388+
_SENSITIVE_DIR_EXCEPTIONS: list[re.Pattern[str]] = [
374389
re.compile(r"/testdata/"),
375390
re.compile(r"/fixtures/"),
376391
re.compile(r"/__fixtures__/"),
@@ -943,9 +958,43 @@ def _load_guard_patterns() -> list[tuple[re.Pattern[str], str, str]]:
943958
return extra
944959

945960

961+
def _repo_worktree_root() -> Path | None:
962+
"""Nearest ancestor of cwd containing `.git`, or None outside a repo.
963+
964+
Walks the filesystem instead of shelling out to git: this runs on every
965+
sensitive-file check and the hook has a hard latency budget.
966+
"""
967+
try:
968+
here = Path.cwd().resolve()
969+
except OSError:
970+
return None
971+
for candidate in (here, *here.parents):
972+
if (candidate / ".git").exists():
973+
return candidate
974+
return None
975+
976+
946977
def _is_sensitive_exception(file_path: str) -> bool:
947-
"""Check if file matches a sensitive-file exception pattern."""
948-
return any(p.search(file_path) for p in _SENSITIVE_EXCEPTIONS)
978+
"""Check if `file_path` matches a sensitive-file exception.
979+
980+
Suffix exceptions (`.env.example` and friends) apply anywhere. Directory
981+
exceptions (`/fixtures/`, `/testdata/`, …) apply ONLY inside the repo
982+
worktree — a "this is test data" claim is only meaningful about a
983+
project's own tree, and honoring it anywhere excused real credential
984+
files elsewhere on the box (audit S4).
985+
"""
986+
if any(p.search(file_path) for p in _SENSITIVE_EXCEPTIONS):
987+
return True
988+
if not any(p.search(file_path) for p in _SENSITIVE_DIR_EXCEPTIONS):
989+
return False
990+
root = _repo_worktree_root()
991+
if root is None:
992+
return False
993+
try:
994+
resolved = Path(file_path).expanduser().resolve()
995+
except (OSError, RuntimeError):
996+
return False
997+
return resolved == root or root in resolved.parents
949998

950999

9511000
def _block(message: str, tool_name: str = "", reason: str = "") -> None:
@@ -1332,6 +1381,133 @@ def check_sensitive_file(file_path: str, *, deny: bool = True) -> None:
13321381
)
13331382

13341383

1384+
# ── sensitive files reached via Bash (audit S4) ──────────────────
1385+
#
1386+
# `check_sensitive_file` ran only in the Write/Edit/Read tool branches, so the
1387+
# identical acts spelled as a shell command were never examined at all:
1388+
# `cat ~/.ssh/id_ed25519`, `cp ~/.env /var/www/html/`, and `cat > ~/.env` all
1389+
# passed while the Write-tool equivalents were denied.
1390+
#
1391+
# Posture deliberately mirrors the tool branches rather than inventing a
1392+
# stricter one for Bash:
1393+
# * COPY/WRITE verbs and redirect targets DENY — they duplicate a secret to a
1394+
# new location or mutate a credential file, which is what Write/Edit deny.
1395+
# * READ verbs WARN only — same as the Read tool, and for the same reason
1396+
# documented there: agents routinely read a project's own .env while
1397+
# debugging, and blocking that breaks common legitimate work.
1398+
1399+
# Verbs that duplicate or mutate a file → deny on a sensitive path in any
1400+
# argument position (source OR destination: copying a secret out is the
1401+
# exfiltration shape the home CLAUDE.md forbids).
1402+
_SENSITIVE_COPY_VERBS = frozenset({"cp", "mv", "scp", "rsync", "install", "tee", "dd", "rclone"})
1403+
1404+
# Verbs that only display a file → warn.
1405+
_SENSITIVE_READ_VERBS = frozenset(
1406+
{
1407+
"cat",
1408+
"bat",
1409+
"less",
1410+
"more",
1411+
"head",
1412+
"tail",
1413+
"od",
1414+
"xxd",
1415+
"hexdump",
1416+
"strings",
1417+
"base64",
1418+
"nl",
1419+
"cut",
1420+
"sort",
1421+
"uniq",
1422+
"wc",
1423+
}
1424+
)
1425+
1426+
# An output redirect target (`> path`, `>> path`, `2> path`). Writing here
1427+
# creates or truncates the named file, so it is a write regardless of verb.
1428+
_REDIRECT_TARGET_RE = re.compile(r"(?<!<)>>?\s*['\"]?([^\s'\"|;&<>]+)")
1429+
1430+
1431+
def _sensitive_bash_paths(segment: str) -> tuple[list[str], list[str]]:
1432+
"""Return (deny_paths, warn_paths) for one shell segment.
1433+
1434+
Redirect targets are always write paths. Otherwise the segment's command
1435+
token picks the posture; a segment led by neither kind of verb yields
1436+
nothing (an unrecognized command's arguments are not assumed to be files).
1437+
"""
1438+
deny_paths = [m.group(1) for m in _REDIRECT_TARGET_RE.finditer(segment)]
1439+
1440+
verb = _command_token(segment)
1441+
if verb not in _SENSITIVE_COPY_VERBS and verb not in _SENSITIVE_READ_VERBS:
1442+
return deny_paths, []
1443+
1444+
try:
1445+
toks = shlex.split(_strip_leading_prefixes(segment), posix=True)
1446+
except ValueError:
1447+
toks = _strip_leading_prefixes(segment).split()
1448+
args = [t for t in toks[1:] if not t.startswith("-")]
1449+
if verb in _SENSITIVE_COPY_VERBS:
1450+
return deny_paths + args, []
1451+
return deny_paths, args
1452+
1453+
1454+
def _matches_sensitive(path: str) -> tuple[str, str] | None:
1455+
"""Return (category, description) if `path` is sensitive, else None.
1456+
1457+
Normalizes `~` so `~/.env` matches the same anchored patterns an absolute
1458+
path does — the patterns are rooted at `/`, so an un-expanded `~` never
1459+
matched anything.
1460+
"""
1461+
expanded = os.path.expanduser(path)
1462+
if not expanded.startswith("/"):
1463+
expanded = str(Path.cwd() / expanded)
1464+
if _is_sensitive_exception(expanded):
1465+
return None
1466+
for pattern, category, description in _SENSITIVE_PATTERNS + _load_guard_patterns():
1467+
if pattern.search(expanded):
1468+
return category, description
1469+
return None
1470+
1471+
1472+
def check_sensitive_bash(command: str) -> None:
1473+
"""Deny/warn when a Bash command reads, copies, or overwrites a secret."""
1474+
if os.environ.get(_SENSITIVE_BYPASS_ENV) == "1":
1475+
return
1476+
1477+
for line in _non_heredoc_lines(command):
1478+
for segment in _SEGMENT_SPLIT_RE.split(line):
1479+
if not segment.strip():
1480+
continue
1481+
deny_paths, warn_paths = _sensitive_bash_paths(segment)
1482+
for path in deny_paths:
1483+
hit = _matches_sensitive(path)
1484+
if hit:
1485+
category, description = hit
1486+
_block(
1487+
f"[sensitive-file-guard] BLOCKED: shell command writes or copies a sensitive file ({category})\n"
1488+
f"[sensitive-file-guard] Path: {path}\n"
1489+
f"[sensitive-file-guard] Pattern: {description}\n"
1490+
f"[sensitive-file-guard] Command: {command}",
1491+
reason=(
1492+
f"Shell command writes or copies a sensitive file ({category}: {description}). "
1493+
f"Path: {path}. Duplicating or overwriting a credential file needs explicit owner "
1494+
f"approval per CLAUDE.md."
1495+
),
1496+
)
1497+
for path in warn_paths:
1498+
hit = _matches_sensitive(path)
1499+
if hit:
1500+
category, description = hit
1501+
print(
1502+
f"[sensitive-file-guard] ADVISORY: shell command reads a sensitive file ({category})\n"
1503+
f"[sensitive-file-guard] Path: {path}\n"
1504+
f"[sensitive-file-guard] Pattern: {description}\n"
1505+
f"[sensitive-file-guard] Reading credential files needs owner approval "
1506+
f"(OWNER-APPROVED-SECRET-READ) per CLAUDE.md. Not blocked — flagged for review.",
1507+
file=sys.stderr,
1508+
)
1509+
1510+
13351511
# ═══════════════════════════════════════════════════════════════
13361512
# 5b. GUARD SELF-PROTECTION (audit S1)
13371513
# ═══════════════════════════════════════════════════════════════
@@ -2662,6 +2838,7 @@ def main() -> None:
26622838
check_dangerous_command(command)
26632839
check_public_dev_server(command)
26642840
check_sysadmin_security(command)
2841+
check_sensitive_bash(command)
26652842

26662843
elif tool == "Write":
26672844
file_path = tool_input.get("file_path", "")

hooks/tests/test_pretool_unified_gate.py

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -615,19 +615,33 @@ def test_env_template_exception_allowed(self):
615615
payload = _make_write_event("/project/.env.template")
616616
assert _run_main(payload) == 0
617617

618-
def test_testdata_exception_allowed(self):
619-
"""Files under /testdata/ are excepted."""
620-
payload = _make_write_event("/project/testdata/credentials.json")
618+
# Directory exceptions are scoped to the repo worktree (audit S4): a
619+
# "this is test data" claim is only meaningful about the project's own
620+
# tree. Matched anywhere, `/fixtures/` excused a real credential file
621+
# sitting in the home directory.
622+
623+
def test_testdata_exception_allowed_inside_repo(self):
624+
"""Files under an in-repo /testdata/ are excepted."""
625+
payload = _make_write_event(str(Path.cwd() / "testdata" / "credentials.json"))
621626
assert _run_main(payload) == 0
622627

623-
def test_fixtures_exception_allowed(self):
624-
payload = _make_write_event("/project/fixtures/credentials.json")
628+
def test_fixtures_exception_allowed_inside_repo(self):
629+
payload = _make_write_event(str(Path.cwd() / "fixtures" / "credentials.json"))
625630
assert _run_main(payload) == 0
626631

627-
def test_dunder_fixtures_exception_allowed(self):
628-
payload = _make_write_event("/project/__fixtures__/credentials.json")
632+
def test_dunder_fixtures_exception_allowed_inside_repo(self):
633+
payload = _make_write_event(str(Path.cwd() / "__fixtures__" / "credentials.json"))
629634
assert _run_main(payload) == 0
630635

636+
def test_fixtures_exception_denied_outside_repo(self):
637+
"""`/fixtures/` outside the worktree is not a test-data claim."""
638+
payload = _make_write_event("/home/feedgen/fixtures/.env")
639+
assert _run_main(payload) == 2
640+
641+
def test_testdata_exception_denied_outside_repo(self):
642+
payload = _make_write_event("/home/feedgen/testdata/credentials.json")
643+
assert _run_main(payload) == 2
644+
631645
def test_bypass_allows_sensitive(self):
632646
"""SENSITIVE_FILE_GUARD_BYPASS=1 allows writes to sensitive files."""
633647
payload = _make_write_event("/project/.env")

0 commit comments

Comments
 (0)