feat: add an optional simulator-only browser preview - #73
Conversation
rohanpandula
left a comment
There was a problem hiding this comment.
Adversarial review — PR #73 (simulator-only web runtime)
Independent adversarial pass over the full diff: Python gateway (ports/web), React frontend (ports/tauri/app), Swift runtime distribution/verification (ScanStudioKit), GitHub Actions release/CI workflows, macOS packaging scripts, Docker, and the new docs/AGENTS.md. Line-by-line on all security-critical modules; claims in docs/WEB-RUNTIME-DISTRIBUTION.md re-verified against code.
Verdict: no CRITICAL or HIGH findings. The security architecture is unusually disciplined for a first cut — fail-closed seams, authenticate-before-interpret on the manifest, server-authoritative lease, and a genuinely secret-isolated signing job. Findings below are MEDIUM and down.
MEDIUM
M1. Actions pinned by major tag — including inside the secret-bearing job. release.yml:50,60-61,155,183,188,193,263,268,456,471-477,504-523 (checkout@v4, download-artifact@v4, upload-artifact@v4, setup-node@v4, setup-uv@v5, and dtolnay/rust-toolchain@stable, which is a moving branch). In web-runtime — the job holding all 8 signing credentials — a tag-move on checkout@v4 or download-artifact@v4 fully defeats the isolation boundary, since action code runs with job access before the secrets step. Given the extraordinary care everywhere else in this PR, SHA-pinning (at minimum the three actions used in web-runtime, and replacing @stable) is the consistent fix. Same applies to adversarial-review.yml:20,26 and ci.yml.
M2. trusted-lan-no-login + "Local network" binds 0.0.0.0 — exposed beyond the LAN on multi-homed hosts. WebServerPreferences.swift:105 resolves the Local-network scope to bindAddress = "0.0.0.0" whenever any private IPv4 exists; the gateway whitelists this (settings.py:81-82). On a machine with a private interface and a public one (hotel Wi-Fi + tethering, public Ethernet + LAN), the unauthenticated gateway is reachable from the public side. The UI warning covers NAT/forwarding/proxies but not the multi-homed case. Impact is bounded (simulator-only, Origin-checked), but binding the specific private interface address instead of 0.0.0.0 would close it cleanly.
M3. Unbounded WS hydration buffer + no timeout on hydration requests. engine/client.ts:184,240 — while hydrating === true, every incoming WS message is pushed to pendingEvents with no cap; hydration needs two webRequest fetches (:199-204) that carry no AbortSignal/timeout (webRequest, :56-63). If the gateway accepts the socket but the request endpoint stalls (hung engine, partial outage, or a hostile gateway in LAN mode), the socket stays open, hydrating stays true, and the buffer grows without bound → tab memory exhaustion. Cap the buffer and/or time out hydration in listenToWebEvents.
M4. Signing runner installs Homebrew software before the secrets step. release.yml:275 (brew install openssl@3 in web-runtime). A compromised formula/bottle is code execution on the signing VM before credentials materialize. The "never executes payload" claim holds — Homebrew is simply part of the signing job's trusted computing base. Either acknowledge it in the threat model or replace with a digest-pinned static openssl.
M5. Native parsers consume attacker-influenced bytes while secrets are live (residual, needs-verification). The unsigned DMG comes from a job that ran npm ci/uv sync --locked, so a dependency compromise can craft arbitrary bytes. Inside the secrets step, codesign signs/verifies every payload Mach-O (sign-runtime.sh:99-118), hdiutil create/attach runs (sign-runtime.sh:120-136), plus stapler/spctl (release.yml:441-446). A memory-corruption bug in any Apple tool on crafted Mach-O/UDIF input → secret theft. Cannot be fully eliminated without a physical air-gap; notarization (Apple scans the payload) partially offsets. State this explicitly in WEB-RUNTIME-DISTRIBUTION.md as accepted residual risk.
M6. 60-second resource timeout on up-to-1 GiB DMG downloads. WebRuntimeHTTPClient.swift:10,33-34 sets timeoutIntervalForResource (default 60 s) for the entire transfer; the same client fetches the DMG (WebRuntimeDistribution.swift:667-671). A 100–300 MB runtime needs sustained >1.7–5 MB/s or install fails. Fail-closed and retryable, but a real defect on slow links.
LOW
- L1. Compose default publishes the gateway on all host interfaces.
ports/web/compose.yaml:17—"${SCANSTUDIO_WEB_PORT:-8787}:8787"binds0.0.0.0on the host. Token mode fails closed on missing token and origins are enforced, but the safe default is127.0.0.1:8787:8787(CI's smoke test already does loopback-only,ci.yml:202). - L2. Readiness probe accepts any HTTP 200; local port pre-bind can phish the token.
WebServerModel.swift:971-974probes/startupzviaURLSession.sharedand accepts any 200. Any local user can pre-bind the configured port (>1024 needs no privilege), serve a fake login UI, and capture the pasted token. Same-user blast radius, simulator-only; design-inherent, noting for awareness. - L3. Orphaned gateway if the app dies without cleanup. The gateway
setsid()s (cli.py:15-44) with no parent-death watchdog. Graceful quit is well covered (ScanStudioApp.swift:236-264,WebServerModel.swift:430-467group sweep with one-shot token), but a SIGKILLed/crashed app leaves the gateway running — in trusted-LAN mode an unauthenticated endpoint persists until logout/reboot. - L4. Dangling server lease after aborted/unreadable claim.
WebRuntimeGate.tsx:365-377,404-418— if the claim POST times out after the server granted, the client never learns the token and can't release; other tabs block until TTL (30 s default, up to 300 s). UX-only. - L5. Residual dual-controller belief window under background-tab timer throttling.
WebRuntimeGate.tsx:567-683— hidden-tab timer throttling can let a hidden controller's heartbeat/watchdog lag server expiry; another tab can legitimately claim meanwhile. Server generation checks (controller_lease.py:105-134) reject stale mutations with 423 and the client fails closed, so this is a documented residual window, not a bug. - L6.
engineState()web path reads fields/healthznever returns.client.ts:288-297—/healthzdeliberately returns only{status}(app.py:309-320), so the web branch always yields{running:false, pid:null}. Currently dead code; a future consumer will silently see "engine not running". Contract mismatch, no exposure. - L7. Observer can edit local transform drafts.
ContactSheet.tsx:100-127— rotate/flip gated bytransformsEditablebut notcanControl. Local-draft only (no wire call reachable by observers in web mode), but silently-inert edits can mislead an operator. - L8. No root
.dockerignore; only BuildKit-specificDockerfile.dockerignore. Compose build context is the repo root (compose.yaml:4); classic-builder builds send the whole repo to the daemon. Nothing sensitive isCOPYed (all paths explicit), so context-leak only. - L9. Token passed via environment.
WebServerModel.swift:904— visible viaps ewwto the same user. Mitigated downstream: the engine child's env strips allSCANSTUDIO_WEB_*(engine_process.py:114-119). - L10. jq interpolation of the tag name.
release.yml:832— git tags may contain", so a hostile tag is technically a jq injection (envbuiltin could readGH_TOKEN). Exploitable only by tag-pushers (repo write), negligible;--argwould be cleaner. - L11. Bare
python3(no-I -S) in the secrets step.emit-integrity.sh:64,payload-tree-hash.sh:18— other invocations correctly usepython3 -I -S. Hygiene-only; PYTHONPATH unset, runner python trusted.
INFO
- Same-origin synthetic events can demote a controller or fake stream state (
client.ts:19-21,125-131) — requires same-origin script execution, at which point CSP has already failed; self-DoS only. - Runtime detection via
"__TAURI_INTERNALS__" in window(runtime.ts:1-3) — spoofable only from an already-compromised context. - No same-version manifest rollback protection beyond tag pinning (no monotonic counter); cross-version rollback is prevented by host-version/tag/arch pinning (
WebRuntimeDistribution.swift:407-431). Key-compromise procedure documented. Residual, accepted. - Launch-time
notarized := developerIDSigned(WebRuntimeMacOSVerification.swift:553) is sound only because the launch-time tree hash pins byte-equality with the stapler-validated DMG payload — worth a comment so a future edit doesn't break the transitivity. assert_no_web_runtime.shis name-based — a renamed payload evades it, but would require a maliciouspackage_app.shvisible in review; enforced at three points (package_app.sh:445,package_dmg.sh:20,75,release.yml:152).- Session-expiry UI detection latency up to 60 s after cookie death (
client.ts:248) — mutations fail server-side in the window; stale-UI only. autoComplete="current-password"on the token field (WebRuntimeGate.tsx:745) — password managers may offer to store it.
Checked and CLEAN
Gateway (ports/web): origin enforcement on all state-changing paths + WS upgrade; CSRF closed (custom lease header forces preflight; HttpOnly/SameSite=Strict cookie; constant-time token compare); trusted-LAN mode checks the real socket peer and rejects Forwarded/X-Forwarded-For/X-Real-IP outright; simulator allowlist is 6 methods with scanner.connect pinned to sim-ls5000-0; reserved methods rejected; NaN/Infinity rejected before the NDJSON wire; engine env strips all SCANSTUDIO_WEB_* plus bridge/hw-motion vars; request bodies bounded even with dishonest Content-Length; engine line length bounded; abandoned-request tombstones prevent response poisoning; single worker enforced; proxy_headers=False; validation errors never echo input.
Swift verification chain: Ed25519 key + Team ID stamped into the signed app bundle, never fetched; signature verified over exact bytes before any field is interpreted; asset URL must equal the internally derived release URL (malicious manifest can't redirect); strict duplicate-rejecting JSON; SHA-256 + size verified before mount; read-only mount with exact one-bundle layout; symlinks and hardlinks rejected; verify-before-copy and verify-after-copy; per-launch re-hash + codesign + spctl; cache tree 0700/uid-checked (no cross-user cache poisoning); no shell anywhere (fixed absolute /usr/bin tools); redirect validation limited to github.com + two pinned CDN hosts, 2-hop cap, no retries to downgrade; consent gate cannot be skipped (resolver miss → signed-metadata fetch → explicit dialog → single-consumption offer); hand-edited UserDefaults fail closed through the resolver. Every load-bearing claim in WEB-RUNTIME-DISTRIBUTION.md checked out.
Frontend: no dangerouslySetInnerHTML/innerHTML/eval/postMessage anywhere; token lives only in React state, cleared on success, never persisted/logged/in-URL; lease machinery fails closed on every uncertainty path (stalled heartbeat, late response, malformed grant, copied-tab token) — demotes to observer, never grants; stale-response vectors all have generation/token guards; canControl is the triple gate owned && controlVerified && eventStream.ready; native builds cannot enter web codepaths (build-time mode gate); Tauri IPC unreachable in web builds; all mutating fetches same-origin relative.
CI: the two central claims hold — (1) assembly runs with zero secrets (structurally enforced by tests/test-release-boundary.sh:58-79), (2) signing/notarization runs on a fresh runner that checks out source only, downloads the pre-built artifact, and never executes payload code (only interpreter execution of payload bytes anywhere is package-runtime.sh:633 in the secret-less assemble job). No signing oracle: the manifest is built inside the signing job from bytes it just signed, and the untrusted job's receipt is independently re-verified before signing. No ${{ }} script injection (all interpolations are hardcoded matrix values or SHAs); the pull_request_target adversarial-review workflow is the correct safe pattern (base-owned validator, PR tree as data-only checkout, contents: read, zero secrets, hardened git plumbing); minimal permissions; no id-token; artifact names fixed per run; ephemeral keychain with credential wipe that fails the job on cleanup failure; all four Docker base images digest-pinned, non-root user, dropped caps, read-only rootfs; uv.lock committed with sha256 pins, zero VCS sources, dev/test deps banned from the shipped closure and re-checked in the signed bundle; canonical-repo assertion guards runtime jobs against fork misuse. AGENTS.md/ADVERSARIAL-REVIEW.md tighten rather than weaken the review gate; no prompt-injection surface.
Process note
The PR body states the exhaustive terminal adversarial-review cycle was stopped early by direction and this PR does not carry the repository's full terminal certification. This independent pass covers the same surface and found no privilege-class issues, but the MEDIUM items above (especially M1 action pinning and M2 multi-homed bind) are worth addressing before this mode ships broadly.
Summary
This adds an optional, off-by-default browser preview for ScanStudio while preserving the existing React interface and Rust engine protocol.
The macOS app now exposes a Browser preview toggle, bind/interface selection, port configuration, and authentication mode. On first enable, ScanStudio shows the exact GitHub-hosted runtime and asks before downloading it. The browser runtime is not bundled in normal app releases.
This milestone is deliberately simulator-only: it cannot access a scanner, move film, capture images, or write scan projects.
Screenshots
Token-protected sign in
Controller workspace
One browser holds the renewable controller lease. Here it is previewing a simulated six-frame strip with frames 2–4 selected.
Read-only observer
Additional browsers can watch the same simulator session without receiving mutation authority.
What it does
How to use it
From the macOS app
Turning the toggle off stops the gateway. Network settings remain editable while it is stopped.
The no-login option is restricted to directly connected trusted-LAN clients. Do not use it with WAN port forwarding, source NAT, or a reverse proxy. For remote access, retain token authentication and use a trusted HTTPS proxy or private VPN.
With Docker
See ports/web/README.md for the full configuration and network boundary.
How it could be used
It is not a remote film-scanning service. Real capture, USB access, project storage, bridge configuration, and unattended scanner operation remain out of scope.
Architecture and security
scanstudio-enginechild over NDJSON pipes.HttpOnly; SameSite=Strictsession cookie.See docs/WEB-RUNTIME-DISTRIBUTION.md and docs/WEB-HEADLESS.md for the detailed contracts.
Validation
Completed validation included:
No live scanner operation, release publication, notarized production release, or app installation was performed.
Review status
A partial authenticated OpenCode/DeepSeek adversarial review was completed. Its confirmed release-security finding—signing credentials being present while dependency-bearing code ran—was fixed by separating no-secret assembly from signing/notarization on a fresh runner.
At the requester's direction, the remaining exhaustive terminal DeepSeek shard cycle and broad retesting were stopped. This PR is implemented and focused-tested, but it does not carry the repository's full terminal adversarial-review certification.