diff --git a/CHANGELOG.md b/CHANGELOG.md index ec2f077..b047552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ On a release, the maintainer moves the accumulated `[Unreleased]` entries into a ## [Unreleased] +- **feat(egress-hard): OPT-IN hard egress boundary for the container sandbox backend (`NILCORE_EGRESS_HARD`).** The container egress allowlist is COOPERATIVE by default — a model command that ignores `HTTP(S)_PROXY` (`curl --noproxy`, raw sockets, `/dev/tcp`) reaches any host. Hard mode makes it UNBYPASSABLE: `applyContainerEgress` (under the opt-in) routes the box through the new `Container.AllowEgressViaHard`, which attaches the sandbox to a per-run `--internal` docker/podman network (no default route) and runs the allowlist proxy as a dual-homed GATEWAY container (a new hidden `nilcore egress-gateway` verb) the sandbox reaches by IP; a raw socket / `--noproxy` then has no route out and fails, while the sandbox keeps `--cap-drop=ALL`. Fail-closed on ANY setup error (stays `--network none`, never cooperative-fallback); the sandbox `--dns` is pinned at the gateway to blackhole in-sandbox DNS; an idempotent teardown + a label-scoped boot reaper (`reapHardEgress`, non-blocking, gated on the opt-in) reclaim orphans. Honest residuals documented (DNS-tunnel only mitigated; requires the nilcore image, not `debian:stable-slim`; Linux-container only, CI-validated). The namespace backend's empty netns remains the RECOMMENDED hard boundary; `NILCORE_EGRESS_STRICT` stays the fail-closed cooperative refusal. Default (opt-out) path is byte-identical. Unit-tested: the sandbox arg assembly (`AllowEgressViaHard`/`--dns`), the STRICT/hard decision seam (fake `setupHardEgressFn`), and the `egress-gateway` proxy logic; the container lifecycle is CI-only. _Owns:_ `internal/sandbox`, `cmd/nilcore/egress_gateway.go`, `cmd/nilcore/egress_hard.go`, `cmd/nilcore/chat.go` (applyContainerEgress), `cmd/nilcore/main.go` (dispatch + serve reaper), `docs/ARCHITECTURE.md`. _(Phase 7 / deferred hardening)_ + - **chore(audit-2026-07-10-remediation): fresh 12-agent adversarial audit → fix every CRITICAL/HIGH/MEDIUM finding + the cheap LOWs; all three gates green.** A fresh adversarial audit (10 commissioned auditors + independent per-finding verification) at 63292d7 surfaced **1 CRITICAL, ~11 HIGH, ~12 MEDIUM** (plus LOW/hygiene), every one personally verified; all are fixed here with a discriminating test, and `make verify` + `make test-race` (0 data races) + `make tui-verify` are green. **CRITICAL:** the browser/desktop `{{secret:NAME}}` type action resolved ANY process env var (a confused-deputy exfil primitive — a hostile/injected page could type `{{secret:ANTHROPIC_API_KEY}}` into a form and submit it to an egress-allowed sink); it now honors an operator-declared per-task secret-name allowlist (`-secrets` / `NILCORE_{BROWSE,DESKTOP}_SECRETS`, default empty = deny-all) and capguard axis-B counts a secret-capable session. **HIGH:** the host-side git tool can no longer be tricked into repo-local-config RCE (writes inside `.git` are refused at the `worktreefs.writeNoFollow` chokepoint + the git tool passes per-command `--no-ext-diff`); container egress is documented as a cooperative proxy (not a hard boundary) with a `NILCORE_EGRESS_STRICT` fail-closed opt-in; vcache no longer replays a stale GREEN past a changed evidence artifact (the `.nilcore/artifacts` digest is folded into the key under evidence-verify); a value-bearing artifact claim can no longer ship a hollow green on a value-blind verifier (url_resolves/not_stale/variance_bounded); `sleep`/suspend preserves committed work on a durable `suspend/` branch instead of `git branch -D`ing it; race-recovered verified work keeps its branch under KeepBranch; the desktop per-action gate is armed on `--mac-host` (every mutation gated); the webhook self-start is rate/label-bounded (denial-of-wallet); `nilcore chat`/`tui` resolve `-backend auto`; the circuit breaker is no longer poisoned by user steers/cancels. **MEDIUM:** the promote gate scopes on the target base (not the source tip); MCP responses are size-bounded (host OOM); `VAR=0` now DISABLES (not enables) NILCORE_{AUTONOMY,FLYWHEEL,LIVE_INDEX,LESSONS}; codeintel indexers read with O_NOFOLLOW + a size cap; the run/chat/serve advisor is metered against the budget wall; `swarm -blast-radius` is wired; the memory table is pruned/bounded; the webhook shares serve's one store handle; a cancelled drive unwinds cleanly (no false "Run errored"); a cleanly-errored task isn't re-executed on serve boot; the converged `nilcore build` deliverable is pinned before cleanup. **LOW:** eventlog redaction covers `[]string`/`json.RawMessage`; Slack WS frames are length-capped; the Telegram token can't ride a URL error. Integration-seam fix caught by the assembled-diff gate: a defense-in-depth `-c diff.external=` (empty) made git exec the empty string and broke every `git diff` — removed (the `.git` write-guard is the real defense). _Owns:_ repo-wide (see the diff). _(remediation)_ - **chore(license-apache-2.0)** — Add the Apache License 2.0, an RNT56/NilCore NOTICE attribution, and README license badge/section. _Owns:_ `LICENSE`, `NOTICE`, `README.md`, `CHANGELOG.md`. _(docs)_ diff --git a/cmd/nilcore/chat.go b/cmd/nilcore/chat.go index 61207dc..b52e5b1 100644 --- a/cmd/nilcore/chat.go +++ b/cmd/nilcore/chat.go @@ -183,6 +183,17 @@ func chatMain(args []string) { egress, proxyAddr, stopProxy := startEgress(ctx, allow, console, proxyBindAddr(*cf.common.sandboxPref, *cf.common.runtime)) defer stopProxy() + // Rule of Two (§2): evaluate the lethal trifecta ONCE at startup — untrusted web input + // (A) ∧ private repo data (B, the mounted worktree) ∧ open egress (C). The shipped + // default egress is deny-all, so the verdict is Allow and this is byte-identical; only + // a wide/wildcard egress allowlist trips it. chat is attended, so a trip prompts once at + // the console. NILCORE_RULE_OF_TWO=0 opts out. + if err := enforceRuleOfTwo(log, ruleOfTwoEnforced(), !egress.Empty(), true, egress, + policy.NewConsoleApprover(os.Stdin, os.Stdout), + "start a coding session combining untrusted web input, private repo data, and open egress"); err != nil { + fatal(err) + } + sess, err := buildChatSession(chatDeps{ flags: cf, provider: prov, @@ -476,9 +487,12 @@ func splitHosts(s string) []string { // for docker, with an --add-host so it resolves on docker-Linux too). // egressWarnOnce/egressStrictWarnOnce bound the container-egress security advisories // to one line per process (applyContainerEgress runs per drive / per swarm worker). +// egressHardWarnOnce/egressHardFailWarnOnce do the same for the HARD-mode advisories. var ( - egressWarnOnce sync.Once - egressStrictWarnOnce sync.Once + egressWarnOnce sync.Once + egressStrictWarnOnce sync.Once + egressHardWarnOnce sync.Once + egressHardFailWarnOnce sync.Once ) func applyContainerEgress(box sandbox.Sandbox, egress policy.Egress, proxyAddr, runtime string) { @@ -489,6 +503,16 @@ func applyContainerEgress(box sandbox.Sandbox, egress policy.Egress, proxyAddr, if !ok { return } + // HARD egress (opt-in NILCORE_EGRESS_HARD): make the allowlist UNBYPASSABLE via a + // --internal network + a dual-homed gateway container, instead of the cooperative + // bridge+proxy below. On ANY setup failure this FAILS CLOSED — the box stays + // --network none (deny-all) — and NEVER silently falls back to cooperative bridge. + // Linux-container only, CI-validated (see egress_hard.go). Default (unset) ⇒ the + // existing STRICT/cooperative paths below run byte-identically. + if envOptIn("NILCORE_EGRESS_HARD") { + applyHardEgress(c, egress, runtime) + return + } // The container backend's egress allowlist is enforced by a COOPERATIVE proxy over a // bridged network — a model-emitted command that ignores HTTP(S)_PROXY (curl // --noproxy, raw sockets, /dev/tcp) can still reach arbitrary hosts, including cloud @@ -524,6 +548,28 @@ func applyContainerEgress(box sandbox.Sandbox, egress policy.Egress, proxyAddr, c.AllowEgressVia(policy.ProxyURL(net.JoinHostPort(hostAlias, port))) } +// applyHardEgress wires the container to a HARD egress boundary (opt-in +// NILCORE_EGRESS_HARD): the allowlist proxy runs as a dual-homed gateway on a +// --internal network with no route out, so it is UNBYPASSABLE (see egress_hard.go). +// It reuses ONE gateway per (runtime,image,allowlist) across drives. On ANY setup +// failure it FAILS CLOSED — the box keeps --network none (deny-all) — and never falls +// back to the cooperative bridge. Callers pass the container box (already cast). +func applyHardEgress(c *sandbox.Container, egress policy.Egress, runtime string) { + egressHardWarnOnce.Do(func() { + fmt.Fprintln(os.Stderr, "nilcore: NILCORE_EGRESS_HARD set — routing container egress through a --internal-network gateway (the allowlist becomes unbypassable). Linux-container only; requires the nilcore image (a debian:stable-slim has no nilcore binary); the DNS-tunnel residual is only mitigated. The namespace backend (Linux) remains the recommended hard boundary.") + }) + h, ok := getHardEgress(runtime, c.Image, egress) + if !ok { + // FAIL CLOSED: leave the box at --network none. Never cooperative-fallback. + egressHardFailWarnOnce.Do(func() { + fmt.Fprintln(os.Stderr, "nilcore: NILCORE_EGRESS_HARD — hard egress setup FAILED; egress stays deny-all (--network none). Ensure a Linux container runtime + the nilcore image, or unset NILCORE_EGRESS_HARD to accept cooperative egress.") + }) + return + } + c.AllowEgressViaHard(h.network, h.proxyURL) + c.DNS = h.dns +} + // containsString reports whether s is present in xs (small linear scan — the slices // it guards, like a box's ExtraHosts, hold only a handful of entries). func containsString(xs []string, s string) bool { diff --git a/cmd/nilcore/chat_test.go b/cmd/nilcore/chat_test.go index 1c7621e..dfabc98 100644 --- a/cmd/nilcore/chat_test.go +++ b/cmd/nilcore/chat_test.go @@ -608,6 +608,107 @@ func TestApplyContainerEgress(t *testing.T) { }) } +// TestApplyContainerEgressHard proves the HARD-mode DECISION seam in +// applyContainerEgress without a real container: with NILCORE_EGRESS_HARD set and a +// FAKE setupHardEgressFn, an ok setup routes the box through the internal-net gateway +// (AllowEgressViaHard + --dns), while a failing setup FAILS CLOSED (the box stays +// --network none, no proxy env) — it must NEVER fall back to the cooperative bridge. +func TestApplyContainerEgressHard(t *testing.T) { + egress := policy.Egress{Allowed: []string{"example.com"}} + + // Isolate package-level hard-egress state so cache/teardown carry-over between + // subtests (and other tests) can't taint these assertions. + reset := func() { + hardMu.Lock() + hardTeardowns = nil + hardCache = map[string]hardEgressHandle{} + hardMu.Unlock() + } + restore := setupHardEgressFn + t.Cleanup(func() { setupHardEgressFn = restore; reset() }) + + t.Run("setup ok routes through the internal-net gateway", func(t *testing.T) { + reset() + t.Setenv("NILCORE_EGRESS_HARD", "1") + var teardownCalls int + setupHardEgressFn = func(runtime string, e policy.Egress, image string) (string, string, func(), error) { + return "nilcore-egr-net-fake", "http://10.42.0.5:3128", func() { teardownCalls++ }, nil + } + box := sandbox.NewContainer("podman", "img", "/work") + applyContainerEgress(box, egress, "0.0.0.0:54321", "podman") + + if box.Network != "nilcore-egr-net-fake" { + t.Errorf("hard ok: Network = %q, want the internal net", box.Network) + } + if box.Network == "bridge" { + t.Errorf("hard mode must NEVER use the cooperative bridge") + } + if box.Env["HTTP_PROXY"] != "http://10.42.0.5:3128" { + t.Errorf("hard ok: HTTP_PROXY = %q, want the gateway", box.Env["HTTP_PROXY"]) + } + if box.DNS != "10.42.0.5" { + t.Errorf("hard ok: DNS = %q, want the gateway IP (blackhole in-sandbox DNS)", box.DNS) + } + if len(box.ExtraHosts) != 0 { + t.Errorf("hard mode must add no --add-host, got %v", box.ExtraHosts) + } + // Reused across drives: a second apply must not spawn a second gateway. + before := teardownCalls + box2 := sandbox.NewContainer("podman", "img", "/work") + applyContainerEgress(box2, egress, "0.0.0.0:54321", "podman") + hardMu.Lock() + n := len(hardTeardowns) + hardMu.Unlock() + if n != 1 { + t.Errorf("hard gateway should be reused per (runtime,image,allowlist); teardowns=%d, want 1", n) + } + if teardownCalls != before { + t.Errorf("reuse must not tear down the shared gateway") + } + // A clean drain fires the single registered teardown exactly once. + stopHardEgress() + if teardownCalls != 1 { + t.Errorf("stopHardEgress should fire the gateway teardown once, got %d", teardownCalls) + } + }) + + t.Run("setup failure fails closed (never bridge)", func(t *testing.T) { + reset() + t.Setenv("NILCORE_EGRESS_HARD", "1") + setupHardEgressFn = func(string, policy.Egress, string) (string, string, func(), error) { + return "", "", nil, errStub + } + box := sandbox.NewContainer("podman", "img", "/work") + applyContainerEgress(box, egress, "0.0.0.0:54321", "podman") + + if box.Network != "none" { + t.Errorf("hard setup failure must FAIL CLOSED at --network none, got %q", box.Network) + } + for _, k := range []string{"HTTP_PROXY", "HTTPS_PROXY"} { + if box.Env[k] != "" { + t.Errorf("failed hard setup must set no proxy env, got %s=%q", k, box.Env[k]) + } + } + }) + + t.Run("default (opt-out) stays cooperative", func(t *testing.T) { + reset() + // NILCORE_EGRESS_HARD unset ⇒ the fake must never be consulted; cooperative wiring. + setupHardEgressFn = func(string, policy.Egress, string) (string, string, func(), error) { + t.Fatal("hard setup must NOT run when NILCORE_EGRESS_HARD is unset") + return "", "", nil, nil + } + box := sandbox.NewContainer("podman", "img", "/work") + applyContainerEgress(box, egress, "0.0.0.0:54321", "podman") + if box.Network != "bridge" { + t.Errorf("default path should stay cooperative (bridge), got %q", box.Network) + } + }) +} + +// errStub is a sentinel error for the hard-egress failure path. +var errStub = errors.New("stub setup failure") + func TestWebEnabled(t *testing.T) { off := chatDeps{} if off.webEnabled() { diff --git a/cmd/nilcore/egress_gateway.go b/cmd/nilcore/egress_gateway.go new file mode 100644 index 0000000..23211cb --- /dev/null +++ b/cmd/nilcore/egress_gateway.go @@ -0,0 +1,63 @@ +// egress_gateway.go wires the hidden `egress-gateway` verb: the process the HARD +// egress GATEWAY container runs (docs/ARCHITECTURE.md §Execution-model/egress). It is +// deliberately NOT advertised in `nilcore help` — it is machinery the hard-egress +// lifecycle (egress_hard.go) launches inside a dual-homed container, not an operator +// command. It binds a policy.EgressProxy (the SAME allowlist + SSRF guard the +// cooperative proxy uses) on -listen and serves it until the process is signalled. +// +// In hard mode the sandbox is attached to a `--internal` network with no route out; +// its only path off-box is this gateway (dual-homed: internal net + a normal net), +// so the allowlist becomes UNBYPASSABLE rather than merely cooperative. See +// sandbox.Container.AllowEgressViaHard and setupHardEgress. +package main + +import ( + "context" + "flag" + "fmt" + "os" + "os/signal" + "syscall" + + "nilcore/internal/policy" +) + +// egressGatewayMain parses -allow/-listen and serves the allowlist proxy until the +// process receives SIGINT/SIGTERM (the container stop signal). It is the entrypoint +// the gateway container invokes as `nilcore egress-gateway -allow -listen …`. +func egressGatewayMain(args []string) { + fs := flag.NewFlagSet("egress-gateway", flag.ExitOnError) + allow := fs.String("allow", "", "comma-separated allowlist of hosts the sandbox may reach through this gateway") + listen := fs.String("listen", "0.0.0.0:3128", "address:port to bind the allowlist proxy on") + _ = fs.Parse(args) + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + if err := runEgressGateway(ctx, splitHosts(*allow), *listen); err != nil { + fatal(err) + } +} + +// startEgressGateway binds the allowlist proxy on listen and serves it in the +// background, returning the bound address and an idempotent stop func. Extracted as +// the testable seam of the verb: a test can bind 127.0.0.1:0, exercise the running +// proxy (deny→403, allow→past-the-allowlist), then stop it — without a process. +func startEgressGateway(ctx context.Context, allow []string, listen string) (addr string, stop func(), err error) { + proxy := &policy.EgressProxy{Egress: policy.Egress{Allowed: allow}} + return proxy.Start(ctx, listen) +} + +// runEgressGateway starts the gateway and blocks until ctx is cancelled (the verb's +// long-running body). The allowlist + SSRF guard in policy.EgressProxy gate every +// request regardless of bind interface. +func runEgressGateway(ctx context.Context, allow []string, listen string) error { + addr, stop, err := startEgressGateway(ctx, allow, listen) + if err != nil { + return fmt.Errorf("egress-gateway: %w", err) + } + defer stop() + fmt.Fprintf(os.Stderr, "nilcore egress-gateway: allowlist proxy on %s (%d allowed host(s))\n", addr, len(allow)) + <-ctx.Done() + return nil +} diff --git a/cmd/nilcore/egress_gateway_test.go b/cmd/nilcore/egress_gateway_test.go new file mode 100644 index 0000000..cd857e8 --- /dev/null +++ b/cmd/nilcore/egress_gateway_test.go @@ -0,0 +1,82 @@ +package main + +import ( + "context" + "io" + "net/http" + "net/url" + "strings" + "testing" + "time" +) + +// TestEgressGatewayServesAllowlist proves the hidden egress-gateway verb's testable +// core: it binds the allowlist proxy on the given address, refuses a denied host +// with a 403 before any dial, lets an allowlisted host PAST the allowlist (the -allow +// entry took effect — a denied host would 403, this one reaches the SSRF guard), and +// shuts down cleanly on ctx cancel. Hermetic: no host is ever dialed off-box (the +// allowed host is a loopback literal the SSRF guard blocks fast). +func TestEgressGatewayServesAllowlist(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + + // Allow a loopback literal: the allowlist passes it, but the proxy's SSRF guard + // then refuses loopback — a fast, network-free failure that is DISTINCT from the + // "egress denied" 403, so it proves the -allow entry was applied. + addr, stop, err := startEgressGateway(ctx, []string{"127.0.0.1"}, "127.0.0.1:0") + if err != nil { + t.Fatalf("start gateway: %v", err) + } + + proxyURL, _ := url.Parse("http://" + addr) + client := &http.Client{ + Transport: &http.Transport{Proxy: http.ProxyURL(proxyURL)}, + Timeout: 5 * time.Second, + } + + // Denied host ⇒ 403 with an "egress denied" body (rejected before any dial/DNS). + resp, err := client.Get("http://denied.example/") + if err != nil { + t.Fatalf("denied request errored at transport: %v", err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Errorf("denied host status = %d, want 403", resp.StatusCode) + } + if !strings.Contains(string(body), "egress denied") { + t.Errorf("denied host body = %q, want an 'egress denied' message", string(body)) + } + + // Allowed host ⇒ PAST the allowlist. It is not the 403 "egress denied" refusal; + // the SSRF guard then blocks loopback with a 502, proving the allowlist let it in. + resp2, err := client.Get("http://127.0.0.1:9/") + if err != nil { + t.Fatalf("allowed request errored at transport: %v", err) + } + body2, _ := io.ReadAll(resp2.Body) + resp2.Body.Close() + if strings.Contains(string(body2), "egress denied") { + t.Errorf("allowlisted host was refused by the allowlist: %q", string(body2)) + } + if resp2.StatusCode == http.StatusForbidden && !strings.Contains(string(body2), "private/local") { + t.Errorf("allowlisted host got an unexpected 403: %q", string(body2)) + } + + // Shutdown: cancel ctx (the container-stop analog) ⇒ the listener closes and a + // fresh proxied request can no longer be served. + cancel() + stop() + deadline := time.Now().Add(2 * time.Second) + down := false + for time.Now().Before(deadline) { + c := &http.Client{Transport: &http.Transport{Proxy: http.ProxyURL(proxyURL)}, Timeout: 200 * time.Millisecond} + if _, err := c.Get("http://denied.example/"); err != nil { + down = true + break + } + time.Sleep(20 * time.Millisecond) + } + if !down { + t.Error("gateway still serving after ctx cancel + stop; want the listener closed") + } +} diff --git a/cmd/nilcore/egress_hard.go b/cmd/nilcore/egress_hard.go new file mode 100644 index 0000000..608e1c9 --- /dev/null +++ b/cmd/nilcore/egress_hard.go @@ -0,0 +1,290 @@ +// egress_hard.go implements the HARD egress lifecycle for the CONTAINER sandbox +// backend (opt-in NILCORE_EGRESS_HARD; Linux-container only, CI-validated — it is +// NEVER exercised on the macOS host, which is why the setup is behind an injectable +// seam that tests fake). +// +// WHY hard mode exists: the cooperative path (AllowEgressVia) attaches the sandbox to +// a bridged NAT network and points HTTP(S)_PROXY at the allowlist proxy — a model +// command that ignores the proxy (curl --noproxy, raw sockets, /dev/tcp) still +// reaches any host. Hard mode makes the allowlist UNBYPASSABLE: a per-run `--internal` +// docker/podman network has NO default route, so a container attached only to it can +// reach ONLY the internal subnet. We run the allowlist proxy as a DUAL-HOMED GATEWAY +// container (internal net + a normal net) and attach the SANDBOX to the internal net +// only, with HTTP(S)_PROXY pointed at the gateway. Cooperative traffic → gateway → +// allowlist; a raw socket / --noproxy has no route out and fails. The sandbox keeps +// --cap-drop=ALL, so escaping the internal net needs host root / NET_ADMIN. +// +// HONEST RESIDUALS (do NOT overclaim "empty-netns equivalent"): +// 1. The internal net still has DNS (aardvark/embedded), so a DNS-tunnel exfil is +// only MITIGATED, not proven-closed — we point the sandbox's --dns at the gateway +// (which serves no :53) to blackhole in-sandbox resolution; proxied traffic still +// works because the client reaches the proxy by IP. +// 2. The gateway runs `nilcore egress-gateway`, so the sandbox IMAGE must contain the +// nilcore binary — a debian:stable-slim does not, so hard mode requires the +// nilcore/sandbox image (images/sandbox), not the default debian. +// 3. Linux-container only; the lifecycle here is CI-validated, never host-run. +// 4. The per-run network+gateway can leak on crash — teardown is idempotent and the +// boot reaper (reapHardEgress) reclaims label-scoped orphans. +// +// The namespace backend's empty netns remains the RECOMMENDED hard boundary; this is +// the hard option for hosts that must run the container backend. +package main + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "net/url" + "os/exec" + "sort" + "strings" + "sync" + "time" + + "nilcore/internal/eventlog" + "nilcore/internal/policy" +) + +// hardEgressLabel tags every per-run internal network + gateway container hard mode +// creates, so the boot reaper can reclaim orphans by label after a crash. +const hardEgressLabel = "nilcore-egr" + +// hardGatewayPort is the fixed in-container port the gateway's allowlist proxy binds. +const hardGatewayPort = "3128" + +// setupHardEgressFn is the INJECTABLE hard-egress setup seam. Production points it at +// setupHardEgress (which shells out to the container runtime); tests replace it with a +// fake so the STRICT/hard DECISION in applyContainerEgress is unit-testable on the +// macOS host, where no real container can run. +var setupHardEgressFn = setupHardEgress + +// Per-process hard-egress state. applyContainerEgress runs PER DRIVE across +// goroutines, so hardCache reuses ONE gateway per (runtime,image,allowlist) rather +// than spawning a container per drive; hardTeardowns holds each gateway's idempotent +// teardown for a clean process-exit drain, backstopped by the boot reaper on crash. +var ( + hardMu sync.Mutex + hardTeardowns []func() + hardCache = map[string]hardEgressHandle{} +) + +// hardEgressHandle is the wiring one hard gateway exposes to a sandbox box. +type hardEgressHandle struct { + network string // the --internal network name (attach the sandbox here) + proxyURL string // HTTP(S)_PROXY value: the gateway's internal IP:port + dns string // --dns value: the gateway IP (blackholes in-sandbox DNS) +} + +// getHardEgress returns the process's gateway for (runtime,image,egress), setting one +// up on first use. Success is cached (and its teardown registered); a FAILURE is NOT +// cached, so a transient runtime error can recover on a later drive. ok=false means +// setup failed and the caller must FAIL CLOSED (leave the box deny-all). +func getHardEgress(runtime, image string, egress policy.Egress) (hardEgressHandle, bool) { + key := hardCacheKey(runtime, image, egress) + hardMu.Lock() + defer hardMu.Unlock() + if h, ok := hardCache[key]; ok { + return h, true + } + network, proxyURL, teardown, err := setupHardEgressFn(runtime, egress, image) + if err != nil { + return hardEgressHandle{}, false + } + h := hardEgressHandle{network: network, proxyURL: proxyURL, dns: hostFromProxyURL(proxyURL)} + hardCache[key] = h + if teardown != nil { + hardTeardowns = append(hardTeardowns, teardown) + } + return h, true +} + +// hardCacheKey is an order-independent key for a hard gateway: same runtime+image and +// same allowlist ⇒ same gateway reused across drives. +func hardCacheKey(runtime, image string, egress policy.Egress) string { + hosts := append([]string(nil), egress.Allowed...) + sort.Strings(hosts) + return runtime + "|" + image + "|" + strings.Join(hosts, ",") +} + +// hostFromProxyURL extracts the host (the gateway IP) from a "http://ip:port" proxy +// URL, so the same IP can pin the sandbox's --dns. "" if unparseable. +func hostFromProxyURL(proxyURL string) string { + u, err := url.Parse(proxyURL) + if err != nil { + return "" + } + return u.Hostname() +} + +// stopHardEgress drains every registered gateway teardown (idempotent per entry) — the +// clean process-exit path. Safe to call when nothing was set up (a no-op), so the +// default (opt-out) path is unaffected. +func stopHardEgress() { + hardMu.Lock() + fns := hardTeardowns + hardTeardowns = nil + hardCache = map[string]hardEgressHandle{} + hardMu.Unlock() + for _, fn := range fns { + fn() + } +} + +// setupHardEgress is the production hard-egress lifecycle: create a per-run +// `--internal` network (no default route), start the allowlist proxy as a DUAL-HOMED +// gateway container (internal net + a normal net), wait for readiness, and return the +// internal network name, the proxy URL the sandbox should use (the gateway's internal +// IP:port — an IP, so the sandbox needs NO DNS to reach the proxy), and an idempotent +// teardown. Linux-container only; CI-validated (the injectable seam fakes it in unit +// tests). image MUST contain the nilcore binary (residual #2). +func setupHardEgress(runtime string, egress policy.Egress, image string) (network, gatewayProxyURL string, teardown func(), err error) { + suffix, err := hardRandSuffix() + if err != nil { + return "", "", nil, fmt.Errorf("hard egress: rand: %w", err) + } + network = "nilcore-egr-net-" + suffix + gwName := "nilcore-egr-gw-" + suffix + // Idempotent teardown built up-front so a partial setup (net created, gateway + // failed) still cleans up on the error paths below. + teardown = hardTeardownFunc(runtime, network, gwName) + + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + defer cancel() + + // 1. The internal network: NO default route, so a container attached only to it + // can reach ONLY the internal subnet (never the host / internet directly). + if out, e := runtimeCmd(ctx, runtime, "network", "create", "--internal", "--label", hardEgressLabel, network); e != nil { + teardown() + return "", "", nil, fmt.Errorf("hard egress: create internal net: %w: %s", e, strings.TrimSpace(out)) + } + + // 2. The gateway: dual-homed (internal net + a normal net so it can reach the + // allowed hosts), running the allowlist proxy on :3128. + runArgs := []string{ + "run", "-d", "--label", hardEgressLabel, + "--network", network, "--network", defaultNetworkName(runtime), + "--name", gwName, image, + "nilcore", "egress-gateway", "-allow", strings.Join(egress.Allowed, ","), "-listen", "0.0.0.0:" + hardGatewayPort, + } + if out, e := runtimeCmd(ctx, runtime, runArgs...); e != nil { + teardown() + return "", "", nil, fmt.Errorf("hard egress: start gateway: %w: %s", e, strings.TrimSpace(out)) + } + + // 3. Resolve the gateway's IP ON THE INTERNAL NET — the sandbox uses it for both + // HTTP(S)_PROXY (no DNS needed to reach the proxy) and --dns (blackholing + // in-sandbox resolution, since the gateway serves no :53). + ip, e := gatewayInternalIP(ctx, runtime, gwName, network) + if e != nil { + teardown() + return "", "", nil, fmt.Errorf("hard egress: gateway ip: %w", e) + } + + // 4. Readiness: wait until the gateway logs that its proxy is listening (bounded). + if e := waitGatewayReady(ctx, runtime, gwName, image); e != nil { + teardown() + return "", "", nil, fmt.Errorf("hard egress: gateway not ready: %w", e) + } + + return network, policy.ProxyURL(ip + ":" + hardGatewayPort), teardown, nil +} + +// hardTeardownFunc returns an idempotent teardown that force-removes the gateway +// container then the internal network. Best-effort: a missing container/network is +// not an error worth surfacing (the goal is reclamation, not verification). +func hardTeardownFunc(runtime, network, gwName string) func() { + var once sync.Once + return func() { + once.Do(func() { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _, _ = runtimeCmd(ctx, runtime, "rm", "-f", gwName) + _, _ = runtimeCmd(ctx, runtime, "network", "rm", network) + }) + } +} + +// gatewayInternalIP inspects the gateway's IP on the internal network. +func gatewayInternalIP(ctx context.Context, runtime, gwName, network string) (string, error) { + tmpl := fmt.Sprintf(`{{ (index .NetworkSettings.Networks %q).IPAddress }}`, network) + out, err := runtimeCmd(ctx, runtime, "inspect", "-f", tmpl, gwName) + if err != nil { + return "", fmt.Errorf("%w: %s", err, strings.TrimSpace(out)) + } + ip := strings.TrimSpace(out) + if ip == "" { + return "", fmt.Errorf("gateway has no IP on %s", network) + } + return ip, nil +} + +// waitGatewayReady polls the gateway's logs until the proxy reports it is listening, +// bailing fast if the container has already exited (a bad image / missing nilcore +// binary — residual #2). Bounded so a wedged setup fails closed rather than hangs. +func waitGatewayReady(ctx context.Context, runtime, gwName, image string) error { + deadline := time.Now().Add(30 * time.Second) + for time.Now().Before(deadline) { + if ctx.Err() != nil { + return ctx.Err() + } + if out, _ := runtimeCmd(ctx, runtime, "logs", gwName); strings.Contains(out, "allowlist proxy on") { + return nil + } + if st, _ := runtimeCmd(ctx, runtime, "inspect", "-f", "{{.State.Running}}", gwName); strings.TrimSpace(st) == "false" { + return fmt.Errorf("gateway container exited before becoming ready (is the nilcore binary present in image %q?)", image) + } + time.Sleep(300 * time.Millisecond) + } + return fmt.Errorf("timed out waiting for gateway readiness") +} + +// defaultNetworkName is the runtime's normal (routable) network the gateway is also +// attached to so it can reach the allowed hosts. +func defaultNetworkName(runtime string) string { + if runtime == "docker" { + return "bridge" + } + return "podman" // podman's default rootless network +} + +// hardRandSuffix returns a short random hex suffix so concurrent runs never collide on +// a network/container name. +func hardRandSuffix() (string, error) { + var b [6]byte + if _, err := rand.Read(b[:]); err != nil { + return "", err + } + return hex.EncodeToString(b[:]), nil +} + +// runtimeCmd runs ` ` and returns its combined output. Every hard- +// egress runtime interaction goes through here so the lifecycle is one small, +// auditable shell-out surface (I6 — no new module; we reuse the container runtime). +func runtimeCmd(ctx context.Context, runtime string, args ...string) (string, error) { + out, err := exec.CommandContext(ctx, runtime, args...).CombinedOutput() + return string(out), err +} + +// reapHardEgress reclaims orphaned hard-egress gateways + internal networks left by a +// crashed prior process, matched by the nilcore-egr label. It is best-effort and +// NON-BLOCKING (runs in a goroutine): pure housekeeping, never a correctness gate, and +// must not delay serve boot. Idempotent — nothing to reap is a no-op. Callers gate it +// on the hard-mode opt-in so a default (opt-out) boot spawns no runtime processes. +func reapHardEgress(runtime string, log *eventlog.Log) { + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + // Gateways run detached, so force-remove any label-tagged containers (running or + // stopped) FIRST — a running container pins its network from `network prune`. + if ids, err := runtimeCmd(ctx, runtime, "ps", "-aq", "--filter", "label="+hardEgressLabel); err == nil { + for _, id := range strings.Fields(ids) { + _, _ = runtimeCmd(ctx, runtime, "rm", "-f", id) + } + } + // Then prune the now-unused internal networks by label. + if out, err := runtimeCmd(ctx, runtime, "network", "prune", "-f", "--filter", "label="+hardEgressLabel); err != nil && log != nil { + log.Append(eventlog.Event{Kind: "maint_error", Detail: map[string]any{"op": "hard_egress_reap", "error": err.Error(), "out": strings.TrimSpace(out)}}) + } + }() +} diff --git a/cmd/nilcore/main.go b/cmd/nilcore/main.go index 18a2d64..1aacae0 100644 --- a/cmd/nilcore/main.go +++ b/cmd/nilcore/main.go @@ -147,6 +147,11 @@ func main() { browseMain(args[1:]) case "desktop": desktopMain(args[1:]) + case "egress-gateway": + // Hidden verb (NOT in `nilcore help`): the process the HARD egress GATEWAY + // container runs — an allowlist proxy the hard-egress lifecycle launches + // inside a dual-homed container. See egress_gateway.go / egress_hard.go. + egressGatewayMain(args[1:]) default: if strings.HasPrefix(args[0], "-") { runMain(args) // documented `nilcore -goal ...` default @@ -1119,6 +1124,24 @@ func serveMain(args []string) { // worktrees whose directory is already gone are candidates (a live run's // worktree directory is present), so this never collects an active worktree. serveGC(context.Background(), absDir, log) + // Reclaim leaked suspend/ recovery anchors: a resumed drive deletes its own anchor, + // but a nap that was resolved/abandoned before it could be resumed leaves the ref + // behind, so bound the backlog on boot. Best-effort — a sweep error must NEVER block + // serve (it is pure housekeeping, not a correctness gate). + if ckpt != nil { + if err := ckpt.SweepSuspended(context.Background(), absDir, 0); err != nil { + log.Append(eventlog.Event{Kind: "maint_error", Detail: map[string]any{"op": "suspend_sweep", "error": err.Error()}}) + } + } + // Reclaim leaked HARD-egress gateways + internal networks (label-scoped) left by a + // crashed prior process. Best-effort + NON-BLOCKING, and only when hard mode is + // opted in — a default (opt-out) boot spawns no runtime processes, so its behaviour + // is byte-identical. A clean shutdown drains this process's own gateways via the + // deferred stopHardEgress below. + if envOptIn("NILCORE_EGRESS_HARD") { + reapHardEgress(*c.runtime, log) + defer stopHardEgress() + } validateConcreteBackendFlag("-prefer-backend", *c.preferBackend) // Resolve `-backend auto` / config backend:auto to a concrete name before serve // reads it. serve still requires native below; auto simply lets the system pick @@ -1223,6 +1246,16 @@ func serveMain(args []string) { fmt.Fprintf(os.Stderr, "nilcore serve: web access on — search: %s, %d allowed host(s)\n", searchBackend, len(webAllow)) } + // Rule of Two (§2): serve is a HEADLESS daemon, so the lethal trifecta (untrusted web + // input ∧ private repo data ∧ open egress) with no human present is exactly the + // combination the Rule of Two refuses. Default egress is deny-all ⇒ Allow ⇒ + // byte-identical; only a wide/wildcard egress config makes serve refuse to start (nil + // gate ⇒ fail-closed Refuse). Narrow the egress allowlist, or set NILCORE_RULE_OF_TWO=0 + // to opt out. + if err := enforceRuleOfTwo(log, ruleOfTwoEnforced(), !egress.Empty(), true, egress, nil, ""); err != nil { + fatal(err) + } + // The self-timer registry behind the `sleep` tool — durable over the checkpointer's // store, so wakes survive a restart (re-fired by the waker on next boot). Off // without a checkpointer (nil ⇒ no `sleep` tool, no waker). diff --git a/cmd/nilcore/ruleoftwo.go b/cmd/nilcore/ruleoftwo.go new file mode 100644 index 0000000..ffa3ad6 --- /dev/null +++ b/cmd/nilcore/ruleoftwo.go @@ -0,0 +1,93 @@ +package main + +import ( + "fmt" + "os" + "strings" + + "nilcore/internal/capguard" + "nilcore/internal/eventlog" + "nilcore/internal/policy" +) + +// envDisabled reports whether env var name is explicitly turned OFF (0/false/no/off, +// any case, whitespace-trimmed). Unset or any other value ⇒ NOT disabled. It is the +// negative twin of envOptIn, for DEFAULT-ON gates with a documented escape hatch +// (mirrors the NILCORE_KERNEL idiom: on unless explicitly opted out). +func envDisabled(name string) bool { + switch strings.ToLower(strings.TrimSpace(os.Getenv(name))) { + case "0", "false", "no", "off": + return true + default: + return false + } +} + +// ruleOfTwoEnforced reports whether the Rule-of-Two trifecta gate is enforced on the +// main agent paths. DEFAULT-ON; NILCORE_RULE_OF_TWO=0 (or false/no/off) is the +// documented operator opt-out — an acknowledged relaxation of the §2 Rule-of-Two, to +// be used only where the operator has independently accepted the risk. +func ruleOfTwoEnforced() bool { return !envDisabled("NILCORE_RULE_OF_TWO") } + +// enforceRuleOfTwo evaluates the capguard LETHAL TRIFECTA (untrusted web input ∧ +// private repo data ∧ open egress) for a MAIN agent path (do/run/chat/serve/swarm) — +// the tiers that, unlike the browse/desktop tiers, historically never evaluated it even +// though they compute the same axes. It ALWAYS records a metadata-only `capguard` audit +// event (the verdict + the active axes + the axis LABELS — never the resolved host +// list, which stays out of the append-only log per I3/I7, exactly as capability.Event +// does). +// +// When enforce is true and all three axes hold: +// - a real human approver (attended chat/tui/run): prompt once; a denial aborts. +// - a headless approver (serve/swarm/batch): the trifecta with NO human present is +// precisely the lethal combination the Rule of Two exists to refuse — the headless +// deny-default approver fails closed, UNLESS a configured graduated-auto-approval +// envelope + earned trust auto-approves inside its blast fence. +// +// The shipped default egress is deny-all / narrow, so axis C is false and the verdict is +// Allow — byte-identical to before for every normal run. Only a genuinely wide egress (a +// wildcard, or more than capguard.OpenEgressThreshold hosts) combined with web tools and +// a mounted repo trips the gate, which is exactly the configuration §2's Rule of Two +// targets. With enforce=false (the opt-out) the verdict is still logged but never blocks. +// +// approver MUST be a concrete non-nil policy.Approver when a gate is intended (a typed-nil +// interface would satisfy != nil and then panic on Approve); pass a nil interface ONLY to +// model "no gate at all" (⇒ Refuse when the trifecta holds). Callers pass a +// NewConsoleApprover (attended) or a wrapAutoApprove(deny-default) (headless). +func enforceRuleOfTwo(log *eventlog.Log, enforce, untrusted, repoMounted bool, egress policy.Egress, approver policy.Approver, prompt string) error { + caps := capguard.Capabilities{ + UntrustedInput: untrusted, + PrivateData: repoMounted, + EgressHosts: egress.Allowed, + Reasons: map[string]string{ + "A": ternary(untrusted, "web-tools", ""), + "B": ternary(repoMounted, "repo-mounted", ""), + "C": ternary(len(egress.Allowed) > 0, "runtime-egress", ""), + }, + } + dec := capguard.Evaluate(caps, approver != nil) + if log != nil { + // Metadata-only (I3/I7): verdict + active axes + axis LABELS. The resolved host + // list lives only in dec.Detail, which is DELIBERATELY not logged here (parity + // with capability.Descriptor.Event) — and, for the same reason, is kept out of the + // error strings below too. + log.Append(eventlog.Event{Kind: "capguard", Detail: map[string]any{ + "verdict": string(dec.Verdict), + "axes": dec.Axes, + "reasons": caps.Reasons, + "enforced": enforce, + }}) + } + if !enforce || dec.Verdict == capguard.Allow { + return nil + } + switch dec.Verdict { + case capguard.GateRequired: + if approver != nil && approver.Approve(prompt) { + return nil + } + return fmt.Errorf("the lethal trifecta (untrusted web input ∧ private repo data ∧ open egress) was denied at the Rule-of-Two gate; narrow the egress allowlist, or set NILCORE_RULE_OF_TWO=0 to opt out") + default: // Refuse: the lethal trifecta with no gate available (headless, no envelope). + return fmt.Errorf("refusing the lethal trifecta (untrusted web input ∧ private repo data ∧ open egress) with no human gate (Rule of Two); narrow the egress allowlist, run attended, configure a graduated-auto-approval envelope, or set NILCORE_RULE_OF_TWO=0 to opt out") + } +} diff --git a/cmd/nilcore/ruleoftwo_test.go b/cmd/nilcore/ruleoftwo_test.go new file mode 100644 index 0000000..f79034d --- /dev/null +++ b/cmd/nilcore/ruleoftwo_test.go @@ -0,0 +1,100 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "nilcore/internal/eventlog" + "nilcore/internal/policy" +) + +type fakeApprover struct { + ok bool + calls int +} + +func (f *fakeApprover) Approve(string) bool { f.calls++; return f.ok } + +// TestEnforceRuleOfTwo pins the trifecta gate on the main paths: a narrow/empty egress +// (the shipped default) is Allow and never prompts (byte-identical), while the lethal +// trifecta (web + repo + WIDE egress) routes through the gate — attended approves, +// attended denies aborts, headless-no-gate refuses — and the opt-out never blocks. +func TestEnforceRuleOfTwo(t *testing.T) { + narrow := policy.Egress{Allowed: []string{"api.example.com"}} // 1 host, no wildcard → not open + wide := policy.Egress{Allowed: []string{"*.example.com"}} // wildcard → open egress (axis C) + deny := policy.Egress{} // deny-all → axis A + C false + + t.Run("narrow egress => Allow, no prompt, no block", func(t *testing.T) { + ap := &fakeApprover{ok: false} + if err := enforceRuleOfTwo(nil, true, true, true, narrow, ap, "p"); err != nil { + t.Fatalf("narrow egress must Allow (byte-identical), got %v", err) + } + if ap.calls != 0 { + t.Fatalf("narrow egress must not reach the gate, calls=%d", ap.calls) + } + }) + + t.Run("deny-all egress => Allow (axis A and C both false)", func(t *testing.T) { + ap := &fakeApprover{ok: false} + if err := enforceRuleOfTwo(nil, true, false, true, deny, ap, "p"); err != nil { + t.Fatalf("deny-all must Allow, got %v", err) + } + if ap.calls != 0 { + t.Fatalf("deny-all must not reach the gate, calls=%d", ap.calls) + } + }) + + t.Run("wide trifecta + attended approve => proceed", func(t *testing.T) { + ap := &fakeApprover{ok: true} + if err := enforceRuleOfTwo(nil, true, true, true, wide, ap, "p"); err != nil { + t.Fatalf("approved trifecta must proceed, got %v", err) + } + if ap.calls != 1 { + t.Fatalf("trifecta must prompt exactly once, calls=%d", ap.calls) + } + }) + + t.Run("wide trifecta + attended deny => error", func(t *testing.T) { + ap := &fakeApprover{ok: false} + if err := enforceRuleOfTwo(nil, true, true, true, wide, ap, "p"); err == nil { + t.Fatal("denied trifecta must abort") + } + if ap.calls != 1 { + t.Fatalf("trifecta must prompt exactly once, calls=%d", ap.calls) + } + }) + + t.Run("wide trifecta + headless (nil gate) => refuse, fail closed", func(t *testing.T) { + if err := enforceRuleOfTwo(nil, true, true, true, wide, nil, "p"); err == nil { + t.Fatal("headless trifecta with no gate must be refused (fail closed)") + } + }) + + t.Run("opt-out (enforce=false) => never blocks even on the trifecta", func(t *testing.T) { + if err := enforceRuleOfTwo(nil, false, true, true, wide, nil, "p"); err != nil { + t.Fatalf("enforce=false must never block, got %v", err) + } + }) + + t.Run("event is metadata-only: verdict+axes, never the host list", func(t *testing.T) { + dir := t.TempDir() + lp := filepath.Join(dir, "e.jsonl") + log, err := eventlog.Open(lp) + if err != nil { + t.Fatal(err) + } + secretHost := "exfil-sink-999.example.net" + _ = enforceRuleOfTwo(log, true, true, true, policy.Egress{Allowed: []string{"*." + secretHost}}, &fakeApprover{ok: true}, "p") + _ = log.Close() + body, _ := os.ReadFile(lp) + s := string(body) + if !strings.Contains(s, "capguard") || !strings.Contains(s, "\"verdict\"") { + t.Fatalf("expected a capguard verdict event, got:\n%s", s) + } + if strings.Contains(s, secretHost) { + t.Fatalf("the resolved egress host list leaked into the append-only log (I3/I7):\n%s", s) + } + }) +} diff --git a/cmd/nilcore/swarm.go b/cmd/nilcore/swarm.go index 32ff117..197ac3b 100644 --- a/cmd/nilcore/swarm.go +++ b/cmd/nilcore/swarm.go @@ -191,6 +191,17 @@ func swarmMain(args []string) { // iff the run converged with an empty worklist AND the report's chain verifies (so a // tampered log can never read green); otherwise os.Exit(1) after printing the scoreboard. func swarmRun(d swarmDeps) { + // Rule of Two (§2): swarm is a HEADLESS batch path, so the lethal trifecta (untrusted + // web input ∧ private repo data ∧ open egress) with no human present is the combination + // the Rule of Two refuses. Axis C is evaluated over the operator's --egress-allow widen + // — the open-egress vector; the preset's own hosts are curated verify-pack hosts and + // roster.EgressFor narrows per role, so this fails safe. Default (no --egress-allow) ⇒ + // empty ⇒ Allow ⇒ byte-identical; a wide/wildcard widen makes swarm refuse to start (nil + // gate ⇒ fail-closed). Set NILCORE_RULE_OF_TWO=0 to opt out. + swarmEgress := policy.Egress{Allowed: splitCSV(*d.flags.egressAllow)} + if err := enforceRuleOfTwo(d.log, ruleOfTwoEnforced(), len(swarmEgress.Allowed) > 0, true, swarmEgress, nil, ""); err != nil { + fatal(err) + } asm, err := buildSwarm(d) if err != nil { fatal(err) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7762524..89a5293 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -120,6 +120,8 @@ The native loop, Codex, and Claude Code are interchangeable behind this. Adding **Egress proxy lifecycle (web access).** `policy.EgressProxy.Start(ctx, bindAddr)` is the listener + ctx-bounded goroutine + clean shutdown around the existing `ServeHTTP` allowlist handler. `nilcore chat -allow-egress host,host` stands it up and routes a **container** sandbox through it via `Container.AllowEgressVia` (using the runtime host alias; `Container.ExtraHosts` adds `--add-host` for docker-Linux); the sandboxed `web_fetch` tool is then advertised (its body `guard.Wrap`'d as untrusted data, I7). Default stays default-deny: no flag ⇒ no proxy, `--network none`, no `web_fetch`. The namespace backend runs in an empty network namespace (`CLONE_NEWNET`, no interface), so it has no proxy egress path — web access requires the container backend (fail-closed). +*Container-backend egress is COOPERATIVE by default,* not a hard wall: `AllowEgressVia` attaches the sandbox to a bridged NAT network and points `HTTP(S)_PROXY` at the allowlist proxy, so a model command that ignores the proxy (`curl --noproxy`, a raw socket, `/dev/tcp`) still reaches any host. Two operator escalations tighten this. `NILCORE_EGRESS_STRICT` **fails closed** — it refuses cooperative egress entirely and leaves the box `--network none` rather than pretend the allowlist is a boundary. `NILCORE_EGRESS_HARD` (**opt-in, Linux-container only, CI-validated**) makes the allowlist *unbypassable*: `applyContainerEgress` routes the box through `Container.AllowEgressViaHard`, which attaches the sandbox to a per-run `--internal` docker/podman network (**no default route**) and runs the allowlist proxy as a **dual-homed gateway container** (internal net + a normal net) that the sandbox reaches via `HTTP(S)_PROXY`; a raw socket / `--noproxy` then has no route out and simply fails, while the sandbox keeps `--cap-drop=ALL` so escaping needs host root / NET_ADMIN. On ANY setup failure it FAILS CLOSED (stays `--network none`) — never a cooperative fallback. **Honest residuals** (do *not* read hard mode as empty-netns-equivalent): (1) the internal net still has DNS, so a DNS-tunnel exfil is only *mitigated* — the sandbox's `--dns` is pinned at the gateway (which serves no `:53`) to blackhole in-sandbox resolution, and proxied traffic still works because the client reaches the proxy by IP; (2) the gateway runs `nilcore egress-gateway` (a hidden verb), so it requires the **nilcore image** (`images/sandbox`), not the default `debian:stable-slim`; (3) it is Linux-container only and unit-tested here only at the arg-assembly + decision + gateway-proxy seams (the container lifecycle is CI-validated, never host-run); (4) the per-run network+gateway can leak on crash — teardown is idempotent and a label-scoped reaper (`reapHardEgress`, on serve boot, non-blocking, gated on the opt-in) reclaims orphans. The **namespace backend's empty netns remains the recommended hard egress boundary**; `NILCORE_EGRESS_HARD` is the hard option for hosts that must run the container backend. + **Nil-gated branch-preservation on the orchestrator (additive, contract-untouched, D4).** The orchestrator's `Outcome` carries an additive `Branch` field, and a nil-gated `KeepBranch` hook preserves the verified worktree branch instead of the default disposable cleanup. When unset — every default path — cleanup is **byte-identical** (the worktree is disposed as before). When set, the verified branch survives so a gated PR can be opened from it. This underpins **gated PR (D4):** `nilcore watch --open-pr` / `nilcore schedule --open-pr` open a **draft** PR via `internal/forge` **only after the human gate** — the push runs inside the approved prepare step, the token comes from the SecretStore (`NILCORE_FORGE_TOKEN`, never logged, never given to the model — I3), and **the agent never merges** (merge stays the human gate). `internal/forge` is **pure stdlib** (no module — I6); credentials are scrubbed from logs (I5). **Multi-backend strength-routing seam (the Trust Ledger goes live, additive, contract-untouched, Phase 13).** The orchestrator carries a nil/empty-gated `Selector` seam plus `Backends []string` and `NewEnvFor func(dir, name) Env`. `multiBackend()` holds when `len(Backends) > 1 && NewEnvFor != nil`; with either unset — every default path — `executeSingle`/`raceEscalate` are **byte-identical** to the single `-backend` path. The `-backends native,codex,claude-code` flag (on `run` and the run-style commands sharing `buildRunOrchestrator`) activates it: `executeSingle` runs the trust-strongest backend first (`orderBackends` → `NewEnvFor(dir, names[0])`), and on a verify-FAIL `raceEscalate` cuts one fresh worktree per **distinct** backend so `route.Race` competes *different* backends and the verifier picks the winner. `agent.Selector` is satisfied by `trust.Selector` (built from `trust.Replay()`) **without** `internal/trust` importing `agent` — the Ledger plugs in, ranks by smoothed verifier-judged pass-rate, and a broken-chain `Replay` degrades to the configured order, never aborting. **I2 is preserved by construction:** the Selector only ORDERS attempt order; the verifier still decides "done" and judges the race. Per-backend providers/creds resolve through the SecretStore seam, never reaching the model (I3). diff --git a/internal/agent/durability.go b/internal/agent/durability.go index 9a6c05e..403182d 100644 --- a/internal/agent/durability.go +++ b/internal/agent/durability.go @@ -10,6 +10,7 @@ import ( "nilcore/internal/backend" "nilcore/internal/store" + "nilcore/internal/worktree" ) // Checkpoint persists orchestrator task state to the store so an interrupted run @@ -69,6 +70,146 @@ type suspendDetail struct { Branch string `json:"branch,omitempty"` } +// sessionPrefix returns the stable conversation key of a session task id, whose shape +// is `-` (internal/session/drivers.go): everything up to the LAST +// '-'. A drive that self-suspended as `-3` and the wake-resumed `-4` that +// follows it therefore share the same prefix, so a resume can correlate the two. It +// returns "" when there is no '-' (not a session shape) or when the only '-' is at +// index 0 (nothing before it to key on) — either way there is no predecessor to +// correlate, so the caller drives fresh. +func sessionPrefix(taskID string) string { + i := strings.LastIndex(taskID, "-") + if i <= 0 { + return "" + } + return taskID[:i] +} + +// ResumeBranch finds the preserved-work ref of a self-suspended predecessor in the +// SAME session as taskID, so a wake-resumed drive can REATTACH onto the committed work +// its earlier self left behind instead of re-driving from a fresh HEAD worktree. It +// correlates by the stable session prefix (sessionPrefix): the suspended `-3` is +// the predecessor of the resuming `-4`. Among the suspended siblings that carry a +// non-empty suspendDetail.Branch it returns the MOST RECENT. +// +// Ordering assumption: TasksByStatus returns rows ORDER BY id ascending, so the LAST +// matching row is the most-recent suspend of that conversation for the `-` +// shape (seq grows monotonically). That is the recency signal this — and SweepSuspended +// — rely on. +// +// No correlatable predecessor (no session prefix, no suspended sibling, or none with a +// recorded branch) ⇒ ("", "", nil): the caller then drives off HEAD exactly as before. +// The suspending task's own row is skipped so a re-driven id never reattaches onto +// itself. A nil receiver is a clean no-op so an orchestrator with no checkpoint is +// unaffected. +func (c *Checkpoint) ResumeBranch(ctx context.Context, taskID string) (branch, suspendedID string, err error) { + if c == nil { + return "", "", nil + } + prefix := sessionPrefix(taskID) + if prefix == "" { + return "", "", nil + } + rows, err := c.store.TasksByStatus(ctx, "suspended") + if err != nil { + return "", "", fmt.Errorf("resume branch: %w", err) + } + for _, row := range rows { + if row.ID == taskID || sessionPrefix(row.ID) != prefix || row.Detail == "" { + continue + } + var d suspendDetail + if json.Unmarshal([]byte(row.Detail), &d) != nil || d.Branch == "" { + continue + } + // Keep scanning: rows are id-ascending, so a later match is more recent — + // take the last one that qualifies. + branch, suspendedID = d.Branch, row.ID + } + return branch, suspendedID, nil +} + +// suspendRefPrefix is the ref namespace a suspended drive pins its committed work +// under (orchestrator: "suspend/"+taskID). It is deliberately outside the throwaway +// task/ rebase/ integrate/ read/ prefixes the run-end sweep reclaims, so a nap's +// recovery anchor survives — SweepSuspended is what eventually reclaims it. +const suspendRefPrefix = "suspend/" + +// defaultSuspendKeep bounds how many still-suspended recovery anchors SweepSuspended +// retains when the caller passes a non-positive keep — a sane backlog cap. +const defaultSuspendKeep = 20 + +// SweepSuspended reclaims leaked suspend/ recovery anchors in baseRepo. Each suspended +// drive pins its committed HEAD under suspend/ as a durable recovery anchor; +// once the drive is resumed (auto-reattach retires its row and deletes the ref) or its +// row is otherwise no longer "suspended", the ref is dead weight that would otherwise +// accumulate forever. This sweep deletes: +// +// - every suspend/ ref whose task row is NOT currently "suspended" (resolved, or the +// row is gone) — these can never be resumed, so the anchor is pure leak, and +// - the OLDEST still-suspended anchors beyond the `keep` most-recent — a bounded +// backlog so a long-lived server cannot grow unboundedly many live anchors. +// +// A still-"suspended" ref WITHIN the keep window is preserved — its committed work may +// yet be resumed, and dropping it would reopen the data-loss the anchor closes. keep<=0 +// applies defaultSuspendKeep. Idempotent and best-effort: DeleteBranch swallows a +// per-ref failure so one bad ref never aborts the sweep, and this is called from serve +// boot where a sweep error must never block startup. A nil receiver is a clean no-op. +// +// The live-anchor ordering (which are "oldest") reuses ResumeBranch's documented +// assumption: TasksByStatus is id-ascending, so iterating those rows yields anchors +// oldest-first and the tail is the most-recent to keep. +func (c *Checkpoint) SweepSuspended(ctx context.Context, baseRepo string, keep int) error { + if c == nil { + return nil + } + if keep <= 0 { + keep = defaultSuspendKeep + } + branches, err := worktree.ListBranches(ctx, baseRepo, suspendRefPrefix) + if err != nil { + return fmt.Errorf("sweep suspended: list refs: %w", err) + } + if len(branches) == 0 { + return nil + } + present := make(map[string]bool, len(branches)) + for _, b := range branches { + present[b] = true + } + + // The still-"suspended" rows — the only anchors whose work may still be resumed. + rows, err := c.store.TasksByStatus(ctx, "suspended") + if err != nil { + return fmt.Errorf("sweep suspended: rows: %w", err) + } + suspended := make(map[string]bool, len(rows)) + for _, r := range rows { + suspended[suspendRefPrefix+r.ID] = true + } + + // Dead anchors: a ref whose row is no longer suspended (resolved/gone) → reclaim now. + for _, b := range branches { + if !suspended[b] { + worktree.DeleteBranch(ctx, baseRepo, b) + } + } + // Live anchors, in TasksByStatus (id-ascending) order — the SAME recency assumption + // ResumeBranch documents (last = most-recent). Delete the oldest beyond keep. + var live []string + for _, r := range rows { + if ref := suspendRefPrefix + r.ID; present[ref] { + live = append(live, ref) + } + } + if len(live) > keep { + for _, ref := range live[:len(live)-keep] { + worktree.DeleteBranch(ctx, baseRepo, ref) + } + } + return nil +} + // Interrupt marks every running task "interrupted" — the clean SIGTERM checkpoint // so the next start knows what to resume. func (c *Checkpoint) Interrupt(ctx context.Context) error { diff --git a/internal/agent/durability_internal_test.go b/internal/agent/durability_internal_test.go new file mode 100644 index 0000000..71d00c4 --- /dev/null +++ b/internal/agent/durability_internal_test.go @@ -0,0 +1,38 @@ +package agent + +import "testing" + +// sessionPrefix is unexported, so this white-box test lives in package agent. It is the +// correlation key auto-reattach uses to pair a suspended `-3` with its resuming +// `-4`, so its edge behavior (last '-', index-0 guard, no '-') is load-bearing. +func TestSessionPrefix(t *testing.T) { + cases := []struct { + in string + want string + }{ + {"conv123-3", "conv123"}, + {"conv123-4", "conv123"}, // the resuming sibling shares the prefix + {"a-b-c-10", "a-b-c"}, // up to the LAST '-' + {"t-1751000000", "t"}, // a run task id (prefix "t") — never a session + {"nodash", ""}, // no '-' ⇒ no session shape + {"-leading", ""}, // only '-' at index 0 ⇒ nothing to key on + {"", ""}, // empty + {"conv-", "conv"}, // trailing '-' still keys on "conv" + } + for _, tc := range cases { + if got := sessionPrefix(tc.in); got != tc.want { + t.Errorf("sessionPrefix(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +// A suspended `-3` and its resuming `-4` correlate; an unrelated run task +// (prefix "t") never does. +func TestSessionPrefixCorrelation(t *testing.T) { + if sessionPrefix("conv-3") != sessionPrefix("conv-4") { + t.Error("a suspended sibling and its wake-resume must share a session prefix") + } + if sessionPrefix("conv-4") == sessionPrefix("t-1751000000") { + t.Error("a session task must not correlate with an unrelated run task") + } +} diff --git a/internal/agent/orchestrator.go b/internal/agent/orchestrator.go index 022663f..bb27f21 100644 --- a/internal/agent/orchestrator.go +++ b/internal/agent/orchestrator.go @@ -306,7 +306,52 @@ func (o *Orchestrator) executeSingle(ctx context.Context, t backend.Task) (Outco _ = o.Checkpoint.Begin(ctx, t) // durable: mark running (P6-T03) } - wt, err := worktree.Create(ctx, o.BaseRepo, t.ID) + // AUTO-REATTACH (durable resume): if a predecessor drive in this SAME session + // self-suspended (the `sleep` tool) and preserved its committed work under a + // suspend/ ref, base this resumed drive on that work instead of a fresh HEAD + // worktree — so the agent picks up where its earlier self left off rather than + // re-driving from scratch and orphaning the preserved commits. Correlation is by + // the stable session prefix (ResumeBranch). No suspended predecessor ⇒ branch=="" + // and the default HEAD path below is byte-identical to before. + var wt *worktree.Worktree + var err error + var resumeBranch, resumedFrom string + if o.Checkpoint != nil { + if b, sid, rerr := o.Checkpoint.ResumeBranch(ctx, t.ID); rerr != nil { + o.Log.Append(eventlog.Event{Task: t.ID, Kind: "task_resume_lookup", + Detail: map[string]any{"error": rerr.Error()}}) + } else { + resumeBranch, resumedFrom = b, sid + } + } + if resumeBranch != "" { + leaf := strings.ReplaceAll(t.ID, "/", "-") + rwt, cerr := worktree.CreateFrom(ctx, o.BaseRepo, "task/"+t.ID, leaf, resumeBranch) + if cerr != nil { + // A stale/unresolvable suspend ref (already swept, or its commit is gone): + // FALL BACK to a fresh HEAD worktree — today's behavior, no regression, no + // data loss. The predecessor row is left "suspended" for a later sweep. + o.Log.Append(eventlog.Event{Task: t.ID, Kind: "task_resume_fallback", + Detail: map[string]any{"branch": resumeBranch, "resumed_from": resumedFrom, "error": cerr.Error()}}) + // wt stays nil ⇒ the HEAD-fallback worktree.Create below runs. resumeBranch is + // not read again, so it is intentionally left as-is (no data lost: the ref remains + // for a later sweep). + } else { + wt = rwt + // Retire the predecessor so it is never reattached twice AND its anchor + // becomes GC-eligible; then delete the consumed ref immediately so it does + // not linger (the sweep is only a backstop for the crash-before-here case). + if o.Checkpoint != nil { + _ = o.Checkpoint.Complete(ctx, resumedFrom, t.Goal, false) + } + worktree.DeleteBranch(ctx, o.BaseRepo, resumeBranch) + o.Log.Append(eventlog.Event{Task: t.ID, Kind: "task_resumed", + Detail: map[string]any{"resumed_from": resumedFrom, "branch": resumeBranch}}) + } + } + if wt == nil { + wt, err = worktree.Create(ctx, o.BaseRepo, t.ID) + } if err != nil { // The checkpoint was already marked running (Begin above). Finalize it so a // restart's Resume does not re-drive a task whose setup already faulted here. A diff --git a/internal/agent/reattach_test.go b/internal/agent/reattach_test.go new file mode 100644 index 0000000..5486849 --- /dev/null +++ b/internal/agent/reattach_test.go @@ -0,0 +1,342 @@ +package agent_test + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "nilcore/internal/agent" + "nilcore/internal/backend" + "nilcore/internal/eventlog" + "nilcore/internal/store" + "nilcore/internal/worktree" +) + +// reattachBackend records the worktree HEAD it is handed at Run time, so a test can +// prove whether the drive was based on a preserved suspend ref (== the ref's commit) +// or a fresh HEAD worktree (== the repo HEAD). It never writes — the verifier passes. +type reattachBackend struct { + headAtRun string + ran bool +} + +func (b *reattachBackend) Name() string { return "reattach" } +func (b *reattachBackend) Run(_ context.Context, t backend.Task) (backend.Result, error) { + b.ran = true + cmd := exec.Command("git", "rev-parse", "HEAD") + cmd.Dir = t.Dir + out, _ := cmd.CombinedOutput() + b.headAtRun = strings.TrimSpace(string(out)) + return backend.Result{Backend: "reattach", Summary: "ran"}, nil +} + +// seedSuspendBranch creates ref in repo pointing at a NEW commit (marker file) that is +// NOT reachable from HEAD — mimicking a suspended drive's preserved committed work. It +// returns that commit's SHA. The temp worktree used to build it is removed (the branch +// is kept), so ref is a bare recovery anchor exactly like a real suspend/. +func seedSuspendBranch(t *testing.T, repo, ref, marker string) string { + t.Helper() + wtDir := filepath.Join(t.TempDir(), "seedwt") + gitTrim(t, repo, "worktree", "add", "-b", ref, wtDir, "HEAD") + if err := os.WriteFile(filepath.Join(wtDir, marker), []byte("preserved before sleep\n"), 0o644); err != nil { + t.Fatalf("write marker: %v", err) + } + gitTrim(t, wtDir, "add", "-A") + gitTrim(t, wtDir, "-c", "user.email=t@nilcore.local", "-c", "user.name=t", "commit", "-q", "-m", "C1 preserved work") + sha := gitTrim(t, wtDir, "rev-parse", "HEAD") + gitTrim(t, repo, "worktree", "remove", "--force", wtDir) + return sha +} + +func refExists(t *testing.T, repo, ref string) bool { + t.Helper() + return gitTrim(t, repo, "rev-parse", "--verify", "--quiet", ref) != "" +} + +// --- FIX 1: ResumeBranch unit behavior (prefix match, cross-prefix, most-recent, nil) --- + +func TestResumeBranch(t *testing.T) { + ctx := context.Background() + + // nil receiver is a clean no-op (an orchestrator with no checkpoint). + var nilC *agent.Checkpoint + if b, id, err := nilC.ResumeBranch(ctx, "conv-4"); b != "" || id != "" || err != nil { + t.Errorf("nil receiver = (%q,%q,%v), want ('','',nil)", b, id, err) + } + + c, _ := newCheckpoint(t) + if err := c.Suspend(ctx, "conv-3", "g", "suspend/conv-3"); err != nil { + t.Fatal(err) + } + // A resuming sibling in the same conversation reattaches onto conv-3's ref. + if b, id, err := c.ResumeBranch(ctx, "conv-4"); err != nil || b != "suspend/conv-3" || id != "conv-3" { + t.Errorf("prefix match = (%q,%q,%v), want ('suspend/conv-3','conv-3',nil)", b, id, err) + } + // A different conversation must NOT match. + if b, id, _ := c.ResumeBranch(ctx, "other-4"); b != "" || id != "" { + t.Errorf("cross-prefix = (%q,%q), want empty (a different conversation must not correlate)", b, id) + } + // A re-driven id must not reattach onto its OWN suspend row. + if b, _, _ := c.ResumeBranch(ctx, "conv-3"); b != "" { + t.Errorf("own-id reattach = %q, want empty", b) + } +} + +func TestResumeBranchMostRecent(t *testing.T) { + ctx := context.Background() + c, _ := newCheckpoint(t) + // Two suspended siblings; id-ascending order makes conv-5 the more recent. + if err := c.Suspend(ctx, "conv-2", "g", "suspend/conv-2"); err != nil { + t.Fatal(err) + } + if err := c.Suspend(ctx, "conv-5", "g", "suspend/conv-5"); err != nil { + t.Fatal(err) + } + b, id, err := c.ResumeBranch(ctx, "conv-9") + if err != nil { + t.Fatal(err) + } + if b != "suspend/conv-5" || id != "conv-5" { + t.Errorf("most-recent = (%q,%q), want the latest sibling ('suspend/conv-5','conv-5')", b, id) + } +} + +func TestResumeBranchSkipsEmptyBranch(t *testing.T) { + ctx := context.Background() + c, _ := newCheckpoint(t) + // A suspended sibling whose preserved branch is empty (nothing was committed) is + // not reattachable and must be ignored. + if err := c.Suspend(ctx, "conv-3", "g", ""); err != nil { + t.Fatal(err) + } + if b, id, _ := c.ResumeBranch(ctx, "conv-4"); b != "" || id != "" { + t.Errorf("empty-branch sibling = (%q,%q), want empty", b, id) + } +} + +// --- FIX 1: executeSingle auto-reattach, fallback, and unchanged default --- + +// A resumed drive whose session predecessor preserved committed work is based on that +// work (the suspend ref), the predecessor is retired, the consumed ref is deleted, and +// a task_resumed correlation event is logged. +func TestExecuteReattachesToSuspendRef(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + repo := initGitRepo(t) + conv := "conv-reattach" + suspendedID := conv + "-3" + resumeID := conv + "-4" + ref := "suspend/" + suspendedID + c1 := seedSuspendBranch(t, repo, ref, "resumed.txt") + + ckpt, s := newCheckpoint(t) + ctx := context.Background() + if err := ckpt.Suspend(ctx, suspendedID, "nap goal", ref); err != nil { + t.Fatal(err) + } + + logPath := filepath.Join(t.TempDir(), "events.log") + lg, err := eventlog.Open(logPath) + if err != nil { + t.Fatalf("eventlog.Open: %v", err) + } + + be := &reattachBackend{} + orch := &agent.Orchestrator{ + BaseRepo: repo, + Log: lg, + NewEnv: func(string) agent.Env { return agent.Env{Backend: be, Verifier: &fakeVerifier{passed: true}} }, + Router: agent.SingleRouter{}, + Spawner: agent.NoSpawner{}, + Checkpoint: ckpt, + } + if _, err := orch.Execute(ctx, backend.Task{ID: resumeID, Goal: "resume goal"}); err != nil { + t.Fatalf("Execute: %v", err) + } + _ = lg.Close() + + if !be.ran { + t.Fatal("backend did not run") + } + if be.headAtRun != c1 { + t.Errorf("resumed worktree HEAD = %q, want C1 %q (must be based on the suspend ref, not HEAD)", be.headAtRun, c1) + } + // The predecessor is retired so it is never reattached twice. + rec, gerr := s.GetTask(ctx, suspendedID) + if gerr != nil { + t.Fatalf("GetTask: %v", gerr) + } + if rec.Status == "suspended" { + t.Error("suspended predecessor still 'suspended' — it must be retired after a reattach") + } + // The consumed anchor is deleted immediately. + if refExists(t, repo, ref) { + t.Errorf("consumed suspend ref %q still present — must be deleted after reattach", ref) + } + // The correlation is auditable. + found := false + for _, e := range readEvents(t, logPath) { + if e.Kind == "task_resumed" { + found = true + if e.Detail["resumed_from"] != suspendedID { + t.Errorf("task_resumed.resumed_from = %v, want %q", e.Detail["resumed_from"], suspendedID) + } + if e.Detail["branch"] != ref { + t.Errorf("task_resumed.branch = %v, want %q", e.Detail["branch"], ref) + } + } + } + if !found { + t.Error("no task_resumed event was logged") + } +} + +// A recorded predecessor whose suspend ref is GONE (stale/swept) must fall back to a +// fresh HEAD worktree — no error, no data loss — and leave the row for a later sweep. +func TestExecuteFallsBackWhenSuspendRefMissing(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + repo := initGitRepo(t) + conv := "conv-fallback" + suspendedID := conv + "-3" + resumeID := conv + "-4" + ref := "suspend/" + suspendedID // recorded, but never created as a real ref + + ckpt, s := newCheckpoint(t) + ctx := context.Background() + if err := ckpt.Suspend(ctx, suspendedID, "nap goal", ref); err != nil { + t.Fatal(err) + } + + be := &reattachBackend{} + orch := &agent.Orchestrator{ + BaseRepo: repo, + NewEnv: func(string) agent.Env { return agent.Env{Backend: be, Verifier: &fakeVerifier{passed: true}} }, + Router: agent.SingleRouter{}, + Spawner: agent.NoSpawner{}, + Checkpoint: ckpt, + } + if _, err := orch.Execute(ctx, backend.Task{ID: resumeID, Goal: "resume goal"}); err != nil { + t.Fatalf("Execute must fall back cleanly, got: %v", err) + } + if !be.ran { + t.Error("backend did not run on the fallback path") + } + repoHead := gitTrim(t, repo, "rev-parse", "HEAD") + if be.headAtRun != repoHead { + t.Errorf("fallback worktree HEAD = %q, want repo HEAD %q (a stale ref must fall back to HEAD)", be.headAtRun, repoHead) + } + // The predecessor is NOT retired on a fallback — its row waits for a later sweep. + rec, _ := s.GetTask(ctx, suspendedID) + if rec.Status != "suspended" { + t.Errorf("on fallback the predecessor row must stay 'suspended', got %q", rec.Status) + } +} + +// No suspended predecessor ⇒ the worktree is created off HEAD exactly as before. +func TestExecuteDefaultOffHeadWhenNoPredecessor(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + repo := initGitRepo(t) + ckpt, _ := newCheckpoint(t) // empty store: no suspended rows + ctx := context.Background() + + be := &reattachBackend{} + orch := &agent.Orchestrator{ + BaseRepo: repo, + NewEnv: func(string) agent.Env { return agent.Env{Backend: be, Verifier: &fakeVerifier{passed: true}} }, + Router: agent.SingleRouter{}, + Spawner: agent.NoSpawner{}, + Checkpoint: ckpt, + } + if _, err := orch.Execute(ctx, backend.Task{ID: "conv-solo-1", Goal: "x"}); err != nil { + t.Fatalf("Execute: %v", err) + } + repoHead := gitTrim(t, repo, "rev-parse", "HEAD") + if be.headAtRun != repoHead { + t.Errorf("default worktree HEAD = %q, want repo HEAD %q (no predecessor ⇒ off HEAD)", be.headAtRun, repoHead) + } +} + +// --- FIX 2: SweepSuspended reclaims resolved + oldest-beyond-keep anchors --- + +func TestSweepSuspended(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + repo := initGitRepo(t) + ckpt, s := newCheckpoint(t) + ctx := context.Background() + + head := gitTrim(t, repo, "rev-parse", "HEAD") + refs := []string{"suspend/conv-1", "suspend/conv-2", "suspend/conv-3", "suspend/conv-4", "suspend/gone-9"} + for _, r := range refs { + if err := worktree.PinBranch(ctx, repo, r, head); err != nil { + t.Fatalf("PinBranch %s: %v", r, err) + } + } + + // Row states: + // - conv-1: resolved (status "failed") → dead anchor, must be swept. + // - conv-2, conv-3, conv-4: still "suspended". + // - gone-9: NO row at all → orphan anchor, must be swept. + _ = s.UpsertTask(ctx, store.Task{ID: "conv-1", Goal: "g", Status: "failed", Detail: `{"branch":"suspend/conv-1"}`}) + for _, id := range []string{"conv-2", "conv-3", "conv-4"} { + if err := ckpt.Suspend(ctx, id, "g", "suspend/"+id); err != nil { + t.Fatal(err) + } + } + + // keep=2 → of the three still-suspended, keep the 2 most-recent (conv-3, conv-4) + // and delete the oldest (conv-2); plus delete conv-1 (resolved) and gone-9 (no row). + if err := ckpt.SweepSuspended(ctx, repo, 2); err != nil { + t.Fatalf("SweepSuspended: %v", err) + } + + if refExists(t, repo, "suspend/conv-1") { + t.Error("resolved anchor suspend/conv-1 must be swept") + } + if refExists(t, repo, "suspend/gone-9") { + t.Error("orphan anchor suspend/gone-9 (no row) must be swept") + } + if refExists(t, repo, "suspend/conv-2") { + t.Error("oldest still-suspended anchor beyond keep must be swept") + } + if !refExists(t, repo, "suspend/conv-3") { + t.Error("still-suspended anchor within keep must survive") + } + if !refExists(t, repo, "suspend/conv-4") { + t.Error("most-recent still-suspended anchor must survive") + } + + // Idempotent: a second sweep over the survivors is a clean no-op (no panic/error). + if err := ckpt.SweepSuspended(ctx, repo, 2); err != nil { + t.Fatalf("second SweepSuspended: %v", err) + } + if !refExists(t, repo, "suspend/conv-4") { + t.Error("a second sweep must not drop a still-kept anchor") + } +} + +// SweepSuspended over a repo with no suspend/ refs is a clean no-op. +func TestSweepSuspendedNoRefs(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + repo := initGitRepo(t) + ckpt, _ := newCheckpoint(t) + if err := ckpt.SweepSuspended(context.Background(), repo, 0); err != nil { + t.Fatalf("SweepSuspended over empty repo: %v", err) + } + // nil receiver is also a clean no-op. + var nilC *agent.Checkpoint + if err := nilC.SweepSuspended(context.Background(), repo, 0); err != nil { + t.Fatalf("nil-receiver SweepSuspended: %v", err) + } +} diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 25f064b..77bae70 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -61,6 +61,14 @@ type Container struct { // (docker on Linux — podman and Docker Desktop provide the host alias already). ExtraHosts []string + // DNS, when non-empty, is emitted as `--dns ` so the container resolves + // names only through the given resolver. It is set by the HARD egress path + // (AllowEgressViaHard): pointing the sandbox's resolver at the dual-homed gateway + // (which serves no :53) blackholes in-sandbox DNS, closing the DNS-tunnel exfil + // residual — proxied traffic still works because the client reaches the proxy by + // IP and the proxy does the resolving. Empty by default (byte-identical run args). + DNS string + // ExtraReadRoots are additional host directories bind-mounted READ-ONLY into the // container at the SAME absolute path (identity-mapped, so a path the host-side // file tools already resolved is the same path the in-box `run` shell sees). They @@ -120,21 +128,58 @@ func (c *Container) Workdir() string { return c.HostDir } // network namespace (deny-all). applyContainerEgress (cmd/nilcore) warns about this // and honors NILCORE_EGRESS_STRICT to fail closed. // +// The container backend has an OPT-IN hard option too: AllowEgressViaHard (wired by +// applyContainerEgress under NILCORE_EGRESS_HARD) attaches the sandbox to a +// `--internal` network with no route out and routes it through a dual-homed gateway +// container, making the allowlist unbypassable (Linux-container only, CI-validated, +// with an honestly-documented DNS-tunnel residual). The namespace backend's empty +// netns remains the recommended hard boundary. +// // NOTE: allowlisted egress is a CONTAINER-backend capability only. The namespace // backend (Auto-preferred on Linux) has no proxy path — it is hard deny-all (see // namespace_linux.go). Callers that need web_fetch / a non-empty egress allowlist // must run on the container backend (`-sandbox container`). func (c *Container) AllowEgressVia(proxyURL string) { c.Network = "bridge" + c.setProxyEnv(proxyURL) +} + +// AllowEgressViaHard is the HARD egress path (opt-in, Linux-container only; wired by +// cmd/nilcore's applyContainerEgress under NILCORE_EGRESS_HARD). Unlike AllowEgressVia +// it does NOT attach the container to a bridged NAT network. Instead the caller has +// created a dedicated `--internal` docker/podman network — which has NO default route +// — and runs the allowlist proxy as a dual-homed GATEWAY container (internal net + +// a normal net). The sandbox is attached to the INTERNAL net only, so its ONLY path +// off-box is the gateway: proxy-cooperative traffic reaches the allowlist, while a +// raw socket / `curl --noproxy` has no route out and simply fails. This makes the +// allowlist UNBYPASSABLE without host root / NET_ADMIN (the sandbox keeps +// --cap-drop=ALL), i.e. a genuine boundary rather than the cooperative one. +// +// network is the internal network name (emitted as `--network`, so no bridge and no +// --add-host are needed); proxyURL points HTTP(S)_PROXY at the gateway. The caller +// additionally sets c.DNS to the gateway so in-sandbox DNS is blackholed (the +// remaining residual — see the DNS field). HONEST RESIDUALS (documented in +// applyContainerEgress + docs/ARCHITECTURE.md): DNS-tunnel exfil is only mitigated, +// not proven-closed; it requires the nilcore image (a debian:stable-slim has no +// `nilcore` to run the gateway); it is Linux-container only and CI-validated. The +// namespace backend's empty netns remains the recommended hard boundary. +func (c *Container) AllowEgressViaHard(network, proxyURL string) { + c.Network = network + c.setProxyEnv(proxyURL) +} + +// setProxyEnv points the four HTTP(S)_PROXY vars at proxyURL and pins NO_PROXY empty +// so an inherited NO_PROXY can't exempt any host from the proxy (defense-in-depth; +// on the cooperative bridge path it does NOT stop a client that bypasses the proxy +// entirely — see the SECURITY note above — whereas on the hard path there is no +// route around the proxy at all). +func (c *Container) setProxyEnv(proxyURL string) { if c.Env == nil { c.Env = map[string]string{} } for _, k := range []string{"HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"} { c.Env[k] = proxyURL } - // Pin no_proxy empty so an inherited NO_PROXY can't exempt any host from the - // proxy (defense-in-depth; it does NOT stop a client that bypasses the proxy - // entirely — see the SECURITY note above). c.Env["NO_PROXY"] = "" c.Env["no_proxy"] = "" } @@ -172,6 +217,12 @@ func (c *Container) runArgs(cmd string, perRun map[string]string) []string { args = append(args, "--add-host", h) } + // Pin the resolver to a single DNS server (the HARD egress path points this at + // the gateway so in-sandbox DNS is blackholed). Empty unless hard egress set it. + if c.DNS != "" { + args = append(args, "--dns", c.DNS) + } + // Per-run secret injection (P2-T03): keys reach the container only for this // invocation, never persisted, never logged. for k, v := range c.Env { diff --git a/internal/sandbox/sandbox_test.go b/internal/sandbox/sandbox_test.go index 9ce8e25..49f0936 100644 --- a/internal/sandbox/sandbox_test.go +++ b/internal/sandbox/sandbox_test.go @@ -96,6 +96,42 @@ func TestAllowEgressVia(t *testing.T) { } } +func TestAllowEgressViaHard(t *testing.T) { + const net = "nilcore-egr-abc123" + c := NewContainer("podman", "img", "/work") + c.AllowEgressViaHard(net, "http://10.88.0.2:3128") + c.DNS = "10.88.0.2" + got := argsString(c, "x") + + // The internal net is emitted verbatim — NOT bridge. + if !strings.Contains(got, "--network "+net) { + t.Errorf("hard egress should attach the internal net: %s", got) + } + if strings.Contains(got, "--network bridge") { + t.Errorf("hard egress must NOT use a bridge network: %s", got) + } + // Proxy env points at the gateway; NO_PROXY is pinned empty. + if c.Env["HTTP_PROXY"] != "http://10.88.0.2:3128" || c.Env["HTTPS_PROXY"] != "http://10.88.0.2:3128" { + t.Errorf("hard egress proxy env not set: %v", c.Env) + } + if v, ok := c.Env["NO_PROXY"]; !ok || v != "" { + t.Errorf("hard egress must pin NO_PROXY empty, got %q (present=%v)", v, ok) + } + // The gateway resolver is pinned via --dns; no --add-host on the hard path. + if !strings.Contains(got, "--dns 10.88.0.2") { + t.Errorf("hard egress should pin --dns at the gateway: %s", got) + } + if strings.Contains(got, "--add-host") { + t.Errorf("hard egress must add no --add-host: %s", got) + } + + // No DNS set ⇒ no --dns emitted (byte-identical default). + c2 := NewContainer("podman", "img", "/work") + if strings.Contains(argsString(c2, "x"), "--dns") { + t.Errorf("--dns present with no DNS set") + } +} + func TestExtraReadRootsMountedReadOnly(t *testing.T) { c := NewContainer("podman", "img", "/work/tree") c.ExtraReadRoots = []string{"/host/lib", "/host/docs"} diff --git a/internal/worktree/worktree.go b/internal/worktree/worktree.go index 1a04bcc..c55714f 100644 --- a/internal/worktree/worktree.go +++ b/internal/worktree/worktree.go @@ -460,6 +460,27 @@ func DeleteBranches(ctx context.Context, baseRepo, prefix string) { } } +// ListBranches returns the short-names of every local branch under prefix (e.g. +// "suspend/") in baseRepo. It is the read half of the durable-anchor sweep +// (agent.Checkpoint.SweepSuspended): the suspend/ recovery anchors a nap leaves behind +// accumulate across restarts and must be reclaimed once resolved. It mirrors +// DeleteBranches' `branch --list *` query and runs the same hardened git (I4), +// so a repo-authored hook/config can never execute on the host. Empty (not an error) +// when nothing matches the prefix. +func ListBranches(ctx context.Context, baseRepo, prefix string) ([]string, error) { + out, err := git(ctx, baseRepo, "branch", "--list", prefix+"*", "--format=%(refname:short)") + if err != nil { + return nil, fmt.Errorf("list branches %s*: %w", prefix, err) + } + var branches []string + for _, b := range strings.Split(strings.TrimSpace(out), "\n") { + if b = strings.TrimSpace(b); b != "" { + branches = append(branches, b) + } + } + return branches, nil +} + // PinBranch force-creates (or moves) `branch` to point at `sha` in baseRepo. It is // the durable-resume anchor: the run-end branch sweep (DeleteBranches) only reclaims // the throwaway task/ rebase/ integrate/ read/ prefixes, so a branch under a