Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 73 additions & 2 deletions src/sbx-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,21 @@ import {
sanitizeEnvForSbx,
SBX_DEFAULT_NAME,
} from './sbx-manager';
import * as fs from 'fs';
import { mockExecaFn } from './test-helpers/mock-execa.test-utils';
import { logger } from './logger';

// eslint-disable-next-line @typescript-eslint/no-require-imports
jest.mock('execa', () => require('./test-helpers/mock-execa.test-utils').execaMockFactory());
// eslint-disable-next-line @typescript-eslint/no-require-imports
jest.mock('./logger', () => require('./test-helpers/mock-logger.test-utils').loggerMockFactory());
// Mock fs so home-mount curation (existsSync) is deterministic per test.
jest.mock('fs', () => {
const actual = jest.requireActual<typeof import('fs')>('fs');
return { ...actual, existsSync: jest.fn(() => false) };
});

const mockedExistsSync = fs.existsSync as jest.Mock;

const mockedLogger = jest.mocked(logger);

Expand Down Expand Up @@ -81,7 +89,16 @@ describe('sbx-manager', () => {
});

describe('createSandbox', () => {
beforeEach(() => {
// Default: no host $HOME subdirs exist, so home-mount curation is a no-op
// unless a test opts in. Individual tests re-mock as needed.
mockedExistsSync.mockReset();
mockedExistsSync.mockReturnValue(false);
});

it('uses shell agent, configured mounts, and sanitized env', async () => {
// No host $HOME subdirs exist → only workspace, extra mounts, /tmp and
// /usr/local/bin are mounted (the whole $HOME is never mounted).
mockExecaFn
.mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }) // auth check
.mockResolvedValueOnce({ exitCode: 0, stdout: 'Created sandbox', stderr: '' }); // sbx create
Expand All @@ -101,7 +118,6 @@ describe('sbx-manager', () => {
'/tmp/gh-aw:ro',
'/tmp',
'/usr/local/bin',
process.env.HOME || '/home/runner',
], expect.objectContaining({
input: 'y\n',
}));
Expand All @@ -112,6 +128,56 @@ describe('sbx-manager', () => {
expect(sbxCreateCall.env).toBeUndefined();
});

it('never mounts the whole $HOME (only whitelisted subdirs that exist)', async () => {
const homePath = process.env.HOME || '/home/runner';
// Simulate a host home that contains both tool dirs AND credential stores.
mockedExistsSync.mockImplementation((p: fs.PathLike) => {
const s = String(p);
return (
s === `${homePath}/.cache` ||
s === `${homePath}/.config` ||
s === `${homePath}/.copilot` ||
s === `${homePath}/.aws` ||
s === `${homePath}/.ssh` ||
s === `${homePath}/.docker`
);
});
mockExecaFn
.mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' })
.mockResolvedValueOnce({ exitCode: 0, stdout: 'Created sandbox', stderr: '' });

await createSandbox({ workspaceDir: '/workspace', squidIp: '172.30.0.10' });

const args: string[] = mockExecaFn.mock.calls[1][1];
// The whole home is never mounted...
expect(args).not.toContain(homePath);
// ...whitelisted tool/state dirs that exist ARE mounted...
expect(args).toContain(`${homePath}/.cache`);
expect(args).toContain(`${homePath}/.config`);
expect(args).toContain(`${homePath}/.copilot`);
// ...and credential stores are NEVER mounted, even though they exist.
expect(args).not.toContain(`${homePath}/.aws`);
expect(args).not.toContain(`${homePath}/.ssh`);
expect(args).not.toContain(`${homePath}/.docker`);
});

it('skips whitelisted home subdirs that do not exist on the host', async () => {
const homePath = process.env.HOME || '/home/runner';
mockedExistsSync.mockImplementation(
(p: fs.PathLike) => String(p) === `${homePath}/.npm`,
);
mockExecaFn
.mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' })
.mockResolvedValueOnce({ exitCode: 0, stdout: 'Created sandbox', stderr: '' });

await createSandbox({ workspaceDir: '/workspace', squidIp: '172.30.0.10' });

const args: string[] = mockExecaFn.mock.calls[1][1];
expect(args).toContain(`${homePath}/.npm`);
expect(args).not.toContain(`${homePath}/.cache`);
expect(args).not.toContain(`${homePath}/.rustup`);
});

it('uses SBX_DEFAULT_NAME when no name provided', async () => {
mockExecaFn
.mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }) // auth check
Expand Down Expand Up @@ -258,6 +324,9 @@ describe('sbx-manager', () => {

it('skips system paths already in workspace or dedup list', async () => {
const home = process.env.HOME || '/home/runner';
mockedExistsSync.mockImplementation(
(p: fs.PathLike) => String(p) === `${home}/.cache`,
);
mockExecaFn
.mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' })
.mockResolvedValueOnce({ exitCode: 0, stdout: 'Created sandbox', stderr: '' });
Expand All @@ -274,7 +343,9 @@ describe('sbx-manager', () => {
const tmpCount = args.filter(a => a === '/tmp').length;
expect(tmpCount).toBe(1);
expect(args).toContain('/usr/local/bin');
expect(args).toContain(home);
// The whole $HOME is never mounted; only existing whitelisted subdirs are.
expect(args).not.toContain(home);
expect(args).toContain(`${home}/.cache`);
});
});

Expand Down
30 changes: 25 additions & 5 deletions src/sbx-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
*/

import execa from 'execa';
import * as fs from 'fs';
import { logger } from './logger';
import { HOME_TOOL_SUBDIRS } from './services/agent-volumes/home-whitelist';

/** Name prefix for AWF-managed sandboxes. */
const SBX_NAME_PREFIX = 'awf-agent';
Expand Down Expand Up @@ -144,17 +146,35 @@ export async function createSandbox(config: SbxConfig): Promise<string> {
}
}

// Mount /tmp so agent runtime files (prompts, logs) are accessible.
// Mount /usr/local/bin for Copilot CLI and other installed tools.
// Mount $HOME for agent writable dirs (.cache, .config, .local, etc.)
const homePath = process.env.HOME || '/home/runner';
for (const sysPath of ['/tmp', '/usr/local/bin', homePath]) {
// Mount /tmp so agent runtime files (prompts, logs) are accessible, and
// /usr/local/bin for Copilot CLI and other installed tools.
for (const sysPath of ['/tmp', '/usr/local/bin']) {
if (!seenPaths.has(sysPath)) {
seenPaths.add(sysPath);
args.push(sysPath);
}
}

// SECURITY: never mount the whole $HOME into the microVM. sbx mounts are
// positional (host path == guest path) and cannot express the per-file
// /dev/null credential overlays that compose mode uses (see
// credential-hiding.ts), so the only way to keep host secrets out of the VM
// is to curate which $HOME subdirs are mounted. We share the same whitelist
// as the compose chroot home strategy (HOME_TOOL_SUBDIRS) plus the agent
// state dirs (.copilot, .gemini). Credential stores such as ~/.aws, ~/.ssh,
// ~/.docker, ~/.kube, ~/.azure, ~/.gnupg, ~/.netrc and ~/.gitconfig are never
// whitelisted, so they never enter the sandbox. Only paths that exist on the
// host are mounted, because sbx requires the mount source to exist.
const homePath = process.env.HOME || '/home/runner';
const homeSubdirs = ['.copilot', ...HOME_TOOL_SUBDIRS, '.gemini'];

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d07a927. .copilot and .gemini are no longer appended wholesale — they are now credential-nesting parents, mounted child-by-child with config.json (Copilot) and oauth_creds.json/google_accounts.json (Gemini) excluded. Only their safe children (session-state, logs, tmp, settings, etc.) reach the VM. New sbx test asserts the token files never appear in the mount args.

for (const subdir of homeSubdirs) {
const hostSubdir = `${homePath}/${subdir}`;
if (seenPaths.has(hostSubdir)) continue;
if (!fs.existsSync(hostSubdir)) continue;
seenPaths.add(hostSubdir);
args.push(hostSubdir);
}

logger.info(`[sbx] Running: sbx ${args.join(' ')}`);

// Do NOT pass a custom `env` to sbx create. The sanitized env (which strips
Expand Down
15 changes: 4 additions & 11 deletions src/services/agent-volumes/home-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as path from 'path';
import { logger } from '../../logger';
import { resolveRunnerToolCachePath } from '../../runner-tool-cache';
import { WrapperConfig } from '../../types';
import { HOME_TOOL_SUBDIRS } from './home-whitelist';

interface HomeMountsParams {
config: WrapperConfig;
Expand Down Expand Up @@ -42,22 +43,14 @@ function buildToolDirectoryMounts(params: HomeMountsParams): string[] {
mounts.push(`${sessionStatePath}:/host${effectiveHome}/.copilot/session-state:rw`);
mounts.push(`${agentLogsPath}:/host${effectiveHome}/.copilot/logs:rw`);

mounts.push(`${effectiveHome}/.cache:/host${effectiveHome}/.cache:rw`);
mounts.push(`${effectiveHome}/.config:/host${effectiveHome}/.config:rw`);
mounts.push(`${effectiveHome}/.local:/host${effectiveHome}/.local:rw`);

mounts.push(`${effectiveHome}/.anthropic:/host${effectiveHome}/.anthropic:rw`);
mounts.push(`${effectiveHome}/.claude:/host${effectiveHome}/.claude:rw`);
for (const subdir of HOME_TOOL_SUBDIRS) {
mounts.push(`${effectiveHome}/${subdir}:/host${effectiveHome}/${subdir}:rw`);
}

if (config.geminiApiKey || config.googleApiKey) {
mounts.push(`${effectiveHome}/.gemini:/host${effectiveHome}/.gemini:rw`);
}

mounts.push(`${effectiveHome}/.cargo:/host${effectiveHome}/.cargo:rw`);
mounts.push(`${effectiveHome}/.rustup:/host${effectiveHome}/.rustup:rw`);
mounts.push(`${effectiveHome}/.npm:/host${effectiveHome}/.npm:rw`);
mounts.push(`${effectiveHome}/.nvm:/host${effectiveHome}/.nvm:rw`);

const runnerToolCacheDir = resolveRunnerToolCachePath(config, effectiveHome);
if (runnerToolCacheDir) {
mounts.push(`${runnerToolCacheDir}:/host${runnerToolCacheDir}:ro`);
Expand Down
37 changes: 37 additions & 0 deletions src/services/agent-volumes/home-whitelist.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Canonical whitelist of `$HOME` subdirectories that agents legitimately need
* (tool caches, language toolchains, agent state).
*
* This list is the single source of truth shared by **both** sandbox backends
* so their home-directory exposure stays in sync:
*
* - **Compose / chroot mode** (`home-strategy.ts`) mounts an empty home volume
* and then bind-mounts these subdirs on top, and additionally blanks known
* credential files with `/dev/null` overlays (`credential-hiding.ts`).
* - **sbx microVM mode** (`sbx-manager.ts`) mounts these subdirs individually
* instead of the whole `$HOME`. sbx uses positional (host path == guest path)
* mounts and cannot express per-file `/dev/null` overlays, so directory
* curation is its only mechanism — which makes this whitelist the primary
* protection there.
*
* SECURITY: never add a directory whose primary purpose is storing credentials
* (for example `.aws`, `.ssh`, `.docker`, `.kube`, `.azure`, `.gnupg`). Any such
* store must stay OUT of the sandbox. Directories listed here can still contain
* stray secret files; compose mode masks the known ones via
* `buildCredentialHidingOverlays()`, but sbx cannot, so keep this list to
* genuinely non-credential tooling paths.
*
* `.gemini` is intentionally NOT included: compose mode mounts it only when a
* Gemini/Google API key is configured, so each caller handles it separately.
*/
export const HOME_TOOL_SUBDIRS = [
'.cache',
'.config',
'.local',
'.anthropic',
'.claude',
'.cargo',
Comment on lines +32 to +33

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in d07a927. .cargo and .claude are now credential-nesting parents: sbx mounts them child-by-child and excludes the secret files (.cargo/credentials, .cargo/credentials.toml, .claude/.credentials.json) rather than mounting the dir wholesale. See CREDENTIAL_EXCLUSIONS_BY_PARENT in home-whitelist.ts, now covered by unit + sbx tests.

'.rustup',
'.npm',
'.nvm',
] as const;
Loading