feat(files): upload and safely extract ZIP archives (#6782)

* feat(files): support zip extraction

* fix(files): harden zip extraction safety

* fix(files): batch extraction notifications

* fix(files): defer rollback storage cleanup

* fix(files): restore reliable drag uploads

* fix(files): use explicit archive extraction route

* fix(ci): account for archive extraction route

* refactor(files): bound every archive extraction and trim the extractor's option surface

`maxMaterializedItems` was opt-in, so only the new unzip route bounded its output
tree — the copilot `materialize_file` and `POST /api/tools/file/manage` extract
paths had no cap on folder creation at all. An archive within MAX_ARCHIVE_ENTRIES
can still imply far more folders than files, so the cap now defaults to
MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS and applies to all three callers.

`materializedRootFolderCount` was a hand-maintained number that had to agree with
what an opaque callback would create, and the callee could not check it; drift
surfaced only as an over-limit archive slipping past the cap. It is now derived
from whether `prepareRootFolder` ran, so the contract is just "the callback
creates exactly one folder".

Also single-sources the ArchiveError -> HTTP status map (it was copied into both
the internal error policy and the tools route), drops IdempotencyService config
that only `executeWithIdempotency` reads (the extraction lease uses
atomicallyClaim/release, so no result is ever stored), hoists the duplicated
predicates in purgeCreatedWorkspaceFile and archiveWorkspaceFileFolderIfEmpty so
a lock and its write cannot diverge, and names UPLOAD_SESSION_LOCAL_PUT_MAX_BYTES
rather than overloading the multipart part size as the local single-PUT ceiling.

Adds coverage for the two guards nothing exercised: the re-validation of the
segments `prepareRootFolder` actually returned, and the default cap applying with
no caller opt-in.

UI: the drop overlay used --surface-4 unconditionally, which renders grey over the
light-mode canvas; matches the canonical overlay's --white/dark:--surface-4 and
swaps arbitrary px type sizes for named tokens.

* fix(files): bound the extraction write loop so it cannot outlive its lease

Cursor Bugbot flagged two related holes, both rooted in the write loop being
unbounded:

1. `maxDuration` is a Next.js route-segment config that serverless platforms
   enforce and self-hosted deployments do not. A slow extraction (up to 1000
   sequential uploads) could therefore outrun the six-minute lease, and
   `IdempotencyService` reclaims an expired in-progress claim — so a second unzip
   of the same archive could start beside the first.
2. Nothing rolls back a process killed mid-pass-2, so a timeout stranded the
   destination folder and every file written so far.

`decompressArchiveBufferToWorkspaceFiles` now takes an `AbortSignal` and checks it
between entries in both passes, and the extraction use case supplies a 180s
deadline. The abort unwinds through the existing all-or-nothing rollback, so the
work stops on our terms with the tree cleaned up, well inside both the route's
300s budget and the 360s lease. That closes (1) outright — the holder can no
longer outlive its lease on any platform — and converts (2) from a stranded
partial tree into a clean rollback for the slow case that actually triggers it. A
SIGKILL still cannot be caught; that needs a durable job and is out of scope here.

The overrun surfaces as a caller-fixable 413 naming the archive rather than an
opaque 500 from the raw DOMException.

* fix(files): only remap the deadline abort itself, and stop overclaiming rollback

Two follow-ups on the budget deadline, both reported by Cursor Bugbot:

`deadline.aborted` stays true for the rest of the request once the timer fires, so
it cannot decide whether *this* error was the abort. An `ArchiveError` or storage
failure thrown mid-entry after the timer fired was being relabelled as a timeout
and returned as a 413, hiding the real cause. The catch now matches the thrown
value against `deadline.reason` — `throwIfAborted()` throws exactly that object,
so the check is identity-exact and cannot capture an unrelated failure.

The message also claimed a rollback that has not necessarily happened: the budget
covers the archive download too, so it can fire before the first write, when there
is nothing to roll back. It now says the unzip was cancelled and claims nothing
about what was written. Including the download in the budget is deliberate — the
lease it has to fit inside starts earlier still — so the TSDoc says that rather
than "the extraction itself".

---------

Co-authored-by: Waleed Latif <walif6@gmail.com>
This commit is contained in:
Justin Blumencranz
2026-08-17 17:06:57 -07:00
committed by GitHub
co-authored by Waleed Latif
parent fe4480d3f7
commit 0c34e69fdc
26 changed files with 1951 additions and 112 deletions
+2 -2
View File
@@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries')
const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors')
const BASELINE = {
totalRoutes: 1119,
zodRoutes: 1119,
totalRoutes: 1120,
zodRoutes: 1120,
nonZodRoutes: 0,
} as const