Skip to content

Commit bd23576

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 f290a6c commit bd23576

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;
@@ -335,14 +357,68 @@ async function buildResponse(
335357
};
336358
}
337359

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

344-
// If excludeBaseImageVulns is true, only retain dependencies that are
345-
// dockerfile-introduced, as defined by dockerfilePkgs.
414+
/**
415+
* Excludes base image dependencies from the dependency tree if excludeBaseImageVulns is true.
416+
*
417+
* @param deps - The dependencies of the image.
418+
* @param dockerfilePkgs - The expanded packages attributed to the Dockerfile.
419+
* @param excludeBaseImageVulns - Whether to exclude base image dependencies.
420+
* @returns The dependencies of the image.
421+
*/
346422
function excludeBaseImageDeps(
347423
deps: {
348424
[depName: string]: types.DepTreeDep;
@@ -355,16 +431,19 @@ function excludeBaseImageDeps(
355431
}
356432

357433
return Object.keys(deps)
358-
.filter(
359-
(depName) =>
360-
dockerfilePkgs[depName] || dockerfilePkgs[packageSource(depName)],
361-
)
434+
.filter((depName) => dockerfilePkgs[depName])
362435
.reduce((extractedDeps, depName) => {
363436
extractedDeps[depName] = deps[depName];
364437
return extractedDeps;
365438
}, {});
366439
}
367440

441+
/**
442+
* Annotates the dependency tree with layer IDs. Mutates the recieved dependency tree.
443+
*
444+
* @param deps - The dependencies of the image.
445+
* @param dockerfilePkgs - The expanded packages attributed to the Dockerfile.
446+
*/
368447
function annotateWithLayerIds(
369448
deps: { [depName: string]: types.DepTreeDep },
370449
dockerfilePkgs: DockerFilePackages | undefined,
@@ -373,40 +452,26 @@ function annotateWithLayerIds(
373452
return;
374453
}
375454

376-
for (const rootKey of Object.keys(deps)) {
377-
const source = packageSource(rootKey);
378-
const dockerfileEntry = dockerfilePkgs[rootKey] || dockerfilePkgs[source];
379-
if (!dockerfileEntry) {
380-
continue;
381-
}
382-
383-
const rootNode = deps[rootKey];
384-
const layerId = instructionDigest(dockerfileEntry.instruction);
385-
rootNode.labels = {
386-
...(rootNode.labels || {}),
387-
dockerLayerId: layerId,
388-
};
389-
if (
390-
rootNode.dependencies &&
391-
Object.keys(rootNode.dependencies).length > 0
392-
) {
393-
annotateSubtreeWithLayerId(rootNode.dependencies, layerId);
455+
function annotateRecursive(currentDeps: {
456+
[depName: string]: types.DepTreeDep;
457+
}) {
458+
for (const depKey of Object.keys(currentDeps)) {
459+
const node = currentDeps[depKey];
460+
const dockerfileEntry = dockerfilePkgs![depKey];
461+
462+
if (dockerfileEntry) {
463+
node.labels = {
464+
...(node.labels || {}),
465+
dockerLayerId: instructionDigest(dockerfileEntry.instruction),
466+
};
467+
468+
// Only progress down the dependency tree if the current node is a dockerfile package.
469+
if (node.dependencies) {
470+
annotateRecursive(node.dependencies);
471+
}
472+
}
394473
}
395474
}
396-
}
397475

398-
function annotateSubtreeWithLayerId(
399-
deps: { [depName: string]: types.DepTreeDep },
400-
dockerLayerId: string,
401-
): void {
402-
for (const depKey of Object.keys(deps)) {
403-
const node = deps[depKey];
404-
node.labels = {
405-
...(node.labels || {}),
406-
dockerLayerId,
407-
};
408-
if (node.dependencies && Object.keys(node.dependencies).length > 0) {
409-
annotateSubtreeWithLayerId(node.dependencies, dockerLayerId);
410-
}
411-
}
476+
annotateRecursive(deps);
412477
}

0 commit comments

Comments
 (0)