fix: harden one-shot-token binary against ELF reconnaissance #776
Conversation
Addresses githubnext/gh-aw-security#279 where cleartext token names were discoverable in the .rodata section via strings/objdump. Changes: - XOR-obfuscate default token names so they don't appear as cleartext in .rodata (decoded at runtime during initialization) - Add -fvisibility=hidden to hide all internal symbols; only getenv and secure_getenv are exported via visibility("default") attribute - Add -s linker flag and strip --strip-unneeded post-build to remove symbol table, debug sections, and build metadata - Add encode-tokens.sh script to regenerate obfuscated byte arrays when default token list changes - Add *.so to .gitignore - Add binutils to Dockerfile BUILD_PKGS for strip command - Update build.sh with verification that no cleartext token names remain in the compiled binary Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
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 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 |
Rust Build Test Results
Overall: PASS ✅ All Rust projects built and tested successfully.
|
Build Test: Node.js - Results
Overall: PASS ✅ All Node.js projects installed successfully and passed their test suites.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS cc: @Mossaka
|
Bun Build Test Results
Overall: PASS All Bun projects built and tested successfully.
|
There was a problem hiding this comment.
Pull request overview
Hardens the one-shot-token.so LD_PRELOAD library to reduce binary reconnaissance value by preventing default token environment variable names from appearing in cleartext within the compiled ELF, while also tightening symbol visibility and adding build-time verification.
Changes:
- XOR-obfuscates built-in default token names (decoded at runtime) and exports only
getenv/secure_getenvvia default visibility. - Strips the shared library during builds and adds a
build.shverification step to ensure token names don’t appear instringsoutput. - Adds
encode-tokens.shto regenerate the obfuscated token byte arrays and updates Docker/README accordingly.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| containers/agent/one-shot-token/one-shot-token.c | Replaces cleartext default token list with runtime-decoded XOR-obfuscated bytes; exports only the interposition symbols. |
| containers/agent/one-shot-token/encode-tokens.sh | Adds helper to generate the obfuscated byte arrays for defaults. |
| containers/agent/one-shot-token/build.sh | Adds hardened compile flags, stripping, and binary verification against cleartext token names. |
| containers/agent/one-shot-token/README.md | Documents the hardened build flags and obfuscation workflow. |
| containers/agent/Dockerfile | Builds the .so with hidden visibility + stripping; installs binutils to support strip. |
| .gitignore | Ignores .so artifacts at repo root. |
Comments suppressed due to low confidence (2)
containers/agent/one-shot-token/one-shot-token.c:377
- secure_getenv() never initializes the token list and calls get_token_index() without holding token_mutex. If a process calls secure_getenv() before getenv() (or concurrently during initialization), sensitive tokens may be missed and left in the environment, defeating the protection. Make secure_getenv follow the same locking + init_token_list() pattern as getenv (lock, init if needed, compute token_idx under the lock) before deciding whether to pass through.
__attribute__((visibility("default")))
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);
}
int token_idx = get_token_index(name);
/* Not a sensitive token - pass through to real secure_getenv */
if (token_idx < 0) {
return real_secure_getenv(name);
}
containers/agent/one-shot-token/build.sh:32
- build.sh now unconditionally runs
strip --strip-unneeded, butstripmay not be available on a developer machine; withset -ethis will hard-fail. Consider checkingcommand -v stripfirst and emitting a clear error instructing how to install it (e.g., binutils), or falling back to just the linker-sflag when unavailable.
# Remove remaining unneeded symbols (debug sections, build metadata)
strip --strip-unneeded "${OUTPUT_FILE}"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Verify hardening: token names should NOT appear in binary | ||
| if strings -a "${OUTPUT_FILE}" | grep -qE '(COPILOT_GITHUB_TOKEN|OPENAI_API_KEY|ANTHROPIC_API_KEY)'; then | ||
| echo "[build] WARNING: Cleartext token names still present in binary" |
There was a problem hiding this comment.
The hardening verification relies on strings, which may not be installed by default on all dev machines (it’s typically provided by binutils). Consider checking command -v strings and failing with a clear message (or skipping verification) rather than failing with a generic “command not found”.
This issue also appears on line 30 of the same file.
| __attribute__((visibility("default"))) | ||
| char *secure_getenv(const char *name) { | ||
| init_real_secure_getenv(); | ||
| init_real_getenv(); |
There was a problem hiding this comment.
There are integration tests for one-shot-token behavior, but none appear to exercise the secure_getenv() interception path (a repo search under tests/ finds no secure_getenv usage). Adding an integration test that calls secure_getenv first would help prevent regressions here.
This issue also appears on line 362 of the same file.
| * via strings(1) or objdump. Internal symbols use hidden visibility. | ||
| * Binary should be stripped after compilation (see build.sh / Dockerfile). | ||
| * | ||
| * Compile: gcc -shared -fPIC -fvisibility=hidden -o one-shot-token.so one-shot-token.c -ldl |
There was a problem hiding this comment.
The header compile command is now missing required link flags used by the actual build (notably -lpthread, and potentially -s/strip if you want the comment to match the hardened build). As written, following this compile line will likely fail to link due to pthread_mutex usage. Update the compile example to match build.sh / Dockerfile so the documentation is accurate.
| * Compile: gcc -shared -fPIC -fvisibility=hidden -o one-shot-token.so one-shot-token.c -ldl | |
| * Compile: gcc -shared -fPIC -fvisibility=hidden -o one-shot-token.so one-shot-token.c -ldl -lpthread -s |
| static const unsigned char OBF_9[] = { 0x19, 0x16, 0x1b, 0x0f, 0x1e, 0x1f, 0x05, 0x1b, 0x0a, 0x13, 0x05, 0x11, 0x1f, 0x03 }; /* length=14 */ | ||
| static const unsigned char OBF_10[] = { 0x19, 0x15, 0x1e, 0x1f, 0x02, 0x05, 0x1b, 0x0a, 0x13, 0x05, 0x11, 0x1f, 0x03 }; /* length=13 */ | ||
|
|
||
| static const struct obf_entry OBFUSCATED_DEFAULTS[11] = { |
There was a problem hiding this comment.
The generated defaults define NUM_DEFAULT_TOKENS but the OBFUSCATED_DEFAULTS array is still sized with a hard-coded literal ("[11]"). This creates a footgun if the token list changes and someone updates the macro but forgets the array size. Prefer sizing the array with NUM_DEFAULT_TOKENS (and update encode-tokens.sh output accordingly) to keep these in sync automatically.
| static const struct obf_entry OBFUSCATED_DEFAULTS[11] = { | |
| static const struct obf_entry OBFUSCATED_DEFAULTS[NUM_DEFAULT_TOKENS] = { |
| # Verify hardening: token names should NOT appear in binary | ||
| if strings -a "${OUTPUT_FILE}" | grep -qE '(COPILOT_GITHUB_TOKEN|OPENAI_API_KEY|ANTHROPIC_API_KEY)'; then | ||
| echo "[build] WARNING: Cleartext token names still present in binary" | ||
| exit 1 | ||
| else | ||
| echo "[build] Verified: no cleartext token names in binary" | ||
| fi |
There was a problem hiding this comment.
The binary hardening verification only greps for three token names, but the default list includes more (e.g., GH_TOKEN, GITHUB_PAT, OPENAI_KEY, etc.). This can let regressions slip through where some default token names still appear in the compiled .so. Consider grepping for the full default token set (ideally sourced from a single list so it stays in sync with encode-tokens.sh / one-shot-token.c).
.NET Build Test Results
Overall: PASS ✅ All .NET projects successfully restored, built, and ran with expected output.
|
|
Merged PRs: fix(docker): hide credentials at direct home mount in chroot mode | fix: replace unanchored regex with string assertions in tests
|
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Java Build Test Results
Overall: PASS ✅ All Java projects built and tested successfully through the AWF firewall with Maven proxy configuration.
|
Chroot Version Comparison Test Results
Test Status: ❌ Failed Not all runtime versions matched between host and chroot environments. Only Go versions are identical.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS
|
Resolve conflicts between the C hardening approach (this PR) and the Rust rewrite (main). Keep the hardened C implementation with XOR-obfuscated token names, hidden symbol visibility, and stripped binary. Remove Rust source files (Cargo.toml, src/lib.rs) that are superseded by the C approach. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
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 prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Build Test Results: Node.js ✅All Node.js projects successfully installed and tested through the AWF firewall.
Overall: ✅ PASS All projects installed dependencies and passed their test suites successfully.
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Build Test: Bun ✅
Overall: PASS ✅ All Bun build tests completed successfully!
|
🧪 Build Test: Go - ResultsAll Go projects tested successfully!
Overall: PASS ✅ All Go module downloads completed successfully and all tests passed.
|
|
fix: pass ANTHROPIC_API_KEY to validation in all Claude workflows
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS 🎉 cc @Mossaka
|
|
Smoke Test Results Last 2 Merged PRs:
Test Results:
Status: PASS
|
✅ Java Build Test ResultsAll projects successfully compiled and passed tests.
Overall: PASS Both projects:
|
Chroot Mode Test Results
Overall Status: ❌ FAILED - Not all runtimes match between host and chroot environments.
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built successfully and tests passed.
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects successfully restored NuGet packages, compiled, and executed.
|
Accept main's C-only one-shot-token library (with XOR obfuscation from PR #776). The Rust implementation (src/lib.rs, Cargo.toml) was deleted on main and is no longer needed. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 participant
Summary
Addresses githubnext/gh-aw-security#279 — the
one-shot-token.solibrary contained cleartext token names in its.rodatasection, discoverable viastringsorobjdump -s -j .rodata. This provided reconnaissance value by revealing exactly which environment variable names AWF monitors.Changes:
strings,objdump, or similar tools.-fvisibility=hiddenhides all internal symbols. Onlygetenvandsecure_getenvare exported (via__attribute__((visibility("default")))).-slinker flag +strip --strip-unneededremoves symbol table, debug sections, and build metadata.build.shnow verifies no cleartext token names remain in the compiled binary.encode-tokens.sh— helper script to regenerate obfuscated byte arrays when the default token list changes.Before/After
Before:
strings -a one-shot-token.soreveals:After:
strings -a one-shot-token.soshows none of the token names.nmreports "no symbols" for the full symbol table. Onlygetenvandsecure_getenvappear in the dynamic symbol table (required for LD_PRELOAD).Test plan
build.shpasses including new verification stepnpm test)🤖 Generated with Claude Code