[build] Fix Bazel NuGet push implementation

[titusfortner]

User description

💥 What does this PR do?

• Fix .NET package publishing via NuGet

🔧 Implementation Notes

This fix was verified successful during yesterday's release

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Fix NuGet package publishing via Bazel by correcting runfiles handling

• Add dotnet executable to runfiles dependencies for proper resolution

• Update Unix script to locate runfiles directory dynamically

• Add --skip-duplicate and --no-symbols flags to push commands

---

Diagram Walkthrough

flowchart LR
  A["nuget_push.bzl"] -->|Add dotnet to runfiles| B["Runfiles Dependencies"]
  A -->|Update Unix script| C["Dynamic Runfiles Detection"]
  A -->|Add CLI flags| D["NuGet Push Command"]
  B --> E["Proper Executable Resolution"]
  C --> E
  D --> F["Improved Push Reliability"]

Loading

File Walkthrough

Relevant files

Bug fix

nuget_push.bzl Fix NuGet push runfiles and add CLI flags                                dotnet/private/nuget_push.bzl Fixed runfiles handling by explicitly adding dotnet executable to runfiles list Refactored Unix script to dynamically locate runfiles directory instead of relying on relative paths Added --skip-duplicate and --no-symbols flags to both Unix and Windows NuGet push commands Improved path handling for external repositories in bzlmod by stripping leading ../ from dotnet path +19/-5

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Runfiles path hijack Description: The Unix push script trusts the environment-provided RUNFILES_DIR to construct DOTNET="$RUNFILES_DIR/{dotnet}", which could allow executing an attacker-controlled dotnet binary if RUNFILES_DIR is spoofed in an untrusted execution context (e.g., local runs with a hostile environment). nuget_push.bzl [40-50] Referred Code # Locate runfiles directory if [[ -d "$0.runfiles/_main" ]]; then RUNFILES_DIR="$0.runfiles" elif [[ -n "${{RUNFILES_DIR:-}}" ]]; then RUNFILES_DIR="$RUNFILES_DIR" else RUNFILES_DIR="$(cd "$(dirname "$0")" && pwd)" fi DOTNET="$RUNFILES_DIR/{dotnet}" {push_commands}
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No explicit audit logs: The PR implements NuGet package publishing but does not add explicit audit logging (who/when/what/outcome) beyond whatever the underlying dotnet nuget push emits, which may be insufficient for an auditable trail. Referred Code '"$DOTNET" nuget push "$RUNFILES_DIR/_main/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate --no-symbols'.format(nupkg = nupkg.short_path), ) # External repos in bzlmod have paths like ../repo_name/path, which in runfiles becomes repo_name/path dotnet_runfiles_path = dotnet.short_path.lstrip("../") script_content = """#!/usr/bin/env bash set -euo pipefail # Locate runfiles directory if [[ -d "$0.runfiles/_main" ]]; then RUNFILES_DIR="$0.runfiles" elif [[ -n "${{RUNFILES_DIR:-}}" ]]; then RUNFILES_DIR="$RUNFILES_DIR" else RUNFILES_DIR="$(cd "$(dirname "$0")" && pwd)" fi DOTNET="$RUNFILES_DIR/{dotnet}" {push_commands} """.format( ... (clipped 3 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Fragile path stripping: The new dotnet_runfiles_path = dotnet.short_path.lstrip("../") may strip more characters than intended (it strips any leading . or / characters, not a literal "../" prefix), potentially breaking resolution for some paths and lacking validation/fallback. Referred Code # External repos in bzlmod have paths like ../repo_name/path, which in runfiles becomes repo_name/path dotnet_runfiles_path = dotnet.short_path.lstrip("../") Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured command output: The publish scripts rely on raw dotnet nuget push console output without structured logging controls, so it is unclear whether logs are parseable and whether any sensitive values could be echoed by the tooling in failure scenarios. Referred Code '"$DOTNET" nuget push "$RUNFILES_DIR/_main/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate --no-symbols'.format(nupkg = nupkg.short_path), ) # External repos in bzlmod have paths like ../repo_name/path, which in runfiles becomes repo_name/path dotnet_runfiles_path = dotnet.short_path.lstrip("../") script_content = """#!/usr/bin/env bash set -euo pipefail # Locate runfiles directory if [[ -d "$0.runfiles/_main" ]]; then RUNFILES_DIR="$0.runfiles" elif [[ -n "${{RUNFILES_DIR:-}}" ]]; then RUNFILES_DIR="$RUNFILES_DIR" else RUNFILES_DIR="$(cd "$(dirname "$0")" && pwd)" fi DOTNET="$RUNFILES_DIR/{dotnet}" {push_commands} """.format( ... (clipped 20 lines) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix invalid syntax in shell script Correct the invalid ${{RUNFILES_DIR:-}} bash syntax to ${RUNFILES_DIR:-} in the generated Unix script to prevent a syntax error. dotnet/private/nuget_push.bzl [40-47] # Locate runfiles directory if [[ -d "$0.runfiles/_main" ]]; then RUNFILES_DIR="$0.runfiles" -elif [[ -n "${{RUNFILES_DIR:-}}" ]]; then +elif [[ -n "${RUNFILES_DIR:-}" ]]; then RUNFILES_DIR="$RUNFILES_DIR" else RUNFILES_DIR="$(cd "$(dirname "$0")" && pwd)" fi Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bash syntax error (${{...}}) in the generated script that would cause it to fail, which is a critical bug introduced in the PR.	High
	✅ Correctly resolve package runfile paths Suggestion Impact: The commit implements dynamic runfiles path resolution by introducing `_to_runfiles_path()`, which prefixes `_main/` for main-workspace artifacts and strips the `../repo_name/` prefix for external repos, then uses the computed path for both nupkg and dotnet runfiles locations in the generated scripts (removing the previous hardcoded `RUNFILES_DIR/_main/...` usage). code diff: +def _to_runfiles_path(short_path): + """Convert a short_path to a runfiles path. + + External repos in bzlmod have paths like ../repo_name/path, + which in runfiles becomes repo_name/path. + Main repo paths need _main/ prefix. + """ + if short_path.startswith("../"): + return short_path[3:] + return "_main/" + short_path + def _create_unix_script(ctx, dotnet, nupkg_files): """Create bash script for Unix/macOS/Linux.""" push_commands = [] for nupkg in nupkg_files: + nupkg_runfiles_path = _to_runfiles_path(nupkg.short_path) push_commands.append( - '"$DOTNET" nuget push "$RUNFILES_DIR/_main/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate --no-symbols'.format(nupkg = nupkg.short_path), + '"$DOTNET" nuget push "$RUNFILES_DIR/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate'.format(nupkg = nupkg_runfiles_path), ) - # External repos in bzlmod have paths like ../repo_name/path, which in runfiles becomes repo_name/path - dotnet_runfiles_path = dotnet.short_path.lstrip("../") + dotnet_runfiles_path = _to_runfiles_path(dotnet.short_path) script_content = """#!/usr/bin/env bash set -euo pipefail @@ -43,7 +54,8 @@ elif [[ -n "${{RUNFILES_DIR:-}}" ]]; then RUNFILES_DIR="$RUNFILES_DIR" else - RUNFILES_DIR="$(cd "$(dirname "$0")" && pwd)" + echo "ERROR: Could not locate runfiles directory" >&2 + exit 1 fi DOTNET="$RUNFILES_DIR/{dotnet}" @@ -65,19 +77,19 @@ """Create batch script for Windows.""" push_commands = [] for nupkg in nupkg_files: - nupkg_path = nupkg.short_path.replace("/", "\\") + nupkg_runfiles_path = _to_runfiles_path(nupkg.short_path).replace("/", "\\") push_commands.append( - '"%%DOTNET%%" nuget push "%s" --api-key "%%NUGET_API_KEY%%" --source "%%NUGET_SOURCE%%" --skip-duplicate --no-symbols' % nupkg_path, + '"%%DOTNET%%" nuget push "%%~dp0%s" --api-key "%%NUGET_API_KEY%%" --source "%%NUGET_SOURCE%%" --skip-duplicate' % nupkg_runfiles_path, ) push_commands.append("if %%ERRORLEVEL%% neq 0 exit /b %%ERRORLEVEL%%") - dotnet_path = dotnet.short_path.replace("/", "\\") + dotnet_runfiles_path = _to_runfiles_path(dotnet.short_path).replace("/", "\\") script_content = """@echo off set DOTNET=%~dp0{dotnet_path} {push_commands} """.format( - dotnet_path = dotnet_path, + dotnet_path = dotnet_runfiles_path, push_commands = "\n".join(push_commands), Instead of hardcoding the _main repository prefix for package paths, dynamically determine the correct runfiles path by checking if the package is from the main workspace. dotnet/private/nuget_push.bzl [29-32] for nupkg in nupkg_files: + # The runfiles path for a file from the main workspace is _main/path/to/file, + # but for external workspaces it is other_repo/path/to/file. + if nupkg.owner.workspace_root == "": + nupkg_runfiles_path = "_main/" + nupkg.short_path + else: + nupkg_runfiles_path = nupkg.short_path + push_commands.append( - '"$DOTNET" nuget push "$RUNFILES_DIR/_main/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate --no-symbols'.format(nupkg = nupkg.short_path), + '"$DOTNET" nuget push "$RUNFILES_DIR/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate --no-symbols'.format(nupkg = nupkg_runfiles_path), ) [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion fixes a significant bug where hardcoding the _main repository prefix would cause failures for packages from external repositories, and it provides a robust solution.	High
	✅ Correct prefix stripping Suggestion Impact: The commit removed the unsafe lstrip("../") usage and introduced a helper that uses startswith("../") and slicing (short_path[3:]) to correctly strip a single "../" prefix when present, then updated call sites to use it. code diff: +def _to_runfiles_path(short_path): + """Convert a short_path to a runfiles path. + + External repos in bzlmod have paths like ../repo_name/path, + which in runfiles becomes repo_name/path. + Main repo paths need _main/ prefix. + """ + if short_path.startswith("../"): + return short_path[3:] + return "_main/" + short_path + def _create_unix_script(ctx, dotnet, nupkg_files): """Create bash script for Unix/macOS/Linux.""" push_commands = [] for nupkg in nupkg_files: + nupkg_runfiles_path = _to_runfiles_path(nupkg.short_path) push_commands.append( - '"$DOTNET" nuget push "$RUNFILES_DIR/_main/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate --no-symbols'.format(nupkg = nupkg.short_path), + '"$DOTNET" nuget push "$RUNFILES_DIR/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate'.format(nupkg = nupkg_runfiles_path), ) - # External repos in bzlmod have paths like ../repo_name/path, which in runfiles becomes repo_name/path - dotnet_runfiles_path = dotnet.short_path.lstrip("../") + dotnet_runfiles_path = _to_runfiles_path(dotnet.short_path) Replace lstrip("../") with a safer check using startswith("../") and slicing to remove only a single leading ../ prefix from the path. dotnet/private/nuget_push.bzl [35] -dotnet_runfiles_path = dotnet.short_path.lstrip("../") +if dotnet.short_path.startswith("../"): + dotnet_runfiles_path = dotnet.short_path[3:] +else: + dotnet_runfiles_path = dotnet.short_path [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that lstrip("../") can over-strip characters and replaces it with a safer, more explicit method to remove a single ../ prefix, improving code robustness.	Medium
General	✅ Fix Windows runfiles path Suggestion Impact: The commit introduced a shared helper (_to_runfiles_path) that strips "../" for external repos and applies it to the Windows script's dotnet path (and also to nupkg paths), ensuring correct runfiles resolution on Windows. code diff: +def _to_runfiles_path(short_path): + """Convert a short_path to a runfiles path. + + External repos in bzlmod have paths like ../repo_name/path, + which in runfiles becomes repo_name/path. + Main repo paths need _main/ prefix. + """ + if short_path.startswith("../"): + return short_path[3:] + return "_main/" + short_path + def _create_unix_script(ctx, dotnet, nupkg_files): """Create bash script for Unix/macOS/Linux.""" push_commands = [] for nupkg in nupkg_files: + nupkg_runfiles_path = _to_runfiles_path(nupkg.short_path) push_commands.append( - '"$DOTNET" nuget push "$RUNFILES_DIR/_main/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate --no-symbols'.format(nupkg = nupkg.short_path), + '"$DOTNET" nuget push "$RUNFILES_DIR/{nupkg}" --api-key "$NUGET_API_KEY" --source "$NUGET_SOURCE" --skip-duplicate'.format(nupkg = nupkg_runfiles_path), ) - # External repos in bzlmod have paths like ../repo_name/path, which in runfiles becomes repo_name/path - dotnet_runfiles_path = dotnet.short_path.lstrip("../") + dotnet_runfiles_path = _to_runfiles_path(dotnet.short_path) script_content = """#!/usr/bin/env bash set -euo pipefail @@ -43,7 +54,8 @@ elif [[ -n "${{RUNFILES_DIR:-}}" ]]; then RUNFILES_DIR="$RUNFILES_DIR" else - RUNFILES_DIR="$(cd "$(dirname "$0")" && pwd)" + echo "ERROR: Could not locate runfiles directory" >&2 + exit 1 fi DOTNET="$RUNFILES_DIR/{dotnet}" @@ -65,19 +77,19 @@ """Create batch script for Windows.""" push_commands = [] for nupkg in nupkg_files: - nupkg_path = nupkg.short_path.replace("/", "\\") + nupkg_runfiles_path = _to_runfiles_path(nupkg.short_path).replace("/", "\\") push_commands.append( - '"%%DOTNET%%" nuget push "%s" --api-key "%%NUGET_API_KEY%%" --source "%%NUGET_SOURCE%%" --skip-duplicate --no-symbols' % nupkg_path, + '"%%DOTNET%%" nuget push "%%~dp0%s" --api-key "%%NUGET_API_KEY%%" --source "%%NUGET_SOURCE%%" --skip-duplicate' % nupkg_runfiles_path, ) push_commands.append("if %%ERRORLEVEL%% neq 0 exit /b %%ERRORLEVEL%%") - dotnet_path = dotnet.short_path.replace("/", "\\") + dotnet_runfiles_path = _to_runfiles_path(dotnet.short_path).replace("/", "\\") script_content = """@echo off set DOTNET=%~dp0{dotnet_path} {push_commands} """.format( - dotnet_path = dotnet_path, + dotnet_path = dotnet_runfiles_path, push_commands = "\n".join(push_commands), Apply the same ../ prefix stripping logic from the Unix script to the dotnet.short_path in the Windows script to ensure the executable is correctly located in external repositories. dotnet/private/nuget_push.bzl [74] -dotnet_path = dotnet.short_path.replace("/", "\\") +external_dotnet = dotnet.short_path +if external_dotnet.startswith("../"): + external_dotnet = external_dotnet[3:] +dotnet_path = external_dotnet.replace("/", "\\") [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the path fix for external repositories was only applied for Unix and not for Windows, and provides a necessary correction for feature parity and correctness.	Medium
Update

[Copilot]

Pull request overview

This PR fixes the .NET NuGet package publishing implementation in Bazel by correcting runfiles handling and improving the reliability of push operations. The fix addresses issues with executable resolution and adds safety flags to prevent duplicate uploads.

Changes:

• Fixed runfiles handling by explicitly including the dotnet executable in runfiles dependencies
• Implemented dynamic runfiles directory location for Unix platforms with proper fallback logic
• Added --skip-duplicate and --no-symbols flags to both Unix and Windows push commands for safer operations

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

dotnet/private/nuget_push.bzl:77

• The Windows script uses a different path resolution approach compared to Unix. While Unix now uses RUNFILES_DIR for both dotnet and nupkg files, Windows continues to use relative paths from the script location. This inconsistency could lead to different behavior and potential failures in scenarios where one approach works but the other doesn't, particularly with external repositories or symbolic links. Consider updating the Windows implementation to use a similar runfiles-based approach for consistency and robustness.

            '"%%DOTNET%%" nuget push "%s" --api-key "%%NUGET_API_KEY%%" --source "%%NUGET_SOURCE%%" --skip-duplicate --no-symbols' % nupkg_path,
        )
        push_commands.append("if %%ERRORLEVEL%% neq 0 exit /b %%ERRORLEVEL%%")

    dotnet_path = dotnet.short_path.replace("/", "\\")

    script_content = """@echo off
set DOTNET=%~dp0{dotnet_path}

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.