fix(xtask): validate the workspace root at its source (CodeQL path-injection alert) - #21
Merged
Merged
Conversation
CodeQL's rust/path-injection alert flags the kernel image path as depending on a user-provided value: CARGO_MANIFEST_DIR flows into workspace_root() and from there into every constructed path. There is no privilege boundary here β cargo sets the variable and xtask runs with the invoker's own authority β but the honest response is to make the validation real rather than argue with the scanner: the root is now canonicalised and checked to hold the workspace manifest, failing closed with a message that names the actual problem, and build() asserts the image path's containment under the root rather than assuming it. A mangled environment now dies at the source instead of surfacing as a confusing downstream failure. One new test pins the canonical-and-carries-the-manifest behaviour. If the alert survives this shape, the remaining step is a dismissal with the trust-model rationale β the maintainer's call, not code's. Co-Authored-By: Claude Fable 5 <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.
Summary
Addresses the repository's one open GitHub security alert: CodeQL
rust/path-injection(high) atxtask/src/main.rs:244, whereCARGO_MANIFEST_DIRflows into the constructed kernel-image path. No Dependabot or secret-scanning alerts exist.Explanation for the maintainer
The alert is a genuine taint flow but a false positive as a risk: cargo itself sets
CARGO_MANIFEST_DIR, every joined component after the root is a fixed string, and xtask runs with the invoker's own authority β whoever controls that environment already runs arbitrary code as that user. Rather than dismissing, this makes the validation real:workspace_root()canonicalises the path and verifies it holds the workspaceCargo.toml(a mangled environment now fails at the source with a message naming the problem), andbuild()asserts the image path's containment under the root β the guard shape CodeQL documents for this query β rather than assuming it. The trust model is stated in a doc comment so the next reader does not re-litigate it.CodeQL runs on this PR, so whether this shape clears the alert is answered empirically. If the alert survives, the remaining step is dismissing it with the trust-model rationale β that is alert triage on GitHub, so it is yours, not mine. Behavioural note: canonicalisation on Windows yields a
\?\-prefixed path; harmless for cargo and the CRT, visible only in printed paths on a Windows host.Which pillar does this serve?
Toil (tooling hygiene), with the fail-closed instinct the rest of the tree follows.
Borrow Ledger
unsaferegisterunsafeblocks: Noneunsafeblock carries a// SAFETY:comment (n/a)unsafeblock is inside a module CLAUDE.md designates for it (n/a)Checklist
Related
GitHub code-scanning alert #1.
π€ Generated with Claude Code