-
Notifications
You must be signed in to change notification settings - Fork 38
feat: add public access for providerS3 #625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add public access for providerS3 #625
Conversation
|
@LukasMod is attempting to deploy a commit to the Callstack Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| s3Config.credentials = { | ||
| accessKeyId: '', | ||
| secretAccessKey: '', | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Public S3 Access Fails with Signed URL Logic
The public access fallback configures empty credentials and a no-op signer, but the list() and upload() methods still call getSignedUrl which requires valid credentials to generate presigned URLs. This will fail or produce invalid URLs when accessing public S3 buckets without authentication. For public buckets, direct URLs should be constructed instead of using getSignedUrl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Authentication for upload/list is still mandatory, there is no workaround for that in S3
| s3Config.credentials = { | ||
| accessKeyId: '', | ||
| secretAccessKey: '', | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: AWS SDK: Credential Fallback Broken
The public access fallback prevents the AWS SDK from using its default credential provider chain. When explicit credentials, roleArn, or profile aren't provided, the code sets empty credentials instead of leaving credentials undefined. This breaks documented support for AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY environment variables and IAM roles on EC2/ECS/Lambda, as the SDK can't fall back to these standard authentication methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on that, I think I should move this behind a config flag eg. publicAccess: boolean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated with publicAccess
Summary
Adds public access support to the S3 provider. Adds additional config flag to allow provider to configure a custom signer that doesn't sign requests, enabling access to public S3 buckets without authentication. This supports public buckets and simplifies configuration when credentials aren't needed.
Test Plan
Unit tests that verifies:
Manual test with public file: https://ad-hoc-expensify-cash.s3.amazonaws.com/rock-artifacts/rock-android-developmentDebug-bc955486ab50749733e23713a2ef32fefa1c7342.zip
Note
Adds a no-op signer and empty credentials fallback to enable accessing public S3 buckets without authentication; updates tests and docs.
packages/provider-s3)S3BuildCacheconstructor: sets3Config.signerto no-op andcredentialsto empty when no auth is provided.__tests__/providerS3.test.ts.website/src/docs/configuration.md.Written by Cursor Bugbot for commit 071b63f. This will update automatically on new commits. Configure here.