Add regression test for AWF_HOST_PATH recovery across the sudo secure_path boundary - #8173
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The test does not exercise the real sudo entrypoint boundary claimed by the regression.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds regression tests for host-path and toolchain-variable recovery under simulated sudo environment stripping.
Changes:
- Tests
$GITHUB_PATHprecedence oversecure_path. - Covers
$GITHUB_ENVrecovery and fallback behavior. - Does not yet test the actual
sudo -E awfboundary.
File summaries
| File | Description |
|---|---|
src/services/agent-environment/host-path-recovery.test.ts |
Adds simulated recovery tests; integration coverage through sudo -E with controlled secure_path is still required. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| process.env.PATH = SECURE_PATH; | ||
|
|
||
| const environment: Record<string, string> = {}; | ||
| recoverHostPaths(environment); |
|
@copilot rebase onto main and address review feedback |
…t-sudo-path-fix Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Merged latest Review feedback addressed in e1e58e6: added Verified locally: both tests pass, and the first one fails (exit 127, stub unresolvable) when |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation...
|
|
✅ Build Test Suite completed successfully!
|
|
❌ Smoke Claude failed Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.anthropic.com"See Network Configuration for more information.
|
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
🚀 Security Guard has started processing this pull request |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
✅ Contribution Check completed successfully! PR #8173 follows the contribution guidelines: it adds regression tests for the new behavior, updates test documentation, keeps new files in the correct src/ and tests/ locations, and the PR description clearly explains the change and links the related issue. No contribution-guideline issues found.
|
Smoke Test: Copilot Engine
Overall: PASS cc
|
Smoke Test: Copilot BYOK (Direct) ✅ PASS
Running in direct BYOK mode with
|
github-actions
Bot
added
smoke-copilot-byok
smoke-copilot-network-isolation
|
EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain (api.github.com) reachable — Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
Smoke Test: Services Connectivity
Overall: FAIL —
|
|
✅ PR #8166: Upgrade gh-aw to latest pre-release Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall PASS
|
Chroot Version Comparison Results
Overall: ❌ FAILED — Node.js version mismatch between host and chroot environment.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Notes: All repositories cloned successfully. Java builds required setting
|
📡 OTEL Tracing Smoke Test Results
Overall: All 5 scenarios pass. ✅
|
|
Recent PRs:
MCP Connectivity: ❌ Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: FAIL cc
|
Under
docker-sudo-iptables, AWF is invoked viasudo -E awf, and sudoers'secure_pathcan silently overwrite the runner's$GITHUB_PATH-augmentedPATHbefore AWF ever observesprocess.env.PATH. This was reported causingruby/setup-ruby-selected versions to be shadowed by/usr/bin/rubyinside the agent container.Investigation showed the fix already exists on
main:readGitHubPathEntries()reads$GITHUB_PATHstraight from disk (unaffected bysecure_path), andrecoverHostPaths()merges those entries intoAWF_HOST_PATHahead of the (possibly stripped)PATH, recovering toolchain vars likeGOROOT/JAVA_HOMEfrom$GITHUB_ENVas well. What was missing was a regression test covering this composed behavior end-to-end —recoverHostPaths()was fully mocked in the existing environment-builder tests, leaving only its lower-level primitives unit-tested in isolation.Changes
src/services/agent-environment/host-path-recovery.test.tsexercisingrecoverHostPaths()directly against a simulated sudo boundary:process.env.PATHset to asecure_path-style value (/usr/local/sbin:...:/bin) with a real$GITHUB_PATHfile containing a hosted-toolcache Ruby bin dir — asserts the toolcache path is prepended ahead of/usr/bininAWF_HOST_PATH.getuid() === 0+SUDO_USER) recoveringGOROOTfrom$GITHUB_ENVwhen absent fromprocess.env.$GITHUB_ENVrecovery is skipped outside of a sudo context.AWF_HOST_PATHfalls back to the strippedPATHunchanged when$GITHUB_PATHis unset.No production code changes were required — this closes the test coverage gap called out in the issue's proposed solution and guards against regressions in this path-recovery logic going forward.