Skip to content

workflows: check out v22.21.1 so Dockerfile.Packages is present#19

Merged
harshita-gupta merged 1 commit intomainfrom
harshitagupta/fix-dockerfile-checkout
Apr 21, 2026
Merged

workflows: check out v22.21.1 so Dockerfile.Packages is present#19
harshita-gupta merged 1 commit intomainfrom
harshitagupta/fix-dockerfile-checkout

Conversation

@harshita-gupta
Copy link
Copy Markdown
Member

Summary

Fixes the build-node-packages.yml workflow to actually work when dispatched from main.

The problem: actions/checkout@v3 defaults to checking out the ref that triggered the workflow. When dispatched from main, it pulls main — but Dockerfile.Packages and the Node source tree only live on the v22.21.1 branch, so the docker build step fails with:

ERROR: failed to build: failed to solve: failed to read dockerfile:
open Dockerfile.Packages: no such file or directory

This was always the case but didn't surface until PR #17 added the S3 upload, because the refs/heads/main OIDC gate forced dispatch from main. Under the original workflow_run trigger (which runs on v22.21.1), the default checkout happened to grab the right tree.

The fix: pin ref: \${{ env.NODE_VERSION }} (= v22.21.1) on the checkout step. This only affects what gets placed into $GITHUB_WORKSPACE; the workflow YAML itself still executes from whichever ref triggered it, so workflow_ref (the OIDC subject claim) remains ...@refs/heads/main and the push_node_gyp_packages IAM role gate still works.

Known security hole (flagged for follow-up)

⚠️ v22.21.1 is not a protected branch. In principle, any of the repo's ~530 collaborators could push a malicious Dockerfile.Packages to this branch and have the workflow build+upload the resulting tarballs to our public S3 cache.

This change does not expand the attack surface. The same ~530 collaborators can already modify the Node source tree on this branch, which is what we compile into the released binaries. The Dockerfile is a lesser attack target than the source itself.

Why we're shipping this anyway:

  • The structural issue (unprotected version branches with write access for hundreds of collaborators) is pre-existing in the Asana/node fork.
  • Blocking this PR on a branch-protection change would block the whole node-gyp → S3 migration, which itself has concrete infra-cost/security benefits (removing 305 MB of mutable binaries from every codez checkout).
  • The S3-upload IAM role (push_node_gyp_packages) still requires refs/heads/main OIDC, so the workflow YAML itself can only be modified through main (which IS protected).

A follow-up PR will propose a structural fix — options under consideration:

  • Add branch protection to v22.21.1 and other active Node version branches
  • Migrate to a patch-series-on-top-of-upstream model (reducing the number of "hot" branches to one)
  • Switch codez to consume upstream nodejs/node directly via submodule, keeping Asana patches in a small fork

Tracked in internal project notes on the Asana/node fork's structural issues.

Test plan

  • After merging, dispatch the workflow from main via the Actions UI.
  • Confirm the checkout step shows ref: v22.21.1 in its logs.
  • Confirm the Docker build step succeeds (it was failing on the previous dispatch).
  • Confirm both packages_x64.tar.gz and packages_arm64.tar.gz upload to s3://asana-oss-cache/node-gyp/.
  • Capture the sha256s printed at the end of the job for use in the codez PR (#388870).

🤖 Generated with Claude Code

When dispatched from main, `actions/checkout@v3` pulls main (the workflow's
trigger ref) — but Dockerfile.Packages only lives on the v22.21.1 branch, so
the Docker build step fails with "open Dockerfile.Packages: no such file or
directory". Pinning `ref: ${{ env.NODE_VERSION }}` on the checkout keeps
workflow_ref (OIDC subject claim) on main while giving the build access to
the v22.21.1 tree.

Does not expand the attack surface: the Node source already lives on the
unprotected v22.21.1 branch, so any collaborator capable of modifying
Dockerfile.Packages could already modify the binaries we ship. A follow-up
PR will propose a structural fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@skeggse skeggse left a comment

Choose a reason for hiding this comment

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

approving with the expectation that we close this hole soon

@harshita-gupta harshita-gupta merged commit d0cdf8c into main Apr 21, 2026
10 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants