fix: add missing secure_getenv interposer initialization#619
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Bun Build Test Results ✅
Overall: PASS All Bun projects built and tested successfully.
|
Deno Build Test Results
Overall: ✅ PASS
|
Smoke Test Results: Claude EngineLast 2 Merged PRs:
Test Results:
Status: PASS
|
Node.js Build Test Results
Overall: PASS ✅ All Node.js projects installed and tested successfully.
|
Smoke Test ResultsLast 2 merged PRs:
✅ GitHub MCP: Retrieved PR data Status: PASS cc
|
Build Test: Go Results
Overall: ✅ PASS All Go projects successfully downloaded dependencies and passed their tests.
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built and tested successfully.
|
C++ Build Test Results
Overall: PASS Both projects built successfully using CMake and make.
|
Chroot Test Results
Overall Status: ❌ Not all versions matched (1/3 passed) The chroot environment did not have matching versions for Python and Node.js, which indicates the host binaries are not fully transparent through the chroot.
|
There was a problem hiding this comment.
Pull request overview
Fixes the agent Docker image build failure by completing the secure_getenv interposer infrastructure in the one-shot-token LD_PRELOAD library, aligning it with the existing getenv initialization pattern.
Changes:
- Add missing
secure_getenvfunction pointer andpthread_once-based initialization helpers. - Add a local
.gitignoreentry to ignore built*.soartifacts in the one-shot-token directory.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| containers/agent/one-shot-token/one-shot-token.c | Adds secure_getenv interposer initialization plumbing to fix compilation and enable runtime symbol resolution. |
| containers/agent/one-shot-token/.gitignore | Ignores local .so build artifacts in this directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static void init_real_secure_getenv(void) { | ||
| pthread_once(&secure_getenv_init_once, init_real_secure_getenv_once); |
There was a problem hiding this comment.
secure_getenv() (below) relies on the token list to classify sensitive variables, but the token list is only initialized in getenv() via init_token_list() under token_mutex. As a result, if a process calls secure_getenv() before any getenv() call, num_tokens is still 0 and get_token_index() will always return -1, bypassing one-shot protection for sensitive tokens. Consider mirroring the getenv() initialization flow in secure_getenv(): lock token_mutex, call init_token_list() if needed, and compute token_idx while holding the mutex before deciding whether to pass through or apply one-shot behavior.
| static void init_real_secure_getenv(void) { | |
| pthread_once(&secure_getenv_init_once, init_real_secure_getenv_once); | |
| static void init_real_secure_getenv(void) { | |
| /* Initialize the real secure_getenv function pointer once */ | |
| pthread_once(&secure_getenv_init_once, init_real_secure_getenv_once); | |
| /* | |
| * Ensure the sensitive token list is initialized before any secure_getenv() | |
| * logic relies on it. This covers the case where secure_getenv() is called | |
| * before getenv(), which would otherwise leave num_tokens == 0 and cause | |
| * get_token_index() to always return -1. | |
| */ | |
| pthread_mutex_lock(&token_mutex); | |
| if (!tokens_initialized) { | |
| init_token_list(); | |
| } | |
| pthread_mutex_unlock(&token_mutex); |
…ss (#604) * feat: add one-shot token LD_PRELOAD library for single-use token access Adds an LD_PRELOAD library that intercepts getenv() calls for sensitive GitHub token environment variables. On first access, returns the real value and immediately unsets the variable, preventing subsequent reads by malicious code. Protected tokens: COPILOT_GITHUB_TOKEN, GITHUB_TOKEN, GH_TOKEN, GITHUB_API_TOKEN, GITHUB_PAT, GH_ACCESS_TOKEN - Add one-shot-token.c with getenv interception logic - Build library in Dockerfile during image build - Enable LD_PRELOAD in entrypoint for both container and chroot modes - Add documentation explaining the mechanism and security properties * fix: improve one-shot token library copy robustness in chroot mode (#606) * Initial plan * fix: improve one-shot token library copy with better error handling Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * Update containers/agent/one-shot-token/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update containers/agent/one-shot-token/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: use pthread_once for thread-safe getenv initialization (#609) * Initial plan * fix: use pthread_once for thread-safe getenv initialization Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * chore: complete thread safety fix Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * test: add integration tests for one-shot token LD_PRELOAD library (#608) * Initial plan * test: add integration tests for one-shot token LD_PRELOAD library Add comprehensive integration tests that verify the one-shot token protection mechanism works correctly in both container and chroot modes. Tests verify: - Protected tokens (GITHUB_TOKEN, COPILOT_GITHUB_TOKEN, OPENAI_API_KEY) can be read once - Subsequent reads return empty/null (token has been cleared) - Non-sensitive environment variables are not affected - Multiple tokens are handled independently - Behavior works with both shell (printenv) and programmatic (Python getenv) access - Edge cases (empty values, nonexistent tokens, special characters) are handled Tests address feedback from PR #604 review requesting integration tests for the one-shot token feature to prevent regressions. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: preserve secure_getenv semantics in one-shot token interposer (#610) * Initial plan * fix: implement proper secure_getenv with dlsym fallback Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: add thread safety to secure_getenv initialization Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Co-authored-by: Landon Cox <landon.cox@microsoft.com> * feat: add runtime configuration for one-shot token protection via AWF_ONE_SHOT_TOKENS (#607) * Initial plan * feat: add runtime configuration for one-shot token list via AWF_ONE_SHOT_TOKENS Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: address code review feedback for one-shot token library - Make initialization thread-safe using existing mutex - Add cleanup of allocated tokens on memory allocation failure - Use isspace() for comprehensive whitespace trimming (handles newlines, tabs, etc.) Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: resolve race condition and buffer underflow in token parsing - Fix potential buffer underflow when trimming empty strings - Fix race condition by keeping mutex held during get_token_index() call - Ensure thread-safe access to token list during lookup Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * refactor: optimize strlen usage and add MAX_TOKENS documentation - Cache strlen result to avoid redundant computation - Add inline comment explaining MAX_TOKENS limit rationale Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * style: improve comment formatting for MAX_TOKENS Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: use strtok_r and fallback to defaults for empty token list - Replace strtok() with strtok_r() to avoid interfering with application code - Fall back to default token list if AWF_ONE_SHOT_TOKENS parses to zero tokens - Add warning messages when falling back due to misconfiguration Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * docs: update README with fallback behavior and strtok_r details Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * refactor: explicitly reset num_tokens in fallback path for clarity Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * docs: clarify defensive programming intent in num_tokens reset Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Co-authored-by: Landon Cox <landon.cox@microsoft.com> * fix: add missing secure_getenv interposer initialization (#619) * Initial plan * fix: add missing secure_getenv declarations and initialization Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * chore: remove build artifacts from git Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * docs: update progress - fix complete Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * chore: remove test files from git Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Docker build fails compiling
one-shot-token.cdue to undeclaredreal_secure_getenvandinit_real_secure_getenv().The secure_getenv interposer implementation was incomplete - it called initialization functions that didn't exist and used an undeclared function pointer.
Changes
Add secure_getenv infrastructure following the getenv pattern:
real_secure_getenvfunction pointersecure_getenv_init_oncepthread_once controlinit_real_secure_getenv_once()for dlsym initializationinit_real_secure_getenv()wrapper for thread-safe initAdd
.gitignorefor*.sobuild artifacts in one-shot-token directoryImplementation
The secure_getenv interposer now mirrors the getenv implementation, using pthread_once for thread-safe lazy initialization.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.