Skip to content

sbom: T8542: bugfix shadowing cmd() function with cmd variable name - #1216

Merged
c-po merged 1 commit into
vyos:rollingfrom
c-po:sbom-fix
Jun 4, 2026
Merged

sbom: T8542: bugfix shadowing cmd() function with cmd variable name#1216
c-po merged 1 commit into
vyos:rollingfrom
c-po:sbom-fix

Conversation

@c-po

@c-po c-po commented Jun 4, 2026

Copy link
Copy Markdown
Member

Change summary

Commit 75a495b (sbom: T8542: create during ISO assembly) will shadow any subsequent calls to vyos.utils.process.cmd() as cmd has be redefined as variable to hold the CLI command strings for SBOM generation using the syft binary.

This has been fixed by replacing the cmd variable with another name syft_cmd.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Commit 75a495b ("sbom: T8542: create during ISO assembly") will shadow any
subsequent calls to vyos.utils.process.cmd() as cmd has be redefined as
variable to hold the CLI command strings for SBOM generation using the syft
binary.

This has been fixed by replacing the cmd variable with another name syft_cmd.
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 8b422b1a-fe1a-4c7a-8f75-1d625b6e34d0

📥 Commits

Reviewing files that changed from the base of the PR and between c5e5c86 and 3c26a48.

📒 Files selected for processing (1)
  • scripts/image-build/build-vyos-image
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build_iso
  • GitHub Check: codeql-analysis-call / Analyze (python)
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-06-01T00:03:20.253Z
Learnt from: CR
Repo: vyos/vyos-build PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-01T00:03:20.253Z
Learning: TOML files used during build (located in `data/architectures/*.toml`, `data/build-flavors/*.toml`, `data/build-types/*.toml`, and `data/defaults.toml`) are combined by `build-vyos-image` into a single Python dict to instruct `live-build` to create the ISO image

Applied to files:

  • scripts/image-build/build-vyos-image
🔍 Remote MCP

I'll search for relevant context about the SBOM generation tools and the shadowing issue to help inform the review of this PR.## Summary of Additional Context

Based on my search, here is the relevant information for reviewing this PR:

Understanding Syft and SBOM Generation

Syft is a CLI tool for generating SBOMs that supports multiple output formats including CycloneDX and SPDX. Syft is Anchore's open source SBOM generation tool that supports a broad range of package ecosystems and can scan source directories, container images, and archive files. CycloneDX and SPDX are the most popular SBOM formats, both of which Syft supports.

In the VyOS build context, the SBOM generation section uses syft to create software bill of materials for the VyOS ISO in multiple formats (CycloneDX and SPDX) with sed post-processing applied to modify the generated output.

Variable Shadowing Issue Details

The fix addresses a classic variable shadowing problem. Variable shadowing in Python is when a local variable within a function hides a variable from an outer scope because they both have the same name, and if a local variable within a function is given the same name as a global variable, Python shadows the global variable and any use of that variable name within the function definition will reference the newly created local variable.

Variable shadowing can change the outputs of functions and produce unexpected results if you are not aware of it, and it is considered good practice to avoid variable shadowing by following the correct naming conventions and giving identifiers within your code unique names.

In this case, the local variable cmd (holding a list of SBOM generation commands) was shadowing the vyos.utils.process.cmd() function, preventing subsequent function calls from accessing the original function. Renaming the variable to syft_cmd resolves this issue by using a more descriptive name that indicates the variable's purpose while avoiding conflicts with imported or available functions.

Key Points for Review

  • The change is minimal (+7/-7 lines), affecting only the variable name and its iteration in the SBOM generation section
  • The fix improves code clarity by using a descriptive variable name (syft_cmd) that explicitly indicates the variable contains Syft commands
  • This is a standard code quality improvement that prevents runtime errors from function shadowing
  • The related PRs (#1198, #1213) suggest this is part of ongoing refinements to SBOM generation logic in the VyOS build system
🔇 Additional comments (1)
scripts/image-build/build-vyos-image (1)

733-742: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Improved internal build script efficiency for Software Bill of Materials generation.

Walkthrough

Rename of the SBOM command list variable from cmd to syft_cmd in lines 730–742 of the build script, with corresponding loop iteration update. No functional behavior change; improves code clarity by using a more specific name for the SBOM-generation command sequence.

Changes

SBOM Command List Refactoring

Layer / File(s) Summary
SBOM command variable rename
scripts/image-build/build-vyos-image
Variable holding syft invocation and CycloneDX/SPDX sed post-processing steps renamed from cmd to syft_cmd; subprocess loop updated to iterate over the renamed variable.

Possibly related PRs

  • vyos/vyos-build#1198: Prior SBOM-generation refactor in the same command-construction block.
  • vyos/vyos-build#1213: Related SBOM generation changes affecting syft command construction and CycloneDX/SPDX output handling.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing a bug where a cmd variable shadows the cmd() function by renaming it to syft_cmd.
Description check ✅ Passed The description clearly explains the bug (variable shadowing from commit 75a495b), the fix (renaming cmd to syft_cmd), and links to the related task T8542.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code

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

@mergify mergify Bot added the rolling label Jun 4, 2026
@mergify mergify Bot assigned c-po Jun 4, 2026
@c-po
c-po requested review from dmbaturin and jestabro and removed request for dmbaturin June 4, 2026 20:12

@jestabro jestabro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix var name to avoid collision with function name.

@c-po
c-po merged commit 35cb196 into vyos:rolling Jun 4, 2026
10 of 12 checks passed
@c-po
c-po deleted the sbom-fix branch June 4, 2026 20:14
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

CI integration 👍 passed!

Details

CI logs

  • Config tests 👍 passed

@vyos-bot vyos-bot Bot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants