fix: correctly recover runner tool on PATH (after sudo w/ secure_path). remove incorrect reading from GITHUB_PATH by zarenner · Pull Request #5144 · github/gh-aw-firewall

zarenner · 2026-06-17T02:59:22Z

Summary

Recover runner toolcache bin directories into the chroot PATH used by the agent container when AWF runs under sudo or when the runner tool cache contains needed binaries. This avoids preflight failures (e.g., missing node) caused by sudo sanitizing PATH on self-hosted/DinD runners.

Root cause

When sudo is used inside some self-hosted runner environments, secure_path in /etc/sudoers may remove the runner's tool-cache bin directories from PATH. AWF's previous attempt to read per-step GITHUB_PATH was incorrect (it's per-step and cannot reliably recover prior step changes), which led to brittle behavior and confusing documentation.

What this change does

Adds discovery of RUNNER_TOOL_CACHE bin directories and merges them ahead of the existing PATH into AWF_HOST_PATH so chrooted agent processes can find tool binaries.
Removes the previous, incorrect attempt to read GITHUB_PATH for PATH recovery.
Restores toolchain env var recovery from GITHUB_ENV when AWF is running under sudo (unchanged behavior, but clarified).
Adds unit tests for edge cases around RUNNER_TOOL_CACHE discovery.

Testing

Unit tests added/updated to cover:
- discovery of nested toolcache/*/<version>/<arch>/bin directories
- behavior when RUNNER_TOOL_CACHE is not set
- behavior when RUNNER_TOOL_CACHE points to non-directories or contains non-directory entries
- recovery of toolchain vars from GITHUB_ENV when running as root with SUDO_UID set
Ran full test suite locally: npm test -- --coverage — all tests passed.

Risk and rollout

Low-risk. Changes are localized to host-path recovery logic and tests. If a regression is seen, revert the commit that modifies src/services/agent-environment/host-path-recovery.ts.

Notes for reviewers

Focus on the discoverRunnerToolCacheBinDirs logic and the test cases that exercise non-directory entries and permission-safe reads.
This change intentionally removes the incorrect GITHUB_PATH merging; see docs update in the branch for rationale.

See draft PR fix: allow node preflight to use explicit binary #5145 for a complementary, separate change that makes the container entrypoint preflight honor an explicit GH_AW_NODE_BIN when set.

github-actions · 2026-06-17T03:15:50Z

Documentation Preview

Documentation build failed for this PR. View logs.

Built from commit 1374fa0

github-actions · 2026-06-17T03:22:17Z

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric	Base	PR	Delta
Lines	97.30%	97.29%	➡️ -0.01%
Statements	97.16%	97.11%	📉 -0.05%
Functions	98.84%	98.84%	➡️ +0.00%
Branches	91.92%	91.88%	📉 -0.04%

📁 Per-file Coverage Changes (2 files)

File	Lines (Before → After)	Statements (Before → After)
`src/services/agent-environment/host-path-recovery.ts`	100.0% → 93.0% (-6.98%)	100.0% → 88.9% (-11.12%)
`src/workdir-setup.ts`	92.7% → 94.5% (+1.82%)	92.7% → 94.5% (+1.82%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

…verage

github-actions · 2026-06-17T03:49:17Z

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric	Base	PR	Delta
Lines	97.30%	97.58%	📈 +0.28%
Statements	97.16%	97.51%	📈 +0.35%
Functions	98.84%	98.68%	📉 -0.16%
Branches	91.92%	92.99%	📈 +1.07%

📁 Per-file Coverage Changes (5 files)

File	Lines (Before → After)	Statements (Before → After)
`src/services/agent-environment/host-path-recovery.ts`	100.0% → 96.4% (-3.58%)	100.0% → 96.7% (-3.34%)
`src/workdir-setup.ts`	92.7% → 94.5% (+1.82%)	92.7% → 94.5% (+1.82%)
`src/logs/audit-enricher.ts`	89.4% → 95.7% (+6.38%)	83.6% → 95.1% (+11.48%)
`src/services/agent-volumes/docker-host-staging.ts`	87.2% → 95.7% (+8.51%)	87.8% → 95.9% (+8.16%)
`src/commands/validators/log-and-limits.ts`	90.3% → 100.0% (+9.68%)	90.3% → 100.0% (+9.68%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

zarenner · 2026-06-17T03:50:44Z

this was wrong - GITHUB_PATH is meant to be written to (to then have the runner automatically add it to PATH in subsequent steps). It is not meant to be read from, and will be empty again on each new step.

Copilot

Pull request overview

This PR updates AWF’s GitHub Actions path-recovery behavior so that tools installed into the runner tool cache remain discoverable inside the chroot when AWF is invoked under sudo (e.g., when secure_path sanitizes PATH). It removes the previous (incorrect) attempt to recover paths by reading $GITHUB_PATH, and instead derives AWF_HOST_PATH from the inherited PATH plus bin directories discovered under RUNNER_TOOL_CACHE.

Changes:

Build AWF_HOST_PATH by prepending discovered RUNNER_TOOL_CACHE/**/bin directories ahead of the current PATH, and stop reading $GITHUB_PATH.
Update/replace unit tests to cover RUNNER_TOOL_CACHE discovery and to align with the new PATH-building logic.
Update documentation to describe the new recovery mechanism and explicitly note that $GITHUB_PATH is not used.

Show a summary per file

File	Description
tests/chroot-path-ordering.test.sh	Updates fallback-path ordering assertions and adds coverage for self-hosted runner toolcache scanning.
src/services/agent-service.test.ts	Reworks PATH-recovery unit tests to validate `RUNNER_TOOL_CACHE`-based merging behavior.
src/services/agent-environment/host-path-recovery.ts	Implements `RUNNER_TOOL_CACHE` bin-dir discovery and merges into `AWF_HOST_PATH`; continues sudo `$GITHUB_ENV` recovery for toolchain vars.
src/github-env.ts	Removes `$GITHUB_PATH` helpers and generalizes path merging helper as `prependPathEntries`.
src/github-env.test.ts	Removes `$GITHUB_PATH` tests and renames/adjusts tests for `prependPathEntries`.
src/docker-manager-utils.test.ts	Drops tests tied to removed `$GITHUB_PATH` helpers.
docs/environment.md	Updates GitHub Actions tool-availability docs to describe `PATH` + `RUNNER_TOOL_CACHE` recovery (and not `$GITHUB_PATH`).
containers/agent/entrypoint.sh	Removes `$GITHUB_PATH` ingestion in the fallback PATH construction branch.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 8/8 changed files
Comments generated: 5

+    const runnerToolCacheBinDirs = discoverRunnerToolCacheBinDirs(process.env.RUNNER_TOOL_CACHE);
+    environment.AWF_HOST_PATH = prependPathEntries(process.env.PATH, runnerToolCacheBinDirs);
+    if (runnerToolCacheBinDirs.length > 0) {
+      logger.debug(`Merged ${runnerToolCacheBinDirs.length} runner tool cache bin path(s) into AWF_HOST_PATH`);
    }


+      // create a file instead of a directory
+      const tmpFile = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-tool-cache-file-')) + '.file';
+      fs.writeFileSync(tmpFile, 'not a dir');
+      process.env.RUNNER_TOOL_CACHE = tmpFile;


  # Run the script in a sub-shell with a clean environment
  (
    export PATH="${base_path}"
-    [ -n "${github_path_file}" ] && export GITHUB_PATH="${github_path_file}"
    eval "${script}"
    eval "${check_expr}"
  )


github-actions · 2026-06-17T16:22:42Z

✅ Copilot review passed with no inline comments.

@zarenner Add the ready-for-aw label to this PR to trigger agentic CI smoke tests.

…etection

…hEntries

…ystem tools

Recover runner tool cache paths in chroot

112f5cd

zarenner mentioned this pull request Jun 17, 2026

fix: allow node preflight to use explicit binary #5145

Merged

Clarify GitHub path recovery behavior

187a137

Remove GITHUB_PATH path recovery

ff7b720

tests: cover edge cases for RUNNER_TOOL_CACHE discovery to restore co…

0199276

…verage

zarenner changed the title ~~Recover runner tool cache paths in chroot~~ Recover runner tool cache paths in chroot (fix PATH preflight failures) Jun 17, 2026

zarenner commented Jun 17, 2026

View reviewed changes

zarenner changed the title ~~Recover runner tool cache paths in chroot (fix PATH preflight failures)~~ fix: correctly recover runner tool on PATH (after sudo w/ secure_path). remove incorrect reading from GITHUB_PATH Jun 17, 2026

fix: suppress eslint warnings for empty catch block in test

1ef400b

zarenner marked this pull request as ready for review June 17, 2026 16:01

Copilot AI review requested due to automatic review settings June 17, 2026 16:01

Copilot started reviewing on behalf of zarenner June 17, 2026 16:01 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

fix: address PR review feedback for toolcache recovery and tests

7920163

zarenner marked this pull request as draft June 17, 2026 18:50

github-actions Bot added 5 commits June 17, 2026 11:54

fix: replace hardcoded TOOL_PROBE_BINARIES with data-driven bin-dir d…

4c36621

…etection

fix: remove anyBinAlreadyOnPath — dedup already handled by prependPat…

21fe575

…hEntries

fix: simplify binDirsByTool map to store one entry per tool

0f9db9c

tests: add edge-case coverage for discoverRunnerToolCacheBinDirs

9e50eb3

fix: restore binary-level dedup to prevent toolcache dirs shadowing s…

32fbfea

…ystem tools

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correctly recover runner tool on PATH (after sudo w/ secure_path). remove incorrect reading from GITHUB_PATH#5144

fix: correctly recover runner tool on PATH (after sudo w/ secure_path). remove incorrect reading from GITHUB_PATH#5144
zarenner wants to merge 11 commits into
mainfrom
zr/recover-runner-tool-cache-path

zarenner commented Jun 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

zarenner Jun 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zarenner commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation Preview

Uh oh!

github-actions Bot commented Jun 17, 2026

⚠️ Coverage Regression Detected

Overall Coverage

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Coverage Regression Detected

Overall Coverage

Uh oh!

zarenner Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zarenner commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading

zarenner Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading