Skip to content

Fix for code scanning alert no. 984: Call to System.IO.Path.Combine #1390

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 3 commits into from
Jul 17, 2025

Conversation

dylan-smith
Copy link
Collaborator

@dylan-smith dylan-smith commented Jul 14, 2025

Fix for https://github.com/github/gh-gei/security/code-scanning/984 and https://github.com/github/gh-gei/security/code-scanning/983

Path.Combine and Path.Join do effectively the same thing, but Path.Combine is susceptible to a potential security vulnerability whereas Path.Join is not. So the best practice is to always use Path.Join.

This should resolve the last 2 outstanding security isseus flagged in this repo.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

dylan-smith and others added 2 commits July 13, 2025 23:15
….Combine

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
….Combine

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@dylan-smith dylan-smith marked this pull request as ready for review July 14, 2025 04:19
@Copilot Copilot AI review requested due to automatic review settings July 14, 2025 04:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses two security vulnerabilities (alerts #984 and #983) by replacing Path.Combine calls with Path.Join to mitigate potential security risks. The change follows security best practices by using the safer Path.Join method which is not susceptible to the same vulnerabilities as Path.Combine.

Key changes:

  • Updated path construction in CLI extension detection logic
  • Modified test helper path generation for temporary repositories

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ado2gh/Program.cs Updates extension path validation to use Path.Join instead of Path.Combine
src/OctoshiftCLI.IntegrationTests/TestHelper.cs Changes temporary repository path creation to use Path.Join

@dylan-smith dylan-smith changed the title Potential fix for code scanning alert no. 984: Call to System.IO.Path.Combine Fix for code scanning alert no. 984: Call to System.IO.Path.Combine Jul 14, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@dylan-smith dylan-smith requested a review from a team July 14, 2025 04:20
Copy link

Unit Test Results

  1 files    1 suites   20s ⏱️
898 tests 898 ✅ 0 💤 0 ❌
899 runs  899 ✅ 0 💤 0 ❌

Results for commit 6a3e12f.

Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
gei 81% 73% 596
bbs2gh 82% 76% 669
ado2gh 84% 78% 618
Octoshift 87% 76% 1439
Summary 84% (7269 / 8607) 76% (1708 / 2258) 3322

@dylan-smith dylan-smith enabled auto-merge July 14, 2025 07:13
@dylan-smith dylan-smith merged commit ca73757 into main Jul 17, 2025
30 checks passed
@dylan-smith dylan-smith deleted the dylan-smith/fix-path-combine branch July 17, 2025 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants