Skip to content

Commit 087f8d6

Browse files
authored
fix(cache): protect cache function references (#3385)
* fix(cache): protect cache function references * fix(actions): preserve production reference ownership * fix(actions): allow hoisted default references * test(cache): expand opaque reference coverage * fix(actions): preserve aliased dev references * fix(actions): match opaque reference shape
1 parent a58a991 commit 087f8d6

17 files changed

Lines changed: 561 additions & 25 deletions

File tree

‎packages/cloudflare/tests/response-store-adapter.e2e.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,18 @@ describe("Cloudflare Workers Response Store adapter", () => {
475475
const firstPage = await cacheStatus("/use-cache");
476476
const firstData = htmlValue(firstPage.body, "use-cache-value");
477477
const firstPageRenders = Number(htmlValue(firstPage.body, "use-cache-route-renders"));
478+
const entries = (await metadataEntries()).flat() as Array<{
479+
revalidator?: { args?: unknown[]; id?: unknown };
480+
}>;
481+
const cacheFunctionEntry = entries.find(
482+
(entry) => entry.revalidator?.id === "vinext:cache-function",
483+
);
484+
assert.ok(cacheFunctionEntry);
485+
const serializedInvocation = cacheFunctionEntry.revalidator?.args?.[1];
486+
assert.ok(typeof serializedInvocation === "string");
487+
const invocation = JSON.parse(serializedInvocation) as { referenceId?: unknown };
488+
assert.ok(typeof invocation.referenceId === "string");
489+
assert.match(invocation.referenceId, /^[0-9a-f]{12}#\$\$vinext_cache_[0-9a-f]{64}$/);
478490

479491
await new Promise((resolve) => setTimeout(resolve, 1_100));
480492

‎packages/vinext/src/plugins/use-cache-callable.ts‎

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { createHmac, randomBytes } from "node:crypto";
12
import { createRequire } from "node:module";
23
import path from "pathslash";
34
import { pathToFileURL } from "node:url";
@@ -166,6 +167,30 @@ function hasFunctionDirective(
166167
);
167168
}
168169

170+
function getFunctionDirectiveExportNames(
171+
transforms: RscTransforms,
172+
ast: Program,
173+
directive: string,
174+
): Set<string> {
175+
const names = new Set<string>();
176+
for (const group of transforms.scanModuleExports(ast)) {
177+
const entries =
178+
group.type === "declaration"
179+
? [group.export]
180+
: group.type === "variable-declaration"
181+
? group.declarators.flatMap((declarator) => declarator.exports)
182+
: group.type === "specifiers"
183+
? group.exports
184+
: group.type === "default"
185+
? [{ exportName: "default", meta: group.meta }]
186+
: [];
187+
for (const entry of entries) {
188+
if (hasFunctionDirective(entry.meta, directive)) names.add(entry.exportName);
189+
}
190+
}
191+
return names;
192+
}
193+
169194
function getCacheWrapperOptions(
170195
options: Options,
171196
id: string,
@@ -193,6 +218,13 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
193218
pathToFileURL(rscModulePath).href
194219
);
195220
const transforms: RscTransforms = await import(pathToFileURL(transformsPath).href);
221+
// Cache functions use React's server-reference transport when they are passed
222+
// to Client Components or invoked by the Response Store. The upstream RSC
223+
// plugin's reference key is a public, deterministic module-path hash, so the
224+
// original export name must not also be the remotely addressable name. A
225+
// per-plugin secret keeps aliases stable across every environment/build pass
226+
// in one Vite build without making sibling exports derivable from each other.
227+
const referenceSecret = randomBytes(32);
196228
let manager: RscPluginManager | undefined;
197229

198230
return {
@@ -241,6 +273,13 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
241273
}
242274

243275
const reference = manager.serverReferences.resolve(id, "rsc");
276+
const relativeImportId = manager.toRelativeId(reference.importId);
277+
const secureExportName = (name: string) =>
278+
`$$vinext_cache_${createHmac("sha256", referenceSecret)
279+
.update(relativeImportId)
280+
.update("\0")
281+
.update(name)
282+
.digest("hex")}`;
244283
const isRsc = this.environment.name === "rsc";
245284

246285
if (!isRsc) {
@@ -263,6 +302,11 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
263302
return;
264303
}
265304

305+
const useServerExportNames = getFunctionDirectiveExportNames(
306+
transforms,
307+
ast,
308+
"use server",
309+
);
266310
const result = transforms.transformDirectiveProxyExport(ast, {
267311
code,
268312
directive: moduleDirective,
@@ -272,7 +316,7 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
272316
return true;
273317
},
274318
runtime: (name) =>
275-
`$$ReactClient.createServerReference(${JSON.stringify(`${reference.referenceKey}#${name}`)},$$ReactClient.callServer,undefined,${this.environment.mode === "dev" ? "$$ReactClient.findSourceMapURL" : "undefined"},${JSON.stringify(name)})`,
319+
`$$ReactClient.createServerReference(${JSON.stringify(`${reference.referenceKey}#${useServerExportNames.has(name) ? name : secureExportName(name)}`)},$$ReactClient.callServer,undefined,${this.environment.mode === "dev" ? "$$ReactClient.findSourceMapURL" : "undefined"},${JSON.stringify(name)})`,
276320
});
277321
if (!result?.output.hasChanged()) {
278322
manager.serverReferences.deleteClaim(PLUGIN_NAME, id);
@@ -281,7 +325,9 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
281325

282326
manager.serverReferences.replaceClaim(PLUGIN_NAME, id, {
283327
...reference,
284-
exportNames: result.exportNames,
328+
exportNames: result.exportNames.map((name) =>
329+
useServerExportNames.has(name) ? name : secureExportName(name),
330+
),
285331
});
286332
const runtimeEnvironment = this.environment.name === "client" ? "browser" : "ssr";
287333
result.output.prepend(
@@ -290,6 +336,7 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
290336
return magicStringTransformResult(result.output, { hires: "boundary", source: id });
291337
}
292338

339+
const secureExports = new Set<string>();
293340
const wrap = (
294341
value: string,
295342
name: string,
@@ -298,9 +345,11 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
298345
isModuleDirective: boolean,
299346
) => {
300347
const variant = directiveMatch[1] ?? "";
348+
const secureName = secureExportName(name);
349+
secureExports.add(secureName);
301350
const wrapperOptions = {
302351
...getCacheWrapperOptions(options, id, name, isModuleDirective, meta),
303-
serverReferenceId: `${reference.referenceKey}#${name}`,
352+
serverReferenceId: `${reference.referenceKey}#${secureName}`,
304353
};
305354
return `$$cacheRuntime.registerCachedFunction(${value}, ${JSON.stringify(`${id}:${name}`)}, ${JSON.stringify(variant)}, ${JSON.stringify(wrapperOptions)})`;
306355
};
@@ -313,8 +362,9 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
313362
isModuleDirective: boolean,
314363
) => {
315364
const cached = wrap(value, name, directiveMatch, meta, isModuleDirective);
365+
const secureName = secureExportName(name);
316366
needsReactServer = true;
317-
return `$$VinextReactServer.registerServerReference(${cached}, ${JSON.stringify(reference.referenceKey)}, ${JSON.stringify(name)})`;
367+
return `(${secureName} = $$VinextReactServer.registerServerReference(${cached}, ${JSON.stringify(reference.referenceKey)}, ${JSON.stringify(secureName)}))`;
318368
};
319369

320370
const result = moduleDirective
@@ -336,6 +386,7 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
336386
directive: USE_CACHE_DIRECTIVE_CANDIDATE,
337387
rejectNonAsyncFunction: true,
338388
hoistRuntime: true,
389+
noExport: true,
339390
runtime: (value, name, meta) =>
340391
runtime(value, name, matchUseCacheDirective(meta.directiveMatch[0]), meta, false),
341392
encode: (value) => `$$cacheRuntime.encryptCacheCaptures(${value})`,
@@ -348,7 +399,7 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
348399

349400
manager.serverReferences.replaceClaim(PLUGIN_NAME, id, {
350401
...reference,
351-
exportNames: "names" in result ? result.names : result.exportNames,
402+
exportNames: [...secureExports],
352403
});
353404
const importPosition =
354405
ast.body.find((node) => !("directive" in node))?.start ?? code.length;
@@ -358,10 +409,14 @@ export async function createUseCacheCallablePlugin(options: Options): Promise<Pl
358409
`import * as $$cacheRuntime from ${JSON.stringify(options.cacheRuntime)};`,
359410
needsReactServer &&
360411
`import * as $$VinextReactServer from "@vitejs/plugin-rsc/react/rsc/server";`,
412+
secureExports.size > 0 && `let ${[...secureExports].join(",")};`,
361413
]
362414
.filter(Boolean)
363415
.join("\n") + "\n",
364416
);
417+
if (secureExports.size > 0) {
418+
result.output.append(`\nexport { ${[...secureExports].join(",")} };\n`);
419+
}
365420
return magicStringTransformResult(result.output, { hires: "boundary", source: id });
366421
},
367422
},

‎packages/vinext/src/server/app-server-action-execution.ts‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,57 @@ function isAppServerActionFunction(action: unknown): action is AppServerActionFu
852852
return typeof action === "function";
853853
}
854854

855+
function normalizeDevServerReferenceId(id: string): string {
856+
const exportSeparator = id.indexOf("#");
857+
if (exportSeparator === -1) return id;
858+
const moduleId = id.slice(0, exportSeparator);
859+
// plugin-rsc's dev createServerManifest() appends this HMR-busting tag to
860+
// serialized module ids. loadServerAction() removes it before importing, so
861+
// compare the registered and requested identities on that same basis.
862+
const cacheTag = moduleId.indexOf("$$cache=");
863+
return (cacheTag === -1 ? moduleId : moduleId.slice(0, cacheTag)) + id.slice(exportSeparator);
864+
}
865+
866+
function requiresRegisteredServerReferenceMatch(actionId: string): boolean {
867+
const exportSeparator = actionId.indexOf("#");
868+
if (exportSeparator === -1) return true;
869+
// Production requests are authorized by the generated action-owner manifest
870+
// before execution. Dev has no manifest, so its path-based references need
871+
// the runtime registration check to prevent arbitrary named-export loading.
872+
return !/^[0-9a-f]{12}$/.test(actionId.slice(0, exportSeparator));
873+
}
874+
875+
function matchesRegisteredServerReference(
876+
action: AppServerActionFunction,
877+
actionId: string,
878+
): boolean {
879+
const registeredId = Reflect.get(action, "$$id");
880+
if (typeof registeredId !== "string") return false;
881+
882+
const normalizedRegisteredId = normalizeDevServerReferenceId(registeredId);
883+
const normalizedActionId = normalizeDevServerReferenceId(actionId);
884+
if (normalizedRegisteredId === normalizedActionId) return true;
885+
886+
// React stores only the most recently registered ID on a function. When one
887+
// ordinary Server Action is exported under multiple names, plugin-rsc
888+
// registers every alias on the same function object, so `$$id` alone cannot
889+
// tell which aliases are valid. Loading any same-module alias still proves
890+
// that it resolves to that registered Server Action. Cache references are the
891+
// exception: their opaque export deliberately must not authorize a source
892+
// export name that happens to resolve to the same wrapper.
893+
const registeredSeparator = normalizedRegisteredId.indexOf("#");
894+
const actionSeparator = normalizedActionId.indexOf("#");
895+
if (registeredSeparator === -1 || actionSeparator === -1) return false;
896+
if (
897+
normalizedRegisteredId.slice(0, registeredSeparator) !==
898+
normalizedActionId.slice(0, actionSeparator)
899+
) {
900+
return false;
901+
}
902+
const registeredExport = normalizedRegisteredId.slice(registeredSeparator + 1);
903+
return !/^\$\$vinext_cache_[0-9a-f]{64}$/.test(registeredExport);
904+
}
905+
855906
function getServerActionFailureMessage(error: unknown): string {
856907
return error instanceof Error && error.message ? error.message : String(error);
857908
}
@@ -1222,6 +1273,17 @@ export async function handleProgressiveServerActionRequest(
12221273
return null;
12231274
}
12241275

1276+
if (
1277+
directActionId &&
1278+
requiresRegisteredServerReferenceMatch(directActionId) &&
1279+
!matchesRegisteredServerReference(action, directActionId)
1280+
) {
1281+
return createActionNotFoundResponse(directActionId, {
1282+
clearRequestContext: options.clearRequestContext,
1283+
getAndClearPendingCookies: options.getAndClearPendingCookies,
1284+
});
1285+
}
1286+
12251287
const decodedActionId = Reflect.get(action, "$$id");
12261288
if (
12271289
typeof decodedActionId === "string" &&
@@ -1515,7 +1577,11 @@ export async function handleServerActionRscRequest<
15151577
throw error;
15161578
}
15171579

1518-
if (!isAppServerActionFunction(loadedAction)) {
1580+
if (
1581+
!isAppServerActionFunction(loadedAction) ||
1582+
(requiresRegisteredServerReferenceMatch(options.actionId) &&
1583+
!matchesRegisteredServerReference(loadedAction, options.actionId))
1584+
) {
15191585
return createActionNotFoundResponse(options.actionId, {
15201586
clearRequestContext: options.clearRequestContext,
15211587
getAndClearPendingCookies: options.getAndClearPendingCookies,
@@ -1558,7 +1624,11 @@ export async function handleServerActionRscRequest<
15581624
throw error;
15591625
}
15601626

1561-
if (!isAppServerActionFunction(loadedAction)) {
1627+
if (
1628+
!isAppServerActionFunction(loadedAction) ||
1629+
(requiresRegisteredServerReferenceMatch(options.actionId) &&
1630+
!matchesRegisteredServerReference(loadedAction, options.actionId))
1631+
) {
15621632
return createActionNotFoundResponse(options.actionId, {
15631633
clearRequestContext: options.clearRequestContext,
15641634
getAndClearPendingCookies: options.getAndClearPendingCookies,

‎packages/vinext/src/shims/cache-runtime.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,12 @@ export function registerCachedFunction<TArgs extends unknown[], TResult>(
599599
): (...args: TArgs) => Promise<TResult> {
600600
const cacheVariant = variant ?? "";
601601
const omitAppPageSearchParamsFromFirstArg = options.appPageDefaultExport === true;
602+
// A replayable entry stores this reference ID for Response Store
603+
// regeneration. Keep entries produced with an older build's opaque alias
604+
// unreachable if a stable deployment/build ID is reused.
605+
const cacheFunctionId = options.serverReferenceId
606+
? JSON.stringify([id, options.serverReferenceId])
607+
: id;
602608

603609
// In dev mode, skip the shared cache so code changes are immediately
604610
// visible after HMR. Without this, the MemoryCacheHandler returns stale
@@ -691,10 +697,10 @@ export function registerCachedFunction<TArgs extends unknown[], TResult>(
691697
const encoded = await rsc.encodeReply(processedArgs, {
692698
temporaryReferences: tempRefs,
693699
});
694-
cacheKey = buildUseCacheKey(id, keySeed, await replyToCacheKey(encoded));
700+
cacheKey = buildUseCacheKey(cacheFunctionId, keySeed, await replyToCacheKey(encoded));
695701
} else {
696702
const argsKey = processedArgs.length > 0 ? stableStringify(processedArgs) : undefined;
697-
cacheKey = buildUseCacheKey(id, keySeed, argsKey);
703+
cacheKey = buildUseCacheKey(cacheFunctionId, keySeed, argsKey);
698704
}
699705
} catch {
700706
// Non-serializable arguments — run without caching

‎tests/app-router-dev-server.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,6 +2170,57 @@ describe("App Router integration", () => {
21702170
expect(res.headers.get("x-nextjs-action-not-found")).toBe("1");
21712171
});
21722172

2173+
it("invokes every export alias for the same server action", async () => {
2174+
for (const exportName of [
2175+
"firstAliasedAction",
2176+
"secondAliasedAction",
2177+
"$$vinext_cache_custom",
2178+
]) {
2179+
const res = await fetch(`${baseUrl}/actions.rsc`, {
2180+
method: "POST",
2181+
headers: {
2182+
"Content-Type": "text/plain",
2183+
"x-rsc-action": `/app/actions/actions.ts#${exportName}`,
2184+
},
2185+
body: JSON.stringify(["proof"]),
2186+
});
2187+
2188+
expect(res.status).toBe(200);
2189+
expect(await res.text()).toContain("aliased:proof");
2190+
}
2191+
});
2192+
2193+
it("rejects hidden cache functions' original dev export names", async () => {
2194+
const anonymous = await fetch(`${baseUrl}/use-cache-hidden-reference?record=victim`);
2195+
expect(anonymous.status).toBe(200);
2196+
expect(await anonymous.text()).toContain("FORBIDDEN");
2197+
2198+
const defaultVictim = await fetch(
2199+
`${baseUrl}/use-cache-hidden-reference?record=victim&source=default`,
2200+
{ headers: { Authorization: "Bearer fixture-victim-session" } },
2201+
);
2202+
expect(defaultVictim.status).toBe(200);
2203+
expect(await defaultVictim.text()).toContain("VICTIM_DEFAULT_PRIVATE_RECORD");
2204+
2205+
for (const [exportName, secret] of [
2206+
["readRecord", "VICTIM_PRIVATE_RECORD"],
2207+
["default", "VICTIM_DEFAULT_PRIVATE_RECORD"],
2208+
] as const) {
2209+
const exploit = await fetch(`${baseUrl}/use-cache-hidden-reference.rsc`, {
2210+
method: "POST",
2211+
headers: {
2212+
"Content-Type": "text/plain",
2213+
"x-rsc-action": `/app/use-cache-hidden-reference/records.ts#${exportName}`,
2214+
},
2215+
body: JSON.stringify(["victim"]),
2216+
});
2217+
2218+
expect(exploit.status).toBe(404);
2219+
expect(exploit.headers.get("x-nextjs-action-not-found")).toBe("1");
2220+
expect(await exploit.text()).not.toContain(secret);
2221+
}
2222+
});
2223+
21732224
it("returns action-not-found for an MPA form POST to a page with no decodable action", async () => {
21742225
// Ported from Next.js: test/e2e/app-dir/no-server-actions/no-server-actions.test.ts
21752226
// ("should error when triggering an MPA action on an app with no server actions")

0 commit comments

Comments
 (0)