Validate repository names and contain sync target paths #47

New Issue

2026-07-14T17:42:18Z

KofTwentyTwo commented

2026-07-14 17:42:18 +00:00

(Migrated from github.com)

Problem

The public Engine API accepts caller-supplied RepoDescriptor.Name values and combines each name with TargetRoot without enforcing that the name is one safe path segment or that the final path remains under the requested root.

GitHub repository names supplied by the built-in lister are safe, but gclo.Engine is a public NuGet library. A caller can provide a rooted path, parent traversal, or path separators and cause clone/pull operations outside TargetRoot.

Evidence

The public sync overload accepts an arbitrary descriptor list: gclo.Engine/OrgSyncEngine.cs:49-69.
Repository names are passed to Path.Combine and then cloned or pulled without containment validation: gclo.Engine/OrgSyncEngine.cs:95-114.
RepoDescriptor documents Name as a local folder name but performs no validation: gclo.Engine/RepoDescriptor.cs:3-8.

This finding was independently verified at commit 119ac26804ef93b98a262cb1c0c7a41afe71fde5. It is related to the Engine findings collected in #31, but is tracked separately as a public API trust-boundary bug.

Acceptance criteria

Reject empty names, rooted paths, . and .., directory separators, and any value that is not exactly one safe leaf segment.
Resolve the final target with Path.GetFullPath and verify it remains within the normalized target root before any directory creation or git operation.
Validation behaves consistently across Windows path forms, including drive-rooted and UNC inputs.
Unit tests cover parent traversal, absolute paths, both separator styles, rooted Windows paths, and valid GitHub repository names.
Invalid input fails before creating or modifying anything outside the target root and returns an actionable error.

## Problem The public Engine API accepts caller-supplied `RepoDescriptor.Name` values and combines each name with `TargetRoot` without enforcing that the name is one safe path segment or that the final path remains under the requested root. GitHub repository names supplied by the built-in lister are safe, but `gclo.Engine` is a public NuGet library. A caller can provide a rooted path, parent traversal, or path separators and cause clone/pull operations outside `TargetRoot`. ## Evidence - The public sync overload accepts an arbitrary descriptor list: `gclo.Engine/OrgSyncEngine.cs:49-69`. - Repository names are passed to `Path.Combine` and then cloned or pulled without containment validation: `gclo.Engine/OrgSyncEngine.cs:95-114`. - `RepoDescriptor` documents `Name` as a local folder name but performs no validation: `gclo.Engine/RepoDescriptor.cs:3-8`. This finding was independently verified at commit `119ac26804ef93b98a262cb1c0c7a41afe71fde5`. It is related to the Engine findings collected in #31, but is tracked separately as a public API trust-boundary bug. ## Acceptance criteria - Reject empty names, rooted paths, `.` and `..`, directory separators, and any value that is not exactly one safe leaf segment. - Resolve the final target with `Path.GetFullPath` and verify it remains within the normalized target root before any directory creation or git operation. - Validation behaves consistently across Windows path forms, including drive-rooted and UNC inputs. - Unit tests cover parent traversal, absolute paths, both separator styles, rooted Windows paths, and valid GitHub repository names. - Invalid input fails before creating or modifying anything outside the target root and returns an actionable error.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/gclo#47