Skip to content

Commit 9313782

Browse files
authored
fix: improve Dockerfile attribution and base image exclusion (#766)
- Update dependency matching to support source-segment attribution (e.g., matching 'foo/lib' if 'foo' is in the Dockerfile). - Ensure consistent expansion of transitive dependencies for both provided and auto-detected Dockerfile analysis. - Annotate layer IDs using the same source-segment matching logic for better sub-package coverage. - Add comprehensive unit tests covering attribution priority, exclusion logic, and shared source segment edge cases.
1 parent c9cbe4e commit 9313782

File tree

5 files changed

+1488
-49
lines changed

5 files changed

+1488
-49
lines changed

lib/response-builder.ts

Lines changed: 79 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,33 @@ async function buildResponse(
2626
options?: Partial<types.PluginOptions>,
2727
): Promise<types.PluginResponse> {
2828
const deps = depsAnalysis.depTree.dependencies;
29-
const dockerfilePkgs = collectDockerfilePkgs(dockerfileAnalysis, deps);
30-
const finalDeps = excludeBaseImageDeps(
31-
deps,
32-
dockerfilePkgs,
33-
excludeBaseImageVulns,
34-
);
35-
/** WARNING! Mutates the depTree.dependencies! */
36-
annotateLayerIds(finalDeps, dockerfilePkgs);
29+
30+
// Expand dockerfile packages and auto detected user instructions if
31+
// they are provided. These objects are mutated in place, ensuring the
32+
// expanded packages are used in the subsequent steps and fact building.
33+
if (dockerfileAnalysis?.dockerfilePackages) {
34+
getUserInstructionDeps(dockerfileAnalysis.dockerfilePackages, deps);
35+
}
36+
if (depsAnalysis.autoDetectedUserInstructions?.dockerfilePackages) {
37+
getUserInstructionDeps(
38+
depsAnalysis.autoDetectedUserInstructions.dockerfilePackages,
39+
deps,
40+
);
41+
}
42+
43+
const dockerfilePkgs =
44+
dockerfileAnalysis?.dockerfilePackages ??
45+
depsAnalysis.autoDetectedUserInstructions?.dockerfilePackages;
46+
47+
if (dockerfilePkgs) {
48+
const finalDeps = excludeBaseImageDeps(
49+
deps,
50+
dockerfilePkgs,
51+
excludeBaseImageVulns,
52+
);
53+
annotateLayerIds(finalDeps, dockerfilePkgs);
54+
depsAnalysis.depTree.dependencies = finalDeps;
55+
}
3756

3857
/** This must be called after all final changes to the DependencyTree. */
3958
const depGraph = await legacy.depTreeToGraph(
@@ -197,17 +216,12 @@ async function buildResponse(
197216
autoDetectedLayers &&
198217
Object.keys(autoDetectedLayers).length > 0
199218
) {
200-
const autoDetectedPackagesWithChildren = getUserInstructionDeps(
201-
autoDetectedPackages,
202-
deps,
203-
);
204-
205219
const autoDetectedUserInstructionsFact: facts.AutoDetectedUserInstructionsFact =
206220
{
207221
type: "autoDetectedUserInstructions",
208222
data: {
209223
dockerfileLayers: autoDetectedLayers,
210-
dockerfilePackages: autoDetectedPackagesWithChildren!,
224+
dockerfilePackages: autoDetectedPackages,
211225
},
212226
};
213227
additionalFacts.push(autoDetectedUserInstructionsFact);
@@ -339,23 +353,22 @@ async function buildResponse(
339353
};
340354
}
341355

342-
function collectDockerfilePkgs(
343-
dockerAnalysis: DockerFileAnalysis | undefined,
344-
deps: {
345-
[depName: string]: types.DepTreeDep;
346-
},
347-
) {
348-
if (!dockerAnalysis) {
349-
return;
350-
}
351-
352-
return getUserInstructionDeps(dockerAnalysis.dockerfilePackages, deps);
353-
}
354-
355-
// Iterate over the dependencies list; if one is introduced by the dockerfile,
356-
// flatten its dependencies and append them to the list of dockerfile
357-
// packages. This gives us a reference of all transitive deps installed via
358-
// the dockerfile, and the instruction that installed it.
356+
/**
357+
* Expands the provided dockerfile packages to include transitive dependencies.
358+
* Transitive dependencies are keyed by their source segments.
359+
*
360+
* @important
361+
* mutates the provided `dockerfilePackages` object.
362+
*
363+
* @warning
364+
* **Known Issue:** In some scenarios, this function can cause over-attribution of
365+
* dependencies to the dockerfile because the `dockerfilePackages` object is mutated
366+
* while iterating. This behavior is retained for downstream compatibility.
367+
*
368+
* @param dockerfilePackages - The dockerfile packages to expand.
369+
* @param dependencies - The dependencies of the image.
370+
* @returns The expanded dockerfile packages.
371+
*/
359372
function getUserInstructionDeps(
360373
dockerfilePackages: DockerFilePackages,
361374
dependencies: {
@@ -389,9 +402,14 @@ function collectDeps(pkg) {
389402
: [];
390403
}
391404

392-
// Skip processing if option disabled or dockerfilePkgs is undefined. We
393-
// can't exclude anything in that case, because we can't tell which deps are
394-
// from dockerfile and which from base image.
405+
/**
406+
* Excludes base image dependencies from the dependency tree if excludeBaseImageVulns is true.
407+
*
408+
* @param deps - The dependencies of the image.
409+
* @param dockerfilePkgs - The expanded packages attributed to the Dockerfile.
410+
* @param excludeBaseImageVulns - Whether to exclude base image dependencies.
411+
* @returns The dependencies of the image.
412+
*/
395413
function excludeBaseImageDeps(
396414
deps: {
397415
[depName: string]: types.DepTreeDep;
@@ -403,31 +421,44 @@ function excludeBaseImageDeps(
403421
return deps;
404422
}
405423

406-
return extractDockerfileDeps(deps, dockerfilePkgs);
407-
}
408-
409-
function extractDockerfileDeps(
410-
allDeps: {
411-
[depName: string]: types.DepTreeDep;
412-
},
413-
dockerfilePkgs: DockerFilePackages,
414-
) {
415-
return Object.keys(allDeps)
416-
.filter((depName) => dockerfilePkgs[depName])
424+
return Object.keys(deps)
425+
.filter((depName) => {
426+
if (dockerfilePkgs[depName] !== undefined) {
427+
return true;
428+
}
429+
const source = depName.split("/")[0];
430+
return dockerfilePkgs[source] !== undefined;
431+
})
417432
.reduce((extractedDeps, depName) => {
418-
extractedDeps[depName] = allDeps[depName];
433+
extractedDeps[depName] = deps[depName];
419434
return extractedDeps;
420435
}, {});
421436
}
422437

423-
function annotateLayerIds(deps, dockerfilePkgs) {
438+
/**
439+
* Annotates the dependencies with the layer ID of the Dockerfile
440+
* instruction that installed them.
441+
*
442+
* @important
443+
* mutates the provided `deps` object.
444+
*
445+
* @param deps - The dependencies of the image.
446+
* @param dockerfilePkgs - The expanded packages attributed to the Dockerfile.
447+
*/
448+
function annotateLayerIds(
449+
deps: {
450+
[depName: string]: types.DepTreeDep;
451+
},
452+
dockerfilePkgs: DockerFilePackages | undefined,
453+
) {
424454
if (!dockerfilePkgs) {
425455
return;
426456
}
427457

428458
for (const dep of Object.keys(deps)) {
429459
const pkg = deps[dep];
430-
const dockerfilePkg = dockerfilePkgs[dep];
460+
const pkgSource = dep.split("/")[0];
461+
const dockerfilePkg = dockerfilePkgs[dep] || dockerfilePkgs[pkgSource];
431462
if (dockerfilePkg) {
432463
pkg.labels = {
433464
...(pkg.labels || {}),

0 commit comments

Comments
 (0)