From 55b3853965b929302a3ac9d3b7603f45beb07af1 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sun, 23 Aug 2026 09:35:47 -0700 Subject: [PATCH 1/3] feat(cloud-hypervisor): stage host mount trees for virtiofsd exports Cloud Hypervisor v53 and virtiofsd v1.10 have no per-path readonly option, and a guest-side `ro` mount is not a security boundary. Add an optional, strongly typed enforcement input to `VirtiofsdManager.start()` that stages a private host mount tree per export: recursive bind, private propagation, recursive read-only enforcement, then nested writable bind overlays. When a tree is staged, virtiofsd is launched with `--announce-submounts`. The API is inert: with no enforcement argument the manager behaves exactly as before, including byte-identical virtiofsd arguments. Runtime wiring lands separately. libmount's `ro=recursive` option argument is deliberately not used. On util-linux 2.39.3 it exits 0 while leaving carried-in submounts read-write, so enforcement instead remounts each mount in the tree read-only deepest-first. Every tree is verified against `/proc/self/mountinfo` before virtiofsd starts and staging fails closed if the boundary cannot be proven. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/cloud-hypervisor-foundation.md | 98 ++++ src/cloud-hypervisor/mount-tree.test.ts | 613 ++++++++++++++++++++++++ src/cloud-hypervisor/mount-tree.ts | 497 +++++++++++++++++++ src/cloud-hypervisor/virtiofsd.test.ts | 163 +++++++ src/cloud-hypervisor/virtiofsd.ts | 114 ++++- 5 files changed, 1478 insertions(+), 7 deletions(-) create mode 100644 src/cloud-hypervisor/mount-tree.test.ts create mode 100644 src/cloud-hypervisor/mount-tree.ts diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index 13e303b35..514c4c821 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -164,6 +164,104 @@ Temporary microVM workspace data lives under: With `--keep-containers`, AWF preserves this directory, the network namespace, and runtime diagnostics for investigation. +## Host mount-tree enforcement + +Cloud Hypervisor v53 and virtiofsd v1.10 expose no per-path read-only option, so +a mixed read-only/read-write export cannot be described to the guest, and a +guest-side read-only mount is not a security boundary. The only trustworthy +boundary is the host VFS. + +`VirtiofsdManager.start()` therefore accepts an optional, strongly typed +enforcement input: + +```ts +interface VirtiofsdWritableOverlay { + readonly source: string; // canonical host path inside the export source + readonly destination: string; // canonical host path inside the export source + readonly kind: 'file' | 'directory'; +} + +interface VirtiofsdExportMountPlan { + readonly tag: string; // export tag the plan applies to + readonly writableOverlays: readonly VirtiofsdWritableOverlay[]; +} + +interface VirtiofsdMountEnforcement { + readonly plans: readonly VirtiofsdExportMountPlan[]; +} +``` + +When the input is omitted, or no plan matches an export tag, that export is +staged exactly as before. When a plan matches, the export is served from a +private staged mount tree under the per-run virtiofsd share directory: + +1. `mount --rbind ` — recursive bind, so nested + host mounts are carried into the tree instead of being silently skipped. +2. `mount --make-rprivate ` — private propagation before anything + writable exists, so neither the read-only attributes nor the later overlays + can leak back into the host or the export's peer group. +3. Every mount in the staged tree is enumerated from `/proc/self/mountinfo` and + remounted read-only one at a time, deepest-first, with + `mount -o remount,bind,ro,nosuid,nodev `. This happens before + any overlay exists. +4. Each writable overlay is bound back in, shallowest first, with + `mount --bind ` followed by + `mount -o remount,bind,rw,nosuid,nodev `. Overlay binds + are deliberately non-recursive, so a writable directory never exposes the + submounts nested inside it, and the explicit remount sets the flags instead + of inheriting whatever the source mount carried. + +virtiofsd receives `--announce-submounts` for staged trees so the guest observes +each writable child bind as its own submount, and keeps its namespace sandbox, +`--seccomp=kill`, `--inode-file-handles=never`, and caching policy unchanged. +The guest mount itself stays read-write for a staged export; the host mount +flags are the enforcement boundary. + +### Fail-closed behaviour + +- libmount's `ro=recursive` option argument is deliberately **not** used. + On util-linux 2.39.3 — the version on GitHub-hosted Ubuntu 24.04 runners — + both `mount -o rbind,ro=recursive` and + `mount -o remount,bind,ro=recursive` exit 0 while leaving carried-in submounts + read-write, which would be a silent security failure. The per-mount remount + loop was verified to work on the same host. A preflight check still requires + util-linux >= 2.23 for `--make-rprivate`, so a non-util-linux `mount` fails + with a clear error. +- After staging, and again after the overlays are applied, AWF parses + `/proc/self/mountinfo` and requires that the staged root exists, that every + mount under it is `ro` except the requested overlay destinations, that every + mount carries `nosuid` and `nodev`, and that no mount in the tree is shared. + The mount tool's exit code is never the only evidence that enforcement + succeeded — this verification is what caught the `ro=recursive` behaviour + above. +- Overlay sources must be canonical (`realpath` equality), must resolve inside + the export source, must not be symbolic links, and must match the declared + kind. Overlay destinations must already exist, may not overlap each other, and + an originally read-only export may not receive overlays at all. +- The staged root must be disjoint from the export source, so the recursive bind + can never nest the staged tree inside itself. + +### Ordering and cleanup + +Teardown reverses setup: writable children are unmounted deepest-first, then the +staged root is unmounted recursively (`umount -R`, because a recursive bind root +can carry submounts) and its staging directory is removed. A failed unmount +stays pending so a later `stop()` retries it. If staging fails part-way, the +partial tree is rolled back and the original failure is preserved; when rollback +itself fails, the residual tree is retained and retried during `stop()`. + +### Residual limitation + +Overlay destinations are validated inside the staged tree, which is already +recursively read-only and privately propagated, so they cannot be swapped +between validation and the bind. Overlay *sources* live in the original, +still-writable export, so a process that can already write to the export could in +principle replace a source path between validation and the bind. Sources are +re-validated immediately before each bind, and both the planner and this layer +require containment inside the export, but this residual setup-time TOCTOU window +cannot be closed without fd-based mount APIs that the current tooling does not +expose. + ## Limitations The preview rejects configurations that weaken or conflict with its boundary, diff --git a/src/cloud-hypervisor/mount-tree.test.ts b/src/cloud-hypervisor/mount-tree.test.ts new file mode 100644 index 000000000..03e50175b --- /dev/null +++ b/src/cloud-hypervisor/mount-tree.test.ts @@ -0,0 +1,613 @@ +import type { CloudHypervisorDirectoryExport } from './exports'; +import { + StagedHostMountTree, + assertMountToolSupported, + parseMountInfo, + selectMountPlan, + type MountTreeDependencies, + type VirtiofsdExportMountPlan, +} from './mount-tree'; + +const workspace: CloudHypervisorDirectoryExport = { + tag: 'workspace', + source: '/host/workspace', + target: '/workspace', + mode: 'rw', +}; +const cache: CloudHypervisorDirectoryExport = { + tag: 'cache', + source: '/host/cache', + target: '/host/cache', + mode: 'ro', +}; +const ROOT = '/run/awf-shares/run/0-workspace'; +const tools = { mount: '/usr/bin/mount', umount: '/usr/bin/umount' }; + +interface MountRecord { + options: string[]; + optionalFields: string[]; +} + +/** + * Fake host mount table modelled on observed util-linux 2.39.3 behaviour: + * records every mount/umount invocation, applies remounts to a single mount at a + * time, and synthesises the matching /proc/self/mountinfo so the manager's + * fail-closed verification is exercised for real. + */ +function mountTable(options: { ineffectiveRemount?: boolean; shared?: boolean } = {}) { + const table = new Map(); + const commands: string[][] = []; + const childrenOf = (target: string): string[] => + [...table.keys()].filter((key) => key === target || key.startsWith(`${target}/`)); + const runTool = jest.fn(async (command: string, args: readonly string[]) => { + commands.push([command, ...args]); + if (command === tools.umount) { + const recursive = args[0] === '-R'; + const target = args[args.length - 1]; + const affected = childrenOf(target); + if (!recursive && affected.length > 1) throw new Error(`target is busy: ${target}`); + for (const key of recursive ? affected : [target]) table.delete(key); + return; + } + if (args[0] === '--rbind') { + const [, source, target] = args; + expect(source.startsWith('/')).toBe(true); + table.set(target, { + options: ['rw', 'relatime'], + optionalFields: options.shared === false ? [] : ['shared:21'], + }); + // Submount carried in by the recursive bind. + table.set(`${target}/nested`, { + options: ['rw', 'relatime'], + optionalFields: options.shared === false ? [] : ['shared:22'], + }); + return; + } + if (args[0] === '--bind') { + table.set(args[2], { options: ['rw', 'relatime'], optionalFields: [] }); + return; + } + if (args[0] === '--make-rprivate') { + for (const key of childrenOf(args[1])) { + table.set(key, { ...(table.get(key) as MountRecord), optionalFields: [] }); + } + return; + } + if (args[0] === '-o') { + const requested = args[1].split(','); + const target = args[2]; + if (!requested.includes('remount')) { + throw new Error(`unexpected mount invocation: ${args.join(' ')}`); + } + const current = table.get(target); + if (!current) throw new Error(`not mounted: ${target}`); + // Simulates a mount tool that reports success without changing anything. + if (options.ineffectiveRemount) return; + table.set(target, { + ...current, + options: [requested.includes('ro') ? 'ro' : 'rw', 'nosuid', 'nodev', 'relatime'], + }); + return; + } + throw new Error(`unexpected mount invocation: ${args.join(' ')}`); + }); + const readMountInfo = jest.fn(async () => + [...table.entries()] + .map(([mountPoint, record], index) => + [ + `${30 + index}`, + '29', + '0:42', + '/', + mountPoint, + record.options.join(','), + ...record.optionalFields, + '-', + 'ext4', + '/dev/root', + record.options.join(','), + ].join(' '), + ) + .join('\n'), + ); + return { table, commands, runTool, readMountInfo }; +} + +function dependencies( + fake: ReturnType, + overrides: Partial = {}, +): MountTreeDependencies { + return { + mkdir: jest.fn().mockResolvedValue(undefined), + rmdir: jest.fn().mockResolvedValue(undefined), + runTool: fake.runTool, + captureTool: jest.fn().mockResolvedValue('mount from util-linux 2.39.3 (libmount 2.39.0)'), + statPath: jest.fn(async (filePath: string) => stats(filePath.endsWith('.json') ? 'file' : 'directory')), + realpath: jest.fn(async (filePath: string) => filePath), + readMountInfo: fake.readMountInfo, + ...overrides, + }; +} + +function stats(kind: 'file' | 'directory' | 'symlink') { + return { + isDirectory: () => kind === 'directory', + isFile: () => kind === 'file', + isSymbolicLink: () => kind === 'symlink', + }; +} + +function plan( + overlays: VirtiofsdExportMountPlan['writableOverlays'], + tag = 'workspace', +): VirtiofsdExportMountPlan { + return { tag, writableOverlays: overlays }; +} + +function tree( + fake: ReturnType, + mountPlan: VirtiofsdExportMountPlan, + directoryExport = workspace, + overrides: Partial = {}, +): StagedHostMountTree { + return new StagedHostMountTree({ + directoryExport, + rootPath: ROOT, + plan: mountPlan, + tools, + dependencies: dependencies(fake, overrides), + }); +} + +describe('selectMountPlan', () => { + it('returns undefined without enforcement or a matching tag', () => { + expect(selectMountPlan(undefined, 'workspace')).toBeUndefined(); + expect(selectMountPlan({ plans: [plan([], 'cache')] }, 'workspace')).toBeUndefined(); + }); + + it('rejects duplicate plans for one export tag', () => { + expect(() => selectMountPlan({ plans: [plan([]), plan([])] }, 'workspace')).toThrow( + /Duplicate Cloud Hypervisor mount plan/, + ); + }); +}); + +describe('assertMountToolSupported', () => { + it.each([ + ['mount from util-linux 2.39.3 (libmount 2.39.0)', true], + ['mount from util-linux 2.41 (libmount 2.41)', true], + ['mount from util-linux 3.1 (libmount 3.1)', true], + ['mount from util-linux 2.23 (libmount 2.23)', true], + ['mount from util-linux 2.22.2 (libmount 2.22)', false], + ['mount from util-linux 1.99 (libmount 1.99)', false], + ])('accepts %s: %s', async (version, supported) => { + const deps = dependencies(mountTable(), { + captureTool: jest.fn().mockResolvedValue(version), + }); + const assertion = assertMountToolSupported(tools, deps); + if (supported) { + await expect(assertion).resolves.toBeUndefined(); + } else { + await expect(assertion).rejects.toThrow(/requires util-linux >= 2\.23/); + } + }); + + it('fails closed when the version cannot be parsed', async () => { + const deps = dependencies(mountTable(), { + captureTool: jest.fn().mockResolvedValue('mount from busybox'), + }); + await expect(assertMountToolSupported(tools, deps)).rejects.toThrow( + /Unable to determine util-linux version/, + ); + }); +}); + +describe('parseMountInfo', () => { + it('parses mount points, options, and optional fields with octal escapes', () => { + const entries = parseMountInfo( + [ + '30 29 0:42 / /run/awf\\040shares ro,nosuid shared:21 master:2 - ext4 /dev/root ro', + '', + 'malformed line', + '31 30 0:43 / /run/awf/child rw - ext4 /dev/root rw', + ].join('\n'), + ); + expect(entries).toEqual([ + { + mountPoint: '/run/awf shares', + options: ['ro', 'nosuid'], + optionalFields: ['shared:21', 'master:2'], + }, + { mountPoint: '/run/awf/child', options: ['rw'], optionalFields: [] }, + ]); + }); +}); + +describe('StagedHostMountTree', () => { + it('stages a recursively read-only tree, remounting each mount deepest-first', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([])); + await staged.stage(); + expect(fake.commands).toEqual([ + [tools.mount, '--rbind', '/host/workspace', ROOT], + [tools.mount, '--make-rprivate', ROOT], + [tools.mount, '-o', 'remount,bind,ro,nosuid,nodev', `${ROOT}/nested`], + [tools.mount, '-o', 'remount,bind,ro,nosuid,nodev', ROOT], + ]); + expect(staged.isStaged).toBe(true); + expect(staged.rootPath).toBe(ROOT); + }); + + it('stages a read-only export without overlays', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([], 'cache'), cache); + await staged.stage(); + expect(fake.commands[0]).toEqual([tools.mount, '--rbind', '/host/cache', ROOT]); + }); + + it('rejects writable overlays on an originally read-only export', async () => { + const fake = mountTable(); + const staged = tree( + fake, + plan( + [{ source: '/host/cache/out', destination: '/host/cache/out', kind: 'directory' }], + 'cache', + ), + cache, + ); + await expect(staged.stage()).rejects.toThrow(/cannot receive writable overlays/); + expect(fake.commands).toEqual([]); + }); + + it('binds selective directory and file overlays after the read-only root', async () => { + const fake = mountTable(); + const staged = tree( + fake, + plan([ + { + source: '/host/workspace/deep/nested/state.json', + destination: '/host/workspace/deep/nested/state.json', + kind: 'file', + }, + { source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }, + ]), + ); + await staged.stage(); + expect(fake.commands.slice(4)).toEqual([ + [tools.mount, '--bind', '/host/workspace/out', `${ROOT}/out`], + [tools.mount, '-o', 'remount,bind,rw,nosuid,nodev', `${ROOT}/out`], + [ + tools.mount, + '--bind', + '/host/workspace/deep/nested/state.json', + `${ROOT}/deep/nested/state.json`, + ], + [ + tools.mount, + '-o', + 'remount,bind,rw,nosuid,nodev', + `${ROOT}/deep/nested/state.json`, + ], + ]); + expect(fake.table.get(`${ROOT}/out`)?.options).toContain('rw'); + expect(fake.table.get(`${ROOT}/out`)?.options).toContain('nosuid'); + expect(fake.table.get(ROOT)?.options).toContain('ro'); + expect(fake.table.get(`${ROOT}/nested`)?.options).toContain('ro'); + }); + + it('unmounts children deepest-first and the staged root last', async () => { + const fake = mountTable(); + const staged = tree( + fake, + plan([ + { source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }, + { + source: '/host/workspace/deep/nested/state.json', + destination: '/host/workspace/deep/nested/state.json', + kind: 'file', + }, + ]), + ); + await staged.stage(); + expect(staged.cleanupOrder()).toEqual([ + `${ROOT}/deep/nested/state.json`, + `${ROOT}/out`, + ROOT, + ]); + await staged.unmount(); + expect(fake.commands.slice(8)).toEqual([ + [tools.umount, `${ROOT}/deep/nested/state.json`], + [tools.umount, `${ROOT}/out`], + [tools.umount, '-R', ROOT], + ]); + expect(staged.hasResidue).toBe(false); + }); + + it('retains residue when an unmount fails so a later attempt can retry', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([])); + await staged.stage(); + const real = fake.runTool.getMockImplementation() as ( + command: string, + args: readonly string[], + ) => Promise; + fake.runTool.mockImplementationOnce(async () => { + throw new Error('target is busy'); + }); + await expect(staged.unmount()).rejects.toThrow(/target is busy/); + expect(staged.hasResidue).toBe(true); + expect(staged.cleanupOrder()).toEqual([ROOT]); + fake.runTool.mockImplementation(real); + await expect(staged.unmount()).resolves.toBeUndefined(); + expect(staged.hasResidue).toBe(false); + expect(fake.table.size).toBe(0); + }); + + it('fails closed when a remount silently leaves the tree writable', async () => { + const fake = mountTable({ ineffectiveRemount: true }); + const staged = tree(fake, plan([])); + await expect(staged.stage()).rejects.toThrow(/not recursively read-only/); + expect(staged.hasResidue).toBe(false); + expect(fake.table.size).toBe(0); + }); + + it('fails closed when propagation would leak', async () => { + const fake = mountTable(); + fake.runTool.mockImplementation(async (command: string, args: readonly string[]) => { + if (args[0] === '--make-rprivate') return; + return undefined; + }); + const deps = dependencies(fake, { + readMountInfo: jest + .fn() + .mockResolvedValue(`30 29 0:42 / ${ROOT} ro,nosuid,nodev shared:21 - ext4 /dev/root ro`), + }); + const staged = new StagedHostMountTree({ + directoryExport: workspace, + rootPath: ROOT, + plan: plan([]), + tools, + dependencies: deps, + }); + await expect(staged.stage()).rejects.toThrow(/propagation would leak/); + }); + + it('fails closed when the staged root mount is missing', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([]), workspace, { + readMountInfo: jest.fn().mockResolvedValue(''), + }); + await expect(staged.stage()).rejects.toThrow(/missing its root mount/); + }); + + it('fails closed on a mount tool too old for private propagation', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([]), workspace, { + captureTool: jest.fn().mockResolvedValue('mount from util-linux 2.22.2'), + }); + await expect(staged.stage()).rejects.toThrow(/requires util-linux >= 2\.23/); + expect(fake.commands).toEqual([]); + }); + + it.each([ + ['relative source', { source: 'out', destination: '/host/workspace/out' }], + ['unnormalized source', { source: '/host/workspace/../etc', destination: '/host/workspace/out' }], + ['outside source', { source: '/host/other/out', destination: '/host/workspace/out' }], + ['export root source', { source: '/host/workspace', destination: '/host/workspace/out' }], + ['outside destination', { source: '/host/workspace/out', destination: '/host/other/out' }], + ['export root destination', { source: '/host/workspace/out', destination: '/host/workspace' }], + ['nul byte', { source: '/host/workspace/o\0ut', destination: '/host/workspace/out' }], + ])('rejects an overlay with a %s', async (_label, overlay) => { + const fake = mountTable(); + const staged = tree(fake, plan([{ ...overlay, kind: 'directory' as const }])); + await expect(staged.stage()).rejects.toThrow(/Cloud Hypervisor export "workspace" overlay/); + expect(fake.commands).toEqual([]); + }); + + it('rejects duplicate and overlapping overlay destinations', async () => { + const fake = mountTable(); + const duplicate = tree( + fake, + plan([ + { source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }, + { source: '/host/workspace/out2', destination: '/host/workspace/out', kind: 'directory' }, + ]), + ); + await expect(duplicate.stage()).rejects.toThrow(/Duplicate .* destination/); + const overlapping = tree( + fake, + plan([ + { source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }, + { + source: '/host/workspace/out/inner', + destination: '/host/workspace/out/inner', + kind: 'directory', + }, + ]), + ); + await expect(overlapping.stage()).rejects.toThrow(/Overlapping .* destinations/); + }); + + it('rejects an overlay source that resolves through a symlink', async () => { + const fake = mountTable(); + const staged = tree( + fake, + plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), + workspace, + { realpath: jest.fn().mockResolvedValue('/host/other/out') }, + ); + await expect(staged.stage()).rejects.toThrow(/must be canonical/); + expect(fake.table.size).toBe(0); + }); + + it('rejects an overlay source that is a symbolic link', async () => { + const fake = mountTable(); + const staged = tree( + fake, + plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), + workspace, + { statPath: jest.fn(async () => stats('symlink')) }, + ); + await expect(staged.stage()).rejects.toThrow(/must not be a symbolic link/); + }); + + it('requires the overlay destination to already exist with the declared kind', async () => { + const fake = mountTable(); + const missing = new Error('ENOENT') as NodeJS.ErrnoException; + missing.code = 'ENOENT'; + const staged = tree( + fake, + plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), + workspace, + { + statPath: jest.fn(async (filePath: string) => { + if (filePath.startsWith(ROOT)) throw missing; + return stats('directory'); + }), + }, + ); + await expect(staged.stage()).rejects.toThrow('ENOENT'); + + const wrongKind = tree( + fake, + plan([ + { source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'file' }, + ]), + ); + await expect(wrongKind.stage()).rejects.toThrow(/must be an existing regular file/); + }); + + it('rolls back every staged mount when an overlay bind fails', async () => { + const fake = mountTable(); + const real = fake.runTool.getMockImplementation() as ( + command: string, + args: readonly string[], + ) => Promise; + fake.runTool.mockImplementation(async (command: string, args: readonly string[]) => { + if (args[0] === '--bind') { + fake.commands.push([command, ...args]); + throw new Error('overlay bind failed'); + } + return real(command, args); + }); + const staged = tree( + fake, + plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), + ); + await expect(staged.stage()).rejects.toThrow('overlay bind failed'); + expect(fake.commands[fake.commands.length - 1]).toEqual([tools.umount, '-R', ROOT]); + expect(staged.hasResidue).toBe(false); + expect(staged.isStaged).toBe(false); + expect(fake.table.size).toBe(0); + }); + + it('reports both failures when rollback cannot complete', async () => { + const fake = mountTable(); + const real = fake.runTool.getMockImplementation() as ( + command: string, + args: readonly string[], + ) => Promise; + fake.runTool.mockImplementation(async (command: string, args: readonly string[]) => { + if (command === tools.umount) throw new Error('target is busy'); + return real(command, args); + }); + const staged = tree( + fake, + plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), + workspace, + { statPath: jest.fn(async () => stats('symlink')) }, + ); + await expect(staged.stage()).rejects.toThrow( + /must not be a symbolic link; staged mount cleanup failed: target is busy/, + ); + expect(staged.hasResidue).toBe(true); + }); + + it('refuses to stage twice', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([])); + await staged.stage(); + await expect(staged.stage()).rejects.toThrow(/already staged/); + }); + + it('rejects a plan whose tag does not match the export', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([], 'cache')); + await expect(staged.stage()).rejects.toThrow(/does not match export tag/); + }); + + it.each([ + ['staged root inside the export', '/host/workspace/.awf-stage'], + ['staged root equal to the export', '/host/workspace'], + ['staged root containing the export', '/host'], + ])('rejects a %s so the recursive bind cannot nest itself', async (_label, rootPath) => { + const fake = mountTable(); + const staged = new StagedHostMountTree({ + directoryExport: workspace, + rootPath, + plan: plan([]), + tools, + dependencies: dependencies(fake), + }); + await expect(staged.stage()).rejects.toThrow(/must be disjoint from export/); + expect(fake.commands).toEqual([]); + }); + + it('rejects more overlays than the supported maximum', async () => { + const fake = mountTable(); + const overlays = Array.from({ length: 65 }, (_value, index) => ({ + source: `/host/workspace/out${index}`, + destination: `/host/workspace/out${index}`, + kind: 'directory' as const, + })); + const staged = tree(fake, plan(overlays)); + await expect(staged.stage()).rejects.toThrow(/exceeds 64 writable overlays/); + }); + + it('rejects an invalid overlay kind', async () => { + const fake = mountTable(); + const staged = tree( + fake, + plan([ + { + source: '/host/workspace/out', + destination: '/host/workspace/out', + kind: 'socket' as unknown as 'file', + }, + ]), + ); + await expect(staged.stage()).rejects.toThrow(/Invalid export "workspace" overlay kind: socket/); + }); + + it('fails closed when a writable mount appears that no overlay requested', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([]), workspace, { + readMountInfo: jest + .fn() + .mockResolvedValueOnce(`30 29 0:42 / ${ROOT} ro,nosuid,nodev - ext4 /dev/root ro`) + .mockResolvedValueOnce(`30 29 0:42 / ${ROOT} ro,nosuid,nodev - ext4 /dev/root ro`) + .mockResolvedValue( + [ + `30 29 0:42 / ${ROOT} ro,nosuid,nodev - ext4 /dev/root ro`, + `31 30 0:43 / ${ROOT}/rogue rw,nosuid,nodev - ext4 /dev/root rw`, + ].join('\n'), + ), + }); + await expect(staged.stage()).rejects.toThrow(/Unexpected writable mount in staged tree/); + }); + + it('fails closed when a requested overlay never became writable', async () => { + const fake = mountTable(); + const staged = tree( + fake, + plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), + workspace, + { + readMountInfo: jest + .fn() + .mockResolvedValue(`30 29 0:42 / ${ROOT} ro,nosuid,nodev - ext4 /dev/root ro`), + }, + ); + await expect(staged.stage()).rejects.toThrow(/Writable overlay was not applied/); + }); +}); diff --git a/src/cloud-hypervisor/mount-tree.ts b/src/cloud-hypervisor/mount-tree.ts new file mode 100644 index 000000000..7edf64081 --- /dev/null +++ b/src/cloud-hypervisor/mount-tree.ts @@ -0,0 +1,497 @@ +import * as path from 'path'; +import type { CloudHypervisorDirectoryExport } from './exports'; + +/** + * Host mount-tree enforcement for virtiofsd exports. + * + * Cloud Hypervisor v53 and virtiofsd v1.10 have no per-path read-only option, so + * a mixed read-only/read-write export cannot be expressed inside the guest. The + * only trustworthy boundary is the host VFS: stage a private mount tree that is + * recursively read-only and then bind the few writable paths back in as nested + * read-write child mounts. virtiofsd then shares the staged tree and announces + * the submounts so the guest observes each child mount separately. + */ + +export type VirtiofsdOverlayKind = 'file' | 'directory'; + +/** A single writable path carved out of an otherwise read-only staged export. */ +export interface VirtiofsdWritableOverlay { + /** + * Canonical, absolute host path that provides the writable content. Must + * already resolve inside the export source. + */ + readonly source: string; + /** + * Canonical, absolute host path, expressed in the original export namespace, + * whose staged counterpart becomes writable. + */ + readonly destination: string; + /** Expected type of both `source` and `destination`. */ + readonly kind: VirtiofsdOverlayKind; +} + +/** Enforcement request for one export, addressed by its export tag. */ +export interface VirtiofsdExportMountPlan { + readonly tag: string; + readonly writableOverlays: readonly VirtiofsdWritableOverlay[]; +} + +/** + * Optional enforcement input for {@link VirtiofsdManager}. When absent, or when + * no plan matches an export tag, the export is staged exactly as before. + */ +export interface VirtiofsdMountEnforcement { + readonly plans: readonly VirtiofsdExportMountPlan[]; +} + +export interface MountTreeStats { + isDirectory(): boolean; + isFile(): boolean; + isSymbolicLink(): boolean; +} + +/** Command/filesystem abstraction so the staging logic is fully testable. */ +export interface MountTreeDependencies { + mkdir(directory: string, options: { recursive: true; mode: number }): Promise; + rmdir(directory: string): Promise; + runTool(command: string, args: readonly string[]): Promise; + captureTool(command: string, args: readonly string[]): Promise; + statPath(filePath: string): Promise; + realpath(filePath: string): Promise; + readMountInfo(): Promise; +} + +export interface MountTreeTools { + readonly mount: string; + readonly umount: string; +} + +export interface StagedHostMountTreeOptions { + readonly directoryExport: CloudHypervisorDirectoryExport; + readonly rootPath: string; + readonly plan: VirtiofsdExportMountPlan; + readonly tools: MountTreeTools; + readonly dependencies: MountTreeDependencies; +} + +interface ResolvedOverlay { + readonly source: string; + readonly stagedDestination: string; + readonly kind: VirtiofsdOverlayKind; +} + +export interface MountInfoEntry { + readonly mountPoint: string; + readonly options: readonly string[]; + readonly optionalFields: readonly string[]; +} + +const MAX_WRITABLE_OVERLAYS = 64; +const READONLY_REMOUNT_OPTIONS = 'remount,bind,ro,nosuid,nodev'; +const WRITABLE_REMOUNT_OPTIONS = 'remount,bind,rw,nosuid,nodev'; +const MINIMUM_UTIL_LINUX = { major: 2, minor: 23 } as const; + +/** + * Recursive read-only enforcement is applied one mount at a time rather than + * through libmount's `ro=recursive` option argument: on util-linux 2.39.3 (the + * GitHub-hosted Ubuntu 24.04 runner version) `mount -o rbind,ro=recursive` and + * `mount -o remount,bind,ro=recursive` both succeed while leaving submounts + * writable, which would be a silent security failure. Per-mount remounts work on + * every supported version; `--make-rprivate` needs util-linux >= 2.23. + */ +export async function assertMountToolSupported( + tools: MountTreeTools, + dependencies: MountTreeDependencies, +): Promise { + const output = await dependencies.captureTool(tools.mount, ['--version']); + const match = /util-linux\s+(\d+)\.(\d+)/.exec(output); + if (!match) { + throw new Error( + `Unable to determine util-linux version for host mount-tree enforcement from: ${output.trim()}`, + ); + } + const major = Number(match[1]); + const minor = Number(match[2]); + const supported = + major > MINIMUM_UTIL_LINUX.major || + (major === MINIMUM_UTIL_LINUX.major && minor >= MINIMUM_UTIL_LINUX.minor); + if (!supported) { + throw new Error( + `Host mount-tree enforcement requires util-linux >= ${MINIMUM_UTIL_LINUX.major}.` + + `${MINIMUM_UTIL_LINUX.minor} for private mount propagation, found ${major}.${minor}`, + ); + } +} + +export function selectMountPlan( + enforcement: VirtiofsdMountEnforcement | undefined, + tag: string, +): VirtiofsdExportMountPlan | undefined { + if (!enforcement) return undefined; + const matches = enforcement.plans.filter((plan) => plan.tag === tag); + if (matches.length > 1) { + throw new Error(`Duplicate Cloud Hypervisor mount plan for export tag: ${tag}`); + } + return matches[0]; +} + +/** + * A staged mount tree. Instances are created unmounted; {@link stage} performs + * the privileged work and {@link unmount} tears it down deepest-first. Failed + * unmounts stay pending so a later call can retry them. + */ +export class StagedHostMountTree { + private readonly pendingMounts = new Set(); + private rootDirectoryCreated = false; + private staged = false; + + constructor(private readonly options: StagedHostMountTreeOptions) {} + + get rootPath(): string { + return this.options.rootPath; + } + + /** True while host state (mounts or the staging directory) still exists. */ + get hasResidue(): boolean { + return this.pendingMounts.size > 0 || this.rootDirectoryCreated; + } + + get isStaged(): boolean { + return this.staged; + } + + /** Cleanup order: writable children deepest-first, staged root last. */ + cleanupOrder(): string[] { + return [...this.pendingMounts].sort((left, right) => { + const depth = pathDepth(right) - pathDepth(left); + return depth !== 0 ? depth : right.localeCompare(left); + }); + } + + async stage(): Promise { + if (this.staged) throw new Error(`Mount tree already staged: ${this.rootPath}`); + const overlays = this.resolveOverlays(); + try { + await this.stageReadonlyRoot(); + await this.stageWritableOverlays(overlays); + this.staged = true; + } catch (error) { + await this.rollback(error); + throw error; + } + } + + async unmount(): Promise { + const { dependencies, tools } = this.options; + for (const target of this.cleanupOrder()) { + // The staged root is a recursive bind, so it can carry submounts of its + // own; children are single non-recursive binds and unmount directly. + const args = target === this.rootPath ? ['-R', target] : [target]; + await dependencies.runTool(tools.umount, args); + this.pendingMounts.delete(target); + } + this.staged = false; + if (this.rootDirectoryCreated) { + await dependencies.rmdir(this.rootPath); + this.rootDirectoryCreated = false; + } + } + + private async stageReadonlyRoot(): Promise { + const { dependencies, tools, directoryExport } = this.options; + await assertMountToolSupported(tools, dependencies); + await dependencies.mkdir(this.rootPath, { recursive: true, mode: 0o700 }); + this.rootDirectoryCreated = true; + await dependencies.runTool(tools.mount, ['--rbind', directoryExport.source, this.rootPath]); + this.pendingMounts.add(this.rootPath); + // Private propagation before anything writable exists, so neither the + // read-only attributes nor the later overlays can leak into the host or the + // original export's peer group. + await dependencies.runTool(tools.mount, ['--make-rprivate', this.rootPath]); + const staged = await this.readTreeMountInfo(); + if (!staged.some((entry) => entry.mountPoint === this.rootPath)) { + throw new Error(`Staged mount tree is missing its root mount: ${this.rootPath}`); + } + const targets = staged + .map((entry) => entry.mountPoint) + .sort((left, right) => pathDepth(right) - pathDepth(left)); + for (const target of targets) { + await dependencies.runTool(tools.mount, ['-o', READONLY_REMOUNT_OPTIONS, target]); + } + await this.assertTreeIsReadonly(); + } + + private async stageWritableOverlays(overlays: readonly ResolvedOverlay[]): Promise { + const { dependencies, tools } = this.options; + for (const overlay of overlays) { + await this.assertOverlaySource(overlay); + await this.assertOverlayDestination(overlay); + // Non-recursive bind: a writable directory never exposes submounts nested + // inside it. The follow-up remount sets the flags explicitly instead of + // inheriting whatever the source mount carried. + await dependencies.runTool(tools.mount, [ + '--bind', + overlay.source, + overlay.stagedDestination, + ]); + this.pendingMounts.add(overlay.stagedDestination); + await dependencies.runTool(tools.mount, [ + '-o', + WRITABLE_REMOUNT_OPTIONS, + overlay.stagedDestination, + ]); + } + await this.assertOnlyOverlaysAreWritable(overlays); + } + + private async rollback(cause: unknown): Promise { + try { + await this.unmount(); + } catch (cleanupError) { + // Residue stays pending so the owning manager can retry it during stop(). + throw new Error( + `${formatError(cause)}; staged mount cleanup failed: ${formatError(cleanupError)}`, + ); + } + } + + private resolveOverlays(): ResolvedOverlay[] { + const { directoryExport, plan, rootPath } = this.options; + if (plan.tag !== directoryExport.tag) { + throw new Error( + `Mount plan tag "${plan.tag}" does not match export tag "${directoryExport.tag}"`, + ); + } + assertCleanAbsolutePath(rootPath, `staged root for export "${directoryExport.tag}"`); + // A staged root inside the export (or an export inside the staged root) + // would make the recursive bind contain itself. + if ( + rootPath === directoryExport.source || + containsPath(directoryExport.source, rootPath) || + containsPath(rootPath, directoryExport.source) + ) { + throw new Error( + `Staged mount tree root ${rootPath} must be disjoint from export "${directoryExport.tag}" ` + + `source ${directoryExport.source}`, + ); + } + if (plan.writableOverlays.length === 0) return []; + if (directoryExport.mode === 'ro') { + throw new Error( + `Read-only Cloud Hypervisor export "${directoryExport.tag}" cannot receive writable overlays`, + ); + } + if (plan.writableOverlays.length > MAX_WRITABLE_OVERLAYS) { + throw new Error( + `Cloud Hypervisor export "${directoryExport.tag}" exceeds ${MAX_WRITABLE_OVERLAYS} writable overlays`, + ); + } + const destinations: string[] = []; + const resolved = plan.writableOverlays.map((overlay) => { + const label = `export "${directoryExport.tag}" overlay`; + if (overlay.kind !== 'file' && overlay.kind !== 'directory') { + throw new Error(`Invalid ${label} kind: ${String(overlay.kind)}`); + } + assertCleanAbsolutePath(overlay.source, `${label} source`); + assertCleanAbsolutePath(overlay.destination, `${label} destination`); + assertContainedPath(directoryExport.source, overlay.source, `${label} source`); + assertContainedPath(directoryExport.source, overlay.destination, `${label} destination`); + for (const existing of destinations) { + if (existing === overlay.destination) { + throw new Error(`Duplicate ${label} destination: ${overlay.destination}`); + } + if (containsPath(existing, overlay.destination) || containsPath(overlay.destination, existing)) { + throw new Error( + `Overlapping ${label} destinations: ${existing} and ${overlay.destination}`, + ); + } + } + destinations.push(overlay.destination); + return { + source: overlay.source, + stagedDestination: path.join( + rootPath, + path.relative(directoryExport.source, overlay.destination), + ), + kind: overlay.kind, + }; + }); + // Shallow paths first so a parent mount point always exists before a child. + return resolved.sort((left, right) => pathDepth(left.stagedDestination) - pathDepth(right.stagedDestination)); + } + + /** + * Defense in depth: the overlay source lives in the still-writable original + * export, so it is re-validated immediately before the bind. `realpath` + * equality rejects symlinked components and any escape out of the export. + */ + private async assertOverlaySource(overlay: ResolvedOverlay): Promise { + const { dependencies, directoryExport } = this.options; + const resolved = await dependencies.realpath(overlay.source); + if (resolved !== overlay.source) { + throw new Error( + `Writable overlay source must be canonical: ${overlay.source} resolves to ${resolved}`, + ); + } + assertContainedPath( + directoryExport.source, + resolved, + `export "${directoryExport.tag}" overlay source`, + ); + const stats = await dependencies.statPath(overlay.source); + assertStatsMatchKind(stats, overlay.kind, `writable overlay source ${overlay.source}`); + } + + /** + * The destination is inspected inside the staged tree, which is already + * recursively read-only and privately propagated, so it cannot be swapped + * between this check and the bind. + */ + private async assertOverlayDestination(overlay: ResolvedOverlay): Promise { + const stats = await this.options.dependencies.statPath(overlay.stagedDestination); + assertStatsMatchKind( + stats, + overlay.kind, + `writable overlay destination ${overlay.stagedDestination}`, + ); + } + + private async assertTreeIsReadonly(): Promise { + const entries = await this.readTreeMountInfo(); + if (!entries.some((entry) => entry.mountPoint === this.rootPath)) { + throw new Error(`Staged mount tree is missing its root mount: ${this.rootPath}`); + } + for (const entry of entries) { + if (!entry.options.includes('ro')) { + throw new Error( + `Staged mount tree is not recursively read-only: ${entry.mountPoint} is writable`, + ); + } + assertHardenedOptions(entry); + assertPrivatePropagation(entry); + } + } + + private async assertOnlyOverlaysAreWritable(overlays: readonly ResolvedOverlay[]): Promise { + const expected = new Set(overlays.map((overlay) => overlay.stagedDestination)); + const entries = await this.readTreeMountInfo(); + const writable = new Set(); + for (const entry of entries) { + assertHardenedOptions(entry); + assertPrivatePropagation(entry); + if (entry.options.includes('ro')) continue; + if (!expected.has(entry.mountPoint)) { + throw new Error( + `Unexpected writable mount in staged tree: ${entry.mountPoint}`, + ); + } + writable.add(entry.mountPoint); + } + for (const destination of expected) { + if (!writable.has(destination)) { + throw new Error(`Writable overlay was not applied: ${destination}`); + } + } + } + + private async readTreeMountInfo(): Promise { + const entries = parseMountInfo(await this.options.dependencies.readMountInfo()); + return entries.filter( + (entry) => entry.mountPoint === this.rootPath || containsPath(this.rootPath, entry.mountPoint), + ); + } +} + +export function parseMountInfo(contents: string): MountInfoEntry[] { + const entries: MountInfoEntry[] = []; + for (const line of contents.split('\n')) { + const trimmed = line.trim(); + if (trimmed.length === 0) continue; + const fields = trimmed.split(' '); + const separator = fields.indexOf('-'); + if (fields.length < 7 || separator < 6) continue; + entries.push({ + mountPoint: unescapeMountInfoPath(fields[4]), + options: fields[5].split(','), + optionalFields: fields.slice(6, separator), + }); + } + return entries; +} + +function assertPrivatePropagation(entry: MountInfoEntry): void { + const shared = entry.optionalFields.some((field) => field.startsWith('shared:')); + if (shared) { + throw new Error( + `Staged mount tree propagation would leak: ${entry.mountPoint} is a shared mount`, + ); + } +} + +function assertHardenedOptions(entry: MountInfoEntry): void { + for (const option of ['nosuid', 'nodev']) { + if (!entry.options.includes(option)) { + throw new Error(`Staged mount ${entry.mountPoint} is missing ${option}`); + } + } +} + +function assertStatsMatchKind( + stats: MountTreeStats, + kind: VirtiofsdOverlayKind, + label: string, +): void { + if (stats.isSymbolicLink()) { + throw new Error(`${label} must not be a symbolic link`); + } + if (kind === 'directory' && !stats.isDirectory()) { + throw new Error(`${label} must be an existing directory`); + } + if (kind === 'file' && !stats.isFile()) { + throw new Error(`${label} must be an existing regular file`); + } +} + +function assertCleanAbsolutePath(value: string, label: string): void { + if ( + typeof value !== 'string' || + !path.isAbsolute(value) || + path.normalize(value) !== value || + value === '/' || + value.endsWith('/') || + value.includes('\0') || + Buffer.byteLength(value) > 4096 + ) { + throw new Error(`Cloud Hypervisor ${label} must be an absolute clean non-root path: ${value}`); + } +} + +function assertContainedPath(parent: string, child: string, label: string): void { + if (!containsPath(parent, child)) { + throw new Error(`Cloud Hypervisor ${label} must stay inside ${parent}: ${child}`); + } +} + +function containsPath(parent: string, child: string): boolean { + const relative = path.relative(parent, child); + return ( + relative !== '' && + relative !== '..' && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative) + ); +} + +function pathDepth(value: string): number { + return value.split(path.sep).length; +} + +function unescapeMountInfoPath(value: string): string { + return value.replace(/\\([0-7]{3})/g, (_match, code: string) => + String.fromCharCode(parseInt(code, 8)), + ); +} + +function formatError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/cloud-hypervisor/virtiofsd.test.ts b/src/cloud-hypervisor/virtiofsd.test.ts index 4a2b0ab27..43845e2d8 100644 --- a/src/cloud-hypervisor/virtiofsd.test.ts +++ b/src/cloud-hypervisor/virtiofsd.test.ts @@ -19,6 +19,7 @@ const cache = { target: '/host/cache', mode: 'ro' as const, }; +const STAGED_ROOT = '/run/awf-shares/run/0-workspace'; function processMock(pid: number): ExecaChildProcess { const child = Promise.resolve({ exitCode: 0 }) as unknown as ExecaChildProcess; @@ -50,11 +51,55 @@ function dependencies( mkdir: jest.fn().mockResolvedValue(undefined), rmdir: jest.fn().mockResolvedValue(undefined), runTool: jest.fn().mockResolvedValue(undefined), + captureTool: jest.fn().mockResolvedValue('mount from util-linux 2.39.3 (libmount 2.39.0)'), + statPath: jest.fn().mockResolvedValue({ + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false, + }), + realpath: jest.fn(async (filePath: string) => filePath), + readMountInfo: jest + .fn() + .mockResolvedValueOnce(`30 29 0:42 / ${STAGED_ROOT} ro,nosuid,nodev - ext4 /dev/root ro`) + .mockResolvedValueOnce(`30 29 0:42 / ${STAGED_ROOT} ro,nosuid,nodev - ext4 /dev/root ro`) + .mockResolvedValue( + [ + `30 29 0:42 / ${STAGED_ROOT} ro,nosuid,nodev - ext4 /dev/root ro`, + `31 30 0:43 / ${STAGED_ROOT}/out rw,nosuid,nodev - ext4 /dev/root rw`, + ].join('\n'), + ), sleep: jest.fn().mockResolvedValue(undefined), ...overrides, }; } +function manager(deps: VirtiofsdDependencies, cgroup?: Pick) { + return new VirtiofsdManager( + '/opt/virtiofsd', + '/run/awf/run', + '/run/awf-shares/run', + { uid: 1000, gid: 1000 }, + cgroup ?? { assign: jest.fn().mockResolvedValue(undefined) }, + { mount: '/usr/bin/mount', umount: '/usr/bin/umount' }, + deps, + ); +} + +const enforcement = { + plans: [ + { + tag: 'workspace', + writableOverlays: [ + { + source: '/host/workspace/out', + destination: '/host/workspace/out', + kind: 'directory' as const, + }, + ], + }, + ], +}; + describe('VirtiofsdManager', () => { it('uses explicit sandbox, seccomp, cache, and inode policy', () => { expect(buildVirtiofsdArgs(cache, '/run/awf/cache.sock', '/run/awf-ro/cache')).toEqual([ @@ -156,3 +201,121 @@ describe('VirtiofsdManager', () => { expect(unmountAttempts).toBe(2); }); }); + +describe('VirtiofsdManager host mount-tree enforcement', () => { + it('leaves behaviour untouched when no plan matches an export', async () => { + const deps = dependencies(); + const started = manager(deps); + await started.start([workspace, cache], { plans: [{ tag: 'other', writableOverlays: [] }] }); + expect(deps.captureTool).not.toHaveBeenCalled(); + const [, workspaceArgs] = (deps.launch as jest.Mock).mock.calls[0]; + expect(workspaceArgs).toEqual([ + '--socket-path=/run/awf/run/virtiofs-0.sock', + '--shared-dir=/host/workspace', + '--sandbox=namespace', + '--seccomp=kill', + '--cache=auto', + '--inode-file-handles=never', + ]); + expect(deps.runTool).toHaveBeenCalledWith('/usr/bin/mount', [ + '--bind', '/host/cache', '/run/awf-shares/run/1-cache', + ]); + expect(deps.runTool).not.toHaveBeenCalledWith( + '/usr/bin/mount', + expect.arrayContaining(['--rbind']), + ); + }); + + it('serves a staged tree and announces submounts for a planned export', async () => { + const deps = dependencies(); + const started = manager(deps); + const devices = await started.start([workspace], enforcement); + expect(devices).toHaveLength(1); + const [binary, args] = (deps.launch as jest.Mock).mock.calls[0]; + expect(binary).toBe('/opt/virtiofsd'); + expect(args).toEqual([ + '--socket-path=/run/awf/run/virtiofs-0.sock', + `--shared-dir=${STAGED_ROOT}`, + '--sandbox=namespace', + '--seccomp=kill', + '--cache=auto', + '--inode-file-handles=never', + '--announce-submounts', + ]); + expect((deps.runTool as jest.Mock).mock.calls).toEqual([ + ['/usr/bin/mount', ['--rbind', '/host/workspace', STAGED_ROOT]], + ['/usr/bin/mount', ['--make-rprivate', STAGED_ROOT]], + ['/usr/bin/mount', ['-o', 'remount,bind,ro,nosuid,nodev', STAGED_ROOT]], + ['/usr/bin/mount', ['--bind', '/host/workspace/out', `${STAGED_ROOT}/out`]], + ['/usr/bin/mount', ['-o', 'remount,bind,rw,nosuid,nodev', `${STAGED_ROOT}/out`]], + ]); + expect(deps.mkdir).toHaveBeenCalledWith(STAGED_ROOT, { recursive: true, mode: 0o700 }); + }); + + it('tears the staged tree down deepest-first on stop', async () => { + const deps = dependencies(); + const started = manager(deps); + await started.start([workspace], enforcement); + (deps.runTool as jest.Mock).mockClear(); + await started.stop(); + expect((deps.runTool as jest.Mock).mock.calls).toEqual([ + ['/usr/bin/umount', [`${STAGED_ROOT}/out`]], + ['/usr/bin/umount', ['-R', STAGED_ROOT]], + ]); + expect(deps.rmdir).toHaveBeenCalledWith(STAGED_ROOT); + }); + + it('unmounts the staged tree when the daemon fails to start', async () => { + const exited = processMock(300); + Object.assign(exited, { exitCode: 1 }); + const deps = dependencies({ launch: jest.fn().mockReturnValue(exited) }); + const started = manager(deps); + await expect(started.start([workspace], enforcement)).rejects.toThrow( + /exited before socket readiness/, + ); + expect(deps.runTool).toHaveBeenCalledWith('/usr/bin/umount', [`${STAGED_ROOT}/out`]); + expect(deps.runTool).toHaveBeenCalledWith('/usr/bin/umount', ['-R', STAGED_ROOT]); + }); + + it('keeps a staged tree that could not be unmounted during a failed start', async () => { + let unmountAttempts = 0; + const deps = dependencies({ + runTool: jest.fn(async (command: string, args: readonly string[]) => { + if (command.endsWith('umount')) { + unmountAttempts += 1; + // Fails during rollback, during start()'s own cleanup, and once more + // during the first explicit stop(). + if (unmountAttempts <= 3) throw new Error('busy'); + return; + } + if (args[0] === '--make-rprivate') throw new Error('propagation change failed'); + }), + }); + const started = manager(deps); + await expect(started.start([workspace], enforcement)).rejects.toThrow( + /propagation change failed; staged mount cleanup failed: busy/, + ); + await expect(started.stop()).rejects.toThrow('busy'); + await expect(started.stop()).resolves.toBeUndefined(); + expect(unmountAttempts).toBe(4); + expect(deps.rmdir).toHaveBeenCalledWith(STAGED_ROOT); + }); + + it('refuses writable overlays for an export that is already read-only', async () => { + const deps = dependencies(); + const started = manager(deps); + await expect( + started.start([cache], { + plans: [ + { + tag: 'cache', + writableOverlays: [ + { source: '/host/cache/out', destination: '/host/cache/out', kind: 'directory' }, + ], + }, + ], + }), + ).rejects.toThrow(/cannot receive writable overlays/); + expect(deps.launch).not.toHaveBeenCalled(); + }); +}); diff --git a/src/cloud-hypervisor/virtiofsd.ts b/src/cloud-hypervisor/virtiofsd.ts index f3553bcdc..14db8c754 100644 --- a/src/cloud-hypervisor/virtiofsd.ts +++ b/src/cloud-hypervisor/virtiofsd.ts @@ -3,6 +3,23 @@ import * as path from 'path'; import execa, { type ExecaChildProcess } from 'execa'; import type { CloudHypervisorCgroup } from './launcher'; import type { CloudHypervisorDirectoryExport } from './exports'; +import { + StagedHostMountTree, + selectMountPlan, + type MountTreeDependencies, + type MountTreeStats, + type VirtiofsdExportMountPlan, + type VirtiofsdMountEnforcement, +} from './mount-tree'; + +export type { + MountTreeDependencies, + MountTreeStats, + VirtiofsdExportMountPlan, + VirtiofsdMountEnforcement, + VirtiofsdOverlayKind, + VirtiofsdWritableOverlay, +} from './mount-tree'; const SOCKET_READY_TIMEOUT_MS = 5_000; const SOCKET_READY_INTERVAL_MS = 25; @@ -15,7 +32,7 @@ export interface VirtiofsdDevice { readonly logPath: string; } -export interface VirtiofsdDependencies { +export interface VirtiofsdDependencies extends MountTreeDependencies { launch( command: string, args: string[], @@ -33,6 +50,10 @@ export interface VirtiofsdDependencies { mkdir(directory: string, options: { recursive: true; mode: number }): Promise; rmdir(directory: string): Promise; runTool(command: string, args: readonly string[]): Promise; + captureTool(command: string, args: readonly string[]): Promise; + statPath(filePath: string): Promise; + realpath(filePath: string): Promise; + readMountInfo(): Promise; sleep(milliseconds: number): Promise; } @@ -58,6 +79,24 @@ const defaultDependencies: VirtiofsdDependencies = { ); } }, + captureTool: async (command, args) => { + const result = await execa(command, [...args], { + reject: false, + stdio: ['ignore', 'pipe', 'pipe'], + env: { PATH: '/usr/sbin:/usr/bin:/sbin:/bin' }, + extendEnv: false, + }); + if (result.exitCode !== 0) { + throw new Error( + `${command} ${args.join(' ')} exited with code ${result.exitCode}: ` + + `${result.stderr.trim() || result.stdout.trim()}`, + ); + } + return result.stdout; + }, + statPath: fs.lstat, + realpath: fs.realpath, + readMountInfo: () => fs.readFile('/proc/self/mountinfo', 'utf8'), sleep: (milliseconds) => new Promise((resolve) => setTimeout(resolve, milliseconds)), }; @@ -66,11 +105,14 @@ interface RunningDaemon extends VirtiofsdDevice { readonly stdout: BoundedCapture; readonly stderr: BoundedCapture; readonlyBindPath?: string; + mountTree?: StagedHostMountTree; socketRemoved: boolean; } export class VirtiofsdManager { private readonly running: RunningDaemon[] = []; + /** Mount trees whose staging failed with residue that still needs unmounting. */ + private readonly orphanedMountTrees: StagedHostMountTree[] = []; constructor( private readonly binaryPath: string, @@ -82,10 +124,19 @@ export class VirtiofsdManager { private readonly dependencies: VirtiofsdDependencies = defaultDependencies, ) {} - async start(exports: readonly CloudHypervisorDirectoryExport[]): Promise { + /** + * Starts one virtiofsd per export. When `enforcement` supplies a plan for an + * export tag, that export is served from a private, recursively read-only + * staged host mount tree with writable child binds. Without a plan the export + * is staged exactly as before. + */ + async start( + exports: readonly CloudHypervisorDirectoryExport[], + enforcement?: VirtiofsdMountEnforcement, + ): Promise { try { for (const [index, directoryExport] of exports.entries()) { - await this.startOne(directoryExport, index); + await this.startOne(directoryExport, index, selectMountPlan(enforcement, directoryExport.tag)); } return this.running.map(({ export: item, socketPath, logPath }) => ({ export: item, @@ -107,6 +158,16 @@ export class VirtiofsdManager { async stop(): Promise { const errors: unknown[] = []; const remaining: RunningDaemon[] = []; + for (const tree of [...this.orphanedMountTrees]) { + try { + await tree.unmount(); + } catch (error) { + errors.push(error); + } + if (!tree.hasResidue) { + this.orphanedMountTrees.splice(this.orphanedMountTrees.indexOf(tree), 1); + } + } for (const daemon of [...this.running].reverse()) { let processTerminated = daemon.process.exitCode !== null || daemon.process.signalCode !== null; try { @@ -154,12 +215,20 @@ export class VirtiofsdManager { errors.push(error); } } - if (!daemon.socketRemoved || daemon.readonlyBindPath) { + if (daemon.mountTree) { + try { + await daemon.mountTree.unmount(); + } catch (error) { + errors.push(error); + } + if (!daemon.mountTree.hasResidue) daemon.mountTree = undefined; + } + if (!daemon.socketRemoved || daemon.readonlyBindPath || daemon.mountTree) { remaining.unshift(daemon); } } this.running.splice(0, this.running.length, ...remaining); - if (this.running.length === 0) { + if (this.running.length === 0 && this.orphanedMountTrees.length === 0) { try { await this.dependencies.rmdir(this.shareDirectory); } catch (error) { @@ -175,12 +244,29 @@ export class VirtiofsdManager { private async startOne( directoryExport: CloudHypervisorDirectoryExport, index: number, + plan?: VirtiofsdExportMountPlan, ): Promise { const socketPath = path.join(this.runDirectory, `virtiofs-${index}.sock`); const logPath = path.join(this.runDirectory, `virtiofs-${index}.log`); let sharedDirectory = directoryExport.source; let readonlyBindPath: string | undefined; - if (directoryExport.mode === 'ro') { + let mountTree: StagedHostMountTree | undefined; + if (plan) { + mountTree = new StagedHostMountTree({ + directoryExport, + rootPath: path.join(this.shareDirectory, `${index}-${directoryExport.tag}`), + plan, + tools: this.tools, + dependencies: this.dependencies, + }); + try { + await mountTree.stage(); + } catch (error) { + if (mountTree.hasResidue) this.orphanedMountTrees.push(mountTree); + throw error; + } + sharedDirectory = mountTree.rootPath; + } else if (directoryExport.mode === 'ro') { readonlyBindPath = path.join(this.shareDirectory, `${index}-${directoryExport.tag}`); await this.dependencies.mkdir(readonlyBindPath, { recursive: true, mode: 0o700 }); let bindMounted = false; @@ -209,7 +295,9 @@ export class VirtiofsdManager { } sharedDirectory = readonlyBindPath; } - const args = buildVirtiofsdArgs(directoryExport, socketPath, sharedDirectory); + const args = buildVirtiofsdArgs(directoryExport, socketPath, sharedDirectory, { + announceSubmounts: mountTree !== undefined, + }); const child = this.dependencies.launch(this.binaryPath, args, { reject: false, stdio: ['ignore', 'pipe', 'pipe'], @@ -228,6 +316,7 @@ export class VirtiofsdManager { stdout, stderr, readonlyBindPath, + mountTree, socketRemoved: false, }; this.running.push(daemon); @@ -264,10 +353,20 @@ export class VirtiofsdManager { } } +export interface VirtiofsdArgOptions { + /** + * Required when the shared directory is a staged mount tree: the guest must + * see each writable child bind as its own submount instead of a hole in an + * otherwise read-only tree. + */ + readonly announceSubmounts?: boolean; +} + export function buildVirtiofsdArgs( directoryExport: CloudHypervisorDirectoryExport, socketPath: string, sharedDirectory = directoryExport.source, + options: VirtiofsdArgOptions = {}, ): string[] { if (!path.isAbsolute(socketPath)) { throw new Error(`virtiofsd socket path must be absolute: ${socketPath}`); @@ -279,6 +378,7 @@ export function buildVirtiofsdArgs( '--seccomp=kill', '--cache=auto', '--inode-file-handles=never', + ...(options.announceSubmounts ? ['--announce-submounts'] : []), ]; } From ac27cab27204e5c3a2a75c7b5c426cebddb1772d Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sun, 23 Aug 2026 09:52:20 -0700 Subject: [PATCH 2/3] fix: canonicalize staged overlay destinations and reject unknown plan tags Two issues found in review of the host mount-tree enforcement foundation. `lstat` on an overlay destination only reveals a symlink in the final component, while the kernel resolves every intermediate component when it binds. A `tools -> /etc` symlink carried in from the export made destination `tools/sudoers` lstat as an ordinary file and then bind over the host's `/etc/sudoers`, escaping the staged root before any post-bind verification ran. Destinations are now canonicalized and must satisfy realpath equality and containment under the staged root, mirroring source validation, and the staged root itself must be canonical for that comparison to mean anything. Verified on a live kernel: both intermediate and final symlink escapes are rejected before any bind reaches the kernel, and the host target is untouched. An enforcement plan naming an export tag that does not exist was silently discarded, so a renamed or mistyped tag would downgrade that export to unrestricted read-write. Plans are now reconciled against exports at start and an unknown tag throws. Exports without a plan still use the existing path, so partial enforcement stays supported. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/cloud-hypervisor-foundation.md | 24 +++++++--- src/cloud-hypervisor/mount-tree.test.ts | 64 ++++++++++++++++++++++++- src/cloud-hypervisor/mount-tree.ts | 55 ++++++++++++++++++--- src/cloud-hypervisor/virtiofsd.test.ts | 41 +++++++++++++++- src/cloud-hypervisor/virtiofsd.ts | 5 +- 5 files changed, 172 insertions(+), 17 deletions(-) diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index 514c4c821..8c0a67028 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -191,9 +191,13 @@ interface VirtiofsdMountEnforcement { } ``` -When the input is omitted, or no plan matches an export tag, that export is -staged exactly as before. When a plan matches, the export is served from a -private staged mount tree under the per-run virtiofsd share directory: +When the input is omitted, or no plan applies to an export tag, that export is +staged exactly as before, which is what makes partial enforcement possible. A +plan naming an export tag that does not exist is rejected outright: silently +dropping it would leave that export unrestricted read-write, so a renamed or +mistyped tag has to fail rather than downgrade. When a plan matches, the export +is served from a private staged mount tree under the per-run virtiofsd share +directory: 1. `mount --rbind ` — recursive bind, so nested host mounts are carried into the tree instead of being silently skipped. @@ -238,6 +242,13 @@ flags are the enforcement boundary. the export source, must not be symbolic links, and must match the declared kind. Overlay destinations must already exist, may not overlap each other, and an originally read-only export may not receive overlays at all. +- Overlay destinations are canonicalized before the bind and must satisfy + `realpath` equality and containment under the staged root. `lstat` alone is + not enough: it only reveals a symlink in the final component, while the kernel + resolves every intermediate component when it binds. A `tools -> /etc` symlink + carried in from the export would let destination `tools/sudoers` lstat as an + ordinary file and then bind over the host's `/etc/sudoers`. The staged root is + itself required to be canonical so that comparison is meaningful. - The staged root must be disjoint from the export source, so the recursive bind can never nest the staged tree inside itself. @@ -252,9 +263,10 @@ itself fails, the residual tree is retained and retried during `stop()`. ### Residual limitation -Overlay destinations are validated inside the staged tree, which is already -recursively read-only and privately propagated, so they cannot be swapped -between validation and the bind. Overlay *sources* live in the original, +Overlay destinations are canonicalized and validated inside the staged tree, +which is already recursively read-only and privately propagated, so they cannot +be swapped between validation and the bind. Overlay *sources* live in the +original, still-writable export, so a process that can already write to the export could in principle replace a source path between validation and the bind. Sources are re-validated immediately before each bind, and both the planner and this layer diff --git a/src/cloud-hypervisor/mount-tree.test.ts b/src/cloud-hypervisor/mount-tree.test.ts index 03e50175b..d77a83f12 100644 --- a/src/cloud-hypervisor/mount-tree.test.ts +++ b/src/cloud-hypervisor/mount-tree.test.ts @@ -434,9 +434,69 @@ describe('StagedHostMountTree', () => { fake, plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), workspace, - { realpath: jest.fn().mockResolvedValue('/host/other/out') }, + { + realpath: jest.fn(async (filePath: string) => + filePath === '/host/workspace/out' ? '/host/other/out' : filePath, + ), + }, + ); + await expect(staged.stage()).rejects.toThrow(/source must be canonical/); + expect(fake.table.size).toBe(0); + }); + + it('rejects a destination whose final component is a symlink out of the tree', async () => { + const fake = mountTable(); + const escaped = `${ROOT}/out`; + const staged = tree( + fake, + plan([{ source: '/host/workspace/out', destination: '/host/workspace/out', kind: 'directory' }]), + workspace, + { + realpath: jest.fn(async (filePath: string) => + filePath === escaped ? '/etc' : filePath, + ), + }, + ); + await expect(staged.stage()).rejects.toThrow(/destination must be canonical/); + // The escaped path must never reach the kernel. + expect(fake.commands.flat()).not.toContain('--bind'); + expect(fake.table.size).toBe(0); + }); + + it('rejects a destination reached through an intermediate symlink', async () => { + // `/tools` is a symlink to /etc, so lstat of `/tools/sudoers` + // reports an ordinary file while the kernel would bind over /etc/sudoers. + const fake = mountTable(); + const staged = tree( + fake, + plan([ + { + source: '/host/workspace/tools/sudoers', + destination: '/host/workspace/tools/sudoers', + kind: 'file', + }, + ]), + workspace, + { + realpath: jest.fn(async (filePath: string) => + filePath === `${ROOT}/tools/sudoers` ? '/etc/sudoers' : filePath, + ), + statPath: jest.fn(async () => stats('file')), + }, ); - await expect(staged.stage()).rejects.toThrow(/must be canonical/); + await expect(staged.stage()).rejects.toThrow(/destination must be canonical/); + expect(fake.commands.flat()).not.toContain('--bind'); + expect(fake.table.size).toBe(0); + }); + + it('rejects a staged root that is not canonical', async () => { + const fake = mountTable(); + const staged = tree(fake, plan([]), workspace, { + realpath: jest.fn(async (filePath: string) => + filePath === ROOT ? '/var/lib/elsewhere' : filePath, + ), + }); + await expect(staged.stage()).rejects.toThrow(/root must be canonical/); expect(fake.table.size).toBe(0); }); diff --git a/src/cloud-hypervisor/mount-tree.ts b/src/cloud-hypervisor/mount-tree.ts index 7edf64081..66aae7df1 100644 --- a/src/cloud-hypervisor/mount-tree.ts +++ b/src/cloud-hypervisor/mount-tree.ts @@ -135,6 +135,28 @@ export function selectMountPlan( return matches[0]; } +/** + * Fails closed when a plan names an export that does not exist. A silently + * dropped plan would downgrade that export to unrestricted read-write, so a + * renamed or mistyped tag must be an error. Exports without a plan keep their + * existing behaviour, which is what makes partial enforcement possible. + */ +export function assertPlansMatchExports( + enforcement: VirtiofsdMountEnforcement | undefined, + exports: readonly { readonly tag: string }[], +): void { + if (!enforcement) return; + const known = new Set(exports.map((item) => item.tag)); + const unknown = enforcement.plans + .map((plan) => plan.tag) + .filter((tag) => !known.has(tag)); + if (unknown.length > 0) { + throw new Error( + `Cloud Hypervisor mount plans reference unknown export tags: ${[...new Set(unknown)].sort().join(', ')}`, + ); + } +} + /** * A staged mount tree. Instances are created unmounted; {@link stage} performs * the privileged work and {@link unmount} tears it down deepest-first. Failed @@ -202,6 +224,15 @@ export class StagedHostMountTree { await assertMountToolSupported(tools, dependencies); await dependencies.mkdir(this.rootPath, { recursive: true, mode: 0o700 }); this.rootDirectoryCreated = true; + // Overlay destinations are validated by `realpath` equality against paths + // built from this root, so the root itself has to be canonical for that + // comparison to mean anything. + const resolvedRoot = await dependencies.realpath(this.rootPath); + if (resolvedRoot !== this.rootPath) { + throw new Error( + `Staged mount tree root must be canonical: ${this.rootPath} resolves to ${resolvedRoot}`, + ); + } await dependencies.runTool(tools.mount, ['--rbind', directoryExport.source, this.rootPath]); this.pendingMounts.add(this.rootPath); // Private propagation before anything writable exists, so neither the @@ -346,14 +377,26 @@ export class StagedHostMountTree { * The destination is inspected inside the staged tree, which is already * recursively read-only and privately propagated, so it cannot be swapped * between this check and the bind. + * + * `lstat` alone is not sufficient: it only reveals a symlink in the final + * component, while the kernel resolves every intermediate component when it + * binds. A staged `tools -> /etc` symlink would make `tools/sudoers` lstat as + * an ordinary file and then bind over the host's `/etc/sudoers`. `realpath` + * equality rejects a symlink in any component, and the containment check + * keeps the resolved target inside the staged root. */ private async assertOverlayDestination(overlay: ResolvedOverlay): Promise { - const stats = await this.options.dependencies.statPath(overlay.stagedDestination); - assertStatsMatchKind( - stats, - overlay.kind, - `writable overlay destination ${overlay.stagedDestination}`, - ); + const { dependencies } = this.options; + const label = `writable overlay destination ${overlay.stagedDestination}`; + const resolved = await dependencies.realpath(overlay.stagedDestination); + if (resolved !== overlay.stagedDestination) { + throw new Error( + `Writable overlay destination must be canonical: ${overlay.stagedDestination} resolves to ${resolved}`, + ); + } + assertContainedPath(this.rootPath, resolved, label); + const stats = await dependencies.statPath(overlay.stagedDestination); + assertStatsMatchKind(stats, overlay.kind, label); } private async assertTreeIsReadonly(): Promise { diff --git a/src/cloud-hypervisor/virtiofsd.test.ts b/src/cloud-hypervisor/virtiofsd.test.ts index 43845e2d8..5f8cfec41 100644 --- a/src/cloud-hypervisor/virtiofsd.test.ts +++ b/src/cloud-hypervisor/virtiofsd.test.ts @@ -203,10 +203,47 @@ describe('VirtiofsdManager', () => { }); describe('VirtiofsdManager host mount-tree enforcement', () => { - it('leaves behaviour untouched when no plan matches an export', async () => { + it('fails closed when a plan names an export that does not exist', async () => { const deps = dependencies(); const started = manager(deps); - await started.start([workspace, cache], { plans: [{ tag: 'other', writableOverlays: [] }] }); + // A renamed or mistyped tag must not silently downgrade an export to + // unrestricted read-write. + await expect( + started.start([workspace, cache], { plans: [{ tag: 'other', writableOverlays: [] }] }), + ).rejects.toThrow(/unknown export tags: other/); + expect(deps.launch).not.toHaveBeenCalled(); + expect(deps.runTool).not.toHaveBeenCalled(); + }); + + it('leaves unplanned exports on the legacy path when other exports are planned', async () => { + const deps = dependencies({ + readMountInfo: jest + .fn() + .mockResolvedValue(`30 29 0:42 / ${STAGED_ROOT} ro,nosuid,nodev - ext4 /dev/root ro`), + }); + const started = manager(deps); + await started.start([workspace, cache], { plans: [{ tag: 'workspace', writableOverlays: [] }] }); + const [, workspaceArgs] = (deps.launch as jest.Mock).mock.calls[0]; + expect(workspaceArgs).toContain('--announce-submounts'); + const [, cacheArgs] = (deps.launch as jest.Mock).mock.calls[1]; + expect(cacheArgs).toEqual([ + '--socket-path=/run/awf/run/virtiofs-1.sock', + '--shared-dir=/run/awf-shares/run/1-cache', + '--sandbox=namespace', + '--seccomp=kill', + '--cache=auto', + '--inode-file-handles=never', + ]); + expect(cacheArgs).not.toContain('--announce-submounts'); + expect(deps.runTool).toHaveBeenCalledWith('/usr/bin/mount', [ + '--bind', '/host/cache', '/run/awf-shares/run/1-cache', + ]); + }); + + it('leaves behaviour untouched when no enforcement is supplied', async () => { + const deps = dependencies(); + const started = manager(deps); + await started.start([workspace, cache]); expect(deps.captureTool).not.toHaveBeenCalled(); const [, workspaceArgs] = (deps.launch as jest.Mock).mock.calls[0]; expect(workspaceArgs).toEqual([ diff --git a/src/cloud-hypervisor/virtiofsd.ts b/src/cloud-hypervisor/virtiofsd.ts index 14db8c754..8c09c1f18 100644 --- a/src/cloud-hypervisor/virtiofsd.ts +++ b/src/cloud-hypervisor/virtiofsd.ts @@ -6,6 +6,7 @@ import type { CloudHypervisorDirectoryExport } from './exports'; import { StagedHostMountTree, selectMountPlan, + assertPlansMatchExports, type MountTreeDependencies, type MountTreeStats, type VirtiofsdExportMountPlan, @@ -128,13 +129,15 @@ export class VirtiofsdManager { * Starts one virtiofsd per export. When `enforcement` supplies a plan for an * export tag, that export is served from a private, recursively read-only * staged host mount tree with writable child binds. Without a plan the export - * is staged exactly as before. + * is staged exactly as before. A plan naming an export that does not exist is + * an error rather than a silent downgrade to unrestricted read-write. */ async start( exports: readonly CloudHypervisorDirectoryExport[], enforcement?: VirtiofsdMountEnforcement, ): Promise { try { + assertPlansMatchExports(enforcement, exports); for (const [index, directoryExport] of exports.entries()) { await this.startOne(directoryExport, index, selectMountPlan(enforcement, directoryExport.tag)); } From bad9321e0fa13c77f37e494fe53e680673be4464 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:09:02 +0000 Subject: [PATCH 3/3] fix: reject slave staged mounts --- src/cloud-hypervisor/mount-tree.test.ts | 24 ++++++++++++++++++++++++ src/cloud-hypervisor/mount-tree.ts | 11 ++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/cloud-hypervisor/mount-tree.test.ts b/src/cloud-hypervisor/mount-tree.test.ts index d77a83f12..b87bef007 100644 --- a/src/cloud-hypervisor/mount-tree.test.ts +++ b/src/cloud-hypervisor/mount-tree.test.ts @@ -372,6 +372,30 @@ describe('StagedHostMountTree', () => { await expect(staged.stage()).rejects.toThrow(/propagation would leak/); }); + it.each(['master:21', 'propagate_from:21'])( + 'fails closed when the staged tree retains %s propagation', + async (propagation) => { + const fake = mountTable(); + fake.runTool.mockImplementation(async (command: string, args: readonly string[]) => { + if (args[0] === '--make-rprivate') return; + return undefined; + }); + const deps = dependencies(fake, { + readMountInfo: jest + .fn() + .mockResolvedValue(`30 29 0:42 / ${ROOT} ro,nosuid,nodev ${propagation} - ext4 /dev/root ro`), + }); + const staged = new StagedHostMountTree({ + directoryExport: workspace, + plan: plan([]), + rootPath: ROOT, + tools, + dependencies: deps, + }); + await expect(staged.stage()).rejects.toThrow(/propagation would leak/); + }, + ); + it('fails closed when the staged root mount is missing', async () => { const fake = mountTable(); const staged = tree(fake, plan([]), workspace, { diff --git a/src/cloud-hypervisor/mount-tree.ts b/src/cloud-hypervisor/mount-tree.ts index 66aae7df1..73c69f02f 100644 --- a/src/cloud-hypervisor/mount-tree.ts +++ b/src/cloud-hypervisor/mount-tree.ts @@ -463,10 +463,15 @@ export function parseMountInfo(contents: string): MountInfoEntry[] { } function assertPrivatePropagation(entry: MountInfoEntry): void { - const shared = entry.optionalFields.some((field) => field.startsWith('shared:')); - if (shared) { + const propagation = entry.optionalFields.find( + (field) => + field.startsWith('shared:') || + field.startsWith('master:') || + field.startsWith('propagate_from:'), + ); + if (propagation !== undefined) { throw new Error( - `Staged mount tree propagation would leak: ${entry.mountPoint} is a shared mount`, + `Staged mount tree propagation would leak: ${entry.mountPoint} has ${propagation}`, ); } }