diff --git a/.claude/settings.json b/.claude/settings.json index 9fa1618d..61871a64 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -36,6 +36,9 @@ "Edit(.claude/agents/**)", "Edit(.github/**)", "Edit(tools/security_guards.py)", + "Edit(tools/**)", + "Edit(.agents/**)", + "Edit(salary_lookup.py)", "Bash(git push:*)" ], "deny": [ diff --git a/CHANGELOG.md b/CHANGELOG.md index da2ddb8b..78362902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -480,6 +480,19 @@ per-file diff commands. automated updates would freeze both legs. The test reads the workflow text with the stdlib, so it needs neither zizmor nor PyYAML. +- **The ask list now covers the files the allow list runs, and the guard reads load-time commands** + (`.claude/settings.json`, `tools/security_guards.py` checks 4 and 5, `SECURITY.md`, + `tests/test_security_guards.py`) - `allow` pre-approves `tools/*.py`, every portal CLI and + `salary_lookup.py` by path, and the skills keep `Edit`/`Write` granted, so in the turn that + reads a posting an agent could rewrite one of those files and run it under its existing allow + rule: the "cannot silently turn that into a wider grant" guarantee covered the files that + define permissions but not the ones the permissions name. `Edit(tools/**)`, `Edit(.agents/**)` + and `Edit(salary_lookup.py)` join the required ask rules; CV, cover-letter and tracker writes + are unaffected. Check 4 now also refuses frontmatter `hooks` and !`command` dynamic-context + lines in skills and commands, which run at load with no prompt (SECURITY.md already said the + template ships no hooks; only settings.json hooks were checked). Check 5 refuses an agent + `permissionMode` that removes prompts and agent `hooks` or `mcpServers`. + ## [1.7.3] - 2026-10-19 ### Added diff --git a/SECURITY.md b/SECURITY.md index af7d4277..4a807490 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -21,14 +21,14 @@ Every grant the template ships, where it lives, how long it lasts, and what chec | Surface | What is granted | Lifetime | Guard | |---|---|---|---| | `.claude/settings.json` `permissions.allow` | Each shipped portal CLI by exact path, `salary_lookup.py`, the `tools/` scripts (`rank_state`, `job_key`, `verify_pdf`, `verify_layout`, `robots_check`, `fetch_page`), `pdftotext`, and `Skill(job-application-assistant)` | Whole session | Exact allowlist; any addition fails CI unless added to `ALLOWED_PERMISSIONS` in the same PR | -| `.claude/settings.json` `permissions.ask` | Always prompt, even over an allow rule or a skill grant: edits to `settings.json`, `settings.local.json`, any `.claude/skills/*/SKILL.md`, any `.claude/commands/*.md`, `.claude/agents/**`, `.github/**`, `tools/security_guards.py`; and `git push` | Whole session | Each rule is required; removal fails CI | +| `.claude/settings.json` `permissions.ask` | Always prompt, even over an allow rule or a skill grant: edits to `settings.json`, `settings.local.json`, any `.claude/skills/*/SKILL.md`, any `.claude/commands/*.md`, `.claude/agents/**`, `.github/**`, `tools/security_guards.py`; edits to the files the allow list runs by path (`tools/**`, `.agents/**`, `salary_lookup.py`); and `git push` | Whole session | Each rule is required; removal fails CI | | `.claude/settings.json` `permissions.deny` | Blocked in every mode, `bypassPermissions` included: `curl`/`wget` with data, form or upload flags, `scp`, `sftp`, `ssh`, `nc`; reads of `.env`, `.env.*`, `~/.ssh/**`, `~/.aws/**`, `~/.gnupg/**` (Read denies also cover Bash `cat`/`head`/`tail`/`sed`) | Whole session | Each rule is required; removal fails CI | | Skill `allowed-tools` (`.claude/skills/*/SKILL.md`) | `job-application-assistant`: Read, Glob, Grep, WebFetch, WebSearch, Edit, Write, AskUserQuestion. `scrape`: the same plus Agent, `bun --version`, `tools/job_key.py`. `upskill`: Read, Write, Glob, Grep, WebFetch, WebSearch. No bare `Bash` anywhere. | The turn that invokes the skill; clears on your next message. Not gated by workspace trust. | Exact allowlist (`ALLOWED_SKILL_TOOLS`); bare tools and wildcards fail CI | | Portal skill `allowed-tools` (`.agents/skills//SKILL.md`) | Exactly that portal's own `cli/src/cli.ts` and nothing else | Invoking turn | Checked by path, so a skill that grants another portal's CLI, or a glob, fails CI | | Slash commands (`.claude/commands/*.md`) | No `allowed-tools` frontmatter; they run under the session's rules above | - | Same frontmatter check as skills | -| Subagents (`.claude/agents/*.md`) | `posting-fetcher` (`/apply` Step 0), `posting-scorer` (`/rank` Step 2) and `application-reviewer` (`/apply` Step 3): Read, Glob/Grep where needed, WebFetch, WebSearch, Bash - never Edit, Write or Agent. Bash inside them is scoped by the allow list above, because an agent's `tools:` list is tool-name granularity. `/scrape`'s portal-search agents still run as `general-purpose`. | While the agent runs | Check 5: every agent must declare `tools:` drawn from `ALLOWED_AGENT_TOOLS`; a missing key (= every tool) or a `Bash(pattern)` entry fails CI | +| Subagents (`.claude/agents/*.md`) | `posting-fetcher` (`/apply` Step 0), `posting-scorer` (`/rank` Step 2) and `application-reviewer` (`/apply` Step 3): Read, Glob/Grep where needed, WebFetch, WebSearch, Bash - never Edit, Write or Agent. Bash inside them is scoped by the allow list above, because an agent's `tools:` list is tool-name granularity. `/scrape`'s portal-search agents still run as `general-purpose`. | While the agent runs | Check 5: every agent must declare `tools:` drawn from `ALLOWED_AGENT_TOOLS`; a missing key (= every tool) or a `Bash(pattern)` entry fails CI, as do a `permissionMode` that removes prompts and agent `hooks` or `mcpServers` | | The posting integrity gate (`tools/posting_guard.py`) | Runs on every posting before `/apply` Step 1 (URL or pasted), inside each `/rank` scorer, and on the archived posting in `/interview`: removes hidden HTML and invisible characters, quotes instruction-shaped lines and hidden URLs, lists mixed-script words, prints a one-line verdict. Advises; the user decides. The archive keeps counts, never the removed excerpts. | - | Pinned by `tests/test_posting_guard.py` and `tests/test_posting_gate_wiring.py` | -| Hooks (`.claude/settings.json` `hooks`) | None. The template ships no hooks: a hook runs with no prompt the moment its event fires, which is the vector the Shai-Hulud worm used in August 2026. | - | Any hook fails CI | +| Hooks (`.claude/settings.json` `hooks`, skill and command frontmatter `hooks`, !`command` lines) | None. The template ships no hooks: a hook runs with no prompt the moment its event fires, which is the vector the Shai-Hulud worm used in August 2026. A !`command` line in a skill or command runs when it loads, the same way. | - | Any of them fails CI | | Compiling (`lualatex`, `xelatex`, a custom template's command) | Not pre-approved: a `lualatex:*` grant would also approve `-shell-escape`. In default mode each compile prompts. Answering "don't ask again" would persist exactly that `lualatex` rule to your `settings.local.json`, so answer per call. | - | - | | Portal CLI manifests (`.agents/**/package.json`) | No lifecycle scripts (`preinstall`, `postinstall`, ...) and no `trustedDependencies`, so `bun install` runs no code | - | Fails CI; `tools/install_portal_clis.py` also skips a manifest that carries one | | The browser-header retry (`tools/fetch_page.py`) | A GET with a fixed argv, http(s) only, 45 s / 5 redirects / 10 MB, `robots.txt` enforced before each redirect target with no bypass flag, output refused inside the repo | - | Pinned by `tests/test_fetch_page.py`; the grant names the script, not `curl` | @@ -40,7 +40,7 @@ Every grant the template ships, where it lives, how long it lasts, and what chec - **WebFetch is unrestricted by domain.** Company research needs it, and a GET to an arbitrary host can carry data in the URL. The instruction-level rule (never fetch URLs from the posting body) is what stands between a hidden link and a fetch; the permission layer does not. - **`/rank` scorers still hold the rubric and the posting in one context.** Each scoring agent fetches and scores a batch, so the candidate rubric and the untrusted posting share a context that also has WebFetch. What the agent definition removes is Edit, Write and unscoped shell. Fully separating fetch from scoring would route every posting through the orchestrator, which `/rank` is built to avoid. - **The gate is deterministic, not an oracle.** `posting_guard.py` catches the payload classes that leave a trace: hidden markup, invisible characters, agent-addressed phrasing, hidden links, look-alike letters. A manipulation written to read naturally to a human and an agent alike leaves none, and the instruction-level rules remain the only defence for that. -- **Edit and Write are what the skill does.** Tailoring a CV means editing files, so those tools stay granted. What the `ask` list guarantees is that an agent cannot silently turn that into a wider grant: the files that define permissions always prompt. +- **Edit and Write are what the skill does.** Tailoring a CV means editing files, so those tools stay granted. What the `ask` list guarantees is that an agent cannot silently turn that into a wider grant: the files that define permissions always prompt, and so do the files the allow list pre-approves to run, since rewriting one would turn its allow rule into whatever was written. Instruction-level defenses raise the bar; permission-level defenses bound the damage. Neither is a guarantee. If you run this workflow against job boards you do not trust at all, review what the agent fetched and wrote before sending anything out. diff --git a/tests/test_security_guards.py b/tests/test_security_guards.py index 473ef431..0616f722 100644 --- a/tests/test_security_guards.py +++ b/tests/test_security_guards.py @@ -353,6 +353,40 @@ class SkillGrantGuardTests(GuardRepoFixture): self.assertIn("frontmatter", result.stdout) self.assertNotIn("Traceback", result.stderr) + def test_skill_hooks_frontmatter_fails(self): + # A skill-scoped hook runs while the skill is active, with no prompt. + self.write_skill( + self.core_skill, + "Read", + extra_frontmatter="hooks:\n PreToolUse:\n - hooks:\n - type: command\n command: ./x.sh\n", + ) + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("frontmatter `hooks`", result.stdout) + self.assertIn(".claude/skills/example/SKILL.md", result.stdout.replace("\\", "/")) + + def test_dynamic_context_line_fails_in_skills_and_commands(self): + # !`command` runs at load and splices its output in, with no permission check. + line = "Today's tracker: !`cat job_scraper/seen_jobs.json`\n" + for path in (self.core_skill, self.command): + with self.subTest(path=path.name): + original = path.read_text(encoding="utf-8") + path.write_text(original + line, encoding="utf-8") + result = run_guards(self.root) + path.write_text(original, encoding="utf-8") + self.assertEqual(result.returncode, 1) + self.assertIn("dynamic-context line", result.stdout) + self.assertIn("!`cat job_scraper/seen_jobs.json`", result.stdout) + + def test_inline_code_and_exclamation_marks_pass(self): + self.core_skill.write_text( + self.core_skill.read_text(encoding="utf-8") + + "Run `python3 tools/job_key.py` first! Then `!important` is CSS, not a command.\n", + encoding="utf-8", + ) + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + class AgentGrantGuardTests(GuardRepoFixture): """A .claude/agents/*.md definition's `tools:` list is an allowlist at tool-name @@ -403,6 +437,38 @@ class AgentGrantGuardTests(GuardRepoFixture): result = run_guards(self.root) self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + def write_agent_with(self, extra): + self.agent.write_text( + "---\nname: example\ndescription: An example agent that reads postings.\n" + f"tools: Read, Grep, WebFetch\n{extra}---\n\nYou read postings and report.\n", + encoding="utf-8", + ) + + def test_agent_permission_mode_that_removes_prompts_fails(self): + for mode in ("bypassPermissions", "acceptEdits", "dontAsk", "auto"): + for spelling in ("permissionMode", "permission-mode"): + with self.subTest(mode=mode, key=spelling): + self.write_agent_with(f"{spelling}: {mode}\n") + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("permissionMode", result.stdout) + self.assertIn(mode, result.stdout) + + def test_agent_permission_mode_that_still_prompts_passes(self): + for mode in ("default", "plan", "'default'"): + with self.subTest(mode=mode): + self.write_agent_with(f"permissionMode: {mode}\n") + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_agent_hooks_and_mcp_servers_fail(self): + for key in ("hooks", "mcpServers"): + with self.subTest(key=key): + self.write_agent_with(f"{key}:\n example: {{}}\n") + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn(f"`{key.lower()}`", result.stdout) + class HookGuardTests(GuardRepoFixture): """A hook in .claude/settings.json runs with no prompt when its event fires. diff --git a/tools/security_guards.py b/tools/security_guards.py index 0ff89aa9..f3ff4e40 100644 --- a/tools/security_guards.py +++ b/tools/security_guards.py @@ -29,14 +29,19 @@ Checks: instead grant exactly its own CLI). Claude Code pre-approves these for the turn that invokes the skill, with no prompt, regardless of workspace trust, so the frontmatter is a second permission allowlist. Catches bare Bash and - glob grants. The settings.json `ask` and `deny` lists are also required to - keep every rule in REQUIRED_ASK_RULES / REQUIRED_DENY_RULES. + glob grants. The same files may carry no frontmatter `hooks` and no + !`command` dynamic-context line: both run a command when the skill or + command loads, before any prompt. The settings.json `ask` and `deny` lists + are also required to keep every rule in REQUIRED_ASK_RULES / + REQUIRED_DENY_RULES. 5. Agent definitions — every .claude/agents/*.md must declare `tools:` (a missing key means every tool) and each entry must be in ALLOWED_AGENT_TOOLS, which has no Edit, Write, NotebookEdit or Agent: the shipped agents read untrusted postings. `tools:` is tool-name granularity (a `Bash(pattern)` entry resolves to plain Bash), so a patterned entry is - rejected as misleading; settings.json's allow list is what scopes Bash. + rejected as misleading; settings.json's allow list is what scopes Bash. A + `permissionMode` must be one that still prompts (ALLOWED_AGENT_PERMISSION_MODES), + and `hooks` and `mcpServers` are refused. Stdlib only. Exit 0 on success, 1 with a failure list otherwise. """ @@ -106,6 +111,12 @@ REQUIRED_ASK_RULES = [ "Edit(.claude/agents/**)", "Edit(.github/**)", "Edit(tools/security_guards.py)", + # The files `allow` pre-approves to run, by path: editing one silently + # turns its allow rule into "run whatever is written here", which is the + # same widening. Every portal CLI lives under .agents/. + "Edit(tools/**)", + "Edit(.agents/**)", + "Edit(salary_lookup.py)", "Bash(git push:*)", ] @@ -255,6 +266,20 @@ ALLOWED_SKILL_TOOLS = { # `Bash(echo *)` entry gives the agent plain Bash. ALLOWED_AGENT_TOOLS = {"Read", "Glob", "Grep", "WebFetch", "WebSearch", "Bash"} +# An agent's permissionMode applies to everything it does, so only modes that +# still prompt: acceptEdits, auto, dontAsk and bypassPermissions each remove +# the prompts this file's allowlists assume. +ALLOWED_AGENT_PERMISSION_MODES = {"default", "manual", "plan"} + +# A skill or command line of the form !`command` runs the command when the +# skill loads and splices its output into the prompt, with no permission check. +DYNAMIC_CONTEXT = re.compile(r"!`[^`\n]+`") + + +def _key(name: str) -> str: + """Frontmatter key spelling-insensitive: permissionMode, permission-mode, ...""" + return name.lower().replace("-", "").replace("_", "") + _FRONTMATTER_KEY = re.compile(r"^([A-Za-z0-9_-]+):\s*(.*)$") @@ -343,10 +368,24 @@ def check_skill_tool_grants() -> None: for path in skills + commands: rel = path.relative_to(ROOT).as_posix() try: - data = _frontmatter(path.read_text(encoding="utf-8")) + text = path.read_text(encoding="utf-8") except (OSError, UnicodeDecodeError) as exc: errors.append(f"{rel}: unreadable (must be UTF-8): {exc}") continue + for match in DYNAMIC_CONTEXT.finditer(text): + errors.append( + f"{rel}: dynamic-context line {match.group(0)!r}. Claude Code runs a !`command` " + "when the skill or command loads and splices its output into the prompt, with no " + "permission check, so this is a pre-approved shell command. Move it into a step " + "the model runs, where settings.json decides." + ) + data = _frontmatter(text) + if data is not None and "hooks" in {_key(k) for k in data}: + errors.append( + f"{rel}: frontmatter `hooks`. A skill or command hook runs its command with no " + "prompt whenever its event fires while the skill is active; the template ships no " + "hooks (SECURITY.md), and settings.json hooks are held to ALLOWED_HOOKS." + ) if data is None: if path in commands: continue # commands may omit frontmatter entirely @@ -573,6 +612,20 @@ def check_agent_tool_grants() -> None: if data is None: errors.append(f"{rel}: missing or unterminated frontmatter (expected --- ... ---)") continue + keyed = {_key(k): v for k, v in data.items()} + mode = keyed.get("permissionmode") + if mode is not None and str(mode).strip().strip("'\"") not in ALLOWED_AGENT_PERMISSION_MODES: + errors.append( + f"{rel}: permissionMode {mode!r}. An agent's mode applies to every call it makes, " + "and this one removes prompts the allowlists here assume; use one of " + f"{sorted(ALLOWED_AGENT_PERMISSION_MODES)} or omit the key." + ) + for key, why in ( + ("hooks", "runs its command with no prompt whenever its event fires inside the agent"), + ("mcpservers", "connects a server whose tools no allowlist here has reviewed"), + ): + if key in keyed: + errors.append(f"{rel}: frontmatter `{key}`: an agent-scoped entry {why}.") if "tools" not in data or not _tool_entries(data.get("tools")): errors.append( f"{rel}: no `tools:` key - an agent without one gets every tool, including " @@ -613,7 +666,8 @@ def main() -> int: return 1 print( "security_guards: OK (permissions allowlist, hooks allowlist, gitignore rules, " - "package manifests, no tracked node_modules, skill/command allowed-tools, agent tools)" + "package manifests, no tracked node_modules, skill/command allowed-tools, " + "load-time commands, agent tools and modes)" ) return 0