surshar / claude-code
Claude Code is an agentic coding tool that lives in your terminal, understands your codebase, and helps you code faster by executing routine tasks, explaining complex code, and handling git workflows - all through natural language commands.
Built by Anthropic · github.com/anthropics
The review diff was built straight from git and the reviewer's SDK sub-agents ran with no settings, so files a session's Read deny rules protect (and plain secret stores like .env or secrets.yaml) could end up in a review prompt, and the agentic reviewer could Read them — or `git show` them through the auto-approved shell — verbatim (#96276). Read/Grep/Glob deny and ask rules are now loaded from the same managed, user, project and local settings files the session uses, applied to every review diff, and forwarded to the sub-agents as disallowed_tools together with a shell-tool ban; well-known secret files are skipped by default (SG_SKIP_SECRET_FILES=0 opts out). Settings are still not loaded into the sub-agent, so plugins and hooks do not recurse.
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | { |
| 2 | 2 | "name": "security-guidance", |
| 3 | - "version": "2.0.0", | |
| 3 | + "version": "2.1.0", | |
| 4 | 4 | "description": "Security review for Claude-generated code. Pattern-based warnings on edits, LLM-powered diff review on Stop, and an agentic commit reviewer that catches injection, XSS, SSRF, hardcoded secrets, and 25+ other vulnerability classes.", |
| 5 | 5 | "author": { |
| 6 | 6 | "name": "David Dworken", |
| @@ -50,6 +50,7 @@ | ||
| 50 | 50 | | `ENABLE_CODE_SECURITY_REVIEW=0` | on | Disable all LLM reviews (Stop hook + commit/push) | |
| 51 | 51 | | `ENABLE_STOP_REVIEW=0` | on | Disable only the Stop-hook diff review, keeping commit/push reviews. Useful for multi-agent / shared-worktree setups where another agent can move HEAD between a worker's turns | |
| 52 | 52 | | `ENABLE_COMMIT_REVIEW=0` | on | Disable layer 3 (agentic commit review) | |
| 53 | +| `SG_SKIP_SECRET_FILES=0` | on | Stop skipping well-known secret files (`.env*`, private keys, `secrets.yaml`, `credentials.json`, `.netrc`, …) so they are reviewed like any other file. Files your `Read` deny rules cover stay excluded either way — see [Privacy and data handling](#privacy-and-data-handling) | | |
| 53 | 54 | |
| 54 | 55 | ### Higher-recall mode |
| 55 | 56 | |
| @@ -86,6 +87,13 @@ | ||
| 86 | 87 | |
| 87 | 88 | The plugin sends data to a model endpoint to perform its reviews. Specifically, each Stop-hook diff review transmits the changed file paths, the diff hunks, and the relevant file contents in the diff; each agentic commit review additionally transmits any files the reviewer pulls in via `Read`/`Grep`/`Glob` while tracing data flow. Your `claude-security-guidance.md` contents (user, project, and local) are appended to the prompt on every review, so don't put secrets in it. |
| 88 | 89 | |
| 90 | +Two kinds of file are kept out of every review, both from the diff the plugin sends and from what the agentic reviewer's sub-agent may `Read`/`Grep`/`Glob` (the sub-agent gets no shell tool, so `cat` or `git show` are not a way around this): | |
| 91 | + | |
| 92 | +- **Files your session's `Read` permission rules deny or ask about.** The plugin reads `permissions.deny` and `permissions.ask` from the same settings files Claude Code loads — managed (`managed-settings.json` and `managed-settings.d/`), user (`~/.claude/settings.json`), project (`.claude/settings.json`) and local (`.claude/settings.local.json`) — and applies the `Read(...)` rules with Claude Code's path conventions (`//abs`, `~/home`, `/project-relative`, gitignore-style globs). The sub-agent receives the same rules as `--disallowedTools`. Rules that exist only in MDM/registry policy, server-managed settings, or `--settings` / `--disallowedTools` command-line flags are not visible to a hook; put them in one of the settings files above if the reviewer must honor them. | |
| 93 | +- **Well-known secret files**, on by default: `.env` / `.env.*` / `*.env`, private keys and keystores (`*.pem`, `*.key`, `*.p12`, `*.jks`, `id_rsa*`, …), credential stores (`.netrc`, `.npmrc`, `.pypirc`, `.git-credentials`, `credentials`, `kubeconfig`) and `secret(s)` / `credential(s)` data files (`.json`, `.yaml`, `.toml`, …). Source files such as `secrets.py` are still reviewed. In the sub-agent these names follow permission-rule (gitignore) semantics, so a directory that shares one — `credentials/`, `.env/` — is off-limits to it as well. Set `SG_SKIP_SECRET_FILES=0` to review these too. | |
| 94 | + | |
| 95 | +The flip side: the reviewer cannot flag a problem inside a file it never sees, and a project's own `.claude/settings.json` decides what is hidden — the same trust Claude Code itself places in project settings. Each withheld path is named in the debug log (`secretpaths: withheld from review …`). | |
| 96 | + | |
| 89 | 97 | Where that data goes depends on your Claude Code configuration: |
| 90 | 98 | - **Default (Anthropic API / subscription):** sent to `api.anthropic.com` and handled under Anthropic's [Commercial Terms](https://www.anthropic.com/legal/commercial-terms) and [Privacy Policy](https://www.anthropic.com/legal/privacy). |
| 91 | 99 | - **LLM gateway** (`ANTHROPIC_BASE_URL` set): sent to your gateway URL instead. The gateway operator's terms apply. |
| @@ -20,6 +20,7 @@ | ||
| 20 | 20 | import subprocess |
| 21 | 21 | |
| 22 | 22 | from _base import debug_log |
| 23 | +import secretpaths | |
| 23 | 24 | |
| 24 | 25 | |
| 25 | 26 | GIT_CMD = [ |
| @@ -548,6 +549,10 @@ | ||
| 548 | 549 | |
| 549 | 550 | |
| 550 | 551 | def _is_reviewable_source(file_path): |
| 552 | + # The session's Read deny rules and well-known secret files come first: | |
| 553 | + # a denied `config/prod.json` has a reviewable extension. | |
| 554 | + if secretpaths.is_excluded(file_path): | |
| 555 | + return False | |
| 551 | 556 | # Normalize for component matching: a path like `.next/x.js` or |
| 552 | 557 | # `pkg/node_modules/y.ts` should both be excluded; matching against |
| 553 | 558 | # `'/' + path` lets each pattern be checked as `'/' + p in '/' + path` |
| @@ -27,6 +27,7 @@ | ||
| 27 | 27 | |
| 28 | 28 | import extensibility |
| 29 | 29 | import review_api |
| 30 | +import secretpaths | |
| 30 | 31 | from _base import debug_log, _record_usage, _PV, PROVENANCE_TAG # noqa: F401 |
| 31 | 32 | from session_state import with_locked_state |
| 32 | 33 | |
| @@ -276,8 +277,9 @@ | ||
| 276 | 277 | 3P providers. Uses the same `output_format` JSON-schema contract so the |
| 277 | 278 | return value shape is identical (parsed dict or None). |
| 278 | 279 | |
| 279 | - No tools (`allowed_tools=[]`) — the security review only needs structured | |
| 280 | - output, not Read/Grep/Glob. Single turn keeps cost predictable. | |
| 280 | + No tools — the security review only needs structured output, so the | |
| 281 | + file and shell tools are passed as `disallowed_tools`. Single turn keeps | |
| 282 | + cost predictable. | |
| 281 | 283 | """ |
| 282 | 284 | global _last_call_claude_http_error |
| 283 | 285 | _last_call_claude_http_error = None |
| @@ -330,6 +332,11 @@ | ||
| 330 | 332 | system_prompt=CLAUDE_CODE_SYSTEM_PROMPT, |
| 331 | 333 | cli_path=cli_path, |
| 332 | 334 | allowed_tools=[], |
| 335 | + # allowed_tools=[] grants nothing but removes nothing either: | |
| 336 | + # Read inside cwd and read-only shell commands are auto-approved | |
| 337 | + # in default mode. This call needs no tools, so take the file and | |
| 338 | + # shell tools away outright. | |
| 339 | + disallowed_tools=secretpaths.no_file_tools(), | |
| 333 | 340 | setting_sources=[], |
| 334 | 341 | max_turns=2, |
| 335 | 342 | model=chosen_model, |
| @@ -1126,6 +1133,8 @@ | ||
| 1126 | 1133 | # trace cross-file data flow. The harness sets SG_AGENTIC_CONTEXT_DIR to a |
| 1127 | 1134 | # full repo (worktree at the commit, or the live clone at HEAD). |
| 1128 | 1135 | context_dir = os.environ.get("SG_AGENTIC_CONTEXT_DIR") or repo_dir |
| 1136 | + disallowed = secretpaths.subagent_disallowed_tools(context_dir) | |
| 1137 | + debug_log(f"agentic_review: {len(disallowed)} disallowed Read pattern(s) for the sub-agent") | |
| 1129 | 1138 | context_note = "" |
| 1130 | 1139 | if context_dir != repo_dir: |
| 1131 | 1140 | context_note = ( |
| @@ -1219,6 +1228,13 @@ | ||
| 1219 | 1228 | # would trip our own agent-permission-bypass guidance). Leaving |
| 1220 | 1229 | # permission_mode unset means an accidental future addition of |
| 1221 | 1230 | # a write/exec tool to allowed_tools is caught by the gate. |
| 1231 | + # | |
| 1232 | + # setting_sources=[] keeps the parent's plugins and hooks (this | |
| 1233 | + # one included) from loading recursively, but it also drops the | |
| 1234 | + # parent's permission rules. The session's Read deny/ask rules | |
| 1235 | + # and the well-known secret-file globs are re-applied here so the | |
| 1236 | + # reviewer cannot pull a denied file into model context. | |
| 1237 | + disallowed_tools=disallowed, | |
| 1222 | 1238 | setting_sources=[], |
| 1223 | 1239 | max_turns=turns if turns is not None else max_turns, |
| 1224 | 1240 | model=model, |
| @@ -1,0 +1,371 @@ | ||
| 1 | +"""Paths the security reviewer must not see. | |
| 2 | + | |
| 3 | +Two sources, one answer: | |
| 4 | + | |
| 5 | +1. The session's own ``Read`` deny/ask permission rules, read from the same | |
| 6 | + settings files Claude Code loads (managed, user, project, project-local). | |
| 7 | + The main session enforces these, but the review prompt is assembled here | |
| 8 | + from ``git diff`` and the reviewer's SDK sub-agents run with | |
| 9 | + ``setting_sources=[]``, so neither knew about them. | |
| 10 | +2. A built-in list of well-known secret-file names (``.env``, private keys, | |
| 11 | + credential stores). On by default; ``SG_SKIP_SECRET_FILES=0`` opts out. | |
| 12 | + | |
| 13 | +``is_excluded()`` drops such files from every review diff (via | |
| 14 | +``gitutil._is_reviewable_source``) and ``subagent_disallowed_tools()`` hands | |
| 15 | +the same rules to the SDK sub-agents as ``disallowed_tools`` so they cannot | |
| 16 | +``Read``/``Grep``/``Glob`` them, nor reach them through a shell. | |
| 17 | + | |
| 18 | +Not covered: rules that only exist in MDM/registry policy, server-managed | |
| 19 | +settings, or the parent's ``--settings``/``--disallowedTools`` flags — a hook | |
| 20 | +subprocess cannot see those. The README says so. | |
| 21 | +""" | |
| 22 | + | |
| 23 | +import fnmatch | |
| 24 | +import json | |
| 25 | +import os | |
| 26 | +import re | |
| 27 | +import sys | |
| 28 | +from typing import List, Optional, Pattern, Tuple | |
| 29 | + | |
| 30 | +from _base import debug_log | |
| 31 | + | |
| 32 | +# Basename globs (case-insensitive) for files whose purpose is to hold | |
| 33 | +# credentials. Deliberately conservative: names that are secret stores by | |
| 34 | +# convention, not names that merely sound sensitive. Users extend this with | |
| 35 | +# ordinary `Read(...)` deny rules. | |
| 36 | +SECRET_BASENAME_GLOBS = ( | |
| 37 | + ".env", ".env.*", "*.env", ".envrc", | |
| 38 | + "*.pem", "*.key", "*.p12", "*.pfx", "*.jks", "*.keystore", "*.ppk", "*.kdbx", | |
| 39 | + "id_rsa*", "id_dsa*", "id_ecdsa*", "id_ed25519*", | |
| 40 | + ".netrc", "_netrc", ".pgpass", ".git-credentials", ".npmrc", ".pypirc", | |
| 41 | + ".htpasswd", "credentials", "kubeconfig", | |
| 42 | +) | |
| 43 | +# `secrets.yaml` / `credentials.json` style stores. Restricted to data | |
| 44 | +# extensions so `secrets.py` / `credentials.ts` (code that HANDLES secrets — | |
| 45 | +# exactly what the reviewer should read) stay reviewable. | |
| 46 | +_SECRET_STEMS = ("secret", "secrets", "credential", "credentials") | |
| 47 | +_DATA_EXTS = ( | |
| 48 | + ".json", ".yaml", ".yml", ".toml", ".ini", ".cfg", ".conf", ".env", | |
| 49 | + ".properties", ".xml", ".txt", | |
| 50 | +) | |
| 51 | + | |
| 52 | +# Shell tools reach file contents without consulting Read(...) rules — their | |
| 53 | +# read-only commands (`cat`, `git show HEAD:path`) are auto-approved — and the | |
| 54 | +# reviewer needs none of them. | |
| 55 | +_SHELL_TOOLS = ["Bash"] + (["PowerShell"] if sys.platform == "win32" else []) | |
| 56 | +_READ_TOOLS = ("Read", "Grep", "Glob") | |
| 57 | +_RULE_RE = re.compile(r"^\s*([A-Za-z_][\w-]*)\s*(?:\((.*)\))?\s*$", re.DOTALL) | |
| 58 | + | |
| 59 | +# Session state: load_for_session() only records the cwd; the settings files | |
| 60 | +# are read on first use so hook events that never review (per-edit pattern | |
| 61 | +# checks) pay nothing. | |
| 62 | +_cwd: Optional[str] = None | |
| 63 | +_loaded = False | |
| 64 | +# (kind, root, pattern, compiled). kind "cwd": root-less rule, relative to the | |
| 65 | +# session cwd (also tried against the repo toplevel, where diff paths live); | |
| 66 | +# "project": `/x`, anchored at `root`; "abs": `//x` or `~/x`, anchored at `root`. | |
| 67 | +_rules: List[Tuple[str, str, str, Optional[Pattern[str]]]] = [] | |
| 68 | +_forward: List[str] = [] # rule strings for the sub-agent, `/x` made absolute | |
| 69 | +_deny_all_reads = False | |
| 70 | +_project_dir: Optional[str] = None | |
| 71 | +_toplevel: Optional[str] = None | |
| 72 | +_reported = set() # paths already named in the debug log | |
| 73 | + | |
| 74 | + | |
| 75 | +def skip_secret_files_enabled() -> bool: | |
| 76 | + return os.environ.get("SG_SKIP_SECRET_FILES", "1").strip().lower() not in ("0", "off", "false", "no") | |
| 77 | + | |
| 78 | + | |
| 79 | +def load_for_session(cwd: Optional[str]) -> None: | |
| 80 | + """Point the module at this hook invocation's cwd; rules load lazily.""" | |
| 81 | + global _cwd, _loaded, _rules, _forward, _deny_all_reads, _project_dir, _toplevel | |
| 82 | + _cwd = _norm_abs(cwd) if cwd else None | |
| 83 | + _loaded, _rules, _forward, _deny_all_reads = False, [], [], False | |
| 84 | + _project_dir = _toplevel = None | |
| 85 | + | |
| 86 | + | |
| 87 | +def is_excluded(file_path: str) -> bool: | |
| 88 | + """True if the reviewer must not see this file: a well-known secret file | |
| 89 | + (unless opted out) or one the session's Read deny/ask rules cover. | |
| 90 | + `file_path` is repo-toplevel-relative (as `git diff` prints it) or absolute. | |
| 91 | + Each withheld path is named once in the debug log, so a rule that hides | |
| 92 | + code from review is at least visible there.""" | |
| 93 | + reason = _exclusion_reason(file_path) | |
| 94 | + if reason and file_path not in _reported: | |
| 95 | + _reported.add(file_path) | |
| 96 | + debug_log(f"secretpaths: withheld from review ({reason}): {file_path}") | |
| 97 | + return reason is not None | |
| 98 | + | |
| 99 | + | |
| 100 | +def subagent_disallowed_tools(context_dir: str) -> List[str]: | |
| 101 | + """`disallowed_tools` for an SDK sub-agent rooted at `context_dir`: the | |
| 102 | + shell tools, the session's Read/Grep/Glob deny+ask rules, and the | |
| 103 | + secret-file globs. | |
| 104 | + | |
| 105 | + The sub-agent anchors root-less and `/x` patterns at its own cwd, which | |
| 106 | + can differ from the session cwd and project dir (a git toplevel above | |
| 107 | + them, or an alternate checkout), so those rules are also sent as absolute | |
| 108 | + copies for every root they should cover.""" | |
| 109 | + _ensure_loaded() | |
| 110 | + out = _SHELL_TOOLS + _forward | |
| 111 | + context = _norm_abs(context_dir) | |
| 112 | + session_cwd = _cwd or _project_dir | |
| 113 | + for kind, root, pattern, _rx in _rules: | |
| 114 | + if kind == "cwd": # verbatim copy already covers `context` itself | |
| 115 | + roots = {session_cwd, _mirrored(session_cwd, context)} - {context} | |
| 116 | + elif kind == "project": # _forward already covers the project dir | |
| 117 | + roots = {_mirrored(root, context)} - {root} | |
| 118 | + else: | |
| 119 | + continue | |
| 120 | + out.extend(f"Read(/{r}/{pattern})" for r in sorted(x for x in roots if x)) | |
| 121 | + if skip_secret_files_enabled(): | |
| 122 | + out.extend(f"Read({g})" for g in SECRET_BASENAME_GLOBS) | |
| 123 | + out.extend(f"Read({stem}{ext})" for stem in _SECRET_STEMS for ext in _DATA_EXTS) | |
| 124 | + return list(dict.fromkeys(out)) | |
| 125 | + | |
| 126 | + | |
| 127 | +def no_file_tools() -> List[str]: | |
| 128 | + """`disallowed_tools` for a sub-agent that must not touch the filesystem at all.""" | |
| 129 | + return _SHELL_TOOLS + list(_READ_TOOLS) | |
| 130 | + | |
| 131 | + | |
| 132 | +# ── internals ─────────────────────────────────────────────────────────────── | |
| 133 | + | |
| 134 | + | |
| 135 | +def _exclusion_reason(file_path: str) -> Optional[str]: | |
| 136 | + if skip_secret_files_enabled() and _is_secret_basename(os.path.basename(file_path)): | |
| 137 | + return "well-known secret file" | |
| 138 | + _ensure_loaded() | |
| 139 | + if _deny_all_reads: | |
| 140 | + return "Read denied" | |
| 141 | + if not _rules: | |
| 142 | + return None | |
| 143 | + abs_path = _norm_abs(file_path if os.path.isabs(file_path) else os.path.join(_toplevel or os.getcwd(), file_path)) | |
| 144 | + for kind, root, _pattern, rx in _rules: | |
| 145 | + if rx is None: | |
| 146 | + continue | |
| 147 | + for base in ({_cwd or _norm_abs(os.getcwd()), _toplevel} if kind == "cwd" else {root}): | |
| 148 | + if not base: | |
| 149 | + continue | |
| 150 | + if not _outside(abs_path, base) and rx.match(os.path.relpath(abs_path, base).replace(os.sep, "/")): | |
| 151 | + return "Read deny/ask rule" | |
| 152 | + return None | |
| 153 | + | |
| 154 | + | |
| 155 | +def _is_secret_basename(base: str) -> bool: | |
| 156 | + low = base.lower() | |
| 157 | + if any(fnmatch.fnmatchcase(low, g) for g in SECRET_BASENAME_GLOBS): | |
| 158 | + return True | |
| 159 | + stem, ext = os.path.splitext(low) | |
| 160 | + return stem in _SECRET_STEMS and ext in _DATA_EXTS | |
| 161 | + | |
| 162 | + | |
| 163 | +def _ensure_loaded() -> None: | |
| 164 | + """Read Read/Grep/Glob deny+ask rules from every settings file the parent | |
| 165 | + session loads. A malformed file is debug-logged and skipped (Claude Code | |
| 166 | + does not enforce rules from a file it cannot parse either); any other | |
| 167 | + failure hides every file from the reviewer rather than risk showing it a | |
| 168 | + denied one.""" | |
| 169 | + global _loaded, _deny_all_reads | |
| 170 | + if _loaded: | |
| 171 | + return | |
| 172 | + _loaded = True | |
| 173 | + try: | |
| 174 | + seen = set() | |
| 175 | + for path, anchor in _settings_files(): | |
| 176 | + try: | |
| 177 | + # utf-8-sig: Claude Code strips a BOM before parsing, so must we. | |
| 178 | + with open(path, encoding="utf-8-sig") as f: | |
| 179 | + data = json.load(f) | |
| 180 | + except OSError: | |
| 181 | + continue | |
| 182 | + except ValueError as e: | |
| 183 | + debug_log(f"secretpaths: cannot parse {path}: {e}") | |
| 184 | + continue | |
| 185 | + perms = data.get("permissions") if isinstance(data, dict) else None | |
| 186 | + if not isinstance(perms, dict): | |
| 187 | + continue | |
| 188 | + # `ask` means "a human decides" — there is no human in the review | |
| 189 | + # sub-agent, so it counts as a deny there. | |
| 190 | + for key in ("deny", "ask"): | |
| 191 | + rules = perms.get(key) | |
| 192 | + for rule in rules if isinstance(rules, list) else []: | |
| 193 | + if isinstance(rule, str) and (rule, anchor) not in seen: | |
| 194 | + seen.add((rule, anchor)) | |
| 195 | + _add_rule(rule, anchor) | |
| 196 | + except Exception as e: | |
| 197 | + debug_log(f"secretpaths: failed to load permission rules ({e}); excluding all files") | |
| 198 | + _deny_all_reads = True | |
| 199 | + if _rules or _deny_all_reads: | |
| 200 | + debug_log(f"secretpaths: {len(_rules)} Read deny/ask pattern(s) loaded" | |
| 201 | + + (" (+ deny-all Read)" if _deny_all_reads else "")) | |
| 202 | + | |
| 203 | + | |
| 204 | +def _settings_files() -> List[Tuple[str, str]]: | |
| 205 | + """(settings file, anchor dir for its `/`-prefixed patterns) — the files | |
| 206 | + Claude Code itself loads: managed (+ drop-ins), user, the project dir's | |
| 207 | + settings.json / settings.local.json, and settings.local.json at the | |
| 208 | + canonical repo root (where Claude Code keeps it for linked worktrees). | |
| 209 | + Managed, project and local rules anchor at the project dir, user rules | |
| 210 | + at the config dir.""" | |
| 211 | + global _project_dir, _toplevel | |
| 212 | + from gitutil import _git_dir, _git_toplevel # gitutil imports this module | |
| 213 | + | |
| 214 | + config_dir = _norm_abs(os.environ.get("CLAUDE_CONFIG_DIR") or os.path.expanduser("~/.claude")) | |
| 215 | + _project_dir = _norm_abs(os.environ.get("CLAUDE_PROJECT_DIR") or _cwd or os.getcwd()) | |
| 216 | + top = _git_toplevel(_cwd or _project_dir) | |
| 217 | + _toplevel = _norm_abs(top) if top else (_cwd or _project_dir) | |
| 218 | + canonical = None | |
| 219 | + if top: | |
| 220 | + common = _git_dir(top) | |
| 221 | + if common and os.path.basename(os.path.normpath(common)) == ".git": | |
| 222 | + canonical = _norm_abs(os.path.dirname(os.path.normpath(common))) | |
| 223 | + managed = _managed_settings_dir() | |
| 224 | + files = [(os.path.join(managed, "managed-settings.json"), _project_dir)] | |
| 225 | + try: | |
| 226 | + dropins = sorted(os.listdir(os.path.join(managed, "managed-settings.d"))) | |
| 227 | + except OSError: | |
| 228 | + dropins = [] | |
| 229 | + files.extend((os.path.join(managed, "managed-settings.d", n), _project_dir) | |
| 230 | + for n in dropins if n.endswith(".json")) | |
| 231 | + files.append((os.path.join(config_dir, "settings.json"), config_dir)) | |
| 232 | + files.append((os.path.join(_project_dir, ".claude", "settings.json"), _project_dir)) | |
| 233 | + files.append((os.path.join(_project_dir, ".claude", "settings.local.json"), _project_dir)) | |
| 234 | + if canonical and canonical != _project_dir: | |
| 235 | + files.append((os.path.join(canonical, ".claude", "settings.local.json"), _project_dir)) | |
| 236 | + return files | |
| 237 | + | |
| 238 | + | |
| 239 | +def _mirrored(directory: Optional[str], context: str) -> Optional[str]: | |
| 240 | + """`directory`'s counterpart inside the checkout rooted at `context` (the | |
| 241 | + sub-agent's cwd): itself when the sub-agent runs inside this repo, else | |
| 242 | + the same toplevel-relative location under `context`.""" | |
| 243 | + if not directory or not _toplevel or not _outside(context, _toplevel): | |
| 244 | + return directory | |
| 245 | + rel = os.path.relpath(directory, _toplevel) | |
| 246 | + return None if _outside(directory, _toplevel) else _norm_abs(os.path.join(context, rel)) | |
| 247 | + | |
| 248 | + | |
| 249 | +def _outside(path: str, root: str) -> bool: | |
| 250 | + try: | |
| 251 | + rel = os.path.relpath(path, root) | |
| 252 | + except ValueError: # different Windows drive | |
| 253 | + return True | |
| 254 | + return rel == ".." or rel.startswith(".." + os.sep) | |
| 255 | + | |
| 256 | + | |
| 257 | +def _managed_settings_dir() -> str: | |
| 258 | + if sys.platform == "darwin": | |
| 259 | + return "/Library/Application Support/ClaudeCode" | |
| 260 | + if sys.platform == "win32": | |
| 261 | + return r"C:\Program Files\ClaudeCode" | |
| 262 | + return "/etc/claude-code" | |
| 263 | + | |
| 264 | + | |
| 265 | +def _norm_abs(path: str) -> str: | |
| 266 | + """Absolute path; on Windows in the `/c/Users/...` form Claude Code | |
| 267 | + matches against, so rules and paths compare in one notation.""" | |
| 268 | + p = os.path.abspath(path) | |
| 269 | + if os.name == "nt": | |
| 270 | + p = p.replace("\\", "/") | |
| 271 | + if re.match(r"^[A-Za-z]:", p): | |
| 272 | + p = "/" + p[0].lower() + p[2:] | |
| 273 | + return p | |
| 274 | + | |
| 275 | + | |
| 276 | +def _add_rule(rule: str, anchor: str) -> None: | |
| 277 | + global _deny_all_reads | |
| 278 | + m = _RULE_RE.match(rule) | |
| 279 | + if not m or m.group(1) not in _READ_TOOLS: | |
| 280 | + return | |
| 281 | + tool, pattern = m.group(1), (m.group(2) or "").strip() | |
| 282 | + if not pattern: | |
| 283 | + # Bare `Read` denies every read; bare `Grep`/`Glob` only remove a tool. | |
| 284 | + if tool == "Read": | |
| 285 | + _deny_all_reads = True | |
| 286 | + _forward.append(tool) | |
| 287 | + return | |
| 288 | + if tool != "Read": | |
| 289 | + # Grep(...)/Glob(...) content rules are not path rules; forward as-is. | |
| 290 | + _forward.append(rule.strip()) | |
| 291 | + return | |
| 292 | + # Same prefix conventions as Claude Code's patternWithRootFor. | |
| 293 | + if os.name == "nt" and re.match(r"^(~\\|\\(?![!#]))", pattern): | |
| 294 | + pattern = pattern.replace("\\", "/") | |
| 295 | + if os.name == "nt" and re.match(r"^[A-Za-z]:[/\\]", pattern): | |
| 296 | + pattern = "//" + pattern[0].lower() + pattern[2:].replace("\\", "/") | |
| 297 | + if pattern.startswith("//"): | |
| 298 | + kind, root, rel = "abs", "/", pattern[2:] | |
| 299 | + if os.name == "nt" and re.match(r"^[A-Za-z]/", rel): | |
| 300 | + root, rel = "/" + rel[0].lower(), rel[2:] | |
| 301 | + elif pattern.startswith("~/"): | |
| 302 | + kind, root, rel = "abs", _norm_abs(os.path.expanduser("~")), pattern[2:] | |
| 303 | + elif pattern.startswith("/"): | |
| 304 | + kind, root, rel = "project", anchor, pattern[1:] | |
| 305 | + rule = f"Read(/{anchor}/{rel})" | |
| 306 | + else: | |
| 307 | + kind, root, rel = "cwd", "", (pattern[2:] if pattern.startswith("./") else pattern) | |
| 308 | + # All three prefixed forms stay anchored at their root, as in Claude Code. | |
| 309 | + _rules.append((kind, root, rel, _compile(rel if kind == "cwd" else "/" + rel))) | |
| 310 | + _forward.append(rule.strip()) | |
| 311 | + | |
| 312 | + | |
| 313 | +def _compile(pattern: str) -> Optional[Pattern[str]]: | |
| 314 | + """gitignore-style pattern → regex over a root-relative POSIX path. | |
| 315 | + Approximates the `ignore` matcher Claude Code uses; where it differs it | |
| 316 | + errs toward matching (excluding) more: negations are skipped, directory | |
| 317 | + patterns also match a same-named file, and — as Claude Code itself does | |
| 318 | + for deny rules — a trailing `/**` is dropped, so `dir/**` means `dir` at | |
| 319 | + any depth.""" | |
| 320 | + p = pattern.strip() | |
| 321 | + if p.startswith("!") or p.startswith("#"): | |
| 322 | + return None | |
| 323 | + if p.endswith("/**"): | |
| 324 | + p = p[:-3] | |
| 325 | + p = p.rstrip("/") | |
| 326 | + if p.startswith("\\"): | |
| 327 | + p = p[1:] | |
| 328 | + anchored = "/" in p | |
| 329 | + p = p.lstrip("/") | |
| 330 | + if not p: | |
| 331 | + return None # `./`, `/`, `~/`: matches nothing, as in Claude Code | |
| 332 | + if p == "**": | |
| 333 | + return re.compile("") # everything under the root | |
| 334 | + out, i = "", 0 | |
| 335 | + while i < len(p): | |
| 336 | + c = p[i] | |
| 337 | + if p.startswith("**/", i): | |
| 338 | + out += "(?:.*/)?" | |
| 339 | + i += 3 | |
| 340 | + elif p.startswith("**", i): | |
| 341 | + out += ".*" | |
| 342 | + i += 2 | |
| 343 | + elif c == "*": | |
| 344 | + out += "[^/]*" | |
| 345 | + i += 1 | |
| 346 | + elif c == "?": | |
| 347 | + out += "[^/]" | |
| 348 | + i += 1 | |
| 349 | + elif c == "[": | |
| 350 | + j = p.find("]", i + 2) | |
| 351 | + if j == -1: | |
| 352 | + out += re.escape(c) | |
| 353 | + i += 1 | |
| 354 | + else: | |
| 355 | + body = p[i + 1:j] | |
| 356 | + if body.startswith("!"): | |
| 357 | + body = "^" + body[1:] | |
| 358 | + out += "[" + body.replace("\\", "\\\\") + "]" | |
| 359 | + i = j + 1 | |
| 360 | + elif c == "\\" and i + 1 < len(p): | |
| 361 | + out += re.escape(p[i + 1]) | |
| 362 | + i += 2 | |
| 363 | + else: | |
| 364 | + out += re.escape(c) | |
| 365 | + i += 1 | |
| 366 | + prefix = "^" if anchored else "^(?:.*/)?" | |
| 367 | + try: | |
| 368 | + return re.compile(prefix + out + "(?:/.*)?$", re.IGNORECASE) | |
| 369 | + except re.error: | |
| 370 | + debug_log(f"secretpaths: unusable pattern {pattern!r}; treating as match-all") | |
| 371 | + return re.compile("") |
| @@ -84,6 +84,7 @@ | ||
| 84 | 84 | _PRICE_PER_MTOK, _PRICE_DEFAULT, _record_usage, _usage_metrics, |
| 85 | 85 | ) |
| 86 | 86 | import extensibility # noqa: E402 |
| 87 | +import secretpaths # noqa: E402 | |
| 87 | 88 | from patterns import ( # noqa: E402,F401 |
| 88 | 89 | _JS_EXTS, _PY_EXTS, _DOC_EXTS, |
| 89 | 90 | _UNSAFE_DESERIALIZATION_REMINDER, _UNSAFE_YAML_LOAD_REMINDER, |
| @@ -2051,6 +2052,7 @@ | ||
| 2051 | 2052 | # per invocation. Failures are non-fatal (debug-logged) so a malformed |
| 2052 | 2053 | # config never prevents the built-in checks from running. |
| 2053 | 2054 | extensibility.load_for_session(input_data.get("cwd")) |
| 2055 | + secretpaths.load_for_session(input_data.get("cwd")) | |
| 2054 | 2056 | |
| 2055 | 2057 | # Remote-pod SDK-bootstrap rescue: PostToolUse is the earliest hook event |
| 2056 | 2058 | # that is guaranteed to fire *after* async plugin sync (its firing proves |
| @@ -1,0 +1,317 @@ | ||
| 1 | +"""Regression tests for anthropics/claude-code#96276: the reviewer must not | |
| 2 | +put files the session's Read deny/ask rules cover (or well-known secret | |
| 3 | +files) into a review prompt, and its SDK sub-agents must carry the same | |
| 4 | +rules as disallowed_tools. | |
| 5 | + | |
| 6 | +Run: python3 -m unittest discover -s plugins/security-guidance/tests -v | |
| 7 | +""" | |
| 8 | + | |
| 9 | +import json | |
| 10 | +import os | |
| 11 | +import shutil | |
| 12 | +import subprocess | |
| 13 | +import sys | |
| 14 | +import tempfile | |
| 15 | +import types | |
| 16 | +import unittest | |
| 17 | + | |
| 18 | +HOOKS_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "hooks") | |
| 19 | +sys.path.insert(0, HOOKS_DIR) | |
| 20 | + | |
| 21 | +_tmp_state = tempfile.mkdtemp(prefix="sg-test-state-") | |
| 22 | +os.environ["SECURITY_WARNINGS_STATE_DIR"] = _tmp_state | |
| 23 | +os.environ["SECURITY_GUIDANCE_DEBUG_LOG"] = os.path.join(_tmp_state, "log.txt") | |
| 24 | + | |
| 25 | +import gitutil # noqa: E402 | |
| 26 | +import llm # noqa: E402 | |
| 27 | +import secretpaths # noqa: E402 | |
| 28 | + | |
| 29 | +SHELL_TOOLS = ["Bash"] + (["PowerShell"] if sys.platform == "win32" else []) | |
| 30 | + | |
| 31 | + | |
| 32 | +def tearDownModule(): | |
| 33 | + shutil.rmtree(_tmp_state, ignore_errors=True) | |
| 34 | + | |
| 35 | + | |
| 36 | +def _diff(*paths): | |
| 37 | + """A minimal unified diff touching each path with one added line.""" | |
| 38 | + out = [] | |
| 39 | + for p in paths: | |
| 40 | + out.append( | |
| 41 | + f"diff --git a/{p} b/{p}\n" | |
| 42 | + f"--- a/{p}\n+++ b/{p}\n" | |
| 43 | + "@@ -0,0 +1,1 @@\n" | |
| 44 | + f"+value_for_{p.replace('/', '_')} = \"DUMMY-SECRET\"\n" | |
| 45 | + ) | |
| 46 | + return "".join(out) | |
| 47 | + | |
| 48 | + | |
| 49 | +class _Env: | |
| 50 | + """Isolated project, config and managed dirs; restores env and module state.""" | |
| 51 | + | |
| 52 | + def __init__(self, project_settings=None, user_settings=None, local_settings=None, | |
| 53 | + managed_settings=None): | |
| 54 | + self.project_settings = project_settings | |
| 55 | + self.user_settings = user_settings | |
| 56 | + self.local_settings = local_settings | |
| 57 | + self.managed_settings = managed_settings | |
| 58 | + | |
| 59 | + def __enter__(self): | |
| 60 | + self._saved = dict(os.environ) | |
| 61 | + self._saved_managed_dir = secretpaths._managed_settings_dir | |
| 62 | + self.root = os.path.realpath(tempfile.mkdtemp(prefix="sg-test-")) | |
| 63 | + self.project = os.path.join(self.root, "project") | |
| 64 | + self.config = os.path.join(self.root, "config-dir") | |
| 65 | + self.managed = os.path.join(self.root, "managed") | |
| 66 | + os.makedirs(os.path.join(self.project, ".claude")) | |
| 67 | + os.makedirs(self.config) | |
| 68 | + os.makedirs(os.path.join(self.managed, "managed-settings.d")) | |
| 69 | + secretpaths._managed_settings_dir = lambda: self.managed | |
| 70 | + if self.managed_settings is not None: | |
| 71 | + with open(os.path.join(self.managed, "managed-settings.d", "10-sec.json"), "w") as f: | |
| 72 | + json.dump(self.managed_settings, f) | |
| 73 | + if self.project_settings is not None: | |
| 74 | + with open(os.path.join(self.project, ".claude", "settings.json"), "w") as f: | |
| 75 | + json.dump(self.project_settings, f) | |
| 76 | + if self.local_settings is not None: | |
| 77 | + with open(os.path.join(self.project, ".claude", "settings.local.json"), "w") as f: | |
| 78 | + json.dump(self.local_settings, f) | |
| 79 | + if self.user_settings is not None: | |
| 80 | + with open(os.path.join(self.config, "settings.json"), "w") as f: | |
| 81 | + json.dump(self.user_settings, f) | |
| 82 | + os.environ["CLAUDE_CONFIG_DIR"] = self.config | |
| 83 | + os.environ["CLAUDE_PROJECT_DIR"] = self.project | |
| 84 | + os.environ.pop("SG_SKIP_SECRET_FILES", None) | |
| 85 | + secretpaths.load_for_session(self.project) | |
| 86 | + return self | |
| 87 | + | |
| 88 | + def __exit__(self, *exc): | |
| 89 | + os.environ.clear() | |
| 90 | + os.environ.update(self._saved) | |
| 91 | + secretpaths._managed_settings_dir = self._saved_managed_dir | |
| 92 | + secretpaths.load_for_session(None) | |
| 93 | + shutil.rmtree(self.root, ignore_errors=True) | |
| 94 | + | |
| 95 | + | |
| 96 | +class DiffFilterTests(unittest.TestCase): | |
| 97 | + DIFF = _diff( | |
| 98 | + "app.py", | |
| 99 | + "config/prod.json", # denied by Read(./config/**) | |
| 100 | + "docs/config/notes.json", # also denied: Claude Code reads `x/**` as `x`, any depth | |
| 101 | + "docs/settings/ui.json", # Read(./settings/*.json) is anchored: NOT denied | |
| 102 | + "private/report.json", # ask rule Read(/private/**) → excluded | |
| 103 | + "secrets.yaml", # well-known secret store name | |
| 104 | + "deploy/credentials", # well-known secret store name (extensionless) | |
| 105 | + "keys/server.pem", # already non-reviewable by extension | |
| 106 | + "src/secrets.py", # code that handles secrets: reviewable | |
| 107 | + ) | |
| 108 | + | |
| 109 | + def _reviewed(self): | |
| 110 | + return [p for p, _ in gitutil.parse_diff_into_files(self.DIFF)] | |
| 111 | + | |
| 112 | + def test_denied_and_secret_files_are_dropped_from_review_diff(self): | |
| 113 | + settings = {"permissions": {"deny": ["Read(./config/**)", "Read(./settings/*.json)", | |
| 114 | + "Bash(curl:*)"], | |
| 115 | + "ask": ["Read(/private/**)"]}} | |
| 116 | + with _Env(project_settings=settings): | |
| 117 | + self.assertEqual( | |
| 118 | + self._reviewed(), | |
| 119 | + ["app.py", "docs/settings/ui.json", "src/secrets.py"], | |
| 120 | + ) | |
| 121 | + | |
| 122 | + def test_opt_out_restores_secret_named_files_but_not_denied_ones(self): | |
| 123 | + settings = {"permissions": {"deny": ["Read(./config/**)"]}} | |
| 124 | + with _Env(project_settings=settings): | |
| 125 | + os.environ["SG_SKIP_SECRET_FILES"] = "0" | |
| 126 | + self.assertEqual( | |
| 127 | + self._reviewed(), | |
| 128 | + ["app.py", "docs/settings/ui.json", "private/report.json", | |
| 129 | + "secrets.yaml", "deploy/credentials", "src/secrets.py"], | |
| 130 | + ) | |
| 131 | + | |
| 132 | + def test_no_settings_still_skips_well_known_secret_files(self): | |
| 133 | + with _Env(): | |
| 134 | + reviewed = self._reviewed() | |
| 135 | + self.assertIn("config/prod.json", reviewed) | |
| 136 | + self.assertNotIn("secrets.yaml", reviewed) | |
| 137 | + self.assertNotIn("deploy/credentials", reviewed) | |
| 138 | + | |
| 139 | + def test_bare_read_deny_excludes_everything(self): | |
| 140 | + with _Env(local_settings={"permissions": {"deny": ["Read"]}}): | |
| 141 | + self.assertEqual(self._reviewed(), []) | |
| 142 | + | |
| 143 | + def test_user_settings_slash_rule_anchors_at_config_dir_not_project(self): | |
| 144 | + # `/x` in user settings means <config dir>/x, so it must not hide the | |
| 145 | + # project's reports/ — but `~/` and `//` rules reach anywhere. | |
| 146 | + user = {"permissions": {"deny": ["Read(/reports/**)", "Read(//**/vault/*.json)"]}} | |
| 147 | + with _Env(user_settings=user): | |
| 148 | + reviewed = [p for p, _ in gitutil.parse_diff_into_files( | |
| 149 | + _diff("reports/q3.json", "app/vault/keys.json", "app.py"))] | |
| 150 | + self.assertEqual(reviewed, ["reports/q3.json", "app.py"]) | |
| 151 | + | |
| 152 | + def test_managed_dropin_rules_and_utf8_bom_are_honored(self): | |
| 153 | + with _Env(managed_settings={"permissions": {"deny": ["Read(/private/**)"]}}) as env: | |
| 154 | + # PowerShell 5 writes settings.json with a BOM; Claude Code strips it. | |
| 155 | + with open(os.path.join(env.project, ".claude", "settings.json"), "w", encoding="utf-8-sig") as f: | |
| 156 | + json.dump({"permissions": {"deny": ["Read(./config/**)"]}}, f) | |
| 157 | + secretpaths.load_for_session(env.project) | |
| 158 | + reviewed = self._reviewed() | |
| 159 | + self.assertNotIn("private/report.json", reviewed) | |
| 160 | + self.assertNotIn("config/prod.json", reviewed) | |
| 161 | + self.assertIn("app.py", reviewed) | |
| 162 | + | |
| 163 | + def test_empty_patterns_match_nothing_instead_of_everything(self): | |
| 164 | + with _Env(project_settings={"permissions": {"deny": ["Read(./)", "Read(/)", "Read(~/)"]}}): | |
| 165 | + self.assertIn("app.py", self._reviewed()) | |
| 166 | + | |
| 167 | + def test_worktree_session_reads_local_settings_at_canonical_root(self): | |
| 168 | + with _Env() as env: | |
| 169 | + git = ["git", "-c", "user.email=t@example.com", "-c", "user.name=t"] | |
| 170 | + run = lambda *a, cwd=env.project: subprocess.run([*git, *a], cwd=cwd, check=True, | |
| 171 | + capture_output=True) | |
| 172 | + run("init", "-q") | |
| 173 | + run("commit", "-q", "--allow-empty", "-m", "init") | |
| 174 | + worktree = os.path.join(env.root, "wt") | |
| 175 | + run("worktree", "add", "-q", worktree) | |
| 176 | + # Claude Code keeps settings.local.json at the main repo root. | |
| 177 | + with open(os.path.join(env.project, ".claude", "settings.local.json"), "w") as f: | |
| 178 | + json.dump({"permissions": {"deny": ["Read(./config/**)"]}}, f) | |
| 179 | + os.environ["CLAUDE_PROJECT_DIR"] = worktree | |
| 180 | + secretpaths.load_for_session(worktree) | |
| 181 | + self.assertNotIn("config/prod.json", self._reviewed()) | |
| 182 | + self.assertIn("app.py", self._reviewed()) | |
| 183 | + | |
| 184 | + def test_malformed_settings_file_is_ignored(self): | |
| 185 | + with _Env() as env: | |
| 186 | + with open(os.path.join(env.project, ".claude", "settings.json"), "w") as f: | |
| 187 | + f.write("{not json") | |
| 188 | + secretpaths.load_for_session(env.project) | |
| 189 | + self.assertIn("app.py", self._reviewed()) | |
| 190 | + | |
| 191 | + | |
| 192 | +class PatternSemanticsTests(unittest.TestCase): | |
| 193 | + CASES = [ | |
| 194 | + # (pattern, root-relative path, matches) | |
| 195 | + ("secrets.yaml", "secrets.yaml", True), | |
| 196 | + ("secrets.yaml", "a/b/secrets.yaml", True), # slashless → any depth | |
| 197 | + ("secrets.yaml", "SECRETS.YAML", True), # case-insensitive, like Claude Code | |
| 198 | + ("config/*.json", "config/a.json", True), | |
| 199 | + ("config/*.json", "x/config/a.json", False), # contains slash → anchored | |
| 200 | + ("config/*.json", "config/sub/a.json", False), # * does not cross / | |
| 201 | + ("config/**", "config/sub/a.json", True), | |
| 202 | + ("config/**", "docs/config/a.json", True), # Claude Code strips /** → slashless | |
| 203 | + ("/config/**", "docs/config/a.json", False), | |
| 204 | + ("config", "config/sub/a.json", True), # dir name covers contents | |
| 205 | + ("/private", "private/x", True), | |
| 206 | + ("/private", "docs/private/x", False), | |
| 207 | + ("**/vault/*.json", "a/b/vault/k.json", True), | |
| 208 | + ("**/vault/*.json", "vault/k.json", True), | |
| 209 | + ("*.pem", "keys/a.PEM", True), | |
| 210 | + ("id_?sa", "home/id_rsa", True), | |
| 211 | + ("key[0-9]", "key7", True), | |
| 212 | + ("key[!0-9]", "key7", False), | |
| 213 | + (r"a\*b", "a*b", True), | |
| 214 | + (r"a\*b", "axb", False), | |
| 215 | + ("docs/", "docs/readme.md", True), | |
| 216 | + ("!keep.json", "keep.json", False), # negation never excludes | |
| 217 | + ] | |
| 218 | + | |
| 219 | + def test_gitignore_style_matching(self): | |
| 220 | + for pattern, path, expected in self.CASES: | |
| 221 | + with self.subTest(pattern=pattern, path=path): | |
| 222 | + rx = secretpaths._compile(pattern) | |
| 223 | + self.assertEqual(bool(rx and rx.match(path)), expected) | |
| 224 | + | |
| 225 | + | |
| 226 | +class SubagentDisallowedToolsTests(unittest.TestCase): | |
| 227 | + def test_rules_are_forwarded_with_absolute_anchors(self): | |
| 228 | + settings = {"permissions": { | |
| 229 | + "deny": ["Read(./config/**)", "Read(/private/**)", "Read(~/.ssh/**)", | |
| 230 | + "Read(//etc/shadow)", "Grep", "Bash(curl:*)", "Edit(**)"], | |
| 231 | + "ask": ["Read(*.tfstate)"], | |
| 232 | + }} | |
| 233 | + with _Env(project_settings=settings) as env: | |
| 234 | + tools = secretpaths.subagent_disallowed_tools(env.project) | |
| 235 | + proj = env.project.strip("/") | |
| 236 | + for expected in (*SHELL_TOOLS, "Read(./config/**)", f"Read(//{proj}/private/**)", | |
| 237 | + "Read(~/.ssh/**)", "Read(//etc/shadow)", "Grep", | |
| 238 | + "Read(*.tfstate)", "Read(.env)", "Read(id_rsa*)", | |
| 239 | + "Read(secrets.yaml)", "Read(credentials.json)"): | |
| 240 | + self.assertIn(expected, tools) | |
| 241 | + # Only read-path rules are forwarded; other tools' rules are not ours. | |
| 242 | + self.assertFalse(any(t.startswith(("Bash(", "Edit")) for t in tools)) | |
| 243 | + # Sub-agent rooted at an alternate checkout: the verbatim relative | |
| 244 | + # rule covers that root, the session cwd gets an absolute copy, and | |
| 245 | + # the project-anchored rule is mirrored into the checkout. | |
| 246 | + alt = os.path.join(env.root, "alt-checkout") | |
| 247 | + elsewhere = secretpaths.subagent_disallowed_tools(alt) | |
| 248 | + for expected in (f"Read(//{proj}/config/**)", f"Read(//{alt.strip('/')}/private/**)"): | |
| 249 | + self.assertIn(expected, elsewhere) | |
| 250 | + self.assertNotIn(expected, tools) | |
| 251 | + | |
| 252 | + def test_opt_out_drops_only_the_builtin_globs(self): | |
| 253 | + with _Env(project_settings={"permissions": {"deny": ["Read(./config/**)"]}}) as env: | |
| 254 | + os.environ["SG_SKIP_SECRET_FILES"] = "0" | |
| 255 | + self.assertEqual(secretpaths.subagent_disallowed_tools(env.project), | |
| 256 | + SHELL_TOOLS + ["Read(./config/**)"]) | |
| 257 | + | |
| 258 | + def test_diff_only_call_gets_no_file_or_shell_tools(self): | |
| 259 | + self.assertEqual(secretpaths.no_file_tools(), SHELL_TOOLS + ["Read", "Grep", "Glob"]) | |
| 260 | + | |
| 261 | + | |
| 262 | +class _FakeSDK(types.ModuleType): | |
| 263 | + """Stand-in for claude_agent_sdk that records the options each query got.""" | |
| 264 | + | |
| 265 | + def __init__(self): | |
| 266 | + super().__init__("claude_agent_sdk") | |
| 267 | + self.options_seen = [] | |
| 268 | + sdk = self | |
| 269 | + | |
| 270 | + class ClaudeAgentOptions: | |
| 271 | + def __init__(self, **kwargs): | |
| 272 | + self.__dict__.update(kwargs) | |
| 273 | + sdk.options_seen.append(kwargs) | |
| 274 | + | |
| 275 | + class AssistantMessage: | |
| 276 | + pass | |
| 277 | + | |
| 278 | + class ResultMessage: | |
| 279 | + subtype = "success" | |
| 280 | + structured_output = {"findings": []} | |
| 281 | + usage = None | |
| 282 | + total_cost_usd = None | |
| 283 | + | |
| 284 | + async def query(prompt, options): | |
| 285 | + async for _ in prompt: | |
| 286 | + pass | |
| 287 | + yield ResultMessage() | |
| 288 | + | |
| 289 | + self.ClaudeAgentOptions = ClaudeAgentOptions | |
| 290 | + self.AssistantMessage = AssistantMessage | |
| 291 | + self.ResultMessage = ResultMessage | |
| 292 | + self.query = query | |
| 293 | + | |
| 294 | + | |
| 295 | +class AgenticReviewWiringTests(unittest.TestCase): | |
| 296 | + def test_agentic_review_passes_deny_rules_as_disallowed_tools(self): | |
| 297 | + fake = _FakeSDK() | |
| 298 | + saved = sys.modules.get("claude_agent_sdk") | |
| 299 | + sys.modules["claude_agent_sdk"] = fake | |
| 300 | + try: | |
| 301 | + with _Env(project_settings={"permissions": {"deny": ["Read(./config/**)"]}}) as env: | |
| 302 | + os.environ["SG_AGENTIC_CLI_PATH"] = os.path.join(env.root, "no-such-claude") | |
| 303 | + llm.agentic_review(env.project, [("app.py", _diff("app.py"))], ["app.py"]) | |
| 304 | + finally: | |
| 305 | + if saved is None: | |
| 306 | + del sys.modules["claude_agent_sdk"] | |
| 307 | + else: | |
| 308 | + sys.modules["claude_agent_sdk"] = saved | |
| 309 | + self.assertTrue(fake.options_seen, "agentic_review never built ClaudeAgentOptions") | |
| 310 | + for opts in fake.options_seen: | |
| 311 | + self.assertEqual(opts.get("setting_sources"), []) # still no recursive plugins/hooks | |
| 312 | + for expected in ("Bash", "Read(./config/**)", "Read(.env)"): | |
| 313 | + self.assertIn(expected, opts.get("disallowed_tools", [])) | |
| 314 | + | |
| 315 | + | |
| 316 | +if __name__ == "__main__": | |
| 317 | + unittest.main() |