Skip to content

Commit 29c3502

Browse files
deyaaeldeenCopilot
andcommitted
fix: verify module identity in cache protocol to prevent stale modules
The module runner's `urlToIdModuleMap` caches modules by URL. When a bare specifier (e.g., `@azure/core-lro`) is used as the URL, the first resolved version is cached and served to all subsequent importers, even if they should receive a different physical package. Additionally, the `{ cache: true }` protocol between the module runner and server had no identity verification. The server confirms a module was transformed without checking if the client's cached module matches the server's resolved module. Fix: - Server now includes the resolved module ID in `{ cache: true }` responses via the new optional `id` field on `CachedFetchResult`. - Client verifies that its cached module ID matches the server's. On mismatch, the client refetches the module without the cache flag. This prevents silent data corruption when multiple versions of a dependency exist in a monorepo with nested `node_modules`. Fixes #22079 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6daa10f commit 29c3502

File tree

5 files changed

+277
-2
lines changed

5 files changed

+277
-2
lines changed
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { describe, expect, test } from 'vitest'
2+
import { EvaluatedModules } from '../evaluatedModules'
3+
4+
describe('EvaluatedModules', () => {
5+
test('ensureModule creates a new module and indexes by id and url', () => {
6+
const modules = new EvaluatedModules()
7+
const mod = modules.ensureModule('/path/to/module.js', '/url/module.js')
8+
9+
expect(mod.id).toBe('/path/to/module.js')
10+
expect(mod.url).toBe('/url/module.js')
11+
expect(modules.getModuleById('/path/to/module.js')).toBe(mod)
12+
expect(modules.getModuleByUrl('/url/module.js')).toBe(mod)
13+
})
14+
15+
test('ensureModule returns existing module when id matches', () => {
16+
const modules = new EvaluatedModules()
17+
const mod1 = modules.ensureModule('/path/to/module.js', '/url/v1')
18+
const mod2 = modules.ensureModule('/path/to/module.js', '/url/v2')
19+
20+
expect(mod1).toBe(mod2)
21+
// New url should also map to the same module
22+
expect(modules.getModuleByUrl('/url/v2')).toBe(mod1)
23+
})
24+
25+
test('different ids with same url creates separate modules but url maps to latest', () => {
26+
const modules = new EvaluatedModules()
27+
const mod1 = modules.ensureModule('/path/v1/index.js', 'shared-dep')
28+
const mod2 = modules.ensureModule('/path/v2/index.js', 'shared-dep')
29+
30+
// They should be different module nodes
31+
expect(mod1).not.toBe(mod2)
32+
expect(mod1.id).toBe('/path/v1/index.js')
33+
expect(mod2.id).toBe('/path/v2/index.js')
34+
35+
// The url 'shared-dep' should map to whichever was registered last
36+
// This is the bug scenario: bare specifier URL maps to wrong module
37+
expect(modules.getModuleByUrl('shared-dep')).toBe(mod2)
38+
39+
// But id-based lookups remain correct
40+
expect(modules.getModuleById('/path/v1/index.js')).toBe(mod1)
41+
expect(modules.getModuleById('/path/v2/index.js')).toBe(mod2)
42+
})
43+
})
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
import { describe, expect, test } from 'vitest'
2+
import type { HotPayload } from '#types/hot'
3+
import type {
4+
CachedFetchResult,
5+
ExternalFetchResult,
6+
FetchResult,
7+
ViteFetchResult,
8+
} from '../../shared/invokeMethods'
9+
import { ModuleRunner } from '../runner'
10+
import type { ModuleEvaluator, ModuleRunnerContext } from '../types'
11+
12+
// Minimal evaluator that resolves without executing code
13+
class MockEvaluator implements ModuleEvaluator {
14+
startOffset = 0
15+
async runInlinedModule(_context: ModuleRunnerContext): Promise<any> {
16+
// no-op
17+
}
18+
async runExternalModule(_file: string): Promise<any> {
19+
return { __esModule: true, version: 'mock' }
20+
}
21+
}
22+
23+
function createMockTransport(
24+
handler: (
25+
url: string,
26+
importer: string | undefined,
27+
options: any,
28+
) => Promise<FetchResult> | FetchResult,
29+
) {
30+
return {
31+
invoke: async (data: HotPayload): Promise<{ result: any }> => {
32+
const payload = data as any
33+
if (payload.data?.name === 'fetchModule') {
34+
const [url, importer, options] = payload.data.data
35+
const result = await handler(url, importer, options)
36+
return { result }
37+
}
38+
if (payload.data?.name === 'getBuiltins') {
39+
return { result: [] }
40+
}
41+
return { result: undefined }
42+
},
43+
}
44+
}
45+
46+
describe('module runner cache identity verification', () => {
47+
// Regression tests for https://github.com/vitejs/vite/issues/22079
48+
//
49+
// When a bare specifier (e.g., "@azure/core-lro") is externalized,
50+
// the module runner caches it by the bare specifier URL. If a second
51+
// import of the same specifier should resolve to a different physical
52+
// module, the cache returns the wrong one. The server's { cache: true }
53+
// response now includes the resolved module ID so the client can detect
54+
// and recover from this mismatch.
55+
56+
test('refetches when server cache response has different module id', async () => {
57+
const fetchCalls: { url: string; cached: boolean }[] = []
58+
let callCount = 0
59+
60+
// Simulate: first resolution externalizes to v1, second should be v2
61+
const externalV1: ExternalFetchResult = {
62+
externalize: 'file:///path/to/dep-v1/index.js',
63+
type: 'module',
64+
}
65+
const moduleV2: ViteFetchResult = {
66+
code: 'export const version = "v2"; export const extraExport = "only-in-v2"',
67+
file: '/path/to/dep-v2/index.js',
68+
id: '/path/to/dep-v2/index.js',
69+
url: '/dep-v2/index.js',
70+
invalidate: false,
71+
}
72+
73+
const runner = new ModuleRunner(
74+
{
75+
transport: createMockTransport((url, _importer, options) => {
76+
callCount++
77+
fetchCalls.push({
78+
url,
79+
cached: !!options?.cached,
80+
})
81+
82+
if (callCount === 1) {
83+
// First fetch: externalize to v1
84+
return externalV1
85+
}
86+
if (callCount === 2) {
87+
// Server resolved to v2 (different module id), but client
88+
// has v1 cached. Return cache with server's resolved id.
89+
return {
90+
cache: true,
91+
id: moduleV2.id,
92+
} as CachedFetchResult
93+
}
94+
// Third: client detected mismatch, refetches without cache flag
95+
return moduleV2
96+
}),
97+
hmr: false,
98+
sourcemapInterceptor: false,
99+
},
100+
new MockEvaluator(),
101+
)
102+
103+
// First import: resolves to v1 (externalized), cached by bare specifier
104+
await runner.import('shared-dep')
105+
expect(fetchCalls).toHaveLength(1)
106+
107+
// Verify v1 is cached under the bare specifier URL
108+
const cachedMod = runner.evaluatedModules.getModuleByUrl('shared-dep')
109+
expect(cachedMod).toBeDefined()
110+
expect(cachedMod!.id).toContain('dep-v1')
111+
112+
// Second import: client sends cached: true, server returns
113+
// { cache: true, id: '/path/to/dep-v2/index.js' } — mismatch!
114+
// Runner should refetch and get v2.
115+
await runner.import('shared-dep')
116+
117+
expect(fetchCalls).toHaveLength(3)
118+
expect(fetchCalls[1]).toEqual({ url: 'shared-dep', cached: true })
119+
expect(fetchCalls[2]).toEqual({ url: 'shared-dep', cached: false })
120+
121+
await runner.close()
122+
})
123+
124+
test('uses cache when server response has matching module id', async () => {
125+
const fetchCalls: { url: string; cached: boolean }[] = []
126+
let callCount = 0
127+
128+
const externalV1: ExternalFetchResult = {
129+
externalize: 'file:///path/to/dep-v1/index.js',
130+
type: 'module',
131+
}
132+
133+
const runner = new ModuleRunner(
134+
{
135+
transport: createMockTransport((url, _importer, options) => {
136+
callCount++
137+
fetchCalls.push({
138+
url,
139+
cached: !!options?.cached,
140+
})
141+
142+
if (callCount === 1) {
143+
return externalV1
144+
}
145+
// Server confirms same module id — cache is valid
146+
return {
147+
cache: true,
148+
id: externalV1.externalize,
149+
} as CachedFetchResult
150+
}),
151+
hmr: false,
152+
sourcemapInterceptor: false,
153+
},
154+
new MockEvaluator(),
155+
)
156+
157+
await runner.import('shared-dep')
158+
await runner.import('shared-dep')
159+
160+
// Only 2 calls: fetch + cache confirmation (no refetch)
161+
expect(fetchCalls).toHaveLength(2)
162+
expect(fetchCalls[1]).toEqual({ url: 'shared-dep', cached: true })
163+
164+
await runner.close()
165+
})
166+
167+
test('uses cache when server response omits id (backward compat)', async () => {
168+
const fetchCalls: { url: string; cached: boolean }[] = []
169+
let callCount = 0
170+
171+
const externalV1: ExternalFetchResult = {
172+
externalize: 'file:///path/to/dep-v1/index.js',
173+
type: 'module',
174+
}
175+
176+
const runner = new ModuleRunner(
177+
{
178+
transport: createMockTransport((url, _importer, options) => {
179+
callCount++
180+
fetchCalls.push({
181+
url,
182+
cached: !!options?.cached,
183+
})
184+
185+
if (callCount === 1) {
186+
return externalV1
187+
}
188+
// Old-style cache response without id
189+
return { cache: true } as FetchResult
190+
}),
191+
hmr: false,
192+
sourcemapInterceptor: false,
193+
},
194+
new MockEvaluator(),
195+
)
196+
197+
await runner.import('shared-dep')
198+
await runner.import('shared-dep')
199+
200+
// Only 2 calls: backward compatible, no refetch
201+
expect(fetchCalls).toHaveLength(2)
202+
expect(fetchCalls[1]).toEqual({ url: 'shared-dep', cached: true })
203+
204+
await runner.close()
205+
})
206+
})

packages/vite/src/module-runner/runner.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from '../shared/moduleRunnerTransport'
99
import { createIsBuiltin } from '../shared/builtin'
1010
import type { EvaluatedModuleNode } from './evaluatedModules'
11-
import { EvaluatedModules } from './evaluatedModules'
11+
import { EvaluatedModules, normalizeModuleId } from './evaluatedModules'
1212
import type {
1313
ModuleEvaluator,
1414
ModuleRunnerContext,
@@ -309,6 +309,25 @@ export class ModuleRunner {
309309
`Module "${url}" was mistakenly invalidated during fetch phase.`,
310310
)
311311
}
312+
// Verify the server resolved to the same module we have cached.
313+
// When a bare specifier maps to different physical packages for
314+
// different importers (e.g. monorepos with duplicate deps), the
315+
// urlToIdModuleMap may hold a stale entry. If the server resolved
316+
// to a different module, refetch without the cache flag.
317+
if (
318+
fetchedModule.id &&
319+
cachedModule.id !== normalizeModuleId(fetchedModule.id)
320+
) {
321+
this.debug?.(
322+
'[module runner] cache identity mismatch for',
323+
url,
324+
'- cached:',
325+
cachedModule.id,
326+
'server:',
327+
fetchedModule.id,
328+
)
329+
return this.getModuleInformation(url, importer, undefined)
330+
}
312331
return cachedModule
313332
}
314333

packages/vite/src/node/ssr/fetchModule.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export async function fetchModule(
8585

8686
// if url is already cached, we can just confirm it's also cached on the server
8787
if (options.cached && cached) {
88-
return { cache: true }
88+
return { cache: true, id: mod.id }
8989
}
9090

9191
let result = await environment.transformRequest(url)

packages/vite/src/shared/invokeMethods.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ export interface CachedFetchResult {
1414
* it wasn't invalidated on the server side.
1515
*/
1616
cache: true
17+
/**
18+
* The resolved module ID on the server. The client uses this to verify
19+
* that its cached module matches what the server resolved. When a bare
20+
* specifier maps to different physical packages for different importers,
21+
* the client's cache may hold the wrong version.
22+
*/
23+
id?: string
1724
}
1825

1926
export interface ExternalFetchResult {

0 commit comments

Comments
 (0)