mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-04 02:14:01 +00:00
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 <[email protected]>
This commit is contained in:
4 files changed
+51
-32
No files matched your search
@@ -12,6 +12,11 @@ WORKDIR /app
|
|||||||
# `npm test` builds fixtures with `git init`, so the suite cannot run in a
|
# `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).
|
# 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/*
|
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 ./
|
COPY package.json package-lock.json ./
|
||||||
RUN npm ci
|
RUN npm ci
|
||||||
|
|||||||
+33
-18
@@ -10,7 +10,7 @@
|
|||||||
* routing table and declares exactly that one address trusted, then starts
|
* routing table and declares exactly that one address trusted, then starts
|
||||||
* Vite the way scripts/pinokio-start.mjs does.
|
* 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 path from 'node:path';
|
||||||
import { fileURLToPath } from 'node:url';
|
import { fileURLToPath } from 'node:url';
|
||||||
import { isDirectInvocation } from './pinokio-install.mjs';
|
import { isDirectInvocation } from './pinokio-install.mjs';
|
||||||
@@ -19,8 +19,6 @@ import { loadViteFromCanonicalRoot } from './pinokio-start.mjs';
|
|||||||
const MODULE_PATH = fileURLToPath(import.meta.url);
|
const MODULE_PATH = fileURLToPath(import.meta.url);
|
||||||
const ROOT = realpathSync(path.resolve(path.dirname(MODULE_PATH), '..'));
|
const ROOT = realpathSync(path.resolve(path.dirname(MODULE_PATH), '..'));
|
||||||
const ROUTE_TABLE = '/proc/net/route';
|
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
|
* 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
|
* A checkout the container cannot write fails quietly: Provider Settings
|
||||||
* inside a container, but build/vite.js reads that as "allow every Host"
|
* answers 500 and `.gev-cache` is never created, with nothing in the log. The
|
||||||
* (allowedHosts: true), dropping the DNS-rebinding protection `npm run dev`
|
* usual cause is a clone made as root on a NAS or Proxmox host while the
|
||||||
* keeps. The published port only ever delivers the host's own browser, so pin
|
* container runs as 1000:1000. Say so at start, naming both sets of ids.
|
||||||
* the same names the native launcher uses, and still honour an operator's
|
* @param {{root: string, writable: boolean, owner: {uid: number, gid: number}, runAs: {uid: number, gid: number}}} input
|
||||||
* GEV_ALLOWED_HOSTS so a LAN hostname stays addable here too. See issue #21.
|
* @returns {string|null} A warning, or null when the checkout is writable.
|
||||||
* @param {NodeJS.ProcessEnv} [env]
|
|
||||||
* @returns {string[]}
|
|
||||||
*/
|
*/
|
||||||
export function resolveAllowedHosts(env = process.env) {
|
export function unwritableRootWarning({ root, writable, owner, runAs }) {
|
||||||
const configured = String(env.GEV_ALLOWED_HOSTS ?? '')
|
if (writable) return null;
|
||||||
.split(',')
|
return (
|
||||||
.map((host) => host.trim())
|
`[Docker] ${root} is owned by ${owner.uid}:${owner.gid} but the container runs as ` +
|
||||||
.filter(Boolean);
|
`${runAs.uid}:${runAs.gid}, so saving keys and caching will fail. Either give ` +
|
||||||
return [...new Set([...LOCAL_ALLOWED_HOSTS, ...configured])];
|
`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() {
|
function readRouteTable() {
|
||||||
@@ -89,6 +87,24 @@ function readRouteTable() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function start() {
|
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() });
|
const trustedPeers = resolveTrustedPeers({ routeTable: readRouteTable() });
|
||||||
if (trustedPeers) {
|
if (trustedPeers) {
|
||||||
// Set before Vite is imported: the dev server snapshots process.env during
|
// Set before Vite is imported: the dev server snapshots process.env during
|
||||||
@@ -109,7 +125,6 @@ async function start() {
|
|||||||
host: '0.0.0.0',
|
host: '0.0.0.0',
|
||||||
port,
|
port,
|
||||||
strictPort: true,
|
strictPort: true,
|
||||||
allowedHosts: resolveAllowedHosts(),
|
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
await server.listen();
|
await server.listen();
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import { readFileSync } from 'node:fs';
|
|||||||
import test from 'node:test';
|
import test from 'node:test';
|
||||||
import {
|
import {
|
||||||
defaultGatewayFromRouteTable,
|
defaultGatewayFromRouteTable,
|
||||||
resolveAllowedHosts,
|
unwritableRootWarning,
|
||||||
resolveTrustedPeers,
|
resolveTrustedPeers,
|
||||||
} from '../scripts/docker-start.mjs';
|
} 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');
|
assert.equal(resolveTrustedPeers({ env: {}, routeTable: '' }), '', 'loopback-only when there is no gateway');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('the container answers the same host names as the native launcher', () => {
|
test('an unwritable checkout is named at start instead of failing quietly', () => {
|
||||||
assert.deepEqual(resolveAllowedHosts({}), ['localhost', '127.0.0.1', '.local'], 'a wildcard bind never means "any Host header"');
|
const ids = { owner: { uid: 0, gid: 0 }, runAs: { uid: 1000, gid: 1000 } };
|
||||||
assert.deepEqual(
|
assert.equal(unwritableRootWarning({ root: '/app', writable: true, ...ids }), null, 'a writable checkout says nothing');
|
||||||
resolveAllowedHosts({ GEV_ALLOWED_HOSTS: ' globe.lan , nas.internal ' }),
|
const warning = unwritableRootWarning({ root: '/app', writable: false, ...ids });
|
||||||
['localhost', '127.0.0.1', '.local', 'globe.lan', 'nas.internal'],
|
assert.match(warning, /owned by 0:0/, 'names the owner');
|
||||||
'an operator can still add a LAN hostname',
|
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.deepEqual(
|
assert.match(warning, /env UID=0 GID=0 docker compose up/, 'or the env form, which bash does not ignore');
|
||||||
resolveAllowedHosts({ GEV_ALLOWED_HOSTS: ' , localhost , ' }),
|
|
||||||
['localhost', '127.0.0.1', '.local'],
|
|
||||||
'blank and duplicate entries change nothing',
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test('the Docker install path keeps its loopback-only, launcher-owned contract', () => {
|
test('the Docker install path keeps its loopback-only, launcher-owned contract', () => {
|
||||||
|
|||||||
@@ -141,7 +141,10 @@ const LOOPBACK_ADDRESSES = new Set(['127.0.0.1', '::1', '::ffff:127.0.0.1']);
|
|||||||
*/
|
*/
|
||||||
function isTrustedPeerAddress(remoteAddress, env) {
|
function isTrustedPeerAddress(remoteAddress, env) {
|
||||||
const normalize = (value) =>
|
const normalize = (value) =>
|
||||||
String(value || '').trim().toLowerCase().replace(/^::ffff:/, '');
|
String(value || '')
|
||||||
|
.trim()
|
||||||
|
.toLowerCase()
|
||||||
|
.replace(/^::ffff:/, '');
|
||||||
const address = normalize(remoteAddress);
|
const address = normalize(remoteAddress);
|
||||||
if (!address) return false;
|
if (!address) return false;
|
||||||
return String(env?.GEV_KEY_SETUP_TRUSTED_PEERS || '')
|
return String(env?.GEV_KEY_SETUP_TRUSTED_PEERS || '')
|
||||||
|
|||||||
Reference in new issue
Block a user