fix(update): disable unverified self-update#1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b727f05d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fmt.Printf("Updated xgit from %s to %s\n", versionpkg.Version, release.TagName) | ||
| fmt.Printf("Installed binary: %s\n", binaryPath) | ||
| return nil | ||
| return fmt.Errorf("automatic self-update is disabled until release authenticity verification is implemented; download and verify %s manually from the project releases", assetName) |
There was a problem hiding this comment.
With the CLI default in cmd/update.go, CheckOnly is false unless the user explicitly passes --check, so a normal xgit update now prints the available release and then exits non-zero whenever an update exists. That contradicts the new check-only behavior advertised by the help/docs and makes the preserved release-check path fail for the default command; if only automatic installation is disabled, this should error only for the deprecated auto-apply path or otherwise return successfully after reporting the asset.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR mitigates a critical supply-chain risk in xgit update by removing the automatic download-and-replace self-update path and leaving release checking in place until authenticity verification (signatures/checksums/trust anchor) is implemented.
Changes:
- Removed the release asset download/extract/replace flow and replaced it with an “update disabled” error after confirming an expected asset exists.
- Updated CLI help text and user-facing docs to reflect that
xgit updatenow checks releases only. - Updated workflow tests to assert that self-update does not download or install unverified binaries.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates feature description to clarify self-update is disabled pending verification. |
| internal/workflow/update.go | Removes download/apply logic; adds asset existence check and returns an explicit disabled-self-update error. |
| internal/workflow/update_test.go | Replaces install-path tests with a disabled-self-update test ensuring no downloads occur. |
| cmd/update.go | Updates command/flag help text to reflect check-only behavior and deprecate --yes. |
| CHANGELOG.md | Documents the security hardening in Unreleased notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| currentVersion := normalizeVersion(versionpkg.Version) | ||
| targetVersion := normalizeVersion(release.TagName) | ||
| assetName := expectedReleaseAssetName(targetVersion) | ||
| assetURL, err := findReleaseAssetURL(release, assetName) | ||
| if err != nil { | ||
| if err := ensureReleaseAssetExists(release, assetName); err != nil { | ||
| return err | ||
| } |
Motivation
browser_download_urland replaced the running binary with downloaded bytes without any checksum or signature verification, enabling arbitrary code execution if a release or asset is compromised.Description
RunUpdatenow performs release checks and verifies the expected asset name exists but returns a clear error and refuses to download or install release binaries until authenticity verification is implemented.findReleaseAssetURL/download/extract/apply flow withensureReleaseAssetExiststo avoid trusting and usingbrowser_download_urlfor installation.cmd/update.goto indicatexgit updateonly checks releases and marks the--yes/auto-apply flag as deprecated/disabled.TestRunUpdateSelfUpdateDisabledto assert the disabled behaviour and updatedREADME.mdandCHANGELOG.mdto document the security change.Testing
go test ./cmdandgo test ./internal/workflowand both package test suites passed.go test ./...to inspect the full suite and the modified packages passed, but unrelated existing failures remain in other packages (internal/gitutilandinternal/validation).git diff --checkto ensure no whitespace/errors were introduced and it completed without new issues.Codex Task