Repository navigation
chore(GAT-9016): final dependency review - #167
Merged
Merged
Conversation
fast-uri 3.1.6 tightened URI parsing, and AJV resolves schema references through it. The AJV cache key was `Name:Version`, which parses as a URI with scheme `GWDM` and path `1.0`, so a `$ref` of the form `GWDM:1.0#/properties/summary` no longer resolves and both `/translate?subsection=` and `/validate?subsection=` return 400. Registering and looking schemas up as `Name/Version` sidesteps the scheme parse entirely. Every string a caller sees — error messages, log lines, translation graph nodes, `/list/translations` output — is built separately and stays `Name:Version`, so this is internal only. This lands ahead of the dependency sweep that moves fast-uri from 3.1.2 to 3.1.8, which is what makes it necessary; on its own it is a no-op at any fast-uri version. Confirmed load-bearing rather than assumed: with the sweep applied and this commit reverted, exactly the two subsection integration tests fail (translate.test.ts and validate.test.ts); with it, all 78 pass. The bug was first found in the closed PR #147.
Redoes the sweep attempted in the closed PR #147, once, against the fully-merged tree rather than mid-stack. npm audit goes from 46 findings (1 critical, 24 high, 19 moderate, 2 low) to 3. The critical and most of the high count came from one place: the `npm` package that @semantic-release/npm bundles. Moving semantic-release to 25 and @semantic-release/npm to 13 replaces that bundled tree wholesale and clears twelve findings at once, tar's critical among them. The remaining three — undici, ip-address and brace-expansion — are still inside node_modules/npm, which is already at 11.20.0, the newest release @semantic-release/npm's `^11.6.2` range allows. They are release-toolchain only: nothing in that subtree is loaded by the running service. react-router and its three sibling packages move 7.16.0 to 7.18.4, which clears the react-router advisory. They stay exactly pinned because each peer-requires the others at an exact version, so they can only move as a set. @google-cloud/pubsub 5.3.1 sits inside the advisory range 5.1.0-6.0.0; 6.1.0 is outside it and pulls @opentelemetry/core 2.8, clearing both findings. audit.server.ts only constructs a client and publishes to a topic, an API unchanged across the major. cookie 1.x ships its own types, so @types/cookie is removed as an orphan of that bump. parse() behaviour is unchanged for the one call site, which reads a base64url JWT out of a header — verified against plain, encoded, quoted and empty inputs. @types/node deliberately stays on ^22 rather than taking 26: it should describe the runtime, and both the Docker image and CI are Node 22. Typechecking against Node 26's lib would admit APIs that do not exist in production. Three majors are blocked upstream, unchanged from what #147 found: eslint 10 by eslint-plugin-react's peer cap of ^9.7, typescript 7 by @react-router/node's ^5.1.0 || ^6.0.0, and MUI 9 by @hdruk/ui's ^7.3.2. Two more are deferred as their own sub-plans rather than absorbed here, because both need a code change: eslint-plugin-react-hooks 7 turns on the React Compiler rules and produces 11 errors across 5 files (11-eslint-react-hooks-7-migration.md), and mermaid 12 switches the default layout engine to ELK and changes the default theme, which would silently alter how /schema-graph and /schema-view render (12-mermaid-12-migration.md). semantic-release 25 and pubsub 6 both require Node ^22.14 or newer. The Docker runtime is node:22-alpine3.24 and CI pins Node 22, so both are satisfied; the full gate below was run under Node 22 rather than the Node 20 that happened to be on PATH, so the verification matches what CI and production actually execute. Gate: typecheck, lint and build clean; npm ci reproduces the lockfile and npm ls reports no unmet non-optional peers; 78 integration and 16 unit tests pass, counts unchanged.
|
🎉 Great job! Your PR title follows the correct format. 🚀 |
equinoxmatt
approved these changes
Sep 30, 2026
This was referenced Sep 30, 2026
calmacx
added a commit
that referenced
this pull request
Sep 30, 2026
Every POST to a UI route has 400'd on Cloud Run since the react-router 7.16.0 -> 7.18.4 bump in #167. Deep Refresh, Sync New, Cancel, per-row refresh and the whole benchmark page are affected; the JSON API is not, because the guard only runs on single-fetch `.data` submissions. The guard tightened in 7.18.3. Up to 7.18.2 it compared the `Origin` header's host against the host of the request — scheme-insensitive, so a TLS-terminating proxy was invisible to it. From 7.18.3 it compares the full origin: let requestUrl = new URL(request.url); let originMatchesRequest = originUrl ? originUrl.origin === requestUrl.origin : originDomain === requestUrl.host; Cloud Run terminates TLS at the front end, so the container sees plain HTTP. `react-router-serve` never sets express's `trust proxy`, so `req.protocol` is "http" and `@react-router/express` builds `request.url` as `http://<host>/results.data` while the browser sends `Origin: https://<host>`. The schemes differ, the guard throws, and singleFetchAction returns a bare 400 the client renders as "Unexpected Server Error". `allowedActionOrigins` is matched on host alone, so listing the deployed origins clears the scheme mismatch without touching how the service is served. The Cloud Run entry is a wildcard rather than the three per-environment hostnames. Matching is whole-segment — `*` covers exactly one segment and `**` only leading segments — so no pattern can anchor on the `hdr-gateway-traser-` service-name prefix; any pattern that matches our hosts also matches every other tenant's service in the region. That exposure is accepted in exchange for a list that does not need editing when an environment or project number changes. This also drops the older `-ew.a.run.app` Cloud Run URLs, which the wildcard does not span. They still resolve and gateway-api still calls TRASER through one in `TRASER_SERVICE_URL`, but those are resource-route requests, which the guard does not apply to. Only a human browsing the admin UI at a legacy URL would see a 400. Upstream has declined to fix the proxy case (remix-run/react-router issues 15454 and 15474), so this is the supported route rather than a stopgap. The alternative, replacing react-router-serve with a custom express server that sets `trust proxy`, was prototyped in #169 and rejected as too much surface area for the problem.
This branch was successfully deployed
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
2 participants
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.
Written with Claude Code.
What
Final dependency sweep for GAT-9016, run once against the fully-merged tree. Takes
npm auditfrom 46 findings to 3, and fixes the?subsection=regression that movingfast-uripast 3.1.6 reintroduces.Why
Doing this once at the end, rather than mid-stack as the closed PR #147 attempted, avoids repeating the audit for every package WS4–WS6 added.
Testing
npm audit46 → 3.npm run typecheck && npm run lint && npm run buildall exit 0.npm cireproduces the lockfile;npm lsreports no unmet non-optional peers. 78 integration and 16 unit tests pass, counts unchanged. All of it run under Node 22, matching CI and thenode:22-alpine3.24runtime rather than whatever is on a developer's PATH.Audit before → after, by severity:
The 3 remaining are all inside
node_modules/npm(undici,ip-address,brace-expansion), bundled by@semantic-release/npm. That is already at npm 11.20.0, the newest its^11.6.2range allows, so they are blocked upstream. Release-toolchain only — nothing in that subtree is loaded by the running service.Notes
?subsection=fix is load-bearing, and verified as such.devhadfast-uri3.1.2; the sweep moves it to 3.1.8, past the 3.1.6 parsing change that breaks AJV'sGWDM:1.0#/properties/summarylookups. With the sweep applied and the fix reverted, exactly the two subsection integration tests fail; with it, all 78 pass. First commit is separate and bisect-safe — it is a no-op at anyfast-uriversion.@types/nodedeliberately stays on^22rather than taking 26, so the types describe the runtime that actually runs in production.@types/cookieremoved:cookie1.x ships its own types, so the bump orphaned it.eslint10 (eslint-plugin-reactpeer-caps at^9.7),typescript7 (@react-router/nodepeers^5.1.0 || ^6.0.0), MUI 9 (@hdruk/uipeers^7.3.2).eslint-plugin-react-hooks7 turns on the React Compiler rules and produces 11 errors across 5 files;mermaid12 makes ELK the default layout engine and changes the default theme, which would silently alter how/schema-graphand/schema-viewrender.semantic-release25 and@google-cloud/pubsub6 both require Node^22.14 || >=24.10. CI pins Node 22, the Docker image isnode:22-alpine3.24, and the org release template uses Node 24 — all satisfied. Worth knowing if anyone develops locally on Node 20.envFiledeprecation lines during typecheck and build are pre-existing, not introduced here — already investigated and closed on 2026-09-28.@react-router/devhardcodesenvFile: falseinternally and Vite deprecates it; 7.18.4 still does, and the upstream fix landed in 8.4.0, a major bump out of scope for a sweep.