Conversation
Co-authored-by: lpcox <[email protected]>
Co-authored-by: lpcox <[email protected]>
Co-authored-by: lpcox <[email protected]>
Claude
AI
changed the title
|
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 END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
✅ Deno Build Test ResultsAll tests passed successfully!
Overall: ✅ PASS All Deno test projects compiled and ran successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects built and tested successfully.
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects built and tested successfully.
|
Smoke Test ResultsLast 2 Merged PRs:
Tests:
Overall Status: PASS cc
|
|
Smoke Test Results ✅ GitHub MCP - Retrieved PRs Status: PASS
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects built and ran successfully.
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Java Build Test Results
Overall: PASS All Java projects compiled successfully and all tests passed through the AWF firewall with Maven proxy configuration.
|
Smoke Test Results ✅ PASSLast 2 merged PRs:
Test Results:
cc @lpcox
|
Smoke Test ResultsLast 2 Merged PRs:
Tests:
Status: PASS
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects built and tested successfully.
|
.NET Build Test Results
Overall: PASS All .NET projects successfully restored, built, and ran with expected output.
|
Chroot Version Comparison Test Results
Overall Status: ❌ Tests Failed Only Go versions match between host and chroot environments. Python and Node.js versions differ, which means chroot mode is not providing transparent access to host runtimes for all tools.
|
☕ Java Build Test Results
Overall: ✅ PASS All Java projects compiled and tested successfully through the AWF proxy.
|
|
Smoke test results:
|
…age manager tests (#797) * Initial plan * fix: add explicit toolchain to rust setup in test workflow Co-authored-by: lpcox <[email protected]> * feat: add RUSTUP_HOME environment variable support for Rust toolchain Co-authored-by: lpcox <[email protected]> --------- Co-authored-by: anthropic-code-agent[bot] <[email protected]> Co-authored-by: lpcox <[email protected]>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 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 prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
|
Smoke Test Results - Claude Engine Last 2 PRs:
✅ GitHub MCP - PRs retrieved Status: PASS
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Bun Build Test Results
Overall: PASS All Bun projects built and tested successfully.
|
Node.js Build Test Results
Overall: PASS ✅ All Node.js test projects built and tested successfully.
|
|
Smoke Test Results for Copilot Engine ✅ GitHub MCP: #797, #794 retrieved Status: PASS ✓ cc: @lpcox
|
Go Build Test Results ✅
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed their tests.
|
✅ Java Build Test ResultsAll Java build tests PASSED successfully!
Overall: PASS Details
|
Chroot Version Comparison Test Results
Overall Result: ❌ Tests Failed - Not all runtime versions match The chroot mode successfully accessed host binaries for Go, but Python and Node.js versions differ between the host and chroot environment. This indicates that the chroot is using container-installed versions rather than host binaries for these runtimes.
|
|
Smoke test results
|
There was a problem hiding this comment.
Pull request overview
This PR migrates the one-shot-token LD_PRELOAD library from C to Rust while also adding RUSTUP_HOME environment variable support for Rust toolchain discovery. The one-shot-token library is a security component that intercepts getenv() calls to sensitive environment variables, caching their values in memory and clearing them from the process environment to prevent token exposure via /proc/self/environ.
Changes:
- Rewrites one-shot-token library in Rust with equivalent functionality to the C version
- Adds RUSTUP_HOME environment variable passthrough to support Rust toolchain in chroot mode
- Updates Docker build to use multi-stage build with official Rust image for supply chain security
- Fixes exit code handling in test runner to properly normalize undefined exit codes
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fixtures/awf-runner.ts | Fixes exit code normalization by using ?? 0 before deriving success field |
| src/docker-manager.ts | Adds RUSTUP_HOME passthrough to container environment for Rust toolchain support |
| containers/agent/one-shot-token/src/lib.rs | New Rust implementation of one-shot-token library with getenv/secure_getenv interception |
| containers/agent/one-shot-token/build.sh | Updates build script to use Cargo instead of GCC |
| containers/agent/one-shot-token/README.md | Updates documentation to reflect Rust implementation and adds environment verification details |
| containers/agent/one-shot-token/Cargo.toml | Adds Rust package configuration with libc and once_cell dependencies |
| containers/agent/one-shot-token/.gitignore | Updates to ignore Rust build artifacts (target/, Cargo.lock) |
| containers/agent/entrypoint.sh | Adds RUSTUP_HOME support and fallback Cargo detection for Rust in chroot mode |
| containers/agent/Dockerfile | Implements multi-stage build using official rust:1.77-slim image for security |
| .github/workflows/test-chroot.yml | Adds RUSTUP_HOME capture and preservation for Rust toolchain tests |
| .github/workflows/build-test-rust.md | Adds crates.io to allowed network domains |
| .github/workflows/build-test-rust.lock.yml | Auto-generated workflow updates with version bumps and domain additions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| libc = "0.2" | ||
| once_cell = "1.19" |
There was a problem hiding this comment.
The Cargo.toml specifies libc = "0.2" and once_cell = "1.19" without patch versions. While this is common practice for Rust libraries, consider pinning to more specific versions (e.g., "0.2.150" and "1.19.0") for reproducible builds, especially since this is a security-critical component that intercepts environment variable access. This would make builds more deterministic and easier to audit for security vulnerabilities.
| libc = "0.2" | |
| once_cell = "1.19" | |
| libc = "0.2.150" | |
| once_cell = "1.19.0" |
| @@ -157,6 +157,8 @@ jobs: | |||
|
|
|||
| - name: Setup Rust | |||
| uses: dtolnay/rust-toolchain@stable | |||
There was a problem hiding this comment.
The dtolnay/rust-toolchain action reference is missing a version pin or SHA. While the with: toolchain: stable configuration is present, the action itself should be pinned to a specific commit SHA for security and reproducibility, consistent with the convention used for other actions in this workflow (e.g., actions/checkout, actions/setup-node, etc. are all pinned to SHAs).
| uses: dtolnay/rust-toolchain@stable | |
| uses: dtolnay/rust-toolchain@4f6abf093ee4c743c3c1e811ce0cfe4631d14d1f # stable |
|
|
||
| # Rust build artifacts | ||
| target/ | ||
| Cargo.lock |
There was a problem hiding this comment.
Cargo.lock is excluded from version control for this library. While this is appropriate for Rust library crates (cdylib), the project uses the library in a Docker multi-stage build. For reproducible builds and supply chain security, consider committing Cargo.lock. This ensures the exact same dependency versions are used across all builds and makes it easier to track dependency updates for security vulnerabilities.
| Cargo.lock |
| // Allocate memory that will never be freed (must persist for caller's use) | ||
| let cached = libc::malloc(value_bytes.len()) as *mut c_char; | ||
| if cached.is_null() { | ||
| eprintln!("[one-shot-token] ERROR: Failed to allocate memory for token value"); | ||
| std::process::abort(); | ||
| } |
There was a problem hiding this comment.
Memory allocated via libc::malloc at line 311 is never freed (by design, as noted in the comment). However, this could lead to memory accumulation if the same token is accessed multiple times before caching. Consider verifying that the cache check at line 291 prevents duplicate allocations. The current implementation appears correct since the cache check happens before allocation, but it would be helpful to add a debug assertion or comment confirming this invariant.
| # Fallback: detect Cargo from default location if CARGO_HOME not provided | ||
| # This ensures Rust binaries work even when CARGO_HOME env var is not set | ||
| echo "# Add Cargo bin for Rust if it exists (fallback when CARGO_HOME not provided)" >> "/host${SCRIPT_FILE}" | ||
| echo "[ -d \"\$HOME/.cargo/bin\" ] && export PATH=\"\$HOME/.cargo/bin:\$PATH\"" >> "/host${SCRIPT_FILE}" |
There was a problem hiding this comment.
The fallback logic adds $HOME/.cargo/bin to PATH when CARGO_HOME is not provided. However, this fallback doesn't set RUSTUP_HOME, which means rustc and cargo may not find the Rust toolchain even if the binaries are in PATH. Consider adding a similar fallback for RUSTUP_HOME (e.g., [ -d "$HOME/.rustup" ] && export RUSTUP_HOME="$HOME/.rustup") to ensure Rust tools work correctly in the fallback scenario.
| echo "[ -d \"\$HOME/.cargo/bin\" ] && export PATH=\"\$HOME/.cargo/bin:\$PATH\"" >> "/host${SCRIPT_FILE}" | |
| echo "[ -d \"\$HOME/.cargo/bin\" ] && export PATH=\"\$HOME/.cargo/bin:\$PATH\"" >> "/host${SCRIPT_FILE}" | |
| echo "[ -d \"\$HOME/.rustup\" ] && export RUSTUP_HOME=\"\$HOME/.rustup\"" >> "/host${SCRIPT_FILE}" |
3 participants
No description provided.