leanos/REVIEW.md

Code Review — NEON NOODLE OS

Five-pass review. One pass per loop iteration. Findings: file, line, severity, fix. Critical/high issues are fixed directly; medium/low are logged for triage.

Pass	Topic	Status
1	Runtime / type / broken-import errors	✅ done
2	Logic bugs (races, edge cases, null checks)	✅ done
3	Security audit	✅ done
4	Error handling	✅ done
5	Consistency (naming, dead code, duplication)	✅ done

---

Pass 1 — Runtime / type / broken-import errors

Method: npx tsc --noEmit (strict), production vite build, import-resolution scan, manual read of the runtime-critical path (appRuntime.tsx Babel transform + new Function scope).

Result: no runtime, type, or import errors.

• tsc --noEmit — clean, zero diagnostics.
• vite build — succeeds (only the pre-existing >500 kB chunk-size advisory, not an error).
• No project ESLint config present (ESLint not configured) — noted, not a defect.
• All import specifiers resolve to existing modules; no dangling paths.
• appRuntime.compileApp passes React + hooks + os into a new Function scope and memoises the built component per os instance — no obvious runtime fault in the happy path.

Findings

None at severity high or above. Observations deferred to later passes:

#	File:line	Sev	Note	Defer to
1.1	kernel/appRuntime.tsx:223	low	Compile cache Map is never evicted; entries accumulate across self-heals (bounded by app count × versions).	Pass 2
1.2	kernel/ai.ts:62	low	currentProvider() exported but unused by callers (Settings reads aiProvider from the store). Possible dead export.	Pass 5

No fixes applied in Pass 1 (nothing at critical/high).

---

Pass 2 — Logic bugs (races, edge cases, off-by-one, null checks)

Method: manual read of state/logic paths — store.ts (window mgr, openApp, installAndOpen), fs.ts, sdk.ts, bus.ts, appRuntime.tsx, AppHost.tsx, Window.tsx (drag/resize/snap), Terminal.tsx, Assistant.tsx.

Findings

#	File:line	Sev	Issue	Fix
2.1	apps/Terminal.tsx:84 (rm)	high	resolve("") returns cwd, so a bare rm (no operand) deletes the current directory — at / it wipes the entire virtual FS. Also affected cat/mkdir/touch (operated on cwd silently).	✅ Fixed — added missing-operand guards to rm, cat, mkdir, touch.
2.2	kernel/sdk.ts:32	medium	ai.available was a one-time snapshot (aiAvailable()) captured when the window's os API is built (memoised per window in AppHost). Configuring the key in Settings afterwards left already-open apps (e.g. Terminal) reporting AI offline.	✅ Fixed — converted available to a getter returning the live value.
2.3	system-ui/Window.tsx:33	low	Title-drag clamps Y to below the menu bar but never clamps X — a window can be dragged fully off-screen left/right with no way back.	deferred (UX, low)
2.4	kernel/store.ts:274	low	installAndOpen fuzzy match: a.name.toLowerCase().includes(query) can match the wrong app on very short queries (e.g. "a"). Intended fuzzy behaviour; acceptable.	deferred (low)
2.5	kernel/fs.ts:65 (write)	low	write only auto-creates the parent if it's absent; if the parent path exists as a file, the child is written anyway (no "not a directory" error, unlike mkdir).	deferred (low)
2.6	kernel/appRuntime.tsx:223	low	(from Pass 1) compile cache Map never evicted. Bounded by app×version count; not a leak in practice.	deferred (low)

Fixes applied: 2.1 (high), 2.2 (medium). Typecheck clean after changes.

---

Pass 3 — Security audit

Method: npm audit, dangerous-sink grep (innerHTML, dangerouslySetInnerHTML, eval, new Function, document.write), secret-handling trace (ai.ts/sdk.ts/db.ts), sandbox/blocklist review (appRuntime.tsx), dev-proxy review (vite.config.ts), .gitignore.

Good:

• npm audit (prod deps) — 0 vulnerabilities.
• No innerHTML / dangerouslySetInnerHTML / document.write anywhere — React auto-escaping intact; Terminal renders into <pre> (safe).
• .env, .env.local are git-ignored — secrets not committed.
• API keys are the user's own and sent only to the selected provider (correct auth headers per API shape).

Findings

#	File:line	Sev	Issue	Fix
3.1	kernel/appRuntime.tsx:25	medium	Generated apps run via new Function in the page realm; the regex blocklist omitted indexedDB, so app code could open the fluid-os DB and read the settings → aiConfig store (all API keys) and exfiltrate them.	✅ Fixed — added indexedDB, globalThis, Function(, import.meta to the blocklist (defense-in-depth).
3.2	kernel/appRuntime.tsx:42	medium	The blocklist is not a real boundary — generated code shares the realm and can still escape via deep prototype tricks ([].constructor.constructor, etc.). Documented inline.	accepted by design (PROMPT.md: in-page execution). True isolation = iframe/worker — large refactor, deferred.
3.3	vite.config.ts:90 (/__ai/proxy)	low (dev-only)	Dev proxy forwards to any x-llm-url with the request's auth headers — an open proxy / SSRF + key-relay vector. Not present in production builds (no middleware ships).	accepted (dev convenience; required for multi-provider CORS bypass).
3.4	vite.config.ts:9 (/__proxy)	low (dev-only)	In-OS browser proxy fetches arbitrary ?url= server-side and strips framing headers — SSRF in dev only.	accepted (dev-only; intended for the browser app).
3.5	kernel/db.ts (settings store)	low/info	API keys persisted in IndexedDB in plaintext — inherent to a backend-less client app; keys never leave except to the chosen provider. Mitigated against in-page theft by 3.1.	accepted by design.
3.6	kernel/ai.ts:100	info	anthropic-dangerous-direct-browser-access: true enables direct browser calls to Anthropic — intentional for local dev.	accepted.

Fixes applied: 3.1 (medium — blocklist hardening). Typecheck clean.

---

Pass 4 — Error handling (uncaught exceptions, missing try/catch, silent failures)

Method: traced every async path and entry point — store.init/installAndOpen/healApp, App.tsx boot, sdk.ts fs/ai, built-in apps (Terminal, Files, Assistant, AppBuilder), ai.ts chat/stream, the render-time AppErrorBoundary.

Already solid:

• installAndOpen, healApp (store) — try/catch with user-facing notify. ✓
• chat / chatStream (ai.ts) — throw with provider + status + body on non-2xx; callers catch. ✓
• Terminal.run, Assistant.send — try/catch surfacing errors to the UI. ✓
• AppErrorBoundary + self-heal catches render errors in generated apps. ✓
• bus.emit — wraps each handler in try/catch. ✓

Findings

#	File:line	Sev	Issue	Fix
4.1	kernel/store.ts:100 + App.tsx:53	high	init() had no try/catch and App calls void init(). Any failure in fs.seed/loadAiConfig/loadApps (e.g. IndexedDB blocked in private mode) left ready=false → OS stuck on the boot screen forever, silently (rejection swallowed by void).	✅ Fixed — init() now try/catches: on failure it boots with built-in apps + in-memory defaults, sets ready=true, and notifies "Storage unavailable". OS always reaches the desktop.
4.2	apps/Files.tsx:13,22,32,44,54	low	refresh/open/save/create/del call os.fs.* without try/catch; an IndexedDB failure becomes an unhandled rejection (console-only, no UI feedback). Error boundary doesn't catch async.	deferred (low — IndexedDB ops reliable; built-in app).
4.3	apps/Settings.tsx onSave	low	await saveAiConfig(cfg) not wrapped; a persistence failure would skip setSaved(true) and reject unhandled.	deferred (low).
4.4	kernel/store.ts setTheme/setWallpaper	low	void setSetting(...) fire-and-forget swallows persistence errors silently (cosmetic settings only).	accepted (low impact).
4.5	apps/Terminal.tsx (ai stream)	low	If os.ai.stream throws mid-stream, the catch reports it but the pre-pushed empty "out" line remains. Cosmetic.	deferred (low).

Fixes applied: 4.1 (high — boot resilience). Typecheck clean.

---

Pass 5 — Consistency (naming, dead code, duplicated logic)

Method: export-usage grep (dead code), stale-string scan, magic-number/duplication scan.

Findings

#	File:line	Sev	Issue	Fix
5.1	kernel/ai.ts:62	low	currentProvider() exported but never imported anywhere (Settings reads aiProvider from the store). Dead code.	✅ Fixed — removed.
5.2	Spotlight:148, Assistant:43, AppBuilder:33, Terminal:109, store:327	medium	Five user-facing strings still said "set VITE_ANTHROPIC_API_KEY in .env" — stale since keys moved to Settings → Language Model (multi-provider). Misleading/inconsistent UX.	✅ Fixed — all reworded to "add an API key in Settings → Language Model".
5.3	store.ts:78 & Window.tsx:8	low	MENUBAR_H = 36 duplicated as a magic number in two modules (plus DOCK_RESERVE, MIN_W/H).	deferred — extract to a shared layout constants module (low risk, low value).
5.4	App.tsx:12 cssVars vs tokens.ts tokensAsCss	low	Two overlapping CSS-var builders. Intentional (apps get a subset; root gets the full set incl. shadows/radius-xl) but the key list is partly duplicated.	accepted (different scopes by design).
5.5	registry.ts icons	low	App Builder uses "⚙️" and Settings uses "⚙" — near-identical gears, visually ambiguous in dock/desktop.	deferred (cosmetic identity choice — recommend a distinct glyph for App Builder, e.g. 🛠️).

Fixes applied: 5.1 (dead code removed), 5.2 (medium — stale strings). Typecheck + full vite build clean.

---

Summary

All 5 passes complete. Verified with tsc --noEmit (clean) and production vite build (succeeds).

Fixes applied (7):

Sev	Pass	Fix
high	2.1	Terminal rm/cat/mkdir/touch now require an operand — a bare rm no longer deletes the cwd / whole virtual FS.
high	4.1	store.init() is now fault-tolerant — IndexedDB failure boots the OS with defaults + a notification instead of hanging on the boot screen forever.
medium	2.2	os.ai.available is a live getter, not a stale snapshot — apps see the key after it's configured in Settings.
medium	3.1	Sandbox blocklist hardened (indexedDB, globalThis, Function(, import.meta) — closes the trivial API-key exfiltration path.
medium	5.2	Five stale "VITE_ANTHROPIC_API_KEY" prompts reworded to point at Settings → Language Model.
low	5.1	Removed dead currentProvider() export.

Notable accepted/deferred items:

• 3.2 (medium): generated-code sandbox is not a true security boundary (in-page new Function). Accepted by design (PROMPT.md); real isolation needs an iframe/worker — a substantial refactor, out of scope for this review.
• 3.3/3.4 (low): dev-server proxies are open-proxy/SSRF vectors but ship only in dev, never in production builds.
• Low-severity polish (window X-clamp, Files/Settings async try/catch, MENUBAR_H dedupe, duplicate gear icons) logged above for future triage.

Overall: healthy codebase. 0 dependency vulnerabilities, no type/import errors, no XSS sinks. The two highest-impact issues (FS-wiping rm, silent boot hang) are fixed; the chief residual risk (sandbox isolation) is inherent to the product's design and documented.

Review complete — loop stopped.