fix: dockerfile attribution

[d3vco]

• Ready for review
• Follows CONTRIBUTING rules
• Reviewed by Snyk internal team

What does this PR do?

This PR fixes 2 related issues:

1. --exclude-base-image-vulns has been disabled in SDP resulting in the flag not being effective in container monitor mode.
2. The logic to identify dockerfile-introduced dependencies under-attributes dependencies to the dockerfile. This resulted in vulnerabilities incorrectly being filtered as "base image vulns" and missing dockerfile instruction labels.

Where should the reviewer start?

buildResponse orchestration

• Removed collectDockerfilePkgs, logic is inlined.
• Expands both versions of dockerfile packages using getUserInstructionDeps up front (in place)
• Derives dockerfilePkgs as dockerfileAnalysis?.dockerfilePackages ?? autoDetected…dockerfilePackages and uses the result to conduct annotation and exclusion (matches CLI logic).
• Stores the result of filtering and annotation! (depsAnalysis.depTree.dependencies = finalDeps)

excludeBaseImageDeps

• Removed extractDockerfileDeps, filtering is inlined.
• Conducts matching on full name OR source segment. (On main, only exact keys matched)

annotateLayerIds

• Conducts matching on full name OR source segment.

How should this be manually tested?

Unit tests have been added to demonstrate the new behavior.

The changes are most easily observed in monitor mode. Creating a small image with packages installed via a run instruction should show those dependencies attributed to the install instruction. Any base image vulns should be filterable, in test or monitor mode.

An example is shown below.

Any background context you want to provide?

More context provided in this write-up.

What are the relevant tickets?

• CN-568
• CN-846

Screenshots

Current Behavior:

PR Behavior:

[d3vco]

snyk-pr-review-bot's "Inconsistent Legacy Compatibility" comment is incorrect. The legacy expansion for autoDetectedUserInstructions is conducted on line 214

[mtstanley-snyk]

couple comments/qs, otherwise lgtm 🎉. Did we decide on a rollout for this? If not, might be worth a short section in your doc on impact and then can start a thread with team on breaking vs non-breaking change.

My understanding of the impact is:

1. no(?) change to snyk container test
2. snyk container monitor -- exclude-base-... now excludes things, whereas before the command would have run but not excluded anything
3. monitored container projects from CLI where dockerfile was provided would now attribute a lot more vulns to user installed
4. monitored container projects from CLI without dockerfile should be no(?) change

[snyk-pr-review-bot]

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Over-attribution Bug 🟠 [major] The combination of transitive dependency expansion in getUserInstructionDeps and source-segment matching in excludeBaseImageDeps (lines 419-424) leads to significant over-attribution of vulnerabilities to the Dockerfile. Any package in the base image that shares a source segment (the prefix before '/') with any transitive dependency of any package installed in the Dockerfile will be incorrectly marked as Dockerfile-introduced. This effectively bypasses the --exclude-base-image-vulns filter for those packages, resulting in noisy reports. While documented as a 'Known Issue' in the code, the change in matching logic from exact names to source-segment prefixes significantly escalates the frequency of this error. return Object.keys(deps) .filter((depName) => { if (dockerfilePkgs[depName] !== undefined) { return true; } const source = depName.split("/")[0]; return dockerfilePkgs[source] !== undefined; }) In-place Mutation 🟡 [minor] The buildResponse function now mutates the dockerfileAnalysis and depsAnalysis input objects in-place (lines 33-41 and 54). While buildResponse is the final step in the current analyzeStatically pipeline (see lib/static.ts), this side effect may cause issues if these objects are reused for logging, caching, or by other callers in the future. Explicit cloning or returning new objects would be safer. getUserInstructionDeps(dockerfileAnalysis.dockerfilePackages, deps); } if (depsAnalysis.autoDetectedUserInstructions?.dockerfilePackages) { getUserInstructionDeps( depsAnalysis.autoDetectedUserInstructions.dockerfilePackages, deps, ); } const dockerfilePkgs = dockerfileAnalysis?.dockerfilePackages ?? depsAnalysis.autoDetectedUserInstructions?.dockerfilePackages; if (dockerfilePkgs) { const finalDeps = excludeBaseImageDeps( deps, dockerfilePkgs, excludeBaseImageVulns, ); annotateLayerIds(finalDeps, dockerfilePkgs); depsAnalysis.depTree.dependencies = finalDeps;

📚 Repository Context Analyzed This review considered 18 relevant code sections from 10 files (average relevance: 0.87) lib/response-builder.ts (4 segments, lines 17-27) lib/dockerfile/instruction-parser.ts (lines 1-247) lib/static.ts (lines 1-78) lib/types.ts (2 segments, lines 1-244) lib/analyzer/static-analyzer.ts (2 segments, lines 89-341) lib/scan.ts (lines 1-247) package.json (lines 1-101) ... and 3 more file(s)

[d3vco]

couple comments/qs, otherwise lgtm 🎉. Did we decide on a rollout for this? If not, might be worth a short section in your doc on impact and then can start a thread with team on breaking vs non-breaking change.

My understanding of the impact is:

1. no(?) change to snyk container test
2. snyk container monitor -- exclude-base-... now excludes things, whereas before the command would have run but not excluded anything
3. monitored container projects from CLI where dockerfile was provided would now attribute a lot more vulns to user installed
4. monitored container projects from CLI without dockerfile should be no(?) change

Doc has been updated to start the rollout discussion.

On # 4 above, monitored container projects without dockerfile will also see an increase in vulns attributed to the dockerfile. Previously there was no path to attribute vulns to the dockerfile, now autoDetectedUserInstructions get the same treatment as those direct from the Dockerfile.

Also, the impact would not be limited to the CLI assuming DRA and kubernetes-monitor also uses the new version of SDP.

[mtstanley-snyk]

Changes lgtm, let's confirm the rollout plan here and then I'll 🚢