Repository navigation
fix: upgrade vulnerable dependencies and migrate worker tests - #102
Conversation
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I checked two more things. The src/routeTree.gen.ts diff only reorders lines: every added line matches a removed one. braces is completely gone from pnpm-lock.yaml, so dropping its patch, its GHSA-vfj7-8cjw-p6xm exception and its tests doesn't leave anything unpatched, and the remaining createRequire tests don't depend on the removed code.
Extended reasoning...
I checked that the route tree diff only reorders lines and that braces no longer appears in the regenerated lockfile. Both rule out problems beyond the inline zod finding.
| '@tanstack/router-generator>zod': 4.6.5 | ||
| '@tanstack/router-plugin>zod': 4.6.5 | ||
| '@tanstack/start-plugin-core>zod': 4.6.5 |
There was a problem hiding this comment.
🟡 (optional) After merging, the production Worker ships two zod copies: the app's pinned 4.4.3 plus 4.6.5 inside accounts, mppx and wata. Base had 4.4.3 alone. The overrides at pnpm-workspace.yaml:67-69 are meant to cover only the three TanStack build packages. But the regenerated lockfile moved every other ^4 zod consumer onto 4.6.5 too, except the root. Fix: resolve every runtime consumer to one zod version, e.g. bump the root "zod": "4.4.3" to 4.6.5 or add a global zod override, and check that pnpm-lock.yaml then holds one runtime zod.
Why this was flagged
The trigger is the lockfile regeneration that came with the new overrides (pnpm-workspace.yaml:67-69). Once zod@ 4.6.5 was in the graph, pnpm picked it for unrelated ^4 dependents as well. In the new pnpm-lock.yaml, both accounts@ 0.15.2(...) snapshots (lines 8871 and 8898) and both mppx@ 0.8.9(...) snapshots (10873 and 10888) list zod: 4.6.5, where the base listed zod: 4.4.3. The root still pins "zod": "4.4.3" (package.json:49) and stays on it. The app imports accounts/server at runtime in src/entry-server.ts:2 and src/lib/auth.ts:1. The Worker bundle therefore carries two zod instances where the base had one. Nothing guards against this: the overrides are scoped by parent package, but pnpm's preferred-version reuse is not scoped, and no dedupe check catches it.
Verification: package.json:49 still pins "zod": "4.4.3" for the app. The lockfile adds a new zod@ 4.6.5 package entry next to the existing zod@ 4.4.3. accounts/server is imported into the Worker (src/lib/auth.ts:1, src/entry-server.ts:2), so the production bundle now carries two zod 4 copies where the base had one runtime copy. The overrides at pnpm-workspace.yaml:67-69 are scoped to the TanStack packages.
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
|
The latest Tipbot preview deployment for b20782a
|
Shut DownCo-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
decofe
changed the title
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
decofe
changed the title
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 2 findings from earlier reviews are still open above.
Still open from earlier reviews (2):
- 🔴
package.json:53—Installs for developers and CI may now fail with pnpm's ignored-builds error, because a new workerd version is not on t… - Also unresolved: 1 minor or pre-existing.
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
There was a problem hiding this comment.
I reviewed the latest push and found no new bugs. The workerd build allow-list issue I raised earlier is fixed by the new workerd@ 1.20261006.1 entry in allowBuilds. A human look is still worthwhile, because this is a broad toolchain upgrade (Vite+ 0.1 to 1.1, Vitest 5, the new Cloudflare vitest plugin) with a regenerated lockfile of about 2.8k lines.
What was reviewed:
- The renamed plugin is updated consistently in package.json, vite.config.ts, config/tsconfig.workers.json, the packageExtensions key and the config/vitest.ts comment.
- Removing the braces patch and its GHSA exception is safe: braces no longer appears in pnpm-lock.yaml.
- The new
vi.spyOn(globalThis, 'fetch')/mockRestorein src/api.workers.test.ts restores fetch infinally, as the old wrapper did.
Extended reasoning...
The PR upgrades TanStack Start/Router and Hono for security fixes. It moves the dev toolchain to Vite+ 1.1.0, Vitest 5.0.3 and @ cloudflare/vitest-plugin 1.4.0, adds overrides, pins compatibility-data packages and regenerates pnpm-lock.yaml. Runtime code is only touched through the dependency bumps themselves. No application auth or data-handling code changes, apart from one worker test and a formatting reflow in test/factory.ts. The latest commit fixes the earlier workerd allowBuilds finding. I deferred rather than approved because the change is large: it includes major toolchain version jumps and a lockfile diff of about 2.8k lines, and their effect on the shipped Worker is best judged by a human.
Still open from earlier reviews (1):
- Unresolved: 1 minor or pre-existing.
## Summary Rebased onto #102, which cleared the earlier `pnpm audit` failure. This PR moves the dependencies #102 upgraded to their newer patch releases, fixes a range mismatch in a pinned dependency, and clears [GHSA-p458-7748-mcp3](GHSA-p458-7748-mcp3). That advisory was published after #102 and also fails `main`'s audit: Vitest servers misapply `server.fs.deny`. It needs vite-plus 1.3.0 and Vitest 5.0.5. | Dependency | `main` (#102) | This PR | | --- | --- | --- | | @tanstack/react-start | 1.168.60 | 1.169.2 | | @tanstack/react-router | 1.170.41 | 1.170.44 | | @tanstack/router-plugin | 1.168.42 | 1.168.46 | | hono (direct and transitive) | 4.13.11 | 4.13.13 | | @modelcontextprotocol/sdk | 1.32.0 | 1.32.1 | | vite-plus / vite-plus-core | 1.1.0 | 1.3.0 | | vitest (and `@vitest/*`) | 5.0.3 | 5.0.5 | | @cloudflare/vitest-plugin | 1.4.0 | 1.4.2 | | seroval | 1.6.7 | 1.6.9 | | @babel/core | 7.29.6 | 7.29.7 | - **@babel/core:** router-plugin requires `^7.29.7`, but `main` forces 7.29.6. - **TanStack Start 1.169:** - `.inputValidator()` was renamed to `.validator()`; this updates 3 call sites. - The lazy server-entry patch is regenerated as `@tanstack__react-start@1.169.2.patch`. Upstream's `createServerEntry` now adds request scoping and error handling, so the patch lazy-loads it too. - **Workers test cache:** Since 5.0.4, Vitest turns on `fsModuleCache` by default. vitest-plugin 1.4.2 can't read that cache, so every Workers test file failed with `readAll … .vitest-cache: no such file or directory` ([fixed in vitest-plugin 1.4.3](cloudflare/workers-sdk#16163)). The cache is disabled for the Workers project only, with a TODO to remove it once vitest-plugin is on 1.4.3. 1.4.3 is still under Aegis's 7-day limit. - **workerd build:** `workerd@1.20261012.1` (from vitest-plugin 1.4.2's Miniflare) is added to `allowBuilds`. - **Release-age excludes:** the `vite-plus@1.1.0` and `@babel/core@7.29.6` entries are removed. The new versions are older than the 1-day minimum release age. - **Aegis age limit:** locally, Socket Aegis blocks npm versions less than about 7 days old. Aegis exceptions were added for vite-plus 1.3.0 / Vitest 5.0.5 and their pinned packages. Every other version is at least 7 days old. Transitive `srvx` is pinned to 1.0.5 in the lockfile only. No overrides or audit suppressions are added. ## Validation - `pnpm audit`: only the two existing image-size findings remain (ignored). - `pnpm check`, `pnpm check:types`, `pnpm build`: pass. The frozen-lockfile install passes, and `pnpm install --lockfile-only` leaves the lockfile unchanged. - Unit tests: 57/57 pass on Vitest 5.0.5. - E2E: 22/22 pass. - Reproduced the Workers failure in a minimal Worker project using this repo's installed vitest-plugin 1.4.2 and Vitest 5.0.4. CI's Workers job passes with the workaround. With `fsModuleCache: true` it fails with CI's exact error; with `false` it passes. The resolved config shows `fsModuleCache` off for `workers` and on for `unit`. - Worker tests were not run locally because the ghcr pull of the pinned Tempo image is denied. CI covers them, as it did for #102 on vitest-plugin 1.4.0.
Upgrade vulnerable runtime and development dependencies so the repository's dependency audit passes:
@cloudflare/vitest-plugin1.4.0, updating the plugin import and worker types. Pin established Cloudflare and browser compatibility-data releases to satisfy CI's package-age policy, and add the workerd build that the new plugin's Miniflare pulls in toallowBuildsso strict dependency builds keep installing cleanly.Regenerate the lockfile and route ordering, remove the now-unused braces patch and its tests/audit exception, and apply the upgraded formatter's mapped-type layout in the test factory. Use a Vitest fetch spy in the receipt test so it works with the newer worker runtime's read-only fetch property.
Validation: CI passes on the latest commit, including dependency audit, generated-code verification, lint/format/type checks, production build, unit tests, worker tests, and E2E tests. Socket and dependency scanning also pass. Audit retains only the two existing image-size exceptions backed by the repository's local patch; no audit or security-policy exceptions were added.
Source changes only; deployment has not been verified.
Prompted by: @grandizzy