feat(cli): upload worker images using presigned URLs

[NathanFlurry]

• Add upload-presigned for worker Dockerfile builds, local images, and archived images without Cloud login or deployment.
• Require Linux/AMD64 images and validate archive integrity before uploading.
• Stream uploads with reqwest, bounded retries and timeouts, disabled redirects, and credential-safe errors; no curl dependency.
• Test all three inputs with Docker and local object storage, including retries and cancellation.
• Clarify that standalone CLIs may use dedicated HTTP clients without Engine pools.
• Refresh Cargo.lock to match the existing workspace version and new CLI dependencies.

[railway-app]

🚅 Deployed to the actors-pr-5735 environment in rivet-frontend

Service	Status	Web	Updated
website	❌ Build Failed (View Logs)	Web	Sep 15, 2026 at 11:31 pm UTC
frontend-cloud	😴 Sleeping (View Logs)	Web	Sep 15, 2026 at 10:41 pm UTC
kitchen-sink	😴 Sleeping (View Logs)	Web	Sep 15, 2026 at 10:41 pm UTC
frontend-inspector	😴 Sleeping (View Logs)	Web	Sep 15, 2026 at 10:38 pm UTC
mcp-hub	✅ Success (View Logs)	Web	Sep 15, 2026 at 10:31 pm UTC
ladle	✅ Success (View Logs)	Web	Sep 15, 2026 at 10:31 pm UTC

[claude]

Review: feat(cli) upload worker images using presigned URLs

Re-reviewed the current diff (single commit 291e0b6c2, unchanged since the last pass). Overall this is a strong implementation: bounded resource limits (5 GiB archive / 32 GiB decompressed / 1 MiB JSON / 100k tar entries), credential-safe error handling that deliberately drops reqwest error internals, response bodies, and redirect Location headers, HTTPS enforcement with a narrow loopback-only HTTP exception, layer-hash verification against the image config, and a real-TCP-server test suite instead of mocks (matches the repository testing policy against vi.mock-style mocking).

Still open: the new test file is likely double-compiled by Cargo

engine/packages/cli/tests/upload_presigned.rs is a direct child of tests/, and upload.rs:502 pulls it in as a private submodule via #[path = "../../../tests/upload_presigned.rs"] mod tests;.

Cargo auto-discovery treats every top-level .rs file under tests/ as its own standalone integration-test crate regardless of whether it is also referenced elsewhere via #[path]. That means this file gets compiled twice: once as a submodule of rivet-cli (where super resolves to upload.rs), and once again as its own crate root via the auto-discovered --test upload_presigned target. In that second context use super::*; at tests/upload_presigned.rs:10 has no parent module to resolve, since the file is the crate root there, so that target should fail with an unresolved-import error.

This repository already has the fix for exactly this pattern: engine/packages/pegboard-envoy/src/ping_task.rs:107 includes its test module from tests/support/ping_task.rs, nested one level under tests/, precisely because subdirectories of tests/ are not auto-discovered. engine/packages/cli/tests/upload_presigned.rs should move to engine/packages/cli/tests/support/upload_presigned.rs, with the #[path] in upload.rs:502 updated to match. Worth confirming with cargo test -p rivet-cli (unable to run cargo in this environment to double check), but the mechanism is exactly what the CLAUDE.md note on reusing tests/ files under the Rust test layout section is warning about.

Cancellation is not prompt during large synchronous work

Opts::execute (upload.rs:74-ish) races execute_inner() against tokio::signal::ctrl_c(), with a comment claiming cancellation drops the temp directory and kills subprocesses immediately. That holds for the docker build/docker save awaits, but execute_inner and inspect_archive also run several synchronous, unbounded-duration operations directly on the async task with no spawn_blocking: copying up to 5 GiB, gzip-compressing up to 32 GiB, and SHA-256 hashing plus tar-scanning up to 32 GiB decompressed (inspect_archive, lines 251-350ish). Tokio cannot preempt a synchronous call mid-execution, so Ctrl+C only takes effect once the current blocking call returns, which for a large image could be a multi-minute wait rather than the immediate cancellation the comment implies, and can starve the ctrl_c() future outright on a single-core host. tokio::task::spawn_blocking for the copy/hash/(de)compress steps would make cancellation actually prompt.

Test coverage gap: the path-traversal and non-regular-file rejections in entry_name/inspect_archive are untested

entry_name (upload.rs:227) rejects any tar path component other than Normal/CurDir ("archive contains an unsafe path", line 231), and inspect_archive (line 251) rejects any non-directory, non-regular tar entry ("Docker archive contains a non-regular file", line 274). These are the core defenses against a crafted or corrupted docker-save archive (directory traversal via ../, symlink/hardlink entries), but tests/upload_presigned.rs has no case that builds a tar with such an entry and asserts inspect_archive rejects it. Worth adding both as regression tests alongside the existing rejects_truncated_and_non_docker_archives case.

Performance: inspect_archive fully scans the archive three times

For every upload, inspect_archive does a raw-file SHA-256 pass over the compressed bytes (line 257), then a full gzip-decompression pass to find manifest.json and validate the expanded size (line 260), then a second full gzip-decompression pass to re-read config.json and hash the layers (line 307). For an image near the 5 GiB compressed / 32 GiB uncompressed limits, this triples the I/O and decompression cost incurred before the upload even starts. Hashing the compressed bytes while doing the first decompression pass (wrap the file reader in a hashing adapter) would cut this to two passes; fully collapsing to one pass is harder since manifest.json position relative to layers is not guaranteed. Likely fine for typical image sizes, flagging for large ones.

Minor: timing-sensitive test may flake under load

upload_reports_progress_every_two_seconds_until_server_response (tests/upload_presigned.rs:318) sleeps a real 4300ms against a 2-second progress-tick interval and asserts exactly 2 ticks were recorded (line 356). The margin to the third tick is about 300ms; a delayed scheduler tick on a loaded CI runner could flip the count to 3 and flake the test. Consider asserting a range (for example at least 2) instead of an exact count.

Nit

rivet byoc workers builds upload-presigned nests four subcommand levels deep for a single leaf command today. Reasonable if more BYOC/worker subcommands are planned soon, worth confirming it is not over-structured for a v1.

No new security issues found beyond what is already carefully handled.