feat(storage): archived cleanup with quarantine (phase 2 of #42)

[Wibias]

Summary

• Phase 2 (Option A) for Add Storage page for Codex session usage and cleanup policy #42: preview-first cleanup of the oldest archived sessions by percentage (slider + 10/25/50% presets), quarantine by default to CODEX_HOME/.trash/<epoch>/, permanent delete only behind an explicit checkbox.
• Active sessions/ are never touched. Cleanup probes state_*.sqlite writability first and returns 409 codex_busy with zero FS changes when Codex holds the lock.
• Reconciles matching threads rows (and thread_spawn_edges when present) with archived JSONL paths.
• Phase 1 hygiene in the same PR: fix stale scanner comment, exclude .trash from scan totals/other, remove dead scanStorage imports, document Storage on the web dashboard guide.

API

• POST /api/storage/cleanup/preview { percent }
• POST /api/storage/cleanup { percent, mode: "quarantine" | "permanent" }

Test plan

• bun test --isolate tests/storage-scanner.test.ts tests/storage-cleanup.test.ts tests/api-storage-cleanup.test.ts tests/api-storage.test.ts
• bun run typecheck
• bun x tsc -p gui --noEmit
• Manual: Storage page → preview → quarantine; confirm .trash + rescan; busy Codex → refused

Summary by CodeRabbit

• New Features
  • Added Storage diagnostics for CODEX_HOME (read-only).
  • Introduced “Archived cleanup” with percent-based preview of the oldest items, then quarantine (default) or permanent deletion after explicit confirmation; active items remain write-protected.
• Bug Fixes
  • Quarantined items in .trash/ no longer affect Storage totals or “largest files.”
• Documentation
  • Updated the Web Dashboard guide to describe Storage and archived-cleanup behavior/restore limitations.
• Localization
  • Added/updated Storage UI text for archived-cleanup flow across supported languages.

[Wibias]

Tracks Phase 2 of #42 (Option A: quarantine default + permanent checkbox). Phase 1 hygiene included in the same PR.

[Wibias]

Security review — PR #529 (storage phase 2 cleanup)

Reviewer: automated security pass (post CodeRabbit/Codex fixes, including pickWireCleanupTestHooks in 3b0c85d2)
Scope: src/storage/cleanup.ts, src/server/management/logs-usage-routes.ts, gui/src/pages/Storage.tsx, tests/api-storage-cleanup.test.ts, tests/storage-cleanup.test.ts
Verdict: No medium, high, or critical findings. Merge-ready from a security standpoint.

@lidge-jun @Ingwannu — CI is green (ubuntu/macos/windows + npm-global + react-doctor). Codex and CodeRabbit review threads are addressed. This PR is merge-ready pending your maintainer sign-off.

---

Auth boundary

Cleanup routes sit behind the same stack as the rest of /api/*:

• requireApiAuth(req, config, "management") in src/server/index.ts
• isAllowedRequestOrigin enforced in handleManagementAPI before handlers run

Bind mode	Behavior
Loopback (127.0.0.1)	No API token required — any local process with access to the proxy can preview/execute cleanup. Same trust model as POST /api/stop, config changes, etc.
Non-loopback	OPENCODEX_API_AUTH_TOKEN / config API keys required

codexHome is never taken from the request body; execution always uses resolveCodexHomeDir(). No new cross-tenant or cross-user boundary is introduced.

---

Path traversal / filesystem scope

Control	Implementation
API inputs	Only percent, mode, digest (+ optional env-gated _test). No caller-supplied paths.
Candidate discovery	Flat archived_sessions/ only; isSafeArchiveFileName rejects /, \, ...
DB path normalization	normalizeArchivedRolloutPath requires archived_sessions/<single-segment>.jsonl; rejects .. and active sessions/.
Resolve before IO	absFromRel rejects ../absolute paths and re-checks containment under CODEX_HOME.
Active sessions	sessions/ is never listed or deleted.

Symlinks under archived_sessions/ move the symlink node via resolve(codexHome, rel), not arbitrary targets outside the home tree.

---

Destructive FS/DB operations

Design matches the stated threat model:

1. Probe all runtime DBs (state, logs, goals, memories) with BEGIN IMMEDIATE before any rename.
2. Digest-bound execute — SHA-256 over percent + candidate paths/sizes/mtimes; drift → stale_preview (409).
3. Stage → manifest → DB reconcile with satellite snapshots, ordered commits, and FS rollback on failure.
4. Referenced history guard (spawn edges, paginated history_mode, fork columns) → referenced_history (409).
5. Quarantine default; permanent only via explicit mode: "permanent" (GUI checkbox).
6. Recovery artifacts: .trash/<epoch>/manifest.json (+ satellite-backup.json during reconcile), 0o700 trash dir / 0o600 manifest via writePrivateFile.

Residual (acceptable): the initial DB probe is not held for the entire operation. If Codex grabs an exclusive lock mid-flight, cleanup returns codex_busy / rollback paths — documented in API messages and covered by tests. This is not an auth bypass.

---

Test hook exposure (pickWireCleanupTestHooks)

const testHooks =
  process.env.OPENCODEX_CLEANUP_TEST_HOOKS === "1" && "_test" in body
    ? pickWireCleanupTestHooks(body._test)
    : undefined;

• Without OPENCODEX_CLEANUP_TEST_HOOKS=1: _test is ignored entirely.
• Allowlist: booleans + string arrays only; afterSatelliteMutations (function callback) is not on the wire.
• Hooks inject failure points for atomicity tests only — they do not run shell commands, change codexHome, or accept arbitrary paths.

Residual risk (low): if someone sets the env var on a production service, an API-authenticated client could force failure modes. That does not exceed what the same client can already do with mode: "permanent". Recommendation (non-blocking): document "test/CI only; never set in service config."

---

Information disclosure in API responses

Endpoint	Disclosure
POST .../preview	Relative relPath only; strips codexHome / absPath (tested).
POST .../cleanup errors	Mapped error codes + fixed message strings; trashDir is relative (e.g. .trash/173…).
GET /api/storage	Still returns full codexHome (phase 1; unchanged by this PR).

Error JSON is asserted not to contain absolute host paths in tests/api-storage-cleanup.test.ts. GUI renders errors as React text (no dangerouslySetInnerHTML); trashDir in i18n is server-controlled relative path.

Manifest on disk may include threadId / rolloutPath for recovery — local session metadata under 0o600 in user-owned .trash; acceptable for same-user recovery.

---

Race conditions

Scenario	Mitigation
Preview → execute TOCTOU	Digest includes per-file mtimes; file set changes → stale_preview.
Concurrent Codex writer	Upfront probe + busy mapping; staged FS rollback on DB failure.
Partial permanent purge	purgeIncomplete manifest; relative trashDir in 500 response (tested).
Satellite DB partial commit	Snapshot + restoreSatelliteBackup; keeps stage when restore fails.
Concurrent satellite writes during cleanup	BEGIN IMMEDIATE write locks held through snapshot/delete; conflict-safe INSERT … ON CONFLICT DO NOTHING restore.

---

GUI (Storage.tsx)

• No dangerouslySetInnerHTML; paths shown as text in <li> / tables.
• Confirm modal + permanent checkbox before destructive execute; checkbox resets on dismiss.
• Preview list capped at 8 rows but count/bytes/digest reflect the full preview response (digest still binds full server-side set).

---

Findings summary

Area	Blocker?	Notes
Auth / CSRF	No	Same model as existing management API.
Path traversal	No	Strong containment on all changed paths.
Destructive ops	No	Scoped to archived files; active sessions protected.
Test hooks	No	Env-gated; allowlisted; no privilege escalation.
Info disclosure	No	Cleanup responses avoid host paths; relative recovery hints only.
Races / recovery	No	Documented partial-failure behavior with manifest/trash.

Optional hardening (not required for merge):

1. API test that _test is ignored when OPENCODEX_CLEANUP_TEST_HOOKS is unset.
2. One-time startup warning if OPENCODEX_CLEANUP_TEST_HOOKS=1 outside NODE_ENV=test.
3. Ops note that remote/LAN deployments must use API auth (already required for non-loopback).

---

Conclusion

PR #529 storage cleanup is well-bounded for a local-first management surface. No concrete medium+ vulnerabilities with realistic exploitability beyond the existing "whoever can call the local management API can manage the local proxy" trust model.

Merge recommendation: approve.

[Ingwannu]

Independent maintainer re-review completed on 3b0c85d2.

The three still-open Codex threads are stale against the current head:

• createExclusiveStageDir creates the quarantine directory without recursive, suffixes timestamp collisions, and applies private permissions;
• the staging manifest is written before the first rename, then finalized before DB reconciliation;
• the memories consolidate post-image is persisted to satellite-backup.json before that transaction commits.

Required Linux/macOS/Windows, npm-global, and React Doctor checks are green. The recorded security review covers the final cleanup routes, filesystem scope, recovery sequence, and wire test-hook gate. Approved for dev.