feat: add AWF_ONE_SHOT_TOKEN_DEBUG env var for silent-by-default logging#864
feat: add AWF_ONE_SHOT_TOKEN_DEBUG env var for silent-by-default logging#864
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Claude
AI
changed the title
Add debug flag support to the C implementation of one-shot-token library to match the Rust implementation. Logging is now off by default and can be enabled by setting AWF_ONE_SHOT_TOKEN_DEBUG=1 or AWF_ONE_SHOT_TOKEN_DEBUG=true. 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! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
Build Test: Deno
Overall: ✅ PASS All Deno tests passed successfully.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Status: PASS 🎉 PR #858 by
|
Node.js Build Test Results
Overall: PASS ✅ All three Node.js projects successfully installed dependencies and passed their test suites.
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects built and tested successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects successfully configured with CMake and built without errors.
|
.NET Build Test ResultsAll .NET projects tested successfully! ✅
Overall: PASS Test Detailshello-world:
json-parse:
|
Smoke Test Results - Claude EngineLast 2 merged PRs:
Test Results:
Status: PASS
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built and tested successfully.
|
|
Smoke Test (Codex)
|
🔍 Chroot Version Comparison Test Results
Overall Result: ❌ FAILED - Not all versions match The chroot mode test detected version mismatches for Python and Node.js. This is expected behavior as the chroot environment uses container binaries which may differ from the host system. Only Go matched between host and chroot environments.
|
There was a problem hiding this comment.
Pull request overview
Adds an AWF_ONE_SHOT_TOKEN_DEBUG environment flag to make one-shot-token logging silent-by-default while allowing opt-in debug output for troubleshooting and test verification.
Changes:
- Added a debug flag (
AWF_ONE_SHOT_TOKEN_DEBUG=1|true) to enable/disable stderr logging (default off) in both Rust and C implementations. - Updated integration tests to set
AWF_ONE_SHOT_TOKEN_DEBUG=1when asserting on one-shot-token log output. - Updated one-shot-token README to document silent-by-default behavior and the debug flag.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/integration/one-shot-tokens.test.ts | Sets AWF_ONE_SHOT_TOKEN_DEBUG=1 in test container env where stderr log assertions are made. |
| containers/agent/one-shot-token/src/lib.rs | Adds debug_enabled state + is_debug_enabled() and gates logging accordingly. |
| containers/agent/one-shot-token/one-shot-token.c | Adds debug_enabled + is_debug_enabled() and gates stderr logging accordingly. |
| containers/agent/one-shot-token/README.md | Documents the new debug flag and updates examples for silent-by-default operation. |
Comments suppressed due to low confidence (2)
containers/agent/one-shot-token/README.md:292
- Same as the earlier example: the expected output shows
INFO: Token ... cleared from process environment, but the C implementation doesn’t emit these INFO lines. Please adjust this expected output (or implement the INFO logging in C under the debug flag) to avoid misleading users.
[one-shot-token] Initialized with 2 custom token(s) from AWF_ONE_SHOT_TOKENS
[one-shot-token] Token MY_API_KEY accessed and cached (value: secr...)
[one-shot-token] INFO: Token MY_API_KEY cleared from process environment
First MY_API_KEY: secret-value-123
Second MY_API_KEY: secret-value-123
[one-shot-token] Token SECRET_TOKEN accessed and cached (value: anot...)
[one-shot-token] INFO: Token SECRET_TOKEN cleared from process environment
First SECRET_TOKEN: another-secret
containers/agent/one-shot-token/src/lib.rs:112
- The module docs and README state the library is silent by default unless
AWF_ONE_SHOT_TOKEN_DEBUGis enabled, but thissecure_getenv-missing warning is emitted unconditionally. Either gate this message behind the debug flag (without using intercepted getenv), or update the documentation to call out this exception so “silent by default” remains accurate.
// Note: We can't check debug flag here because it would cause infinite recursion
// during initialization. This is a rare case (secure_getenv unavailable) so we
// always log it.
eprintln!("[one-shot-token] WARNING: secure_getenv not available, falling back to getenv");
None
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Check if debug logging is enabled */ | ||
| debug_enabled = is_debug_enabled(); | ||
|
|
||
| /* Get the configuration from environment */ | ||
| const char *config = real_getenv("AWF_ONE_SHOT_TOKENS"); |
There was a problem hiding this comment.
AWF_ONE_SHOT_TOKEN_DEBUG is documented as never being cached/cleared, but the custom token list parsing will currently accept it like any other token name. Consider explicitly skipping AWF_ONE_SHOT_TOKEN_DEBUG when parsing AWF_ONE_SHOT_TOKENS to make that guarantee true.
| **Important notes:** | ||
| - Debug logging is **off by default** to reduce noise in production environments | ||
| - When enabled, the library logs initialization messages and token access events to stderr | ||
| - The `AWF_ONE_SHOT_TOKEN_DEBUG` variable is never cached or cleared (prevents infinite recursion) |
There was a problem hiding this comment.
This bullet says AWF_ONE_SHOT_TOKEN_DEBUG is “never cached or cleared”, but neither the C nor Rust implementation currently prevents a user from including it in AWF_ONE_SHOT_TOKENS (in which case it would be treated like a protected token). Either enforce the exclusion in code or soften this statement to reflect the actual behavior.
| - The `AWF_ONE_SHOT_TOKEN_DEBUG` variable is never cached or cleared (prevents infinite recursion) | |
| - By default, the `AWF_ONE_SHOT_TOKEN_DEBUG` variable is not cached or cleared (to prevent infinite recursion when checking it) | |
| - If you explicitly add `AWF_ONE_SHOT_TOKEN_DEBUG` to `AWF_ONE_SHOT_TOKENS`, it will be treated like any other protected token and may be cached/cleared |
| ``` | ||
| [one-shot-token] Initialized with 11 default token(s) | ||
| [one-shot-token] Token GITHUB_TOKEN accessed and cached (value: test...) | ||
| [one-shot-token] INFO: Token GITHUB_TOKEN cleared from process environment |
There was a problem hiding this comment.
The expected output includes an INFO: Token ... cleared from process environment line, but the agent Docker image builds the C implementation (one-shot-token.c), which does not emit these INFO lines. Update the example to match the C library’s actual output, or add equivalent logging to the C implementation when debug is enabled.
This issue also appears on line 285 of the same file.
| [one-shot-token] INFO: Token GITHUB_TOKEN cleared from process environment |
| return; | ||
| } | ||
|
|
||
| // Check if debug logging is enabled | ||
| state.debug_enabled = is_debug_enabled(); | ||
|
|
||
| // Get configuration from environment | ||
| let config_cstr = CString::new("AWF_ONE_SHOT_TOKENS").unwrap(); | ||
| // SAFETY: We're calling the real getenv with a valid C string |
There was a problem hiding this comment.
AWF_ONE_SHOT_TOKEN_DEBUG is now a special control variable, but init_token_list will still accept it as a protected token if a user includes it in AWF_ONE_SHOT_TOKENS. That contradicts the documented guarantee that this variable is never cached/cleared. Consider filtering out AWF_ONE_SHOT_TOKEN_DEBUG when parsing the custom token list (and/or preventing it from being added to state.tokens).
| libc::unsetenv(name); | ||
|
|
||
| // Verify the token was cleared from the process environment | ||
| check_task_environ_exposure(name_str); | ||
| check_task_environ_exposure(name_str, debug_enabled); | ||
|
|
There was a problem hiding this comment.
check_task_environ_exposure still scans the entire environ even when debug logging is disabled, but its only observable effect is emitting debug logs. To avoid unnecessary work in the default (silent) mode, gate the call to check_task_environ_exposure(...) behind if debug_enabled { ... } (or early-return at the top of the function when debug_enabled is false).
This issue also appears on line 108 of the same file.
See below for a potential fix:
// Verify the token was cleared from the process environment only when debug logging is enabled
if debug_enabled {
check_task_environ_exposure(name_str, debug_enabled);
}
|
|
||
| fprintf(stderr, "[one-shot-token] Token %s accessed and cached (value: %s) (via secure_getenv)\n", | ||
| name, format_token_value(token_cache[token_idx])); | ||
| if (debug_enabled) { | ||
| fprintf(stderr, "[one-shot-token] Token %s accessed and cached (value: %s) (via secure_getenv)\n", | ||
| name, format_token_value(token_cache[token_idx])); | ||
| } |
There was a problem hiding this comment.
secure_getenv() does not initialize the token list (and debug_enabled is only set inside init_token_list). If secure_getenv is the first call in a process, num_tokens will still be 0 so no tokens are protected and debug logging will never enable. Fix by ensuring token initialization (under the mutex) happens in secure_getenv() before checking get_token_index() / using debug_enabled.
Uh oh!
There was an error while loading. Please reload this page.