diff --git a/AGENTS.md b/AGENTS.md index c5326d1..08ffcb4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,6 +81,16 @@ sources must stay erasable (`erasableSyntaxOnly` is on: no enums, no parameter p container can create one. Compare real paths (`realpathSync`), and ask what else can make a path point elsewhere: the first fix for this was a prefix check, and the bypass was found by a review bot asking the question the fix had not. + + **The other half is what it compares against.** The directories shared with the container are the + user's to name, so there is no list to hard-code. + A process inside the container reads its own mount table (`/proc/self/mountinfo`); a host process asks + a container runtime — docker, podman or nerdctl, whichever is there — for the paths its containers + bind-mount, running or stopped, which is a superset on purpose: any container that can rewrite the + broker's code is as good a reason not to stage there as the agent is. A runtime that knows of no + container is not an answer — that is exactly when the check is blind, since the agent container may + simply be stopped with its mounts still configured — so it refuses rather than guesses, as it does + when nothing can answer at all. - **The process that owns a boundary decides what crosses it, not the error class.** A response body put into the message of a `ProviderConfigError` reached the container, because a caller that wraps that error to name a missing permission wraps it in the same class and so passes the whole message through. Detail @@ -164,8 +174,10 @@ sources must stay erasable (`erasableSyntaxOnly` is on: no enums, no parameter p is still the broker — `--via-host` is asked for, never assumed. The opener is started once per machine, not once per repository: it discovers the checkouts under - its `--root`s (default `$HOME`, so `~/Workspaces`, `~/.dotfiles` and anything added later are - served). The paths on the two sides never have to match — each side reads and writes its own view + its `--root`s (default `$HOME`) within `--depth` levels of each (4 by default), so anything added + later is picked up by the next sweep. The bound is deliberate — the walk repeats every `--interval` + seconds — and `--depth` raises it for a deeper layout. The paths on the two sides never have to + match — each side reads and writes its own view of the same `.git` directory — so only the roots it scans are host-side paths. A request is served in the checkout it was written in. That is a mechanism, not a boundary: the container can edit any checkout it can write, its remote included, so what a deployment controls is which roots are served and which program runs. @@ -229,14 +241,16 @@ Full detail and the measurements behind them: - **Egress requires the proxy.** Direct connections to github.com hang rather than fail. The credential channel is a unix socket, so it is unaffected — that is why it is a socket. - **The container is recreated often**, so its writable layer is ephemeral. Anything that must - survive belongs in compose `environment:` or on a mounted path. `~/Workspaces`, `~/.dsh` and - `~/.dotfiles` are mounts; `/home/app` itself is not; `/volume1` does not exist inside. + survive belongs in compose `environment:` or on a mounted path. Which directories this deployment + mounts is the deployment's business — the tool discovers them rather than knowing them — and + `/home/app` itself is not one of them, nor does `/volume1` exist inside. - **Do not rely on the executable bit in commits**: this share's ACLs defeat git's exec-bit detection, so every sibling repository has `core.fileMode=false`. The exec bit for the CLI entry points is applied by `scripts/postbuild.mjs` at build time, and npm sets it for the installed bins, so nothing depends on it being tracked. -- **`~/.dsh` lives *inside* the `~/Workspaces` mount**, so "put it in `~/.dsh`" means "put it on - the shared NAS share". Never treat it as a private location for secrets. +- **A directory the container shares is not a private location**, even one that looks like the + tool's own home: this deployment's socket directory lives inside another shared directory, so + treating it as private would put a secret on the share. ## Where things are diff --git a/README.md b/README.md index 95ab39e..d84bf54 100644 --- a/README.md +++ b/README.md @@ -103,7 +103,7 @@ read-only mount, so nothing is fetched or built at boot. ```sh DEPLOY=/srv/git-cred-broker # broker.config.json, app.pem and log/ live here -SOCKET_DIR=~/.dsh/git-broker # a host directory the pushing container already mounts +SOCKET_DIR=/srv/git-broker-socket # a host directory the pushing container already mounts git-credential-broker init --mode sidecar --dir "$DEPLOY" \ --cert ~/Downloads/app.private-key.pem \ @@ -115,6 +115,15 @@ git-credential-broker compose --code "$DEPLOY/app" --socket-dir "$SOCKET_DIR" \ docker compose -f "$DEPLOY/docker-compose.broker.yml" up -d ``` +Nothing above has to name the shared directories. A host process asks the container runtime — docker, +podman or nerdctl, whichever is there — for the host paths its containers bind-mount, running or +stopped; a process inside the container reads `/proc/self/mountinfo`. When neither can answer — no +runtime, a runtime that knows of no container at all, or a host whose only runtime is plain +`containerd`, which has no interface this knows — `init`, `stage` and `authorize` refuse rather than +guess. The runtime route is deliberately a superset: a directory bind-mounted into *any* container +counts as shared, because any container that can rewrite the broker's code is as good a reason not to +stage there as the agent is. + `stage` prints the commit it exported; check that sha against your own clone or the remote before trusting it, since the repository you export from is one an agent can write to. It leaves a `STAGED.json` beside the code recording where it came from. @@ -157,7 +166,7 @@ when the image has none. In compose: - GIT_BROKER_SOCKET=/run/git-broker/broker.sock - GIT_BROKER_REQUIRE=1 volumes: - - ~/.dsh/git-broker:/run/git-broker + - /srv/git-broker-socket:/run/git-broker ``` ## Everyday changes @@ -284,8 +293,10 @@ git-credential-broker pr --via-host --repo owner/repo --head feature --base main The opener creates pull requests with the credentials of whoever started it, which is the point; it never runs a shell, and the program it calls is configuration rather than a hard-coded `gh`: `--command /path/to/gh`, or `$GIT_BROKER_PR_COMMAND`. It cannot push, merge, close or read -anything. `--root` may be repeated and defaults to `$HOME`, so `~/Workspaces`, `~/.dotfiles` and -checkouts created later are all served with no further setup. To start it once and keep it, +anything. `--root` may be repeated and defaults to `$HOME`; checkouts within `--depth` levels of a root +(4 by default) are served with no further setup, including ones created later, which the next sweep +picks up. The bound exists because the walk repeats every `--interval` seconds; raise `--depth` for +deeper layouts. To start it once and keep it, `examples/host-opener.service` is a systemd user unit; on a system without user services, `examples/host-opener-boot.sh` is the same thing for a DSM boot-up task — absolute paths, one instance at a time, and its output in a log file. Without `--via-host`, or with no opener running, `pr` behaves @@ -453,8 +464,9 @@ matter: can rewrite that code, the agent owns the allowlist. That is what `stage` is for. With a shared `.git/config` between host and container: never set credentials with `git config ---local` (it is the same file on both sides); the container's own `~/.gitconfig` is private, but -`~/.dotfiles` is mounted, which is why `setup` refuses to write through a symlink into it. +--local` (it is the same file on both sides); the container's own `~/.gitconfig` is private, but any +directory the container shares is not, which is why `setup` refuses to write the config through a +symlink or into one. Credential helpers are only consulted for HTTP(S) remotes, so none of this affects the host's SSH pushes. diff --git a/examples/docker-compose.snippet.yml b/examples/docker-compose.snippet.yml index 81f7658..318b0ca 100644 --- a/examples/docker-compose.snippet.yml +++ b/examples/docker-compose.snippet.yml @@ -88,7 +88,7 @@ services: - ./log:/var/log/git-cred-broker # The socket directory, shared with the DSH container below. This is the only interface # between the two: no ports are published, and none should be added. - - /volume1/homes//Workspaces//.dsh/git-broker:/run/git-broker + - /srv/git-broker-socket:/run/git-broker ``` Pin a version (`npx --yes --package 'git-credential-broker@0.1.0'`) if you would rather not @@ -110,10 +110,8 @@ services: - GIT_BROKER_REQUIRE=1 volumes: # ... existing mounts ... - - git-cred-broker-run:/run/git-broker - -volumes: - git-cred-broker-run: + # The *same* host directory the broker binds, or the two are not sharing a socket at all. + - /srv/git-broker-socket:/run/git-broker ``` Then, inside the container: @@ -122,6 +120,6 @@ Then, inside the container: git-credential-broker setup ``` -Do **not** put the private key or any credential in `~/Workspaces`, `~/.dsh` or `~/.dotfiles`: -all three are mounted into the container. `~/.dsh` in particular is *inside* the -`~/Workspaces` mount, so it is not a safe place for anything secret. +Do **not** put the private key or any credential anywhere the agent container mounts: every such +directory is shared with it. The broker's configuration, key and token belong outside all of them, +and `init`, `stage` and `authorize` refuse a target they detect as shared. diff --git a/src/commands/compose.ts b/src/commands/compose.ts index dd89c04..9c9916f 100644 --- a/src/commands/compose.ts +++ b/src/commands/compose.ts @@ -196,7 +196,7 @@ export function runCompose(argv: readonly string[]): number { const configPath = resolveConfigPath(args.value('config')); const socketDir = args.value('socket-dir'); if (!socketDir) { - fail('--socket-dir is required (a host directory the pushing container already mounts, e.g. its ~/.dsh)'); + fail('--socket-dir is required (a host directory the pushing container already mounts)'); } const dir = args.value('dir') ?? deploymentDir(configPath); diff --git a/src/commands/init.ts b/src/commands/init.ts index ba3c45a..14770b8 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -42,6 +42,8 @@ export interface InitEnvironment { readonly isInsideContainer?: () => boolean; /** The home directory whose mounts must not contain the key. */ readonly home?: string; + /** The directories the container shares, when the caller already has them. */ + readonly mounts?: readonly string[]; } /** Everything `init` needs, resolved. */ @@ -235,7 +237,7 @@ export function performInit(input: InitInput): InitResult { } const home = input.env?.home ?? (process.env['HOME'] ?? ''); - const mounted = insideMountedPath(input.dir, home); + const mounted = insideMountedPath(input.dir, home, input.env?.mounts); if (mounted) { throw new Error( `${input.dir} is inside ${mounted}, which is mounted into the container; the private key must live outside every mount`, diff --git a/src/commands/setup.ts b/src/commands/setup.ts index c433aab..2f25152 100644 --- a/src/commands/setup.ts +++ b/src/commands/setup.ts @@ -25,7 +25,9 @@ import { findSystemCaBundle, gitConfigList, gitConfigSet, + insideMountedPath, listFlag, + mountedPaths, parseArgs, say, warn, @@ -51,6 +53,10 @@ export interface SetupInput { readonly rewriteSshHost: string | null; /** Report what would happen without writing anything. */ readonly dryRun: boolean; + /** Home whose mounts the config must not be written into; defaults to the real home. */ + readonly home?: string; + /** The directories the container shares, when the caller already has them. */ + readonly mounts?: readonly string[]; /** Platform to answer for; injectable so the refusal is testable. */ readonly platform?: NodeJS.Platform; } @@ -92,7 +98,7 @@ export function defaultHelperPath(): string { * * @param input - the resolved options. * @returns what was done. - * @throws {Error} when the target config is a symlink or resolves into the host's dotfiles. + * @throws {Error} when the target config is a symlink, or inside a directory the container shares. */ export async function performSetup(input: SetupInput): Promise { // Refuse on native Windows: configuring git there would look like success and never work, because @@ -101,16 +107,25 @@ export async function performSetup(input: SetupInput): Promise { if (refusal !== null) { throw new Error(refusal); } - // Writing through a symlink would edit whatever it points at. On the host, ~/.gitconfig is a - // symlink into ~/.dotfiles, which is mounted into the container — so this is not theoretical. + // Writing through a symlink would edit whatever it points at, and a config that lives in a directory + // the container shares is one the container can rewrite. if (fs.existsSync(input.gitconfig) && fs.lstatSync(input.gitconfig).isSymbolicLink()) { const target = fs.realpathSync(input.gitconfig); throw new Error(`${input.gitconfig} is a symlink to ${target}; refusing to write through it`); } - const dotfiles = path.join(os.homedir(), '.dotfiles'); - const resolved = path.resolve(input.gitconfig); - if (resolved === dotfiles || resolved.startsWith(`${dotfiles}${path.sep}`)) { - throw new Error(`${input.gitconfig} resolves into ~/.dotfiles, which is the host's own configuration`); + // The config must not be written into any directory the container can rewrite. Canonical, so the + // host's spelling of a shared directory and the container's spelling of the same one are one place. + // `setup` runs in the container by definition, so the mount table it is standing on is the + // authority. Reading it directly means a container that leaves no marker for `isInsideContainer` + // (plain containerd, a Kubernetes pod) still gets the right answer instead of hunting for a runtime + // it does not have. + const mounts = input.mounts ?? mountedPaths(input.home, { container: true }); + const mounted = insideMountedPath(input.gitconfig, input.home, mounts); + if (mounted) { + throw new Error( + `${input.gitconfig} is inside ${mounted}, which is mounted into the container; refusing to write ` + + "this environment's git config where the container can rewrite it", + ); } const warnings: string[] = []; @@ -228,6 +243,7 @@ export async function runSetup(argv: readonly string[]): Promise { caBundlePath: args.value('ca-bundle') ?? path.join(home, '.config', 'git-credential-broker', 'ca-bundle.pem'), rewriteSshHost: args.has('no-rewrite-ssh') ? null : (args.value('rewrite-ssh') ?? 'github.com'), dryRun: args.has('dry-run'), + home, }); say(`git config : ${result.gitconfig}${args.has('dry-run') ? ' (dry run)' : ''}`); diff --git a/src/commands/stage.ts b/src/commands/stage.ts index 9d0acb7..1715370 100644 --- a/src/commands/stage.ts +++ b/src/commands/stage.ts @@ -21,7 +21,8 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { canonicalPath, fail, insideMountedPath, parseArgs, say, warn, writeSecretFile } from './support.ts'; +import { canonicalPath } from '../paths.ts'; +import { fail, insideMountedPath, parseArgs, say, warn, writeSecretFile } from './support.ts'; /** The record `stage` leaves behind, so the running code is identifiable afterwards. */ export const STAGED_RECORD = 'STAGED.json'; @@ -36,6 +37,8 @@ export interface StageInput { readonly ref: string; /** The home whose mounts the target must not be inside. */ readonly home?: string; + /** The directories the container shares, when the caller already has them. */ + readonly mounts?: readonly string[]; /** Replace a target that this command did not stage itself. */ readonly force?: boolean; } @@ -62,7 +65,7 @@ export function performStage(input: StageInput): StageResult { const to = path.resolve(input.to); const home = input.home ?? process.env['HOME'] ?? ''; - const mounted = insideMountedPath(to, home); + const mounted = insideMountedPath(to, home, input.mounts); if (mounted) { throw new Error( `${to} is inside ${mounted}, which is mounted into the container. The broker runs with the ` + diff --git a/src/commands/support.ts b/src/commands/support.ts index bfe17ae..926e5ce 100644 --- a/src/commands/support.ts +++ b/src/commands/support.ts @@ -9,6 +9,8 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { canonicalPath } from '../paths.ts'; + /** One parsed command line. */ export interface Args { /** Every value given for a flag, in order. */ @@ -100,82 +102,338 @@ export function listFlag(args: Args, name: string): string[] { } /** - * Whether this process is running inside the agent container rather than on the host. + * Whether this process is running inside a container rather than on the host. + * + * A credential must not be written anywhere a container can read it, and the broker must not run + * inside the agent container. Docker and Podman each leave a marker; nothing here names a directory, + * because no directory is this tool's to assume. * - * The container mounts the workspace but has no `/volume1`, which is where the host keeps - * everything this project needs to protect. + * Those markers are also this function's known gap, and it does double duty. A container that leaves + * neither — plain containerd, a Kubernetes pod — reads as a host: the "run this on the host" refusal + * in `init` and `authorize` does not fire, and `mountedPaths` looks for a runtime instead of reading + * the mount table it is standing on. `setup`, which is in the container by definition, says so itself + * and reads the table. The refusal cases still fail closed afterwards, when no runtime can be asked, + * but they fail with a message about the runtime rather than about the container. * - * @returns true when running inside the container. + * @returns true when running inside a container. */ export function isInsideContainer(): boolean { - return fs.existsSync('/home/app/Workspaces') && !fs.existsSync('/volume1'); + return fs.existsSync('/.dockerenv') || fs.existsSync('/run/.containerenv'); +} + +/** Where the shared directories came from, when a caller supplies them instead of detecting them. */ +export interface MountSource { + /** The contents of `/proc/self/mountinfo`. */ + readonly mountinfo?: string; + /** `Mounts` arrays as the container runtime prints them; null when it could not be asked. */ + readonly inspect?: string | null; + /** Whether this process is in the container, whose own mount table is the authority. */ + readonly container?: boolean; + /** How to run a container runtime command, so a test need not have one installed. */ + readonly exec?: (cli: string, args: readonly string[]) => string; +} + +/** + * The commands that can report what running containers mount. + * + * Docker, Podman and nerdctl share the `ps`/`inspect` interface used here, so one code path covers all + * three. Plain containerd has no equivalent — `ctr` speaks a different language, in a namespace it + * chooses — so a host that offers only that is refused rather than guessed at. + */ +const CONTAINER_CLIS = ['docker', 'podman', 'nerdctl'] as const; + +/** + * Decode one field of `/proc/self/mountinfo`. + * + * The kernel escapes space, tab, newline and backslash as octal, so a mount point containing a space + * arrives as `\040` instead of splitting the line. + * + * @param value - the raw field. + * @returns the decoded path. + */ +function decodeMountField(value: string): string { + return value.replace(/\\([0-7]{3})/g, (_whole, octal: string) => String.fromCharCode(Number.parseInt(octal, 8))); +} + +/** + * `canonicalPath`, but a path that cannot be resolved is answered lexically. + * + * For a *candidate* in a list this is the safe direction. A host runs containers whose mounts this + * user cannot read — measured: a Synology document-viewer mount answers EACCES — and one of those + * must not stop the whole list being read. A candidate that is lexically under the home is still + * treated as shared, which over-refuses rather than under-refuses. The strict form stays for the path + * being guarded, where an answer that cannot be known is a refusal. + * + * @param target - the path to resolve. + * @returns the real path, or the resolved path when it cannot be read. + */ +function canonicalOrResolved(target: string): string { + try { + return canonicalPath(target); + } catch { + return path.resolve(target); + } +} + +/** + * Whether a path is the root of its filesystem. + * + * Every mount table lists the container's own root filesystem, and that root is the image's, not a + * directory the host handed over. Counting it as shared would put every path "inside a mount" and + * refuse `setup` its own config; a *bind* of the host root arrives through the runtime instead, where + * it does count. + * + * @param target - the path to test. + * @returns true when it is a filesystem root. + */ +function filesystemRoot(target: string): boolean { + const resolved = path.resolve(target); + return resolved === path.parse(resolved).root; +} + +/** + * Whether one canonical directory contains another. + * + * The root is special in its spelling and nothing else: `'/'.startsWith('/' + '/')` is false, so the + * ordinary prefix test says the host root contains nothing at all. + * + * @param parent - the directory that would contain. + * @param child - the path it would contain. + * @returns true when parent holds child. + */ +function contains(parent: string, child: string): boolean { + if (child === parent) return true; + return filesystemRoot(parent) ? child.startsWith(parent) : child.startsWith(`${parent}${path.sep}`); +} + +/** + * Whether one canonical directory contains the other. + * + * Both directions matter. A mount *under* the home is shared, and so is a mount that *contains* the + * home: a container that binds `/srv` while the home is `/srv/u` can rewrite everything in the home, + * so dropping that mount for sitting above the home is the same blind spot in the other direction. + * + * @param a - one canonical path. + * @param b - the other canonical path. + * @returns true when either contains the other. + */ +function overlaps(a: string, b: string): boolean { + return contains(a, b) || contains(b, a); +} + +/** + * The mount points that overlap a home, read from a mount table. + * + * The container is the thing that knows what it mounts. A list of directory names in this file would + * be a guess about a home this code has never seen. + * + * @param mountinfo - the contents of `/proc/self/mountinfo`. + * @param home - the home to measure against. + * @returns the mount points, spelled as the table spells them. + */ +export function mountsTouchingHome(mountinfo: string, home: string): string[] { + const root = canonicalOrResolved(home); + const found: string[] = []; + for (const line of mountinfo.split('\n')) { + const field = line.split(' ')[4]; + if (field === undefined) continue; + const point = path.resolve(decodeMountField(field)); + // The container's own root filesystem is in every mount table, and it is not a shared directory. + if (filesystemRoot(point)) continue; + if (overlaps(canonicalOrResolved(point), root)) found.push(point); + } + return found; } /** - * A path with the symlinks in its existing part resolved. + * The host paths a container runtime bound that overlap the user's home. * - * `realpathSync` throws for a path that does not exist yet, and `stage` is asked to write to one, so the - * nearest existing ancestor is resolved and the rest is appended. Comparing the raw spellings is what let - * `/var/services/homes/u/Workspaces` and `/volume1/homes/u/Workspaces` — one directory with two names — - * look like different places, which is exactly the bypass the mount and overlap checks exist to close. + * `docker inspect --format '{{json .Mounts}}'` prints one array per container. Only bind mounts count: + * a volume or a tmpfs is not a directory the user named. Every container is considered, not only the + * agent's — any container that can rewrite a directory is a reason not to put the broker's code there + * — so this is deliberately a superset. * - * @param target - the path to canonicalize. - * @returns the real path, or the resolved path when nothing on it exists. - * @throws {Error} when the path cannot be resolved for a reason other than being absent, because a - * lexical answer there could call a path inside a mount outside it. + * @param output - the runtime's output, one array per line. + * @param home - the home to compare against. + * @returns the host paths, unique and canonical. */ -export function canonicalPath(target: string): string { - const absolute = path.resolve(target); - let existing = absolute; - const missing: string[] = []; - for (;;) { +export function boundPathsTouchingHome(output: string, home: string): string[] { + const root = canonicalOrResolved(home); + const found = new Set(); + for (const line of output.split('\n')) { + if (line.trim() === '') continue; + let mounts: unknown; try { - const real = fs.realpathSync(existing); - return missing.length === 0 ? real : path.join(real, ...missing.toReversed()); - } catch (error) { - // Only a path that is not there is walked up. Anything else — EACCES, ELOOP — means the real path - // cannot be known, and a lexical fallback could miss a symlinked component and pass a path the - // check exists to refuse, so it propagates and the caller fails closed. - const code = (error as NodeJS.ErrnoException).code; - if (code !== 'ENOENT' && code !== 'ENOTDIR') throw error; - const parent = path.dirname(existing); - // Unreachable while the root exists and realpaths; kept so a filesystem that refuses even `/` - // cannot spin here forever. - if (parent === existing) return absolute; - missing.push(path.basename(existing)); - existing = parent; + mounts = JSON.parse(line); + } catch { + continue; + } + if (!Array.isArray(mounts)) continue; + for (const mount of mounts as Array<{ Type?: unknown; Source?: unknown }>) { + if (mount.Type !== 'bind' || typeof mount.Source !== 'string') continue; + const real = canonicalOrResolved(mount.Source); + if (overlaps(real, root)) found.add(real); } } + return [...found]; +} + +/** + * What a runtime had to say: it is not installed (skip it), it is there and could not be asked (a + * blind spot, so the whole check refuses), or it answered. + */ +type RuntimeQuery = + | { readonly kind: 'absent' } + | { readonly kind: 'failed' } + | { readonly kind: 'ok'; readonly containers: number; readonly mounts: string[] }; + +/** Run a container runtime command; a failure means that runtime could not be asked. */ +function runtimeExec(cli: string, args: readonly string[]): string { + return execFileSync(cli, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }); } /** - * The container's mounts under the user's home, as seen from the host. + * Whether a failed spawn means the command is not installed, rather than installed and failing. * - * A private key must never live under one of these. + * The difference decides whether the guard can proceed at all. A host that has only docker must not be + * refused because podman is missing; a host whose docker daemon is unreachable must be, because its + * containers are precisely the ones nothing else can see. * - * @param home - the home directory to resolve against. - * @returns the absolute paths. + * @param error - the error a spawn threw. + * @returns true when the command was not found at all. */ -export function mountedPaths(home: string = os.homedir()): string[] { - return [path.join(home, 'Workspaces'), path.join(home, '.dsh'), path.join(home, '.dotfiles')]; +function notInstalled(error: unknown): boolean { + return (error as { code?: unknown } | null | undefined)?.code === 'ENOENT'; +} + +/** + * One runtime's answer. + * + * `--all`, not only the running containers: a stopped container's bind mounts are still configured and + * it can be started again at any time. This is most likely to run while the agent container is down — + * `init` on a fresh host, or a boot script — which is exactly when a running-only list is blind. + * + * @param cli - the command to run. + * @param home - the home to compare against. + * @param exec - how to run it. + * @returns what this runtime had to say. + */ +function mountsFromRuntime( + cli: string, + home: string, + exec: (cli: string, args: readonly string[]) => string, +): RuntimeQuery { + let ids: string; + try { + ids = exec(cli, ['ps', '--all', '--quiet']); + } catch (error) { + return notInstalled(error) ? { kind: 'absent' } : { kind: 'failed' }; + } + const known = ids.split('\n').map((id) => id.trim()).filter((id) => id !== ''); + if (known.length === 0) return { kind: 'ok', containers: 0, mounts: [] }; + try { + const listed = exec(cli, ['inspect', '--format', '{{json .Mounts}}', ...known]); + return { kind: 'ok', containers: known.length, mounts: boundPathsTouchingHome(listed, home) }; + } catch { + // It listed containers a moment ago, so it is installed. Failing now is a blind spot, not absence. + return { kind: 'failed' }; + } +} + +/** + * Ask every container runtime on this host which directories it has bound near the user's home. + * + * The host's own mount table cannot answer this: a bind mount exists only in the container's + * namespace, so the runtimes are asked instead. Their answers are unioned, because one host can run + * more than one of them and a container under either is still a container that can rewrite the code. + * + * Two states are not answers, and both make the caller refuse. A runtime that knows of no container at + * all is one — the agent container may simply be stopped, its mounts still configured. A runtime that + * is installed but cannot be queried is the other: its containers are invisible, and another runtime's + * list cannot cover for them. + * + * @param home - the home to compare against. + * @param exec - how to run a runtime. + * @returns the paths, or undefined when they cannot be known. + */ +function detectedMounts(home: string, exec: (cli: string, args: readonly string[]) => string): string[] | undefined { + const found = new Set(); + let containers = 0; + for (const cli of CONTAINER_CLIS) { + const answer = mountsFromRuntime(cli, home, exec); + if (answer.kind === 'absent') continue; + if (answer.kind === 'failed') return undefined; + containers += answer.containers; + for (const mount of answer.mounts) found.add(mount); + } + return containers === 0 ? undefined : [...found]; +} + +/** + * The directories the container shares with the host. + * + * The container's own mount table answers when this process is in the container; the container runtime + * answers when it is not. Only when neither can does this refuse — the guess it will not make is what + * would put the broker's code, or the private key, in a directory a container can rewrite. + * + * @param home - the home whose mounts are wanted. + * @param source - a mount table or runtime output, for a test. + * @returns the shared directories. + * @throws {Error} when nothing can answer. + */ +export function mountedPaths(home: string = os.homedir(), source: MountSource = {}): string[] { + const container = source.container ?? isInsideContainer(); + let mounts: string[] | undefined; + if (source.mountinfo !== undefined) { + mounts = mountsTouchingHome(source.mountinfo, home); + } else if (source.inspect !== undefined) { + mounts = source.inspect === null ? undefined : boundPathsTouchingHome(source.inspect, home); + } else if (container) { + try { + mounts = mountsTouchingHome(fs.readFileSync('/proc/self/mountinfo', 'utf8'), home); + } catch { + mounts = undefined; + } + } else { + mounts = detectedMounts(home, source.exec ?? runtimeExec); + } + + if (mounts === undefined) { + throw new Error( + 'cannot tell which directories the container shares: ' + + (container + ? 'its own mount table could not be read' + : 'none of docker, podman or nerdctl could be asked, or none knows of a container') + + '; refusing rather than guessing', + ); + } + return mounts; } /** - * Whether a path is inside one of the container's mounts. + * Whether a path is inside one of the directories the container shares with the host. * - * Both sides are canonicalized before they are compared: `$HOME` on the host may be one name for a - * directory the container sees under another (`/var/services/homes/u` and `/volume1/homes/u`), and a - * lexical prefix check calls the same directory two different places. + * Both sides are canonicalized before they are compared: the host may spell a directory one way and + * the container another (a symlinked home, a bind mount reached two ways), and a lexical prefix check + * calls the same directory two different places. * * @param target - the path to check. * @param home - the home directory to resolve against. + * @param mounts - the shared directories, when the caller already has them. * @returns the mount it is inside, or null. */ -export function insideMountedPath(target: string, home: string = os.homedir()): string | null { +export function insideMountedPath( + target: string, + home: string = os.homedir(), + mounts?: readonly string[], +): string | null { const resolved = canonicalPath(target); - for (const mounted of mountedPaths(home)) { - const real = canonicalPath(mounted); - if (resolved === real || resolved.startsWith(`${real}${path.sep}`)) return mounted; + for (const mounted of mounts ?? mountedPaths(home)) { + // The mount side takes the tolerant form: discovery may have kept a mount that answers EACCES in + // its lexical spelling, and one unreadable mount elsewhere must not fail a check about this target. + // `contains`, not a prefix test, or a container binding the host root would be discovered and then + // never matched — `'/x'.startsWith('//')` is false. + if (contains(canonicalOrResolved(mounted), resolved)) return mounted; } return null; } diff --git a/src/host-request.ts b/src/host-request.ts index bfc0203..3067ce9 100644 --- a/src/host-request.ts +++ b/src/host-request.ts @@ -24,7 +24,9 @@ import { } from 'node:fs'; import { realpathSync } from 'node:fs'; import { lstatSync } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; +import { dirname, join, resolve, sep } from 'node:path'; + +import { canonicalPath } from './paths.ts'; /** The directory under the git directory that carries requests and answers. */ export const CHANNEL_PATH = 'git-credential-broker/pr-requests'; @@ -148,7 +150,7 @@ const SKIP = new Set(['.git', 'node_modules', '@eaDir', '#recycle', '$RECYCLE.BI * * The opener is meant to be started once for a machine, so it is given roots rather than one * repository: every checkout below them is served, including ones created later. Dotted - * directories are *not* skipped — a dotfiles repository is one, and it is a checkout like any + * directories are *not* skipped — a repository whose name begins with a dot is a checkout like any * other. A directory holding a checkout is not descended into, which keeps the walk cheap. * * @param root - where to start. @@ -157,15 +159,20 @@ const SKIP = new Set(['.git', 'node_modules', '@eaDir', '#recycle', '$RECYCLE.BI */ export function discoverCheckouts(root: string, maxDepth = 4): { repo: string; dir: string }[] { const found: { repo: string; dir: string }[] = []; + // The root can be one name for a directory the filesystem knows by another, and a worktree records its + // git directory under the real one. Both sides are canonical, or the plain checkout is served and the + // worktree beside it is silently not. + const realRoot = canonicalPath(root).replace(/[/\\]+$/, ''); const walk = (current: string, depth: number): void => { const gitDir = gitDirAt(current); if (gitDir !== undefined) { - // A `gitdir:` pointer is written by whoever has the checkout, which is the container. Serving a channel - // at a path it names would let it choose where the opener creates files, and running `gh` there would let - // it choose which repository that checkout means. A worktree's pointer stays under the same root. - if (gitDir.startsWith(`${root.replace(/[/\\]+$/, '')}/`)) { - found.push({ repo: current, dir: channelDir(gitDir) }); - } + // A `gitdir:` pointer is written by whoever has the checkout, which is the container. Serving a + // channel at a path it names would let it choose where the opener creates files, and running `gh` + // there would let it choose which repository that checkout means. A worktree's pointer stays under + // the same root. + if (canonicalPath(gitDir).startsWith(`${realRoot}${sep}`)) { + found.push({ repo: current, dir: channelDir(gitDir) }); + } return; } if (depth >= maxDepth) return; diff --git a/src/paths.ts b/src/paths.ts new file mode 100644 index 0000000..98bdca2 --- /dev/null +++ b/src/paths.ts @@ -0,0 +1,47 @@ +/** + * Canonical paths, for the checks that compare one place against another. + * + * A directory can have more than one name — a symlinked `$HOME`, a mount the host spells one way and + * the container another — and a lexical prefix comparison calls those different places. The checks + * that keep the broker's code out of a container mount, and the opener inside the roots it serves, + * have to compare real paths rather than spellings. + */ +import fs from 'node:fs'; +import path from 'node:path'; + +/** + * A path with the symlinks in its existing part resolved. + * + * `realpathSync` throws for a path that does not exist yet, and `stage` is asked to write to one, so the + * nearest existing ancestor is resolved and the rest is appended. Comparing raw spellings is what lets + * one directory with two names — a symlinked home, a bind mount reached two ways — look like two + * different places, which is exactly the bypass these checks exist to close. + * + * @param target - the path to canonicalize. + * @returns the real path, or the resolved path when nothing on it exists. + * @throws {Error} when the path cannot be resolved for a reason other than being absent, because a + * lexical answer there could call a path inside a mount outside it. + */ +export function canonicalPath(target: string): string { + const absolute = path.resolve(target); + let existing = absolute; + const missing: string[] = []; + for (;;) { + try { + const real = fs.realpathSync(existing); + return missing.length === 0 ? real : path.join(real, ...missing.toReversed()); + } catch (error) { + // Only a path that is not there is walked up. Anything else — EACCES, ELOOP — means the real path + // cannot be known, and a lexical fallback could miss a symlinked component and pass a path the + // check exists to refuse, so it propagates and the caller fails closed. + const code = (error as NodeJS.ErrnoException).code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') throw error; + const parent = path.dirname(existing); + // Unreachable while the root exists and realpaths; kept so a filesystem that refuses even `/` + // cannot spin here forever. + if (parent === existing) return absolute; + missing.push(path.basename(existing)); + existing = parent; + } + } +} diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index cb592bb..0dedad0 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -97,34 +97,185 @@ describe('parseArgs', () => { }); describe('mounted path detection', () => { - it('knows what the container can see', () => { - const home = '/home/app'; - assert.deepEqual(mountedPaths(home), ['/home/app/Workspaces', '/home/app/.dsh', '/home/app/.dotfiles']); - assert.equal(insideMountedPath('/home/app/Workspaces/x', home), '/home/app/Workspaces'); - assert.equal(insideMountedPath('/home/app/.dsh/git-broker', home), '/home/app/.dsh'); - assert.equal(insideMountedPath('/volume1/docker/git-cred-broker', home), null); - assert.equal(insideMountedPath('/home/app/Workspaces-evil', home), null, 'must not prefix-match a sibling'); + /** A home and a shared-directory list stated here, so no case reads the machine's own mount table. */ + const home = '/home/u'; + const mounts = ['/home/u/shared', '/home/u/cache', '/home/u/notes']; + + it('is inside a shared directory, and not a sibling that merely starts the same', () => { + assert.equal(insideMountedPath('/home/u/shared/x', home, mounts), '/home/u/shared'); + assert.equal(insideMountedPath('/home/u/cache/git-broker', home, mounts), '/home/u/cache'); + assert.equal(insideMountedPath('/srv/other', home, mounts), null); + assert.equal(insideMountedPath('/home/u/shared-evil', home, mounts), null, 'must not prefix-match a sibling'); + }); + + it('reads them from the container mount table', () => { + const table = [ + `675 573 0:36 /x ${home}/shared rw - btrfs /dev/x rw`, + `676 573 0:36 /y ${home}/with\\040space rw - btrfs /dev/x rw`, + '677 573 0:36 /z /somewhere/else rw - btrfs /dev/x rw', + ].join('\n'); + assert.deepEqual(mountedPaths(home, { mountinfo: table, container: true }), [ + `${home}/shared`, + `${home}/with space`, + ]); + }); + + it('asks the container runtime on the host, and keeps the bind mounts that touch the home', () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-inspect-')); + const inside = path.join(base, 'shared'); + const deeper = path.join(base, 'sub', 'deep'); + fs.mkdirSync(inside, { recursive: true }); + fs.mkdirSync(deeper, { recursive: true }); + const inspect = [ + JSON.stringify([ + { Type: 'bind', Source: inside, Destination: '/home/u/shared' }, + { Type: 'bind', Source: deeper, Destination: '/home/u/cache' }, + { Type: 'volume', Source: path.join(base, 'volume'), Destination: '/vol' }, + { Type: 'bind', Source: '/srv/elsewhere', Destination: '/srv/elsewhere' }, + ]), + ].join('\n'); + try { + const got = mountedPaths(base, { container: false, inspect }) + .map((entry) => fs.realpathSync(entry)) + .sort(); + assert.deepEqual( + got, + [inside, deeper].map((entry) => fs.realpathSync(entry)).sort(), + 'a volume and an unrelated path are not shared directories', + ); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); + + it('asks for every container, not only the running ones', () => { + // A stopped container still has its bind mounts configured, and can be started again at any time. + // `init` runs on a fresh host and from boot scripts, which is exactly when a running-only list is + // blind. Regression: an empty runtime answer used to be trusted as "nothing is shared". + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-runtime-')); + const inside = path.join(base, 'shared'); + fs.mkdirSync(inside, { recursive: true }); + const calls: string[][] = []; + const exec = (cli: string, args: readonly string[]): string => { + calls.push([cli, ...args]); + if (args[0] === 'ps') return 'b3f1c2d4e5f6\n'; + return JSON.stringify([{ Type: 'bind', Source: inside, Destination: '/home/u/shared' }]); + }; + try { + assert.deepEqual( + mountedPaths(base, { container: false, exec }).map((entry) => fs.realpathSync(entry)), + [fs.realpathSync(inside)], + 'a stopped container is still a container that can rewrite the staged code', + ); + assert.equal(calls[0]?.includes('--all'), true, 'ps must include the containers that are not running'); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); + + it('counts a mount that contains the home, not only ones under it', () => { + // A container that binds the home's parent can rewrite everything in the home. + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-ancestor-')); + const homeDir = path.join(base, 'u'); + fs.mkdirSync(homeDir, { recursive: true }); + const exec = (_cli: string, args: readonly string[]): string => + args[0] === 'ps' ? 'b3f1c2d4e5f6\n' : JSON.stringify([{ Type: 'bind', Source: base, Destination: '/mnt' }]); + try { + assert.deepEqual( + mountedPaths(homeDir, { container: false, exec }).map((entry) => fs.realpathSync(entry)), + [fs.realpathSync(base)], + ); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); + + it('counts a container that binds the host root, which contains everything', () => { + // The root is where the prefix test used to fail: `'/'.startsWith('//')` is false, so a bind of + // the host root was dropped even though such a container can rewrite every path on the host. + const root = path.parse(path.resolve(path.sep)).root; + const exec = (_cli: string, args: readonly string[]): string => + args[0] === 'ps' ? 'b3f1c2d4e5f6\n' : JSON.stringify([{ Type: 'bind', Source: root, Destination: '/host' }]); + assert.deepEqual(mountedPaths(home, { container: false, exec }), [root]); + assert.equal( + insideMountedPath(`${home}/Workspaces/x`, home, [root]), + root, + 'and the guard refuses a target under it, rather than matching nothing', + ); + }); + + it("ignores the container's own root filesystem, which the image made", () => { + // Every mount table lists it. Counting it would put every path inside a mount and refuse `setup` + // its own `~/.gitconfig`. A *bind* of the host root arrives through the runtime, above. + const table = [ + '675 573 0:36 / / rw,relatime - overlay overlay rw', + `676 573 0:36 /x ${home}/shared rw - btrfs /dev/x rw`, + ].join('\n'); + assert.deepEqual(mountedPaths(home, { mountinfo: table, container: true }), [`${home}/shared`]); + }); + + it('refuses when the runtime knows of no container at all', () => { + // Installed but empty is not "nothing is shared": it is the state in which the check is blind. + const exec = (_cli: string, args: readonly string[]): string => (args[0] === 'ps' ? '' : '[]'); + assert.throws( + () => mountedPaths(home, { container: false, exec }), + /could be asked, or none knows of a container/, + ); + }); + + it('treats a missing runtime as absent, but an installed failing one as a blind spot', () => { + const failure = (code: string): NodeJS.ErrnoException => Object.assign(new Error(code), { code }); + const one = (cli: string, args: readonly string[]): string => { + if (cli !== 'podman') throw failure('ENOENT'); + return args[0] === 'ps' ? 'id-1\n' : JSON.stringify([{ Type: 'bind', Source: '/srv', Destination: '/srv' }]); + }; + assert.deepEqual( + mountedPaths('/srv/u', { container: false, exec: one }), + ['/srv'], + 'a host with one runtime must not be refused because the others are not installed', + ); + const unreachable = (cli: string, args: readonly string[]): string => { + if (cli === 'docker') throw failure('ECONNREFUSED'); + return one(cli, args); + }; + assert.throws( + () => mountedPaths('/srv/u', { container: false, exec: unreachable }), + /could be asked, or none knows of a container/, + 'a runtime that is installed but unreachable hides its containers, so another list cannot cover for it', + ); + }); + + it('refuses rather than guess when nothing can answer', () => { + assert.throws( + () => mountedPaths(home, { container: false, inspect: null }), + /refusing rather than guessing/, + ); }); it('resolves the aliases of one directory instead of trusting the spelling', () => { - // Measured on the host this was found on: `$HOME` is /var/services/homes/u while the container sees the - // same directory as /volume1/homes/u, so a lexical prefix check called a path inside the mount outside - // it — and `stage` wrote the broker's code into a directory the agent can rewrite. + // One shared directory can have two spellings — a symlinked home, a bind mount reached two ways — + // so a lexical prefix check calls a path inside it outside, which is how `stage` wrote the broker's + // code into a directory the agent can rewrite. const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-mount-alias-')); const real = path.join(root, 'real'); - fs.mkdirSync(path.join(real, 'Workspaces'), { recursive: true }); + fs.mkdirSync(path.join(real, 'data'), { recursive: true }); const alias = path.join(root, 'alias'); // 'junction' so the case also runs on Windows, where a plain symlink needs a privilege; the type is // ignored on the platforms that have no junctions. fs.symlinkSync(real, alias, 'junction'); try { - const mounted = path.join(alias, 'Workspaces'); - assert.equal(insideMountedPath(path.join(real, 'Workspaces', 'x'), alias), mounted, 'real target, aliased home'); - assert.equal(insideMountedPath(path.join(real, 'Workspaces'), alias), mounted, 'and the mount itself'); - assert.equal(insideMountedPath(path.join(real, 'elsewhere'), alias), null, 'a sibling is still outside'); + const mounted = path.join(alias, 'data'); + const shared = [mounted]; assert.equal( - insideMountedPath(path.join(alias, 'Workspaces', 'y'), real), - path.join(real, 'Workspaces'), + insideMountedPath(path.join(real, 'data', 'x'), alias, shared), + mounted, + 'real target, aliased mount', + ); + assert.equal(insideMountedPath(path.join(real, 'data'), alias, shared), mounted, 'and the mount itself'); + assert.equal(insideMountedPath(path.join(real, 'elsewhere'), alias, shared), null, 'a sibling is still outside'); + assert.equal( + insideMountedPath(path.join(alias, 'data', 'y'), real, [path.join(real, 'data')]), + path.join(real, 'data'), 'and the reverse spelling', ); } finally { @@ -141,11 +292,12 @@ describe('mounted path detection', () => { const alias = path.join(root, 'alias'); fs.symlinkSync(real, alias, 'junction'); try { - const mounted = path.join(alias, 'Workspaces'); - assert.equal(insideMountedPath(path.join(real, 'Workspaces', 'new', 'deep'), alias), mounted); - assert.equal(insideMountedPath(path.join(real, 'Workspaces'), alias), mounted); + const mounted = path.join(alias, 'data'); + const shared = [mounted]; + assert.equal(insideMountedPath(path.join(real, 'data', 'new', 'deep'), alias, shared), mounted); + assert.equal(insideMountedPath(path.join(real, 'data'), alias, shared), mounted); assert.equal( - insideMountedPath(path.join(real, 'Workspaces-evil'), alias), + insideMountedPath(path.join(real, 'data-evil'), alias, shared), null, 'a sibling is not inside, with the mount missing too', ); @@ -173,6 +325,8 @@ describe('setup', () => { caBundlePath: path.join(base, '.config', 'git-credential-broker', 'ca-bundle.pem'), rewriteSshHost: 'github.com', dryRun: false, + // Stated, so the shared-directory resolution does not read this machine's mount table. + mounts: [], ...overrides, }; } @@ -225,6 +379,27 @@ describe('setup', () => { await performSetup(input); assert.equal(gitEntries(input.gitconfig).some((entry) => entry.includes('insteadof')), false); }); + + it('refuses a config inside a container mount, whatever the mount is spelled', async () => { + // One shared directory can have two spellings (a symlinked home, a bind mount). The check this + // replaced compared spellings, so the container's was accepted and the host's own git config would + // be written where the container can rewrite it. + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'setup-mount-alias-')); + const real = path.join(base, 'real'); + fs.mkdirSync(path.join(real, 'shared'), { recursive: true }); + const alias = path.join(base, 'alias'); + fs.symlinkSync(real, alias, 'junction'); + try { + const gitconfig = path.join(real, 'shared', 'gitconfig'); + await assert.rejects( + () => performSetup(setupInput({ gitconfig, home: alias, mounts: [path.join(alias, 'shared')], dryRun: true })), + /mounted into the container/, + ); + assert.equal(fs.existsSync(gitconfig), false, 'and nothing was written'); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); }); describe('init', () => { @@ -239,7 +414,7 @@ describe('init', () => { force: false, // This suite runs inside the very container init refuses to run in, so the detected // environment is injected. The guard itself is exercised by the tests below. - env: { isInsideContainer: () => false, home: fakeHome }, + env: { isInsideContainer: () => false, home: fakeHome, mounts: [] }, }; it('installs the key 0600 and writes a config the daemon accepts', () => { @@ -316,9 +491,15 @@ describe('init', () => { it('refuses a key inside a container mount', () => { const root = dir(`init-mount-${Math.random().toString(36).slice(2)}`); const cert = writeKey(path.join(root, 'downloaded.pem')); - const mounted = path.join(fakeHome, 'Workspaces', 'somewhere'); + const mounted = path.join(fakeHome, 'shared', 'somewhere'); assert.throws( - () => performInit({ ...baseInput, cert, dir: mounted }), + () => + performInit({ + ...baseInput, + cert, + dir: mounted, + env: { ...baseInput.env, mounts: [path.join(fakeHome, 'shared')] }, + }), /mounted into the container/, 'the private key must never live where the container can read it', ); @@ -653,7 +834,7 @@ describe('config discovery', () => { appId: 123456, permissions: { contents: 'write' }, force: false, - env: { isInsideContainer: () => false, home: dir('discovery-home') }, + env: { isInsideContainer: () => false, home: dir('discovery-home'), mounts: [] }, }); assert.equal(result.configPath, custom); @@ -665,7 +846,7 @@ describe('config discovery', () => { describe('compose', () => { const rendered = renderCompose({ dir: '/volume1/docker/git-cred-broker', - socketDir: '/volume1/homes/u/Workspaces/h/.dsh/git-broker', + socketDir: '/srv/shared/git-broker', user: '1026:100', image: 'node:24-slim', packageSpec: 'git-credential-broker@0.1.0', @@ -693,7 +874,7 @@ describe('compose', () => { it('can run staged code from a mount instead, fetching and building nothing at boot', () => { const fromCode = renderCompose({ dir: '/volume1/docker/git-cred-broker', - socketDir: '/volume1/homes/u/Workspaces/h/.dsh/git-broker', + socketDir: '/srv/shared/git-broker', user: '1026:100', image: 'node:24-slim', packageSpec: 'git-credential-broker@0.1.0', @@ -741,7 +922,7 @@ describe('compose', () => { assert.equal(rendered.includes(`${deployDir}/app.pem:${SIDECAR.keyPath}:ro`), true); assert.equal(rendered.includes(`${deployDir}/log:${SIDECAR.auditDir}`), true); assert.equal( - rendered.includes('/volume1/homes/u/Workspaces/h/.dsh/git-broker:' + SIDECAR.socketDir), + rendered.includes('/srv/shared/git-broker:' + SIDECAR.socketDir), true, ); }); diff --git a/test/unit/host-request.test.ts b/test/unit/host-request.test.ts index ab3cd8b..33fa9f1 100644 --- a/test/unit/host-request.test.ts +++ b/test/unit/host-request.test.ts @@ -185,29 +185,54 @@ describe('reading a pull request URL out of a program', () => { }); describe('finding the checkouts to serve', () => { - it('finds every checkout under a root, at any depth, dotfiles included', () => { + it('finds the checkouts within the given depth, hidden directories included', () => { const root = mkdtempSync(join(tmpdir(), 'discover-test-')); - for (const repo of ['.dotfiles', 'Workspaces/one', 'Workspaces/two/deep', 'plain']) { + for (const repo of ['.hidden', 'trees/one', 'trees/two/deep', 'plain']) { mkdirSync(join(root, repo, '.git'), { recursive: true }); writeFileSync(join(root, repo, '.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); writeFileSync(join(root, repo, '.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); } // never descended into, and never enough on its own - mkdirSync(join(root, 'Workspaces/one/node_modules/decoy/.git'), { recursive: true }); - writeFileSync(join(root, 'Workspaces/one/node_modules/decoy/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); - writeFileSync(join(root, 'Workspaces/one/node_modules/decoy/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); - mkdirSync(join(root, 'Workspaces/three/four/five/six/seven/.git'), { recursive: true }); - writeFileSync(join(root, 'Workspaces/three/four/five/six/seven/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); - writeFileSync(join(root, 'Workspaces/three/four/five/six/seven/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); + mkdirSync(join(root, 'trees/one/node_modules/decoy/.git'), { recursive: true }); + writeFileSync(join(root, 'trees/one/node_modules/decoy/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); + writeFileSync(join(root, 'trees/one/node_modules/decoy/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); + mkdirSync(join(root, 'trees/three/four/five/six/seven/.git'), { recursive: true }); + writeFileSync(join(root, 'trees/three/four/five/six/seven/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); + writeFileSync(join(root, 'trees/three/four/five/six/seven/.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); const repos = discoverCheckouts(root, 4) .map((found) => found.repo.slice(root.length + 1)) .sort(); - assert.deepEqual(repos, ['.dotfiles', 'Workspaces/one', 'Workspaces/two/deep', 'plain']); + assert.deepEqual(repos, ['.hidden', 'plain', 'trees/one', 'trees/two/deep']); rmSync(root, { recursive: true, force: true }); }); + it('serves a worktree whose git directory is spelled through an alias of the root', () => { + // The root can be one name for a directory the filesystem knows by another ($HOME on the host this was + // found on), and a worktree records its git directory under the real one. Comparing the raw spellings + // served the plain checkout and silently skipped the worktree beside it. + const base = mkdtempSync(join(tmpdir(), 'discover-alias-')); + const real = join(base, 'real'); + mkdirSync(join(real, 'checkout', 'worktree'), { recursive: true }); + mkdirSync(join(real, 'gitdir'), { recursive: true }); + writeFileSync(join(real, 'gitdir', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); + writeFileSync(join(real, 'checkout', 'worktree', '.git'), `gitdir: ${join(real, 'gitdir')}\n`, 'utf8'); + mkdirSync(join(real, 'plain', '.git'), { recursive: true }); + writeFileSync(join(real, 'plain', '.git', 'HEAD'), 'ref: refs/heads/main\n', 'utf8'); + const alias = join(base, 'alias'); + symlinkSync(real, alias, 'junction'); + + try { + const repos = discoverCheckouts(alias, 4) + .map((found) => found.repo.slice(alias.length + 1)) + .sort(); + assert.deepEqual(repos, ['checkout/worktree', 'plain']); + } finally { + rmSync(base, { recursive: true, force: true }); + } + }); + it('returns a channel directory per checkout', () => { const root = mkdtempSync(join(tmpdir(), 'discover-test-')); mkdirSync(join(root, 'a', '.git'), { recursive: true }); diff --git a/test/unit/stage.test.ts b/test/unit/stage.test.ts index 3edc457..c830ab3 100644 --- a/test/unit/stage.test.ts +++ b/test/unit/stage.test.ts @@ -37,6 +37,14 @@ function makeRepo(name: string): string { } describe('stage', () => { + /** + * `performStage` with the shared directories supplied. + * + * Resolving them for real asks the container runtime, which is right in production and noise here: + * each case states its own list, and the mount cases pass theirs. + */ + const stage = (input: Parameters[0]) => performStage({ mounts: [], ...input }); + it('exports the commit, records where it came from, and ignores the working tree', () => { const repo = makeRepo('clean'); const to = path.join(scratch, 'staged/clean'); @@ -44,7 +52,7 @@ describe('stage', () => { // An uncommitted edit that must NOT reach the staged tree. fs.writeFileSync(path.join(repo, 'src', 'daemon.ts'), 'export const version = 999;\n'); - const result = performStage({ repo, to, ref: 'HEAD', home: scratch }); + const result = stage({ repo, to, ref: 'HEAD', home: scratch }); assert.equal(result.dirtyWorkingTree, true, 'the dirty tree is reported'); assert.equal(result.files > 0, true); @@ -58,13 +66,13 @@ describe('stage', () => { const repo = makeRepo('restage'); const to = path.join(scratch, 'staged/restage'); - performStage({ repo, to, ref: 'HEAD', home: scratch }); + stage({ repo, to, ref: 'HEAD', home: scratch }); fs.writeFileSync(path.join(repo, 'src', 'old.ts'), 'export const gone = true;\n'); execFileSync('git', ['add', '-A'], { cwd: repo }); execFileSync('git', ['-c', 'user.name=t', '-c', 'user.email=t@example.com', 'commit', '-q', '-m', 'second'], { cwd: repo, }); - performStage({ repo, to, ref: 'HEAD', home: scratch }); + stage({ repo, to, ref: 'HEAD', home: scratch }); assert.equal(fs.existsSync(path.join(to, 'src', 'old.ts')), true, 'the new commit added it'); @@ -74,38 +82,45 @@ describe('stage', () => { execFileSync('git', ['-c', 'user.name=t', '-c', 'user.email=t@example.com', 'commit', '-q', '-m', 'third'], { cwd: repo, }); - performStage({ repo, to, ref: 'HEAD', home: scratch }); + stage({ repo, to, ref: 'HEAD', home: scratch }); assert.equal(fs.existsSync(path.join(to, 'src', 'old.ts')), false, 'no stale file survives a restage'); }); it('refuses a target inside a container mount', () => { const repo = makeRepo('mounted'); const home = path.join(scratch, 'home'); - const inside = path.join(home, 'Workspaces', 'somewhere'); + const inside = path.join(home, 'shared', 'somewhere'); fs.mkdirSync(inside, { recursive: true }); assert.throws( - () => performStage({ repo, to: inside, ref: 'HEAD', home }), + () => stage({ repo, to: inside, ref: 'HEAD', home, mounts: [path.join(home, 'shared')] }), /mounted into the container/, 'code the agent can rewrite must never be staged where it will run with the key', ); }); it('refuses a mount whose path is spelled through an alias', () => { - // The host spells $HOME /var/services/homes/u; the container sees the same directory as - // /volume1/homes/u. A lexical check let a target under the second name through, which staged the - // broker's code into a directory the agent can rewrite — the one thing this command exists to stop. + // One shared directory can have two spellings (a symlinked home, a bind mount). A lexical check let + // a target under the second name through, which staged the broker's code into a directory the agent + // can rewrite — the one thing this command exists to stop. const repo = makeRepo('alias'); const real = path.join(scratch, 'alias-real'); - fs.mkdirSync(path.join(real, 'Workspaces'), { recursive: true }); + fs.mkdirSync(path.join(real, 'shared'), { recursive: true }); const alias = path.join(scratch, 'alias-link'); fs.symlinkSync(real, alias, 'junction'); assert.throws( - () => performStage({ repo, to: path.join(real, 'Workspaces', 'app'), ref: 'HEAD', home: alias }), + () => + stage({ + repo, + to: path.join(real, 'shared', 'app'), + ref: 'HEAD', + home: alias, + mounts: [path.join(alias, 'shared')], + }), /mounted into the container/, ); - assert.equal(fs.existsSync(path.join(real, 'Workspaces', 'app')), false, 'and nothing was written'); + assert.equal(fs.existsSync(path.join(real, 'shared', 'app')), false, 'and nothing was written'); }); it('refuses to clear the deployment directory that holds the key', () => { @@ -115,7 +130,7 @@ describe('stage', () => { fs.writeFileSync(path.join(to, 'app.pem'), 'the key\n'); fs.writeFileSync(path.join(to, 'broker.config.json'), '{}\n'); - assert.throws(() => performStage({ repo, to, ref: 'HEAD', home: scratch, force: true }), /deployment directory/); + assert.throws(() => stage({ repo, to, ref: 'HEAD', home: scratch, force: true }), /deployment directory/); assert.equal(fs.readFileSync(path.join(to, 'app.pem'), 'utf8'), 'the key\n', 'the key is untouched'); }); @@ -125,15 +140,15 @@ describe('stage', () => { fs.mkdirSync(to, { recursive: true }); fs.writeFileSync(path.join(to, 'something.txt'), 'mine\n'); - assert.throws(() => performStage({ repo, to, ref: 'HEAD', home: scratch }), /not ours to clear/); + assert.throws(() => stage({ repo, to, ref: 'HEAD', home: scratch }), /not ours to clear/); assert.equal(fs.existsSync(path.join(to, 'something.txt')), true, 'nothing was removed'); }); it('refuses a target that overlaps the repository', () => { const repo = makeRepo('overlap'); - assert.throws(() => performStage({ repo, to: repo, ref: 'HEAD', home: scratch }), /overlap/); + assert.throws(() => stage({ repo, to: repo, ref: 'HEAD', home: scratch }), /overlap/); assert.throws( - () => performStage({ repo, to: path.join(repo, 'dist'), ref: 'HEAD', home: scratch }), + () => stage({ repo, to: path.join(repo, 'dist'), ref: 'HEAD', home: scratch }), /overlap/, ); }); @@ -146,14 +161,14 @@ describe('stage', () => { const link = path.join(scratch, 'link-to-repo'); fs.symlinkSync(repo, link, 'junction'); assert.throws( - () => performStage({ repo, to: link, ref: 'HEAD', home: scratch }), + () => stage({ repo, to: link, ref: 'HEAD', home: scratch }), /repository and the target overlap/, ); const sub = path.join(scratch, 'link-into-repo'); fs.symlinkSync(path.join(repo, 'src'), sub, 'junction'); assert.throws( - () => performStage({ repo, to: sub, ref: 'HEAD', home: scratch }), + () => stage({ repo, to: sub, ref: 'HEAD', home: scratch }), /repository and the target overlap/, ); });