From b5a1f3914115e688c4d4f48f3905844693d3ae63 Mon Sep 17 00:00:00 2001 From: RNT56 Date: Fri, 10 Jul 2026 18:56:35 +0200 Subject: [PATCH 1/4] feat(capguard): enforce Rule-of-Two trifecta on the main paths (chat/serve/swarm) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lethal trifecta (untrusted web input ∧ private repo data ∧ open egress) was evaluated only on browse/desktop; the main agent paths never checked it. Add a shared enforceRuleOfTwo choke point evaluated once at each front door's startup (where egress/web/repo are known), reusing capguard.Evaluate: - chat/tui/run (attended): a trifecta prompts once at the console. - serve/swarm (headless): a trifecta with no human FAILS CLOSED (Refuse) — the exact combination the Rule of Two exists to refuse; a graduated-auto-approval envelope may still auto-proceed inside its blast fence. Default egress is deny-all/narrow, so the verdict is Allow and every normal run is byte-identical; only a wide/wildcard egress config trips it. The audit event is metadata-only (verdict + axes + labels, never the host list — I3/I7). Enforced by default; NILCORE_RULE_OF_TWO=0 is the documented operator opt-out. Co-Authored-By: Claude Opus 4.8 --- cmd/nilcore/chat.go | 11 ++++ cmd/nilcore/main.go | 10 ++++ cmd/nilcore/ruleoftwo.go | 93 +++++++++++++++++++++++++++++++ cmd/nilcore/ruleoftwo_test.go | 100 ++++++++++++++++++++++++++++++++++ cmd/nilcore/swarm.go | 11 ++++ 5 files changed, 225 insertions(+) create mode 100644 cmd/nilcore/ruleoftwo.go create mode 100644 cmd/nilcore/ruleoftwo_test.go diff --git a/cmd/nilcore/chat.go b/cmd/nilcore/chat.go index 61207dc..22d4008 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, diff --git a/cmd/nilcore/main.go b/cmd/nilcore/main.go index 18a2d64..87190eb 100644 --- a/cmd/nilcore/main.go +++ b/cmd/nilcore/main.go @@ -1223,6 +1223,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..112f490 --- /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("Rule of Two: the lethal trifecta (untrusted web input ∧ private repo data ∧ open egress) was denied at the 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("Rule of Two: refusing the lethal trifecta (untrusted web input ∧ private repo data ∧ open egress) with no human gate; 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) From 04621ab0ff2087aa3d04350b4ee93d59daa3ec09 Mon Sep 17 00:00:00 2001 From: RNT56 Date: Fri, 10 Jul 2026 19:15:03 +0200 Subject: [PATCH 2/4] feat(suspend): auto-reattach a woken drive to its preserved work + GC suspend/ refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit fixed the sleep/suspend DATA-LOSS (committed work is pinned to a durable suspend/ ref instead of being branch -D'd), but nothing read it back (the wake re-drove from a fresh HEAD worktree) and the refs leaked. AUTO-REATTACH: on a drive whose session has a suspended predecessor with a preserved branch (correlated by the stable prefix of the task id; only serve drives can sleep, so run tasks never collide), the orchestrator now bases the resumed worktree on that committed work via worktree.CreateFrom instead of HEAD, retires the predecessor, deletes the consumed ref, and logs task_resumed. A stale/unresolvable ref falls back to HEAD (today's behavior — no regression, no data loss); no predecessor ⇒ byte-identical. GC: Checkpoint.SweepSuspended (wired best-effort on serve boot) prunes suspend/ refs whose task is resolved and caps the survivors to the most-recent N, closing the branch leak. New worktree.ListBranches helper. Backend contract I1 untouched. Co-Authored-By: Claude Opus 4.8 --- cmd/nilcore/main.go | 9 + internal/agent/durability.go | 141 +++++++++ internal/agent/durability_internal_test.go | 38 +++ internal/agent/orchestrator.go | 45 ++- internal/agent/reattach_test.go | 342 +++++++++++++++++++++ internal/worktree/worktree.go | 21 ++ 6 files changed, 595 insertions(+), 1 deletion(-) create mode 100644 internal/agent/durability_internal_test.go create mode 100644 internal/agent/reattach_test.go diff --git a/cmd/nilcore/main.go b/cmd/nilcore/main.go index 87190eb..5a84ca5 100644 --- a/cmd/nilcore/main.go +++ b/cmd/nilcore/main.go @@ -1119,6 +1119,15 @@ 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()}}) + } + } 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 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..09c46dd 100644 --- a/internal/agent/orchestrator.go +++ b/internal/agent/orchestrator.go @@ -306,7 +306,50 @@ 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()}}) + resumeBranch, resumedFrom = "", "" + } 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/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 From 3da6748e423098913dd6eca18c23ee3a32a4326e Mon Sep 17 00:00:00 2001 From: RNT56 Date: Fri, 10 Jul 2026 19:34:09 +0200 Subject: [PATCH 3/4] feat(sandbox): opt-in HARD container egress boundary via an --internal network + gateway MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The container backend's egress allowlist was COOPERATIVE (--network bridge + HTTP(S)_PROXY env) — bypassable by a command that ignores the proxy (curl --noproxy, raw sockets, /dev/tcp). Add an OPT-IN hard mode (NILCORE_EGRESS_HARD): - a per-run --internal docker/podman network (NO default route) to which the sandbox attaches ONLY, so the proxy is the sole egress path — unbypassable at --cap-drop=ALL, no host root/NET_ADMIN; - the allowlist proxy runs as a dual-homed GATEWAY container via a new hidden "nilcore egress-gateway" verb (reuses policy.EgressProxy — no new module, I6); - the sandbox's --dns points at the gateway to blackhole in-sandbox DNS. Fails CLOSED to --network none on any setup error (never cooperative-fallback); idempotent teardown + a label-scoped reaper on serve boot reclaim orphans. HONEST residuals (documented in docs/ARCHITECTURE.md + comments): DNS-tunnel exfil is only mitigated, hard mode needs the nilcore/sandbox image (not bare debian), and the lifecycle is Linux-container CI-validated (only the arg assembly + decision seam + gateway proxy are unit-tested on the host). The namespace backend's empty netns stays the recommended hard boundary; NILCORE_EGRESS_STRICT stays the fail-closed cooperative refusal. Default (opt-out) path is byte-identical. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 + cmd/nilcore/chat.go | 39 +++- cmd/nilcore/chat_test.go | 101 ++++++++++ cmd/nilcore/egress_gateway.go | 63 +++++++ cmd/nilcore/egress_gateway_test.go | 82 ++++++++ cmd/nilcore/egress_hard.go | 290 +++++++++++++++++++++++++++++ cmd/nilcore/main.go | 14 ++ docs/ARCHITECTURE.md | 2 + internal/sandbox/sandbox.go | 57 +++++- internal/sandbox/sandbox_test.go | 36 ++++ 10 files changed, 681 insertions(+), 5 deletions(-) create mode 100644 cmd/nilcore/egress_gateway.go create mode 100644 cmd/nilcore/egress_gateway_test.go create mode 100644 cmd/nilcore/egress_hard.go 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 22d4008..b52e5b1 100644 --- a/cmd/nilcore/chat.go +++ b/cmd/nilcore/chat.go @@ -487,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) { @@ -500,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 @@ -535,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 5a84ca5..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 @@ -1128,6 +1133,15 @@ func serveMain(args []string) { 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 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/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"} From 03db05cf5a84c9649b48e8b6601756eeb9588f11 Mon Sep 17 00:00:00 2001 From: RNT56 Date: Fri, 10 Jul 2026 19:40:20 +0200 Subject: [PATCH 4/4] chore: satisfy golangci-lint (ineffassign + ST1005) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - orchestrator.go: drop the dead resumeBranch/resumedFrom reset on the resume-fallback path (wt stays nil ⇒ HEAD fallback runs; the vars are not read again) — ineffassign. - ruleoftwo.go: lowercase the leading word of two error strings — ST1005. Both are behavior-preserving. Co-Authored-By: Claude Opus 4.8 --- cmd/nilcore/ruleoftwo.go | 4 ++-- internal/agent/orchestrator.go | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/nilcore/ruleoftwo.go b/cmd/nilcore/ruleoftwo.go index 112f490..ffa3ad6 100644 --- a/cmd/nilcore/ruleoftwo.go +++ b/cmd/nilcore/ruleoftwo.go @@ -86,8 +86,8 @@ func enforceRuleOfTwo(log *eventlog.Log, enforce, untrusted, repoMounted bool, e if approver != nil && approver.Approve(prompt) { return nil } - return fmt.Errorf("Rule of Two: the lethal trifecta (untrusted web input ∧ private repo data ∧ open egress) was denied at the gate; narrow the egress allowlist, or set NILCORE_RULE_OF_TWO=0 to opt out") + 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("Rule of Two: refusing the lethal trifecta (untrusted web input ∧ private repo data ∧ open egress) with no human gate; narrow the egress allowlist, run attended, configure a graduated-auto-approval envelope, or set NILCORE_RULE_OF_TWO=0 to opt out") + 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/internal/agent/orchestrator.go b/internal/agent/orchestrator.go index 09c46dd..bb27f21 100644 --- a/internal/agent/orchestrator.go +++ b/internal/agent/orchestrator.go @@ -333,7 +333,9 @@ func (o *Orchestrator) executeSingle(ctx context.Context, t backend.Task) (Outco // 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()}}) - resumeBranch, resumedFrom = "", "" + // 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