Skip to content

STAC-25540: keep the private-index credential off the pull request path - #365

Merged
LouisParkin merged 1 commit into
STAC-25463-integrations-github-cifrom
STAC-25540-private-index-off-pr-path
Aug 5, 2026
Merged

STAC-25540: keep the private-index credential off the pull request path#365
LouisParkin merged 1 commit into
STAC-25463-integrations-github-cifrom
STAC-25540-private-index-off-pr-path

Conversation

@LouisParkin

Copy link
Copy Markdown
Contributor

Follow-up to @LouisLotter's second-pass review comment on #362 (checks-tests.yml:389). He was right, and my reply in the workflow was wrong.

What I got wrong

The merged revision (#364) confined the GitLab package registry credential to a single step and stated that reaching it "would require editing this workflow". That was inaccurate, and it understated the problem in two ways:

  1. .setup-scripts/fetch_private_wheels.sh is itself checked out from the pull request head. A PR can edit the script that holds the credential — no workflow edit needed.
  2. source .setup-scripts/setup_env.sh runs as root, in the container, before the credential step. It can overwrite the system interpreter outright, defeating the workspace-interpreter guard that made "system python" look like a guarantee.

More fundamentally: a pull_request run executes the pull request's own copy of the workflow and every script it calls. Repository secrets plus PR-controlled code cannot be arranged into a security boundary, however carefully the code in between is written. The reviewer's wording was precise on exactly this point.

What this changes

The private-index suites no longer run on pull requests at all. They run on push, tag and workflow_dispatch events, whose contents are reviewed before reaching the release branch.

before after
PR run holds the registry secret yes no
vsphere on PRs yes deferred to release branch
vsphere on release branch / tags yes yes
Approval gate removed in #364 still none
  • select-checks.sh moves private-index suites to a new deferred_private_checks output on pull requests, so the matrix is simply empty rather than gated.
  • check-tests-private-index is conditioned on the event; the fork guard is now redundant and removed.
  • fetch_private_wheels.sh refuses to run on a pull_request event, so the rule survives an edit to the workflow condition rather than depending on it.
  • ci-success reports the deferral, so a green PR never implies vsphere was covered when it was not.

The single-step credential handling from #364 is kept. It is genuine hardening — it keeps the password away from the suite's dependency tree, which needed no malice at all to read it — but it is now labelled as defence in depth rather than as the boundary.

The cost, stated plainly

vsphere is verified on the release branch rather than on the PR that changes it. That is a real gap, accepted knowingly: vsphere/ changes a handful of times a year, and most of its current runs are triggered by unrelated CI edits via SHARED_PATHS, not by vsphere changes.

Alternatives considered and rejected for now:

The durable fix

This job should not exist. VMware now publishes the SDK to public PyPI under renamed packages — vmware-vapi-runtime, vmware-vapi-common-client, pyvmomi, all at 9.1.0.0 — and serves the NSX/VMC wheels from its own public PEP 503 index. The names we are blocked on (vsphere-automation-sdk, vapi-runtime) are squatted 0.0.1 placeholders. We are on the private index only because of the ancient vsphere-automation-sdk==1.82.0 pin. Modernising it deletes the credential, this job, and the PR coverage gap in one change. Ticket to follow.

Validation

  • shellcheck clean on both scripts; actionlint clean; Zizmor clean (4 ignored, 2 suppressed).
  • Selector behaviour verified against a real diff:
    • pull_requestprivate_checks=[], deferred_private_checks=["vsphere"], notice emitted.
    • push / workflow_dispatchprivate_checks=["vsphere"], deferred=[].
  • fetch_private_wheels.sh with GITHUB_EVENT_NAME=pull_request exits 1 before touching any credential.

Jira: STAC-25540

The previous revision confined the GitLab package registry credential to a
single step and claimed that reaching it would require editing the workflow.
That claim was wrong. A `pull_request` run executes the pull request's own copy
of the workflow *and* of every script it calls, so a pull request could rewrite
fetch_private_wheels.sh, reorder the steps, or add one of its own. Confining a
secret inside pull-request-controlled code is hardening, not a boundary.

So the private-index suites no longer run on pull requests at all. They run on
push, tag and workflow_dispatch events, whose contents are reviewed before they
reach the release branch, and where the secret is therefore not exposed to
unreviewed code.

  * select-checks.sh moves private-index suites to a new
    `deferred_private_checks` output on pull requests, so the matrix is empty
    rather than gated, and the reason is reported.
  * check-tests-private-index is conditioned on the event, replacing the fork
    guard that is now redundant.
  * fetch_private_wheels.sh refuses to run on a pull_request event, so the rule
    survives an edit to the workflow condition.
  * ci-success reports the deferral, so a green pull request never implies
    vsphere was covered when it was not.

The cost is that vsphere is verified on the release branch rather than on the
pull request that changes it. That is accepted knowingly: the suite changes a
handful of times a year, and the alternatives cost more than they return. The
durable fix is to stop needing the private index -- VMware now publishes this
SDK to public PyPI under renamed packages -- which deletes the credential, the
job and this gap together.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@LouisParkin

Copy link
Copy Markdown
Contributor Author

CI evidence — run 31014217891: 18 success, 1 skipped

The skip is the point, and it is the right skip:

DEFERRED_PRIVATE_CHECKS: ["vsphere"]
check-tests-private      skipped (selected: [], deferred: ["vsphere"])
::notice title=Not covered by this run::["vsphere"] need the private package
registry and do not run on pull requests. They run on the release branch after merge.
All required jobs succeeded; every skip was legitimate.

So on this pull request:

  • no job had a registry secret in scopecheck-tests-private-index was never instantiated;
  • the 14 credential-free suites all ran and passed;
  • CI success distinguished a deferred suite from a missing one, and said so out loud.

That last part is what stops this becoming a silent regression: SELECTED_PRIVATE_CHECKS is [] while DEFERRED_PRIVATE_CHECKS is ["vsphere"], so a green tick can never be read as "vsphere passed".

Local verification

  • shellcheck clean (both scripts), actionlint clean, zizmor clean (4 ignored, 2 suppressed).
  • Selector, against a real diff: pull_requestprivate_checks=[] + deferred=["vsphere"]; push and workflow_dispatchprivate_checks=["vsphere"] + deferred=[].
  • fetch_private_wheels.sh with GITHUB_EVENT_NAME=pull_request exits 1 before the credential-presence check, so it cannot be coaxed into fetching on a PR even if the workflow condition is edited away.

The one thing this run cannot demonstrate is the push path, since that only fires once this lands on STAC-25463-integrations-github-ci. vsphere was proven green on that path in run 31009548529 and the fetch step itself is unchanged here.

@LouisParkin
LouisParkin merged commit d092b06 into STAC-25463-integrations-github-ci Aug 5, 2026
19 checks passed
@LouisParkin
LouisParkin deleted the STAC-25540-private-index-off-pr-path branch August 5, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant