From e22e1a698086109269a70c483364c2e80587d4e3 Mon Sep 17 00:00:00 2001 From: Su Yang Date: Mon, 5 Oct 2026 19:57:42 +0800 Subject: [PATCH] fix: preserve literal RPM repository URL arguments Inspired by https://github.com/docker/docker-install/pull/449. Apply reliable POSIX shell quoting to the current dnf5, dnf and yum repository commands, with offline execution tests for exact arguments and literal metacharacters. Signed-off-by: Su Yang (cherry picked from commit 451b2228d10859e516609f779d12c6f3cebe8b41) --- .github/workflows/ci.yml | 13 +++ README.md | 15 +++ install.sh | 26 ++++- scripts/test-repository-url-quoting.py | 152 +++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 3 deletions(-) create mode 100755 scripts/test-repository-url-quoting.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ae499a0..b797bfbe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,3 +50,16 @@ jobs: make build/${{ matrix.channel }}/rootless-install.sh \ ${{ matrix.stable_latest && format('STABLE_LATEST={0}', matrix.stable_latest) || '' }} FORCE_ROOTLESS_INSTALL=1 ./build/${{ matrix.channel }}/rootless-install.sh + + # Execute repository commands against inert mocks; never contact a mirror. + test-repository-url-quoting: + runs-on: ubuntu-latest + timeout-minutes: 2 + steps: + - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3 + - name: Check installer syntax + run: sh -n install.sh + - name: Check RPM repository URL arguments (offline only) + run: | + python3 scripts/test-repository-url-quoting.py + TEST_SHELL=bash python3 scripts/test-repository-url-quoting.py diff --git a/README.md b/README.md index 9135fb86..93c63bf9 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,21 @@ To verify that the install script works amongst the supported operating systems make shellcheck ``` +### Offline RPM repository URL regression checks + +Run the repository URL argument checks with Python 3: + +```shell +python3 scripts/test-repository-url-quoting.py +TEST_SHELL=bash python3 scripts/test-repository-url-quoting.py +``` + +The suite executes the installer's generated shell commands against inert dnf5, +dnf and yum mocks. It checks exact URL arguments, literal special characters, +mirror precedence, staging defaults and both channels. It makes no network +requests, installs no packages and never writes host repository files. These +checks do not validate live RPM repositories or package-manager compatibility. + ## Legal *Brought to you courtesy of our legal counsel. For more context, please see the [NOTICE](NOTICE) document in this repo.* diff --git a/install.sh b/install.sh index 82ec27f4..d4e35e18 100755 --- a/install.sh +++ b/install.sh @@ -215,6 +215,25 @@ command_exists() { command -v "$@" > /dev/null 2>&1 } +# Quote one argument for commands executed through the POSIX shell in $sh_c. +# A closing quote preserves trailing newlines when the result is captured. +shell_quote() ( + value=$1 + printf "'" + while :; do + case "$value" in + *"'"*) + printf '%s' "${value%%\'*}" "'\''" + value=${value#*\'} + ;; + *) + printf "%s'" "$value" + break + ;; + esac + done +) + # version_gte checks if the version specified in $VERSION is at least the given # SemVer (Maj.Minor[.Patch]), or CalVer (YY.MM) version.It returns 0 (success) # if $VERSION is either unset (=latest) or newer or equal than the specified @@ -676,13 +695,14 @@ do_install() { ;; centos|fedora|rhel|rocky) repo_file_url="$DOWNLOAD_URL/linux/$lsb_dist/$REPO_FILE" + repo_file_url_quoted=$(shell_quote "$repo_file_url") ( if ! is_dry_run; then set -x fi if command_exists dnf5; then $sh_c "dnf -y -q --setopt=install_weak_deps=False install dnf-plugins-core" - $sh_c "dnf5 config-manager addrepo --overwrite --save-filename=docker-ce.repo --from-repofile='$repo_file_url'" + $sh_c "dnf5 config-manager addrepo --overwrite --save-filename=docker-ce.repo --from-repofile=$repo_file_url_quoted" if [ "$CHANNEL" != "stable" ]; then $sh_c "dnf5 config-manager setopt \"docker-ce-*.enabled=0\"" @@ -692,7 +712,7 @@ do_install() { elif command_exists dnf; then $sh_c "dnf -y -q --setopt=install_weak_deps=False install dnf-plugins-core" $sh_c "rm -f /etc/yum.repos.d/docker-ce.repo /etc/yum.repos.d/docker-ce-staging.repo" - $sh_c "dnf config-manager --add-repo $repo_file_url" + $sh_c "dnf config-manager --add-repo $repo_file_url_quoted" if [ "$CHANNEL" != "stable" ]; then $sh_c "dnf config-manager --set-disabled \"docker-ce-*\"" @@ -702,7 +722,7 @@ do_install() { else $sh_c "yum -y -q install yum-utils" $sh_c "rm -f /etc/yum.repos.d/docker-ce.repo /etc/yum.repos.d/docker-ce-staging.repo" - $sh_c "yum-config-manager --add-repo $repo_file_url" + $sh_c "yum-config-manager --add-repo $repo_file_url_quoted" if [ "$CHANNEL" != "stable" ]; then $sh_c "yum-config-manager --disable \"docker-ce-*\"" diff --git a/scripts/test-repository-url-quoting.py b/scripts/test-repository-url-quoting.py new file mode 100755 index 00000000..0d46226f --- /dev/null +++ b/scripts/test-repository-url-quoting.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +"""Offline checks that execute RPM repository commands against inert mocks.""" +import json +import os +from pathlib import Path +import shutil +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parents[1] +INSTALLER = Path(os.environ.get("INSTALLER_UNDER_TEST", ROOT / "install.sh")) +TEST_SHELL = shutil.which(os.environ.get("TEST_SHELL", "sh")) +if TEST_SHELL is None: + raise SystemExit("ERROR: TEST_SHELL was not found") + +# Keep the installer unchanged except for deferring its final entry point. +source = INSTALLER.read_text() +if not source.endswith("\ndo_install\n"): + raise SystemExit("ERROR: expected the installer's final do_install entry point") + +mock_source = """#!/usr/bin/env python3 +import json +import os +from pathlib import Path +import sys + +command = Path(sys.argv[0]).name +args = sys.argv[1:] +with open(os.environ["MOCK_ARGUMENT_LOG"], "a") as log: + log.write(json.dumps({"command": command, "args": args}) + "\\n") +if command == "rm": + expected = ["-f", "/etc/yum.repos.d/docker-ce.repo", "/etc/yum.repos.d/docker-ce-staging.repo"] + if args != expected: + raise SystemExit("ERROR: unexpected removal command in offline test") +elif command not in ("dnf5", "dnf", "yum", "yum-config-manager"): + raise SystemExit("ERROR: forbidden command executed in offline test: " + command) +""" + +wrapper_source = """#!/bin/sh +set -e +. "$TEST_INSTALLER" +# Control host detection only; repository command generation and sh -c execution +# still come from the actual installer. +id() { printf '%s\\n' root; } +get_distribution() { printf '%s\\n' "$TEST_DISTRO"; } +is_wsl() { false; } +check_forked() { :; } +command_exists() { + case "$1" in + dnf5) [ "$TEST_MANAGER" = dnf5 ] ;; + dnf) [ "$TEST_MANAGER" != yum ] ;; + docker|lsb_release) return 1 ;; + *) command -v "$@" >/dev/null 2>&1 ;; + esac +} +dist_version=44 +do_install +""" + +with tempfile.TemporaryDirectory(prefix="docker-install-repository-url-") as temp: + directory = Path(temp) + mock_bin = directory / "bin" + mock_bin.mkdir() + production = directory / "install-functions.sh" + production.write_text(source[:-len("do_install\n")]) + wrapper = directory / "run-installer.sh" + wrapper.write_text(wrapper_source) + mock = directory / "mock-command" + mock.write_text(mock_source) + mock.chmod(0o755) + for command in ("dnf5", "dnf", "yum", "yum-config-manager", "rm", + "curl", "wget", "sudo", "su", "docker", "systemctl"): + (mock_bin / command).symlink_to(mock) + # Exercise the chosen shell for both the installer and its inner sh -c. + (mock_bin / "sh").symlink_to(TEST_SHELL) + log = directory / "arguments.jsonl" + substitution_marker = directory / "substitution-executed" + backtick_marker = directory / "backtick-executed" + default_url = "https://download.docker.com" + special = 'space & single\' double" $HOME $(touch "$TEST_SUBSTITUTION_MARKER") ' + chr(96) + 'touch "$TEST_BACKTICK_MARKER"' + chr(96) + cases = [ + ("default", {}, [], default_url, "docker-ce.repo"), + ("custom URL", {"DOWNLOAD_URL": "https://mirror.invalid/" + special}, + [], "https://mirror.invalid/" + special, "docker-ce.repo"), + ("custom repo file", {"REPO_FILE": special + ".repo"}, + [], default_url, special + ".repo"), + ("URL and repo file", {"DOWNLOAD_URL": "https://mirror.invalid/" + special, + "REPO_FILE": special + ".repo"}, + [], "https://mirror.invalid/" + special, special + ".repo"), + ("newline characters", {"DOWNLOAD_URL": "https://mirror.invalid/a\nb\t", + "REPO_FILE": "line\nfile.repo\n"}, + [], "https://mirror.invalid/a\nb\t", "line\nfile.repo\n"), + ("staging default", {"DOWNLOAD_URL": "https://download-stage.docker.com"}, + [], "https://download-stage.docker.com", "docker-ce-staging.repo"), + ("staging explicit repo", {"DOWNLOAD_URL": "https://download-stage.docker.com", + "REPO_FILE": special + ".repo"}, + [], "https://download-stage.docker.com", special + ".repo"), + ("mirror overrides custom URL", {"DOWNLOAD_URL": "https://mirror.invalid/" + special, + "REPO_FILE": special + ".repo"}, + ["--mirror", "Aliyun"], "https://mirrors.aliyun.com/docker-ce", + special + ".repo"), + ] + checks = 0 + for manager in ("dnf5", "dnf", "yum"): + distro = "centos" if manager == "yum" else "fedora" + for channel in ("stable", "test"): + for label, overrides, arguments, base_url, repo_file in cases: + log.write_text("") + substitution_marker.unlink(missing_ok=True) + backtick_marker.unlink(missing_ok=True) + # Discard inherited VERSION, mirror, dry-run and installer settings. + environment = { + "PATH": str(mock_bin) + os.pathsep + os.environ["PATH"], + "HOME": str(directory), + "TEST_INSTALLER": str(production), + "TEST_MANAGER": manager, + "TEST_DISTRO": distro, + "MOCK_ARGUMENT_LOG": str(log), + "TEST_SUBSTITUTION_MARKER": str(substitution_marker), + "TEST_BACKTICK_MARKER": str(backtick_marker), + **overrides, + } + result = subprocess.run( + [TEST_SHELL, str(wrapper), "--setup-repo", "--channel", channel, *arguments], + env=environment, text=True, capture_output=True, timeout=10) + name = f"{manager}/{channel}/{label}" + if result.returncode: + raise SystemExit(f"FAIL (offline): {name}\n{result.stdout}{result.stderr}") + calls = [json.loads(line) for line in log.read_text().splitlines()] + expected_url = f"{base_url}/linux/{distro}/{repo_file}" + if manager == "dnf5": + expected = {"command": "dnf5", "args": ["config-manager", "addrepo", + "--overwrite", "--save-filename=docker-ce.repo", + "--from-repofile=" + expected_url]} + repository_calls = [call for call in calls if call["command"] == "dnf5" + and "addrepo" in call["args"]] + else: + expected = {"command": "dnf" if manager == "dnf" else "yum-config-manager", + "args": (["config-manager"] if manager == "dnf" else []) + + ["--add-repo", expected_url]} + repository_calls = [call for call in calls if "--add-repo" in call["args"]] + if repository_calls != [expected]: + raise SystemExit(f"FAIL (offline): {name}: URL arguments changed\n" + f"expected={expected!r}\nactual={repository_calls!r}") + if substitution_marker.exists() or backtick_marker.exists(): + raise SystemExit(f"FAIL (offline): {name}: command substitution executed") + if any("docker-ce" in call["args"] and "install" in call["args"] for call in calls): + raise SystemExit(f"FAIL (offline): {name}: setup-repo installed Docker") + checks += 1 + print(f"PASS (offline): {manager}: exact URL argument and no command substitution") + print(f"PASS (offline): {checks} RPM repository cases with {TEST_SHELL}") + print("No network requests, package installation, or host repository changes were performed.")