fix: add missing secure_getenv initialization in one-shot-token library#612
fix: add missing secure_getenv initialization in one-shot-token library#612
Conversation
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
) * 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>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* 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>
* 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>
) * 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>
Add missing declarations and initialization functions for real_secure_getenv: - Add real_secure_getenv function pointer declaration - Add secure_getenv_init_once pthread_once control - Add init_real_secure_getenv_once() initialization function - Add init_real_secure_getenv() wrapper function This fixes the compilation error in the Docker build where secure_getenv was being called before its function pointer was initialized. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
The one-shot-token.so file is generated during Docker build and should not be committed to the repository. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
There was a problem hiding this comment.
Pull request overview
Fixes the agent container build failure caused by missing secure_getenv interposer initialization in the one-shot-token LD_PRELOAD library, and adds integration coverage for one-shot token behavior in both container and chroot modes.
Changes:
- Add
secure_getenv“real” function pointer +pthread_onceinitialization viadlsym(RTLD_NEXT, ...), with fallback behavior. - Build and enable the one-shot-token library in the agent container, including chroot-mode copying + cleanup in
entrypoint.sh. - Add Jest integration tests validating one-shot behavior across multiple tokens and execution modes.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/one-shot-tokens.test.ts | Adds integration tests for one-shot token behavior in container and chroot modes |
| src/docker-manager.ts | Updates volume-mount comment to document /tmp usage for one-shot-token in chroot mode |
| containers/agent/one-shot-token/one-shot-token.c | Implements one-shot getenv/secure_getenv interposition with thread-safe init |
| containers/agent/one-shot-token/build.sh | Adds a local build script for the shared library |
| containers/agent/one-shot-token/README.md | Documents the library, usage, integration, and limitations |
| containers/agent/entrypoint.sh | Enables LD_PRELOAD in container mode; copies library into chroot-accessible /tmp and cleans up |
| containers/agent/Dockerfile | Compiles one-shot-token.so during image build and removes build deps afterward |
| _codeql_detected_source_root | Adds CodeQL source root marker |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await cleanup(false); | ||
| runner = createRunner(); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await cleanup(false); |
There was a problem hiding this comment.
cleanup is being called with a single boolean argument, but the provided cleanup definition takes (workDir: string, keepFiles: boolean, proxyLogsDir?). As written, this should be a TypeScript compile-time error (and at runtime would treat false as the workDir). Update these calls to pass the runner/work directory string and the keepFiles boolean (or use the correct helper if there is a different overload intended for tests).
| await cleanup(false); | |
| runner = createRunner(); | |
| }); | |
| afterAll(async () => { | |
| await cleanup(false); | |
| runner = createRunner(); | |
| await cleanup(runner.workDir, false); | |
| }); | |
| afterAll(async () => { | |
| await cleanup(runner.workDir, false); |
| `.trim(); | ||
|
|
||
| const result = await runner.runWithSudo( | ||
| `python3 -c '${pythonScript}'`, |
There was a problem hiding this comment.
The command wraps the Python program in single quotes (python3 -c '...'), but the embedded Python contains single-quoted strings ('GITHUB_TOKEN', ''). This will break shell quoting and cause the test command to fail or run truncated code. Use a safer quoting strategy (e.g., pass -c via an argv array if supported by the runner, escape single quotes, or write the script to a temporary file and execute it).
| `python3 -c '${pythonScript}'`, | |
| `python3 -c "${pythonScript}"`, |
| char *secure_getenv(const char *name) { | ||
| init_real_secure_getenv(); | ||
| init_real_getenv(); | ||
|
|
||
| /* If secure_getenv is not available, fall back to our intercepted getenv */ | ||
| if (real_secure_getenv == NULL) { | ||
| return getenv(name); | ||
| } |
There was a problem hiding this comment.
The comment/docstring states this “preserves secure_getenv semantics (returns NULL in privileged contexts)”, but the fallback to getenv(name) does not preserve that behavior and could expose variables in contexts where secure_getenv would return NULL. If you need semantic parity, consider implementing an AT_SECURE check (e.g., via getauxval(AT_SECURE) when available) and returning NULL when secure, or adjust the documentation to explicitly state that secure semantics are not preserved when the real secure_getenv symbol is unavailable.
| static void init_real_getenv_once(void) { | ||
| real_getenv = dlsym(RTLD_NEXT, "getenv"); | ||
| if (real_getenv == NULL) { | ||
| fprintf(stderr, "[one-shot-token] FATAL: Could not find real getenv: %s\n", dlerror()); | ||
| /* Cannot recover - abort to prevent undefined behavior */ | ||
| abort(); | ||
| } | ||
| } |
There was a problem hiding this comment.
dlsym() failure detection is more robust when paired with dlerror() usage correctly (clear any existing error with dlerror() before dlsym, then call dlerror() after). Checking real_getenv == NULL alone can be misleading per dlsym’s contract. Using the standard dlerror pattern will avoid false positives/negatives and make diagnostics more accurate.
| if [ -f /usr/local/lib/one-shot-token.so ]; then | ||
| # Create the library directory in /tmp (always writable) | ||
| if mkdir -p /host/tmp/awf-lib 2>/dev/null; then | ||
| # Copy the library and verify it exists after copying | ||
| if cp /usr/local/lib/one-shot-token.so /host/tmp/awf-lib/one-shot-token.so 2>/dev/null && \ | ||
| [ -f /host/tmp/awf-lib/one-shot-token.so ]; then | ||
| ONE_SHOT_TOKEN_LIB="/tmp/awf-lib/one-shot-token.so" | ||
| echo "[entrypoint] One-shot token library copied to chroot at ${ONE_SHOT_TOKEN_LIB}" | ||
| else | ||
| echo "[entrypoint][WARN] Could not copy one-shot-token library to /tmp/awf-lib" | ||
| echo "[entrypoint][WARN] Token protection will be disabled (tokens may be readable multiple times)" | ||
| fi | ||
| else | ||
| echo "[entrypoint][ERROR] Could not create /tmp/awf-lib directory" |
There was a problem hiding this comment.
Using a fixed /tmp/awf-lib path (and rm -rf on exit) can collide across concurrent runs on the same host and risks deleting another run’s library directory mid-execution. Prefer creating a per-run unique directory (e.g., include $$/timestamp/UUID in the path), store that exact path in ONE_SHOT_TOKEN_LIB, and only remove that unique directory during cleanup.
| if [ -f /usr/local/lib/one-shot-token.so ]; then | |
| # Create the library directory in /tmp (always writable) | |
| if mkdir -p /host/tmp/awf-lib 2>/dev/null; then | |
| # Copy the library and verify it exists after copying | |
| if cp /usr/local/lib/one-shot-token.so /host/tmp/awf-lib/one-shot-token.so 2>/dev/null && \ | |
| [ -f /host/tmp/awf-lib/one-shot-token.so ]; then | |
| ONE_SHOT_TOKEN_LIB="/tmp/awf-lib/one-shot-token.so" | |
| echo "[entrypoint] One-shot token library copied to chroot at ${ONE_SHOT_TOKEN_LIB}" | |
| else | |
| echo "[entrypoint][WARN] Could not copy one-shot-token library to /tmp/awf-lib" | |
| echo "[entrypoint][WARN] Token protection will be disabled (tokens may be readable multiple times)" | |
| fi | |
| else | |
| echo "[entrypoint][ERROR] Could not create /tmp/awf-lib directory" | |
| ONE_SHOT_TOKEN_LIB_DIR="" | |
| if [ -f /usr/local/lib/one-shot-token.so ]; then | |
| # Create a per-run unique library directory in /tmp (always writable) | |
| ONE_SHOT_TOKEN_LIB_DIR_HOST="$(mktemp -d /host/tmp/awf-lib.XXXXXX 2>/dev/null || true)" | |
| if [ -n "${ONE_SHOT_TOKEN_LIB_DIR_HOST}" ] && [ -d "${ONE_SHOT_TOKEN_LIB_DIR_HOST}" ]; then | |
| # Copy the library and verify it exists after copying | |
| if cp /usr/local/lib/one-shot-token.so "${ONE_SHOT_TOKEN_LIB_DIR_HOST}/one-shot-token.so" 2>/dev/null && \ | |
| [ -f "${ONE_SHOT_TOKEN_LIB_DIR_HOST}/one-shot-token.so" ]; then | |
| # Derive the in-chroot path from the host directory basename | |
| _awf_lib_basename="$(basename "${ONE_SHOT_TOKEN_LIB_DIR_HOST}")" | |
| ONE_SHOT_TOKEN_LIB_DIR="/tmp/${_awf_lib_basename}" | |
| ONE_SHOT_TOKEN_LIB="${ONE_SHOT_TOKEN_LIB_DIR}/one-shot-token.so" | |
| echo "[entrypoint] One-shot token library copied to chroot at ${ONE_SHOT_TOKEN_LIB}" | |
| else | |
| echo "[entrypoint][WARN] Could not copy one-shot-token library to ${ONE_SHOT_TOKEN_LIB_DIR_HOST}" | |
| echo "[entrypoint][WARN] Token protection will be disabled (tokens may be readable multiple times)" | |
| fi | |
| else | |
| echo "[entrypoint][ERROR] Could not create per-run /tmp/awf-lib.* directory" |
| fi | ||
| # Clean up the one-shot-token library if it was copied | ||
| if [ -n "${ONE_SHOT_TOKEN_LIB}" ]; then | ||
| CLEANUP_CMD="${CLEANUP_CMD}; rm -rf /tmp/awf-lib 2>/dev/null || true" |
There was a problem hiding this comment.
Using a fixed /tmp/awf-lib path (and rm -rf on exit) can collide across concurrent runs on the same host and risks deleting another run’s library directory mid-execution. Prefer creating a per-run unique directory (e.g., include $$/timestamp/UUID in the path), store that exact path in ONE_SHOT_TOKEN_LIB, and only remove that unique directory during cleanup.
| CLEANUP_CMD="${CLEANUP_CMD}; rm -rf /tmp/awf-lib 2>/dev/null || true" | |
| ONE_SHOT_TOKEN_LIB_DIR="${ONE_SHOT_TOKEN_LIB%/*}" | |
| if [ -n "${ONE_SHOT_TOKEN_LIB_DIR}" ] && [ "${ONE_SHOT_TOKEN_LIB_DIR}" != "${ONE_SHOT_TOKEN_LIB}" ]; then | |
| CLEANUP_CMD="${CLEANUP_CMD}; rm -rf '${ONE_SHOT_TOKEN_LIB_DIR}' 2>/dev/null || true" | |
| fi |
Build Test: Deno - PASS ✅
Overall: PASS ✅ All Deno tests completed successfully.
|
Build Test: Node.js - Results
Overall: PASS ✅ All projects installed and tested successfully.
|
Build Test: Bun ✅All Bun projects tested successfully.
Overall: PASS
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully through the AWF firewall.
|
Rust Build Test Results
Overall: PASS All Rust projects built successfully and passed their test suites.
|
Smoke Test Results ✅Last 2 Merged PRs:
Test Results:
Status: PASS cc @lpcox
|
|
Claude Smoke Test Results
✅ GitHub MCP - Fetched last 2 merged PRs Overall: PASS
|
Go Build Test Results
Overall: PASS ✅ All Go projects built successfully and passed their test suites.
|
Chroot Test Results
Overall Result: Some tests failed - chroot mode is using different versions for Python and Node.js than the host system.
|
|
Closing this PR — all the fixes here (secure_getenv initialization, pthread_once, secure_getenv interposer) are already on main via PR #604. Additionally, merging this branch would introduce regressions:
|
Docker build for agent container fails during compilation of
one-shot-token.cwith implicit function declaration errors forreal_secure_getenv. Thesecure_getenvinterposer added in PR #610 is missing its initialization infrastructure.Changes
real_secure_getenvfunction pointer declarationsecure_getenv_init_oncepthread_once control for thread-safe initializationinit_real_secure_getenv_once()to resolve symbol viadlsym(RTLD_NEXT, "secure_getenv")init_real_secure_getenv()wrapper forpthread_onceinitializationPattern matches existing
real_getenvinitialization. Gracefully handles systems withoutsecure_getenvby falling back togetenv.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.