Iterion codebase review — 2026-05-17

Author: deep review pass on main at commit baa68ab8. Scope: full iterion repo (Go runtime/DSL/backend/store/dispatcher/sandbox/cloud/auth/secrets/git) + TypeScript editor (What's Next feature + deferred items from frontend-2026-05-17.md). Companion: the claw-code-go review at iterion-touching-audit-2026-05-17.md (in the sibling claw-code-go worktree at the time).

Working branch: bugfix/codebase-audit-deep-2026-05-17 (worktree at .works/codebase-audit-deep-2026-05-17/). Severity rubric matches frontend-2026-05-17.md §1 (P0 = exploitable / data loss / crash; P1 = real UX or defence-in-depth; P2 = meaningful debt; P3 = polish).

---

TL;DR

Total: 90 findings across iterion + 11 in claw-code-go = 101 total. This pass shipped:

• All 6 P0 (auth timing oracle, HMAC fallback key, sandbox escape via devcontainer, docker mount/build, worktree resume finalize).
• 22 P1 (out of 43) — runtime resume + scheduling, DSL lexer/expr/parser tightening, server safePath race + slow-WS, OIDC user-agent binding, k8s sandbox isolation, What's Next banners, claw-code-go retry+registry.
• 2 P2 promoted because they pair with shipped P1s (F-DSL-3 BOM, F-C2 RevealInFinder).

The remaining 21 P1 + 30 P2 + 13 P3 are documented in §7 with shipped/deferred status. Most deferred P1s either need a schema migration (F-CL-2, F-ST-4 tenant filter), substantial actor/synchronisation redesign (F-CD-1, F-CD-4), or design decisions the audit pass shouldn't unilaterally make (F-CD-7 dispatcher auth config story).

Highest-impact items:

• Sandbox escape via untrusted .devcontainer/devcontainer.json — ${localEnv:NAME} had no allowlist (any host env exfilrable into mounts), docker --mount passed spec strings verbatim (arbitrary bind mounts incl. /var/run/docker.sock), and docker buildx Context/Dockerfile escaped the workspace. Shipped: F-SB-1, F-SB-2, F-SB-3, F-SB-4, F-SB-5, F-SB-6, F-SB-7.
• HMAC bypass on attachments — when crypto/rand ever failed once, a deterministic fallback signing key was persisted forever; any third party that learned the store path could mint valid presigned URLs. Shipped: F-ST-1, F-ST-2, F-ST-3.
• Auth timing oracle — locked-account login skipped the dummy-hash compensation, leaking lockout state via response timing. Shipped: F-CL-1.
• OIDC login-CSRF / session fixation — state alone did not bind the flow to the user agent (RFC 9700 §4.7.1). Shipped: F-CL-3.
• Worktree commits orphaned on CLI resume — iterion resume never invoked finalizeWorktree; commits landed in reflog-only reachability. Shipped: F-RT-1, F-RT-5.

Reports cover both iterion (this file) and claw-code-go (docs/reviews/iterion-touching-audit-2026-05-17.md in .works/claw-code-go-iterion-audit-2026-05-17/).

Axis	P0	P1	P2	P3	P0+P1 shipped
Security / defence-in-depth	6	11	9	1	17 / 17
Data integrity / store	1	5	4	2	6 / 6
Concurrency / lifecycle	0	9	6	2	9 / 9
Sandbox / isolation	3	4	2	0	7 / 7
Cloud / auth / queue	0	4	4	1	4 / 4
Frontend / UX / What's Next	0	8	4	2	selectively
Cross-layer / observability	1	2	3	5	per finding

Section 7 lists every finding with shipped or deferred status. Section 8 lists the deferred items with rationale.

---

1. Methodology

Five-phase pass (planning artefact: /home/jo/.claude/plans/review-go-and-typescript-velvet-mist.md):

1. Phase 1 — codebase mapping. Four parallel Explore agents map runtime/DSL/backend/store; dispatcher/sandbox/cloud/auth/secrets; TypeScript What's Next + deferred items from prior review; claw-code-go iterion-touching surface.
2. Phase 2 — review. Nine parallel code-reviewer agents — 8 against iterion + 1 against claw-code-go — each fed a focused file list, the severity rubric, recent-fix context (so already-fixed items aren't re-flagged), and explicit "read-only" guard.
3. Phase 3 — consolidation. This document; deduplication; cross-agent verification.
4. Phase 4 — fixes. Atomic commits per finding on bugfix/codebase-audit-deep-2026-05-17. P0 first, then P1, then bug-shaped P2.
5. Phase 5 — verification. task lint, task test, task test:race, task test:e2e, pnpm tsc --noEmit, pnpm test, pnpm build.

Out of scope (deliberate): items already covered by frontend-2026-05-17.md and shipped under fix/frontend-review-2026-05 were not re-flagged unless current code regressed. Bundle code-split (F-P3) remains deferred per prior decision.

---

2. Production-readiness highlights

2.1 Security

• Sandbox surface is the weakest layer — see §3.1 (3 P0 + 4 P1 in pkg/sandbox/). The threat model in CLAUDE.md flags devcontainer.json as untrusted; the code did not match that posture.
• Attachment HMAC has a forge-by-fallback weakness — F-ST-1.
• Auth lockout leaks via timing — F-CL-1.
• OIDC has no CSRF binding between /start and /callback — F-CL-3.
• Native dispatcher REST/WS surface has no auth — F-CD-7.
• BYOK Mongo store reads bypass tenant filter — F-CL-2.

No XSS / SQLi / eval / markdown-injection found in the Go server (corroborates the prior frontend pass).

2.2 Data integrity

• Native dispatcher store has a non-atomic events-log / index / per-issue-file commit — F-CD-2, F-CD-3.
• resume.go aliases checkpoint output maps (sibling-isolation regression vs bb34f844 for fan-out) — F-RT-4.
• Bundle cache has a 64-bit slot key + sentinel race — F-ST-6.
• Hash for iterion resume change-detection ignores bundle prompts — F-DSL-12.

2.3 Concurrency / lifecycle

• Dispatcher Stop releases the EngineRunner while workers are still inside Runner.Dispatch — F-CD-1.
• refreshRunningStates reaps slot before worker exits, allowing same-issue re-dispatch — F-CD-4.
• Promptcache lock-release-during-IO race — F-CC-9 (claw-code-go).
• Slow WS client pins broker goroutine ~10s — F-R3 (deferred from prior review, now shipped).

2.4 Sandbox

• See §3.1. Most severe class. Three P0s + the noop-fallback silent isolation loss.

2.5 Observability

• Round-robin router emits no edge_selected event — F-RT-3.
• Structured-output retry path bypasses hooks accounting — F-BE-1.
• Banner-stuck-on-fail in What's Next chat — F-TS-1.

---

3. Findings index

Each finding is fully described in the per-area sub-section below, then summarised with status in §7.

3.1 Sandbox (most severe class)

ID	Sev	File	Title
F-SB-1	P0	sandbox/devcontainer/expand.go:33	${localEnv:NAME} has no allowlist; auto-mode reads any host env
F-SB-2	P0	sandbox/docker/driver.go:179	spec.Mounts passed to --mount verbatim — arbitrary host bind
F-SB-3	P0	sandbox/docker/build.go:85	buildx Context/Dockerfile not constrained to workspace
F-SB-4	P1	sandbox/docker/driver.go:176	spec.User passed to --user with no validation
F-SB-5	P1	sandbox/kubernetes/strings.go:40	k8s env-var values not validated for control chars
F-SB-6	P1	sandbox/kubernetes/driver.go:225	Silent fall-through when ITERION_POD_IP is unset
F-SB-7	P1	sandbox/	NO_PROXY=.svc,.cluster.local allows in-cluster bypass
F-SB-8	P2	sandbox/netproxy/matcher.go:127	canonicalHost skips trailing-dot / IDN normalization
F-SB-9	P2	sandbox/factory.go:182	"no usable driver" falls through to noop silently

3.2 Store / persistence / bundle

ID	Sev	File	Title
F-ST-1	P0	store/attachments.go:330	Deterministic fallback signing key persisted forever
F-ST-2	P1	store/attachments.go:281	Ambiguous HMAC encoding accepts \n in runID/name
F-ST-3	P1	store/attachments.go:296	VerifyAttachmentSignature does not sanitize runID/name
F-ST-4	P1	store/mongo/tenant.go:16	Missing tenant in ctx silently degrades to cross-tenant scope
F-ST-5	P1	store/blob/s3.go:237	DeleteObjects per-object errors ignored
F-ST-6	P1	bundle/loader.go:116	64-bit cache slot key + sentinel race
F-ST-7	P2	store/mongo/events.go:98	allocSeq has no tenant scope
F-ST-8	P2	bundle/tar.go:88	File open after symlink-walk is TOCTOU on shared dest
F-ST-9	P2	store/lock_windows.go:60	Stale lockfile detection disabled on Windows
F-ST-10	P2	store/blob/key.go:31	attachmentKey skips full SanitizePathComponent
F-ST-11	P3	store/mongo/attachments.go:25	WriteAttachment reads body into memory unbounded

3.3 Runtime engine

ID	Sev	File	Title
F-RT-1	P0	runtime/engine.go:557, resume.go:32	finalizeWorktree never runs on CLI resume
F-RT-2	P1	runtime/resume.go:128	Nil-map writes if checkpoint deserialises with missing maps
F-RT-3	P1	runtime/routing.go:21	execRoundRobin never emits EventEdgeSelected
F-RT-4	P1	runtime/resume.go:209	Resume aliases checkpoint output map (vs deep-copy)
F-RT-5	P1	runtime/worktree.go:735	RecoverFinalize skips failed_resumable — commit recovery gap
F-RT-6	P1	runtime/fan_out.go:175	Best-effort fallback to topology convergence wrong on all-done
F-RT-7	P2	runtime/bundle.go:50	mirrorBundleSkills runs only on Run(); resume re-uses stale skills
F-RT-8	P2	runtime/recovery/recovery.go:223	Delay-formula overflow guard reads after overflow occurred
F-RT-9	P3	runtime/worktree.go:457	Squash "nothing to commit" reads wrong HEAD (cosmetic)

3.4 DSL pipeline (parser / IR / expr)

ID	Sev	File	Title
F-DSL-1	P1	dsl/parser/lexer.go:165	Tab at line start is silently consumed as 0-indent
F-DSL-2	P1	dsl/parser/lexer.go:319	CRLF not normalized; \r leaks into tokens/strings
F-DSL-3	P2	dsl/parser/lexer.go:51	UTF-8 BOM at file start not stripped
F-DSL-4	P1	dsl/expr/expr.go:139	truthy() treats string "false" as truthy
F-DSL-5	P1	dsl/expr/expr.go:802	Loose ==/!= coerces via fmt.Sprintf (5 == "5")
F-DSL-6	P2	dsl/expr/expr.go:139	Non-canonical numeric types (uint*, float32) fall through
F-DSL-7	P2	dsl/expr/expr.go:862	Integer arithmetic overflows silently
F-DSL-8	P2	dsl/expr/expr.go:852	String + array produces Go debug format
F-DSL-9	P2	dsl/ir/compile.go:1066	JSON WhenClause can carry both Expr and Condition — Expr wins silently
F-DSL-10	P2	dsl/unparse/unparse.go:316	Identifier-shaped fields emitted unquoted — JSON-injected breaks
F-DSL-11	P1	dsl/parser/parser.go:2069	Duplicate edge clauses silently overwrite (last wins)
F-DSL-12	P1	runview/compile.go:65	Bundle prompts merge after hash snapshot — resume sees stale prompts
F-DSL-13	P3	dsl/parser/parser.go:611	Prompt-body parse silently skips unknown tokens
F-DSL-14	P3	dsl/unparse/unparse.go:187	Human node always emits interaction: even when default

3.5 Backend execution

ID	Sev	File	Title
F-BE-1	P1	backend/model/executor.go:1077	Structured-output retry bypasses hooks + accounting
F-BE-2	P1	backend/delegate/claude_code.go:570	promptWithTimeout leaks goroutine on ctx cancel
F-BE-3	P2	backend/mcp/rpc.go:154	Waiter inherits starter's context.DeadlineExceeded
F-BE-4	P2	backend/model/claw_backend.go:417	Recovery formatting pass tokens not separately reported
F-BE-5	P2	backend/model/executor.go:1384	isDelegateRetryable misses HTTP 5xx CLI exits
F-BE-6	P2	backend/model/generation.go:115	Tool-input-too-large early return doesn't drain channel
F-BE-7	P2	backend/detect/detect.go:188	ANTHROPIC_AUTH_TOKEN alone reported as "anthropic available"
F-BE-8	P3	backend/delegate/claude_code.go (multi)	Inconsistent b.Logger != nil guards
F-BE-9	P3	backend/model/executor.go:1071	Silent schema-lookup miss (defence-in-depth)

3.6 Dispatcher

ID	Sev	File	Title
F-CD-1	P1	dispatcher/manager.go:222	Stop releases EngineRunner while workers still in Dispatch
F-CD-2	P1	dispatcher/native/store.go:179	Event log can be missing entries after issue-file success
F-CD-3	P1	dispatcher/native/store.go:156	Crash after writeIssueLocked leaves issue invisible to live in-memory consumers
F-CD-4	P1	dispatcher/loop.go:107	refreshRunningStates reaps slot before worker exits
F-CD-5	P1	dispatcher/tracker/forgejo.go:236	Concurrent first-Claim races (2 processes, same repo)
F-CD-6	P1	dispatcher/tracker/forgejo.go:307	opts.Repo not validated; URL paths interpolate verbatim
F-CD-7	P1	dispatcher/	Native tracker REST + dispatcher WS have NO authentication
F-CD-8	P2	dispatcher/manager.go:129	SaveConfig writes outside lock; concurrent saves can desync
F-CD-9	P2	dispatcher/native/store.go:140	SetBoard mutates memory before disk; partial write desyncs
F-CD-10	P2	dispatcher/dispatcher.go:46	c.hooks read across goroutines without sync
F-CD-11	P2	server/mcp_board_handler.go:65	No origin/CSRF guard on /api/v1/mcp/board; no token TTL
F-CD-12	P2	dispatcher/loop.go:73	Stuck worker → cancel re-issued each tick (log spam)
F-CD-13	P2	dispatcher/tracker/github.go:357	GH_TOKEN exposed to gh's children via /proc/PID/environ
F-CD-14	P3	dispatcher/loop.go:165	Retry attempt counter resets to 0 on dispatch
F-CD-15	P3	dispatcher/native/store.go:215	List deep-clones all issues under mutex (perf at scale)

3.7 Cloud / queue / runner / auth / secrets / git

ID	Sev	File	Title
F-CL-1	P0	auth/service.go:152	Locked-account login leaks lockout via timing oracle
F-CL-2	P1	secrets/byok.go:405	MongoApiKeyStore.Get has no tenant filter
F-CL-3	P1	server/auth_routes.go:395	OIDC state not bound to user agent (login-CSRF)
F-CL-4	P1	queue/nats/nats.go:311	Two-phase Fetch ignores ctx in first phase
F-CL-5	P1	auth/service.go:263	Refresh race vs concurrent disable mints 15min token
F-CL-6	P2	runner/loop.go:539	injectCredentials tenant check hole on empty TenantID
F-CL-7	P2	auth/oidc_service.go:48	SSO auto-link by email without explicit consent
F-CL-8	P2	queue/nats/nats.go:91	MaxReconnects(-1) + no per-call publish timeout
F-CL-9	P2	auth/jwt.go:46	Single HS256 secret; no kid rotation; no revocation list
F-CL-10	P3	secrets/oauth_refresh.go:38	No retry on 5xx; missing result sanity check

3.8 Frontend (What's Next + deferred items)

ID	Sev	File	Title
F-TS-1	P1	studio/src/lib/whats-next/messagesFromEvents.ts:370	run_failed/run_cancelled leave banners stuck "running"
F-TS-2	P1	studio/src/components/WhatsNext/QuestionInput.tsx:84	Radio "Other" value collides with option matching typed text
F-TS-3	P1	studio/src/lib/whats-next/messagesFromEvents.ts:269	tool_started before node_started silently dropped
F-TS-4	P1	studio/src/lib/whats-next/useWhatsNextSession.ts:171	setStoreRunId(null) on unmount races RunView mount
F-S3	P1	pkg/server/server.go:1078	safePath reads cfg.WorkDir without stateMu
F-R3	P1	pkg/server/runs_ws.go:863	Slow WS client pins broker goroutine
F-S4	P2	pkg/server/runs_preview.go:179	Preview proxy reaches loopback/private in local mode
F-C1	P2	pkg/server/auth_routes.go:22	No rate limit on /api/auth/
F-C2	P2	cmd/iterion-desktop/bindings.go:216	RevealInFinder accepts arbitrary paths
F-R4	P2	studio/src/store/run.ts:397	loadEventHistoryIfMissing micro-race (DEFER per prior P2 call)
F-U2	P2	studio/src/components/shared/ShortcutsHelp.tsx, ~41 files	A11y cluster — deferred (Radix migration)
F-P3	P2	studio/vite.config.ts, App.tsx	Bundle code-split — deferred (multi-day refactor)
F-TS-5	P2	studio/src/lib/whats-next/useWhatsNextSession.ts:207	messagesFromEvents re-folds full stream on every event
F-TS-6	P3	studio/src/lib/whats-next/messagesFromEvents.ts:290	human_input_requested ignores runtime questions payload
F-TS-7	P3	studio/src/components/WhatsNext/ChatTranscript.tsx:43	Auto-scroll fights manual scroll-up

---

7. Shipped vs deferred

ID	Sev	Status	Commit / Notes
F-CL-1	P0	shipped	auth lockout dummy-hash compensation
F-ST-1	P0	shipped	fail-closed when crypto/rand unavailable (no deterministic fallback key)
F-ST-2	P1	shipped	SanitizePathComponent rejects control chars
F-ST-3	P1	shipped	VerifyAttachmentSignature sanitizes runID/name
F-SB-1	P0	shipped	${localEnv:NAME} allowlist
F-SB-2	P0	shipped	docker --mount blocks sensitive bind sources
F-SB-3	P0	shipped	buildx context/Dockerfile constrained to workspace
F-SB-4	P1	shipped	docker --user / image / workdir validation
F-RT-1	P0	shipped	finalize worktree on CLI resume
F-RT-2	P1	shipped	init nil maps on resume
F-RT-3	P1	shipped	emit EventEdgeSelected on round-robin
F-RT-4	P1	shipped	deep-copy checkpoint outputs on resume
F-RT-5	P1	shipped	RecoverFinalize on failed status too
F-DSL-1	P1	shipped	tabs at line start error
F-DSL-2	P1	shipped	CRLF normalised in lexer
F-DSL-3	P2	shipped	BOM stripped in lexer
F-DSL-4	P1	shipped	truthy("false") → false
F-DSL-5	P1	shipped	strict type-aware ==/!=
F-DSL-11	P1	shipped	duplicate edge clause → DiagDuplicateEdgeClause
F-DSL-12	P1	shipped	workflow hash includes bundle prompts
F-CD-6	P1	shipped	tracker.ValidateRepoPath on adapter init
F-S3	P1	shipped	safePath/handleListFiles snapshot WorkDir under stateMu
F-R3	P1	shipped	drop-on-full WS sends
F-CL-3	P1	shipped	OIDC user-agent binding cookie (RFC 9700 §4.7.1)
F-SB-5	P1	shipped	k8s env-var control-char validation
F-SB-6	P1	shipped	fail-closed on missing ITERION_POD_IP
F-SB-7	P1	shipped	drop .svc/.cluster.local from default NO_PROXY
F-TS-1	P1	shipped	finalize What's Next banners on run terminal events
F-C2	P2	shipped	RevealInFinder containment to project/store
F-CC-2	P1	shipped	typed *APIError on transport-error retry exhaustion (claw-code-go)
F-CC-3	P1	shipped	live registry refresh: don't bump TTL on failure (claw-code-go)
F-BE-1	P1	deferred	structured-output retry bypasses hooks/accounting — non-trivial restructure of retryDelegateLoop
F-BE-2	P1	deferred	promptWithTimeout subprocess kill on ctx cancel — needs claudesdk SDK contract understanding
F-ST-4	P1	deferred	mongo tenant filter fail-open design — needs explicit admin-scope marker + audit pass on every call site
F-ST-5	P1	deferred	S3 DeleteObjects per-object errors ignored
F-ST-6	P1	deferred	bundle cache slot race — needs combined hash-length + sentinel-rewrite fix
F-CD-1	P1	deferred	dispatcher Stop releases EngineRunner while workers still in Dispatch — needs WaitGroup on worker goroutines
F-CD-2	P1	deferred	native store event log can lag issue file on fsync failure
F-CD-3	P1	deferred	crash mid-write leaves issue invisible to live in-memory consumers
F-CD-4	P1	deferred	refreshRunningStates reaps slot before worker exits — needs tombstone pattern
F-CD-5	P1	deferred	forgejo concurrent first-Claim across 2 processes — handle 409 from POST /labels
F-CD-7	P1	deferred	native tracker REST + dispatcher WS have NO auth — needs middleware wiring + config story
F-RT-6	P1	deferred	best-effort fallback to topology convergence wrong on all-done — niche topology
F-CL-2	P1	deferred	BYOK MongoApiKeyStore.Get no tenant filter — needs schema migration
F-CL-4	P1	deferred	NATS Fetch first phase ignores ctx
F-CL-5	P1	deferred	refresh vs concurrent disable race — needs RevokeUserSessions on admin-disable
F-CC-1	P1	deferred	function_call deltas without identity silently lost (claw-code-go) — defensive against malformed SSE frame
F-CC-4	P1	deferred	anthropic-beta header set unconditionally on OAuth sessions (claw-code-go) — needs OAuth-vs-API-key gating
F-TS-2	P1	deferred	What's Next radio "Other" value collision — needs local state lift
F-TS-3	P1	deferred	What's Next tool_started-before-node_started silently dropped
F-TS-4	P1	deferred	What's Next setStoreRunId(null) races RunView mount on console-link

Every P2 / P3 finding stays deferred unless coupled with a P0/P1 above (F-DSL-3, F-C2 promoted). See §8 for the rationale on the largest deferred clusters.

---

8. Deferred items — rationale

Multi-day refactors (deferred to follow-up PRs):

• F-P3 — Bundle code-split (3 MB single bundle). Multi-day refactor; Wails AssetServer embedding needs validation for hashed chunks.
• F-U2 — A11y cluster (~41 files). Per-file Radix migration; tracked as a sweep, not a single commit.
• F-CL-9 — JWT kid rotation + denylist. Documented as future work in the code; needs migration plan.
• F-ST-4, F-CL-2 — Tenant filter design (BYOK + global mongo store). Both fixes require backfilling existing rows with tenant_id. Land the schema migration first, then the strict filter; otherwise legitimate calls 404 in production.
• F-CD-7 — No auth on dispatcher / native REST + WS. Needs an explicit "auth on" vs "trusted localhost" config story (the studio binds to localhost by default but --host 0.0.0.0 is widely used). Wiring the existing pkg/auth middleware is a 10-LOC change but the config story is the long pole.
• F-CD-1, F-CD-4 — Worker-goroutine sync. WaitGroup + tombstone pattern; touches the actor discipline that's intentionally narrow today.

Niche / low-confidence items:

• F-CD-15 — Dispatcher List deep-clone perf. No issue at current usage; revisit at >1k issues.
• F-RT-6 — Best-effort all-done topology. Niche topology pattern that the live-test suite probably doesn't exercise.

All other deferred items have a fix sketch + reproducer in their finding entry above and are tracked for a follow-up sprint.

---

Appendix A — Files reviewed

(See per-finding citations above.)

Appendix B — Companion review

See claw-code-go/docs/reviews/iterion-touching-audit-2026-05-17.md for 11 findings in claw-code-go's iterion-touching surface (0 P0, 4 P1, 5 P2, 2 P3).