archive/rsnext
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Change allowed forwarded hosts to be allowed origins for Server Actions (#58023)

Browse source 
The allowlist should be origin domains that are allowed to send the
requests, not the list of forwarded hosts (i.e. reverse proxies).
This commit is contained in:
Shu Ding 2023-11-08 19:20:32 +09:00 committed by GitHub
parent f7b979c9cf
commit 24a617c24f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 142 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts
View file

@ -28,7 +28,7 @@ export type EdgeSSRLoaderQuery = {
middlewareConfig: string
serverActions?: {
bodySizeLimit?: SizeLimit
allowedForwardedHosts?: string[]
allowedOrigins?: string[]
}
}

2
packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts
View file

@ -57,7 +57,7 @@ export function getRender({
serverActionsManifest?: any
serverActions?: {
bodySizeLimit?: SizeLimit
allowedForwardedHosts?: string[]
allowedOrigins?: string[]
}
config: NextConfigComplete
buildId: string

2
packages/next/src/lib/turbopack-warning.ts
View file

@ -79,7 +79,7 @@ const supportedTurbopackNextConfigOptions = [
'experimental.scrollRestoration',
'experimental.forceSwcTransforms',
'experimental.serverActions.bodySizeLimit',
'experimental.serverActions.allowedForwardedHosts',
'experimental.serverActions.allowedOrigins',
'experimental.memoryBasedWorkersCount',
'experimental.clientRouterFilterRedirects',
'experimental.webpackBuildWorker',

27
packages/next/src/server/app-render/action-handler.ts
View file

@ -257,7 +257,7 @@ export async function handleAction({
requestStore: RequestStore
serverActions?: {
bodySizeLimit?: SizeLimit
allowedForwardedHosts?: string[]
allowedOrigins?: string[]
}
ctx: AppRenderContext
}): Promise<
@ -288,7 +288,7 @@ export async function handleAction({
return
}
const originHostname =
const originDomain =
typeof req.headers['origin'] === 'string'
? new URL(req.headers['origin']).host
: undefined
@ -311,29 +311,28 @@ export async function handleAction({
// This is to prevent CSRF attacks. If `x-forwarded-host` is set, we need to
// ensure that the request is coming from the same host.
if (!originHostname) {
if (!originDomain) {
// This might be an old browser that doesn't send `host` header. We ignore
// this case.
console.warn(
'Missing `origin` header from a forwarded Server Actions request.'
)
} else if (!host || originHostname !== host.value) {
// If the customer sets a list of allowed hosts, we'll allow the request.
// These can be their reverse proxies or other safe hosts.
if (
host &&
typeof host.value === 'string' &&
serverActions?.allowedForwardedHosts?.includes(host.value)
) {
} else if (!host || originDomain !== host.value) {
// If the customer sets a list of allowed origins, we'll allow the request.
// These are considered safe but might be different from forwarded host set
// by the infra (i.e. reverse proxies).
if (serverActions?.allowedOrigins?.includes(originDomain)) {
// Ignore it
} else {
if (host) {
// This is an attack. We should not proceed the action.
// This seems to be an CSRF attack. We should not proceed the action.
console.error(
`\`${!host.type}\` header with value \`${limitUntrustedHeaderValueForLogs(
`\`${
host.type
}\` header with value \`${limitUntrustedHeaderValueForLogs(
host.value
)}\` does not match \`origin\` header with value \`${limitUntrustedHeaderValueForLogs(
originHostname
originDomain
)}\` from a forwarded Server Actions request. Aborting the action.`
)
} else {

3
packages/next/src/server/app-render/types.ts
View file

@ -134,9 +134,8 @@ export interface RenderOptsPartial {
) => Promise<NextConfigComplete>
serverActions?: {
bodySizeLimit?: SizeLimit
allowedForwardedHosts?: string[]
allowedOrigins?: string[]
}
allowedForwardedHosts?: string[]
params?: ParsedUrlQuery
isPrefetch?: boolean
experimental: { ppr: boolean }

2
packages/next/src/server/base-server.ts
View file

@ -250,7 +250,7 @@ type BaseRenderOpts = {
clientReferenceManifest?: ClientReferenceManifest
serverActions?: {
bodySizeLimit?: SizeLimit
allowedForwardedHosts?: string[]
allowedOrigins?: string[]
}
serverActionsManifest?: any
nextFontManifest?: NextFontManifest

3
packages/next/src/server/config-schema.ts
View file

@ -254,10 +254,9 @@ export const configSchema: zod.ZodType<NextConfig> = z.lazy(() =>
serverActions: z
.object({
bodySizeLimit: zSizeLimit.optional(),
allowedForwardedHosts: z.array(z.string()).optional(),
allowedOrigins: z.array(z.string()).optional(),
})
.optional(),
allowedForwardedHosts: z.array(z.string()).optional(),
// The original type was Record<string, any>
extensionAlias: z.record(z.string(), z.any()).optional(),
externalDir: z.boolean().optional(),

7
packages/next/src/server/config-shared.ts
View file

@ -309,11 +309,12 @@ export interface ExperimentalConfig {
bodySizeLimit?: SizeLimit
/**
* Allowed domains that can bypass CSRF check.
* Allowed origins that can bypass Server Action's CSRF check. This is helpful
* when you have reverse proxy in front of your app.
* @example
* ["my-reverse-proxy.com"]
* ["my-app.com"]
*/
allowedForwardedHosts?: string[]
allowedOrigins?: string[]
}
/**

3
packages/next/src/server/render.tsx
View file

@ -275,9 +275,8 @@ export type RenderOptsPartial = {
serverComponents?: boolean
serverActions?: {
bodySizeLimit?: SizeLimit
allowedForwardedHosts?: string[]
allowedOrigins?: string[]
}
allowedForwardedHosts?: string[]
customServer?: boolean
crossOrigin?: 'anonymous' | 'use-credentials' | '' | undefined
images: ImageConfigComplete

29
test/e2e/app-dir/actions-allowed-origins/app-action-allowed-origins.test.ts Normal file
View file

@ -0,0 +1,29 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
import { join } from 'path'
createNextDescribe(
'app-dir action allowed origins',
{
files: join(__dirname, 'safe-origins'),
skipDeployment: true,
dependencies: {
react: 'latest',
'react-dom': 'latest',
'server-only': 'latest',
},
// An arbitrary & random port.
forcedPort: '41831',
},
({ next }) => {
it('should pass if localhost is set as a safe origin', async function () {
const browser = await next.browser('/')
await browser.elementByCss('button').click()
await check(async () => {
return await browser.elementByCss('#res').text()
}, 'hi')
})
}
)

22
test/e2e/app-dir/actions/app-action-allowed-hosts.test.ts → test/e2e/app-dir/actions-allowed-origins/app-action-disallowed-origins.test.ts
View file

@ -1,10 +1,11 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
import { join } from 'path'
createNextDescribe(
'app-dir action allowed fowarding hosts',
'app-dir action disallowed origins',
{
files: __dirname,
files: join(__dirname, 'unsafe-origins'),
skipDeployment: true,
dependencies: {
react: 'latest',
@ -13,10 +14,10 @@ createNextDescribe(
},
},
({ next }) => {
it('should error if setting an invalid x-forwarded-host header', async function () {
const browser = await next.browser('/safe-hosts')
// Origin should be localhost
it('should error if x-forwarded-host does not match the origin', async function () {
const browser = await next.browser('/')
await browser.eval(`window.__override_forwarded_host = 'bad.com'`)
await browser.elementByCss('button').click()
await check(async () => {
@ -28,16 +29,5 @@ createNextDescribe(
: 'no'
}, 'yes')
})
it('should pass if using an allowed host', async function () {
const browser = await next.browser('/safe-hosts')
await browser.eval(`window.__override_forwarded_host = 'safe.com'`)
await browser.elementByCss('button').click()
await check(() => {
return browser.elementByCss('#res').text()
}, 'hi')
})
}
)

0
test/e2e/app-dir/actions/app/safe-hosts/action.js → test/e2e/app-dir/actions-allowed-origins/safe-origins/app/action.js
View file

8
test/e2e/app-dir/actions-allowed-origins/safe-origins/app/layout.js Normal file
View file

@ -0,0 +1,8 @@
export default function RootLayout({ children }) {
return (
<html>
<head />
<body>{children}</body>
</html>
)
}

4
test/e2e/app-dir/actions/app/safe-hosts/page.js → test/e2e/app-dir/actions-allowed-origins/safe-origins/app/page.js
View file

@ -7,12 +7,12 @@ if (typeof window !== 'undefined') {
// hijack fetch
const originalFetch = window.fetch
window.fetch = function (url, init) {
if (window.__override_forwarded_host && init?.method === 'POST') {
if (init?.method === 'POST') {
console.log('fetch', url, init)
// override forwarded host
init.headers = init.headers || {}
init.headers['x-forwarded-host'] = window.__override_forwarded_host
init.headers['x-forwarded-host'] = 'my-proxy.com'
}
return originalFetch(url, init)

12
test/e2e/app-dir/actions-allowed-origins/safe-origins/next.config.js Normal file
View file

@ -0,0 +1,12 @@
/** @type {import('next').NextConfig} */
module.exports = {
productionBrowserSourceMaps: true,
logging: {
fetches: {},
},
experimental: {
serverActions: {
allowedOrigins: ['localhost:41831'],
},
},
}

5
test/e2e/app-dir/actions-allowed-origins/unsafe-origins/app/action.js Normal file
View file

@ -0,0 +1,5 @@
'use server'
export async function log() {
return 'hi'
}

8
test/e2e/app-dir/actions-allowed-origins/unsafe-origins/app/layout.js Normal file
View file

@ -0,0 +1,8 @@
export default function RootLayout({ children }) {
return (
<html>
<head />
<body>{children}</body>
</html>
)
}

41
test/e2e/app-dir/actions-allowed-origins/unsafe-origins/app/page.js Normal file
View file

@ -0,0 +1,41 @@
'use client'
import { useState } from 'react'
import { log } from './action'
if (typeof window !== 'undefined') {
// hijack fetch
const originalFetch = window.fetch
window.fetch = function (url, init) {
if (init?.method === 'POST') {
console.log('fetch', url, init)
// override forwarded host
init.headers = init.headers || {}
init.headers['x-forwarded-host'] = 'my-proxy.com'
}
return originalFetch(url, init)
}
}
export default function Page() {
const [res, setRes] = useState(null)
return (
<div>
<div id="res">{res}</div>
<button
onClick={async () => {
try {
setRes(await log())
} catch (err) {
setRes(err.message)
}
}}
>
fetch
</button>
</div>
)
}

7
test/e2e/app-dir/actions-allowed-origins/unsafe-origins/next.config.js Normal file
View file

@ -0,0 +1,7 @@
/** @type {import('next').NextConfig} */
module.exports = {
productionBrowserSourceMaps: true,
logging: {
fetches: {},
},
}

5
test/e2e/app-dir/actions/next.config.js
View file

@ -4,9 +4,4 @@ module.exports = {
logging: {
fetches: {},
},
experimental: {
serverActions: {
allowedForwardedHosts: ['safe.com'],
},
},
}