Clear the 6 open dependency advisories, and decide what the audit job gates #81
Labels
No labels
area/ai
area/backend
area/frontend
area/infra
area/scheduler
area/wled
good-first-issue
priority/high
priority/low
priority/medium
type/bug
type/chore
type/ci-cd
type/docs
type/feature
type/qa
v1.0.0
v1.1.0
v1.2.0
v2.0.0
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
rbrooks/Iris-WLED#81
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two related problems surfaced on 2026-08-30 while unblocking the Renovate queue.
1. Six open advisories on
mainFrom the
Dependency auditjob on run #7625 (43d3a32) — 6 vulnerabilities (2 moderate, 4 high):brace-expansion@typescript-eslint/typescript-estree, root)js-yaml4.0.0-4.3.0@eslint/eslintrcnanoid<=3.3.17postcss<=8.5.22react-router6.0.0-7.17.0react-router-dom ^6.26.2)pip-auditreports nothing — this is entirely npm-side.Remediation route
The four high transitive ones are all in-range, so #39 (lock file maintenance) and #38 (npm non-major) clear them once rebased onto current
main.react-routeris the only one needing judgement. The advisory range covers all of 6.x, so the non-major bump already in #38 does not fix it — it needsreact-router-domv7. That major was gated behinddependencyDashboardApprovalinrenovate.json; approved on the dashboard (#34) on 2026-08-30, along with a rebase-all.The migration should be small. Total router usage is 4 files and 5 symbols, all still exported by
react-router-domv7:src/App.tsx—BrowserRouter,Route,Routessrc/components/layout/Header.tsx—Link,useLocationsrc/pages/AuditLogPage.tsx—Linksrc/pages/SettingsPage.tsx—LinkNo data routers, no loaders/actions, no SSR — and one of the two advisories (
deserializeErrors()SSR hydration) does not apply to this app at all.npm audit --audit-level=highexits clean2.
continue-on-error: truedoes not do what the workflow intendssecurity-auditis markedcontinue-on-error: trueand commented "Non-blocking for now (advisory)". On this Forgejo runner that does not hold — a failingcontinue-on-errorjob still marks the whole runfailure.Evidence: run #4506 (PR #36) had
Dependency auditas the only failing job — every other job including the full E2E passed — and the run is recorded asfailure. Same on #7642 (b3ce3a2), where all five other jobs are green.Consequences:
Decision needed (not taken here)
continue-on-error, keep dependencies clean, accept that a newly published CVE can block unrelated merges until triaged.|| trueon the audit steps so the job passes and findings stay in the log. Honest about being non-blocking, but nobody reads a green log.Inclination is (c), with (a) reasonable if the dependency surface stays small. Whichever is chosen should land before #56.
Blocks: #56.
Part 2 resolved: option (c), split it out —
124a8c4New
.forgejo/workflows/audit.ymlrunspip-audit+npm auditon a weekly schedule (0 9 * * 1) plusworkflow_dispatch. Thesecurity-auditjob is gone fromci.yml, which is now five jobs with nocontinue-on-erroranywhere.Details worth recording:
continue-on-errorin the new workflow either. The flag was the problem — it promised something this runner does not honour. Standalone, the audit can just fail honestly; failing gates nothing because nothing depends on it.before 6am on mondaywindow, so each report reflects the state once that week's dependency PRs exist rather than the state just before them.if: always(). In the old job apip-auditfailure short-circuited the steps after it, so backend findings hid frontend ones. Now one run reports both stacks.main— noted in the file.#56 is unblocked. Branch protection can now require green CI without a transitive CVE blocking unrelated merges.
Part 1 (the six advisories) is still open and routed through Renovate: rebase-all and the react-router v7 approval are ticked on #34, awaiting Renovate's next run.
Advisory progress
Two of the five packages are cleared:
postcssdeb3b26)react-routerreact-router-domv7brace-expansionjs-yamlnanoidpip-auditremains clean; this was always npm-side only.On the react-router major
It needed no source changes. The whole router surface is
BrowserRouter/Route/Routes,LinkanduseLocationacross four files, all still exported by v7 — matching the assessment in this issue when the major was approved. The full pipeline passed, including the E2E specs that exercise routing, so the upgrade is verified rather than assumed.Worth noting one of the two advisories (
GHSA-337j-9hxr-rhxg, arbitrary constructor injection viadeserializeErrors()in SSR hydration) never applied to this app at all — Iris has no SSR. It is cleared regardless.What remains
All three outstanding highs are transitive and clear together via #39 (lock file maintenance), which is currently green. It cannot merge yet only because it rewrites the whole lockfile and #35 / #36 are also queued against the same file — they have to land one at a time with a Renovate regeneration between each.
On part 2 of this issue
The audit split (
124a8c4) is working as designed. The scheduledDependency auditworkflow fired on its own at 09:00 Monday (run #7729) and failed — correctly, since the advisories are real — without gating a single merge. That is precisely the behaviour this issue asked for, and it is what made #56 (branch protection) safe to enable.Audited 2026-09-01 by running the audit rather than reading the last report. 6 advisories → 2.
npm audit --audit-level=highagainst the currentmainlockfile, in a cleannode:22container:Part 1 — advisories
nanoidandpostcssare clear.react-router-domv7 landed —frontend/package.jsonnow pins^7.0.0, so both moderates are cleared. This was the only one needing judgement and it's done.npm audit --audit-level=highexits clean — not yet:brace-expansion(×3) andjs-yamlremain.Both survivors are transitive and in-range, npm reports a fix is available, and both arrive through the ESLint tree —
js-yamlvia@eslint/eslintrc,brace-expansionvia@typescript-eslint/typescript-estreeplus a root copy. #35 (chore(deps): update eslint) is open and mergeable and should clear both. Worth merging that and re-running before doing anything manual;npm audit fixby hand would just fight Renovate.Part 2 — what the audit job gates
audit.ymlnow runspip-audit+npm audit --audit-level=highweekly (Mondays 09:00 UTC, after Renovate's window) plusworkflow_dispatch, with nocontinue-on-erroranywhere — a red audit is a real signal that gates nothing.security-auditjob was removed fromci.ymlentirely, andaudit.yml's header documents whycontinue-on-errorcould not express "advisory" on this runner.This issue is one merge away from closing. #56 is no longer blocked by the gating half — that's already resolved — only by the two remaining advisories if you want a clean audit before turning on branch protection.
Correcting my previous comment: #35 is merged, and it does not clear the remaining two. I said this issue was "one merge away" — that was wrong, and I only found out by test-merging #35 onto current
mainand re-running the audit rather than assuming.npm audit --audit-level=highwith #35 applied — still 2 high. But the picture underneath changed, and the dependency paths explain why:So #35 did fix the
typescript-estreehalf ofbrace-expansion. What survives is entirely insideeslint's own dependency tree, at the latest published eslint (9.39.5): an oldminimatch@3and an@eslint/eslintrcthat pinsjs-yaml@4.3.0.That's the important correction: Renovate cannot fix these. There is no newer eslint to bump to. Waiting for the dependency bot is waiting for upstream eslint to drop
minimatch@3and bump@eslint/eslintrc— indefinite.What would actually close this
Patched versions exist and are in-range for their parents:
brace-expansion@1.1.18(advisory is<=1.1.17;minimatch@3wants^1.1.7, so 1.1.18 satisfies it)js-yaml@4.3.2(advisory is4.0.0 - 4.3.0)So an npm
overridesblock infrontend/package.json, scoped by version so it doesn't clobber the healthybrace-expansion@5:I have not done this — it's a judgement call that belongs to whoever owns the dependency policy, not a mechanical fix:
npm lsoutput, so they're easy to forget.overrides, so they go stale with no PR to remind anyone.The alternative is to accept two DoS-class advisories in dev-only tooling (eslint is not shipped to the browser) and let them clear whenever eslint updates. Given the attack surface is a linter running on our own source, that's defensible — but it means
npm audit --audit-level=highstays red, which matters for #56 if you want a clean audit before turning on branch protection.Recommendation: add the overrides with a comment naming the advisories and the condition for removal. It's the only route that gets a clean audit this side of upstream.
Updated checklist for part 1:
nanoid,postcssclearreact-router-domv7 landed — both moderates cleartypescript-estreeroute tobrace-expansionnpm audit --audit-level=highexits clean — needs the overrides decision above; not reachable via Renovateoverridespins once eslint ships the fixes upstream #101Done — #102 merged.
npm audit --audit-level=highreportsfound 0 vulnerabilities.Final state of both halves:
Part 1 — the six advisories
nanoid,postcssclearreact-router-domv7 landed — both moderates cleartypescript-estreeroute tobrace-expansionnpm audit --audit-level=highexits clean — the last two neededoverrides(#102), since they lived inside eslint's own tree at the latest eslint and no dependency PR could reach themPart 2 — what the audit job gates
audit.yml— weekly +workflow_dispatch, nocontinue-on-error, gates nothing#56 is now unblocked on both counts — the gating coupling is removed and the audit is clean, so requiring green CI won't be blocked by pre-existing advisories.
Carrying forward: #101 tracks removing the two pins once eslint fixes them upstream. Renovate doesn't manage
overrides, so that issue and theCONTRIBUTING.mdnote are the only things that will remind anyone.Closing.