Skip to content

toolinstall: verify asset checksums and support aqua version_overrides#3046

Merged
dgageot merged 1 commit into
docker:mainfrom
dgageot:toolinstall-checksum-version-overrides
Jun 10, 2026
Merged

toolinstall: verify asset checksums and support aqua version_overrides#3046
dgageot merged 1 commit into
docker:mainfrom
dgageot:toolinstall-checksum-version-overrides

Conversation

@dgageot

@dgageot dgageot commented Jun 10, 2026

Copy link
Copy Markdown
Member

What

Make tool auto-install both more reliable and safer by leveraging the aqua registry more fully:

  1. Checksum verification of downloaded tool binaries before they are extracted, made executable, or linked onto the agent's PATH.
  2. version_overrides support, so per-version package configuration is resolved correctly.

Why

  • The installer previously only understood top-level aqua package fields, so the ~80% of registry packages that keep their config under version_overrides (e.g. fzf) resolved an empty asset and failed to install.
  • Downloaded release assets were installed with no integrity check — a corrupted or tampered download would be extracted and run.

How

  • Parse the aqua checksum schema on Package / Override / VersionOverride. Spool the asset to disk, fetch the release's checksum manifest, hash the asset, and verify before extraction/chmod/symlink.
    • Fails closed when an enabled, supported checksum is advertised.
    • Skips unsupported checksum types and weak (md5/sha1) algorithms with a warning (no false assurance), so installs without usable metadata still succeed.
  • Resolve version_overrides / version_constraint using expr-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/...).
  • Harden resolveForPlatform to never mutate the package's shared Replacements map (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

  • Checksum parsing & end-to-end verification (success, mismatch aborts with nothing installed, disabled, fail-closed on missing manifest, unsupported-type skip).
  • Constraint evaluation (semver / equality / startsWith / in / or / caret / invalid).
  • version_overrides resolution against real registry shapes (fzf old vs newest, per-OS checksum disable, no_asset, per-version type change).
  • Regression test that resolveForPlatform does not mutate shared state.

task build, task lint (0 issues), and task test all pass.

User impact

  • Common CLI tools that previously failed to auto-install now work.
  • Auto-installed binaries are integrity-checked before use.
  • No new configuration or commands — fully automatic.

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).
@dgageot dgageot requested a review from a team as a code owner June 10, 2026 08:30
@aheritier aheritier added area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/feat PR adds a new feature (maps to feat: commit prefix) labels Jun 10, 2026

@docker-agent docker-agent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@dgageot dgageot merged commit 896feab into docker:main Jun 10, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/feat PR adds a new feature (maps to feat: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants