From a73688e598c7b9c0734a44121e885c4836c9bf27 Mon Sep 17 00:00:00 2001 From: Tai Le Date: Thu, 20 Aug 2026 13:35:05 +0700 Subject: [PATCH 1/2] ci: stop lending the checkout token to PR code CodeRabbit on !354, the back-port of #1, flagged the workflows the MR carried across unchanged. Three of the four findings hold. `ci.yml` and `wheel.yml` check out with credentials so the CUBIN packs can be pulled, and every step after that -- `setup.py`, the test suite -- is code a pull request controls. actions/checkout only clears the auth header in its post step, which runs last, so an untrusted step had a usable token in `.git/config` for the length of the job. The drop goes in the cubin-packs action rather than in each caller, next to the pull that is the only reader. `Vulnerability-scan` is the one job in blossom-ci.yml that checks out unreviewed contributor code, and it inherited `pull-requests: write` from the workflow with nothing to use it; a job-level block replaces the inherited set with what blossom-action demonstrably needs. The fourth asked for `github.actor == 'blossom-jenkins-bot'` on Upload-Log. That login is not published, and a wrong guess fails silent -- the run goes green and the log link never posts. spark-rapids and nvMolKit both gate on `github.event_name` alone, so the `args` check here is already stricter. Left as a comment naming when to pin it. Signed-off-by: Tai Le --- .github/actions/cubin-packs/action.yml | 19 +++++++++++++++++++ .github/workflows/blossom-ci.yml | 17 +++++++++++++++++ .github/workflows/ci.yml | 6 ++++-- .github/workflows/wheel.yml | 3 ++- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/.github/actions/cubin-packs/action.yml b/.github/actions/cubin-packs/action.yml index 15baa7bf..5686e3c4 100644 --- a/.github/actions/cubin-packs/action.yml +++ b/.github/actions/cubin-packs/action.yml @@ -23,3 +23,22 @@ runs: run: | git lfs pull --include="${PACKS}" echo "materialized $(git lfs ls-files -I "${PACKS}" | wc -l) packs" + + # That pull is the last thing in either caller that talks to the remote, and + # everything after it -- `setup.py`, the test suite -- is code the pull + # request controls. actions/checkout clears the header in its own post step, + # which runs after all of that, so clear it here instead of leaving a usable + # token in `.git/config` for the length of the job. Dropping it in the action + # rather than in each caller keeps this paired with the pull that needs it. + # + # `--unset-all` exits 5 when the key is absent, which is the case whenever a + # caller checked out with `persist-credentials: false`; `if` swallows that + # without tripping the `-e` that `shell: bash` sets. + - name: Drop the checkout credentials the pull needed + shell: bash + run: | + if git config --local --unset-all "http.https://github.com/.extraheader"; then + echo "dropped the checkout auth header" + else + echo "no checkout auth header to drop" + fi diff --git a/.github/workflows/blossom-ci.yml b/.github/workflows/blossom-ci.yml index bda410f4..4b2f9040 100644 --- a/.github/workflows/blossom-ci.yml +++ b/.github/workflows/blossom-ci.yml @@ -40,6 +40,8 @@ on: # on the pull request head, and comment the Jenkins log link back. Narrow it # further once a run has succeeded and the audit log shows what it really used; # widen it only against a specific failure, never speculatively. +# +# `Vulnerability-scan` overrides this with a narrower set of its own -- see there. permissions: contents: read statuses: write @@ -101,6 +103,14 @@ jobs: needs: [Authorization] runs-on: ubuntu-latest timeout-minutes: 30 + # The one job here that checks out unreviewed contributor code, so it does + # not inherit the workflow-level set. A job-level block replaces that set + # rather than adding to it: blossom-action posts a commit status on the head, + # and the comment back is Upload-Log's job, so `pull-requests: write` has no + # reader in this job and is dropped. + permissions: + contents: read + statuses: write steps: # The pull request head, named by the Authorization output rather than by # the event, so the scan reads the code the maintainer approved for a run. @@ -159,6 +169,13 @@ jobs: # workflow button appears for anyone with write access all the same, so guard # on the payload Jenkins always sends -- otherwise a curious click starts a job # on the self-hosted runner with nothing to post. + # + # Not `github.actor == ''`: the login Jenkins dispatches as is not + # published anywhere, and a wrong guess fails closed and silent -- the run + # succeeds, the log link never appears. The sibling repos (NVIDIA/spark-rapids, + # NVIDIA-BioNeMo/nvMolKit) gate on `github.event_name` alone, so the `args` + # check here is already the stricter one. Pin the actor once a real callback + # has run and the audit log names it. Upload-Log: name: Upload log runs-on: blossom diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7546627b..e7093638 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,8 +57,10 @@ jobs: # against the 2.5 MB of packs these suites read; the rest is sample MSAs # and structures nothing here opens. So fetch the packs by pattern. # - # The credentials stay: this is an internal repo, and `git lfs pull` below - # authenticates with the header checkout leaves in the local git config. + # The credentials stay for the checkout: this is an internal repo, and + # `git lfs pull` authenticates with the header checkout leaves in the local + # git config. The action drops that header once the pull is done, before + # any step below runs code the pull request controls. # # No submodules: neither suite imports bionemo_ir or reads 3rdparty/. - uses: actions/checkout@v7 diff --git a/.github/workflows/wheel.yml b/.github/workflows/wheel.yml index f69ec991..11f84914 100644 --- a/.github/workflows/wheel.yml +++ b/.github/workflows/wheel.yml @@ -37,7 +37,8 @@ jobs: # `lfs: true` would pull every LFS object in the repo -- measured at # 88.2 MB, of which the 36 CUBIN packs are 2.5 MB. The build reads only # the packs, so fetch only those. The credentials stay for the same reason - # as in ci.yml: `git lfs pull` needs them on an internal repo. + # as in ci.yml: `git lfs pull` needs them on an internal repo, and the + # action drops the header again before `setup.py` runs. - uses: actions/checkout@v7 with: lfs: false From ac7da6ed6a2ea8b5d6c7d8a3dba9a3ead162d677 Mon Sep 17 00:00:00 2001 From: Tai Le Date: Thu, 20 Aug 2026 13:41:15 +0700 Subject: [PATCH 2/2] fix: drop the token checkout v7 actually persists The first attempt at this was a no-op. It unset `http.https://github.com/.extraheader` in `.git/config`, which is where checkout v5 and earlier put the token -- v7 writes it to a file under RUNNER_TEMP and points `.git/config` at that file with `includeIf.gitdir:`. The step reported "no checkout auth header to drop" and passed while the credential stayed readable for the rest of the job, which is the failure CodeRabbit and zizmor were pointing at in the first place. Clear both shapes, and remove the RUNNER_TEMP file rather than only unlinking it from the config, since that file is what holds the secret. Then assert it: plain `git config --get-all` reads the effective configuration, following any `includeIf` still in place, so the step now fails if either branch missed. A checkout bump that moves the credential again turns this back into a no-op otherwise -- exercised against the v5 layout, the v7 layout, a caller with no credentials at all, and a credentials file the cleanup does not recognise, which fails the step. Signed-off-by: Tai Le --- .github/actions/cubin-packs/action.yml | 47 ++++++++++++++++++++------ 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/.github/actions/cubin-packs/action.yml b/.github/actions/cubin-packs/action.yml index 5686e3c4..bdfcc533 100644 --- a/.github/actions/cubin-packs/action.yml +++ b/.github/actions/cubin-packs/action.yml @@ -26,19 +26,44 @@ runs: # That pull is the last thing in either caller that talks to the remote, and # everything after it -- `setup.py`, the test suite -- is code the pull - # request controls. actions/checkout clears the header in its own post step, - # which runs after all of that, so clear it here instead of leaving a usable - # token in `.git/config` for the length of the job. Dropping it in the action - # rather than in each caller keeps this paired with the pull that needs it. + # request controls. actions/checkout only revokes the token in its own post + # step, which runs after all of that, so drop it here instead of leaving it + # usable for the length of the job. Dropping it in the action rather than in + # each caller keeps this paired with the pull that is its only reader. # - # `--unset-all` exits 5 when the key is absent, which is the case whenever a - # caller checked out with `persist-credentials: false`; `if` swallows that - # without tripping the `-e` that `shell: bash` sets. + # Where the token lives depends on the checkout version, and getting this + # wrong is silent: v5 and earlier wrote the header straight into + # `.git/config`, while v7 writes it to a file under RUNNER_TEMP and points + # `.git/config` at that file with `includeIf.gitdir:`. Clear both shapes -- + # the file is what actually holds the credential, so it is removed rather + # than just unlinked from the config. - name: Drop the checkout credentials the pull needed shell: bash + env: + HEADER_KEY: http.https://github.com/.extraheader run: | - if git config --local --unset-all "http.https://github.com/.extraheader"; then - echo "dropped the checkout auth header" - else - echo "no checkout auth header to drop" + # v5 and earlier. + git config --local --unset-all "${HEADER_KEY}" 2>/dev/null || true + + # v7. `--get-regexp` exits 1 when nothing matches, which `|| true` keeps + # from tripping the `-e` that `shell: bash` sets. + while read -r key cred; do + case "${cred}" in + */git-credentials-*.config) + git config --local --unset-all "${key}" 2>/dev/null || true + rm -f "${cred}" + ;; + esac + done < <(git config --local --get-regexp '^includeIf\.gitdir:.*\.path$' || true) + + # The point of the step, asserted rather than assumed. Plain `git config` + # reads the effective configuration, following any `includeIf` still in + # place, so this fails if either branch above missed. Without it a + # checkout version bump that moves the credential again turns this step + # back into a silent no-op -- which is exactly how v7 slipped past a + # first attempt at this. + if git config --get-all "${HEADER_KEY}" >/dev/null 2>&1; then + echo "::error::checkout credentials still readable after cleanup" + exit 1 fi + echo "checkout credentials dropped"