From e6d160f1ecf0e7735accdca35cf2607d7c6721bc Mon Sep 17 00:00:00 2001 From: Louis Parkin Date: Wed, 5 Aug 2026 14:56:21 +0200 Subject: [PATCH] STAC-25540: fetch private-index wheels, then drop the credential The vsphere suite is the only one that needs a credential: it pins vsphere-automation-sdk==1.82.0, an unmodified upstream VMware wheel that VMware withdrew from public PyPI, so it resolves only from our GitLab Package Registry. Public PyPI serves a 0.0.1 placeholder squatting the name, which installs cleanly and fails much later. setup_artifact_registry.sh left the registry password in a 0600 ~/.netrc for the remainder of the job, so the suite's tests, its tox environment and its whole dependency tree could read it. Test code is PR-authored, which made the credential effectively readable by whoever opened the pull request (STAC-25463 review, P1). That was contained by putting the job behind the private-package-index GitHub Environment, releasing the credential only after a human approved each run. It worked, but SHARED_PATHS covers the CI files, so it fired on roughly one commit in six and blocked authors on their own pull requests. Replace it with a narrower fix. fetch_private_wheels.sh writes the netrc, downloads one hardcoded package set into a wheelhouse under RUNNER_TEMP, deletes the netrc, and rewrites ~/.pip/pip.conf to resolve that wheelhouse via find-links. Everything afterwards runs with no credential on disk and no authenticated index configured, and the environment gate is gone. Details that matter: * The package set is hardcoded, not read from the tree: while the credential is on disk, a pull request must not be able to redirect pip. * --only-binary=:all:, because downloading an sdist executes its setup.py and would hand upstream code a readable ~/.netrc. * A system interpreter, never the toolchain virtualenv, whose pip is built by repository code. The path is resolved rather than hardcoded -- bci/python:3.13 ships /usr/bin/python3.13 and no /usr/bin/python3, and `python3` on PATH is a /usr/local/bin shim -- and the result is then rejected if it resolves inside GITHUB_WORKSPACE, which is what makes "system" a guarantee rather than an assumption. * An EXIT trap revokes the credential on the failure paths too, and the script then asserts the netrc is gone and the wheel actually arrived. Rejected: a separate credentialed job publishing the wheels with upload-artifact. Workflow artifacts on a public repository are anonymously downloadable, so that would have amounted to publicly redistributing a vendor artifact VMware withdrew, under no SPDX licence. Residual risk, recorded in the workflow: a pull request that edits the workflow itself can still reach the secret, since pull_request runs the PR's own copy. That holds for every secret in every repository and is contained by review, not CI. What changed is that ordinary test code and its dependencies no longer see it. setup_artifact_registry.sh is untouched; .gitlab-ci.yml still uses it. Verified: shellcheck and actionlint clean, Zizmor clean (no findings). Verified inside the pinned BCI image that the interpreter resolves to /usr/bin/python3.13 and that a workspace-resident interpreter is refused. Verified that a missing argument, missing credentials, a workspace interpreter and a failed download all exit non-zero with no netrc left behind and the password in no file or log; and that find-links in ~/.pip/pip.conf is honoured with no command-line flags, which is what tox relies on via passenv HOME. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/scripts/select-checks.sh | 9 +- .github/workflows/checks-tests.yml | 97 +++++++++-------- .setup-scripts/fetch_private_wheels.sh | 142 +++++++++++++++++++++++++ vsphere/tox.ini | 18 ++-- 4 files changed, 212 insertions(+), 54 deletions(-) create mode 100755 .setup-scripts/fetch_private_wheels.sh diff --git a/.github/scripts/select-checks.sh b/.github/scripts/select-checks.sh index 13924722..1b844442 100755 --- a/.github/scripts/select-checks.sh +++ b/.github/scripts/select-checks.sh @@ -27,10 +27,11 @@ # # The split is a security boundary, not a convenience. The credential-free suites # run on GitHub-hosted runners with no secrets in scope at all, so a fork PR can -# run them safely. The private-index suites need a registry password written to -# ~/.netrc, which any test code executing afterwards can read, so they run behind -# a protected GitHub Environment that requires a human approval first (STAC-25463 -# review). Keeping them in one matrix would hand that credential to every suite. +# run them safely. The private-index suites need a registry password, and any test +# code executing alongside it could read it, so they run in a separate job that +# downloads the private packages into a local wheelhouse and destroys the +# credential before the suite starts (STAC-25463 review, STAC-25540). Keeping them +# in one matrix would hand that credential to every suite. set -euo pipefail diff --git a/.github/workflows/checks-tests.yml b/.github/workflows/checks-tests.yml index 5cab099f..5a48c84f 100644 --- a/.github/workflows/checks-tests.yml +++ b/.github/workflows/checks-tests.yml @@ -71,16 +71,22 @@ name: Check tests # least-privilege by construction; this repo's *publishing* role is still # deferred, per the note above. # -# Scoping it to a single step is not enough, because the script writes it to -# ~/.netrc and the suite's own test code runs afterwards in the same job and can -# read it. So the suites that need it are split into their own job, -# `check-tests-private-index`, behind the `private-package-index` GitHub -# Environment. Required reviewers there mean the credential is released only -# after a human approves that specific run. The remaining suites never see it. +# Scoping it to a single step was not enough on its own, because the old setup +# script wrote it to ~/.netrc and the suite's own test code runs afterwards in +# the same job and could read it. So the suites that need it are split into +# their own job, `check-tests-private-index`, and that job now downloads the +# private packages into a local wheelhouse and deletes the credential before any +# repository code runs (STAC-25540). The remaining suites never see it. +# +# This replaced the `private-package-index` GitHub Environment, whose required +# reviewers released the credential only after a human approved each run. It +# worked, but SHARED_PATHS covers the CI files, so it fired on roughly one +# commit in six and blocked authors on their own pull requests for no gain the +# wheelhouse does not also provide. # # Residual, and deliberately left alone here: the underlying secrets are still # repo- and org-level, so a pull request that edits this workflow could add a -# new reference outside the gated job. That change is visible in the diff and is +# new reference outside that step. That change is visible in the diff and is # what review and the Zizmor audit are for. Making them environment-only secrets # would remove even that path, but the org-level password is shared with other # repos (stackstate-agent among them), so it needs its own change. @@ -298,26 +304,35 @@ jobs: check-tests-private-index: name: Check tests, private index (${{ matrix.check }}) # Isolated from `check-tests` because this is the only job that handles a - # credential. `setup_artifact_registry.sh` writes the GitLab Package Registry - # password into a 0600 ~/.netrc, and everything that runs afterwards -- the - # suite's own tests, its tox environment, its transitive dependencies -- can - # read that file. Test code is PR-authored, so the credential is effectively - # exposed to whoever opens the pull request. + # credential at all: vsphere pins a package that resolves solely from the + # private GitLab Package Registry. + # + # The credential is confined to a single step (STAC-25540). It is written, + # used for one fixed pip download, and destroyed before any repository code + # runs; the suite then installs from the resulting local wheelhouse with + # nothing to authenticate against. That replaces the protected GitHub + # Environment this job used to sit behind, which answered the same STAC-25463 + # review finding by requiring a human to release every run -- and, because + # SHARED_PATHS covers the CI files, fired on roughly one commit in six and + # blocked authors on their own pull requests. + # + # The fork guard stays here, and only here. A fork PR gets no Actions secrets + # on a public repository, so it could not fetch the wheel in any case; it + # skips this job and still gets a full credential-free verdict from the other + # suites, so nothing silently reports green without tests. # - # Two things contain that (STAC-25463 review): - # * `environment:` puts the job behind a protected GitHub Environment with - # required reviewers, so the credential is only released after a human has - # approved this specific run, having seen the diff. - # * The fork guard stays here, and only here. A fork PR skips this job and - # still gets a full credential-free verdict from the other suites, so - # nothing silently reports green without tests. + # Residual risk, stated plainly: a pull request that edits this workflow can + # still reach the secret, because `pull_request` runs the PR's own copy of the + # workflow. That is true of every secret in every repository and is contained + # by review and branch protection, not by CI. What changed is that the + # credential is no longer readable by ordinary test code and its dependency + # tree, which needed no workflow edit at all. if: >- ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && needs.select-checks.outputs.private_checks != '[]' }} needs: select-checks runs-on: ubuntu-latest timeout-minutes: 45 - environment: private-package-index strategy: fail-fast: false matrix: @@ -345,37 +360,33 @@ jobs: git config --global --add safe.directory '*' source .setup-scripts/setup_env.sh - - name: Configure the private PyPI index (GitLab Package Registry) + - name: Fetch private-index wheels and revoke the credential env: GITLAB_PACKAGE_REGISTRY_PYPI_SIMPLE_URL: ${{ vars.GITLAB_PACKAGE_REGISTRY_PYPI_SIMPLE_URL }} GITLAB_PACKAGE_REGISTRY_USER: ${{ secrets.GITLAB_PACKAGE_REGISTRY_USER }} GITLAB_PACKAGE_REGISTRY_READONLY_PASSWORD: ${{ secrets.GITLAB_PACKAGE_REGISTRY_READONLY_PASSWORD }} - # Mirrors what the GitLab `.linux_test` anchor did immediately before - # `checksdev test`. The script writes ~/.pip/pip.conf and a 0600 ~/.netrc, - # keeping credentials out of the index URL. + # The only step in this workflow with a secret in scope. The script writes + # ~/.netrc, downloads one fixed package set, deletes the netrc, and leaves + # ~/.pip/pip.conf pointing at a local wheelhouse. Everything after it -- + # checksdev, tox, the suite's tests and their dependency tree -- runs with + # no credential on disk and no authenticated index configured. # - # Unguarded, unlike the earlier revision: this job only runs for suites - # that cannot resolve without the private index, so a missing credential - # is a hard configuration error and must fail loudly rather than warn and - # let pip fall through to public PyPI. + # It replaces setup_artifact_registry.sh here, which left the netrc in + # place for the rest of the job (STAC-25463 review P1, STAC-25540). That + # script is untouched and still serves the GitLab pipeline definitions. # - # NOTE: writing these under $HOME is only half the job. tox drops every - # variable absent from a testenv's `passenv`, HOME included, and pip then - # resolves `~` from the passwd database rather than the environment. Because - # a container job here runs with HOME=/github/home instead of the account - # default, a testenv that installs from this index must list HOME in its - # passenv or it will silently read a pip.conf that was never written and fall - # back to public PyPI alone -- with no auth error to show for it. See - # vsphere/tox.ini. + # The wheelhouse lives in RUNNER_TEMP rather than the workspace so it + # cannot be mistaken for repository content or swept into a build. + # + # NOTE: pip.conf is read from $HOME, so tox must still pass HOME into the + # testenv. tox drops every variable absent from `passenv`, and pip then + # resolves `~` from the passwd database rather than the environment -- + # which points at the wrong home in a container job, where HOME is + # /github/home. Without it the suite silently falls back to public PyPI + # and installs the 0.0.1 placeholder. See vsphere/tox.ini. run: | set -eo pipefail - if [ -z "${GITLAB_PACKAGE_REGISTRY_PYPI_SIMPLE_URL}" ] \ - || [ -z "${GITLAB_PACKAGE_REGISTRY_USER}" ] \ - || [ -z "${GITLAB_PACKAGE_REGISTRY_READONLY_PASSWORD}" ]; then - echo "::error title=Private PyPI index not configured::This suite installs from the GitLab Package Registry, but its credentials are not available to this job." - exit 1 - fi - .setup-scripts/setup_artifact_registry.sh + .setup-scripts/fetch_private_wheels.sh "${RUNNER_TEMP}/private-wheels" - name: checksdev test ${{ matrix.check }} env: diff --git a/.setup-scripts/fetch_private_wheels.sh b/.setup-scripts/fetch_private_wheels.sh new file mode 100755 index 00000000..f62319df --- /dev/null +++ b/.setup-scripts/fetch_private_wheels.sh @@ -0,0 +1,142 @@ +#!/usr/bin/env bash +# Makes the packages that exist only in the private GitLab Package Registry +# available to a local wheelhouse, and destroys the credential before returning. +# +# Why this exists (STAC-25540): vsphere pins vsphere-automation-sdk==1.82.0, an +# unmodified upstream VMware wheel that VMware withdrew from public PyPI. We +# self-host it in the GitLab Package Registry only because that org was private; +# public PyPI now serves a 0.0.1 placeholder squatting the name. +# +# The predecessor, setup_artifact_registry.sh, left a 0600 ~/.netrc in place for +# the remainder of the job, so every later step -- the tox environment, the +# suite's own tests, their transitive dependencies -- could read the password. +# Test code is PR-authored, which made that credential effectively readable by +# whoever opened the pull request (STAC-25463 review, P1). +# +# Here the credential exists only for the duration of one pip invocation whose +# package set is fixed below, and pip is then pointed at the resulting wheelhouse +# so the rest of the job resolves offline with nothing to authenticate against. +set -euo pipefail + +WHEELHOUSE_ARG="${1:-}" +if [ -z "${WHEELHOUSE_ARG}" ]; then + echo "usage: ${0##*/} " >&2 + exit 2 +fi + +# Absolute: pip.conf's find-links is resolved against the working directory of +# whichever process reads it, and tox runs pip from the suite directory. +mkdir -p "${WHEELHOUSE_ARG}" +WHEELHOUSE="$(cd "${WHEELHOUSE_ARG}" && pwd)" + +# Hardcoded on purpose, and deliberately not read from the working tree. While +# the credential is on disk, a pull request must not be able to redirect pip at a +# package of its choosing. +PRIVATE_REQUIREMENTS=( + "vsphere-automation-sdk==1.82.0" +) + +for var in GITLAB_PACKAGE_REGISTRY_PYPI_SIMPLE_URL GITLAB_PACKAGE_REGISTRY_USER GITLAB_PACKAGE_REGISTRY_READONLY_PASSWORD; do + if [ -z "${!var:-}" ]; then + echo "::error title=Private PyPI index not configured::${var} is not available to this job, but this suite cannot resolve without the private index." + exit 1 + fi +done + +NETRC="${HOME}/.netrc" +PIP_CONF_DIR="${HOME}/.pip" + +revoke_credential() { + rm -f "${NETRC}" +} +# Covers the error paths too: a failed download must not leave the password on a +# disk that PR-authored test code goes on to run against. +trap revoke_credential EXIT + +# Hostname only; the simple URL carries a path after the first '/'. +NETRC_HOST="${GITLAB_PACKAGE_REGISTRY_PYPI_SIMPLE_URL%%/*}" + +umask 077 +cat > "${NETRC}" </dev/null || true)"; do + if [ -n "${candidate}" ] && [ -x "${candidate}" ]; then + PYTHON="${candidate}" + break + fi +done +if [ -z "${PYTHON}" ]; then + echo "::error title=No system interpreter::Could not locate a python3 to download with." + exit 1 +fi +if [ -n "${GITHUB_WORKSPACE:-}" ]; then + PYTHON_DIR="$(cd "$(dirname "${PYTHON}")" && pwd)" + case "${PYTHON_DIR}/" in + "${GITHUB_WORKSPACE%/}/"*) + echo "::error title=Refusing a workspace interpreter::Resolved python3 at ${PYTHON}, which is inside the checkout and therefore PR-controlled." + exit 1 + ;; + esac +fi + +echo "→ Downloading private-index packages into ${WHEELHOUSE}" +printf ' %s\n' "${PRIVATE_REQUIREMENTS[@]}" +echo " using ${PYTHON}" + +# --only-binary=:all: matters as much as the interpreter choice. Downloading an +# sdist executes its setup.py, so allowing one would hand arbitrary upstream code +# a process with the registry password readable at ~/.netrc. +"${PYTHON}" -m pip download \ + --disable-pip-version-check \ + --no-cache-dir \ + --only-binary=:all: \ + --extra-index-url "https://${GITLAB_PACKAGE_REGISTRY_PYPI_SIMPLE_URL}" \ + --dest "${WHEELHOUSE}" \ + "${PRIVATE_REQUIREMENTS[@]}" + +revoke_credential +trap - EXIT + +if [ -f "${NETRC}" ]; then + echo "::error title=Credential not revoked::${NETRC} still exists after download; refusing to continue." + exit 1 +fi + +# A silent miss here would fall through to public PyPI and install the 0.0.1 +# placeholder, which fails much later and far less legibly. +if ! find "${WHEELHOUSE}" -maxdepth 1 -iname 'vsphere_automation_sdk-*.whl' | grep -q .; then + echo "::error title=Private wheel missing::vsphere-automation-sdk was not downloaded into ${WHEELHOUSE}." + exit 1 +fi + +# Replaces the extra-index-url that setup_artifact_registry.sh used to write. +# Nothing after this point authenticates anywhere: the private packages resolve +# from the local wheelhouse, and everything else still comes from public PyPI. +mkdir -p "${PIP_CONF_DIR}" +cat > "${PIP_CONF_DIR}/pip.conf" <