Repository navigation
Branch-name validation in Refs (path traversal fix) #105
Description
Question
Fix audit finding #1 (critical): CommitInput.branch is untrusted socket input passed unchanged into Refs::write_branch; branch_path() joins it directly under refs/heads, so ../… or an absolute path escapes the ref directory and write_atomic() creates parent dirs and renames a file there — arbitrary filesystem writes as the daemon user.
Scope:
- Validate branch names centrally in every
Refsread/write/resolve path (evidence:crates/agentic-core/src/refs.rs:59,crates/agentic-core/src/refs.rs:88,crates/agenticd/src/commit.rs:67). - Permit only normal relative components; reject
.., absolute paths, empty components, and.tmp. - Regression tests: traversal, absolute path, empty component,
.tmpcollision.
Resolution records: PR link, the validation rule as implemented, test names.
Activity
tonibergholm commentedon Jul 9, 2026
Resolution
Shipped as PR #115 (pending Toni's review/merge per the map's PR gate).
What landed: central validation at the
branch_path()chokepoint inagentic-core::refs— everyRefsread/write/resolve path is covered by one rule, plus explicit validation inwrite_head_symbolic. Permitted:/-separated normal relative components (nested names likefeature/foostay supported). Rejected with typedError::InvalidBranchName: empty names/components,./.., absolute paths, backslashes, control characters,.tmp-suffixed components, and the reserved nameHEAD. Wire classification: non-retryableValidation/invalid_branch_name(typed downcast arm, forced by the exhaustive match). Reads reject loudly rather than returningNone, so traversal probes are distinguishable from missing branches.Tests: hostile-name matrix at the Refs layer including no-file-at-escape-target assertions for relative and absolute escapes; read/resolve rejection with a planted outside file; daemon-level
commit::executeregression with a traversal branch name (typed rejection before staging, nothing written, no HEAD published); wire-classification test. Workspace tests, clippy-D warnings, fmt all green.Facts later tickets depend on:
validate_branch_nameis public inagentic_core::refs— the rollbacktargetparameter flows throughresolve→read_branch, so rollback targets are covered by the same gate with no daemon-side changes.Validate branch names before building ref paths under refs/heads (#115)