Skip to content

fix(ci): trust the checkout inside image-push-manual's bazel-ci container - #1335

Merged
priyaselvaganesan merged 1 commit into
mainfrom
fix/image-push-manual-safe-directory-6664046
Aug 28, 2026
Merged

fix(ci): trust the checkout inside image-push-manual's bazel-ci container#1335
priyaselvaganesan merged 1 commit into
mainfrom
fix/image-push-manual-safe-directory-6664046

Conversation

@priyaselvaganesan

@priyaselvaganesan priyaselvaganesan commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

TL;DR

event-ledger's manual-dispatch images embed mr-unknown instead of a real commit short SHA. Adds a step to trust the checkout inside the bazel-ci container so git rev-parse stops failing silently.

Additional Details

image-push-manual.yml's push to ncp-dev job runs inside container: ghcr.io/nvidia/nvcf/bazel-ci. actions/checkout clones on the runner host; the container then mounts that checkout under a different UID. Since git 2.35.2 (CVE-2022-24765), git refuses to operate on a repo it does not own ("detected dubious ownership"). tools/workspace_status.sh's git rev-parse --short HEAD 2>/dev/null || echo "unknown" swallows that failure and silently falls back to the literal "unknown", so the job log shows no error at all. This is on top of, not caused by, #1315's --stamp fix: that fix got stamping to actually run, which is what surfaced this second bug.

Added git config --global --add safe.directory "$GITHUB_WORKSPACE" right after Checkout in the push job.

For QA

Verified on a live workflow_dispatch run against event-ledger (run https://github.com/NVIDIA/nvcf/actions/runs/33214488561, on a scratch branch with this same fix): pulled the pushed image, extracted the binary, strings shows mr-0c4521cc matching the real commit short SHA, with zero occurrences of mr-unknown. An equivalent dispatch before this fix (run 33207590871, post-#1315 merge) produced mr-unknown.

Issues

Relates to #315

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@priyaselvaganesan
priyaselvaganesan requested a review from a team as a code owner August 28, 2026 22:23
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: dd2c1c76-39a6-4b4c-98e4-5092715b5c94

📥 Commits

Reviewing files that changed from the base of the PR and between 605e8d3 and a4f2e82.

📒 Files selected for processing (1)
  • .github/workflows/image-push-manual.yml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The manual image push workflow now marks the checked-out workspace as a trusted Git directory inside the container before Git-dependent build steps run.

Changes

Manual image push workflow

Layer / File(s) Summary
Configure Git workspace trust
.github/workflows/image-push-manual.yml
Adds a step that configures $GITHUB_WORKSPACE as a global Git safe directory inside the container.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to a4f2e

This CI-only change addresses commit labeling in manually pushed images, and no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: balajinvda

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits format with the required scope. The fix(ci) type accurately describes the workflow bug fix.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/image-push-manual-safe-directory-6664046

Comment @coderabbitai help to get the list of available commands.

…iner

git rev-parse fails silently with "dubious ownership" (CVE-2022-24765)
when the bazel-ci container job runs against a checkout owned by the
runner host's UID, so workspace_status.sh falls back to the literal
"unknown" and event-ledger's manual-dispatch images embed mr-unknown
instead of a real short SHA.

Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
@priyaselvaganesan
priyaselvaganesan force-pushed the fix/image-push-manual-safe-directory-6664046 branch from a4f2e82 to c035868 Compare August 28, 2026 22:48
@priyaselvaganesan priyaselvaganesan self-assigned this Aug 28, 2026
@priyaselvaganesan
priyaselvaganesan added this pull request to the merge queue Aug 28, 2026
Merged via the queue into main with commit 0bed4a0 Aug 28, 2026
20 checks passed
@priyaselvaganesan
priyaselvaganesan deleted the fix/image-push-manual-safe-directory-6664046 branch August 28, 2026 23:05
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.

2 participants