Spaces:

build-small-hackathon
/

Snap2Sim

Running

The Space is public now. Findings that were only mitigated by private visibility were treated as blockers and were resolved before publication.
Screen-only viewing; no VR/AR goal. WebXR is not required, which removes A-Frame's main structural advantage.

File references use path:line from the state of the repo at review time.

1. Endpoint Security

Architecture summary

app.py is the public Hugging Face Space (gradio.Server). It serves index.html at / and exposes POST /analyze_image and POST /generate_scene. The browser calls these same-origin, and the server attaches the Modal bearer token when it forwards to Modal. This split (token stays server-side) is correct and matches SECURITY.md.
modal_app.py exposes the GPU/inference web endpoints, each guarded by a bearer token via require_authorization.

The Modal layer is in good shape. The Space layer is the exposure.

HIGH — The Space's own `/analyze_image` and `/generate_scene` are unauthenticated (`app.py:64`, `app.py:74`)

These endpoints have no auth of their own. At review time, they were gated only by the Space being private. Because the Space is now public, anyone on the internet can POST to them unless the implemented abuse controls remain active.

With INFERENCE_BACKEND=local (current default), they return placeholder data — low impact but a free compute/bandwidth sink (base64 decode + PIL decode + scene build per request).
With INFERENCE_BACKEND=modal, every public request is proxied to Modal using the server's own bearer token, turning the Space into an open proxy that spends GPU credits on behalf of any anonymous caller. The Modal bearer token protects Modal from direct callers, but the Space is a trusted caller, so the token does not help here. This directly contradicts the stated intent in SECURITY.md that the token is "the real protection against credit-spending spam."

Recommendation (implemented before publication):

Add abuse controls at the Space layer that do not require leaking a secret to the browser. Options, roughly in order of preference:
1. Server-side rate limiting / quota per client IP and a global ceiling (e.g. slowapi/limits, or a small in-process token bucket). This is the most important mitigation since the Space is intentionally token-free on the client side.
2. A short-lived, server-issued session/CSRF-style token handed to the page on GET / and required on the POST endpoints, to stop trivial scripted abuse (not a strong control, but raises the bar).
3. Hard caps: max upload size, max requests/min, and a circuit breaker that falls back to local placeholder mode if a Modal spend threshold is hit.
Consider HF Space-level protections too (HF supports gating), but do not rely on them as the only layer once public.

HIGH — Untrusted scene HTML is injected via `innerHTML` (`index.html:545`)

renderAframe() does viewport.innerHTML = sceneHtml; where sceneHtml comes from /generate_scene. In the deterministic path the HTML is built server-side with html.escape (snap2sim/aframe_scene.py:14-16, safe). But the model-generation path (generate_scene_llamacpp in modal_app.py:462) returns raw model output, extracted by parse_scene_response (snap2sim/model_io.py:49) which does no sanitization — it slices everything between <a-scene> and </a-scene> verbatim.

Because the scene is derived from a user-uploaded image, an attacker can prompt-inject the vision/scene model (text in the photo, adversarial content) into emitting markup such as <a-entity onloaded="..."> or <img src=x onerror=...>. innerHTML will not execute injected <script> tags, but it does run inline event-handler attributes and A-Frame can execute component JS — so this is a realistic stored/reflected XSS vector that becomes internet-reachable when the Space is public.

The validation in validate_analysis protects the JSON analysis path but does nothing for the free-form HTML scene path.

Recommendation: Do not inject free-form model HTML into the DOM. See §3 — the cleanest fix is to make scene rendering deterministic from the validated JSON (which is already escaped/validated) and stop returning model-authored HTML. If any model-authored HTML is ever rendered, sanitize it with an allowlist (e.g. DOMPurify with an A-Frame-aware tag/attribute allowlist) before innerHTML, and strip all on* attributes.

MEDIUM — No upload size limit or decompression-bomb hardening (`app.py:88`, `modal_app.py:327`)

_decode_image and write_payload_image do base64.b64decode(...) then Image.open(BytesIO(...)) with no cap on the base64 string length or decoded pixel count. A large or crafted image (decompression bomb) can exhaust memory on cpu-basic. PIL's default DecompressionBombWarning only warns; it does not block at typical sizes.

Recommendation:

Reject requests whose base64 body exceeds a sane limit (e.g. ~8–12 MB) before decoding.
Set Image.MAX_IMAGE_PIXELS to an explicit ceiling and catch Image.DecompressionBombError.
Wrap decode failures and return a clean 400 instead of a 500/stack trace.

MEDIUM — Fixed temp file paths cause cross-request collisions (`modal_app.py:199`, `modal_app.py:342`)

Both the smoke test and write_payload_image write to fixed paths (/tmp/snap2sim-request-image.jpg). Concurrent requests on the same container overwrite each other's input image, so one user could be analyzed against another user's photo. This is both a correctness and a privacy concern.

Recommendation: Use tempfile.NamedTemporaryFile/mkstemp (unique per request) and clean up in a finally.

LOW — Third-party scripts loaded without Subresource Integrity (`index.html:7-13`)

A-Frame (aframe.io), the Gradio client (jsdelivr), and fonts (bunny.net) are loaded from CDNs with no integrity/SRI hashes. A CDN compromise would run arbitrary JS in the (soon public) page.

Recommendation: Pin versions and add SRI hashes where the CDN supports it. Note: if A-Frame is dropped (see §3) and Three.js is bundled/self-hosted, this surface shrinks substantially.

LOW — Unused third-party dependency loaded (`index.html:10-13`)

The Gradio JS client is imported into window.snap2simGradioClient but never used; all calls go through a plain fetch (postJson, index.html:765). Remove the import to cut an unnecessary CDN dependency and load.

Positives (keep these)

Modal bearer check uses constant-time comparison (token_secrets.compare_digest, modal_app.py:113) and returns 503 when the token is unconfigured, 401 on mismatch (modal_app.py:105-115). Unauthenticated 401 was verified per AGENTS.md.
Token never reaches the browser; server-side attaches it (snap2sim/backend.py:62-68). Matches SECURITY.md guidance.
The deterministic scene builders escape text with html.escape (snap2sim/aframe_scene.py:14-16) and the analysis readout uses textContent rather than innerHTML (index.html:513-536) — no XSS on that path.
GitHub→HF workflow uses least-privilege permissions: contents: read and keeps HF_TOKEN in secrets (.github/workflows/sync_to_hf.yml:19-21).
Schema validation (snap2sim/schema.py:176) runs before scene generation.

2. UI/UX

The visual direction is strong and cohesive: a "technical cutaway / field manual" aesthetic (amber #E8A33D + cyan #5FD4D0 on dark, blueprint grid, Chakra Petch / Fira Code). The two-pane layout (viewport + readout) is clear and the loading choreography (progress bar, scan-line reveal, staggered mesh reveal, cold-start "WAKING THE WORKSHOP" message after 6.5 s) is a genuinely nice touch for HF cold starts. Drag-and-drop plus a Load button, and a play/pause control, cover the core interactions. The fallback chain (A-Frame → deterministic Three.js → "3D runtime unavailable") is thoughtful.

Issues, roughly by impact

Accessibility (MEDIUM — matters more once public):

Status messages (index.html:392) are not announced to screen readers. Add aria-live="polite" (and role="status") so analysis progress/errors are read out.
#viewport injected 3D content has no text alternative. Provide an aria-label/visually-hidden description, or rely on the readout panel as the accessible representation and mark the viewport aria-hidden.
Low-contrast text: --text-muted #6B7280 on the dark panel is below WCAG AA for body text (trigger, raw JSON, drop-zone hint). Nudge it lighter.
The file input is triggered via a <label> drop zone (index.html:382); make sure keyboard focus + Enter works to open the picker, and the Load button is already focusable (good).

Functional gaps:

No preview of the uploaded photo. Users lose their reference image once the cutaway renders. Show a thumbnail of the source photo in the panel — it also reinforces the "this came from my object" story for a demo.
playButton initial state is confusing (index.html:379): it reads "Pause" while disabled, before any scene exists. Start as disabled "Play" or hide until a scene is ready.
Error UX is thin: errors render as uppercased status text only (index.html:495). Add a visible retry affordance and keep the drop zone reachable so the user can try another photo without hunting for the Load button.
No client-side file validation: accept="image/*" only. Reject non-images and oversized files before base64 inflation (~33% size increase) and surface a friendly message. (Pairs with the server-side size cap in §1.)
Mobile: the layout has a breakpoint at 860 px (good), but A-Frame look-controls can capture touch/scroll. If Three.js becomes the primary renderer (see §3), use OrbitControls tuned for touch and ensure the page still scrolls. Also consider capture="environment" to offer "take a photo" on mobile.
No confidence visualization: confidence is shown as a number (index.html:516); a small bar/gauge would read faster and fit the instrument aesthetic.
First-run onboarding is just "Drop component photo." A one-line example ("try a lock, a gear, a ratchet…") lowers the cold-start barrier for judges.

Verdict: UI/UX is above-average for a hackathon and the aesthetic is a real asset. The gaps are accessibility, an uploaded-image preview, and error/retry polish — none are large, and they raise the demo from "looks great" to "feels finished."

3. Three.js vs A-Frame for Rendering

Recommendation: consolidate on Three.js as the primary (and only) renderer, driven deterministically from the validated JSON analysis. Drop the A-Frame dependency and stop returning model-authored scene HTML.

Reasoning

No VR/AR goal (confirmed). A-Frame's defining advantage is declarative WebXR. Screen-only viewing removes the main reason to carry it. Without VR, A-Frame is mostly a ~1.2 MB declarative wrapper over Three.js.
The Three.js path is already the more capable implementation. The "fallback" in index.html:565-755 has OrbitControls, DOM-projected part labels, staggered reveal, gear extrusion, fog/lighting, and motion handling. It is richer than the A-Frame output (snap2sim/aframe_scene.py). You'd be promoting your best renderer, not rewriting one.
True "cutaway" cross-sections need clipping planes. The product is an annotated technical cutaway. Three.js exposes renderer.localClippingEnabled + material.clippingPlanes natively, which is the clean way to slice a housing and reveal internals. A-Frame has no first-class clipping-plane support. If "cutaway" is to be more than parts floating in space, Three.js is required anyway.
It removes the §1 HIGH XSS vector. Rendering deterministically from validated JSON means no innerHTML of untrusted model HTML. The model already produces the structured JSON (snap2sim/schema.py), which is validated and safe; let a deterministic Three.js builder turn that JSON into geometry. This is both safer and more reliable than asking an LLM to emit well-formed scene markup.
Smaller, self-hostable footprint. Three.js can be pinned/bundled and served same-origin, shrinking the CDN/SRI surface from §1.

What this means concretely for implementation

Keep the model's job as the JSON analysis contract (it already is, snap2sim/schema.py). The model should not author HTML/JS scenes.
Make generate_scene deterministic from that JSON. The browser already has buildDeterministicScene() — promote it from fallback to the main path. You can drop the server returning scene HTML entirely, or keep the server returning the validated analysis and let the browser build the scene.
Remove A-Frame (index.html:9, the <a-scene> injection in renderAframe, and the A-Frame branch of generate_scene_llamacpp) once the Three.js path is the default. Delete snap2sim/aframe_scene.py and the A-Frame prompt template after migration, or keep them only if you want a server-rendered static preview.
If you keep any model-authored HTML path, it must be sanitized (DOMPurify, strip on*) per §1.

This also simplifies the mental model: one renderer, one safe data contract, no dual A-Frame/Three.js maintenance.

Suggested Implementation Order (for Codex)

Blockers resolved before the Space went public:

Add server-side rate limiting / quota + max upload size to app.py endpoints (§1 HIGH).
Eliminate the untrusted-HTML innerHTML path: move to deterministic Three.js rendering from validated JSON, or sanitize (§1 HIGH + §3).
Add decompression-bomb guards and unique temp files (§1 MEDIUM).

Then:

Consolidate rendering on Three.js; remove A-Frame and the dead Gradio client import; add SRI/self-hosting (§3, §1 LOW).
UI/UX polish: uploaded-image preview, aria-live status, contrast fix, error/retry affordance, play-button initial state (§2).

Out of scope / leave as-is: Modal bearer auth (already correct), the GitHub→HF sync workflow, the schema contract.

Security Hardening Review