fix(security): validate remote URL and escape commit messages in generate-release.ps1#673
fix(security): validate remote URL and escape commit messages in generate-release.ps1#673xiaolai wants to merge 2 commits intoIvanMurzak:mainfrom
Conversation
…rate-release.ps1 Two content-injection fixes: 1. Validate that $repoUrl matches the expected GitHub URL pattern before making any gh api calls. A tampered git remote (e.g. via `git remote set-url`) could otherwise direct API calls to an attacker-controlled repo. 2. Escape markdown metacharacters ([, ], <, >) in commit messages before writing them to the release notes file. Raw commit subjects are attacker-controlled strings that could inject markdown links or HTML if the release file is later rendered. Co-Authored-By: Claude Code <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the commands/generate-release.ps1 helper script against content injection by validating the origin remote URL before using it in GitHub API requests, and by escaping commit subjects before emitting them into release-note markdown.
Changes:
- Added a guard to reject unexpected
originremote URL formats before callinggh api. - Escaped certain markdown/HTML characters in commit subjects before writing release notes.
| # Validate remote URL is a standard GitHub repo path (guard against tampered remotes) | ||
| if ($repoUrl -notmatch '^https://github\.com/[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$') { | ||
| Write-Error "Unexpected remote URL '$repoUrl' — aborting to prevent unintended GitHub API calls." |
There was a problem hiding this comment.
The remote URL validation only checks that the URL is on github.com and has an owner/repo shape, but it still allows origin to point to a different GitHub repository. That means a tampered remote can still cause gh api repos/$repoPath/... to fetch and embed data from an attacker-controlled repo. Consider sourcing the repo identity from a trusted channel when available (e.g., $env:GITHUB_REPOSITORY in CI) and/or adding an explicit -RepoPath/-RepoUrl parameter, and then validating that the remote-derived value matches it before continuing.
| # Validate remote URL is a standard GitHub repo path (guard against tampered remotes) | ||
| if ($repoUrl -notmatch '^https://github\.com/[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$') { | ||
| Write-Error "Unexpected remote URL '$repoUrl' — aborting to prevent unintended GitHub API calls." | ||
| Pop-Location | ||
| exit 1 | ||
| } |
There was a problem hiding this comment.
The URL regex is very strict and will reject common valid git remote formats like ssh://git@github.com/owner/repo(.git) or a trailing slash. If contributors use those remote URL styles, the script will now fail even though the repo is legitimate. Consider normalizing additional git remote URL formats to the canonical https URL before validation, or extending the validation to accept those equivalent forms.
| # Escape markdown special characters in commit message to prevent content injection | ||
| $safemessage = $message -replace '\[', '\[' -replace '\]', '\]' -replace '<', '<' -replace '>', '>' |
There was a problem hiding this comment.
Escaping </> without escaping & first can be bypassed by a commit subject containing pre-escaped entities like <script>.... Consider performing proper HTML escaping (escape & before </>, or use a built-in HTML encoder) before writing the message into markdown.
| # Escape markdown special characters in commit message to prevent content injection | |
| $safemessage = $message -replace '\[', '\[' -replace '\]', '\]' -replace '<', '<' -replace '>', '>' | |
| # Escape HTML first, then escape markdown link delimiters in the commit message to prevent content injection | |
| $safemessage = [System.Net.WebUtility]::HtmlEncode($message) -replace '\[', '\[' -replace '\]', '\]' |
| $safemessage = $message -replace '\[', '\[' -replace '\]', '\]' -replace '<', '<' -replace '>', '>' | ||
|
|
||
| # Add commit line to release.md | ||
| Add-Content -Path $filename -Value "- [``$shortSha``]($repoUrl/commit/$sha) — $message by @$username" | ||
| Add-Content -Path $filename -Value "- [``$shortSha``]($repoUrl/commit/$sha) — $safemessage by @$username" |
There was a problem hiding this comment.
Variable name $safemessage is inconsistent with the casing used elsewhere in this script (e.g., $repoUrl, $shortSha). Consider renaming to $safeMessage for readability and consistency.
IvanMurzak
left a comment
There was a problem hiding this comment.
There are critical issues were detected by Copilot
Test Results 12 files 468 suites 40m 37s ⏱️ Results for commit 03cea9a. |
Security Issues (Medium)
commands/generate-release.ps1has two related content-injection risks:1. Untrusted
originremote URLThe script calls
git remote get-url originand derives$repoPathfrom the result, which it then passes directly togh api "repos/$repoPath/commits/$sha". If an attacker can modify theoriginremote (e.g. viagit remote set-url origin), the script will make GitHub API calls for an attacker-controlled repo and embed that repo's data into the release notes.2. Unsanitized commit message in markdown output
Commit subjects (
git log --pretty=format:'%s') are written verbatim into the release notes file:A commit subject like
[evil link](https://attacker.example)or<script>alert(1)</script>would be rendered as-is if the markdown file is published or displayed in a web context.Fix
Remote URL validation — added a regex guard immediately after parsing
$repoUrl. If the URL does not match the expectedhttps://github.com/owner/repopattern, the script aborts before making any API call:Commit message escaping — markdown brackets and HTML angle brackets are escaped before writing to the output file:
Both changes are additive — no existing functionality is affected.