Skip to content

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

Draft
zarenner wants to merge 5 commits into
mainfrom
zr/recover-runner-tool-cache-path
Draft

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

Conversation

@zarenner

@zarenner zarenner commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

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.

Related

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Documentation Preview

Documentation build failed for this PR. View logs.

Built from commit 00640f8

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ 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

@zarenner 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
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 97.30% 97.31% 📈 +0.01%
Statements 97.16% 97.17% 📈 +0.01%
Functions 98.84% 98.84% ➡️ +0.00%
Branches 91.92% 91.98% 📈 +0.06%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/services/agent-environment/host-path-recovery.ts 100.0% → 95.3% (-4.66%) 100.0% → 95.5% (-4.45%)
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

@zarenner zarenner Jun 17, 2026

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.

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.

@zarenner 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant