Clarify Path.Combine call behavior #21038
Conversation
Updated the name and description to clarify the issue with Path.Combine.
There was a problem hiding this comment.
Pull request overview
This PR updates the CodeQL query metadata for PathCombine.ql to better communicate the actual security/behavior concern with Path.Combine. The change clarifies that the issue is not simply calling the method, but rather that later absolute path arguments can silently discard earlier arguments.
Key Changes:
- Updated the query name from a generic "Call to System.IO.Path.Combine" to a more descriptive warning about the silent argument dropping behavior
- Enhanced the description to explain that absolute paths in later arguments cause earlier arguments to be dropped
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
michaelnebel
left a comment
michaelnebel
left a comment
There was a problem hiding this comment.
Thank you for doing this @jonjanego !
We should probably also add a change note, since we are changing the query metadata (documentation can be seen here)
Co-authored-by: Tom Hvitved <hvitved@github.com>
Updated the `name` and `description` of PathCombine.ql to provide more details about the issue.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
michaelnebel
left a comment
michaelnebel
left a comment
There was a problem hiding this comment.
Once again, thank you for doing this! I really appreciate that you take on the challenge @jonjanego !
Updated the name and description to clarify why using Path.Combine is problematic.
Reflecting detail in https://codeql.github.com/codeql-query-help/csharp/cs-path-combine/