From b13a419d9c7c83bf0b1b82881e0dc7a59cc6e0f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:38:54 +0000 Subject: [PATCH 1/3] Initial plan From d66c1aac8e0cfb41588b0b2dd91fb70674428a21 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:45:28 +0000 Subject: [PATCH 2/3] test: cover sudo secure_path PATH recovery boundary --- .../host-path-recovery.test.ts | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 src/services/agent-environment/host-path-recovery.test.ts diff --git a/src/services/agent-environment/host-path-recovery.test.ts b/src/services/agent-environment/host-path-recovery.test.ts new file mode 100644 index 000000000..379c61a16 --- /dev/null +++ b/src/services/agent-environment/host-path-recovery.test.ts @@ -0,0 +1,117 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { recoverHostPaths } from './host-path-recovery'; + +// Mock the logger to keep test output clean and allow assertions if needed. +jest.mock('../../logger', () => ({ + logger: { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }, +})); + +/** + * Regression tests for the `sudo -E awf` boundary described in + * github/gh-aw-firewall#8141: sudoers `secure_path` can silently replace the + * runner's $GITHUB_PATH-augmented PATH with a fixed value (typically + * "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") before AWF + * ever sees process.env.PATH. These tests simulate that exact boundary by + * setting process.env.PATH to a "secure_path"-style value while pointing + * $GITHUB_PATH / $GITHUB_ENV at real files containing entries a setup-* + * action (e.g. ruby/setup-ruby) would have written *before* sudo stripped + * them from PATH. + */ +describe('recoverHostPaths (sudo secure_path boundary)', () => { + const originalEnv = process.env; + const originalGetuid = process.getuid; + let tmpDir: string; + + beforeEach(() => { + process.env = { ...originalEnv }; + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-host-path-recovery-')); + }); + + afterEach(() => { + process.env = originalEnv; + if (originalGetuid) { + Object.defineProperty(process, 'getuid', { value: originalGetuid, configurable: true }); + } + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + const SECURE_PATH = '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'; + + it('prepends the hosted-toolcache Ruby bin dir ahead of /usr/bin even when secure_path stripped it from PATH', () => { + const rubyBin = '/opt/hostedtoolcache/Ruby/3.4.8/x64/bin'; + + // Simulate ruby/setup-ruby having called core.addPath() *before* sudo ran: + // the $GITHUB_PATH file retains the entry regardless of what sudo does to PATH. + const githubPathFile = path.join(tmpDir, 'github_path'); + fs.writeFileSync(githubPathFile, `${rubyBin}\n`); + process.env.GITHUB_PATH = githubPathFile; + + // Simulate the sudo secure_path boundary: process.env.PATH as observed by + // AWF (running as root under `sudo -E`) is the sudoers-fixed value, with + // no trace of the runner's setup-ruby PATH prepend. + process.env.PATH = SECURE_PATH; + + const environment: Record = {}; + recoverHostPaths(environment); + + expect(environment.AWF_HOST_PATH).toBeDefined(); + const entries = environment.AWF_HOST_PATH.split(':'); + const rubyIdx = entries.indexOf(rubyBin); + const usrBinIdx = entries.indexOf('/usr/bin'); + + expect(rubyIdx).toBeGreaterThanOrEqual(0); + expect(usrBinIdx).toBeGreaterThanOrEqual(0); + expect(rubyIdx).toBeLessThan(usrBinIdx); + }); + + it('recovers toolchain env vars (e.g. GOROOT) from $GITHUB_ENV when sudo stripped them from process.env', () => { + Object.defineProperty(process, 'getuid', { value: () => 0, configurable: true }); + process.env.SUDO_USER = 'runner'; + delete process.env.SUDO_UID; + delete process.env.GOROOT; + + const githubEnvFile = path.join(tmpDir, 'github_env'); + fs.writeFileSync(githubEnvFile, 'GOROOT=/opt/hostedtoolcache/go/1.22.0/x64\n'); + process.env.GITHUB_ENV = githubEnvFile; + process.env.PATH = SECURE_PATH; + + const environment: Record = {}; + recoverHostPaths(environment); + + expect(environment.AWF_GOROOT).toBe('/opt/hostedtoolcache/go/1.22.0/x64'); + }); + + it('does not attempt $GITHUB_ENV recovery when not running under sudo (no SUDO_UID/SUDO_USER)', () => { + Object.defineProperty(process, 'getuid', { value: () => 0, configurable: true }); + delete process.env.SUDO_UID; + delete process.env.SUDO_USER; + delete process.env.GOROOT; + + const githubEnvFile = path.join(tmpDir, 'github_env'); + fs.writeFileSync(githubEnvFile, 'GOROOT=/opt/hostedtoolcache/go/1.22.0/x64\n'); + process.env.GITHUB_ENV = githubEnvFile; + process.env.PATH = SECURE_PATH; + + const environment: Record = {}; + recoverHostPaths(environment); + + expect(environment.AWF_GOROOT).toBeUndefined(); + }); + + it('falls back to the (already stripped) PATH unmodified when $GITHUB_PATH is not set', () => { + delete process.env.GITHUB_PATH; + process.env.PATH = SECURE_PATH; + + const environment: Record = {}; + recoverHostPaths(environment); + + expect(environment.AWF_HOST_PATH).toBe(SECURE_PATH); + }); +}); From e1e58e6846b4989d35aed816ce4fb2291763dd48 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:08:35 +0000 Subject: [PATCH 3/3] test: cover sudo secure_path PATH recovery end-to-end --- .github/workflows/test-integration-suite.yml | 2 +- tests/README.md | 2 + tests/integration/sudo-secure-path.test.ts | 155 +++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 tests/integration/sudo-secure-path.test.ts diff --git a/.github/workflows/test-integration-suite.yml b/.github/workflows/test-integration-suite.yml index 45b3706ce..2700e718c 100644 --- a/.github/workflows/test-integration-suite.yml +++ b/.github/workflows/test-integration-suite.yml @@ -274,7 +274,7 @@ jobs: run: | echo "=== Running container & ops tests ===" npm run test:integration -- \ - --testPathPatterns="(container-workdir|environment-variables|error-handling|exit-code-propagation|filesystem-allowwrite|log-commands|no-docker|volume-mounts|skip-pull)" \ + --testPathPatterns="(container-workdir|environment-variables|error-handling|exit-code-propagation|filesystem-allowwrite|log-commands|no-docker|volume-mounts|skip-pull|sudo-secure-path)" \ --verbose env: JEST_TIMEOUT: 180000 diff --git a/tests/README.md b/tests/README.md index c2db9b9b8..fba039de1 100644 --- a/tests/README.md +++ b/tests/README.md @@ -23,6 +23,7 @@ This directory contains comprehensive integration tests that verify firewall beh - **DNS Servers** (`dns-servers.test.ts`) - DNS server configuration and resolution - **Environment Variables** (`environment-variables.test.ts`) - Environment variable passing - **Volume Mounts** (`volume-mounts.test.ts`) - Volume mount configuration +- **Sudo secure_path** (`sudo-secure-path.test.ts`) - $GITHUB_PATH recovery across the `sudo -E awf` boundary ### Protocol & Network - **Protocol Support** (`protocol-support.test.ts`) - HTTP/HTTPS, HTTP/2, IPv4/IPv6 @@ -291,6 +292,7 @@ The project uses TypeScript-based integration tests that run in CI via `.github/ | Config | `dns-servers.test.ts` | DNS configuration | | Config | `environment-variables.test.ts` | Environment variables | | Config | `volume-mounts.test.ts` | Volume mounts | +| Config | `sudo-secure-path.test.ts` | $GITHUB_PATH recovery under sudo `secure_path` | | Protocol | `protocol-support.test.ts` | HTTP/HTTPS, HTTP/2 | | Protocol | `git-operations.test.ts` | Git over HTTPS | | Errors | `error-handling.test.ts` | Error scenarios | diff --git a/tests/integration/sudo-secure-path.test.ts b/tests/integration/sudo-secure-path.test.ts new file mode 100644 index 000000000..72caa6501 --- /dev/null +++ b/tests/integration/sudo-secure-path.test.ts @@ -0,0 +1,155 @@ +/** + * Sudo `secure_path` Boundary Tests + * + * These tests exercise the real `sudo -E awf` entrypoint used by the + * docker-sudo-iptables setup. sudoers' `secure_path` replaces the runner's + * $GITHUB_PATH-augmented PATH with a fixed value before AWF ever observes + * `process.env.PATH`, which previously let `/usr/bin/` shadow the + * version selected by a setup-* action (e.g. ruby/setup-ruby). + * + * Rather than mocking the boundary, each test launches the built CLI through + * `sudo -E env PATH= ...` so the stripped PATH is what the CLI + * process actually starts with, then asserts on the PATH observed inside the + * agent container. + */ + +/// + +import { describe, test, expect, beforeAll, afterAll } from '@jest/globals'; +import execa = require('execa'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { cleanup } from '../fixtures/cleanup'; + +// The value most sudoers files ship as `Defaults secure_path`. +const SECURE_PATH = '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'; + +const AWF_PATH = path.resolve(__dirname, '../../dist/cli.js'); +const STUB_NAME = 'awf-secure-path-probe'; +const STUB_MARKER = 'AWF_SECURE_PATH_STUB_OK'; + +describe('sudo secure_path boundary', () => { + let fixtureDir: string; + let stubBinDir: string; + let githubPathFile: string; + + beforeAll(async () => { + await cleanup(false); + + // /tmp is bind-mounted read-write into the agent container, so a stub + // placed here stands in for a hosted-toolcache bin directory. + fixtureDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-secure-path-')); + stubBinDir = path.join(fixtureDir, 'toolcache', 'bin'); + fs.mkdirSync(stubBinDir, { recursive: true }); + + const stub = path.join(stubBinDir, STUB_NAME); + fs.writeFileSync(stub, `#!/bin/sh\necho "${STUB_MARKER}"\n`); + fs.chmodSync(stub, 0o755); + // mkdtemp creates 0700; the agent runs as the mapped host user. + fs.chmodSync(fixtureDir, 0o755); + fs.chmodSync(path.dirname(stubBinDir), 0o755); + fs.chmodSync(stubBinDir, 0o755); + + // Simulates a setup-* action having called core.addPath() before sudo ran. + githubPathFile = path.join(fixtureDir, 'github_path'); + fs.writeFileSync(githubPathFile, `${stubBinDir}\n`); + fs.chmodSync(githubPathFile, 0o644); + }); + + afterAll(async () => { + await cleanup(false); + fs.rmSync(fixtureDir, { recursive: true, force: true }); + }); + + /** + * Run the CLI through sudo with a hard-coded `secure_path`-style PATH. + * + * `sudo -E env PATH=...` reproduces the sudoers behaviour deterministically: + * whatever the host PATH was, the AWF process starts with only the fixed + * secure_path entries. + */ + async function runUnderSecurePath( + command: string, + githubPath: string | undefined, + ): Promise> { + // env options must precede NAME=VALUE assignments. + const envArgs: string[] = githubPath ? [] : ['-u', 'GITHUB_PATH']; + envArgs.push(`PATH=${SECURE_PATH}`); + if (githubPath) { + envArgs.push(`GITHUB_PATH=${githubPath}`); + } + + return execa( + 'sudo', + [ + '-E', + 'env', + ...envArgs, + // Absolute node path: the stripped PATH may not resolve `node`. + process.execPath, + AWF_PATH, + '--legacy-security', + '--allow-domains', + 'github.com', + '--log-level', + 'debug', + '--', + command, + ], + { + reject: false, + all: true, + timeout: 180000, + }, + ); + } + + /** + * The entrypoint echoes the command line before running it, so the literal + * `AWF_PROBED_PATH=$PATH` text appears in stdout too. Keep only lines where + * the marker was actually expanded to a PATH value. + */ + function extractProbedPath(stdout: string): string { + const values = stdout + .split('\n') + .map(line => /AWF_PROBED_PATH=(.*)/.exec(line)) + .filter((match): match is RegExpExecArray => match !== null) + .map(match => match[1].trim()) + .filter(value => value.startsWith('/')); + + expect(values.length).toBeGreaterThan(0); + return values[values.length - 1]; + } + + test('recovers $GITHUB_PATH entries ahead of /usr/bin despite secure_path', async () => { + const result = await runUnderSecurePath( + `bash -c 'echo AWF_PROBED_PATH=$PATH; ${STUB_NAME}'`, + githubPathFile, + ); + + expect(result.exitCode).toBe(0); + // The stub is only reachable if the $GITHUB_PATH entry survived the + // sudo boundary and was merged into the agent's PATH. + expect(result.stdout).toContain(STUB_MARKER); + + const entries = extractProbedPath(result.stdout).split(':'); + const stubIdx = entries.indexOf(stubBinDir); + const usrBinIdx = entries.indexOf('/usr/bin'); + + expect(stubIdx).toBeGreaterThanOrEqual(0); + expect(usrBinIdx).toBeGreaterThanOrEqual(0); + expect(stubIdx).toBeLessThan(usrBinIdx); + }, 240000); + + test('does not add the toolcache dir when $GITHUB_PATH is unset', async () => { + const result = await runUnderSecurePath( + `bash -c 'echo AWF_PROBED_PATH=$PATH; command -v ${STUB_NAME} || echo AWF_STUB_NOT_FOUND'`, + undefined, + ); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('AWF_STUB_NOT_FOUND'); + expect(extractProbedPath(result.stdout).split(':')).not.toContain(stubBinDir); + }, 240000); +});