feat: add runtime configuration for one-shot token protection via AWF_ONE_SHOT_TOKENS#607
Conversation
…HOT_TOKENS Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
- 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 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>
There was a problem hiding this comment.
Pull request overview
Adds runtime configurability to the one-shot token LD_PRELOAD interposer so the set of protected environment variables can be provided via AWF_ONE_SHOT_TOKENS, with updated documentation for defaults and custom configuration.
Changes:
- Introduces
AWF_ONE_SHOT_TOKENSparsing to build a runtime-sensitive token list (falling back to built-in defaults). - Updates token tracking to support a configurable list (up to 100 tokens).
- Updates README with configuration, examples, and expected output for default/custom token scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| containers/agent/one-shot-token/one-shot-token.c | Adds runtime token list initialization from AWF_ONE_SHOT_TOKENS and updates lookup/tracking to use the configured list. |
| containers/agent/one-shot-token/README.md | Documents the new configuration mechanism, defaults, and testing/integration examples. |
Comments suppressed due to low confidence (3)
containers/agent/one-shot-token/one-shot-token.c:166
- The initialization guard in getenv() (
static int initializing) isn’t synchronized, so multiple threads can still enter initialization concurrently and/or observe inconsistent state. If you switch topthread_once(or lock around init), this ad-hoc guard can be removed or made thread-safe.
This issue also appears on line 78 of the same file.
/**
* Intercepted getenv function
*
* For sensitive tokens:
* - First call: returns the real value, then unsets the variable
* - Subsequent calls: returns NULL
containers/agent/one-shot-token/one-shot-token.c:102
- The whitespace-trimming logic can invoke undefined behavior for empty tokens (e.g., consecutive commas in AWF_ONE_SHOT_TOKENS). When strlen(token) is 0,
token + strlen(token) - 1produces an invalid pointer. Handle the zero-length case before computing theendpointer (and skip empty entries safely).
}
char *token = strtok(config_copy, ",");
while (token != NULL && num_tokens < MAX_TOKENS) {
/* Trim leading whitespace */
while (*token && isspace((unsigned char)*token)) token++;
/* Trim trailing whitespace */
char *end = token + strlen(token) - 1;
containers/agent/one-shot-token/one-shot-token.c:85
- Token list initialization is not thread-safe:
tokens_initialized,num_tokens, andsensitive_tokens[]are mutated without synchronization, so concurrent early getenv() calls can race and corrupt the token list / counts. Consider protecting init withpthread_onceor the existingtoken_mutex(double-checked) so initialization happens exactly once.
* Note: This function must be called with token_mutex held.
*/
static void init_token_list(void) {
if (tokens_initialized) {
return;
}
/* Get the configuration from environment */
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const char *config = real_getenv("AWF_ONE_SHOT_TOKENS"); | ||
|
|
||
| if (config != NULL && config[0] != '\0') { | ||
| /* Parse comma-separated token list */ | ||
| char *config_copy = strdup(config); | ||
| if (config_copy == NULL) { | ||
| fprintf(stderr, "[one-shot-token] ERROR: Failed to allocate memory for token list\n"); | ||
| abort(); | ||
| } | ||
|
|
||
| char *token = strtok(config_copy, ","); | ||
| while (token != NULL && num_tokens < MAX_TOKENS) { | ||
| /* Trim leading whitespace */ | ||
| while (*token && isspace((unsigned char)*token)) token++; | ||
|
|
||
| /* Trim trailing whitespace (only if string is non-empty) */ | ||
| if (strlen(token) > 0) { | ||
| char *end = token + strlen(token) - 1; | ||
| while (end > token && isspace((unsigned char)*end)) { | ||
| *end = '\0'; | ||
| end--; | ||
| } | ||
| } | ||
|
|
||
| if (*token != '\0') { | ||
| sensitive_tokens[num_tokens] = strdup(token); | ||
| if (sensitive_tokens[num_tokens] == NULL) { | ||
| fprintf(stderr, "[one-shot-token] ERROR: Failed to allocate memory for token name\n"); | ||
| /* Clean up previously allocated tokens */ | ||
| for (int i = 0; i < num_tokens; i++) { | ||
| free(sensitive_tokens[i]); | ||
| } | ||
| free(config_copy); | ||
| abort(); |
There was a problem hiding this comment.
If AWF_ONE_SHOT_TOKENS is set but parses to zero non-empty names (e.g., whitespace-only or ",,"), the library ends up protecting no tokens while still reporting successful initialization. Consider treating an empty parsed list as invalid (fallback to defaults or emit an error) to avoid silently disabling protection due to misconfiguration.
This issue also appears on line 94 of the same file.
There was a problem hiding this comment.
Fixed in commit 2245791. The library now falls back to the default token list if AWF_ONE_SHOT_TOKENS parses to zero tokens, with warning messages:
[one-shot-token] WARNING: AWF_ONE_SHOT_TOKENS was set but parsed to zero tokens
[one-shot-token] WARNING: Falling back to default token list to maintain protection
This prevents accidentally disabling all protection due to misconfiguration like ",,," or " ".
| } | ||
|
|
There was a problem hiding this comment.
Using strtok() inside an LD_PRELOAD interposer is risky because strtok maintains internal state and can interfere with application code that is also using strtok (if getenv() is called between the app’s strtok() calls). Prefer strtok_r() (or manual parsing) to avoid clobbering global/tokenizer state.
There was a problem hiding this comment.
Fixed in commit 2245791. Replaced strtok() with strtok_r() which uses a separate state pointer (saveptr) instead of global state. This prevents interference with application code that uses strtok().
Verified with test case that shows application's strtok() calls work correctly even when interleaved with getenv() calls that trigger the library's tokenization.
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Build Test: Node.js - Results
Overall: PASS ✅ All Node.js projects successfully installed dependencies and passed tests.
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built successfully and all tests passed.
|
|
Smoke Test: Claude - PASS ✅ GitHub MCP: Retrieved last 2 merged PRs
✅ Playwright: Navigated to github.com, title verified Status: PASS ✅
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects installed and tested successfully.
|
Build Test: Deno - Results
Overall: ✅ PASS All Deno tests completed successfully.
|
❌ Java Build Test FailedStatus: CONFIGURATION ERROR IssueJava/Maven builds cannot access Maven Central repository through AWF firewall. The firewall does not automatically configure Java proxy settings. Error DetailsRoot Cause
Test Results
Overall: FAIL Required FixAWF's
|
- 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>
|
@lpcox Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
- 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>
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>
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
lpcox
merged commit 5d0fe07
into
lpcox/one-shot-token-ld-preload
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Bun Build Test Results
Overall: PASS ✅ All Bun tests completed successfully.
|
Go Build & Test Results ✅
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Node.js Build Test Results
Overall: PASS ✅ All Node.js test projects installed dependencies successfully and passed their test suites.
|
Build Test: Rust - FAILED ❌Error: Rust toolchain not available in the runner environment. The workflow requires Missing Tool
Test Status
Overall: FAIL
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS cc @lpcox
|
|
Smoke Test Results: PASS ✅ GitHub MCP: #597 chore(deps): bump the github-actions group, #606 chore(deps): bump the actions group Status: PASS - All tests completed successfully.
|
Build Test: Java - FAILED ❌Test Results
Overall: FAILED ❌ Error DetailsProblem: Maven cannot download dependencies through the AWF firewall due to missing Java proxy configuration. Error Message: Root Cause: The Expected Behavior: According to AGENTS.md, AWF should automatically configure JAVA_TOOL_OPTIONS="-Dhttp.proxyHost=172.30.0.10 -Dhttp.proxyPort=3128
-Dhttps.proxyHost=172.30.0.10 -Dhttps.proxyPort=3128"Manual Test: When setting Next StepsThis test failure indicates a bug in either:
The workflow cannot pass until Java proxy configuration is automatically provided by the runtime.
|
…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>
The one-shot token library hardcoded protected token names, requiring recompilation to add custom tokens. This PR adds runtime configuration via the
AWF_ONE_SHOT_TOKENSenvironment variable.Changes
Runtime Configuration
AWF_ONE_SHOT_TOKENSat library initialization for custom token listisspace())strtok_r()instead ofstrtok()to avoid interfering with application code's tokenizer stateAWF_ONE_SHOT_TOKENSparses to zero tokens (e.g., whitespace-only or comma-only strings) to prevent accidental loss of protectionThread Safety
get_token_index()sees consistent token list statestrtok_r()usage ensures no interference with application'sstrtok()callsError Handling
Usage
When
AWF_ONE_SHOT_TOKENSis set with valid tokens, only those tokens are protected (defaults not included). If the variable parses to zero tokens (e.g.,",,,"or" "), the library falls back to defaults with warning messages:Documentation
Updated
containers/agent/one-shot-token/README.mdwith configuration details, fallback behavior, thread-safety notes, and examples for both default and custom token lists.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.