Skip to content

feat(upload): signed upload URLs - #13

Closed
chenrui333 wants to merge 1 commit into
fix/redact-query-strings-in-logsfrom
feat/signed-upload-urls
Closed

chenrui333 wants to merge 1 commit into
fix/redact-query-strings-in-logsfrom
feat/signed-upload-urls

Conversation

@chenrui333

Copy link
Copy Markdown
Owner

Review copy of a proposed upstream change, based on falcondev-oss/github-actions-cache-server@dev (6ae06c2). Upstream accepts pull requests from collaborators only, so this branch is offered through an upstream issue.

Stacked on fix/redact-query-strings-in-logs.

signed_upload_url now carries a per-upload signature in its query (like the SAS of an Azure Blob URL), and the upload endpoint requires it on single-shot, block and blocklist requests. The signature is a random 256-bit value; only its SHA-256 is stored, in a nullable uploads.capabilityHash column added by an existence-guarded $9_upload_capability_hash migration, and it is compared in constant time. Upload URLs accept only PUT and answer other methods with 405 instead of forwarding them to the Results service. Clients already preserve the URL query, so @actions/cache, BuildKit and other Azure-style clients need no change. Uploads in flight during the upgrade lose their URLs.

Tests: sqlite+filesystem 75 passed / 5 skipped; postgres and mysql 20 passed each; the migration test covers fresh and already-present columns.

`signed_upload_url` now carries a per-upload signature, like the SAS
query of an Azure Blob URL: a random 256-bit value issued with each
upload and returned as `?sig=`. Single-shot, block and blocklist
requests must present it. Requests without the right signature, and
requests for unknown uploads, are answered 403; an authorized part whose
upload is deleted while it streams still gets 404.

Only the SHA-256 of the signature is stored (`uploads.capabilityHash`,
migration `$9_upload_capability_hash`) and it is compared in constant
time. The migration keeps a `capabilityHash` column that already exists
instead of failing on it. Requests to upload URLs with a method other
than PUT are answered 405 instead of being forwarded to the Results
service, and upload storage errors are logged by the route without the
request URL.

Clients already keep the signed URL's query and append their own
parameters, so no client change is needed. Uploads that are in progress
while a server is upgraded were issued URLs without a signature and can
no longer receive parts; `cleanup:uploads` removes them once idle.

See ADR 0011.

Signed-off-by: Rui Chen <rui@chenrui.dev>
@chenrui333

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-24T20:44:57.032952Z 4eca871 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 4eca8719b6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chenrui333

Copy link
Copy Markdown
Owner Author

Superseded by upstream v10.0.0 (falcondev-oss#308).

@chenrui333 chenrui333 closed this Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant