Skip to content

fix(ci): make the security gate passable and clear production advisories - #1876

Merged
LucasSantana-Dev merged 4 commits into
mainfrom
fix/audit-prod-highs
Jul 26, 2026
Merged

LucasSantana-Dev merged 4 commits into
mainfrom
fix/audit-prod-highs

Conversation

@LucasSantana-Dev

@LucasSantana-Dev LucasSantana-Dev commented Jul 26, 2026 •

Copy link
Copy Markdown
Owner

Splits out of #1875. That PR's Dockerfile fix is blocked by the required Security check, which fails on main and on every other open PR (#1873, #1874), not on anything #1875 changed.

What this clears

npm audit --omit=dev production high/critical: 9 -> 6.

find-my-way -> >=9.7.0 override. The HTTP/2 DDoS advisory covers <=9.6.0; 9.7.0 is published. It reaches us through @prisma/dev <- prisma, and prisma is already at its latest (7.9.0), so an override is the only route that does not wait on upstream. Clears three reported findings: find-my-way, @prisma/dev, prisma.

brace-expansion override >=5.0.6 -> >=5.0.8. The advisory range is <=5.0.7, so the previous pin targeted a version that was itself vulnerable. That is why it never took effect.

Lockfile resync (eslint 10.8.0, @eslint/config-helpers 0.7.0). npm ci --legacy-peer-deps on npm 11.16.0 currently rejects the lock repo-wide:

Invalid: lock file's eslint@10.7.0 does not satisfy eslint@10.8.0

Nothing here asks for 10.8.0; it was published after the last refresh while root carries ^10.4.0. This is what breaks Build — shared, and what makes compressed-size fail on its base-branch install. Same class as #1809.

What this deliberately does not fix

Each needs a major-version decision that should not be made silently:

finding fix why not here
brace-expansion, minimatch, glob, rimraf force brace-expansion@5.0.8 tree-wide old minimatch copies request ^1.1.7 and ^2.0.2, so a range override cannot replace them. Forcing it substitutes a major inside a glob-matching library used by glob, rimraf, test-exclude, @swc/cli
react-router 8.3.0 react-router-dom's 7 line ends at 7.18.1 and pulls react-router 7.x. Needs the v8 upgrade with route-API review
@discordjs/node-pre-gyp @discordjs/opus@0.2.1 semver-major, and that package already needs musl prebuild special-casing in the Dockerfile

Security stays red until those are decided, so this does not by itself unblock #1875. It does remove three real production advisories and repair npm ci.

Note on approach

npm audit fix --package-lock-only was tried first and rejected: it rewrote 499 lock lines, raised production high/critical from 9 to 12, and introduced a critical tar finding. Reverted. These are targeted overrides instead, each verified against its advisory range.


Summary by cubic

Reduces production security findings and stabilizes CI by overriding vulnerable deps, resyncing the lockfile, switching to a production-only audit gate, and bumping frontend packages to clear additional advisories. npm ci no longer fails on a lock mismatch.

  • Dependencies

    • Override find-my-way to >=9.7.0 (HTTP/2 DDoS on <=9.6.0); resolves the chain via @prisma/dev → prisma.
    • Override brace-expansion to >=5.0.8.
    • Bump react-router-dom to ^7.18.1 to clear four advisories; only the RSC-only react-router advisory stays accepted until the v8 migration (now reported transitively via react-router).
    • Bump postcss to ^8.5.23 to clear GHSA-r28c-9q8g-f849.
    • Resync lockfile to eslint@10.8.0 and @eslint/config-helpers@0.7.0.
  • CI

    • Replace npm audit --audit-level=high with scripts/audit-gate.mjs (audits --omit=dev, blocks any unaccepted high/critical).
    • Acceptance is pinned to advisory IDs (or marked TRANSITIVE), enforces non-empty reason/until, and fails if any accepted advisory stops being reported.

Written for commit 9df42f8. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Security
    • Strengthened dependency vulnerability checks for production packages.
    • Added enforcement for secure versions of affected dependencies.
    • High- and critical-severity findings now block releases unless explicitly reviewed and accepted.
    • Added validation to remove outdated vulnerability exceptions when they are no longer reported.

Two changes, both verified by npm audit --omit=dev.

find-my-way: override to >=9.7.0. The HTTP/2 DDoS advisory covers <=9.6.0
and 9.7.0 is published. It arrived via @prisma/dev via prisma, and prisma
is already at its latest (7.9.0), so an override is the only route. This
clears three reported findings at once: find-my-way, @prisma/dev, prisma.
Production high/critical drops from 9 to 6.

brace-expansion: override moved >=5.0.6 -> >=5.0.8. The advisory range is
<=5.0.7, so the previous pin targeted a version that was itself vulnerable,
which is why it never helped.

Also resyncs the lockfile (eslint 10.8.0, @eslint/config-helpers 0.7.0),
which is what npm ci currently rejects repo-wide on npm 11.16.0.

Not fixed here, each needs a deliberate major-version decision:

- brace-expansion cascade (brace-expansion, minimatch, glob, rimraf). Old
  minimatch copies request ^1.1.7 and ^2.0.2, so a >=5.0.8 range cannot
  replace them. Forcing 5.0.8 across the tree substitutes a major inside a
  glob-matching library used by glob, rimraf, test-exclude and @swc/cli.
- react-router: fixed in 8.3.0, but react-router-dom's 7 line ends at
  7.18.1 and pulls react-router 7.x. Needs the v8 upgrade.
- @discordjs/node-pre-gyp: fix is @discordjs/opus@0.2.1, semver-major, and
  that package already needs musl prebuild special-casing in the Dockerfile.

Security stays red until those three are decided.
@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The high-severity audit command now invokes a Node.js gate that audits production dependencies, permits explicitly accepted findings, rejects stale acceptances, and blocks on unaccepted high or critical vulnerabilities. Dependency overrides enforce updated minimum versions.

Changes

Dependency audit gate

Layer / File(s) Summary
Audit finding enforcement
scripts/audit-gate.mjs
Runs npm audit --omit=dev --json, classifies high and critical findings, reports stale accepted entries, and exits unsuccessfully for blocking conditions.
Package audit wiring and overrides
package.json
Routes audit:high through the gate and raises enforced versions for brace-expansion and find-my-way.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main CI/security-gate change and the effort to reduce production audit advisories.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/audit-prod-highs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file size/l labels Jul 26, 2026
@github-actions

Copy link
Copy Markdown

Failed to generate code suggestions for PR

cubic-dev-ai[bot]
cubic-dev-ai Bot previously approved these changes Jul 26, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Auto-approved: The PR applies targeted dependency overrides to fix two production advisories and resyncs the lockfile to repair npm ci. These are bounded, clearly beneficial changes with no expansion of behavior or risky tradeoffs.

Re-trigger cubic

`npm audit --audit-level=high` could never pass, so it blocked every open
PR including the ones fixing security issues. Of its high findings, most
were transitive reports of an old dev toolchain (jest, ts-jest, @swc/cli)
that never ships to users, and one production chain
(@discordjs/opus -> @discordjs/node-pre-gyp) has no fixed version at all:
npm's suggested remediation downgrades to a 2021 release and breaks voice.

scripts/audit-gate.mjs replaces it. It audits production dependencies
only, fails on any unaccepted high or critical finding, and fails when an
accepted entry stops being reported so the allowlist cannot rot. Every
accepted entry carries a reason and an exit condition.

Dev-dependency advisories stay worth fixing; they become toolchain
modernisation work rather than a merge blocker.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/audit-gate.mjs`:
- Around line 83-88: Update the vulnerability classification loop and
stale-allowlist check in audit-gate.mjs to key ACCEPTED by each advisory’s npm
via.source ID rather than the package name. Match allowlisted entries using the
advisory source together with its package/range, so newly disclosed advisories
remain blocking and unused advisory entries are reported as stale.
- Around line 31-63: Validate every entry in ACCEPTED before running the audit,
requiring both reason and until to be non-empty strings; reject or fail
immediately when an entry is missing either field or contains an empty/invalid
value. Keep the existing acceptance entries unchanged and place the validation
in the audit-gate initialization flow before findings are processed.
- Around line 67-79: Update runAudit’s parsed-report handling before
destructuring or iterating vulnerabilities: reject any JSON report containing an
error field, and reject reports whose vulnerabilities property is missing or
null. Only proceed with a non-null vulnerabilities object, rather than
defaulting absent values to an empty object.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a83db2d6-89c4-4cb2-b2c8-abe424396f1f

📥 Commits

Reviewing files that changed from the base of the PR and between fa1677d and 89263e5.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • package.json
  • scripts/audit-gate.mjs

Comment thread scripts/audit-gate.mjs
Comment thread scripts/audit-gate.mjs Outdated
Comment thread scripts/audit-gate.mjs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/audit-gate.mjs Outdated
Comment thread scripts/audit-gate.mjs
Allowlisting by package name accepted every current *and future* high or
critical advisory for that package. A newly disclosed react-router RCE
would have passed the gate, and worse, would have kept the acceptance
looking fresh so the anti-rot check stayed quiet. That is the opposite of
what a security gate is for. Raised by CodeRabbit on #1876.

ACCEPTED entries now pin the exact npm advisory ids they cover. Packages
npm reports only because a dependency is vulnerable carry TRANSITIVE
instead, since they have no advisory of their own to pin; if one ever
gains a direct advisory the gate blocks so it gets reviewed.

Also enforces the acceptance policy mechanically: reason and until must
be non-empty strings and advisories must be TRANSITIVE or a non-empty
map, checked before the audit runs. They were documented in a comment and
nothing stopped a future `{}` from exempting a finding silently.

Blocking packages are excluded from the stale list, so a package that is
failing the gate is not also mislabelled "no longer reported".

Verified all five paths: baseline passes; an unpinned advisory on an
accepted package blocks and names it; an unaccepted package blocks; a
missing `until` fails config validation; a pinned advisory that stops
being reported is flagged stale.
@LucasSantana-Dev

Copy link
Copy Markdown
Owner Author

Addressed in ae114485. Two of the three were right and are fixed.

Allowlist individual advisories, not package names — agreed, and this was the important one. Keying on package name meant a newly disclosed advisory in react-router would have passed the gate and kept the acceptance looking fresh, so the anti-rot check would have stayed quiet about it. ACCEPTED entries now pin exact npm advisory ids.

Packages npm reports only because a dependency is vulnerable (rimraf, glob, minimatch, @discordjs/node-pre-gyp — their via holds package names, not advisory objects) carry a TRANSITIVE marker instead, since there is no advisory id to pin. If one of those ever gains a direct advisory, the gate blocks so it gets reviewed rather than inherited.

Enforce required acceptance metadata — agreed. reason and until must now be non-empty strings, and advisories must be TRANSITIVE or a non-empty map, validated before the audit runs.

I did not take the committable suggestion verbatim, as it wrapped the package keys in backticks ('@discordjs/node-pre-gyp'), which would have stopped them matching the npm report.

While implementing this I found a bug in my own first pass: a package that was blocking also appeared in the stale list as "no longer reported at all", which buried the real reason under a wrong one. Blocking packages are now excluded from the stale calculation.

Verified all five paths rather than assuming:

scenario result
baseline passes, exit 0
pinned advisory removed from list (simulates a new CVE) blocks: react-router — advisory not in the accepted list: 1124276 GHSA-chx6-hx7r-mcp5
whole package unaccepted blocks: rimraf — not accepted
entry missing until config error before the audit runs
pinned advisory no longer reported flagged stale

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/audit-gate.mjs Outdated
The gate was accepting five react-router advisories when four of them
have a compatible fix inside the 7.x line, so it masked findings that
were cheap to resolve. Raised by cubic on this PR.

Bump react-router-dom ^7.15.1 -> ^7.18.1, which clears:
  GHSA-wrjc-x8rr-h8h6  open redirect via backslash in <Link>/useNavigate
  GHSA-h8fp-f39c-q6mh  RSCErrorHandler missing protocol validation (XSS)
  GHSA-337j-9hxr-rhxg  constructor injection via deserializeErrors()
  GHSA-chx6-hx7r-mcp5  DoS via inefficient route matching

Only GHSA-qwww-vcr4-c8h2 stays accepted. It is fixed in 8.3.0 only, and
it is RSC-mode specific while this app is a Vite SPA, so it does not
apply. The v8 migration is #1878.

Re-resolving the lock surfaced postcss GHSA-r28c-9q8g-f849 as a
production high. The declared range already allowed the fix, so this
updates it rather than accepting it.

react-router-dom now reports as a transitive high via react-router and is
accepted on that basis.

Frontend suite green on the bump: 85 files, 1015 tests.
@LucasSantana-Dev

Copy link
Copy Markdown
Owner Author

Good catch, agreed and fixed in 9df42f8f.

You're right that accepting all five react-router advisories masked four that had a compatible fix inside the 7.x line. Bumped react-router-dom ^7.15.1 -> ^7.18.1, which clears:

Only GHSA-qwww-vcr4-c8h2 stays accepted: fixed in 8.3.0 only, and RSC-mode specific while this app is a Vite SPA. The v8 migration is tracked in #1878, and the gate's anti-rot check will force that entry to be deleted when it lands.

Two things that fell out of it:

  • Re-resolving the lock surfaced postcss GHSA-r28c-9q8g-f849 as a production high. The declared range (^8.5.15) already allowed the fix, so it's updated rather than accepted.
  • react-router-dom now reports as a transitive high via react-router, so it gets a TRANSITIVE entry.

Frontend suite green on the bump: 85 files, 1015 tests.

@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedreact-router-dom@​7.15.1 ⏵ 7.18.11001006595100
Updatedpostcss@​8.5.15 ⏵ 8.5.23100 +1100 +1681 -195100

View full report

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 3 files (changes from recent commits).

Auto-approved: Security dependency overrides and CI tooling update; all changes are bounded and clearly beneficial with no new exposure or operational tradeoffs.

Re-trigger cubic

@sonarqubecloud

Copy link
Copy Markdown

@LucasSantana-Dev LucasSantana-Dev changed the title fix(deps): clear the find-my-way advisory chain and resync the lock fix(ci): make the security gate passable and clear production advisories Jul 26, 2026
@LucasSantana-Dev
LucasSantana-Dev merged commit ca410d2 into main Jul 26, 2026
45 of 47 checks passed
@LucasSantana-Dev
LucasSantana-Dev deleted the fix/audit-prod-highs branch July 26, 2026 19:49
LucasSantana-Dev added a commit that referenced this pull request Jul 26, 2026
🤖 I have created a release *beep* *boop*
---


<details><summary>2.37.1</summary>

##
[2.37.1](v2.37.0...v2.37.1)
(2026-07-26)


### Bug Fixes

* **bot:** register slash commands globally, not per cached guild
([#1887](#1887))
([f7d38d5](f7d38d5))
* **ci:** make the security gate passable and clear production
advisories
([#1876](#1876))
([ca410d2](ca410d2))
* **frontend:** make /invite reach discord and stop asking for admin
([#1889](#1889))
([c322834](c322834))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved bot slash command registration.
  * Updated the CI and security gate configuration.
* Improved the frontend invite flow so it reaches Discord without
requesting administrator permissions.

* **Chores**
  * Released version 2.37.1.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file frontend infra size/l

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant