Skip to content

Close PYSEC-2026-2132 and pin the Node the frontend is built with - #11206

Open
MarkusNeusinger wants to merge 2 commits into
mainfrom
chore/deps-click-node-pin
Open

Close PYSEC-2026-2132 and pin the Node the frontend is built with#11206
MarkusNeusinger wants to merge 2 commits into
mainfrom
chore/deps-click-node-pin

Conversation

@MarkusNeusinger

Copy link
Copy Markdown
Owner

Summary

  • click 8.3.1 → 8.3.3 closes PYSEC-2026-2132. It is the only advisory pip-audit reports against the resolved runtime dependency set, and the audit comes back clean afterwards. click is transitive, so the fix is a lock-file bump with no pyproject.toml constraint (CLAUDE.md, "Package Management").
  • The frontend now declares the Node it is actually built with, and something enforces it. app/package.json asked for node >=20 while app/Dockerfile builds the deployed bundle on Node 22 and CI tests on Node 24 — so the only version the manifest still admitted was the one nothing tests and that reached end of life in April 2026. Floor → >=22, plus app/.nvmrc (22) and app/.npmrc (engine-strict=true).
  • Transferred from the sibling repo kurrentschrift, where a Node 20 install went through and then failed deep inside the build with node: bad option.

Evidence

Before, in this branch's base:

$ uv export --no-dev --no-emit-project -o req.txt && uvx pip-audit -r req.txt
Found 1 known vulnerability in 1 package
Name  Version ID              Fix Versions
----- ------- --------------- ------------
click 8.3.1   PYSEC-2026-2132 8.3.3

After uv lock --upgrade-package click==8.3.3:

$ uv export --no-dev --no-emit-project -o req.txt && uvx pip-audit -r req.txt
No known vulnerabilities found

That is the whole advisory list — no other package in the runtime set has one.

Decisions worth stating

  • 8.3.3, not 8.5.0. uv lock --upgrade-package click alone resolves to 8.5.0. The minimal bump is what clears the advisory; a minor bump of the library behind every console script (uvicorn, typer, the automation CLIs) belongs in its own Dependabot PR with its own CI run, not folded into a security patch.
  • The lock carries four unrelated marker rewriteshttpcore2, plotnine (three resolution branches) and secretstorage gain sys_platform/python_full_version markers on dependencies that had none. No version moves; this is uv 0.10.9 normalising markers the previously-recorded lock wrote differently. Reverting it by hand would leave uv.lock different from what uv lock produces on the next run.
  • engines moves too, not just .nvmrc. With >=20 still declared, engine-strict would enforce a floor that the untested, EOL version satisfies — the pin would look present and check nothing.

Test plan

  • uvx pip-audit over uv export --no-dev — 1 finding before, 0 after (output above).
  • yarn install --frozen-lockfile on Node 22 in app/ — resolves unchanged, engines satisfied.
  • yarn fm:check — "All matched files use Prettier code style!" (.nvmrc/.npmrc have no Prettier parser and are not matched).
  • yarn lint — clean.
  • No Python source changed, so ci-lint and ci-tests skip their Python jobs by design; the frontend job runs on the app/ change.

Checklist

  • CHANGELOG.md updated under [Unreleased] — a ### Security bullet for the advisory and a ### Changed bullet for the Node pin.
  • No behaviour documented elsewhere changes; app/.npmrc carries its own rationale in comments.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke

`pip-audit` over the resolved runtime dependency set (`uv export --no-dev`)
reported one advisory: PYSEC-2026-2132 against `click` 8.3.1. `click` is
transitive, so the fix is a lock-file bump with no `pyproject.toml`
constraint — 8.3.3, the minimal version that clears it. The audit comes back
clean afterwards. `click` 8.5.0 exists and is deliberately left to
Dependabot: a minor bump of the library behind every console script deserves
its own PR and its own CI run, not a ride on a security patch. The lock also
picks up uv 0.10.9's marker normalisation on four unrelated packages
(httpcore2, plotnine x3, secretstorage) — no version moves, and reverting it
by hand would leave the file different from what `uv lock` produces.

The frontend half: `app/package.json` asked for `node >=20` while the image
that produces the deployed bundle builds on Node 22 and CI tests on Node 24.
The only version the manifest still admitted was the one nothing tests and
that reached end of life in April 2026. The floor moves to `>=22`,
`app/.nvmrc` names 22 for `nvm use` and `setup-node`, and `app/.npmrc` sets
`engine-strict=true` so an npm install in `app/` refuses an unsupported
runtime at install time instead of failing later inside the build with a
message that never names the version. yarn 1, the app's own package manager,
checks `engines` itself; the `.npmrc` covers everyone who reaches for npm.

Same pin as the sibling repo kurrentschrift, where a Node 20 install failed
deep inside the build with "node: bad option".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI balanced review requested due to automatic review settings September 2, 2026 21:04
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The declared Node.js floor is incompatible with Vite’s requirement, and related documentation and changelog issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Closes PYSEC-2026-2132 by upgrading Click and strengthens frontend Node.js version enforcement.

Changes:

  • Upgrades Click to 8.3.3.
  • Raises and enforces the frontend Node.js requirement.
  • Documents the security and runtime changes.
File summaries
File Description
uv.lock Upgrades Click and normalizes dependency markers.
CHANGELOG.md Records the changes but needs accuracy and PR-reference corrections.
app/package.json Raises the Node.js floor, which must align with Vite’s Node 22.12+ requirement.
app/.nvmrc Selects Node.js 22 for local environments.
app/.npmrc Enables strict engine enforcement and configures npm behavior.
Review details

Suppressed comments (5)

CHANGELOG.md:181

  • This claim is inaccurate: >=20 admitted every later Node release, not only Node 20. Rephrase it to say that the range still admitted the untested, EOL Node 20 line; otherwise the changelog records a false description of the previous contract.
  the deployed bundle builds on Node 22 and CI tests on Node 24, so the only version the
  manifest still admitted was the one nothing tests and that reached end of life in April
  2026. The floor moves to `>=22`, `app/.nvmrc` names 22 for `nvm use` and `setup-node`,

CHANGELOG.md:185

  • The changelog contract at CHANGELOG.md:8-9 requires every entry to carry its PR reference. Append (#11206) to this entry.
  `engines` itself). Same pin as the sibling repo kurrentschrift.

CHANGELOG.md:195

  • The changelog contract at CHANGELOG.md:8-9 requires every entry to carry its PR reference. Append (#11206) to this entry.
  own CI run.

CHANGELOG.md:181

  • This says .nvmrc supplies the version to setup-node, but the frontend workflow still passes node-version: '24' directly (.github/workflows/ci-tests.yml:208-214) and no workflow uses node-version-file. Either remove the setup-node claim or configure a workflow to consume this file.
  2026. The floor moves to `>=22`, `app/.nvmrc` names 22 for `nvm use` and `setup-node`,

app/package.json:21

  • Raising the engine floor makes the development guide incorrect: docs/development.md:10 still lists “Node.js 20+” as a prerequisite, but the documented yarn install now rejects Node 20. Update that prerequisite to Node.js 22+ in this PR so the setup path remains usable.
    "node": ">=22"
  • Files reviewed: 4/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread app/package.json Outdated
Copilot review: `>=22` advertised Node 22.0-22.11 as supported, but the
locked Vite 8.0.16 (and rolldown) declare `^20.19.0 || >=22.12.0`, so those
runtimes pass the project-level check and are then rejected by Vite's own
dependency-level engine check - the exact late, confusing failure this pin
exists to prevent. The floor becomes `>=22.12.0`.

`docs/development.md` still said "Node.js 20+" in its prerequisites and now
matches; it also points at `.nvmrc` and says to take the latest 22.x, because
the dev-only test stack reaches higher still (jsdom wants `^22.22.2`). The
declared floor deliberately tracks the build toolchain rather than a
transitive dev dependency's patch version, which would need editing on every
bump; `app/.npmrc` records that reasoning.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 21:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The lock-only security update currently bypasses Python CI and must be covered before approval.

Review details

Suppressed comments (1)

app/.npmrc:10

  • This rationale incorrectly includes npm ci: app/ has no package-lock.json, so npm ci cannot restore this project, and npm does not reliably apply engine-strict to the root package during npm ci. Limit this claim to npm install (or add an explicit version-check script if npm ci must be covered).
# yarn 1 checks `engines` on its own. This file covers the other half: anyone
# who reaches for npm in `app/` — a quick `npm ci`, a tool that shells out to
# npm — gets the same refusal instead of a silently unsupported toolchain.
  • Files reviewed: 5/6 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

MarkusNeusinger added a commit that referenced this pull request Sep 2, 2026
Merge resolves the CHANGELOG conflict by keeping both Added entries (#11205
and #11206 landed on main while this branch was open).

Copilot review: _candidate_paths treated a whole code span as one token, so
'.claude/commands/ -> ../agentic/commands/' - a sentence about two paths -
was not path-shaped and went unchecked as a whole. Spans are now split on
whitespace before the shape test; fragments that are not path-shaped (the
arrow, a command word, a flag) drop out, which is what makes the split safe.
A '../'-relative fragment is skipped rather than resolved against the
repository root, where it would mean something else entirely.

That leaves the arrow's meaning unpinned, and the sharper failure is the one
where somebody replaces the symlink with a real directory: both ends still
exist, the guide still reads true, and commands written on either side
quietly stop matching the other. So the symlink gets its own test, which
follows the link instead of matching its text.

(The specific example was in fact already covered, because
'.claude/commands/' appears standalone later in the same sentence - but the
hole in the heuristic was real.)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
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.

2 participants