feat: add ant update command and --version flag#24
feat: add ant update command and --version flag#24jacderida merged 3 commits intoWithAutonomi:mainfrom
Conversation
Checks GitHub Releases for a newer version of the ant binary, downloads it if available, and replaces the current executable in place. Uses the self-replace crate for cross-platform binary replacement (handles Windows exe locking). Supports --force to re-download and --json for structured output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
-V shows the short version (ant 0.1.1), --version shows the full version with project description, repository URL, and licence. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Nic-dorman
left a comment
There was a problem hiding this comment.
Full Code Review — PR #24: ant update command and --version flag
PR: #24
Author: jacderida
Base: main ← feat-update_cmd
Scope: 10 files, +485 / -15 (net +470)
1. Scope & Architecture
The PR adds three things:
ant update— self-update command that queries GitHub Releases, downloads the platform archive, extracts it, and atomically replaces the running binary usingself-replace.--version/-Vflags — short and long version output with metadata (repo URL, licence).extract_tar_gz/extract_ziprefactoring — parameterises the binary name so the update module can reuse these functions.
Layering is clean. Core logic (check_for_update, perform_update, build_download_url) lives in ant-core/src/update.rs. The CLI adapter in ant-cli/src/commands/update.rs handles presentation only (progress bars, JSON vs human output, force-flag logic). This means future consumers of ant-core (e.g., a GUI updater) can reuse the core without pulling in CLI concerns.
Extraction reuse is well done. Rather than duplicating tar/zip extraction, the PR parameterises the existing functions with a binary_name argument, makes them pub, and calls them from both the node downloader and the update module. The path traversal protections in extract_tar_gz and the enclosed_name() safety in extract_zip are inherited automatically.
2. Security
2.1 No integrity verification on downloaded binary (High)
This is the most significant finding in the review.
The release pipeline invests heavily in artifact integrity:
- ML-DSA-65 post-quantum
.sigsignatures viaant-keygen - DigiCert EV code signing on Windows
SHA256SUMS.txtchecksums for all archives
The self-update bypasses all of this. The flow is:
GitHub Releases API -> construct URL -> download archive -> extract -> self_replace
No checksum verification. No signature check. The only protection is HTTPS transport security.
Attack surface: A compromised GitHub token, a supply-chain attack on CI, or a compromised CDN edge node could serve a malicious binary. Since ant handles wallet private keys (SECRET_KEY) and financial transactions, this is a high-value target.
Recommended fix (minimal, ~15 lines):
// In download_and_extract_cli, after downloading the archive:
let checksums_url = format!(
"https://github.com/{GITHUB_REPO}/releases/download/{TAG_PREFIX}{version}/SHA256SUMS.txt"
);
let checksums = client.get(&checksums_url)...;
let expected = parse_checksum_for(&asset_name, &checksums_body)?;
let actual = sha256_digest(&bytes);
if expected != actual {
return Err(Error::UpdateFailed(
"checksum mismatch - archive may be corrupted or tampered".into()
));
}Recommended fix (stronger): Ship the release-signing-key.pub as an embedded constant and verify the .sig file against the archive. This makes ant update as trustworthy as ant-keygen verify.
2.2 Path traversal protection is inherited (Good)
The extract_tar_gz function checks for Component::ParentDir in all archive entries. The extract_zip function uses enclosed_name(), which is the zip crate's safe API. Since update.rs calls these functions directly, the update path is protected against zip-slip style attacks. No action needed.
2.3 HTTPS-only download URLs (Good)
All URLs are hardcoded to https://github.com/.... No user-controlled URL input. Transport security is enforced.
3. Correctness
3.1 Asset naming matches CI exactly (Good)
The cli_platform_asset_name() function constructs names like ant-{version}-x86_64-unknown-linux-musl.tar.gz. Verified against the actual release ant-cli-v0.1.2-rc.1:
ant-0.1.2-rc.1-aarch64-apple-darwin.tar.gz matches
ant-0.1.2-rc.1-x86_64-unknown-linux-musl.tar.gz matches
ant-0.1.2-rc.1-x86_64-pc-windows-msvc.zip matches
The CI release workflow (ant-cli-release.yml) uses the same ant-{version}-{target} pattern. No mismatch.
3.2 Pre-release filtering is correct (Good)
if release["draft"].as_bool().unwrap_or(false)
|| release["prerelease"].as_bool().unwrap_or(false)
{
continue;
}Stable users will never be offered an RC. The semver comparison is also correct — even without this filter, 0.2.0-rc.1 < 0.2.0 in semver, so pre-releases would not win the best comparison against the same stable version.
3.3 Version not verified post-extraction (Medium)
The existing download_and_extract in binary.rs (for node binaries) runs binary --version after extraction to confirm the actual version. The update module does not:
// update.rs - perform_update
let new_binary = download_and_extract_cli(download_url, progress).await?;
replace_binary(¤t_exe, &new_binary)?;
Ok(UpdateResult {
previous_version: check.current_version.clone(),
new_version: check.latest_version.clone(), // <- from API, not from the binary
})new_version is what the GitHub API reported, not what was actually extracted. If there is ever a mismatch between the release tag and the binary inside the archive (e.g., a CI build config error), the update would silently install the wrong version and report success.
Running new_binary --version before replace_binary would catch this.
3.4 Testing claim vs pre-release filter (Question)
The PR says: "Manually tested full update flow against ant-cli-v0.1.2-rc.1 RC release."
But ant-cli-v0.1.2-rc.1 is the only release and it is marked prerelease: true. The code skips pre-releases, so fetch_latest_cli_version() should return Err("no ant-cli release found on GitHub").
How was this tested? Possible explanations:
- The release was temporarily unmarked as prerelease
- The filter was temporarily removed during development
- The test used
--forcewith a code path that did not go throughfetch_latest_cli_version
None of these is concerning, but clarification would help reviewers trust the test coverage.
3.5 per_page=50 pagination (Low, future concern)
let url = format!("https://api.github.com/repos/{GITHUB_REPO}/releases?per_page=50");The GitHub API returns releases across all tag prefixes. With ant-core-v*, ant-cli-v*, and any future crate releases, the 50-item window could eventually miss CLI releases. Not urgent (1 release today), but worth a // TODO or bumping to per_page=100 (the API maximum).
4. Code Quality
4.1 Dead UpdateOpts struct
/// Options for the update operation.
pub struct UpdateOpts {
/// Force re-download even if already on the latest version.
pub force: bool,
}Defined in update.rs, never referenced anywhere. The force logic is handled by mutating UpdateCheck in the CLI adapter. Either remove it or wire it into the API.
4.2 update_available mutation from outside the module
// ant-cli/src/commands/update.rs
let mut check = update::check_for_update(current_version).await?;
if !check.update_available && self.force {
check.update_available = true;
check.download_url = Some(update::build_download_url(&check.latest_version)?);
}The CLI adapter reaches into UpdateCheck public fields to override the core library's decision. This works but is fragile — if UpdateCheck fields are ever made private or the struct gains invariants, this breaks.
Suggestion: Either make check_for_update accept a force: bool parameter, or add a method:
impl UpdateCheck {
pub fn force(&mut self) -> Result<()> {
self.update_available = true;
self.download_url = Some(build_download_url(&self.latest_version)?);
Ok(())
}
}4.3 Fixed temp directory
let tmp_dir = std::env::temp_dir().join("ant-update");Two concurrent ant update invocations would collide. Unlikely for a CLI tool, but tempfile::tempdir() (already a dev-dependency) would be strictly better and zero effort.
4.4 _current_exe unused parameter
fn replace_binary(_current_exe: &Path, new_binary: &Path) -> Result<()> {
self_replace::self_replace(new_binary)The first parameter is unused because self_replace determines the current exe internally. The _ prefix suppresses the warning but the parameter exists only for documentation purposes. Acceptable, but removing it and letting self_replace be the obvious single-arg call would be cleaner.
4.5 Error type consistency (Good)
New Error::UpdateFailed(String) variant follows the existing pattern (BinaryResolution(String), HttpRequest(String), etc.). The CLI adapter converts to anyhow::Result, consistent with all other commands. No issues.
4.6 --version implementation (Good)
fn long_version() -> &'static str {
concat!(
env!("CARGO_PKG_VERSION"),
"\n",
"Autonomi network client: ...\n",
"\n",
"Repository: https://github.com/WithAutonomi/ant-client\n",
"License: MIT or Apache-2.0",
)
}Uses concat! + env! for compile-time construction. Standard clap pattern. The version attribute on #[command] gives the short -V form automatically. Clean.
5. API Surface Change
The PR makes two functions pub in ant-core:
// Was: fn extract_tar_gz(data: &[u8], install_dir: &Path) -> Result<PathBuf>
// Now:
pub fn extract_tar_gz(data: &[u8], install_dir: &Path, binary_name: &str) -> Result<PathBuf>
// Was: fn extract_zip(data: &[u8], install_dir: &Path) -> Result<PathBuf>
// Now:
pub fn extract_zip(data: &[u8], install_dir: &Path, binary_name: &str) -> Result<PathBuf>ant-core is published to crates.io (the release workflow calls cargo publish). Making these pub adds them to the public API. The signatures are clean and the functions are genuinely useful, so this is fine — just noting that it is a semver-relevant change (minor version bump territory, which is appropriate for a feature PR).
The existing internal call site in download_and_extract passes BINARY_NAME as the third argument — semantically identical to before. All three test call sites are updated consistently.
6. Testing
What is covered (8 unit tests in update.rs):
| Test | Coverage |
|---|---|
parse_version_valid |
Semver parsing with/without v prefix |
parse_version_invalid |
Rejects garbage input |
version_comparison |
Correct ordering (0.1.0 < 0.2.0 < 1.0.0) |
check_result_no_update |
UpdateCheck struct when already current |
check_result_with_update |
UpdateCheck struct when update available |
platform_asset_name_format |
Asset name starts with ant- and has correct extension |
build_download_url_format |
Full URL matches expected pattern |
update_check_serializes |
JSON round-trip for --json output |
What is not covered:
| Gap | Risk | Notes |
|---|---|---|
fetch_latest_cli_version |
Medium | Would need HTTP mocking (e.g., wiremock). Understandable to skip for now. |
download_and_extract_cli |
Medium | Same — real HTTP required. |
replace_binary / self_replace |
Low | Platform-specific, hard to unit test safely. |
perform_update E2E |
Medium | Covered by manual testing per PR description. |
--force code path |
Low | Simple mutation, but no test exercises the !check.update_available && self.force branch. |
| Error paths (HTTP 404, network timeout, corrupt archive) | Low | Would benefit from mocked tests long-term. |
Existing tests updated correctly:
The three extract_tar_gz tests in binary.rs all pass BINARY_NAME as the new third argument. No behavior change — these are pure signature adaptations.
7. Summary of Findings
| # | Finding | Severity | Category | Recommendation |
|---|---|---|---|---|
| 1 | No checksum or signature verification on downloaded binary | High | Security | Verify SHA256SUMS.txt at minimum; verify .sig ideally |
| 2 | Downloaded binary version not verified before replacement | Medium | Correctness | Run new_binary --version before replace_binary |
| 3 | Testing claim unclear given pre-release filter | Medium | Process | Clarify how E2E test was performed |
| 4 | Dead UpdateOpts struct |
Low | Code quality | Remove |
| 5 | update_available mutation from CLI adapter |
Low | Design | Encapsulate in a method or parameter |
| 6 | Fixed temp directory (ant-update) |
Low | Robustness | Use tempfile::tempdir() |
| 7 | per_page=50 will not scale |
Low | Correctness | Bump to 100 or add pagination |
| 8 | Unused _current_exe parameter |
Nit | Code quality | Remove if not needed for API clarity |
8. Verdict
The feature design is sound and the code is well-structured. The separation between core and CLI is clean, the extraction code reuse avoids duplication, and the platform target mapping matches CI exactly.
Finding #1 (no integrity verification) is the one I would want addressed before merge. The release pipeline invests significant effort in signing — ML-DSA-65, DigiCert EV, SHA256 checksums — and ant update bypasses all of it. For a tool that handles wallet keys and financial transactions, this creates a gap between the security posture of manual installation and self-update. A SHA256 check is ~15 lines and closes the most obvious vector; signature verification closes it fully.
Findings #2-#8 are improvements worth considering but not blocking.
Add ML-DSA-65 signature verification using the ant-client release signing key and ant-release-v1 context. Download and verify the detached .sig file before extracting the archive. Also: verify binary version post-extraction, remove dead UpdateOpts, encapsulate force logic in UpdateCheck::force(), use tempfile::tempdir(), bump per_page to 100, remove unused _current_exe parameter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thanks for the thorough review! All findings have been addressed in commit 67fac5f. Responses to findings#1 — No integrity verification (High) ✅ FixedAdded full ML-DSA-65 signature verification. The update flow now:
The signing context is Tested end-to-end against the #2 — Version not verified post-extraction (Medium) ✅ FixedAfter extraction, the binary is now run with #3 — Testing claim vs pre-release filter (Medium) — ClarifiedThe pre-release filter was temporarily removed during development for testing, then restored. The final code skips drafts and pre-releases as intended. The E2E test against the RC used a temporary commit that was reset before pushing. #4 — Dead
|
Summary
ant updateself-update command that checks GitHub Releases for a newer version, downloads it, and replaces the current binary usingself-replacefor cross-platform support--version/-Vflags with project description, repository URL, and licenceextract_tar_gzandextract_zipinbinary.rsto accept a configurable binary name, enabling reuse by the update moduleDetails
ant updateant-cli-v*release (skips drafts and pre-releases)semvercrateant-{version}-{target_triple}.{tar.gz|zip})self-replacecrate (handles Windows exe locking)--forceto re-download even when already on latest--jsonfor structured outputant-core/src/update.rs, thin CLI adapter inant-cli/src/commands/update.rs--version-V→ short version (ant 0.1.1)--version→ full version with description, repo URL, and licenceTest plan
cargo clippycleancargo fmtcleanant-cli-v0.1.2-rc.1RC release — binary downloaded, extracted, and replaced successfully🤖 Generated with Claude Code