Document remaining limits enforcement work
ober
b0e5b72ebe408e35107c3a011937b59c7b17d15b
new file mode 100644 --- /dev/null +++ b/docs/limits-followup.md @@ -0,0 +1,449 @@ +# Limits Follow-Up: Missing Enforcement and Test Work + +Reviewed: 2026-05-21 + +This document is a follow-up to `docs/limits.md` after reviewing the new +limits/sandbox/audit module set added around commit `97eea41`. + +The short version: the new modules are a useful start, and the modules load +under the normal repo Scheme runtime, but the `docs/limits.md` requirements +are not fully implemented yet. Several pieces are prototypes, static reports, +or partial helpers without fail-closed enforcement. + +## Current Implemented Pieces + +New Jerboa modules exist for the main proposed primitives: + +- `(std os limits)` +- `(std os limits sandbox)` +- `(std os supervise)` +- `(std os exec-id)` +- `(std os tracefs)` +- `(std net allow-proxy)` +- `(std security env)` +- `(std os temp-home)` +- `(std security audit-log)` + +Basic import smoke tests passed for all of those modules under: + +```sh +/Users/user/mine/jerboa/.chez/bin/scheme --libdirs lib --script <script> +``` + +`make build` also completed successfully, but it only reported a small number +of compiled modules and does not prove these new modules have behavioral test +coverage. + +## Required Standard Before Calling This Done + +Do not mark `docs/limits.md` implemented until every item is in one of these +states: + +- Enforced by a tested backend. +- Explicitly reported as degraded or unavailable at runtime. +- Protected by a caller-visible fail-closed option. +- Moved to a documented non-goal. + +Static "this OS should support X" reporting is not enough. Callers need to know +what actually happened for the specific child launch. + +## 1. Unified Sandbox API + +File: `lib/std/os/limits/sandbox.ss` + +Implemented: + +- `sandbox-policy` record with read, write, exec, net, syscall, ptrace, and + no-new-privs fields. +- `sandbox-capabilities`. +- `sandbox-launch`. +- macOS SBPL generation for path policies. +- Linux Landlock hook via `jerboa_landlock_sandbox`. + +Missing or wrong: + +- There is no fail-closed API. `docs/limits.md` says a caller must be able to + require full filesystem confinement or fail. The current API reports support + but does not let the caller say "abort if fs is degraded/unavailable". +- `sandbox-launch` returns static capability data, not per-launch install + results. The child calls `sandbox-prepare-child!`, but that report cannot + cross the fork boundary, so the parent result cannot say whether Landlock, + Seatbelt, pledge, or limits were actually installed. +- `sandbox-result-limit-report` is misleading. A test with only `(mem . 1)` + requested reported every limit capability as `attempted`, including + `platform`. It should report only requested limits and their actual status. +- Default policy semantics are unclear for callers. The policy record defaults + `net` to `deny`, but a simple macOS `sandbox-launch` with no path policy ran + `/bin/echo` successfully. That may be correct for a network-only SBPL, but it + needs tests proving network is actually blocked while normal process startup + still works. +- macOS `sandbox-capabilities` reports `(fs . installed)`, but path policies are + implemented by wrapping with `/usr/bin/sandbox-exec` and default allow mode is + used when no paths are specified. The capability report should distinguish + "can wrap target in sandbox-exec" from "this process is already confined". +- FreeBSD and OpenBSD helpers mostly return `degraded`. That is acceptable only + if callers can fail closed and tests assert the degraded report. +- Network allowlists are always degraded. That is honest, but the API should + expose a structured requirement for a proxy handoff instead of treating + allowlist as just another net mode. + +Required next work: + +1. Add a `sandbox-requirements` or keyword options to `sandbox-launch`, for + example `fail-closed?: #t`, `require-fs?: #t`, `require-net?: #t`. +2. Create a child-to-parent status pipe so `sandbox-prepare-child!` and + `limit-policy-install!` can report actual install status before exec. +3. Make `sandbox-result-report` dynamic per launch. +4. Add tests for: + - readable path allowed + - unreadable path denied + - writable path allowed + - write outside allowlist denied + - exec outside allowlist denied where backend supports it + - net denied + - degraded/unavailable reported when backend cannot enforce + - fail-closed aborts when a required axis is not fully enforced + +## 2. Process Supervision + +File: `lib/std/os/supervise.ss` + +Implemented: + +- `launch-spec`. +- `process-result`. +- `supervise-run`. +- process-group setup. +- optional stdout/stderr capture. +- parent-side timeout and output caps. + +Observed problems: + +- Timeout behavior is wrong when output capture is enabled. Running + `/bin/sleep 2` with `timeout-ms: 100` and capture enabled waited roughly the + full sleep duration and returned status `0` while still reporting + `killed-reason timeout`. +- Signal status is computed incorrectly. The code uses `(- 128 sig)`, which + produced `113` for SIGTERM. Conventionally this should be `(+ 128 sig)`, + which would be `143`. +- Output caps kill the child but also use the incorrect signal status formula. + A stdout cap test returned status `119` for SIGKILL instead of `137`. +- The timeout loop should not sleep or wait in a way that lets a captured child + finish normally after the timeout has been marked. +- There is no exact tree tracking yet. Process-group kill is useful, but + `docs/limits.md` asks for the support precision to be tested and reported. + +Required next work: + +1. Fix signal-derived statuses to `128 + signal`. +2. Fix timeout handling with captured pipes. After killing a timed-out process, + continue draining only until the process is reaped, and preserve the killed + status. +3. Add regression tests for: + - timeout without capture + - timeout with stdout capture + - timeout with stderr capture + - stdout cap + - stderr cap + - child that forks a background process and gets process-group killed +4. Include a test that verifies elapsed time is below a sane threshold for a + timed-out command. + +## 3. Resource Limits + +File: `lib/std/os/limits.ss` + +Implemented: + +- `limit-policy`. +- `limit-policy-set!`. +- `limit-policy-install!`. +- support statuses for setrlimit-backed and parent-side limits. + +Missing or incomplete: + +- Linux cgroup v2 is only reported as available/unavailable. There is no cgroup + creation or delegated-controller integration for tree-wide memory, pids, or + CPU. +- `time-ms` and `out-bytes` are only parent-side markers. They depend on + `(std os supervise)`, and supervise currently has timeout/capture bugs. +- macOS memory is reported as degraded or unavailable, which is honest, but a + caller needs fail-closed semantics if memory enforcement is required. +- `RLIMIT_NPROC` is known to be user-wide or otherwise not tree-local on some + systems. The module reports `degraded` for pids on Linux, but tests need to + assert this behavior. +- There are no dedicated tests for each limit kind. + +Required next work: + +1. Add cgroup v2 support where a delegated cgroup is available. +2. Keep setrlimit fallback, but expose exact caveats per platform. +3. Make `limit-policy-install!` return exact requested-limit results, not a + capability summary. +4. Add tests for `mem`, `cpu-sec`, `nofile`, `fsize`, `core`, `time-ms`, and + `out-bytes`, with platform-aware skips. + +## 4. Executable Identity + +File: `lib/std/os/exec-id.ss` + +Implemented: + +- PATH resolution through explicit path strings. +- realpath canonicalization. +- device and inode capture. +- optional SHA-256 hashing. + +Remaining work: + +- Add tests for symlinks, PATH shadowing, non-executable files, missing files, + and empty PATH components. +- Hashing reads the whole file into memory. That is acceptable for small tools + but should be documented or replaced with streaming hashing. +- Higher-level consumers need to compare identities, not just render them. + Add helpers such as `exec-id-same-file?` or `exec-id-matches?`. + +## 5. Filesystem Access Tracing + +File: `lib/std/os/tracefs.ss` + +Implemented: + +- Linux `strace` command builder when `strace` exists. +- parser for a subset of `strace -f -e trace=file,desc` lines. +- summary by syscall/path. + +Missing relative to `docs/limits.md`: + +- On macOS this reports `unavailable`; no fallback tracing is implemented. +- No native ptrace/eBPF backend. +- No dtrace, EndpointSecurity, ktrace, truss, or Capsicum-aware backend. +- The parser does not maintain enough process state for full path resolution: + cwd per process, fd table, fd inheritance, openat directory fd mappings, and + exec identity changes are still missing. +- Events do not use the richer `fs-event` shape from the design document. They + record syscall and one path, not normalized operation classes such as read, + write, create, delete, rename, metadata, directory listing, symlink, or exec. +- `tracefs-strace-cmd` silently returns the raw target command when tracing is + unavailable. That can look like success unless the caller separately checks + `tracefs-mode`. + +Required next work: + +1. Add an explicit `tracefs-capabilities` report. +2. Make unavailable tracing impossible to confuse with successful empty traces. +3. Track cwd and fd state for Linux `strace`. +4. Normalize events into the design's `fs-event` format. +5. Add tests for open, stat, unlink, rename, exec, relative paths, openat, and + forked children. + +## 6. Network Allowlist Proxy + +File: `lib/std/net/allow-proxy.ss` + +Implemented: + +- A local CONNECT proxy skeleton. +- host:port pattern matching with exact, `*`, and `**` host wildcards. +- allow/deny counters and logger callback. + +Critical missing security requirements: + +- No denial of IP literals. +- No denial of loopback, link-local, private, or local network ranges. +- No DNS resolution in the trusted parent followed by deny-range rechecking. +- No optional SOCKS5 proxy. +- No structured network audit event type. +- No integration with a child sandbox that denies all outbound network except + the local proxy endpoint. + +Observed behavior: + +- With an allowlist of `*:443`, both `127.0.0.1:443` and `10.0.0.1:443` were + accepted by `allow-proxy-host-allowed?`. That violates the localnet and + IP-literal denial requirements. + +Required next work: + +1. Add proxy policy fields: + - `deny-ip-literals?` + - `deny-localnet?` + - `allow-localhost-proxy-only?` +2. Parse host literals, bracketed IPv6 literals, and plain IPv4 literals. +3. Resolve DNS in the proxy before connecting. +4. Reject resolved addresses in loopback, link-local, private, multicast, and + other local ranges unless explicitly allowed. +5. Emit structured events for allowed, denied, DNS failure, and connect failure. +6. Add tests for: + - exact allow + - wildcard allow + - denied host + - denied IPv4 literal + - denied IPv6 literal + - denied 127.0.0.1 + - denied 10/8, 172.16/12, 192.168/16 + - denied link-local + +## 7. Environment and Secret Broker + +File: `lib/std/security/env.ss` + +Implemented: + +- Allowlist-based env construction. +- deny patterns. +- explicit secret injection. +- redaction helpers. +- command argv secret scanner. + +Missing or incomplete: + +- No default policy is provided. Callers must remember to add deny patterns + such as `*TOKEN*`, `*KEY*`, and `AWS_*`. +- `env-policy-scan-command` only reports leaks. There is no helper that refuses + launch or returns a typed error. +- No runtime-only versus install-phase secret distinction. +- No memory wiping/zeroization for secret values. +- No integration with `(std security audit-log)` beyond callers manually + passing redactors. + +Required next work: + +1. Add a hardened default env policy constructor. +2. Add `env-policy-validate-command` that returns ok/error and can be used by + launchers before exec. +3. Add install-phase/runtime-phase support. +4. Add tests proving token-like env vars are stripped even when accidentally + allowlisted. +5. Add tests proving redaction catches nested audit strings and argv. + +## 8. Temporary Home and Cache Helpers + +File: `lib/std/os/temp-home.ss` + +Implemented: + +- Per-run temp root. +- fake HOME. +- scratch directory. +- named cache directories. +- copy selected config files. +- env override helper. +- cleanup modes. + +Remaining work: + +- Not integrated with sandbox policy generation. The helper returns paths, but + callers still have to remember to grant exactly those paths. +- Copy-config only copies flat files by basename into fake HOME. That is safe + but limited; if recursive config copying is later added, it must avoid + symlink escapes and permission broadening. +- Needs tests for preserve-on-error, cleanup modes, cache env vars, and copied + config behavior. +- Needs audit metadata integration. + +## 9. Structured Audit Model + +File: `lib/std/security/audit-log.ss` + +Implemented: + +- Append-only in-memory record list. +- JSONL rendering. +- human summary. +- redactor callback. + +Missing: + +- No fixed schema validation. +- No integration with sandbox, limits, supervise, tracefs, proxy, or env modules. +- No guarantee that all launchers use the redactor before emitting argv or + arbitrary fields. +- No event constructors for the named design record types. + +Required next work: + +1. Add constructors for specific event types, for example: + - `audit-process-start` + - `audit-process-exit` + - `audit-sandbox-installed` + - `audit-limit-installed` + - `audit-net-event` + - `audit-fs-event` +2. Add tests that raw secret values are redacted across strings, arrays, alists, + command argv, and nested structures. +3. Use this module from the other new primitives instead of leaving audit as a + separate optional tool. + +## 10. Tests Missing From This Repo + +At review time, searching `tests/` did not find dedicated tests for the new +modules by name. Add a focused test file, for example: + +```text +tests/test-limits-primitives.ss +``` + +Minimum coverage: + +- `limits-capabilities` shape. +- `limit-policy-install!` per requested kind. +- `supervise-run` success, failure, timeout, output cap, process-group kill. +- `sandbox-capabilities` shape and fail-closed behavior. +- `sandbox-launch` dynamic report. +- `exec-id-resolve` path, symlink, missing, hash. +- `tracefs-parse-strace` for basic syscalls. +- `allow-proxy-host-allowed?` exact, wildcard, localnet denial. +- `env-policy-build` strips denied vars and injects explicit secrets. +- `temp-home` cleanup and env overrides. +- `audit-log` redaction and JSONL validity. + +## Suggested Implementation Order + +1. Fix `(std os supervise)` status and timeout behavior first. Many other + features depend on the supervisor being correct. +2. Add dynamic child-to-parent status reporting for sandbox and limits. +3. Add fail-closed support to sandbox and limits. +4. Fix allow-proxy IP/localnet denial and DNS recheck. +5. Add tests for all of the above. +6. Wire audit-log into the primitives. +7. Expand tracefs from a parser prototype into a real traced-run API. + +## Commands Used During Review + +These commands are useful regression checks: + +```sh +make build + +/Users/user/mine/jerboa/.chez/bin/scheme --libdirs lib --script /dev/stdin <<'EOF' +(import (std os limits) + (std os supervise) + (std os limits sandbox) + (std os exec-id) + (std os tracefs) + (std net allow-proxy) + (std security env) + (std os temp-home) + (std security audit-log)) +(display "loaded all new limit primitives\n") +EOF +``` + +Targeted supervisor timeout check: + +```scheme +(import (std os supervise)) +(define r + (supervise-run + (launch-spec + 'command: '("/bin/sleep" "2") + 'timeout-ms: 100 + 'capture-stdout?: #t + 'capture-stderr?: #t))) +(write (process-result->alist r)) +(newline) +``` + +The expected result should be a timeout kill with a nonzero conventional signal +status. Returning status `0` is a bug.