Skip to content

chore: update install-snyk script to allow os and arch overrides#6786

Merged
j-luong merged 1 commit into
mainfrom
chore/cli-1489_installSnykFilenameParam
May 11, 2026
Merged

chore: update install-snyk script to allow os and arch overrides#6786
j-luong merged 1 commit into
mainfrom
chore/cli-1489_installSnykFilenameParam

Conversation

@j-luong
Copy link
Copy Markdown
Contributor

@j-luong j-luong commented May 8, 2026

User description

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages
    are release-note ready, emphasizing
    what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

This PR updates the install-snyk.py script so that callers can provide an override the automatically detected OS and Architecture.

This change is meant to support cross compilation or other scenarios where the host os and arch are not the target ones.

How should this be manually tested?

on a mac, run
python scripts/install-snyk.py 1.1304.2 --os linux --arch amr64

What's the product update that needs to be communicated to CLI users?

None, this is an internal change

Risk assessment - Low?

Low, as these a pure overrides for special cases

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented May 8, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@j-luong j-luong marked this pull request as ready for review May 8, 2026 10:19
@j-luong j-luong requested review from a team as code owners May 8, 2026 10:19
@snyk-pr-review-bot

This comment has been minimized.

Comment thread scripts/install-snyk.py Outdated
@j-luong j-luong force-pushed the chore/cli-1489_installSnykFilenameParam branch 2 times, most recently from 20a60f9 to 02d4158 Compare May 11, 2026 10:17
@j-luong j-luong changed the title chore: update install-snyk script to allow filename overrides chore: update install-snyk script to allow os and arch overrides May 11, 2026
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

Comment thread scripts/install-snyk.py Outdated
@j-luong j-luong force-pushed the chore/cli-1489_installSnykFilenameParam branch from 02d4158 to 35e4629 Compare May 11, 2026 13:03
@snyk-pr-review-bot

This comment has been minimized.

@j-luong j-luong force-pushed the chore/cli-1489_installSnykFilenameParam branch from 35e4629 to c3122a3 Compare May 11, 2026 13:55
@j-luong j-luong enabled auto-merge May 11, 2026 13:56
@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Runtime Crash 🟠 [major]

Providing an unsupported string to --os or --arch (e.g., --arch powerpc) will bypass the detection safeguards but fail to match any condition in get_filename (see context). This results in filename being an empty string. The code then attempts to open this path for writing at with open(downloaded_file_path, "wb"), which will raise a FileNotFoundError or IsADirectoryError and crash the script.

if os_override:
    os_type = os_override
if arch_override:
    arch_type = arch_override
Documentation Mismatch 🟠 [major]

The PR description and manual testing instructions explicitly state that a --filename flag was added to override the download target. However, the implementation adds --os and --arch flags instead. A user following the PR's own testing instructions will encounter an 'unrecognized arguments' error.

parser.add_argument(
    "--arch", help="Explicitly specify the architecture to download", default=None
)
parser.add_argument(
    "--os", help="Explicitly specify the OS to download", default=None
)
Host-Dependent Logic 🟡 [minor]

The get_filename function (visible in context) uses os.path.exists to check for the presence of musl-libc on the host filesystem to decide between 'linux' and 'alpine' binaries. Even with the new overrides, a user on a glibc-based host cannot force the download of an Alpine binary (or vice versa), which limits the script's utility for the cross-platform container image builds mentioned in the PR description.

if os_override:
    os_type = os_override
if arch_override:
    arch_type = arch_override
📚 Repository Context Analyzed

This review considered 8 relevant code sections from 8 files (average relevance: 0.88)

@PeterSchafer
Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Description updated to latest commit (c3122a3)

@j-luong j-luong merged commit 791d2e3 into main May 11, 2026
10 checks passed
@j-luong j-luong deleted the chore/cli-1489_installSnykFilenameParam branch May 11, 2026 15:44
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