toolinstall: verify asset checksums and support aqua version_overrides#3046
Conversation
Add integrity verification for auto-installed tool binaries by leveraging the aqua registry's checksum metadata, and resolve per-version package configuration so the installer works for the majority of registry packages that keep their config in version_overrides. - Parse the aqua "checksum" schema (Package/Override/VersionOverride); spool downloaded assets to disk and verify their digest before extraction/chmod/symlink. Fails closed when an enabled, supported checksum is advertised; skips unsupported types and weak (md5/sha1) algorithms with a warning. The manifest is fetched from the same release, so it guards against in-transit/CDN tampering, not a fully compromised release (pinned digests / cosign-SLSA remain future work). - Implement aqua version_overrides/version_constraint resolution using expr-lang/expr + Masterminds/semver, layering the first matching override (asset/format/files/checksum/type/...) onto the base package. - Harden resolveForPlatform to never mutate the package's shared Replacements map (copy-on-write). - Tests for checksum parsing/verification, constraint evaluation, and override resolution against real registry shapes (fzf/gh/gopls).
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
This PR adds well-structured checksum verification and version_override resolution to the tool installer. The implementation is security-conscious (fail-closed, bounded reads, copy-on-write for shared state, size limits) and the test coverage is thorough. No high or medium severity bugs were found.
| } | ||
| } | ||
|
|
||
| if len(lines) == 1 { |
There was a problem hiding this comment.
[LOW] Single-line checksum heuristic accepts digest without verifying asset name
When the checksum manifest has exactly one non-empty line and that line contains exactly one whitespace-separated field, parseChecksumFile returns that value as the digest for any assetName — no filename match is attempted:
if len(lines) == 1 {
if fields := strings.Fields(lines[0]); len(fields) == 1 {
return fields[0], nil // assetName is ignored
}
}This is intentional for tools that publish a dedicated per-asset checksum file (one digest per file, no filename column). The security invariant relies on the checksum asset template pointing to the correct file. In practice this is fine — but if a registry entry misconfigures the checksum.asset template so it resolves to the wrong single-digest file, the mismatch goes undetected rather than returning an error.
Consider adding an optional assetName cross-check even for single-line files, or documenting the assumption that single-digest files are always asset-specific, to make the intent explicit for future maintainers.
What
Make tool auto-install both more reliable and safer by leveraging the aqua registry more fully:
version_overridessupport, so per-version package configuration is resolved correctly.Why
version_overrides(e.g.fzf) resolved an empty asset and failed to install.How
checksumschema onPackage/Override/VersionOverride. Spool the asset to disk, fetch the release's checksum manifest, hash the asset, and verify before extraction/chmod/symlink.version_overrides/version_constraintusingexpr-lang/expr(aqua's own engine) +Masterminds/semver(already a dependency). The first matching override is layered onto the base package (asset/format/files/checksum/type/...).resolveForPlatformto never mutate the package's sharedReplacementsmap (copy-on-write).Threat model / limitation
The checksum manifest is fetched from the same GitHub release as the asset, so this guards against in-transit / CDN tampering but not a fully compromised release (attacker controls both files). Stronger supply-chain guarantees — pinned digests or cosign/SLSA provenance — remain follow-up work.
Tests
version_overridesresolution against real registry shapes (fzfold vs newest, per-OS checksum disable,no_asset, per-version type change).resolveForPlatformdoes not mutate shared state.task build,task lint(0 issues), andtask testall pass.User impact