From 9e6bddb2b9dd58916a14ebbdfed1e87540bd7f0b Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Sat, 13 Sep 2025 09:53:53 +0200 Subject: [PATCH 1/5] Add a status endpoint which can be used by the admin val app to see if val is configured correctly --- packages/server/src/ValServer.ts | 94 +++++++++++++++++++++++ packages/shared/src/internal/ApiRoutes.ts | 24 ++++++ 2 files changed, 118 insertions(+) diff --git a/packages/server/src/ValServer.ts b/packages/server/src/ValServer.ts index ca016c6ca..111b9ca33 100644 --- a/packages/server/src/ValServer.ts +++ b/packages/server/src/ValServer.ts @@ -48,6 +48,7 @@ import { import path from "path"; import { hasRemoteFileSchema } from "./hasRemoteFileSchema"; import { ReifiedRender } from "@valbuild/core"; +import { VERSION as uiVersion } from "@valbuild/ui"; export type ValServerOptions = { route: string; @@ -364,8 +365,101 @@ export const ValServer = ( json: { remoteFileAuth }, }; }; + const serverVersion = ((): string | null => { + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + return require("../package.json").version; + } catch { + return null; + } + })(); return { + "/admin/status": { + POST: async (req) => { + const timestamp = Date.now(); + // We imagine it is safer to not return the status directly, but to write the status back to the build url with the status + if (req.body.apiKey !== options.apiKey || !req.body.code) { + await new Promise((resolve) => + setTimeout(resolve, 5000 + Math.random() * 5000), + ); // make it harder to guess if the api key was wrong or the request code was wrong + return { + status: 401, + json: { + message: "Unauthorized", + }, + }; + } + const getStatus = () => { + const versions = { + core: Internal.VERSION.core || "unknown", + ui: uiVersion, + server: serverVersion || "unknown", + }; + if (options.mode === "fs") { + return { + versions, + mode: options.mode, + project: options.project, + }; + } + return { + versions, + mode: options.mode, + project: options.project, + commit: options.commit, + branch: options.branch, + root: options.root, + }; + }; + const searchParams = new URLSearchParams(); + searchParams.set("code", req.body.code); + const url = new URL( + `/api/val/status?${searchParams.toString()}`, + options.valBuildUrl, + ); + const res = await fetch(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${req.body.apiKey}`, + }, + body: JSON.stringify({ + status: getStatus(), + timestamp, + }), + }); + if (res.status === 401) { + try { + const resBody = await res.json(); + await new Promise((resolve) => + // Both here and if apiKey is wrong, we wait a bit to make it harder to guess if the api key was wrong or the request code was wrong + setTimeout(resolve, resBody.timestamp - timestamp), + ); + } catch (err) { + console.error("Could not parse response body: ", err); + } + return { + status: 401, + json: { + message: "Unauthorized", + }, + }; + } + if (res.status !== 200) { + return { + status: 400, + json: { + status: res.status, + statusText: res.statusText, + }, + }; + } + return { + status: 200, + }; + }, + }, "/draft/enable": { GET: async (req) => { const cookies = req.cookies; diff --git a/packages/shared/src/internal/ApiRoutes.ts b/packages/shared/src/internal/ApiRoutes.ts index e4f90e5b3..ba7eb424f 100644 --- a/packages/shared/src/internal/ApiRoutes.ts +++ b/packages/shared/src/internal/ApiRoutes.ts @@ -125,6 +125,30 @@ const onlyOneBooleanQueryParam = onlyOneStringQueryParam .transform((arg) => arg === "true"); export const Api = { + "/admin/status": { + POST: { + req: { + body: z.object({ + apiKey: z.string(), + code: z.string(), + }), + cookies: {}, + }, + res: z.union([ + unauthorizedResponse, + z.object({ + status: z.literal(200), + }), + z.object({ + status: z.literal(400), + json: z.object({ + status: z.number(), + statusText: z.string(), + }), + }), + ]), + }, + }, "/draft/enable": { GET: { req: { From 40bbfd8dd8e3620265482999aa40641b60c6264d Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Sat, 13 Sep 2025 09:54:24 +0200 Subject: [PATCH 2/5] Changeset --- .changeset/rotten-donuts-cough.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .changeset/rotten-donuts-cough.md diff --git a/.changeset/rotten-donuts-cough.md b/.changeset/rotten-donuts-cough.md new file mode 100644 index 000000000..d4baa5d27 --- /dev/null +++ b/.changeset/rotten-donuts-cough.md @@ -0,0 +1,14 @@ +--- +"@valbuild/server": patch +"@valbuild/shared": patch +"@valbuild/ui": patch +"@valbuild/cli": patch +"@valbuild/core": patch +"@valbuild/create": patch +"@valbuild/eslint-plugin": patch +"@valbuild/init": patch +"@valbuild/next": patch +"@valbuild/react": patch +--- + +Add admin status endpoint From a9632c7f0634cd58b26a5cef0c0d0244244ec09c Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Sun, 14 Sep 2025 10:34:33 +0200 Subject: [PATCH 3/5] Improve timing attack on /admin/status --- packages/server/src/ValServer.ts | 55 ++++++++++++++++------- packages/shared/src/internal/ApiRoutes.ts | 12 +++++ 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/packages/server/src/ValServer.ts b/packages/server/src/ValServer.ts index 111b9ca33..78e6cd56a 100644 --- a/packages/server/src/ValServer.ts +++ b/packages/server/src/ValServer.ts @@ -377,12 +377,14 @@ export const ValServer = ( return { "/admin/status": { POST: async (req) => { - const timestamp = Date.now(); + // This endpoint is the only that uses only the API key to authenticate. + // The reason is that it is called by the admin app to get the status of the current build and the admin app only has the api key + // The status information is not very sensitive, but the api key is so we're taking extra precautions to protect against brute force attacks and timing attacks... + const authTimeout = 7500; // constant timeout for 401s to make it harder to guess if the api key was wrong or the request code was wrong + const start = Date.now(); // We imagine it is safer to not return the status directly, but to write the status back to the build url with the status if (req.body.apiKey !== options.apiKey || !req.body.code) { - await new Promise((resolve) => - setTimeout(resolve, 5000 + Math.random() * 5000), - ); // make it harder to guess if the api key was wrong or the request code was wrong + await new Promise((resolve) => setTimeout(resolve, authTimeout)); // make it harder to guess if the api key was wrong or the request code was wrong return { status: 401, json: { @@ -418,27 +420,43 @@ export const ValServer = ( `/api/val/status?${searchParams.toString()}`, options.valBuildUrl, ); + type Versions = { + core: string; + ui: string; + server: string; + }; + const statusData = getStatus(); + const body: + | { + versions: Versions; + mode: "fs"; + project: string | undefined; + timestamp: number; + } + | { + mode: "http"; + versions: Versions; + root: string | undefined; + project: string | undefined; + commit: string | undefined; + branch: string | undefined; + timestamp: number; + } = { + ...statusData, + timestamp: start, + }; const res = await fetch(url, { method: "POST", headers: { "Content-Type": "application/json", Authorization: `Bearer ${req.body.apiKey}`, }, - body: JSON.stringify({ - status: getStatus(), - timestamp, - }), + body: JSON.stringify(body), }); if (res.status === 401) { - try { - const resBody = await res.json(); - await new Promise((resolve) => - // Both here and if apiKey is wrong, we wait a bit to make it harder to guess if the api key was wrong or the request code was wrong - setTimeout(resolve, resBody.timestamp - timestamp), - ); - } catch (err) { - console.error("Could not parse response body: ", err); - } + const timestamp = Date.now(); + const delay = Math.max(0, authTimeout - (start - timestamp)); + await new Promise((resolve) => setTimeout(resolve, delay)); return { status: 401, json: { @@ -457,6 +475,9 @@ export const ValServer = ( } return { status: 200, + json: { + ...statusData, + }, }; }, }, diff --git a/packages/shared/src/internal/ApiRoutes.ts b/packages/shared/src/internal/ApiRoutes.ts index ba7eb424f..f10496754 100644 --- a/packages/shared/src/internal/ApiRoutes.ts +++ b/packages/shared/src/internal/ApiRoutes.ts @@ -138,6 +138,18 @@ export const Api = { unauthorizedResponse, z.object({ status: z.literal(200), + json: z.object({ + versions: z.object({ + core: z.string(), + ui: z.string(), + server: z.string(), + }), + mode: z.union([z.literal("fs"), z.literal("http")]), + project: z.string().optional(), + commit: z.string().optional(), + branch: z.string().optional(), + root: z.string().optional(), + }), }), z.object({ status: z.literal(400), From 8b6ed20e6b9113cfc365a1741d26178dee3ae35c Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Sun, 14 Sep 2025 10:54:17 +0200 Subject: [PATCH 4/5] Avoid obvious DDOS attack vector for Vercel installations... ... earlier we would wait 7.5s in Val servers, which would mean that an attacker could induce extreme costs by calling the endpoint with the wrong api keys... NOTE: also want to add: we weren't sure if we actually need to lock this information down. --- packages/server/src/ValServer.ts | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/server/src/ValServer.ts b/packages/server/src/ValServer.ts index 78e6cd56a..08baacf2e 100644 --- a/packages/server/src/ValServer.ts +++ b/packages/server/src/ValServer.ts @@ -379,19 +379,7 @@ export const ValServer = ( POST: async (req) => { // This endpoint is the only that uses only the API key to authenticate. // The reason is that it is called by the admin app to get the status of the current build and the admin app only has the api key - // The status information is not very sensitive, but the api key is so we're taking extra precautions to protect against brute force attacks and timing attacks... - const authTimeout = 7500; // constant timeout for 401s to make it harder to guess if the api key was wrong or the request code was wrong - const start = Date.now(); - // We imagine it is safer to not return the status directly, but to write the status back to the build url with the status - if (req.body.apiKey !== options.apiKey || !req.body.code) { - await new Promise((resolve) => setTimeout(resolve, authTimeout)); // make it harder to guess if the api key was wrong or the request code was wrong - return { - status: 401, - json: { - message: "Unauthorized", - }, - }; - } + // The status information is not very sensitive but we figured it is might be used for reconnaissance, so we're taking extra precautions to protect against brute force attacks and timing attacks... const getStatus = () => { const versions = { core: Internal.VERSION.core || "unknown", @@ -414,8 +402,11 @@ export const ValServer = ( root: options.root, }; }; + // We do NOT check the api key prior to sending the request, because it would open us up to a timing attack const searchParams = new URLSearchParams(); - searchParams.set("code", req.body.code); + searchParams.set("request_code", req.body.code); + const clientCode = crypto.randomUUID(); + searchParams.set("client_code", clientCode); const url = new URL( `/api/val/status?${searchParams.toString()}`, options.valBuildUrl, @@ -443,7 +434,7 @@ export const ValServer = ( timestamp: number; } = { ...statusData, - timestamp: start, + timestamp: Date.now(), }; const res = await fetch(url, { method: "POST", @@ -453,10 +444,19 @@ export const ValServer = ( }, body: JSON.stringify(body), }); + const resBody = await res.json(); + // The response might include a delay to make it harder to brute force the api key + const delay = resBody.delay || 0; + await new Promise((resolve) => setTimeout(resolve, delay)); + if (resBody.client_code !== clientCode) { + return { + status: 401, + json: { + message: "Unauthorized", + }, + }; + } if (res.status === 401) { - const timestamp = Date.now(); - const delay = Math.max(0, authTimeout - (start - timestamp)); - await new Promise((resolve) => setTimeout(resolve, delay)); return { status: 401, json: { From c5ed0d581a7021bd66742ccda37de8f364dbfd4a Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 17 Sep 2025 21:05:28 +0200 Subject: [PATCH 5/5] Update status endpoint: add timeout, move from params to body (safety? convince?) --- packages/server/src/ValServer.ts | 81 +++++++++++++++++------ packages/shared/src/internal/ApiRoutes.ts | 21 +++++- 2 files changed, 78 insertions(+), 24 deletions(-) diff --git a/packages/server/src/ValServer.ts b/packages/server/src/ValServer.ts index 08baacf2e..9ad87f983 100644 --- a/packages/server/src/ValServer.ts +++ b/packages/server/src/ValServer.ts @@ -380,6 +380,17 @@ export const ValServer = ( // This endpoint is the only that uses only the API key to authenticate. // The reason is that it is called by the admin app to get the status of the current build and the admin app only has the api key // The status information is not very sensitive but we figured it is might be used for reconnaissance, so we're taking extra precautions to protect against brute force attacks and timing attacks... + + // Input validation + if ( + !req.body?.apiKey || + !req.body?.code || + typeof req.body.apiKey !== "string" || + typeof req.body.code !== "string" + ) { + return { status: 400, json: { message: "Invalid request" } }; + } + const getStatus = () => { const versions = { core: Internal.VERSION.core || "unknown", @@ -403,14 +414,8 @@ export const ValServer = ( }; }; // We do NOT check the api key prior to sending the request, because it would open us up to a timing attack - const searchParams = new URLSearchParams(); - searchParams.set("request_code", req.body.code); const clientCode = crypto.randomUUID(); - searchParams.set("client_code", clientCode); - const url = new URL( - `/api/val/status?${searchParams.toString()}`, - options.valBuildUrl, - ); + const url = new URL(`/api/val/status`, options.valBuildUrl); type Versions = { core: string; ui: string; @@ -423,6 +428,8 @@ export const ValServer = ( mode: "fs"; project: string | undefined; timestamp: number; + request_code: string; + client_code: string; } | { mode: "http"; @@ -432,23 +439,52 @@ export const ValServer = ( commit: string | undefined; branch: string | undefined; timestamp: number; + request_code: string; + client_code: string; } = { ...statusData, timestamp: Date.now(), + request_code: req.body.code, + client_code: clientCode, }; - const res = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${req.body.apiKey}`, - }, - body: JSON.stringify(body), - }); - const resBody = await res.json(); - // The response might include a delay to make it harder to brute force the api key - const delay = resBody.delay || 0; + + // Network request with timeout and error handling + let res, resBody; + try { + const controller = new AbortController(); + setTimeout(() => controller.abort(), 30000); // 30s timeout + + res = await fetch(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${req.body.apiKey}`, + }, + body: JSON.stringify(body), + signal: controller.signal, + }); + resBody = await res.json(); + } catch (error) { + return { + status: 503, + json: { message: "Service temporarily unavailable" }, + }; + } + + // Response validation + if (!resBody || typeof resBody !== "object") { + return { + status: 502, + json: { message: "Invalid response from service" }, + }; + } + + // Bounded delay to prevent DDoS + const delay = Math.min(Math.max(resBody.delay || 0, 0), 10000); // Cap at 10 seconds await new Promise((resolve) => setTimeout(resolve, delay)); - if (resBody.client_code !== clientCode) { + + // Safe client code validation + if (!resBody.client_code || resBody.client_code !== clientCode) { return { status: 401, json: { @@ -456,6 +492,7 @@ export const ValServer = ( }, }; } + if (res.status === 401) { return { status: 401, @@ -464,12 +501,12 @@ export const ValServer = ( }, }; } + if (res.status !== 200) { return { - status: 400, + status: 500, json: { - status: res.status, - statusText: res.statusText, + message: "Internal server error", }, }; } diff --git a/packages/shared/src/internal/ApiRoutes.ts b/packages/shared/src/internal/ApiRoutes.ts index f10496754..b9ba6f9cd 100644 --- a/packages/shared/src/internal/ApiRoutes.ts +++ b/packages/shared/src/internal/ApiRoutes.ts @@ -154,8 +154,25 @@ export const Api = { z.object({ status: z.literal(400), json: z.object({ - status: z.number(), - statusText: z.string(), + message: z.string(), + }), + }), + z.object({ + status: z.literal(500), + json: z.object({ + message: z.string(), + }), + }), + z.object({ + status: z.literal(502), + json: z.object({ + message: z.string(), + }), + }), + z.object({ + status: z.literal(503), + json: z.object({ + message: z.string(), }), }), ]),