Skip to content

feat(guardrails): add the identity-domain guard with domain classification and its kill switch - #9894

Merged
melodic-automation-lanes[bot] merged 2 commits into
mainfrom
work-resolve-issue/issue-9743-40135907565-1
Oct 24, 2026
Merged

melodic-automation-lanes[bot] merged 2 commits into
mainfrom
work-resolve-issue/issue-9743-40135907565-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Refs: #9743

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Adds block-cross-domain, the plumbing slice of the identity-domain guard. It runs last on the guardrails Bash|PowerShell row and reads ${CLAUDE_PLUGIN_DATA}/identity-domains.json. It compiles the map with jq once per change into a cache it reads back with builtins, classifies the session cwd into a domain or neutral, and parses a command only when its text names a configured root or marker. It never denies a call: results go to telemetry, and a malformed map is reported to the user once per session. With no map it reads and parses nothing, and block_cross_domain_enabled turns it off.

Remaining work

  • Measure the Bash|PowerShell row's added wall time with block-cross-domain on the Windows reference host, in S as docs/conventions/hook-budget describes. Report it in the PR and in the README's Hook budget accounting entry; that entry states k = 0 on the common path but has no measured cost.

Not verified

  • No test suite was executed in this run: Bash here runs only the lane wrappers, and check-affected does not pass --tests. That leaves unrun scripts/run-test-file.sh plugins/guardrails/hooks/block-cross-domain.test.sh, run-guards.test.sh (including the #4528 long-command case), abort-boundary.test.sh,
  • The strace pins in block-cross-domain.test.sh (0 processes with no map or a cached map; exactly 2 creations and 2 execve on a compile) are reasoned from the code, not measured. The Linux CI lane is where they first run.
  • typos was skipped by check-affected (typos is not installed here); CI's typos gate checks the changed files.
  • CI-only class audits for the new hook and test files were not run here: killswitch-hoist, silent-skips, hooks-description, hook-slow-shapes, hook-wiring-liveness, test-tmp-cleanup, pipefail-grep-q and discriminating-test-skips. They were checked by reading only, and are left to the PR's CI.

Fix

  • plugins/guardrails/hooks/block-cross-domain.sh: new guard. Kill switch hoisted above every source; fail-open abort boundary with 0 as the only chosen status; no-map exit before the library loads. A jq compiler validates the full map shape (domains.<name> with per-OS roots, owners, identity_mar
  • plugins/guardrails/hooks/block-cross-domain.test.sh: new contract suite. Covers the issue's required cases: no map gives empty stdout, exit 0 and no step traced; with a map, a command naming no root or marker returns before the parser runs (trace); the kill switch gives empty stdout; malformed maps
  • plugins/guardrails/hooks/hooks.json: block-cross-domain.sh added last on the Bash|PowerShell run-guards row; statusMessage and top-level description updated, with the description's list grammar fixed.
  • plugins/guardrails/hooks/guard-requires.sh: declares the guard's primed fields (.tool_input.command .tool_name .cwd).
  • plugins/guardrails/hooks/run-guards.test.sh: block-cross-domain added to the list of guards hooks.json must dispatch.
  • plugins/guardrails/.claude-plugin/plugin.json: new userConfig block_cross_domain_enabled (boolean, default true, title Block-cross-domain guard); description count corrected from the stale Thirteen to Fifteen, plus Tracks identity domains (494 of 500 characters).
  • plugins/guardrails/README.md: guard count fifteen; guard table row with config shape, behavior, telemetry, declared gaps and kill switch; note that it never blocks; Scope notes bullet on the data directory with a dated pointer to the plugins manifest reference; Hook budget accounting entry stating k
  • docs/catalog.md: generated guardrails line updated to the new manifest description, which validate-plugins requires.
  • .changes/guardrails/feat-9743-identity-domain-guard-3f9a7c21.md: minor changelog fragment (guardrails is in fragment mode, so plugin.json is not version-bumped). Written by hand because new-changelog-fragment refuses a detached HEAD.

Verification

The runner ran each check from the base copy against 35973d3:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • /home/runner/work/_temp/lane-checks/bin/check-affected: 10 passed, 6 skipped over 9 changed files (shellcheck, shell portability, markdown, exec-bit, changelog fragments, stale-base overlap, validate-plugins, spoke-plugin-root, plugin-options-docs, changelog parity). The first two runs failed: a she
  • /home/runner/work/_temp/lane-checks/bin/check-changelog-fragments: All 1 changelog fragment(s) under .changes/ are valid.
  • /home/runner/work/_temp/lane-verify/bin/cant-fail-scan --check --file plugins/guardrails/hooks/block-cross-domain.test.sh: PASS, no gating findings, 1 test file fully read (run before and after the review fixes).
  • read https: //code.claude.com/docs/en/plugins/manifest-reference: ${CLAUDE_PLUGIN_DATA} resolves to ~/.claude/plugins/data/<id>/, is created on first reference and kept across plugin updates, and is deleted when the plugin is uninstalled from the last place it is installed (the uninstall refer
  • Fresh-context review subagent over the whole diff (no review-venue.txt, so local review): 10 findings. Fixed: a word holding two separate roots named only one; jq's $ accepted a trailing newline in a domain name; undeclared heredoc gap; undocumented map-malformed form; overstated telemetry wording;
  • Grep: block-cross-domain appears at plugins/guardrails/hooks/hooks.json:134 in the Bash|PowerShell row; a case-insensitive search for melodic|kyle|sexton in the two new files finds 0 matches.
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40135907565

Left out: the items under Not verified.

Related

#9743

Lane-Activity: work-resolve-issue/resolve-issue
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 35973d304fc6d860753e1c5640d693e5d12ec50d. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate refuses its verifier verdict (verify:missing)

Lane-Activity: pr-unblock/resolve-conflicts
@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 1422ca11bfd76945bd522eece69a0a879dd53beb: 0 finding(s), 0 as inline comments.

I found no security issues in this pull request. I read the whole diff (997 lines) and the PR context.

Scope. No changed file matches a trusted path (trusted-actors, check-trusted-trigger, CODEOWNERS, .gitmodules). The PR adds block-cross-domain.sh to the guardrails plugin's Bash|PowerShell hook row, with its test, docs and a manifest option. The guard never denies a call. It reads ${CLAUDE_PLUGIN_DATA}/identity-domains.json, compiles it with jq, and reports to telemetry.

Manifest check (plugins/guardrails/.claude-plugin/plugin.json)

  • manifest description: pass. This field is in the repository's inert set.
  • manifest userConfig.block_cross_domain_enabled: pass. The new key is the guard's kill switch, and block-cross-domain.sh:226 reads it. It defaults to true, which is the protective value, and the guard denies nothing either way. No existing guard default is changed or removed.
  • marketplace.json is not touched. hooks.json adds block-cross-domain.sh last on the existing Bash|PowerShell run-guards.sh row. It is a repository-local script with no new external source, and it runs after the blocking guards on that row. That is not a manifest field, and I found no problem with it.

What I checked

  • Map injection. The jq compiler checks the map shape before writing any record. Every value must be a non-empty one-line string, with control characters including TAB rejected. Domain names are limited to [A-Za-z0-9_.-] and anchored with \A…\z. The tab-separated cache records therefore can't be split or forged through map content. $os is passed with --arg.
  • Cache. The cache file lives in the plugin data directory, which belongs to the user. The key is the map's full bytes plus the format and OS, and a stale or foreign cache is compiled again. A temp file in the same directory is moved into place. Changing the cache already needs write access to that directory, which gives nothing the user didn't have.
  • Command text. The attacker-controlled command text is only used as quoted [[ == *"$n"* ]] and case patterns, and in quoted ${t//"$n"/ } substitutions. Nothing is expanded, evaluated or used as a path to read. The jq call receives the map on stdin and the program as a fixed string.
  • Telemetry and notices. The telemetry envelope carries cwd as domain or neutral, plus the form and the command word. It never carries a domain name or path, and the test asserts this. The malformed-map notice goes only to the user channel and names keys and reasons, with a sanitised 300-character cap.
  • Failure handling. Every exit is 0, and the abort boundary fails open with 0. The guard sits last on the row, so a failure doesn't weaken the guards before it. The README states that this is a deterrent, not a sandbox, and lists the gaps: computed paths, scripts run by name, heredoc bodies and symlinks. Those are documented limits of a non-blocking classifier, not regressions.
  • PR context. The PR description and context contain no embedded instructions aimed at the review.

Instruction-surface deletions. The diff removes no standing instruction. The README and hooks.json description edits are additive.

Not security findings. guard-requires.sh changes mode from 100644 to 100755 in the same PR. That is a code-quality question, not a security one. The Windows wall-time measurement is listed as remaining work in the PR.

Spend. The change spends nothing beyond hook run time, and there is no billing or paid-plan change, so no cost hold.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 1422ca11bfd76945bd522eece69a0a879dd53beb: 0 finding(s), 0 as inline comments.

Code review: PR #9894, block-cross-domain guard (guardrails)

I found nothing a careful reviewer would block on. I read the full diff (.claude-lane/pr.diff, all 997 lines) and the PR context. I also read hook::bash_parse_segments in hook-utils.sh and its run-guards.sh wrapper. I had no shell, so I ran no tests. Everything below comes from reading the code. The PR's own verification claims (check-affected results, the fresh-context review, the strace pins) are author-claimed and I have not confirmed them.

What I checked

  • Never-deny contract. Every exit path in block-cross-domain.sh is 0. The kill switch sits above all sources, and a missing map or missing CLAUDE_PLUGIN_DATA exits before the library loads. A malformed map, a missing jq and an unreadable map each fail open, with a once-per-session notice.
  • jq compiler. The map is validated before any record is written, so a rejected map yields no partial set. The error reasons name keys, never values. Each malformed-map case in the test suite maps to a fail(...) message that exists in the compiler. The \A...\z anchors close the trailing-newline hole in domain names. Roots are validated for every OS but only this host's are emitted.
  • Cache. The key is written last and includes the format, the OS and the map bytes. A truncated or foreign cache recompiles. An empty record set round-trips. A jq that exits non-zero is not cached.
  • Prefilter and boundary matching. Needles are folded to lowercase with \ read as /, and need a non-[a-z0-9._-] character on each side. A root that names its domain spends its text, so nested roots resolve to the inner domain while an outer root named elsewhere in the text still counts. The ~, $HOME and %USERPROFILE% spellings are only added for roots below $HOME.
  • Parse callback. bcd::segment runs in-process through bash_parse_segments, including the run-guards.sh record-and-replay path, so BCD_NAMED survives. It is not lost to a subshell.
  • Wiring. hooks.json, guard-requires.sh, the run-guards.test.sh dispatch list, the README kill-switch and options tables, and the catalog line are all consistent.

Manifest items (plugins/guardrails/.claude-plugin/plugin.json)

  • manifest userConfig.block_cross_domain_enabled: pass. The field is new and boolean, with no options. The hook reads CLAUDE_PLUGIN_OPTION_BLOCK_CROSS_DOMAIN_ENABLED, so the key is read. No existing default changed and no field was removed.
  • manifest version bump: pass. The repo is in fragment mode and the fragment says bump: minor. A new guard with a new option is a minor change.
  • manifest description / marketplace entry: pass. The description now says "Fifteen" and "Tracks identity domains", which matches docs/catalog.md, the README and the hook count.

Closing issues: the PR body uses Refs: #9743 and the context file has no ## Closing issue section, so there are no closing-issue criteria to check. I did not read #9743 itself.

Noted, not counted as findings

  • The PR lists a remaining item: the Windows reference-host wall time for the row, in S, is unmeasured. The README's Hook budget entry says "not measured yet", so the docs are honest about it.
  • guard-requires.sh changes mode 100644 → 100755. The PR says the exec-bit check passed. I left it as an incidental change.
  • This change spends no billed resources, only CI runs.

No hold.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 1422ca11bfd76945bd522eece69a0a879dd53beb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants