Hardening: workflows safety/robustness + installer integrity #1293
Conversation
- Reduce injection risk in workflows (quoting, env use)\n- Add timeouts, concurrency, repo guards\n- Pin actions to SHAs where applicable\n- Harden install.sh (prerelease selection, checksum, trap, pipefail)\n- Add public repo privacy notice to bug report template
There was a problem hiding this comment.
Pull request overview
Hardening-focused PR improving GitHub Actions workflow safety/robustness and strengthening the install.sh installer’s integrity checks.
Changes:
- Harden multiple workflows with repo guards, timeouts, safer quoting/env passing, and action pinning.
- Improve the installer with strict mode, cleanup traps, download retries, prerelease selection logic, and checksum/tarball validation.
- Add documentation/template updates (workflow badges in README; public-repo security notice in bug template).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
install.sh |
Adds strict mode, retries, prerelease selection, checksum validation, tarball validation, and cleanup trap. |
README.md |
Adds workflow status badges. |
.github/workflows/winget.yml |
Hardens release-handling PowerShell logic, adds timeout/repo guard, validates secret, adds download retry loop. |
.github/workflows/unable-to-reproduce-comment.yml |
Adds repo guard + timeout to label-trigger workflow. |
.github/workflows/triage-issues.yml |
Adds concurrency control, repo guard, and timeouts. |
.github/workflows/stale-issues.yml |
Adds repo guard + timeout; pins actions/stale to a SHA. |
.github/workflows/remove-triage-label.yml |
Adds repo guard + timeout; simplifies if condition formatting. |
.github/workflows/on-issue-close.yml |
Adds repo guard + timeout. |
.github/workflows/no-response.yml |
Adds repo guard + timeout; pins actions/stale to a SHA. |
.github/workflows/feature-request-comment.yml |
Adds repo guard + timeout; re-scopes env to step for GH CLI invocation. |
.github/workflows/close-single-word-issues.yml |
Adds timeout; pins actions/github-script; improves robustness with await + logging. |
.github/workflows/close-invalid.yml |
Mitigates shell injection by using issue/PR numbers + repo; adds repo/fork guard + timeout. |
.github/ISSUE_TEMPLATE/bug_report.yml |
Adds public-repo security/privacy notice for reporters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Determine download URL based on VERSION | ||
| if [ "${VERSION}" = "latest" ] || [ -z "$VERSION" ]; then | ||
| DOWNLOAD_URL="https://github.com/github/copilot-cli/releases/latest/download/copilot-${PLATFORM}-${ARCH}.tar.gz" | ||
| CHECKSUMS_URL="https://github.com/github/copilot-cli/releases/latest/download/SHA256SUMS.txt" | ||
| elif [ "${VERSION}" = "prerelease" ]; then |
There was a problem hiding this comment.
set -u makes expanding an unset VERSION a hard error. The first if uses ${VERSION}/$VERSION without defaults, so running the installer without exporting VERSION will now exit with “unbound variable”. Initialize VERSION near the top (e.g., VERSION=${VERSION:-}) or change these expansions to use ${VERSION:-}.
| # GitHub token permissions needed for winget-create to submit a PR | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
There was a problem hiding this comment.
The comment says the job’s “GitHub token permissions” are needed for winget-create to submit a PR, but the workflow actually uses WINGET_CREATE_GITHUB_TOKEN (a secret/PAT) and the permissions: block now only sets contents: read. Please update the comment to reflect which token needs which permissions (or re-add the permission if it’s still required).
There was a problem hiding this comment.
Dete red line 14, add the Green line this is the fix
There was a problem hiding this comment.
Dete red line 14, add the Green line this is the fix
| # Download wingetcreate (with retries) | ||
| $maxRetries = 3 | ||
| $retryDelaySeconds = 5 | ||
| for ($i = 1; $i -le $maxRetries; $i++) { | ||
| try { | ||
| curl.exe -JLO https://aka.ms/wingetcreate/latest | ||
| if (Test-Path "wingetcreate.exe") { break } | ||
| } catch { | ||
| Write-Warning "Download attempt $i failed: $($_.Exception.Message)" |
There was a problem hiding this comment.
The retry loop wraps curl.exe in try/catch, but external executables don’t throw terminating PowerShell exceptions on failure; they set $LASTEXITCODE. This means transient download failures may not be logged as intended, and a non-zero exit can still leave a partial file. Consider using Invoke-WebRequest with -ErrorAction Stop, or explicitly check $LASTEXITCODE/file size and use curl’s --fail/--retry options.
There was a problem hiding this comment.
The retry loop wraps curl.exe in try/catch, but external executables don’t throw terminating PowerShell exceptions on failure; they set $LASTEXITCODE. This means transient download failures may not be logged as intended, and a non-zero exit can still leave a partial file. Consider using Invoke-WebRequest with -ErrorAction Stop, or explicitly check $LASTEXITCODE/file size and use curl’s --fail/--retry options.
There was a problem hiding this comment.
The retry loop wraps curl.exe in try/catch, but external executables don’t throw terminating PowerShell exceptions on failure; they set $LASTEXITCODE. This means transient download failures may not be logged as intended, and a non-zero exit can still leave a partial file. Consider using Invoke-WebRequest with -ErrorAction Stop, or explicitly check $LASTEXITCODE/file size and use curl’s --fail/--retry options.
Configure
There was a problem hiding this comment.
The retry loop wraps curl.exe in try/catch, but external executables don’t throw terminating PowerShell exceptions on failure; they set $LASTEXITCODE. This means transient download failures may not be logged as intended, and a non-zero exit can still leave a partial file. Consider using Invoke-WebRequest with -ErrorAction Stop, or explicitly check $LASTEXITCODE/file size and use curl’s --fail/--retry options.
Configure
|
|
||
| ⚠️ **Security notice**: This is a public repository. Before sharing logs or files, please review them for: | ||
| - API keys, tokens, or credentials | ||
| - Personal identifiable information (PII) |
There was a problem hiding this comment.
Grammar: “Personal identifiable information (PII)” should be “Personally identifiable information (PII)”.
| - Personal identifiable information (PII) | |
| - Personally identifiable information (PII) |
There was a problem hiding this comment.
Delete red 15 , Keep add green 15 use suggest
There was a problem hiding this comment.
Delete red 15 , Keep add green 15
|
hjh |
|
guiuih |
3 participants
This PR focuses on low-risk hardening changes within this repo (workflows + installer script + templates).\n\nHighlights\n- Workflows: reduce script injection patterns (quoting, env passing), add timeouts, add repo guards for scheduled/label workflows, add concurrency for triage, improve github-script robustness (await + logging), add retry when downloading wingetcreate, and pin common actions to SHAs where applied.\n- Installer: add strict mode (set -euo pipefail), cleanup trap, retries for downloads, more reliable prerelease selection (API with git fallback), targeted checksum verification with optional SKIP_CHECKSUM escape hatch, and validate tarball contents before extracting.\n- Templates/docs: add a public-repo privacy notice to bug report template; add workflow status badges to README.\n\nNotes\n- No behavioral change is intended beyond improved safety/robustness and clearer failures.\n- Changes are scoped to files in this repo; no external dependencies were added.\n\nRelated\n- Addresses patterns discussed in #1099.