[marcuspat]

Sentinel 0.2.0 — release summary

Draft. Not merged and not tagged — both wait for the maintainer.

20 commits on feat/sota-round-2, 101 files, +14,592 / −1,090. Workspace tests 454 → 667 (1 ignored), all passing. cargo fmt --check, clippy -D warnings, cargo test --workspace and cargo deny check are green locally and in CI at 42adf57.

What shipped

#	Item	ADR
0	Ed25519-signed audit checkpoints, audit-keygen, verify-audit --pubkey	015
1, 1b	Landlock write confinement per capability; production now runs through the hardened executor; no_new_privs	016
2	seccomp network denial; children start from an empty environment with a fixed PATH	016
3, 4	Provider-native tool use for Anthropic, OpenAI and Ollama	017
5	Policy files: --policy FILE, tighten and replace modes	018
6	Plan-store locking: an approved plan can no longer run twice	013
7, 8	One plan executor (sentinel-runner) for run, the TUI and execute; rollback in reverse order	019
9	LLM retries, per-attempt deadlines, per-session call and token budgets	020
10	Metrics derived from the audit stream, LLM metrics, GenAI spans	021
11	Fleet path under policy, approval and audit; new agent-exec	022
12	TUI Gate tab for plans proposed over MCP	—
13	cargo-deny in CI, pinned action SHAs, read-only tokens, SBOM and build provenance	docs/SUPPLY_CHAIN.md
14, 14b	Property tests and protocol fuzzing (32 properties); resource guards cover every argument and spelling	—
15	Docs brought in line with the code; version 0.2.0; ADR index	—

Defects found and fixed along the way

• Production ran commands with no allowlist, timeout or rlimits — the documented executor was not the one in use.
• log_vacuum could delete arbitrary files via a newline in a file name.
• An approved plan could be executed twice (race reproduced by a test).
• sentinel run never performed approved Medium-risk mutating steps, and reported failed steps as completed.
• sentinel fleet consulted no policy, wrote no audit event, and had two injection routes (--hosts → local command execution, --capability → remote shell injection).
• Resource guards could be bypassed through log_dir, cache_dirs, sshd.service, ssh, /var/../etc and //etc; the default path guard protected fewer directories than SECURITY.md said.
• A float in MCP tool arguments made an untouched audit log fail verification.
• The Prometheus metrics were defined but never incremented.
• TUI goal field could not take q, a, s, r, j, k and panicked on non-ASCII input.
• Four dependency advisories (rustls, h2, crossbeam-epoch, anyhow); rustls-pemfile removed.
• Docs claimed fleet traffic used mTLS (it is SSH) and that Ollama was selectable from the CLI (it is not).

Still open (unchecked in docs/SOTA_ROADMAP.md)

• 13b. Toolchain bump. CI is pinned to 1.97.0; 1.99's double_must_use fires in async-trait output. Needs a machine that can install 1.99.
• 14c. Policy provenance in the audit log. The chain does not record which policy file was in force.
• 14d. Stuck Executing plans. A process killed mid-run leaves its plan Executing; no fail-plan command yet.
• 14e. OTLP export. Spans are emitted through tracing only.
• 14f. Fleet follow-ups. Staged rollout is not wired to sentinel fleet, approval travels as a flag, and the unused mTLS controller code is still in the tree.

Not verified

• Provider wire formats are tested against wiremock and fakes only; nothing was sent to a live API (no paid calls were made).
• Fleet is tested with a fake ssh, not a real SSH server.
• systemctl and package-manager Landlock profiles were not run against live systemd or package managers.
• The release workflow's SBOM and attestation steps run only on a tag, so they are untested end to end.
• No coverage-guided fuzzing (cargo-fuzz needs nightly).
• TUI rendering is tested as text lines, not in a real terminal.

Before merging

• Review SECURITY.md → Known Limitations; it lists everything above.
• 0.1.x is marked unsupported there because several fixes above are exploitable in it — change that if you want a different support policy.
• The CHANGELOG heading is [0.2.0] - Unreleased; set the date when tagging.

🤖 Generated with Claude Code

https://claude.ai/code/session_011quWn3GSjo8BNBbMo65ysi

---

Generated by Claude Code

[marcuspat]

Gate review — reviewer: claude

Lanes: reviewed by claude

GATE USAGE reviewer=claude in=2 cached=141,610 out=3,756 51s model=claude-haiku-4-5-20251001,claude-opus-5[1m] $1.494 API-equiv (not billed on subscription)

Review of the diff, read-only, as given.

Findings

1. log_vacuum now silently drops every matching file when find is not GNU-compatible, and the fix's prefix check rejects log_dir itself — sentinel-capabilities/src/filesystem.rs:247-281

The filter changed from !l.is_empty() to p.starts_with(&prefix) where prefix = format!("{}/", log_dir.trim_end_matches('/')). Two consequences visible in the diff:

• find with -print0 emits a trailing NUL, so the final split('\0') element is "", which correctly fails the prefix test — fine.
• But the paths find prints are <log_dir-as-given>/.... If the caller passes log_dir with a trailing slash (/var/log/), find prints /var/log//app.log — wait, GNU find prints /var/log//app.log for find /var/log/ -name '*.log', and prefix is /var/log/, so that still matches. The real break is the opposite normalization: a caller passing a relative or symlinked log_dir (e.g. ./logs) gets find output ./logs/a.log and prefix ./logs/ — still matches. So the prefix check is sound for the cases the diff shows.

Withdrawing this one; the stated concern does not survive tracing. (Flagging only the real remainder below.)

2. rm argument is not ---terminated; a file named -rf or --no-preserve-root under log_dir becomes an option — sentinel-capabilities/src/filesystem.rs:283-294

rm -f <f> where f comes from find output under an attacker-writable log directory. The new prefix check guarantees f starts with <log_dir>/, so f cannot itself begin with - — this is actually closed by the prefix check. Not a finding.

3. HardenedExecutor::sandbox_for drops no_new_privs from the base profile for Unrestricted/Off — no, it clones self.sandbox, which carries it. Not a finding.

4. Real finding: deny_network is forced on for FsAccess::Unrestricted commands too, contradicting the documented package-manager exemption only when landlock != Off — sentinel-exec/src/executor.rs (spawn_checked)

let mut sandbox = sandbox.clone();
if !network && self.landlock != LandlockMode::Off {
    sandbox.deny_network = true;
}

network is NETWORK_COMMANDS.contains(&program), so package managers are exempt. This matches ADR-016. Not a finding.

5. Confirmed finding: apply_sandbox returns SandboxUnavailable for deny_network whenever require_enforcement is set, which sandbox_for sets from LandlockMode::Require — so SENTINEL_LANDLOCK=require on a non-x86_64/aarch64 Linux (or any non-Linux) makes every confined built-in command fail to spawn, including read-only ones, with a message about the sandbox rather than about the toolchain. sentinel-exec/src/sandbox.rs (apply_sandbox, net-filter arm) + sentinel-exec/src/executor.rs (sandbox_for). This is arguably intended ("require refuses to run"), but the one knob now gates two independent mechanisms and ADR-016 documents require as being about Landlock only ("SENTINEL_LANDLOCK=require refuses to run a confined command on a kernel without Landlock"). On aarch32/riscv Linux with Landlock present, require fails closed on the network filter — behaviour the ADR does not describe. No test covers this combination (non_network_commands_run_with_inet_sockets_denied skips when the filter is unavailable, and it runs in BestEffort).

6. Confirmed finding: HardenedExecutor::run ignores self.timeout_ms's SIGTERM→SIGKILL claim for the output half of the join — sentinel-exec/src/executor.rs (spawn_checked)

let ((stdout, stderr, truncated), waited) = tokio::join!(
    capture.capture(stdout_pipe, stderr_pipe),
    guard.wait_with_timeout()
);
let (status, _timed_out) = waited?;

tokio::join! waits for both futures. If the child is SIGKILLed on timeout but a grandchild inherited the pipes and keeps them open, capture.capture does not return, so spawn_checked hangs past timeout_ms despite waited being ready. deny_new_processes is not set in for_builtin_capabilities(), and package_upgrade runs FsAccess::Unrestricted with no no_new_processes limit, so a package-manager hook that daemonizes with inherited stdout is exactly this case. hardened_times_out_and_kills_the_child uses sleep, which has no children, so the test cannot fail on this.

7. Confirmed finding: the pre_exec closure's map_err change silently alters semantics for non-errno values — sentinel-exec/src/sandbox.rs

let map_err = |e: nix::errno::Errno| std::io::Error::from_raw_os_error(e as i32);

This is fine for real errnos. Minor, not worth a finding on its own.

8. Confirmed finding: CI's clippy annotation step cannot fail and can silently annotate nothing — .github/workflows/ci.yml:50-67

The Annotate clippy failures step runs if: failure(), which fires for any prior failure (fmt, build, test), not just clippy. When fmt fails, clippy.log does not exist and the step prints only ::notice::rustc --version — harmless. But the pipeline ends in || true, and grep with no matches returns 1, so a clippy failure whose output does not match (error|warning)(\[...\])?: (e.g. the short format emits path:line:col: error: msg, which does match) produces zero annotations and still exits 0. The step therefore cannot fail and gives no signal that it found nothing — a check that verifies nothing. Not a correctness bug in the product, but the diff's CHANGELOG claims "clippy failures are surfaced as check annotations" and head -9 silently caps at 9 with no "…and N more" line.

9. Confirmed finding: verify_checkpoints's unsigned_tail underflows on the success path when a checkpoint covers more events than the log — sentinel-audit/src/signing.rs

The fail closure guards with covered.min(total), but the final success return does not:

SignatureVerification {
    ...
    unsigned_tail: total - covered,
    error: None,
}

covered can only exceed total if a checkpoint with event_count > total is accepted — and that case returns via fail. So covered <= total holds on the success path. Not a finding.

10. Confirmed finding: sentinel verify-audit reports VALID for an empty log with --require-signature only via the no signed checkpoints branch — correct. Not a finding.

11. Real finding: plan_document passes model-controlled JSON through to_string() into PlanParser, but InvestigationTools::action puts response.text (model prose, possibly attacker-echoed) into the reasoning field — sentinel-agent-llm/src/tools.rs

reasoning is not an action, and ADR-017 says text is commentary. But that commentary is then stored in the plan/observation and fed back into the next prompt. The diff shows no spotlighting applied to response.text on the tool path, whereas the text path's content went through detect_injection_markers in reasoning_loop.rs (per PR #10). I cannot confirm from this diff whether the tool-mode reasoning reaches a later prompt unspotlighted — reasoning_loop.rs's audit of injection markers is shown operating on observations, not on reasoning. Marking as unverified: the base-ref behaviour of reasoning propagation is not in the diff.

12. Confirmed finding: ReasoningConfig::default() reads the environment at construction, so SENTINEL_NATIVE_TOOLS is sampled per-default()-call rather than once — sentinel-agent-llm/src/reasoning_loop.rs

Not a bug; documented switch. Not a finding.

---

Let me state only what survived verification, cleanly.

ReportFindings is not applicable here (this is a gate review, not the code-review tool path), so the findings are above in prose. Three survive:

• sentinel-exec/src/executor.rs spawn_checked — tokio::join! on capture + timeout means a grandchild holding the pipes defeats the 15-minute budget; package_upgrade runs Unrestricted with no RLIMIT_NPROC, so this is reachable. The new timeout test uses sleep, which cannot exhibit it.
• sentinel-exec/src/sandbox.rs + executor.rs sandbox_for — SENTINEL_LANDLOCK=require now also fails closed on the seccomp network filter, so on a Linux arch without the filter every confined built-in refuses to spawn. ADR-016 documents require as Landlock-only; no test covers Require + unavailable seccomp.
• .github/workflows/ci.yml:53-67 — the annotation step is if: failure() (fires for fmt/test failures too), ends in || true, and head -9 truncates silently, so it can emit zero annotations and still pass. It is a check that cannot fail, contradicting the CHANGELOG's "surfaced as check annotations" for the general case.

The audit-signing work (signing.rs, the CLI wiring, audit_signing.rs) is solid: the trusted key comes from the caller, public_key in the checkpoint is explicitly untrusted, verify_strict is used, one bad checkpoint fails the whole run, and the integration test exercises the whole-file rewrite against the real binary. The log_vacuum NUL-separation fix plus prefix check is correct and the confinement.rs test actually reproduces the newline attack.

VERDICT: REVISE — HardenedExecutor::spawn_checked joins output capture with the timeout, so a child that leaks its pipes to a grandchild outlives the documented 15-minute kill; plus SENTINEL_LANDLOCK=require silently extends to the seccomp filter (undocumented, untested) and the CI clippy-annotation step cannot fail.

---

VERDICT: REVISE — HardenedExecutor::spawn_checked joins output capture with the timeout, so a child that leaks its pipes to a grandchild outlives the documented 15-minute kill; plus SENTINEL_LANDLOCK=require silently extends to the seccomp filter (undocumented, untested) and the CI clippy-annotation step cannot fail.

[marcuspat]

Gate review — reviewer: claude

Lanes: reviewed by claude

GATE USAGE reviewer=claude in=2 cached=340,537 out=3,049 50s model=claude-opus-5[1m] $3.385 API-equiv (not billed on subscription)

I reviewed the diff as-built. Findings, most severe first.

1. log_vacuum prefix check rejects every path when log_dir has a trailing slash — and a path-traversal gap remains

sentinel-capabilities/src/filesystem.rs:270-277:

let prefix = format!("{}/", log_dir.trim_end_matches('/'));
let files: Vec<String> = find_out.stdout
    .split('\0')
    .filter(|p| p.starts_with(&prefix))

find is invoked with log_dir verbatim (&[log_dir, "-name", ...]), so its output lines are prefixed with the caller's spelling. If the caller passes log_dir = "/var/log/app/", find prints /var/log/app//a.log… actually find normalizes nothing: GNU find /var/log/app/ -print0 emits /var/log/app/a.log, which does match. But for log_dir = "/var/log/app//" the emitted paths are /var/log/app//a.log while prefix is /var/log/app/ — that still matches by prefix. The real failure is the inverse: the check is purely lexical on an un-normalized string, so log_dir = "/var/log/app/../../etc" produces prefix = "/var/log/app/../../etc/" and find emits /var/log/app/../../etc/x.log, which passes the prefix test and is then rm -f'd. The only thing stopping that is validate_args, which the diff does not show — and SECURITY.md:+60 explicitly states guards "compare paths lexically and do not resolve symlinks". Landlock is FsAccess::write_under([log_dir]), which grants the resolved target of that same traversal, so the kernel allows it too.

Failure scenario: log_vacuum with log_dir = "/var/log/app/../../etc" on a kernel with Landlock — find lists /etc/*.log, every path passes the starts_with prefix check, Landlock's rule was opened on the same traversed path so the write is permitted, and rm -f /etc/*.log runs. The normalization added in sentinel-policy (normalise_path) is not applied here.

Fix: normalize log_dir before building both prefix and the Landlock allowlist, or canonicalize and compare against the canonical prefix.

2. read_limited panics on limit == 0

sentinel-agent-llm/src/http.rs:27:

let room = limit - body.len();

limit - body.len() is usize subtraction. On the first iteration body.len() == 0, so limit == 0 gives 0 - 0 == 0, then chunk.len() > 0 is true and &chunk[..0] is fine — no panic there. The panic is on a later iteration: if chunk.len() == room exactly, the > branch is skipped and the full chunk is appended, making body.len() == limit; the next chunk then computes limit - limit == 0, chunk.len() > 0 → &chunk[..0], returns (body, true). Still safe. However text_capped(response, 0) and the json_capped path are only ever called with the module constants, so this is latent, not live — I'm dropping it rather than asserting an unverified break.

Correcting myself: there is no overflow here. Ignore this item.

3. Checkpoint sidecar write failure is swallowed into the audit append result after the event is already durable

sentinel-audit/src/log.rs:+148-176. The comment claims "if the checkpoint cannot be written the caller still gets an error, so a signing-enabled deployment never silently produces unsigned events." The event is pushed to self.events and next_sequence is incremented before the signing block, and the JSONL line was already write_all'd and flushed earlier in append. So on a sidecar write error the function returns Err, but the event is already in the log file and in memory. A caller that treats the error as "the event was not recorded" (e.g. sentinel-runner's RunError::Audit, which aborts before invoking the capability) will abort a step whose CapabilityInvoked event is already on disk — and on the next append the chain continues from last_hash, so the log now contains an invoked-but-never-executed event with no corresponding outcome. The stated invariant ("audit before action", runner rule 6) still holds in the safe direction, but the claim that signing failure is atomic with the event is not what the code does.

Failure scenario: SENTINEL_AUDIT_KEY set, sidecar directory becomes read-only mid-run. append(CapabilityInvoked) writes the JSONL line, fails on <log>.sig, returns Err. The runner aborts; the audit log claims the capability was invoked. verify-audit --require-signature then reports an unsigned tail for an event that is genuinely in the chain.

4. seccomp filter: SYS_io_uring_setup on a kernel/libc without it

sentinel-exec/src/sandbox.rs:+~790: jump(JEQ_K, libc::SYS_io_uring_setup as u32, 7, 0). libc::SYS_io_uring_setup is a compile-time constant and is defined for both targets in the diff's cfg, so this is fine. The jump-offset test (seccomp_filter_jump_targets_stay_in_bounds) only asserts filter.len() == 14 — it verifies the instruction count, not that any jump target lands on a valid index. That check cannot fail for an off-by-one in a jt/jf value, which is precisely the bug the comment says it exists to catch:

// A wrong offset would be rejected by the kernel at install time and
// fail every spawn; catch it here with a readable message.
assert_eq!(filter.len(), 14);

Tracing offsets by hand: index 4 JGE X32, jt=8 → 4+1+8 = 13 ✓; index 5 jt=7 → 5+1+7 = 13 ✓; index 6 jf=4 → 6+1+4 = 11 ✓; index 8 jt=3 → 12 ✓; 9 jt=2 → 12 ✓; 10 jt=1 → 12 ✓. The offsets are correct, but the test asserting them is an always-pass length check. This is the "verification that cannot fail" case: it would stay green under any offset edit.

Fix: assert each jump target resolves to an index < len and, for the terminal cases, that the target is a RET instruction.

5. ResourceGuard::blocks treats every absolute-looking string as a path, including service names and free text

sentinel-policy/src/resource_guard.rs:+61-90. collect_strings gathers every string at any depth with no key filter for paths, then normalise_path accepts anything starting with /. A capability argument that is a description, a regex, a log line, or a URL path ("/etc/..." inside a message or filter field) now trips the path guard and denies a mutating request that has nothing to do with /etc. The property test unprotected_targets_are_not_blocked_by_guards only generates paths under non-protected roots, so it cannot surface this. process_list's filter is a free-text substring — {"filter": "/usr/bin/python"} on a mutating capability would be denied.

This is the intended trade-off direction (fail closed), and SECURITY.md says guards "check every string argument at any depth" — so it is documented, not hidden. Flagging as a usability/false-positive risk rather than a security defect.

6. fsync of the state directory is #[cfg(unix)]-gated inside a closure that returns io::Result

sentinel-mcp/src/store.rs:+281-299. Correct as written; the non-Unix path simply skips the directory fsync. No finding.

---

Items 1, 3 and 4 are the ones I'd hold the PR on. Item 1 is a privilege-relevant path-traversal gap in a capability whose own CHANGELOG entry (CHANGELOG.md:+52-60) claims "every path must sit under log_dir" — the prefix check is lexical on an unnormalized caller string, so that claim covers the newline case it was written for but not ... Item 4 is a check that cannot fail, in exactly the place the diff adds verification.

ReportFindings is not applicable here (no code-review instruction asked for it), so the findings are above as text.

VERDICT: REVISE — log_vacuum's lexical prefix check does not normalise log_dir, so a .. traversal still passes it (and the Landlock rule opened on the same path), contradicting the "every path must sit under log_dir" claim; plus the seccomp jump-offset test is an always-pass length assertion.

---

VERDICT: REVISE — log_vacuum's lexical prefix check does not normalise log_dir, so a .. traversal still passes it (and the Landlock rule opened on the same path), contradicting the "every path must sit under log_dir" claim; plus the seccomp jump-offset test is an always-pass length assertion.

[marcuspat]

Gate review — reviewer: claude

Lanes: reviewed by claude

GATE USAGE reviewer=claude in=2 cached=342,430 out=4,162 67s model=claude-opus-5[1m] $3.407 API-equiv (not billed on subscription)

I'm in review mode: diff-only, read-only, no tools. Here are my findings.

Findings

1. sentinel-capabilities/src/filesystem.rs:259 — log_vacuum arbitrary-file-deletion fix is incomplete for a trailing-slash log_dir

normalise_components("/var/log/app/") returns /var/log/app (trailing empty segment dropped), so prefix is /var/log/app/ — good. But the find invocation receives the normalised log_dir, while the Landlock allowlist is FsAccess::write_under([log_dir]) on the same normalised value, so these agree. The real gap is elsewhere: validate_args (lines 229–248, unchanged region) is the only place that would reject a relative log_dir, and the new normalise_components in invoke returns None only for .. — a relative path such as var/log yields Some("/var/log") because the function unconditionally prepends /. If validate_args does not require an absolute path, invoke silently rewrites a relative log_dir into an absolute one and then vacuums /var/log.

Failure scenario: log_vacuum with {"log_dir": "var/log", "older_than_days": 0}. normalise_components → /var/log; find /var/log -name '*.log' -mtime +0 -print0; every match passes the /var/log/ prefix test; rm -f under a Landlock profile that was itself derived from the rewritten path, so confinement does not catch it. The diff does not show the validate_args body for LogVacuum, so I cannot confirm it rejects relative paths — but the new test at line 562 only asserts rejection of a .. traversal, not of a relative path, and the unit test at line 536 asserts normalise_components("/var/log")-style inputs only. Marked as unverified: this is exploitable only if validate_args permits a non-absolute log_dir. Either way, normalise_components should return None for input not starting with / (as sentinel-policy's normalise_path correctly does at resource_guard.rs:142), so the enforcement point does not depend on a caller one layer up — which is exactly the property the comment at line 196 claims.

2. sentinel-exec/src/sandbox.rs:398 — seccomp filter does not deny socket for AF_INET when SOCK_* flags are set, and the arg-0 read is 32-bit-truncating by design but the test asserting coverage is weaker than the claim

The BPF reads OFF_ARG0 = 16 (low 32 bits of args[0], the domain). AF_INET comparisons are exact-equality on the full 32 bits. socket(2)'s domain has no flag bits, so exact equality is right. However SECCOMP_RET_ERRNO | EACCES is returned for socket, while socketpair(2) and accept/accept4 are not filtered — socketpair(AF_INET, …) fails in the kernel anyway, so that is fine. The genuine gap: io_uring_setup returns ENOSYS, but io_uring_enter/io_uring_register are unfiltered. A process that inherited an io_uring fd across execve (the child does inherit fds other than stdio; kill_on_drop/env_clear do not close inherited descriptors) can still submit IORING_OP_SOCKET/CONNECT through it. ADR-016's amendment says io_uring "can create sockets without the socket syscall" and blocks only setup — correct for a child that starts clean, but the filter is applied after fork from a parent that may hold rings. This is a narrow residual, and the SECURITY.md text ("denies socket families, not a network namespace") arguably covers it. Low severity; flagging because the ADR states the io_uring route is closed.

3. sentinel-exec/src/executor.rs:420 — spawn_checked ignores SandboxReport, so LandlockMode::Require can silently fail to deny the network

sandbox_for sets require_enforcement = (self.landlock == LandlockMode::Require) only for the ReadOnly/WriteUnder arms. For FsAccess::Unrestricted it returns self.sandbox.clone() — whose require_enforcement is false for for_builtin_capabilities(). Then spawn_checked sets sandbox.deny_network = true for any non-network command when landlock != Off. With SENTINEL_LANDLOCK=require and FsAccess::Unrestricted (i.e. package_upgrade, which is in NETWORK_COMMANDS, so not reachable there) — but also for the self.landlock == LandlockMode::Off early-return path, which is correct. The reachable case: a capability calling run_confined(..., &FsAccess::Unrestricted) for a non-network program under require. apply_sandbox then takes the Err(reason) if config.require_enforcement branch → false, so it logs a warning and runs with network access, despite the operator asking for fail-closed. No built-in capability hits this today (FsAccess::Unrestricted is used only by package_upgrade, which is network-exempt), so it is latent rather than live — but SENTINEL_LANDLOCK=require is documented as "refuses to run on kernels without Landlock" and this path does not.

4. sentinel-audit/src/log.rs:137 — metrics are observed before the signing step can fail, and append returns Err for an event that is already in the chain

The comment at line 150 is explicit and honest about the contract ("recorded but unsigned"), and I agree aborting is the right direction. The defect is narrower: when the sidecar write fails, self.last_checkpoint is not updated but self.events and self.next_sequence already advanced, and global().metrics().observe() has already counted the event. A caller that treats Err as "not recorded" and retries append with the same event will produce a second chain entry and a second metrics increment. sentinel-runner's record() maps any error to RunError::Audit and halts (correct), but AuditSink::record in sentinel-mcp/src/audit.rs:78 is called with let _ = in several places in execute.rs (lines 89, 100), so a failure there is swallowed and the next append proceeds — the chain is fine, but the operator gets no signal that the tail is unsigned. Worth a sentence in the ADR-015 "Not covered" list at minimum.

5. sentinel-mcp/src/execute.rs:168 — ExecuteReport.halted is reported but the error field of ExecutionRecord is never populated

claim_for_execution sets error: None; the finally-style block at line 195 sets finished_at, steps_completed, steps_rolled_back, steps_failed, steps_skipped — but nothing ever writes ex.error, and the RunError::Audit path returns early via ? before that block runs at all. So a plan whose execution aborted on an audit-write failure is left Executing with error: None and finished_at: None, indistinguishable from a process killed mid-run. SECURITY.md:168 lists stuck Executing plans as a known limitation (roadmap 14d), which covers the process-killed case; this is the clean-abort case, which the code could record and does not.

6. sentinel-tui/src/event_handler.rs:150 — Right arrow bound on app.goal_input.len() is correct, but Tab/BackTab on the Goal tab now unreachable for text and Enter still submits, leaving no way to type a literal tab

Minor and probably intended; noting only because the new early-return at line 148 captures every KeyCode::Char, including ' ', before the KeyCode::Char('d') dry-run toggle and KeyCode::Char('h') host shortcuts. Those shortcuts are now dead on the Goal tab. The test at line 405 asserts the new behaviour and the test at line 437 asserts shortcuts still work off the Goal tab, so this is a deliberate trade-off — but d (dry-run) and the host field have no remaining binding while the Goal tab is focused, and dry_run is read at submit time (line 143). An operator cannot toggle dry-run from the tab where they type the goal. Not a correctness bug; a usability regression the diff does not acknowledge.

7. sentinel-exec/src/sandbox.rs:603 — landlock_read_allowlist_hides_everything_else depends on /bin, /usr, /lib, /lib64, /etc all existing

add_rule skips NotFound paths silently. On a distribution without /lib64 (most aarch64 Linux), that entry is skipped — which only tightens, so the test still passes. But the test's positive assertion (cat /etc/hostname || cat /etc/passwd) and the shell exec itself depend on /bin and the loader path being grantable. If the shell is at /usr/bin/sh with libraries under /usr/lib64, the skip makes the test fail for an environment reason rather than a logic reason. Pre-existing pattern in this file; flagging because this diff adds the test.

8. sentinel-exec/src/sandbox.rs:742 — the new seccomp jump-target walk asserts jumps >= 5, which is a tautology for the current literal filter

The test is a real improvement over a length assertion and does catch off-by-one jt/jf. But assert!(jumps >= 5) over a hard-coded 14-instruction array can never fail unless someone edits the array — it does not guard against a future generated filter with no jumps, which is what the comment implies. The in-bounds and RET-target checks are the load-bearing part and they do work. Cosmetic.

Not findings, verified as sound

• normalise_unit suffix stripping loops, so sshd.service.service reduces to sshd (resource_guard.rs:170); the @ split happens before suffix stripping, so getty@tty1.service → getty. Correct.
• collect_strings inherits the array's key for elements (resource_guard.rs:105), so {"services": ["sshd"]} is matched as a service. Correct, and is_service_key covers the plural.
• strictness ordering and with_tightening_rules (evaluator.rs:296) cannot weaken a decision; the property test at properties.rs:171 checks both directions.
• claim_for_execution under flock on a separate inode (store.rs:244) — the rename-drops-the-lock reasoning in the ADR amendment is right, and the 16-way race test is a real test.
• verify_checkpoints never trusts cp.public_key (signing.rs:338); the wholesale_rewrite_is_detected test genuinely fails without the signature.
• single_call rejecting zero and >1 calls (tools.rs:214) is what makes json_in_text_is_never_executed meaningful.
• read_limited arithmetic: limit - body.len() cannot underflow because the chunk.len() > room branch returns (http.rs:28).

Findings 1 and 3 are the ones I'd want addressed before merge: finding 1 because the CHANGELOG and the code comment both assert the vacuum boundary holds at the enforcement point and it still depends on validate_args rejecting relative paths, and finding 3 because SENTINEL_LANDLOCK=require is documented as fail-closed and has a path that warns-and-continues.

VERDICT: REVISE — normalise_components prepends / to relative input so the log_vacuum boundary still relies on validate_args, and FsAccess::Unrestricted under SENTINEL_LANDLOCK=require warns instead of failing closed

---

VERDICT: REVISE — normalise_components prepends / to relative input so the log_vacuum boundary still relies on validate_args, and FsAccess::Unrestricted under SENTINEL_LANDLOCK=require warns instead of failing closed

[marcuspat]

Gate review — reviewer: claude

Lanes: reviewed by claude

GATE USAGE reviewer=claude in=4 cached=695,537 out=3,468 55s model=claude-opus-5[1m] $3.664 API-equiv (not billed on subscription)

Two findings, both reported above. Summary:

1. sentinel-exec/src/sandbox.rs — the new seccomp jump test asserts a false property of BPF. seccomp_jump_targets_land_in_bounds_on_ret_instructions requires both jt and jf of every conditional jump to land on a BPF_RET. In the filter built in the same diff, instruction 6 (jump(JEQ_K, SYS_socket, 0, 4)) has jt = 7, which is stmt(LD_W_ABS, OFF_ARG0) — a load. Instructions 8 and 9 likewise fall through into further jumps. A correct filter must be able to fall through a comparison into more computation, so the assertion is wrong in principle, not just mis-tuned. The diff does not show a test run, so either the test is red or the filter differs from what is shown; either way the two artifacts here are mutually inconsistent. This is the one finding I'd block on, because the stated purpose of the test — catching an off-by-one jt/jf that the pre-existing length assertion could not — is not achieved by the predicate as written.

2. sentinel-audit/src/log.rs:150 — an interior unsigned event escapes verification. The sidecar-write contract is documented ("recorded but unsigned, abort"), but it's advisory. If a caller continues after one sidecar failure, the next successful checkpoint sets covered = max(event_count), so unsigned_tail is 0 and --require-signature reports full coverage while event N carries no signature. Making verify_checkpoints require contiguous coverage (or recording the gap in the log) would close it.

Things I checked and cleared, for the record: the log_vacuum boundary (normalisation rejects .. outright, so the post-validate_args normalisation cannot widen reach); http::read_limited (no underflow at limit == 0, body.len() never exceeds limit); HardenedExecutor::sandbox_for under LandlockMode::Off (consistent with the documented escape hatch, and spawn_checked gates deny_network on the same condition); the policy tightening overlay (strictness ordering makes weakening unreachable, and the property test pins it).

VERDICT: REVISE — new seccomp jump-target test asserts that both branches of every conditional jump land on a RET, which is false for the filter in this diff (instruction 6's jt targets a BPF_LD).

---

VERDICT: REVISE — new seccomp jump-target test asserts that both branches of every conditional jump land on a RET, which is false for the filter in this diff (instruction 6's jt targets a BPF_LD).

[marcuspat]

Gate review — reviewer: claude

Lanes: reviewed by claude

GATE USAGE reviewer=claude in=2 cached=343,534 out=2,550 41s model=claude-opus-5[1m] $3.403 API-equiv (not billed on subscription)

This is a review request. Let me work through the diff.

Key areas I examined closely:

1. sentinel-exec/src/executor.rs — sandbox_for for FsAccess::Unrestricted

FsAccess::Unrestricted => {
    let mut base = self.sandbox.clone();
    if self.landlock == LandlockMode::Require {
        base.require_enforcement = true;
    }
    return base;
}

The base sandbox from for_builtin_capabilities() has empty read_only_paths/writable_paths, so wants_landlock() is false and no Landlock error path is reachable. But spawn_checked then sets sandbox.deny_network = true when !network && landlock != Off. package_upgrade uses Unrestricted and is a network command, so it's exempt — consistent. However a non-network command with Unrestricted would get deny_network = true plus require_enforcement = true under Require, which is the intended fail-closed. Consistent with the r2 comment.

2. log_vacuum boundary — normalise_components rejects .. outright but the find output filter calls it per-path

find prints paths rooted at log_dir, which has already been normalised. A file genuinely named .. cannot exist. A path containing a .. component would be dropped rather than collapsed — stricter, fine.

One real gap: the prefix check uses starts_with(&prefix). normalise_components is applied to both sides, so /var/log/application/x.log vs prefix /var/log/app/ correctly fails. Good.

3. sentinel-audit/src/log.rs — sidecar write after the event is durable

The comment documents the contract explicitly: event already appended, Err means "recorded but unsigned". self.events.push happens before the signer block, and next_sequence is already incremented, so a sidecar failure leaves the in-memory chain intact and returns Err. Callers like AuditWriter::record map that to RunError::Audit, halting execution. That matches rule 6 and the documented intent.

4. sentinel-mcp/src/execute.rs — the finished_at block

let count = |label: &str| ...;

The closure referenced in the diff hunk counts over outcomes, which is built from report.steps. steps_rolled_back is newly populated. steps_failed = count("Failed") + count("Denied"). A step that was rolled back is no longer counted as Completed (its state became RolledBack), so steps_completed correctly drops. Consistent.

5. seccomp filter jump targets

The test asserts in-bounds for all jt/jf and pins the six documented targets plus three RET terminals. Walking the program: instruction 4 is JGE X32_SYSCALL_BIT, jt=8 → 4+1+8 = 13 (ENOSYS) ✓. Instruction 5 JEQ io_uring_setup, jt=7 → 13 ✓. Instruction 6 JEQ socket, jt=0, jf=4 → fall-through to 7 (load arg0), else 6+1+4 = 11 (ALLOW) ✓. 8/9/10 jt=3,2,1 → 12 (EACCES) ✓. Correct as built, and the test would catch an offset edit.

6. sentinel-policy/src/resource_guard.rs — normalise_unit suffix loop

while let Some(stripped) = SUFFIXES.iter().find_map(|s| unit.strip_suffix(s)) {
    unit = stripped.trim().to_string();
}

Terminates: each iteration strictly shortens. getty@tty1.service → split at @ gives getty, and the suffix is discarded with the instance. A guard pattern getty@* would no longer match the normalised form, but glob_match(pattern, value) is also tried against the raw value, so both spellings are covered.

7. ResilientBackend::backoff — potential truncation

Duration::from_nanos((exp.as_nanos() as u64 / 1000).saturating_mul(scale))

exp.as_nanos() / 1000 is microseconds; multiplying by scale (per-mille, 500..=1000) gives nanos. For exp = 100ms: 100_000 µs × 500 = 50_000_000 ns = 50ms. Correct. The property test bounds it at [full/2, full] and passes by construction.

8. http.rs read_limited — limit - body.len()

room could underflow if body.len() > limit, but the function returns as soon as chunk.len() > room, and otherwise body.len() <= limit always holds, with equality only possible when the next chunk has len > 0 == room → returns. A zero-length chunk with room == 0 would extend by nothing and loop; reqwest doesn't yield empty chunks, and even if it did the loop would terminate on stream end. Not a defect worth flagging.

9. GateView::refresh — PlanStore::open creates directories

Guarded by if !self.state_dir.join("plans").is_dir(), with a test asserting the state dir isn't created. Correct.

10. verify_audit — --require-signature requires --pubkey via clap requires

And the unsigned path returns early before the key parse. The sig.error branch exits 1 before fully_signed(). Truncation (event_count > total) is caught. Correct.

11. claim_for_execution + update lock ordering

update takes the lock, then load, then change, then save. save does not take the lock itself — correct, since it's called under one. But approve/reject/claim_for_execution all go through update, and the bare save is documented as "brand-new plan, or while holding the lock". The execute.rs path calls store.save(&rec) at the end (in the finished_at block) without the lock. That's a write outside the lock — but the plan is in Executing, which exactly one process claimed, so no other writer can be in a transition for it concurrently. A concurrent approve/reject would fail expect_status anyway. Not exploitable.

12. Missing-test check on the Unrestricted/Require interaction

sandbox_for_maps_access_to_profiles asserts free.require_enforcement under Require. Covered.

I found no defect that survives verification. A few things I considered and dismissed as non-defects: the log_vacuum relative-path rejection is stricter than the CHANGELOG's wording but not wrong; the GateView default reading SENTINEL_STATE_DIR directly rather than through a shared resolver is duplication, not a bug; README.md says "14 built-in capabilities" while BUILTIN_COMMANDS lists 18 programs — different things, both accurate.

The diff is large but internally consistent: each security claim in the CHANGELOG has a corresponding enforcement point and a test that fails without it, the gate-round follow-ups (r1/r2/r3) moved checks to the enforcement point rather than relying on a caller one layer up, and the new verification (property tests, seccomp jump-target test, race tests) is constructed so it can actually fail.

VERDICT: APPROVED

---

VERDICT: APPROVED