fix: re-assert locked mount flags in the sandbox read-only seal (#8386)

[CrysisDeu]

Problem / Motivation

On Linux hosts where the filesystem holding a READONLY_DIRS target is mounted nosuid and/or nodev — the systemd tmp.mount default for /tmp on Amazon Linux 2023, Fedora and RHEL, and common for /home on fleet hosts — the sandbox launcher's read-only seal fails with errno 1 (EPERM). _mount_or_die correctly fails closed, so every sandboxed spawn aborts with sandbox: BLOCKED -- sealing read-only path ... errno 1, and 35 backend tests refuse to run on unpatched main because pytest's tmp_path lives under /tmp (most of ops_mission_control/tests/test_ledger_sync_git.py, 6 in auto_improvement/tests/test_profile_capture.py, 1 each in auto_improvement/tests/test_runner.py and ops_mission_control/tests/test_providers.py). GitHub's Ubuntu runners keep /tmp on the root filesystem without these flags, which is why CI stayed green.

Why it matters

Every sandboxed spawn on an affected host is dead on arrival — an availability/portability defect (not an exposure: the failure direction is closed). AL2023/Fedora/RHEL defaults make this the common case on exactly the fleets most likely to run the gateway.

What changed (motivation → approach → change)

Symptom: the seal's second step — _MS_REMOUNT | _MS_BIND | _MS_RDONLY — is refused with EPERM. Root cause: inside an unprivileged user namespace the kernel treats a mount's nosuid/nodev/noexec bits as LOCKED (MNT_LOCK_*) and rejects any remount whose flag set would clear them; the bind created in step one inherits those bits — locks included — from its source mount, so a remount carrying only MS_RDONLY reads as an attempt to drop them.

The fix is restriction-only: re-assert the flags the target already carries, so the kernel's locked-flag check passes.

• Added _MS_NOSUID = 2, _MS_NODEV = 4, _MS_NOEXEC = 8 next to the other mount-flag constants in the launcher template (src/kiro_crew/sandbox.py).
• Added _locked_mount_flags(target): reads os.statvfs(target).f_flag AFTER the MS_BIND step (so it sees the new bind's effective flags — the mount the kernel checks) and maps ST_NOSUID/ST_NODEV/ST_NOEXEC to their MS_* values via getattr(os, "ST_*", 0) (the ST_NODEV/ST_NOEXEC constants are Linux-only; the extracted helper source also runs in POSIX-wide tests). atime is left alone — a remount passing no atime flag preserves the existing mode, which already satisfies MNT_LOCK_ATIME. On statvfs failure it falls back to 0 extra flags: the remount then behaves exactly as before and still fails closed via _mount_or_die. The seal is never degraded.
• The sealing remount now ORs the helper's bits in. Re-asserting a bit already in force can only keep restrictions, never widen access. _mount_or_die's fail-closed contract is untouched; no silent-degrade path was added.
• One-paragraph doc note in docs/system-specs/modules/governance.md (the section describing the READONLY_DIRS seal).

Scope guard honored: no change to the backend probe, _mount_or_die, or the SENSITIVE_DIRS hiding mounts (empty-dir binds are not remounted and are unaffected); #8008's symlinked-HOME/ownership-walk classes are not addressed here.

Tests

New test/test_sandbox_seal_locked_flags.py, following the established extraction pattern (sources under test are pulled from the GENERATED launcher script, so no test can pass against code the launcher no longer contains):

• Unit: the helper maps ST_NOSUID|ST_NODEV → _MS_NOSUID|_MS_NODEV (and all three bits, and 0 for a plain flag set); statvfs failure falls back to 0 extra flags; a host missing the ST_* constants (macOS) reads them as 0 instead of raising AttributeError.
• Integration: the seal loop's remount call receives the helper's bits OR'd in, and the helper reads the target AFTER the bind step (event order asserted).
• Real kernel: NESTED namespaces — flags are only locked on mounts a namespace INHERITED, so an outer unshare -Urm mounts a fresh tmpfs nosuid,nodev (inside the namespace; independent of the host's /tmp options) and a nested inner namespace, which inherits it locked, runs the shipped seal path. An in-band control first proves the test's power: the pre-fix remount (MS_RDONLY alone) must be refused with EPERM — the exact sandbox read-only seal fails EPERM where /tmp is nosuid,nodev (AL2023/Fedora/RHEL defaults) — 35 backend tests refuse to run #8386 failure — then the fixed path must land and a write must be refused with EROFS. Skips (never fails) where user namespaces, unshare, tmpfs mounting, or flag locking are unavailable, mirroring the existing sandbox tests.

Pinning-test updates: test_sandbox_mount_checked.py's site4 mutation anchor tracks the new call text and its region namespace gains the three constants; test_sandbox_absent_ceiling_seal.py's extracted-loop globals gain a 0-returning _locked_mount_flags stub so it keeps asserting the bind+remount pair exactly (the real helper is covered by the new file).

Pre-push review: two model-pinned lanes (GPT + Opus mirrors) found 1 blocking + 2 advisory findings (subprocess cwd=, macOS ST_* constants, in-namespace tmpfs not locked); all three fixed and verifier-confirmed closed on this head.

Manual verification

Local gates run (isort, flake8, mypy, diff-scoped black/brand/scrub gates) plus a compile-level smoke of the generated launcher and both staged namespace scripts. Test suites run in CI per this host's policy. No host with a nosuid,nodev /tmp was reachable from this session, so the 35 previously-failing tests are verified via CI plus the unshare-based nested-namespace regression test, which reproduces the locked-flag EPERM in-band before exercising the fixed path.

Screenshots / video

N/A — backend-only change, no UI surface.

Related Issues

Fixes #8386

Pattern harvest

Rule candidate: review-prompt
Pattern: "remount inside an unprivileged user namespace must re-assert the source mount's locked nosuid/nodev/noexec bits; a flag-narrowing remount EPERMs on hosts with hardened mount options"

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of a9eee7808abfe5a5ce97727f2be921c7ebbe1318 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Root-cause fix mirroring the standard container-runtime handling of userns-locked mount flags: restriction-only OR, fail-closed path untouched, verified by an in-band kernel repro.

[DESIGN-REVIEWED] a9eee78

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of a9eee7808abfe5a5ce97727f2be921c7ebbe1318 — 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 checks pan out. Final review:

First-Principles-Verdict: PASS

A cause-level fix at the kernel's actual rule — locked MNT_LOCK_* bits — with every item declared, one remount site, and no rider.

What this change ships

Intent: make sandboxed spawns start on hosts whose /tmp//home is mounted nosuid,nodev (issue #8386) — a FIX.

1. Sandboxed spawns no longer abort on nosuid/nodev hosts — justified, the fix.
2. Seal remount re-asserts the bind's locked bits (_locked_mount_flags + 3 constants) — justified, cause-level.
3. statvfs failure falls back to pre-fix flags, still fail-closed — declared, restriction-only.
4. Missing ST_* constants read as 0 instead of raising — declared, derived (verified: test_sandbox_mount_checked.py skips only win32 and extracts the helper's region, so it executes on macOS where ST_NODEV/ST_NOEXEC don't exist).
5. Governance spec paragraph documents the re-assertion — mandated (AGENTS.md same-commit spec rule).
6. New three-layer regression test with an in-band EPERM control — declared, pins the fix.
7. Two pinning tests updated to track the new call text — declared, required by the extraction pattern.

Depth and sibling counts were run, not assumed: the root cause is a kernel constraint (locked flags in unprivileged user namespaces), not an earlier repo choice, and the fix removes it rather than catching the symptom (no EPERM-swallow, no seal skip). Grepped _MS_REMOUNT in sandbox.py: exactly one remount site (line 3202); the other _mount_or_die calls are plain binds the locked-flag check doesn't apply to — zero unfixed siblings. Grepped statvfs across src/kiro_crew: no pre-existing mechanism. _locked_mount_flags has exactly one non-test consumer (sandbox.py:3203), correct for a launcher-template helper that must be extraction-testable. Framing matches the diff throughout.

[FIRST-PRINCIPLES-REVIEWED] a9eee78

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed a9eee7808abfe5a5ce97727f2be921c7ebbe1318 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] a9eee78

Verdict parsed from the review's SHA-scoped output markers for commit a9eee7808abfe5a5ce97727f2be921c7ebbe1318.

False positive or not applicable? A repository writer can comment:
/ai-review override fable a9eee7808abfe5a5ce97727f2be921c7ebbe1318: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of a9eee7808abfe5a5ce97727f2be921c7ebbe1318 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] a9eee78

False positive or not applicable? A repository writer can comment:
/ai-review override gpt a9eee7808abfe5a5ce97727f2be921c7ebbe1318: <one-sentence reason>