Skip to content

QVAC-23347 infra: stop persisting git credentials in wrapper publish jobs - #3795

Open
covert-oddity wants to merge 2 commits into
mainfrom
fix/QVAC-23347-persist-credentials-publish-jobs
Open

QVAC-23347 infra: stop persisting git credentials in wrapper publish jobs#3795
covert-oddity wants to merge 2 commits into
mainfrom
fix/QVAC-23347-persist-credentials-publish-jobs

Conversation

@covert-oddity

Copy link
Copy Markdown
Contributor

Problem

The 18 credentialed publish jobs across the nine on-merge-*.yml TypeScript wrapper pipelines check out without persist-credentials: false. actions/checkout therefore writes the checkout token into .git/config as an http.extraheader, where it stays readable by any process in the job for the job's lifetime. (Confirmed the default is still true at the pinned revision de0fac2e4500dabe0009e67214ff5f5447ce83dd.)

What sits in those jobs:

exposure count where
secrets.PAT_TOKEN persisted 8 jobs / 5 workflows diffusion-cpp, vla, llm-llamacpp (both publish jobs each); ocr-ggml, translation-nmtcpp (publish-npm only)
GITHUB_TOKEN persisted the other 10 jobs four workflows use the default checkout token
contents: write 11 of 18 push capability, not just read
id-token: write 9 jobs every publish-npm (needed for npm publish --provenance)

Each of these jobs also runs npm install --ignore-scripts --package-lock=false followed by a tsc invocation, so every package in the devDependency closure — eslint, typescript-eslint, brittle, cmake-bare, cmake-vcpkg and everything transitive — gets code execution in a job where that credential is on disk. --ignore-scripts blocks install-time hooks but does nothing once tsc runs.

Two notes that refine the original write-up, both in the direction of more exposure:

  • PAT_TOKEN reaches 8 jobs, not 4. Counting files and only looking at publish-npm undercounts: in diffusion-cpp, vla and llm-llamacpp the release-environment publish-gpr job also checks out with PAT_TOKEN.
  • id-token: write is not in every step. Job-level permissions: fully replaces the workflow-level block, and every publish-gpr omits id-token, so OIDC minting is confined to the nine publish-npm jobs.

What this PR does

Adds persist-credentials: false to all 18 checkouts. Nothing else. +23 lines, purely additive.

This is the cheapest independent cut and lands on its own. Removing the execution surface itself — the npm install + tsc pair, which is redundant because every package commits its tsc output — is a separate PR so this one stays trivially reviewable.

Why dropping the credential is inert here

None of these 18 jobs performs an authenticated git operation:

  • .github/actions/publish-library-to-gpr — only a local git rev-parse --short HEAD (whose SHORT_SHA is assigned and never used), then npm publish with NODE_AUTH_TOKEN.
  • check:generated (llm-llamacpp) — only git diff --exit-code, no auth needed.
  • tetherto/qvac-actions/publish-library-to-npm@38693d93 — sparse-checks out its gate into a separate _qvac-actions-gate path and publishes via OIDC --provenance.
  • Tag creation is a separate job, create-release-tag.yml, using the REST API with GITHUB_TOKEN — not a git push from these jobs.

Verification

  • YAML re-parse confirms 18 set / 0 missing across the nine files.
  • node .github/scripts/lint-workflows.mjs on all nine: actionlint structural gate clean.
  • yamlfmt reports pre-existing formatting debt in .github/workflows (untouched pr-gate-merge.yml reports the same); the only yamlfmt run these files trigger, on-pr-ocr-ggml.yml:142, is scoped to workdir: packages/ocr-ggml and is continue-on-error: true, so it never sees these paths.

Precedent

persist-credentials: false is already used in 20+ workflows here, including on-pr-classification-ggml.yml, on-pr-diffusion-cpp.yml, on-pr-ocr-ggml.yml and on-pr-vla.yml.

Asana: QVAC-23347. Distinct from the completed H1 (label-gated fork checkout), C2 (ungated secrets) and M5 (continue-on-error) work — this is post-merge publish-time exposure, not PR-time fork exposure.

…jobs

The 18 credentialed publish jobs across the nine on-merge-*.yml TypeScript
wrapper pipelines check out without `persist-credentials: false`, so the
checkout token is written into .git/config as an http.extraheader and stays
readable by any process in the job. Eight of those checkouts use
secrets.PAT_TOKEN; the rest persist GITHUB_TOKEN, and 11 of the 18 jobs hold
`contents: write`.

Each of these jobs also runs `npm install --ignore-scripts` followed by a tsc
invocation, so every package in the devDependency closure gets execution in a
job where that credential sits on disk. Removing the execution surface is
tracked separately; this is the cheapest independent cut and lands on its own.

Verified none of these jobs performs an authenticated git operation, so
dropping the credential is inert:

  - .github/actions/publish-library-to-gpr only runs a local `git rev-parse`
    (its SHORT_SHA is assigned and never used) plus `npm publish` with
    NODE_AUTH_TOKEN.
  - `check:generated` only runs `git diff --exit-code`.
  - tetherto/qvac-actions/publish-library-to-npm sparse-checks out its gate
    into a separate `_qvac-actions-gate` path and publishes with OIDC.
  - Tag creation is a separate job (create-release-tag.yml) using the REST
    API with GITHUB_TOKEN, not a git push.

Existing precedent for this flag in the repo: 20+ workflows, including
on-pr-classification-ggml.yml, on-pr-diffusion-cpp.yml, on-pr-ocr-ggml.yml
and on-pr-vla.yml.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

License compliance — clean

No new dependency license findings in this PR.

Warn-only (shadow) mode — this check does not block merges yet.

Updated automatically by the canonical license compliance workflow.

NOTICE presence (advisory)

Missing NOTICE (advisory, does not block):

  • ./.github/actions/release-merge-guard
  • ./docs/website
  • ./packages/ggml-coload-smoke
  • ./packages/fabric/test/integration
  • ./packages/inference-addon-cpp/mobile
  • ./packages/sdk/e2e
  • ./packages/llm-llamacpp/benchmarks/performance
  • ./packages/llm-llamacpp/benchmarks/server
  • ./packages/vla-ggml/sim/server
  • ./packages/embed-llamacpp/benchmarks/performance
  • ./packages/embed-llamacpp/benchmarks/server
  • ./packages/asr-ggml/benchmarks/server

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Workflow security (shadow mode)

zizmor found 1090 finding(s) in .github/ (highest severity: high). This check is warn-only and does not block the merge.

Findings are annotated inline on the changed files and listed in the job summary.

Reproduce locally:

pipx run zizmor==1.27.0 --offline .github/

@github-actions

Copy link
Copy Markdown
Contributor

Review Status

Current Status: ❌ PENDING
Approvals so far: none

Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member.

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.

1 participant