including script to gather more metadta in buildbuddy #2260

Merged

mfreeman451 merged 3 commits from refs/pull/2260/head into main

2025-10-02 17:05:35 +00:00

mfreeman451 commented

2025-10-02 16:50:54 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1682
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1682
Original created: 2025-10-02T16:50:54Z
Original updated: 2025-10-02T17:05:39Z
Original head: carverauto/serviceradar:chore/buildbuddy_workspace_update
Original base: main
Original merged: 2025-10-02T17:05:35Z by @mfreeman451

PR Type

Enhancement

Description

Add workspace status script for BuildBuddy metadata collection
Configure Bazel to use workspace status command
Extract Git repository information and version tags

Diagram Walkthrough

flowchart LR
  A["workspace_status.sh"] --> B["Git metadata extraction"]
  B --> C["BuildBuddy integration"]
  D[".bazelrc"] --> E["Workspace status command config"]
  E --> C

File Walkthrough

Relevant files

Enhancement

workspace_status.sh

Add Git metadata extraction script

scripts/workspace_status.sh

Create new bash script to extract Git repository metadata
Generate key-value pairs for repo URL, commit SHA, branch, and tree
status
Include stable version tag and commit SHA for build stamping
Add error handling with set -eo pipefail

+36/-0

Configuration changes

.bazelrc

Configure workspace status command

.bazelrc

Configure Bazel to use workspace status command
Point to the new workspace_status.sh script

+3/-0

Imported from GitHub pull request. Original GitHub pull request: #1682 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1682 Original created: 2025-10-02T16:50:54Z Original updated: 2025-10-02T17:05:39Z Original head: carverauto/serviceradar:chore/buildbuddy_workspace_update Original base: main Original merged: 2025-10-02T17:05:35Z by @mfreeman451 --- ### **PR Type** Enhancement ___ ### **Description** - Add workspace status script for BuildBuddy metadata collection - Configure Bazel to use workspace status command - Extract Git repository information and version tags ___ ### Diagram Walkthrough ```mermaid flowchart LR A["workspace_status.sh"] --> B["Git metadata extraction"] B --> C["BuildBuddy integration"] D[".bazelrc"] --> E["Workspace status command config"] E --> C ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>workspace_status.sh</strong><dd><code>Add Git metadata extraction script</code>                                              </dd></summary> <hr> scripts/workspace_status.sh <ul><li>Create new bash script to extract Git repository metadata<br> <li> Generate key-value pairs for repo URL, commit SHA, branch, and tree <br>status<br> <li> Include stable version tag and commit SHA for build stamping<br> <li> Add error handling with <code>set -eo pipefail</code></ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1682/files#diff-31442ee54466f451fa76cecc2f586dd19991fbe1eb88a7173afdeddb2b1a2b2c">+36/-0</a>    </td> </tr> </table></td></tr><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>.bazelrc</strong><dd><code>Configure workspace status command</code>                                              </dd></summary> <hr> .bazelrc <ul><li>Configure Bazel to use workspace status command<br> <li> Point to the new <code>workspace_status.sh</code> script</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1682/files#diff-544556920c45b42cbfe40159b082ce8af6bd929e492d076769226265f215832f">+3/-0</a>      </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-10-02 16:51:15 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1682#issuecomment-3362155587
Original created: 2025-10-02T16:51:15Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Untrusted script execution Description: Executing an external script via a relative path ('./tools/latest_version_tag.sh') during build can be exploited if the path is writable or the script is untrusted; consider pinning, validating existence, and using an absolute, repo-root-resolved path. workspace_status.sh [34-34] Referred Code `latest_version_tag=$(./tools/latest_version_tag.sh) echo "STABLE_VERSION_TAG $latest_version_tag"`
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
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

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

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1682#issuecomment-3362155587 Original created: 2025-10-02T16:51:15Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td rowspan=1>⚪</td> <td><details><summary><strong>Untrusted script execution </strong></summary><br> <b>Description:</b> Executing an external script via a relative path ('./tools/latest_version_tag.sh') during <br>build can be exploited if the path is writable or the script is untrusted; consider <br>pinning, validating existence, and using an absolute, repo-root-resolved path.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1682/files#diff-31442ee54466f451fa76cecc2f586dd19991fbe1eb88a7173afdeddb2b1a2b2cR34-R34'>workspace_status.sh [34-34]</a></strong><br> <details open><summary>Referred Code</summary> ```shell latest_version_tag=$(./tools/latest_version_tag.sh) echo "STABLE_VERSION_TAG $latest_version_tag" ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </summary></strong> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>No custom compliance provided</strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/'>guide</a> to enable custom compliance check. </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2025-10-02 16:52:29 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1682#issuecomment-3362159284
Original created: 2025-10-02T16:52:29Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Improve script robustness and portability To improve robustness and portability, replace `$(pwd)` with `%workspace%` in `.bazelrc`, make internal script paths relative to the script's location, and use `sed` instead of `perl` to avoid a potential credential leak. Examples: .bazelrc [33] `build --workspace_status_command=$(pwd)/scripts/workspace_status.sh` scripts/workspace_status.sh [16-34] `which perl >/dev/null && perl -pe 's#//.?:.?@#//#' \|\| cat } repo_url=$(git config --get remote.origin.url \| remove_url_credentials) echo "REPO_URL $repo_url" commit_sha=$(git rev-parse HEAD) echo "COMMIT_SHA $commit_sha" git_branch=$(git rev-parse --abbrev-ref HEAD) ... (clipped 9 lines)` Solution Walkthrough: Before: `# .bazelrc build --workspace_status_command=$(pwd)/scripts/workspace_status.sh # scripts/workspace_status.sh function remove_url_credentials() { # Uses perl, falls back to cat (leaking credentials) which perl >/dev/null && perl -pe 's#//.?:.?@#//#' \|\| cat } ... # Path is relative to CWD, not script location latest_version_tag=$(./tools/latest_version_tag.sh)` After: `# .bazelrc build --workspace_status_command=%workspace%/scripts/workspace_status.sh # scripts/workspace_status.sh SCRIPT_DIR=$(dirname "$0") # Or a more robust equivalent function remove_url_credentials() { # Uses sed, which is more portable and avoids leaks sed -E 's\|//[^:]+:[^@]+@\|//\|' } ... # Path is relative to script location latest_version_tag=$($SCRIPT_DIR/../tools/latest_version_tag.sh)` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies multiple critical issues: a fragile path in `.bazelrc` using `$(pwd)`, a potential credential leak due to a non-portable `perl` dependency, and a non-robust relative path within the script.	High
Security	✅ ~~Avoid potential credential leak~~ Suggestion Impact: The remove_url_credentials function was removed along with the rest of the script, eliminating the insecure perl fallback and thus addressing the credential leak risk by removing the code path entirely. code diff: `-function remove_url_credentials() { - which perl >/dev/null && perl -pe 's#//.?:.?@#//#' \|\| cat -} - -repo_url=$(git config --get remote.origin.url \| remove_url_credentials) -echo "REPO_URL $repo_url"` Replace the `perl` command with `sed` in the `remove_url_credentials` function to prevent a potential credential leak if `perl` is not installed. scripts/workspace_status.sh [15-17] `function remove_url_credentials() { - which perl >/dev/null && perl -pe 's#//.?:.?@#//#' \|\| cat + sed -E 's\|//[^:]+:[^@]+@\|//\|' }` `[Suggestion processed]` Suggestion importance[1-10]: 9 __ Why: This suggestion addresses a significant security risk of leaking credentials into build metadata if `perl` is not present. The proposed fix using `sed` is more portable and eliminates the insecure fallback.	High
Possible issue	Use a workspace-relative path In `.bazelrc`, replace `$(pwd)` with `%workspace%` in the `workspace_status_command` to ensure the path to the script is always relative to the workspace root. .bazelrc [33] `-build --workspace_status_command=$(pwd)/scripts/workspace_status.sh +build --workspace_status_command=%workspace%/scripts/workspace_status.sh` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using `$(pwd)` is fragile and can cause build failures. Replacing it with the Bazel-specific `%workspace%` variable is the standard and more robust approach.	Medium
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1682#issuecomment-3362159284 Original created: 2025-10-02T16:52:29Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Improve script robustness and portability</summary> ___ **To improve robustness and portability, replace <code>$(pwd)</code> with <code>%workspace%</code> in <br><code>.bazelrc</code>, make internal script paths relative to the script's location, and use <br><code>sed</code> instead of <code>perl</code> to avoid a potential credential leak.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1682/files#diff-544556920c45b42cbfe40159b082ce8af6bd929e492d076769226265f215832fR33-R33">.bazelrc [33]</a> </summary> ```bazel build --workspace_status_command=$(pwd)/scripts/workspace_status.sh ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1682/files#diff-31442ee54466f451fa76cecc2f586dd19991fbe1eb88a7173afdeddb2b1a2b2cR16-R34">scripts/workspace_status.sh [16-34]</a> </summary> ```bash which perl >/dev/null && perl -pe 's#//.*?:.*?@#//#' || cat } repo_url=$(git config --get remote.origin.url | remove_url_credentials) echo "REPO_URL $repo_url" commit_sha=$(git rev-parse HEAD) echo "COMMIT_SHA $commit_sha" git_branch=$(git rev-parse --abbrev-ref HEAD) ... (clipped 9 lines) ``` </details> ### Solution Walkthrough: #### Before: ```bash # .bazelrc build --workspace_status_command=$(pwd)/scripts/workspace_status.sh # scripts/workspace_status.sh function remove_url_credentials() { # Uses perl, falls back to cat (leaking credentials) which perl >/dev/null && perl -pe 's#//.*?:.*?@#//#' || cat } ... # Path is relative to CWD, not script location latest_version_tag=$(./tools/latest_version_tag.sh) ``` #### After: ```bash # .bazelrc build --workspace_status_command=%workspace%/scripts/workspace_status.sh # scripts/workspace_status.sh SCRIPT_DIR=$(dirname "$0") # Or a more robust equivalent function remove_url_credentials() { # Uses sed, which is more portable and avoids leaks sed -E 's|//[^:]+:[^@]+@|//|' } ... # Path is relative to script location latest_version_tag=$($SCRIPT_DIR/../tools/latest_version_tag.sh) ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies multiple critical issues: a fragile path in `.bazelrc` using `$(pwd)`, a potential credential leak due to a non-portable `perl` dependency, and a non-robust relative path within the script. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Security</td> <td> <details><summary>✅ <s>Avoid potential credential leak</s></summary> ___ <details><summary><b>Suggestion Impact:</b></summary>The remove_url_credentials function was removed along with the rest of the script, eliminating the insecure perl fallback and thus addressing the credential leak risk by removing the code path entirely. code diff: ```diff -function remove_url_credentials() { - which perl >/dev/null && perl -pe 's#//.*?:.*?@#//#' || cat -} - -repo_url=$(git config --get remote.origin.url | remove_url_credentials) -echo "REPO_URL $repo_url" ``` </details> ___ **Replace the <code>perl</code> command with <code>sed</code> in the <code>remove_url_credentials</code> function to <br>prevent a potential credential leak if <code>perl</code> is not installed.** [scripts/workspace_status.sh [15-17]](https://github.com/carverauto/serviceradar/pull/1682/files#diff-31442ee54466f451fa76cecc2f586dd19991fbe1eb88a7173afdeddb2b1a2b2cR15-R17) ```diff function remove_url_credentials() { - which perl >/dev/null && perl -pe 's#//.*?:.*?@#//#' || cat + sed -E 's|//[^:]+:[^@]+@|//|' } ``` `[Suggestion processed]` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion addresses a significant security risk of leaking credentials into build metadata if `perl` is not present. The proposed fix using `sed` is more portable and eliminates the insecure fallback. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Use a workspace-relative path</summary> ___ **In <code>.bazelrc</code>, replace <code>$(pwd)</code> with <code>%workspace%</code> in the <code>workspace_status_command</code> to <br>ensure the path to the script is always relative to the workspace root.** [.bazelrc [33]](https://github.com/carverauto/serviceradar/pull/1682/files#diff-544556920c45b42cbfe40159b082ce8af6bd929e492d076769226265f215832fR33-R33) ```diff -build --workspace_status_command=$(pwd)/scripts/workspace_status.sh +build --workspace_status_command=%workspace%/scripts/workspace_status.sh ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that using `$(pwd)` is fragile and can cause build failures. Replacing it with the Bazel-specific `%workspace%` variable is the standard and more robust approach. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>