From ac353d1c0cebddec7542e419899c84108bdf5f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Proch=C3=A1zka?= Date: Thu, 20 Aug 2026 13:27:22 +0000 Subject: [PATCH 01/10] Add opt-in upstream API fallback with two independent toggles When a call misses locally, the runtime can replay it against the real Apify platform instead of failing. Two independent toggles gate it: one for paths this runtime does not serve at all, one for records it has not seen. Both default off and reset on restart. Readable and writable on the runtime-internal API and from the console's new settings page. Relays only a successful upstream reply, and only ever forwards the token the caller themselves presented; any upstream failure returns the original local error unchanged. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- CLAUDE.MD | 10 + requirements/api.md | 56 +- requirements/console.md | 28 +- src/api/routes/api-fallback.ts | 82 +++ src/api/routes/dev-folder.ts | 25 +- src/api/server.ts | 49 +- src/console/server.ts | 63 +- src/console/templates.ts | 46 +- src/services/api-fallback.ts | 182 +++++ test/integration/api-fallback.test.ts | 854 ++++++++++++++++++++++ test/integration/settings-console.test.ts | 198 +++++ test/unit/api-fallback-state.test.ts | 87 +++ 12 files changed, 1642 insertions(+), 38 deletions(-) create mode 100644 src/api/routes/api-fallback.ts create mode 100644 src/services/api-fallback.ts create mode 100644 test/integration/api-fallback.test.ts create mode 100644 test/integration/settings-console.test.ts create mode 100644 test/unit/api-fallback-state.test.ts diff --git a/CLAUDE.MD b/CLAUDE.MD index 79310c6..a0aa4e8 100644 --- a/CLAUDE.MD +++ b/CLAUDE.MD @@ -30,6 +30,16 @@ Local Actor runtime is an Actor development tool for developing, running, and de hardcodes a `/v2`-suffixed base URL). From then on, edit locally, recompile locally (`tsc` or the language-appropriate equivalent), and `apify call` again - no `apify push`/build in between. Submitting `--body '""'` clears the registration. Dependency changes still need a real rebuild. +- If a call fails because this runtime doesn't have the Actor/run/build/storage id you're after, or + doesn't implement that endpoint at all, you can opt in to having such calls transparently relayed to + the real Apify platform instead of failing: + `apify api POST /actor-runtime/api-fallback --body '{"fallbackUnimplementedEnabled": true, "fallbackNotFoundEnabled": true}'` + (either field alone is also accepted; `apify api GET /actor-runtime/api-fallback` reads the current + state). Both default off and reset to off on every restart. Enabling either forwards whatever token you + authenticated the failing call with to the real platform, and - since all HTTP methods are eligible - + can turn a locally-missing `POST`/`PUT`/`DELETE` into a real write against your real account; only turn + this on with a token/account you're comfortable with that. A relayed response carries an + `x-actor-runtime-fallback` header naming which platform served it. ## Through direct API calls diff --git a/requirements/api.md b/requirements/api.md index 624f82e..3ab2518 100644 --- a/requirements/api.md +++ b/requirements/api.md @@ -152,4 +152,58 @@ ## Upstream fallback (opt-in, off by default, all HTTP methods) -- Not implemented +- Two independent booleans, `fallbackUnimplementedEnabled` and `fallbackNotFoundEnabled`, gate whether a + request this runtime cannot satisfy locally is instead relayed to the real Apify platform. Both default + to `false` on a fresh process and neither is persisted anywhere - a restart always brings both back to + `false`, regardless of how they were last set. Either can be on without the other; all four + combinations are valid. +- **`GET /actor-runtime/api-fallback`** (also reachable at `/v2/actor-runtime/api-fallback`, like every + other endpoint in this namespace) returns + `{ "data": { "fallbackUnimplementedEnabled": , "fallbackNotFoundEnabled": , "upstreamBaseUrl": } }`. + `upstreamBaseUrl` is the platform this runtime would relay to (default `https://api.apify.com`, or the + value of `APIFY_UPSTREAM_API_BASE_URL` if set) - reported for visibility, but read-only: no request + body can change it. +- **`POST /actor-runtime/api-fallback`** (same two mounts) accepts a **partial** body - either field, or + both - and merges it into the existing state, leaving any field the body doesn't mention untouched. The + response is the same shape `GET` returns, showing the state immediately after the merge. + - **Authenticated** the same way as every other route in this namespace: no token is `401` + `user-not-authenticated`, with no state change. + - **Error responses**: a body that isn't a JSON object (a JSON array, scalar, or `null`), a body + present but empty (`{}`), a body containing a key other than the two above, or a body where a + present key's value isn't a boolean, is `400` `invalid-request`, with no state change. +- **Which local outcome each toggle covers** (exhaustive - every other error response is never eligible, + under any toggle combination): + - `fallbackUnimplementedEnabled` covers a request whose path/method this runtime does not serve at + all - either an off-spec path (matches no entry in the vendored spec table, `501 vs 404` above) or a + spec-known path this runtime hasn't built (the `501` case, same section). From the caller's point of + view both are "nothing local answers this", so one toggle covers both. + - `fallbackNotFoundEnabled` covers a request that reaches a route this runtime does serve, but whose + specific record id doesn't exist locally (`record-not-found`, see "Response envelopes" above). + - Every other error type - `invalid-request`, `user-not-authenticated`, `cannot-remove-running-run`, + `deleting-unfinished-build`, any `dev-folder-*` type, `internal-error` - is never relayed, regardless + of either toggle's state. +- **All HTTP methods are eligible for both toggles, writes included**: a `POST`/`PUT`/`DELETE` that would + otherwise 404/501 locally is relayed exactly like a `GET` when its toggle is on - and, if the platform + accepts it, becomes a real write against the caller's real account. This is a deliberate consequence of + opting in, not an oversight. +- **What a successful relay looks like**: the platform's response is returned to the caller verbatim - + status, body, and headers - with two markers added: `x-actor-runtime-fallback: ` (which + platform served it) and `x-actor-runtime-fallback-trigger: unimplemented` or `record-not-found` (which + toggle let it through). Only a final `2xx` counts as successful. +- **Fail-closed guarantee**: anything else - a non-`2xx` response, a timeout, or the platform being + unreachable - reproduces the exact response the caller would have gotten with both toggles off: the + original local error, unchanged, with neither marker header present. The platform's own status or body + is never surfaced to the caller. One attempt is made per request; nothing is retried. +- **Only the caller's own presented token is ever forwarded.** A relayed request's `Authorization` header + is always the exact bearer token the caller themselves sent on that request - never a different or + runtime-internal credential, and never sent at all for a request this runtime didn't authenticate. + Enabling either toggle therefore means the caller's own Apify token reaches the configured + `upstreamBaseUrl` on every eligible request; this is the risk being opted into. +- **Never enriches a call that already succeeds locally**: a collection/list endpoint (e.g. + `GET /v2/datasets`) that already returns `200` from local data never consults either toggle and never + gains platform objects. Fallback only ever resolves an otherwise-failing request; it does not make a + local listing "complete". +- One line is logged per fallback attempt: a relayed request logs once at the informational level a + successful relay happened; an abandoned attempt (any fail-closed case above) logs once at the warning + level, including the platform's status or failure reason. Neither line appears when the relevant + toggle is off. diff --git a/requirements/console.md b/requirements/console.md index 12ac932..ed066c9 100644 --- a/requirements/console.md +++ b/requirements/console.md @@ -9,8 +9,8 @@ - The console has no login of its own, so with multiple users it lists and shows every user's objects rather than scoping to one - the API's own endpoints stay strictly scoped to the calling token's user (`storage.md`'s "Users" section). -- The console is unauthenticated. Every route is a read except the dev-folder form below, which is the - console's one write - it is no longer strictly view-only. +- The console is unauthenticated. Every route is a read except the dev-folder form below and the + Settings form below, which are the console's only two writes - it is no longer strictly view-only. - There are three types of objects: key-value store, dataset, request queue. - For each object type there must be exactly one widget for inspection. - The request-queue widget leads with the authoritative counts from `RequestQueue.getInfo()` @@ -28,6 +28,7 @@ - key-value stores - datasets - request queues + - Settings (a single page, not a list/detail pair - see "Settings page" below) - List view is a list of objects that can be clicked on to open detail view. - Detail view of an object is showing only one object with all the available data @@ -49,3 +50,26 @@ value is rejected as a malformed path, also matching the API. - A submission that fails validation redirects back to the same detail page with the classified error message shown inline, never swallowed by the redirect. + +## Settings page + +- The last entry in every page's header navigation is "Settings", linking to `/settings` - the one page + for the upstream API fallback toggles (`api.md`'s "Upstream fallback" section). Every other page's + header nav also shows both toggles' current state next to that link, in the form + `Settings — fallback (unimplemented: on|off, not-found: on|off)`, so neither toggle can ever be on + without being visible from anywhere in the console; the two states are shown independently, never + collapsed into a single word (a mixed state - one on, one off - is visually distinct from both-on and + both-off). +- `/settings` itself shows `fallbackUnimplementedEnabled`, `fallbackNotFoundEnabled`, and + `upstreamBaseUrl` (the same values the API's toggle endpoint reports), plus a one-line warning that + enabling either toggle forwards the caller's own Apify token to that URL. +- A single form on the page has two checkboxes, "Fall back for unimplemented endpoints" and "Fall back + for not-found records", and one submit. Submitting it always sends both checkboxes' current state + together - an unchecked box is read as `false`, not as "leave this toggle unchanged" - and redirects + back to `/settings` showing the result. This differs from the API's own partial `POST` (`api.md`), + which only touches the field(s) a caller's body actually names; both surfaces write through the same + underlying toggle state, so a flip made on one is immediately visible on the other and via the API's + own `GET`, with no restart needed either way. +- Since the console has no login of its own, anyone who can reach it can flip either toggle for every + caller of the API - the same unauthenticated, cross-user model the rest of the console already has, not + a new exposure specific to this page. diff --git a/src/api/routes/api-fallback.ts b/src/api/routes/api-fallback.ts new file mode 100644 index 0000000..0a8f039 --- /dev/null +++ b/src/api/routes/api-fallback.ts @@ -0,0 +1,82 @@ +/** + * `GET`/`POST /actor-runtime/api-fallback` (`api.md`'s "Upstream fallback" section) - mounted on the + * same `/actor-runtime` sub-router `dev-folder.ts` already registers on, so it shares that router's + * single `auth()` registration (`server.ts`) rather than adding its own, and is served at both mounts + * (`/actor-runtime/api-fallback` and `/v2/actor-runtime/api-fallback`) the same way the dev-folder route + * is. + * + * `GET` reads the current toggle state; `POST` accepts a **partial** body - either + * `fallbackUnimplementedEnabled`, `fallbackNotFoundEnabled`, or both - and merges it into the existing + * state via `setApiFallbackState`, leaving any field the body didn't mention untouched. `upstreamBaseUrl` + * is reported on every response but is never itself a settable field. + */ +import type { Router } from 'express'; + +import { sendData } from '../envelope.js'; +import { invalidRequest } from '../errors.js'; +import { h, jsonBody } from '../handler.js'; +import { + getApiFallbackState, + setApiFallbackState, + upstreamBaseUrl, + type ApiFallbackState, +} from '../../services/api-fallback.js'; + +const SETTABLE_FIELDS = new Set(['fallbackUnimplementedEnabled', 'fallbackNotFoundEnabled']); + +function respondWithState(): { data: ApiFallbackState & { upstreamBaseUrl: string } } { + return { data: { ...getApiFallbackState(), upstreamBaseUrl: upstreamBaseUrl() } }; +} + +/** Parses and validates a `POST` body into a `setApiFallbackState` patch, throwing `invalid-request` for + * every malformed shape the spec names: not a JSON object (array, string, number, `null`), present but + * empty (`{}`), an unknown key, or a present key whose value isn't a boolean. Never partially applies a + * rejected body - the caller only ever sees the merged state after every field in the body has passed + * this check. */ +function parsePatch(raw: unknown): Partial { + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { + throw invalidRequest( + 'Request body must be a JSON object with fallbackUnimplementedEnabled and/or fallbackNotFoundEnabled', + ); + } + + const entries = Object.entries(raw as Record); + if (entries.length === 0) { + throw invalidRequest( + 'Request body must set at least one of fallbackUnimplementedEnabled or fallbackNotFoundEnabled', + ); + } + + const patch: Partial = {}; + for (const [key, value] of entries) { + if (!SETTABLE_FIELDS.has(key as keyof ApiFallbackState)) { + throw invalidRequest(`Unknown field "${key}"`); + } + if (typeof value !== 'boolean') { + throw invalidRequest(`Field "${key}" must be a boolean`); + } + patch[key as keyof ApiFallbackState] = value; + } + return patch; +} + +/** Mounts the `/api-fallback` routes onto `router`, matching every other route module's + * `mount*(router, ...): void` convention. `router` is expected to already have `auth()` registered on + * it by the caller (`server.ts`), same as `mountDevFolder`. */ +export function mountApiFallback(router: Router): void { + router.get( + '/api-fallback', + h(async (_req, res) => { + sendData(res, respondWithState().data); + }), + ); + + router.post( + '/api-fallback', + h(async (req, res) => { + const patch = parsePatch(jsonBody(req)); + setApiFallbackState(patch); + sendData(res, respondWithState().data); + }), + ); +} diff --git a/src/api/routes/dev-folder.ts b/src/api/routes/dev-folder.ts index be77fd1..8eaa46e 100644 --- a/src/api/routes/dev-folder.ts +++ b/src/api/routes/dev-folder.ts @@ -1,11 +1,13 @@ /** * `POST /actor-runtime/dev-folder/:actorId` - deliberately outside the emulated `/v2` surface - * (`api.md`'s `/actor-runtime/*` namespace). `server.ts` creates its own sub-router, calls - * `mountDevFolder` on it once, and mounts that same router instance at both `/actor-runtime` (canonical) - * and `/v2/actor-runtime` (an alias existing solely because `apify api` hardcodes a `/v2`-suffixed base - * URL - see `server.ts`'s doc comment). Neither mount is nested under the `v2` router, so this route - * needs its own `auth()` rather than inheriting `v2`'s - and since only one of the two mounts ever - * matches a given request, that `auth()` still runs exactly once per request either way. + * (`api.md`'s `/actor-runtime/*` namespace). `server.ts` creates one shared sub-router (with its own + * `auth()`, registered once there - not by this module) for the whole `/actor-runtime/*` namespace, + * calls this and `mountApiFallback` on it, and mounts that same router instance at both + * `/actor-runtime` (canonical) and `/v2/actor-runtime` (an alias existing solely because `apify api` + * hardcodes a `/v2`-suffixed base URL - see `server.ts`'s doc comment). Neither mount is nested under + * the `v2` router, so this namespace needs its own `auth()` rather than inheriting `v2`'s - and since + * only one of the two mounts ever matches a given request, that `auth()` still runs exactly once per + * request either way. * * Canonical body is a JSON string: `'"/abs/path"'` to set, `'""'` to clear (`api.md`). A JSON value that * parses but isn't a string is rejected the same way a malformed body is. @@ -15,7 +17,7 @@ */ import type { Router } from 'express'; -import { auth, requireUser } from '../auth.js'; +import { requireUser } from '../auth.js'; import { sendData } from '../envelope.js'; import { ApiError, invalidRequest, recordNotFound } from '../errors.js'; import { h, jsonBody } from '../handler.js'; @@ -50,13 +52,10 @@ function toApiError(result: Exclude): ApiErr } /** Mounts the `/dev-folder/:actorId` route onto `router`, matching every other route module's - * `mount*(router, deps): void` convention - `server.ts` creates the sub-router, calls this on it, and - * mounts the result at `/actor-runtime` itself (owning the path prefix the same way it owns `/v2`). - * Registers its own `auth()` on `router` rather than inheriting `v2`'s, since this route lives outside - * the `v2` router entirely (`api.md`'s `/actor-runtime/*` namespace). */ + * `mount*(router, deps): void` convention - `server.ts` creates the sub-router (with its own shared + * `auth()`, registered by the caller, not here), calls this on it, and mounts the result at + * `/actor-runtime` itself (owning the path prefix the same way it owns `/v2`). */ export function mountDevFolder(router: Router, deps: ApiServerDeps): void { - router.use(auth()); - router.post( '/dev-folder/:actorId', h(async (req, res) => { diff --git a/src/api/server.ts b/src/api/server.ts index f981037..5420830 100644 --- a/src/api/server.ts +++ b/src/api/server.ts @@ -14,6 +14,8 @@ import { mountRuns } from './routes/runs.js'; import { mountLogs } from './routes/logs.js'; import { mountRunStorageAliases } from './routes/run-storage-aliases.js'; import { mountDevFolder } from './routes/dev-folder.js'; +import { mountApiFallback } from './routes/api-fallback.js'; +import { attemptFallback } from '../services/api-fallback.js'; import type { Driver } from '../driver/types.js'; export interface ApiServerDeps { @@ -30,10 +32,15 @@ export function createApiServer(deps: ApiServerDeps): Express { // `/actor-runtime/*` - a deliberately non-Apify, local-runtime-only namespace (`api.md`), registered // before the `v2` router (and its own `auth()`) below entirely, so it gets its own sub-router with its - // own `auth()` (see `mountDevFolder`'s doc comment) rather than inheriting `v2.use(auth())`. - const devFolder = express.Router(); - mountDevFolder(devFolder, deps); - app.use('/actor-runtime', devFolder); + // own `auth()` rather than inheriting `v2.use(auth())`. Registered once here, shared by every route + // module mounted on this router (`mountDevFolder`, `mountApiFallback`) rather than each registering + // its own - they are the same router instance, so a second registration would just run `auth()` + // twice per request for no benefit. + const actorRuntime = express.Router(); + actorRuntime.use(auth()); + mountDevFolder(actorRuntime, deps); + mountApiFallback(actorRuntime); + app.use('/actor-runtime', actorRuntime); // Also served at `/v2/actor-runtime/*` - the *same* router instance, no duplicated route logic - solely // because `apify api`'s own URL-building hardcodes a `/v2`-suffixed base (`${baseUrl}/${endpoint}`, // `baseUrl` already ending in `/v2`) and its `normalizePath` only strips a leading `/` and a leading @@ -46,7 +53,7 @@ export function createApiServer(deps: ApiServerDeps): Express { // namespace as `/actor-runtime/*`, reachable a second way purely for CLI ergonomics (`api.md`). The // dev-folder fields are still never exposed on any real `/v2` Actor response either way - `actorDto` // is explicit field-by-field regardless of which path reached this router. - app.use('/v2/actor-runtime', devFolder); + app.use('/v2/actor-runtime', actorRuntime); const v2 = express.Router(); v2.use(auth()); @@ -63,19 +70,37 @@ export function createApiServer(deps: ApiServerDeps): Express { app.use('/v2', v2); - app.use((req: Request, res: Response) => { + // The first of the two seams `attemptFallback` (`services/api-fallback.ts`) can serve a response + // from instead of this local error: a request that fell through every mounted router without any + // route matching at all - a genuinely off-spec path, or a spec-known path this runtime hasn't built + // (`spec-table.ts`). Both local error shapes are gated by `fallbackUnimplementedEnabled` - from the + // caller's point of view, "nothing local answers this" either way. + app.use(async (req: Request, res: Response) => { const path = req.path.replace(/^\/+/, ''); const entry = matchSpecPath(req.method, path); - if (entry && !entry.implemented) { - sendError(res, 501, 'not-implemented', `${req.method} ${req.path} is not implemented by this runtime`); - return; - } - sendError(res, 404, 'not-found', `${req.method} ${req.path} was not found`); + const localError = + entry?.implemented === false + ? { + status: 501, + type: 'not-implemented', + message: `${req.method} ${req.path} is not implemented by this runtime`, + } + : { status: 404, type: 'not-found', message: `${req.method} ${req.path} was not found` }; + + if (await attemptFallback(req, res, localError)) return; + sendError(res, localError.status, localError.type, localError.message); }); + // The second seam: a route handler under a matched router rejected with an `ApiError` (`handler.ts`'s + // `h()` forwards it here via `.catch(next)`). Only a `record-not-found` rejection is ever eligible for + // fallback (`fallbackNotFoundEnabled`) - `attemptFallback` itself is what enforces that, from the + // error's own `type`, so every other `ApiError` (`invalid-request`, `cannot-remove-running-run`, + // `deleting-unfinished-build`, any `dev-folder-*` type, ...) always falls straight through to the + // local response below, untouched. // eslint-disable-next-line @typescript-eslint/no-unused-vars - app.use((err: unknown, req: Request, res: Response, next: NextFunction) => { + app.use(async (err: unknown, req: Request, res: Response, next: NextFunction) => { if (err instanceof ApiError) { + if (await attemptFallback(req, res, { status: err.status, type: err.type, message: err.message })) return; sendError(res, err.status, err.type, err.message); return; } diff --git a/src/console/server.ts b/src/console/server.ts index af8c2f9..23e415e 100644 --- a/src/console/server.ts +++ b/src/console/server.ts @@ -5,14 +5,15 @@ * shared rather than reimplemented. * * The console itself has no login of its own - it is unauthenticated, and every route is a read except - * exactly one mutation (`console.md`'s "Every route is a read except the dev-folder form below, which is - * the console's one write"): the dev-folder form on the Actor detail view. With multiple users it does - * not scope reads to any one of them: every list/detail route below reads through the - * `listAll*`/`get*ById` cross-user service functions (see e.g. `services/actors.ts: listAllActors`), - * never the API's own per-user `listOwned*`/`getOwned*`, and every list row and detail view shows the - * object's owner `userId` (`console.md`: "Frontend shows for each object the owner (userId)"). The - * dev-folder form writes cross-user the same way - a deliberate deviation from the API's own - * strictly-owner-scoped write, not an accident. + * two mutations (`console.md`): the dev-folder form on the Actor detail view, and the `/settings` form + * below. With multiple users it does not scope reads to any one of them: every list/detail route below + * reads through the `listAll*`/`get*ById` cross-user service functions (see e.g. + * `services/actors.ts: listAllActors`), never the API's own per-user `listOwned*`/`getOwned*`, and every + * list row and detail view shows the object's owner `userId` (`console.md`: "Frontend shows for each + * object the owner (userId)"). The dev-folder form writes cross-user the same way - a deliberate + * deviation from the API's own strictly-owner-scoped write, not an accident; the `/settings` form is + * runtime-global by nature (`api.md`'s "Upstream fallback" section), so ownership doesn't apply to it at + * all. */ import express, { type Express } from 'express'; @@ -32,7 +33,17 @@ import { openDataset, openKeyValueStore, openRequestQueue } from '../storage/ope import { pageKeys } from '../services/kv-key-listing.js'; import { applyDatasetProjection, type DatasetItem } from '../services/dataset-projection.js'; import { ansiToHtml } from './ansi.js'; -import { definitionList, devFolderForm, escapeHtml, layout, table, type LinkedCell } from './templates.js'; +import { + apiFallbackWarning, + definitionList, + devFolderForm, + escapeHtml, + layout, + settingsForm, + table, + type LinkedCell, +} from './templates.js'; +import { getApiFallbackState, setApiFallbackState, upstreamBaseUrl } from '../services/api-fallback.js'; import type { Driver } from '../driver/types.js'; /** A run's default-storage id rendered as a link to that storage's detail view instead of plain text. */ @@ -392,5 +403,39 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { res.send(layout(`Request queue ${record.id}`, body)); }); + // --- Settings: the shared upstream-fallback toggle state (`services/api-fallback.ts`), read/written + // through the same module the API's `GET`/`POST /actor-runtime/api-fallback` route uses - never + // through the API port itself (the dev-folder form's precedent). This is the console's second + // mutation alongside the dev-folder form - `console.md`'s "every route is a read except..." now + // names both. + + app.get('/settings', async (_req, res) => { + const state = getApiFallbackState(); + const body = + apiFallbackWarning() + + definitionList([ + ['fallbackUnimplementedEnabled', state.fallbackUnimplementedEnabled], + ['fallbackNotFoundEnabled', state.fallbackNotFoundEnabled], + ['upstreamBaseUrl', upstreamBaseUrl()], + ]) + + '

Change settings

' + + settingsForm(state); + res.send(layout('Settings', body)); + }); + + /** Always submits both checkboxes' current state, per the form's own contract + * (`templates.ts: settingsForm`'s doc comment) - an unchecked box is simply absent from the + * urlencoded body, read as `false` here, never as "leave this field unchanged". Funnels into the + * same `setApiFallbackState` the API route calls, so the two surfaces can never observe or produce + * different toggle states for the same request. */ + app.post('/settings', async (req, res) => { + const body = req.body as Record | undefined; + setApiFallbackState({ + fallbackUnimplementedEnabled: body?.fallbackUnimplementedEnabled === 'on', + fallbackNotFoundEnabled: body?.fallbackNotFoundEnabled === 'on', + }); + res.redirect('/settings'); + }); + return app; } diff --git a/src/console/templates.ts b/src/console/templates.ts index 6be72dc..14a8950 100644 --- a/src/console/templates.ts +++ b/src/console/templates.ts @@ -1,5 +1,7 @@ /** Minimal server-rendered HTML helpers. No SPA, no bundler, no build step (`console.md`). */ +import { getApiFallbackState, type ApiFallbackState } from '../services/api-fallback.js'; + export function escapeHtml(value: unknown): string { return String(value ?? '') .replace(/&/g, '&') @@ -18,8 +20,23 @@ const NAV = [ ['/request-queues', 'Request queues'], ] as const; +function onOff(enabled: boolean): 'on' | 'off' { + return enabled ? 'on' : 'off'; +} + +/** The final nav entry, present on every page (`console.md`'s "header state indicator") - "Settings" + * plus both fallback toggles' current state, so neither toggle can ever be on without being visible from + * anywhere in the console. Read fresh on every render, straight from `services/api-fallback.ts` - the + * one module both the API route and the `/settings` form write through - never threaded in as an + * argument, so this needs no change to `layout()`'s signature or any of its call sites. */ +function fallbackNavEntry(): string { + const state = getApiFallbackState(); + const label = `Settings — fallback (unimplemented: ${onOff(state.fallbackUnimplementedEnabled)}, not-found: ${onOff(state.fallbackNotFoundEnabled)})`; + return `${label}`; +} + export function layout(title: string, body: string): string { - const nav = NAV.map(([href, label]) => `${label}`).join(' | '); + const nav = [...NAV.map(([href, label]) => `${label}`), fallbackNavEntry()].join(' | '); return ` @@ -37,6 +54,7 @@ export function layout(title: string, body: string): string { pre { background: #f5f5f5; padding: 1rem; overflow-x: auto; white-space: pre-wrap; } .empty { color: #777; font-style: italic; } .error { color: #b00020; } + .warning { color: #94600b; } .wide-input { width: 28rem; } h1 { margin-top: 0; } @@ -102,6 +120,32 @@ export function devFolderForm(actorId: string, currentValue: string, errorMessag ); } +/** The one-line credential-forwarding warning the `/settings` page shows above its form + * (`console.md`'s "Settings page" section) - both toggles forward the caller's own Apify token the + * moment either is on, so this is shown unconditionally, not only once a toggle is already on. */ +export function apiFallbackWarning(): string { + return '

Enabling either option below forwards the caller\'s own Apify token to the upstream API shown above.

'; +} + +/** The `/settings` page's one form (`console.md`): two checkboxes, one submit, always submitting both + * checkboxes' current state together - an unchecked box is simply absent from the submitted body, which + * the POST route (`console/server.ts`) reads as `false` for that field, never as "leave unchanged" (the + * console form's own single-submit contract, unlike the API route's genuinely partial `POST`). */ +export function settingsForm(state: ApiFallbackState): string { + const checkedAttr = (enabled: boolean) => (enabled ? ' checked' : ''); + return ( + '
' + + '

' + + '

' + + '' + + '
' + ); +} + export function definitionList(fields: Array<[string, unknown]>): string { const rows = fields.map(([key, value]) => `
${escapeHtml(key)}
${renderValue(value)}
`).join(''); return `
${rows}
`; diff --git a/src/services/api-fallback.ts b/src/services/api-fallback.ts new file mode 100644 index 0000000..415c433 --- /dev/null +++ b/src/services/api-fallback.ts @@ -0,0 +1,182 @@ +/** + * Upstream API fallback (`api.md`'s "Upstream fallback" section): when a call locally misses - either + * because nothing in this runtime serves the path/method at all, or because it does but the specific + * record id doesn't exist - and the matching toggle is on, the request is replayed verbatim against the + * real Apify platform instead of failing. Both toggles default off and reset on every restart; this + * module is the only place either fact is read or written, by the API route (`api/routes/api-fallback.ts`), + * the console's `/settings` page, and `console/templates.ts: layout()`'s per-page state indicator alike. + * + * `attemptFallback` is the single seam both of `server.ts`'s local-miss sites (the terminal catch-all + * and the generic error middleware) call through - it alone knows the eligibility mapping below, the + * replay request, and how a response does or doesn't get relayed. + */ +import type { Request, Response } from 'express'; + +import { rawBody } from '../api/handler.js'; + +export interface ApiFallbackState { + fallbackUnimplementedEnabled: boolean; + fallbackNotFoundEnabled: boolean; +} + +function defaultState(): ApiFallbackState { + return { fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: false }; +} + +let state: ApiFallbackState = defaultState(); + +export function getApiFallbackState(): ApiFallbackState { + return { ...state }; +} + +/** Merges `patch` into the existing state - either field, or both, whichever the caller supplies. Both + * the API route (a genuinely partial `POST`) and the console form (which always sends both fields) + * call this identically. */ +export function setApiFallbackState(patch: Partial): ApiFallbackState { + state = { ...state, ...patch }; + return { ...state }; +} + +/** Test-only: forget any toggle flips a previous test made, matching `services/users.ts`'s + * `resetUsersForTests` convention. Never call this from runtime code. */ +export function resetApiFallbackStateForTests(): void { + state = defaultState(); +} + +/** Same env var `services/identity-resolution.ts` already established for the identity probe - reused + * verbatim rather than inventing a second one. Trailing slashes trimmed so ` + * ` never produces a doubled `//`. */ +export function upstreamBaseUrl(): string { + const configured = process.env.APIFY_UPSTREAM_API_BASE_URL ?? 'https://api.apify.com'; + return configured.replace(/\/+$/, ''); +} + +/** No retries, and short enough that a hanging upstream never leaves the caller waiting indefinitely - + * this only ever runs after a local miss, on an opt-in toggle. */ +const FALLBACK_TIMEOUT_MS = 30_000; + +/** RFC 7230's hop-by-hop set, plus `content-encoding`/`content-length`: the body handed to `fetch()` + * already arrives decoded, and Express recomputes framing itself when `res.send()` writes the buffered + * relayed body, so forwarding either would describe bytes that are no longer on the wire. */ +const EXCLUDED_RESPONSE_HEADERS = new Set([ + 'connection', + 'keep-alive', + 'proxy-authenticate', + 'proxy-authorization', + 'te', + 'trailer', + 'transfer-encoding', + 'upgrade', + 'content-encoding', + 'content-length', +]); + +export type FallbackTrigger = 'unimplemented' | 'record-not-found'; + +/** The exhaustive mapping from a local error's `type` to the toggle that gates it (`api.md`). Every + * other error `type` - `invalid-request`, `user-not-authenticated`, `cannot-remove-running-run`, + * `deleting-unfinished-build`, any `dev-folder-*` type, `internal-error` - is never eligible, `null`. */ +function triggerForErrorType(type: string): FallbackTrigger | null { + if (type === 'not-found' || type === 'not-implemented') return 'unimplemented'; + if (type === 'record-not-found') return 'record-not-found'; + return null; +} + +/** `/v2/*` only, and never this runtime's own non-Apify `/v2/actor-runtime/*` namespace (nothing + * upstream to call for either exclusion - a request that never reached `/v2` at all was never + * authenticated on this path either, see `server.ts`'s mount order). Read off `req.originalUrl` (never + * `req.path`), since that is the one representation router mount-prefix-stripping never touches. */ +function isEligibleUpstreamPath(originalUrl: string): boolean { + const pathname = originalUrl.split('?')[0] ?? originalUrl; + if (pathname === '/v2/actor-runtime' || pathname.startsWith('/v2/actor-runtime/')) return false; + return pathname === '/v2' || pathname.startsWith('/v2/'); +} + +export interface LocalError { + status: number; + type: string; + message: string; +} + +/** + * The one function both local-miss seams in `server.ts` call. Returns `true` when the response has been + * fully sent from upstream (the caller must not also send the local error), `false` when this call was + * never eligible, or eligible but abandoned - either way the original local error is still the caller's + * to send. + * + * Eligibility, in order: the local error's `type` must map to a trigger (above) and that trigger's + * toggle must be on; the request must be under `/v2/*`, excluding `/v2/actor-runtime/*`; the request + * must be authenticated (`req.user` - every `/v2/*` request, off-spec paths included, passes `auth()` + * before reaching either seam, so this only ever fails for a request this runtime never authenticated at + * all, e.g. one outside `/v2` entirely). All HTTP methods are eligible once these hold, writes included. + * + * Replay is `` (byte-exact, percent-encoding intact), the caller's + * own presented token (`req.user.token`, unconditionally - see `services/users.ts`) as the only + * `Authorization` header, `content-type`/`accept` forwarded when the inbound request carried them, + * nothing else. One attempt, redirects followed, a 30s timeout. Only a final `2xx` is relayed verbatim, + * with both marker headers added; anything else - non-2xx, timeout, DNS/connect failure - is fail-closed + * (this function returns `false`, changing nothing about the response), logged at `warn`. A relay is + * logged at `log`. + */ +export async function attemptFallback(req: Request, res: Response, localError: LocalError): Promise { + if (res.headersSent) return false; + + const trigger = triggerForErrorType(localError.type); + if (!trigger) return false; + + const current = getApiFallbackState(); + const toggleOn = + trigger === 'unimplemented' ? current.fallbackUnimplementedEnabled : current.fallbackNotFoundEnabled; + if (!toggleOn) return false; + + if (!isEligibleUpstreamPath(req.originalUrl)) return false; + if (!req.user) return false; + + const method = req.method.toUpperCase(); + const headers: Record = { authorization: `Bearer ${req.user.token}` }; + const contentType = req.header('content-type'); + if (contentType) headers['content-type'] = contentType; + const accept = req.header('accept'); + if (accept) headers['accept'] = accept; + + const target = `${upstreamBaseUrl()}${req.originalUrl}`; + + let upstreamResponse: Awaited>; + try { + upstreamResponse = await fetch(target, { + method, + headers, + body: method === 'GET' || method === 'HEAD' ? undefined : rawBody(req), + redirect: 'follow', + signal: AbortSignal.timeout(FALLBACK_TIMEOUT_MS), + }); + } catch (err) { + console.warn( + `api-fallback: upstream request failed for ${method} ${req.originalUrl} (trigger=${trigger}): ${ + err instanceof Error ? err.message : String(err) + }`, + ); + return false; + } + + if (upstreamResponse.status < 200 || upstreamResponse.status >= 300) { + console.warn( + `api-fallback: upstream answered ${upstreamResponse.status} for ${method} ${req.originalUrl} ` + + `(trigger=${trigger}); returning the original local error instead`, + ); + return false; + } + + const bodyBuffer = Buffer.from(await upstreamResponse.arrayBuffer()); + res.status(upstreamResponse.status); + upstreamResponse.headers.forEach((value, name) => { + if (EXCLUDED_RESPONSE_HEADERS.has(name.toLowerCase())) return; + res.append(name, value); + }); + res.append('x-actor-runtime-fallback', upstreamBaseUrl()); + res.append('x-actor-runtime-fallback-trigger', trigger); + res.send(bodyBuffer); + + console.log(`api-fallback: relayed ${method} ${req.originalUrl} to ${upstreamBaseUrl()} (trigger=${trigger})`); + return true; +} diff --git a/test/integration/api-fallback.test.ts b/test/integration/api-fallback.test.ts new file mode 100644 index 0000000..98dafbd --- /dev/null +++ b/test/integration/api-fallback.test.ts @@ -0,0 +1,854 @@ +/** + * Covers the upstream API fallback (`api.md`'s "Upstream fallback" section, `services/api-fallback.ts`): + * the `GET`/`POST /actor-runtime/api-fallback` toggle-state endpoint, the eligibility mapping (both + * toggles, in isolation and together), the fail-closed guarantee, own-token-only forwarding, and the two + * marker headers - against a stubbed upstream, exactly the pattern + * `test/integration/identity-resolution.test.ts` established for the identity probe. Never real egress + * to `api.apify.com`. + * + * Console-side coverage (the `/settings` page, its form, and the nav indicator on every page) lives in + * `test/integration/settings-console.test.ts`. + */ +import { createServer } from 'node:http'; +import type { IncomingMessage, Server, ServerResponse } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import axios from 'axios'; + +import { startTestServer, type TestServerHandle } from './helpers/test-server.js'; +import { resetApiFallbackStateForTests, setApiFallbackState } from '../../src/services/api-fallback.js'; +import { getRegistries } from '../../src/storage/registries.js'; +import { generateId } from '../../src/storage/ids.js'; +import type { RunRecord } from '../../src/storage/entities.js'; +import type { Driver, DevFolderProbeOutcome } from '../../src/driver/types.js'; + +interface CapturedRequest { + method: string; + url: string; + headers: Record; + body: Buffer; +} + +interface StubUpstream { + baseUrl: string; + hitCount: () => number; + requests: () => CapturedRequest[]; + close: () => Promise; +} + +/** Stands in for `https://api.apify.com`, generically: `respond` decides the status/body/headers for + * every request; passing `'hang'` never calls back at all (simulating a stalled upstream past any + * timeout). Every hit is recorded (method/url/headers/body), so a test can assert what the runtime + * actually sent upstream, not just what it got back. */ +function startStubUpstream( + respond: (req: CapturedRequest) => { status: number; body?: unknown; headers?: Record } | 'hang', +): Promise { + const requests: CapturedRequest[] = []; + return new Promise((resolveServer) => { + const server: Server = createServer((req: IncomingMessage, res: ServerResponse) => { + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => chunks.push(chunk)); + req.on('end', () => { + const captured: CapturedRequest = { + method: req.method ?? '', + url: req.url ?? '', + headers: req.headers, + body: Buffer.concat(chunks), + }; + requests.push(captured); + const outcome = respond(captured); + if (outcome === 'hang') return; // never respond - the client's own timeout must fire + res.writeHead(outcome.status, { 'content-type': 'application/json', ...outcome.headers }); + res.end(outcome.body === undefined ? '' : JSON.stringify(outcome.body)); + }); + }); + server.listen(0, () => { + const { port } = server.address() as AddressInfo; + resolveServer({ + baseUrl: `http://127.0.0.1:${port}`, + hitCount: () => requests.length, + requests: () => requests, + close: () => new Promise((resolve) => server.close(() => resolve())), + }); + }); + }); +} + +/** Makes one authenticated request so `services/users.ts: getOrCreateUserForToken()`'s one-time + * identity probe for `token` runs and gets cached *now*, against whatever upstream is currently + * configured - before a test points `APIFY_UPSTREAM_API_BASE_URL` at its own fallback stub. Without + * this, the identity probe for a never-before-seen token would itself be the first request to reach + * that stub. */ +async function warmUpIdentity(baseUrl: string, token: string): Promise { + await axios.get(`${baseUrl}/v2/users/me`, { + headers: { Authorization: `Bearer ${token}` }, + validateStatus: () => true, + }); +} + +/** A fixed `2xx` JSON response with a distinguishing header, the "the caller receives exactly this" + * shape criteria 11-15 check for every successful-relay case. */ +function fixedOkResponse(distinguishingValue: string) { + return () => ({ + status: 200, + body: { fromUpstream: true, marker: distinguishingValue }, + headers: { 'x-stub-marker': distinguishingValue }, + }); +} + +describe('api-fallback: toggle-state endpoint', () => { + let server: TestServerHandle; + + beforeEach(async () => { + server = await startTestServer(); + }); + + afterEach(async () => { + resetApiFallbackStateForTests(); + await server.close(); + }); + + async function get(path: string, token: string | null = server.token) { + return axios.get(`${server.baseUrl}${path}`, { + headers: token ? { Authorization: `Bearer ${token}` } : {}, + validateStatus: () => true, + }); + } + + async function post(path: string, body: unknown, token: string | null = server.token) { + return axios.post(`${server.baseUrl}${path}`, body, { + headers: { + 'Content-Type': 'application/json', + ...(token ? { Authorization: `Bearer ${token}` } : {}), + }, + validateStatus: () => true, + }); + } + + it('both toggles are off by default, upstreamBaseUrl reported', async () => { + const res = await get('/actor-runtime/api-fallback'); + expect(res.status).toBe(200); + expect(res.data).toEqual({ + data: { + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: false, + upstreamBaseUrl: 'https://api.apify.com', + }, + }); + }); + + it('a restart (simulated: the in-memory state is reset) brings both toggles back to false regardless of how they were set', async () => { + await post('/actor-runtime/api-fallback', { + fallbackUnimplementedEnabled: true, + fallbackNotFoundEnabled: true, + }); + let res = await get('/actor-runtime/api-fallback'); + expect(res.data.data.fallbackUnimplementedEnabled).toBe(true); + expect(res.data.data.fallbackNotFoundEnabled).toBe(true); + + resetApiFallbackStateForTests(); + + res = await get('/actor-runtime/api-fallback'); + expect(res.data.data).toEqual({ + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: false, + upstreamBaseUrl: 'https://api.apify.com', + }); + + // Also true starting from only one toggle on. + await post('/actor-runtime/api-fallback', { fallbackNotFoundEnabled: true }); + resetApiFallbackStateForTests(); + res = await get('/actor-runtime/api-fallback'); + expect(res.data.data.fallbackNotFoundEnabled).toBe(false); + }); + + it('GET is reachable both at /actor-runtime/api-fallback and /v2/actor-runtime/api-fallback, identically', async () => { + const bothOff = await get('/actor-runtime/api-fallback'); + const bothOffAlias = await get('/v2/actor-runtime/api-fallback'); + expect(bothOffAlias.status).toBe(bothOff.status); + expect(bothOffAlias.data).toEqual(bothOff.data); + + await post('/actor-runtime/api-fallback', { + fallbackUnimplementedEnabled: true, + fallbackNotFoundEnabled: true, + }); + const bothOn = await get('/actor-runtime/api-fallback'); + const bothOnAlias = await get('/v2/actor-runtime/api-fallback'); + expect(bothOnAlias.status).toBe(bothOn.status); + expect(bothOnAlias.data).toEqual(bothOn.data); + }); + + it('a partial POST flips only the field it mentions, leaving the other untouched - matching the spec worked example verbatim', async () => { + const first = await post('/actor-runtime/api-fallback', { fallbackUnimplementedEnabled: true }); + expect(first.status).toBe(200); + expect(first.data).toEqual({ + data: { + fallbackUnimplementedEnabled: true, + fallbackNotFoundEnabled: false, + upstreamBaseUrl: 'https://api.apify.com', + }, + }); + + const second = await post('/actor-runtime/api-fallback', { fallbackNotFoundEnabled: true }); + expect(second.data.data).toEqual({ + fallbackUnimplementedEnabled: true, + fallbackNotFoundEnabled: true, + upstreamBaseUrl: 'https://api.apify.com', + }); + + const third = await post('/actor-runtime/api-fallback', { fallbackUnimplementedEnabled: false }); + expect(third.data.data).toEqual({ + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: true, + upstreamBaseUrl: 'https://api.apify.com', + }); + + const confirmed = await get('/actor-runtime/api-fallback'); + expect(confirmed.data.data).toEqual(third.data.data); + }); + + it('POST with no token is 401 user-not-authenticated and does not change state; also true through the /v2 alias', async () => { + const before = await get('/actor-runtime/api-fallback'); + + const res = await post('/actor-runtime/api-fallback', { fallbackUnimplementedEnabled: true }, null); + expect(res.status).toBe(401); + expect(res.data.error.type).toBe('user-not-authenticated'); + + const resAlias = await post('/v2/actor-runtime/api-fallback', { fallbackUnimplementedEnabled: true }, null); + expect(resAlias.status).toBe(401); + expect(resAlias.data.error.type).toBe('user-not-authenticated'); + + const after = await get('/actor-runtime/api-fallback'); + expect(after.data).toEqual(before.data); + }); + + const malformedBodies: Array<[string, unknown, boolean]> = [ + ['a non-JSON body', '{not json', false], + ['a JSON array', [true], true], + ['a JSON scalar (string)', 'true', true], + ['a JSON scalar (number)', 42, true], + ['a JSON null', null, true], + ['an empty object', {}, true], + ['an unknown key', { fallbackUnimplementedEnabled: true, typo: true }, true], + ['a non-boolean value (string)', { fallbackUnimplementedEnabled: 'true' }, true], + ['a non-boolean value (number)', { fallbackNotFoundEnabled: 1 }, true], + ['a non-boolean value (null)', { fallbackUnimplementedEnabled: null }, true], + ]; + + for (const [label, body, isJson] of malformedBodies) { + it(`rejects ${label} as 400 invalid-request, with no state change`, async () => { + const before = await get('/actor-runtime/api-fallback'); + + const res = isJson + ? await post('/actor-runtime/api-fallback', body) + : await axios.post(`${server.baseUrl}/actor-runtime/api-fallback`, body as string, { + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${server.token}` }, + validateStatus: () => true, + }); + expect(res.status).toBe(400); + expect(res.data.error.type).toBe('invalid-request'); + + const after = await get('/actor-runtime/api-fallback'); + expect(after.data).toEqual(before.data); + }); + } + + it('upstreamBaseUrl is read-only: an attacker-supplied value in the POST body never becomes the reported upstream', async () => { + const res = await post('/actor-runtime/api-fallback', { + fallbackUnimplementedEnabled: true, + upstreamBaseUrl: 'https://evil.example', + }); + // Either shape is acceptable per the spec (rejected as an unknown key, or ignored) - either way + // the reported upstreamBaseUrl must never be the attacker-supplied one. + if (res.status === 200) { + expect(res.data.data.upstreamBaseUrl).toBe('https://api.apify.com'); + } else { + expect(res.status).toBe(400); + } + + const after = await get('/actor-runtime/api-fallback'); + expect(after.data.data.upstreamBaseUrl).toBe('https://api.apify.com'); + }); +}); + +describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { + let server: TestServerHandle; + let previousUpstreamUrl: string | undefined; + + beforeEach(async () => { + server = await startTestServer(); + previousUpstreamUrl = process.env.APIFY_UPSTREAM_API_BASE_URL; + // Force the one-time identity probe (`services/identity-resolution.ts`) to happen now, against + // whichever upstream is configured *before* any test below points `APIFY_UPSTREAM_API_BASE_URL` + // at its own stub - otherwise that very probe would be the first request to land on a per-test + // stub, inflating its hit count and logging its own "using local identity" line into a spy meant + // to observe only `attemptFallback`'s own logging. + await warmUpIdentity(server.baseUrl, server.token); + }); + + afterEach(async () => { + resetApiFallbackStateForTests(); + if (previousUpstreamUrl === undefined) delete process.env.APIFY_UPSTREAM_API_BASE_URL; + else process.env.APIFY_UPSTREAM_API_BASE_URL = previousUpstreamUrl; + await server.close(); + }); + + async function call( + method: 'get' | 'post' | 'put' | 'delete', + path: string, + options: { body?: unknown; token?: string } = {}, + ) { + return axios.request({ + method, + url: `${server.baseUrl}${path}`, + data: options.body, + headers: { Authorization: `Bearer ${options.token ?? server.token}` }, + validateStatus: () => true, + }); + } + + /** Seeds and returns a non-terminal (`RUNNING`) run owned by the test's default token, so + * `DELETE /v2/actor-runs/:runId` throws `cannot-remove-running-run` - one of the "never forwards" + * conflict-style error types (criterion 18). */ + async function seedRunningRun(): Promise { + const actor = await server.client.actors().create({ name: `fallback-conflict-actor-${generateId()}` }); + const actorRecord = (await getRegistries().actors.get(actor.id))!; + const run: RunRecord = { + id: generateId(), + userId: actorRecord.userId, + actorId: actor.id, + buildId: generateId(), + buildNumber: '0.0.1', + status: 'RUNNING', + startedAt: new Date().toISOString(), + defaultDatasetId: generateId(), + defaultKeyValueStoreId: generateId(), + defaultRequestQueueId: generateId(), + options: { memoryMbytes: 1024, timeoutSecs: 300 }, + meta: { origin: 'API' }, + }; + await getRegistries().runs.set(run.id, run); + return run.id; + } + + describe('both toggles off: no behavior change, zero outbound traffic', () => { + it('an off-spec path, a spec-known 501 path, and a record-not-found id all answer exactly as before, and the stub is never hit', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-never-be-seen')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const offSpec = await call('get', '/v2/totally-made-up-path'); + expect(offSpec.status).toBe(404); + expect(offSpec.data.error.type).toBe('not-found'); + expect(offSpec.headers['x-actor-runtime-fallback']).toBeUndefined(); + + const notImplemented = await call('get', '/v2/schedules'); + expect(notImplemented.status).toBe(501); + expect(notImplemented.data.error.type).toBe('not-implemented'); + + const notFound = await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(notFound.status).toBe(404); + expect(notFound.data.error.type).toBe('record-not-found'); + + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + }); + + describe('fallbackUnimplementedEnabled alone (fallbackNotFoundEnabled off)', () => { + beforeEach(() => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: false }); + }); + + it('relays an off-spec path, with the trigger header "unimplemented"', async () => { + const stub = await startStubUpstream(fixedOkResponse('off-spec-marker')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('get', '/v2/totally-made-up-path'); + expect(res.status).toBe(200); + expect(res.data).toEqual({ fromUpstream: true, marker: 'off-spec-marker' }); + expect(res.headers['x-stub-marker']).toBe('off-spec-marker'); + expect(res.headers['x-actor-runtime-fallback']).toBe(stub.baseUrl); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBe('unimplemented'); + expect(stub.hitCount()).toBe(1); + } finally { + await stub.close(); + } + }); + + it('relays a spec-known 501 path, with the trigger header "unimplemented"', async () => { + const stub = await startStubUpstream(fixedOkResponse('schedules-marker')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('get', '/v2/schedules'); + expect(res.status).toBe(200); + expect(res.data).toEqual({ fromUpstream: true, marker: 'schedules-marker' }); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBe('unimplemented'); + expect(stub.hitCount()).toBe(1); + } finally { + await stub.close(); + } + }); + + it('does NOT relay a record-not-found case - the other toggle is off', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(res.status).toBe(404); + expect(res.data.error.type).toBe('record-not-found'); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + + it('relays a write method (POST) against an unbuilt endpoint family', async () => { + const stub = await startStubUpstream(fixedOkResponse('actor-tasks-post-marker')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('post', '/v2/actor-tasks', { body: { name: 'whatever' } }); + expect(res.status).toBe(200); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBe('unimplemented'); + expect(stub.hitCount()).toBe(1); + expect(stub.requests()[0]?.method).toBe('POST'); + } finally { + await stub.close(); + } + }); + }); + + describe('fallbackNotFoundEnabled alone (fallbackUnimplementedEnabled off)', () => { + beforeEach(() => { + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: true }); + }); + + it('relays a record-not-found case, with the trigger header "record-not-found"', async () => { + const stub = await startStubUpstream(fixedOkResponse('record-not-found-marker')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(res.status).toBe(200); + expect(res.data).toEqual({ fromUpstream: true, marker: 'record-not-found-marker' }); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBe('record-not-found'); + expect(stub.hitCount()).toBe(1); + } finally { + await stub.close(); + } + }); + + it('does NOT relay an off-spec path or a spec-known 501 path - the other toggle is off', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const offSpec = await call('get', '/v2/totally-made-up-path'); + expect(offSpec.status).toBe(404); + expect(offSpec.data.error.type).toBe('not-found'); + + const notImplemented = await call('get', '/v2/schedules'); + expect(notImplemented.status).toBe(501); + + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + + it('relays a write method (DELETE) against a missing record', async () => { + const stub = await startStubUpstream(fixedOkResponse('delete-marker')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('delete', '/v2/datasets/does-not-exist-for-delete'); + expect(res.status).toBe(200); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBe('record-not-found'); + expect(stub.hitCount()).toBe(1); + expect(stub.requests()[0]?.method).toBe('DELETE'); + } finally { + await stub.close(); + } + }); + }); + + describe('both toggles on', () => { + beforeEach(() => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + }); + + it('relays both trigger kinds in the same run, each with its own correct trigger header', async () => { + const stub = await startStubUpstream((req) => ({ + status: 200, + body: { sawUrl: req.url }, + headers: { 'x-stub-marker': 'both-on' }, + })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const unimplemented = await call('get', '/v2/schedules'); + expect(unimplemented.status).toBe(200); + expect(unimplemented.headers['x-actor-runtime-fallback-trigger']).toBe('unimplemented'); + + const notFound = await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(notFound.status).toBe(200); + expect(notFound.headers['x-actor-runtime-fallback-trigger']).toBe('record-not-found'); + + expect(stub.hitCount()).toBe(2); + } finally { + await stub.close(); + } + }); + + it('replays the byte-exact original URL (path + query, percent-encoding intact) to the upstream', async () => { + const stub = await startStubUpstream(fixedOkResponse('url-check')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + await call('get', '/v2/totally-made-up-path?q=a%23b%3Fc'); + expect(stub.requests()[0]?.url).toBe('/v2/totally-made-up-path?q=a%23b%3Fc'); + } finally { + await stub.close(); + } + }); + }); + + describe('fail-closed: upstream trouble never surfaces upstream detail', () => { + async function localBothOffResponse(method: 'get' | 'delete', path: string) { + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: false }); + const res = await call(method, path); + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + return res; + } + + it('upstream 404 -> original local error, unchanged, no marker headers', async () => { + const stub = await startStubUpstream(() => ({ status: 404, body: { error: 'upstream 404' } })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const baseline = await localBothOffResponse('get', '/v2/totally-made-up-path'); + const res = await call('get', '/v2/totally-made-up-path'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBeUndefined(); + expect(stub.hitCount()).toBe(1); + } finally { + await stub.close(); + } + }); + + it('upstream 500 -> original local error, unchanged', async () => { + const stub = await startStubUpstream(() => ({ status: 500, body: { error: 'upstream 500' } })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const baseline = await localBothOffResponse('get', '/v2/schedules'); + const res = await call('get', '/v2/schedules'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + } finally { + await stub.close(); + } + }); + + it('upstream non-not-found 4xx (401) -> original local error, unchanged', async () => { + const stub = await startStubUpstream(() => ({ status: 401, body: { error: 'upstream 401' } })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const baseline = await localBothOffResponse('get', '/v2/datasets/does-not-exist-at-all'); + const res = await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + } finally { + await stub.close(); + } + }); + + it('upstream non-not-found 4xx (409) -> original local error, unchanged', async () => { + const stub = await startStubUpstream(() => ({ status: 409, body: { error: 'upstream 409' } })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const baseline = await localBothOffResponse('get', '/v2/datasets/does-not-exist-at-all'); + const res = await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + } finally { + await stub.close(); + } + }); + + it('upstream unreachable (connection refused) -> original local error, unchanged', async () => { + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:1'; // nothing listens here + const baseline = await localBothOffResponse('get', '/v2/totally-made-up-path'); + const res = await call('get', '/v2/totally-made-up-path'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + }); + + it('upstream hangs past the timeout -> original local error, unchanged (slow: waits out the real timeout)', async () => { + const stub = await startStubUpstream(() => 'hang'); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const baseline = await localBothOffResponse('get', '/v2/totally-made-up-path'); + const res = await call('get', '/v2/totally-made-up-path'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + } finally { + await stub.close(); + } + }, 35_000); + }); + + describe('never forwards a token the caller did not present', () => { + beforeEach(() => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + }); + + it('the upstream sees exactly the caller-presented token, never a different one', async () => { + // Warm this distinct token's identity first (see `warmUpIdentity`'s doc comment) - otherwise + // its own one-time identity probe would be the first request the stub below sees. + await warmUpIdentity(server.baseUrl, 'the-exact-caller-token'); + + const stub = await startStubUpstream(fixedOkResponse('token-check')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + await call('get', '/v2/totally-made-up-path', { token: 'the-exact-caller-token' }); + expect(stub.hitCount()).toBe(1); + const auth = stub.requests()[0]?.headers.authorization; + expect(auth).toBe('Bearer the-exact-caller-token'); + } finally { + await stub.close(); + } + }); + + it('a request with no token at all never reaches the stub (rejected by auth() first)', async () => { + const stub = await startStubUpstream(fixedOkResponse('no-token-check')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await axios.get(`${server.baseUrl}/v2/totally-made-up-path`, { + validateStatus: () => true, + }); + expect(res.status).toBe(401); + expect(res.data.error.type).toBe('user-not-authenticated'); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + }); + + describe('local list endpoints never gain platform objects', () => { + beforeEach(() => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + }); + + it('GET /v2/datasets (a collection route) never hits the stub and never gains upstream items', async () => { + const stub = await startStubUpstream(() => ({ + status: 200, + body: { data: { items: [{ id: 'platform-only-dataset' }], total: 1, count: 1, offset: 0, limit: 20 } }, + })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('get', '/v2/datasets'); + expect(res.status).toBe(200); + expect(JSON.stringify(res.data)).not.toContain('platform-only-dataset'); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + + it('GET /v2/actors (a collection route) never hits the stub either', async () => { + const stub = await startStubUpstream(() => ({ status: 200, body: { data: { items: [] } } })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('get', '/v2/actors'); + expect(res.status).toBe(200); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + }); + + describe('every other local error type never forwards, under any toggle combination', () => { + beforeEach(() => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + }); + + it('400 invalid-request never forwards', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await axios.post(`${server.baseUrl}/v2/actors`, '{not valid json', { + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${server.token}` }, + validateStatus: () => true, + }); + expect(res.status).toBe(400); + expect(res.data.error.type).toBe('invalid-request'); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + + it('401 user-not-authenticated never forwards', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await axios.get(`${server.baseUrl}/v2/actors`, { validateStatus: () => true }); + expect(res.status).toBe(401); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + + it('a conflict-style error (cannot-remove-running-run) never forwards', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const runId = await seedRunningRun(); + const res = await call('delete', `/v2/actor-runs/${runId}`); + expect(res.status).toBe(400); + expect(res.data.error.type).toBe('cannot-remove-running-run'); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + + // dev-folder-* errors and internal-error are covered by the standalone describe block below + // (`api-fallback: dev-folder-* and internal-error types never forward`) - that test needs its own + // `startTestServer()` with a custom probing driver, which cannot coexist with this block's own + // `server` (the storage bootstrap is a process-wide singleton - only one `TestServerHandle` can be + // open at a time, see `storage/bootstrap.ts`). + }); + + describe('log lines: one console.log per relay, one console.warn per abandon', () => { + it('a relayed request logs exactly one console.log line and no console.warn', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: false }); + const stub = await startStubUpstream(fixedOkResponse('log-check')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + await call('get', '/v2/totally-made-up-path'); + expect(logSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).not.toHaveBeenCalled(); + } finally { + logSpy.mockRestore(); + warnSpy.mockRestore(); + await stub.close(); + } + }); + + it('an abandoned request (upstream 500) logs exactly one console.warn line and no console.log, for the record-not-found trigger too', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: true }); + const stub = await startStubUpstream(() => ({ status: 500 })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(String(warnSpy.mock.calls[0]?.[0])).toContain('500'); + expect(logSpy).not.toHaveBeenCalled(); + } finally { + logSpy.mockRestore(); + warnSpy.mockRestore(); + await stub.close(); + } + }); + + it('neither line appears while the relevant toggle is off', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: false }); + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + await call('get', '/v2/totally-made-up-path'); + expect(logSpy).not.toHaveBeenCalled(); + expect(warnSpy).not.toHaveBeenCalled(); + } finally { + logSpy.mockRestore(); + warnSpy.mockRestore(); + await stub.close(); + } + }); + }); +}); + +describe('api-fallback: dev-folder-* and internal-error types never forward', () => { + // A standalone describe block (its own `TestServerHandle`, not the shared `server` from the block + // above) because it needs a driver whose `probeDevFolder` outcome changes mid-test - and because the + // storage bootstrap this needs is a process-wide singleton (`storage/bootstrap.ts`), so it cannot run + // while another `startTestServer()`-created server is still open. + let server: TestServerHandle; + let previousUpstreamUrl: string | undefined; + let outcome: DevFolderProbeOutcome; + + const probingDriver: Driver = { + available: true, + async init() {}, + async startBuild() { + throw new Error('not used by this stub'); + }, + async abortBuild() {}, + async startRun() { + throw new Error('not used by this stub'); + }, + async abortRun() {}, + async reconcileOrphans() {}, + async probeDevFolder() { + return outcome; + }, + async ensureProbeImage() { + return 'stub-probe-image:test'; + }, + }; + + beforeEach(async () => { + outcome = { ok: false, reason: 'not-found' }; + server = await startTestServer(probingDriver); + previousUpstreamUrl = process.env.APIFY_UPSTREAM_API_BASE_URL; + await warmUpIdentity(server.baseUrl, server.token); + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + }); + + afterEach(async () => { + resetApiFallbackStateForTests(); + if (previousUpstreamUrl === undefined) delete process.env.APIFY_UPSTREAM_API_BASE_URL; + else process.env.APIFY_UPSTREAM_API_BASE_URL = previousUpstreamUrl; + await server.close(); + }); + + it("dev-folder-path-not-found (400) and internal-error (500), raised from the dev-folder route's recordNotFound-adjacent rejections, never forward - not because of their type alone, but also because /actor-runtime/* is never an eligible path", async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const actor = await server.client.actors().create({ name: `fallback-devfolder-actor-${generateId()}` }); + const post = (body: string) => + axios.post(`${server.baseUrl}/actor-runtime/dev-folder/${actor.id}`, body, { + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${server.token}` }, + validateStatus: () => true, + }); + + const notFoundRes = await post(JSON.stringify('/some/path')); + expect(notFoundRes.status).toBe(400); + expect(notFoundRes.data.error.type).toBe('dev-folder-path-not-found'); + expect(notFoundRes.headers['x-actor-runtime-fallback']).toBeUndefined(); + + outcome = { ok: false, reason: 'image-missing' }; + const internalErrorRes = await post(JSON.stringify('/some/other/path')); + expect(internalErrorRes.status).toBe(500); + expect(internalErrorRes.data.error.type).toBe('internal-error'); + expect(internalErrorRes.headers['x-actor-runtime-fallback']).toBeUndefined(); + + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); +}); diff --git a/test/integration/settings-console.test.ts b/test/integration/settings-console.test.ts new file mode 100644 index 0000000..aaf28ad --- /dev/null +++ b/test/integration/settings-console.test.ts @@ -0,0 +1,198 @@ +/** + * Console-side coverage for the upstream API fallback (`console.md`'s "Settings page" section): the + * `/settings` page itself, its two-checkbox form, and the state indicator that every other console page + * shows in its header nav. API-side coverage (the toggle endpoint, eligibility, relay, fail-closed) lives + * in `test/integration/api-fallback.test.ts`. + */ +import type { AddressInfo } from 'node:net'; +import type { Server } from 'node:http'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import axios from 'axios'; + +import { createConsoleServer } from '../../src/console/server.js'; +import { resetApiFallbackStateForTests, setApiFallbackState } from '../../src/services/api-fallback.js'; +import { startTestServer, type TestServerHandle } from './helpers/test-server.js'; + +describe('console: /settings page and the fallback nav indicator', () => { + let server: TestServerHandle; + let consoleServer: Server; + let consoleBaseUrl: string; + + beforeEach(async () => { + server = await startTestServer(); + const app = createConsoleServer({ driver: server.driver }); + consoleServer = await new Promise((resolve) => { + const s = app.listen(0, () => resolve(s)); + }); + consoleBaseUrl = `http://127.0.0.1:${(consoleServer.address() as AddressInfo).port}`; + }); + + afterEach(async () => { + resetApiFallbackStateForTests(); + await new Promise((resolve) => consoleServer.close(() => resolve())); + await server.close(); + }); + + it('GET /settings renders both toggles and upstreamBaseUrl, matching the API state, plus the warning line', async () => { + const res = await axios.get(`${consoleBaseUrl}/settings`); + expect(res.status).toBe(200); + expect(res.data).toContain('fallbackUnimplementedEnabled'); + expect(res.data).toContain('fallbackNotFoundEnabled'); + expect(res.data).toContain('upstreamBaseUrl'); + expect(res.data).toContain('https://api.apify.com'); + expect(res.data).toMatch(/forwards.*(Apify )?token/i); + }); + + it('flipping a toggle via the API and reloading /settings updates the rendered values with no restart', async () => { + const before = await axios.get(`${consoleBaseUrl}/settings`); + expect(before.data).toMatch(/
fallbackUnimplementedEnabled<\/dt>\s*
false<\/dd>/); + + setApiFallbackState({ fallbackUnimplementedEnabled: true }); + + const after = await axios.get(`${consoleBaseUrl}/settings`); + expect(after.data).toMatch(/
fallbackUnimplementedEnabled<\/dt>\s*
true<\/dd>/); + }); + + it('renders one form with two checkboxes and a single submit', async () => { + const res = await axios.get(`${consoleBaseUrl}/settings`); + expect(res.data).toContain('
'); + expect(res.data).toContain('name="fallbackUnimplementedEnabled"'); + expect(res.data).toContain('name="fallbackNotFoundEnabled"'); + expect((res.data.match(//g) ?? []).length).toBe(1); + }); + + it('unchecking only one checkbox and submitting turns that one off while leaving the other on - an absent box is read as false, not "unchanged"', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + + // Submits only `fallbackNotFoundEnabled=on` - the unimplemented checkbox is unchecked, so a real + // browser would simply omit it from the body. + const submit = await axios.post(`${consoleBaseUrl}/settings`, 'fallbackNotFoundEnabled=on', { + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + maxRedirects: 0, + validateStatus: () => true, + }); + expect(submit.status).toBeGreaterThanOrEqual(300); + expect(submit.status).toBeLessThan(400); + expect(submit.headers.location).toBe('/settings'); + + const stateRes = await axios.get(`${server.baseUrl}/actor-runtime/api-fallback`, { + headers: { Authorization: `Bearer ${server.token}` }, + }); + expect(stateRes.data.data).toEqual({ + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: true, + upstreamBaseUrl: 'https://api.apify.com', + }); + }); + + it('submitting both checkboxes checked turns both on, landing on the shared toggle state (not a console-local copy)', async () => { + const submit = await axios.post( + `${consoleBaseUrl}/settings`, + 'fallbackUnimplementedEnabled=on&fallbackNotFoundEnabled=on', + { + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + maxRedirects: 0, + validateStatus: () => true, + }, + ); + expect(submit.headers.location).toBe('/settings'); + + const stateRes = await axios.get(`${server.baseUrl}/v2/actor-runtime/api-fallback`, { + headers: { Authorization: `Bearer ${server.token}` }, + }); + expect(stateRes.data.data.fallbackUnimplementedEnabled).toBe(true); + expect(stateRes.data.data.fallbackNotFoundEnabled).toBe(true); + }); + + it('submitting with neither checkbox present turns both off', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + + const submit = await axios.post(`${consoleBaseUrl}/settings`, '', { + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + maxRedirects: 0, + validateStatus: () => true, + }); + expect(submit.headers.location).toBe('/settings'); + + const stateRes = await axios.get(`${server.baseUrl}/actor-runtime/api-fallback`, { + headers: { Authorization: `Bearer ${server.token}` }, + }); + expect(stateRes.data.data).toEqual({ + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: false, + upstreamBaseUrl: 'https://api.apify.com', + }); + }); + + describe('the nav indicator, on every page, for all four toggle combinations', () => { + const combinations: Array<[boolean, boolean]> = [ + [false, false], + [true, false], + [false, true], + [true, true], + ]; + + const pages = ['/actors', '/datasets', '/settings']; + + for (const [unimplementedEnabled, notFoundEnabled] of combinations) { + it(`renders "unimplemented: ${unimplementedEnabled ? 'on' : 'off'}, not-found: ${notFoundEnabled ? 'on' : 'off'}" on ${pages.join(', ')}`, async () => { + setApiFallbackState({ + fallbackUnimplementedEnabled: unimplementedEnabled, + fallbackNotFoundEnabled: notFoundEnabled, + }); + const expected = `Settings — fallback (unimplemented: ${unimplementedEnabled ? 'on' : 'off'}, not-found: ${notFoundEnabled ? 'on' : 'off'})`; + + for (const page of pages) { + const res = await axios.get(`${consoleBaseUrl}${page}`); + expect(res.status).toBe(200); + expect(res.data).toContain(expected); + } + }); + } + + it('the mixed states render distinctly from both-on and both-off (no collapse to a single on/off word)', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: false }); + const mixedA = (await axios.get(`${consoleBaseUrl}/actors`)).data as string; + + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: true }); + const mixedB = (await axios.get(`${consoleBaseUrl}/actors`)).data as string; + + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + const bothOn = (await axios.get(`${consoleBaseUrl}/actors`)).data as string; + + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: false }); + const bothOff = (await axios.get(`${consoleBaseUrl}/actors`)).data as string; + + expect(mixedA).toContain('Settings — fallback (unimplemented: on, not-found: off)'); + expect(mixedB).toContain('Settings — fallback (unimplemented: off, not-found: on)'); + expect(bothOn).toContain('Settings — fallback (unimplemented: on, not-found: on)'); + expect(bothOff).toContain('Settings — fallback (unimplemented: off, not-found: off)'); + + const distinct = new Set([mixedA, mixedB, bothOn, bothOff]); + expect(distinct.size).toBe(4); + }); + + it('the trailing parenthesized segment is a link to /settings', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + const res = await axios.get(`${consoleBaseUrl}/actors`); + expect(res.data).toContain( + 'Settings — fallback (unimplemented: on, not-found: on)', + ); + }); + + it('toggling and reloading each page updates the indicator on all of them, with no restart', async () => { + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: false }); + for (const page of pages) { + const res = await axios.get(`${consoleBaseUrl}${page}`); + expect(res.data).toContain('Settings — fallback (unimplemented: off, not-found: off)'); + } + + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + for (const page of pages) { + const res = await axios.get(`${consoleBaseUrl}${page}`); + expect(res.data).toContain('Settings — fallback (unimplemented: on, not-found: on)'); + } + }); + }); +}); diff --git a/test/unit/api-fallback-state.test.ts b/test/unit/api-fallback-state.test.ts new file mode 100644 index 0000000..9638195 --- /dev/null +++ b/test/unit/api-fallback-state.test.ts @@ -0,0 +1,87 @@ +/** + * Pure-function/state coverage for `services/api-fallback.ts` that needs no HTTP server at all: the + * default state, the merge-in setter's partiality, the test-reset helper, and `upstreamBaseUrl()`'s + * env-var override and trailing-slash trim. The eligibility mapping and replay/relay behaviour + * (`attemptFallback`) need a running server and a stub upstream - covered by + * `test/integration/api-fallback.test.ts`. + */ +import { afterEach, describe, expect, it } from 'vitest'; + +import { + getApiFallbackState, + resetApiFallbackStateForTests, + setApiFallbackState, + upstreamBaseUrl, +} from '../../src/services/api-fallback.js'; + +describe('api-fallback state', () => { + afterEach(() => { + resetApiFallbackStateForTests(); + delete process.env.APIFY_UPSTREAM_API_BASE_URL; + }); + + it('both toggles default to false', () => { + expect(getApiFallbackState()).toEqual({ + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: false, + }); + }); + + it('setApiFallbackState merges a partial patch, leaving the other field untouched', () => { + setApiFallbackState({ fallbackUnimplementedEnabled: true }); + expect(getApiFallbackState()).toEqual({ + fallbackUnimplementedEnabled: true, + fallbackNotFoundEnabled: false, + }); + + setApiFallbackState({ fallbackNotFoundEnabled: true }); + expect(getApiFallbackState()).toEqual({ + fallbackUnimplementedEnabled: true, + fallbackNotFoundEnabled: true, + }); + + setApiFallbackState({ fallbackUnimplementedEnabled: false }); + expect(getApiFallbackState()).toEqual({ + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: true, + }); + }); + + it('setApiFallbackState returns the merged state', () => { + const result = setApiFallbackState({ fallbackUnimplementedEnabled: true }); + expect(result).toEqual({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: false }); + }); + + it('getApiFallbackState returns a fresh copy each call, not a live reference', () => { + const first = getApiFallbackState(); + first.fallbackUnimplementedEnabled = true; + expect(getApiFallbackState().fallbackUnimplementedEnabled).toBe(false); + }); + + it('resetApiFallbackStateForTests restores both toggles to false regardless of how they were set', () => { + setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); + resetApiFallbackStateForTests(); + expect(getApiFallbackState()).toEqual({ + fallbackUnimplementedEnabled: false, + fallbackNotFoundEnabled: false, + }); + }); + + it('upstreamBaseUrl defaults to the real Apify platform when no env var is set', () => { + delete process.env.APIFY_UPSTREAM_API_BASE_URL; + expect(upstreamBaseUrl()).toBe('https://api.apify.com'); + }); + + it('upstreamBaseUrl reflects APIFY_UPSTREAM_API_BASE_URL when set', () => { + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:9999'; + expect(upstreamBaseUrl()).toBe('http://127.0.0.1:9999'); + }); + + it('upstreamBaseUrl trims a trailing slash (or several) so replay never doubles it', () => { + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:9999/'; + expect(upstreamBaseUrl()).toBe('http://127.0.0.1:9999'); + + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:9999///'; + expect(upstreamBaseUrl()).toBe('http://127.0.0.1:9999'); + }); +}); From b37a13d283bf2fe8b0690824ba43e37c886ed1b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Proch=C3=A1zka?= Date: Thu, 20 Aug 2026 13:54:24 +0000 Subject: [PATCH 02/10] Address review findings on the upstream fallback Relay each Set-Cookie the platform set as its own header line, since the HTTP client only guarantees separate entries for that one name and a cookie value may itself contain a comma. Other repeated header names are relayed comma-joined; the requirements now state that contract rather than promising byte-level preservation of repeated lines. Drop the duplicate upstream-base-URL helper in favour of the existing one, which now trims trailing slashes, and stop the fallback service reaching into the api layer for a two-line buffer coercion. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- requirements/api.md | 14 ++++-- src/api/routes/api-fallback.ts | 10 ++--- src/console/server.ts | 5 ++- src/services/api-fallback.ts | 63 ++++++++++++++++---------- src/services/identity-resolution.ts | 7 ++- test/integration/api-fallback.test.ts | 64 ++++++++++++++++++++++++++- test/unit/api-fallback-state.test.ts | 25 ++++++----- 7 files changed, 136 insertions(+), 52 deletions(-) diff --git a/requirements/api.md b/requirements/api.md index 3ab2518..4179e5a 100644 --- a/requirements/api.md +++ b/requirements/api.md @@ -186,10 +186,16 @@ otherwise 404/501 locally is relayed exactly like a `GET` when its toggle is on - and, if the platform accepts it, becomes a real write against the caller's real account. This is a deliberate consequence of opting in, not an oversight. -- **What a successful relay looks like**: the platform's response is returned to the caller verbatim - - status, body, and headers - with two markers added: `x-actor-runtime-fallback: ` (which - platform served it) and `x-actor-runtime-fallback-trigger: unimplemented` or `record-not-found` (which - toggle let it through). Only a final `2xx` counts as successful. +- **What a successful relay looks like**: the platform's response status and body are returned to the + caller unchanged. Response headers are relayed too, minus the standard hop-by-hop set + (`Connection`, `Keep-Alive`, `Transfer-Encoding`, ...) plus `Content-Encoding`/`Content-Length` (the + relayed body is re-framed, not streamed through byte-for-byte). `Set-Cookie` is always relayed as one + header line per cookie the platform set - never merged into one, since a cookie's own value can contain + a comma. Any other header name the platform repeats is relayed as a single, comma-joined value (the + standard representation for a repeated header field), not as separate repeated lines. Two markers are + added: `x-actor-runtime-fallback: ` (which platform served it) and + `x-actor-runtime-fallback-trigger: unimplemented` or `record-not-found` (which toggle let it through). + Only a final `2xx` counts as successful. - **Fail-closed guarantee**: anything else - a non-`2xx` response, a timeout, or the platform being unreachable - reproduces the exact response the caller would have gotten with both toggles off: the original local error, unchanged, with neither marker header present. The platform's own status or body diff --git a/src/api/routes/api-fallback.ts b/src/api/routes/api-fallback.ts index 0a8f039..f774fda 100644 --- a/src/api/routes/api-fallback.ts +++ b/src/api/routes/api-fallback.ts @@ -15,17 +15,13 @@ import type { Router } from 'express'; import { sendData } from '../envelope.js'; import { invalidRequest } from '../errors.js'; import { h, jsonBody } from '../handler.js'; -import { - getApiFallbackState, - setApiFallbackState, - upstreamBaseUrl, - type ApiFallbackState, -} from '../../services/api-fallback.js'; +import { getApiFallbackState, setApiFallbackState, type ApiFallbackState } from '../../services/api-fallback.js'; +import { upstreamApiBaseUrl } from '../../services/identity-resolution.js'; const SETTABLE_FIELDS = new Set(['fallbackUnimplementedEnabled', 'fallbackNotFoundEnabled']); function respondWithState(): { data: ApiFallbackState & { upstreamBaseUrl: string } } { - return { data: { ...getApiFallbackState(), upstreamBaseUrl: upstreamBaseUrl() } }; + return { data: { ...getApiFallbackState(), upstreamBaseUrl: upstreamApiBaseUrl() } }; } /** Parses and validates a `POST` body into a `setApiFallbackState` patch, throwing `invalid-request` for diff --git a/src/console/server.ts b/src/console/server.ts index 23e415e..0a790b3 100644 --- a/src/console/server.ts +++ b/src/console/server.ts @@ -43,7 +43,8 @@ import { table, type LinkedCell, } from './templates.js'; -import { getApiFallbackState, setApiFallbackState, upstreamBaseUrl } from '../services/api-fallback.js'; +import { getApiFallbackState, setApiFallbackState } from '../services/api-fallback.js'; +import { upstreamApiBaseUrl } from '../services/identity-resolution.js'; import type { Driver } from '../driver/types.js'; /** A run's default-storage id rendered as a link to that storage's detail view instead of plain text. */ @@ -416,7 +417,7 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { definitionList([ ['fallbackUnimplementedEnabled', state.fallbackUnimplementedEnabled], ['fallbackNotFoundEnabled', state.fallbackNotFoundEnabled], - ['upstreamBaseUrl', upstreamBaseUrl()], + ['upstreamBaseUrl', upstreamApiBaseUrl()], ]) + '

Change settings

' + settingsForm(state); diff --git a/src/services/api-fallback.ts b/src/services/api-fallback.ts index 415c433..3665995 100644 --- a/src/services/api-fallback.ts +++ b/src/services/api-fallback.ts @@ -1,10 +1,15 @@ /** * Upstream API fallback (`api.md`'s "Upstream fallback" section): when a call locally misses - either * because nothing in this runtime serves the path/method at all, or because it does but the specific - * record id doesn't exist - and the matching toggle is on, the request is replayed verbatim against the - * real Apify platform instead of failing. Both toggles default off and reset on every restart; this - * module is the only place either fact is read or written, by the API route (`api/routes/api-fallback.ts`), - * the console's `/settings` page, and `console/templates.ts: layout()`'s per-page state indicator alike. + * record id doesn't exist - and the matching toggle is on, the request is replayed against the real + * Apify platform instead of failing, and a successful reply's status/body/headers are relayed back + * (see the response-header note on `attemptFallback` below - not every repeated header line survives + * relay byte-for-byte, because the HTTP client this module uses does not hand back every repeated + * upstream header as separate entries; `Set-Cookie` is the one name it always keeps separate, and is + * the one name this module always preserves as separate lines for exactly that reason). Both toggles + * default off and reset on every restart; this module is the only place either fact is read or written, + * by the API route (`api/routes/api-fallback.ts`), the console's `/settings` page, and + * `console/templates.ts: layout()`'s per-page state indicator alike. * * `attemptFallback` is the single seam both of `server.ts`'s local-miss sites (the terminal catch-all * and the generic error middleware) call through - it alone knows the eligibility mapping below, the @@ -12,7 +17,7 @@ */ import type { Request, Response } from 'express'; -import { rawBody } from '../api/handler.js'; +import { upstreamApiBaseUrl } from './identity-resolution.js'; export interface ApiFallbackState { fallbackUnimplementedEnabled: boolean; @@ -43,14 +48,6 @@ export function resetApiFallbackStateForTests(): void { state = defaultState(); } -/** Same env var `services/identity-resolution.ts` already established for the identity probe - reused - * verbatim rather than inventing a second one. Trailing slashes trimmed so ` - * ` never produces a doubled `//`. */ -export function upstreamBaseUrl(): string { - const configured = process.env.APIFY_UPSTREAM_API_BASE_URL ?? 'https://api.apify.com'; - return configured.replace(/\/+$/, ''); -} - /** No retries, and short enough that a hanging upstream never leaves the caller waiting indefinitely - * this only ever runs after a local miss, on an opt-in toggle. */ const FALLBACK_TIMEOUT_MS = 30_000; @@ -110,13 +107,20 @@ export interface LocalError { * before reaching either seam, so this only ever fails for a request this runtime never authenticated at * all, e.g. one outside `/v2` entirely). All HTTP methods are eligible once these hold, writes included. * - * Replay is `` (byte-exact, percent-encoding intact), the caller's - * own presented token (`req.user.token`, unconditionally - see `services/users.ts`) as the only + * Replay is `` (byte-exact, percent-encoding intact), the + * caller's own presented token (`req.user.token`, unconditionally - see `services/users.ts`) as the only * `Authorization` header, `content-type`/`accept` forwarded when the inbound request carried them, - * nothing else. One attempt, redirects followed, a 30s timeout. Only a final `2xx` is relayed verbatim, - * with both marker headers added; anything else - non-2xx, timeout, DNS/connect failure - is fail-closed - * (this function returns `false`, changing nothing about the response), logged at `warn`. A relay is - * logged at `log`. + * nothing else. One attempt, redirects followed, a 30s timeout. Only a final `2xx` is relayed: status + * and body unchanged, headers minus the hop-by-hop exclusion set below. `Set-Cookie` is always relayed + * as one line per cookie the platform set, via `Headers.getSetCookie()` - the one header name the + * platform's HTTP client (`fetch`/undici) guarantees it can hand back as separate entries, which is also + * the one name where comma-joining would be wrong (a cookie's own value can contain a comma). Any other + * header the platform repeats is relayed as a single, comma-joined value - the RFC 7230-legitimate + * representation for a repeated list-valued field, and the only representation `fetch`'s `Headers` + * exposes for anything other than `Set-Cookie` (it joins repeated non-cookie header lines together + * before this function ever sees them). Anything other than a final `2xx` - non-2xx, timeout, DNS/connect + * failure - is fail-closed (this function returns `false`, changing nothing about the response), logged + * at `warn`. A relay is logged at `log`. */ export async function attemptFallback(req: Request, res: Response, localError: LocalError): Promise { if (res.headersSent) return false; @@ -139,14 +143,18 @@ export async function attemptFallback(req: Request, res: Response, localError: L const accept = req.header('accept'); if (accept) headers['accept'] = accept; - const target = `${upstreamBaseUrl()}${req.originalUrl}`; + const target = `${upstreamApiBaseUrl()}${req.originalUrl}`; + // Every body arrives as a raw `Buffer` (`api/server.ts`'s `express.raw({ type: () => true })`), with + // no other type ever assigned to `req.body` - same one-line coercion `api/routes/key-value-stores.ts` + // inlines at its own call site, kept local here rather than imported from the API layer. + const requestBody = Buffer.isBuffer(req.body) ? req.body : Buffer.alloc(0); let upstreamResponse: Awaited>; try { upstreamResponse = await fetch(target, { method, headers, - body: method === 'GET' || method === 'HEAD' ? undefined : rawBody(req), + body: method === 'GET' || method === 'HEAD' ? undefined : requestBody, redirect: 'follow', signal: AbortSignal.timeout(FALLBACK_TIMEOUT_MS), }); @@ -170,13 +178,20 @@ export async function attemptFallback(req: Request, res: Response, localError: L const bodyBuffer = Buffer.from(await upstreamResponse.arrayBuffer()); res.status(upstreamResponse.status); upstreamResponse.headers.forEach((value, name) => { - if (EXCLUDED_RESPONSE_HEADERS.has(name.toLowerCase())) return; + const lower = name.toLowerCase(); + // `set-cookie` is handled separately below, via `getSetCookie()` - skipped here so it is never + // also appended from this generic loop, which would double every cookie the platform set. + if (lower === 'set-cookie') return; + if (EXCLUDED_RESPONSE_HEADERS.has(lower)) return; res.append(name, value); }); - res.append('x-actor-runtime-fallback', upstreamBaseUrl()); + for (const cookie of upstreamResponse.headers.getSetCookie()) { + res.append('set-cookie', cookie); + } + res.append('x-actor-runtime-fallback', upstreamApiBaseUrl()); res.append('x-actor-runtime-fallback-trigger', trigger); res.send(bodyBuffer); - console.log(`api-fallback: relayed ${method} ${req.originalUrl} to ${upstreamBaseUrl()} (trigger=${trigger})`); + console.log(`api-fallback: relayed ${method} ${req.originalUrl} to ${upstreamApiBaseUrl()} (trigger=${trigger})`); return true; } diff --git a/src/services/identity-resolution.ts b/src/services/identity-resolution.ts index 005feaf..e184272 100644 --- a/src/services/identity-resolution.ts +++ b/src/services/identity-resolution.ts @@ -8,9 +8,12 @@ /** Overridable for tests (a tiny local stub server) and for pointing at a non-production platform; * defaults to the real Apify API. Read fresh on every call, not frozen at import time, so a test can - * flip it between cases within the same file/process (see `identity-resolution.test.ts`). */ + * flip it between cases within the same file/process (see `identity-resolution.test.ts`). Trailing + * slashes are trimmed so a caller concatenating a leading-`/` path (as `services/api-fallback.ts`'s + * replay does, and as this module's own `/v2/users/me` probe below does) never produces a doubled `//`. */ export function upstreamApiBaseUrl(): string { - return process.env.APIFY_UPSTREAM_API_BASE_URL ?? 'https://api.apify.com'; + const configured = process.env.APIFY_UPSTREAM_API_BASE_URL ?? 'https://api.apify.com'; + return configured.replace(/\/+$/, ''); } /** Short and non-retried on purpose - this runs lazily, on the first request for a given token, and diff --git a/test/integration/api-fallback.test.ts b/test/integration/api-fallback.test.ts index 98dafbd..8514cff 100644 --- a/test/integration/api-fallback.test.ts +++ b/test/integration/api-fallback.test.ts @@ -39,9 +39,14 @@ interface StubUpstream { /** Stands in for `https://api.apify.com`, generically: `respond` decides the status/body/headers for * every request; passing `'hang'` never calls back at all (simulating a stalled upstream past any * timeout). Every hit is recorded (method/url/headers/body), so a test can assert what the runtime - * actually sent upstream, not just what it got back. */ + * actually sent upstream, not just what it got back. A header value may be a `string[]` (not just a + * `string`) so a test can make the stub send the same header name as two separate raw wire lines - e.g. + * two `Set-Cookie` lines - rather than one value; `http.ServerResponse.writeHead` sends an array value as + * repeated lines for any header name, not only `set-cookie`. */ function startStubUpstream( - respond: (req: CapturedRequest) => { status: number; body?: unknown; headers?: Record } | 'hang', + respond: ( + req: CapturedRequest, + ) => { status: number; body?: unknown; headers?: Record } | 'hang', ): Promise { const requests: CapturedRequest[] = []; return new Promise((resolveServer) => { @@ -405,6 +410,26 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { } }); + it('does NOT relay a genuine local miss inside /v2/actor-runtime/* itself, even though the toggle is on and the error type otherwise maps to "unimplemented" - the path exclusion, not the error type, is what blocks this', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + // No route inside the `actor-runtime` sub-router matches this sub-path, so it falls through + // to the terminal catch-all exactly like a genuine off-spec `/v2/*` path would - the same + // local error shape (`not-found`) that the sibling test above confirms *does* relay when + // it's outside `/v2/actor-runtime/*`. Here it must not, because `isEligibleUpstreamPath` + // excludes this namespace regardless of the error's type. + const res = await call('get', '/v2/actor-runtime/does-not-exist-at-all'); + expect(res.status).toBe(404); + expect(res.data.error.type).toBe('not-found'); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBeUndefined(); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + it('relays a write method (POST) against an unbuilt endpoint family', async () => { const stub = await startStubUpstream(fixedOkResponse('actor-tasks-post-marker')); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; @@ -508,6 +533,41 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { await stub.close(); } }); + + // The response-header relay contract, pinned in both directions rather than left incidental + // (`api.md`'s "What a successful relay looks like"): `Set-Cookie` is the one repeated header name + // this runtime's HTTP client can hand back as genuinely separate entries, and the one name where + // comma-joining would corrupt the value (a cookie's own attributes routinely contain a comma, e.g. + // `Expires=Wed, 21 Oct 2026 07:28:00 GMT`) - so it is always relayed as one line per cookie. Every + // other repeated header name is relayed as a single, comma-joined value, because that is the only + // representation the client's `Headers` object exposes for a non-`Set-Cookie` repeat; this is the + // RFC 7230-legitimate form for a repeated list-valued field, not a loss of information the caller + // needs restored. + it('relays a repeated Set-Cookie header as separate lines, one per cookie, and a repeated non-cookie header as a single comma-joined value', async () => { + const stub = await startStubUpstream(() => ({ + status: 200, + body: { ok: true }, + headers: { + 'set-cookie': ['session=abc123; Path=/', 'theme=dark; Path=/'], + 'x-multi': ['a', 'b'], + }, + })); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const res = await call('get', '/v2/totally-made-up-path'); + expect(res.status).toBe(200); + + // axios/Node's http client itself always exposes repeated `set-cookie` response lines as an + // array (the same special-casing this runtime's own relay code relies on for the inbound + // leg) - so an array of exactly the two original cookies, in order, confirms both lines + // reached the caller separately, not merged into one. + expect(res.headers['set-cookie']).toEqual(['session=abc123; Path=/', 'theme=dark; Path=/']); + // The documented contract for any other repeated header name: one value, comma-joined. + expect(res.headers['x-multi']).toBe('a, b'); + } finally { + await stub.close(); + } + }); }); describe('fail-closed: upstream trouble never surfaces upstream detail', () => { diff --git a/test/unit/api-fallback-state.test.ts b/test/unit/api-fallback-state.test.ts index 9638195..c872b66 100644 --- a/test/unit/api-fallback-state.test.ts +++ b/test/unit/api-fallback-state.test.ts @@ -1,8 +1,11 @@ /** * Pure-function/state coverage for `services/api-fallback.ts` that needs no HTTP server at all: the - * default state, the merge-in setter's partiality, the test-reset helper, and `upstreamBaseUrl()`'s - * env-var override and trailing-slash trim. The eligibility mapping and replay/relay behaviour - * (`attemptFallback`) need a running server and a stub upstream - covered by + * default state, the merge-in setter's partiality, and the test-reset helper. `upstreamApiBaseUrl()`'s + * env-var override and trailing-slash trim are covered here too, even though the function itself lives + * in `services/identity-resolution.ts` (the fallback module reuses it rather than defining its own) - + * this is the fallback-relevant behavior (`` never doubling a + * `//`), so it stays exercised alongside the rest of this module's state. The eligibility mapping and + * replay/relay behaviour (`attemptFallback`) need a running server and a stub upstream - covered by * `test/integration/api-fallback.test.ts`. */ import { afterEach, describe, expect, it } from 'vitest'; @@ -11,8 +14,8 @@ import { getApiFallbackState, resetApiFallbackStateForTests, setApiFallbackState, - upstreamBaseUrl, } from '../../src/services/api-fallback.js'; +import { upstreamApiBaseUrl } from '../../src/services/identity-resolution.js'; describe('api-fallback state', () => { afterEach(() => { @@ -67,21 +70,21 @@ describe('api-fallback state', () => { }); }); - it('upstreamBaseUrl defaults to the real Apify platform when no env var is set', () => { + it('upstreamApiBaseUrl defaults to the real Apify platform when no env var is set', () => { delete process.env.APIFY_UPSTREAM_API_BASE_URL; - expect(upstreamBaseUrl()).toBe('https://api.apify.com'); + expect(upstreamApiBaseUrl()).toBe('https://api.apify.com'); }); - it('upstreamBaseUrl reflects APIFY_UPSTREAM_API_BASE_URL when set', () => { + it('upstreamApiBaseUrl reflects APIFY_UPSTREAM_API_BASE_URL when set', () => { process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:9999'; - expect(upstreamBaseUrl()).toBe('http://127.0.0.1:9999'); + expect(upstreamApiBaseUrl()).toBe('http://127.0.0.1:9999'); }); - it('upstreamBaseUrl trims a trailing slash (or several) so replay never doubles it', () => { + it('upstreamApiBaseUrl trims a trailing slash (or several) so replay never doubles it', () => { process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:9999/'; - expect(upstreamBaseUrl()).toBe('http://127.0.0.1:9999'); + expect(upstreamApiBaseUrl()).toBe('http://127.0.0.1:9999'); process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:9999///'; - expect(upstreamBaseUrl()).toBe('http://127.0.0.1:9999'); + expect(upstreamApiBaseUrl()).toBe('http://127.0.0.1:9999'); }); }); From 02860df6180e61687e1ccaa3e8829998352bd1a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Proch=C3=A1zka?= Date: Thu, 20 Aug 2026 14:51:48 +0000 Subject: [PATCH 03/10] Keep the fallback fail-closed when the platform fails mid-response An upstream that sent its status line and headers and then died left the relay throwing after the local error was already decided: one seam answered 500 instead of the original error, the other let the rejection reach Express's final handler, which rendered a stack trace as HTML. The relay now absorbs any failure and reports that it did not relay, and both seams answer through one helper. Also match the path exclusion to how Express routes, so a differently-cased or double-slashed spelling of the runtime's own namespace can no longer forward a token upstream; stop the new tests reaching the real platform during identity warm-up; make the relay timeout injectable so the hang test no longer costs 30 seconds; and reject cross-site form submissions to the console's writes. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- CLAUDE.MD | 3 +- requirements/cli.md | 4 +- requirements/console.md | 5 + requirements/system.md | 4 +- src/api/routes/api-fallback.ts | 8 +- src/api/server.ts | 21 ++- src/console/server.ts | 35 +++- src/services/api-fallback.ts | 103 ++++++++--- test/integration/api-fallback.test.ts | 202 ++++++++++++++++++++-- test/integration/dev-folder.test.ts | 37 +++- test/integration/helpers/test-server.ts | 2 + test/integration/settings-console.test.ts | 65 ++++++- 12 files changed, 424 insertions(+), 65 deletions(-) diff --git a/CLAUDE.MD b/CLAUDE.MD index a0aa4e8..5f54c7b 100644 --- a/CLAUDE.MD +++ b/CLAUDE.MD @@ -39,7 +39,8 @@ Local Actor runtime is an Actor development tool for developing, running, and de authenticated the failing call with to the real platform, and - since all HTTP methods are eligible - can turn a locally-missing `POST`/`PUT`/`DELETE` into a real write against your real account; only turn this on with a token/account you're comfortable with that. A relayed response carries an - `x-actor-runtime-fallback` header naming which platform served it. + `x-actor-runtime-fallback` header naming which platform served it, and an + `x-actor-runtime-fallback-trigger` header naming which toggle let it through. ## Through direct API calls diff --git a/requirements/cli.md b/requirements/cli.md index 020bc8e..4365078 100644 --- a/requirements/cli.md +++ b/requirements/cli.md @@ -67,7 +67,9 @@ `apify-cli` fetches the actor-templates manifest from the internet the first time it needs to create an Actor. Every push/call/log-stream/storage-access afterwards, and every build of an already-pulled base image, works with no outbound network access (see `system.md`'s offline-after- - first-build note). + first-build note) - unless the opt-in upstream API fallback (`api.md`'s "Upstream fallback" section) + is switched on, in which case any such call that misses locally and is eligible for fallback makes one + outbound request to the configured upstream instead of failing offline. - The bundled sample Actors crawl the live web (`https://crawlee.dev/` by default), so an `apify call` that runs one of them needs outbound network access from the Actor container even though the CLI-to-runtime interaction itself does not (see `system.md`). diff --git a/requirements/console.md b/requirements/console.md index ed066c9..554ec5a 100644 --- a/requirements/console.md +++ b/requirements/console.md @@ -11,6 +11,11 @@ (`storage.md`'s "Users" section). - The console is unauthenticated. Every route is a read except the dev-folder form below and the Settings form below, which are the console's only two writes - it is no longer strictly view-only. +- Both of those two writes reject a cross-site form submission (a browser sending `Sec-Fetch-Site` with + any value other than `same-origin`/`none`) with a plain `403`, so a page from another origin cannot + silently drive either write through a visitor's browser. A submission with no `Sec-Fetch-Site` header + at all (an older browser, or a non-browser caller) is unaffected - this narrows the console's existing + "anyone who can reach it" model by one specific vector, it does not add a login. - There are three types of objects: key-value store, dataset, request queue. - For each object type there must be exactly one widget for inspection. - The request-queue widget leads with the authoritative counts from `RequestQueue.getInfo()` diff --git a/requirements/system.md b/requirements/system.md index 2c62814..e302868 100644 --- a/requirements/system.md +++ b/requirements/system.md @@ -64,7 +64,9 @@ network access because the stock CLI fetches its actor-templates manifest from the internet. Once both of those have happened at least once, the runtime itself operates fully offline: repeat builds reuse the already-pulled base image and every other push/call/log-stream/storage-access needs no - outbound network access at all (see `cli.md`'s offline-capability note). + outbound network access at all (see `cli.md`'s offline-capability note) - unless the opt-in upstream + API fallback (`api.md`'s "Upstream fallback" section) has been switched on, in which case a local miss + it's eligible for makes one outbound request per such call. - The bundled sample Actors (`sample_actor_ts`, `sample_actor_py`) are not offline: they crawl a live site (`https://crawlee.dev/` by default). Running them needs outbound network access from the Actor container, unlike operating the runtime around them (see `test.md`). diff --git a/src/api/routes/api-fallback.ts b/src/api/routes/api-fallback.ts index f774fda..2acf6f6 100644 --- a/src/api/routes/api-fallback.ts +++ b/src/api/routes/api-fallback.ts @@ -20,8 +20,8 @@ import { upstreamApiBaseUrl } from '../../services/identity-resolution.js'; const SETTABLE_FIELDS = new Set(['fallbackUnimplementedEnabled', 'fallbackNotFoundEnabled']); -function respondWithState(): { data: ApiFallbackState & { upstreamBaseUrl: string } } { - return { data: { ...getApiFallbackState(), upstreamBaseUrl: upstreamApiBaseUrl() } }; +function respondWithState(): ApiFallbackState & { upstreamBaseUrl: string } { + return { ...getApiFallbackState(), upstreamBaseUrl: upstreamApiBaseUrl() }; } /** Parses and validates a `POST` body into a `setApiFallbackState` patch, throwing `invalid-request` for @@ -63,7 +63,7 @@ export function mountApiFallback(router: Router): void { router.get( '/api-fallback', h(async (_req, res) => { - sendData(res, respondWithState().data); + sendData(res, respondWithState()); }), ); @@ -72,7 +72,7 @@ export function mountApiFallback(router: Router): void { h(async (req, res) => { const patch = parsePatch(jsonBody(req)); setApiFallbackState(patch); - sendData(res, respondWithState().data); + sendData(res, respondWithState()); }), ); } diff --git a/src/api/server.ts b/src/api/server.ts index 5420830..15bea5b 100644 --- a/src/api/server.ts +++ b/src/api/server.ts @@ -15,13 +15,22 @@ import { mountLogs } from './routes/logs.js'; import { mountRunStorageAliases } from './routes/run-storage-aliases.js'; import { mountDevFolder } from './routes/dev-folder.js'; import { mountApiFallback } from './routes/api-fallback.js'; -import { attemptFallback } from '../services/api-fallback.js'; +import { attemptFallback, type LocalError } from '../services/api-fallback.js'; import type { Driver } from '../driver/types.js'; export interface ApiServerDeps { driver: Driver; } +/** The one place either local-miss seam below produces its response: try the fallback first (which + * never rejects - `services/api-fallback.ts`'s own contract), and only send the local error when the + * fallback declines or abandons. Collapsing both seams' identical two-line sequence into this single + * helper means there is exactly one place that can get the ordering wrong, not two. */ +async function respondWithLocalError(req: Request, res: Response, localError: LocalError): Promise { + if (await attemptFallback(req, res, localError)) return; + sendError(res, localError.status, localError.type, localError.message); +} + export function createApiServer(deps: ApiServerDeps): Express { const app = express(); app.disable('x-powered-by'); @@ -87,8 +96,7 @@ export function createApiServer(deps: ApiServerDeps): Express { } : { status: 404, type: 'not-found', message: `${req.method} ${req.path} was not found` }; - if (await attemptFallback(req, res, localError)) return; - sendError(res, localError.status, localError.type, localError.message); + await respondWithLocalError(req, res, localError); }); // The second seam: a route handler under a matched router rejected with an `ApiError` (`handler.ts`'s @@ -100,8 +108,11 @@ export function createApiServer(deps: ApiServerDeps): Express { // eslint-disable-next-line @typescript-eslint/no-unused-vars app.use(async (err: unknown, req: Request, res: Response, next: NextFunction) => { if (err instanceof ApiError) { - if (await attemptFallback(req, res, { status: err.status, type: err.type, message: err.message })) return; - sendError(res, err.status, err.type, err.message); + await respondWithLocalError(req, res, { + status: err.status, + type: err.type, + message: err.message, + }); return; } diff --git a/src/console/server.ts b/src/console/server.ts index 0a790b3..e7ff89b 100644 --- a/src/console/server.ts +++ b/src/console/server.ts @@ -15,7 +15,7 @@ * runtime-global by nature (`api.md`'s "Upstream fallback" section), so ownership doesn't apply to it at * all. */ -import express, { type Express } from 'express'; +import express, { type Express, type Request } from 'express'; import { getActorById, listAllActors } from '../services/actors.js'; import { @@ -52,6 +52,25 @@ function storageLink(prefix: '/datasets' | '/key-value-stores' | '/request-queue return { text: id, href: `${prefix}/${encodeURIComponent(id)}` }; } +/** Whether `req` carries positive evidence of being a cross-site form submission, for either of the + * console's two mutating `POST` routes. The console is deliberately unauthenticated - anyone who can + * reach it can already flip a toggle or register a dev folder (`console.md`) - but a cross-site page + * silently POSTing to it is a wider threat model than "reachable", since either mutation can now also + * make the caller's real Apify token leave the machine once fallback is enabled. Every modern browser + * sends `Sec-Fetch-Site` on a form submission (a same-origin one - the only way a human actually uses + * either form - is always `same-origin` or `none`); a request without the header at all (an older + * browser, or a non-browser caller like `curl`, which `console.md`'s unauthenticated-by-design model + * already has to tolerate) reports `false` here - only a header that positively says otherwise blocks + * the request. This closes off the specific cross-site-form vector without adding authentication or + * changing either route's documented behaviour for a legitimate same-origin submission. Written as a + * plain predicate (checked at the top of each handler) rather than an Express middleware, so it needs no + * generic parameter shared across the handler chain - `req.params` keeps the type each route's own path + * literal already gives it. */ +function isCrossSiteWrite(req: Request): boolean { + const site = req.header('sec-fetch-site'); + return site !== undefined && site !== 'same-origin' && site !== 'none'; +} + export interface ConsoleServerDeps { driver: Driver; } @@ -74,9 +93,9 @@ function devFolderSection(actorId: string, status: DevFolderStatus, errorMessage export function createConsoleServer(deps: ConsoleServerDeps): Express { const app = express(); app.disable('x-powered-by'); - // Only the dev-folder form below posts anything - every other console route is a plain `GET` - // (`console.md`'s "Every route is a read except the dev-folder form below, which is the console's - // one write"). + // The dev-folder form and the `/settings` form below are the console's only two writes - every other + // route is a plain `GET` (`console.md`'s "Every route is a read except the dev-folder form and the + // Settings form below, which are the console's only two writes"). app.use(express.urlencoded({ extended: false })); app.get('/', async (_req, res) => { @@ -135,6 +154,10 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { * `resolveOwnedActor`. A failure redirects back with `describeDevFolderFailure`'s message in a query * param, so it's surfaced inline rather than swallowed by the redirect. */ app.post('/actors/:id/dev-folder', async (req, res) => { + if (isCrossSiteWrite(req)) { + res.status(403).send('Cross-site form submissions are not allowed.'); + return; + } const actor = await getActorById(req.params.id); if (!actor) { res.status(404).send(layout('Not found', '

Actor not found.

')); @@ -430,6 +453,10 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { * same `setApiFallbackState` the API route calls, so the two surfaces can never observe or produce * different toggle states for the same request. */ app.post('/settings', async (req, res) => { + if (isCrossSiteWrite(req)) { + res.status(403).send('Cross-site form submissions are not allowed.'); + return; + } const body = req.body as Record | undefined; setApiFallbackState({ fallbackUnimplementedEnabled: body?.fallbackUnimplementedEnabled === 'on', diff --git a/src/services/api-fallback.ts b/src/services/api-fallback.ts index 3665995..00608d9 100644 --- a/src/services/api-fallback.ts +++ b/src/services/api-fallback.ts @@ -49,8 +49,22 @@ export function resetApiFallbackStateForTests(): void { } /** No retries, and short enough that a hanging upstream never leaves the caller waiting indefinitely - - * this only ever runs after a local miss, on an opt-in toggle. */ -const FALLBACK_TIMEOUT_MS = 30_000; + * this only ever runs after a local miss, on an opt-in toggle. Mutable only for tests (see + * `setFallbackTimeoutMsForTests` below) - runtime code never changes it. */ +const DEFAULT_FALLBACK_TIMEOUT_MS = 30_000; +let fallbackTimeoutMs = DEFAULT_FALLBACK_TIMEOUT_MS; + +/** Test-only: shrink the upstream timeout so the "hangs past the timeout" fail-closed case can be + * asserted in real wall-clock time instead of waiting out the full 30s production value. Never call + * from runtime code. */ +export function setFallbackTimeoutMsForTests(ms: number): void { + fallbackTimeoutMs = ms; +} + +/** Test-only: restore the production timeout value. Never call from runtime code. */ +export function resetFallbackTimeoutMsForTests(): void { + fallbackTimeoutMs = DEFAULT_FALLBACK_TIMEOUT_MS; +} /** RFC 7230's hop-by-hop set, plus `content-encoding`/`content-length`: the body handed to `fetch()` * already arrives decoded, and Express recomputes framing itself when `res.send()` writes the buffered @@ -82,9 +96,16 @@ function triggerForErrorType(type: string): FallbackTrigger | null { /** `/v2/*` only, and never this runtime's own non-Apify `/v2/actor-runtime/*` namespace (nothing * upstream to call for either exclusion - a request that never reached `/v2` at all was never * authenticated on this path either, see `server.ts`'s mount order). Read off `req.originalUrl` (never - * `req.path`), since that is the one representation router mount-prefix-stripping never touches. */ + * `req.path`), since that is the one representation router mount-prefix-stripping never touches. + * + * Lower-cased and collapsed to single slashes before either comparison: Express routes case- + * insensitively and does not collapse repeated slashes, so `/v2/ACTOR-RUNTIME/...` and + * `/v2//actor-runtime/...` both reach this function with the exclusion's casing/spelling intact but + * still describe the excluded namespace - a naive string comparison would miss both and relay the + * caller's token to `/v2/actor-runtime/*` itself, which has nothing upstream to answer it. Normalising + * only affects this eligibility check, never the byte-exact replay URL below. */ function isEligibleUpstreamPath(originalUrl: string): boolean { - const pathname = originalUrl.split('?')[0] ?? originalUrl; + const pathname = (originalUrl.split('?')[0] ?? originalUrl).toLowerCase().replace(/\/{2,}/g, '/'); if (pathname === '/v2/actor-runtime' || pathname.startsWith('/v2/actor-runtime/')) return false; return pathname === '/v2' || pathname.startsWith('/v2/'); } @@ -119,8 +140,15 @@ export interface LocalError { * representation for a repeated list-valued field, and the only representation `fetch`'s `Headers` * exposes for anything other than `Set-Cookie` (it joins repeated non-cookie header lines together * before this function ever sees them). Anything other than a final `2xx` - non-2xx, timeout, DNS/connect - * failure - is fail-closed (this function returns `false`, changing nothing about the response), logged - * at `warn`. A relay is logged at `log`. + * failure, or the upstream dying *after* a final `2xx` status/headers but before the body finishes - + * is fail-closed (this function returns `false`, changing nothing about the response), logged at + * `warn`. A relay is logged at `log`. + * + * This function never rejects: both call sites in `server.ts` are the terminal middleware for their + * respective seam, so a rejection here would escape to Express's own `finalhandler` instead of + * producing the local error response - everything from the status check onward is therefore wrapped in + * its own `try`/`catch` that logs and returns `false` on any throw, exactly like the initial `fetch` + * itself already does. */ export async function attemptFallback(req: Request, res: Response, localError: LocalError): Promise { if (res.headersSent) return false; @@ -156,7 +184,7 @@ export async function attemptFallback(req: Request, res: Response, localError: L headers, body: method === 'GET' || method === 'HEAD' ? undefined : requestBody, redirect: 'follow', - signal: AbortSignal.timeout(FALLBACK_TIMEOUT_MS), + signal: AbortSignal.timeout(fallbackTimeoutMs), }); } catch (err) { console.warn( @@ -175,23 +203,48 @@ export async function attemptFallback(req: Request, res: Response, localError: L return false; } - const bodyBuffer = Buffer.from(await upstreamResponse.arrayBuffer()); - res.status(upstreamResponse.status); - upstreamResponse.headers.forEach((value, name) => { - const lower = name.toLowerCase(); - // `set-cookie` is handled separately below, via `getSetCookie()` - skipped here so it is never - // also appended from this generic loop, which would double every cookie the platform set. - if (lower === 'set-cookie') return; - if (EXCLUDED_RESPONSE_HEADERS.has(lower)) return; - res.append(name, value); - }); - for (const cookie of upstreamResponse.headers.getSetCookie()) { - res.append('set-cookie', cookie); + // From here on, the upstream already committed to a final 2xx status line and headers - but the body + // itself can still fail mid-stream (the connection resets, a declared Content-Length is never fully + // delivered, ...). That failure surfaces as a rejection from `arrayBuffer()` below, and everything + // after it must never let such a rejection escape this function - see the doc comment above. + try { + const bodyBuffer = Buffer.from(await upstreamResponse.arrayBuffer()); + res.status(upstreamResponse.status); + upstreamResponse.headers.forEach((value, name) => { + const lower = name.toLowerCase(); + // `set-cookie` is handled separately below, via `getSetCookie()` - skipped here so it is never + // also appended from this generic loop, which would double every cookie the platform set. + if (lower === 'set-cookie') return; + if (EXCLUDED_RESPONSE_HEADERS.has(lower)) return; + res.append(name, value); + }); + for (const cookie of upstreamResponse.headers.getSetCookie()) { + res.append('set-cookie', cookie); + } + res.append('x-actor-runtime-fallback', upstreamApiBaseUrl()); + res.append('x-actor-runtime-fallback-trigger', trigger); + res.send(bodyBuffer); + + console.log( + `api-fallback: relayed ${method} ${req.originalUrl} to ${upstreamApiBaseUrl()} (trigger=${trigger})`, + ); + return true; + } catch (err) { + console.warn( + `api-fallback: upstream response for ${method} ${req.originalUrl} (trigger=${trigger}) failed while ` + + `relaying its body: ${err instanceof Error ? err.message : String(err)}; returning the original ` + + `local error instead`, + ); + // Nothing has been sent yet (a throw here always happens before `res.send`), but earlier lines in + // this same block may have already set the status or appended some headers before the throw - + // undo exactly what this function itself could have added, so the caller's local-error response + // (which re-sets the status itself) isn't contaminated with a partial relay's leftovers. + if (!res.headersSent) { + res.removeHeader('x-actor-runtime-fallback'); + res.removeHeader('x-actor-runtime-fallback-trigger'); + res.removeHeader('set-cookie'); + upstreamResponse.headers.forEach((_value, name) => res.removeHeader(name)); + } + return false; } - res.append('x-actor-runtime-fallback', upstreamApiBaseUrl()); - res.append('x-actor-runtime-fallback-trigger', trigger); - res.send(bodyBuffer); - - console.log(`api-fallback: relayed ${method} ${req.originalUrl} to ${upstreamApiBaseUrl()} (trigger=${trigger})`); - return true; } diff --git a/test/integration/api-fallback.test.ts b/test/integration/api-fallback.test.ts index 8514cff..232b324 100644 --- a/test/integration/api-fallback.test.ts +++ b/test/integration/api-fallback.test.ts @@ -16,7 +16,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import axios from 'axios'; import { startTestServer, type TestServerHandle } from './helpers/test-server.js'; -import { resetApiFallbackStateForTests, setApiFallbackState } from '../../src/services/api-fallback.js'; +import { + resetApiFallbackStateForTests, + resetFallbackTimeoutMsForTests, + setApiFallbackState, + setFallbackTimeoutMsForTests, +} from '../../src/services/api-fallback.js'; import { getRegistries } from '../../src/storage/registries.js'; import { generateId } from '../../src/storage/ids.js'; import type { RunRecord } from '../../src/storage/entities.js'; @@ -79,6 +84,47 @@ function startStubUpstream( }); } +/** An upstream that completes a final `2xx` status line and headers - the point at which a relay would + * normally commit - and then dies before the body finishes: it declares a `content-length` larger than + * the bytes it actually writes, then destroys the connection outright. This is the shape the fail-closed + * guarantee must also cover, distinct from every other stub above (which all fail before or instead of + * ever sending a status line at all). */ +function startHeadersThenDieUpstream(): Promise { + const requests: CapturedRequest[] = []; + return new Promise((resolveServer) => { + const server: Server = createServer((req: IncomingMessage, res: ServerResponse) => { + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => chunks.push(chunk)); + req.on('end', () => { + requests.push({ + method: req.method ?? '', + url: req.url ?? '', + headers: req.headers, + body: Buffer.concat(chunks), + }); + res.writeHead(200, { 'content-type': 'application/json', 'content-length': '1000' }); + res.flushHeaders(); + res.write('{"neverFinishes":true'); // far fewer bytes than the declared content-length + // A short delay before killing the connection, so the client has actually received and + // parsed the status line/headers (and its `fetch()` call has settled) before the failure - + // without this, a same-tick `destroy()` can reset the connection before the client ever + // gets past establishing the response, which would fail inside `fetch()` itself rather + // than the later `arrayBuffer()` read this stub exists to exercise. + setTimeout(() => res.destroy(), 50); + }); + }); + server.listen(0, () => { + const { port } = server.address() as AddressInfo; + resolveServer({ + baseUrl: `http://127.0.0.1:${port}`, + hitCount: () => requests.length, + requests: () => requests, + close: () => new Promise((resolve) => server.close(() => resolve())), + }); + }); + }); +} + /** Makes one authenticated request so `services/users.ts: getOrCreateUserForToken()`'s one-time * identity probe for `token` runs and gets cached *now*, against whatever upstream is currently * configured - before a test points `APIFY_UPSTREAM_API_BASE_URL` at its own fallback stub. Without @@ -106,10 +152,23 @@ describe('api-fallback: toggle-state endpoint', () => { beforeEach(async () => { server = await startTestServer(); + // Unlike the other two describe blocks in this file, this one never points + // `APIFY_UPSTREAM_API_BASE_URL` anywhere - several assertions below expect the endpoint's reported + // `upstreamBaseUrl` to read as the real, unconfigured default (`https://api.apify.com`). But every + // authenticated request still runs the one-time identity probe first, and that probe targets + // whichever upstream is configured *at the moment it runs* - so warm it up now, against a + // guaranteed-dead address, and restore the (absent) env var immediately afterward. The probe fails + // and caches instantly with zero real egress, and every request the tests below actually make + // still sees the true default. + const savedUpstreamUrl = process.env.APIFY_UPSTREAM_API_BASE_URL; + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:1'; + await warmUpIdentity(server.baseUrl, server.token); + if (savedUpstreamUrl === undefined) delete process.env.APIFY_UPSTREAM_API_BASE_URL; + else process.env.APIFY_UPSTREAM_API_BASE_URL = savedUpstreamUrl; }); afterEach(async () => { - resetApiFallbackStateForTests(); + // `server.close()` itself resets the toggle state (`helpers/test-server.ts`) - nothing to do here. await server.close(); }); @@ -201,7 +260,9 @@ describe('api-fallback: toggle-state endpoint', () => { upstreamBaseUrl: 'https://api.apify.com', }); - const third = await post('/actor-runtime/api-fallback', { fallbackUnimplementedEnabled: false }); + const third = await post('/actor-runtime/api-fallback', { + fallbackUnimplementedEnabled: false, + }); expect(third.data.data).toEqual({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: true, @@ -247,7 +308,10 @@ describe('api-fallback: toggle-state endpoint', () => { const res = isJson ? await post('/actor-runtime/api-fallback', body) : await axios.post(`${server.baseUrl}/actor-runtime/api-fallback`, body as string, { - headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${server.token}` }, + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${server.token}`, + }, validateStatus: () => true, }); expect(res.status).toBe(400); @@ -283,16 +347,19 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { beforeEach(async () => { server = await startTestServer(); previousUpstreamUrl = process.env.APIFY_UPSTREAM_API_BASE_URL; - // Force the one-time identity probe (`services/identity-resolution.ts`) to happen now, against - // whichever upstream is configured *before* any test below points `APIFY_UPSTREAM_API_BASE_URL` - // at its own stub - otherwise that very probe would be the first request to land on a per-test - // stub, inflating its hit count and logging its own "using local identity" line into a spy meant - // to observe only `attemptFallback`'s own logging. + // Force the one-time identity probe (`services/identity-resolution.ts`) to happen now, before any + // test below points `APIFY_UPSTREAM_API_BASE_URL` at its own stub - otherwise that very probe + // would be the first request to land on a per-test stub, inflating its hit count and logging its + // own "using local identity" line into a spy meant to observe only `attemptFallback`'s own + // logging. Pointed at a guaranteed-dead address first (never the real default + // `https://api.apify.com`), so the probe fails instantly with zero real egress - the same pattern + // `identity-resolution.test.ts` uses throughout. + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:1'; await warmUpIdentity(server.baseUrl, server.token); }); afterEach(async () => { - resetApiFallbackStateForTests(); + // `server.close()` itself resets the toggle state (`helpers/test-server.ts`) - nothing to do here. if (previousUpstreamUrl === undefined) delete process.env.APIFY_UPSTREAM_API_BASE_URL; else process.env.APIFY_UPSTREAM_API_BASE_URL = previousUpstreamUrl; await server.close(); @@ -430,6 +497,40 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { } }); + it('does NOT relay a case-varied /v2/ACTOR-RUNTIME/* path either - the exclusion must not be case-sensitive even though Express itself routes case-insensitively', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + // Express's own default mount matching is case-insensitive, so this reaches the same + // `actor-runtime` sub-router as the lowercase path above, finds no matching PUT route, and + // falls through to the terminal catch-all with the original casing intact in `originalUrl`. + const res = await call('put', '/v2/ACTOR-RUNTIME/api-fallback'); + expect(res.status).toBe(404); + expect(res.data.error.type).toBe('not-found'); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + + it('does NOT relay a duplicate-slash /v2//actor-runtime/* path either', async () => { + const stub = await startStubUpstream(fixedOkResponse('should-not-be-hit')); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + // The doubled slash means Express's own mount matching for `/v2/actor-runtime` misses too, + // so this also falls through to the terminal catch-all - exactly the shape that bypassed + // the exclusion before it normalised repeated slashes. + const res = await call('get', '/v2//actor-runtime/api-fallback'); + expect(res.status).toBe(404); + expect(res.data.error.type).toBe('not-found'); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expect(stub.hitCount()).toBe(0); + } finally { + await stub.close(); + } + }); + it('relays a write method (POST) against an unbuilt endpoint family', async () => { const stub = await startStubUpstream(fixedOkResponse('actor-tasks-post-marker')); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; @@ -579,7 +680,10 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { } it('upstream 404 -> original local error, unchanged, no marker headers', async () => { - const stub = await startStubUpstream(() => ({ status: 404, body: { error: 'upstream 404' } })); + const stub = await startStubUpstream(() => ({ + status: 404, + body: { error: 'upstream 404' }, + })); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { const baseline = await localBothOffResponse('get', '/v2/totally-made-up-path'); @@ -595,7 +699,10 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { }); it('upstream 500 -> original local error, unchanged', async () => { - const stub = await startStubUpstream(() => ({ status: 500, body: { error: 'upstream 500' } })); + const stub = await startStubUpstream(() => ({ + status: 500, + body: { error: 'upstream 500' }, + })); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { const baseline = await localBothOffResponse('get', '/v2/schedules'); @@ -609,7 +716,10 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { }); it('upstream non-not-found 4xx (401) -> original local error, unchanged', async () => { - const stub = await startStubUpstream(() => ({ status: 401, body: { error: 'upstream 401' } })); + const stub = await startStubUpstream(() => ({ + status: 401, + body: { error: 'upstream 401' }, + })); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { const baseline = await localBothOffResponse('get', '/v2/datasets/does-not-exist-at-all'); @@ -623,7 +733,10 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { }); it('upstream non-not-found 4xx (409) -> original local error, unchanged', async () => { - const stub = await startStubUpstream(() => ({ status: 409, body: { error: 'upstream 409' } })); + const stub = await startStubUpstream(() => ({ + status: 409, + body: { error: 'upstream 409' }, + })); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { const baseline = await localBothOffResponse('get', '/v2/datasets/does-not-exist-at-all'); @@ -644,8 +757,30 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); }); - it('upstream hangs past the timeout -> original local error, unchanged (slow: waits out the real timeout)', async () => { - const stub = await startStubUpstream(() => 'hang'); + it('upstream hangs past the timeout -> original local error, unchanged', async () => { + // The production timeout is 30s; shrunk here so this assertion runs in real time instead of + // waiting out the full value - the assertion itself (fail-closed on a hang) is unaffected by + // how long the timeout actually is. + setFallbackTimeoutMsForTests(200); + try { + const stub = await startStubUpstream(() => 'hang'); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const baseline = await localBothOffResponse('get', '/v2/totally-made-up-path'); + const res = await call('get', '/v2/totally-made-up-path'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + } finally { + await stub.close(); + } + } finally { + resetFallbackTimeoutMsForTests(); + } + }); + + it('upstream sends a final 2xx status line and headers, then dies mid-body (catch-all seam) -> original local error, unchanged, no rejection escapes to finalhandler', async () => { + const stub = await startHeadersThenDieUpstream(); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { const baseline = await localBothOffResponse('get', '/v2/totally-made-up-path'); @@ -653,10 +788,28 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.status).toBe(baseline.status); expect(res.data).toEqual(baseline.data); expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBeUndefined(); + expect(String(res.headers['content-type'])).toContain('application/json'); } finally { await stub.close(); } - }, 35_000); + }); + + it('upstream sends a final 2xx status line and headers, then dies mid-body (error-middleware seam) -> original local error, unchanged, no rejection escapes to finalhandler', async () => { + const stub = await startHeadersThenDieUpstream(); + process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; + try { + const baseline = await localBothOffResponse('get', '/v2/datasets/does-not-exist-at-all'); + const res = await call('get', '/v2/datasets/does-not-exist-at-all'); + expect(res.status).toBe(baseline.status); + expect(res.data).toEqual(baseline.data); + expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expect(res.headers['x-actor-runtime-fallback-trigger']).toBeUndefined(); + expect(String(res.headers['content-type'])).toContain('application/json'); + } finally { + await stub.close(); + } + }); }); describe('never forwards a token the caller did not present', () => { @@ -705,7 +858,15 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { it('GET /v2/datasets (a collection route) never hits the stub and never gains upstream items', async () => { const stub = await startStubUpstream(() => ({ status: 200, - body: { data: { items: [{ id: 'platform-only-dataset' }], total: 1, count: 1, offset: 0, limit: 20 } }, + body: { + data: { + items: [{ id: 'platform-only-dataset' }], + total: 1, + count: 1, + offset: 0, + limit: 20, + }, + }, })); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { @@ -873,12 +1034,15 @@ describe('api-fallback: dev-folder-* and internal-error types never forward', () outcome = { ok: false, reason: 'not-found' }; server = await startTestServer(probingDriver); previousUpstreamUrl = process.env.APIFY_UPSTREAM_API_BASE_URL; + // Dead address first, so the one-time identity probe fails instantly with zero real egress + // (see the sibling `beforeEach` above for the full explanation). + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:1'; await warmUpIdentity(server.baseUrl, server.token); setApiFallbackState({ fallbackUnimplementedEnabled: true, fallbackNotFoundEnabled: true }); }); afterEach(async () => { - resetApiFallbackStateForTests(); + // `server.close()` itself resets the toggle state (`helpers/test-server.ts`) - nothing to do here. if (previousUpstreamUrl === undefined) delete process.env.APIFY_UPSTREAM_API_BASE_URL; else process.env.APIFY_UPSTREAM_API_BASE_URL = previousUpstreamUrl; await server.close(); diff --git a/test/integration/dev-folder.test.ts b/test/integration/dev-folder.test.ts index e4c65d7..a45bd84 100644 --- a/test/integration/dev-folder.test.ts +++ b/test/integration/dev-folder.test.ts @@ -36,7 +36,10 @@ const STUB_PROBE_IMAGE_ID = 'stub-probe-image:probe'; function devFolderDriver( initialOutcome: DevFolderProbeOutcome, available = true, -): Driver & { probeDevFolderCalls: Array<[string, string]>; setOutcome(next: DevFolderProbeOutcome): void } { +): Driver & { + probeDevFolderCalls: Array<[string, string]>; + setOutcome(next: DevFolderProbeOutcome): void; +} { let outcome = initialOutcome; const probeDevFolderCalls: Array<[string, string]> = []; return { @@ -623,6 +626,28 @@ describe('console: dev-folder registration form on the Actor detail view', () => expect(stored?.localDevFolder).toBe('/abs/path'); }); + it('rejects a cross-site form submission (Sec-Fetch-Site: cross-site) with 403, and never touches the registration', async () => { + await setUpConsole(devFolderDriver({ ok: true })); + const actor = await server.client.actors().create({ name: 'devfolder-cross-site-actor' }); + + const submit = await axios.post( + `${consoleBaseUrl}/actors/${actor.id}/dev-folder`, + 'localDevFolder=%2Fabs%2Fpath', + { + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + 'Sec-Fetch-Site': 'cross-site', + }, + maxRedirects: 0, + validateStatus: () => true, + }, + ); + expect(submit.status).toBe(403); + + const stored = await getRegistries().actors.get(actor.id); + expect(stored?.localDevFolder).toBeUndefined(); + }); + it('submitting an empty value clears a previously-registered path', async () => { await setUpConsole(devFolderDriver({ ok: true })); const actor = await server.client.actors().create({ name: 'devfolder-clear-actor' }); @@ -647,7 +672,10 @@ describe('console: dev-folder registration form on the Actor detail view', () => it('submitting a whitespace-only value does not clear - it redirects with an inline error and the prior path survives', async () => { await setUpConsole(devFolderDriver({ ok: true })); const actor = await server.client.actors().create({ name: 'devfolder-whitespace-actor' }); - await updateActor(actor.id, (current) => ({ ...current, localDevFolder: '/abs/existing-path' })); + await updateActor(actor.id, (current) => ({ + ...current, + localDevFolder: '/abs/existing-path', + })); const submit = await axios.post(`${consoleBaseUrl}/actors/${actor.id}/dev-folder`, 'localDevFolder=%20%20%20', { headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, @@ -741,7 +769,10 @@ describe('console: dev-folder registration form on the Actor detail view', () => * so `services/runs.ts`'s actor-fields -> `RunContext.devMount` derivation can be exercised end to end * through the real `startRun` service path, without a real Docker socket. */ -function devMountCapturingDriver(): { driver: Driver; getCapturedDevMount: () => DevFolderMount | undefined } { +function devMountCapturingDriver(): { + driver: Driver; + getCapturedDevMount: () => DevFolderMount | undefined; +} { let capturedDevMount: DevFolderMount | undefined; const driver: Driver = { available: true, diff --git a/test/integration/helpers/test-server.ts b/test/integration/helpers/test-server.ts index 4604622..d35b157 100644 --- a/test/integration/helpers/test-server.ts +++ b/test/integration/helpers/test-server.ts @@ -9,6 +9,7 @@ import { ApifyClient } from 'apify-client'; import { bootstrapStorage, resetStorageForTests, shutdownStorage } from '../../../src/storage/bootstrap.js'; import { openRegistries, resetRegistriesForTests } from '../../../src/storage/registries.js'; import { resetUsersForTests } from '../../../src/services/users.js'; +import { resetApiFallbackStateForTests } from '../../../src/services/api-fallback.js'; import { createApiServer } from '../../../src/api/server.js'; import { resetLogsForTests, stopLogFlusher } from '../../../src/services/logs.js'; import type { BuildOutcome, Driver, RunOutcome } from '../../../src/driver/types.js'; @@ -253,6 +254,7 @@ export async function startTestServer( resetStorageForTests(); resetRegistriesForTests(); resetUsersForTests(); + resetApiFallbackStateForTests(); // A background write (late log flush, run-record update) can land while the tree is // being removed, recreating entries under an already-emptied directory — seen in CI as // ENOTEMPTY. fs.rm retries exactly that class of error when maxRetries is set. diff --git a/test/integration/settings-console.test.ts b/test/integration/settings-console.test.ts index aaf28ad..5b52a49 100644 --- a/test/integration/settings-console.test.ts +++ b/test/integration/settings-console.test.ts @@ -10,7 +10,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import axios from 'axios'; import { createConsoleServer } from '../../src/console/server.js'; -import { resetApiFallbackStateForTests, setApiFallbackState } from '../../src/services/api-fallback.js'; +import { setApiFallbackState } from '../../src/services/api-fallback.js'; import { startTestServer, type TestServerHandle } from './helpers/test-server.js'; describe('console: /settings page and the fallback nav indicator', () => { @@ -25,11 +25,27 @@ describe('console: /settings page and the fallback nav indicator', () => { const s = app.listen(0, () => resolve(s)); }); consoleBaseUrl = `http://127.0.0.1:${(consoleServer.address() as AddressInfo).port}`; + + // Several assertions below expect the reported `upstreamBaseUrl` to read as the real, + // unconfigured default (`https://api.apify.com`), so this file never points + // `APIFY_UPSTREAM_API_BASE_URL` anywhere. But the tests below still authenticate against the API + // server with `server.token`, which runs that token's one-time identity probe against whichever + // upstream is configured *at the moment it runs* - warm it up now, against a guaranteed-dead + // address, and restore the (absent) env var immediately afterward, so the probe fails and caches + // instantly with zero real egress. + const savedUpstreamUrl = process.env.APIFY_UPSTREAM_API_BASE_URL; + process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:1'; + await axios.get(`${server.baseUrl}/v2/users/me`, { + headers: { Authorization: `Bearer ${server.token}` }, + validateStatus: () => true, + }); + if (savedUpstreamUrl === undefined) delete process.env.APIFY_UPSTREAM_API_BASE_URL; + else process.env.APIFY_UPSTREAM_API_BASE_URL = savedUpstreamUrl; }); afterEach(async () => { - resetApiFallbackStateForTests(); await new Promise((resolve) => consoleServer.close(() => resolve())); + // `server.close()` itself resets the toggle state (`helpers/test-server.ts`) - nothing to do here. await server.close(); }); @@ -86,6 +102,51 @@ describe('console: /settings page and the fallback nav indicator', () => { }); }); + it('rejects a cross-site form submission (Sec-Fetch-Site: cross-site) with 403, and never changes the toggle state', async () => { + const before = await axios.get(`${server.baseUrl}/actor-runtime/api-fallback`, { + headers: { Authorization: `Bearer ${server.token}` }, + }); + + const submit = await axios.post( + `${consoleBaseUrl}/settings`, + 'fallbackUnimplementedEnabled=on&fallbackNotFoundEnabled=on', + { + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + 'Sec-Fetch-Site': 'cross-site', + }, + maxRedirects: 0, + validateStatus: () => true, + }, + ); + expect(submit.status).toBe(403); + + const after = await axios.get(`${server.baseUrl}/actor-runtime/api-fallback`, { + headers: { Authorization: `Bearer ${server.token}` }, + }); + expect(after.data).toEqual(before.data); + }); + + it("still accepts the submission when Sec-Fetch-Site is same-origin (the real shape a browser sends for this page's own form) or absent entirely (older browsers, non-browser callers)", async () => { + for (const site of ['same-origin', 'none', undefined]) { + setApiFallbackState({ fallbackUnimplementedEnabled: false, fallbackNotFoundEnabled: false }); + const submit = await axios.post(`${consoleBaseUrl}/settings`, 'fallbackUnimplementedEnabled=on', { + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + ...(site ? { 'Sec-Fetch-Site': site } : {}), + }, + maxRedirects: 0, + validateStatus: () => true, + }); + expect(submit.status).toBe(302); + + const stateRes = await axios.get(`${server.baseUrl}/actor-runtime/api-fallback`, { + headers: { Authorization: `Bearer ${server.token}` }, + }); + expect(stateRes.data.data.fallbackUnimplementedEnabled).toBe(true); + } + }); + it('submitting both checkboxes checked turns both on, landing on the shared toggle state (not a console-local copy)', async () => { const submit = await axios.post( `${consoleBaseUrl}/settings`, From f784fb11975839489c850d5ba551cca420864bd0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 08:35:42 +0000 Subject: [PATCH 04/10] Restore byte-identical fail-closed response headers The mid-relay error path stripped every upstream header name from the response, including ones it had never appended and ones whose removal has side effects in Node (`date` clears `sendDate`, `connection` sets `_removedConnection`). A fail-closed response therefore dropped `Date`, `Connection` and `Keep-Alive` relative to the both-toggles-off response it is documented to reproduce. The block could not run for the failure it guarded against: the body is read as the first statement of the `try`, before any response mutation, so nothing is ever appended when that read fails. Removed it, and moved the relay's `return true` and success log outside the `try` so a successful relay can no longer be turned back into a fallthrough. Fail-closed tests now compare the full response header set against each request's own both-toggles-off baseline instead of status, body and markers alone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- requirements/system.md | 2 +- src/console/server.ts | 25 +++++++------- src/services/api-fallback.ts | 34 +++++++++---------- test/integration/api-fallback.test.ts | 47 ++++++++++++++++++++++----- 4 files changed, 70 insertions(+), 38 deletions(-) diff --git a/requirements/system.md b/requirements/system.md index e302868..7403aa8 100644 --- a/requirements/system.md +++ b/requirements/system.md @@ -66,7 +66,7 @@ reuse the already-pulled base image and every other push/call/log-stream/storage-access needs no outbound network access at all (see `cli.md`'s offline-capability note) - unless the opt-in upstream API fallback (`api.md`'s "Upstream fallback" section) has been switched on, in which case a local miss - it's eligible for makes one outbound request per such call. + that is eligible for it makes one outbound request per such call. - The bundled sample Actors (`sample_actor_ts`, `sample_actor_py`) are not offline: they crawl a live site (`https://crawlee.dev/` by default). Running them needs outbound network access from the Actor container, unlike operating the runtime around them (see `test.md`). diff --git a/src/console/server.ts b/src/console/server.ts index e7ff89b..fcf70d1 100644 --- a/src/console/server.ts +++ b/src/console/server.ts @@ -54,18 +54,19 @@ function storageLink(prefix: '/datasets' | '/key-value-stores' | '/request-queue /** Whether `req` carries positive evidence of being a cross-site form submission, for either of the * console's two mutating `POST` routes. The console is deliberately unauthenticated - anyone who can - * reach it can already flip a toggle or register a dev folder (`console.md`) - but a cross-site page - * silently POSTing to it is a wider threat model than "reachable", since either mutation can now also - * make the caller's real Apify token leave the machine once fallback is enabled. Every modern browser - * sends `Sec-Fetch-Site` on a form submission (a same-origin one - the only way a human actually uses - * either form - is always `same-origin` or `none`); a request without the header at all (an older - * browser, or a non-browser caller like `curl`, which `console.md`'s unauthenticated-by-design model - * already has to tolerate) reports `false` here - only a header that positively says otherwise blocks - * the request. This closes off the specific cross-site-form vector without adding authentication or - * changing either route's documented behaviour for a legitimate same-origin submission. Written as a - * plain predicate (checked at the top of each handler) rather than an Express middleware, so it needs no - * generic parameter shared across the handler chain - `req.params` keeps the type each route's own path - * literal already gives it. */ + * reach it can already flip a toggle or register a dev folder (`console.md`) - but both routes are + * unauthenticated, state-changing form `POST`s reachable from any origin, and a cross-site page silently + * driving either one is a wider threat model than "reachable": that's true of both routes on its own, and + * one of them (`/settings`) also enables credential egress once fallback is switched on, which raises the + * stakes further. Every modern browser sends `Sec-Fetch-Site` on a form submission (a same-origin one - + * the only way a human actually uses either form - is always `same-origin` or `none`); a request without + * the header at all (an older browser, or a non-browser caller like `curl`, which `console.md`'s + * unauthenticated-by-design model already has to tolerate) reports `false` here - only a header that + * positively says otherwise blocks the request. This closes off the specific cross-site-form vector + * without adding authentication or changing either route's documented behaviour for a legitimate + * same-origin submission. Written as a plain predicate (checked at the top of each handler) rather than + * an Express middleware, so it needs no generic parameter shared across the handler chain - `req.params` + * keeps the type each route's own path literal already gives it. */ function isCrossSiteWrite(req: Request): boolean { const site = req.header('sec-fetch-site'); return site !== undefined && site !== 'same-origin' && site !== 'none'; diff --git a/src/services/api-fallback.ts b/src/services/api-fallback.ts index 00608d9..fcb8966 100644 --- a/src/services/api-fallback.ts +++ b/src/services/api-fallback.ts @@ -205,8 +205,17 @@ export async function attemptFallback(req: Request, res: Response, localError: L // From here on, the upstream already committed to a final 2xx status line and headers - but the body // itself can still fail mid-stream (the connection resets, a declared Content-Length is never fully - // delivered, ...). That failure surfaces as a rejection from `arrayBuffer()` below, and everything - // after it must never let such a rejection escape this function - see the doc comment above. + // delivered, ...). That failure surfaces as a rejection from `arrayBuffer()` below - the *first* + // statement of this try, before `res` is touched at all - and everything after it must never let such + // a rejection escape this function - see the doc comment above. No cleanup of `res` is needed on catch: + // nothing in this block can mutate `res` and then have a *later* statement in the same block throw. + // `arrayBuffer()` rejecting is first, so a throw there leaves `res` untouched. Past that point, + // `res.status()` only ever receives the already-range-checked 2xx integer above, and every + // `res.append()` call relays a header value that already survived the upstream HTTP response parser + // (a value that parser wouldn't accept - e.g. a raw control character - fails `fetch()` itself, which + // is caught by the earlier `try` around the request, never reaching here) or is one of this module's + // own literal strings, neither of which Node's header validation rejects. So the only way into this + // `catch` is `arrayBuffer()` rejecting, before any mutation - there is nothing to undo. try { const bodyBuffer = Buffer.from(await upstreamResponse.arrayBuffer()); res.status(upstreamResponse.status); @@ -224,27 +233,18 @@ export async function attemptFallback(req: Request, res: Response, localError: L res.append('x-actor-runtime-fallback', upstreamApiBaseUrl()); res.append('x-actor-runtime-fallback-trigger', trigger); res.send(bodyBuffer); - - console.log( - `api-fallback: relayed ${method} ${req.originalUrl} to ${upstreamApiBaseUrl()} (trigger=${trigger})`, - ); - return true; } catch (err) { console.warn( `api-fallback: upstream response for ${method} ${req.originalUrl} (trigger=${trigger}) failed while ` + `relaying its body: ${err instanceof Error ? err.message : String(err)}; returning the original ` + `local error instead`, ); - // Nothing has been sent yet (a throw here always happens before `res.send`), but earlier lines in - // this same block may have already set the status or appended some headers before the throw - - // undo exactly what this function itself could have added, so the caller's local-error response - // (which re-sets the status itself) isn't contaminated with a partial relay's leftovers. - if (!res.headersSent) { - res.removeHeader('x-actor-runtime-fallback'); - res.removeHeader('x-actor-runtime-fallback-trigger'); - res.removeHeader('set-cookie'); - upstreamResponse.headers.forEach((_value, name) => res.removeHeader(name)); - } return false; } + + // Deliberately outside the `try`: once `res.send()` above returns without throwing, the relay has + // unconditionally happened, and nothing past this point may turn that back into a `false` - logging + // the success can't retroactively fail the relay it's merely describing. + console.log(`api-fallback: relayed ${method} ${req.originalUrl} to ${upstreamApiBaseUrl()} (trigger=${trigger})`); + return true; } diff --git a/test/integration/api-fallback.test.ts b/test/integration/api-fallback.test.ts index 232b324..e7580a9 100644 --- a/test/integration/api-fallback.test.ts +++ b/test/integration/api-fallback.test.ts @@ -679,7 +679,27 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { return res; } - it('upstream 404 -> original local error, unchanged, no marker headers', async () => { + /** Criterion 16's "byte-identical (status, body, and header set)" - checked here as the header + * *name* set plus every value except `date`, whose value legitimately differs run-to-run (its + * mere presence on both sides is asserted instead). This is what would have caught the fail-closed + * mid-body-death path silently dropping `date`/`connection`/`keep-alive`: that regression left + * status, body, and the two marker headers alone, so only a full-header-set comparison against the + * same request's both-toggles-off baseline surfaces it. */ + function expectSameHeaderSet( + actual: Record, + baseline: Record, + ): void { + const withoutDate = (headers: Record) => { + const rest = { ...headers }; + delete rest.date; + return rest; + }; + expect(withoutDate(actual)).toEqual(withoutDate(baseline)); + expect(actual['date']).toBeDefined(); + expect(baseline['date']).toBeDefined(); + } + + it('upstream 404 -> original local error, unchanged, no marker headers, full header set matches the both-toggles-off baseline', async () => { const stub = await startStubUpstream(() => ({ status: 404, body: { error: 'upstream 404' }, @@ -692,13 +712,14 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.data).toEqual(baseline.data); expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); expect(res.headers['x-actor-runtime-fallback-trigger']).toBeUndefined(); + expectSameHeaderSet(res.headers, baseline.headers); expect(stub.hitCount()).toBe(1); } finally { await stub.close(); } }); - it('upstream 500 -> original local error, unchanged', async () => { + it('upstream 500 -> original local error, unchanged, full header set matches the both-toggles-off baseline', async () => { const stub = await startStubUpstream(() => ({ status: 500, body: { error: 'upstream 500' }, @@ -710,12 +731,13 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.status).toBe(baseline.status); expect(res.data).toEqual(baseline.data); expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expectSameHeaderSet(res.headers, baseline.headers); } finally { await stub.close(); } }); - it('upstream non-not-found 4xx (401) -> original local error, unchanged', async () => { + it('upstream non-not-found 4xx (401) -> original local error, unchanged, full header set matches the both-toggles-off baseline', async () => { const stub = await startStubUpstream(() => ({ status: 401, body: { error: 'upstream 401' }, @@ -727,12 +749,13 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.status).toBe(baseline.status); expect(res.data).toEqual(baseline.data); expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expectSameHeaderSet(res.headers, baseline.headers); } finally { await stub.close(); } }); - it('upstream non-not-found 4xx (409) -> original local error, unchanged', async () => { + it('upstream non-not-found 4xx (409) -> original local error, unchanged, full header set matches the both-toggles-off baseline', async () => { const stub = await startStubUpstream(() => ({ status: 409, body: { error: 'upstream 409' }, @@ -743,21 +766,23 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { const res = await call('get', '/v2/datasets/does-not-exist-at-all'); expect(res.status).toBe(baseline.status); expect(res.data).toEqual(baseline.data); + expectSameHeaderSet(res.headers, baseline.headers); } finally { await stub.close(); } }); - it('upstream unreachable (connection refused) -> original local error, unchanged', async () => { + it('upstream unreachable (connection refused) -> original local error, unchanged, full header set matches the both-toggles-off baseline', async () => { process.env.APIFY_UPSTREAM_API_BASE_URL = 'http://127.0.0.1:1'; // nothing listens here const baseline = await localBothOffResponse('get', '/v2/totally-made-up-path'); const res = await call('get', '/v2/totally-made-up-path'); expect(res.status).toBe(baseline.status); expect(res.data).toEqual(baseline.data); expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expectSameHeaderSet(res.headers, baseline.headers); }); - it('upstream hangs past the timeout -> original local error, unchanged', async () => { + it('upstream hangs past the timeout -> original local error, unchanged, full header set matches the both-toggles-off baseline', async () => { // The production timeout is 30s; shrunk here so this assertion runs in real time instead of // waiting out the full value - the assertion itself (fail-closed on a hang) is unaffected by // how long the timeout actually is. @@ -771,6 +796,7 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.status).toBe(baseline.status); expect(res.data).toEqual(baseline.data); expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); + expectSameHeaderSet(res.headers, baseline.headers); } finally { await stub.close(); } @@ -779,7 +805,7 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { } }); - it('upstream sends a final 2xx status line and headers, then dies mid-body (catch-all seam) -> original local error, unchanged, no rejection escapes to finalhandler', async () => { + it('upstream sends a final 2xx status line and headers, then dies mid-body (catch-all seam) -> original local error, unchanged, no rejection escapes to finalhandler, full header set matches the both-toggles-off baseline', async () => { const stub = await startHeadersThenDieUpstream(); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { @@ -790,12 +816,16 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); expect(res.headers['x-actor-runtime-fallback-trigger']).toBeUndefined(); expect(String(res.headers['content-type'])).toContain('application/json'); + // The regression this guards against: a prior implementation's cleanup path stripped + // `date`/`connection`/`keep-alive` from this response even though it never appended them - + // status/body/markers alone don't catch that; the full set comparison does. + expectSameHeaderSet(res.headers, baseline.headers); } finally { await stub.close(); } }); - it('upstream sends a final 2xx status line and headers, then dies mid-body (error-middleware seam) -> original local error, unchanged, no rejection escapes to finalhandler', async () => { + it('upstream sends a final 2xx status line and headers, then dies mid-body (error-middleware seam) -> original local error, unchanged, no rejection escapes to finalhandler, full header set matches the both-toggles-off baseline', async () => { const stub = await startHeadersThenDieUpstream(); process.env.APIFY_UPSTREAM_API_BASE_URL = stub.baseUrl; try { @@ -806,6 +836,7 @@ describe('api-fallback: eligibility, relay, and fail-closed behaviour', () => { expect(res.headers['x-actor-runtime-fallback']).toBeUndefined(); expect(res.headers['x-actor-runtime-fallback-trigger']).toBeUndefined(); expect(String(res.headers['content-type'])).toContain('application/json'); + expectSameHeaderSet(res.headers, baseline.headers); } finally { await stub.close(); } From 157ab60975675eba57933d3816b649c896ccd6de Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 08:51:38 +0000 Subject: [PATCH 05/10] Correct two comments the fallback work left stale The dev-folder route's doc comment still called it the console's one mutation, which the settings form falsified; the module header and `requirements/console.md` were updated but this sibling sentence was not. `attemptFallback`'s doc comment described its `try`/`catch` as covering everything from the status check onward, which stopped being true when the success log and `return true` moved out of it. The contract it states is unchanged; only its stated basis was wrong, in the comment an editor would consult before adding a statement after `res.send()`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- src/console/server.ts | 2 +- src/services/api-fallback.ts | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/console/server.ts b/src/console/server.ts index fcf70d1..99dad4a 100644 --- a/src/console/server.ts +++ b/src/console/server.ts @@ -150,7 +150,7 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { res.send(layout(`Actor ${actor.name}`, body)); }); - /** The console's one mutation - funnels through the same `setDevFolder` the API endpoint uses, + /** One of the console's two mutations - funnels through the same `setDevFolder` the API endpoint uses, * resolving the Actor cross-user by the id already in the page URL (no token) rather than through * `resolveOwnedActor`. A failure redirects back with `describeDevFolderFailure`'s message in a query * param, so it's surfaced inline rather than swallowed by the redirect. */ diff --git a/src/services/api-fallback.ts b/src/services/api-fallback.ts index fcb8966..fb8020e 100644 --- a/src/services/api-fallback.ts +++ b/src/services/api-fallback.ts @@ -146,9 +146,11 @@ export interface LocalError { * * This function never rejects: both call sites in `server.ts` are the terminal middleware for their * respective seam, so a rejection here would escape to Express's own `finalhandler` instead of - * producing the local error response - everything from the status check onward is therefore wrapped in - * its own `try`/`catch` that logs and returns `false` on any throw, exactly like the initial `fetch` - * itself already does. + * producing the local error response - every fallible step between the status check and `res.send()` is + * therefore wrapped in its own `try`/`catch` that logs and returns `false` on any throw, exactly like the + * initial `fetch` itself already does. The one deliberate exception is the success log and `return true` + * that follow `res.send()`: they sit outside that `try` on purpose, and cannot themselves turn a + * completed relay back into `false` - see the comment just above them. */ export async function attemptFallback(req: Request, res: Response, localError: LocalError): Promise { if (res.headersSent) return false; From b9e83a2ab5a7905a8778609be5f2d5aba4d2c400 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 09:18:54 +0000 Subject: [PATCH 06/10] Keep the fallback requirements at contract level Two sentences described internals rather than behaviour a caller can observe: the relay bullet explained that the body is re-framed rather than streamed, and the console section explained that both surfaces write through one shared toggle state. Either would have to change if the implementation were rewritten without any observable difference. Both now state only the consequence a caller or console user can see, and one paragraph is rewrapped to the surrounding width after the edit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- requirements/api.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/requirements/api.md b/requirements/api.md index 4179e5a..4f75411 100644 --- a/requirements/api.md +++ b/requirements/api.md @@ -188,14 +188,14 @@ opting in, not an oversight. - **What a successful relay looks like**: the platform's response status and body are returned to the caller unchanged. Response headers are relayed too, minus the standard hop-by-hop set - (`Connection`, `Keep-Alive`, `Transfer-Encoding`, ...) plus `Content-Encoding`/`Content-Length` (the - relayed body is re-framed, not streamed through byte-for-byte). `Set-Cookie` is always relayed as one - header line per cookie the platform set - never merged into one, since a cookie's own value can contain - a comma. Any other header name the platform repeats is relayed as a single, comma-joined value (the - standard representation for a repeated header field), not as separate repeated lines. Two markers are - added: `x-actor-runtime-fallback: ` (which platform served it) and - `x-actor-runtime-fallback-trigger: unimplemented` or `record-not-found` (which toggle let it through). - Only a final `2xx` counts as successful. + (`Connection`, `Keep-Alive`, `Transfer-Encoding`, ...) plus `Content-Encoding`/`Content-Length` - a + caller must not rely on the platform's own `Content-Encoding`/`Content-Length` values being echoed. + `Set-Cookie` is always relayed as one header line per cookie the platform set - never merged into one, + since a cookie's own value can contain a comma. Any other header name the platform repeats is relayed + as a single, comma-joined value (the standard representation for a repeated header field), not as + separate repeated lines. Two markers are added: `x-actor-runtime-fallback: ` + (which platform served it) and `x-actor-runtime-fallback-trigger: unimplemented` or + `record-not-found` (which toggle let it through). Only a final `2xx` counts as successful. - **Fail-closed guarantee**: anything else - a non-`2xx` response, a timeout, or the platform being unreachable - reproduces the exact response the caller would have gotten with both toggles off: the original local error, unchanged, with neither marker header present. The platform's own status or body From 7ed5a4176c25c2d3f570bac19d34e55ef372d99e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 09:31:24 +0000 Subject: [PATCH 07/10] Drop the shared-state claim from the console requirements The Settings-page section explained that both surfaces write through one underlying toggle state. An implementation keeping two stores in sync would falsify that while behaving identically, so it described internals rather than a contract. The guarantee it was there to support - a flip on either surface is immediately visible on the other and via the API's own GET, with no restart - is unchanged. The previous commit's message described this edit, but it was dropped from the working tree before that commit was made. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- requirements/console.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/requirements/console.md b/requirements/console.md index 554ec5a..db381c2 100644 --- a/requirements/console.md +++ b/requirements/console.md @@ -72,9 +72,8 @@ for not-found records", and one submit. Submitting it always sends both checkboxes' current state together - an unchecked box is read as `false`, not as "leave this toggle unchanged" - and redirects back to `/settings` showing the result. This differs from the API's own partial `POST` (`api.md`), - which only touches the field(s) a caller's body actually names; both surfaces write through the same - underlying toggle state, so a flip made on one is immediately visible on the other and via the API's - own `GET`, with no restart needed either way. + which only touches the field(s) a caller's body actually names; a flip made on either surface is + immediately visible on the other and via the API's own `GET`, with no restart needed either way. - Since the console has no login of its own, anyone who can reach it can flip either toggle for every caller of the API - the same unauthenticated, cross-user model the rest of the console already has, not a new exposure specific to this page. From caa63ff266a9d85ece8cc7a9e164cd0a15d0a4b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 12:45:31 +0000 Subject: [PATCH 08/10] State the fallback requirements as requirements The upstream fallback sections explained how the runtime works rather than what it guarantees: which headers are hop-by-hop, why cookie values cannot be comma-joined, what the spec table matches, how a partial body merges into stored state, and which log level each outcome writes at. None of that is something a caller can observe or rely on. What a caller does rely on stays: the toggles and their defaults, the endpoint contracts, which local outcome each toggle covers, all methods being eligible and what that costs, the fail-closed guarantee, that only the caller's own token is forwarded, and how a relayed response is marked. The retry wording is restated as the guarantee behind it - an eligible request reaches the platform at most once, so a relayed write is never duplicated. The console's cross-site rejection now states the limit of the guarantee as well as the guarantee: a submission that does not identify itself as cross-site is not rejected. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- requirements/api.md | 78 ++++++++++++++++++----------------------- requirements/console.md | 40 +++++++++------------ 2 files changed, 51 insertions(+), 67 deletions(-) diff --git a/requirements/api.md b/requirements/api.md index 4f75411..dcbd392 100644 --- a/requirements/api.md +++ b/requirements/api.md @@ -152,64 +152,54 @@ ## Upstream fallback (opt-in, off by default, all HTTP methods) -- Two independent booleans, `fallbackUnimplementedEnabled` and `fallbackNotFoundEnabled`, gate whether a - request this runtime cannot satisfy locally is instead relayed to the real Apify platform. Both default - to `false` on a fresh process and neither is persisted anywhere - a restart always brings both back to - `false`, regardless of how they were last set. Either can be on without the other; all four - combinations are valid. +- Two independent booleans, `fallbackUnimplementedEnabled` and `fallbackNotFoundEnabled`, gate whether + a request this runtime cannot satisfy locally is instead relayed to the real Apify platform. Both + default to `false`, and a restart always brings both back to `false`, regardless of how they were + last set. Either can be on without the other; all four combinations are valid. - **`GET /actor-runtime/api-fallback`** (also reachable at `/v2/actor-runtime/api-fallback`, like every other endpoint in this namespace) returns `{ "data": { "fallbackUnimplementedEnabled": , "fallbackNotFoundEnabled": , "upstreamBaseUrl": } }`. `upstreamBaseUrl` is the platform this runtime would relay to (default `https://api.apify.com`, or the value of `APIFY_UPSTREAM_API_BASE_URL` if set) - reported for visibility, but read-only: no request body can change it. -- **`POST /actor-runtime/api-fallback`** (same two mounts) accepts a **partial** body - either field, or - both - and merges it into the existing state, leaving any field the body doesn't mention untouched. The - response is the same shape `GET` returns, showing the state immediately after the merge. +- **`POST /actor-runtime/api-fallback`** (same two mounts) accepts a body naming either field, or both; + a field the body doesn't mention keeps its current value. The response is the same shape `GET` + returns, showing the state immediately after the change. - **Authenticated** the same way as every other route in this namespace: no token is `401` `user-not-authenticated`, with no state change. - **Error responses**: a body that isn't a JSON object (a JSON array, scalar, or `null`), a body present but empty (`{}`), a body containing a key other than the two above, or a body where a present key's value isn't a boolean, is `400` `invalid-request`, with no state change. -- **Which local outcome each toggle covers** (exhaustive - every other error response is never eligible, - under any toggle combination): - - `fallbackUnimplementedEnabled` covers a request whose path/method this runtime does not serve at - all - either an off-spec path (matches no entry in the vendored spec table, `501 vs 404` above) or a - spec-known path this runtime hasn't built (the `501` case, same section). From the caller's point of - view both are "nothing local answers this", so one toggle covers both. - - `fallbackNotFoundEnabled` covers a request that reaches a route this runtime does serve, but whose - specific record id doesn't exist locally (`record-not-found`, see "Response envelopes" above). - - Every other error type - `invalid-request`, `user-not-authenticated`, `cannot-remove-running-run`, - `deleting-unfinished-build`, any `dev-folder-*` type, `internal-error` - is never relayed, regardless - of either toggle's state. -- **All HTTP methods are eligible for both toggles, writes included**: a `POST`/`PUT`/`DELETE` that would - otherwise 404/501 locally is relayed exactly like a `GET` when its toggle is on - and, if the platform - accepts it, becomes a real write against the caller's real account. This is a deliberate consequence of - opting in, not an oversight. -- **What a successful relay looks like**: the platform's response status and body are returned to the - caller unchanged. Response headers are relayed too, minus the standard hop-by-hop set - (`Connection`, `Keep-Alive`, `Transfer-Encoding`, ...) plus `Content-Encoding`/`Content-Length` - a - caller must not rely on the platform's own `Content-Encoding`/`Content-Length` values being echoed. - `Set-Cookie` is always relayed as one header line per cookie the platform set - never merged into one, - since a cookie's own value can contain a comma. Any other header name the platform repeats is relayed - as a single, comma-joined value (the standard representation for a repeated header field), not as - separate repeated lines. Two markers are added: `x-actor-runtime-fallback: ` - (which platform served it) and `x-actor-runtime-fallback-trigger: unimplemented` or - `record-not-found` (which toggle let it through). Only a final `2xx` counts as successful. +- **Which local outcome each toggle covers** (exhaustive - every other error response is never + eligible, under any toggle combination): + - `fallbackUnimplementedEnabled` covers a request the runtime does not serve at all: a local `404` + or `501` response (see "501 vs 404" above). From the caller's point of view both mean "nothing + local answers this", so one toggle covers both. + - `fallbackNotFoundEnabled` covers a request that reaches a route this runtime does serve, but + whose specific record id doesn't exist locally (`record-not-found`, see "Response envelopes" + above). + - Every other error type - `invalid-request`, `user-not-authenticated`, + `cannot-remove-running-run`, `deleting-unfinished-build`, any `dev-folder-*` type, + `internal-error` - is never relayed, regardless of either toggle's state. +- **All HTTP methods are eligible for both toggles, writes included**: a `POST`/`PUT`/`DELETE` that + would otherwise 404/501 locally is relayed exactly like a `GET` when its toggle is on - and, if the + platform accepts it, becomes a real write against the caller's real account. This is a deliberate + consequence of opting in, not an oversight. An eligible request reaches the platform at most once, so + a relayed write is never duplicated. +- **A successful relay** returns the platform's response status and body to the caller unchanged, + marked with two response headers: `x-actor-runtime-fallback: ` naming which platform + served it, and `x-actor-runtime-fallback-trigger: unimplemented` or `record-not-found` naming which + toggle let it through. Only a final `2xx` status counts as successful. - **Fail-closed guarantee**: anything else - a non-`2xx` response, a timeout, or the platform being unreachable - reproduces the exact response the caller would have gotten with both toggles off: the - original local error, unchanged, with neither marker header present. The platform's own status or body - is never surfaced to the caller. One attempt is made per request; nothing is retried. -- **Only the caller's own presented token is ever forwarded.** A relayed request's `Authorization` header - is always the exact bearer token the caller themselves sent on that request - never a different or - runtime-internal credential, and never sent at all for a request this runtime didn't authenticate. - Enabling either toggle therefore means the caller's own Apify token reaches the configured - `upstreamBaseUrl` on every eligible request; this is the risk being opted into. + original local error, unchanged, with neither marker header present. The platform's own status or + body is never surfaced to the caller. +- **Only the caller's own presented token is ever forwarded.** A relayed request's `Authorization` + header is always the exact bearer token the caller themselves sent on that request - never a + different or runtime-internal credential, and never sent at all for a request this runtime didn't + authenticate. Enabling either toggle therefore means the caller's own Apify token reaches the + configured `upstreamBaseUrl` on every eligible request; this is the risk being opted into. - **Never enriches a call that already succeeds locally**: a collection/list endpoint (e.g. `GET /v2/datasets`) that already returns `200` from local data never consults either toggle and never gains platform objects. Fallback only ever resolves an otherwise-failing request; it does not make a local listing "complete". -- One line is logged per fallback attempt: a relayed request logs once at the informational level a - successful relay happened; an abandoned attempt (any fail-closed case above) logs once at the warning - level, including the platform's status or failure reason. Neither line appears when the relevant - toggle is off. diff --git a/requirements/console.md b/requirements/console.md index db381c2..9a4ceda 100644 --- a/requirements/console.md +++ b/requirements/console.md @@ -11,11 +11,10 @@ (`storage.md`'s "Users" section). - The console is unauthenticated. Every route is a read except the dev-folder form below and the Settings form below, which are the console's only two writes - it is no longer strictly view-only. -- Both of those two writes reject a cross-site form submission (a browser sending `Sec-Fetch-Site` with - any value other than `same-origin`/`none`) with a plain `403`, so a page from another origin cannot - silently drive either write through a visitor's browser. A submission with no `Sec-Fetch-Site` header - at all (an older browser, or a non-browser caller) is unaffected - this narrows the console's existing - "anyone who can reach it" model by one specific vector, it does not add a login. +- Both of those two writes reject a submission that identifies itself as cross-site (via the + `Sec-Fetch-Site` header) with a plain `403`; a submission that does not is unaffected. This + narrows the console's existing "anyone who can reach it" model by one specific vector, it does + not add a login. - There are three types of objects: key-value store, dataset, request queue. - For each object type there must be exactly one widget for inspection. - The request-queue widget leads with the authoritative counts from `RequestQueue.getInfo()` @@ -58,22 +57,17 @@ ## Settings page -- The last entry in every page's header navigation is "Settings", linking to `/settings` - the one page - for the upstream API fallback toggles (`api.md`'s "Upstream fallback" section). Every other page's - header nav also shows both toggles' current state next to that link, in the form - `Settings — fallback (unimplemented: on|off, not-found: on|off)`, so neither toggle can ever be on - without being visible from anywhere in the console; the two states are shown independently, never - collapsed into a single word (a mixed state - one on, one off - is visually distinct from both-on and - both-off). -- `/settings` itself shows `fallbackUnimplementedEnabled`, `fallbackNotFoundEnabled`, and - `upstreamBaseUrl` (the same values the API's toggle endpoint reports), plus a one-line warning that - enabling either toggle forwards the caller's own Apify token to that URL. -- A single form on the page has two checkboxes, "Fall back for unimplemented endpoints" and "Fall back - for not-found records", and one submit. Submitting it always sends both checkboxes' current state - together - an unchecked box is read as `false`, not as "leave this toggle unchanged" - and redirects - back to `/settings` showing the result. This differs from the API's own partial `POST` (`api.md`), - which only touches the field(s) a caller's body actually names; a flip made on either surface is - immediately visible on the other and via the API's own `GET`, with no restart needed either way. +- Every page's header navigation includes a link to `/settings`, the one page for the upstream API + fallback toggles (`api.md`'s "Upstream fallback" section). The link itself shows both toggles' + current values, independently of each other, so neither toggle can ever be on without being visible + from anywhere in the console. +- `/settings` shows `fallbackUnimplementedEnabled`, `fallbackNotFoundEnabled`, and `upstreamBaseUrl` + (the same values the API's toggle endpoint reports), plus a warning that enabling either toggle + forwards the caller's own Apify token to that URL. +- The Settings page lets a caller set both toggles at once. Unlike the API's partial `POST` (`api.md`), + submitting the form always sets both toggles explicitly - leaving one unchecked sets it to `false`, + never "leave this toggle unchanged". A change made through either surface is immediately visible on + the other, and via the API's own `GET`, with no restart needed either way. - Since the console has no login of its own, anyone who can reach it can flip either toggle for every - caller of the API - the same unauthenticated, cross-user model the rest of the console already has, not - a new exposure specific to this page. + caller of the API - the same unauthenticated, cross-user model the rest of the console already has, + not a new exposure specific to this page. From 3f83bbe4efef7367d00c696a29d93c99fb77eb3c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 14:11:34 +0000 Subject: [PATCH 09/10] Order the console's build and run listings by start time The builds, runs and logs views rendered whatever order the registry happened to return, which it makes no promise about. They now show the most recently started record first, and the logs view interleaves builds and runs by start time instead of listing all builds and then all runs. Records sharing a start time are ordered by id so the sequence is stable across renders. The API's own ordering is unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- requirements/console.md | 2 + src/console/order.ts | 17 +++++ src/console/server.ts | 29 +++++++-- test/integration/console.test.ts | 107 +++++++++++++++++++++++++++++++ test/unit/console-order.test.ts | 37 +++++++++++ 5 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 src/console/order.ts create mode 100644 test/unit/console-order.test.ts diff --git a/requirements/console.md b/requirements/console.md index 9a4ceda..a37c758 100644 --- a/requirements/console.md +++ b/requirements/console.md @@ -35,6 +35,8 @@ - Settings (a single page, not a list/detail pair - see "Settings page" below) - List view is a list of objects that can be clicked on to open detail view. +- The Actor builds list, the Actor runs list, and the combined Logs list all show the most recently + started build or run first. - Detail view of an object is showing only one object with all the available data - A run's default storage ids (`defaultDatasetId`, `defaultKeyValueStoreId`, `defaultRequestQueueId`) are rendered as links to the corresponding storage detail views (in the run detail view and in the runs diff --git a/src/console/order.ts b/src/console/order.ts new file mode 100644 index 0000000..59dfa68 --- /dev/null +++ b/src/console/order.ts @@ -0,0 +1,17 @@ +/** + * Newest-`startedAt`-first order for every console view that lists builds and/or runs (`console.md`). + * `Registry.list()` - what `listAllBuilds`/`listAllRuns` (`server.ts`) read through - documents its own + * iteration as having "no particular order" (`storage/registry.ts`), so two records sharing the same + * `startedAt` cannot rely on input order to stay put across renders; `id` (unique per record) breaks the + * tie the same way every time, regardless of the order the registry happened to hand them back in. + * + * Deliberately its own module, not a reuse of the API's `sortByTimestamp` (`api/envelope.ts`): that + * helper sorts ascending, for a different shape of data (already-paginated DTOs, reversed afterwards by + * `paginate`'s own `desc` handling), and reaching for it here would make the console depend on the API + * layer - the two are sibling consumers of `services/`, neither of the other. This two-line comparator is + * cheap enough to keep that boundary intact, and small enough to unit-test on its own (see + * `ansi.ts`/`ansi.test.ts` for the same pattern: a pure console-layer helper, tested directly). + */ +export function newestFirst(items: readonly T[]): T[] { + return [...items].sort((a, b) => b.startedAt.localeCompare(a.startedAt) || a.id.localeCompare(b.id)); +} diff --git a/src/console/server.ts b/src/console/server.ts index 99dad4a..c023c8a 100644 --- a/src/console/server.ts +++ b/src/console/server.ts @@ -33,6 +33,7 @@ import { openDataset, openKeyValueStore, openRequestQueue } from '../storage/ope import { pageKeys } from '../services/kv-key-listing.js'; import { applyDatasetProjection, type DatasetItem } from '../services/dataset-projection.js'; import { ansiToHtml } from './ansi.js'; +import { newestFirst } from './order.js'; import { apiFallbackWarning, definitionList, @@ -219,7 +220,7 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { }); app.get('/builds', async (_req, res) => { - const builds = await listAllBuilds(); + const builds = newestFirst(await listAllBuilds()); const rows = builds.map((b) => [b.id, b.userId, b.actorId, b.buildNumber, b.status, b.startedAt]); res.send( layout( @@ -256,7 +257,7 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { }); app.get('/runs', async (_req, res) => { - const runs = await listAllRuns(); + const runs = newestFirst(await listAllRuns()); const rows = runs.map((r) => [ r.id, r.userId, @@ -301,10 +302,26 @@ export function createConsoleServer(deps: ConsoleServerDeps): Express { app.get('/logs', async (_req, res) => { const [builds, runs] = await Promise.all([listAllBuilds(), listAllRuns()]); - const rows = [ - ...builds.map((b) => [b.id, b.userId, 'build', b.status]), - ...runs.map((r) => [r.id, r.userId, 'run', r.status]), - ]; + // Builds and runs share this one list, so they're merged before sorting rather than each sorted on + // its own and concatenated - otherwise every build would still render before every run (or vice + // versa) regardless of which is actually newer. + const entries = newestFirst([ + ...builds.map((b) => ({ + id: b.id, + userId: b.userId, + kind: 'build' as const, + status: b.status, + startedAt: b.startedAt, + })), + ...runs.map((r) => ({ + id: r.id, + userId: r.userId, + kind: 'run' as const, + status: r.status, + startedAt: r.startedAt, + })), + ]); + const rows = entries.map((e) => [e.id, e.userId, e.kind, e.status]); res.send(layout('Logs', table(['id', 'userId', 'kind', 'status'], rows, 0, '/logs'))); }); diff --git a/test/integration/console.test.ts b/test/integration/console.test.ts index f89debf..7963663 100644 --- a/test/integration/console.test.ts +++ b/test/integration/console.test.ts @@ -78,6 +78,113 @@ describe('console pages (HTTP fetch)', () => { expect(logDetail.data).toContain('Docker'); }); + it('builds list renders newest startedAt first (regression: registry list order carries no timestamp guarantee of its own)', async () => { + const actor = await server.client.actors().create({ name: 'console-builds-order-actor' }); + const actorRecord = (await getRegistries().actors.get(actor.id))!; + const { builds } = getRegistries(); + + const startedAts = ['2024-01-01T00:00:00.000Z', '2024-06-01T00:00:00.000Z', '2025-01-01T00:00:00.000Z']; + const seeded: BuildRecord[] = []; + for (const startedAt of startedAts) { + const record: BuildRecord = { + id: generateId(), + userId: actorRecord.userId, + actorId: actor.id, + versionNumber: '0.0', + buildNumber: '0.0.1', + tag: 'latest', + status: 'SUCCEEDED', + startedAt, + finishedAt: startedAt, + imageId: 'fake-image:latest', + }; + await builds.set(record.id, record); + seeded.push(record); + } + const [oldest, middle, newest] = seeded; + + const page = (await axios.get(`${consoleBaseUrl}/builds`)).data as string; + const indexOf = (b: BuildRecord) => page.indexOf(b.id); + expect(indexOf(newest!)).toBeGreaterThanOrEqual(0); + expect(indexOf(newest!)).toBeLessThan(indexOf(middle!)); + expect(indexOf(middle!)).toBeLessThan(indexOf(oldest!)); + }); + + it('runs list renders newest startedAt first', async () => { + const actor = await server.client.actors().create({ name: 'console-runs-order-actor' }); + const actorRecord = (await getRegistries().actors.get(actor.id))!; + const { runs } = getRegistries(); + + const startedAts = ['2024-01-01T00:00:00.000Z', '2024-06-01T00:00:00.000Z', '2025-01-01T00:00:00.000Z']; + const seeded: RunRecord[] = []; + for (const startedAt of startedAts) { + const record: RunRecord = { + id: generateId(), + userId: actorRecord.userId, + actorId: actor.id, + buildId: generateId(), + buildNumber: '0.0.1', + status: 'SUCCEEDED', + startedAt, + finishedAt: startedAt, + defaultDatasetId: 'd', + defaultKeyValueStoreId: 'k', + defaultRequestQueueId: 'r', + options: { memoryMbytes: 1024, timeoutSecs: 300 }, + meta: { origin: 'API' }, + }; + await runs.set(record.id, record); + seeded.push(record); + } + const [oldest, middle, newest] = seeded; + + const page = (await axios.get(`${consoleBaseUrl}/runs`)).data as string; + const indexOf = (r: RunRecord) => page.indexOf(r.id); + expect(indexOf(newest!)).toBeGreaterThanOrEqual(0); + expect(indexOf(newest!)).toBeLessThan(indexOf(middle!)); + expect(indexOf(middle!)).toBeLessThan(indexOf(oldest!)); + }); + + it('logs list interleaves builds and runs by newest startedAt first, rather than grouping all builds before all runs', async () => { + const actor = await server.client.actors().create({ name: 'console-logs-order-actor' }); + const actorRecord = (await getRegistries().actors.get(actor.id))!; + const { builds, runs } = getRegistries(); + + const olderBuild: BuildRecord = { + id: generateId(), + userId: actorRecord.userId, + actorId: actor.id, + versionNumber: '0.0', + buildNumber: '0.0.1', + tag: 'latest', + status: 'SUCCEEDED', + startedAt: '2024-01-01T00:00:00.000Z', + finishedAt: '2024-01-01T00:00:00.000Z', + imageId: 'fake-image:latest', + }; + const newerRun: RunRecord = { + id: generateId(), + userId: actorRecord.userId, + actorId: actor.id, + buildId: olderBuild.id, + buildNumber: olderBuild.buildNumber, + status: 'SUCCEEDED', + startedAt: '2025-01-01T00:00:00.000Z', + finishedAt: '2025-01-01T00:00:00.000Z', + defaultDatasetId: 'd', + defaultKeyValueStoreId: 'k', + defaultRequestQueueId: 'r', + options: { memoryMbytes: 1024, timeoutSecs: 300 }, + meta: { origin: 'API' }, + }; + await builds.set(olderBuild.id, olderBuild); + await runs.set(newerRun.id, newerRun); + + const page = (await axios.get(`${consoleBaseUrl}/logs`)).data as string; + expect(page.indexOf(newerRun.id)).toBeGreaterThanOrEqual(0); + expect(page.indexOf(newerRun.id)).toBeLessThan(page.indexOf(olderBuild.id)); + }); + it('logs/:id 404s for a nonexistent id and 200s for an owned build or run id (regression: it used to render any id with no ownership/existence check at all, unlike /builds/:id and /runs/:id)', async () => { const missing = await axios.get(`${consoleBaseUrl}/logs/totally-made-up-id-not-in-any-registry`, { validateStatus: () => true, diff --git a/test/unit/console-order.test.ts b/test/unit/console-order.test.ts new file mode 100644 index 0000000..92c0fbe --- /dev/null +++ b/test/unit/console-order.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest'; + +import { newestFirst } from '../../src/console/order.js'; + +describe('newestFirst', () => { + it('sorts descending by startedAt, without mutating the input', () => { + const input = [ + { id: 'a', startedAt: '2024-01-01T00:00:00.000Z' }, + { id: 'b', startedAt: '2024-01-03T00:00:00.000Z' }, + { id: 'c', startedAt: '2024-01-02T00:00:00.000Z' }, + ]; + const sorted = newestFirst(input); + expect(sorted.map((item) => item.id)).toEqual(['b', 'c', 'a']); + expect(input.map((item) => item.id)).toEqual(['a', 'b', 'c']); // original order untouched + }); + + it('breaks a startedAt tie by id ascending, even when the input hands the higher id in first (regression: a plain stable sort with no explicit tiebreak would just preserve this input order instead)', () => { + const tie = '2024-01-01T00:00:00.000Z'; + const input = [ + { id: 'z-tied', startedAt: tie }, + { id: 'a-tied', startedAt: tie }, + ]; + const sorted = newestFirst(input); + expect(sorted.map((item) => item.id)).toEqual(['a-tied', 'z-tied']); + }); + + it('applies the id tiebreak only among equal startedAt values, not globally', () => { + const tie = '2024-01-01T00:00:00.000Z'; + const input = [ + { id: 'z-tied', startedAt: tie }, + { id: 'newer', startedAt: '2024-06-01T00:00:00.000Z' }, + { id: 'a-tied', startedAt: tie }, + ]; + const sorted = newestFirst(input); + expect(sorted.map((item) => item.id)).toEqual(['newer', 'a-tied', 'z-tied']); + }); +}); From 9719aef1008365a8fccdf59e23747c37cf70ec14 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 14:26:33 +0000 Subject: [PATCH 10/10] Correct why the console orders its own listings The comment justified keeping this out of the API's sort helper by saying that helper works on already-paginated DTOs. It does not - at all seven of its call sites it sorts raw records, before pagination and before any DTO conversion. What actually separates the two is the sort direction and the fact that the console and the API are sibling consumers of the service layer, neither importing the other. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G1KN9bXcgAUCyzevGb4viG --- src/console/order.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/console/order.ts b/src/console/order.ts index 59dfa68..4a4bb13 100644 --- a/src/console/order.ts +++ b/src/console/order.ts @@ -1,16 +1,11 @@ /** - * Newest-`startedAt`-first order for every console view that lists builds and/or runs (`console.md`). - * `Registry.list()` - what `listAllBuilds`/`listAllRuns` (`server.ts`) read through - documents its own - * iteration as having "no particular order" (`storage/registry.ts`), so two records sharing the same - * `startedAt` cannot rely on input order to stay put across renders; `id` (unique per record) breaks the - * tie the same way every time, regardless of the order the registry happened to hand them back in. + * Tiebreak for every builds/runs list: `Registry.list()` documents "no particular order" of its own + * (`storage/registry.ts`), so two records sharing a `startedAt` could otherwise swap position between + * renders; `id` (unique per record) breaks the tie the same way every time. * - * Deliberately its own module, not a reuse of the API's `sortByTimestamp` (`api/envelope.ts`): that - * helper sorts ascending, for a different shape of data (already-paginated DTOs, reversed afterwards by - * `paginate`'s own `desc` handling), and reaching for it here would make the console depend on the API - * layer - the two are sibling consumers of `services/`, neither of the other. This two-line comparator is - * cheap enough to keep that boundary intact, and small enough to unit-test on its own (see - * `ansi.ts`/`ansi.test.ts` for the same pattern: a pure console-layer helper, tested directly). + * Not a reuse of the API's `sortByTimestamp` (`api/envelope.ts`): that helper sorts ascending where this + * sorts descending, and reusing it would make the console layer depend on the API layer - the two are + * sibling consumers of `services/`, neither of the other. */ export function newestFirst(items: readonly T[]): T[] { return [...items].sort((a, b) => b.startedAt.localeCompare(a.startedAt) || a.id.localeCompare(b.id));