Skip to content

Record why the npm-audit checkout is not hardened, and what CodeQL actually scans - #247

Open
nursoda wants to merge 2 commits into
mainfrom
docs/reject-sparse-checkout-hardening
Open

Record why the npm-audit checkout is not hardened, and what CodeQL actually scans#247
nursoda wants to merge 2 commits into
mainfrom
docs/reject-sparse-checkout-hardening

Conversation

@nursoda

@nursoda nursoda commented Sep 1, 2026

Copy link
Copy Markdown
Member

CodeQL's actions/cache-poisoning/poisonable-step flags the npm i -g step in
npm-audit.yml, because the job runs in the context of the default branch while
checking out a ref from the matrix. REVIEW.md already recorded why the alert is
dismissed. It did not record why the obvious hardening — a sparse-checkout narrowed
to the two files the job reads — was not applied, so the suggestion comes back at
every scan.

It comes back for a reason that turns out to be stronger than expected: a sparse
checkout does not close the path at all. package.json has to stay, because the
version step reads it, and its engines.npm is the value that reaches npm i -g.
The only thing a sparse checkout could remove is a root-level .npmrc, and even that
needs sparse-checkout-cone-mode: false, since cone mode always materialises the
repository root.

The same alert exposed a second inaccuracy: doc/developers.md called the default
setup frontend SAST for JavaScript, when it also runs the GitHub Actions queries — the
very ones that produced this finding. The second commit corrects that line.

Documentation only; no workflow or runtime file changes. The conditions that would
make the hardening worth revisiting are recorded next to the decision: a secret in
the job, an Actions cache, or a fork trigger.

🤖 Generated with Claude Code, verified, tweaked and approved by @nursoda.

CodeQL alert 4 flags the `npm i -g` step in `npm-audit.yml` as poisonable
because the job checks out a ref other than the default branch. The alert
is dismissed, and REVIEW.md already said why. What it did not say is why
the obvious hardening was not applied, so the question comes back at every
scan.

A `sparse-checkout` does not help here. `package.json` has to remain in the
checkout for the version step, and its `engines.npm` is the value that
reaches `npm i -g`; the only path a sparse checkout could remove is a
root-level `.npmrc`, and even that needs cone mode turned off, because cone
mode always materialises the repository root.

The conditions that would make the hardening worth doing are recorded with
it: a secret in the job, a cache, or a fork trigger.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Olav Seyfarth <olav@seyfarth.de>
@nursoda nursoda added documentation Improvements or additions to documentation: instructions, concepts, analyses security Related to a security vulnerability labels Sep 1, 2026
@nursoda nursoda self-assigned this Sep 1, 2026
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@nursoda
nursoda requested a review from seyfahni September 1, 2026 05:22
The developer notes described the default setup as frontend SAST for
JavaScript. It also runs the GitHub Actions queries, which is where the
cache poisoning alert on `npm-audit.yml` came from. Someone reading the
old line would not expect a workflow finding at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Olav Seyfarth <olav@seyfarth.de>
@nursoda nursoda changed the title Record why the npm-audit checkout is not hardened Record why the npm-audit checkout is not hardened, and what CodeQL actually scans Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation: instructions, concepts, analyses security Related to a security vulnerability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant