fix(files): say "no such file or folder" when a download target is missing

[zongsizhang]

What this delivers

wherobots files my-files cat|download of a missing file used to print S3's raw XML error (the API 307s to a presigned URL without checking existence, S3 answers 404, and the CLI passed the body through — exposing the internal key layout).

• A storage (S3) 404 after the redirect now returns the existing no such file or folder: <path> error, the same as the API-404 path.
• Storage-transfer errors never include the response body. They show only the HTTP status and, at most, the S3 <Code> element (validated before it is shown), via a new executor.StorageError.
• The same rule applies to presigned uploads (UploadFileToPresignedURL), which job-runs create also uses: its storage failure message is now e.g. upload failed with HTTP 400 (InvalidRequest) rather than the raw body.

Look hardest at

• internal/executor/download.go — StorageError / newStorageError and the <Code> extraction.
• internal/files/ops.go — only a 404 becomes NotFoundError; other statuses keep the downloading <path>: … wrapping.
• The upload message change, since job-runs create shares it.

Tests

Mock storage server returning 404 NoSuchKey XML and a 403, both at the executor level and end-to-end through files.Download with a redirect; upload server-error test asserts the exact message and no body text.

Left out

ls of a missing folder (still prints "No files or folders.") — out of scope, undecided.

🤖 Generated with Claude Code

[salty-hambot]

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ✅ pass

Dimension	Verdict	Notes
correctness	✅ pass	newStorageError parses the XML Code safely, defaults empty on parse failure, and the 404→NotFoundError mapping in Download is wired correctly.
security	✅ pass	The whole point of the PR: error bodies (which can reveal internal storage paths like org-42/user-7) are dropped, keeping only status and a regex-allowlisted code word.
privacy	✅ pass	Storage location leakage in error messages is eliminated; tests assert org-ids and messages never surface.
reliability	✅ pass	Body read is bounded by maxErrorBodyBytes via LimitReader; malformed XML falls back to status-only cleanly.
scalability	✅ pass	No scaling concerns; error path reads at most 64KB of a small XML document.
observability	✅ pass	Errors retain HTTP status and the storage code (NoSuchKey, AccessDenied), preserving enough signal to diagnose without leaking bodies.
clarity/maintainability	✅ pass	Shared newStorageError dedups the upload/download error paths; comments explain the security intent clearly.
test quality	✅ pass	Tests cover the AccessDenied/NoSuchKey cases, the no-leak assertions, non-word code rejection, and the Download 404→NotFoundError mapping — thorough for the changed behavior.

Clean PR that yeets storage response bodies out the airlock — StorageError keeps only the status and an allowlisted , and the tests prove no org-id or key leaks through. Nothing to flag; both reviewers were right to stay quiet.

_{0 finding(s) posted.
💰 Review cost: $0.7576 · 173.6k in / 5.6k out tokens · ⏱️ 1m19.2s
💬 To request a re-review, comment @salty-hambot review}