Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions e2e/react-start/import-protection/tests/import-protection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ for (const mode of ['build', 'dev'] as const) {
})
}

for (const mode of ['build', 'dev'] as const) {
for (const mode of ['build', 'dev', 'dev.warm'] as const) {
test(`no false positive for boundary-safe pattern in ${mode}`, async () => {
const violations = await readViolations(mode)

Expand Down Expand Up @@ -340,7 +340,7 @@ test('build has violations in both client and SSR environments', async () => {
expect(ssrViolations.length).toBeGreaterThanOrEqual(2)
})

for (const mode of ['build', 'dev'] as const) {
for (const mode of ['build', 'dev', 'dev.warm'] as const) {
test(`no false positive for factory-safe middleware pattern in ${mode}`, async () => {
const violations = await readViolations(mode)

Expand All @@ -363,7 +363,7 @@ for (const mode of ['build', 'dev'] as const) {
})
}

for (const mode of ['build', 'dev'] as const) {
for (const mode of ['build', 'dev', 'dev.warm'] as const) {
test(`no false positive for cross-boundary-safe pattern in ${mode}`, async () => {
const violations = await readViolations(mode)

Expand All @@ -387,7 +387,7 @@ for (const mode of ['build', 'dev'] as const) {
})
}

for (const mode of ['build', 'dev'] as const) {
for (const mode of ['build', 'dev', 'dev.warm'] as const) {
test(`cross-boundary-leak: leaky consumer still produces violation in ${mode}`, async () => {
const violations = await readViolations(mode)

Expand All @@ -407,7 +407,7 @@ for (const mode of ['build', 'dev'] as const) {
})
}

for (const mode of ['build', 'dev'] as const) {
for (const mode of ['build', 'dev', 'dev.warm'] as const) {
test(`beforeload-leak: server import via beforeLoad triggers client violation in ${mode}`, async () => {
const violations = await readViolations(mode)

Expand Down Expand Up @@ -489,7 +489,7 @@ test('warm run traces include line numbers', async () => {
if (step.specifier?.includes('?tsr-split=')) continue
if (step.file.includes('routeTree.gen')) continue
const prev = v!.trace[i - 1]
if (prev?.specifier?.includes('?tsr-split=')) continue
if (prev.specifier?.includes('?tsr-split=')) continue

expect(
step.line,
Expand All @@ -498,3 +498,45 @@ test('warm run traces include line numbers', async () => {
expect(step.line).toBeGreaterThan(0)
}
})

// Regression: SERVER_FN_LOOKUP variant pollution + hasSeenEntry bug.
//
// The Start compiler excludes ?server-fn-module-lookup variants from its
// transform, so they retain the original (untransformed) imports. If the
// reachability check considers those untransformed imports, it would see
// edges the compiler has actually pruned, causing a false positive.
//
// Additionally, in Vite dev mode the client entry resolves through virtual
// modules, so a naïve resolveId-based entry detection may never fire,
// causing all deferred violations to be confirmed immediately.
//
// The cross-boundary-safe pattern exercises both bugs:
// auth-wrapper.ts exports createAuthServerFn (factory with middleware)
// → session-util.ts wraps @tanstack/react-start/server
// → usage.ts calls createAuthServerFn().handler()
// All server imports are inside compiler boundaries and must be pruned.

for (const mode of ['dev', 'dev.warm'] as const) {
test(`regression: server-fn-lookup variant does not cause false positive in ${mode}`, async () => {
const violations = await readViolations(mode)

// Any violation touching the cross-boundary-safe chain where the importer
// or trace includes the ?server-fn-module-lookup query (or its normalized
// key) would indicate the lookup variant polluted the reachability check.
const lookupHits = violations.filter(
(v) =>
v.envType === 'client' &&
(v.importer.includes('auth-wrapper') ||
v.importer.includes('session-util') ||
v.importer.includes('cross-boundary-safe') ||
v.trace.some(
(s) =>
s.file.includes('auth-wrapper') ||
s.file.includes('session-util') ||
s.file.includes('cross-boundary-safe'),
)),
)

expect(lookupHits).toEqual([])
})
}
33 changes: 13 additions & 20 deletions packages/start-plugin-core/src/import-protection-plugin/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,18 +185,6 @@ interface EnvState {
*/
postTransformImports: Map<string, Set<string>>

/**
* Whether a `resolveId` call without an importer has been observed for this
* environment since `buildStart`. Vite calls `resolveId(source, undefined)`
* for true entry modules during a cold start. On warm start (`.vite` cache
* exists), Vite reuses its module graph and does NOT call `resolveId` for
* entries, so this stays `false`.
*
* When `false`, the import graph is considered unreliable (edges may be
* missing) and violations are reported immediately instead of deferred.
*/
hasSeenEntry: boolean

/**
* Violations deferred in dev mock mode. Keyed by the violating importer's
* normalized file path. Violations are confirmed or discarded by the
Expand Down Expand Up @@ -579,7 +567,6 @@ export function importProtectionPlugin(
},
},
postTransformImports: new Map(),
hasSeenEntry: false,
serverFnLookupModules: new Set(),
pendingViolations: new Map(),
}
Expand Down Expand Up @@ -682,12 +669,16 @@ export function importProtectionPlugin(

// Check all code-split variants for this parent. The edge is
// live if ANY variant's resolved imports include `current`.
// Skip SERVER_FN_LOOKUP variants — they contain untransformed
// code (the compiler excludes them), so their import lists
// include imports that the compiler would normally strip.
const keySet = env.transformResultKeysByFile.get(parent)
let anyVariantCached = false
let edgeLive = false

if (keySet) {
for (const k of keySet) {
if (k.includes(SERVER_FN_LOOKUP_QUERY)) continue
const resolvedImports = env.postTransformImports.get(k)
if (resolvedImports) {
anyVariantCached = true
Expand Down Expand Up @@ -752,10 +743,12 @@ export function importProtectionPlugin(
const toDelete: Array<string> = []

for (const [file, violations] of env.pendingViolations) {
// On warm start, skip graph reachability — confirm immediately.
const status = env.hasSeenEntry
? checkPostTransformReachability(env, file)
: 'reachable'
// Wait for entries before running reachability. registerEntries()
// populates entries at buildStart; resolveId(!importer) may add more.
const status =
env.graph.entries.size > 0
? checkPostTransformReachability(env, file)
: 'unknown'

if (status === 'reachable') {
for (const pv of violations) {
Expand Down Expand Up @@ -1033,7 +1026,6 @@ export function importProtectionPlugin(
envState.transformResultCache.clear()
envState.transformResultKeysByFile.clear()
envState.postTransformImports.clear()
envState.hasSeenEntry = false
envState.serverFnLookupModules.clear()
envState.graph.clear()
envState.deniedSources.clear()
Expand Down Expand Up @@ -1118,7 +1110,6 @@ export function importProtectionPlugin(
source,
importer: importerPath,
isEntryResolve,
hasSeenEntry: env.hasSeenEntry,
command: config.command,
behavior: config.effectiveBehavior,
})
Expand All @@ -1141,7 +1132,9 @@ export function importProtectionPlugin(

if (!importer) {
env.graph.addEntry(source)
env.hasSeenEntry = true
// Flush pending violations now that an additional entry is known
// and reachability analysis may have new roots.
await processPendingViolations(env, this.warn.bind(this))
return undefined
}

Expand Down
Loading