workflows: swap softprops for gh CLI, add CloudFront reachability check, remove --acl#18
Merged
harshita-gupta merged 3 commits intomainfrom Apr 21, 2026
Merged
Conversation
…ck, remove --acl Three follow-up corrections to PR #17: 1. Remove `--acl public-read` from `aws s3 cp`. The bucket has `disable_confusing_acls = true` (BucketOwnerEnforced), which disables ACLs entirely. `BlockPublicAcls` + `IgnorePublicAcls` provide additional coverage. The ACL flag is silently ignored. The IAM role (`S3_ACCESS_MODE.PUT`) also doesn't grant `PutObjectAcl`. Reads go via CloudFront OAC, not public-S3. 2. Replace `softprops/action-gh-release` with GitHub's first-party `gh` CLI. `gh release upload` is pre-installed on GitHub-hosted runners, removes a third-party (single-maintainer) supply-chain dependency, and behaves equivalently with `--clobber`. 3. Add a post-upload CloudFront reachability check (`curl -fI`). If the CloudFront path_patterns allowlist doesn't include the key's prefix, Mac Bazel builds will silently 403. Failing the workflow here surfaces the issue before consumers hit it. S3 path stays `node-gyp/*` (this PR no longer changes it — see codez PR #390222 which adds `node-gyp/*` to CloudFront's path_patterns in system_packages.tf). Action pinning: tag-pinned per codez convention (100% of codez workflows use tags, not SHAs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
harshita-gupta
force-pushed
the
harshitagupta/node-gyp-s3-followups
branch
from
22846ef to
b96733c
Compare
harshita-gupta
changed the title
skeggse
reviewed
Apr 21, 2026
Co-authored-by: Eli Skeggs <1348991+skeggse@users.noreply.github.com>
skeggse
reviewed
Apr 21, 2026
skeggse
reviewed
Apr 21, 2026
…titution in run: blocks
Eli's review flagged `${{ matrix.arch }}` in a run: block as an injectable
pattern even though the matrix values are hardcoded and not truly exploitable.
Apply the pattern consistently across the whole workflow:
- Hoist PLATFORM, ARCH, BAZEL_ARCH, REPO to job-level env so each step can
reference them as shell variables ($ARCH etc.) rather than GitHub Actions
expressions (${{ matrix.arch }}). Job-level env evaluates matrix context
since the job is instantiated per matrix combination, so this DRYs up the
per-step env blocks.
- Rewrite every `run:` block to reference the job-level env vars. No more
`${{ ... }}` expressions inside shell scripts.
- Secret references (GITHUB_TOKEN) remain step-scoped per least-privilege.
- Minor cleanup: collapse three separate `echo ... >> $GITHUB_ENV` lines into
a single `{ ...; } >> "$GITHUB_ENV"` block.
Addresses Eli's inline comment on line 114 of the pre-hoist file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
skeggse
approved these changes
Apr 21, 2026
7 tasks
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
3 participants
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
Three follow-up corrections to #17 based on post-merge audit findings:
Remove
--acl public-readfromaws s3 cp. The bucket hasdisable_confusing_acls = true(BucketOwnerEnforced), which disables ACLs entirely.BlockPublicAcls+IgnorePublicAclsprovide additional coverage. The ACL flag is silently ignored. The IAM role (S3_ACCESS_MODE.PUT) also doesn't grantPutObjectAcl. Reads go via CloudFront OAC, not public-S3.Replace
softprops/action-gh-releasewithgh release upload(first-party GitHub CLI).ghis pre-installed on GitHub-hosted runners. Removes a third-party (single-maintainer) supply-chain dependency.--clobbermatches softprops's default overwrite behavior.Add a post-upload CloudFront reachability check. If the CloudFront
path_patternsallowlist doesn't include the key's prefix, Mac Bazel builds will silently 403.curl -fsSIagainst the CloudFront URL fails the workflow at upload time rather than at consumer-build time.S3 path stays
node-gyp/*— this PR no longer changes it. The CloudFront allowlist entry fornode-gyp/*is being added in codez #390222.Action pinning
Tag-pinned per codez convention (100% of codez workflows use tags, not SHAs).
Merge prerequisites
node-gyp/*to CloudFrontpath_patterns). Without this, the post-upload CloudFront reachability check will 403.Test plan
build-node-packages.ymlfrommain(not from a version branch)asana-oss-cache.asana.biztools_repositories.bzlRisks
🤖 Generated with Claude Code