fix(e2e): run Docker Hub auth from an immutable pinned action - #7079
Conversation
Move the anchored Docker Hub authentication body out of e2e.yaml into .github/scripts/docker-auth-setup.sh, paired with the existing docker-auth-cleanup.sh. The step keeps its credential predicate and secret mapping in YAML and runs the checked-in helper from the trusted checkout, so the ~35 lines of security-critical bash are linted, shell -checked, and unit-tested instead of embedded in workflow YAML. The privileged expect host installs stay inline. Refs #6961 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe E2E workflow replaces inline Docker authentication and host dependency installation with pinned composite actions backed by checked-in scripts. Boundary validators and tests verify immutable provenance, exact wiring, credential gating, package allowlists, and fail-closed execution. ChangesE2E helper action consolidation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LiveJob as GitHub Actions live job
participant AuthAction as docker-auth-setup composite action
participant AuthScript as docker-auth-setup.sh
participant HostAction as host-dependency-setup composite action
participant HostScript as host-dependency-setup.sh
participant DockerHub as Docker Hub
participant Apt as apt package repositories
LiveJob->>AuthAction: Pass guarded authentication inputs
AuthAction->>AuthScript: Execute Docker auth helper
AuthScript->>DockerHub: Retry timed login when required
DockerHub-->>AuthScript: Return login status
LiveJob->>HostAction: Pass validated package list
HostAction->>HostScript: Execute dependency helper
HostScript->>Apt: Retry apt update and install allowlisted packages
Apt-->>HostScript: Return installation status
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yaml:
- Line 309: Bind Docker Hub authentication to the trusted workflow revision:
update .github/workflows/e2e.yaml lines 309-309 to disable credential mapping
for alternate checkouts or load the helper from trusted github.sha; update
tools/e2e/workflow-boundary.mts lines 95-95 so the canonical contract includes
trusted helper provenance, and lines 2002-2036 to reject credential-bearing auth
steps when checkout provenance is not trusted; add a mutation in
test/e2e/support/dockerhub-auth-workflow-boundary.test.ts lines 151-181 proving
alternate checkouts cannot receive Docker Hub credentials.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c65999c5-9a99-4302-b500-241894a81d9b
📒 Files selected for processing (4)
.github/scripts/docker-auth-setup.sh.github/workflows/e2e.yamltest/e2e/support/dockerhub-auth-workflow-boundary.test.tstools/e2e/workflow-boundary.mts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
laitingsheng
added
the
refactor
The auth helper script now runs from the checked-out working tree instead of trusted inline workflow YAML, so a maintainer-dispatched run against an untrusted checkout_sha could load and execute attacker-controlled script content while still receiving real Docker Hub credentials. Gate DOCKERHUB_AUTH_REQUIRED, DOCKERHUB_USERNAME, and DOCKERHUB_TOKEN on inputs.checkout_sha == '', matching the existing NVIDIA_INFERENCE_API_KEY guard in the same job. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Add the composite action wrapper ahead of the workflow switch so the follow-up commit can pin the Docker Hub auth step to this commit's immutable SHA, matching the prepare-e2e and upload-e2e-artifacts convention. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Route the credential-bearing auth step through the docker-auth-setup composite action, pinned to its introducing commit SHA, instead of a run: command resolved from the checked-out workspace. Per issue #6961's acceptance criteria, authentication must execute only from an immutable trusted action revision, never from the tested checkout. Update the workflow-boundary contract and its test to assert the pinned uses/with shape in place of the former run/env shape. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
changed the title
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/source-shape-test-budget.json`:
- Around line 135-136: Update the test title for the immutable reviewed revision
check in test/e2e/support/dockerhub-auth-workflow-boundary.test.ts to end with
the exact suffix “(`#6961`)”, then mirror that complete renamed title in the
corresponding allowlist entry in ci/source-shape-test-budget.json.
In `@test/e2e/support/dockerhub-auth-workflow-boundary.test.ts`:
- Around line 122-124: Update the describe title for the shared Docker Hub
authentication workflow boundary suite to append the required local issue
suffix, leaving the child test title unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2c56c2a5-d99b-46aa-b27e-e0ec8e63a859
📒 Files selected for processing (3)
ci/source-shape-test-budget.jsontest/e2e/support/dockerhub-auth-workflow-boundary.test.tstools/e2e/workflow-boundary.mts
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Protected E2E status for exact head |
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/host-dependency-setup.sh:
- Around line 12-24: Update the HOST_DEPENDENCY_PACKAGES validation before
iterating in the setup script to reject whitespace-only input, reject any
embedded newline or other non-space-separated format, and require at least one
parsed package token. Preserve the existing allowlist validation for each
accepted token.
In `@tools/e2e/workflow-boundary.mts`:
- Around line 697-723: Update validateHostDependencyActionStep to reject any
defined step.continue-on-error value, requiring it to remain unset for
privileged host setup. Add a mutation test that sets continue-on-error: true and
verifies validation fails, matching the existing Docker authentication contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1964fe21-9bcc-48bf-a711-6b3ccb0b5642
📒 Files selected for processing (7)
.github/actions/host-dependency-setup/action.yaml.github/scripts/host-dependency-setup.sh.github/workflows/e2e.yamlci/source-shape-test-budget.jsontest/e2e/support/dockerhub-auth-workflow-boundary.test.tstest/e2e/support/e2e-host-dependency-workflow-boundary.test.tstools/e2e/workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/support/dockerhub-auth-workflow-boundary.test.ts
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
… to hardened revision Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Requesting changes on refreshed head 7050aff88. The immutable action pins, checkout-sha credential guard, private Docker config, exact package mapping, and fail-closed scripts review cleanly, but the new privileged host-dependency helper has no direct behavioral regression test. The current E2E-support coverage validates YAML/provenance mutations only; it never executes .github/scripts/host-dependency-setup.sh. Please add a fake-sudo harness that proves invalid/newline/non-allowlisted packages make zero apt calls, two failed updates then success install exactly expect iptables, and three failed updates exit nonzero without installing. This is required evidence for the new privileged retry/allowlist boundary.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Deferred at exact head a032ad9. The requested privileged-helper fake-sudo, retry/exhaustion, invalid-input, and positional-argument coverage is implemented; security review and every selected E2E lane passed with no skips. Approval remains blocked because the deterministic maintainer checker fails closed when the retry-marked failed coordination check and its validated successful replacement share the exact-diff external ID.
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Approved. Security review: PASS. Docker authentication and host setup execute from immutable pinned actions, and the trust predicates prevent untrusted revisions from receiving secrets or sudo execution. Workflow-boundary and fake-sudo coverage pass; the authorized credential-bearing E2E run remains a merge gate.
6 participants
Summary
Docker Hub authentication and privileged apt host-dependency setup for live E2E jobs now run only from immutable, full-SHA-pinned first-party composite actions. The workflow keeps the trust predicates, secret expressions, and package selections visible as guarded
withinputs, while the pinned actions map those inputs to the reviewed setup helpers. Alternatecheckout_sharuns receive no Docker Hub credentials, and no selected target ref runssudowith its own code.Related Issue
Resolves #6961.
Changes
.github/scripts/docker-auth-setup.shfor a private job-scopedDOCKER_CONFIG, bounded three-attempt--password-stdinlogin, a protected auth-attempt marker, argument rejection, and fail-closed credential handling..github/actions/docker-auth-setup/action.yamland invoke it from the shared E2E auth anchor at an immutable revision.auth-required,username, andtokenon the trusted NVIDIA repository,main, scheduled/manual execution, and an emptycheckout_sha, so tested alternate checkouts never receive Docker Hub credentials..github/scripts/host-dependency-setup.shand.github/actions/host-dependency-setup/action.yamlfor reviewed apt installs with argument rejection, anexpect/iptablesallowlist guard, and the bounded apt-update retry contract.sudo apt-get … expectblocks (live Deep Agents Code, network-policy, cloud-onboard, issue [DGX Spark][CLI&UX] openclaw tui shows indefinite spinner with no error when inference endpoint is unreachable #4434, OpenClaw TUI) with the pinned host-dependency action, preserving each target condition and the before-workspace-prep ordering.sudocredential and package setup now execute only from immutable trusted action revisions, never from the tested checkout.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project e2e-supportgreen, 1165 tests, including the Docker-auth and host-dependency workflow boundariesnpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes