Skip to content

Potential fix for code scanning alert no. 1: Incomplete regular expression for hostnames#116

Merged
mroth merged 2 commits into
mainfrom
alert-autofix-1
May 5, 2026
Merged

Potential fix for code scanning alert no. 1: Incomplete regular expression for hostnames#116
mroth merged 2 commits into
mainfrom
alert-autofix-1

Conversation

@mroth
Copy link
Copy Markdown
Owner

@mroth mroth commented May 5, 2026

Potential fix for https://github.com/mroth/bump/security/code-scanning/1

The best fix is to make the hostname literal in the regex by escaping the dot in github.com.

In git.go, in parseGithubRemote (line 80 in the provided snippet), replace:

  • github.com with github\.com

This keeps existing behavior (matching HTTPS and SSH GitHub remotes and extracting owner/repo) while removing unintended host matches. No new methods, imports, or dependencies are required.

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

…ssion for hostnames

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

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 tightens GitHub remote parsing in the local git-repo detection path by making the hostname match literal, which is intended to prevent non-GitHub hosts from being accepted as github.com remotes.

Changes:

  • Escapes the dot in the parseGithubRemote hostname regex so it matches only github.com.
  • Keeps the existing HTTPS and SCP-style SSH remote parsing flow unchanged otherwise.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread git.go
// git@github.com:mroth/bump.git
func parseGithubRemote(remoteURL string) (owner, repo string, ok bool) {
re := regexp.MustCompile(`^(?:https://|git@)github.com[:/](.*)/(.*?)(?:\.git$|$)`)
re := regexp.MustCompile(`^(?:https://|git@)github\.com[:/](.*)/(.*?)(?:\.git$|$)`)
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added two negative test cases in commit 5708a67:

  • nearMiss_HTTPS_dotReplaced: https://githubXcom/mroth/bump.gitwantOk: false
  • nearMiss_SSH_dotReplaced: git@githubXcom:mroth/bump.gitwantOk: false

These would have matched before the dot was escaped and will now fail CI if the literal \. is ever reverted.

@mroth mroth marked this pull request as ready for review May 5, 2026 14:29
Copilot finished work on behalf of mroth May 5, 2026 14:29
@mroth mroth merged commit ec08266 into main May 5, 2026
5 checks passed
@mroth mroth deleted the alert-autofix-1 branch May 5, 2026 14:30
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.

3 participants