From 85c2d7b09da2e0dd05cac69f400b5f312e7d7ba5 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 30 Jul 2026 17:17:33 -0700 Subject: [PATCH 1/4] fix: isolate bounded query private state Move broker-only bounded-query state outside agent-visible mounts and reject realpath or symlink overlaps before staging. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651 --- CLAUDE.md | 2 +- containers/bounded-query/Dockerfile | 2 +- containers/bounded-query/broker/config.js | 6 +- containers/bounded-query/broker/server.js | 3 +- docs/awf-config-spec.md | 20 ++- docs/bounded-queries.md | 2 +- src/artifact-preservation.ts | 3 +- src/bounded-query/manager.test.ts | 46 +++++- src/bounded-query/manager.ts | 71 ++++++--- src/bounded-query/mount-policy.test.ts | 112 +++++++++++++++ src/bounded-query/mount-policy.ts | 158 +++++++++++++++++++++ src/bounded-query/paths.test.ts | 20 ++- src/bounded-query/paths.ts | 67 +++++---- src/bounded-query/types.ts | 2 +- src/docker-manager-diagnostics.test.ts | 4 +- src/services/bounded-query-compose.test.ts | 8 +- src/services/bounded-query-service.test.ts | 19 +-- src/services/bounded-query-service.ts | 33 +---- src/services/optional-services.ts | 6 +- 19 files changed, 471 insertions(+), 113 deletions(-) create mode 100644 src/bounded-query/mount-policy.test.ts create mode 100644 src/bounded-query/mount-policy.ts diff --git a/CLAUDE.md b/CLAUDE.md index 729d047b6..25e1a1bdc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,7 @@ The system is orchestrated by `src/cli.ts` and managed by `src/docker-manager.ts **4. Bounded-Query Broker (optional)** — `containers/bounded-query/`, no network - Enabled via `boundedQueries.enabled` in the AWF config file (config-only; there is no CLI flag family) - The only AWF service with `network_mode: none`: no `awf-net`, no external bridge, no DNS, no Squid, no host gateway -- Reachable only through one Unix socket in `/bounded-queries/run/`, bind-mounted into the agent at `/run/awf-bounded-query/broker.sock` +- Reachable only through one Unix socket in a run-specific `/var/tmp` ingress root, bind-mounted into the agent at `/run/awf-bounded-query/broker.sock`; all seeds, workspaces, maps, control state, and audits live in a disjoint broker-private `/var/tmp` root - Receives the resolved Docker socket so it can launch per-invocation query containers; that path never enters the agent's env or volumes - The broker (`bounded-query-broker`) and query sandbox (`bounded-query`) are separate published images; a one-shot networkless Compose service pulls the sandbox image before broker startup so the broker (which has no network) can launch query containers - Queries run `python3` with `--network none`, `--read-only`, non-root, `--cap-drop ALL`, `no-new-privileges`, a seccomp profile, and time/memory/CPU/PID/file-size bounds diff --git a/containers/bounded-query/Dockerfile b/containers/bounded-query/Dockerfile index 28ab60c9b..0f9dd3b23 100644 --- a/containers/bounded-query/Dockerfile +++ b/containers/bounded-query/Dockerfile @@ -56,7 +56,7 @@ RUN chmod -R a-w /opt/awf \ && node --check /opt/awf/broker/healthcheck.js # Fixed broker-only mount points. -RUN mkdir -p /srv/awf/seeds /srv/awf/work /run/awf-bounded-query /var/log/awf-bounded-query +RUN mkdir -p /srv/awf/seeds /srv/awf/work /run/awf-bounded-query /run/awf-bounded-query-control /var/log/awf-bounded-query # The broker is root only to copy host-owned read-only seeds into private # workspaces and hand those workspaces to the unprivileged query uid. diff --git a/containers/bounded-query/broker/config.js b/containers/bounded-query/broker/config.js index 2047ea05b..9a1cd7283 100644 --- a/containers/bounded-query/broker/config.js +++ b/containers/bounded-query/broker/config.js @@ -19,9 +19,10 @@ const WORK_DIR = '/srv/awf/work'; const SEED_MAP_PATH = '/srv/awf/seed-map.json'; const SOCKET_DIR = '/run/awf-bounded-query'; const SOCKET_PATH = path.join(SOCKET_DIR, 'broker.sock'); +const CONTROL_DIR = '/run/awf-bounded-query-control'; const AUDIT_DIR = '/var/log/awf-bounded-query'; -/** Broker-private readiness marker; the audit directory is never agent-mounted. */ -const READY_PATH = path.join(AUDIT_DIR, 'broker.ready'); +/** Broker-private readiness marker; the control directory is never agent-mounted. */ +const READY_PATH = path.join(CONTROL_DIR, 'broker.ready'); const QUERY_SECCOMP_PATH = '/opt/awf/query-seccomp.json'; /** Mount points inside the query container. Fixed, never caller-supplied. */ @@ -84,6 +85,7 @@ function loadConfig() { seedMapPath: SEED_MAP_PATH, socketDir: SOCKET_DIR, socketPath: SOCKET_PATH, + controlDir: CONTROL_DIR, readyPath: READY_PATH, auditDir: AUDIT_DIR, querySeccompPath: QUERY_SECCOMP_PATH, diff --git a/containers/bounded-query/broker/server.js b/containers/bounded-query/broker/server.js index 53db9a13e..be266797a 100644 --- a/containers/bounded-query/broker/server.js +++ b/containers/bounded-query/broker/server.js @@ -113,7 +113,8 @@ async function main() { await listenOnSocket(server, config, audit); // Write the ready file AFTER the socket is accepting connections. The - // compose healthcheck polls this file in the broker-only audit mount. + // compose healthcheck polls this file in the broker-only control mount. + fs.mkdirSync(config.controlDir, { recursive: true, mode: 0o700 }); fs.writeFileSync(config.readyPath, '', { mode: 0o644 }); audit.lifecycle('listening', { diff --git a/docs/awf-config-spec.md b/docs/awf-config-spec.md index 52b4fbe03..319d0aacc 100644 --- a/docs/awf-config-spec.md +++ b/docs/awf-config-spec.md @@ -1837,7 +1837,9 @@ runs a trusted host-side staging phase (`src/bounded-query/staging.ts`): 1. resolves the staging credential from `GH_TOKEN` or `GITHUB_TOKEN`; 2. clones each configured repository from an AWF-constructed `https://github.com//.git` URL into a run-unique, opaque seed - directory under `/bounded-queries/seeds/`. The credential is passed + directory under a dedicated per-run private root outside `/tmp`, the + workspace, mounted home/tool directories, and configured agent mounts. The + credential is passed only through a `GIT_ASKPASS` helper reading it from the child process environment — never in argv, never in the URL, never in a log line, and never in the generated compose file; @@ -1906,7 +1908,7 @@ Query stdout/stderr is capped and discarded — never parsed, never returned, never logged in a form reachable by the agent. Failure reasons (with protected detail, e.g. `repo-not-allowed`, `bit-budget-exhausted`, `invalid-request`, `query-launch-failed`, `timing-bucket-overflow`, -`cleanup-failed`) are written only to `/bounded-queries/audit/`, +`cleanup-failed`) are written only below the dedicated broker-private root, which is mounted into the broker alone. ### 14.8 Agent Interface @@ -1934,7 +1936,7 @@ responsibilities are enforcing the fixed CLI shape, base64url-encoding the schema into a request header, transporting the script body unmodified, and passing the broker's response through unmodified. -The generated `SKILL.md` is written under `/bounded-queries/agent/` and +The generated `SKILL.md` is written under the run-specific ingress root and mounted read-only at `/run/awf-bounded-query-skill/SKILL.md`. It documents, per configured repository, its sensitivity and run budget (e.g. `` `octo/alpha` — 64 bits/run (`internal`) ``), the finite schema DSL, the bit-charge @@ -1946,6 +1948,18 @@ or inside the checked-out workspace. Agents therefore discover it through `AWF_BOUNDED_QUERY_SKILL` rather than through automatic skill discovery. This is a documented limitation, not an oversight. +All seeds, invocation workspaces, the seed map, broker control state, and +protected audit data live below +`/var/tmp/awf-bounded-query-private--/`. Only the disjoint +`/var/tmp/awf-bounded-query-ingress--/run/` and generated +skill directory are agent-visible through explicit bind mounts. Before +credential-bearing staging, AWF resolves each path through +its longest existing ancestor (following symlinks) and rejects any private-root +overlap with the union of Docker, gVisor, and sbx agent-visible mounts, +including `/tmp`, the workspace, custom volumes, and whitelisted home tool +directories. Docker-in-Docker host-path translation is checked and applied to +the private broker mounts and ingress mounts symmetrically. + For the same reason, a microVM primary agent runtime (`sbx`) is rejected at preflight: it does not receive Compose bind mounts, so the socket and skill could not be exposed. Bounded queries are never partially enabled. diff --git a/docs/bounded-queries.md b/docs/bounded-queries.md index 80d92dd0b..148144b62 100644 --- a/docs/bounded-queries.md +++ b/docs/bounded-queries.md @@ -263,7 +263,7 @@ Failures are indistinguishable from each other by design: the agent cannot infer `maxInvocations` counts **every** response, including rejected requests. It is a separate operational limit unrelated to per-repository bit budgets. Once exhausted, all further requests return `{"status":"error"}` without consulting the bit ledger. -Failure details (with protected labels such as `repo-not-allowed`, `bit-budget-exhausted`, `invalid-request`, `launch-failed`, `timing-bucket-overflow`, and `cleanup-failed`) are written only to the protected audit log at `/bounded-queries/audit/`. They are never returned to the agent. +Failure details (with protected labels such as `repo-not-allowed`, `bit-budget-exhausted`, `invalid-request`, `launch-failed`, `timing-bucket-overflow`, and `cleanup-failed`) are written only below the dedicated broker-private root (`/var/tmp/awf-bounded-query-private--/audit/`). The root is rejected before staging if realpath-aware preflight finds any overlap with a Compose, gVisor, or sbx agent mount. They are never returned to the agent. ## Security limitations diff --git a/src/artifact-preservation.ts b/src/artifact-preservation.ts index 720ceb2e7..cd5e8dcec 100644 --- a/src/artifact-preservation.ts +++ b/src/artifact-preservation.ts @@ -5,6 +5,7 @@ import execa from 'execa'; import { logger } from './logger'; import { fixArtifactPermissionsForRootless } from './artifact-permissions'; import { getLocalDockerEnv } from './host-env'; +import { resolveBoundedQueryPaths } from './bounded-query/paths'; const BOUNDED_QUERY_AUDIT_CONTAINER_PATH = 'awf-bounded-query-broker:/var/log/awf-bounded-query/bounded-query.jsonl'; @@ -16,7 +17,7 @@ const BOUNDED_QUERY_AUDIT_CONTAINER_PATH = */ export function preserveIptablesAudit(workDir: string, auditDir?: string): void { const iptablesAuditSrc = path.join(workDir, 'init-signal', 'iptables-audit.txt'); - const boundedQueryRoot = path.join(workDir, 'bounded-queries'); + const boundedQueryRoot = resolveBoundedQueryPaths(workDir).root; const targetAuditDir = auditDir || path.join(workDir, 'audit'); if (!fs.existsSync(targetAuditDir)) return; diff --git a/src/bounded-query/manager.test.ts b/src/bounded-query/manager.test.ts index 0003c5a94..ada49a77b 100644 --- a/src/bounded-query/manager.test.ts +++ b/src/bounded-query/manager.test.ts @@ -71,7 +71,9 @@ describe('prepareBoundedQueries', () => { }); afterEach(() => { - releaseSeedPermissions(resolveBoundedQueryPaths(workDir).seedsDir); + const paths = resolveBoundedQueryPaths(workDir); + releaseSeedPermissions(paths.seedsDir); + fs.rmSync(paths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); }); @@ -88,7 +90,9 @@ describe('prepareBoundedQueries', () => { expect(fs.existsSync(paths.workDir)).toBe(true); expect(fs.existsSync(paths.runDir)).toBe(true); expect(fs.existsSync(paths.auditDir)).toBe(true); + expect(fs.existsSync(paths.controlDir)).toBe(true); expect(fs.existsSync(paths.skillPath)).toBe(true); + expect(paths.root.startsWith(workDir)).toBe(false); const seedMap = JSON.parse(fs.readFileSync(paths.seedMapPath, 'utf8')); expect(seedMap.version).toBe(2); @@ -155,6 +159,26 @@ describe('prepareBoundedQueries', () => { } }); + it('rejects a pre-existing private root instead of reusing attacker-controlled state', async () => { + const paths = resolveBoundedQueryPaths(workDir); + fs.mkdirSync(paths.root); + await expect(prepareBoundedQueries(buildConfig(workDir), { env: { GH_TOKEN: 't' }, gitRunner })) + .rejects.toThrow(/EEXIST|file already exists/); + }); + + it('rejects a pre-existing ingress root instead of following a planted symlink', async () => { + const paths = resolveBoundedQueryPaths(workDir); + const target = fs.mkdtempSync(path.join('/var/tmp', 'awf-bounded-query-ingress-target-')); + fs.symlinkSync(target, paths.ingressRoot); + try { + await expect(prepareBoundedQueries(buildConfig(workDir), { env: { GH_TOKEN: 't' }, gitRunner })) + .rejects.toThrow(/EEXIST|file already exists/); + } finally { + fs.rmSync(paths.ingressRoot, { force: true }); + fs.rmSync(target, { recursive: true, force: true }); + } + }); + it('aborts when a seed cannot be staged', async () => { const failing: GitRunner = async () => { throw new Error('fatal: repository not found'); @@ -178,9 +202,9 @@ describe('teardownBoundedQueries', () => { it('restores seed write permissions so generic cleanup can remove them', async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-bounded-query-teardown-')); + const paths = resolveBoundedQueryPaths(workDir); try { await prepareBoundedQueries(buildConfig(workDir), { env: { GH_TOKEN: 't' }, gitRunner }); - const paths = resolveBoundedQueryPaths(workDir); expect(() => fs.rmSync(paths.seedsDir, { recursive: true })).toThrow(); @@ -188,9 +212,11 @@ describe('teardownBoundedQueries', () => { // no-op; the permission restore is what must happen. await teardownBoundedQueries(buildConfig(workDir)); - expect(() => fs.rmSync(paths.seedsDir, { recursive: true })).not.toThrow(); + expect(fs.existsSync(paths.root)).toBe(false); + expect(fs.existsSync(paths.ingressRoot)).toBe(false); } finally { - releaseSeedPermissions(resolveBoundedQueryPaths(workDir).seedsDir); + releaseSeedPermissions(paths.seedsDir); + fs.rmSync(paths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); } }); @@ -205,7 +231,9 @@ describe('teardownBoundedQueries', () => { expect(() => fs.rmSync(paths.seedsDir, { recursive: true })).toThrow(); } finally { - releaseSeedPermissions(resolveBoundedQueryPaths(workDir).seedsDir); + const cleanupPaths = resolveBoundedQueryPaths(workDir); + releaseSeedPermissions(cleanupPaths.seedsDir); + fs.rmSync(cleanupPaths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); } }); @@ -253,6 +281,7 @@ describe('teardownBoundedQueries', () => { await teardownBoundedQueries(buildConfig(workDir)); expect(mockExeca).not.toHaveBeenCalled(); } finally { + fs.rmSync(paths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); } }); @@ -264,9 +293,11 @@ describe('teardownBoundedQueries', () => { mockExeca.mockRejectedValueOnce(new Error('docker unavailable')); await expect(teardownBoundedQueries(buildConfig(workDir))).resolves.toBeUndefined(); - expect(() => fs.rmSync(resolveBoundedQueryPaths(workDir).seedsDir, { recursive: true })).not.toThrow(); + expect(fs.existsSync(resolveBoundedQueryPaths(workDir).root)).toBe(false); } finally { - releaseSeedPermissions(resolveBoundedQueryPaths(workDir).seedsDir); + const cleanupPaths = resolveBoundedQueryPaths(workDir); + releaseSeedPermissions(cleanupPaths.seedsDir); + fs.rmSync(cleanupPaths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); } }); @@ -285,6 +316,7 @@ describe('teardownBoundedQueries', () => { await expect(teardownBoundedQueries(buildConfig(workDir))).resolves.toBeUndefined(); expect(mockReleaseSeedPermissions).toHaveBeenCalledWith(paths.seedsDir); } finally { + fs.rmSync(paths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); } }); diff --git a/src/bounded-query/manager.ts b/src/bounded-query/manager.ts index 3aa734ecf..e73e3cdd3 100644 --- a/src/bounded-query/manager.ts +++ b/src/bounded-query/manager.ts @@ -13,6 +13,8 @@ import { assertQueryRuntimeAvailable, validateBoundedQueryConfig } from './prefl import { writeBoundedQuerySkill } from './skill'; import { releaseSeedPermissions, resolveStagingToken, stageBoundedQuerySeeds, type GitRunner } from './staging'; import { BOUNDED_QUERY_SEED_MAP_VERSION, type BoundedQuerySeedMap } from './types'; +import { assertBoundedQueryPrivateRootIsolated } from './mount-policy'; +import { fixArtifactPermissionsForRootless } from '../artifact-permissions'; /** * Bounded-query lifecycle orchestration. @@ -25,9 +27,8 @@ import { BOUNDED_QUERY_SEED_MAP_VERSION, type BoundedQuerySeedMap } from './type * agent, and any query exist; * - compose generation can rely on the on-disk layout already being present. * - * `teardownBoundedQueries` removes orphaned query containers and restores write - * permissions on the immutable seeds so AWF's generic work-directory cleanup - * can delete them. + * `teardownBoundedQueries` removes orphaned query containers and the separate + * broker-private host root. */ /** Docker label applied to every query container, used for orphan cleanup. */ @@ -47,26 +48,24 @@ function ensureModeDirectory(target: string, mode: number): void { /** * Creates the bounded-query directory layout. * - * The socket and skill directories are handed to the host user because the - * agent process runs under the host UID/GID; everything else stays - * root-owned (0700) inside the already-hardened work directory. - * - * The mask directory is created as an empty, read-only-to-others directory. - * It is bind-mounted into the agent at the bounded-query root path, replacing - * the agent's view of the entire bounded-query subtree (including seeds, work, - * and audit) through the broad `/tmp` bind mount. Only the socket and skill - * (mounted at separate container paths) remain agent-visible. + * The private root is created without `recursive` so a pre-existing path, + * including a symlink planted between preflight and creation, fails closed. */ function prepareDirectories(paths: BoundedQueryPaths): void { - ensureModeDirectory(paths.root, 0o700); + fs.mkdirSync(paths.root, { mode: 0o700 }); + if (fs.lstatSync(paths.root).isSymbolicLink()) { + throw new Error(`Refusing to use symlink as bounded-query private root: ${paths.root}`); + } + fs.mkdirSync(paths.ingressRoot, { mode: 0o700 }); + if (fs.lstatSync(paths.ingressRoot).isSymbolicLink()) { + throw new Error(`Refusing to use symlink as bounded-query ingress root: ${paths.ingressRoot}`); + } ensureModeDirectory(paths.seedsDir, 0o700); ensureModeDirectory(paths.workDir, 0o700); + ensureModeDirectory(paths.controlDir, 0o700); ensureModeDirectory(paths.auditDir, 0o700); ensureModeDirectory(paths.runDir, 0o770); ensureModeDirectory(paths.agentDir, 0o755); - // Empty directory used as a masking mount in the agent container. - // Mode 0o755 so Docker can bind-mount it without special privileges. - ensureModeDirectory(paths.maskDir, 0o755); try { fs.chownSync(paths.runDir, parseInt(getSafeHostUid(), 10), parseInt(getSafeHostGid(), 10)); @@ -120,6 +119,9 @@ export async function prepareBoundedQueries( throw new Error(`Bounded-query configuration is invalid:\n - ${errors.join('\n - ')}`); } + const paths = resolveBoundedQueryPaths(config.workDir); + assertBoundedQueryPrivateRootIsolated(config, paths, env); + await assertQueryRuntimeAvailable(boundedQueries); const token = resolveStagingToken(env); @@ -129,8 +131,6 @@ export async function prepareBoundedQueries( throw new Error('Bounded-query staging credential disappeared between validation and staging'); } - const paths = resolveBoundedQueryPaths(config.workDir); - // Guard against symlink injection before writing any credential-bearing state. // The generic work-directory check in config-writer.ts runs later (during // writeConfigs), so we apply the same symlink rejection here explicitly. @@ -226,7 +226,12 @@ export async function teardownBoundedQueries(config: WrapperConfig): Promise { + let testRoot: string; + let workDir: string; + let privateBase: string; + + beforeEach(() => { + testRoot = fs.mkdtempSync(path.join('/var/tmp', 'awf-bounded-query-policy-')); + workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-bounded-query-visible-')); + privateBase = path.join(testRoot, 'private'); + fs.mkdirSync(privateBase); + }); + + afterEach(() => { + fs.rmSync(testRoot, { recursive: true, force: true }); + fs.rmSync(workDir, { recursive: true, force: true }); + }); + + it('accepts a dedicated private root outside all agent-visible mounts', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => assertBoundedQueryPrivateRootIsolated(config(workDir), paths)).not.toThrow(); + }); + + it('rejects private state beneath the broad /tmp mount', () => { + const paths = resolveBoundedQueryPaths(workDir, '/tmp'); + expect(() => assertBoundedQueryPrivateRootIsolated(config(workDir), paths)) + .toThrow(/overlaps agent-visible temporary directory/); + }); + + it('rejects a broad custom mount containing the private root', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => + assertBoundedQueryPrivateRootIsolated(config(workDir, [`${testRoot}:/data:ro`]), paths), + ).toThrow(/custom volume/); + }); + + it('rejects a nested custom mount inside the private root', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + const nested = path.join(paths.root, 'seeds'); + expect(() => + assertBoundedQueryPrivateRootIsolated(config(workDir, [`${nested}:/data:ro`]), paths), + ).toThrow(/custom volume/); + }); + + it('normalizes path traversal before checking overlap', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + const traversing = path.join(paths.root, 'seeds', '..'); + expect(() => + assertBoundedQueryPrivateRootIsolated(config(workDir, [`${traversing}:/data:ro`]), paths), + ).toThrow(/custom volume/); + }); + + it('resolves symlink aliases in existing ancestors', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + const alias = path.join(testRoot, 'private-alias'); + fs.symlinkSync(privateBase, alias); + expect(() => + assertBoundedQueryPrivateRootIsolated(config(workDir, [`${alias}:/data:ro`]), paths), + ).toThrow(/custom volume/); + }); + + it('checks daemon-prefixed paths used by DinD bind mounts', () => { + const daemonRoot = path.join(testRoot, 'daemon'); + fs.mkdirSync(daemonRoot); + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => + assertBoundedQueryPrivateRootIsolated( + { ...config(workDir, [`${testRoot}:/data:ro`]), dockerHostPathPrefix: daemonRoot }, + paths, + ), + ).toThrow(/custom volume/); + }); + + it('rejects a workspace that contains the private root', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => + assertBoundedQueryPrivateRootIsolated(config(workDir), paths, {}, testRoot), + ).toThrow(/agent-visible workspace/); + }); + + it('rejects a configured session-state mount containing the private root', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => + assertBoundedQueryPrivateRootIsolated( + { ...config(workDir), sessionStateDir: testRoot }, + paths, + ), + ).toThrow(/agent session-state directory/); + }); + + it('resolves a missing suffix through a symlinked ancestor', () => { + const target = path.join(testRoot, 'target'); + const alias = path.join(testRoot, 'alias'); + fs.mkdirSync(target); + fs.symlinkSync(target, alias); + expect(resolvePathThroughExistingAncestor(path.join(alias, 'missing', 'leaf'))) + .toBe(path.join(target, 'missing', 'leaf')); + }); +}); diff --git a/src/bounded-query/mount-policy.ts b/src/bounded-query/mount-policy.ts new file mode 100644 index 000000000..7515ed15d --- /dev/null +++ b/src/bounded-query/mount-policy.ts @@ -0,0 +1,158 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import { etcAllowlist, HOME_TOOL_SUBDIRS, systemDirectories } from '../config/mount-policy'; +import { getRealUserHome } from '../host-identity'; +import type { WrapperConfig } from '../types'; +import { applyHostPathPrefixToVolumes } from '../services/host-path-prefix'; +import { resolveDockerSocketPath } from '../services/agent-volumes/docker-socket'; +import type { BoundedQueryPaths } from './paths'; + +interface VisiblePath { + label: string; + source: string; +} + +/** + * Resolves symlinks in the longest existing ancestor, then appends any missing + * suffix. This matches how a later mkdir/bind operation will resolve a path + * without requiring the final path to exist during preflight. + */ +export function resolvePathThroughExistingAncestor(candidate: string): string { + if (!path.isAbsolute(candidate)) { + throw new Error(`Bounded-query mount policy requires an absolute path: ${candidate}`); + } + + const missing: string[] = []; + let existing = path.resolve(candidate); + while (!fs.existsSync(existing)) { + const parent = path.dirname(existing); + if (parent === existing) { + throw new Error(`Bounded-query mount policy could not resolve an existing ancestor: ${candidate}`); + } + missing.unshift(path.basename(existing)); + existing = parent; + } + + const resolvedAncestor = fs.realpathSync.native(existing); + return path.resolve(resolvedAncestor, ...missing); +} + +function pathsOverlap(left: string, right: string): boolean { + const relativeLeft = path.relative(left, right); + const relativeRight = path.relative(right, left); + const leftContainsRight = relativeLeft === '' || (!relativeLeft.startsWith('..') && !path.isAbsolute(relativeLeft)); + const rightContainsLeft = relativeRight === '' || (!relativeRight.startsWith('..') && !path.isAbsolute(relativeRight)); + return leftContainsRight || rightContainsLeft; +} + +function mountSource(volume: string): string | undefined { + const source = volume.split(':', 1)[0]; + return source && path.isAbsolute(source) ? source : undefined; +} + +function daemonVisiblePath(source: string, prefix: string | undefined): string { + const translated = applyHostPathPrefixToVolumes([`${source}:/awf-mount-policy:ro`], prefix)[0]; + return mountSource(translated) ?? source; +} + +/** + * Returns the union of host paths exposed by Compose/runc, Compose/gVisor, and + * sbx. The union intentionally includes optional paths even when absent: Docker + * can create missing bind sources, and a later-created home/tool path must not + * turn a previously safe private root into an exposed one. + */ +function collectAgentVisiblePaths( + config: WrapperConfig, + env: NodeJS.ProcessEnv, + cwd: string, +): VisiblePath[] { + const home = getRealUserHome(); + const workspace = env.GITHUB_WORKSPACE || cwd; + const visible: VisiblePath[] = [ + { label: 'temporary directory', source: '/tmp' }, + { label: 'workspace', source: workspace }, + { label: 'sbx system tools', source: '/usr/local/bin' }, + { label: 'Compose chroot home', source: `${config.workDir}-chroot-home` }, + { label: 'AWF work directory', source: config.workDir }, + ...[...systemDirectories(false), ...systemDirectories(true)].map((source) => ({ + label: 'Compose system mount', + source, + })), + ...etcAllowlist().map((source) => ({ label: 'Compose /etc mount', source })), + { label: 'Compose identity mount', source: '/etc/passwd' }, + { label: 'Compose identity mount', source: '/etc/group' }, + ...HOME_TOOL_SUBDIRS.map((subdir) => ({ + label: `home tool directory ${subdir}`, + source: path.join(home, subdir), + })), + { label: 'runner tool cache fallback', source: path.join(home, 'work', '_tool') }, + ]; + + for (const runnerToolCache of [config.runnerToolCachePath, env.RUNNER_TOOL_CACHE]) { + if (runnerToolCache) { + visible.push({ label: 'runner tool cache', source: runnerToolCache }); + } + } + if (config.sessionStateDir) { + visible.push({ label: 'agent session-state directory', source: config.sessionStateDir }); + } + if (config.chrootBinariesSourcePath) { + visible.push({ label: 'chroot binaries source', source: config.chrootBinariesSourcePath }); + } + if (config.enableDind) { + visible.push({ label: 'agent Docker socket', source: resolveDockerSocketPath(config) }); + } + for (const volume of config.volumeMounts ?? []) { + const source = mountSource(volume); + if (!source) { + throw new Error(`Bounded-query mount policy could not parse custom bind mount: ${volume}`); + } + visible.push({ label: `custom volume ${volume}`, source }); + } + + return visible; +} + +/** + * Fails closed when the broker-private root aliases, contains, or is contained + * by any path visible to a primary agent in any supported sandbox backend. + */ +export function assertBoundedQueryPrivateRootIsolated( + config: WrapperConfig, + paths: BoundedQueryPaths, + env: NodeJS.ProcessEnv = process.env, + cwd = process.cwd(), +): void { + const privateRoot = resolvePathThroughExistingAncestor(paths.root); + const privateDaemonRoot = resolvePathThroughExistingAncestor( + daemonVisiblePath(paths.root, config.dockerHostPathPrefix), + ); + const visiblePaths = [ + ...collectAgentVisiblePaths(config, env, cwd), + { label: 'bounded-query ingress', source: paths.ingressRoot }, + ]; + + for (const visible of visiblePaths) { + const resolvedVisible = resolvePathThroughExistingAncestor(visible.source); + const resolvedDaemonVisible = resolvePathThroughExistingAncestor( + daemonVisiblePath(visible.source, config.dockerHostPathPrefix), + ); + if ( + pathsOverlap(privateRoot, resolvedVisible) + || pathsOverlap(privateDaemonRoot, resolvedDaemonVisible) + ) { + throw new Error( + `Unsafe bounded-query private root "${paths.root}" overlaps agent-visible ${visible.label} ` + + `"${visible.source}" after path and symlink resolution`, + ); + } + } +} + +/** @internal Exported for focused adversarial tests. */ +// ts-prune-ignore-next +export const mountPolicyTestHelpers = { + collectAgentVisiblePaths, + daemonVisiblePath, + pathsOverlap, +}; diff --git a/src/bounded-query/paths.test.ts b/src/bounded-query/paths.test.ts index a12903a03..562357833 100644 --- a/src/bounded-query/paths.test.ts +++ b/src/bounded-query/paths.test.ts @@ -12,14 +12,22 @@ import { describe('bounded-query paths', () => { const workDir = '/tmp/awf-12345'; + const privateBaseDir = '/var/tmp/awf-test-private'; - it('derives every artifact path under a single bounded-queries subtree', () => { - const paths = resolveBoundedQueryPaths(workDir); + it('separates broker-private state from agent-visible ingress', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBaseDir); - expect(paths.root).toBe(path.join(workDir, 'bounded-queries')); - for (const value of Object.values(paths)) { - expect(value.startsWith(paths.root)).toBe(true); - } + expect(paths.root.startsWith(`${privateBaseDir}/awf-bounded-query-private-`)).toBe(true); + expect(paths.root.startsWith('/tmp')).toBe(false); + expect(paths.ingressRoot.startsWith(`${privateBaseDir}/awf-bounded-query-ingress-`)).toBe(true); + expect(paths.ingressRoot.startsWith(workDir)).toBe(false); + expect(paths.seedsDir.startsWith(paths.root)).toBe(true); + expect(paths.workDir.startsWith(paths.root)).toBe(true); + expect(paths.controlDir.startsWith(paths.root)).toBe(true); + expect(paths.auditDir.startsWith(paths.root)).toBe(true); + expect(paths.seedMapPath.startsWith(paths.root)).toBe(true); + expect(paths.runDir.startsWith(paths.ingressRoot)).toBe(true); + expect(paths.agentDir.startsWith(paths.ingressRoot)).toBe(true); }); it('places the socket and skill inside their advertised directories', () => { diff --git a/src/bounded-query/paths.ts b/src/bounded-query/paths.ts index f30001171..c37834541 100644 --- a/src/bounded-query/paths.ts +++ b/src/bounded-query/paths.ts @@ -4,29 +4,36 @@ import * as path from 'path'; /** * Filesystem layout and fixed container paths for the bounded-query subsystem. * - * Everything bounded queries need lives under a single run-unique subtree of - * `config.workDir` so that the existing work-directory hardening (0700, - * symlink rejection, end-of-run removal) applies to it unchanged. + * Broker-private state and the only agent-visible artifacts live in disjoint, + * run-specific host roots outside `/tmp`. Only the ingress roots are mounted + * into the primary agent. * * Layout (host side): * * ```text - * /bounded-queries/ + * /var/tmp/awf-bounded-query-private--/ * seeds// immutable, read-only repository seed (one per repo) * work/ broker-owned per-invocation writable copies - * run/ broker Unix socket, shared read-write with the agent - * agent/ generated SKILL.md, shared read-only with the agent + * control/ broker readiness and other private control state * audit/ protected broker diagnostics (never agent-visible) * seed-map.json normalized repo -> opaque seed id map (broker input) + * + * /var/tmp/awf-bounded-query-ingress--/ + * run/ broker Unix socket, shared read-write with the agent + * skill/ generated SKILL.md, shared read-only with the agent * ``` */ export interface BoundedQueryPaths { - /** `/bounded-queries` — parent of every bounded-query artifact. */ + /** Dedicated broker-private host root. Never mounted into the primary agent. */ root: string; /** Immutable per-repository seeds. Mounted read-only into the broker. */ seedsDir: string; /** Broker-owned scratch space for per-invocation writable repo copies. */ workDir: string; + /** Broker-private readiness and control state. */ + controlDir: string; + /** Parent of the only bounded-query artifacts visible to the primary agent. */ + ingressRoot: string; /** Directory holding the broker's Unix socket, shared with the agent. */ runDir: string; /** Directory holding agent-visible artifacts (the generated SKILL.md). */ @@ -39,19 +46,11 @@ export interface BoundedQueryPaths { socketPath: string; /** Host path of the generated skill document. */ skillPath: string; - /** - * Empty directory used to mask the entire bounded-query root from the agent's - * broad `/tmp` bind mount. - * - * The agent receives `run/` (socket) and `agent/` (skill) as separate, - * more-specific bind mounts at different container paths. The parent - * `/bounded-queries/` is masked with this empty directory so the - * agent cannot enumerate seeds, work, audit, or the seed-map through `/tmp`. - * Located OUTSIDE the bounded-query root to avoid self-referential masking. - */ - maskDir: string; } +/** Broker-private state is deliberately outside the agent's broad `/tmp` mount. */ +export const BOUNDED_QUERY_PRIVATE_BASE_DIR = '/var/tmp'; + /** Name of the broker's Unix domain socket inside {@link BoundedQueryPaths.runDir}. */ export const BOUNDED_QUERY_SOCKET_FILENAME = 'broker.sock'; @@ -91,6 +90,9 @@ export const BROKER_SOCKET_DIR = '/run/awf-bounded-query'; /** Protected diagnostics directory inside the broker container. */ export const BROKER_AUDIT_DIR = '/var/log/awf-bounded-query'; +/** Broker-private control directory inside the broker container. */ +export const BROKER_CONTROL_DIR = '/run/awf-bounded-query-control'; + /** Docker socket mount point inside the broker container. */ export const BROKER_DOCKER_SOCKET_PATH = '/var/run/docker.sock'; @@ -100,24 +102,39 @@ export const QUERY_MOUNT_DIR = '/query'; /** Fixed read-only path the submitted query script is mounted at. */ export const QUERY_SCRIPT_PATH = '/awf/query-script.py'; +/** Derives the private root identity without revealing the work-directory path. */ +function deriveRootIdentity(awfWorkDir: string): string { + const uid = process.getuid?.() ?? 0; + const digest = crypto + .createHash('sha256') + .update(path.resolve(awfWorkDir), 'utf8') + .digest('hex') + .slice(0, 20); + return `${uid}-${digest}`; +} + /** Derives every bounded-query path from the AWF work directory. */ -export function resolveBoundedQueryPaths(awfWorkDir: string): BoundedQueryPaths { - const root = path.join(awfWorkDir, 'bounded-queries'); - const runDir = path.join(root, 'run'); - const agentDir = path.join(root, 'agent'); +export function resolveBoundedQueryPaths( + awfWorkDir: string, + privateBaseDir = BOUNDED_QUERY_PRIVATE_BASE_DIR, +): BoundedQueryPaths { + const rootIdentity = deriveRootIdentity(awfWorkDir); + const root = path.join(privateBaseDir, `awf-bounded-query-private-${rootIdentity}`); + const ingressRoot = path.join(privateBaseDir, `awf-bounded-query-ingress-${rootIdentity}`); + const runDir = path.join(ingressRoot, 'run'); + const agentDir = path.join(ingressRoot, 'skill'); return { root, seedsDir: path.join(root, 'seeds'), workDir: path.join(root, 'work'), + controlDir: path.join(root, 'control'), + ingressRoot, runDir, agentDir, auditDir: path.join(root, 'audit'), seedMapPath: path.join(root, 'seed-map.json'), socketPath: path.join(runDir, BOUNDED_QUERY_SOCKET_FILENAME), skillPath: path.join(agentDir, BOUNDED_QUERY_SKILL_FILENAME), - // Sibling of the bounded-query root — never inside it — so the mask mount - // does not accidentally mask itself. - maskDir: path.join(awfWorkDir, 'bounded-queries-mask'), }; } diff --git a/src/bounded-query/types.ts b/src/bounded-query/types.ts index 7c7e26e69..b7eeb78d6 100644 --- a/src/bounded-query/types.ts +++ b/src/bounded-query/types.ts @@ -34,7 +34,7 @@ export interface BoundedQuerySeed { } /** - * The document written to `/bounded-queries/seed-map.json` and mounted + * The document written to the dedicated broker-private host root and mounted * read-only into the broker. * * It intentionally contains only what the broker needs: the mapping from a diff --git a/src/docker-manager-diagnostics.test.ts b/src/docker-manager-diagnostics.test.ts index d67b99aca..20575a38a 100644 --- a/src/docker-manager-diagnostics.test.ts +++ b/src/docker-manager-diagnostics.test.ts @@ -2,6 +2,7 @@ import { preserveIptablesAudit } from './artifact-preservation'; import { collectDiagnosticLogs } from './diagnostic-collector'; import * as fs from 'fs'; import * as path from 'path'; +import { resolveBoundedQueryPaths } from './bounded-query/paths'; import { mockExecaFn, mockExecaSync } from './test-helpers/mock-execa.test-utils'; import { useTempDir } from './test-helpers/docker-test-fixtures.test-utils'; @@ -161,7 +162,7 @@ describe('docker-manager diagnostics', () => { }); it('should copy the bounded-query broker audit before work directory cleanup', () => { - const brokerAuditDir = path.join(getDir(), 'bounded-queries', 'audit'); + const brokerAuditDir = resolveBoundedQueryPaths(getDir()).auditDir; fs.mkdirSync(brokerAuditDir, { recursive: true }); fs.writeFileSync( path.join(brokerAuditDir, 'bounded-query.jsonl'), @@ -182,6 +183,7 @@ describe('docker-manager diagnostics', () => { ], expect.objectContaining({ reject: false }), ); + fs.rmSync(resolveBoundedQueryPaths(getDir()).root, { recursive: true, force: true }); }); }); }); diff --git a/src/services/bounded-query-compose.test.ts b/src/services/bounded-query-compose.test.ts index 2b16ea1c0..18518c69e 100644 --- a/src/services/bounded-query-compose.test.ts +++ b/src/services/bounded-query-compose.test.ts @@ -19,7 +19,7 @@ const boundedQueries: BoundedQueriesConfig = { /** * End-to-end compose assembly checks for bounded queries: the broker must appear - * as an optional, network-less service, gate the agent, and inject exactly two + * as an optional, network-less service, gate the agent, and inject only ingress * mounts plus three environment variables into the agent — and nothing at all * when the feature is off. */ @@ -93,11 +93,9 @@ describe('bounded-query broker in generated Docker Compose', () => { const agent = result.services['agent'] as unknown as Record; const boundedQueryMounts = (agent.volumes as string[]).filter((v) => v.includes('/bounded-queries')); - // 2 masking mounts (hide the bounded-query root) + 2 socket mounts + 2 skill mounts = 6 - expect(boundedQueryMounts).toHaveLength(6); - // Masking mounts are read-only; socket mounts are read-write; skill mounts are read-only + expect(boundedQueryMounts).toHaveLength(4); expect(boundedQueryMounts.filter((v) => v.endsWith(':rw'))).toHaveLength(2); - expect(boundedQueryMounts.filter((v) => v.endsWith(':ro'))).toHaveLength(4); + expect(boundedQueryMounts.filter((v) => v.endsWith(':ro'))).toHaveLength(2); expect(boundedQueryMounts.join(' ')).not.toContain('/seeds'); expect(boundedQueryMounts.join(' ')).not.toContain('docker.sock'); }); diff --git a/src/services/bounded-query-service.test.ts b/src/services/bounded-query-service.test.ts index 29673a6dd..b543a5466 100644 --- a/src/services/bounded-query-service.test.ts +++ b/src/services/bounded-query-service.test.ts @@ -83,8 +83,12 @@ describe('buildBoundedQueryService', () => { expect(volumes).toContain(`${paths.auditDir}:/var/log/awf-bounded-query:rw`); }); + it('keeps broker control state on a broker-only mount', () => { + expect(volumes).toContain(`${paths.controlDir}:/run/awf-bounded-query-control:rw`); + }); + it('mounts nothing else', () => { - expect(volumes).toHaveLength(6); + expect(volumes).toHaveLength(7); }); it('passes only AWF-chosen limits and the resolved query image', () => { @@ -196,13 +200,8 @@ describe('buildBoundedQueryService', () => { it('mounts the socket read-write and the skill read-only, for chroot and non-chroot paths', () => { expect(agentVolumes).toEqual([ - // Masking mounts first — hide the bounded-query root visible through /tmp. - `${paths.maskDir}:${paths.root}:ro`, - `${paths.maskDir}:/host${paths.root}:ro`, - // Socket mounts. `${paths.runDir}:${AGENT_SOCKET_DIR}:rw`, `${paths.runDir}:/host${AGENT_SOCKET_DIR}:rw`, - // Skill mounts. `${paths.agentDir}:${AGENT_SKILL_DIR}:ro`, `${paths.agentDir}:/host${AGENT_SKILL_DIR}:ro`, ]); @@ -210,8 +209,11 @@ describe('buildBoundedQueryService', () => { it('never mounts the seeds, the broker work directory, or the audit log into the agent', () => { const joined = agentVolumes.join(' '); + expect(agentVolumes).toHaveLength(4); + expect(joined).not.toContain(paths.root); expect(joined).not.toContain(paths.seedsDir); expect(joined).not.toContain(paths.workDir); + expect(joined).not.toContain(paths.controlDir); expect(joined).not.toContain(paths.auditDir); expect(joined).not.toContain(paths.seedMapPath); }); @@ -229,9 +231,8 @@ describe('buildBoundedQueryService', () => { }); it('prefixes the agent socket and skill mounts symmetrically', () => { - // Masking mounts are at [0] and [1]; socket mounts start at [2]. - expect(agentVolumes[2]).toBe(`/host${paths.runDir}:${AGENT_SOCKET_DIR}:rw`); - expect(agentVolumes[4]).toBe(`/host${paths.agentDir}:${AGENT_SKILL_DIR}:ro`); + expect(agentVolumes[0]).toBe(`/host${paths.runDir}:${AGENT_SOCKET_DIR}:rw`); + expect(agentVolumes[2]).toBe(`/host${paths.agentDir}:${AGENT_SKILL_DIR}:ro`); }); it('hands the daemon-visible work directory to the broker for query mounts', () => { diff --git a/src/services/bounded-query-service.ts b/src/services/bounded-query-service.ts index c206f9a6b..8ddce68b8 100644 --- a/src/services/bounded-query-service.ts +++ b/src/services/bounded-query-service.ts @@ -10,6 +10,7 @@ import { AGENT_SOCKET_DIR, AGENT_SOCKET_PATH, BROKER_AUDIT_DIR, + BROKER_CONTROL_DIR, BROKER_DOCKER_SOCKET_PATH, BROKER_SEED_MAP_PATH, BROKER_SEEDS_DIR, @@ -182,6 +183,7 @@ export function buildBoundedQueryService(params: BoundedQueryServiceParams): Bou `${paths.seedsDir}:${BROKER_SEEDS_DIR}:ro`, `${paths.workDir}:${BROKER_WORK_DIR}:rw`, `${paths.runDir}:${BROKER_SOCKET_DIR}:rw`, + `${paths.controlDir}:${BROKER_CONTROL_DIR}:rw`, `${paths.auditDir}:${BROKER_AUDIT_DIR}:rw`, `${paths.seedMapPath}:${BROKER_SEED_MAP_PATH}:ro`, `${dockerSocketPath}:${BROKER_DOCKER_SOCKET_PATH}:rw`, @@ -233,29 +235,12 @@ export function buildBoundedQueryService(params: BoundedQueryServiceParams): Bou AWF_BOUNDED_QUERY_REPOS: boundedQueries.privateRepos.map((repository) => repository.repo).join(','), }; - // The agent receives four bounded-query mounts: - // - // 1+2. Masking mounts: an empty directory is mounted at the bounded-query - // root as seen through the agent's broad /tmp bind mount. This hides - // seeds, work, audit, and the seed-map from the agent even in rootless - // mode where directory permissions alone are insufficient. - // - // 3+4. Socket mount: the broker's Unix socket directory at its contract path. - // - // 5+6. Skill mount: the generated SKILL.md at its contract path. - // - // Paths are duplicated (bare and /host-prefixed) because the agent runs - // chrooted into /host. The masking mounts come first; Docker applies mounts - // in order, so the more-specific socket/skill mounts take precedence. + // The agent receives only the socket and skill mounts. Paths are duplicated + // (bare and /host-prefixed) because the agent runs chrooted into /host. const agentVolumes = applyHostPathPrefixToVolumes( [ - // Masking mounts — cover the bounded-query root visible through /tmp. - `${paths.maskDir}:${paths.root}:ro`, - `${paths.maskDir}:/host${paths.root}:ro`, - // Socket mounts. `${paths.runDir}:${AGENT_SOCKET_DIR}:rw`, `${paths.runDir}:/host${AGENT_SOCKET_DIR}:rw`, - // Skill mounts. `${paths.agentDir}:${AGENT_SKILL_DIR}:ro`, `${paths.agentDir}:/host${AGENT_SKILL_DIR}:ro`, ], @@ -272,9 +257,8 @@ export function buildBoundedQueryService(params: BoundedQueryServiceParams): Bou /** * True when a volume entry is one of the bounded-query agent mounts. * - * The ARC/DinD sysroot filter drops bind mounts sourced from `workDir`; the - * bounded-query socket, skill, and masking mounts are sourced there but are - * mandatory, so they are exempted explicitly rather than silently disappearing. + * Recognizing these mounts centrally lets sysroot filtering preserve mandatory + * bounded-query ingress without coupling that code to dynamic host paths. */ export function isBoundedQueryAgentMount(volume: string): boolean { const target = volume.split(':')[1]; @@ -282,10 +266,7 @@ export function isBoundedQueryAgentMount(volume: string): boolean { const normalized = target.startsWith('/host') ? target.slice('/host'.length) : target; return ( normalized === AGENT_SOCKET_DIR || - normalized === AGENT_SKILL_DIR || - // The masking mount's target is the bounded-query root itself (paths.root). - // We check by suffix since paths.root includes the dynamic workDir prefix. - normalized.endsWith('/bounded-queries') + normalized === AGENT_SKILL_DIR ); } diff --git a/src/services/optional-services.ts b/src/services/optional-services.ts index 7547383d6..67b89ce57 100644 --- a/src/services/optional-services.ts +++ b/src/services/optional-services.ts @@ -77,9 +77,9 @@ function filterAgentVolumesForSysroot( const source = parts[0]; const target = parts[1]; - // Bounded-query mounts are sourced from workDir but are mandatory: dropping - // them would leave bounded queries half-enabled (wrapper present, broker - // unreachable) instead of failing loudly. + // Bounded-query ingress mounts are mandatory: dropping them would leave + // bounded queries half-enabled (wrapper present, broker unreachable) + // instead of failing loudly. if (isBoundedQueryAgentMount(volume)) return true; // Drop sysroot-shadowed targets (system binaries provided by volume) From eb246d6ca8e63e60e1bb8fa11902258c1c041d27 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 30 Jul 2026 17:26:45 -0700 Subject: [PATCH 2/4] test: align bounded query path assertions Update staging, skill, and compose expectations for the disjoint private and ingress roots. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651 --- src/bounded-query/skill.test.ts | 5 ++++- src/bounded-query/staging.test.ts | 10 +++++++--- src/services/bounded-query-compose.test.ts | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/bounded-query/skill.test.ts b/src/bounded-query/skill.test.ts index fa70a6025..b62e58ee1 100644 --- a/src/bounded-query/skill.test.ts +++ b/src/bounded-query/skill.test.ts @@ -75,6 +75,8 @@ describe('writeBoundedQuerySkill', () => { }); afterEach(() => { + const paths = resolveBoundedQueryPaths(workDir); + fs.rmSync(paths.ingressRoot, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); }); @@ -87,7 +89,8 @@ describe('writeBoundedQuerySkill', () => { }); expect(containerPath).toBe(AGENT_SKILL_PATH); - expect(paths.skillPath.startsWith(workDir)).toBe(true); + expect(paths.skillPath.startsWith(paths.ingressRoot)).toBe(true); + expect(paths.skillPath.startsWith(workDir)).toBe(false); // Open with O_NOFOLLOW to avoid TOCTOU between stat and read. const fd = fs.openSync(paths.skillPath, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW); try { diff --git a/src/bounded-query/staging.test.ts b/src/bounded-query/staging.test.ts index 4580a704c..d0d275024 100644 --- a/src/bounded-query/staging.test.ts +++ b/src/bounded-query/staging.test.ts @@ -196,7 +196,9 @@ describe('stageBoundedQuerySeeds', () => { }); afterEach(() => { - releaseSeedPermissions(resolveBoundedQueryPaths(workDir).seedsDir); + const paths = resolveBoundedQueryPaths(workDir); + releaseSeedPermissions(paths.seedsDir); + fs.rmSync(paths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); }); @@ -293,7 +295,7 @@ describe('stageBoundedQuerySeeds', () => { expect(fs.existsSync(path.join(gitDir, 'FETCH_HEAD'))).toBe(false); expect(fs.existsSync(path.join(gitDir, 'refs', 'remotes'))).toBe(false); expect(fs.readFileSync(path.join(gitDir, 'packed-refs'), 'utf8')).not.toContain('refs/remotes/'); - expect(paths.seedsDir).toContain('bounded-queries'); + expect(paths.seedsDir).toContain('awf-bounded-query-private-'); }); it('records the staged commit and an opaque seed id', async () => { @@ -413,7 +415,9 @@ describe('releaseSeedPermissions', () => { releaseSeedPermissions(paths.seedsDir); expect(() => fs.rmSync(paths.seedsDir, { recursive: true })).not.toThrow(); } finally { - releaseSeedPermissions(resolveBoundedQueryPaths(workDir).seedsDir); + const paths = resolveBoundedQueryPaths(workDir); + releaseSeedPermissions(paths.seedsDir); + fs.rmSync(paths.root, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true }); } }); diff --git a/src/services/bounded-query-compose.test.ts b/src/services/bounded-query-compose.test.ts index 18518c69e..e1548c762 100644 --- a/src/services/bounded-query-compose.test.ts +++ b/src/services/bounded-query-compose.test.ts @@ -91,7 +91,7 @@ describe('bounded-query broker in generated Docker Compose', () => { it('gives the agent the socket and skill mounts and nothing else bounded-query related', () => { const result = generateDockerCompose(enabled(), mockNetworkConfig); const agent = result.services['agent'] as unknown as Record; - const boundedQueryMounts = (agent.volumes as string[]).filter((v) => v.includes('/bounded-queries')); + const boundedQueryMounts = (agent.volumes as string[]).filter((v) => v.includes('awf-bounded-query')); expect(boundedQueryMounts).toHaveLength(4); expect(boundedQueryMounts.filter((v) => v.endsWith(':rw'))).toHaveLength(2); From ab42e2e35eef74b2da7c4eeaaec3352c210e7a9f Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 30 Jul 2026 17:36:03 -0700 Subject: [PATCH 3/4] test: cover private state cleanup branches Exercise rootless repair, cleanup failures, and all mount-policy rejection paths while removing unreachable post-mkdir checks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651 --- src/bounded-query/manager.test.ts | 53 +++++++++++++++++++ src/bounded-query/manager.ts | 72 +++++++++++++++----------- src/bounded-query/mount-policy.test.ts | 36 +++++++++++++ src/bounded-query/mount-policy.ts | 3 -- 4 files changed, 131 insertions(+), 33 deletions(-) diff --git a/src/bounded-query/manager.test.ts b/src/bounded-query/manager.test.ts index ada49a77b..6267c9b40 100644 --- a/src/bounded-query/manager.test.ts +++ b/src/bounded-query/manager.test.ts @@ -320,4 +320,57 @@ describe('teardownBoundedQueries', () => { fs.rmSync(workDir, { recursive: true, force: true }); } }); + + it('repairs rootless private-state permissions and retries cleanup', () => { + const paths = resolveBoundedQueryPaths('/tmp/rootless-cleanup'); + const permissionError = Object.assign(new Error('permission denied'), { code: 'EACCES' }); + const removeTree = jest.fn() + .mockImplementationOnce(() => { throw permissionError; }) + .mockImplementation(() => undefined); + const repairPermissions = jest.fn(); + + managerTestHelpers.removePrivateState( + buildConfig('/tmp/rootless-cleanup'), + paths, + { removeTree, repairPermissions }, + ); + + expect(repairPermissions).toHaveBeenCalledWith( + [paths.root, paths.ingressRoot], + undefined, + undefined, + undefined, + undefined, + ); + expect(removeTree).toHaveBeenCalledTimes(3); + }); + + it('surfaces cleanup failures after rootless permission repair', () => { + const paths = resolveBoundedQueryPaths('/tmp/rootless-retry-failure'); + const permissionError = Object.assign(new Error('permission denied'), { code: 'EACCES' }); + const removeTree = jest.fn() + .mockImplementationOnce(() => { throw permissionError; }) + .mockImplementationOnce(() => { throw new Error('still denied'); }); + + expect(() => managerTestHelpers.removePrivateState( + buildConfig('/tmp/rootless-retry-failure'), + paths, + { removeTree, repairPermissions: jest.fn() }, + )).not.toThrow(); + }); + + it('surfaces non-permission cleanup failures without attempting repair', () => { + const paths = resolveBoundedQueryPaths('/tmp/private-cleanup-failure'); + const repairPermissions = jest.fn(); + + expect(() => managerTestHelpers.removePrivateState( + buildConfig('/tmp/private-cleanup-failure'), + paths, + { + removeTree: () => { throw new Error('I/O failure'); }, + repairPermissions, + }, + )).not.toThrow(); + expect(repairPermissions).not.toHaveBeenCalled(); + }); }); diff --git a/src/bounded-query/manager.ts b/src/bounded-query/manager.ts index e73e3cdd3..62bdb6511 100644 --- a/src/bounded-query/manager.ts +++ b/src/bounded-query/manager.ts @@ -53,13 +53,7 @@ function ensureModeDirectory(target: string, mode: number): void { */ function prepareDirectories(paths: BoundedQueryPaths): void { fs.mkdirSync(paths.root, { mode: 0o700 }); - if (fs.lstatSync(paths.root).isSymbolicLink()) { - throw new Error(`Refusing to use symlink as bounded-query private root: ${paths.root}`); - } fs.mkdirSync(paths.ingressRoot, { mode: 0o700 }); - if (fs.lstatSync(paths.ingressRoot).isSymbolicLink()) { - throw new Error(`Refusing to use symlink as bounded-query ingress root: ${paths.ingressRoot}`); - } ensureModeDirectory(paths.seedsDir, 0o700); ensureModeDirectory(paths.workDir, 0o700); ensureModeDirectory(paths.controlDir, 0o700); @@ -73,6 +67,46 @@ function prepareDirectories(paths: BoundedQueryPaths): void { // Non-root host (e.g. network-isolation mode): the broker chowns/chmods // the socket itself once it is bound. } + + interface RemovePrivateStateDeps { + removeTree?: (target: string) => void; + repairPermissions?: typeof fixArtifactPermissionsForRootless; + } + + function removePrivateState( + config: WrapperConfig, + paths: BoundedQueryPaths, + deps: RemovePrivateStateDeps = {}, + ): void { + const removeTree = deps.removeTree ?? ((target: string) => { + fs.rmSync(target, { recursive: true, force: true }); + }); + const repairPermissions = deps.repairPermissions ?? fixArtifactPermissionsForRootless; + + try { + removeTree(paths.root); + removeTree(paths.ingressRoot); + } catch (error: unknown) { + if (error && typeof error === 'object' && 'code' in error && error.code === 'EACCES') { + logger.debug('Bounded queries: repairing rootless private-state permissions before cleanup'); + repairPermissions( + [paths.root, paths.ingressRoot], + config.dockerHostPathPrefix, + config.imageRegistry, + config.imageTag, + config.agentImage, + ); + try { + removeTree(paths.root); + removeTree(paths.ingressRoot); + } catch (retryError) { + logger.warn('Bounded queries: failed to remove private state after permission repair', retryError); + } + return; + } + logger.warn('Bounded queries: failed to remove private state during cleanup', error); + } + } } /** Writes the broker's repo → opaque seed map. */ @@ -254,30 +288,7 @@ export async function teardownBoundedQueries(config: WrapperConfig): Promise { ).toThrow(/agent session-state directory/); }); + it('rejects malformed custom mounts instead of ignoring their source', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => + assertBoundedQueryPrivateRootIsolated(config(workDir, ['named-volume:/data:ro']), paths), + ).toThrow(/could not parse custom bind mount/); + }); + + it('rejects a chroot binaries source containing the private root', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => + assertBoundedQueryPrivateRootIsolated( + { ...config(workDir), chrootBinariesSourcePath: testRoot }, + paths, + ), + ).toThrow(/chroot binaries source/); + }); + + it('rejects an agent-visible Docker socket path inside the private root', () => { + const paths = resolveBoundedQueryPaths(workDir, privateBase); + expect(() => + assertBoundedQueryPrivateRootIsolated( + { + ...config(workDir), + enableDind: true, + awfDockerHost: `unix://${path.join(paths.root, 'docker.sock')}`, + }, + paths, + ), + ).toThrow(/agent Docker socket/); + }); + it('resolves a missing suffix through a symlinked ancestor', () => { const target = path.join(testRoot, 'target'); const alias = path.join(testRoot, 'alias'); @@ -109,4 +140,9 @@ describe('bounded-query private-root mount policy', () => { expect(resolvePathThroughExistingAncestor(path.join(alias, 'missing', 'leaf'))) .toBe(path.join(target, 'missing', 'leaf')); }); + + it('rejects relative paths before filesystem resolution', () => { + expect(() => resolvePathThroughExistingAncestor('../private')) + .toThrow(/requires an absolute path/); + }); }); diff --git a/src/bounded-query/mount-policy.ts b/src/bounded-query/mount-policy.ts index 7515ed15d..2c3783e9f 100644 --- a/src/bounded-query/mount-policy.ts +++ b/src/bounded-query/mount-policy.ts @@ -26,9 +26,6 @@ export function resolvePathThroughExistingAncestor(candidate: string): string { let existing = path.resolve(candidate); while (!fs.existsSync(existing)) { const parent = path.dirname(existing); - if (parent === existing) { - throw new Error(`Bounded-query mount policy could not resolve an existing ancestor: ${candidate}`); - } missing.unshift(path.basename(existing)); existing = parent; } From ca399a0b8aa29af4c10a0ec9bf4c667541a591a8 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 30 Jul 2026 17:40:44 -0700 Subject: [PATCH 4/4] fix: scope private state cleanup helper Keep rootless cleanup available to teardown and focused tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651 --- src/bounded-query/manager.ts | 68 ++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/bounded-query/manager.ts b/src/bounded-query/manager.ts index 62bdb6511..ef2bb1867 100644 --- a/src/bounded-query/manager.ts +++ b/src/bounded-query/manager.ts @@ -67,45 +67,45 @@ function prepareDirectories(paths: BoundedQueryPaths): void { // Non-root host (e.g. network-isolation mode): the broker chowns/chmods // the socket itself once it is bound. } +} - interface RemovePrivateStateDeps { - removeTree?: (target: string) => void; - repairPermissions?: typeof fixArtifactPermissionsForRootless; - } +interface RemovePrivateStateDeps { + removeTree?: (target: string) => void; + repairPermissions?: typeof fixArtifactPermissionsForRootless; +} - function removePrivateState( - config: WrapperConfig, - paths: BoundedQueryPaths, - deps: RemovePrivateStateDeps = {}, - ): void { - const removeTree = deps.removeTree ?? ((target: string) => { - fs.rmSync(target, { recursive: true, force: true }); - }); - const repairPermissions = deps.repairPermissions ?? fixArtifactPermissionsForRootless; +function removePrivateState( + config: WrapperConfig, + paths: BoundedQueryPaths, + deps: RemovePrivateStateDeps = {}, +): void { + const removeTree = deps.removeTree ?? ((target: string) => { + fs.rmSync(target, { recursive: true, force: true }); + }); + const repairPermissions = deps.repairPermissions ?? fixArtifactPermissionsForRootless; - try { - removeTree(paths.root); - removeTree(paths.ingressRoot); - } catch (error: unknown) { - if (error && typeof error === 'object' && 'code' in error && error.code === 'EACCES') { - logger.debug('Bounded queries: repairing rootless private-state permissions before cleanup'); - repairPermissions( - [paths.root, paths.ingressRoot], - config.dockerHostPathPrefix, - config.imageRegistry, - config.imageTag, - config.agentImage, - ); - try { - removeTree(paths.root); - removeTree(paths.ingressRoot); - } catch (retryError) { - logger.warn('Bounded queries: failed to remove private state after permission repair', retryError); - } - return; + try { + removeTree(paths.root); + removeTree(paths.ingressRoot); + } catch (error: unknown) { + if (error && typeof error === 'object' && 'code' in error && error.code === 'EACCES') { + logger.debug('Bounded queries: repairing rootless private-state permissions before cleanup'); + repairPermissions( + [paths.root, paths.ingressRoot], + config.dockerHostPathPrefix, + config.imageRegistry, + config.imageTag, + config.agentImage, + ); + try { + removeTree(paths.root); + removeTree(paths.ingressRoot); + } catch (retryError) { + logger.warn('Bounded queries: failed to remove private state after permission repair', retryError); } - logger.warn('Bounded queries: failed to remove private state during cleanup', error); + return; } + logger.warn('Bounded queries: failed to remove private state during cleanup', error); } }