fix: remediate 2026-07-10 adversarial audit (1 CRITICAL + ~11 HIGH + ~12 MED) + adversarial re-verification#100
Merged
Conversation
…~12 MED) Fix every finding from a fresh 12-agent adversarial audit at 63292d7, each with a discriminating test. All three gates green: make verify, make test-race (0 data races), make tui-verify. CRITICAL: {{secret:NAME}} browser/desktop typing resolved any host env var (confused-deputy exfil) -> per-task secret-name allowlist, default deny-all; capguard axis-B counts a secret-capable session. HIGH: git repo-local-config RCE closed via a .git write-guard + per-command --no-ext-diff; container egress documented as cooperative (+ NILCORE_EGRESS_STRICT fail-closed); vcache no longer replays stale-green past a changed evidence artifact; artifact hollow-green refused; sleep/suspend preserves committed work; race-recovered work keeps its branch under KeepBranch; desktop --mac-host gates every mutation; webhook self-start rate/label-bounded; chat/tui resolve -backend auto; circuit breaker not poisoned by steers/cancels. MEDIUM: promote gate scopes on target base; MCP responses size-bounded; VAR=0 disables (not enables) AUTONOMY/FLYWHEEL/LIVE_INDEX/LESSONS; codeintel O_NOFOLLOW + size cap; advisor metered against the budget wall; swarm -blast-radius wired; memory table bounded; webhook shares serve's store; clean cancel/error task lifecycle; converged build deliverable pinned. LOW: eventlog redaction covers []string/json.RawMessage; Slack WS frame cap; Telegram token can't ride a URL error. Integration-seam fix: -c diff.external= (empty) made git exec "" and broke every git diff -> removed; the .git write-guard is the real defense. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Second-wave adversarial verification of the remediation diff (30 fixes revert-tested in isolated worktrees + a completeness critic) found one live regression and several under-tested fixes; addressed here. fix(worktreefs): the .git write-guard was case-sensitive (comp == ".git"), so .GIT/config bypassed it on case-insensitive filesystems (macOS APFS, Windows) and landed in the REAL .git/config -- reopening the repo-local-config RCE. Now folds case + strips trailing dots/spaces. Proven discriminating: with the case-sensitive guard the new test plants a filter.*.clean into the real .git/config and fails. docs(agent): make the sleep/suspend comments honest -- committed work is preserved on suspend/<id> as a durable RECOVERY anchor (never lost), but the wake re-engages a fresh drive; auto-reattach + suspend/ GC are documented follow-ups (was over-claiming reattach). test: add discriminating tests for every previously-untested fix -- secret-name allowlist axis-B, egress STRICT fail-closed, meteredAdvisor, swarm blast fence, build deliverable pin, backend auto-resolve, redaction/slack-ws/telegram -- and fix safeRev's test (it passed for the wrong reason in an empty tempdir). All gates green: make verify, make test-race (0 races), make tui-verify. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…serve/swarm) 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 <noreply@anthropic.com>
… suspend/ refs The audit fixed the sleep/suspend DATA-LOSS (committed work is pinned to a durable suspend/<id> 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 <conversation> 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 <noreply@anthropic.com>
…l network + gateway 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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
Rebasing the audit-remediation + deferred-hardening branch (#100) onto main pulled in #101's features-review, which independently fixed three of the same defects #100 did. Dedupe to main's canonical implementation (which fully subsumes #100's intent) so the gate passes: - MCP response OOM guard: drop #100's boundedReader/maxMCPResponseBytes in favor of main's capReader/errResponseTooLarge (both cap at 8 MiB). Same guarantee, one implementation. - Slack WS frame cap: take main's maxFramePayload (16 MiB) over #100's maxWSFrameBytes (64 MiB). main checks the raw uint64 before narrowing to int, so it rejects both the merely-huge and the high-bit wrap-negative length before make() — realign #100's discriminating ws_test.go to it. - Artifact hollow-green: keep #100's H4 rename (urlClaim -> quoteClaim, moving the shared helper off the value-blind web.url_resolves onto the value-checking web.quote_exists) and point #101's two new EventSink tests (which only need a well-formed claim, then blank the title) at quoteClaim. Also close a pre-existing CHANGELOG gap: the deferred Rule-of-Two and suspend auto-reattach features shipped in #100 without ledger entries (only egress-hard had one). Added both per CLAUDE.md §6. No behavior change beyond the dedupe; make verify + test-race + tui-verify all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RNT56
force-pushed
the
claude/nilcore-codebase-audit-ec39d8
branch
from
ceae630 to
308c0a4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remediates every finding from a fresh 2026-07-10 adversarial audit (1 CRITICAL, ~11 HIGH, ~12 MEDIUM, plus LOW/hygiene), then adversarially re-verifies the whole diff before shipping. Each fix has a discriminating test (proven to redden if reverted). All three gates are green:
make verify,make test-race(0 data races),make tui-verify.The remediation was produced by parallel file-disjoint fix-agents; the assembled diff was then re-checked by a second wave of independent verifiers (each revert-testing in an isolated worktree) plus a completeness critic. That review caught one live regression — the
.gitwrite-guard was case-sensitive, so.GIT/configbypassed it on macOS/Windows and reopened the git-config RCE — which is fixed here (case+dot fold) with a proven discriminating test.Fixes by severity
CRITICAL —
{{secret:NAME}}browser/desktop typing resolved any process env var (a confused-deputy exfil: a hostile/injected page could type{{secret:ANTHROPIC_API_KEY}}into a form and submit it to an egress-allowed sink). Now gated by an operator-declared per-task secret-name allowlist (-secrets/NILCORE_{BROWSE,DESKTOP}_SECRETS, default empty = deny-all); capguard axis-B counts a secret-capable session.HIGH — host-side git repo-local-config RCE closed at the
worktreefs.writeNoFollowchokepoint (refuses writes inside.git, now case/dot-folded so.GIT/.git.can't bypass on case-insensitive filesystems) + per-command--no-ext-diff; container egress documented as a cooperative proxy (not a hard boundary) with aNILCORE_EGRESS_STRICTfail-closed opt-in; vcache no longer replays a stale GREEN past a changed evidence artifact; artifact "hollow green" (value-blind verifier on a value-bearing claim) refused;sleep/suspend preserves committed work on a durablesuspend/<id>recovery ref instead ofgit branch -Ding it; race-recovered verified work keeps its branch under KeepBranch; the desktop per-action gate is armed on--mac-host; the webhook self-start is rate/label-bounded;nilcore chat/tuiresolve-backend auto; the circuit breaker is no longer poisoned by user steers/cancels; SearchTool reads O_NOFOLLOW;safeRevrejects a leading-dash rev.MEDIUM — promote gate scopes on the target base (not the source tip); MCP responses size-bounded (host OOM);
VAR=0now disablesNILCORE_{AUTONOMY,FLYWHEEL,LIVE_INDEX,LESSONS}; codeintel indexers read O_NOFOLLOW + size-capped; run/chat/serve advisor metered against the budget wall;swarm -blast-radiuswired; memory table pruned/bounded; webhook shares serve's one store handle; clean cancel/error task lifecycle; convergedbuilddeliverable pinned; build/supervise convergence judges the merged tip, not the empty base.LOW — eventlog redaction covers
[]string/json.RawMessage; Slack WS frame length-capped; Telegram token can't ride a URL error.Verification
make verify+make test-race(0 data races) +make tui-verifyall green..gitguard) is fixed and re-proven; WEAK items (mechanism-sound but under-tested) hardened with discriminating tests.Known follow-ups (out of scope for this remediation)
do/run/chat/serve/swarm): capguard's trifecta gate is still only enforced on browse/desktop. It's a standing defense-in-depth gap (the open-egress axis is default-deny on those paths, and they lack the{{secret}}typing primitive); wiring a blocking gate there risks regressing unattended runs, so it's deferred.curl --noproxy/raw sockets); a hard boundary needs runtime netfilter (the namespace backend is the deny-all option today). Documented +NILCORE_EGRESS_STRICTfails closed.suspend/<id>GC: committed work now survives a nap (recoverable undersuspend/<id>), but the wake re-engages a fresh drive rather than auto-reattaching, and the recovery refs are not yet swept.🤖 Generated with Claude Code