Add regression test for AWF_HOST_PATH recovery across the sudo secure_path boundary

.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

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<string, string> = {};
+    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<string, string> = {};
+    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<string, string> = {};
+    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<string, string> = {};
+    recoverHostPaths(environment);
+
+    expect(environment.AWF_HOST_PATH).toBe(SECURE_PATH);
+  });
+});

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 |

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/<tool>` 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=<secure_path> ...` so the stripped PATH is what the CLI
+ * process actually starts with, then asserts on the PATH observed inside the
+ * agent container.
+ */
+
+/// <reference path="../jest-custom-matchers.d.ts" />
+
+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<execa.ExecaReturnValue<string>> {
+    // 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);
+});