Skip to content

Commit 8ebd5df

Browse files
brookslybrandbrookslybrand
authored
Harden RSC CSRF codepaths (backport of #15311) (#15353)
1 parent afdf85d commit 8ebd5df

3 files changed

Lines changed: 183 additions & 7 deletions

File tree

‎integration/rsc-csrf-action-test.ts‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
import { test, expect, type Page } from "@playwright/test";
2+
import getPort from "get-port";
3+
4+
import { PlaywrightFixture } from "./helpers/playwright-fixture.js";
5+
import {
6+
createAppFixture,
7+
createFixture,
8+
js,
9+
} from "./helpers/create-fixture.js";
10+
import type { AppFixture, Fixture } from "./helpers/create-fixture.js";
11+
import { implementations, setupRscTest, validateRSCHtml } from "./rsc/utils.js";
12+
13+
const csrfActionRoute = js`
14+
let actionCalls = 0;
15+
16+
export function loader() {
17+
return { actionCalls };
18+
}
19+
20+
export async function action() {
21+
actionCalls++;
22+
return null;
23+
}
24+
25+
export default function Component({ loaderData }) {
26+
return (
27+
<p data-action-calls={loaderData.actionCalls}>
28+
Action calls: {loaderData.actionCalls}
29+
</p>
30+
);
31+
}
32+
`;
33+
34+
async function expectActionCalls(page: Page, count: string) {
35+
await page.waitForSelector("[data-action-calls]");
36+
expect(await page.locator("[data-action-calls]").textContent()).toContain(
37+
`Action calls: ${count}`,
38+
);
39+
expect(
40+
await page.locator("[data-action-calls]").getAttribute("data-action-calls"),
41+
).toBe(count);
42+
}
43+
44+
test.describe("RSC CSRF action protection", () => {
45+
test.describe("RSC Framework", () => {
46+
let fixture: Fixture;
47+
let appFixture: AppFixture | undefined;
48+
49+
test.beforeAll(async () => {
50+
fixture = await createFixture({
51+
templateName: "rsc-vite-framework",
52+
files: {
53+
"app/routes/csrf-action.tsx": csrfActionRoute,
54+
},
55+
});
56+
57+
appFixture = await createAppFixture(fixture);
58+
});
59+
60+
test.afterAll(() => {
61+
appFixture?.close();
62+
});
63+
64+
test("does not call actions on cross-origin document POST requests", async ({
65+
page,
66+
request,
67+
}) => {
68+
let app = new PlaywrightFixture(appFixture!, page);
69+
70+
await app.goto("/csrf-action");
71+
await expectActionCalls(page, "0");
72+
validateRSCHtml(await page.content());
73+
74+
let response = await request.post(
75+
`${appFixture!.serverUrl}/csrf-action`,
76+
{
77+
form: { intent: "mutate" },
78+
headers: {
79+
Origin: "https://attacker.example",
80+
},
81+
},
82+
);
83+
expect(response.status()).toBe(400);
84+
85+
await app.goto("/csrf-action");
86+
await expectActionCalls(page, "0");
87+
});
88+
});
89+
90+
implementations.forEach((implementation) => {
91+
test.describe(`RSC Data (${implementation.name})`, () => {
92+
let port: number;
93+
let stopAfterAll: () => void;
94+
95+
test.beforeAll(async () => {
96+
port = await getPort();
97+
stopAfterAll = await setupRscTest({
98+
implementation,
99+
port,
100+
files: {
101+
"src/routes.ts": js`
102+
import type { unstable_RSCRouteConfig as RSCRouteConfig } from "react-router";
103+
104+
export const routes = [
105+
{
106+
id: "root",
107+
path: "",
108+
lazy: () => import("./routes/root"),
109+
children: [
110+
{
111+
id: "csrf-action",
112+
path: "csrf-action",
113+
lazy: () => import("./routes/csrf-action"),
114+
},
115+
],
116+
},
117+
] satisfies RSCRouteConfig;
118+
`,
119+
120+
"src/routes/root.tsx": js`
121+
import { Outlet } from "react-router";
122+
123+
export function Layout({ children }: { children: React.ReactNode }) {
124+
return (
125+
<html>
126+
<body>{children}</body>
127+
</html>
128+
);
129+
}
130+
131+
export default function RootRoute() {
132+
return <Outlet />;
133+
}
134+
`,
135+
136+
"src/routes/csrf-action.tsx": csrfActionRoute,
137+
},
138+
});
139+
});
140+
141+
test.afterAll(() => {
142+
stopAfterAll?.();
143+
});
144+
145+
test("does not call actions on cross-origin document POST requests", async ({
146+
page,
147+
request,
148+
}) => {
149+
await page.goto(`http://localhost:${port}/csrf-action`);
150+
await expectActionCalls(page, "0");
151+
validateRSCHtml(await page.content());
152+
153+
let response = await request.post(
154+
`http://localhost:${port}/csrf-action`,
155+
{
156+
form: { intent: "mutate" },
157+
headers: {
158+
Origin: "https://attacker.example",
159+
},
160+
},
161+
);
162+
expect(response.status()).toBe(400);
163+
164+
await page.goto(`http://localhost:${port}/csrf-action`);
165+
await expectActionCalls(page, "0");
166+
});
167+
});
168+
});
169+
});

‎packages/react-router/.changes/patch.rsc-csrf.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Harden RSC CSRF codepaths.

‎packages/react-router/lib/rsc/server.rsc.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,19 @@ async function generateRenderResponse(
861861
if (isMutationMethod(request.method)) {
862862
try {
863863
throwIfPotentialCSRFAttack(request, allowedActionOrigins);
864+
} catch (error) {
865+
onError?.(error);
866+
potentialCSRFAttackError = error;
867+
// Downgrade the request to a GET so that `query` below cannot
868+
// execute any route `action` for this potential CSRF attack
869+
request = new Request(request.url, {
870+
method: "GET",
871+
headers: request.headers,
872+
signal: request.signal,
873+
});
874+
}
864875

876+
if (!potentialCSRFAttackError) {
865877
ctx.runningAction = true;
866878
let result = await processServerAction(
867879
request,
@@ -904,18 +916,12 @@ async function generateRenderResponse(
904916
undefined,
905917
);
906918
}
907-
} catch (error) {
908-
potentialCSRFAttackError = error;
909919
}
910920
}
911921

912922
let staticContext = await query(
913923
request,
914-
skipRevalidation || !!potentialCSRFAttackError
915-
? {
916-
filterMatchesToLoad: () => false,
917-
}
918-
: undefined,
924+
skipRevalidation ? { filterMatchesToLoad: () => false } : undefined,
919925
);
920926

921927
if (isResponse(staticContext)) {

0 commit comments

Comments
 (0)