mixboard/.orchestra/deliberations/2026-03-17-234445-Gemini-3-Pro--Preview--technical.md

Deliberation: technical — Gemini 3 Pro (Preview)

Date: 2026-03-17 23:44 Model: Gemini 3 Pro (Preview) Type: technical Artifact: ## Cloud Upload v1 — Full Deliverable ### 1. Server-side: PUT /api/upload (Common Lisp, add to serv

---

Here is the critique of the provided artifact.

Hallucination Detection

• trivial-utf-8:string-to-utf-8-bytes: [VERIFIED] A common library in the Lisp ecosystem.
• myway:*env*, myway:connect: [VERIFIED] Matches standard MyWay usage.
• gethash "content-type" headers: [VERIFIED] Clack environments standardly provide headers as a hash table.
• :raw-body: [VERIFIED] Clack standard environment key for the input stream.
• bt:with-lock-held: [VERIFIED] Bordeaux-Threads API.

Result: No hallucinations detected.

---

Architectural & Security Critique

This implementation is currently unsafe for a production or internet-facing environment (even behind Nginx), primarily due to disk exhaustion risks and inadequate input validation.

1. Security: DOS & Disk Exhaustion

Critique: The implementation relies solely on the Content-Length header for size validation.

• Problem: A malicious client can send a Content-Length of 100 but stream 50GB of data. The current loop reads until the stream ends, which will fill the disk.
• Fix: You must enforce the limit on the actual bytes read inside the I/O loop. Throw an error and delete the temp file if the byte count exceeds *max-upload-size*.

2. Data Integrity: Partial Files

Critique: You are writing directly to the destination folder with :if-exists :supersede.

• Problem: If the upload is interrupted (network failure, server crash), a corrupt/truncated music file remains in the library folder. The subsequent rescan will attempt to index a broken file, potentially crashing the scanner or polluting the database.
• Fix: Write to a temporary file (e.g., in /tmp or a .staging subdirectory). Only after the upload stream is fully successful and validated should you atomically rename-file to the final destination.

3. Concurrency: Collision Risk

Critique: (get-universal-time) has 1-second resolution.

• Problem: Two uploads occurring within the same second with the same filename will result in a race condition. Since you use :supersede, one will silently overwrite the other.
• Fix: Use a GUID/UUID or a high-precision timestamp (if available via local-time) plus a random suffix for the filename.

4. Security: Input Sanitization

Critique: sanitize-filename is insufficient.

• Problem:
  • It allows control characters (e.g., newlines).
  • It allows filenames starting with - (which can confuse CLI tools if those files are ever processed by shell scripts).
  • It does not truncate extremely long filenames (filesystem limits).
• Fix: Whitelist allowed characters (alphanumeric, ., -, _). Truncate to 255 chars. Reject null bytes explicitly.

5. Validation: Content-Type

Critique: (member content-type ... :test #'string-equal) is brittle.

• Problem: It fails on audio/mpeg; charset=binary or other parameter-appended MIME types.
• Fix: Parse the content-type string to extract the base media type before comparison.

6. Performance: Synchronous Rescan

Critique: Calling rescan inside the request handler while holding *rescan-lock*.

• Problem: If scanning is slow (calculating durations, hashing, cover art extraction), the HTTP request will hang. While acceptable for a single-user app roughly, it blocks the thread.
• Fix: Offload the rescan to a worker queue or a separate thread, or simply return "Upload accepted" and let the scan happen asynchronously. At the very least, acknowledge the potential timeout risk.

7. Code Correctness: HTTP Status Codes

Critique: Sending 200 for "Upload not configured".

• Problem: This is a server configuration error.
• Fix: Return 503 Service Unavailable or 500 Internal Server Error.

Actionable Feedback for the AI Agent

1. Implement Byte Counting: Modify the loop to count bytes read. If current-bytes > *max-upload-size*, stop reading, close stream, delete parts, and return 413.
2. Atomic Write Pattern:
   • with-open-file to a temporary path (e.g., (uiop:with-temporary-file ...) or a manual random path).
   • On success: (rename-file temp-path final-dest).
   • On error: (delete-file temp-path).
3. Refine Sanitization:
   • Replace the filter with an allowlist approach.
   • Ensure the filename doesn't start with a dot or dash.
4. Fix Content-Type Matching: Use a utility to strip ; parameter=value before checking against +allowed-upload-types+.
5. Robust Path Handling: When using merge-pathnames, ensure *upload-dir* is treated as a directory pathname (ends in /), otherwise standard CL merge-pathnames behavior might merge it into the filename of the parent directory. Use uiop:ensure-directory-pathname.

Session State

**Blocking findings**: Safety (Disk exhaustion vulnerability), Irreversibility (Interrupted uploads corrupt library). **Advisory findings**: Filename sanitization improvements, async processing for rescan, strict content-type parsing. **Decisions**: Must use atomic rename pattern and enforce streaming read limit. **Open Questions**: None. **Key Entities**: `server.lisp`, `sanitize-filename`, `upload-file`.