feat(upload): presigned direct-to-storage uploads

[zfarrell]

Switch managed-table loads to the presigned/direct-to-storage upload path (SDK upload_file, published in hotdata 0.5.0), removing the legacy /v1/files proxy seam. Multipart concurrency defaults to 12, overridable via HOTDATA_UPLOAD_CONCURRENCY.

[codecov]

Codecov Report

❌ Patch coverage is 81.31313% with 37 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/databases.rs	61.62%	33 Missing ⚠️
src/sdk.rs	96.42%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

[claude]

Review

Blocking Issues

• src/databases.rs (upload_parquet_url / TempGuard) — The downloaded temp file is leaked on every failed --url upload. upload_parquet_path exits via ApiError::exit -> std::process::exit(1) (sdk.rs:280-283), which does not unwind or run destructors, so the TempGuard never drops. This directly contradicts the code comments ("removed on every exit (success or failure)"). On large files this leaves multi-GB files in the temp dir on the common error paths (e.g. 501 PRESIGN_UNSUPPORTED, storage rejection).

Action Required

• Ensure the temp file is removed before exiting on the upload-failure path — e.g. have upload_parquet_path return Result<String, ApiError> and clean up (drop / remove_file) before calling e.exit(), rather than exiting from inside a function whose caller owns the guard.

Non-blocking nits left inline (insecure temp-file creation; stale Cargo.toml comment).

[claude]

Blocking: the temp file is leaked on the upload-failure path, contradicting the comment ("removed on every exit (success or failure)" / "once this returns (or unwinds)").

upload_parquet_path exits on failure via Err(e) => e.exit(), and ApiError::exit calls std::process::exit(1) (sdk.rs:280-283). std::process::exit neither unwinds nor runs destructors, so the TempGuard in this function's scope is not dropped — the downloaded temp file (potentially many GB) is left behind in the system temp dir on every failed --url upload (e.g. 501 PRESIGN_UNSUPPORTED, storage rejection). Repeated failures can fill the temp partition (often a small tmpfs).

The success path is fine (normal return drops the guard). Only the process::exit error path leaks. Consider having upload_parquet_path return Result<String, ApiError> so this function can clean up (drop/remove the temp file) before exiting, e.g.:

let id = match upload_parquet_path(api, temp.path(), size) {
    Ok(id) => id,
    Err(e) => {
        drop(temp); // remove temp file before exiting
        e.exit();
    }
};

or explicitly std::fs::remove_file(temp.path()) before the exit.

[claude]

nit: (not blocking) this hand-rolls a predictable temp filename (hotdata-upload-{pid}-{nanos}.parquet) and opens it with File::create, which is O_CREAT|O_TRUNC without O_EXCL. In a shared/world-writable temp dir this is the classic insecure-temp-file pattern (CWE-377/CWE-59): an attacker who pre-creates a symlink at the path can redirect the write (std::io::copy follows it) and clobber a victim-writable file. tempfile is already a regular dependency (Cargo.toml:57) and is used in the tests — tempfile::Builder::new().suffix(".parquet").tempfile() creates the file atomically with O_EXCL and 0600 perms, and gives you the same path-based handle. Worth using here too. (Note: a tempfile handle still won't auto-delete across process::exit — see the leak comment above.)

[claude]

Both prior review issues are resolved:

• The temp-file leak on the upload-failure path is fixed: upload_temp_file owns the NamedTempFile and drops it before returning the Err the caller exits on, so cleanup precedes process::exit. Tests pin both the success and failure arms.
• The insecure temp-file pattern is fixed: download_to_temp now creates the file atomically via tempfile::Builder (O_EXCL + 0600).

LGTM.

[claude]

Prior review items resolved: the temp-file leak on the upload-failure path is now fixed (upload_temp_file drops the NamedTempFile before returning the Err the caller exits on, with tests pinning both arms), and the temp file is created atomically via tempfile::Builder (O_EXCL + 0600). No new blocking issues.