From 98298e9a2102a3724f591d9b604fb36994d3ad4a Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sun, 23 Aug 2026 09:19:52 -0700 Subject: [PATCH 1/4] feat(cloud-hypervisor): add filesystem.allowWrite policy planner Add a pure, strongly typed planner that computes how a filesystem.allowWrite allowlist would narrow validated Cloud Hypervisor directory exports. The planner classifies each export as unrestricted, read-only, fully writable, or selectively writable, and returns canonical host/guest overlay paths so a later integration can mount them without re-resolving symlinks. It only removes write access: read-only exports are never widened and no host path outside an existing read-write export is ever exposed. The planner is inert. It is not wired into runtime execution and filesystem.allowWrite remains rejected for the Cloud Hypervisor runtime. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/cloud-hypervisor-foundation.md | 14 + .../filesystem-write-policy.test.ts | 271 ++++++++++++++++++ .../filesystem-write-policy.ts | 236 +++++++++++++++ 3 files changed, 521 insertions(+) create mode 100644 src/cloud-hypervisor/filesystem-write-policy.test.ts create mode 100644 src/cloud-hypervisor/filesystem-write-policy.ts diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index 13e303b35..35303f26b 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -164,6 +164,20 @@ Temporary microVM workspace data lives under: With `--keep-containers`, AWF preserves this directory, the network namespace, and runtime diagnostics for investigation. +### Write-policy planning (inert) + +[`src/cloud-hypervisor/filesystem-write-policy.ts`](../src/cloud-hypervisor/filesystem-write-policy.ts) +plans how a `filesystem.allowWrite` allowlist would narrow validated exports. It +maps each guest path to the canonical host path beneath the deepest matching +export, rejects `..`, missing paths, and symlink escapes, and classifies every +export as unrestricted, read-only, fully writable, or selectively writable. + +The planner only removes write access: it never widens a read-only export and +never introduces a host path that an existing read-write export does not +already cover. It is pure policy planning and is not yet wired into runtime +execution — `filesystem.allowWrite` is still rejected for the Cloud Hypervisor +runtime by [`src/filesystem-policy.ts`](../src/filesystem-policy.ts). + ## Limitations The preview rejects configurations that weaken or conflict with its boundary, diff --git a/src/cloud-hypervisor/filesystem-write-policy.test.ts b/src/cloud-hypervisor/filesystem-write-policy.test.ts new file mode 100644 index 000000000..c0436c510 --- /dev/null +++ b/src/cloud-hypervisor/filesystem-write-policy.test.ts @@ -0,0 +1,271 @@ +import { promises as fs } from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { CloudHypervisorDirectoryExport } from './exports'; +import { planCloudHypervisorFilesystemWrites } from './filesystem-write-policy'; + +describe('Cloud Hypervisor filesystem write policy planner', () => { + let directory: string; + let workspaceSource: string; + let toolsSource: string; + let exports: CloudHypervisorDirectoryExport[]; + + beforeEach(async () => { + directory = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'ch-write-policy-'))); + workspaceSource = path.join(directory, 'workspace'); + toolsSource = path.join(directory, 'tools'); + await fs.mkdir(path.join(workspaceSource, 'nested', 'deep'), { recursive: true }); + await fs.mkdir(toolsSource, { recursive: true }); + await fs.writeFile(path.join(workspaceSource, 'nested', 'file.txt'), 'data'); + await fs.writeFile(path.join(toolsSource, 'tool.txt'), 'tool'); + exports = [ + { tag: 'workspace', source: workspaceSource, target: '/workspace', mode: 'rw' }, + { tag: 'runner-tool-cache', source: toolsSource, target: '/tools', mode: 'ro' }, + ]; + }); + + afterEach(async () => { + await fs.rm(directory, { recursive: true, force: true }); + }); + + it('preserves unrestricted per-export behavior when allowWrite is undefined', () => { + const plan = planCloudHypervisorFilesystemWrites(exports, undefined); + + expect(plan.restricted).toBe(false); + expect(plan.allowedPaths).toEqual([]); + expect(plan.overlays).toEqual([]); + expect(plan.exports).toEqual([ + { + export: exports[0], + disposition: 'unrestricted', + effectiveMode: 'rw', + internal: false, + overlays: [], + }, + { + export: exports[1], + disposition: 'unrestricted', + effectiveMode: 'ro', + internal: false, + overlays: [], + }, + ]); + }); + + it('makes every writable non-internal export read-only for an empty allowlist', () => { + const withInternal = [ + ...exports, + { tag: 'tmp-gh-aw', source: directory, target: '/tmp/gh-aw', mode: 'rw' as const }, + ]; + + const plan = planCloudHypervisorFilesystemWrites(withInternal, [], { + internalTags: ['tmp-gh-aw'], + }); + + expect(plan.restricted).toBe(true); + expect(plan.overlays).toEqual([]); + expect(plan.exports.map((entry) => [entry.export.tag, entry.disposition, entry.effectiveMode])) + .toEqual([ + ['workspace', 'read-only', 'ro'], + ['runner-tool-cache', 'read-only', 'ro'], + ['tmp-gh-aw', 'writable', 'rw'], + ]); + expect(plan.exports[2].internal).toBe(true); + }); + + it('keeps a whole export writable when its target is allowed', () => { + const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace']); + + expect(plan.allowedPaths).toEqual(['/workspace']); + expect(plan.exports[0]).toEqual({ + export: exports[0], + disposition: 'writable', + effectiveMode: 'rw', + internal: false, + overlays: [], + }); + expect(plan.exports[1].disposition).toBe('read-only'); + expect(plan.overlays).toEqual([]); + }); + + it('narrows an export to a nested writable directory overlay', () => { + const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace/nested/deep']); + + expect(plan.exports[0].disposition).toBe('selective'); + expect(plan.exports[0].effectiveMode).toBe('ro'); + expect(plan.overlays).toEqual([ + { + exportTag: 'workspace', + guestPath: '/workspace/nested/deep', + hostPath: path.join(workspaceSource, 'nested', 'deep'), + relativePath: 'nested/deep', + kind: 'directory', + }, + ]); + }); + + it('supports an existing file as an allowed path', () => { + const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace/nested/file.txt']); + + expect(plan.overlays).toEqual([ + { + exportTag: 'workspace', + guestPath: '/workspace/nested/file.txt', + hostPath: path.join(workspaceSource, 'nested', 'file.txt'), + relativePath: 'nested/file.txt', + kind: 'file', + }, + ]); + }); + + it('translates guest paths to host source paths for non-identity targets', async () => { + const source = path.join(directory, 'exported'); + await fs.mkdir(path.join(source, 'sub'), { recursive: true }); + const plan = planCloudHypervisorFilesystemWrites( + [ + { tag: 'workspace', source, target: '/workspace', mode: 'rw' }, + ], + ['/workspace/sub'], + ); + + expect(plan.overlays).toEqual([ + { + exportTag: 'workspace', + guestPath: '/workspace/sub', + hostPath: path.join(source, 'sub'), + relativePath: 'sub', + kind: 'directory', + }, + ]); + }); + + it('normalizes duplicates and drops descendants covered by an ancestor', () => { + const plan = planCloudHypervisorFilesystemWrites(exports, [ + '/workspace/nested', + '/workspace/./nested/', + '/workspace/nested/deep', + '/workspace/nested/file.txt', + ]); + + expect(plan.allowedPaths).toEqual(['/workspace/nested']); + expect(plan.overlays).toHaveLength(1); + expect(plan.overlays[0].guestPath).toBe('/workspace/nested'); + }); + + it('rejects relative paths and paths containing ".."', () => { + expect(() => planCloudHypervisorFilesystemWrites(exports, ['workspace/nested'])) + .toThrow("filesystem.allowWrite path must be absolute without '..': workspace/nested"); + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/workspace/../etc'])) + .toThrow("filesystem.allowWrite path must be absolute without '..': /workspace/../etc"); + }); + + it('rejects a path that does not exist on the host', () => { + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/workspace/missing'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /workspace/missing', + ); + }); + + it('rejects a symlink that escapes the export source', async () => { + const outside = path.join(directory, 'outside'); + await fs.mkdir(outside); + await fs.symlink(outside, path.join(workspaceSource, 'escape')); + + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/workspace/escape'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /workspace/escape', + ); + }); + + it('never upgrades a read-only export', () => { + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/tools/tool.txt'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /tools/tool.txt', + ); + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/tools'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /tools', + ); + + const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace/nested']); + expect(plan.exports[1]).toEqual({ + export: exports[1], + disposition: 'read-only', + effectiveMode: 'ro', + internal: false, + overlays: [], + }); + }); + + it('rejects a path outside every export target', () => { + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/elsewhere'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /elsewhere', + ); + }); + + it('reports every unmatched path in one error', () => { + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/elsewhere', '/workspace/missing'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /elsewhere, /workspace/missing', + ); + }); + + it('resolves overlapping exports to the deepest matching export', () => { + const nestedSource = path.join(workspaceSource, 'nested'); + const overlapping: CloudHypervisorDirectoryExport[] = [ + { tag: 'workspace', source: workspaceSource, target: '/workspace', mode: 'rw' }, + { tag: 'nested', source: nestedSource, target: '/workspace/nested', mode: 'rw' }, + ]; + + const plan = planCloudHypervisorFilesystemWrites(overlapping, ['/workspace/nested/deep']); + + expect(plan.exports[0].disposition).toBe('read-only'); + expect(plan.exports[1].disposition).toBe('selective'); + expect(plan.overlays).toEqual([ + { + exportTag: 'nested', + guestPath: '/workspace/nested/deep', + hostPath: path.join(nestedSource, 'deep'), + relativePath: 'deep', + kind: 'directory', + }, + ]); + }); + + it('does not widen a deeper read-only export through a shallower writable one', () => { + const overlapping: CloudHypervisorDirectoryExport[] = [ + { tag: 'workspace', source: workspaceSource, target: '/workspace', mode: 'rw' }, + { + tag: 'nested', + source: path.join(workspaceSource, 'nested'), + target: '/workspace/nested', + mode: 'ro', + }, + ]; + + expect(() => planCloudHypervisorFilesystemWrites(overlapping, ['/workspace/nested/deep'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /workspace/nested/deep', + ); + }); + + it('collects multiple overlays for one export in allowlist order', () => { + const plan = planCloudHypervisorFilesystemWrites(exports, [ + '/workspace/nested/deep', + '/workspace/nested/file.txt', + ]); + + expect(plan.exports[0].disposition).toBe('selective'); + expect(plan.exports[0].overlays.map((overlay) => overlay.guestPath)) + .toEqual(['/workspace/nested/deep', '/workspace/nested/file.txt']); + expect(plan.overlays).toHaveLength(2); + }); +}); diff --git a/src/cloud-hypervisor/filesystem-write-policy.ts b/src/cloud-hypervisor/filesystem-write-policy.ts new file mode 100644 index 000000000..d8db1b200 --- /dev/null +++ b/src/cloud-hypervisor/filesystem-write-policy.ts @@ -0,0 +1,236 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import type { CloudHypervisorDirectoryExport, CloudHypervisorExportMode } from './exports'; + +/** + * How a single Cloud Hypervisor directory export is affected by + * `filesystem.allowWrite`. + * + * - `unrestricted`: no policy was supplied, the declared export mode stands. + * - `read-only`: the export is exposed read-only under the policy. + * - `writable`: the whole export stays read-write. + * - `selective`: the export itself is read-only, but the listed overlays below + * it must be re-exported read-write. + */ +export type CloudHypervisorExportWriteDisposition = + | 'unrestricted' + | 'read-only' + | 'writable' + | 'selective'; + +/** + * A canonical host/guest path pair that must stay writable inside an otherwise + * read-only export. Both paths are absolute and fully resolved, so a later + * integration can mount them directly without re-resolving symlinks. + */ +export interface CloudHypervisorWritableOverlay { + /** Tag of the export this overlay is carved out of. */ + readonly exportTag: string; + /** Guest-visible absolute path that must be writable. */ + readonly guestPath: string; + /** Canonical host path backing {@link guestPath}. */ + readonly hostPath: string; + /** Path of the overlay relative to the export target/source root. */ + readonly relativePath: string; + readonly kind: 'directory' | 'file'; +} + +export interface CloudHypervisorExportWritePlan { + readonly export: CloudHypervisorDirectoryExport; + readonly disposition: CloudHypervisorExportWriteDisposition; + /** Mode the export itself must be published with. */ + readonly effectiveMode: CloudHypervisorExportMode; + /** True when the export is AWF-owned and stays writable under any policy. */ + readonly internal: boolean; + /** Non-empty only when {@link disposition} is `selective`. */ + readonly overlays: readonly CloudHypervisorWritableOverlay[]; +} + +export interface CloudHypervisorFilesystemWritePlan { + /** False when `filesystem.allowWrite` was absent (`undefined`). */ + readonly restricted: boolean; + /** Normalized allowlist with duplicates and covered descendants removed. */ + readonly allowedPaths: readonly string[]; + readonly exports: readonly CloudHypervisorExportWritePlan[]; + /** Every overlay across all exports, in export order. */ + readonly overlays: readonly CloudHypervisorWritableOverlay[]; +} + +export interface CloudHypervisorFilesystemWritePolicyOptions { + /** + * Tags of AWF-owned exports that must remain writable for the sandbox to + * operate. They are never narrowed, mirroring the always-writable Docker + * mounts. + */ + readonly internalTags?: Iterable; +} + +/** + * Plans how `filesystem.allowWrite` narrows Cloud Hypervisor directory exports. + * + * The planner only ever removes write access: it never upgrades a read-only + * export and never exposes a host path that is not already reachable through an + * existing read-write export. `exports` is expected to already satisfy + * {@link validateCloudHypervisorExports}; overlapping targets are tolerated so + * that a future export layout resolves to the deepest matching export. + * + * This module is pure policy planning: it computes a plan and performs no + * mounting, launching, or other side effects. + */ +export function planCloudHypervisorFilesystemWrites( + exports: readonly CloudHypervisorDirectoryExport[], + allowWrite: string[] | undefined, + options: CloudHypervisorFilesystemWritePolicyOptions = {}, +): CloudHypervisorFilesystemWritePlan { + const internalTags = new Set(options.internalTags ?? []); + + if (allowWrite === undefined) { + return { + restricted: false, + allowedPaths: [], + exports: exports.map((entry) => ({ + export: entry, + disposition: 'unrestricted', + effectiveMode: entry.mode, + internal: internalTags.has(entry.tag), + overlays: [], + })), + overlays: [], + }; + } + + const allowedPaths = normalizeAllowedPaths(allowWrite); + const matched = new Set(); + const plans: CloudHypervisorExportWritePlan[] = exports.map((entry) => { + const internal = internalTags.has(entry.tag); + if (entry.mode !== 'rw') { + return { + export: entry, + disposition: 'read-only', + effectiveMode: 'ro', + internal, + overlays: [], + }; + } + if (internal) { + return { export: entry, disposition: 'writable', effectiveMode: 'rw', internal, overlays: [] }; + } + + const overlays: CloudHypervisorWritableOverlay[] = []; + for (const allowedPath of allowedPaths) { + if (isPathAtOrBelow(entry.target, allowedPath)) { + matched.add(allowedPath); + return { export: entry, disposition: 'writable', effectiveMode: 'rw', internal, overlays: [] }; + } + if (!isDeepestWritableExport(exports, entry, allowedPath)) continue; + + const overlay = resolveWritableOverlay(entry, allowedPath); + if (overlay) { + matched.add(allowedPath); + overlays.push(overlay); + } + } + + return { + export: entry, + disposition: overlays.length > 0 ? 'selective' : 'read-only', + effectiveMode: 'ro', + internal, + overlays, + }; + }); + + const unmatched = allowedPaths.filter((allowedPath) => !matched.has(allowedPath)); + if (unmatched.length > 0) { + throw new Error( + 'filesystem.allowWrite path is not an existing path within a writable ' + + `Cloud Hypervisor export: ${unmatched.join(', ')}`, + ); + } + + return { + restricted: true, + allowedPaths, + exports: plans, + overlays: plans.flatMap((plan) => plan.overlays), + }; +} + +function normalizeAllowedPaths(allowWrite: readonly string[]): string[] { + for (const value of allowWrite) { + if (!path.posix.isAbsolute(value) || value.split('/').includes('..') || value.includes('\0')) { + throw new Error(`filesystem.allowWrite path must be absolute without '..': ${value}`); + } + } + + const normalized = [...new Set(allowWrite.map(normalizeGuestPath))]; + return normalized.filter((candidate) => + !normalized.some((parent) => parent !== candidate && isPathAtOrBelow(candidate, parent)) + ); +} + +/** + * Current export validation rejects overlapping targets, so at most one export + * matches today. The deepest-match rule keeps the planner correct if nested + * exports are ever introduced, and refuses to widen a nested read-only export. + */ +function isDeepestWritableExport( + exports: readonly CloudHypervisorDirectoryExport[], + entry: CloudHypervisorDirectoryExport, + allowedPath: string, +): boolean { + const covering = exports.filter((candidate) => isPathAtOrBelow(allowedPath, candidate.target)); + if (!covering.includes(entry)) return false; + + const deepest = Math.max(...covering.map((candidate) => pathDepth(candidate.target))); + if (pathDepth(entry.target) !== deepest) return false; + return !covering.some( + (candidate) => pathDepth(candidate.target) === deepest && candidate.mode !== 'rw', + ); +} + +function resolveWritableOverlay( + entry: CloudHypervisorDirectoryExport, + allowedPath: string, +): CloudHypervisorWritableOverlay | undefined { + const relativePath = path.posix.relative(entry.target, allowedPath); + if (relativePath === '') return undefined; + + let realSourceRoot: string; + let realSource: string; + let stats: fs.Stats; + try { + realSourceRoot = fs.realpathSync(entry.source); + realSource = fs.realpathSync(path.join(entry.source, relativePath)); + stats = fs.statSync(realSource); + } catch { + return undefined; + } + + // Same invariant as the Docker policy: the allowlist may not escape the + // export source through a symlink anywhere below its root. + if (realSource !== path.resolve(realSourceRoot, relativePath)) return undefined; + if (!stats.isDirectory() && !stats.isFile()) return undefined; + + return { + exportTag: entry.tag, + guestPath: allowedPath, + hostPath: realSource, + relativePath, + kind: stats.isDirectory() ? 'directory' : 'file', + }; +} + +function normalizeGuestPath(value: string): string { + const normalized = path.posix.normalize(value); + return normalized.length > 1 ? normalized.replace(/\/+$/, '') : normalized; +} + +function isPathAtOrBelow(candidate: string, parent: string): boolean { + const relative = path.posix.relative(parent, candidate); + return relative === '' || (!relative.startsWith('..') && !path.posix.isAbsolute(relative)); +} + +function pathDepth(value: string): number { + return value.split('/').filter(Boolean).length; +} From 9c845e273a43eec959d209b9f33e834d9df16de4 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sun, 23 Aug 2026 09:25:03 -0700 Subject: [PATCH 2/4] fix(cloud-hypervisor): split host root mode from guest mount mode A selective export previously reported a single effectiveMode of "ro", which would be wrong for the guest mount. virtio-fs submounts are attached through d_automount, and finish_automount() calls do_add_mount(..., path->mnt->mnt_flags | MNT_SHRINKABLE), so an announced submount inherits MNT_READONLY from its parent mount. A guest-level MS_RDONLY on a composite tree would therefore block writes to every writable node below it. Replace effectiveMode with hostRootMode and guestMountMode. Read-only enforcement is a host-side property: a selective export stages a read-only host backing tree root while the guest mount stays read-write so the overlays remain writable. Also clarify that overlay guestPath is only lexically normalized while hostPath is realpath-canonical. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/cloud-hypervisor-foundation.md | 16 +++++ .../filesystem-write-policy.test.ts | 43 +++++++++--- .../filesystem-write-policy.ts | 67 +++++++++++++++---- 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index 35303f26b..a83ec90c4 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -172,6 +172,22 @@ maps each guest path to the canonical host path beneath the deepest matching export, rejects `..`, missing paths, and symlink escapes, and classifies every export as unrestricted, read-only, fully writable, or selectively writable. +Read-only enforcement is a host-side property. Each plan entry therefore carries +two modes: `hostRootMode`, the mode the host backing tree root is staged with — +the read-only bind that `virtiofsd.ts` already builds for read-only exports — +and `guestMountMode`, the flags of the guest virtio-fs mount. A selectively +writable export reports `hostRootMode: 'ro'` with `guestMountMode: 'rw'`: +mounting a composite tree read-only in the guest would also block its writable +nodes, because virtio-fs submounts are attached through `d_automount` and +`finish_automount()` calls +`do_add_mount(..., path->mnt->mnt_flags | MNT_SHRINKABLE)`, so an announced +submount inherits `MNT_READONLY` from its parent mount. The host VFS, not the +guest mount flag, denies writes outside the overlays. + +Overlay paths are absolute but canonical in different senses: `guestPath` is +lexically normalized, while `hostPath` is realpath-canonical and verified not to +escape the export source. + The planner only removes write access: it never widens a read-only export and never introduces a host path that an existing read-write export does not already cover. It is pure policy planning and is not yet wired into runtime diff --git a/src/cloud-hypervisor/filesystem-write-policy.test.ts b/src/cloud-hypervisor/filesystem-write-policy.test.ts index c0436c510..50eac9ba0 100644 --- a/src/cloud-hypervisor/filesystem-write-policy.test.ts +++ b/src/cloud-hypervisor/filesystem-write-policy.test.ts @@ -38,14 +38,16 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { { export: exports[0], disposition: 'unrestricted', - effectiveMode: 'rw', + hostRootMode: 'rw', + guestMountMode: 'rw', internal: false, overlays: [], }, { export: exports[1], disposition: 'unrestricted', - effectiveMode: 'ro', + hostRootMode: 'ro', + guestMountMode: 'ro', internal: false, overlays: [], }, @@ -64,11 +66,12 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { expect(plan.restricted).toBe(true); expect(plan.overlays).toEqual([]); - expect(plan.exports.map((entry) => [entry.export.tag, entry.disposition, entry.effectiveMode])) + expect(plan.exports.map((entry) => + [entry.export.tag, entry.disposition, entry.hostRootMode, entry.guestMountMode])) .toEqual([ - ['workspace', 'read-only', 'ro'], - ['runner-tool-cache', 'read-only', 'ro'], - ['tmp-gh-aw', 'writable', 'rw'], + ['workspace', 'read-only', 'ro', 'ro'], + ['runner-tool-cache', 'read-only', 'ro', 'ro'], + ['tmp-gh-aw', 'writable', 'rw', 'rw'], ]); expect(plan.exports[2].internal).toBe(true); }); @@ -80,7 +83,8 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { expect(plan.exports[0]).toEqual({ export: exports[0], disposition: 'writable', - effectiveMode: 'rw', + hostRootMode: 'rw', + guestMountMode: 'rw', internal: false, overlays: [], }); @@ -92,7 +96,8 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace/nested/deep']); expect(plan.exports[0].disposition).toBe('selective'); - expect(plan.exports[0].effectiveMode).toBe('ro'); + expect(plan.exports[0].hostRootMode).toBe('ro'); + expect(plan.exports[0].guestMountMode).toBe('rw'); expect(plan.overlays).toEqual([ { exportTag: 'workspace', @@ -104,6 +109,25 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { ]); }); + it('keeps the guest mount read-write wherever a writable overlay exists', () => { + const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace/nested/deep']); + + // A guest-level MS_RDONLY would propagate to announced virtio-fs submounts + // (finish_automount passes the parent's mnt_flags), so read-only enforcement + // for a selective export must come from the host backing tree instead. + for (const entry of plan.exports) { + if (entry.overlays.length > 0) { + expect(entry.disposition).toBe('selective'); + expect(entry.guestMountMode).toBe('rw'); + expect(entry.hostRootMode).toBe('ro'); + } else { + expect(entry.guestMountMode).toBe(entry.hostRootMode); + } + // A read-write host root is never published to a read-only guest mount. + expect(entry.hostRootMode === 'rw' && entry.guestMountMode === 'ro').toBe(false); + } + }); + it('supports an existing file as an allowed path', () => { const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace/nested/file.txt']); @@ -195,7 +219,8 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { expect(plan.exports[1]).toEqual({ export: exports[1], disposition: 'read-only', - effectiveMode: 'ro', + hostRootMode: 'ro', + guestMountMode: 'ro', internal: false, overlays: [], }); diff --git a/src/cloud-hypervisor/filesystem-write-policy.ts b/src/cloud-hypervisor/filesystem-write-policy.ts index d8db1b200..3e413fa17 100644 --- a/src/cloud-hypervisor/filesystem-write-policy.ts +++ b/src/cloud-hypervisor/filesystem-write-policy.ts @@ -9,8 +9,8 @@ import type { CloudHypervisorDirectoryExport, CloudHypervisorExportMode } from ' * - `unrestricted`: no policy was supplied, the declared export mode stands. * - `read-only`: the export is exposed read-only under the policy. * - `writable`: the whole export stays read-write. - * - `selective`: the export itself is read-only, but the listed overlays below - * it must be re-exported read-write. + * - `selective`: the host backing tree is staged read-only outside the listed + * overlays, which stay read-write. */ export type CloudHypervisorExportWriteDisposition = | 'unrestricted' @@ -19,16 +19,18 @@ export type CloudHypervisorExportWriteDisposition = | 'selective'; /** - * A canonical host/guest path pair that must stay writable inside an otherwise - * read-only export. Both paths are absolute and fully resolved, so a later - * integration can mount them directly without re-resolving symlinks. + * A host/guest path pair that must stay writable inside an otherwise read-only + * export. Both paths are absolute, but they are canonical in different senses: + * `guestPath` is only lexically normalized (the guest filesystem does not exist + * yet at planning time), while `hostPath` is realpath-canonical and verified not + * to escape the export source. */ export interface CloudHypervisorWritableOverlay { /** Tag of the export this overlay is carved out of. */ readonly exportTag: string; - /** Guest-visible absolute path that must be writable. */ + /** Guest-visible absolute path that must be writable, lexically normalized. */ readonly guestPath: string; - /** Canonical host path backing {@link guestPath}. */ + /** Realpath-canonical host path backing {@link guestPath}. */ readonly hostPath: string; /** Path of the overlay relative to the export target/source root. */ readonly relativePath: string; @@ -38,8 +40,25 @@ export interface CloudHypervisorWritableOverlay { export interface CloudHypervisorExportWritePlan { readonly export: CloudHypervisorDirectoryExport; readonly disposition: CloudHypervisorExportWriteDisposition; - /** Mode the export itself must be published with. */ - readonly effectiveMode: CloudHypervisorExportMode; + /** + * Mode the host backing tree root must be staged with. `ro` means the host + * VFS — a read-only bind of the export source, as `virtiofsd.ts` already does + * for read-only exports — is what denies writes, independently of any guest + * mount flag. + */ + readonly hostRootMode: CloudHypervisorExportMode; + /** + * Mode the guest virtio-fs mount must use. + * + * A `selective` export deliberately reports `rw` here while `hostRootMode` is + * `ro`. Mounting the composite tree read-only in the guest would also block + * the writable overlays: virtio-fs submounts are attached through + * `d_automount`, and `finish_automount()` calls + * `do_add_mount(..., path->mnt->mnt_flags | MNT_SHRINKABLE)`, so an announced + * submount inherits `MNT_READONLY` from its parent mount. Guest-side `ro` is + * therefore only correct when the whole export is read-only. + */ + readonly guestMountMode: CloudHypervisorExportMode; /** True when the export is AWF-owned and stays writable under any policy. */ readonly internal: boolean; /** Non-empty only when {@link disposition} is `selective`. */ @@ -91,7 +110,8 @@ export function planCloudHypervisorFilesystemWrites( exports: exports.map((entry) => ({ export: entry, disposition: 'unrestricted', - effectiveMode: entry.mode, + hostRootMode: entry.mode, + guestMountMode: entry.mode, internal: internalTags.has(entry.tag), overlays: [], })), @@ -107,20 +127,35 @@ export function planCloudHypervisorFilesystemWrites( return { export: entry, disposition: 'read-only', - effectiveMode: 'ro', + hostRootMode: 'ro', + guestMountMode: 'ro', internal, overlays: [], }; } if (internal) { - return { export: entry, disposition: 'writable', effectiveMode: 'rw', internal, overlays: [] }; + return { + export: entry, + disposition: 'writable', + hostRootMode: 'rw', + guestMountMode: 'rw', + internal, + overlays: [], + }; } const overlays: CloudHypervisorWritableOverlay[] = []; for (const allowedPath of allowedPaths) { if (isPathAtOrBelow(entry.target, allowedPath)) { matched.add(allowedPath); - return { export: entry, disposition: 'writable', effectiveMode: 'rw', internal, overlays: [] }; + return { + export: entry, + disposition: 'writable', + hostRootMode: 'rw', + guestMountMode: 'rw', + internal, + overlays: [], + }; } if (!isDeepestWritableExport(exports, entry, allowedPath)) continue; @@ -131,10 +166,14 @@ export function planCloudHypervisorFilesystemWrites( } } + // A selective export keeps a read-write guest mount so that writable + // overlays are not blocked by an inherited MNT_READONLY; the read-only host + // backing tree is what denies writes everywhere else. return { export: entry, disposition: overlays.length > 0 ? 'selective' : 'read-only', - effectiveMode: 'ro', + hostRootMode: 'ro', + guestMountMode: overlays.length > 0 ? 'rw' : 'ro', internal, overlays, }; From 2a79de862bd06cddac0753a5fcd7fc13d82d5898 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:31:01 +0000 Subject: [PATCH 3/4] fix(cloud-hypervisor): reject ancestor paths in write policy --- src/cloud-hypervisor/filesystem-write-policy.test.ts | 11 +++++++++++ src/cloud-hypervisor/filesystem-write-policy.ts | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/cloud-hypervisor/filesystem-write-policy.test.ts b/src/cloud-hypervisor/filesystem-write-policy.test.ts index 50eac9ba0..c688c4a09 100644 --- a/src/cloud-hypervisor/filesystem-write-policy.test.ts +++ b/src/cloud-hypervisor/filesystem-write-policy.test.ts @@ -226,6 +226,17 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { }); }); + it('rejects an ancestor of an export target instead of widening the whole export', () => { + // `/` is above every export target, not an existing path reachable within + // one, so it must not be treated as a full-export match even though it is + // lexically "at or below" itself for every candidate target. + expect(() => planCloudHypervisorFilesystemWrites(exports, ['/'])) + .toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /', + ); + }); + it('rejects a path outside every export target', () => { expect(() => planCloudHypervisorFilesystemWrites(exports, ['/elsewhere'])) .toThrow( diff --git a/src/cloud-hypervisor/filesystem-write-policy.ts b/src/cloud-hypervisor/filesystem-write-policy.ts index 3e413fa17..2304d0da2 100644 --- a/src/cloud-hypervisor/filesystem-write-policy.ts +++ b/src/cloud-hypervisor/filesystem-write-policy.ts @@ -146,7 +146,11 @@ export function planCloudHypervisorFilesystemWrites( const overlays: CloudHypervisorWritableOverlay[] = []; for (const allowedPath of allowedPaths) { - if (isPathAtOrBelow(entry.target, allowedPath)) { + // Only an exact match against the export target keeps the whole export + // writable. A strict ancestor (e.g. `/` above `/workspace`) is not itself + // an existing path reachable within this export, so it must go through + // the same host-resolution as any other overlay candidate below. + if (allowedPath === entry.target) { matched.add(allowedPath); return { export: entry, From bd9cfb5d306d1c32f88cce25ef3f44dc4fe78e82 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sun, 23 Aug 2026 09:55:08 -0700 Subject: [PATCH 4/4] fix(cloud-hypervisor): consume allowWrite entries covered by internal exports The internal-export short-circuit returned before matching allowedPaths, so an entry such as /tmp/gh-aw or /tmp/gh-aw/cache was reported as unmatched even though the internal read-write export keeps it writable. Internal exports now walk the allowlist and consume the entries they cover. The exact export target matches directly; a nested path must still pass the same existence, realpath-equality, and file-or-directory validation as a normal overlay, so a missing path or a symlink escape is still rejected. No overlay is emitted because the whole export stays writable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../filesystem-write-policy.test.ts | 67 +++++++++++++++++++ .../filesystem-write-policy.ts | 18 ++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/cloud-hypervisor/filesystem-write-policy.test.ts b/src/cloud-hypervisor/filesystem-write-policy.test.ts index c688c4a09..3814f1131 100644 --- a/src/cloud-hypervisor/filesystem-write-policy.test.ts +++ b/src/cloud-hypervisor/filesystem-write-policy.test.ts @@ -76,6 +76,73 @@ describe('Cloud Hypervisor filesystem write policy planner', () => { expect(plan.exports[2].internal).toBe(true); }); + describe('internal exports', () => { + let internalSource: string; + let withInternal: CloudHypervisorDirectoryExport[]; + + beforeEach(async () => { + internalSource = path.join(directory, 'internal'); + await fs.mkdir(path.join(internalSource, 'cache'), { recursive: true }); + withInternal = [ + ...exports, + { tag: 'tmp-gh-aw', source: internalSource, target: '/tmp/gh-aw', mode: 'rw' }, + ]; + }); + + const plan = (allowWrite: string[]) => + planCloudHypervisorFilesystemWrites(withInternal, allowWrite, { + internalTags: ['tmp-gh-aw'], + }); + + it('consumes an allowlist entry naming the internal export target exactly', () => { + const result = plan(['/tmp/gh-aw']); + + expect(result.exports[2]).toEqual({ + export: withInternal[2], + disposition: 'writable', + hostRootMode: 'rw', + guestMountMode: 'rw', + internal: true, + overlays: [], + }); + expect(result.overlays).toEqual([]); + expect(result.exports[0].disposition).toBe('read-only'); + }); + + it('consumes an existing nested path inside the internal export without an overlay', () => { + const result = plan(['/tmp/gh-aw/cache']); + + expect(result.exports[2].disposition).toBe('writable'); + expect(result.exports[2].overlays).toEqual([]); + expect(result.overlays).toEqual([]); + }); + + it('rejects a nested path that does not exist inside the internal export', () => { + expect(() => plan(['/tmp/gh-aw/missing'])).toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /tmp/gh-aw/missing', + ); + }); + + it('rejects a strict ancestor of the internal export target', () => { + expect(() => plan(['/tmp'])).toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /tmp', + ); + }); + + it('rejects a symlink that escapes the internal export source', async () => { + const outside = path.join(directory, 'internal-outside'); + await fs.mkdir(outside); + await fs.symlink(outside, path.join(internalSource, 'escape')); + + expect(() => plan(['/tmp/gh-aw/escape'])).toThrow( + 'filesystem.allowWrite path is not an existing path within a writable ' + + 'Cloud Hypervisor export: /tmp/gh-aw/escape', + ); + }); + }); + it('keeps a whole export writable when its target is allowed', () => { const plan = planCloudHypervisorFilesystemWrites(exports, ['/workspace']); diff --git a/src/cloud-hypervisor/filesystem-write-policy.ts b/src/cloud-hypervisor/filesystem-write-policy.ts index 2304d0da2..c9612f2a6 100644 --- a/src/cloud-hypervisor/filesystem-write-policy.ts +++ b/src/cloud-hypervisor/filesystem-write-policy.ts @@ -79,7 +79,9 @@ export interface CloudHypervisorFilesystemWritePolicyOptions { /** * Tags of AWF-owned exports that must remain writable for the sandbox to * operate. They are never narrowed, mirroring the always-writable Docker - * mounts. + * mounts. An allowlist entry that resolves inside such an export is still + * validated and consumed, so it is not reported as unmatched, but it produces + * no overlay because the whole export is already writable. */ readonly internalTags?: Iterable; } @@ -133,7 +135,21 @@ export function planCloudHypervisorFilesystemWrites( overlays: [], }; } + // An internal export stays fully writable, but it still consumes allowlist + // entries it covers so they are not misreported as unmatched. As above, only + // an exact target match is taken directly; a nested path goes through the + // same existence/realpath validation as a normal overlay. The resolved + // overlay is discarded because the whole export is already writable. if (internal) { + for (const allowedPath of allowedPaths) { + if ( + allowedPath === entry.target || + (isDeepestWritableExport(exports, entry, allowedPath) && + resolveWritableOverlay(entry, allowedPath) !== undefined) + ) { + matched.add(allowedPath); + } + } return { export: entry, disposition: 'writable',