Skip to content

bug: unchecked nil in OAuth causing mcp-go to panic #903

Description

@lmorg

Description

Unchecked nil causing OAuth validation to panic under some conditions.

Source taken from: github.com/mark3labs/mcp-go@v0.54.1/client/transport/oauth.go

func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetadata, error) {

	// ...

	// line 546
	once.Do(func() {
		result, err := h.fetchServerMetadata(ctx)
		h.metadataMu.Lock()
		defer h.metadataMu.Unlock()
		// If SetProtectedResourceMetadataURL swapped in a fresh sync.Once
		// while the fetch was in flight, the configuration that produced
		// `result` is now stale. Discard the result instead of letting it
		// clobber the newer state; the next getServerMetadata call will
		// drive re-discovery through the new once.
		if h.metadataOnce != once {
			return
		}
		if err != nil {
			h.metadataFetchErr = err
			return
		}
		// Only overwrite serverMetadata on a positive result. Preserving the
		// existing value on a no-op fetch (nil metadata, nil error) matches
		// the prior behavior where fetchMetadataFromURL silently returned on
		// non-200 responses without touching shared state.
		if result.metadata != nil {
			h.serverMetadata = result.metadata
			h.metadataFetchErr = nil
		}
		if result.resourceURL != "" {
			h.resourceURL = result.resourceURL
		}
	})

You're checking if:

  1. result, err := h.fetchServerMetadata(ctx) returns an error, then return that error
  2. if result.metadata is not nil, then return that metadata
  3. BUT you're if err is nil AND result.metadata is nil, then the method silently returns a nil interface and no errors

This then causes a panic in:

func (h *OAuthHandler) RegisterClient(ctx context.Context, clientName string) error {
	metadata, err := h.getServerMetadata(ctx) // returns nil, nil
	if err != nil {
		return fmt.Errorf("failed to get server metadata: %w", err)
	}

	if metadata.RegistrationEndpoint == "" {
	// .RegistrationEndpoint doesn't exist on nil, so panic raised
		return errors.New("server does not support dynamic client registration")
	}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions