feat(security): add deterministic public-surface scrub Claude Code mod (part of #1497)

[Kaap10]

Summary

• Adds a zero-token Claude Code mod in plugins/magpie-security/hooks/ that intercepts Bash commands (git commit, git tag, gh pr create, gh pr edit, gh pr review, gh pr comment, gh issue comment, gh api) and guards public surfaces against accidental security embargo leaks.
• Intercepts and parses commit messages (-m, -am, -sm, -sam, -F), attached short flags (-m"...", -Fpath), PR descriptions/comments, heredocs, and shell pipelines.
• Evaluates composite multi-command pipelines per subcommand across all shell separators (;, &&, ||, |, \n).
• Implements strict regex matching for forbidden embargo terms aligned with plugins/magpie-security/skills/issue-fix/implementation-plan.md:31-43 (CVE references, vulnerability wording, security fix phrasing, and vulnerability classes like SSRF, XSS, CSRF, SQL injection, path traversal, RCE, privilege escalation, auth bypass).
• Dynamically resolves upstream_repo and tracker_repo from <project-config>/project.md via resolveProjectConfig(), scoping the scrub strictly to upstream_repo and exempting tracker_repo per subcommand without hardcoded naming conventions.
• Enforces deterministic fail-closed behavior on unreadable message files, --amend/--fill, and indeterminate dynamic inputs ($VAR, $(...), stdin -). Resolves -F / --body-file relative to git -C <dir> without Node built-in imports.
• Provides a human-set options.env.MAGPIE_ALLOW_EMBARGO_TERMS=1 environment escape hatch for legitimate post-coordinated-disclosure maintenance (in-command bypasses removed).
• Passes clean behavioral changes through with zero token consumption and zero latency.

Type of change

• Skill change (.claude/skills/<name>/) — eval fixtures updated below
• Tool / bridge contract (tools/<system>/*.md)
• Python package (tools/*/ with pyproject.toml)
• Groovy reference impl
• Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
• Documentation (docs/, README.md, CONTRIBUTING.md)
• Project template (projects/_template/)
• CI / dev loop (prek, workflows, validators)
• Other: Claude Code mod hooks (plugins/magpie-security/hooks/)

Test plan

• Ran claude plugin validate --strict plugins/magpie-security (Validation passed, minimal API surface: $.fs.read).
• Ran claude plugin test plugins/magpie-security (47 pass, 0 fail).
• Ran claude plugin test plugins/magpie-utilities and plugins/magpie-setup for regression testing (83 total tests pass across repo).
• Verified prek linters (end-of-file-fixer, trailing-whitespace, mixed-line-ending) pass cleanly.

RFC-AI-0004 compliance

• HITL — any new mutation is gated on explicit user confirmation
• Sandbox — minimal declared surface area ($.fs.read), no node:fs or node:path built-in imports, deterministic execution.
• Vendor neutrality — mod is strictly additive; existing issue-fix skill workflow and Markdown verification checklist remain completely untouched for non-Claude harnesses.
• Conversational + correctable — agentic-override path documented if behaviour is adopter-tunable
• Write-access discipline — no autonomous outbound messages; drafts only, sent on confirmation
• Privacy LLM — zero model calls made; operates 100% locally on the host machine.

Linked issues

• Part of Explore Claude Code mods across the skill families #1497
• Follow-up to feat(utilities): add zero-token /magpie-skills Claude Code mod command (part of #1497) #1667

Notes for reviewers (optional)

• Mirrors the mod hook and testing patterns proven in feat(setup): add zero-token setup drift check Claude Code mod (part of #1497) #1552, feat(setup): add zero-token /magpie-status Claude Code mod command (part of #1497) #1601, and feat(utilities): add zero-token /magpie-skills Claude Code mod command (part of #1497) #1667.
• Operates as a gating middleware hook on tool.call for Bash tools, halting execution with actionable remediation if unredacted embargo terms appear in public-surface commands.

[Kaap10]

Implemented zero-token deterministic public-surface embargo scrub mod.
Ready for review, please let me know if any adjustments or changes are needed!

[potiuk]

Thanks for picking up a deterministic scrub — the idea is right, but as written the agent can switch the guard off from inside the command it is checking, several ordinary shell shapes get a message past the parser, and the scope (every commit in every repository) is wider than the rule it enforces.

Outputs are probabilistic; gates are deterministic (plugins/magpie-security/hooks/public-surface-scrub.ts:237)

Skills produce drafts. Tool calls enforce schemas. Humans or deterministic checks decide whether a draft becomes state. Probabilistic at the input, deterministic at every state change. The boundary never blurs, even when the draft looks reliable enough to short-circuit the gate.

    /(?:^|\s)MAGPIE_ALLOW_EMBARGO_TERMS=1(?:\s|$)/.test(commandLine) ||
    commandLine.includes('--allow-embargo-terms')

The escape hatch can be switched on by the very command it is meant to stop. An agent that hits the block (and whose reason text tells it how to proceed) only has to prefix MAGPIE_ALLOW_EMBARGO_TERMS=1 or append --allow-embargo-terms anywhere in the command, including inside the commit message or a trailing # comment, and the guard passes it. --allow-embargo-terms is also not a real git or gh flag, so a literal use makes the underlying command fail. A deterministic gate has to be outside the agent's reach.

Suggested fix: honour only options.env (the hook process environment, set by the human before the session), drop the command-line forms, and leave the override out of the blocking message, so the agent is told to ask the maintainer rather than how to bypass the guard.

Flagged by both the primary and adversarial reviewers.

Writing a literal project value directly into a skill is a refactor bug (plugins/magpie-security/hooks/public-surface-scrub.ts:88)

Concretely, gh issue view <N> --repo <tracker> means "substitute
<tracker> for <project-config>/project.md → tracker_repo before
running this". Writing a literal project value directly into a skill is a
refactor bug — skills must stay project-agnostic so swapping projects is
a config change, not a code change.

    repo.endsWith('-s') ||
    repo.includes('-s/') ||
    repo.includes('/private') ||
    repo.includes('-private') ||
    repo.includes('security-tracker')

The private-tracker exemption is a naming guess, and it fails open: any public repository whose name ends in -s or contains -private / /private / security-tracker is exempt, so security wording on that public surface passes unchecked. A tracker named any other way (for example apache/foo-security) gets blocked instead. Only the airflow-s/airflow-s convention is encoded. It also exempts the whole command: isPrivateTrackerTarget searches the entire string and line 118 returns early, so gh pr view 1 -R org/private-tracker; gh pr comment 2 -R apache/airflow -b "CVE-… SSRF" passes although the second command targets the public upstream (found by codex).

Suggested fix: read tracker_repo (and upstream_repo) from the resolved <project-config>/project.md and exempt exactly that slug, and nothing when it cannot be resolved; decide the exemption per sub-command, not for the whole script.

Flagged by both the primary and adversarial reviewers.

This is the single forbidden-term list for every public surface of this skill. (plugins/magpie-security/hooks/public-surface-scrub.ts:128)

This is the single forbidden-term list for every public surface of this skill.

  if (/(?:^|[;&|]\s*)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*commit(?:\s|$)/i.test(trimmed)) {

The list governs the public surfaces of the security-issue-fix flow, the <upstream> fix PR before its advisory ships. The hook applies it to every git commit, gh pr create and gh pr comment in every repository the plugin is loaded in. In this repository that blocks ordinary work: \bCVE\b (line 26, case-insensitive) matches fix(generate-cve-json): …, and vulnerability / security advisory appear in many routine commit messages and PR descriptions here and in any adopter's own docs.

Suggested fix: scope it to the configured upstream_repo (from project.md), and preferably to an active fix (a branch or marker the issue-fix skill sets), and let every other repository pass.

This is the single forbidden-term list for every public surface of this skill. (plugins/magpie-security/hooks/public-surface-scrub.ts:128)

This is the single forbidden-term list for every public surface of this skill.

  if (/(?:^|[;&|]\s*)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*commit(?:\s|$)/i.test(trimmed)) {

Several ordinary shell shapes get a leaking message past the parser (the first four found by codex; each traced in the code):

• a newline between commands: the detectors accept only start-of-string or ;, &, |, so git status + newline + git commit -m "CVE-… SSRF" is not recognised;
• an attached message, git commit -m"CVE-… SSRF" (and an attached -F): the extraction requires = or whitespace after the flag;
• a commit followed by a PR: the git commit branch returns before the gh branches, so git commit -m "neutral" && gh pr create -t "CVE-… SSRF" checks only the commit message, and && does not trigger the whole-command scan;
• $(…), backticks or a variable in the message, or a piped body (… | git commit -F -): only the literal command text is scanned, and - is skipped;
• not covered at all: git commit --amend reusing a message, git tag -m, gh pr create --fill, gh pr edit --title/--body, gh pr review --body, gh issue comment on <upstream>, gh api … -f body=.

Suggested fix: split the command into sub-commands on every shell separator (newline included) and evaluate each one; accept attached short-option arguments; when a message comes from command substitution, a variable or stdin, scan the whole command and block when the published text cannot be determined; add the missing subcommands, with a test per shape.

Flagged by both the primary and adversarial reviewers.

Outputs are probabilistic; gates are deterministic (plugins/magpie-security/hooks/public-surface-scrub.ts:263)

Skills produce drafts. Tool calls enforce schemas. Humans or deterministic checks decide whether a draft becomes state.

      } catch {
        // Safe skip on unreadable file; pure regex inspection continues
      }

A message file the hook cannot read is skipped and the command is allowed, and the path is read relative to the session's working directory even after git -C <dir>, where git reads <dir>/<file> (found by codex). So git -C /upstream commit -F msg.txt with an embargoed msg.txt passes whenever the hook resolves the path differently from git.

Suggested fix: resolve -F / --body-file paths against the -C directory, and block (fail closed, naming the file) when a referenced message file cannot be read.

Flagged by codex (adversarial review); cross-checked.

Smaller observations

• See inline comments on plugins/magpie-security/hooks/public-surface-scrub.ts:28.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. After you've
addressed the points above and pushed an update, an Apache Magpie
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

More on how Apache Magpie handles maintainer review:
Contributing guide.

[Kaap10]

Thanks for the thorough review @potiuk. I’ve addressed all findings: the override is now restricted strictly to options.env (removing in-command bypasses and guidance text), and repository scoping resolves upstream_repo/tracker_repo dynamically from project.md evaluated per subcommand.

The parser now splits multi-command pipelines across all shell separators (including newlines), supports attached short flags (-m"...", -Fpath), and enforces deterministic fail-closed behavior on unreadable files, --amend/--fill, and indeterminate dynamic inputs ($VAR, $(...), stdin -).

All 47 unit tests pass cleanly covering each of these review shapes, and plugin validation passes strict static analysis without Node runtime built-in dependencies.

[potiuk]

Thanks for the round of fixes — the in-command bypass is gone, the tracker / upstream slugs now come from project.md, each sub-command is judged on its own, and unreadable message files and dynamic input now fail closed. Two things still block it: the mod keeps its own copy of the term list instead of using the canonical one, and the gate still fails open in several shapes.

On my earlier threads:

• Message-file / -C handling — fixed; resolving.
• In-command bypass, tracker exemption, ordinary shell shapes — answered on their own terms, but the replacements open the gaps in the inline comments (the escape hatch never reaching the hook, cd … &&, git -c, escaped quotes, --repo read from a body, directory-name heuristics).
• Scope, and the bare advisory — folded into the term-list comment: deriving the list from DEFAULT_TERMS settles both.

On the architecture question in the first inline comment, moving this into tools/agent-guard as a Python guard is worth weighing before more work goes into the TypeScript version: it would import the canonical list directly and run on every harness agent-guard supports.

Smaller, not blocking: in upstream scope this fails closed on the common git commit -m "$(cat <<'EOF' … EOF)" form; that is safe, but the block message could say -F <file> is the expected shape.

single forbidden-term list (plugins/magpie-security/hooks/public-surface-scrub.ts:28)

This is the single forbidden-term list for every public surface of this skill. 5g, Step 7, Step 9 and the Guardrails check against it rather than keeping their own copies.

export const FORBIDDEN_TERM_RULES: ReadonlyArray<{ name: string; pattern: RegExp }> = [

Derive the list from magpie_vcs.publication.DEFAULT_TERMS (generated data file plus a sync check), or move the guard into tools/agent-guard where it can import the canonical list.

Humans or deterministic checks decide whether a draft becomes state. (plugins/magpie-security/hooks/public-surface-scrub.ts:726)

Humans or deterministic checks decide whether a draft becomes state.

const evaluation = await evaluatePublicSurfaceCommand(command, {
        readFile: async (filePath: string) => {

Pass env: $.env, and add a register()-level test with MAGPIE_ALLOW_EMBARGO_TERMS=1.

The boundary never blurs (plugins/magpie-security/hooks/public-surface-scrub.ts:624)

The boundary never blurs, even when the draft looks reliable enough to short-circuit the gate.

} else if (projectConfig.isTrackerCheckout) {
        // Bare git command in the tracker repository itself without upstream context
        continue;

Track cd <dir> across sub-commands; block when the target repository cannot be determined; resolve config in the upstream clone instead of passing everything.

Probabilistic at the input, deterministic at every state change. (plugins/magpie-security/hooks/public-surface-scrub.ts:293)

Probabilistic at the input, deterministic at every state change.

const cwdMatch = trimmed.match(/(?:^|\s)git\s+.*?-C(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/i);

Tokenise argv with a shell-word parser; read -C only from git's global options, case-sensitively; same for -R/--repo.

Smaller observations

• See inline comments on plugins/magpie-security/hooks/public-surface-scrub.ts:613, plugins/magpie-security/hooks/hooks.json:5.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. After you've
addressed the points above and pushed an update, an Apache Magpie
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

More on how Apache Magpie handles maintainer review:
Contributing guide.

[potiuk]

This is a second, hand-kept copy of the term list, and it has already drifted from the canonical one (DEFAULT_TERMS in tools/vcs/src/magpie_vcs/publication.py, which security-management issue-fix scrub uses since #1712). All of these pass here but are caught there: CVEs, vulnerable, a bare advisory (my earlier thread on this line), security fixes, exploit, cveprocess.apache.org, directory traversal, arbitrary code execution, buffer overflow, use-after-free. It also lacks the reporter-name and verbatim-private-text checks. Please don't keep a copy: either generate the list from DEFAULT_TERMS (a data file plus a sync check in prek), or move the guard into tools/agent-guard as a Python PreToolUse guard that imports it directly, which would also make it work on every harness agent-guard supports, not only Claude Code.

[potiuk]

register() passes only readFile here, never env, so the MAGPIE_ALLOW_EMBARGO_TERMS escape hatch can never fire in production, while the block message still tells the maintainer to "set the environment bypass". The test that covers it calls evaluatePublicSurfaceCommand directly, so it does not notice. Please forward $.env and add a test through register().

[potiuk]

The gate passes exactly where a fix commit is most likely made. (1) A session in the tracker checkout: cd ../<upstream> && git commit -m "Fix CVE-2026-1234" passes, because cd is not followed and a bare git here is skipped. (2) A session started in the upstream clone has no project.md, so every command is allowed. When the target repository cannot be determined, this should block, not pass.

[potiuk]

Regex matching over the raw command lets three shapes through: the /i flag makes -c count as -C, so git -c user.name=x commit -m "… CVE-2026-1234 …" is treated as not-upstream; [^"]* stops at an escaped quote (-m "Refactor \"parser\" for CVE-…" captures only Refactor \); and a --repo <tracker> mentioned inside a -b body is taken as the target repository and exempts the command. Tokenising the sub-command into argv once and reading flags from that would close all three.

[potiuk]

Upstream vs tracker is decided from substrings of the directory name, so git -C ~/src/af commit … counts as neither and passes, and any path containing the tracker slug is exempt. The git remote of the -C directory (or the upstream clone path from user.md) is the reliable signal. The upstreamRepo === 'apache/magpie' literal can go too: with no upstream/tracker configured the existing early return already covers this repository.

[potiuk]

This is the first mod in magpie-security and the PR touches no docs. docs/setup/secure-agent-setup.md should cover its trade-offs (it runs unsandboxed and reads any -F path via $.fs.read) and how the escape hatch is set, and the issue-fix skill should mention it next to the scrub step so a maintainer knows where a block came from.

[Kaap10]

will resolve CI shortly!

[Kaap10]

Thanks @potiuk! Addressed all review items:

• Derived forbidden terms directly from canonical magpie_vcs.publication.DEFAULT_TERMS with a pre-commit sync check.
• Added a POSIX shell-words parser, chained cd tracking across subcommands, remote/worktree fork detection, and fail-closed dynamic inputs.
• Forwarded env into evaluation with register-level tests, documented mod trade-offs in secure-agent-setup.md, and rebased onto main.

Ready for another look!