Skip to content
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

Return false from checkIfCacheExists when error is 403 #616

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leighpascoe
Copy link

Fixes issue #433

When GetHeadObject command is returned if you don't have s3:ListBucket permissions, then you will receive a 403 error. This case would still indicate that the bucket object exists.

See: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/s3/command/HeadObjectCommand/

General purpose bucket permissions - To use HEAD, you must have the s3:GetObject permission. You need the relevant read object (or version) permission for this operation. For more information, see Actions, resources, and condition keys for Amazon S3
in the Amazon S3 User Guide.

If the object you request doesn't exist, the error that Amazon S3 returns depends on whether you also have the s3:ListBucket permission.

If you have the s3:ListBucket permission on the bucket, Amazon S3 returns an HTTP status code 404 Not Found error.

If you don’t have the s3:ListBucket permission, Amazon S3 returns an HTTP status code 403 Forbidden error

The same solution exists in the nx-remotecache-s3 project

https://github.com/robinpellegrims/pellegrims/blob/master/libs/nx-remotecache-s3/src/lib/setup-s3-task-runner.ts#L44-L45

Copy link

nx-cloud bot commented May 30, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 6e4bdb4. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 5 targets

Sent with 💌 from NxCloud.

@@ -246,7 +246,7 @@ export class AwsCache implements RemoteCache {
await this.s3.send(params);
return true;
} catch (err) {
if ((err as Error).name === 'NotFound') {
if ((err as Error).name === '403' || (err as Error).name === 'NotFound') {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct, that the name equals status code? Should it be Forbidden or not?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bojanbass
Copy link
Owner

THank you for the PR, I left just a minor question.

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.

2 participants