Skip to content

ci(osv): scan all tracked first-party lockfiles - #39452

Closed
EvilHumphrey wants to merge 1 commit into
NousResearch:mainfrom
EvilHumphrey:chore/osv-whatsapp-lock-20260605
Closed

ci(osv): scan all tracked first-party lockfiles#39452
EvilHumphrey wants to merge 1 commit into
NousResearch:mainfrom
EvilHumphrey:chore/osv-whatsapp-lock-20260605

Conversation

@EvilHumphrey

@EvilHumphrey EvilHumphrey commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Adds the two tracked first-party lockfiles OSV wasn't scanning — scripts/whatsapp-bridge/package-lock.json and plugins/platforms/photon/sidecar/package-lock.json — bringing explicit coverage to all five.

Rebased onto current main. osv-scanner.yml has moved twice since this branch was last refreshed, so the change is re-expressed against the workflow as it stands today — the pinned reusable-workflow form with an explicit scan-args: block — rather than replayed. Nothing else in the file is touched: the pinned action SHA, results-file-name, fail-on-vuln: false, the emit-status wrapper and the ci.yml invocation are all unchanged. This branch also predated 05c896cf5 ci: refactor paths & clones, so the stale trigger-filter half stays dropped rather than restored: osv-scanner.yml is workflow_call-only now and ci.yml invokes it with no needs/if gate, so the scan already runs on every PR and every push to main — there is nothing left to path-filter. The diff is one file: the two scan-args lines plus the two coverage comments.

Coverage is now every lockfile tracked in the repo:

lockfile tree
uv.lock Python
package-lock.json root npm workspace
website/package-lock.json docs site
scripts/whatsapp-bridge/package-lock.json newhermes-whatsapp-bridge
plugins/platforms/photon/sidecar/package-lock.json new@hermes-agent/photon-sidecar

optional-skills/finance/dcf-model/requirements.txt is the only other dependency file in the tree, and it stays out of scope deliberately: it pins ranges (openpyxl>=3.0.0, requests>=2.28.0), so it's a manifest rather than a lockfile and doesn't identify installed versions. Scanning ranged manifests is a separate policy call — happy to take it in a follow-up if you want it.

What this will surface

I re-queried both new lockfiles' resolved package sets against the OSV API immediately before this push. The result changed since the last refresh of this PR, and the change is the clearest argument for merging it.

WhatsApp bridge — 163 resolved packages, 1 advisory:

  • body-parser@1.20.5GHSA-v422-hmwv-36x6. Low, availability-only (an invalid limit value silently disables size enforcement). Transitive, via express@^4.21.0. Fixed in 1.20.6.

Photon sidecar — 133 resolved packages, 3 advisories:

  • @opentelemetry/core@2.7.1GHSA-8988-4f7v-96qf / CVE-2026-54285. Moderate, availability-only — W3CBaggagePropagator.extract() doesn't bound inbound baggage header size. Fixed in 2.8.0. Worth noting the top-level node_modules/@opentelemetry/core is already 2.8.0; the 2.7.1 copies are nested under six @opentelemetry/* packages.
  • protobufjs@8.6.1GHSA-j3f2-48v5-ccww, moderate, DoS via infinite loop in .proto option parsing. Fixed in 8.6.6.
  • protobufjs@8.6.1GHSA-jfj6-75fj-8934, moderate, Text Format string-map parsing can mutate the returned map's prototype. Fixed in 8.6.5.

Both protobufjs findings are transitive, via spectrum-ts@8.0.0.

Three of those four advisories were published on 2026-07-20 — after this PR was last refreshed. The resolved package sets did not change (163 and 133, same as before), so these are not new dependencies: they are newly-published advisories against versions that were already installed and already unscanned. That is precisely the "currently-pinned dep became known-vulnerable" case this workflow's own header says it exists to catch, and it went undetected here for five days because these two lockfiles aren't in scan-args.

Detection-only and unchanged: fail-on-vuln: false, so the job stays green and the all-checks-pass required-check story is untouched — findings land in the Security tab as alerts, not merge blocks. Bumping any of these is a separate change; happy to open those if you want them.

(The overrides block in the sidecar's package.json doesn't interfere: the npm extractor reads resolved versions out of the lockfile's packages map, so OSV matches what's actually installed.)


One note on the scan-args contract, since it constrains future PRs: OSV-Scanner hard-errors on a missing --lockfile path (failed to resolve path), and fail-on-vuln: false does not suppress that — it only suppresses findings. So any PR that deletes or renames one of these five packages needs to drop its --lockfile line in the same change. I deliberately did not add "skip if absent" handling, since that would silently weaken the coverage this PR is promising. All five paths are verified present on main as of this push.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening area/nix Nix flake, NixOS module, container packaging P3 Low — cosmetic, nice to have labels Jun 5, 2026
@EvilHumphrey

Copy link
Copy Markdown
Contributor Author

The failing osv-scan here isn't a regression from this change.

This PR adds scripts/whatsapp-bridge/package-lock.json to the scan (its intended purpose), so the scan now reports more packages than before — but none of the newly-covered whatsapp-bridge npm deps are Critical.

The only Critical failing the gate is aiohttp GHSA-63hf-3vf5-4wqf (9.1) in uv.lock (pinned at aiohttp 3.13.3), which is unrelated to this PR and fixed by #39467 (aiohttp3.14.0; its own osv-scan is green, 0 Critical).

Suggested order:

  1. Merge chore(deps): bump aiohttp to clear security advisories #39467 first.
  2. Rebase this PR — the osv-scan red then clears. The remaining High/Medium advisories are report-only; the gate fails on Critical.

@EvilHumphrey

Copy link
Copy Markdown
Contributor Author

Rebased onto current main — this resolves the conflict and shrinks the diff.

What it does: adds the first-party whatsapp-bridge lockfile (scripts/whatsapp-bridge/package-lock.json, ~76 KB, previously unscanned) to the OSV scan — as a --lockfile= arg and in the push paths filter — and bumps the workflow comment from "three sources of truth" to "four."

Why the diff is smaller than when this was opened: main has since dropped the pull_request paths filter on this workflow so the required check always runs. I deliberately did not re-add a paths filter there; only the push filter and the scan-args needed the new lockfile. Net change is +3/−1 in .github/workflows/osv-scanner.yml.

Scope: detection-only — fail-on-vuln: false is unchanged, so this can't block any PR; it only extends coverage to a first-party lockfile. No dependency or remediation changes are bundled.

The earlier failing check was a SARIF-upload rate-limit flake (the scanner steps passed; only the code-scanning upload step failed) — the re-run triggered by this push should clear it.

@teknium1 teknium1 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.

Thanks for extending OSV coverage. The WhatsApp bridge lockfile is a real current gap: .github/workflows/osv-scanner.yml:43-46 still lists only uv.lock, the root npm lockfile, and website/package-lock.json.

Problems

  • The proposed fourth entry still leaves the tracked first-party Photon sidecar lockfile uncovered. plugins/platforms/photon/sidecar/package.json:2-16 identifies an @hermes-agent/photon-sidecar npm package, and its tracked package-lock.json is not in the explicit OSV list at .github/workflows/osv-scanner.yml:43-46.
  • The trigger-filter portion is stale. Current CI owns pull_request/push in .github/workflows/ci.yml:13-16 and invokes the reusable scanner at .github/workflows/ci.yml:126-128; GitHub currently reports this PR as conflicting.

Suggested changes

  • Carry the scan-args addition onto current main without restoring direct workflow path filters, and add the Photon sidecar lockfile alongside the WhatsApp bridge lockfile.
  • Update the coverage comment to describe all five tracked first-party lockfiles.

Automated hermes-sweeper review.

Comment thread .github/workflows/osv-scanner.yml Outdated
scan-args: |-
--lockfile=uv.lock
--lockfile=package-lock.json
--lockfile=website/package-lock.json
--lockfile=scripts/whatsapp-bridge/package-lock.json

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.

Please include --lockfile=plugins/platforms/photon/sidecar/package-lock.json alongside this. Current main tracks an active first-party Photon sidecar lockfile, while the workflow's explicit list omits it; otherwise the revised “four sources of truth” comment still leaves an npm dependency tree unscanned.

@teknium1 teknium1 added sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@EvilHumphrey
EvilHumphrey force-pushed the chore/osv-whatsapp-lock-20260605 branch from fd668c1 to 41031fe Compare July 14, 2026 04:34
@EvilHumphrey EvilHumphrey changed the title ci(osv): scan whatsapp-bridge lockfile ci(osv): scan all tracked first-party lockfiles Jul 14, 2026
@EvilHumphrey

Copy link
Copy Markdown
Contributor Author

Both points addressed — force-pushed onto current main.

Photon sidecar lockfile — added. scan-args now covers all five tracked first-party lockfiles, and the coverage comment says so.

Stale trigger filters — dropped, not replayed. The old pull_request/push paths: hunk is gone rather than carried forward: ci.yml:126-128 invokes this workflow with no needs/if gate, so OSV already runs on every PR and every push to main — there's nothing left to path-filter. The diff is down to a single hunk (+4/−2), and the conflict with main is cleared.

Worth flagging before merge: enabling the sidecar scan surfaces one existing advisory — @opentelemetry/core@2.7.1, GHSA-8988-4f7v-96qf / CVE-2026-54285 (moderate, availability-only, fixed in 2.8.0). It was published 2026-06-15, after this PR was opened, which is a decent argument for the coverage. fail-on-vuln: false is unchanged, so it lands as a Security-tab alert, not a merge block. The bridge lockfile itself came back clean. Bumping otel is a separate PR — happy to open it if you want.

I left optional-skills/finance/dcf-model/requirements.txt out of scope: it specifies ranges (openpyxl>=3.0.0, requests>=2.28.0), so it's a manifest rather than a lockfile and doesn't identify installed versions. Glad to fold it in if you want ranged manifests scanned too.

@EvilHumphrey
EvilHumphrey force-pushed the chore/osv-whatsapp-lock-20260605 branch from 41031fe to ba9c060 Compare July 19, 2026 01:23
@EvilHumphrey

EvilHumphrey commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current maincfb9459 (#66410) inlined the reusable workflow's steps in place of the uses: call, which is what this branch's previous head conflicted with. Same one-hunk change, re-expressed against the inlined Run scanner step: the two added --lockfile= args plus the coverage comment.

Re-ran the OSV querybatch pre-flight against both lockfiles at today's main: the WhatsApp bridge (163 resolved packages) is still clean, and the Photon sidecar (133) still surfaces exactly one moderate advisory — the nested @opentelemetry/core@2.7.1 under exporter-logs-otlp-http (GHSA-8988-4f7v-96qf; the top-level copy is already at 2.8.0). Detection-only as before: --fail-on-vuln=false (now on the reporter step) keeps the finding in the Security tab rather than blocking merges.


On the two red checks: the only failing job is Python lints / CI-sensitive file review (and All required checks pass inheriting it) — the new policy gate that requires a maintainer-applied ci-reviewed label on any PR touching workflow files, which this PR necessarily does. Its explanatory bot comment is skipped on fork PRs, hence this note. Every substantive check is green, including the OSV scan running the expanded five-lockfile args. Ready for review whenever someone gets a chance to look + label.

@EvilHumphrey
EvilHumphrey force-pushed the chore/osv-whatsapp-lock-20260605 branch from ba9c060 to f90632d Compare July 25, 2026 23:21
@EvilHumphrey

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (9de7dfe) — the conflict is cleared.

osv-scanner.yml changed shape twice since this branch was last refreshed, so rather than replay the old diff I re-expressed the change against the workflow as it stands now: two --lockfile= lines added to the existing scan-args: block, plus the coverage comments. The pinned action SHA, results-file-name, fail-on-vuln: false, the emit-status wrapper and the ci.yml invocation are all untouched, and the stale trigger-path filters stay dropped. One file, +8/−4.

I re-ran the OSV pre-flight against both new lockfiles before pushing, and the result moved in a way that seems worth flagging:

lockfile packages advisories
scripts/whatsapp-bridge/package-lock.json 163 1 — body-parser@1.20.5 (low)
plugins/platforms/photon/sidecar/package-lock.json 133 3 — @opentelemetry/core@2.7.1 (moderate), protobufjs@8.6.1 ×2 (moderate)

Three of those four were published on 2026-07-20, and the resolved package sets are unchanged (163 / 133). So no dependency moved — these are newly-published advisories against versions that were already installed, in the two lockfiles CI isn't scanning. They went unnoticed for five days. That's the case this PR is making, and I'd rather show it than assert it. All are transitive and detection-only; fail-on-vuln: false is unchanged, so nothing here blocks a merge. Details and fixed-in versions are in the updated description.

The only red check is Python lints / CI-sensitive file review, which is red by design — this touches a workflow file, so it needs a maintainer-applied ci-reviewed label before it can go green. Everything else passes. Could someone take a look and apply the label if the change looks right? Happy to adjust anything.

Also still on offer, as separate PRs if wanted: bumping @opentelemetry/core to 2.8.0, protobufjs to 8.6.6, and body-parser to 1.20.6.

@EvilHumphrey

Copy link
Copy Markdown
Contributor Author

Closing: the coverage this PR argued for is now on main. f21332f07 (merged 2026-08-01, a maintainer reapply of #46747 by @tank321, fixing #46738) added the same two lockfiles — plugins/platforms/photon/sidecar/package-lock.json and scripts/whatsapp-bridge/package-lock.json — to scan-args, so all five tracked first-party lockfiles are scanned. Re-verified against today's main: 5 --lockfile= entries, all paths present, and a fresh OSV querybatch run over both added lockfiles comes back clean (the advisories this PR's description cited have since been patched upstream — 19c771017, 7537de9e7, 8789cf9f0).

One cosmetic residue, not worth a PR that would trip the ci-reviewed workflow gate on its own: the header comment on osv-scanner.yml still says "Scans lockfiles (uv.lock, package-lock.json)…" — a rider for the next workflow-touching change.

Thanks to everyone who reviewed along the way.

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

Labels

area/nix Nix flake, NixOS module, container packaging P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants