-
Notifications
You must be signed in to change notification settings - Fork 39
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?
Changes from all commits
882753c
edfd132
071b63f
a829780
8b568f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,10 @@ type ProviderConfig = { | |
| * External ID when assuming a role (for additional security). | ||
| */ | ||
| externalId?: string; | ||
| /** | ||
| * If true, the provider will not sign requests and will try to access the S3 bucket without authentication. | ||
| */ | ||
| publicAccess?: boolean; | ||
| }; | ||
|
|
||
| export class S3BuildCache implements RemoteBuildCache { | ||
|
|
@@ -104,6 +108,15 @@ export class S3BuildCache implements RemoteBuildCache { | |
| } else if (config.profile) { | ||
| // Use shared config file (e.g. ~/.aws/credentials) with a profile | ||
| s3Config.credentials = fromIni({ profile: config.profile }); | ||
| } else if (config.publicAccess) { | ||
| // Access the S3 bucket without authentication | ||
| s3Config.signer = { | ||
| sign: async (request) => request, | ||
| }; | ||
| s3Config.credentials = { | ||
| accessKeyId: '', | ||
| secretAccessKey: '', | ||
| }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: AWS SDK: Credential Fallback BrokenThe public access fallback prevents the AWS SDK from using its default credential provider chain. When explicit credentials,
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated with |
||
| } | ||
|
|
||
| this.s3 = new clientS3.S3Client(s3Config); | ||
|
|
@@ -185,17 +198,25 @@ export class S3BuildCache implements RemoteBuildCache { | |
| }: { | ||
| artifactName: string; | ||
| }): Promise<Response> { | ||
| const res = await this.s3.send( | ||
| new clientS3.GetObjectCommand({ | ||
| Bucket: this.bucket, | ||
| Key: `${this.directory}/${artifactName}.zip`, | ||
| }), | ||
| ); | ||
| return new Response(toWebStream(res.Body as Readable), { | ||
| headers: { | ||
| 'content-length': String(res.ContentLength), | ||
| }, | ||
| }); | ||
| try { | ||
| const res = await this.s3.send( | ||
| new clientS3.GetObjectCommand({ | ||
| Bucket: this.bucket, | ||
| Key: `${this.directory}/${artifactName}.zip`, | ||
| }), | ||
| ); | ||
| return new Response(toWebStream(res.Body as Readable), { | ||
| headers: { | ||
| 'content-length': String(res.ContentLength), | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| if (this.config.publicAccess) { | ||
| (error as Error).message = | ||
| `Build not found or not accessible to the public`; | ||
| } | ||
| throw error; | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| async delete({ | ||
|
|
||
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()andupload()methods still callgetSignedUrlwhich 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 usinggetSignedUrl.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