diff --git a/packages/plugins/apps/README.md b/packages/plugins/apps/README.md index f7e98d130..34b4f7807 100644 --- a/packages/plugins/apps/README.md +++ b/packages/plugins/apps/README.md @@ -88,6 +88,8 @@ The dev server's backend function endpoints (`/__dd/executeAction`, `/__dd/execu - its `Origin`, when sent, names the same host and port as its `Host`; - its body is sent with `Content-Type: application/json`. +The dev server warns once for each distinct refusal and logs repeats at `debug`. Past a per-server cap, it warns once more to say so, then logs new refusals at `debug` too. + To call one by hand, send that content type, for example `curl -X POST -H 'Content-Type: application/json' -d '{"functionName":"..."}' http://localhost:5173/__dd/debugBundle`. ## Cloud execution diff --git a/packages/plugins/apps/src/vite/dev-server-request-guard.test.ts b/packages/plugins/apps/src/vite/dev-server-request-guard.test.ts index 9146985f3..6efed38c9 100644 --- a/packages/plugins/apps/src/vite/dev-server-request-guard.test.ts +++ b/packages/plugins/apps/src/vite/dev-server-request-guard.test.ts @@ -4,6 +4,7 @@ import type { HostCheckConfig } from '@dd/apps-plugin/vite/dev-server-request-guard'; import { + ALLOWED_HOSTS_HINT, checkDevServerRequest, createViteHostCheck, MISSING_HOST_MESSAGE, @@ -373,6 +374,25 @@ describe('dev-server-request-guard — checkDevServerRequest', () => { expect(rejection?.message).toContain('"[fe80::zz]"'); }); + test("Should escape a Host's non-ASCII characters in the rejection message", () => { + const rejection = checkDevServerRequest( + { host: 'evil\u009b2J\u0085.example:5173', 'content-type': json }, + isHostAllowed, + ); + + expect(rejection?.message).toContain('"evil\\u009b2J\\u0085.example"'); + expect(rejection?.message).toMatch(/^[\x20-\x7e]*$/); + }); + + test('Should end a refused-host message with the allowedHosts hint', () => { + const rejection = checkDevServerRequest( + { host: 'tunnel.example:5173', 'content-type': json }, + isHostAllowed, + ); + + expect(rejection?.message.split(ALLOWED_HOSTS_HINT)[1]).toBe(''); + }); + test('Should treat an empty Host header as missing, even under a "." allowedHosts entry', () => { const dotConfig = withServer({ allowedHosts: ['.'] }); const allowDot = createViteHostCheck(dotConfig); diff --git a/packages/plugins/apps/src/vite/dev-server-request-guard.ts b/packages/plugins/apps/src/vite/dev-server-request-guard.ts index 2d8975fe7..57370d412 100644 --- a/packages/plugins/apps/src/vite/dev-server-request-guard.ts +++ b/packages/plugins/apps/src/vite/dev-server-request-guard.ts @@ -15,6 +15,9 @@ export interface HostCheckConfig { export const MISSING_HOST_MESSAGE = 'Blocked request with no Host header.'; +export const ALLOWED_HOSTS_HINT = + 'If you trust this host, add it to `server.allowedHosts` in vite.config.js.'; + export interface RequestRejection { statusCode: 403 | 415; message: string; @@ -116,6 +119,14 @@ function isJsonContentType(contentType: string | undefined): boolean { return contentType?.split(';')[0].trim().toLowerCase() === 'application/json'; } +// Node decodes a Host as latin1, and JSON.stringify leaves C1 controls raw for the terminal. +function quoteHostname(hostname: string): string { + return JSON.stringify(hostname).replace( + /[^\x20-\x7e]/g, + (char) => `\\u${char.charCodeAt(0).toString(16).padStart(4, '0')}`, + ); +} + export function checkDevServerRequest( headers: IncomingHttpHeaders, isHostAllowed: HostCheck, @@ -128,11 +139,10 @@ export function checkDevServerRequest( return { statusCode: 403, message: MISSING_HOST_MESSAGE }; } if (!isHostAllowed(host)) { - const hostname = getHostname(host); - const quotedHostname = JSON.stringify(hostname); + const quotedHostname = quoteHostname(getHostname(host)); return { statusCode: 403, - message: `Blocked request. This host (${quotedHostname}) is not allowed. To allow it, add it to \`server.allowedHosts\` in vite.config.js.`, + message: `Blocked request. This host (${quotedHostname}) is not allowed. ${ALLOWED_HOSTS_HINT}`, }; } if (origin !== undefined && !isSameOriginAsHost(origin, host)) { diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index 98cdab93f..9bbf2a44a 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -5,14 +5,20 @@ import { getAuthenticatedRequest } from '@dd/apps-plugin/auth'; import { createBackendStaticChecks } from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; import { buildBackendFunctions } from '@dd/apps-plugin/vite/build-backend-functions'; -import { createViteHostCheck } from '@dd/apps-plugin/vite/dev-server-request-guard'; +import { + ALLOWED_HOSTS_HINT, + createViteHostCheck, +} from '@dd/apps-plugin/vite/dev-server-request-guard'; import { createDevServerMiddleware, DEBUG_BUNDLE_ROUTE, DEV_SERVER_ROUTES, + EXECUTE_ACTION_ROUTE, formatRef, getRetryDelay, + MAX_WARNED_REJECTIONS, oversizedBundleMessage, + REJECTION_WARNINGS_CAPPED_MESSAGE, } from '@dd/apps-plugin/vite/dev-server'; import type { LocalExecutionRequest, @@ -30,7 +36,7 @@ import { mockLogFn, } from '@dd/tests/_jest/helpers/mocks'; import { emitModuleParsed } from '@dd/tests/_jest/helpers/moduleParsed'; -import type { IncomingMessage } from 'http'; +import type { IncomingHttpHeaders, IncomingMessage } from 'http'; import nock from 'nock'; import { fork } from 'node:child_process'; import type { InlineConfig } from 'vite'; @@ -543,7 +549,7 @@ describe('Dev Server Middleware', () => { expect(mockViteBuild).not.toHaveBeenCalled(); expect(mockExecuteLocally).not.toHaveBeenCalled(); const routeMessage = expect.stringContaining(route); - expect(mockLogFn).toHaveBeenCalledWith(routeMessage, 'debug'); + expect(mockLogFn).toHaveBeenCalledWith(routeMessage, 'warn'); }, ); @@ -590,6 +596,143 @@ describe('Dev Server Middleware', () => { }); }); + describe('createDevServerMiddleware refusal logging', () => { + async function sendRequest( + middleware: ReturnType, + headers: IncomingHttpHeaders, + ): Promise { + const functionName = encodeQueryName(mockFunctions[0]); + const req = createMockRequest( + EXECUTE_ACTION_ROUTE, + { functionName, args: [] }, + headers, + ); + const res = createMockResponse(); + const next = jest.fn(); + + middleware(req, res, next); + await res.done; + } + + function refusalLogLevels(): string[] { + return mockLogFn.mock.calls + .filter(([text]) => String(text).includes(EXECUTE_ACTION_ROUTE)) + .map(([, level]) => level); + } + + const tunnelHost = { host: 'tunnel.example:5173' }; + const lanHost = { host: 'lan-box:5173' }; + const crossOrigin = { origin: 'https://attacker.example' }; + const cases = [ + { + description: 'warn the first time a host is refused and log repeats at debug', + requests: [tunnelHost, tunnelHost, tunnelHost], + expectedLevels: ['warn', 'debug', 'debug'], + }, + { + description: 'warn once for each refused hostname, whatever its port', + requests: [ + tunnelHost, + lanHost, + tunnelHost, + lanHost, + { host: 'tunnel.example:5174' }, + ], + expectedLevels: ['warn', 'warn', 'debug', 'debug', 'debug'], + }, + { + description: 'warn once for each distinct HTTP/2 :authority', + requests: [ + { host: undefined, ':authority': 'tunnel.example:5173' }, + { host: undefined, ':authority': 'lan-box:5173' }, + { host: undefined, ':authority': 'tunnel.example:5173' }, + ], + expectedLevels: ['warn', 'warn', 'debug'], + }, + { + description: 'warn once for a missing Host, whether absent or empty', + requests: [{ host: undefined }, { host: '' }], + expectedLevels: ['warn', 'debug'], + }, + { + description: 'warn once for a refusal whose message is the same from another host', + requests: [crossOrigin, { ...crossOrigin, host: '127.0.0.1:5173' }], + expectedLevels: ['warn', 'debug'], + }, + { + description: 'warn again when the same host is refused for a different reason', + requests: [crossOrigin, { 'content-type': 'text/plain' }, crossOrigin], + expectedLevels: ['warn', 'warn', 'debug'], + }, + ]; + + test.each(cases)('Should $description', async ({ requests, expectedLevels }) => { + const middleware = createTestMiddleware(); + + for (const headers of requests) { + // eslint-disable-next-line no-await-in-loop + await sendRequest(middleware, headers); + } + + const levels = refusalLogLevels(); + expect(levels).toEqual(expectedLevels); + }); + + test('Should say once that warnings stop after the maximum number of warned rejections', async () => { + const middleware = createTestMiddleware(); + const hostCount = MAX_WARNED_REJECTIONS + 2; + + for (let index = 0; index < hostCount; index++) { + // eslint-disable-next-line no-await-in-loop + await sendRequest(middleware, { host: `tunnel-${index}.example:5173` }); + } + await sendRequest(middleware, { host: 'tunnel-0.example:5173' }); + + const expectedLevels = [ + ...Array(MAX_WARNED_REJECTIONS + 1).fill('warn'), + 'debug', + 'debug', + ]; + const levels = refusalLogLevels(); + expect(levels).toEqual(expectedLevels); + const capNotices = mockLogFn.mock.calls.filter(([text]) => + String(text).endsWith(REJECTION_WARNINGS_CAPPED_MESSAGE), + ); + const firstUnwarnedHost = expect.stringContaining( + `tunnel-${MAX_WARNED_REJECTIONS}.example`, + ); + expect(capNotices).toEqual([[firstUnwarnedHost, 'warn']]); + }); + + test('Should track warned hosts per middleware instance', async () => { + const firstMiddleware = createTestMiddleware(); + const secondMiddleware = createTestMiddleware(); + + await sendRequest(firstMiddleware, tunnelHost); + await sendRequest(secondMiddleware, tunnelHost); + + const levels = refusalLogLevels(); + expect(levels).toEqual(['warn', 'warn']); + }); + + test('Should warn with the actionable message and without request credentials', async () => { + const credential = 'do-not-log-credential'; + const middleware = createTestMiddleware(); + + await sendRequest(middleware, { + ...tunnelHost, + cookie: `session=${credential}`, + authorization: `Bearer ${credential}`, + }); + + const loggedText = mockLogFn.mock.calls.map(([text]) => String(text)).join('\n'); + const [hostWarning] = mockLogFn.mock.calls.find(([, level]) => level === 'warn') ?? []; + expect(String(hostWarning)).toContain('"tunnel.example"'); + expect(String(hostWarning).split(ALLOWED_HOSTS_HINT)[1]).toBe(''); + expect(loggedText).not.toContain(credential); + }); + }); + describe('debugBundle handler', () => { const middleware = createTestMiddleware(); diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index a9cadb087..6c7251618 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -49,6 +49,11 @@ export const DEBUG_BUNDLE_ROUTE = DEBUG_BUNDLE_PATH; export const EXECUTE_ACTION_ROUTE = '/__dd/executeAction'; export const EXECUTE_ACTION_VIA_CLOUD_ROUTE = '/__dd/executeActionViaCloud'; +// Caps the warned set so requests cycling through Host values can't grow it without bound. +export const MAX_WARNED_REJECTIONS = 32; +export const REJECTION_WARNINGS_CAPPED_MESSAGE = + "Further rejections are only logged with the Datadog plugin's `logLevel: 'debug'`."; + export const DEV_SERVER_ROUTES = [ DEBUG_BUNDLE_ROUTE, EXECUTE_ACTION_ROUTE, @@ -892,6 +897,26 @@ export function createDevServerMiddleware( [EXECUTE_ACTION_VIA_CLOUD_ROUTE]: runViaCloud, }; + const warnedRejections = new Set(); + let hasLoggedWarningCap = false; + const logRejection = (rejectionMessage: string): void => { + if (warnedRejections.has(rejectionMessage)) { + log.debug(rejectionMessage); + return; + } + if (warnedRejections.size < MAX_WARNED_REJECTIONS) { + warnedRejections.add(rejectionMessage); + log.warn(rejectionMessage); + return; + } + if (!hasLoggedWarningCap) { + hasLoggedWarningCap = true; + log.warn(`${rejectionMessage} ${REJECTION_WARNINGS_CAPPED_MESSAGE}`); + return; + } + log.debug(rejectionMessage); + }; + return (req: IncomingMessage, res: ServerResponse, next: () => void) => { const { url } = req; if (req.method !== 'POST' || !isDevServerRoute(url)) { @@ -901,7 +926,7 @@ export function createDevServerMiddleware( const rejection = checkDevServerRequest(req.headers, isHostAllowed); if (rejection) { - log.debug(`Rejected POST ${url}: ${rejection.message}`); + logRejection(`Rejected POST ${url}: ${rejection.message}`); sendError(res, rejection.statusCode, rejection.message); return; }