fix: improve one-shot token library copy robustness in chroot mode#606
fix: improve one-shot token library copy robustness in chroot mode#606lpcox merged 2 commits intolpcox/one-shot-token-ld-preloadfrom
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves robustness and observability of staging the one-shot token LD_PRELOAD library for chroot executions, ensuring the system degrades gracefully when the copy cannot be performed and clarifying how /tmp is used in chroot mode.
Changes:
- Adds explicit mkdir/copy success checks and post-copy validation for
one-shot-token.soincontainers/agent/entrypoint.sh. - Clarifies
/tmpusage in chroot volume mounts documentation insrc/docker-manager.ts. - Updates
package-lock.jsonwith new"peer": trueflags on multiple packages.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
containers/agent/entrypoint.sh |
Adds more robust copy/validation logic and clearer warnings when token protection is disabled in chroot mode. |
src/docker-manager.ts |
Clarifies that /tmp mount supports both temp command scripts and the one-shot token library staging directory. |
package-lock.json |
Introduces new "peer": true metadata on multiple packages (appears unrelated to the PR’s stated scope). |
Comments suppressed due to low confidence (1)
containers/agent/entrypoint.sh:184
- These warnings mention copying to "/tmp/awf-lib", but the copy target in this block is "/host/tmp/awf-lib" (which later appears as /tmp inside the chroot). To avoid confusion when debugging mount/permission problems, consider logging the real path used for the copy and/or explicitly noting the /host/tmp ↔ /tmp-in-chroot mapping in the message.
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)"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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 |
There was a problem hiding this comment.
The mkdir/cp failures are redirected to /dev/null, so the warnings/errors here won’t include the underlying reason (e.g., permission denied, ENOSPC). For the “improved diagnostics” goal, consider capturing stderr and including it in the log output (or at least logging it when the operation fails).
| echo "[entrypoint][ERROR] Could not create /tmp/awf-lib directory" | ||
| echo "[entrypoint][ERROR] This should not happen - /tmp is mounted read-write in chroot mode" |
There was a problem hiding this comment.
The mkdir failure message refers to "/tmp/awf-lib", but the failing operation is creating "/host/tmp/awf-lib" (pre-chroot). Using the actual path here (and optionally noting it maps to /tmp in the chroot) would make the error easier to act on.
This issue also appears on line 183 of the same file.
See below for a potential fix:
echo "[entrypoint][WARN] Could not copy one-shot-token library to /host/tmp/awf-lib (chroot path: /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 /host/tmp/awf-lib directory (chroot path: /tmp/awf-lib)"
echo "[entrypoint][ERROR] This should not happen - /host/tmp is mounted read-write and mapped to /tmp in chroot mode"
…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 LD_PRELOAD library copy in chroot mode lacked proper error handling and could fail silently if
/host/tmpwas not writable.Changes:
Enhanced copy logic (
containers/agent/entrypoint.sh):mkdir -p /host/tmp/awf-libsucceeds before copyImproved diagnostics:
/tmpis always mounted rw in chroot modeUpdated mount documentation (
src/docker-manager.ts):/tmpmount is used for both command scripts and one-shot token libraryImplementation:
💡 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.