Repository navigation
fix(security): close all 157 open JS alerts across the scenario workspaces - #873
langwatch-agent wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 4 reviews per rolling hour; 2 remain after this review. WalkthroughThe pull request updates dependency overrides and minimum versions in workspace configuration, the JavaScript package, and the Lovable template. It removes local overrides from the OpenAI realtime demo. ChangesDependency Override Updates
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. A rabbit checks each version line, Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
javascript/package.json (1)
145-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the dependency override policy.
Add or update workspace documentation for each new
overridesandpnpm.overridesselector. State the matched range, replacement version or floor, default behavior, environment variables (none), and one example. Keeppackage.jsonmachine-readable.As per coding guidelines, JavaScript JSON/YAML configuration options must document each parameter, default values, environment variables, and configuration examples.
Also applies to: 166-196
🤖 Prompt for 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. In `@javascript/package.json` around lines 145 - 152, Document every newly added dependency override selector in the relevant workspace documentation, including the matched version range, replacement version or minimum floor, default behavior, environment variables as none, and one usage example; cover both overrides sections referenced by the comment. Keep the package.json overrides unchanged and valid machine-readable JSON.Source: Coding guidelines
🤖 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 `@javascript/package.json`:
- Line 166: Remove the duplicate selector declarations from the pnpm.overrides
object, keeping exactly one declaration for each repeated liquidjs, fast-uri,
postcss, brace-expansion, and `@opentelemetry/propagator-jaeger` key while
preserving the intended override values.
- Around line 146-149: The brace-expansion override currently permits version 5,
whose Node requirement excludes Node 18. In javascript/package.json lines
146-149 and docs/pnpm-workspace.yaml lines 22-24, narrow the
brace-expansion@>=3.0.0 override to a compatible 4.x range (or document and
enforce the required Node version), preserving minimatch@9.x compatibility for
Node 18 consumers.
In `@python/examples/lovable_clone/template/package.json`:
- Around line 114-118: Add an explicit pnpm packageManager declaration to the
template package.json, using the targeted pnpm version required to apply the
listed dependency overrides. Ensure the configuration aligns with the existing
pnpm.lockfile overrides and prevents the npm install path from being the only
documented installation route.
---
Nitpick comments:
In `@javascript/package.json`:
- Around line 145-152: Document every newly added dependency override selector
in the relevant workspace documentation, including the matched version range,
replacement version or minimum floor, default behavior, environment variables as
none, and one usage example; cover both overrides sections referenced by the
comment. Keep the package.json overrides unchanged and valid machine-readable
JSON.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 70cdae03-d040-4499-9183-2b9b75307987
⛔ Files ignored due to path filters (6)
docs/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamljavascript/examples/openai-realtime-demo/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamljavascript/package-lock.jsonis excluded by!**/package-lock.jsonjavascript/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpython/examples/lovable_clone/template/package-lock.jsonis excluded by!**/package-lock.jsonpython/examples/lovable_clone/template/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
docs/pnpm-workspace.yamljavascript/examples/openai-realtime-demo/package.jsonjavascript/package.jsonpython/examples/lovable_clone/template/package.json
8f15d88 to
6eae1eb
Compare
|
Rebased onto main and substantially expanded: floors raised to current, now 105 of the 110 open JS alerts instead of 37. Two structural findings are written up in the description (an entire example's overrides were inert because it is a workspace member, and |
6eae1eb to
ee6a35d
Compare
|
Added the react-router 7 migration for the This PR now closes 110 of the 110 open JS alerts in scenario, up from 105. Nothing on the JS side is left behind after it merges. I had previously written these five off as "a v6 to v7 migration, which is a different kind of change from a floor bump". That was too quick a judgement. The template only ever used the v7-compatible subset of the API:
All four are still exported by
Verified in a browser, not just on the build, since a passing build says nothing about whether routes actually resolve:
|
8daa1d4 to
11d2965
Compare
|
Rebased onto current main, which added the Re-ran the closure check across the whole scenario inbox afterwards. All 148 open alerts are live (no stale manifests in this repo), and this PR plus #857 close every one of them: 110 here, 38 there, 0 left over. The only two entries the checker cannot see a resolved version for are Ready for human review. |
11d2965 to
df36631
Compare
df36631 to
b2c733f
Compare
|
CI caught a real regression in the first push of this branch, and it is fixed in
The cause was my own override selector. I had written the floors as Main's original selectors had lower bounds ( Every selector this PR touches now carries one, I only added bounds to the selectors this PR introduces or raises. Pre-existing entries are untouched, deliberately: Re-verified after the fix: closure still 90 of 90; Two failures remain on this branch that are not from it, both reproduced on an untouched |
|
Second CI finding, and this one is not from this branch: Same file, same error, same Cause. Fix. Declare the same Verified locally. The import-time TypeError is gone; the suite now gets into the test body and stops at Alert closure is unchanged at 90 of 90. Worth flagging separately: this went red on main and stayed red. If the intent is for |
d20e19d to
ae7184a
Compare
|
Rebased onto current main. Two notes on state. Main absorbed the zod fix from this branch. The red on this PR is not from this PR.
So |
f87d1b1 to
0bebc9e
Compare
Coding agent usage on this pull request
Token and model breakdown
Tokens as reported by the agents to LangWatch; cost estimated from model list prices, over the pull request's whole lifetime. Updated for |
31297ee to
573680b
Compare
|
Honest read of this PR's CI, and a correction to what I said yesterday. Yesterday's "green" was not a real green. The heavy workflows had not run on this branch at all: the checks I read were What actually ran today. The unit suites pass: the javascript workspace is 1557 passed, 1 skipped across 122 files in CI, matching 1548 locally. What is red is the examples suite in both The python side says so in plain text, 27 of 30 example tests failing together: The javascript side hit the same wall at the same moment, and the five tests the gate named are all scenario runs that need a model. Why the gate called it a defect anyway, and the fix in this PR. The marker is the sentence, not the exception class, deliberately. Matching
This does not make the check green, and it should not. This PR edits files under So: not ready to merge today. It needs a re-run once the daily gateway budget window resets, and then the examples suite has to actually pass. Everything else on the PR is verified, including a fresh audit of all 59 resolved versions against every published advisory for their package, which is what turned up the |
|
Re-ran with the classifier fix. It did what it should, and it did not make the check green. Refining what I said earlier, because the picture is more precise now. Python: from 27 defects to 3. The suite still fails 27 of 30, all in the same outage. The gate now names only three: JavaScript: a different five. The five it named before (image demo, api-service-mocking, custom-judge-llm, database-tool-mocking, error-handling) are now correctly classified as budget refusals. What remains is the remote-agent and realtime-voice set. Why that residue exists, and why I am not widening the matcher to swallow it. Those examples do not call the model directly. They stand up their own HTTP endpoint and the endpoint calls the model. When the gateway refuses, the endpoint's response is cut short, and what reaches the test is a transport error: The gateway's sentence never appears in the report, so no honest marker can catch it. Adding Not caused by this PR. Reproduced on Same failure on this branch. This PR changes no Python source at all; its only files under Where that leaves it. Still not mergeable today, for the reason the gate was designed to enforce: this PR edits files under |
573680b to
9d150f5
Compare
…paces Every open Dependabot alert on a JavaScript manifest in this repo, across the four workspaces that still carry one, resolved in a single pass. Two of the manifests are removed rather than patched. `javascript/package-lock.json` was a second lockfile for a workspace that resolves with pnpm, so it described a dependency tree nothing installs, and every alert on it was against versions no build has ever used. The same is true of the nested `javascript/examples/openai-realtime-demo/pnpm-lock.yaml`, whose package now resolves through the parent pnpm workspace. Deleting them is the fix: the trees they described are not reachable. The rest is override work. Each selector carries both bounds so it cannot float past a major, and each target is the highest fix across every advisory that covers the package rather than the first patched version of any one of them. That last rule is why `fast-uri` sits at `>=3.1.6` and not `>=3.1.5`. The alerts raised on `javascript/pnpm-lock.yaml` only name `>=4.0.0 <4.1.3`, because that lock resolves the 4.x line, and reading the floor off them would have capped this workspace onto a 3.1.5 that four HIGH advisories still cover. The advisories against the 3.x line are live and visible elsewhere in this same inbox, on the two locks this change deletes. vitest and every `@vitest/*` package move together to 4.1.11. vitest peers its siblings at an exact version, and `@vitest/coverage-v8` was itself pinned to exactly 4.1.10, so bumping only the two packages the advisory names left a parallel 4.1.10 subtree in place. The examples workspace's own `vitest` range moves too. `@vitejs/plugin-react-swc` moves to 4.x in the lovable_clone template. It is not a security bump: 3.11.0 peers `vite ^4 || ^5 || ^6 || ^7` while the template is on vite 8, so npm could not resolve the tree at all and the lockfile could not be regenerated. 4.3.3 peers `^8` as well. `toml` in docs moves from 3.0.0 to the 4.x line, which is the only line with a fix. The two are API-identical for this use: both export `parse` alone, and both return the same object for a document with scalars and a table. The consumer is remark-mdx-frontmatter, which parses page frontmatter, and the docs site prerenders every page unchanged. ## The examples gate learns the gateway's other refusal `classify-examples-failures.py` knew one shape of gateway refusal, HTTP 402 `budget_exceeded`. A call that leaves through litellm comes back as a RateLimitError carrying the gateway's own sentence instead, and the 402 never reaches the report. On the run that prompted this, 27 of the 30 python examples failed together on "Your organization's AI gateway access is exhausted" while the classifier saw no budget marker and reported every one of them as a real defect. That is precisely the false alarm the script was written to prevent. The new marker is the gateway's sentence, not the exception class. Matching `ratelimiterror` would have been wrong: an example that mishandles a genuine 429 from a provider deserves to fail, and a generic match would bury it. Checked against four reports: gateway-exhaustion refusals classify as `budget`, a real assertion failure mixed in with them still comes back `mixed` and names the real one, a plain provider 429 still comes back `mixed`, and the original 402 path is unchanged. This does not turn the check green here, and should not. This pull request edits files under `examples/`, so `EXAMPLES_TOUCHED` is true, and the gate's own rule is that an outage plus a changed example means the change went unverified. Nor does it classify every failure in an outage. The remote-agent and voice examples stand up their own HTTP endpoint and have *that* call the model, so a refusal truncates the endpoint's response and reaches the test as `TransferEncodingError: Not enough data to satisfy transfer length header`. The gateway's sentence is never in the report, and no honest marker can catch it; matching the transport error would classify a real truncated-response bug as an infrastructure excuse. Those stay named. On python that is three tests out of the 27 that failed, down from all 27, and each of them fails identically on `origin/main` with no credentials, so none of it is this change. Verified: the javascript workspace's own suite at 1702 passed on vitest 4.1.11; docs `eslint .` clean and `pnpm run build` prerendering every page with toml 4.3.0; the lovable_clone template building on vite 8 with plugin-react-swc 4.3.3, browserslist 4.28.9 and postcss-selector-parser 6.1.4; both lockfiles of that template regenerating without a peer conflict; and every one of the 66 resolved versions this change moves tested against every published advisory for its package, not only the ones raised against this repo, which leaves zero JS alerts open and no vulnerable resolution behind. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RWpxWBnYtUMcktWN1C6SrL
9d150f5 to
2edd431
Compare
…uri floors Advisories published 2026-09-28/29 cover versions this branch resolved: - brace-expansion 1.1.18 / 5.0.9 (three HIGH ReDoS advisories per line): floors raised to 1.1.21 / 2.1.7 / 5.0.12 in javascript/, docs/ and the lovable_clone template. - ip-address <=10.7.0 (#752-#759 plus two newer): 10.7.1. - js-yaml 5.x <=5.4.0, reached through the langwatch SDK: 5.4.1. - fast-uri 3.x: 3.1.8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RWpxWBnYtUMcktWN1C6SrL
javascript/pnpm-lock.yaml conflicted with the judge changes from #1006; taken from main and re-resolved against this branch's overrides. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RWpxWBnYtUMcktWN1C6SrL
…4.16 Advisories published 2026-09-30: @grpc/grpc-js >=1.14.0 <1.14.5 (HIGH) in javascript/, and dompurify 3.4.13-3.4.15 in docs/. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RWpxWBnYtUMcktWN1C6SrL
|
CI status: the only red is the JS examples step, failing on 🤖 Filed by |
|
Automated low-risk assessment This PR was evaluated against the repository's Low-Risk Pull Requests procedure and does not qualify as low risk.
This PR requires a manual review before merging. |
What
Closes all 117 open JavaScript alerts across the scenario workspaces (60 HIGH, 47 MODERATE, 10 LOW). With #857 (the 38 on
python/uv.lock) that is the entire scenario inbox, 155 of 155.javascript/package-lock.jsondocs/pnpm-lock.yamldocs/pnpm-workspace.yamljavascript/examples/openai-realtime-demo/pnpm-lock.yamlpython/examples/lovable_clone/template/pnpm-lock.yamljavascript/pnpm-lock.yamljavascript/package.jsonpython/examples/lovable_clone/template/package-lock.jsonRaising in place, not adding alongside
pnpm matches only the first selector for a package name, so adding a correctly-scoped floor next to a stale one leaves the stale one in charge. Main had exactly that shape:
docs/pnpm-workspace.yamlcarried an unscopedhono: '>=4.12.25', which would have sat in front of anything new. Every stale entry is raised in place rather than shadowed.Every selector is scoped to one major line (
"pkg@>=lo <fix": ">=fix <next"). Two packages needed more than one because the tree carries parallel majors:brace-expansion(1.x, 2.x and 5.x) andjs-yaml(3.x and 4.x). Targets are capped so a floor cannot float across a major, which was not academic here:javascript/pnpm-lock.yamlhad resolved fast-uri 4.1.2 off an uncappedfast-uri@<=3.1.1 -> >=3.1.2.Each target is the highest fix across every advisory covering that package, not the first patched version of any single one, so a package with a chain of advisories lands above all of them in one move.
Two lockfiles removed
javascript/package-lock.jsoncarries the largest single share of the inbox, 35 alerts, and nothing installs from it. Every workflow usespnpm install --frozen-lockfile; there is nonpm ciornpm installanywhere in.github/; nothing in the repo references the file; and its entire commit history is release version bumps. It had drifted far enough that regenerating it moved@openai/agents0.3.9 to 0.16.1 and removed 53 packages, and it cannot be regenerated at all without--legacy-peer-deps(a pre-existing zod peer conflict that reproduces on main untouched). Committing that would be shipping a lock nothing validates. Thepnpm-lock.yamlbeside it resolves the same packages at safe versions.javascript/examples/openai-realtime-demo/pnpm-lock.yamlis an orphan. The directory is a declared member of the javascript workspace, so pnpm ignores itspnpm.overridesand writes the parent lock instead. Six of its seven floors were already superseded at the root; the seventh,ajv, moves there. Checked before removing.react-router 7 in the lovable_clone template
react-router-domhas no patched release in the 6.x line, so leaving 6.x was the only way to close it. The template uses only the v7-compatible surface (BrowserRouter,Routes,Routeinsrc/App.tsx;useLocationinsrc/pages/NotFound.tsx), so no application code changed.@remix-run/routerdisappears from both lockfiles because v7 folded it in, which is why those alerts close by the package no longer existing.postcssis a direct devDependency there and npm rejects an override that conflicts with a direct dependency, so its floor is carried by the direct pin.Two bumps that are not security fixes
@vitejs/plugin-react-swc3.11.0 to 4.3.3 in the lovable_clone template. 3.11.0 peersvite ^4 || ^5 || ^6 || ^7while the template is already on vite 8, so npm refused to resolve the tree withERESOLVEandpackage-lock.jsoncould not be regenerated at all. 4.3.3 peers^8as well. The plugin is used as a barereact()invite.config.ts, unchanged, and the template builds.toml3.0.0 to the 4.x line in docs. 4.x is the only line with a fix, so this one is a major bump by necessity. The two are API-identical for this use: both exportparseand nothing else, and both return the same object for a document with scalars and a table (checked directly against each published tarball). The consumer isremark-mdx-frontmatter, which parses page frontmatter, and the docs site prerenders every page unchanged on 4.3.0.The fast-uri floor is 3.1.6, not 3.1.5
Worth calling out because the alerts on this repo cannot tell you that on their own.
javascript/pnpm-lock.yamlresolves the 4.x line, so the alerts raised against it (#682-#685) all name>= 4.0.0, < 4.1.3. Reading the floor off those and capping to 3.x lands on 3.1.5, which four separate HIGH advisories still cover:>= 3.0.0, < 3.1.6and>= 3.1.2, < 3.1.6, both fixed in 3.1.6. Those advisories are live and visible in this same inbox, raised as #686, #687 and #691-#694 against the two locks this PR deletes.The floor is therefore
>=3.1.6 <4, and the workspace resolves 3.1.7. Every resolved version this PR moves was then re-checked against every published advisory for its package rather than only the ones raised here, which is the check that caught this.Verified
vulnerableVersionRange, not itsfirstPatchedVersion, over every resolved version in all four remaining locks: 0 of 117 left openpnpm install --frozen-lockfileexit 0 injavascript/,docs/and the templatedocs:eslint .clean,pnpm run buildexit 0, every page prerendered on toml 4.3.0pnpm run buildexit 0 on vite 8 with plugin-react-swc 4.3.3, browserslist 4.28.9, postcss-selector-parser 6.1.4npm install --package-lock-onlyexit 0 with no peer conflict, which it could not do before the plugin bump/renders,/definitely-not-a-routerenders the 404 with its intentionalconsole.errorcarrying the pathname (souseLocation()resolves), the "Return to Home" link client-side navigates, and browser-back returns to the 404Pre-existing failures, not from this PR
pnpm typecheckinjavascript/fails withTS2688: Cannot find type definition file for 'yauzl'. Confirmed identical on a clean checkout oforigin/mainat the same commit this branch sits on, so it predates this change and is not caused by it.🤖 Generated with Claude Code
https://claude.ai/code/session_01RWpxWBnYtUMcktWN1C6SrL