security: harden data-derived paths + fix unchecked writable Close()#60
Merged
Conversation
Addresses CodeQL code-scanning findings on the runtime/registry packages.
Real hardening (path-injection on data-derived components):
- Add internal/safepath.LocalJoin, a filepath.IsLocal-based choke point that
rejects components escaping their base dir (CodeQL-recognized barrier).
- Route the three sinks whose filename comes from data (not a human operator)
through it:
* runtime/fetch.go — artifact target name from registry record fields
* artifact/artifact.go — staged basename from the source path
* runtime/audit.go — decision-trace filename from caller-supplied DecisionID
(reachable from the network-facing API)
Real hygiene (unhandled-writable-file-close):
- Surface Close() errors on append-mode writers instead of deferring and
dropping them, so a failed buffer flush isn't silently lost:
agent/load_ledger.go, cloudregistry/store.go, registry/history.go,
registry/local.go, runtime/audit.go
- Drop a dead assignment (cmd = newCmd) in vm/qemu.go; only qemuProc is read
afterward.
Operator-controlled CLI/config paths and the already-validated
api/report_paths.go sink are intentionally not changed; those alerts are
false positives against existing sanitizers and will be dismissed with reason.
Co-Authored-By: Claude Opus 4.8 <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
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.
Cleans up the open CodeQL code-scanning alerts on the runtime/registry packages. Splits cleanly into real fixes and documented false positives.
Real hardening — path-injection on data-derived components
internal/safepath.LocalJoin: afilepath.IsLocal-based choke point (a CodeQL-recognized traversal barrier) that rejects any component escaping its base directory. Unit-tested for local names,..escapes, and absolute paths.runtime/fetch.go— artifact target name built from registry record fieldsartifact/artifact.go— staged basename from the source pathruntime/audit.go— decision-trace filename from caller-suppliedDecisionID(reachable from the network-facing API), the highest-value oneReal hygiene —
go/unhandled-writable-file-close(5) +go/useless-assignment(1)Close()errors on append-mode writers instead ofdefer-and-drop, so a failed buffer flush isn't silently lost:agent/load_ledger.go,cloudregistry/store.go,registry/history.go,registry/local.go,runtime/audit.go.cmd = newCmdassignment invm/qemu.go(onlyqemuProcis read afterward).Intentionally not changed (false positives, to be dismissed with reason)
artifact.Inspect,compare.LoadReport, thefetch/auditrotation paths under the configured workdir, and the trusted-signing-key paths inregistry/history.go. These name files the operator already owns; forcing containment would break legitimate absolute-path usage.api/report_paths.go:68— already guarded byvalidatePathUnderRoots(EvalSymlinks + double containment check); CodeQL just doesn't recognize the custom barrier.All of these are in
internal/(the frozen runtime agent + VM runner). None touchpkg/bpfcompat(library mode) or the in-guest validator.Verification
go build ./...,go vet ./...,go test ./...all green.internal/safepathtests pass.🤖 Generated with Claude Code