-
Notifications
You must be signed in to change notification settings - Fork 0
Fix: absolute glob-paths silently match no files in publish command #83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
9d416af
Initial plan
Copilot f3a6524
fix: absolute glob-paths now work in FindMatchingFiles
Copilot a96f6c8
fix: address code review - remove { wildcard, improve comments
Copilot d20c0f3
refactor: move glob-matching logic to Utilities/GlobMatcher subsystem
Copilot ede720a
refactor: move PathHelpers from SelfTest to Utilities subsystem
Copilot 1227063
fix: apply code review feedback - definition yamls, root pattern test…
Copilot d894e84
test: replace skipped Unix-only root pattern test with cross-platform…
Copilot 1b51ebc
fix: correct platform-specific path root in forward-slash pattern tes…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| ## Utilities Subsystem | ||
|
|
||
| ### Overview | ||
|
|
||
| The Utilities subsystem provides general-purpose helper classes used by other subsystems | ||
| within VersionMark. It consists of two units: `GlobMatcher`, which implements glob-pattern | ||
| file matching for the Publish mode, and `PathHelpers`, which provides safe path combination | ||
| to protect against path-traversal attacks. | ||
|
|
||
| This subsystem satisfies requirements `VersionMark-Utilities-GlobMatch` and | ||
| `VersionMark-Utilities-SafePath`. | ||
|
|
||
| ### Units | ||
|
|
||
| #### GlobMatcher | ||
|
|
||
| The `GlobMatcher` class (`GlobMatcher.cs`) provides glob-pattern file matching. It exposes | ||
| two methods: `FindMatchingFiles`, which accepts an array of glob patterns and returns a | ||
| sorted, deduplicated list of matching file paths; and `SplitAbsolutePattern`, which splits | ||
| an absolute glob pattern into its root directory and relative pattern components. | ||
|
|
||
| See *GlobMatcher Unit Design* for the full unit design. | ||
|
|
||
| #### PathHelpers | ||
|
|
||
| The `PathHelpers` class (`PathHelpers.cs`) provides a single static method, | ||
| `SafePathCombine`, which safely combines a base path and a relative path while | ||
| preventing path-traversal attacks. It is used by `SelfTest.Validation` when | ||
| constructing paths inside temporary directories. | ||
|
|
||
| See *PathHelpers Unit Design* for the full unit design. | ||
|
|
||
| ### Subsystem Interactions | ||
|
|
||
| `GlobMatcher.FindMatchingFiles` is called by the Cli Subsystem (`Program.RunPublish`) to | ||
| resolve the glob patterns supplied on the command line into a concrete list of JSON capture | ||
| files. `PathHelpers.SafePathCombine` is called by the SelfTest subsystem (`Validation.Run`) | ||
| when constructing paths inside temporary directories. The Utilities subsystem has no | ||
| dependencies on other VersionMark subsystems; it depends only on | ||
| `Microsoft.Extensions.FileSystemGlobbing` for pattern evaluation. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| ### GlobMatcher Unit | ||
|
|
||
| #### Overview | ||
|
|
||
| `GlobMatcher` is a static utility class that provides glob-pattern file matching. It | ||
| supports both relative patterns (evaluated against the current directory) and absolute | ||
| patterns (evaluated from their own root directory), and returns a sorted, deduplicated | ||
| list of full file paths. It uses `Microsoft.Extensions.FileSystemGlobbing` for pattern | ||
| evaluation. | ||
|
|
||
| #### FindMatchingFiles Method | ||
|
|
||
| ```csharp | ||
| internal static List<string> FindMatchingFiles(string[] globPatterns) | ||
| ``` | ||
|
|
||
| Finds all files matching the specified glob patterns and returns them as a sorted list of | ||
| full paths. | ||
|
|
||
| **Processing steps:** | ||
|
|
||
| 1. Iterate over each pattern in `globPatterns`. | ||
| 2. If a pattern is rooted (`Path.IsPathRooted`), call `SplitAbsolutePattern` to obtain the | ||
| root directory and relative pattern, then use a `Matcher` against that directory. | ||
| 3. If the pattern is relative, collect it into a separate list. | ||
| 4. After iterating, if any relative patterns were collected, run a single `Matcher` against | ||
| `Directory.GetCurrentDirectory()` covering all relative patterns. | ||
| 5. Combine all matches into a `HashSet<string>` (case-insensitive) to deduplicate, then | ||
| return the sorted result. | ||
|
|
||
| #### SplitAbsolutePattern Helper | ||
|
|
||
| ```csharp | ||
| internal static (string rootDir, string relativePattern) SplitAbsolutePattern(string absolutePattern) | ||
| ``` | ||
|
|
||
| Splits an absolute glob pattern into its root directory and the relative pattern to be | ||
| passed to the `Matcher`. | ||
|
|
||
| **Algorithm:** | ||
|
|
||
| 1. Determine the path root via `Path.GetPathRoot`. | ||
| 2. Find the index of the first wildcard character (`*`, `?`, or `[`). | ||
| 3. If no wildcard is found, return `(Path.GetDirectoryName, Path.GetFileName)`. | ||
| 4. Find the last directory separator before the wildcard using `LastIndexOfAny` searching | ||
| backwards from the wildcard position. | ||
| 5. Split at that separator, handling the drive-root edge case where the separator is the | ||
| first character (e.g. `/`) or where the root segment lacks a trailing separator (e.g. | ||
| `C:` on Windows). | ||
|
|
||
| #### Design Decisions | ||
|
|
||
| - **Separate absolute and relative handling**: Absolute patterns are rooted at a specific | ||
| directory and must be evaluated there, while relative patterns are evaluated relative to | ||
| the current directory. Separating the two cases avoids incorrect matches. | ||
| - **Single Matcher for relative patterns**: Collecting all relative patterns into one | ||
| `Matcher` run reduces directory enumeration overhead compared to one run per pattern. | ||
| - **Case-insensitive deduplication**: Using a case-insensitive `HashSet` prevents | ||
| duplicates when patterns overlap or when the file system is case-insensitive. | ||
| - **Sorted output**: Returning a sorted list makes the output deterministic, simplifying | ||
| testing and producing a consistent report order. | ||
|
|
||
| `GlobMatcher` is used by `Program.RunPublish` to resolve command-line glob patterns into | ||
| a concrete file list. This satisfies requirements `VersionMark-GlobMatcher-FindFiles` and | ||
| `VersionMark-GlobMatcher-AbsolutePaths`. |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| --- | ||
| sections: | ||
| - title: VersionMark Requirements | ||
| sections: | ||
| - title: Utilities | ||
| requirements: | ||
| - id: VersionMark-Utilities-GlobMatch | ||
| title: The Utilities subsystem shall provide glob-pattern file matching. | ||
| justification: | | ||
| Centralizing glob-pattern file matching in a dedicated subsystem separates | ||
| the matching concern from the CLI dispatch logic, making both easier to test | ||
| and maintain independently. | ||
| children: | ||
| - VersionMark-GlobMatcher-FindFiles | ||
| - VersionMark-GlobMatcher-AbsolutePaths | ||
|
|
||
| - id: VersionMark-Utilities-SafePath | ||
| title: The Utilities subsystem shall provide safe path combination. | ||
| justification: | | ||
| Centralizing safe path combination in a dedicated subsystem makes the | ||
| path-traversal protection reusable across all subsystems that construct | ||
| file paths from partially-trusted input. | ||
| children: | ||
| - VersionMark-PathHelpers-SafeCombine |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| --- | ||
| sections: | ||
| - title: GlobMatcher Unit Requirements | ||
| requirements: | ||
| - id: VersionMark-GlobMatcher-FindFiles | ||
| title: The GlobMatcher class shall find files matching relative glob patterns relative to the current directory. | ||
| justification: | | ||
| Publish mode accepts glob patterns supplied on the command line, which are | ||
| typically relative to the working directory. GlobMatcher must evaluate these | ||
| relative patterns against the current directory so callers do not need to | ||
| resolve them manually. | ||
| tests: | ||
| - GlobMatcher_FindMatchingFiles_RelativePattern_ReturnsMatchingFiles | ||
| - GlobMatcher_FindMatchingFiles_EmptyPatterns_ReturnsEmptyList | ||
| - GlobMatcher_FindMatchingFiles_PatternMatchingNoFiles_ReturnsEmptyList | ||
| - GlobMatcher_FindMatchingFiles_MixedPatterns_ReturnsCombinedFiles | ||
|
|
||
| - id: VersionMark-GlobMatcher-AbsolutePaths | ||
| title: >- | ||
| The GlobMatcher class shall find files matching absolute glob patterns | ||
| regardless of the current working directory. | ||
| justification: | | ||
| CI/CD pipelines frequently pass fully-qualified artifact paths to VersionMark. | ||
| GlobMatcher must evaluate absolute patterns from their own root directory so | ||
| that the caller's current working directory does not affect the result. | ||
| tests: | ||
| - GlobMatcher_FindMatchingFiles_AbsolutePattern_ReturnsMatchingFiles | ||
| - GlobMatcher_FindMatchingFiles_SingleFileAbsolutePath_ReturnsSingleFile | ||
| - GlobMatcher_FindMatchingFiles_MixedPatterns_ReturnsCombinedFiles | ||
| - GlobMatcher_SplitAbsolutePattern_PatternWithWildcard_SplitsCorrectly | ||
| - GlobMatcher_SplitAbsolutePattern_PatternWithoutWildcard_SplitsAtLastSeparator |
File renamed without changes.
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.