Skip to content

security: harden data-derived paths + fix unchecked writable Close()#60

Merged
ErenAri merged 1 commit into
mainfrom
security/path-hardening
Jun 28, 2026
Merged

security: harden data-derived paths + fix unchecked writable Close()#60
ErenAri merged 1 commit into
mainfrom
security/path-hardening

Conversation

@ErenAri

@ErenAri ErenAri commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

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

  • New internal/safepath.LocalJoin: a filepath.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.
  • Applied at the three sinks whose filename is derived from data, not a human operator:
    • runtime/fetch.go — artifact target name built 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), the highest-value one

Real hygiene — go/unhandled-writable-file-close (5) + go/useless-assignment (1)

  • Surface Close() errors on append-mode writers instead of defer-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.
  • Drop a dead cmd = newCmd assignment in vm/qemu.go (only qemuProc is read afterward).

Intentionally not changed (false positives, to be dismissed with reason)

  • Operator-controlled CLI/config paths: artifact.Inspect, compare.LoadReport, the fetch/audit rotation paths under the configured workdir, and the trusted-signing-key paths in registry/history.go. These name files the operator already owns; forcing containment would break legitimate absolute-path usage.
  • api/report_paths.go:68 — already guarded by validatePathUnderRoots (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 touch pkg/bpfcompat (library mode) or the in-guest validator.

Verification

  • go build ./..., go vet ./..., go test ./... all green.
  • New internal/safepath tests pass.

🤖 Generated with Claude Code

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>
@ErenAri ErenAri merged commit f86d906 into main Jun 28, 2026
7 of 8 checks passed
@ErenAri ErenAri deleted the security/path-hardening branch June 28, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant