chore(security): automated monthly SBOM & VEX report

[Gustavohsdp]

What this PR adds

Installs the automated software supply-chain security pipeline for openplc-runtime (hybrid C/C++ core + Python Flask web server). Three flows, all driven by the same data:

• Monthly SBOM & VEX report — .github/workflows/security-monthly.yml. On the 1st of each month: resolves requirements.txt into a clean venv and builds the SBOM with cyclonedx-py (CycloneDX 1.6 + SPDX 2.3), scans against OSV honoring the osv-scanner.toml VEX baseline, renders a report from security/report-config.json, opens a PR with the dated snapshot under security/<YYYY-MM>/. Runs against the default branch (main).
• PR security gate — .github/workflows/security-pr-gate.yml. On every PR: fails only when the PR introduces a NEW HIGH/CRITICAL advisory (pre-existing never blocks). Resolves base vs head SBOMs; skips the base resolve when requirements.txt is unchanged. Posts a sticky comment with package, version, advisory link, fixed version, and how to resolve.
• Archive on merge — .github/workflows/security-pr-archive.yml. On merge, commits the SBOM + report under security/pr-<number>-<date>/.

Supporting files: scripts/ (Python SBOM generator, report renderer, OSV diff/gate), osv-scanner.toml (VEX baseline), security/report-config.json, security/README.md.

Security posture (assessment)

45 Python components (the single third-party C library, SOEM, is a pinned git submodule with no known advisory, tracked separately). All 5 Python advisories are not applicable (test/dev tooling, or functions the running Flask server never calls): 0 require remediation, 0 exploitable.

⚠️ Licensing note (material for IP/legal)

The runtime is MIT, but the optional EtherCAT plugin links SOEM, which is GPLv3-or-commercial (RT-Labs). Under the open-source option the plugin becomes GPLv3; a commercial product likely needs a purchased SOEM license. Affects only the EtherCAT plugin — the MIT core and other plugins are unaffected. Flagged for acquisition due diligence.

What to review

• osv-scanner.toml + security/report-config.json — the VEX triage + the SOEM licensing note. Best reviewed by product security (Thiago).
• .github/workflows/security-*.yml — the three flows.

Notes

• Not enforced yet: the gate reports pass/fail but does not block merges until set as a required status check.

[Gustavohsdp]

Review feedback addressed ✅

Thanks for the thorough review. Every point raised has been addressed across follow-up commits — the gate check is green and the branch is up to date.

Correctness / security bugs

• PR gate now fails closed — a scanner error (missing/invalid JSON, or an exit code other than 0/1) hard-fails the step instead of silently passing (“no new vulnerabilities”) or blocking every advisory.
• Severity — MEDIUM is normalized to MODERATE (no longer blocks as HIGH); CVSS 4.0 is treated conservatively and flagged; the CVSS 3.1 base-score formula cites the spec (§7.1).
• Diff keyed by advisory id + package — the same CVE on a new package now counts as introduced; a version bump of an already-vulnerable package does not.
• Report HTML injection — config strings are escaped (only attribute-less <b>/<code>/… re-enabled) and the PDF renders with --disable-javascript.
• SPDX — components are de-duplicated by ref (no more two packages sharing one SPDXID); multiple licenses join with OR (dual-licensed), not AND.
• gen-osv-ignores requires a verdict column and refuses to run without one (can no longer suppress the entire scan).
• Monthly scan / scan-vulns fail closed too — a broken scan can no longer produce a clean report.

Supply-chain / hardening

• osv-scanner pinned to v2.4.0 and verified against its SHA256SUMS; cdxgen / cyclonedx-py pinned to exact versions.
• Inline node -e CSV builder extracted to scripts/cdx-to-csv.mjs.
• Unit tests (node:test) for the gate diff + SPDX, executed inside the gate.
• Report methodology corrected to OSV / osv-scanner (was “npm/pnpm/pip audit”).

Report & archive integrity

• The report’s headline numbers are now derived from the live scan (raw vs. VEX-suppressed vs. surfacing), so it cannot attest stale figures.
• The per-PR archive moved to a push-on-default-branch trigger (writable token — works for merged fork PRs) that stores SBOM + raw scan only on a dedicated orphan security-archive branch — no repo/history bloat and no false attestation.
• The monthly PR uses secrets.SECURITY_BOT_TOKEN (with GITHUB_TOKEN fallback) so its own checks can run.

Since this is a public repository, the optional Claude/OAuth triage step and its token guidance were removed — the monthly report is deterministic + human-reviewed.

Two notes

1. The monthly and archive workflows run on cron / push-to-default, so they execute for the first time after merge — the deterministic pieces are validated locally and the gate is green, but they don’t run on this PR itself.
2. To make the monthly PR trigger checks, a one-time SECURITY_BOT_TOKEN secret (a GitHub App installation token or a fine-grained PAT with contents + PR write) is needed; without it the PR still opens, just without checks.

Ready for another look 🙏

[marconetsf]

Reviewed the full diff plus the actual CI logs for this branch. The engineering is solid and the fail-closed reasoning is above average for this kind of pipeline, but there are a few things I think need to be resolved before merge — the first one only shows up in the run logs, so starting there.

Blocking

1. The entire VEX baseline is dead, and the generated report will contradict itself

From run 30915809962 (the latest gate run on this branch):

Scanned .../sbom/openplc-runtime.cdx.json file and found 41 packages
osv-scanner.toml has unused ignores:
 - GHSA-58qw-9mgm-455v
 - GHSA-5rjg-fvgr-3xxf
 ... (all 23 IDs)

23 of 23 suppressions are unused. Because requirements.txt is unpinned, resolution has already moved to fixed versions, so the current scan finds zero advisories across the 41 packages.

That breaks the report internally. build-report.mjs:632-640 computes the "Key metrics" table live from the scan, so it will print 0 raw advisories, while report-config.json still asserts in headline that "its 5 advisories are all either in test tooling or in vulnerable functions the runtime never calls", with notAffected listing 5 and criticalNote restating it. The static advisories.total: 5 also feeds the vulnerabilities.csv row count at build-report.mjs:717, which is generated from the live scan.

A compliance deliverable that contradicts its own live numbers is worse than no deliverable. Either the narrative fields have to be derived from the scan too, or the config has to be regenerated as part of the same run that renders the report.

2. Suppressions have no version constraint, so they fail open silently

[[IgnoredVulns]] matches on advisory ID only. If a future resolution (or an explicit pin) brings back the vulnerable version of python-dotenv, click, pytest or filelock, the advisory stays suppressed until 2026-10-01 — and since the PR gate reads the same TOML, it will not block either. Suggest trimming the baseline to what the scanner actually reports today (apparently nothing) and pinning requirements.txt, which also removes the drift that caused item 1.

3. PYSEC/GHSA aliasing causes false HIGH blocks

severityOf() (scripts/pr-gate-diff.mjs:1193-1212) only reads database_specific.severity. PYSEC records typically carry neither that field nor a CVSS vector, so they fall through to case 4 and return { sev: 'HIGH', approx: true }. With GATE_THRESHOLD=HIGH that blocks the merge.

Worse, osv-scanner emits GHSA and PYSEC as separate vulnerabilities[] entries for the same underlying issue, so a MODERATE advisory is counted twice: once bucketed correctly, once as an approximate HIGH that blocks. scan-vulns.mjs double-counts the same way in the CSV register.

Suggested fix: consume results[].packages[].groups[] (it carries aliases and max_severity) and dedupe per group; and/or treat approx: true as warn-only rather than blocking. A test case for exactly this alias pair would be the highest-value addition to the existing suite.

4. Untrusted pip install runs in jobs that hold a write token

security-pr-archive.yml (contents: write, pushes to main) and security-monthly.yml (contents + pull-requests: write) both run scripts/generate-sbom.sh, which does pip install -r requirements.txt and therefore executes setup.py from any sdist in the resolution.

For a PR whose whole subject is supply chain, worth splitting: one job with no permissions resolves and scans and uploads an artifact, a second minimal job publishes it. --only-binary :all: (or --require-hashes once pinned) removes code execution from the resolve step entirely. The gate already got this right with contents: read — that is the pattern to replicate.

5. Third-party actions pinned to mutable tags

peter-evans/create-pull-request@v6 (v7 is current), browser-actions/setup-chrome@v1, and the actions/* uses. Full-SHA pinning is the baseline expectation in a supply-chain pipeline.

Should fix

6. The SBOM does not cover the Python plugins. There are core/src/drivers/plugins/python/{modbus_master,modbus_slave,opcua}/requirements.txt files, and scripts/manage_plugin_venvs.sh installs those venvs into the runtime. The SBOM only resolves the root requirements.txt, and the gate's manifest-change list (security-pr-gate.yml:291) does not include those paths — so a PR adding a vulnerable dependency to a plugin passes in ~40s with no base resolve and never appears in the inventory. That conflicts with the report's "full transitive graph" claim.

7. requirements.txt mixes runtime and dev deps (pytest, pytest-flask, pre-commit). That is the root cause of roughly 18 of the 23 suppressions ("component_not_present: test/developer tooling"). Splitting out requirements-dev.txt — which the gate already looks for — fixes the baseline at the source and makes the SBOM reflect the deployed surface.

8. Two of the three workflows have never run. gh run list shows only 5 runs, all "Security — PR gate". The whole monthly path (Chrome/PDF, ls | grep, the security/latest symlink, create-pull-request) and the archive/orphan-branch path are unexercised. Worth triggering the monthly via workflow_dispatch before merging.

9. REPORT="$DIR/$(ls "$DIR" | grep -E 'Security-Report\.html$')" (security-monthly.yml:100): without pipefail, a non-matching grep yields "$DIR/" and Chrome fails with an opaque error. build-report.mjs:797 already computes the deterministic basename — have it print the path and consume that.

10. --disable-javascript is not in the modern Chromium switch list (it was removed); the working equivalent is --blink-settings=scriptEnabled=false. Worth confirming on the first manual run, since the comment presents it as defense-in-depth. The escaping in build-report.mjs remains the real control either way.

11. One wasted full resolve per PR (security-pr-gate.yml:329): the trailing generate-sbom.sh "restores" HEAD's sbom/, but nothing after it consumes sbom/ (the archive is a separate workflow). cp -r sbom /tmp/head-sbom beforehand, or just drop the step.

12. Token on the git command line (security-pr-archive.yml:208,223): https://x-access-token:$GH_TOKEN@... is visible in /proc and in any error that echoes the URL. Prefer http.extraheader or GIT_ASKPASS. Also, git clone ... 2>/dev/null swallows auth failures and falls into the "create orphan branch" path for the wrong reason.

13. The gate can suppress itself without mandatory review. It reads osv-scanner.toml from the PR head, so a PR can unblock itself by adding an [[IgnoredVulns]] entry (documented as option 3). Reasonable by design, but it wants CODEOWNERS on osv-scanner.toml and security/report-config.json, otherwise the control is effectively optional.

Conventions and minor

• Emojis violate the project standard. CLAUDE.md: "No emojis anywhere in code, comments, or documentation". scripts/pr-gate-diff.mjs uses them in the error output and in the PR comment body.
• Base branch. CLAUDE.md says feature branches start from and target development (which exists; releases reach main by merging it). This PR targets main, and all three workflows hardcode main. Confirm that is intentional.
• A Node toolchain in a C/C++/Python repo. .pre-commit-config.yaml has no JS hook (only check-yaml/check-json), so the five .mjs files get no lint or format coverage — and the preceding commit messages say "(Python)". Either add prettier/eslint to pre-commit, or port to Python (Black/isort/Ruff are already configured).
• Tests sit outside the repo suite. scripts/__tests__/ only runs in the gate workflow; scripts/run-pytest.sh does not reach it. Coverage is partial: scan-vulns.mjs, cdx-to-csv.mjs, gen-osv-ignores.mjs and build-report.mjs (the script that produces the deliverable) have no tests at all.
• Hand-maintained versions are already stale. report-config.json says 4.1.8, main is at v4.1.9, pyproject.toml says 0.1.0. Worth deriving from VERSION/tag. The PR description says "45 Python components"; the actual run reports 41.
• Stale docs. security/README.md still says "has Claude triage any new advisory" and "check Claude's justification", and its tree shows 00-READ-ME-FIRST.md, which no script generates. The header comment in security-monthly.yml ends mid-sentence ("Auth for the triage step: your Claude"). The PR description states the opposite — no AI triage on this public repo.
• gen-osv-ignores.mjs is not invoked by any workflow; the header reads as if it were a pipeline step.
• Nits: bucketFromScore has s > 0 ? 'LOW' : 'LOW'; both CSV writers omit a trailing newline (the end-of-file-fixer hook would flag them if committed); security/latest as a symlink plus the destroyed-symlinks hook plus Windows checkouts (the repo has MSYS2 branches) is a likely broken-symlink commit; SBOM_PIP_ARGS is declared in two workflows but missing from the monthly one, despite the comment at line 49 saying it is set per repo; the osv-scanner install block is duplicated three times and is a good candidate for a composite action.

What works well

Worth saying explicitly, because a lot of this is careful:

• Consistent, deliberate fail-closed behavior: any osv-scanner exit code outside {0,1} is an error, load() exits 2 on missing or shapeless input, and scan-vulns.mjs refuses to write an empty register.
• contents: read on the gate, explicitly justified so it is safe to require in branch protection.
• Idempotent sticky comment via an HTML marker, and the guard that skips commenting on fork PRs.
• Escaping plus a tag allowlist for report-config.json strings in the rendered HTML.
• SPDXID dedup and OR instead of AND in licenseExpr, both covered by tests — AND would invert the legal obligation in a procurement artifact, so that is exactly the right thing to have pinned down.
• Keying by ${id}|${package} so a version bump of an already-vulnerable package is not reported as introduced.
• Verified checksum on the osv-scanner download; CVSS 3.1 implemented against the FIRST.org spec with the section referenced.
• The SOEM (GPLv3-or-commercial) licensing finding is genuinely material and well documented.

Verdict

Requesting changes on items 1-3 and 6 (and preferably 4-5) — as it stands, merging would ship a compliance report that contradicts its own live numbers, plus a gate with a known false-block mode. Everything else is fine as follow-up.

[Gustavohsdp]

Review feedback — resolution

Thanks for the very thorough review. Here is a point-by-point status. Two commits carry the changes: ede59a0 (blocking items) and 6f89344 (the non-blocking follow-ups worth doing now).

Blocking

• 1 — dead baseline + self-contradicting report — Fixed in ede59a0. osv-scanner.toml is emptied (all 23 entries were reported "unused ignores"), and report-config.json is reconciled to zero with a qualitative headline/criticalNote, so the static narrative no longer contradicts the live 0. build-report.mjs derives the counts from the live scan and, if anything ever surfaces above the baseline, renders a DRAFT banner + a "surfacing this scan — awaiting triage" table instead of a stale all-clear.
• 2 — suppressions fail open (no version constraint) — Fixed in ede59a0. The stale suppressions are removed rather than kept; a revived vulnerable version now surfaces instead of staying hidden until ignoreUntil.
• 3 — PYSEC/GHSA aliasing → false HIGH + double count — Fixed in ede59a0. pr-gate-diff.mjs now iterates OSV groups[] (aliased advisories = one issue) and takes the group max_severity, so a PYSEC record with no severity of its own is scored at its real (GHSA) severity and counted once, represented by its GHSA id. Verified against a live scan: the four filelock/pygments PYSEC+GHSA pairs collapse to four MODERATE/LOW findings and the gate PASSES instead of falsely blocking. A test case for this pair is in the suite.
• 6 — SBOM omits the Python plugins — Fixed in ede59a0. generate-sbom.sh resolves every core/src/drivers/plugins/python/*/requirements.txt into the SBOM env, and the PR gate includes those manifests in its change detection and restores their base versions for the base resolve, so a PR adding a vulnerable plugin dependency is caught instead of passing without a diff.

Preferably (4–5)

• 5 — third-party actions on mutable tags — Fixed in 6f89344. create-pull-request, setup-chrome, checkout, setup-node and setup-python are pinned to full commit SHAs in all three security workflows.
• 4 — untrusted pip install in a write-token job — Deferred to an immediate follow-up. The correct fix is architectural: split the resolve (no permissions, uploads an artifact) from the publish (minimal permissions), mirroring the contents: read split the gate already uses. --only-binary :all: alone would break the resolve for any sdist-only dependency, so I'd rather land the job split as its own reviewable change than bolt it on here.

Should fix (7–13)

• 9 — ls | grep report path — Fixed in 6f89344. The PDF step now uses build-report.mjs's deterministic basename and asserts the file exists.
• 10 — --disable-javascript no-op — Fixed in 6f89344. Replaced with --blink-settings=scriptEnabled=false.
• 7 — requirements.txt mixes runtime + dev; pin it — Deferred (maintainer decision). Pinning requirements.txt changes the product's dependency-management policy, and splitting requirements-dev.txt is a runtime change beyond this PR. The root cause you flagged (a dead baseline built from dev tooling) is already resolved by emptying the baseline.
• 8 — two workflows never ran — Can't be exercised from a PR: a scheduled workflow (monthly) and a push-triggered one (archive) only run from the default branch. I'll validate both via workflow_dispatch / a real merge immediately after this lands.
• 11 — one wasted resolve per PR — Correctness is unaffected (it only re-resolves sbom/ after the diff is already computed, and the archive is a separate workflow). Batched into the follow-up as a minor perf trim.
• 12 — token on the git command line — Archive workflow only, runs post-merge. Switching to http.extraheader is a good hardening; grouped with item 4's follow-up.
• 13 — gate can self-suppress; wants CODEOWNERS — Agreed on the control. I just need the security team's GitHub handle to add CODEOWNERS on osv-scanner.toml, report-config.json, scripts/, and .github/workflows/; ready to push it as soon as you point me at the team/user.

Conventions & minor

• Stale AI-triage docs (00-READ-ME-FIRST.md, "Claude triage", the mid-sentence header) — Fixed in 6f89344. This public repo has no AI triage; the docs now describe the human flow.
• Base branch (main vs development) — Needs your confirmation. The other products target development; happy to switch the three workflows if that's intended here too.
• Emojis in pr-gate-diff.mjs / small script nits (bucketFromScore no-op, trailing CSV newline) — The .mjs scripts are byte-identical across all five product PRs (ci-sync). To keep them identical I'd rather apply these cosmetic touch-ups in one shared-scripts pass across every repo than diverge this one.
• Node toolchain / tests outside the repo suite — The scripts are shared, language-agnostic tooling exercised by the gate's node --test; wiring a JS pre-commit hook (or porting) is a follow-up.
• SBOM_PIP_ARGS not declared in the monthly — Functionally harmless (it defaults to empty via ${SBOM_PIP_ARGS:-}); will add the empty env for consistency in the follow-up.

Happy to fold items 4, 11, 12 (and 7/13 once you confirm the policy calls) into a single follow-up if you'd prefer them in this PR instead.

[marconetsf]

Approving, conditional on one small edit landing before merge (details at the end).

I re-checked the four blocking items against 6f89344, ran the suite locally (17/17 pass) and verified the pinned SHAs upstream. The follow-up work is good:

• 2 — baseline emptied rather than kept, with the fail-open reasoning written down in the file. Right call.
• 3 — pr-gate-diff.mjs now iterates groups[] and takes max_severity, so an aliased PYSEC record is scored at its real severity and counted once. The two new tests pin exactly the pair that would have false-blocked, including the inverse case (a PYSEC-only group with a genuine HIGH max_severity still blocks). That is the test I was hoping for.
• 6 — generate-sbom.sh resolves the plugin manifests, and the gate both detects them in the change list and restores their base versions for the base resolve. The || echo "::warning::" per plugin is the right trade-off: a resolution conflict degrades to visible partial coverage instead of silently dropping it.
• 1 — the structural half is solved: counts derived from the live scan, DRAFT banner, and a "surfacing this scan" table so the document can never publish a stale all-clear. registerRows from the delta scan closes the CSV drift.
• 5 — all five uses: on full SHAs across the three workflows. I resolved each SHA against its upstream repo; they check out.
• 9 / 10 — deterministic basename with an existence assert, and the working Blink switch.

The deferrals on 4, 7, 8, 11 and 12 are reasonable, and I agree with the reasoning on 4 specifically: --only-binary :all: would break any sdist-only dependency, and the resolve/publish job split deserves its own reviewable change rather than being bolted on here. Please do land it soon though — both workflows that run generate-sbom.sh still hold a write token, and they only start executing after this merges.

---

Before merge

security/report-config.json:26 still carries the old number, and it renders verbatim into section 7 of the deliverable (build-report.mjs:240 and :321, with no draft conditional):

"No action required for exploitable vulnerabilities - all 5 advisories are not reachable in the running product"

while the Key metrics table in the same document now prints 0 from the live scan. That is the same self-contradiction that blocked the first pass, just reduced to one line. Two neighbours worth fixing in the same edit:

• practices asserts "Automated dependency updates (Dependabot / advisory databases)" as "In place", but there is no .github/dependabot.yml in this repo. A practice claimed as in place in a compliance artifact should either exist or be marked Recommended.
• "version": "4.1.8", while VERSION is v4.1.9. Deriving it from VERSION at render time would stop this from recurring.

Follow-up (not blocking)

Residual half of item 3. The gate is fixed, but scan-vulns.mjs:30 and idSet() in build-report.mjs:46 still count per v.id with no alias grouping. Harmless today at zero advisories, but on the first real one the gate and the report will state different totals for the same scan, and the DRAFT table will show the issue twice (once as GHSA, once as PYSEC with severity UNKNOWN). Same groups[] treatment applies.

Item 13 / CODEOWNERS. Nothing exists in the repo yet. Until it does, the suppression path is an optional control. I will get you the security team handle so you can cover osv-scanner.toml, security/report-config.json, scripts/ and .github/workflows/.

Base branch — already discussed offline, nothing needed here.

Emojis in pr-gate-diff.mjs. Accepted for now on the ci-sync argument: keeping the five copies byte-identical is worth more than the convention here. Please fold it into the shared-scripts pass across all repos when that happens, along with the bucketFromScore no-op and the trailing CSV newline.

Approving on the understanding that the report-config.json edit lands before you merge.