test: add integration tests for one-shot token LD_PRELOAD library#608
Conversation
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>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. |
There was a problem hiding this comment.
Pull request overview
Adds a new integration test suite to validate the one-shot token LD_PRELOAD library introduced in PR #604, ensuring sensitive environment variables can only be read once (in both container and chroot execution modes) and guarding against regressions.
Changes:
- Added container-mode integration tests validating one-time reads for common token env vars and non-interference with normal env vars.
- Added chroot-mode integration tests validating the same behavior when the agent runs with
--enable-chroot. - Added edge-case tests for empty values, unset vars, and special characters.
Comments suppressed due to low confidence (1)
tests/integration/one-shot-tokens.test.ts:300
- Same quoting issue as the container-mode Python test:
python3 -c '${pythonScript}'cannot safely embed a script that contains single quotes/newlines. This will cause the chroot-mode test to fail; use a quoting-safe way to pass the Python code topython3(heredoc / stdin / proper escaping).
const result = await runner.runWithSudo(
`python3 -c '${pythonScript}'`,
{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
|
|
||
| expect(result).toSucceed(); | ||
| // Empty token should be treated as no token |
There was a problem hiding this comment.
The comment "Empty token should be treated as no token" is misleading given the current one-shot-token implementation: an empty-but-set env var still results in getenv() returning a non-NULL pointer, so it will be treated as a first access and cleared (and will emit the access log). Consider rewording the comment to reflect that the value is empty but the one-shot logic still runs, or assert the intended behavior explicitly.
| // Empty token should be treated as no token | |
| // Empty token value should be returned as empty; one-shot logic still runs and clears it after the first access, | |
| // so both reads appear empty in the script output. |
| `.trim(); | ||
|
|
||
| const result = await runner.runWithSudo( | ||
| `python3 -c '${pythonScript}'`, |
There was a problem hiding this comment.
python3 -c '${pythonScript}' will break because pythonScript contains single quotes (e.g. os.getenv('GITHUB_TOKEN', '')), so the shell will terminate the -c argument early and the test will fail. Pass the Python snippet without conflicting quoting (e.g., use a heredoc, base64, or wrap the -c payload in double-quotes with proper escaping).
This issue also appears on line 298 of the same file.
| `python3 -c '${pythonScript}'`, | |
| `python3 - << 'EOF' | |
| ${pythonScript} | |
| EOF`, |
…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>
PR #604 introduced a one-shot token LD_PRELOAD library that prevents sensitive environment variables from being read multiple times, but lacked integration tests to verify the behavior and prevent regressions.
Tests Added
Container Mode (6 tests)
os.getenv()) and shell access (printenv) both workChroot Mode (5 tests)
/tmp/awf-lib/and loaded via LD_PRELOADEdge Cases (3 tests)
Test Pattern
All tests require
buildLocal: truesince the library is compiled during Docker image build and may not exist in pre-built GHCR images.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.