Skip to content

Bugs in OSV-Scanner workflow and helper scripts, found while porting to kubeflow/mcp-server #748

Description

@Kartikeya-trivedi

What happened?

I recently ported this repo's .github/workflows/osv-scanner.yaml and its helper scripts into kubeflow/mcp-server (kubeflow/mcp-server#30, merged and now running nightly there — its auto-fix pipeline has already landed a real security bump, kubeflow/mcp-server#137). The review rounds on that port surfaced several bugs that all still exist in the SDK's original copy on main:

1. update_overrides.py writes invalid TOML when pyproject.toml has no pre-existing [tool.uv] section
The creation path appends a [tool.uv] + header, and the insertion logic below adds a second header + override-dependencies block — producing duplicate keys that uv lock rejects (Cannot overwrite a value). Repro:

printf '[project]\nname = "x"\nversion = "0.1.0"\n' > pyproject.toml
python3 .github/scripts/update_overrides.py requests "requests==2.31.0" "2026-01-01" "https://example.com/adv"
python3 -c "import tomllib; tomllib.load(open('pyproject.toml','rb'))"   # -> TOMLDecodeError

2. update_overrides.py misses an indented override-dependencies key (valid TOML), producing a duplicate key
The removal regexes are anchored at column 0 (r"^override-dependencies...", r"^# Security overrides..."), so an indented key survives removal and the rewritten block is appended alongside it → invalid TOML. Fix is ^[ \t]* (not ^\s*, which also eats newlines).
Note: #573 touches the same pattern (trailing-comment tolerance) but keeps the ^ anchor, so both changes compose cleanly.

3. labels: "area/security" in the auto-fix PR step creates a label with literal quotes
peter-evans/create-pull-request splits the labels input on [\n,]+ and does not strip quote characters, so the YAML literal block's "area/security" becomes a malformed label named "area/security" (quotes included).

4. continue-on-error: true on the SARIF scan step swallows real scanner crashes
The step's own error handling re-exits 1 on unexpected codes (e.g. exit 2 = scanner crash), but continue-on-error neutralizes it — a genuine crash leaves the job green. Replacing it with if: always() on the following JSON-scan step keeps the intended "SARIF failure doesn't block auto-fix" behavior while making crashes fail the job.

5. Auto-fix PR body renders a broken table
The PR body declares a two-column markdown table header, but fix_details lines are emitted as - pkg | [id](url) bullets — bullets don't render as table rows. Emitting | pkg | [id](url) | rows fixes the rendering.

6. (minor) compare_versions.py depends on packaging being transitively present
Under uv run a missing packaging import exits 1, which the fixer loop misreads as "natural upgrade sufficient" — silently skipping a needed override. uv run --with packaging makes the dependency explicit. Related: echo "$TREE_OUTPUT" is safer as printf '%s\n' "$TREE_OUTPUT" for multi-line tree output.

7. The scanner pin is stale: 2.3.8, current release is v2.5.0
The pin + EXPECTED_SHA pattern is right, but nothing alerts when it goes stale — the file comment says "update version + checksum manually" and that hasn't happened across two releases. In mcp-server this is being addressed with a weekly auto-bump workflow (kubeflow/mcp-server#90) that pulls the new checksum from the release's official SHA256SUMS, cross-verifies it against the downloaded binary, and opens a PR for review — happy to contribute that here as a follow-up once it merges there.

What did you expect to happen?

  • update_overrides.py always produces valid TOML (both fresh and indented-key cases)
  • A scanner crash (exit >= 2) fails the workflow visibly
  • The auto-fix PR gets a correctly named area/security label and a properly rendered changelog table
  • The scanner pin either stays current or something alerts when it doesn't

All of the above are fixed in kubeflow/mcp-server (merge commit kubeflow/mcp-server@e89648a, plus regression tests for the two TOML cases in its test_scripts.py). I'm happy to send the mirror PR with the fixes + tests — filing this first for visibility and to coordinate with #573, which touches the same regex in update_overrides.py.

Environment

N/A — CI workflow and repo scripts (.github/workflows/osv-scanner.yaml, .github/scripts/update_overrides.py), not the SDK runtime.

Kubernetes version: N/A
Kubeflow Trainer version: N/A
Kubeflow Python SDK version: N/A

Impacted by this bug?

Give it a 👍 We prioritize the issues with most 👍

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions