fix(sandbox): surface fingerprint-erasure and probe-script failures#22
Merged
Conversation
eraseFingerprints, plantHoneypotFiles, restoreLocalBin, WriteProbeScripts, and WriteProbeScriptsMulti previously discarded the result of every docker-exec call (`_ = cmd.Run()`). A failure here — e.g. /.dockerenv removal rejected by a corrupted runtime, or a probe-script write that silently dropped — would let the scan continue against a partially prepared sandbox: /.dockerenv intact, honeypot files missing, or an import phase that runs without its OS-spoofing wrapper. Sandbox-aware malware that simply detected the gap and stayed dormant would surface as a "clean" verdict — breaking the detection contract that "clean means no malicious behavior was observed." This change makes the entire post-start setup fail loud: - dockerExecRoot / dockerWriteFile return errors wrapped with the command (or path) for diagnosis. Honeypot content is omitted from the error to avoid spilling generated credentials into logs. - The three setup helpers and both probe-script writers fail-fast on the first underlying error. - StartPaused and Start route through a shared prepareSandboxState helper that propagates any setup failure. Callers in cmd/root.go abort the scan rather than reporting clean. - TestEraseFingerprints_FailLoud / TestPlantHoneypotFiles_FailLoud / TestRestoreLocalBin_FailLoud / TestWriteProbeScripts*_FailLoud pin the contract: a failing docker command must surface as an error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Real-world scan of `six` failed with the previous commit's fail-loud
change:
Error: starting sandbox: erasing sandbox fingerprints: \
docker exec rm -f /.dockerenv: exit status 1
Root cause: the rootfs is mounted --read-only, so `rm -f /.dockerenv`
inside the container has never succeeded. The original silent-fail
hid the bug; surfacing the error exposed it. Anti-fingerprinting was
effectively never applied for /.dockerenv since --read-only was
introduced.
Fix: bind-mount an empty regular file from the host over /.dockerenv
at container creation time. The mask file lives in the per-scan
seccomp temp dir (already cleaned up by Cleanup), is created by
writeSeccompProfile, and is wired in via
`--mount=type=bind,src=...,dst=/.dockerenv,readonly`. Sandbox-aware
payloads that read /.dockerenv now see empty content. A regular
empty file is used (not /dev/null) so stat().S_ISREG() still returns
true — a char device would itself be a fingerprint.
eraseFingerprints had only this one operation, so the function and
its two tests are removed; prepareSandboxState now calls only
restoreLocalBin and plantHoneypotFiles. SECURITY.md is corrected to
describe the actual mechanism. TestContainerArgs gains an assertion
that the bind-mount flag and host-side mask file are present.
Path-existence checks (`os.path.exists("/.dockerenv")`) still see
something at the path; --runtime=runsc remains the recommendation
for defeating those.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- gocritic appendCombine: combine the seccomp and dockerenv-mask appends into a single args = append(...) call (introduced when the bind-mount line was added in the previous commit). - goconst: extract "test-container" into testContainerID — the new *_FailLoud tests pushed the literal's count from 3 to 7. No behaviour change. Net lint count drops by one vs. the pre-series baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix a silent-fail class of bug in the post-start sandbox preparation that could produce false-
cleanverdicts. WheneraseFingerprints,plantHoneypotFiles,restoreLocalBin, or the probe-script writers failed at thedocker execlayer, the error was discarded (_ = cmd.Run()). The scan would continue against a partially-prepared sandbox —/.dockerenvstill present, honeypot files missing, or an import phase running without its OS-spoofing wrapper — and any sandbox-aware payload that simply detected the gap and stayed dormant would surface asclean. The detection contract ("cleanmeans no malicious behavior was observed") was therefore not enforced.Changes
dockerExecRoot/dockerWriteFilenow return errors, wrapped with the command (or path) for diagnosis. Honeypot content is omitted from the error to avoid spilling generated credentials into logs.restoreLocalBin,eraseFingerprints,plantHoneypotFiles,WriteProbeScripts, andWriteProbeScriptsMultinow fail-fast on the first underlying error.Start/StartPausedroute through a sharedprepareSandboxStatehelper that propagates any setup failure.cmd/root.go(batch screening, eBPF probe, container-strace probe) abort the scan on prep failure rather than continuing.TestEraseFingerprints_FailLoud,TestPlantHoneypotFiles_FailLoud,TestRestoreLocalBin_FailLoud,TestWriteProbeScripts_FailLoud,TestWriteProbeScriptsMulti_FailLoud) — they pin the contract that a failing docker command must surface as an error. Existing happy-path tests updated to assertnil.[Unreleased] / Fixed.Test Plan
go test ./... -short -race -count=1)go vet ./...cleangofmt -l .clean*_FailLoudregression tests cover every affected functiondocker exec(e.g. by revoking the container ID mid-run) now produces an error instead ofclean[Unreleased] / Fixedentry addedRelated Issues
N/A — surfaced during a v1.0 readiness review; not tracked in an existing issue.