fix(job-runs): resolve upload path and S3 presigned URL signature errors#9
Open
haizhou-zhao wants to merge 1 commit intomainfrom
Open
fix(job-runs): resolve upload path and S3 presigned URL signature errors#9haizhou-zhao wants to merge 1 commit intomainfrom
haizhou-zhao wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
Author
|
@salty-hambot review |
Discovered while submitting a local Python file via:
wherobots job-runs create test_connection.py --name test_connection --watch
Three bugs blocked the upload step before the job could be created:
Bug 1 (jobs.go): /files/dir called with s3:// URI instead of bare bucket name
The dir query param was constructed as s3://bucket/ but the API only accepts a
bare bucket name. This caused HTTP 400 "No storage source found for bucket: s3:"
on every auto-upload attempt. Also normalize the response path field (which lacks
the s3:// prefix) before parsing with splitS3Path.
Bug 2 (jobs.go): upload key missing bucket prefix; wrong upload target directory
The key sent to /files/upload-url was prefix/name/file, but the API expects the
full path starting with the bucket: bucket/prefix/name/file. Without it, the
server parsed the org ID as the bucket and returned HTTP 400 "No storage source
found for bucket: <org_id>". Additionally, the upload targeted the org-level
managed storage root (uploadProtected: true). Added a GET /users/me call to
resolve the caller's user ID and navigate to their personal writable directory
(bucket/org_id/data/customer-{user_id}) before falling back to the legacy root.
Bug 3 (upload.go): Content-Type header breaks S3 presigned URL signature
AWS S3 Signature V2 includes Content-Type in the canonical string. The presigned
URL from /files/upload-url is signed without a Content-Type constraint, but
UploadFileToPresignedURL was setting Content-Type: application/octet-stream on
the PUT request. This caused HTTP 403 SignatureDoesNotMatch on every upload.
Fixed by removing the Content-Type header from the presigned PUT request.
Update three test assertions that encoded the old (buggy) behavior:
- Expected s3://managed-bucket/ dir param → now bare managed-bucket
- Expected key prefix flag-prefix/ → now flag-bucket/flag-prefix/
- Expected key prefix env-prefix/ → now env-bucket/env-prefix/
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
84167cb to
f6a6eae
Compare
Member
|
@haizhou-zhao was this tested manually? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
These bugs were discovered while using the CLI to submit a local Python file as a Wherobots job (via
wherobots job-runs create test_connection.py --name test_connection --watch). All three blocked the upload step before the job could even be created.Bug 1:
/files/dircalled withs3://URI instead of bare bucket name (jobs.go)The
resolveManagedUploadTargetfunction constructedrootDirass3://bucket/and passed it as thedirquery parameter. The/files/dirAPI only accepts a bare bucket name (e.g.wbts-wbc-m97rcg45xi), so the call failed with HTTP 400InvalidInputException (No storage source found for bucket: s3:).Fix: Pass the bucket name directly. Also normalize the response
pathfield (which lacks thes3://prefix) before parsing it withsplitS3Path.Bug 2:
/files/upload-urlkey missing bucket prefix, and wrong upload target directory (jobs.go)The key sent to
/files/upload-urlwasprefix/name/file, but the API expects the full path starting with the bucket name:bucket/prefix/name/file. Without it, the server parsed the org ID as the bucket and returned HTTP 400InvalidInputException (No storage source found for bucket: <org_id>).Additionally, the upload was targeting the org-level managed storage root, which has
uploadProtected: true. The actual writable directory per user isbucket/org_id/data/customer-{user_id}/. Added aGET /users/mecall to resolve the caller's user ID and navigate to their personal directory before falling back to the legacy root path.Fix: Prepend bucket to the upload key. Add
getMeoperation and resolve the user's personal directory first.Bug 3:
Content-Type: application/octet-streambreaks S3 presigned URL signature (upload.go)AWS S3 Signature Version 2 includes
Content-Typein the canonical string used to verify presigned URLs. The presigned URL from/files/upload-urlis signed without aContent-Typeconstraint, butUploadFileToPresignedURLwas unconditionally settingContent-Type: application/octet-streamon the PUT request. This caused HTTP 403SignatureDoesNotMatchon every upload.Fix: Remove the
Content-Typeheader from the presigned PUT request.Test plan
wherobots job-runs create <local-script.py> --name <name> --watchand verify it uploads, submits, and streams logs to completions3://bucket/org_id/data/customer-{user_id}/name/script.py--upload-path s3://custom/pathoverride still works (code path unchanged)--no-uploadwith an s3:// URI still works (skips upload entirely)🤖 Generated with Claude Code