refactor(ci): pin E2E Docker Hub auth to trusted setup action - #7082
Closed
Shaurya2k06 wants to merge 7 commits into
Closed
refactor(ci): pin E2E Docker Hub auth to trusted setup action#7082Shaurya2k06 wants to merge 7 commits into
Shaurya2k06 wants to merge 7 commits into
Conversation
Extract the shared Docker Hub login/isolation/retry program into a first-party composite action so secret-bearing authentication executes from an immutable revision rather than duplicated workflow YAML. Refs NVIDIA#6961 Signed-off-by: shaurya2k06 <shaurya2k06@gmail.com>
Replace the duplicated inline Docker Hub login program in e2e.yaml and sandbox-images-and-e2e.yaml with the immutable docker-auth-setup action. Credential predicates stay in workflow YAML; boundary validators now assert the pinned action provenance instead of an inline run script. Refs NVIDIA#6961 Signed-off-by: shaurya2k06 <shaurya2k06@gmail.com>
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis change extracts Docker Hub authentication into a pinned composite action, updates E2E workflows to use it, adds provenance and workflow-boundary validation, and expands tests and risk mappings for the new action files. ChangesDocker authentication helper
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant E2EWorkflow
participant DockerAuthSetup
participant DockerHub
participant BoundaryValidator
E2EWorkflow->>DockerAuthSetup: invoke pinned action with auth environment
DockerAuthSetup->>DockerAuthSetup: create and persist isolated DOCKER_CONFIG
DockerAuthSetup->>DockerHub: attempt docker login using password-stdin
DockerHub-->>DockerAuthSetup: login result
DockerAuthSetup-->>E2EWorkflow: continue or fail after retries
BoundaryValidator->>DockerAuthSetup: verify hashes and script contract
BoundaryValidator-->>E2EWorkflow: validate canonical action boundary
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
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: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@test/e2e/support/dockerhub-auth-workflow-boundary.test.ts`:
- Around line 249-250: Update the test around SETUP_HELPER_PATH to remove
executable-mode and source-text assertions, and verify the action contract
through the fake Docker boundary instead. Assert the observable Docker argv and
stdin passed when setup.sh is invoked, including the expected script content or
invocation behavior, and apply the same change to the assertion at the
referenced later location.
🪄 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: 10186381-9a36-4ea0-9a72-b5976bd1c9f4
📒 Files selected for processing (14)
.github/actions/docker-auth-setup/action.yaml.github/actions/docker-auth-setup/setup.sh.github/workflows/e2e.yaml.github/workflows/sandbox-images-and-e2e.yamltest/e2e/support/dockerhub-auth-workflow-boundary.test.tstest/e2e/support/hermes-secret-boundary-workflow.test.tstest/helpers/vitest-watch-triggers.tstest/pr-risk-plan.test.tstest/vitest-watch-triggers.test.tstools/advisors/e2e-recommendations.mtstools/advisors/risk-plan.mtstools/e2e/docker-auth-setup-workflow-boundary.mtstools/e2e/sandbox-images-workflow-boundary.mtstools/e2e/workflow-boundary.mts
Drop executable-bit and setup.sh source-text checks from the Docker Hub auth boundary test; the fake Docker invocation already proves --password-stdin and retry behavior. Refs NVIDIA#6961 Signed-off-by: shaurya2k06 <shaurya2k06@gmail.com>
Shaurya2k06
force-pushed
the
refactor/6961-docker-auth-setup
branch
from
July 17, 2026 07:37
613eab8 to
1662833
Compare
21 tasks
wscurran
added
area: ci
Contributor
|
✨ Thanks for the refactor. Consolidating the duplicated Docker Hub auth setup into a pinned composite action improves maintainability and security. Ready for maintainer review. Related open PRs: Related open issues: |
Collaborator
|
Closing as superseded by merged #7079 and #6996, which close #6961 and provide the complete pinned-action/live-test implementation with the current trusted-checkout boundary and coverage. Rebasing this older branch would overlap that work and weaken current-main behavior. Thank you @Shaurya2k06 for developing the trusted-action direction; the remaining non-overlapping workflow adoption ideas can be carried forward as a narrow follow-up with your contribution credited. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Secret-bearing Docker Hub authentication no longer lives as a duplicated inline
run:program in E2E workflows. Compatible jobs call a pinned first-partydocker-auth-setupaction; credential predicates and secret mapping stay in workflow YAML. Live-Vitest consolidation from #6996 is left as-is.Related Issue
Fixes #6961
Changes
.github/actions/docker-auth-setup/(compositeaction.yaml+setup.sh): job-scopedDOCKER_CONFIGunderRUNNER_TEMP, auth-attempt marker, bounded--password-stdinlogin retries, fail-closed missing-credential path. Current consumers: every image-consuming job in.github/workflows/e2e.yamland.github/workflows/sandbox-images-and-e2e.yaml. Direct inline YAML is insufficient because the same secret-bearing program was aliased ~70 times and must not execute from the tested checkout. Protected bytools/e2e/docker-auth-setup-workflow-boundary.mtsandtest/e2e/support/dockerhub-auth-workflow-boundary.test.ts.&dockerhub-authinline script withuses: NVIDIA/NemoClaw/.github/actions/docker-auth-setup@66cf71b3af6c32ab972a1ce0e505499da0b495f2while keeping the YAML alias and per-workflow env predicates (schedule|workflow_dispatchfor E2E;push|workflow_dispatchfor sandbox images).contentSha256foraction.yamlandsetup.sh), preserving auth-before-pull, auth-before-prepare, artifact-before-cleanup, and no-image exemptions.expect/iptablesinstalls inline (privilegedsudo; existing allowlist tests rejectuses:).Intentionally left explicit
shared-e2eVitest--project+ credential-free tags; live helper hard-codese2e-livegateway-guard-recoverymcp-bridgesecondary credential-window run-t/--no-file-parallelismshapeexpect/iptableshost installsenv -uscrub sets; out of #6961 Work list.github/scripts/docker-auth-cleanup.sh; no secretsType of Change
Quality Gates
NVIDIA/NemoClaw/.github/actions/docker-auth-setup@66cf71b3af6c32ab972a1ce0e505499da0b495f2, never from the tested checkout. Provenance bindsaction.yaml+setup.shdigests; YAML retains credential predicates; cleanup remains the always-run final step.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-support test/e2e/support/dockerhub-auth-workflow-boundary.test.ts test/e2e/support/hermes-secret-boundary-workflow.test.ts test/e2e/support/sandbox-images-workflow-boundary.test.ts test/e2e/support/prepare-e2e-workflow-boundary.test.ts→ 26 passed; related Docker Hub / sandbox-images / prepare-e2e boundary coverage also green.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: N/A; change is CI workflow/action trust-boundary only. Targetede2e-supportboundary tests exercise the contract; fullnpm test/npm run checknot required for this scope.npm run docsbuilds without warnings (doc changes only) — justification: N/A; no docs changes.Signed-off-by: shaurya2k06 shaurya2k06@gmail.com
Summary by CodeRabbit