From 79a6a0db5cd86f130f73e2c4d29e32bcd47cf343 Mon Sep 17 00:00:00 2001 From: SM-26 <39513660+SM-26@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:39:32 +0200 Subject: [PATCH] fix(docker): defer to main's Host allowlist and name an unwritable checkout Two findings from PoBruno's Docker Engine on Linux run (#208), and one from re-running the suite in the container after rebasing: - The launcher pinned its own allowedHosts, including '.local'. Vite reads a leading dot as a suffix match, so `Host: anything.local` reached the page and the data proxies. Main has since moved the policy into build/allowedHosts.js, which drops suffix entries on purpose and applies before the app's routes, so the launcher's override is deleted rather than patched. As a side effect GEV_ALLOWED_HOSTS now also works from `.env`, as it does under `npm run dev`. - A checkout the container cannot write - typically a clone made as root on a NAS or Proxmox host - made Save Keys answer 500 and never created .gev-cache, with nothing logged. The launcher now warns at start, naming the owner, the ids it runs as, and the two fixes. - On Docker Desktop the bind-mounted repo always shows an owner other than the container user, so git refuses it as "dubious ownership" and the tests that shell out to git fail (src/data/bundledJson.test.mjs, new on main). The image now marks /app, and only /app, as a safe directory. Verified in a container on this base: `anything.local` 403 on the page and on /api routes, localhost 200, GEV_ALLOWED_HOSTS honoured from the environment and from .env, the warning on a root-owned /app run as 1000:1000, and bundledJson.test.mjs 6/6. Reported-by: PoBruno Co-Authored-By: Claude Opus 5.5 --- Dockerfile | 5 ++++ scripts/docker-start.mjs | 51 ++++++++++++++++++++++++------------- src/dockerLauncher.test.mjs | 22 +++++++--------- src/keySetupCore.mjs | 5 +++- 4 files changed, 51 insertions(+), 32 deletions(-) diff --git a/Dockerfile b/Dockerfile index de89347f9..b1b0876d1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,6 +12,11 @@ WORKDIR /app # `npm test` builds fixtures with `git init`, so the suite cannot run in a # container without it (71 failures on node:24-slim, all ENOENT on git). RUN apt-get update && apt-get install -y --no-install-recommends git && rm -rf /var/lib/apt/lists/* +# The repo is bind-mounted, so inside the container it can show an owner other +# than the user the container runs as - always, on Docker Desktop. git then +# refuses it as "dubious ownership", which fails the tests that shell out to +# git. It is the user's own checkout, so trust that one path. +RUN git config --system --add safe.directory /app COPY package.json package-lock.json ./ RUN npm ci diff --git a/scripts/docker-start.mjs b/scripts/docker-start.mjs index 25ddf76ba..47519af97 100644 --- a/scripts/docker-start.mjs +++ b/scripts/docker-start.mjs @@ -10,7 +10,7 @@ * routing table and declares exactly that one address trusted, then starts * Vite the way scripts/pinokio-start.mjs does. */ -import { readFileSync, realpathSync } from 'node:fs'; +import { accessSync, constants, readFileSync, realpathSync, statSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { isDirectInvocation } from './pinokio-install.mjs'; @@ -19,8 +19,6 @@ import { loadViteFromCanonicalRoot } from './pinokio-start.mjs'; const MODULE_PATH = fileURLToPath(import.meta.url); const ROOT = realpathSync(path.resolve(path.dirname(MODULE_PATH), '..')); const ROUTE_TABLE = '/proc/net/route'; -/** Host names the native launcher accepts; the container matches it. */ -const LOCAL_ALLOWED_HOSTS = Object.freeze(['localhost', '127.0.0.1', '.local']); /** * Extract the IPv4 default gateway from Linux `/proc/net/route` text. The @@ -63,21 +61,21 @@ export function resolveTrustedPeers({ env = process.env, routeTable = '' } = {}) } /** - * Host headers this container answers to. A wildcard bind is unavoidable - * inside a container, but build/vite.js reads that as "allow every Host" - * (allowedHosts: true), dropping the DNS-rebinding protection `npm run dev` - * keeps. The published port only ever delivers the host's own browser, so pin - * the same names the native launcher uses, and still honour an operator's - * GEV_ALLOWED_HOSTS so a LAN hostname stays addable here too. See issue #21. - * @param {NodeJS.ProcessEnv} [env] - * @returns {string[]} + * A checkout the container cannot write fails quietly: Provider Settings + * answers 500 and `.gev-cache` is never created, with nothing in the log. The + * usual cause is a clone made as root on a NAS or Proxmox host while the + * container runs as 1000:1000. Say so at start, naming both sets of ids. + * @param {{root: string, writable: boolean, owner: {uid: number, gid: number}, runAs: {uid: number, gid: number}}} input + * @returns {string|null} A warning, or null when the checkout is writable. */ -export function resolveAllowedHosts(env = process.env) { - const configured = String(env.GEV_ALLOWED_HOSTS ?? '') - .split(',') - .map((host) => host.trim()) - .filter(Boolean); - return [...new Set([...LOCAL_ALLOWED_HOSTS, ...configured])]; +export function unwritableRootWarning({ root, writable, owner, runAs }) { + if (writable) return null; + return ( + `[Docker] ${root} is owned by ${owner.uid}:${owner.gid} but the container runs as ` + + `${runAs.uid}:${runAs.gid}, so saving keys and caching will fail. Either give ` + + `the checkout to that user (chown -R ${runAs.uid}:${runAs.gid} on the host) or ` + + `run as its owner (env UID=${owner.uid} GID=${owner.gid} docker compose up)` + ); } function readRouteTable() { @@ -89,6 +87,24 @@ function readRouteTable() { } async function start() { + let writable = true; + try { + accessSync(ROOT, constants.W_OK); + } catch { + writable = false; + } + if (!writable) { + const { uid, gid } = statSync(ROOT); + console.warn( + unwritableRootWarning({ + root: ROOT, + writable, + owner: { uid, gid }, + runAs: { uid: process.getuid?.() ?? -1, gid: process.getgid?.() ?? -1 }, + }), + ); + } + const trustedPeers = resolveTrustedPeers({ routeTable: readRouteTable() }); if (trustedPeers) { // Set before Vite is imported: the dev server snapshots process.env during @@ -109,7 +125,6 @@ async function start() { host: '0.0.0.0', port, strictPort: true, - allowedHosts: resolveAllowedHosts(), }, }); await server.listen(); diff --git a/src/dockerLauncher.test.mjs b/src/dockerLauncher.test.mjs index 85ac525a4..ae9987b01 100644 --- a/src/dockerLauncher.test.mjs +++ b/src/dockerLauncher.test.mjs @@ -3,7 +3,7 @@ import { readFileSync } from 'node:fs'; import test from 'node:test'; import { defaultGatewayFromRouteTable, - resolveAllowedHosts, + unwritableRootWarning, resolveTrustedPeers, } from '../scripts/docker-start.mjs'; @@ -56,18 +56,14 @@ test('an operator list wins, and no gateway trusts nothing', () => { assert.equal(resolveTrustedPeers({ env: {}, routeTable: '' }), '', 'loopback-only when there is no gateway'); }); -test('the container answers the same host names as the native launcher', () => { - assert.deepEqual(resolveAllowedHosts({}), ['localhost', '127.0.0.1', '.local'], 'a wildcard bind never means "any Host header"'); - assert.deepEqual( - resolveAllowedHosts({ GEV_ALLOWED_HOSTS: ' globe.lan , nas.internal ' }), - ['localhost', '127.0.0.1', '.local', 'globe.lan', 'nas.internal'], - 'an operator can still add a LAN hostname', - ); - assert.deepEqual( - resolveAllowedHosts({ GEV_ALLOWED_HOSTS: ' , localhost , ' }), - ['localhost', '127.0.0.1', '.local'], - 'blank and duplicate entries change nothing', - ); +test('an unwritable checkout is named at start instead of failing quietly', () => { + const ids = { owner: { uid: 0, gid: 0 }, runAs: { uid: 1000, gid: 1000 } }; + assert.equal(unwritableRootWarning({ root: '/app', writable: true, ...ids }), null, 'a writable checkout says nothing'); + const warning = unwritableRootWarning({ root: '/app', writable: false, ...ids }); + assert.match(warning, /owned by 0:0/, 'names the owner'); + assert.match(warning, /runs as 1000:1000/, 'names the ids the container runs as'); + assert.match(warning, /chown -R 1000:1000/, 'offers handing the checkout to the container user first'); + assert.match(warning, /env UID=0 GID=0 docker compose up/, 'or the env form, which bash does not ignore'); }); test('the Docker install path keeps its loopback-only, launcher-owned contract', () => { diff --git a/src/keySetupCore.mjs b/src/keySetupCore.mjs index 73044f003..7ecc58eb8 100644 --- a/src/keySetupCore.mjs +++ b/src/keySetupCore.mjs @@ -141,7 +141,10 @@ const LOOPBACK_ADDRESSES = new Set(['127.0.0.1', '::1', '::ffff:127.0.0.1']); */ function isTrustedPeerAddress(remoteAddress, env) { const normalize = (value) => - String(value || '').trim().toLowerCase().replace(/^::ffff:/, ''); + String(value || '') + .trim() + .toLowerCase() + .replace(/^::ffff:/, ''); const address = normalize(remoteAddress); if (!address) return false; return String(env?.GEV_KEY_SETUP_TRUSTED_PEERS || '')