fix(executor): repair MCP auth round-trip across config-sync#709
Open
aryasaatvik wants to merge 2 commits intoRhysSullivan:mainfrom
Open
fix(executor): repair MCP auth round-trip across config-sync#709aryasaatvik wants to merge 2 commits intoRhysSullivan:mainfrom
aryasaatvik wants to merge 2 commits intoRhysSullivan:mainfrom
Conversation
Boot-time addSourceFromConfig was forwarding endpoint/headers/queryParams from executor.jsonc into executor.mcp.addSource(...) but silently dropping the auth block. Keychain-backed remote MCP sources started unauthenticated on every boot, the SSE handshake fired with no Authorization header, and upstreams rejected with the "Failed connecting via sse" error class. Add an inline mcpAuthFromConfig converter that strips the secret-public-ref: prefix from file-shape auth into runtime McpConnectionAuth.secretId, and thread it through the MCP remote addSource call. none and oauth2 are identity passthroughs (oauth2's runtime extras are optional, so the file shape is structurally valid as runtime auth). New config-sync.test.ts covers all three auth kinds against an unreachable endpoint, which still persists the source row so the stored auth shape can be asserted without seeding secrets or running an MCP server.
updateSource wrote new auth to the DB but never to executor.jsonc, so the next boot replayed the file's stale auth (or none) and silently overwrote the DB. The smoking gun: after signing into Sentry via the UI, the mcp-oauth2-sentry connection survived in keychain with valid tokens, but the next reboot cleared the source row's auth_connection_id because the file never reflected the link. Mirror the existing addSource/removeSource pattern by calling configFile.upsertSource(toMcpConfigEntry(...)) after putSource. The existing toMcpConfigEntry/authToConfig path handles the runtime->file conversion (writes secret-public-ref:<id> back), so the file stays authoritative for boot replay. Test exercises updateSource against a stub ConfigFileSink and asserts the upsert payload uses secret-public-ref:<new-id> in file shape.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
config-sync.tswas forwarding endpoint/headers/queryParams fromexecutor.jsoncintoexecutor.mcp.addSource(...)but silently dropping theauthblock — and after the credential-binding cutover, the same call also lacked the new mandatorycredentialTargetScope. Result: every authenticated remote MCP source from a config file booted unauthenticated, the SSE handshake fired withoutAuthorization, and upstreams rejected. Adds an inlinemcpAuthFromConfigconverter (filesecret-public-ref:<id>→ inputsecretId) and threadsauth+credentialTargetScopethrough the MCP/OpenAPI/GraphQL add calls.mcp.updateSourceupdated the DB but neverexecutor.jsonc, so the next boot replayed stale or empty auth and overwrote the DB's good state. Mirrors the existingaddSource/removeSourcepattern. Naive write-back would silently drop slot-form auth (authToConfigonly knows thesecretId/connectionIdinput shape), so adds aninputFormFromStoredhelper that resolvessecretSlot/connectionSlotand slot-bound headers/queryParams back to the input form via the source'scredential_bindingrows before rendering the file entry.Test plan
bun run typecheck— 33/33bunx --bun vitest run apps/local/src/server/config-sync.test.ts— 3/3bunx --bun vitest run -t "writes auth changes" packages/plugins/mcp/src/sdk/plugin.test.ts— 1/1executor webagainst a realexecutor.jsoncwith mixed header-bearer and OAuth2 MCP sources: 10/10 sources sync, tool calls return real data through both paths.