Skip to content

fix(discovery): authorize exact-name GossipSub tool results - #290

Merged
aojea merged 3 commits into
google:mainfrom
rhein1:agent/authorize-exact-tool-gossip
Aug 21, 2026
Merged

fix(discovery): authorize exact-name GossipSub tool results#290
aojea merged 3 commits into
google:mainfrom
rhein1:agent/authorize-exact-tool-gossip

Conversation

@rhein1

@rhein1 rhein1 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses the exact-name discovery gap in #176.

The GossipSub fast path previously returned ServiceAnnounce rows directly. Announcements are useful routing hints, but they do not prove that the current caller is authorized to access the advertised MCP service.

This change:

  • treats exact-name GossipSub rows as candidates rather than final results
  • verifies each candidate through the existing authenticated MCP session
  • confirms the exact tool is present in tools/list before returning it
  • omits authorization, connection, malformed-peer, timeout, and missing-tool failures
  • preserves candidate order and operator-declared routing labels
  • falls back to the existing catalog discovery path when no gossip candidate verifies
  • limits verification to eight workers and one shared five-second window
  • reuses the same description helper as describe_remote_tool

There is no dependency, wire-format, or policy-model change.

Regression coverage

The new test advertises two services on the same peer with the same tool name. The caller's Biscuit authorizes one service and denies the other. Only the authenticated service is returned, with its description populated from the verified MCP session.

Validation

go test -race ./internal/node -run TestVerifyGossipToolRows_AuthenticatesEachServiceOnSamePeer -count=1
go test ./internal/node -count=1

Both pass.

I also ran go test ./... -count=1 in an unprivileged Go 1.25.7 container. The changed package and neighboring packages passed. The repository-wide command remained red on two environment-specific checks outside this patch: the Windows-copied embedded skill fixture retained CRLF, and TestSamBoxNanoInitIntegration could not call unshare in the unprivileged container.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces verification for gossip tool rows in the MCP handlers. Unsolicited announcements are now treated as routing hints and verified through authenticated MCP sessions using the new verifyGossipToolRows and fetchRemoteToolDescription methods. A comprehensive unit test has been added to ensure correct authentication and label preservation. The review feedback suggests a minor optimization to return early from verifyGossipToolRows if the candidates slice is empty, avoiding unnecessary resource allocation and goroutine overhead.

Comment thread internal/node/mcp_handlers.go Outdated
@rhein1

rhein1 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

@aojea This is ready for maintainer review. It closes the exact-name GossipSub authorization gap in #176 by treating announcements as candidates and confirming each through an authenticated service-specific MCP tools/list; unauthorized, malformed, missing, and timed-out candidates are omitted. Current signed head: 7a57715d. All hosted checks are green, including CLA, test, lint, govulncheck, and e2e; the focused regression covers allowed and denied services advertised by the same peer.

Comment thread internal/node/mcp_handlers.go Outdated
@aojea

aojea commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks

@aojea
aojea merged commit 6d57ed2 into google:main Aug 21, 2026
18 checks passed
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.

3 participants