deckhearth/.convoys/scanner-desktop-layout/audits/security-20260815.md
varutasu 938c161a26
feat(scanner): add desktop workstation layout (#165)
Give /scanner a md+ camera, live match inspector, and history strip
(with device picker, batch scan, and tips) without regressing the
mobile immersive checkout.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-15 17:21:23 -05:00

4.5 KiB
Raw Permalink Blame History

Security Audit

Check Status Notes
Auth boundary No new API route or auth flow. /scanner keeps its useAuth redirect, and the unchanged scan endpoint verifies the Bearer token before its per-user scan rate limit.
Authorization (IDOR) The desktop list picker uses the existing collection-card API; its server-side POST path confirms authenticated owner/editor permission before mutation. Client-held collectionId is not trusted as authorization.
Input / injection ⚠️ File names and server error text are rendered as React text (no HTML sink), but new batch intake has no client-side file-size or count bound before decoding images into a canvas.
Secrets exposure Device IDs are stored only in tab-scoped sessionStorage; no new secret, NEXT_PUBLIC_* value, token, or credential was found in the scanner additions.
Dependencies ⚠️ No dependency manifest change is in the supplied diff, but npm audit --omit=dev --json reports five pre-existing production high-severity advisories (including next, jws, nanoid, postcss, and sharp).

Findings

  1. [Severity 4 — scope expansion] git diff origin/main -- . ':!.convoys/.metrics.jsonl' contains 15 tracked paths outside Briefs 16, including dashboard/nav work, components/Layout.js, pages/dashboard.js, pages/my-cards.js, and pages/api/user-cards.js; it also deletes the separate dashboard-home-realignment convoy artifacts. This violates every brief's explicit file scope and prevents a trustworthy scanner-only review. Fix: rebase/cherry-pick the scanner implementation onto origin/main (or split the unrelated dashboard/API changes into their own convoy) and re-run the audit. Note that the supplied git diff does not show untracked scanner files, so this audit additionally inspected their working-tree contents.

  2. [Severity 1 — file intake hardening] pages/scanner.js:269-300 accepts an arbitrary number of arbitrary-sized image/* files, and lib/use-scanner-identification.js:20-42 decodes each selected image at full natural dimensions into a canvas before the existing server-side 6 MB request cap can apply. A logged-in user can select a very large or decompression-bomb image set, freezing their tab and creating repeated scan attempts. accept="image/*" is only a picker hint, not validation. Fix: before runSequentialGalleryIdentify, enforce a maximum file count, MIME allowlist, and conservative byte limit compatible with the 6 MB base64 server limit; reject failures per file before calling readFileToImageData.

  3. [Severity 2 — inherited production dependency advisories] npm audit --omit=dev --json reports five high-severity production vulnerabilities. The most material is the direct next range <16.2.11; the audit also identifies transitive jws, nanoid, postcss, and sharp. No package manifest changed in this convoy, so this is not introduced by the scanner work, but it remains a release risk. Fix: open or link the dependency-upgrade/accepted-risk work before release, and verify the deployed Next.js version against the advisories.

Layer notes

  • Layer 1: Existing scanner endpoints retain authentication and the scan rate-limit gate; this convoy adds no server action or API route.
  • Layer 2: The list picker is UI convenience only. pages/api/collections/[identifier]/cards.js:90-101 independently requires an authenticated owner/editor for POST, preventing a forged picker ID from becoming an IDOR.
  • Layer 3: React escapes the batch-derived file basename (pages/scanner.js:226-235) and failure strings (components/scanner/ScannerHistoryStrip.js:91-94); no dangerouslySetInnerHTML, eval, or shell construction was found. The file-bound finding above remains.
  • Layer 4: lib/use-camera-scanner.js:37-90 stores only opaque camera device IDs in sessionStorage, using a fixed key and guarded reads/writes. No scanner secret is exposed to the client.
  • Layer 5: See dependency finding 3. The full audit reports 16 issues including dev dependencies; the production-only audit reports five high findings.
  • Layer 6: No middleware, Next config, CORS, cookie, or header change was included.

Recommendation

Do not merge as currently scoped. Resolve the severity-4 scope expansion first. After isolating the scanner-only diff, add the file size/count/type guard (or document an accepted risk) and re-audit; the existing dependency findings should be tracked or risk-accepted separately because they were not introduced here.