diff --git a/apps/api/test/e2e/edge-upstream-down-page.e2e.test.ts b/apps/api/test/e2e/edge-upstream-down-page.e2e.test.ts new file mode 100644 index 000000000..fd983d116 --- /dev/null +++ b/apps/api/test/e2e/edge-upstream-down-page.e2e.test.ts @@ -0,0 +1,345 @@ +/** + * The upstream-down page (#556), against a real OpenResty. + * + * Its siblings cover the two halves this cannot: `edge-upstream-down.test.ts` asserts the + * page's bytes survive nginx's tokenizer, and `nginx.test.ts` asserts the handler lands in + * the vhosts that proxy and no others. Both read config TEXT. Neither can answer the two + * questions that actually decide whether the fix works: + * + * - Does OpenResty ACCEPT a project vhost carrying ~2 KB of inlined HTML? Nothing on a PR + * builds `apps/edge/Dockerfile`, so `RUN openresty -t` there would find a broken page at + * release time, on a build whose failure mode is every routed site on the box going down. + * - Does a visitor actually GET the page, with the right status? The handler deliberately + * omits `=` before the named location so an intercepted 504 stays a 504 — a claim about + * nginx's behaviour, not about our string, and one a text assertion cannot make. + * + * The third case here is the one most worth having. `proxy_intercept_errors` is off by + * default, which is why an app that answers 502 ITSELF keeps its own body instead of being + * replaced by our page. That default is inherited, not written down in any vhost, so a + * future `proxy_intercept_errors on;` added for an unrelated reason would silently start + * masking every real error response from every app on the box. This test is what fails then. + * + * Vhosts come from the REAL `NginxProvider` — a hand-written nginx block would prove nothing + * about what a deploy emits. Config text travels as quoted heredocs rather than a bind mount, + * for the reason `edge-not-found-page.e2e.test.ts` documents: a mount needs the daemon to see + * the host path, which is false under Colima and true in CI. + * + * Skips without a reachable daemon, FAILS under RUN_DOCKER_E2E=1 (what CI sets). + * See test/helpers/docker-e2e.ts. + */ + +import { it, expect, beforeAll, afterAll } from "vitest"; +import { readdir, readFile } from "node:fs/promises"; +import { request as httpRequest } from "node:http"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { + DockerRuntime, + EDGE_UPSTREAM_DOWN_SENTINEL, + NginxProvider, + OPENRESTY_DEFAULT_PATHS, + type RootChecked, + type RouteConfig, +} from "@repo/adapters"; +import type Dockerode from "dockerode"; +import { describeDockerE2E, requireDocker } from "../helpers/docker-e2e"; + +const REPO_ROOT = join(dirname(fileURLToPath(import.meta.url)), "../../../.."); +const LUA_SRC = join(REPO_ROOT, "packages/adapters/src/infra/lua"); +const LUA_DEST = "/usr/local/openresty/site/lualib/openship"; +const SITES_DIR = OPENRESTY_DEFAULT_PATHS.sitesDir; +const CONF_PATH = `${OPENRESTY_DEFAULT_PATHS.confDir}/nginx.conf`; +/** Must be under /opt/openship — `assertValidStaticRoot` refuses anything else. */ +const WWW = "/opt/openship/www"; + +/** Ports the envelope's upstream stand-ins listen on. 9911 is deliberately UNUSED. */ +const PORT_DEAD = 9911; +const PORT_HANG = 9912; +const PORT_OWN_502 = 9913; +const PORT_HEALTHY = 9914; + +/** `cat > path <<'EOF'` — quoted delimiter, so nothing in the body is expanded. */ +function heredoc(path: string, content: string): string { + const delim = "OSH_EOF_c3d9"; + if (content.includes(delim)) throw new Error(`heredoc delimiter collides: ${path}`); + return `cat > ${path} <<'${delim}'\n${content}\n${delim}\n`; +} + +/** The image the edge is built FROM, read from the Dockerfile so a base bump can't leave + * this testing a version nothing ships. */ +async function edgeBaseImage(): Promise { + const dockerfile = await readFile(join(REPO_ROOT, "apps/edge/Dockerfile"), "utf8"); + const from = dockerfile.match(/^FROM\s+(\S+)/m)?.[1]; + if (!from) throw new Error("apps/edge/Dockerfile has no FROM line"); + return from; +} + +/** + * Render a vhost with the REAL `NginxProvider`. Only the TRANSPORT is faked — an in-memory + * file map plus the atomic-rename `mv` the provider performs. + */ +async function renderVhost(route: RouteConfig): Promise { + const files = new Map(); + const executor = { + exec: async (command: string): Promise => { + if (/\s-V\b|command -v|which\s/.test(command)) throw new Error("no openresty here"); + const mv = command.match(/^mv '([^']+)' '([^']+)'$/); + if (mv) { + const v = files.get(mv[1]); + if (v !== undefined) { + files.set(mv[2], v); + files.delete(mv[1]); + } + } + return ""; + }, + writeFile: async (p: string, c: string) => void files.set(p, c), + readFile: async (p: string) => { + const c = files.get(p); + if (c === undefined) throw new Error(`ENOENT ${p}`); + return c; + }, + exists: async (p: string) => files.has(p), + mkdir: async () => {}, + rm: async (p: string) => void files.delete(p), + } as unknown as RootChecked; + + const nginx = new NginxProvider({ paths: OPENRESTY_DEFAULT_PATHS, executor }); + await nginx.registerRoute(route); + const conf = [...files.entries()].find(([p]) => p.endsWith(".conf")); + if (!conf) throw new Error(`registerRoute wrote no vhost for ${route.domain}`); + return conf[1]; +} + +interface Answer { + status: number; + body: string; + headers: Record; +} + +/** One request with an explicit `Host`, which `fetch` refuses to set. */ +function ask(port: number, path: string, host: string): Promise { + return new Promise((resolve, reject) => { + const req = httpRequest( + { host: "127.0.0.1", port, path, method: "GET", headers: { Host: host } }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (c: Buffer) => chunks.push(c)); + res.on("error", reject); + res.on("end", () => + resolve({ + status: res.statusCode ?? 0, + body: Buffer.concat(chunks).toString("utf8"), + headers: res.headers, + }), + ); + }, + ); + // Without this a stalled response hangs until vitest's own timeout, which reports as + // "the test took 300s" and names nothing. + req.setTimeout(20_000, () => req.destroy(new Error(`timed out: ${host}${path}`))); + req.on("error", reject); + req.end(); + }); +} + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +describeDockerE2E("edge upstream-down page, real OpenResty", () => { + let runtime: DockerRuntime; + let image = ""; + let lua: Record = {}; + let port = 0; + const started: Dockerode.Container[] = []; + + /** + * `http {}` envelope plus the upstream stand-ins that produce each failure mode. + * + * `PORT_DEAD` is intentionally absent — a refused connection is how nginx generates a 502, + * and it is the exact shape of a container that crashed or has not booted yet. + */ + const ENVELOPE = ` +worker_processes 1; +error_log stderr warn; +events { worker_connections 128; } +http { + include ${OPENRESTY_DEFAULT_PATHS.confDir}/mime.types; + default_type application/octet-stream; + access_log off; + lua_package_path "/usr/local/openresty/site/lualib/?.lua;;"; + + # Accepts the connection and never answers → a read timeout, i.e. nginx's own 504. + server { listen ${PORT_HANG}; location / { content_by_lua_block { ngx.sleep(10) } } } + # An app that answers 502 ITSELF. Its body must reach the client untouched. + server { listen ${PORT_OWN_502}; location / { return 502 "BODY-FROM-APP"; } } + # A healthy app, so "the page never fires on a working request" is observable. + server { listen ${PORT_HEALTHY}; location / { return 200 "BODY-FROM-HEALTHY-APP"; } } + + include ${SITES_DIR}/*.conf; +} +`; + + beforeAll(async () => { + await requireDocker(); + runtime = await DockerRuntime.create({ transport: "socket" }); + image = await edgeBaseImage(); + await runtime.pullImage(image); + // The REAL scripts: a generated vhost references rules_guard/site_logger, and a missing + // file makes OpenResty error on every request instead of routing it. + const names = (await readdir(LUA_SRC)).filter((f) => f.endsWith(".lua")); + lua = Object.fromEntries( + await Promise.all(names.map(async (f) => [f, await readFile(join(LUA_SRC, f), "utf8")])), + ); + port = await bootAll(); + }, 600_000); + + afterAll(async () => { + for (const c of started) await c.remove({ force: true }).catch(() => {}); + await runtime?.dispose().catch(() => {}); + }); + + /** Every vhost under test, one per hostname, in ONE container. */ + async function vhosts(): Promise> { + return { + dead: await renderVhost({ + domain: "dead.test", + tls: false, + targetUrl: `http://127.0.0.1:${PORT_DEAD}`, + }), + // 1s read timeout so the 504 arm costs a second instead of nginx's 60s default. + slow: await renderVhost({ + domain: "slow.test", + tls: false, + targetUrl: `http://127.0.0.1:${PORT_HANG}`, + proxy: { proxyConnectTimeout: "1s", proxyReadTimeout: "1s" }, + }), + appown: await renderVhost({ + domain: "appown.test", + tls: false, + targetUrl: `http://127.0.0.1:${PORT_OWN_502}`, + }), + healthy: await renderVhost({ + domain: "healthy.test", + tls: false, + targetUrl: `http://127.0.0.1:${PORT_HEALTHY}`, + }), + // No upstream at all: the page must not be in this vhost, so a missing file stays the + // plain 404 it has always been. + staticsite: await renderVhost({ domain: "staticsite.test", tls: false, staticRoot: WWW }), + }; + } + + /** Boot one container holding every vhost; return the published :80 port. */ + async function bootAll(): Promise { + const confs = await vhosts(); + const script = + `set -e\n` + + `mkdir -p ${SITES_DIR} ${LUA_DEST} ${WWW} /var/www/acme/oblien\n` + + heredoc(CONF_PATH, ENVELOPE) + + Object.entries(lua) + .map(([name, body]) => heredoc(join(LUA_DEST, name), body)) + .join("") + + heredoc(`${WWW}/index.html`, "STATIC-INDEX") + + Object.entries(confs) + .map(([name, body]) => heredoc(join(SITES_DIR, `${name}.conf`), body)) + .join("") + + // The pre-merge equivalent of the Dockerfile's gate: a page that broke the tokenizer + // fails HERE, with its own [emerg] line, instead of at release time. + `openresty -t\n` + + `exec openresty -g 'daemon off;'\n`; + + const container = await runtime.docker.createContainer({ + Image: image, + Entrypoint: ["sh", "-c"], + Cmd: [script], + Tty: true, + ExposedPorts: { "80/tcp": {} }, + HostConfig: { + PortBindings: { "80/tcp": [{ HostIp: "127.0.0.1", HostPort: "" }] }, + AutoRemove: false, + }, + }); + started.push(container); + await container.start(); + + const info = await container.inspect(); + const bound = info.NetworkSettings.Ports?.["80/tcp"]?.[0]?.HostPort; + if (!bound) throw new Error("docker published no host port for 80/tcp"); + const published = Number(bound); + + // Ready when it answers, dead when it exits — and a dead edge must report the reason + // OpenResty gave, not a timeout. + for (let i = 0; i < 60; i++) { + const state = await container.inspect(); + if (!state.State.Running) { + const log = (await container.logs({ stdout: true, stderr: true, tail: 40 })).toString(); + throw new Error(`edge exited (${state.State.ExitCode}):\n${log}`); + } + try { + await ask(published, "/", "healthy.test"); + return published; + } catch { + await sleep(250); + } + } + throw new Error("edge never answered on :80"); + } + + it("a refused upstream answers 502 with OUR page, not OpenResty's", async () => { + const res = await ask(port, "/", "dead.test"); + expect(res.status).toBe(502); + expect(res.body).toContain(EDGE_UPSTREAM_DOWN_SENTINEL); + expect(res.body).toContain("Application unavailable"); + // The stock page is what the report was about: it is what a Cloud-fronted visitor saw + // on the operator's own domain. + expect(res.body).not.toContain("
502 Bad Gateway
"); + expect(res.body).not.toMatch(/openresty\/\d/); + // No link at all — the whole point of the report. + expect(res.body).not.toMatch(/https?:\/\//); + expect(res.body).not.toContain(" { + // With `error_page 502 504 = @loc;` this would arrive as whatever the named location's + // own `return` says, and a timeout would stop being reportable as a timeout. This is the + // assertion that pins the semantics in the real server rather than in a comment. + const res = await ask(port, "/", "slow.test"); + expect(res.status).toBe(504); + expect(res.body).toContain(EDGE_UPSTREAM_DOWN_SENTINEL); + }, 60_000); + + it("an app's OWN 502 passes through untouched", async () => { + // `proxy_intercept_errors` is off by default and set nowhere in this repo. If that ever + // changes, this page starts masking every real error response from every app on the box + // — and this is the test that catches it. + const res = await ask(port, "/", "appown.test"); + expect(res.status).toBe(502); + expect(res.body).toBe("BODY-FROM-APP"); + expect(res.body).not.toContain(EDGE_UPSTREAM_DOWN_SENTINEL); + }); + + it("a healthy app is untouched", async () => { + const res = await ask(port, "/", "healthy.test"); + expect(res.status).toBe(200); + expect(res.body).toBe("BODY-FROM-HEALTHY-APP"); + }); + + it("answers text/html even when the URI looks like a stylesheet", async () => { + // `return` with a body types the response from the request URI, so without the empty + // `types { }` map a down app's `/app.css` would be `Content-Type: text/css` holding HTML + // — which browsers drop silently instead of rendering. + const res = await ask(port, "/app.css", "dead.test"); + expect(res.status).toBe(502); + expect(String(res.headers["content-type"])).toContain("text/html"); + expect(res.body).toContain("Application unavailable"); + }); + + it("a static vhost never serves it — a missing file is still a plain 404", async () => { + // The gate in nginx.ts: a route that serves from disk has no upstream, so intercepting + // 5xx there would be dead config on every static site. + const res = await ask(port, "/nope", "staticsite.test"); + expect(res.body).not.toContain(EDGE_UPSTREAM_DOWN_SENTINEL); + // The SPA fallback serves index.html for an unknown path; either way, never our page. + expect([200, 404]).toContain(res.status); + }); +}); diff --git a/packages/adapters/src/index.ts b/packages/adapters/src/index.ts index c874e850a..74a69e23f 100644 --- a/packages/adapters/src/index.ts +++ b/packages/adapters/src/index.ts @@ -143,6 +143,9 @@ export { scopedVolumeName, scopeVolumeBinds, isHostPathSource } from "./runtime/ // ─── Infrastructure layer ──────────────────────────────────────────────────── export type { RoutingProvider, SslProvider } from "./infra/types"; export { NginxProvider, type NginxProviderOptions, type RateLimitConfig } from "./infra/nginx"; +// For the upstream-down e2e in apps/api: it asserts on the real marker rather than a copy of +// the string, which could drift from the page it is checking for. +export { EDGE_UPSTREAM_DOWN_SENTINEL } from "./infra/edge-upstream-down"; export { compileVercelRouting, sourceToLocation, diff --git a/packages/adapters/src/infra/edge-upstream-down.test.ts b/packages/adapters/src/infra/edge-upstream-down.test.ts new file mode 100644 index 000000000..990c8d212 --- /dev/null +++ b/packages/adapters/src/infra/edge-upstream-down.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, test } from "vitest"; +import { + EDGE_UPSTREAM_DOWN_HANDLER, + EDGE_UPSTREAM_DOWN_HTML, + EDGE_UPSTREAM_DOWN_LOCATION_NAME, + EDGE_UPSTREAM_DOWN_SENTINEL, +} from "./edge-upstream-down"; +import { stripComments, extractBlocks } from "../system/proxy/import/parse-utils"; + +/** + * The page is embedded in an nginx `return 502 ''` inside every proxying vhost, so + * the config tokenizer is part of its contract. These are not style checks: each one of + * them, violated, produces a config that fails `openresty -t` — which means a + * crash-looping fresh edge, or an existing edge that refuses this and every LATER reload, + * freezing route changes for every site on the box. + * + * The reasons are the same ones `edge-not-found.test.ts` documents at length; this file + * repeats the assertions rather than the essays, because the two pages are written and + * edited independently and a shared helper would let one drift. + */ +describe("edge upstream-down page", () => { + test("contains no single quote — it would end the nginx token mid-page", () => { + expect(EDGE_UPSTREAM_DOWN_HTML).not.toContain("'"); + }); + + test("contains no $ — nginx interpolates variables in a return body", () => { + expect(EDGE_UPSTREAM_DOWN_HTML).not.toContain("$"); + }); + + test("is a single line — both writers must serve identical bytes", () => { + expect(EDGE_UPSTREAM_DOWN_HTML).not.toContain("\n"); + }); + + test("contains no # — Openship's OWN config reader cuts lines at one", () => { + // Use rgb() for colours. A CSS hex triplet here truncates the line mid-page in + // `stripComments`, taking its closing braces with it and breaking the brace balance + // for every vhost the reader sees after it. + expect(EDGE_UPSTREAM_DOWN_HTML).not.toContain("#"); + }); + + test("keeps its braces balanced — the same reader counts them", () => { + const open = (EDGE_UPSTREAM_DOWN_HTML.match(/\{/g) ?? []).length; + const close = (EDGE_UPSTREAM_DOWN_HTML.match(/\}/g) ?? []).length; + expect(open).toBe(close); + }); + + test("survives a round trip through the reader that parses these files back", () => { + // The end-to-end version of the two above, against a vhost shaped like the ones the + // generator emits: the handler sits at server scope, so a brace or comment character + // escaping the page would swallow the rest of the block and the box would scan as + // fewer sites than it serves. This is the assertion whose absence let #431's fix take + // the migrate scan down with it. + const conf = `# Auto-generated by Openship - do not edit manually +server { + listen 80; + server_name app.example.com; + +${EDGE_UPSTREAM_DOWN_HANDLER} + + location / { + proxy_pass http://127.0.0.1:3009; + } +} + +server { + listen 443 ssl; + server_name app.example.com; + + location / { + proxy_pass http://127.0.0.1:3009; + } +} +`; + expect(extractBlocks(stripComments(conf), "server")).toHaveLength(2); + }); + + test("carries the sentinel in the first 200 bytes", () => { + // So `curl -s https://host | head -c 200` says "our edge, upstream down" rather than + // leaving an operator to guess whose 502 they are looking at. + expect(EDGE_UPSTREAM_DOWN_HTML.indexOf(EDGE_UPSTREAM_DOWN_SENTINEL)).toBeGreaterThan(-1); + expect(EDGE_UPSTREAM_DOWN_HTML.indexOf(EDGE_UPSTREAM_DOWN_SENTINEL)).toBeLessThan(200); + }); + + test("makes no external request and carries no link at all", () => { + // The report this page fixes is a THIRD PARTY's link on an operator's error page, so + // the absence of links is the fix, not a detail. It is also served to strangers, so a + // third-party fetch would leak their visit and a dashboard URL would advertise + // infrastructure. + expect(EDGE_UPSTREAM_DOWN_HTML).not.toMatch(/https?:\/\//); + expect(EDGE_UPSTREAM_DOWN_HTML).not.toContain(" { + // `$host` is attacker-chosen and nginx's host validation permits `<`, `>` and `"`. + // The `$` assertion above covers the mechanism; this names the reason so it is not + // "fixed" by escaping and re-adding it. + expect(EDGE_UPSTREAM_DOWN_HANDLER).not.toContain("$host"); + }); + + test("answers text/html regardless of the URI's extension", () => { + // `return` with a body types the response from the request URI, so a down app's + // `/app.css` would be `Content-Type: text/css` with an HTML body. Clearing the map is + // what makes `default_type` apply. + const typesAt = EDGE_UPSTREAM_DOWN_HANDLER.indexOf("types { }"); + const defaultAt = EDGE_UPSTREAM_DOWN_HANDLER.indexOf("default_type text/html;"); + expect(typesAt).toBeGreaterThan(-1); + expect(defaultAt).toBeGreaterThan(typesAt); + }); + + test("intercepts 502 and 504 but NEVER 503", () => { + // 503 is reachable on purpose: `blockStatus` (403) and `rateLimit.status` (429) are + // operator-overridable, and `limit_req_status` is 429. Branding an operator's + // deliberate block "application unavailable" would report it as an outage. + expect(EDGE_UPSTREAM_DOWN_HANDLER).toContain( + `error_page 502 504 ${EDGE_UPSTREAM_DOWN_LOCATION_NAME};`, + ); + expect(EDGE_UPSTREAM_DOWN_HANDLER).not.toMatch(/error_page[^;]*\b503\b/); + }); + + test("passes the intercepted code through — no `=` before the named location", () => { + // Verified against openresty 1.27.1.1: with `=` the named location's own return code + // replaces the original, collapsing a real 504 into whatever it says. Without it the + // intercepted code survives and one body serves both. A refactor that "tidies" this + // into `= @osh_upstream_down` silently stops 504s being reportable as 504s. + expect(EDGE_UPSTREAM_DOWN_HANDLER).not.toContain(`= ${EDGE_UPSTREAM_DOWN_LOCATION_NAME}`); + }); + + test("is reachable only from error_page — the location name is @-prefixed", () => { + // A named location cannot be selected by a request URI, so a visitor can never ask + // for the error page directly and the handler can never shadow a real path. + expect(EDGE_UPSTREAM_DOWN_LOCATION_NAME.startsWith("@")).toBe(true); + expect(EDGE_UPSTREAM_DOWN_HANDLER).toContain(`location ${EDGE_UPSTREAM_DOWN_LOCATION_NAME} {`); + }); + + test("does not set server_tokens — that would be an http-wide side effect", () => { + // The 404 page carries it scoped to the catch-alls on purpose. This handler is + // emitted into EVERY proxying vhost, so carrying it here would change the `Server:` + // header for every managed site on the box as a side effect of an error page. + expect(EDGE_UPSTREAM_DOWN_HANDLER).not.toContain("server_tokens"); + }); + + test("says the address is routed and the app is not answering", () => { + // The one thing the page exists to communicate, and the distinction the stock + // OpenResty 502 destroys — "nothing is deployed here" (the 404 page) vs "something + // is, and it is not answering" (this one). + expect(EDGE_UPSTREAM_DOWN_HTML).toContain("Application unavailable"); + expect(EDGE_UPSTREAM_DOWN_HTML).toContain("not responding"); + expect(EDGE_UPSTREAM_DOWN_HTML).toContain("noindex"); + }); + + test("names no status code in the body — one page answers 502 and 504", () => { + // The status line carries the real code. A "502" printed in the tag would be wrong + // every time the handler was reached by a read timeout. + expect(EDGE_UPSTREAM_DOWN_HTML).not.toContain("502"); + expect(EDGE_UPSTREAM_DOWN_HTML).not.toContain("504"); + }); +}); diff --git a/packages/adapters/src/infra/edge-upstream-down.ts b/packages/adapters/src/infra/edge-upstream-down.ts new file mode 100644 index 000000000..ec627e902 --- /dev/null +++ b/packages/adapters/src/infra/edge-upstream-down.ts @@ -0,0 +1,171 @@ +/** + * The page the edge serves for a hostname it DOES route, whose upstream did not answer + * (#556) — the sibling of {@link ./edge-not-found.ts} and built the same way. + * + * Before this, the edge defined no `error_page` for any 5xx at all, so a container that + * was restarting, crashed, or still booting produced OpenResty's stock + * `502 Bad Gateway` / `openresty/1.27.1.1` page. On a free `*.opsh.io` host the visitor + * ended up looking at a page carrying the CLOUD provider's branding and links — a third + * party, from the point of view of the operator whose own domain was requested. That is + * the report this fixes: whatever the box answers with is what the visitor sees, because + * Openship Cloud's shared edge forwards by IP with the `Host:` header and relays the + * box's response (see `edge-target.ts`). + * + * It cannot cover every 502. When the BOX ITSELF is unreachable, the Cloud edge answers + * on its own and no page here is involved — that case needs a Cloud-side change and is + * named as a known limitation on #556, not silently implied to be fixed. + * + * WHY THE BODY IS INLINE IN THE CONFIG rather than a file the block `root`s to, and why + * the HTML has to survive nginx's config tokenizer (no `'`, no `$`, no newline, no `#`, + * balanced braces) — see the header of `edge-not-found.ts`. Every reason there applies + * here unchanged, and each invariant is checked by a test, because violating one produces + * a config that fails `openresty -t`: the reload is gated on it, so NO route change lands + * on the box until someone finds it, and on a fresh container it is a crash loop. + * + * One reason is NEW here, and it is why an `include` of a single shared snippet was + * rejected even though it would avoid repeating this body per vhost: a missing `include` + * is itself a hard `-t` failure. The inline form cannot half-arrive; an include can, and + * the blast radius is every later reload on the box, not just this page. + * + * Deliberately NOT on the page: any link, the project name, the operator's dashboard URL, + * and `$host`. This is served to strangers — the same audience as the 404 page — so it + * must not advertise infrastructure, and reflecting `$host` would be a reflected-XSS sink + * (nginx's host validation permits `<`, `>` and `"`). Removing the third-party link IS + * the fix; re-pointing it at the operator's own domain would only link to the site that + * is currently down. + * + * Deliberately NOT here either: `server_tokens off`. The 404 page carries it scoped to + * the catch-alls, and its docblock explains why it is not set at `http` level — doing so + * would change the `Server:` header for every managed vhost on the box as a side effect + * of an error page. This handler is emitted into every proxying vhost, so carrying it + * would be exactly that side effect. The stock `Server:` header is a separate decision + * from the page body. + */ + +/** + * Machine-readable marker that THIS page — our edge, for a routed host whose upstream is + * down — is what answered, rather than some other 502. Sits immediately after the doctype + * so `curl -s https://host | head -c 200` shows it, and distinct from the unrouted page's + * sentinel so the two cases are told apart at a glance: "nothing is deployed at this + * name" vs "something is, and it is not answering". + * + * A support/diagnostic marker, and ONLY that. Nothing branches on it. + */ +export const EDGE_UPSTREAM_DOWN_SENTINEL = "openship-edge-upstream-down"; + +/** The named location the `error_page` redirects into. `@`-prefixed, so it is reachable + * only from `error_page` and can never be selected by a request URI. */ +export const EDGE_UPSTREAM_DOWN_LOCATION_NAME = "@osh_upstream_down"; + +/** + * The page, in fragments. Joined with no separator, so every fragment must be + * self-contained at its boundaries (a break only ever falls between tags or between CSS + * declarations — never inside a text node, where the missing newline would run two words + * together). + */ +const HTML_FRAGMENTS: readonly string[] = [ + ``, + ``, + ``, + ``, + ``, + ``, + // A transient outage must never be the version of the page that enters an index. + ``, + `Application unavailable`, + ``, + ``, + ``, + `
`, + // No status number in the tag: this one page answers both 502 and 504, and nginx keeps + // the real code on the status line (see the handler below). Naming one of them here + // would be wrong half the time. + `upstream not responding`, + `

Application unavailable

`, + // "routed, but not answering" is the whole message — the distinction the stock page + // destroys, and the mirror of the 404 page separating "nothing is deployed here". + `

This address is configured, but the application behind it is not responding.

`, + `
    `, + `
  • The deployment may be starting up or restarting
    • `, + `
  • The application may have stopped or crashed
    • `, + `
  • Trying again in a moment may work
    • `, + `
`, + ``, + `
`, + ``, + ``, +]; + +/** The page as one line, ready to be embedded in a single-quoted nginx token. */ +export const EDGE_UPSTREAM_DOWN_HTML = HTML_FRAGMENTS.join(""); + +/** + * The `error_page` + named location that serve it, emitted at SERVER scope in every + * generated vhost that proxies to an upstream. Server scope rather than inside + * `location /` so the extra locations a compiled `vercel.json` adds + * (`renderProxyLocations`) are covered by the same handler. + * + * `502 504` and deliberately NOT `503`. 502 is nginx failing to reach the upstream and + * 504 is the upstream accepting and never answering — both are "the app is not serving". + * 503 is reachable on purpose: `blockStatus` (default 403) and `rateLimit.status` + * (default 429) are operator-overridable, and `limit_req_status` is set to 429 in + * `nginx.ts`. If an operator points either of those at 503, branding it "application + * unavailable" would report a deliberate block as an outage. + * + * NO `=` before the location name, which is load-bearing and was verified against + * `openresty/openresty:1.27.1.1-alpine` rather than assumed: + * - `error_page 502 504 @loc;` → the ORIGINAL code survives (502 stays 502, a real + * read-timeout stays 504) and the body is ours. One shared body covers both codes. + * - `error_page 502 504 = @loc;` → the named location's own `return` code replaces it, + * so both collapse to whatever that says and a 504 stops being reportable as one. + * The `return 502` below therefore does NOT set the response status — the intercepted + * error does. It is written as 502 so the directive reads honestly on its own; a probe + * with `return 503` there still produced 502 and 504 on the wire. + * + * `types { }` before `default_type`, because `return` with a body picks the content type + * from the request URI's extension and only falls back to `default_type`. Without the + * empty map, a request for a down app's `/app.css` would be answered + * `Content-Type: text/css` with an HTML body. + * + * An app's OWN 502/504 is untouched by this: `proxy_intercept_errors` is off (nginx's + * default, and never set anywhere in this repo), so an upstream that answers 502 itself + * has its own body passed straight through. Verified as case D of the same probe — this + * page never hijacks a real response from a running app. + */ +export const EDGE_UPSTREAM_DOWN_HANDLER = `\ + error_page 502 504 ${EDGE_UPSTREAM_DOWN_LOCATION_NAME}; + + location ${EDGE_UPSTREAM_DOWN_LOCATION_NAME} { + types { } + default_type text/html; + return 502 '${EDGE_UPSTREAM_DOWN_HTML}'; + }`; diff --git a/packages/adapters/src/infra/nginx.test.ts b/packages/adapters/src/infra/nginx.test.ts index a25998b53..3154d449f 100644 --- a/packages/adapters/src/infra/nginx.test.ts +++ b/packages/adapters/src/infra/nginx.test.ts @@ -516,6 +516,88 @@ describe("NginxProvider.serveEdgeChallenge", () => { }); }); +/** + * #556 — a routed host whose app is not answering used to get OpenResty's stock 502. That + * matters beyond looks: Openship Cloud's shared edge forwards to the box and relays the + * box's response, so the stock page is what a visitor to the operator's OWN domain reads, + * branded for a third party. The page only belongs in vhosts where something can 502. + */ +describe("NginxProvider upstream-down page", () => { + const handlerCount = (c: string) => (c.match(/location @osh_upstream_down \{/g) ?? []).length; + + test("a proxy vhost carries it in BOTH serving blocks", async () => { + // A named location is server-scoped, so a block missing it would answer the + // `error_page` with a 500 instead of the page. + const { nginx, conf } = setup({ certDomains: ["app.example.com"] }); + await nginx.registerRoute(PROXY); + const c = conf("app-example-com")!; + expect(handlerCount(c)).toBe(2); + // And the body actually ships, rather than an empty handler that yields a blank 502. + expect(c).toContain("openship-edge-upstream-down"); + expect(c).toContain("Application unavailable"); + }); + + test("the error_page sits at SERVER scope, not inside location /", async () => { + // Server scope is what makes it cover the extra locations a compiled vercel.json adds. + // Indentation is the readable proxy for scope here: 4 spaces is the server block. + const { nginx, conf } = setup(); + await nginx.registerRoute(PROXY); + expect(conf("app-example-com")!).toMatch(/^ {4}error_page 502 504 @osh_upstream_down;$/m); + }); + + test("intercepts 502 and 504 only — never 503", async () => { + // `blockStatus` and `rateLimit.status` are operator-overridable and `limit_req_status` + // is 429; a 503 arm would brand a deliberate block as an outage. + const { nginx, conf } = setup(); + await nginx.registerRoute(PROXY); + const line = conf("app-example-com")!.match(/^ {4}error_page .*$/m)![0]; + expect(line).toContain("502"); + expect(line).toContain("504"); + expect(line).not.toContain("503"); + }); + + test("passes the intercepted code through — no `=` before the named location", async () => { + // Verified against openresty 1.27.1.1: `= @loc` makes the named location's own return + // code replace the original, collapsing a real 504 into a 502. + const { nginx, conf } = setup(); + await nginx.registerRoute(PROXY); + expect(conf("app-example-com")!).not.toContain("= @osh_upstream_down"); + }); + + test("a host redirect carries none — it has no upstream to be down", async () => { + const { nginx, conf } = setup({ certDomains: ["www.example.com"] }); + await nginx.registerRoute({ + ...OURS, + domain: "www.example.com", + redirectHost: { target: "example.com", statusCode: 301 }, + }); + expect(handlerCount(conf("www-example-com")!)).toBe(0); + }); + + test("a static vhost carries none — it serves from disk", async () => { + const { nginx, conf } = setup(); + await nginx.registerRoute({ + domain: "site.example.com", + tls: false, + staticRoot: "/opt/openship/static/site/dist", + }); + expect(handlerCount(conf("site-example-com")!)).toBe(0); + }); + + test("a static vhost WITH vercel.json proxy locations does carry it", async () => { + // The case a `!staticRoot` gate would have missed: the disk root cannot 502, but the + // `/api/` location proxying to a real upstream can. + const { nginx, conf } = setup(); + await nginx.registerRoute({ + domain: "hybrid.example.com", + tls: false, + staticRoot: "/opt/openship/static/hybrid/dist", + proxyLocations: [{ pathPrefix: "/api/", targetUrl: "http://10.0.0.5:3000" }], + }); + expect(handlerCount(conf("hybrid-example-com")!)).toBe(1); + }); +}); + describe("NginxProvider config generation", () => { test("proxy route with no cert yet → HTTP-only block", async () => { const { nginx, conf, files } = setup(); diff --git a/packages/adapters/src/infra/nginx.ts b/packages/adapters/src/infra/nginx.ts index 7c761abfc..3e67702aa 100644 --- a/packages/adapters/src/infra/nginx.ts +++ b/packages/adapters/src/infra/nginx.ts @@ -35,6 +35,7 @@ import type { RoutingProvider, SslProvider } from "./types"; import { LUA_LOGGER_PATH, RULES_GUARD_PATH, luaSourceAvailable, buildReloadCommand, detectOpenRestyPaths, ACME_HTTP01_PORT, ACME_CHALLENGE_LOCATION, EDGE_CHALLENGE_DIR, EDGE_CHALLENGE_LOCATION, EDGE_CHALLENGE_URL_PREFIX, EDGE_SAME_PATH_MOUNTS, OPENRESTY_DEFAULT_PATHS, edgeChallengeVhostConf, type OpenRestyPaths } from "./openresty-lua"; import { safeErrorMessage, sanitizeProxySettings, resolveRedirectStatus, PROXY_DIRECTIVES, parseProxyValue, resolveProxyDirectives, type NginxVersion, type ProxySettings } from "@repo/core"; import { cloudEdgeRealIpConf, isCloudFrontedHost } from "./edge-real-ip"; +import { EDGE_UPSTREAM_DOWN_HANDLER } from "./edge-upstream-down"; import { sq } from "../system/local-shell"; import type { RootChecked } from "../system/privilege"; import { edgeDownExplanation } from "../system/edge-exec-error"; @@ -158,8 +159,11 @@ const FORWARD_VARS = ` set $openship_fwd_proto $scheme; * 2 — `proxy_pass_header X-Accel-Buffering`, so an upstream's no-buffering instruction * survives the second proxy hop. Vhosts written before it stall SSE behind Cloud's * edge (GH-570). + * 3 — the upstream-down `error_page` handler ({@link EDGE_UPSTREAM_DOWN_HANDLER}). Vhosts + * written before it serve OpenResty's stock 502 — which, behind Openship Cloud's + * edge, is a page branded for a third party on the operator's own domain (#556). */ -export const VHOST_GENERATION = 2; +export const VHOST_GENERATION = 3; /** Marker line carrying {@link VHOST_GENERATION}, matched by {@link readVhostGeneration}. */ const GENERATION_MARKER = `# openship-vhost-gen: ${VHOST_GENERATION}`; @@ -1726,10 +1730,22 @@ export class NginxProvider implements RoutingProvider, SslProvider { ? `\n\n${renderSlashFallback(route.staticRoot)}` : ""; + // The upstream-down page (#556), appended to every block that serves — same reason as + // `slashFallback` above: it is a named location, so it has to exist in whichever server + // block the `error_page` fired in. + // + // Only where something can actually 502. A host redirect has no upstream at all, and a + // static route serves from disk — EXCEPT when a compiled `vercel.json` gave it proxy + // locations, which is why this tests for an upstream rather than for `!staticRoot`. + const proxiesUpstream = + !hostRedirect && + (!("staticRoot" in route && route.staticRoot) || (route.proxyLocations?.length ?? 0) > 0); + const upstreamDown = proxiesUpstream ? `\n\n${EDGE_UPSTREAM_DOWN_HANDLER}` : ""; + // `location /` for a block that serves the app. const serveLocation = ` location / { ${redirectRules}${urlShape}${locationBody} - }${slashFallback}`; + }${slashFallback}${upstreamDown}`; // `location /` for the :80 block of a route that has a real cert: send the // visitor to https — unless a CDN already terminated TLS and reached us on @@ -1745,7 +1761,7 @@ export class NginxProvider implements RoutingProvider, SslProvider { ? serveLocation : ` location / { ${HTTPS_UPGRADE}${redirectRules}${urlShape}${locationBody} - }${slashFallback}`; + }${slashFallback}${upstreamDown}`; // Everything both server blocks share, after the per-block preamble. const sharedBody = `${serverHeaders}${proxyOpts}