fix: deny mutating tailscale subcommands so the agent cannot widen its own network exposure

[GoZippy]

Problem / Motivation

No DeniedCommandRule in BUILTIN_DENIED_RULES names tailscale. Putting the
dashboard on the tailnet is a governed action — tailnet_serve.publish() is
owner-only, SEL-audited, and honours capabilities.tailnet_origin — but that
governance lives at the dashboard/CLI seam. The raw tailscale CLI sits below
it, and the agent has a bash tool. So from bash the agent can run
tailscale serve to publish the dashboard, tailscale funnel to expose a local
service to the public internet, or tailscale up/set to enable an SSH
server or advertise this node as an exit/subnet router — every one of them
bypassing publish() and its governance pin.

Why it matters

"The agent can never widen its own network exposure" is a security invariant the
governed publish path is built to uphold, and today a single bash call defeats
it. tailscale funnel is the sharpest case: it makes a local service reachable
from the open internet, with no governance gate in the path. On Windows there is
no OS sandbox beneath the bash tool to catch it either.

What changed (motivation → approach → change)

Root cause. Governance for tailnet exposure is enforced at publish(), not at
the command layer, and the denied-command gate — the one place that does sit on
the agent's bash — had no rule for tailscale.

Approach. Add a network-exposure denied-command family that blocks the
mutating tailscale subcommands at the same hooks.py PreToolUse gate as every
other denied command, while keeping the read subcommands the status card and any
diagnostic tooling rely on. Reuse the existing linearized flag-run idiom
(tailscale(?:\s+--?…)*\s+<verb>) so _linearize_deny_pattern keeps the
patterns ReDoS-safe, exactly like the AWS and self-protection rules. A
(?!\s+status) carve-out on serve/funnel preserves serve status /
funnel status.

Change. Three rules (category network-exposure):

• network-exposure-tailscale-serve — tailscale serve mutations (publish/
  withdraw); serve status stays allowed.
• network-exposure-tailscale-funnel — tailscale funnel (public-internet
  exposure); funnel status stays allowed.
• network-exposure-tailscale-node-mutate — up/set/login/logout/
  switch/cert.

Kiro Crew's own publish path is unaffected: tailnet_serve._run spawns
tailscale via subprocess.run directly, never through the hooks.py gate, so
kirocrew tailnet up and the Phone-access card keep working. Reads (status,
serve status, netcheck, ping, whois) stay allowed.

Tests

test/test_denied_commands_security.py::TestCatalog::test_tailscale_network_exposure_family
asserts the deny/allow matrix: 15 mutation forms are blocked (incl. an interposed
top-level flag and a sudo prefix) and 11 read/incidental forms stay allowed
(incl. serve status --json, funnel status, and the word inside an echo/
grep). The catalog count pins move 148 → 151 and the golden manifest gains the
three entries (append-only, verified byte-identical for the existing 148).

Manual verification

N/A — unit coverage is sufficient: the rules are pure DeniedCommandRule data
evaluated by the existing gate, and the deny/allow matrix exercises the regex
tier directly. The linearization/ReDoS suite in the same file already covers the
flag-run idiom these patterns reuse.

Related Issues

N/A — no existing issue; hardening found while wiring the Phone-access flow. The
invariant it upholds ("the agent cannot widen its own network exposure") is the
same one the governed publish() path and capabilities.tailnet_origin already
serve at the dashboard seam.

Pattern harvest

Rule candidate: a checklist item rather than a mechanical lint — a capability that is
governed at a high-level seam must also be denied at the command layer whenever a
raw CLI reaches the same effect below that seam. Tailnet exposure is governed at
tailnet_serve.publish() (owner-only, audited, capabilities.tailnet_origin),
but the tailscale CLI sits under it and the agent has a bash tool — so the
governance was reachable-around until these deny rules closed it. The
generalizable audit: for each governed capability, ask "is there a CLI/binary
that produces the same effect, and does a DeniedCommandRule cover it?" Not
expressible as a syntactic pattern (it is a semantic mapping between a governance
seam and a command surface), so it is guidance for the next capability review
rather than a semgrep rule.

Checklist

• At most two commits (one here), with a Conventional Commits title
• Tests added that fail before and pass after
• Spec updated in the same change (docs/system-specs/modules/security.md)
• Tightens the security posture (adds deny rules); no gate is widened
• No secrets, credentials, or real hostnames in the diff

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #7169 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7169: KEEP. Additive catalog growth with a numeric collision; both can land, in either order, with a count adjustment on the second. Files: test/test_denied_commands_security.py.
• This PR is OVERLAPPING with PR #7999. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8262: KEEP. Same behavior domain, opposite sides of the governance seam. 8262's carve-out is what keeps 7999's read path working; no conflict. Files: src/kiro_crew/dashboard/tailnet_serve.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
• This PR is OVERLAPPING with PR #8240. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8262: KEEP. Same file, disjoint hunks and disjoint rules. No coordination needed. Files: src/kiro_crew/security.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
• This PR is OVERLAPPING with PR #8283. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8262: KEEP. Two independent hardening families in the same catalog. Both are wanted; the only interaction is the mechanical count-pin/manifest collision, resolved by rebasing whichever merges second. Files: src/kiro_crew/security.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of b79ea3b4794a36a1da741c92dd65d94b4472fa84 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Deny-list enumerates today's mutating verbs on a third-party CLI, so the invariant "agent can never widen exposure" is only as current as the verb list.

Watch

• The rules are allow-by-default over tailscale's subcommand set: verbs not enumerated still reach the same effect — tailscale web (management UI through which serve/funnel/exit-node toggles are reachable) and tailscale drive share (exposes a directory over the tailnet) are un-denied today, and Tailscale adds subcommands over time. The stated invariant ("the agent must never be able to make a service publicly reachable") wants deny-by-default on tailscale with an enumerated read carve-out (status, serve status, funnel status, netcheck, ping, whois, version) — the exact same carve-outs already tested, strictly stronger against verb growth, and Kiro Crew's own publish path is unaffected either way since tailnet_serve._run bypasses the gate.
  Clears when: the family inverts to default-deny with read allow-outs, or web/drive are added with a stated decision that residual verbs are accepted.
• Description misstates the pins: "catalog count pins move 148 → 151 … byte-identical for the existing 148", but the base pin is 111 (diff moves it to 114) and the golden fixture has 111 entries — the prose was written against a different tree, which is exactly the staleness AGENTS.md's "never restate the denied-rule count in prose" rule warns about.
  Clears when: the PR description's counts match the diff (111 → 114) or are removed.

Suggestions

• If default-deny is adopted, the three rules collapse to one pattern plus one negative-lookahead read list — less catalog surface than three verb-enumerated rows.

[DESIGN-REVIEWED] b79ea3b

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of b79ea3b4794a36a1da741c92dd65d94b4472fa84 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification is complete. The governed publish() seam exists as claimed (tailnet_serve.py:523, capabilities.tailnet_origin checked before spawn), no existing rule or mechanism covers tailscale, the flag-run idiom (46 uses) and negative lookaheads (10 uses) are established catalog grammar, deny guidance is fail-soft for the new category, and the golden append is byte-identical for existing entries. Two premise risks survive: the description's counts (148→151) contradict the diff and base (111→114), and the invariant the PR quotes remains open via three tunneling siblings the repo itself names.

First-Principles-Verdict: CONCERNS

The invariant claimed — "the agent can never widen its own network exposure" — stays open via ngrok/cloudflared/ssh -R; this closes only the tailscale spelling. And the description's counts contradict the diff.

Not justified as shipped

4. Bare tailscale serve (a config dump, a read) is now refused too — undeclared; only a code comment admits the over-block, the description says reads stay allowed.

What this change ships

Intent: stop the agent's bash from bypassing the governed tailnet-publish seam by calling the raw tailscale CLI — a FIX of a verifiable governance gap (publish() is governed at tailnet_serve.py:523; zero deny rules named tailscale on base).

1. Agent bash can no longer run tailscale serve mutations; serve status still works — justified
2. Agent bash can no longer run tailscale funnel; funnel status still works — justified
3. Agent bash can no longer run tailscale up/set/login/logout/switch/cert — justified
4. Bare tailscale serve (config-dump read) is also refused — undeclared
5. New network-exposure refusal category in catalog and golden manifest — justified
6. Spec paragraph added to security.md — justified
7. Catalog pin 111→114 plus three append-only golden entries — justified

Watch

• Point patch against the quoted invariant: grepped ngrok|cloudflared|ssh.*-R in denied_rules.py → 0 rules, while tunnel/manager.py:14 names exactly ssh -R, cloudflared, ngrok as the OSS way to expose the dashboard — the same bash tool reaches the same public exposure funnel blocks. Deferral is fine; the description never says what is left.
  Clears when: a linked issue or follow-up rules cover the three named siblings, or the description scopes the invariant claim to the tailscale CLI.
• Description says "catalog count pins move 148 → 151 … verified byte-identical for the existing 148"; base pin is 111 (test_denied_commands_security.py:46) and the golden fixture has 111 entries, moving to 114. The "verified" claims were not run against this tree.
  Clears when: CI is green on HEAD b79ea3b and the description's counts are corrected.

[FIRST-PRINCIPLES-REVIEWED] b79ea3b

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed b79ea3b4794a36a1da741c92dd65d94b4472fa84 via the fork AI-review pipeline; updated in place on each push.

1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- src/kiro_crew/security/denied_rules.py:1456 -- shell expansion bypasses all new Tailscale denials
pattern=".*tailscale...\\s+serve..." and equivalent funnel/node-mutation patterns
t=tailscale; "$t" funnel 3000 -> hook checks unresolved text -> shell expands $t and exposes the service publicly
Anchor: residual/security
Fix: enforce mutating Tailscale subcommands structurally at execution time after executable resolution.
[BLOCK-MERGE] b79ea3b
[GPT-REVIEWED] b79ea3b

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

Fenced finding F1: the deny tier matches text via _deny_segment_views/_shell_tokens, which deliberately stops before variable expansion — its own residual note (__init__.py:1006) confirms rm -rf $HOME-style variable operands are "by construction not expanded here." So t=tailscale; "$t" funnel 3000 reaches the gate with tailscale only in the assignment, never adjacent to funnel, and the pattern (denied_rules.py:1456) does not match — the bypass is real. The condition to trigger it — a two-token variable assignment then use — is trivially producible by the agent's own bash, which is precisely the threat model here; it is not extreme, contradictory, or unproducible. I cannot establish any recovery path (no evidence the OS sandbox blocks the tailscale→tailscaled socket path; the rules exist at the hook gate precisely because it doesn't). The rarity record required for FLAG cannot be completed.

Harm rung: UNBOUNDED (widening the agent's own network exposure past a governed, owner-only ceiling — exposing a service to the public internet). No recovery path confirmed. FLAG's rarity argument fails: the bypass is trivially reachable.

[ADJUDICATION] b79ea3b total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] b79ea3b
[ADJUDICATION-FENCED] b79ea3b fenced=1 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/security/denied_rules.py:1456 -- The deny tier matches text without variable expansion (its own residual note pins this), so t=tailscale; "$t" funnel bypasses the pattern; the trigger is trivial agent-producible bash with no confirmed recovery path, so no rarity argument for FLAG can be made.
[GPT-ADJUDICATED-FENCED] b79ea3b

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed b79ea3b4794a36a1da741c92dd65d94b4472fa84 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

The sole candidate (node-mutate rule over-blocking incidental "tailscale up" prose) describes real behavior — .*tailscale…\s+(?:up|set|…)\b.* does match adjacent-token prose like echo "is tailscale up?" — but this is the codebase's documented, intentional over-block posture ("bare serve … over-blocked in the safe direction"), byte-identical in shape to the accepted .*aws-… family, and fails closed (a self-inflicted false denial, never a bypass). The read subcommands the status/doctor paths use are carved out and verified by the test; the discovery pass itself rated this low and could not establish a command that occurs in practice. It is an intended safe-direction tradeoff, not a defect at the required bar.

[OPUS-REVIEWED] b79ea3b

[bolichen97]

Rebased onto main 8534cbf75 by a maintainer as part of the 2026-09-08 open-PR audit. Old head 00b0360cc, new head b79ea3b47.

Four files conflicted:

• src/kiro_crew/security.py: deleted on main by refactor(security): split security.py into a package and drop path regex #9183, which split it into src/kiro_crew/security/. The three network-exposure rules were re-landed verbatim at the tail of BUILTIN_DENIED_RULES in src/kiro_crew/security/denied_rules.py.
• test/test_denied_commands_security.py: took main's rewritten file and re-applied only test_tailscale_network_exposure_family. The count pins are now derived on main, so only the single literal moved (111 to 114). The ~60 unrelated reformat hunks were dropped; the file is black-baselined and did not need them.
• test/fixtures/denied_commands_golden.json: took main's 111 entries and appended the three new ones.
• docs/system-specs/modules/security.md: kept main's rewritten self-protection paragraph, inserted your network-exposure paragraph above it.

Gates run locally on the changed files: black, isort, flake8, and test/test_denied_commands_security.py (705 passed) plus the security facade, regex-linearity and posture suites.

Please review the resolution. A maintainer push makes the maintainer the last pusher, so a second approver is needed under the repo's last-push rule. Reply if anything looks wrong.