Skip to content

Commit 3688f80

Browse files
committed
fix: resolve attribution bug without changing downstream contract
This version of resolving the dockerfile attribution bug does so with a reduced impact on the downstream CLI and registry logic. It does so by expanding both the dockerfile packages and auto-detected packages, ensuring those facts retain the same intent. Internal to snyk-docker-plugin, dependencies are always referenced by full name (i.e. zlib/zlib) when it is available, preventing earlier bugs caused by splitting the source segment of the name.
1 parent 8aadce3 commit 3688f80

File tree

2 files changed

+630
-470
lines changed

2 files changed

+630
-470
lines changed

lib/response-builder.ts

Lines changed: 114 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ import * as types from "./types";
1212
import { truncateAdditionalFacts } from "./utils";
1313
import { PLUGIN_VERSION } from "./version";
1414

15-
export { buildResponse };
15+
export {
16+
buildResponse,
17+
expandDockerfilePackages,
18+
excludeBaseImageDeps,
19+
annotateWithLayerIds,
20+
};
1621

1722
async function buildResponse(
1823
depsAnalysis: StaticAnalysis & {
@@ -26,19 +31,36 @@ async function buildResponse(
2631
options?: Partial<types.PluginOptions>,
2732
): Promise<types.PluginResponse> {
2833
const deps = depsAnalysis.depTree.dependencies;
34+
35+
// Expand both the Dockerfile packages and the auto-detected user instructions packages,
36+
// storing the results back to the original objects.
37+
if (dockerfileAnalysis?.dockerfilePackages) {
38+
dockerfileAnalysis.dockerfilePackages = expandDockerfilePackages(
39+
dockerfileAnalysis.dockerfilePackages,
40+
deps,
41+
);
42+
}
43+
44+
if (depsAnalysis.autoDetectedUserInstructions?.dockerfilePackages) {
45+
depsAnalysis.autoDetectedUserInstructions.dockerfilePackages =
46+
expandDockerfilePackages(
47+
depsAnalysis.autoDetectedUserInstructions.dockerfilePackages,
48+
deps,
49+
);
50+
}
51+
52+
// Select a dockerfilePackages object to use for the annotation and exclusion of base image dependencies.
53+
// Prioritize the Dockerfile packages over the auto-detected user instructions packages.
2954
const dockerfilePkgs =
3055
dockerfileAnalysis?.dockerfilePackages ||
31-
depsAnalysis.autoDetectedUserInstructions?.dockerfilePackages ||
32-
{};
33-
34-
/** WARNING! Mutates the depTree.dependencies! */
35-
annotateWithLayerIds(deps, dockerfilePkgs);
56+
depsAnalysis.autoDetectedUserInstructions?.dockerfilePackages;
3657

3758
const finalDeps = excludeBaseImageDeps(
3859
deps,
3960
dockerfilePkgs,
4061
excludeBaseImageVulns,
4162
);
63+
annotateWithLayerIds(finalDeps, dockerfilePkgs);
4264

4365
// Apply the filtered dependencies back to the depTree
4466
depsAnalysis.depTree.dependencies = finalDeps;
@@ -342,14 +364,68 @@ async function buildResponse(
342364
};
343365
}
344366

345-
// Returns the package source name from a dependency key. A package source refers
346-
// to the top-level Linux package name, such as "bzip2" in "bzip2/libbz2-dev".
347-
function packageSource(depKey: string): string {
348-
return depKey.split("/")[0];
367+
/**
368+
* Returns the package source name from a full dependency name.
369+
*
370+
* A package source refers to the top-level package name, such as "bzip2" in "bzip2/libbz2-dev".
371+
*
372+
* @param depName - The full dependency name.
373+
* @returns The package source name.
374+
*/
375+
function packageSource(depName: string): string {
376+
return depName.split("/")[0];
377+
}
378+
379+
/**
380+
* Expands the list of packages explicitly requested in the Dockerfile to include all transitive dependencies.
381+
*
382+
* The returned package map is keyed by the full dependency names. Package names extracted from the Dockerfile
383+
* (typically in the form of source segments) are copied from the input map into the returned map to maintain
384+
* compatibility with the CLI dockerfile-attribution logic.
385+
*
386+
* @param dockerfilePackages - The packages explicitly requested in a Dockerfile.
387+
* @param deps - The dependencies of the image.
388+
* @returns A map of packages attributed to the Dockerfile.
389+
*/
390+
function expandDockerfilePackages(
391+
dockerfilePackages: DockerFilePackages,
392+
deps: { [depName: string]: types.DepTreeDep },
393+
): DockerFilePackages {
394+
const expandedPkgs = { ...dockerfilePackages };
395+
396+
function collectChildPackages(node: types.DepTreeDep, parentEntry: any) {
397+
if (!node.dependencies) {
398+
return;
399+
}
400+
for (const childKey of Object.keys(node.dependencies)) {
401+
if (!expandedPkgs[childKey]) {
402+
expandedPkgs[childKey] = parentEntry;
403+
collectChildPackages(node.dependencies[childKey], parentEntry);
404+
}
405+
}
406+
}
407+
408+
for (const rootKey of Object.keys(deps)) {
409+
const source = packageSource(rootKey);
410+
const dockerfileEntry = expandedPkgs[rootKey] || expandedPkgs[source];
411+
if (dockerfileEntry) {
412+
// Ensure the full dependency name is in the expanded packages.
413+
expandedPkgs[rootKey] = dockerfileEntry;
414+
collectChildPackages(deps[rootKey], dockerfileEntry);
415+
}
416+
}
417+
418+
return expandedPkgs;
349419
}
350420

351-
// If excludeBaseImageVulns is true, only retain dependencies that are
352-
// dockerfile-introduced, as defined by dockerfilePkgs.
421+
/**
422+
* Excludes base image dependencies from the dependency tree if excludeBaseImageVulns is true.
423+
*
424+
* @param deps - The dependencies of the image.
425+
* @param dockerfilePkgs - The expanded packages attributed to the Dockerfile.
426+
* @param excludeBaseImageVulns - Whether to exclude base image dependencies.
427+
* @returns The dependencies of the image.
428+
*/
353429
function excludeBaseImageDeps(
354430
deps: {
355431
[depName: string]: types.DepTreeDep;
@@ -362,16 +438,19 @@ function excludeBaseImageDeps(
362438
}
363439

364440
return Object.keys(deps)
365-
.filter(
366-
(depName) =>
367-
dockerfilePkgs[depName] || dockerfilePkgs[packageSource(depName)],
368-
)
441+
.filter((depName) => dockerfilePkgs[depName])
369442
.reduce((extractedDeps, depName) => {
370443
extractedDeps[depName] = deps[depName];
371444
return extractedDeps;
372445
}, {});
373446
}
374447

448+
/**
449+
* Annotates the dependency tree with layer IDs. Mutates the recieved dependency tree.
450+
*
451+
* @param deps - The dependencies of the image.
452+
* @param dockerfilePkgs - The expanded packages attributed to the Dockerfile.
453+
*/
375454
function annotateWithLayerIds(
376455
deps: { [depName: string]: types.DepTreeDep },
377456
dockerfilePkgs: DockerFilePackages | undefined,
@@ -380,40 +459,26 @@ function annotateWithLayerIds(
380459
return;
381460
}
382461

383-
for (const rootKey of Object.keys(deps)) {
384-
const source = packageSource(rootKey);
385-
const dockerfileEntry = dockerfilePkgs[rootKey] || dockerfilePkgs[source];
386-
if (!dockerfileEntry) {
387-
continue;
388-
}
389-
390-
const rootNode = deps[rootKey];
391-
const layerId = instructionDigest(dockerfileEntry.instruction);
392-
rootNode.labels = {
393-
...(rootNode.labels || {}),
394-
dockerLayerId: layerId,
395-
};
396-
if (
397-
rootNode.dependencies &&
398-
Object.keys(rootNode.dependencies).length > 0
399-
) {
400-
annotateSubtreeWithLayerId(rootNode.dependencies, layerId);
462+
function annotateRecursive(currentDeps: {
463+
[depName: string]: types.DepTreeDep;
464+
}) {
465+
for (const depKey of Object.keys(currentDeps)) {
466+
const node = currentDeps[depKey];
467+
const dockerfileEntry = dockerfilePkgs![depKey];
468+
469+
if (dockerfileEntry) {
470+
node.labels = {
471+
...(node.labels || {}),
472+
dockerLayerId: instructionDigest(dockerfileEntry.instruction),
473+
};
474+
475+
// Only progress down the dependency tree if the current node is a dockerfile package.
476+
if (node.dependencies) {
477+
annotateRecursive(node.dependencies);
478+
}
479+
}
401480
}
402481
}
403-
}
404482

405-
function annotateSubtreeWithLayerId(
406-
deps: { [depName: string]: types.DepTreeDep },
407-
dockerLayerId: string,
408-
): void {
409-
for (const depKey of Object.keys(deps)) {
410-
const node = deps[depKey];
411-
node.labels = {
412-
...(node.labels || {}),
413-
dockerLayerId,
414-
};
415-
if (node.dependencies && Object.keys(node.dependencies).length > 0) {
416-
annotateSubtreeWithLayerId(node.dependencies, dockerLayerId);
417-
}
418-
}
483+
annotateRecursive(deps);
419484
}

0 commit comments

Comments
 (0)