Skip to content

Commit bf704ae

Browse files
authored
fix(actions): authorize server references in fetch action arguments (#3991)
* fix(actions): authorize server references in fetch action arguments decodeReply() resolves `$h` argument references through the RSC server manifest, which accepts any export of a server-reference module. A crafted fetch action body could therefore pass a page's default export or another route's Server Action into an action as a callable argument, which Next.js never decodes. Scan the reply body for server references before decoding, resolving their metadata the way React does (including async-iterable entries that only parse inside an iterator result), and reject the request as action-not-found unless every reference is a Server Action owned by the current route (production owner manifest) and loads as a registered Server Action (dev). Apply the same registration check to bound references in progressive `$ACTION_REF_` submissions. * fix(actions): scope fetch action argument references to the route's manifest Replace the raw reply scanner with a route-scoped server manifest passed to React's decodeReply(), so every server reference React resolves goes through the action-owner check without vinext mirroring the reply wire format. * fix(actions): stop React's module-id fallback after a denied argument reference
1 parent 218790e commit bf704ae

6 files changed

Lines changed: 232 additions & 6 deletions

File tree

‎packages/vinext/src/entries/app-rsc-entry.ts‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -828,17 +828,30 @@ import {
828828
hasServerActions
829829
? `decodeAction,
830830
decodeFormState,
831-
decodeReply,
832831
loadServerAction,
833832
createTemporaryReferenceSet,`
834833
: ""
835834
}
836835
} from ${JSON.stringify(
837836
hasServerActions ? "@vitejs/plugin-rsc/rsc" : "@vitejs/plugin-rsc/react/rsc",
838837
)};
839-
import { createClientManifest as _createClientManifest } from "@vitejs/plugin-rsc/core/rsc";
838+
import {
839+
createClientManifest as _createClientManifest,${
840+
hasServerActions
841+
? `
842+
createServerManifest as _createServerManifest,`
843+
: ""
844+
}
845+
} from "@vitejs/plugin-rsc/core/rsc";
840846
import { prerender as _prerender } from "@vitejs/plugin-rsc/vendor/react-server-dom/static.edge";
841-
import { createRscPrerenderer, createRscRenderer } from ${JSON.stringify(rscStreamHintsPath)};
847+
${
848+
hasServerActions
849+
? `// plugin-rsc's decodeReply() fixes an unscoped server manifest; React's own
850+
// takes the manifest, so fetch actions can scope argument references per route.
851+
import { decodeReply as _decodeReply } from "@vitejs/plugin-rsc/vendor/react-server-dom/server.edge";
852+
`
853+
: ""
854+
}import { createRscPrerenderer, createRscRenderer } from ${JSON.stringify(rscStreamHintsPath)};
842855
843856
const renderToReadableStream = createRscRenderer(_renderToReadableStream);
844857
const prerenderToReadableStream = createRscPrerenderer(async (model, options) =>
@@ -1809,6 +1822,7 @@ ${responseStageOnly ? "const __responseStageOptions = {" : "const __appRscHandle
18091822
searchParams,
18101823
}) {
18111824
const {
1825+
areServerActionsOwnedByRoute: __areServerActionsOwnedByRoute,
18121826
createForwardedActionContext: __createForwardedActionContext,
18131827
forwardServerActionIfNeeded: __forwardServerActionIfNeeded,
18141828
} = await __loadAppActionForwarding();
@@ -1896,8 +1910,9 @@ ${responseStageOnly ? "const __responseStageOptions = {" : "const __appRscHandle
18961910
createRscOnErrorHandler(actionRequest, actionPathname, routePattern, overrides) {
18971911
return createRscOnErrorHandler(actionRequest, actionPathname, routePattern, overrides);
18981912
},
1913+
createServerManifest: _createServerManifest,
18991914
createTemporaryReferenceSet,
1900-
decodeReply,
1915+
decodeReply: _decodeReply,
19011916
dispatchRedirectRequest(redirectRequest) {
19021917
return __appRscHandler(redirectRequest, undefined);
19031918
},
@@ -1915,6 +1930,13 @@ ${responseStageOnly ? "const __responseStageOptions = {" : "const __appRscHandle
19151930
},
19161931
isRscRequest,
19171932
loadServerAction,
1933+
validateActionReferences(__argumentActionIds) {
1934+
return __areServerActionsOwnedByRoute(
1935+
__VINEXT_ACTION_OWNERS(),
1936+
__argumentActionIds,
1937+
__actionMatch?.route.pattern ?? null,
1938+
);
1939+
},
19181940
// Redirect targets are rendered as if the client had navigated to them,
19191941
// so they must route on the raw pathname a real request would use.
19201942
matchRoute(pathnameToMatch) {

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,8 +381,15 @@ export type HandleServerActionRscRequestOptions<
381381
overrides?: AppRenderErrorContextOverrides,
382382
) => (error: unknown) => unknown;
383383
createTemporaryReferenceSet: () => TTemporaryReferences;
384+
/** plugin-rsc's server manifest, which resolves any export of a server-reference module. */
385+
createServerManifest: () => object;
386+
/**
387+
* React's `decodeReply()`. Every server reference in the arguments resolves
388+
* through `serverManifest`, so the manifest bounds which functions can decode.
389+
*/
384390
decodeReply: (
385391
body: string | FormData,
392+
serverManifest: object,
386393
options: DecodeServerActionReplyOptions<TTemporaryReferences>,
387394
) => Promise<unknown[]> | unknown[];
388395
draftModeSecret: string;
@@ -400,6 +407,12 @@ export type HandleServerActionRscRequestOptions<
400407
isEdgeRuntime?: boolean;
401408
isRscRequest: boolean;
402409
loadServerAction: (actionId: string) => Promise<unknown>;
410+
/**
411+
* Whether these Server Actions belong to the current route. Next.js resolves
412+
* argument references through the current page's server module map, so an
413+
* action owned by another route must not decode into a callable function.
414+
*/
415+
validateActionReferences: (actionIds: readonly string[]) => boolean;
403416
/**
404417
* Match an action redirect target. Must use *request* route identity — the
405418
* raw, undecoded pathname — because the target is rendered as if the client
@@ -1703,9 +1716,23 @@ export async function handleServerActionRscRequest<
17031716
action = loadedAction;
17041717
}
17051718

1719+
// Scope the manifest React resolves argument references through to this
1720+
// route's Server Actions, as Next.js scopes its server module map to the
1721+
// current page. React then fails to resolve any other id wherever the
1722+
// payload carries it, so that module never loads. A denied lookup throws
1723+
// because React retries a missing `module#a#b` as module `module#a` with
1724+
// export `b`, which would reach a non-action export of an owned module.
1725+
const serverManifest = new Proxy(options.createServerManifest(), {
1726+
get(target, id, receiver) {
1727+
if (typeof id === "string" && !options.validateActionReferences([id])) {
1728+
throw new Error(`Server reference "${id}" is not a Server Action of this route.`);
1729+
}
1730+
return Reflect.get(target, id, receiver);
1731+
},
1732+
});
17061733
const temporaryReferences = options.createTemporaryReferenceSet();
17071734
const args = await loadOutsideRequestScopes(() =>
1708-
options.decodeReply(body, { temporaryReferences }),
1735+
options.decodeReply(body, serverManifest, { temporaryReferences }),
17091736
);
17101737
let returnValue: AppServerActionReturnValue;
17111738
let actionRedirect: AppServerActionRedirect | null = null;

‎tests/app-server-action-execution.test.ts‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,9 @@ function createRscOptions(
296296
createRscOnErrorHandler(_request, pathname, pattern) {
297297
return () => `${pathname}:${pattern}`;
298298
},
299+
createServerManifest() {
300+
return {};
301+
},
299302
createTemporaryReferenceSet() {
300303
return { marker: "refs" };
301304
},
@@ -377,6 +380,9 @@ function createRscOptions(
377380
toInterceptOpts(intercept) {
378381
return { slot: intercept.slotKey };
379382
},
383+
validateActionReferences() {
384+
return true;
385+
},
380386
...overrides,
381387
cleanPathname,
382388
currentRouteMatch:
@@ -1348,7 +1354,7 @@ describe("app server action execution helpers", () => {
13481354
createTemporaryReferenceSet() {
13491355
return temporaryReferences;
13501356
},
1351-
decodeReply(body, options) {
1357+
decodeReply(body, _serverManifest, options) {
13521358
expect(body).toBe("encoded-flight-body");
13531359
expect(options.temporaryReferences).toBe(temporaryReferences);
13541360
return Promise.resolve(["first", "second"]);
@@ -3058,6 +3064,59 @@ describe("app server action execution helpers", () => {
30583064
expect(hidden).not.toHaveBeenCalled();
30593065
});
30603066

3067+
// React resolves each argument reference with `serverManifest[id]` and throws
3068+
// when the entry is missing; this decoder stands in for that lookup.
3069+
function createArgumentReferenceOptions(referenceIds: readonly string[], ownedIds: string[]) {
3070+
const action = vi.fn(() => "saved");
3071+
const validateActionReferences = vi.fn((actionIds: readonly string[]) =>
3072+
actionIds.every((actionId) => ownedIds.includes(actionId)),
3073+
);
3074+
const options = createRscOptions({
3075+
createServerManifest: () =>
3076+
new Proxy({}, { get: (_target, id) => ({ id, name: "", chunks: [] }) }),
3077+
decodeReply(_body, serverManifest) {
3078+
return referenceIds.map((referenceId) => {
3079+
const entry: unknown = Reflect.get(serverManifest, referenceId);
3080+
if (!entry) throw new Error(`Could not find the module "${referenceId}"`);
3081+
return () => entry;
3082+
});
3083+
},
3084+
loadServerAction: () => Promise.resolve(action),
3085+
validateActionReferences,
3086+
});
3087+
return { action, options, validateActionReferences };
3088+
}
3089+
3090+
it("decodes argument references to Server Actions of the current route", async () => {
3091+
const { action, options, validateActionReferences } = createArgumentReferenceOptions(
3092+
["a#owned"],
3093+
["a#owned"],
3094+
);
3095+
3096+
const response = await handleServerActionRscRequest(options);
3097+
3098+
expect(response?.status).toBe(200);
3099+
expect(validateActionReferences).toHaveBeenCalledWith(["a#owned"]);
3100+
expect(action).toHaveBeenCalledWith(expect.any(Function));
3101+
});
3102+
3103+
it("does not resolve argument references to Server Actions of another route", async () => {
3104+
const { action, options } = createArgumentReferenceOptions(
3105+
["a#owned", "b#foreign"],
3106+
["a#owned"],
3107+
);
3108+
const error = vi.spyOn(console, "error").mockImplementation(() => {});
3109+
3110+
try {
3111+
const response = await handleServerActionRscRequest(options);
3112+
3113+
expect(response?.status).toBe(500);
3114+
expect(action).not.toHaveBeenCalled();
3115+
} finally {
3116+
error.mockRestore();
3117+
}
3118+
});
3119+
30613120
// Reproduces the prod-build error path where the @vitejs/plugin-rsc server
30623121
// references manifest doesn't include the requested action id. In a build
30633122
// with NO server actions defined at all, the action loader throws

‎tests/e2e/app-router/action-owner-production.browser.spec.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ type ProductionApp = {
2020
boundaryOnly: string;
2121
cachedProtected: string;
2222
cookieOwner: string;
23+
describeArgument: string;
2324
dynamicProtected: string;
2425
globalErrorOnly: string;
2526
globalNotFoundOnly: string;
27+
inlineArgument: string;
2628
loop: string;
2729
objectWrapped: string;
2830
packageClient: string;
@@ -123,9 +125,11 @@ async function buildAndServeFixture(): Promise<ProductionApp> {
123125
boundaryOnly: findActionId(builtSource, "boundaryOnlyAction"),
124126
cachedProtected: findOnlyCacheActionId(builtSource),
125127
cookieOwner: findActionId(builtSource, "$$hoist_0_readForwardedCredentials"),
128+
describeArgument: findActionId(builtSource, "describeArgument"),
126129
dynamicProtected: findActionId(builtSource, "$$hoist_0_dynamicProtectedAction"),
127130
globalErrorOnly: findActionId(builtSource, "globalErrorOnlyAction"),
128131
globalNotFoundOnly: findActionId(builtSource, "globalNotFoundOnlyAction"),
132+
inlineArgument: findActionId(builtSource, "$$hoist_0_inlineArgumentAction"),
129133
loop: findActionId(builtSource, "$$hoist_0_loopAction"),
130134
objectWrapped: findActionId(builtSource, "objectWrappedAction"),
131135
packageClient: findActionId(builtSource, "packageClientAction"),
@@ -200,6 +204,32 @@ async function postRawAction(
200204
);
201205
}
202206

207+
async function postMultipartAction(
208+
page: Page,
209+
pathname: string,
210+
actionId: string,
211+
parts: ReadonlyArray<readonly [string, string]>,
212+
) {
213+
await page.goto(`${app.baseUrl}${pathname}`);
214+
return page.evaluate(
215+
async ({ actionId, parts, pathname }) => {
216+
const body = new FormData();
217+
for (const [key, value] of parts) body.append(key, value);
218+
const response = await fetch(pathname, {
219+
body,
220+
headers: { "next-action": actionId },
221+
method: "POST",
222+
});
223+
return {
224+
body: await response.text(),
225+
headers: Object.fromEntries(response.headers),
226+
status: response.status,
227+
};
228+
},
229+
{ actionId, parts, pathname },
230+
);
231+
}
232+
203233
async function postProgressiveAction(
204234
page: Page,
205235
pathname: string,
@@ -524,6 +554,62 @@ test.describe("production server action ownership", () => {
524554
expect(response.body).not.toContain("LOOP_ACTION_EXECUTED");
525555
});
526556

557+
test("decodes only route-owned Server Actions as argument references", async ({ page }) => {
558+
const postArgumentReference = (parts: ReadonlyArray<readonly [string, string]>) =>
559+
postMultipartAction(page, "/ownership/argument-refs", app.actionIds.describeArgument, parts);
560+
const metadata = (id: string, bound: string | null = null) => JSON.stringify({ id, bound });
561+
const reference = (id: string) =>
562+
[
563+
["0", JSON.stringify(["$h1"])],
564+
["1", metadata(id)],
565+
] as const;
566+
// React reads stream and async-iterable arguments from `S<id>` parts.
567+
const streamed = (type: "$R1" | "$x1", id: string) =>
568+
[
569+
["0", JSON.stringify([type])],
570+
["S1", JSON.stringify("$h2")],
571+
["2", metadata(id)],
572+
] as const;
573+
// The page module also exports its component; it is a server-reference
574+
// module because of its inline action, but `default` is not an action.
575+
const pageComponentId = app.actionIds.inlineArgument.replace(/#.*$/, "#default");
576+
577+
for (const parts of [
578+
reference(app.actionIds.describeArgument),
579+
reference(app.actionIds.inlineArgument),
580+
streamed("$R1", app.actionIds.inlineArgument),
581+
streamed("$x1", app.actionIds.inlineArgument),
582+
]) {
583+
const response = await postArgumentReference(parts);
584+
expect(response.status).toBe(200);
585+
expect(response.body).toContain("FUNCTION_ARGUMENT");
586+
}
587+
588+
// Like Next.js's page-scoped server module map, an id outside this route
589+
// fails to resolve: the reply or the argument carrying it errors.
590+
for (const parts of [
591+
reference(app.actionIds.protected),
592+
reference(pageComponentId),
593+
// React retries a missing id without its last `#name`, keeping `name`
594+
// as the export, which would reach this owned module's `default`.
595+
reference(`${app.actionIds.inlineArgument}#default`),
596+
[
597+
["0", JSON.stringify(["$h1"])],
598+
["1", metadata(app.actionIds.describeArgument, "$@2")],
599+
["2", JSON.stringify(["$h3"])],
600+
["3", metadata(app.actionIds.protected)],
601+
],
602+
// A repeated part must not swap in a reference after an owned one.
603+
[...reference(app.actionIds.describeArgument), ["1", metadata(app.actionIds.protected)]],
604+
streamed("$R1", app.actionIds.protected),
605+
streamed("$x1", app.actionIds.protected),
606+
] as const) {
607+
const response = await postArgumentReference(parts);
608+
expect(response.status).toBe(500);
609+
expect(response.body).not.toContain("_ARGUMENT");
610+
}
611+
});
612+
527613
test("fails closed for unknown action ids", async ({ page }) => {
528614
await page.goto(`${app.baseUrl}/ownership/direct`);
529615
const status = await page.evaluate(async () => {
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
"use server";
2+
3+
function describe(value: unknown) {
4+
return typeof value === "function" ? "FUNCTION_ARGUMENT" : `VALUE_ARGUMENT:${typeof value}`;
5+
}
6+
7+
export async function describeArgument(value?: unknown) {
8+
// Stream and iterable parts decode as they are read, so describe the first item.
9+
if (value instanceof ReadableStream) {
10+
const { value: item } = await value.getReader().read();
11+
return describe(item);
12+
}
13+
if (typeof value === "object" && value !== null && Symbol.asyncIterator in value) {
14+
for await (const item of value as AsyncIterable<unknown>) return describe(item);
15+
}
16+
return describe(value);
17+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { ActionButton } from "../action-button";
2+
import { describeArgument } from "./actions";
3+
4+
export default function Page() {
5+
async function inlineArgumentAction() {
6+
"use server";
7+
return "INLINE_ARGUMENT_OK";
8+
}
9+
return (
10+
<main>
11+
<ActionButton id="argument-refs" action={describeArgument} />
12+
<ActionButton id="argument-refs-inline" action={inlineArgumentAction} />
13+
</main>
14+
);
15+
}

0 commit comments

Comments
 (0)