[buluoray]

Problem / Motivation

Goal: Download a file of any size from the dashboard, with a clear warning before a file too large to scan.

Download (file tree right-click, the viewer's Download, the binary card) refused any file over 50 MB with "Download failed". GET /api/file-download read the whole file into memory for its credential scans and answered 413 above the upload cap, so a screen recording, a dataset or a build artifact could not be retrieved from the dashboard at all.

Why it matters

The owner's own large files on the gateway host were unreachable from a remote or tunnelled browser, which is exactly when Download is the only way to get them. Raising the number would not fix it: a whole-file read of a multi-GB file would exhaust gateway memory.

Not a goal

• Scanning files over the threshold. The maintainer chose to deliver them unscanned rather than build a chunked scanner.
• Changing any upload cap, the office or sheet preview, /api/file-raw, /api/file-stream or /api/outbox/{filename}.
• The Files app's own Download, which builds a blob from /apps/file-explorer/api/read (a separate 4 MB read cap), not /api/file-download.

What changed (motivation → approach → change)

Symptom: Download fails for any file over 50 MB. Cause: the route used the whole-read envelope (_open_checked, max_bytes=_MAX_UPLOAD_BYTES) so its scans could see the whole buffer.

Approach, per the maintainer's decision: scan small files exactly as before, and stream large files unscanned behind an explicit confirmation.

• dashboard/file_api/transfer.py api_file_download opens the file once through the shared checked prefix (_open_checked_file: validation, sensitive-path filter, O_NOFOLLOW open, fstat) and decides small or large from the fstat of that one descriptor, so no other file can be swapped in between the check and the send.
• At or under _DOWNLOAD_SCAN_MAX_BYTES (= _MAX_UPLOAD_BYTES, 50 MB): today's path. Bounded read on the same fd, then the same redact / binary_content_is_flagged / wide_content_is_flagged scans, the content_redacted refusal, the owner-grant release and its audit, unchanged. A file that grows past the threshold between the fstat and the read is refused 409 file_changed.
• Over the threshold: delivered UNSCANNED. The route is owner-only and the scan cannot hold a multi-GB file in memory. Without confirm_unscanned=1 it answers 409 {"code": "large_file_unscanned", "size", "limit"} and sends no bytes. With it, _stream_unscanned_download streams _STREAM_CHUNK_BYTES chunks from the same fd with Content-Length = the fstat size, never past it (a file that grows is cut there, one that shrinks drops the connection). Same attachment + nosniff headers. SEL logs the success with error="unscanned_large_file" (and the unconfirmed ask with large_file_unconfirmed), so the audit shows the file was not scanned.
• The flag never skips the owner check, validate_tool_args, path validation or the sensitive-path refusal: a .env or credential path is refused at any size.
• dashboard/file_api/pinned_io.py: the open-refusal mapping moves out of _open_checked into _open_refusal, so the download route and the whole-read envelope share one SEL vocabulary and response set instead of a copy.
• Frontend: downloadFileToDisk (utils/fileReadUrl.ts) reads the 409 and asks through confirmUnscannedDownload; UnscannedDownloadConfirmHost, mounted once in App.tsx, answers with the shared useConfirm dialog (Modal focus trap; Escape, backdrop and Cancel all cancel). "Download anyway" starts a plain browser download of the confirm_unscanned=1 URL (downloadUrl in utils/download.ts), so the file streams to disk instead of being held as a blob. Sizes use fmtBytes. The binary/office card, media fallback and sheet "Download" links become FileDownloadLink, which keeps the <a href download> but routes a plain click through the same helper, so they ask too and show a refusal instead of saving the error JSON.
• Specs: docs/system-specs/modules/security.md gains § Dashboard File Download, recording that files over the threshold are delivered unscanned by the maintainer's decision and why; learn-cron-dashboard.md no longer lists file-download among the 50 MB in-memory readers.

Backwards compatibility

Compatible: every file that downloads today (up to 50 MB) takes the same scanned path with the same answers; the only response that changes is the one for a file over 50 MB, which was 413 and is now 409 large_file_unscanned (or the file, with confirm_unscanned=1).

Removes nothing: no route, key or reader goes away. The one guarantee that narrows is deliberate and documented: a file over 50 MB that was refused before is now delivered without a content scan once the owner confirms.

Tests

• test/test_file_download.py
  • test_a_large_file_asks_before_it_sends_a_byte: over the (patched) threshold, no flag → 409 with size and limit, SEL large_file_unconfirmed.
  • test_a_confirmed_large_file_streams_whole_and_unscanned: with the flag the bytes arrive whole with Content-Length, attachment and nosniff; no scan runs; SEL unscanned_large_file.
  • test_a_large_file_is_never_read_whole: every read on the large path is one bounded chunk.
  • test_a_growing_large_file_stops_at_its_fstat_size: bytes appended after the fstat are not sent.
  • test_a_small_file_ignores_the_flag_and_is_still_scanned: a credential file under the threshold is refused content_redacted even with the flag.
  • test_a_sensitive_path_over_the_threshold_is_refused_even_confirmed and test_a_non_owner_is_refused_even_confirmed: 403 at any size.
  • test_a_stalled_chunk_read_holds_no_default_worker_and_ends_the_stream and test_a_busy_transfer_pool_ends_the_stream: each chunk read runs on the bounded transfer pool (mc-pathxfer) within _DOWNLOAD_CHUNK_STALL_SECS; a read that never returns, or a refused hop, is audited and drops the connection. Both fail with the old asyncio.to_thread read.
  • test_after_a_stall_the_handler_returns_and_the_close_is_deferred: after a stalled chunk read the handler returns at once, never awaits the close on the pool, and the close runs on its own file-download-close thread (the fake close blocks like one behind the abandoned read's buffer lock). Fails when the close is awaited.
  • test_a_busy_pool_closes_the_file_off_the_event_loop: when the transfer pool refuses the close, the file is closed on a thread of its own, never the loop (fails with a plain f.close() there).
  • The existing scan, grant, symlink, sensitive-path and resolve tests pass unchanged.
• website/src/test/unscannedDownload.test.tsx: the dialog shows on 409 with the name and both sizes; "Download anyway" retries with confirm_unscanned=1 as a URL download, not a blob; Cancel and Escape download nothing; a FileDownloadLink click takes the same path; a small file still downloads with no dialog; the dialog uses the top layer so it paints above the full-screen preview; a Windows path (C:\work\report.docx) saves as report.docx; a failed link download offers the agent hand-off.
• website/src/test/BinaryFileCard.test.tsx: a 404 Download on the file card shows its notice (office-card-download-error) in its own row, outside the Open / Download group, which keeps at most two controls. Fails when the card does not take the error.
• test/test_dashboard_files_composition_contract.py: shape digest recaptured for api_file_download's web.StreamResponse return type.

Proof: PROVEN test/test_file_download.py (proven on bd8c80759194; the review fixes amended since add tests of their own; the off-loop close test was run against the old close and fails)

Manual verification

On a pod built from this branch, with a 3 GB sparse file in the workspace: GET /api/file-download answered 409 large_file_unscanned with size: 3221225472, limit: 52428800, a small file answered 200; right-click → Download in the chat Files tab showed the dialog below. The Electron shell's auto-save for the confirmed URL download was not exercised (Linux host, no desktop shell).

Screenshots / video

Files tab → right-click a 3 GB file → Download, light and dark themes:

Related Issues

N/A — requested by the maintainer.

Sibling sweep

Swept: git grep -nE "_MAX_UPLOAD_BYTES|max_bytes=" -- src/kiro_crew/dashboard/file_api/ and git grep -n "file-download|fileDownloadUrl" -- website/src
Sibling: src/kiro_crew/dashboard/file_api/editor.py -- not affected -- file-raw is the inline viewer read, not a Download, and the brief keeps it capped
Sibling: website/src/components/FileRenderers.tsx -- changed here
Sibling: website/src/pierre/PierreWorkspaceTreeImpl.tsx -- changed here through the shared downloadFileToDisk helper, no edit needed
Sibling: website/src/apps/file-explorer/FileExplorerPage.tsx -- not affected -- its Download reads the Files app backend, not this route

Pattern harvest

Not generalizable: the cap came from one route borrowing the whole-read envelope; other readers keep that envelope on purpose.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Contributions are accepted under the project's Apache-2.0 license; see the Licensing section of CONTRIBUTING.md.

[github-actions]

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of 2492efff81bc311eca097a20310762d3332c9ec6 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks: the 413 on large files is replaced by a confirm-then-stream path the Goal names. One mechanism costs more than the Goal needs.

Notes

• Item 8 — oversized: "Each chunk is read on the bounded transfer pool … within _DOWNLOAD_CHUNK_STALL_SECS … the file's close then runs on a thread of its own". The hung-mount harm is hypothetical, with no linked defect. Its sibling, the chunk loop of api_file_stream (transfer.py:728-753), streams the same way over plain asyncio.to_thread reads and closes and was left as is. That leaves two chunk-loop spellings in one file.
  Subtraction: drop _DOWNLOAD_CHUNK_STALL_SECS, read_in_flight, _download_read_chunk and the file-download-close thread. Read and close the way api_file_stream's loop does.

What this change ships

Inventory (10 items) — 7 justified

Intent: let the owner download a dashboard file of any size, with a warning before a file too large to scan. This is a FIX.

1. A file over 50 MB now downloads instead of showing "Download failed" — justified
2. A new "Too large to scan" confirmation appears before that download — justified
3. Files over 50 MB are delivered without the credential scan once the owner confirms — justified
4. The Download links on the binary card, media fallback and sheet now ask first and show a refusal instead of saving error JSON — justified
5. A failed Download on the binary card shows its notice in its own row — rides along
6. A Windows path saves under its file name, not the whole path — rides along, undeclared in What changed
7. A small file that grows during the read now gets 409 file_changed (was 413) — justified
8. A large-download chunk read is cut off after 60 s, runs on the transfer pool, and the file closes on a separate thread — oversized; a second chunk-loop spelling beside api_file_stream
9. New SEL tags record unscanned, unconfirmed and stalled downloads — justified
10. One refusal mapping is now shared by the whole-read envelope and the download route — justified

[FIRST-PRINCIPLES-REVIEWED] 2492eff

Description read

[DESCRIPTION-READ] ec06347913e6baa3ffaac5bdaf51fa3eaa44a4829d2e3a964c41e7b8e5c9f625

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

Design Review (Opus 5.5) — ✅ PASS

Design-level review of 2492efff81bc311eca097a20310762d3332c9ec6 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: the size split is decided by the fstat of the one open descriptor, the owner and sensitive-path gates hold at every size, and unscanned delivery needs the owner's confirmation and is audited.

Notes

• The src/kiro_crew/dashboard/upload_limits.py module docstring (line 11) still lists file-download among the in-memory read paths bounded by the upload cap. The PR fixes this claim in learn-cron-dashboard.md but leaves this copy stale.

[DESIGN-REVIEWED] 2492eff

Description read

[DESCRIPTION-READ] 7a12e66cb7b2a236b92384edac35b6dd6cfc3475d80dd3dbe8a13a0c9c9290d7

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of 2492efff81bc311eca097a20310762d3332c9ec6 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 2492eff

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 2492efff81bc311eca097a20310762d3332c9ec6 false-positive: <reason, 20 to 500 characters>

[github-actions]

UX Review (Opus 5.5) — ✅ PASS (a BLOCK with no Trigger and no Outcome does not block)

UX-level review of 2492efff81bc311eca097a20310762d3332c9ec6 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: PASS

cannot evaluate: missing the new download-refusal notice on the file card, the media fallback and the sheet bar. The diff adds it to all three places, and no screenshot shows any of them.

Evidence gaps

• Binary/office file card, office-card-download-error: a failed Download now shows an ErrorNotice with an ask-the-agent control in its own row, beside the Open error. No screenshot shows this row. To close the gap, attach a still of the card after a refused Download, in both the compact and the full layout.
• Sheet viewer bar and media fallback (FileDownloadLink's file-download-link-error): the notice is drawn right after an inline "Download" link, inside the centred 12px sheet_formulas_note bar and under the "media preview failed" button. Nobody has seen how it wraps or lines up there. To close the gap, attach a still of a refused Download in the sheet viewer and one in the media fallback.

Notes

• "Download anyway" is red. unscannedDownloadOptions doesn't pass danger: false, so ConfirmDialog uses its default destructive style. The blind reader said: "I'd hesitate. The red tells me it's risky, but the box doesn't say what the risk is." Nothing is deleted, so pass danger: false, or have the body say what the scan protects against.
• The limit reads "52.4MB". The PR and the spec say 50 MB, but fmtBytes uses decimal units, so a 50 MiB limit shows as 52.4MB. The reader asked why the limit is "such an odd number like 52.4MB". Format the limit so it reads 50 MB.
• The body says "checked for secrets" and the title says "scan". The reader wasn't sure these were the same check, or what a "secret" is. Use one word for both, and name what is checked: "passwords, keys and tokens".

[UX-REVIEWED] 2492eff

Description read

[DESCRIPTION-READ] 34d168bc2901b2c4098db9e7bb53c5cad53e502f94f45028f715f2659644c2be

That sha256 names everything this verdict read: the PR description, plus the 5 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

Opus 5.5 Review — ✅ no blocking findings

Reviewed 2492efff81bc311eca097a20310762d3332c9ec6 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 2492eff

Verdict parsed from the review's SHA-scoped output markers for commit 2492efff81bc311eca097a20310762d3332c9ec6.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 2492efff81bc311eca097a20310762d3332c9ec6 false-positive: <reason, 20 to 500 characters>