sec(deps): clear all HIGH advisories via override floors; fix OSV SARIF gate - #70
Merged
Merged
Conversation
…V SARIF gate
CI has been red on `main` since the advisory database picked up four HIGH
findings against the locked tree. Two separate gates were failing:
* Security job — `npm audit --audit-level=high`: brace-expansion <=5.0.7
(GHSA-3jxr-9vmj-r5cp, GHSA-mh99-v99m-4gvg), fast-uri 4.0.0
(GHSA-v2hh-gcrm-f6hx, GHSA-4c8g-83qw-93j6), js-yaml 4.2.0
(GHSA-52cp-r559-cp3m), postcss <=8.5.17 (GHSA-r28c-9q8g-f849).
* Docker job — the Trivy image gate reported the same three of those. Trivy
scans `/app/package-lock.json` inside the runtime image, so it sees
dev-only locked packages even though the runner installs `--omit=dev`;
a dev-tooling HIGH therefore still fails the image gate.
Fixed by raising the transitive `overrides` **security floors** — the lowest
patched release per advisory, not a version bump — and relocking. No direct
dependency changes its declared range, so the tree stays semver-compatible
with what every dependent asks for. js-yaml is capped below 5 because
`@langchain/classic` targets the 4.x API.
The remaining moderates are cleared in the same pass (CI does not gate on
them, but they were the only findings left): hono >=4.12.32,
protobufjs >=8.6.6, and @modelcontextprotocol/sdk 1.29.0 -> 1.30.0 — already
in range of the declared `^1.27.1` — which pulls @hono/node-server 2.0.12
(the SDK declares `^1.19.9 || ^2.0.5`) and closes GHSA-frvp-7c67-39w9.
`npm audit` now reports 0 vulnerabilities for the root package and `board/`.
Also fixes a latent CI defect surfaced by the above: `Upload OSV SARIF` ran
with a bare `if: always()`, so whenever an earlier gate failed it added a
bogus second failure ("Path does not exist: osv.sarif") that masked the real
one. It is now guarded on the file existing.
Supersedes #62 (brace-expansion), #68 (hono) and #69 (postcss), which each
fixed one finding and left fast-uri and js-yaml — neither of which had a
Dependabot PR — still failing the gate. Also folds in #64
(ossf/scorecard-action 2.4.3 -> 2.4.4).
Verification (local, Node 22 / npm 10):
lint 0 · typecheck 0 · lint:arch (no cycles) · api:check (both entries, no
drift) · 108 test files / 1155 tests / 100% coverage of src/** · board:
14 files / 148 tests + build · npm audit 0 vulns (root + board) · Trivy
image scan exit 0 (no fixable CRITICAL/HIGH) · e2e against real containers:
redis/bullmq 69, kafka 3, security 37, chaos 1 — all green.
No source change; no public API change; no breaking change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR unblocks failing CI security gates by raising npm overrides “security floors” for vulnerable transitive dependencies (and relocking), and improves CI signal by preventing a secondary, misleading SARIF upload failure.
Changes:
- Raised transitive dependency override minimums (and updated the lockfile) to clear HIGH advisories (and some moderates).
- Guarded the OSV SARIF upload step so it only runs when
osv.sarifexists, preventing a bogus failure when OSV-Scanner didn’t run. - Bumped
ossf/scorecard-actionfromv2.4.3tov2.4.4.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| SECURITY.md | Updates security policy text to reflect the new override floors and current advisory status. |
| package.json | Raises overrides floors for affected transitive packages (e.g., brace-expansion, fast-uri, js-yaml, postcss, etc.). |
| package-lock.json | Relocks to versions satisfying the new security floors (including @modelcontextprotocol/sdk and transitive updates). |
| CHANGELOG.md | Documents the security-floor changes, action bump, and the OSV SARIF guard fix. |
| .github/workflows/scorecard.yml | Bumps ossf/scorecard-action to v2.4.4. |
| .github/workflows/ci.yml | Prevents SARIF upload from failing when osv.sarif isn’t present (guarded if: condition). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+110
to
+114
| # `always()` so the SARIF still reaches the Security tab when an earlier | ||
| # gate (e.g. the audit) fails — but guarded on the file actually existing: | ||
| # if the audit fails, OSV-Scanner never runs and an unguarded upload adds | ||
| # a bogus second failure ("Path does not exist: osv.sarif") that masks the | ||
| # real one. |
Owner
Author
There was a problem hiding this comment.
Fixed in a424aaa — comment rewritten to distinguish the two paths exactly as suggested.
Live-stack verification surfaced a crash: on networks whose DNS blocks nom.telemetrydeck.com (common with ad-blocking resolvers), KaibanJS's fire-and-forget telemetry fetch rejects with EAI_AGAIN/SERVFAIL and the unhandled rejection kills the agent process before its first task completes — every searcher/writer/editor node crash-loops and the orchestrator times out. kaibanjs already honors KAIBAN_TELEMETRY_OPT_OUT (truthy = opt out), but the example compose files did not forward it, so it could not be set for the containerized nodes. Forward it (default empty = telemetry stays on, current behavior unchanged) and document the variable + the DNS-block caveat in .env.example. Verified: with the opt-out set, the full global-research live stack (redis + gateway + 4 searchers + writer + reviewer + editor, real LLM via OpenRouter) completes search -> write -> governance -> editorial -> HITL end-to-end. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The live global-research suite (CI-skipped, so the drift went unnoticed)
still asserted the pre-v2.0 orchestrator's log lines — "Search task N/N
queued", "GOVERNANCE REVIEW BY SAGE", "EDITORIAL REVIEW BY MORGAN",
"RESEARCH PUBLISHED" — none of which exist anywhere in the source since
the v2.0 refactor (shared WorkflowOrchestrator + phases.ts + RunLogger,
verified absent on origin/main too). Every scenario failed against a
pipeline that was actually completing successfully.
Rewritten against the real, current contract — stricter than before:
* stdout phase banners (STEP 1-4, HITL) and phase summaries
("SEARCH PHASE COMPLETE — n/N results", "SYNTHESIS COMPLETE (n
chars)", "Compliance Score: … Recommendation: …")
* economics/metadata report (tokens, cost, active nodes, context id)
* NEW: the machine-readable RunLogger JSON is parsed from the "Run log
saved to <path>" line and its `outcome` field is asserted as the
authoritative terminal verdict (PUBLISHED on the golden path), plus
real token totals and per-phase task records.
A governance REJECTED verdict is treated as a legitimate clean stop
(the reviewer gate doing its job on live LLM output), never as a pass
for a broken pipeline: search→write→governance is always asserted, and
whenever the pipeline proceeds the full editorial + HITL + PUBLISHED
chain is required.
Also tightens the OSV SARIF step comment per Copilot review: `always()`
covers OSV-Scanner failing after writing the file; the `hashFiles` guard
covers OSV being skipped entirely.
Verification (live, real LLM via OpenRouter, Docker stack):
* global-research: npm run test:e2e:live — 4/4 scenarios PASS
(golden path PUBLISHED, governance CONDITIONAL with recorded task
tokens, metadata report, 3-searcher fault-tolerance run).
* blog-team: full Docker stack (redis+gateway+researcher+writer+editor)
+ board dev server — research → write → editorial 8.7/10 → task
AWAITING_VALIDATION on the state:update stream → PUBLISH decision
emitted through the board's Vite proxy socket (the Approve button
path) → gateway ack {ok:true} → outcome PUBLISHED, Board: FINISHED,
run log clean, exit 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Addressed the Copilot review comment in a424aaa — the OSV SARIF step comment now correctly distinguishes the two paths: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed and why
CI has been red on
mainsince the npm advisory database picked up four HIGH findings against the locked tree. Two separate gates were failing:npm audit --audit-level=highbrace-expansion<=5.0.7,fast-uri4.0.0,js-yaml4.2.0,postcss<=8.5.17brace-expansion5.0.6,fast-uri4.0.0,js-yaml4.2.0Trivy scans
/app/package-lock.jsoninside the runtime image, so it sees dev-only locked packages even though the runner stage installs--omit=dev. A dev-tooling HIGH therefore still fails the image gate, and the fix has to be in the lockfile.Fixed by raising the transitive
overridessecurity floors — the lowest patched release per advisory, not a version bump — and relocking:brace-expansion>=5.0.8fast-uri>=4.1.1js-yaml>=4.3.0 <5postcss>=8.5.25hono>=4.12.32protobufjs>=8.6.6@modelcontextprotocol/sdk^1.27.1)@hono/node-server2.0.12, closing GHSA-frvp-7c67-39w9 (moderate)No direct dependency changes its declared range, so the tree stays semver-compatible with what every dependent asks for.
js-yamlis deliberately capped below5because@langchain/classictargets the 4.x API.@hono/node-server2.x is inside the range the MCP SDK itself declares (^1.19.9 || ^2.0.5).npm auditnow reports 0 vulnerabilities for the root package and forboard/.Also fixes a latent CI defect the above surfaced:
Upload OSV SARIFran with a bareif: always(), so whenever an earlier gate failed it added a bogus second failure (Path does not exist: osv.sarif) that masked the real one. It is now guarded on the file existing. And folds inossf/scorecard-action2.4.3 → 2.4.4.Supersedes
brace-expansion), chore(deps): bump hono from 4.12.25 to 4.12.32 #68 (hono), chore(deps-dev): bump postcss from 8.5.15 to 8.5.25 #69 (postcss) — each fixed one finding and leftfast-uriandjs-yaml, neither of which had a Dependabot PR, still failing the gate.ossf/scorecard-action) — folded in here.Test coverage for the change
No source change, so no new tests. Full local verification on Node 22 / npm 10:
lint0 ·typecheck0 ·lint:arch(no cycles) ·api:check(both entries, no drift)src/**(statements 1919/1919, branches 1143/1143, functions 477/477, lines 1793/1793)npm run buildnpm audit0 vulnerabilities (root + board)test:e2e69 ·test:e2e:kafka3 ·test:e2e:security37 ·test:e2e:chaos1 — all greenBreaking changes
None. No source change, no public API change (
api-extractorreports unchanged).Security implications
Strictly positive: 4 HIGH + 4 moderate advisories cleared, and the OSV SARIF guard means a failing audit now reports one real error instead of two.
🤖 Generated with Claude Code