Skip to content

feat(guard): F1 confine — user-reachable via enable --confine + doctor-confine#111

Merged
m1ngshum merged 6 commits into
mainfrom
feat/guard-confine-cli
Jul 2, 2026
Merged

feat(guard): F1 confine — user-reachable via enable --confine + doctor-confine#111
m1ngshum merged 6 commits into
mainfrom
feat/guard-confine-cli

Conversation

@m1ngshum

@m1ngshum m1ngshum commented Jul 2, 2026

Copy link
Copy Markdown
Member

What

The final PR of the F1 arc — wires the confine engine (#110) to the enable path so it's actually usable.

  • mcpm guard enable --confine (bare → standard tier; --confine off disables) enrolls every unwrapped stdio server it wraps into an OS sandbox. The cli layer derives each server's ConfineProfile once (a single capturedAt → an identical content hash across every client that wraps it, so the spawn-time verify never mismatches → decide.ts row 3), merges them into ~/.mcpm/guard-confine.yaml, and hands the orchestrator a name → ConfineMarker map.
  • Orchestrator seam: the single wrapEntry call embeds --confine-profile-hash (+ --confine-required) via a new optional confineMarkers dep — the only behavioral change there.
  • mcpm guard disable removes the marker (unconfines); the lingering profile is harmless (spawn row 5b: no marker → not confined).
  • mcpm guard doctor-confine [--json] — read-only report of backend availability (platform + sandbox-exec presence) and enrolled servers (tier / net / require_confine), pointing at guard status for per-client wrap state.

Hybrid posture, surfaced honestly: when no OS backend is present (Linux/CI/Windows today), enable still enrolls + embeds markers but prints a loud notice that those servers run UNCONFINED until a backend exists (a require_confine server fails closed at spawn per the #110 decision table).

Verification

  • 1886 tests pass (+10: orchestrator marker-embed, computeConfineMarkers dedup/skip-url/skip-wrapped/merge, enable --confine end-to-end, doctor-confine JSON/text/empty) · tsc + pnpm build clean.
  • macOS dogfood on the built binary (isolated HOME, real sandbox-exec): enable --confine → the rewritten config entry's args carry both --orig-hash and --confine-profile-hash before -- (neutrality holds live — disable unwraps by verifying orig-hash through the confine tokens); the profile landed in guard-confine.yaml; doctor-confine --json reported backendAvailable: true + the enrolled server; disable removed the marker cleanly.

Scope note

Built the clean, load-bearing core (enable path + doctor + orchestrator seam), which makes confine fully user-reachable. Deferred to a fast-follow: the per-server guard confine <server> / --off / --show / --require / --allow-* command — its re-wrap-already-wrapped logic + the --off re-wrap-before-delete ordering contract are a distinct chunk best done on its own. Per-server confinement is still achievable today via enable --confine --server X + disable --server X.

Risk

Confine is opt-in (--confine only). Without the flag, enable/disable behavior is unchanged (confineMarkers undefined → wrapEntry called exactly as before). The spawn path (#110) only acts on servers present in guard-confine.yaml.

m1ngshum added 3 commits July 2, 2026 11:22
…e` + `doctor-confine`

Wires the PR3 confine engine to the enable path. `mcpm guard enable --confine`
(bare → standard tier; `--confine off` disables) now enrolls every unwrapped
stdio server it wraps into an OS sandbox: the cli layer derives each server's
ConfineProfile ONCE (a single captured_at → an identical content hash across
every client that wraps it, so the spawn-time verify never mismatches), merges
them into ~/.mcpm/guard-confine.yaml, and hands the orchestrator a name→marker
map. The orchestrator's single wrapEntry call embeds `--confine-profile-hash`
(+ `--confine-required`) via the new optional `confineMarkers` dep — the only
behavioral change there. `mcpm guard disable` removes the marker (unconfines);
the lingering profile is harmless (spawn row 5b: no marker → not confined).

`mcpm guard doctor-confine [--json]` reports backend availability (platform +
sandbox-exec presence) and the enrolled servers (tier/net/require_confine),
pointing at `guard status` for per-client wrap state. Read-only.

Hybrid posture surfaced honestly: when no OS backend is present (Linux/CI/
Windows today), enable still enrolls + embeds markers but prints a loud notice
that those servers run UNCONFINED until a backend exists (a require_confine
server would fail closed at spawn per the PR3 decision table).

Verified: 1886 tests pass (+10: orchestrator marker-embed, computeConfineMarkers
dedup/skip/merge, enable --confine end-to-end, doctor JSON/text/empty); tsc +
build clean. macOS dogfood on the built binary: `enable --confine` wrote both
markers before `--` (neutrality holds live — unwrap still verifies orig-hash
through them), the profile landed in guard-confine.yaml, `doctor-confine --json`
reported backendAvailable=true + the enrolled server, and `disable` removed the
marker cleanly.

Deferred to a fast-follow: the per-server `guard confine <server>` /
`--off` / `--show` / `--require` / `--allow-*` command (re-wrap-already-wrapped +
the --off re-wrap-before-delete ordering contract are a distinct chunk).
Document the confine feature across every doc it touches, driven by one
authoritative fact sheet + a consistency critic:

- docs/GUARD.md — new "OS confinement (--confine)" section (enforcement-vs-
  detection, the standard READ/WRITE/NET tier, `enable --confine` +
  `doctor-confine`, unconfine-via-disable, hybrid posture, marker tokens,
  fail-closed cases, macOS-only + CI-untested caveats, deferred per-server
  command); the two new commands + confine store/sidecar in the tables; the
  CONFINE-category event ids; and the --orig-hash spawn-verify (Phase-1) note.
- README.md — concise "Confinement (opt-in enforcement)" note + the two commands.
- docs/ROADMAP.md — F1 marked SHIPPED (engine + enable-path, unreleased); the
  XL-not-L / "no spawn-verify existed, built from scratch" correction; deferred
  items; Next-up → F10/F8/F9.
- CLAUDE.md — v0.15.0 header + confine-unreleased note, guard doctor-confine /
  enable --confine in the commands list, an F1 SHIPPED roadmap block (#108#111),
  7 Decisions-Log rows (2026-07-02), and a stale Next-up line reconciled (F7 done).
- CHANGELOG.md — [Unreleased] entries for #108/#109/#110/#111 (no invented version).
- docs/SECURITY-HARDENING.md — cross-reference note (runtime containment now has
  an implementation via ROADMAP F1); H1–H12 table untouched.
- docs/SIGNATURES.md — clarify confine adds no OWASP signatures (catalog still
  9/8); its CONFINE events + orig-hash-mismatch are relay/spawn events.
- docs/ARCHITECTURE.md — confine layer in the guard subsystem (modules, store,
  spawn-decision diagram, 13 guard subcommands).

Honest throughout: macOS-only, opt-in, CI-untested-on-sandbox-path, unreleased,
per-server `guard confine` deferred; no cross-platform / general-egress / release
overclaims (critic-verified).
…iums)

Two independent agent reviews (security spawn-path + TS-correctness) on #111.

HIGH — false "tamper" fail-closed on a multi-client partial-enable retry.
`hashConfineProfile` included `captured_at`, and `computeConfineMarkers`
re-timestamped each run, so re-deriving an already-enrolled server minted a new
hash that no longer matched the already-wrapped clients' markers → spawn row 3
(fail-closed ALWAYS) bricked a working server with a bogus tamper alarm. Fixed at
the root: hash ONLY the enforcement-policy fields (captured_at is provenance, not
policy), AND reuse an already-stored profile instead of re-deriving (only derive +
store genuinely-new names; write only when ≥1 added).

HIGH — silent clobber of a corrupt/tampered confine store. computeConfineMarkers
caught any readConfineStore() error, fell back to an empty store, and overwrote
the file — erasing previously-enrolled servers and masking the tamper signal.
Now the error PROPAGATES (ENOENT still returns empty, never throws); runEnableCommand
catches it, prints "⚠ OS confinement aborted", and returns WITHOUT wrapping — the
store is left byte-for-byte intact (fail-closed, mirrors cleanup's PinsIntegrityError).

HIGH — the load-bearing invariant was untested + 3 functions over the 50-line rule.
The e2e test now asserts the embedded --confine-profile-hash VALUE equals
hashConfineProfile(readConfineStore().servers[name]); extracted printEnableDryRun /
collectConfineTargets / renderDoctorConfine{Json,Text} to get under 50 lines.

MEDIUM — thread --client into computeConfineMarkers (was ignored; also removes the
multi-client net-misclassification); acknowledge `--confine off` (was a silent
no-op); sanitize storeError in the JSON doctor output.

LOW — accept `--confine true`/`1` as standard (friendlier than exit 1); strengthen
the orchestrator arg-adjacency assertion.

Verified: 1890 tests pass (+4); tsc + build clean. macOS dogfood on the built
binary: embedded hash == stored-profile hash; a retry re-run enrolls nothing new
and does NOT churn captured_at (proves the no-false-tamper fix); --client scopes
enrollment; disable still unwraps through the confine tokens.
@m1ngshum

m1ngshum commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Adversarial review pass (two independent agents) + fixes

Ran two independent adversarial reviews on this PR's code delta (matching the rigor #110 got): a security / spawn-path lens and a TypeScript-correctness lens. Both confirmed the core is sound — the marker↔store hash binding is stable within a run, the store merge is a pure immutable upsert, marker-embedding scope matches what the orchestrator wraps, doctor-confine is read-only, the SBPL scratch-read re-allow ordering is correct, and a hostile server name can't traverse or reach SBPL. Findings fixed in da95057:

HIGH — false "tamper" fail-closed on a multi-client partial-enable retry. hashConfineProfile included captured_at and computeConfineMarkers re-timestamped each run, so re-deriving an already-enrolled server minted a new hash → the already-wrapped clients' markers mismatched → spawn decision row 3 (fail-closed) bricked a working server. Fixed at the root: hash only the enforcement-policy fields (captured_at is provenance, excluded), and reuse an already-stored profile instead of re-deriving.

HIGH — silent clobber of a corrupt/tampered store. The read-error path fell back to an empty store and overwrote the file, erasing prior enrollments and masking the tamper signal. Fixed: the integrity/shape error now propagates; enable aborts with ⚠ OS confinement aborted, store left byte-for-byte intact (ENOENT still returns empty and never throws).

HIGH — untested invariant + 3 functions over the 50-line rule. The e2e test now asserts the embedded --confine-profile-hash value equals hashConfineProfile(store.servers[name]); helpers extracted to satisfy the length rule.

MEDIUM/LOW--client now threaded into marker computation (was ignored; also removes a multi-client net-misclassification); --confine off acknowledged (was a silent no-op); storeError sanitized in JSON; --confine true/1 accepted; orchestrator arg-adjacency assertion strengthened.

Verification: 1890 tests pass (+4 regression tests for the retry, corrupt-store abort, --client scoping, and the hash-value invariant); tsc + pnpm build clean; macOS dogfood on the built binary confirms embedded hash == stored hash, a retry enrolls nothing new (no captured_at churn → no false tamper), --client scopes enrollment, and disable unwraps through the confine tokens.

m1ngshum added 3 commits July 2, 2026 12:49
…ength + tie-breaker doc)

Post-fix re-review of #111 confirmed all HIGH/MEDIUM resolved and a clean confine
security verdict, leaving two minor items:

- MEDIUM (house rule <50 lines): `computeConfineMarkers` was ~54 lines. Extract the
  reuse-or-derive loop into `resolveConfineMarkers(targets, store, opts)`; both
  functions are now well under 50. Behavior identical.
- LOW: document the first-client-wins tie-breaker in `collectConfineTargets` — when
  the same server name runs a different command across clients, the net
  classification comes from the first-seen command; `--client` avoids the ambiguity.

No behavior change. 1890 tests pass; tsc + build clean.
Drives the real enable -> guard run -> decide -> sandbox-exec -> relay chain in
a throwaway $HOME (canonicalized via pwd -P so Seatbelt subpath rules match).
Asserts, with a positive control (same secret reads fine unconfined): stdio
intact through the relay+sandbox, secret read denied inside the sandbox (EPERM,
not ENOENT), tamper fails closed via the CONFINE gate, and a CONFINE event is
logged. Wired as 'pnpm dogfood:confine' and added to the launch checklist.
macOS-only (sandbox-exec); SKIPs elsewhere. Covers the gap ubuntu CI can't.
writePins touched pins.json to 0 bytes (flag: wx) before locking + writing real
content. A crash/kill -- or a concurrent, unlocked readPins -- in that window
left an empty file, so the next launch did JSON.parse("") -> PINS-READ-ERROR
and failed the guard closed (bricked until manual rm). Pre-existing; surfaced by
the confine dogfood. Fix: touch with the valid serialized content, not "", so
an interrupted write is still readable (absent sidecar => first-run path).

Regression test simulates the crash window by holding proper-lockfile's lock dir
so writePins throws after the touch, before finalize; asserts the file left
behind is non-empty valid JSON and readPins does not brick. Verified it fails on
the old code and passes on the fix.
@m1ngshum m1ngshum merged commit b754c4a into main Jul 2, 2026
7 checks passed
@m1ngshum m1ngshum deleted the feat/guard-confine-cli branch July 2, 2026 10:11
m1ngshum added a commit that referenced this pull request Jul 2, 2026
Guard --confine (F1) enforcement arc (#108-#111) + writePins 0-byte-brick fix
+ hermetic macOS confine dogfood. Version is derived from the git tag by CI.
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.

1 participant