Skip to content

fix: add tests for sdp upgrade#6714

Merged
kateeselius merged 1 commit into
mainfrom
CN-1011-add-tests
Apr 8, 2026
Merged

fix: add tests for sdp upgrade#6714
kateeselius merged 1 commit into
mainfrom
CN-1011-add-tests

Conversation

@kateeselius
Copy link
Copy Markdown
Contributor

@kateeselius kateeselius commented Apr 8, 2026

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 adds test coverage for the snyk-docker-plugin upgrade from version 9.3.0 to 9.6.0, focusing on the key fixes and features introduced in this upgrade.

Where should the reviewer start?

1. mssql-jdbc pom.properties Fix (CN-1011)

Issue: com.microsoft.sqlserver:mssql-jdbc JARs (e.g., mssql-jdbc-12.10.2.jre11.jar) have pom.properties files with incomplete version info (only version=12.10.2), causing Snyk to incorrectly report them as version 12.10.2 instead of 12.10.2.jre11. This led to false positive vulnerability alerts for SNYK-JAVA-COMMICROSOFTSQLSERVER-13821835.

Fix: Added package override to skip pom.properties JAR resolution and use maven-deps for resolution instead.

Test Coverage:

  • Created test fixture with real mssql-jdbc JAR from Maven Central
  • Test verifies version is correctly identified as 12.10.2.jre11 (not 12.10.2)
  • Test verifies no false positive vulnerability alerts

2. JVM Release File Parsing (CN-444)

Feature: Added ability to parse JVM release files at /opt/java/openjdk/release to detect Java runtime version and scan for JVM vulnerabilities.

Test Coverage:

  • Test uses public eclipse-temurin:11-jre image
  • Verifies Java version detection from release file
  • Verifies baseRuntimes fact is added to scan results

3. Go stdlib Detection (PR #767)

Feature: Enhanced Go binary scanning to detect vulnerabilities in Go standard library.

Test Coverage:

  • Test shows we now detect the go binary

How should this be manually tested?

n/a

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

n/a

Risk assessment (Low | Medium | High)?

n/a

Any background context you want to provide?

n/a

What are the relevant tickets?

n/a

Screenshots (if appropriate)

n/a

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented Apr 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.

@kateeselius kateeselius marked this pull request as ready for review April 8, 2026 16:43
@kateeselius kateeselius requested review from a team as code owners April 8, 2026 16:43
Comment thread test/jest/acceptance/snyk-container/jvm-release-detection.spec.ts Outdated
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@kateeselius kateeselius enabled auto-merge April 8, 2026 17:05
@kateeselius kateeselius disabled auto-merge April 8, 2026 17:26
@kateeselius kateeselius force-pushed the CN-1011-add-tests branch 3 times, most recently from 22d5e9e to 7227fa2 Compare April 8, 2026 17:35
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@kateeselius kateeselius force-pushed the CN-1011-add-tests branch 2 times, most recently from 4c02f81 to 4a18593 Compare April 8, 2026 20:00
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@kateeselius kateeselius merged commit b144a8f into main Apr 8, 2026
9 checks passed
@kateeselius kateeselius deleted the CN-1011-add-tests branch April 8, 2026 21:23
@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

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

Missing API Mocking 🟠 [major]

The new acceptance tests in mssql-jdbc-pom-properties.spec.ts and jvm-release-detection.spec.ts invoke Snyk CLI commands (test, monitor) that interact with live Snyk APIs without setting up fake-server for mocking. This violates the repository's guidelines in CONTRIBUTING.md which state that tests must never call remote endpoints. This will cause failures in CI environments without network access or valid tokens.

const { code, stdout } = await runSnykCLI(
  `container test ${TEST_FIXTURE} --json`,
);
Brittle DB Dependency 🟠 [major]

The regression check for the mssql-jdbc fix relies on the hardcoded vulnerability ID SNYK-JAVA-COMMICROSOFTSQLSERVER-13821835. This makes the test suite brittle to upstream metadata changes in the Snyk vulnerability database. If this vulnerability is renamed, removed, or its version range is updated in the database, the test will pass vacuously even if the underlying version detection bug returns.

(vuln: any) =>
  vuln.id === 'SNYK-JAVA-COMMICROSOFTSQLSERVER-13821835' &&
Missing Direct Verification 🟡 [minor]

The test mssql-jdbc-pom-properties.spec.ts does not directly verify that the mssql-jdbc dependency is identified as 12.10.2.jre11 as claimed in the PR description. It only asserts the absence of a vulnerability associated with the incorrect version (12.10.2). If the package detection logic fails entirely to find the JAR, the test will pass silently. A robust test should inspect jsonOutput.applications or the facts tree for the expected version string.

expect(falsePositive).toBeUndefined();
📚 Repository Context Analyzed

This review considered 4 relevant code sections from 4 files (average relevance: 1.00)

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