Skip to content

Commit 37a5ed4

Browse files
committed
fix(security): route automatic skill isolation away from directory-snapshot
Reduce SEC-08 automatic exposure by changing skill isolation auto-routing: low-risk, default fallback, and WASM fallback paths now choose Docker instead of directory-snapshot. Directory-snapshot remains available only as an explicit compatibility mode and actual subagent preparation remains gated by acknowledge_no_os_isolation=True. This removes the silent no-OS-isolation route while preserving LOW-risk local usability through the existing docker_fallback_subprocess backend when Docker is unavailable. Non-low-risk Docker/WASM unavailability remains fail-closed. Risk register SEC-08 is reduced from H/M=6 to M/M=4 with test evidence; action register S-P2-10 records the hardening; reflective-risk note added. Action: S-P2-10 Roadmap-Status: unchanged Constraint: security hardening only; explicit directory-snapshot compatibility preserved and ack-gated; no Docker runtime internals changed Tested: 73 passed across skill executor/router, sandbox acceptance, isolation acknowledgment, subagent isolation; validate_docs_consistency 29/29 risk + 21/21 ticket Confidence: high
1 parent 6e4a1f1 commit 37a5ed4

8 files changed

Lines changed: 189 additions & 54 deletions

File tree

docs/generated/docs-inventory.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
Generated by `python3 scripts/generate_docs_inventory.py`.
77
Do not edit this file manually — regenerate instead.
88

9-
**Markdown files:** 615
9+
**Markdown files:** 616
1010

1111
| Path | Tier | Bytes | SHA256 (12) |
1212
| --- | --- | ---: | --- |
@@ -522,7 +522,7 @@ Do not edit this file manually — regenerate instead.
522522
| `retrospective/03-architecture-quality.md` | working | 27525 | `fd28fdd70665` |
523523
| `retrospective/04-ux-usability.md` | working | 26253 | `a9350bc35c4b` |
524524
| `retrospective/05-compliance-matrix.md` | working | 8285 | `cd2568b5eddb` |
525-
| `retrospective/06-action-register.md` | working | 32420 | `7cd6aa04a40e` |
525+
| `retrospective/06-action-register.md` | working | 33250 | `182a47ab5b31` |
526526
| `retrospective/automation-plan.md` | working | 18716 | `0e0027c4b1c5` |
527527
| `retrospective/README.md` | working | 8718 | `d93f2a71f6b3` |
528528
| `retrospective/review-system.md` | working | 15180 | `61db6643e4aa` |
@@ -538,6 +538,7 @@ Do not edit this file manually — regenerate instead.
538538
| `reviews/dynamic-skill-critical-questioning-2026-06-05.md` | archive | 10579 | `7924fed20806` |
539539
| `reviews/project-state-critical-questioning-2026-06-04.md` | archive | 7340 | `78b9b54c3a9c` |
540540
| `reviews/runner-tool-decision-split-risk.md` | working | 2978 | `597a932c0ae6` |
541+
| `reviews/sec08-skill-routing-risk.md` | working | 5072 | `07ec11cbddd5` |
541542
| `reviews/sec09-sec15-multisig-hardening-risk.md` | working | 11076 | `ae895567bb02` |
542543
| `reviews/security-risk-assessment-2026-06-02.md` | archive | 24112 | `4c9e2e00d001` |
543544
| `reviews/seven-control-loops-critical-questioning-2026-06-05.md` | archive | 7531 | `ae1e34b8369d` |
@@ -552,7 +553,7 @@ Do not edit this file manually — regenerate instead.
552553
| `security/path-and-root-threat-scenarios-2026-06-02.md` | archive | 1270 | `bb44fc81a366` |
553554
| `security/phase-0-trust-repair-risk-brief-2026-06-04.md` | archive | 5801 | `b123bdc43b26` |
554555
| `security/rate-limiting.md` | working | 2409 | `dfb2d06f42b2` |
555-
| `security/risk-register-and-threat-model-2026-06-02.md` | archive | 67633 | `ed4549b183fb` |
556+
| `security/risk-register-and-threat-model-2026-06-02.md` | archive | 68654 | `527c22b6b8b3` |
556557
| `security/severity-calibration-rubric.md` | working | 2828 | `8af4af679012` |
557558
| `security/shell-safety.md` | working | 7163 | `7885c6a889dc` |
558559
| `skill-governance.md` | working | 12561 | `44967dfcc037` |

docs/retrospective/06-action-register.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ Legend: ✅ Done (committed) · 🟡 In progress · ⬜ Not started · 🔵 Alre
5555
| S-P2-7 | Security | Reconcile the remaining resolved-but-`OPEN` risk-register rows (DS-04, SC-01, SC-03) with current code/lock state. | `docs/security/risk-register-and-threat-model-2026-06-02.md:156,163,165` (+ remediation/sprint/compliance cross-refs) | The three rows and all their cross-references show FIXED with test/code/lock evidence; `validate_docs_consistency.py` risk-register coverage rises to 29/29. || This session — DS-04 → FIXED (`audit_trail` placeholder removed; RunStore authoritative; `resume.py:56-82,416`; tests `test_suspension_data_no_audit_trail`, `test_suspension_data_has_no_audit_trail_field`; commit `08cda72`); SC-01 → FIXED (alpha OTel pins gone from `uv.lock`; `[tool.uv]` overrides force stable `>=1.12.0,<2.0.0`; commit `bad05d1`); SC-03 → FIXED (`aiohttp`/`mcp` absent from `uv.lock`, not imported in `teaagent/`; commit `bad05d1`); coverage 26/29 → 29/29 |
5656
| S-P2-8 | Security | Close SEC-09: consolidate the duplicated multi-sig approval hash into one canonical helper and bind the unique `request_id` so a captured peer signature cannot be replayed. | `teaagent/approval/_multisig_crypto.py`; `teaagent/policy.py:295-309`; `teaagent/approval/manager.py:540-554` | One `generate_approval_hash` helper; both callers delegate; the signed hash binds `request_id` and drops the wall-clock time bucket; no `time.time() / 3600` remains; tests added. || This session — `generate_approval_hash` in `teaagent/approval/_multisig_crypto.py`; `policy.py`/`approval/manager.py` delegate; `tests/test_sec_tier1_hardening.py` (`test_policy_approval_hash_binds_request_id`, `test_approval_hash_is_single_source_of_truth`, `test_approval_hash_has_no_wallclock_time_bucket`) |
5757
| S-P2-9 | Security | Close SEC-15: add a runtime guard that fails closed when dev-hash signatures are requested while a signature relay is non-loopback (previously advisory-only via `config_lint`). | `teaagent/approval/_multisig_crypto.py`; `teaagent/policy.py:319-326`; `teaagent/approval/manager.py:562-569` | `resolve_allow_dev_signatures` raises `ConfigError` for dev signatures + non-loopback relay; enforced before broadcast in both quorum paths; advisory `config_lint`/`selftest` retained; tests added. || This session — `resolve_allow_dev_signatures` in `teaagent/approval/_multisig_crypto.py`; `tests/test_sec_tier1_hardening.py` (`test_dev_signatures_rejected_on_non_loopback_relay`, `test_env_dev_signatures_rejected_on_non_loopback_relay`, `test_collect_peer_signatures_fails_closed_before_wan_broadcast`) |
58+
| S-P2-10 | Security | Reduce SEC-08 automatic exposure: stop routing low-risk/default/WASM-fallback skill isolation to `directory-snapshot`; keep directory-snapshot explicit-only and ack-gated. | `teaagent/skill_router.py:55-63,151-167,210-265`; `teaagent/subagents/_isolation.py:288-298` | Auto skill/subagent isolation chooses Docker by default; explicit `directory-snapshot` still maps for compatibility but requires `acknowledge_no_os_isolation=True`; tests cover low-risk routing, WASM fallback, mapping, and ack rejection. || This session — `SkillRouter` low/default/WASM fallback now returns `SandboxType.DOCKER`; `isolation_for_sandbox_type(WASM)` maps to `docker`; tests: `tests/test_skill_router.py`, `tests/acceptance/test_sandbox_enhancement_flow.py`, `tests/test_isolation_acknowledgment_flag.py` |
5859
| G-P2-1 | Governance | Gate use of `directory-snapshot` isolation for untrusted content behind an `--acknowledge-no-os-isolation` flag. | `teaagent/subagents/_isolation.py:287-298` | Flag exists; isolation gated. || `acknowledge_no_os_isolation` is required for untrusted directory-snapshot isolation; `tests/test_subagent_isolation.py` |
5960
| G-P2-2 | Governance | Remove `preapproved_call_ids` after the deprecation window and standardize on payload digest. | `teaagent/policy.py:115-121`; `teaagent/approval_manager.py:953-968` | No `preapproved_call_ids` references remain in approval/decision paths; CLI `--approve-call-id` ignored. || `25a234e``--approve-call-id` deprecated/ignored; docs updated; `--approve-scoped TOOL:SHA256` is the standard |
6061
| G-P2-3 | Governance | Add iteration and tool-call warnings alongside `BudgetMonitor`'s existing cost monitoring. | `teaagent/budget_monitor.py:74-106` | Warnings added. || Iteration and tool-call threshold warnings are implemented; `tests/test_budget_monitor_warnings.py` |
@@ -79,14 +80,14 @@ Legend: ✅ Done (committed) · 🟡 In progress · ⬜ Not started · 🔵 Alre
7980
| --- | --- | --- |
8081
| P0 | 8 | Security 3, Architecture 2, UX 2, Governance 1 |
8182
| P1 | 18 | Security 4, Governance 3, Architecture 5, UX 6 (including cross-dimensional items) |
82-
| P2 | 26 | Security 9, Governance 5, Architecture 7, UX 5 |
83-
| **Total** | **52** | |
83+
| P2 | 27 | Security 10, Governance 5, Architecture 7, UX 5 |
84+
| **Total** | **53** | |
8485

8586
### Status Summary
8687

8788
| Status | Count | Items |
8889
| --- | --- | --- |
89-
| ✅ Done (verified) | 48 | S-P0-1, S-P0-2, S-P0-3, A-P0-1, A-P0-2, U-P0-1, U-P0-2, G-P0-1; S-P1-1, S-P1-2, S-P1-3, S-P1-4, G-P1-1, G-P1-2, G-P1-3, A-P1-1, A-P1-2, A-P1-3, A-P1-4, A-P1-5, U-P1-1, U-P1-2, U-P1-3, U-P1-4, U-P1-5, U-P1-6; S-P2-1, S-P2-4, S-P2-5, S-P2-6, S-P2-7, S-P2-8, S-P2-9, G-P2-1, G-P2-2, G-P2-3, G-P2-5, A-P2-1, A-P2-2, A-P2-3, A-P2-5, A-P2-6, A-P2-7, U-P2-1, U-P2-2, U-P2-3, U-P2-4, U-P2-5 |
90+
| ✅ Done (verified) | 49 | S-P0-1, S-P0-2, S-P0-3, A-P0-1, A-P0-2, U-P0-1, U-P0-2, G-P0-1; S-P1-1, S-P1-2, S-P1-3, S-P1-4, G-P1-1, G-P1-2, G-P1-3, A-P1-1, A-P1-2, A-P1-3, A-P1-4, A-P1-5, U-P1-1, U-P1-2, U-P1-3, U-P1-4, U-P1-5, U-P1-6; S-P2-1, S-P2-4, S-P2-5, S-P2-6, S-P2-7, S-P2-8, S-P2-9, S-P2-10, G-P2-1, G-P2-2, G-P2-3, G-P2-5, A-P2-1, A-P2-2, A-P2-3, A-P2-5, A-P2-6, A-P2-7, U-P2-1, U-P2-2, U-P2-3, U-P2-4, U-P2-5 |
9091
| 🟡 In progress | 0 ||
9192
| ⬜ Not started | 0 ||
9293
| 🔵 Already existed | 4 | S-P2-2, S-P2-3, G-P2-4, A-P2-4 |
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
# Reflective-Risk Report: SEC-08 Skill Routing Hardening
2+
3+
Date: 2026-07-01. Action ID: S-P2-10.
4+
Security surface: skill/subagent isolation routing and `directory-snapshot` compatibility mode.
5+
6+
## Goal
7+
8+
Reduce SEC-08 automatic exposure by removing silent auto-selection of
9+
`directory-snapshot` for skill isolation. `directory-snapshot` is a workspace file
10+
copy only; it does not provide OS/process isolation. It remains available only as
11+
an explicit compatibility mode, and actual subagent preparation still requires
12+
`acknowledge_no_os_isolation=True`.
13+
14+
## Stakeholders
15+
16+
Owner-operator, skill authors, subagent users, and maintainers of the skill
17+
execution/routing contract.
18+
19+
## Assets at Risk
20+
21+
Host process namespace, host filesystem outside the workspace (`~/.ssh`, `/etc`,
22+
`/proc`), environment variables, and operator trust in the term "sandbox".
23+
24+
## Threat Model
25+
26+
A skill or subagent path is auto-routed to `directory-snapshot`; the operator sees
27+
an isolation-like label but the code runs as a host process and can read host
28+
paths. Prior warning/ack gates protected direct subagent preparation, but
29+
`SkillRouter` still used `directory-snapshot` as the low-risk/default/WASM-fallback
30+
auto choice.
31+
32+
## Assumption Audit
33+
34+
- ASSUMPTION: `prepare_subagent_isolation` already blocks direct
35+
`directory-snapshot` use without an explicit ack. VERIFIED at
36+
`_isolation.py:288-298` and by `test_directory_snapshot_without_acknowledgment_is_rejected`.
37+
- ASSUMPTION: auto skill routing was the remaining silent directory-snapshot path.
38+
VERIFIED by `skill_router.py` and tests that previously asserted low-risk skills
39+
used directory-snapshot.
40+
- ASSUMPTION: using Docker for auto routing preserves local LOW-risk usability on
41+
machines without Docker. VERIFIED by `execute_skill`: LOW-risk Docker
42+
unavailability keeps the existing `docker_fallback_subprocess` backend, but the
43+
routing no longer labels the run as directory-snapshot isolation. Non-low-risk
44+
remains fail-closed on Docker unavailability.
45+
46+
## Evidence Check
47+
48+
- `SkillRouter._auto_select_sandbox` now returns `SandboxType.DOCKER` for LOW risk
49+
and default fallback.
50+
- WASM unavailable/incompatible fallback returns Docker, not directory-snapshot.
51+
- `isolation_for_sandbox_type(SandboxType.WASM)` maps to Docker for subagent
52+
isolation wrappers because `prepare_subagent_isolation` has no WASM process
53+
boundary.
54+
- Explicit `SandboxType.DIRECTORY_SNAPSHOT` still maps to `directory-snapshot` for
55+
compatibility and is governed by the existing ack gate.
56+
- Tests: `tests/test_skill_router.py`,
57+
`tests/acceptance/test_sandbox_enhancement_flow.py`,
58+
`tests/test_isolation_acknowledgment_flag.py`,
59+
`tests/test_subagent_isolation.py`,
60+
`tests/acceptance/test_subagent_directory_snapshot_isolation_flow.py`.
61+
62+
## Authority / Tool Boundary
63+
64+
In scope: routing defaults in `teaagent/skill_router.py`, acceptance/unit tests,
65+
risk register, action register, and docs inventory. Out of scope: removing
66+
`directory-snapshot` entirely, changing Docker runtime internals, or changing the
67+
LOW-risk Docker fallback subprocess policy.
68+
69+
## Failure Modes
70+
71+
- Docker unavailable for LOW-risk skill: unchanged fallback subprocess path, now
72+
labelled as `SandboxType.DOCKER` with backend `docker_fallback_subprocess`.
73+
- User explicitly requests directory-snapshot: compatibility path remains, but
74+
direct subagent preparation still requires `acknowledge_no_os_isolation=True`.
75+
- Non-low-risk skill without Docker/WASM: existing fail-closed behavior remains.
76+
77+
## Worst-case Scenario
78+
79+
A low-risk skill still executes locally because Docker is unavailable. This is not
80+
represented as directory-snapshot isolation anymore; non-low-risk remains
81+
fail-closed. Full elimination of subprocess fallback is a separate breaking UX
82+
change and should have its own ADR.
83+
84+
## Safe Dry-run Plan
85+
86+
Run focused skill routing/isolation tests and docs validator. No network, no
87+
credentials, no destructive operations.
88+
89+
## Rollback Plan
90+
91+
`git revert` the commit. The change is routing/test/docs only; no persisted-state
92+
or migration concern.
93+
94+
## Bounded Execution
95+
96+
Single commit. No production deployment. No external I/O.
97+
98+
## Audit Log Plan
99+
100+
No runtime audit event added or removed. Risk/action register rows record the
101+
change for governance audit.
102+
103+
## Human Review Required
104+
105+
Yes for security posture semantics, though the changed code path is not listed in
106+
`high_risk_paths.yaml`.
107+
108+
## Human Approval Gate
109+
110+
Owner instructed: "Fix all known high risks one by one" and then "continue".
111+
This is the next bounded SEC-08 hardening slice after SEC-09/SEC-15.
112+
113+
## Acceptance Criteria
114+
115+
- Auto LOW/default/WASM-fallback skill routing chooses Docker, not directory-snapshot.
116+
- Explicit directory-snapshot still maps for compatibility and remains ack-gated.
117+
- Focused tests pass.
118+
- SEC-08 risk register row reflects reduced likelihood (M/M=4) with evidence.
119+
120+
## Go / No-go Decision
121+
122+
**GO** — small, reversible hardening that removes the silent directory-snapshot
123+
auto route while preserving explicit compatibility and existing LOW-risk fallback
124+
semantics.

0 commit comments

Comments
 (0)