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>
4.5 KiB
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
-
[Severity 4 — scope expansion]
git diff origin/main -- . ':!.convoys/.metrics.jsonl'contains 15 tracked paths outside Briefs 1–6, including dashboard/nav work,components/Layout.js,pages/dashboard.js,pages/my-cards.js, andpages/api/user-cards.js; it also deletes the separatedashboard-home-realignmentconvoy artifacts. This violates every brief's explicit file scope and prevents a trustworthy scanner-only review. Fix: rebase/cherry-pick the scanner implementation ontoorigin/main(or split the unrelated dashboard/API changes into their own convoy) and re-run the audit. Note that the suppliedgit diffdoes not show untracked scanner files, so this audit additionally inspected their working-tree contents. -
[Severity 1 — file intake hardening]
pages/scanner.js:269-300accepts an arbitrary number of arbitrary-sizedimage/*files, andlib/use-scanner-identification.js:20-42decodes 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: beforerunSequentialGalleryIdentify, enforce a maximum file count, MIME allowlist, and conservative byte limit compatible with the 6 MB base64 server limit; reject failures per file before callingreadFileToImageData. -
[Severity 2 — inherited production dependency advisories]
npm audit --omit=dev --jsonreports five high-severity production vulnerabilities. The most material is the directnextrange<16.2.11; the audit also identifies transitivejws,nanoid,postcss, andsharp. 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-101independently 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); nodangerouslySetInnerHTML,eval, or shell construction was found. The file-bound finding above remains. - Layer 4:
lib/use-camera-scanner.js:37-90stores only opaque camera device IDs insessionStorage, 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.