test: expand credential hiding tests to all 14 protected paths#1163
test: expand credential hiding tests to all 14 protected paths#1163
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (3 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Expands the integration test suite to verify credential-hiding behavior across all 14 protected credential paths (home and /host chroot), ensuring the files are effectively empty when mounted from /dev/null.
Changes:
- Updates the existing “Test 4” to more explicitly target the original 3 credential paths and adjusts its shell check.
- Adds a new test group that covers the 11 previously untested credential file paths.
- Verifies 0-byte size for direct home paths and
/hostpaths, plus acat-content check.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const homeDir = os.homedir(); | ||
| const paths = untestedPaths.map(p => `${homeDir}/${p.path}`).join(' '); |
There was a problem hiding this comment.
Building a shell command by interpolating paths directly into sh -c '...' is unsafe and can break when homeDir contains spaces or shell-special characters (and is also an injection risk since this runs with sudo). Prefer passing file paths as positional args to sh -c and iterating over "$@" (or otherwise robustly quoting/escaping each path) so the loop receives the exact paths regardless of characters.
| const homeDir = os.homedir(); | |
| const paths = untestedPaths.map(p => `${homeDir}/${p.path}`).join(' '); | |
| const paths = untestedPaths.map(p => `"$HOME/${p.path}"`).join(' '); |
| // Check all credential files in a single container run for efficiency. | ||
| // wc -c reports byte count; /dev/null-mounted files should be 0 bytes. | ||
| const result = await runner.runWithSudo( | ||
| `sh -c 'for f in ${paths}; do wc -c "$f" 2>/dev/null; done' 2>&1 | grep -v "^\\["`, |
There was a problem hiding this comment.
Building a shell command by interpolating paths directly into sh -c '...' is unsafe and can break when homeDir contains spaces or shell-special characters (and is also an injection risk since this runs with sudo). Prefer passing file paths as positional args to sh -c and iterating over "$@" (or otherwise robustly quoting/escaping each path) so the loop receives the exact paths regardless of characters.
|
|
||
| // cat all files and concatenate output - should be empty | ||
| const result = await runner.runWithSudo( | ||
| `sh -c 'for f in ${paths}; do cat "$f" 2>/dev/null; done' 2>&1 | grep -v "^\\["`, |
There was a problem hiding this comment.
This test can produce a false pass if one or more files don’t exist or aren’t mounted: cat errors are suppressed (2>/dev/null), so the combined stdout can be empty even when paths are missing. To make the assertion meaningful, ensure each file is actually present/checked (e.g., fail the loop if a path is missing, or emit/validate per-file markers), or remove this test and rely on the wc -c tests that already validate counts.
| `sh -c 'for f in ${paths}; do cat "$f" 2>/dev/null; done' 2>&1 | grep -v "^\\["`, | |
| `sh -c 'for f in ${paths}; do cat "$f"; done' 2>&1 | grep -v "^\\["`, |
| const lines = result.stdout.split('\n').filter(l => l.match(/^\s*\d+/)); | ||
| // Each file should be 0 bytes (hidden via /dev/null) | ||
| lines.forEach(line => { | ||
| const size = parseInt(line.trim().split(/\s+/)[0]); |
There was a problem hiding this comment.
parseInt should be called with an explicit radix to avoid edge-case parsing issues. Use parseInt(value, 10) here (and in the similar parsing block in the /host test) to make the intent unambiguous.
| test('Test 4: Original 3 credential files are mounted from /dev/null', async () => { | ||
| const homeDir = os.homedir(); | ||
|
|
||
| // Check multiple credential files in one command | ||
| // Check the originally-tested credential files in one command | ||
| const result = await runner.runWithSudo( | ||
| `sh -c 'for f in ${homeDir}/.docker/config.json ${homeDir}/.npmrc ${homeDir}/.config/gh/hosts.yml; do if [ -f "$f" ]; then wc -c "$f"; fi; done' 2>&1 | grep -v "^\\["`, | ||
| `sh -c 'for f in ${homeDir}/.docker/config.json ${homeDir}/.npmrc ${homeDir}/.config/gh/hosts.yml; do wc -c "$f" 2>/dev/null; done' 2>&1 | grep -v "^\\["`, |
There was a problem hiding this comment.
With 2>/dev/null added and no follow-up assertion that all three files produced output, this test can now silently skip missing/unmounted files (no wc output) and potentially still pass depending on downstream expectations. To preserve the “mounted from /dev/null” guarantee, parse the wc output and assert you got exactly 3 size lines and that each is 0 (or make the shell loop fail if any file is missing).
🧪 Build Test: Bun
Overall: ✅ PASS
|
Build Test: Node.js Results ✅
Overall: PASS
|
Go Build Test Results
Overall: PASS ✅
|
C++ Build Test Results
Overall: PASS
|
|
Smoke test results for ✅ GitHub MCP: #1159 fix(security): eliminate TOCTOU race conditions in ssl-bump.ts, #1158 fix(security): stop logging partial token values Overall: PASS
|
Build Test: Deno ✅
Overall: PASS
|
Java Build Test Results ✅
Overall: PASS
|
|
Test results:
|
.NET Build Test Results
Overall: PASS Run outputhello-world:
|
🦀 Rust Build Test Results
Overall: ✅ PASS
|
Smoke Test Results
Overall: PASS
|
Add 3 new integration tests covering all 11 untested credential paths: SSH keys (4), AWS creds/config, Kube config, Azure creds, GCloud creds, Cargo creds, Composer auth. Tests verify 0 bytes at both direct home and /host chroot paths. Uses robust patterns (if -f, || true, extractCommandOutput) consistent with existing tests. Fixes #761 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mossaka
force-pushed
the
fix/052-credential-hiding-tests
branch
from
c28ea23 to
e10ad83
Compare
|
Smoke Test Results — PASS
|
|
GitHub MCP merged PRs: ✅
|
This comment has been minimized.
This comment has been minimized.
|
✅ GitHub MCP: fix(squid): block direct IP connections that bypass domain filtering | feat: combine all build-test workflows into single build-test.md
|
|
Smoke test results for
Overall: PASS
|
Replace existsSync+writeFileSync with writeFileSync({flag:'wx'}) to
eliminate the file-system-race CodeQL alert in the test beforeAll hook.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Smoke test results for run 22932379687 —
Overall: PASS
|
|
Smoke Test Results — PASS
|
This comment has been minimized.
This comment has been minimized.
|
PR titles:
|
/dev/null-mounted credential files are character special devices, not regular files. [ -f ] returns false for them, causing wc -c to produce no output. Use [ -e ] (exists) to correctly detect these mounts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Smoke Test Results
Overall: PASS
|
|
Smoke test results for ✅ GitHub MCP — Last 2 merged PRs: #1222 "test: add --skip-pull integration test", #1221 "test: add --allow-host-ports validation tests" Overall: PASS
|
This comment has been minimized.
This comment has been minimized.
|
Merged PRs: test: add --skip-pull integration test | test: add --allow-host-ports validation tests ✅
|
AWF always runs in chroot mode (chroot /host), so /host$HOME/... paths don't exist inside the container. Changed the test from expecting 0-byte files at /host paths to verifying those paths are inaccessible, which is the correct security assertion for chroot mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Smoke Test Results — ✅ GitHub MCP: Last 2 merged PRs: #1229 "feat(cli): add short flags for frequently used options", #1228 "docs: clarify --image-tag behavior with agent-image presets" (both by Overall: PASS
|
Smoke Test Results✅ GitHub MCP: feat(cli): add short flags for frequently used options (#1229), docs: clarify --image-tag behavior with agent-image presets (#1228) Overall: PASS
|
|
PR titles: #1240 test(docker): verify capsh execution chain after PR #715; #1234 fix(proxy): add lowercase proxy vars and NODE_EXTRA_CA_CERTS
|
🏗️ Build Test Suite Results
Overall: 0/8 ecosystems passed — ❌ FAIL ❌ ALL_CLONES_FAILEDAll 8 repository clones failed. The Error: Action required: Ensure
|
Summary
Test plan
npm run buildpassesnpm testpasses (831 tests)npm run lintpasses (0 errors)Fixes #761
🤖 Generated with Claude Code