fix(cli): fix secure_getenv() bypass of one-shot token protection by Mossaka · Pull Request #1244 · github/gh-aw-firewall

Mossaka · 2026-03-11T18:46:36Z

Summary

Fix critical bug where secure_getenv() in the C one-shot-token library bypassed all token protection when called before getenv()
secure_getenv() was calling get_token_index() before init_token_list() and without the mutex, so the empty token list always returned -1 (not sensitive), passing raw token values through
Added initialization, mutex acquisition, and recursion guard to secure_getenv() matching the correct getenv() implementation

Fixes #756

Test plan

npm run build passes
npm test passes (856 tests)
npm run lint passes
Rust implementation already correct (uses shared handle_getenv_impl)
CI integration tests pass (one-shot-tokens.test.ts)

🤖 Generated with Claude Code

github-actions · 2026-03-11T18:47:37Z

✅ Coverage Check Passed

Overall Coverage

Metric	Base	PR	Delta
Lines	84.23%	84.36%	📈 +0.13%
Statements	84.21%	84.34%	📈 +0.13%
Functions	84.37%	84.37%	➡️ +0.00%
Branches	77.09%	77.17%	📈 +0.08%

📁 Per-file Coverage Changes (1 files)

File	Lines (Before → After)	Statements (Before → After)
`src/docker-manager.ts`	86.8% → 87.3% (+0.52%)	86.1% → 86.6% (+0.50%)

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

Copilot

Pull request overview

This PR fixes a critical security flaw in the agent’s one-shot-token LD_PRELOAD library where secure_getenv() could bypass token protection if invoked before getenv(), ensuring sensitive environment variables are still cached/unset on first access.

Changes:

Add mutex acquisition and init_token_list() initialization to secure_getenv() before calling get_token_index().
Add the same thread-local recursion guard behavior to secure_getenv() that getenv() already uses, preventing re-entrant deadlocks.
Ensure the non-sensitive path releases the mutex and clears the recursion guard before returning.

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

github-actions · 2026-03-11T18:50:59Z

Smoke Test Results

Recent merged PRs:

test: expand credential hiding tests to all 14 protected paths #1163: test: expand credential hiding tests to all 14 protected paths
test(docker): verify capsh execution chain after PR #715 #1240: test(docker): verify capsh execution chain after PR fix: eliminate nested bash layer in chroot command execution for Java/.NET #715

Test	Result
GitHub MCP (list merged PRs)	✅
Playwright (github.com title)	✅
File write	✅
Bash verify	✅

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1244

github-actions · 2026-03-11T18:51:33Z

Smoke Test Results

✅ GitHub MCP — Last 2 merged PRs:

test(docker): verify capsh execution chain after PR #715 #1240: test(docker): verify capsh execution chain after PR #715 by @Mossaka
fix(cli): clear LD_PRELOAD after one-shot-token library loads #1232: fix(cli): clear LD_PRELOAD after one-shot-token library loads by @Mossaka

✅ Playwright — github.com title contains "GitHub"
✅ File Write — /tmp/gh-aw/agent/smoke-test-copilot-22968955590.txt created and verified
✅ Bash — File read back successfully

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1244

github-actions · 2026-03-11T18:51:41Z

🔮 The oracle has spoken through Smoke Codex for issue #1244

github-actions · 2026-03-11T18:52:37Z

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.12	Python 3.12.3	❌ NO
Node.js	v24.14.0	v20.20.0	❌ NO
Go	go1.22.12	go1.22.12	✅ YES

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1244

secure_getenv() was calling get_token_index() before init_token_list() and without the mutex, causing all token protection to be bypassed when secure_getenv() was the first call into the library (empty token list returns -1 for all lookups). Added initialization, mutex acquisition, and recursion guard matching the getenv() implementation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-03-12T23:25:19Z

Smoke Test Results — claude-sonnet-4-6

Test	Result
GitHub MCP: feat(proxy): add GitHub Enterprise Cloud/Server support (#1264)	✅
GitHub MCP: fix(cli): fix secure_getenv() bypass (#1244 current)	✅
Playwright: github.com title contains "GitHub"	✅
File write + read back	✅

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1244

github-actions · 2026-03-12T23:25:28Z

PR Titles:
feat(proxy): add GitHub Enterprise Cloud/Server support with automatic endpoint detection
fix: drop -f from curl to avoid GitHub API rate-limit flakiness
Test 1 GitHub MCP review: ✅
Test 2 safeinputs-gh: ✅
Test 3 Playwright: ✅
Test 4 Tavily search: ❌
Test 5-7 file/bash/discussion: ✅
Test 8 build: ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1244

github-actions · 2026-03-12T23:26:26Z

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.13	Python 3.12.3	❌ NO
Node.js	v24.14.0	v20.20.1	❌ NO
Go	go1.22.12	go1.22.12	✅ YES

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1244

github-actions · 2026-03-12T23:26:56Z

Smoke Test Results — Run 23028546935

Test	Result
GitHub MCP (last 2 merged PRs)	✅ #1267 `fix: drop -f from curl to avoid GitHub API rate-limit flakiness`, #1265 `fix: add missing formatItem and program imports in cli.test.ts`
Playwright (github.com title)	✅ Title contains "GitHub"
File write (`smoke-test-copilot-23028546935.txt`)	✅
Bash verify (`cat`)	✅

Overall: PASS

PR author: @Mossaka · No assignees.

📰 BREAKING: Report filed by Smoke Copilot for issue #1244

Multi-threaded programs like rustc call getenv() from many threads during startup. The previous fix held the mutex for every getenv() call to protect lazy initialization, which serialized all threads and caused rustc to timeout (60s) in CI. Move init_token_list() into the library constructor which runs before any threads are created. This eliminates the data race without needing the mutex for non-sensitive variable lookups. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-03-13T00:24:44Z

Smoke Test Results

Test	Status
GitHub MCP (last 2 merged PRs: #1264, #1245)	✅
Playwright (github.com title contains "GitHub")	✅
File write (`smoke-test-claude-23030235945.txt`)	✅
Bash verify (file content confirmed)	✅

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1244

github-actions · 2026-03-13T00:26:30Z

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.12	Python 3.12.3	❌ NO
Node.js	v24.14.0	v20.20.0	❌ NO
Go	go1.22.12	go1.22.12	✅ YES

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1244

github-actions · 2026-03-13T00:26:35Z

🤖 Smoke test results for @Mossaka:

✅ GitHub MCP: Last 2 merged PRs — fix: drop -f from curl to avoid GitHub API rate-limit flakiness #1267 "fix: drop -f from curl to avoid GitHub API rate-limit flakiness", fix: add missing formatItem and program imports in cli.test.ts #1265 "fix: add missing formatItem and program imports in cli.test.ts"
✅ Playwright: github.com title contains "GitHub"
✅ File write: /tmp/gh-aw/agent/smoke-test-copilot-23030235947.txt created
✅ Bash: file verified via cat

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1244

github-actions · 2026-03-13T00:28:05Z

🏗️ Build Test Suite Results

Ecosystem	Project	Build/Install	Tests	Status
Bun	elysia	✅	1/1 passed	✅ PASS
Bun	hono	✅	1/1 passed	✅ PASS
C++	fmt	✅	N/A	✅ PASS
C++	json	✅	N/A	✅ PASS
Deno	oak	N/A	1/1 passed	✅ PASS
Deno	std	N/A	1/1 passed	✅ PASS
.NET	hello-world	✅	N/A	✅ PASS
.NET	json-parse	✅	N/A	✅ PASS
Go	color	✅	passed	✅ PASS
Go	env	✅	passed	✅ PASS
Go	uuid	✅	passed	✅ PASS
Java	gson	✅	1/1 passed	✅ PASS
Java	caffeine	✅	1/1 passed	✅ PASS
Node.js	clsx	✅	passed	✅ PASS
Node.js	execa	✅	passed	✅ PASS
Node.js	p-limit	✅	passed	✅ PASS
Rust	fd	✅	1/1 passed	✅ PASS
Rust	zoxide	✅	1/1 passed	✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Notes

Java: Maven proxy configured with 172.30.0.10:3128 (direct IP) instead of squid-proxy hostname, which does not resolve in the container's DNS environment.
All other ecosystems ran without issues.

Generated by Build Test Suite for issue #1244 · ◷

github-actions · 2026-03-13T00:40:42Z

PRs reviewed: feat(ci): add weekly performance monitoring workflow; fix(squid): run Squid container as non-root proxy user
GitHub MCP review: ✅
safeinputs-gh pr list: ✅
Playwright title check: ✅
Tavily search: ❌
File write: ✅
Bash cat verify: ✅
Discussion comment: ✅
Build (npm ci && npm run build): ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1244

Copilot AI review requested due to automatic review settings March 11, 2026 18:46

Copilot started reviewing on behalf of Mossaka March 11, 2026 18:47 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

github-actions bot added the smoke-claude label Mar 11, 2026

github-actions bot added the smoke-copilot label Mar 11, 2026

github-actions bot mentioned this pull request Mar 11, 2026

[agentics] No-Op Runs #1086

Closed

Mossaka force-pushed the fix/055-secure-getenv-bypass branch from d780697 to 7cde54a Compare March 12, 2026 23:20

github-actions bot added the build-test label Mar 12, 2026

This comment has been minimized.

Sign in to view

Mossaka enabled auto-merge (squash) March 13, 2026 00:11

github-actions bot mentioned this pull request Mar 13, 2026

[agentics] Smoke Codex failed #1275

Open

github-actions bot mentioned this pull request Mar 13, 2026

[agentics] No-Op Runs #1274

Open

Mossaka merged commit 940b809 into main Mar 13, 2026
65 of 66 checks passed

Mossaka deleted the fix/055-secure-getenv-bypass branch March 13, 2026 00:51

github-actions bot mentioned this pull request Mar 13, 2026

feat(cli): add DNS-over-HTTPS support via --dns-over-https flag #1280

Open

5 tasks

Conversation

Mossaka commented Mar 11, 2026

Summary

Test plan

Uh oh!

github-actions bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Coverage Check Passed

Overall Coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions bot commented Mar 11, 2026

Smoke Test Results

Uh oh!

github-actions bot commented Mar 11, 2026

Smoke Test Results

Uh oh!

github-actions bot commented Mar 11, 2026

Uh oh!

github-actions bot commented Mar 11, 2026

Chroot Version Comparison Results

Uh oh!

github-actions bot commented Mar 12, 2026

Uh oh!

github-actions bot commented Mar 12, 2026

Uh oh!

github-actions bot commented Mar 12, 2026

Chroot Version Comparison Results

Uh oh!

github-actions bot commented Mar 12, 2026

Smoke Test Results — Run 23028546935

Uh oh!

This comment has been minimized.

github-actions bot commented Mar 13, 2026

Smoke Test Results

Uh oh!

github-actions bot commented Mar 13, 2026

Chroot Version Comparison Results

Uh oh!

github-actions bot commented Mar 13, 2026

Uh oh!

github-actions bot commented Mar 13, 2026

🏗️ Build Test Suite Results

Uh oh!

github-actions bot commented Mar 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Mar 11, 2026 •

edited

Loading