fix: CN-360 recreate bug #719

sathvi-k · 2025-10-23T21:06:18Z

Ready for review
Follows CONTRIBUTING rules
Reviewed by Snyk internal team

What does this PR do?

Where should the reviewer start?

How should this be manually tested?

Any background context you want to provide?

What are the relevant tickets?

Screenshots

Additional questions

snyk-pr-review-bot · 2025-10-23T21:07:31Z

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Non-Assertive Test

The test uses console.log to report whether the bug is reproduced but lacks expect calls to actually fail the test based on the outcome. This makes it a manual check rather than an automated one. The test should be updated to assert the expected behavior based on the platform.

if (process.platform === 'win32' && esbuildResult) {
  console.log("⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)");
} else if (process.platform !== 'win32' && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

Incomplete Mocking

A comment on line 121 suggests platform mocking is intended to reproduce a platform-specific bug, but the implementation is missing. The test currently only runs against the actual platform of the test runner. The variable originalPlatform is also unused.

// Mock Windows platform to trigger the bug
const originalPlatform = process.platform;
console.log("💻 Real OS platform:", originalPlatform);

Potential Runtime Crash

The test uses the non-null assertion operator ! multiple times after .find() calls (e.g., lines 160, 165, 190, 228). If a fact is not found, this will cause the test to crash with a runtime error instead of a clear assertion failure. It would be safer to first assert that the fact exists before accessing its properties.

const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "depGraph",
)!.data;

Excessive Logging

The new test case contains a large number of console.log statements. While useful for debugging during development, they create noisy test output and should be removed before merging to keep CI logs clean.

console.log("💻 Real OS platform:", originalPlatform);
const fixturePath = getFixture("docker-archives/docker-save/gobinaries-test.tar");
const imageNameAndTag = `docker-archive:${fixturePath}`;

console.log("🔍 Starting scan for:", imageNameAndTag);

// The bug is likely in the Go binary file path matching logic
// Let's see what gets detected with Windows platform mocking

const pluginResult = await plugin.scan({
  path: imageNameAndTag,
  "app-vulns": true, // Enable application vulnerability scanning for Go binaries
});

console.log("📊 Scan completed. Results count:", pluginResult.scanResults.length);
console.log("📋 Scan results overview:", 
  pluginResult.scanResults.map(r => ({ 
    type: r.identity.type, 
    target: r.identity.targetFile || 'container' 
  }))
);

// Check if esbuild binary is found (this should differ between Windows and Linux)
const esbuildResult = pluginResult.scanResults.find(r => 
  r.identity.targetFile && r.identity.targetFile.includes('esbuild')
);
console.log("🔍 esbuild binary result:", esbuildResult ? "FOUND" : "NOT FOUND");
if (esbuildResult) {
  console.log("📍 esbuild path:", esbuildResult.identity.targetFile);
}

// The bug: Windows should find fewer results (missing esbuild)
// Linux finds 4 projects, Windows finds 3 projects  
console.log("🧮 Expected behavior: Linux=4 projects, Windows=3 projects (missing esbuild)");

const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "depGraph",
)!.data;
expect(depGraph.rootPkg.name).toEqual("docker-image|gobinaries-test.tar");
expect(depGraph.rootPkg.version).toBeUndefined();

const imageId: string = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "imageId",
)!.data;

console.log("🏷️  Image ID:", imageId);

// Check that we get the correct image ID from our gobinaries archive
expect(imageId).toBeDefined();
expect(imageId).toMatch(/^sha256:[a-f0-9]{64}$/);

// Verify that Go binaries are detected and scanned
const goBinariesFact = pluginResult.scanResults.find(
  (result) => result.identity.type === "gomodules" || 
             result.identity.type === "gobinary"
);

console.log("🔍 Looking for Go binaries...");
console.log("🧩 All scan result types:", pluginResult.scanResults.map(r => r.identity.type));

if (goBinariesFact) {
  console.log("✅ Found Go binaries fact:", goBinariesFact.identity);

  expect(goBinariesFact.identity.type).toMatch(/^go(modules|binary)$/);

  const goBinaryDepGraph: DepGraph = goBinariesFact.facts.find(
    (fact) => fact.type === "depGraph",
  )!.data;

  console.log("📦 Go packages found:", goBinaryDepGraph.getDepPkgs().length);
  console.log("📝 Go package names:", goBinaryDepGraph.getDepPkgs().map(p => p.name).slice(0, 10));

  // The Go binary was detected (which is the main goal)
  console.log("🎯 Go binary detected successfully!");

  if (goBinaryDepGraph.getDepPkgs().length > 0) {
    console.log("📦 Go dependencies found - this is great!");
    const goPackages = goBinaryDepGraph.getDepPkgs();
    expect(goPackages.some(pkg => pkg.name.includes("go") || pkg.name.includes("golang"))).toBeTruthy();
  } else {
    console.log("ℹ️  Go binary detected but no embedded dependencies found (this is normal for some binaries)");
  }
} else {
  console.log("❌ No Go binaries found in scan results");
  console.log("🔍 Available fact types for first result:", 
    pluginResult.scanResults[0].facts.map(f => f.type));
}

// DEMONSTRATE THE BUG: 
// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === 'win32' && esbuildResult) {
  console.log("⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)");
} else if (process.platform !== 'win32' && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

snyk-pr-review-bot · 2025-10-23T21:13:48Z

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Ineffective Test

The core logic for verifying the bug (lines 211-224) relies on console.log statements instead of assertions like expect. This causes the test to pass regardless of the outcome, defeating its purpose. This should be converted to proper assertions to validate the expected behavior.

// DEMONSTRATE THE BUG: 
// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === 'win32' && esbuildResult) {
  console.log("⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)");
} else if (process.platform !== 'win32' && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

Debugging Logs

The new test case is filled with numerous console.log statements, which were likely used for debugging. These should be removed to keep test outputs clean and focused on failures.

console.log("💻 Real OS platform:", originalPlatform);
const fixturePath = getFixture("docker-archives/docker-save/gobinaries-test.tar");
const imageNameAndTag = `docker-archive:${fixturePath}`;

console.log("🔍 Starting scan for:", imageNameAndTag);

// The bug is likely in the Go binary file path matching logic
// Let's see what gets detected with Windows platform mocking

const pluginResult = await plugin.scan({
  path: imageNameAndTag,
  "app-vulns": true, // Enable application vulnerability scanning for Go binaries
});

console.log("📊 Scan completed. Results count:", pluginResult.scanResults.length);
console.log("📋 Scan results overview:", 
  pluginResult.scanResults.map(r => ({ 
    type: r.identity.type, 
    target: r.identity.targetFile || 'container' 
  }))
);

// Check if esbuild binary is found (this should differ between Windows and Linux)
const esbuildResult = pluginResult.scanResults.find(r => 
  r.identity.targetFile && r.identity.targetFile.includes('esbuild')
);
console.log("🔍 esbuild binary result:", esbuildResult ? "FOUND" : "NOT FOUND");
if (esbuildResult) {
  console.log("📍 esbuild path:", esbuildResult.identity.targetFile);
}

// The bug: Windows should find fewer results (missing esbuild)
// Linux finds 4 projects, Windows finds 3 projects  
console.log("🧮 Expected behavior: Linux=4 projects, Windows=3 projects (missing esbuild)");

const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "depGraph",
)!.data;
expect(depGraph.rootPkg.name).toEqual("docker-image|gobinaries-test.tar");
expect(depGraph.rootPkg.version).toBeUndefined();

const imageId: string = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "imageId",
)!.data;

console.log("🏷️  Image ID:", imageId);

// Check that we get the correct image ID from our gobinaries archive
expect(imageId).toBeDefined();
expect(imageId).toMatch(/^sha256:[a-f0-9]{64}$/);

// Verify that Go binaries are detected and scanned
const goBinariesFact = pluginResult.scanResults.find(
  (result) => result.identity.type === "gomodules" || 
             result.identity.type === "gobinary"
);

console.log("🔍 Looking for Go binaries...");
console.log("🧩 All scan result types:", pluginResult.scanResults.map(r => r.identity.type));

if (goBinariesFact) {
  console.log("✅ Found Go binaries fact:", goBinariesFact.identity);

  expect(goBinariesFact.identity.type).toMatch(/^go(modules|binary)$/);

  const goBinaryDepGraph: DepGraph = goBinariesFact.facts.find(
    (fact) => fact.type === "depGraph",
  )!.data;

  console.log("📦 Go packages found:", goBinaryDepGraph.getDepPkgs().length);
  console.log("📝 Go package names:", goBinaryDepGraph.getDepPkgs().map(p => p.name).slice(0, 10));

  // The Go binary was detected (which is the main goal)
  console.log("🎯 Go binary detected successfully!");

  if (goBinaryDepGraph.getDepPkgs().length > 0) {
    console.log("📦 Go dependencies found - this is great!");
    const goPackages = goBinaryDepGraph.getDepPkgs();
    expect(goPackages.some(pkg => pkg.name.includes("go") || pkg.name.includes("golang"))).toBeTruthy();
  } else {
    console.log("ℹ️  Go binary detected but no embedded dependencies found (this is normal for some binaries)");
  }
} else {
  console.log("❌ No Go binaries found in scan results");
  console.log("🔍 Available fact types for first result:", 
    pluginResult.scanResults[0].facts.map(f => f.type));
}

// DEMONSTRATE THE BUG: 
// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === 'win32' && esbuildResult) {
  console.log("⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)");
} else if (process.platform !== 'win32' && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

Unused Variable

The variable originalPlatform is declared but never used. The comment on line 121 suggests an intent to mock process.platform, but this is not implemented. The code should be completed to perform the mock or the unused variable and comment should be removed.

// Mock Windows platform to trigger the bug
const originalPlatform = process.platform;

snyk-pr-review-bot · 2025-10-23T21:20:34Z

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Ineffective Test

The test logic from lines 211-224 is designed to demonstrate a bug but relies on console.log instead of expect assertions. This means the test will pass regardless of whether the bug is present or fixed. The test should be updated to use assertions to properly validate the behavior and fail if the conditions are not met.

// DEMONSTRATE THE BUG: 
// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === 'win32' && esbuildResult) {
  console.log("⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)");
} else if (process.platform !== 'win32' && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

Excessive Logging

The new test case contains a large number of console.log statements. While useful for debugging, they should be removed from the final version of the test to keep the test output clean and focused on failures.

console.log("💻 Real OS platform:", originalPlatform);
const fixturePath = getFixture("docker-archives/docker-save/gobinaries-test.tar");
const imageNameAndTag = `docker-archive:${fixturePath}`;

console.log("🔍 Starting scan for:", imageNameAndTag);

// The bug is likely in the Go binary file path matching logic
// Let's see what gets detected with Windows platform mocking

const pluginResult = await plugin.scan({
  path: imageNameAndTag,
  "app-vulns": true, // Enable application vulnerability scanning for Go binaries
});

console.log("📊 Scan completed. Results count:", pluginResult.scanResults.length);
console.log("📋 Scan results overview:", 
  pluginResult.scanResults.map(r => ({ 
    type: r.identity.type, 
    target: r.identity.targetFile || 'container' 
  }))
);

// Check if esbuild binary is found (this should differ between Windows and Linux)
const esbuildResult = pluginResult.scanResults.find(r => 
  r.identity.targetFile && r.identity.targetFile.includes('esbuild')
);
console.log("🔍 esbuild binary result:", esbuildResult ? "FOUND" : "NOT FOUND");
if (esbuildResult) {
  console.log("📍 esbuild path:", esbuildResult.identity.targetFile);
}

// The bug: Windows should find fewer results (missing esbuild)
// Linux finds 4 projects, Windows finds 3 projects  
console.log("🧮 Expected behavior: Linux=4 projects, Windows=3 projects (missing esbuild)");

const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "depGraph",
)!.data;
expect(depGraph.rootPkg.name).toEqual("docker-image|gobinaries-test.tar");
expect(depGraph.rootPkg.version).toBeUndefined();

const imageId: string = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "imageId",
)!.data;

console.log("🏷️  Image ID:", imageId);

// Check that we get the correct image ID from our gobinaries archive
expect(imageId).toBeDefined();
expect(imageId).toMatch(/^sha256:[a-f0-9]{64}$/);

// Verify that Go binaries are detected and scanned
const goBinariesFact = pluginResult.scanResults.find(
  (result) => result.identity.type === "gomodules" || 
             result.identity.type === "gobinary"
);

console.log("🔍 Looking for Go binaries...");
console.log("🧩 All scan result types:", pluginResult.scanResults.map(r => r.identity.type));

if (goBinariesFact) {
  console.log("✅ Found Go binaries fact:", goBinariesFact.identity);

  expect(goBinariesFact.identity.type).toMatch(/^go(modules|binary)$/);

  const goBinaryDepGraph: DepGraph = goBinariesFact.facts.find(
    (fact) => fact.type === "depGraph",
  )!.data;

  console.log("📦 Go packages found:", goBinaryDepGraph.getDepPkgs().length);
  console.log("📝 Go package names:", goBinaryDepGraph.getDepPkgs().map(p => p.name).slice(0, 10));

  // The Go binary was detected (which is the main goal)
  console.log("🎯 Go binary detected successfully!");

  if (goBinaryDepGraph.getDepPkgs().length > 0) {
    console.log("📦 Go dependencies found - this is great!");
    const goPackages = goBinaryDepGraph.getDepPkgs();
    expect(goPackages.some(pkg => pkg.name.includes("go") || pkg.name.includes("golang"))).toBeTruthy();
  } else {
    console.log("ℹ️  Go binary detected but no embedded dependencies found (this is normal for some binaries)");
  }
} else {
  console.log("❌ No Go binaries found in scan results");
  console.log("🔍 Available fact types for first result:", 
    pluginResult.scanResults[0].facts.map(f => f.type));
}

// DEMONSTRATE THE BUG: 
// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === 'win32' && esbuildResult) {
  console.log("⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)");
} else if (process.platform !== 'win32' && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

Risky Assertions

The test uses non-null assertions (!) when accessing properties of objects returned by find(). This can lead to runtime errors if the find() method returns undefined. It would be safer to add explicit checks or assertions to ensure the objects exist before accessing their properties.

const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "depGraph",
)!.data;
expect(depGraph.rootPkg.name).toEqual("docker-image|gobinaries-test.tar");
expect(depGraph.rootPkg.version).toBeUndefined();

const imageId: string = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "imageId",
)!.data;

console.log("🏷️  Image ID:", imageId);

// Check that we get the correct image ID from our gobinaries archive
expect(imageId).toBeDefined();
expect(imageId).toMatch(/^sha256:[a-f0-9]{64}$/);

// Verify that Go binaries are detected and scanned
const goBinariesFact = pluginResult.scanResults.find(
  (result) => result.identity.type === "gomodules" || 
             result.identity.type === "gobinary"
);

console.log("🔍 Looking for Go binaries...");
console.log("🧩 All scan result types:", pluginResult.scanResults.map(r => r.identity.type));

if (goBinariesFact) {
  console.log("✅ Found Go binaries fact:", goBinariesFact.identity);

  expect(goBinariesFact.identity.type).toMatch(/^go(modules|binary)$/);

  const goBinaryDepGraph: DepGraph = goBinariesFact.facts.find(
    (fact) => fact.type === "depGraph",
  )!.data;

  console.log("📦 Go packages found:", goBinaryDepGraph.getDepPkgs().length);
  console.log("📝 Go package names:", goBinaryDepGraph.getDepPkgs().map(p => p.name).slice(0, 10));

  // The Go binary was detected (which is the main goal)
  console.log("🎯 Go binary detected successfully!");

  if (goBinaryDepGraph.getDepPkgs().length > 0) {
    console.log("📦 Go dependencies found - this is great!");
    const goPackages = goBinaryDepGraph.getDepPkgs();
    expect(goPackages.some(pkg => pkg.name.includes("go") || pkg.name.includes("golang"))).toBeTruthy();
  } else {
    console.log("ℹ️  Go binary detected but no embedded dependencies found (this is normal for some binaries)");
  }
} else {
  console.log("❌ No Go binaries found in scan results");
  console.log("🔍 Available fact types for first result:", 
    pluginResult.scanResults[0].facts.map(f => f.type));
}

// DEMONSTRATE THE BUG: 
// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === 'win32' && esbuildResult) {
  console.log("⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)");
} else if (process.platform !== 'win32' && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

const imageLayers: string[] = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "imageLayers",
)!.data;

Incomplete Mocking

The comment on line 121 suggests that the Windows platform should be mocked to trigger the bug, but the implementation is missing. The variable originalPlatform is declared but process.platform is never changed. This makes the test dependent on the operating system it runs on. If the intent is to simulate a Windows environment, platform mocking should be properly implemented.

// Mock Windows platform to trigger the bug
const originalPlatform = process.platform;

snyk-pr-review-bot · 2025-10-23T21:24:37Z

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Ineffective Test Assertion

The test identifies and logs whether a bug is reproduced but does not use an assertion to fail the test run. Tests should use expect to fail when behavior is incorrect, rather than logging warnings, to be effective in a CI/CD pipeline.

if (process.platform === "win32" && esbuildResult) {
  console.log(
    "⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)",
  );
} else if (process.platform !== "win32" && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

Non-Deterministic Test

The test's behavior depends on the runtime environment (process.platform), making its outcome non-deterministic. The comment on line 122 suggests mocking the platform, which would be a better approach to create consistent and reliable tests for both Windows and non-Windows behavior. Also, the originalPlatform variable is declared but never used.

const originalPlatform = process.platform;

Excessive Logging

The new test case contains a large number of console.log statements. While useful for debugging, they should be removed from the final version of the test to keep test output clean and focused on failures.

console.log("💻 Real OS platform:", originalPlatform);
const fixturePath = getFixture(
  "docker-archives/docker-save/gobinaries-test.tar",
);
const imageNameAndTag = `docker-archive:${fixturePath}`;

console.log("🔍 Starting scan for:", imageNameAndTag);

// The bug is likely in the Go binary file path matching logic
// Let's see what gets detected with Windows platform mocking

const pluginResult = await plugin.scan({
  path: imageNameAndTag,
  "app-vulns": true, // Enable application vulnerability scanning for Go binaries
});

console.log(
  "📊 Scan completed. Results count:",
  pluginResult.scanResults.length,
);
console.log(
  "📋 Scan results overview:",
  pluginResult.scanResults.map((r) => ({
    type: r.identity.type,
    target: r.identity.targetFile || "container",
  })),
);

// Check if esbuild binary is found (this should differ between Windows and Linux)
const esbuildResult = pluginResult.scanResults.find(
  (r) => r.identity.targetFile && r.identity.targetFile.includes("esbuild"),
);
console.log(
  "🔍 esbuild binary result:",
  esbuildResult ? "FOUND" : "NOT FOUND",
);
if (esbuildResult) {
  console.log("📍 esbuild path:", esbuildResult.identity.targetFile);
}

// The bug: Windows should find fewer results (missing esbuild)
// Linux finds 4 projects, Windows finds 3 projects
console.log(
  "🧮 Expected behavior: Linux=4 projects, Windows=3 projects (missing esbuild)",
);

const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "depGraph",
)!.data;
expect(depGraph.rootPkg.name).toEqual("docker-image|gobinaries-test.tar");
expect(depGraph.rootPkg.version).toBeUndefined();

const imageId: string = pluginResult.scanResults[0].facts.find(
  (fact) => fact.type === "imageId",
)!.data;

console.log("🏷️  Image ID:", imageId);

// Check that we get the correct image ID from our gobinaries archive
expect(imageId).toBeDefined();
expect(imageId).toMatch(/^sha256:[a-f0-9]{64}$/);

// Verify that Go binaries are detected and scanned
const goBinariesFact = pluginResult.scanResults.find(
  (result) =>
    result.identity.type === "gomodules" ||
    result.identity.type === "gobinary",
);

console.log("🔍 Looking for Go binaries...");
console.log(
  "🧩 All scan result types:",
  pluginResult.scanResults.map((r) => r.identity.type),
);

if (goBinariesFact) {
  console.log("✅ Found Go binaries fact:", goBinariesFact.identity);

  expect(goBinariesFact.identity.type).toMatch(/^go(modules|binary)$/);

  const goBinaryDepGraph: DepGraph = goBinariesFact.facts.find(
    (fact) => fact.type === "depGraph",
  )!.data;

  console.log(
    "📦 Go packages found:",
    goBinaryDepGraph.getDepPkgs().length,
  );
  console.log(
    "📝 Go package names:",
    goBinaryDepGraph
      .getDepPkgs()
      .map((p) => p.name)
      .slice(0, 10),
  );

  // The Go binary was detected (which is the main goal)
  console.log("🎯 Go binary detected successfully!");

  if (goBinaryDepGraph.getDepPkgs().length > 0) {
    console.log("📦 Go dependencies found - this is great!");
    const goPackages = goBinaryDepGraph.getDepPkgs();
    expect(
      goPackages.some(
        (pkg) => pkg.name.includes("go") || pkg.name.includes("golang"),
      ),
    ).toBeTruthy();
  } else {
    console.log(
      "ℹ️  Go binary detected but no embedded dependencies found (this is normal for some binaries)",
    );
  }
} else {
  console.log("❌ No Go binaries found in scan results");
  console.log(
    "🔍 Available fact types for first result:",
    pluginResult.scanResults[0].facts.map((f) => f.type),
  );
}

// DEMONSTRATE THE BUG:
// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === "win32" && esbuildResult) {
  console.log(
    "⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)",
  );
} else if (process.platform !== "win32" && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

snyk-pr-review-bot · 2025-10-23T21:44:55Z

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Confusing Test Logic

The new test case appears to be written to demonstrate the original bug, rather than to verify the fix. The logic and comments (lines 244-258) suggest that finding the esbuild binary on Windows is an unexpected state. However, with the fix applied in lib/extractor/layer.ts, this should be the correct and expected behavior. The test should be refactored to assert the correct, post-fix behavior (i.e., that the esbuild binary is found on all platforms).

// - On Linux: should find esbuild binary (4 total results)
// - On Windows: missing esbuild binary (3 total results)
console.log("🐛 BUG REPRODUCTION ATTEMPT:");
console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
console.log(`   Expected on Linux: 4 results with esbuild`);
console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

if (process.platform === "win32" && esbuildResult) {
  console.log(
    "⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)",
  );
} else if (process.platform !== "win32" && !esbuildResult) {
  console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
}

Excessive Logging

The new test case includes a large number of console.log statements. While these are helpful for debugging during development, they should be removed from the final test code to keep the test output clean and focused on failures.

it("can scan docker-archive with go binaries", async () => {
  // Mock Windows platform to trigger the bug
  const originalPlatform = process.platform;
  console.log("💻 Real OS platform:", originalPlatform);
  const fixturePath = getFixture(
    "docker-archives/docker-save/gobinaries-test.tar",
  );
  const imageNameAndTag = `docker-archive:${fixturePath}`;

  console.log("🔍 Starting scan for:", imageNameAndTag);

  // The bug is likely in the Go binary file path matching logic
  // Let's see what gets detected with Windows platform mocking

  const pluginResult = await plugin.scan({
    path: imageNameAndTag,
    "app-vulns": true, // Enable application vulnerability scanning for Go binaries
  });

  console.log(
    "📊 Scan completed. Results count:",
    pluginResult.scanResults.length,
  );
  console.log(
    "📋 Scan results overview:",
    pluginResult.scanResults.map((r) => ({
      type: r.identity.type,
      target: r.identity.targetFile || "container",
    })),
  );

  // Check if esbuild binary is found (this should differ between Windows and Linux)
  const esbuildResult = pluginResult.scanResults.find(
    (r) => r.identity.targetFile && r.identity.targetFile.includes("esbuild"),
  );
  console.log(
    "🔍 esbuild binary result:",
    esbuildResult ? "FOUND" : "NOT FOUND",
  );
  if (esbuildResult) {
    console.log("📍 esbuild path:", esbuildResult.identity.targetFile);
  }

  // The bug: Windows should find fewer results (missing esbuild)
  // Linux finds 4 projects, Windows finds 3 projects
  console.log(
    "🧮 Expected behavior: Linux=4 projects, Windows=3 projects (missing esbuild)",
  );

  const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
    (fact) => fact.type === "depGraph",
  )!.data;
  expect(depGraph.rootPkg.name).toEqual("docker-image|gobinaries-test.tar");
  expect(depGraph.rootPkg.version).toBeUndefined();

  const imageId: string = pluginResult.scanResults[0].facts.find(
    (fact) => fact.type === "imageId",
  )!.data;

  console.log("🏷️  Image ID:", imageId);

  // Check that we get the correct image ID from our gobinaries archive
  expect(imageId).toBeDefined();
  expect(imageId).toMatch(/^sha256:[a-f0-9]{64}$/);

  // Verify that Go binaries are detected and scanned
  const goBinariesFact = pluginResult.scanResults.find(
    (result) =>
      result.identity.type === "gomodules" ||
      result.identity.type === "gobinary",
  );

  console.log("🔍 Looking for Go binaries...");
  console.log(
    "🧩 All scan result types:",
    pluginResult.scanResults.map((r) => r.identity.type),
  );

  if (goBinariesFact) {
    console.log("✅ Found Go binaries fact:", goBinariesFact.identity);

    expect(goBinariesFact.identity.type).toMatch(/^go(modules|binary)$/);

    const goBinaryDepGraph: DepGraph = goBinariesFact.facts.find(
      (fact) => fact.type === "depGraph",
    )!.data;

    console.log(
      "📦 Go packages found:",
      goBinaryDepGraph.getDepPkgs().length,
    );
    console.log(
      "📝 Go package names:",
      goBinaryDepGraph
        .getDepPkgs()
        .map((p) => p.name)
        .slice(0, 10),
    );

    // The Go binary was detected (which is the main goal)
    console.log("🎯 Go binary detected successfully!");

    if (goBinaryDepGraph.getDepPkgs().length > 0) {
      console.log("📦 Go dependencies found - this is great!");
      const goPackages = goBinaryDepGraph.getDepPkgs();
      expect(
        goPackages.some(
          (pkg) => pkg.name.includes("go") || pkg.name.includes("golang"),
        ),
      ).toBeTruthy();
    } else {
      console.log(
        "ℹ️  Go binary detected but no embedded dependencies found (this is normal for some binaries)",
      );
    }
  } else {
    console.log("❌ No Go binaries found in scan results");
    console.log(
      "🔍 Available fact types for first result:",
      pluginResult.scanResults[0].facts.map((f) => f.type),
    );
  }

  // DEMONSTRATE THE BUG:
  // - On Linux: should find esbuild binary (4 total results)
  // - On Windows: missing esbuild binary (3 total results)
  console.log("🐛 BUG REPRODUCTION ATTEMPT:");
  console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
  console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
  console.log(`   Expected on Linux: 4 results with esbuild`);
  console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

  if (process.platform === "win32" && esbuildResult) {
    console.log(
      "⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)",
    );
  } else if (process.platform !== "win32" && !esbuildResult) {
    console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
  }

  const imageLayers: string[] = pluginResult.scanResults[0].facts.find(
    (fact) => fact.type === "imageLayers",
  )!.data;
  expect(imageLayers.length).toBeGreaterThan(0);
}, 60000); // Increase timeout for Go binary scanning

snyk-pr-review-bot · 2025-10-27T04:12:19Z

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Verbose Logging

The new test case can scan docker-archive with go binaries includes a large number of console.log statements. While this is helpful for debugging, it's best practice to remove them from the final test code to keep CI logs clean and readable.

it("can scan docker-archive with go binaries", async () => {
  // Mock Windows platform to trigger the bug
  const originalPlatform = process.platform;
  console.log("💻 Real OS platform:", originalPlatform);
  const fixturePath = getFixture(
    "docker-archives/docker-save/gobinaries-test.tar",
  );
  const imageNameAndTag = `docker-archive:${fixturePath}`;

  console.log("🔍 Starting scan for:", imageNameAndTag);

  // The bug is likely in the Go binary file path matching logic
  // Let's see what gets detected with Windows platform mocking

  const pluginResult = await plugin.scan({
    path: imageNameAndTag,
    "app-vulns": true, // Enable application vulnerability scanning for Go binaries
  });

  console.log(
    "📊 Scan completed. Results count:",
    pluginResult.scanResults.length,
  );
  console.log(
    "📋 Scan results overview:",
    pluginResult.scanResults.map((r) => ({
      type: r.identity.type,
      target: r.identity.targetFile || "container",
    })),
  );

  // Check if esbuild binary is found (this should differ between Windows and Linux)
  const esbuildResult = pluginResult.scanResults.find(
    (r) => r.identity.targetFile && r.identity.targetFile.includes("esbuild"),
  );
  console.log(
    "🔍 esbuild binary result:",
    esbuildResult ? "FOUND" : "NOT FOUND",
  );
  if (esbuildResult) {
    console.log("📍 esbuild path:", esbuildResult.identity.targetFile);
  }

  // The bug: Windows should find fewer results (missing esbuild)
  // Linux finds 4 projects, Windows finds 3 projects
  console.log(
    "🧮 Expected behavior: Linux=4 projects, Windows=3 projects (missing esbuild)",
  );

  const depGraph: DepGraph = pluginResult.scanResults[0].facts.find(
    (fact) => fact.type === "depGraph",
  )!.data;
  expect(depGraph.rootPkg.name).toEqual("docker-image|gobinaries-test.tar");
  expect(depGraph.rootPkg.version).toBeUndefined();

  const imageId: string = pluginResult.scanResults[0].facts.find(
    (fact) => fact.type === "imageId",
  )!.data;

  console.log("🏷️  Image ID:", imageId);

  // Check that we get the correct image ID from our gobinaries archive
  expect(imageId).toBeDefined();
  expect(imageId).toMatch(/^sha256:[a-f0-9]{64}$/);

  // Verify that Go binaries are detected and scanned
  const goBinariesFact = pluginResult.scanResults.find(
    (result) =>
      result.identity.type === "gomodules" ||
      result.identity.type === "gobinary",
  );

  console.log("🔍 Looking for Go binaries...");
  console.log(
    "🧩 All scan result types:",
    pluginResult.scanResults.map((r) => r.identity.type),
  );

  if (goBinariesFact) {
    console.log("✅ Found Go binaries fact:", goBinariesFact.identity);

    expect(goBinariesFact.identity.type).toMatch(/^go(modules|binary)$/);

    const goBinaryDepGraph: DepGraph = goBinariesFact.facts.find(
      (fact) => fact.type === "depGraph",
    )!.data;

    console.log(
      "📦 Go packages found:",
      goBinaryDepGraph.getDepPkgs().length,
    );
    console.log(
      "📝 Go package names:",
      goBinaryDepGraph
        .getDepPkgs()
        .map((p) => p.name)
        .slice(0, 10),
    );

    // The Go binary was detected (which is the main goal)
    console.log("🎯 Go binary detected successfully!");

    if (goBinaryDepGraph.getDepPkgs().length > 0) {
      console.log("📦 Go dependencies found - this is great!");
      const goPackages = goBinaryDepGraph.getDepPkgs();
      expect(
        goPackages.some(
          (pkg) => pkg.name.includes("go") || pkg.name.includes("golang"),
        ),
      ).toBeTruthy();
    } else {
      console.log(
        "ℹ️  Go binary detected but no embedded dependencies found (this is normal for some binaries)",
      );
    }
  } else {
    console.log("❌ No Go binaries found in scan results");
    console.log(
      "🔍 Available fact types for first result:",
      pluginResult.scanResults[0].facts.map((f) => f.type),
    );
  }

  // DEMONSTRATE THE BUG:
  // - On Linux: should find esbuild binary (4 total results)
  // - On Windows: missing esbuild binary (3 total results)
  console.log("🐛 BUG REPRODUCTION ATTEMPT:");
  console.log(`   Total scan results: ${pluginResult.scanResults.length}`);
  console.log(`   esbuild found: ${esbuildResult ? "YES" : "NO"}`);
  console.log(`   Expected on Linux: 4 results with esbuild`);
  console.log(`   Expected on Windows: 3 results WITHOUT esbuild`);

  if (process.platform === "win32" && esbuildResult) {
    console.log(
      "⚠️  BUG NOT REPRODUCED: esbuild was found on Windows (should be missing)",
    );
  } else if (process.platform !== "win32" && !esbuildResult) {
    console.log("⚠️  UNEXPECTED: esbuild missing on non-Windows platform");
  }

  const imageLayers: string[] = pluginResult.scanResults[0].facts.find(
    (fact) => fact.type === "imageLayers",
  )!.data;
  expect(imageLayers.length).toBeGreaterThan(0);
}, 60000); // Increase timeout for Go binary scanning

Misleading Comment

The comment // Mock Windows platform to trigger the bug is misleading because the test does not actually mock process.platform. It seems to check the real platform the test is running on. This comment should be removed or clarified to avoid confusion.

// Mock Windows platform to trigger the bug

fix CN-360: recreate bug

92e2ce1

sathvi-k requested a review from a team as a code owner October 23, 2025 21:06

sathvi-k requested review from adrobuta and removed request for adrobuta October 23, 2025 21:06

sathvi-k changed the title ~~fix CN-360: recreate bug~~ fix: CN-360 recreate bug Oct 23, 2025

rerun

4797748

fix: rerun

2b621b4

fix: formatting in test

4e03cd2

fix: use posix path in extractor/layer.ts

0b9fec9

fix: more tests

dcbb832

sathvi-k marked this pull request as draft October 27, 2025 04:52

sathvi-k added 8 commits October 27, 2025 17:47

fix: change extension parsing

3c799d4

fix: use posix in extractor/layer.ts

0fe8141

fix: remove unneeded code and tests

b9f1c7d

fix: add back posix

9e5e1f9

fix: remove posix path change

b8019d7

fix: spec with windows formatting

dec1a53

fix: spec

89cc7a5

fix: posix paths

7b5edc8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: CN-360 recreate bug #719

fix: CN-360 recreate bug #719

Uh oh!

sathvi-k commented Oct 23, 2025

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

Uh oh!

snyk-pr-review-bot bot commented Oct 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: CN-360 recreate bug #719

Are you sure you want to change the base?

fix: CN-360 recreate bug #719

Uh oh!

Conversation

sathvi-k commented Oct 23, 2025

What does this PR do?

Where should the reviewer start?

How should this be manually tested?

Any background context you want to provide?

What are the relevant tickets?

Screenshots

Additional questions

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

PR Reviewer Guide 🔍

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

PR Reviewer Guide 🔍

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

PR Reviewer Guide 🔍

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

PR Reviewer Guide 🔍

Uh oh!

snyk-pr-review-bot bot commented Oct 23, 2025

PR Reviewer Guide 🔍

Uh oh!

snyk-pr-review-bot bot commented Oct 27, 2025

PR Reviewer Guide 🔍

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants