docs: fix binary download URLs to match release artifact naming#2129
Conversation
The install script used raw uname -s and uname -m values (e.g. Linux-x86_64), but release binaries use Go's GOOS-GOARCH convention (e.g. linux-amd64). Map uname output to the correct lowercase OS and architecture names. Fixes docker#2117 Assisted-By: docker-agent
There was a problem hiding this comment.
Review Summary
Assessment: 🟡 NEEDS ATTENTION
This PR successfully fixes the primary issue (OS/arch naming mismatch), but the installation script has three robustness issues that could affect users on edge-case platforms or during download failures.
Critical Finding
HIGH severity: The curl download lacks error handling, which could lead to installing a corrupted or non-existent binary with sudo privileges.
Additional Findings
MEDIUM severity:
- Missing default case in architecture detection could produce confusing errors on unsupported platforms
- No validation of OS/ARCH variables before URL construction
Recommendation
While these issues don't break the primary use cases (Linux/macOS on x86_64/aarch64), adding basic error handling and validation would significantly improve the user experience and security posture of the installation script.
Note: This is an automated review. All findings have been verified against the actual code changes.
| # Download the latest release | ||
| OS=$(uname -s | tr '[:upper:]' '[:lower:]') | ||
| ARCH=$(uname -m); case "$ARCH" in x86_64) ARCH=amd64;; aarch64) ARCH=arm64;; esac | ||
| curl -L "https://github.com/docker/docker-agent/releases/latest/download/docker-agent-${OS}-${ARCH}" -o docker-agent |
There was a problem hiding this comment.
🔴 HIGH: Missing error handling for curl download failure
The curl command has no error checking. If the download fails (network issue, 404 error, release not published), the script will continue executing chmod +x and sudo mv on a potentially non-existent or corrupted file.
Security risk: This could install a broken binary or, in a MITM attack scenario, blindly install malicious content with sudo privileges.
Recommendation: Add error handling:
curl -fL "https://github.com/docker/docker-agent/releases/latest/download/docker-agent-${OS}-${ARCH}" -o docker-agent || {
echo "Error: Failed to download docker-agent for ${OS}-${ARCH}"
exit 1
}
# Verify the file exists and has content
[ -s docker-agent ] || { echo "Error: Downloaded file is empty"; exit 1; }
chmod +x docker-agent
sudo mv docker-agent /usr/local/bin/| curl -L https://github.com/docker/docker-agent/releases/latest/download/docker-agent-$(uname -s)-$(uname -m) -o docker-agent | ||
| # Download the latest release | ||
| OS=$(uname -s | tr '[:upper:]' '[:lower:]') | ||
| ARCH=$(uname -m); case "$ARCH" in x86_64) ARCH=amd64;; aarch64) ARCH=arm64;; esac |
There was a problem hiding this comment.
🟡 MEDIUM: Missing default case in architecture detection
The case statement only handles x86_64 and aarch64, but doesn't have a default case for unsupported architectures. While macOS ARM returns arm64 (which works because it's already the correct Go arch name), other platforms might return unexpected values like i686, armv7l, etc.
Impact: Users on unsupported platforms will get a confusing 404 error instead of a clear "unsupported platform" message.
Recommendation: Add a default case:
ARCH=$(uname -m)
case "$ARCH" in
x86_64) ARCH=amd64;;
aarch64|arm64) ARCH=arm64;;
*) echo "Error: Unsupported architecture: $ARCH"; exit 1;;
esac| # Download the latest release (adjust URL for your platform) | ||
| curl -L https://github.com/docker/docker-agent/releases/latest/download/docker-agent-$(uname -s)-$(uname -m) -o docker-agent | ||
| # Download the latest release | ||
| OS=$(uname -s | tr '[:upper:]' '[:lower:]') |
There was a problem hiding this comment.
🟡 MEDIUM: No validation of OS and ARCH variables
The script directly uses OS and ARCH variables without validating they contain expected values. If uname -s returns an unsupported OS (e.g., FreeBSD, SunOS), the download URL will be malformed.
Impact: Users on unsupported platforms get a 404 error instead of a clear message about platform support.
Recommendation: Add validation after setting the variables:
OS=$(uname -s | tr '[:upper:]' '[:lower:]')
ARCH=$(uname -m)
case "$ARCH" in
x86_64) ARCH=amd64;;
aarch64|arm64) ARCH=arm64;;
*) echo "Error: Unsupported architecture: $ARCH"; exit 1;;
esac
# Validate OS
case "$OS" in
linux|darwin) ;;
*) echo "Error: Unsupported OS: $OS (supported: linux, darwin)"; exit 1;;
esac
Problem
The installation guide's download script used
$(uname -s)-$(uname -m)which produces OS/arch values likeLinux-x86_64, but the release binaries use Go'sGOOS-GOARCHconvention (e.g.linux-amd64). This made the download command fail on Linux (and potentially other platforms).Fix
uname -sto lowercase (Linux→linux,Darwin→darwin)uname -mto Go arch names (x86_64→amd64,aarch64→arm64)docker-agent-windows-amd64.exe(lowercase)Fixes #2117