Skip to content

chore: replace chocolatey with native binaries#6639

Open
mihaibuzgau wants to merge 4 commits intomainfrom
chore/cli-1379_useNativeWinDeps
Open

chore: replace chocolatey with native binaries#6639
mihaibuzgau wants to merge 4 commits intomainfrom
chore/cli-1379_useNativeWinDeps

Conversation

@mihaibuzgau
Copy link
Contributor

@mihaibuzgau mihaibuzgau commented Mar 12, 2026 •
edited
Loading

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?

  • Migrates Windows CI from Chocolatey to native installers for Node.js, Python 3.12.8, .NET SDK 8, Maven, Gradle, and GNU Make, each with pinned versions and SHA256 verification.
  • Centralizes Windows PATH management via C:\tools-cache\snyk-env.ps1, making installed tools consistently available across CircleCI steps and jobs.

Where should the reviewer start?

  • .circleci/config.yml
  • .circleci/windows/*.ps1

How should this be manually tested?

In CircleCI, run the full Windows pipeline on a test branch:

  • Confirm build windows amd64 and sign windows amd64 pass and produce expected artifacts.
  • Confirm acceptance-tests windows amd64 pass, including SBOM tests that rely on Maven and Python.

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

There is no functional CLI behavior change for end users; this PR only changes how Windows CI builds and tests the CLI.

@mihaibuzgau mihaibuzgau requested review from a team as code owners March 12, 2026 15:22
@mihaibuzgau mihaibuzgau marked this pull request as draft March 12, 2026 15:23
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch 3 times, most recently from 821cecc to bdef5ed Compare March 12, 2026 16:31
@snyk-io
Copy link

snyk-io bot commented Mar 12, 2026 •
edited
Loading

✅ 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.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch 2 times, most recently from cdd8b29 to 69056fa Compare March 16, 2026 08:44
@mihaibuzgau
Copy link
Contributor Author

mihaibuzgau commented Mar 16, 2026

/review

@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@mihaibuzgau
Copy link
Contributor Author

mihaibuzgau commented Mar 16, 2026

/review

@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 110e69f to 765b462 Compare March 16, 2026 12:40
@mihaibuzgau
Copy link
Contributor Author

mihaibuzgau commented Mar 16, 2026

/review

@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 765b462 to 42b39b5 Compare March 16, 2026 12:56
@mihaibuzgau
Copy link
Contributor Author

mihaibuzgau commented Mar 16, 2026

/review

@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 42b39b5 to aa336dc Compare March 16, 2026 13:10
@mihaibuzgau
Copy link
Contributor Author

mihaibuzgau commented Mar 16, 2026

/review

@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from aa336dc to 9d7b8ac Compare March 16, 2026 13:26
@mihaibuzgau
Copy link
Contributor Author

mihaibuzgau commented Mar 16, 2026

/review

@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch 4 times, most recently from 48e9b8d to 87bc5f9 Compare March 16, 2026 15:32
PeterSchafer
PeterSchafer reviewed Mar 19, 2026
View reviewed changes
scripts/windows/install-dotnet-sdk.ps1 Outdated

try {
$dotnetVersion = '8.0.100'
$cacheDir = 'C:\tools-cache'
Copy link
Contributor

@PeterSchafer PeterSchafer Mar 19, 2026

Choose a reason for hiding this comment

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

Suggestion/Question: Is there a reason why we don't inject the cachedir as a parameter. This would enable to have a single source of truth.

PeterSchafer
PeterSchafer reviewed Mar 19, 2026
View reviewed changes
.circleci/config.yml Outdated
name: Restoring Windows tools cache
keys:
- chocolatey-cache-full-v5-{{ arch }}-{{ checksum ".circleci/chocolatey.config" }}
- windows-tools-cache-v1-{{ arch }}
Copy link
Contributor

@PeterSchafer PeterSchafer Mar 19, 2026

Choose a reason for hiding this comment

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

Issue: Based on the logs it seems the cache is not working and the dependencies are downloaded each team. This could be due to an older cache being used. Probably worth increasing to v2 to get all deps in the cache.

@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch from f606343 to 11f1ba1 Compare March 19, 2026 16:35
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2026 •
edited
Loading

Warnings
⚠️ There are multiple commits on your branch, please squash them locally before merging!

Generated by 🚫 dangerJS against 1720659

@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch 2 times, most recently from 669be61 to a31cc6f Compare March 19, 2026 18:56
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch from a31cc6f to 1837732 Compare March 19, 2026 19:25
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch from 1837732 to 8dcc308 Compare March 20, 2026 07:56
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch from 8dcc308 to b626996 Compare March 20, 2026 08:49
@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch from b626996 to 7e2f571 Compare March 20, 2026 08:49
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch from 7e2f571 to 484e989 Compare March 20, 2026 09:07
@snyk-pr-review-bot

This comment has been minimized.

Sign in to view
@j-luong j-luong force-pushed the chore/cli-1379_useNativeWinDeps branch from 484e989 to 1720659 Compare March 20, 2026 09:15
@snyk-pr-review-bot
Copy link

snyk-pr-review-bot bot commented Mar 20, 2026

PR Reviewer Guide 🔍

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

Brittle Version Lock 🟠 [major]

The script hard-pins expectedNodeVersion to '20.11.1' and throws an error if it does not exactly match .nvmrc. This tightly couples the CI infrastructure to the application config. If a developer updates .nvmrc to a newer security patch (e.g., 20.12.0) without also finding and updating this PowerShell script, the entire Windows build pipeline will break immediately.

 
if ($nvmVersion -ne $expectedNodeVersion) {
  throw ".nvmrc version '$nvmVersion' does not match expected Node.js version '$expectedNodeVersion' used by Windows CI."
}
Possible Missing PATH Update 🟡 [minor]

If the Python installer places the binary in a path not included in candidateDirs (e.g. a different drive or a version-specific subpath not anticipated), $selectedDir remains $null. The script does not throw or signal a failure in this case, meaning Python will not be added to the persistent snyk-env.ps1, likely causing subsequent CI steps to fail with 'python: command not found' errors that are harder to debug than an explicit installer error.

 
$selectedDir = $null
foreach ($dir in $candidateDirs) {
  if ($null -ne $dir -and (Test-Path $dir)) {
    $selectedDir = $dir
    break
  }
}

if ($selectedDir) {
📚 Repository Context Analyzed

This review considered 28 relevant code sections from 11 files (average relevance: 0.52)

CatalinSnyk
CatalinSnyk reviewed Mar 23, 2026
View reviewed changes
.circleci/config.yml
name: Restoring Windows tools cache
keys:
- chocolatey-cache-full-v5-{{ arch }}-{{ checksum ".circleci/chocolatey.config" }}
- windows-tools-cache-v2-{{ arch }}
Copy link
Contributor

@CatalinSnyk CatalinSnyk Mar 23, 2026

Choose a reason for hiding this comment

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

suggestion: I don't think this caching logic will work... Previously we generated this based on the choco config (thus when a dependency version in the config change, the cache key would change). But now the cache key is static and since caches are immutable this is probably not the right behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PeterSchafer PeterSchafer PeterSchafer left review comments

@CatalinSnyk CatalinSnyk CatalinSnyk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mihaibuzgau @PeterSchafer @CatalinSnyk @j-luong