Conversation
WalkthroughThe change removes a top-level Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Vulnerabilities of
|
| digest | sha256:f3d58d04d8081ac0ea1aa88f4915c186a4081c95cb1ef74f41d33b37bd7bb01d |
| vulnerabilities | |
| platform | linux/amd64 |
| size | 173 MB |
| packages | 972 |
📦 Base Image node:24-alpine
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/modules/`@apostrophecms/uploadfs/index.js:
- Line 12: The uploadfs module hardcodes acl: 'private', but init-localstack.sh
creates buckets with --acl public-read; update init-localstack.sh to create
buckets with --acl private to match the hardcoded acl and avoid permission
mismatches (and remove or stop relying on granting s3:PutObjectAcl if present).
Locate the bucket creation commands in init-localstack.sh and replace the --acl
public-read flag with --acl private so the bucket ACL aligns with the acl:
'private' setting in the module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 955d9741-009c-4e9d-9b56-7630a306da90
📒 Files selected for processing (1)
website/modules/@apostrophecms/uploadfs/index.js
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
website/modules/@apostrophecms/uploadfs/index.js (2)
35-38:⚠️ Potential issue | 🟠 MajorThis default conflicts with current LocalStack bootstrap ACL behavior.
init-localstack.sh(Lines 41-101 in the provided snippet) still provisions public ACL/policy (public-read,s3:PutObjectAcl). With Line 36 defaulting to'private', local behavior diverges from runtime behavior and can break direct-object-access expectations in dev.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/modules/`@apostrophecms/uploadfs/index.js around lines 35 - 38, The default ACL constant aposS3BucketObjectsAcl currently defaults to 'private' which conflicts with LocalStack bootstrap provisioning; change the default to match the LocalStack bootstrap (e.g., 'public-read') or otherwise derive it from the same env/flag used by init-localstack so uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL use the same ACL as the local bootstrap; update the assignment for aposS3BucketObjectsAcl and ensure both uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL are set from that corrected/defaulted value.
35-38:⚠️ Potential issue | 🟠 MajorDon't force a default ACL value when
APOS_S3_ACLis unset.Lines 37–38 assign an ACL to every upload when
process.env.APOS_S3_ACLis not set. This creates unnecessary coupling with infrastructure configuration. The LocalStack setup (init-localstack.sh) is configured for public access and public-read ACLs, but the app now defaults to sending'private'ACL headers on every upload, creating a mismatch between app behavior and infrastructure expectations. Only set ACL options when explicitly configured.Suggested fix
-const aposS3BucketObjectsAcl = - process.env.APOS_S3_ACL || 'private'; -uploadfsOptions.bucketObjectsACL = aposS3BucketObjectsAcl; -uploadfsOptions.disabledBucketObjectsACL = aposS3BucketObjectsAcl; +const aposS3BucketObjectsAcl = process.env.APOS_S3_ACL; +if (aposS3BucketObjectsAcl) { + uploadfsOptions.bucketObjectsACL = aposS3BucketObjectsAcl; + uploadfsOptions.disabledBucketObjectsACL = aposS3BucketObjectsAcl; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/modules/`@apostrophecms/uploadfs/index.js around lines 35 - 38, The code unconditionally sets uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL to a default 'private' via aposS3BucketObjectsAcl even when process.env.APOS_S3_ACL is unset; change this so these properties are only assigned when APOS_S3_ACL is explicitly provided: check process.env.APOS_S3_ACL, and if it is defined (non-empty), set aposS3BucketObjectsAcl and then assign uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL; otherwise leave those uploadfsOptions fields undefined so no ACL header is forced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/modules/`@apostrophecms/uploadfs/index.js:
- Around line 35-36: Validate the APOS_S3_ACL env value at startup before it is
used: define the allowed canned ACLs array (private, public-read,
public-read-write, authenticated-read, aws-exec-read, bucket-owner-read,
bucket-owner-full-control), read process.env.APOS_S3_ACL into
aposS3BucketObjectsAcl as currently done, check that aposS3BucketObjectsAcl is
one of the allowed values, and if not log a clear error and exit (or throw) so
the app fails fast; update the initialization around the aposS3BucketObjectsAcl
assignment in index.js to perform this whitelist check and fail immediately on
invalid values.
---
Duplicate comments:
In `@website/modules/`@apostrophecms/uploadfs/index.js:
- Around line 35-38: The default ACL constant aposS3BucketObjectsAcl currently
defaults to 'private' which conflicts with LocalStack bootstrap provisioning;
change the default to match the LocalStack bootstrap (e.g., 'public-read') or
otherwise derive it from the same env/flag used by init-localstack so
uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL
use the same ACL as the local bootstrap; update the assignment for
aposS3BucketObjectsAcl and ensure both uploadfsOptions.bucketObjectsACL and
uploadfsOptions.disabledBucketObjectsACL are set from that corrected/defaulted
value.
- Around line 35-38: The code unconditionally sets
uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL to
a default 'private' via aposS3BucketObjectsAcl even when process.env.APOS_S3_ACL
is unset; change this so these properties are only assigned when APOS_S3_ACL is
explicitly provided: check process.env.APOS_S3_ACL, and if it is defined
(non-empty), set aposS3BucketObjectsAcl and then assign
uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL;
otherwise leave those uploadfsOptions fields undefined so no ACL header is
forced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 380d1ef5-e3d1-4969-b7f3-37b8b89f0db1
📒 Files selected for processing (1)
website/modules/@apostrophecms/uploadfs/index.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/modules/`@apostrophecms/uploadfs/index.js:
- Around line 35-37: The ACL logic sets uploadfsOptions.bucketObjectsACL and
uploadfsOptions.disabledBucketObjectsACL unconditionally using
aposS3BucketObjectsAcl which defaults to 'private'; instead, read
process.env.APOS_S3_ACL into a variable and only assign
uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL
when APOS_S3_ACL is explicitly defined (non-undefined), leaving those properties
absent (or null) for ACL-disabled buckets; update references to
aposS3BucketObjectsAcl/APOS_S3_ACL accordingly so the code mirrors the
optional-setting pattern used elsewhere in this module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97d01d27-db08-4a62-ac51-bc575fc8a828
📒 Files selected for processing (1)
website/modules/@apostrophecms/uploadfs/index.js
There was a problem hiding this comment.
♻️ Duplicate comments (1)
website/modules/@apostrophecms/uploadfs/index.js (1)
35-38:⚠️ Potential issue | 🟡 MinorFail fast on invalid
APOS_S3_ACLvalues.This still accepts any truthy string and forwards it to S3 via uploadfs; invalid canned ACL values will fail at upload time instead of at startup. Add a whitelist check and throw on invalid values.
What are the currently valid Amazon S3 canned ACL values for PutObject / x-amz-acl, and are non-listed values rejected?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/modules/`@apostrophecms/uploadfs/index.js around lines 35 - 38, Validate the APOS_S3_ACL value against the allowed Amazon S3 canned ACL list before assigning it to uploadfsOptions: check the environment variable aposS3BucketObjectsAcl and if present ensure it matches one of the official canned ACLs (private, public-read, public-read-write, aws-exec-read, authenticated-read, bucket-owner-read, bucket-owner-full-control, log-delivery-write); if it does not match, throw an Error during startup so the process fails fast instead of passing an invalid value to uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL. Ensure the check references the existing variable name aposS3BucketObjectsAcl and the option properties uploadfsOptions.bucketObjectsACL / uploadfsOptions.disabledBucketObjectsACL so the invalid value is rejected at boot.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@website/modules/`@apostrophecms/uploadfs/index.js:
- Around line 35-38: Validate the APOS_S3_ACL value against the allowed Amazon
S3 canned ACL list before assigning it to uploadfsOptions: check the environment
variable aposS3BucketObjectsAcl and if present ensure it matches one of the
official canned ACLs (private, public-read, public-read-write, aws-exec-read,
authenticated-read, bucket-owner-read, bucket-owner-full-control,
log-delivery-write); if it does not match, throw an Error during startup so the
process fails fast instead of passing an invalid value to
uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL.
Ensure the check references the existing variable name aposS3BucketObjectsAcl
and the option properties uploadfsOptions.bucketObjectsACL /
uploadfsOptions.disabledBucketObjectsACL so the invalid value is rejected at
boot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c1fda04-78b2-43c7-a065-1d0f4ebed7cc
📒 Files selected for processing (1)
website/modules/@apostrophecms/uploadfs/index.js
Change S3 uploadfs config to set object-level ACLs from APOS_S3_ACL when defined. Instead of a top-level acl, the code reads APOS_S3_ACL into aposS3BucketObjectsAcl and, if truthy, assigns it to uploadfsOptions.bucketObjectsACL and uploadfsOptions.disabledBucketObjectsACL. No default is applied; fields are omitted when the env var is unset. This ensures object ACLs are only configured when explicitly specified.