Skip to content

Fix nil pointer panic in resolveTransportType for protocol scheme images#3944

Merged
amirejaz merged 1 commit intomainfrom
fix/nil-pointer-panic-protocol-scheme-3940
Mar 4, 2026
Merged

Fix nil pointer panic in resolveTransportType for protocol scheme images#3944
amirejaz merged 1 commit intomainfrom
fix/nil-pointer-panic-protocol-scheme-3940

Conversation

@JAORMX
Copy link
Collaborator

@JAORMX JAORMX commented Feb 24, 2026

Summary

  • Fix a nil pointer panic when running protocol scheme images (npx://, uvx://, go://) without an explicit --transport flag. The panic was caused by a classic Go "typed nil in interface" bug where handleProtocolScheme returned a nil *ImageMetadata that was wrapped as a non-nil ServerMetadata interface.
  • Guard against typed nil pointers at the source (GetMCPServer) and defensively at all call sites across CLI, API, and MCP server packages.
  • Add unit tests for resolveTransportType and resolveServerName covering nil interface, typed-nil-in-interface, and valid metadata scenarios.

Root cause

handleProtocolScheme declared var imageMetadata *types.ImageMetadata (nil) and returned it without assignment. GetMCPServer returned this nil *ImageMetadata as a types.ServerMetadata interface, creating a non-nil interface wrapping a nil pointer. resolveTransportType checked serverMetadata != nil (true), then called serverMetadata.GetTransport() which panicked because the promoted method from BaseServerMetadata requires dereferencing the nil outer struct.

Why e2e tests didn't catch it

Every protocol scheme e2e test passes --transport explicitly ("--transport", "stdio" or "--transport", "streamable-http"), causing resolveTransportType to return early without ever calling serverMetadata.GetTransport().

Changes

File Change
pkg/runner/retriever/retriever.go Nil guard before returning imageMetadata as ServerMetadata interface; cleaned up handleProtocolScheme to return nil directly
cmd/thv/app/run_flags.go Safe type assertion in resolveTransportType, buildRunConfig, and configureRemoteAuth
pkg/api/v1/workload_service.go Safe type assertions for both ImageMetadata and RemoteServerMetadata; safe transport resolution
pkg/mcp/server/run_server.go Safe type assertion for ImageMetadata
cmd/thv/app/run_flags_test.go Unit tests for resolveTransportType and resolveServerName with typed-nil cases

Test plan

  • go build ./... passes
  • task lint passes (0 issues)
  • Unit tests pass for all changed packages
  • Verify thv run npx://@zereight/mcp-gitlab --name gitlab no longer panics (manual)

Fixes #3940

🤖 Generated with Claude Code

@github-actions github-actions bot added the size/S Small PR: 100-299 lines changed label Feb 24, 2026
rdimitrov
rdimitrov previously approved these changes Feb 24, 2026
lujunsan
lujunsan previously approved these changes Feb 24, 2026
ChrisJBurns
ChrisJBurns previously approved these changes Feb 24, 2026
@JAORMX JAORMX dismissed stale reviews from ChrisJBurns, lujunsan, and rdimitrov via 9855633 February 25, 2026 08:00
@JAORMX JAORMX force-pushed the fix/nil-pointer-panic-protocol-scheme-3940 branch from 89ca7d4 to 9855633 Compare February 25, 2026 08:00
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Feb 25, 2026
rdimitrov
rdimitrov previously approved these changes Feb 25, 2026
@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

❌ Patch coverage is 25.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.49%. Comparing base (c46ae47) to head (c9239a9).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
pkg/runner/retriever/retriever.go 28.57% 5 Missing ⚠️
pkg/api/v1/workload_service.go 33.33% 1 Missing and 3 partials ⚠️
pkg/mcp/server/run_server.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3944      +/-   ##
==========================================
- Coverage   68.56%   68.49%   -0.07%     
==========================================
  Files         437      437              
  Lines       44662    44665       +3     
==========================================
- Hits        30621    30593      -28     
- Misses      11657    11686      +29     
- Partials     2384     2386       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Mar 2, 2026
amirejaz
amirejaz previously approved these changes Mar 3, 2026
@JAORMX JAORMX force-pushed the fix/nil-pointer-panic-protocol-scheme-3940 branch from 4c0f5ce to 63386a2 Compare March 3, 2026 12:55
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Mar 3, 2026
@JAORMX JAORMX force-pushed the fix/nil-pointer-panic-protocol-scheme-3940 branch from 026abcc to e44fed1 Compare March 4, 2026 08:14
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Mar 4, 2026
Guard against typed nil pointers wrapped in non-nil ServerMetadata
interfaces across all call sites. Protocol scheme images (npx://,
uvx://, go://) returned nil *ImageMetadata from handleProtocolScheme,
which when converted to a ServerMetadata interface became non-nil,
causing panics on method calls like GetTransport().

Fixes #3940

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@JAORMX JAORMX dismissed stale reviews from rdimitrov and amirejaz via c9239a9 March 4, 2026 08:42
@JAORMX JAORMX force-pushed the fix/nil-pointer-panic-protocol-scheme-3940 branch from e44fed1 to c9239a9 Compare March 4, 2026 08:42
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Mar 4, 2026
@amirejaz amirejaz merged commit d527eb2 into main Mar 4, 2026
38 of 39 checks passed
@amirejaz amirejaz deleted the fix/nil-pointer-panic-protocol-scheme-3940 branch March 4, 2026 10:52
reyortiz3 pushed a commit that referenced this pull request Mar 4, 2026
…ges (#3944)

Guard against typed nil pointers wrapped in non-nil ServerMetadata
interfaces across all call sites. Protocol scheme images (npx://,
uvx://, go://) returned nil *ImageMetadata from handleProtocolScheme,
which when converted to a ServerMetadata interface became non-nil,
causing panics on method calls like GetTransport().

Fixes #3940

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nil pointer panic in resolveTransportType for protocol scheme images

5 participants