Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
59 changes: 59 additions & 0 deletions src/compose-generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,65 @@ describe('generateDockerCompose', () => {
expect(volumes.some(v => v.split(':')[0] === '/tmp/awf-12345-chroot-home')).toBe(false);
expect(volumes.some(v => v.split(':')[0].startsWith('/tmp/awf-12345'))).toBe(false);
});

// Regression test for gh-aw-firewall#7994: on real ARC/DinD runners
// `--docker-host-path-prefix` is set alongside `runnerTopology: arc-dind`.
// `buildAgentVolumes` applies that prefix before this split-fs filter
// ever sees the volumes, so a chroot-home/`$HOME` dot-dir mount that
// should have been dropped as workDir/home-derived instead survived
// pointing at a daemon-invisible path — and Docker failed creating the
// `/dev/null` -> `.npmrc` mountpoint with a read-only-filesystem error.
it('drops the prefixed chroot-home volume and credential overlays when docker-host-path-prefix is set', () => {
const config = {
...mockConfig,
runnerTopology: 'arc-dind' as const,
workDir: '/tmp/awf-12345',
dockerHostPathPrefix: '/host',
};
const result = generateDockerCompose(config, mockNetworkConfig);
const volumes = result.services.agent.volumes as string[];
const effectiveHome = getRealUserHome();

// No mount sourced from the (prefixed) workDir or its chroot-home
// sibling should survive — the daemon cannot resolve either path.
expect(volumes.some(v => v.split(':')[0].includes('/tmp/awf-12345'))).toBe(false);

// Without an explicit writable `--mount` for the home root, the
// credential-hiding overlays for these exact files (the ones named in
// the issue) must be dropped rather than emitted against a mountpoint
// Docker cannot create.
for (const credentialFile of ['.npmrc', '.docker/config.json', '.composer/auth.json']) {
expect(volumes).not.toContain(`/dev/null:/host${effectiveHome}/${credentialFile}:ro`);
}

// The direct (non-chroot) overlays on the container's own rootfs are
// unaffected and still mask the same files.
for (const credentialFile of ['.npmrc', '.docker/config.json', '.composer/auth.json']) {
expect(volumes).toContain(`/dev/null:${effectiveHome}/${credentialFile}:ro`);
}
});

it('keeps prefixed credential overlays mountable when an explicit writable home mount is supplied', () => {
const effectiveHome = getRealUserHome();
const config = {
...mockConfig,
runnerTopology: 'arc-dind' as const,
workDir: '/tmp/awf-12345',
dockerHostPathPrefix: '/host',
volumeMounts: [`${effectiveHome}/:/host${effectiveHome}:rw`],
};
const result = generateDockerCompose(config, mockNetworkConfig);
const volumes = result.services.agent.volumes as string[];

// The explicitly supplied home-root mount survives, prefixed exactly
// once, and is still writable — so runc can create the credential
// mountpoints nested inside it.
expect(volumes).toContain(`/host${effectiveHome}/:/host${effectiveHome}:rw`);

for (const credentialFile of ['.npmrc', '.docker/config.json', '.composer/auth.json']) {
expect(volumes).toContain(`/dev/null:/host${effectiveHome}/${credentialFile}:ro`);
}
});
});

// Regression: `filesystem.allowWrite` is expressed in guest-visible paths,
Expand Down
41 changes: 40 additions & 1 deletion src/services/host-path-prefix.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,43 @@
import { applyHostPathPrefixToVolumes } from './host-path-prefix';
import { applyHostPathPrefixToVolumes, prefixHostPath } from './host-path-prefix';

describe('prefixHostPath', () => {
it('prepends the normalized prefix to an absolute path', () => {
expect(prefixHostPath('/tmp/awf-12345', '/host')).toBe('/host/tmp/awf-12345');
});

it('matches the source rewrite applied to a mount derived from the same path', () => {
// A caller comparing a bare path (e.g. workDir) against an already
// host-path-prefixed mount source must apply the exact same rewrite, or
// the comparison silently stops matching once a prefix is set.
const workDir = '/tmp/awf-12345';
const mount = `${workDir}-chroot-home:/host/home/runner:rw`;
const [translatedMount] = applyHostPathPrefixToVolumes([mount], '/host');
const translatedSource = translatedMount.split(':')[0];
expect(translatedSource.startsWith(prefixHostPath(workDir, '/host'))).toBe(true);
});

it('leaves relative paths untouched', () => {
expect(prefixHostPath('relative/path', '/host')).toBe('relative/path');
});

it('is a no-op when the prefix is "/"', () => {
expect(prefixHostPath('/home/runner', '/')).toBe('/home/runner');
});

it('leaves a path already under the prefix untouched', () => {
expect(prefixHostPath('/host/home/runner', '/host')).toBe('/host/home/runner');
expect(prefixHostPath('/host', '/host')).toBe('/host');
});

it('re-prefixes an already-prefixed path when translateAlreadyPrefixedPaths is set', () => {
expect(prefixHostPath('/host/home/runner', '/host', { translateAlreadyPrefixedPaths: true }))
.toBe('/host/host/home/runner');
});

it('maps the bare root to the prefix itself', () => {
expect(prefixHostPath('/', '/host')).toBe('/host');
});
});

describe('applyHostPathPrefixToVolumes', () => {
it('returns volumes unchanged when prefix is undefined', () => {
Expand Down
50 changes: 36 additions & 14 deletions src/services/host-path-prefix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,41 @@ interface HostPathPrefixOptions {
translateAlreadyPrefixedPaths?: boolean;
}

/**
* Applies the same rewrite `translateBindMountHostPath` uses for a mount's
* host-side path, but to a bare absolute path rather than a full mount spec.
*
* Callers that need to compare a *pre-translation* path (such as `workDir` or
* `$HOME`) against a *post-translation* mount source — for example a sysroot
* filter deciding whether a volume was derived from `workDir` — must apply
* this first, or the comparison silently stops matching as soon as
* `--docker-host-path-prefix` is set. This is exported precisely so those
* comparisons cannot drift from the rewrite `applyHostPathPrefixToVolumes`
* actually performs.
*
* Deliberately does not special-case kernel virtual filesystems or `/etc`
* identity files — those only matter for concrete mount specs, never for the
* `workDir`/`$HOME` roots this is meant for.
*/
export function prefixHostPath(
hostPath: string,
dockerHostPathPrefix: string,
options: HostPathPrefixOptions = {},
): string {
if (!hostPath.startsWith('/') || dockerHostPathPrefix === '/') {
return hostPath;
}

if (
!options.translateAlreadyPrefixedPaths
&& (hostPath === dockerHostPathPrefix || hostPath.startsWith(`${dockerHostPathPrefix}/`))
) {
return hostPath;
}

return hostPath === '/' ? dockerHostPathPrefix : `${dockerHostPathPrefix}${hostPath}`;
}

function translateBindMountHostPath(
mount: string,
dockerHostPathPrefix: string,
Expand Down Expand Up @@ -110,20 +145,7 @@ function translateBindMountHostPath(
return mount;
}

if (dockerHostPathPrefix === '/') {
return mount;
}

if (
!options.translateAlreadyPrefixedPaths
&& (hostPath === dockerHostPathPrefix || hostPath.startsWith(`${dockerHostPathPrefix}/`))
) {
return mount;
}

const translatedHostPath = hostPath === '/'
? dockerHostPathPrefix
: `${dockerHostPathPrefix}${hostPath}`;
const translatedHostPath = prefixHostPath(hostPath, dockerHostPathPrefix, options);

return mode ? `${translatedHostPath}:${containerPath}:${mode}` : `${translatedHostPath}:${containerPath}`;
}
Expand Down
58 changes: 58 additions & 0 deletions src/services/optional-services.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,5 +240,63 @@ describe('optional-services helpers', () => {
home,
)).toThrow('filesystem.allowWrite cannot safely protect');
});

// Regression test for gh-aw-firewall#7994: `buildAgentVolumes` applies
// `--docker-host-path-prefix` translation *before* this filter runs, so a
// volume derived from `workDir`/`effectiveHome` arrives here with a
// prefixed source (e.g. `/host/tmp/awf-work-chroot-home`). Comparing that
// against the raw, unprefixed `config.workDir`/`effectiveHome` silently
// stopped matching, letting the chroot-home mount and $HOME dot-directory
// binds survive pointing at a path the Docker daemon cannot see — which
// is what produced the `/dev/null` -> `.npmrc` mountpoint EROFS failure.
describe('with --docker-host-path-prefix set', () => {
it('still drops the prefixed chroot-home volume and home dot-dir mounts', () => {
const config: WrapperConfig = {
...baseConfig,
workDir: '/tmp/awf-work',
dockerHostPathPrefix: '/host',
};
const home = '/home/runner';

const filtered = testHelpers.filterAgentVolumesForSysroot(
[
'/host/usr:/host/usr:ro',
'/host/tmp/awf-work-chroot-home:/host/home/runner:rw',
'/host/home/runner/.npm:/host/home/runner/.npm:rw',
'/host/home/runner/_work/_temp/gh-aw:/host/home/runner/_work/_temp/gh-aw:rw',
'/host/tmp:/tmp:rw',
`/dev/null:/host${home}/.npmrc:ro`,
`/dev/null:/host${home}/.docker/config.json:ro`,
`/dev/null:/host${home}/.composer/auth.json:ro`,
],
config,
home,
);

expect(filtered).toEqual([
'/host/home/runner/_work/_temp/gh-aw:/host/home/runner/_work/_temp/gh-aw:rw',
'/host/tmp:/tmp:rw',
]);
});

it('keeps the credential overlays when an explicit --mount backs the prefixed home root', () => {
const home = '/home/runner';
const config: WrapperConfig = {
...baseConfig,
workDir: '/tmp/awf-work',
dockerHostPathPrefix: '/host',
volumeMounts: [`${home}/:/host${home}:rw`],
};

const volumes = [
`/host${home}/:/host${home}:rw`,
`/dev/null:/host${home}/.npmrc:ro`,
`/dev/null:/host${home}/.docker/config.json:ro`,
`/dev/null:/host${home}/.composer/auth.json:ro`,
];

expect(testHelpers.filterAgentVolumesForSysroot(volumes, config, home)).toEqual(volumes);
});
});
});
});
19 changes: 15 additions & 4 deletions src/services/optional-services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { buildEnclaveMcpService } from './enclave-mcp-service';
import { buildSysrootStageService, isSysrootEnabled } from './sysroot-service';
import { resolveDockerHostGateway } from './host-gateway';
import { runtimeUsesIptables } from '../container-runtime';
import { applyHostPathPrefixToVolumes } from './host-path-prefix';
import { applyHostPathPrefixToVolumes, normalizeDockerHostPathPrefix, prefixHostPath } from './host-path-prefix';
import { buildCustomVolumeMounts } from './agent-volumes/workspace-mounts';
import { resolveComposeFilesystemAllowWrite } from './agent-volumes/filesystem-write-policy';
import { NetworkConfig, ImageBuildConfig } from './squid-service';
Expand Down Expand Up @@ -71,7 +71,18 @@ function filterAgentVolumesForSysroot(
'/host/lib64',
'/host/opt',
]);
const normalizedWorkDirPrefix = config.workDir.replace(/\/+$/, '');
// `agentVolumes` has already had `--docker-host-path-prefix` applied (it's
// the last step of `buildAgentVolumes`), so a bare `workDir`/`effectiveHome`
// no longer matches its own derived mount sources once that prefix is set.
// Prefix them the same way before comparing, or every match below silently
// stops firing on split-fs (ARC/DinD) runs — see gh-aw-firewall#7994.
const normalizedDockerHostPathPrefix = config.dockerHostPathPrefix
? normalizeDockerHostPathPrefix(config.dockerHostPathPrefix)
: '';
const prefixedHostPath = (hostPath: string): string =>
normalizedDockerHostPathPrefix ? prefixHostPath(hostPath, normalizedDockerHostPathPrefix) : hostPath;
const normalizedWorkDirPrefix = prefixedHostPath(config.workDir).replace(/\/+$/, '');
const prefixedEffectiveHome = prefixedHostPath(effectiveHome);
const hostHomeMountPrefix = `/host${effectiveHome}`;
// Source:target pairs of explicitly supplied `--mount` specs. Their sources
// are chosen by the caller (the gh-aw compiler or the user), who asserts the
Expand Down Expand Up @@ -108,12 +119,12 @@ function filterAgentVolumesForSysroot(
// daemon visibility, and a writable `/host$HOME` is required for the
// credential-hiding overlays and the agent entrypoint to work.
if (
source.startsWith(effectiveHome) &&
source.startsWith(prefixedEffectiveHome) &&
target.startsWith(hostHomeMountPrefix) &&
!explicitMountSpecs.has(mountSpecKey(source, target))
) {
const normalizedSource = source.replace(/\/+$/, '') || '/';
const relPath = normalizedSource.slice(effectiveHome.length);
const relPath = normalizedSource.slice(prefixedEffectiveHome.length);
if (relPath.startsWith('/.') || relPath === '') return false;
}

Expand Down
Loading