Skip to content

Conversation

@adrobuta
Copy link
Contributor

@adrobuta adrobuta commented Nov 14, 2025

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

What does this PR do?

This PR enables scanning for Go binaries that are missing the .gopclntab section:

  • Stripped binaries (built with -ldflags='-s -w')
  • CGo binaries (may lack .gopclntab depending on build configuration)

Previously, these binaries caused scanning failures with error "no pcln tab present in Go binary", resulting in 0 dependencies detected and all vulnerabilities missed.

Where should the reviewer start?

For getting a little bit of context , start here.
The main code change starts here : if .gopclntab is undefined -> module.packages is empty-> so we add the module as dependency to the depgraph.

How should this be manually tested?

Test the fix on a public image that embeds stripped/CGO binaries in the filesystem:

SNYK_API=https://api.snyk.io node dist/cli/index.js container test \
  docker.elastic.co/elastic-agent/elastic-agent-complete:8.18.8
Binary Path Type Modules Status
cloudbeat /usr/share/elastic-agent/.../components/cloudbeat Stripped 502 Detected
agentbeat /usr/share/elastic-agent/.../components/agentbeat Stripped 399 Detected
fleet-server /usr/share/elastic-agent/.../components/fleet-server Stripped 76 Detected
elastic-agent /usr/share/elastic-agent/.../components/elastic-agent Stripped 534 Detected

Any background context you want to provide?

.gopclntab - shows what is compiled into the binary(build output) → can help to build 100% accurate dependency graphs
.go.buildinfo - shows what modules were available during build, there is not way to tell which have compiled → can add false positives

How we use them?

With both .go.buildinfo available and .gopclntab available:

`

  1. Read .go.buildinfo → Get module list: [redis/v9, xxhash, rendezvous]
  2. Read .gopclntab → Get file list: [redis/v9/client.go, redis/v9/internal/pool.go]
  3. Extract packages from files: [redis/v9, redis/v9/internal]
  4. Match packages to modules → Report only: redis/[email protected]
    `

With only .go.buildinfo context (stripped binaries):
`

  1. Read .go.buildinfo → Get module list: [redis/v9, xxhash, rendezvous]
  2. .gopclntab missing → Cannot filter
  3. Report all modules: redis/[email protected], [email protected], [email protected]
  4. `

What are the relevant tickets?

CN-421

Screenshots

Additional questions

@adrobuta adrobuta requested a review from a team as a code owner November 14, 2025 16:45
@adrobuta adrobuta requested a review from dan-arpino November 14, 2025 16:45
@snyk-pr-review-bot
Copy link

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@adrobuta adrobuta changed the title feat: support for stripped/cgo go binareis feat: support for stripped/cgo go binaries Nov 14, 2025
try {
this.matchFilesToModules(new LineTable(pclnTab.data).go12MapFiles());
} catch (err) {
// If pclntab parsing fails, continue with module-only reporting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit/suggestion: should we still log or print to debug if it fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, definitely! thanks for suggesting it!

parker-snyk
parker-snyk previously approved these changes Nov 14, 2025
Copy link
Contributor

@parker-snyk parker-snyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment on logging, but otherwise lgtm!

Make .gopclntab optional when scanning Go binaries. When the section
is missing (stripped/CGo builds), report module-level dependencies
instead of failing. This enables scanning of previously undetectable
binaries.
@snyk-pr-review-bot
Copy link

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Conditional Logic

The dependency graph construction in depGraph now contains three distinct logical paths for handling modules, depending on the presence of .gopclntab and whether a module contains packages. The new fallback for stripped binaries reports all modules from .go.buildinfo, which can include transitive dependencies not compiled into the binary, as noted in the comments (lines 102-105). This is a reasonable trade-off, but the reviewer should confirm this behavior is acceptable. The third path, which skips modules without packages when a .gopclntab is present (lines 113-115), should also be validated to ensure it correctly handles modules used only for version resolution.

for (const module of this.modules) {
  if (eventLoopSpinner.isStarving()) {
    await eventLoopSpinner.spin();
  }

  // If we have package-level information (from pclntab), use it
  if (module.packages.length > 0) {
    for (const pkg of module.packages) {
      const nodeId = `${pkg}@${module.version}`;
      goModulesDepGraph.addPkgNode(
        { name: pkg, version: module.version },
        nodeId,
      );
      goModulesDepGraph.connectDep(goModulesDepGraph.rootNodeId, nodeId);
    }
  } else if (!this.hasPclnTab) {
    // ONLY if .gopclntab is missing (stripped/CGo binaries), report module-level
    // dependencies from .go.buildinfo.
    //
    // Note: .go.buildinfo contains ALL modules from the build graph, including
    // modules required only for version resolution (transitive dependencies with
    // no code actually compiled into the binary). Without .gopclntab, we cannot
    // distinguish these from modules with actual code present.
    const nodeId = `${module.name}@${module.version}`;
    goModulesDepGraph.addPkgNode(
      { name: module.name, version: module.version },
      nodeId,
    );
    goModulesDepGraph.connectDep(goModulesDepGraph.rootNodeId, nodeId);
  }
  // else: pclnTab exists but module has no packages - don't report anything
  // This can happen for modules that are required for version resolution
  // but have no actual code compiled into the binary
}
Type Safety

The new unit tests use binary as any when instantiating GoBinary. This suppresses potential type mismatches between the output of elf.parse() and the Elf type expected by the constructor. Consider creating a test-specific mock or a type assertion function to make the contract explicit and improve test robustness against future changes to either the elfy library or the internal Elf type definition.

const goBinary = new GoBinary(binary as any);
📚 Repository Context Analyzed

This review considered 14 relevant code sections from 7 files (average relevance: 0.84)

Copy link
Contributor

@dan-arpino dan-arpino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like some minor linting errors, other than that, looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants