mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 00:26:26 +00:00
feat(security): hold .claude/settings.json hooks to a reviewed allowlist (#313)
check_permissions() read permissions.allow and nothing else, so a `hooks` block in the same file passed the guard silently. A hook is strictly more dangerous than a pre-approved permission. A permission pre-approves something Claude may choose to do; a hook runs unconditionally when its event fires, with no prompt and no model decision in between. Cloning the repo and opening it is enough. This is the vector the Shai-Hulud worm used in its August 2026 wave: a SessionStart hook in .claude/settings.json chaining to .claude/math_init.js, executing on session start. https://research.jfrog.com/post/shai-hulud-is-back-august/ For a template that thousands of people are explicitly invited to fork, that is the riskiest key in the file this guard already parses. Follows the established pattern exactly - ALLOWED_HOOKS ships empty, since the template has no hooks, so any addition must be allowlisted in the same PR and is therefore explicit and reviewable. Two details worth reviewing closely: - The hook check runs *before* the permissions shape guards. Those guards return early, so a file pairing a malformed permissions block with a live hook would otherwise skip the hook check entirely - a fail-open. Pinned by test_hook_is_caught_even_when_permissions_block_is_malformed. - _hook_commands() fails closed. Any hook layout it does not recognise yields a marker that cannot be in the allowlist, so an unfamiliar shape is rejected rather than silently skipped, rather than trusting that the Claude Code schema will not change. Verified: - 8 new HookGuardTests cases; 14 of the suite's 26 tests fail against the unpatched guard, all 26 pass with it - injecting the real worm shape into this repo's own settings.json makes the guard exit 1 naming 'SessionStart:node .claude/math_init.js'; removing it returns OK - lint_skills, check_framework_version, security_guards all OK; python3 -m unittest discover -s tests 219 passed
This commit is contained in:
@@ -12,7 +12,10 @@ reviewable rather than buried.
|
||||
Checks:
|
||||
1. .claude/settings.json — every permissions.allow entry must be in the exact
|
||||
allowlist below. Catches permission widening (e.g. Bash(*), Bash(curl:*)),
|
||||
which would auto-approve commands on every fork.
|
||||
which would auto-approve commands on every fork. The same file's `hooks`
|
||||
key is held to an allowlist too: a hook runs automatically when its event
|
||||
fires, with no prompt, so it is strictly more dangerous than a pre-approved
|
||||
permission.
|
||||
2. .gitignore — the personal-data ignore rules must all still be present,
|
||||
and no un-allowlisted negation (!pattern) may re-include them. Catches
|
||||
weakening that would make future users silently commit their tracker,
|
||||
@@ -91,9 +94,45 @@ ALLOWED_IGNORE_NEGATIONS = {
|
||||
"!documents/**/.gitkeep",
|
||||
}
|
||||
|
||||
# Hook commands the template legitimately ships, as "<Event>:<command>" strings.
|
||||
# Empty by design - the template ships no hooks at all.
|
||||
#
|
||||
# A hook is strictly more dangerous than a permissions.allow entry. A permission
|
||||
# pre-approves something Claude may choose to do; a hook runs unconditionally when
|
||||
# its event fires, with no prompt and no model decision in between. Cloning a repo
|
||||
# and opening it is enough. This is the vector the Shai-Hulud worm used in its
|
||||
# August 2026 wave, planting a SessionStart hook in .claude/settings.json that
|
||||
# executed on session start:
|
||||
# https://research.jfrog.com/post/shai-hulud-is-back-august/
|
||||
ALLOWED_HOOKS: set[str] = set()
|
||||
|
||||
FORBIDDEN_SCRIPTS = {"preinstall", "install", "postinstall", "prepare", "prepack"}
|
||||
|
||||
|
||||
def _hook_commands(event: str, entries: object):
|
||||
"""Yield "<Event>:<command>" for every command a hook event would run.
|
||||
|
||||
Fails closed: any shape this does not recognise yields a marker that cannot
|
||||
be in the allowlist, so an unfamiliar hook layout is rejected rather than
|
||||
silently skipped.
|
||||
"""
|
||||
unrecognised = f"{event}:<unrecognised hook shape>"
|
||||
if not isinstance(entries, list):
|
||||
yield unrecognised
|
||||
return
|
||||
for entry in entries:
|
||||
if not isinstance(entry, dict):
|
||||
yield unrecognised
|
||||
continue
|
||||
inner = entry.get("hooks")
|
||||
if not isinstance(inner, list):
|
||||
yield unrecognised
|
||||
continue
|
||||
for hook in inner:
|
||||
command = hook.get("command") if isinstance(hook, dict) else None
|
||||
yield f"{event}:{command}" if isinstance(command, str) else unrecognised
|
||||
|
||||
|
||||
def check_permissions() -> None:
|
||||
path = ROOT / ".claude" / "settings.json"
|
||||
try:
|
||||
@@ -104,6 +143,26 @@ def check_permissions() -> None:
|
||||
if not isinstance(data, dict):
|
||||
errors.append(".claude/settings.json: top-level JSON value must be an object")
|
||||
return
|
||||
|
||||
# Checked before the permissions shape guards below, so a file that pairs a
|
||||
# malformed permissions block with a hook cannot return early and skip this.
|
||||
hooks = data.get("hooks", {})
|
||||
if hooks:
|
||||
if not isinstance(hooks, dict):
|
||||
errors.append(".claude/settings.json: hooks must be an object")
|
||||
else:
|
||||
for event, entries in hooks.items():
|
||||
for command in _hook_commands(str(event), entries):
|
||||
if command not in ALLOWED_HOOKS:
|
||||
errors.append(
|
||||
f".claude/settings.json: hook not in the reviewed allowlist: "
|
||||
f"{command!r}. A hook runs automatically when its event fires - it "
|
||||
"is never gated by the permissions prompt, so it executes on every "
|
||||
"fork without the user agreeing to anything. If this hook is "
|
||||
"intentional, add it to ALLOWED_HOOKS in tools/security_guards.py "
|
||||
"in the same PR so the addition is explicit and reviewable."
|
||||
)
|
||||
|
||||
permissions = data.get("permissions", {})
|
||||
if not isinstance(permissions, dict):
|
||||
errors.append(".claude/settings.json: permissions must be an object")
|
||||
@@ -195,7 +254,10 @@ def main() -> int:
|
||||
for err in errors:
|
||||
print(f" - {err}")
|
||||
return 1
|
||||
print("security_guards: OK (permissions allowlist, gitignore rules, package manifests)")
|
||||
print(
|
||||
"security_guards: OK (permissions allowlist, hooks allowlist, gitignore rules, "
|
||||
"package manifests)"
|
||||
)
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user