Fix (high): sandbox launch-gate rerun bypass, crash cleanup, isolation probe seeding#798
Fix (high): sandbox launch-gate rerun bypass, crash cleanup, isolation probe seeding#798Rigidity wants to merge 1 commit into
Conversation
…tion probe Three related sandbox issues: - Launch-gate bypass on rerun (H6): effective_cap fell back to the previous baseline for any Pending/Running capability, so a user-triggered rerun (which sets all caps to Running) kept the gate open using stale, previously-Passed results. Effective state now reflects the live run status, so the gate blocks until a rerun re-establishes each result. - Crash cleanup (H6): if the runner task exits abnormally before finalizing, a Drop guard now clears current_run/running and resets the baseline to pending, so a crashed run can't leave apps launchable against a stale baseline. - Vacuous isolation probe (H5): the isolation probes read fixed keys that nothing ever wrote, so the test passed even if isolation were broken. The backend now seeds those keys in the host (main Sage) webview's localStorage and IndexedDB before launching the probes, so a sandbox app that can observe them proves an isolation failure.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 445eeed. Configure here.
| tauri::async_runtime::spawn(async move { | ||
| let apps_state = app.state::<AppsHostState>(); | ||
| *apps_state.sandbox.current_run.lock().await = None; | ||
| *apps_state.sandbox.baseline.lock().await = build_initial_sandbox_state(); |
There was a problem hiding this comment.
Cleanup clears run before baseline
High Severity
When SandboxRunCleanup runs after an abnormal runner exit, it sets current_run to None before resetting baseline. build_effective_state then returns the unchanged pre-rerun baseline (often all Passed) with no live run, so check_gates can allow user app launches even though the crashed rerun had failing or incomplete capability results.
Reviewed by Cursor Bugbot for commit 445eeed. Configure here.
| tx.oncomplete = function () {{ db.close(); }}; | ||
| }} catch (e) {{}} | ||
| }}; | ||
| }} catch (e) {{}} |
There was a problem hiding this comment.
Host seed errors swallowed silently
Medium Severity
The injected host seed script wraps localStorage and IndexedDB writes in empty catch blocks, and Rust treats a successful webview.eval as success. If those writes fail, seed_host_isolation_probe still returns Ok, so isolation probes may see no host data and the test passes even when storage isolation is broken.
Reviewed by Cursor Bugbot for commit 445eeed. Configure here.
1 participant
Severity: High (H5 + H6, same subsystem)
H6 — launch gate bypassed during a sandbox rerun (stale baseline)
effective_capreturned the previousbaselinefor anyPending/Runningcapability.begin_sandbox_runsets every capability toRunningwithout resetting a previously-Passedbaseline, so during a user-triggered rerun the effective state stayedPassedand user apps remained launchable using results that no longer applied. A runner that crashed mid-rerun could leave this "passed but dead" state indefinitely.Fix:
effective_capnow surfaces the live run status (a run is only consulted whencurrent_runisSome, so during a run the gate correctly blocks onRunning/Pendinguntil each result is re-established).SandboxRunCleanupDrop guard resetscurrent_run/runningand thebaseline(to pending) ifsandbox_runnerexits abnormally before finalizing, so a crash fails safe (gate blocks) instead of trusting a stalePassedbaseline.H5 — storage-isolation probe passed vacuously
The isolation probes read fixed keys (
sage_probe_local_storage,sage_probe_db/sage_probe_key) and passed when absent — and nothing ever wrote them, so a completely broken isolation implementation would still pass.Fix: before launching the probes, the backend seeds those keys into the host (main Sage) webview's
localStorageandIndexedDBviawebview.eval, then waits briefly for the async IndexedDB write to commit. A sandbox app that can now observe the seeded values proves an isolation failure; genuine isolation still passes.Files
crates/sage-apps/src/sandbox/state_view.rscrates/sage-apps/src/sandbox/runner.rscrates/sage-apps/src/sandbox/probes/isolation.rsNotes / follow-ups
No local build (per request); relying on CI. This gate blocks all user-app launches, so it's worth exercising the sandbox run manually before merge.
Note
High Risk
Touches the gate that blocks all user-app launches and sandbox security probes; behavior changes are intentional but should be manually verified before merge.
Overview
Hardens the sandbox app launch gate and storage-isolation checks so user apps cannot launch on stale or meaningless probe results.
Launch gate during reruns:
effective_capnow always reflects the in-flight capability status instead of falling back to a previouslyPassedbaseline while capabilities arePending/Running. Manual reruns therefore block launches until each check finishes again.Crash fail-safe:
sandbox_runnerinstalls aSandboxRunCleanupDropguard that clearscurrent_run, resetsrunning, and restores a pendingbaselineif the runner exits before finalize—avoiding a stuck “passed but no run” state.Isolation probe seeding: Before isolation probe apps run, the host Sage webview is seeded via
evalwith matchinglocalStorageandIndexedDBprobe keys (plus a short delay for IndexedDB). Probes that read those values now detect real isolation breaks; seed failure surfaces as a test error rather than a vacuous pass.Reviewed by Cursor Bugbot for commit 445eeed. Bugbot is set up for automated code reviews on this repo. Configure here.