docs: add needs_reauth state, reauthorize endpoint, per-client tool sync/timeout, allowed_extra_headers, and OAuth config rotation to MCP OpenAPI spec - #5723
Conversation
|
|
oauth_configs to new mcp_oauth_flows table
#5709
4e71e90 to
eeabf77
Compare
2a24549 to
c2adb1e
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
eeabf77 to
68f08e8
Compare
c2adb1e to
1b9b96a
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Merge activity
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transports/bifrost-http/handlers/mcp.go`:
- Around line 1214-1218: Extend the validation around req.ToolExecutionTimeout
in the create and update handlers to reject values greater than the maximum
whole-second value representable by time.Duration, ((1 << 63) - 1) /
int64(time.Second), returning HTTP 400 before the time.Duration conversion.
Preserve the existing negative-value rejection and use the same validation
behavior for both request paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e9a4469e-664f-4ad0-bf7b-e2caacb1efe0
📒 Files selected for processing (8)
docs/openapi/openapi.jsondocs/openapi/openapi.yamldocs/openapi/paths/management/mcp.yamldocs/openapi/paths/management/oauth.yamldocs/openapi/schemas/management/config.yamldocs/openapi/schemas/management/mcp.yamldocs/openapi/schemas/management/oauth.yamltransports/bifrost-http/handlers/mcp.go
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/openapi/openapi.yaml
- docs/openapi/schemas/management/config.yaml
- docs/openapi/paths/management/oauth.yaml
- docs/openapi/schemas/management/oauth.yaml
- docs/openapi/schemas/management/mcp.yaml
- docs/openapi/paths/management/mcp.yaml
- docs/openapi/openapi.json
| if req.ToolExecutionTimeout < 0 { | ||
| SendError(ctx, fasthttp.StatusBadRequest, "tool_execution_timeout must not be negative") | ||
| return | ||
| } | ||
| resolvedToolExecutionTimeout := time.Duration(req.ToolExecutionTimeout) * time.Second |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'tool_execution_timeout|ToolExecutionTimeout' transports docs/openapi
python - <<'PY'
max_duration_ns = (1 << 63) - 1
max_seconds = max_duration_ns // 1_000_000_000
candidate = max_seconds + 1
wrapped = (candidate * 1_000_000_000) & ((1 << 64) - 1)
if wrapped >= (1 << 63):
wrapped -= (1 << 64)
assert wrapped < 0
print(f"{candidate} seconds wraps to {wrapped} nanoseconds")
PYRepository: maximhq/bifrost
Length of output: 49188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- handler request type and create path ---'
sed -n '1125,1235p' transports/bifrost-http/handlers/mcp.go
sed -n '1540,1585p' transports/bifrost-http/handlers/mcp.go
printf '%s\n' '--- persistence conversion and timeout consumers ---'
sed -n '2160,2220p' transports/bifrost-http/lib/config.go
rg -n -C 4 'ToolExecutionTimeout|tool_execution_timeout' transports/bifrost-http/handlers/mcp.go transports/bifrost-http/lib/config.go transports/bifrost-http/server
printf '%s\n' '--- relevant stack summary ---'
git diff --stat
git status --shortRepository: maximhq/bifrost
Length of output: 31154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- request type definition ---'
rg -n -C 8 'type MCPClientRequest struct|type TableMCPClient struct' transports/bifrost-http/handlers/mcp.go transports
printf '%s\n' '--- exact overflow outcomes ---'
python3 - <<'PY'
MAX_DURATION_NS = (1 << 63) - 1
NANOSECONDS_PER_SECOND = 1_000_000_000
MAX_SECONDS = MAX_DURATION_NS // NANOSECONDS_PER_SECOND
def duration_seconds(seconds):
raw = (seconds * NANOSECONDS_PER_SECOND) & ((1 << 64) - 1)
return raw - (1 << 64) if raw >= (1 << 63) else raw
for seconds in (MAX_SECONDS, MAX_SECONDS + 1, 2 * MAX_SECONDS + 2):
wrapped = duration_seconds(seconds)
print(f"{seconds}: {wrapped} ns, negative={wrapped < 0}")
PY
printf '%s\n' '--- validation around create and update ---'
sed -n '1688,1710p' transports/bifrost-http/handlers/mcp.goRepository: maximhq/bifrost
Length of output: 4808
Reject tool_execution_timeout values that overflow time.Duration.
Before the create and update conversions, reject values above ((1 << 63) - 1) / int64(time.Second) with HTTP 400. Otherwise, 9223372037 seconds wraps negative and can cause HTTP 500 errors, while larger values can wrap to an incorrect short timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@transports/bifrost-http/handlers/mcp.go` around lines 1214 - 1218, Extend the
validation around req.ToolExecutionTimeout in the create and update handlers to
reject values greater than the maximum whole-second value representable by
time.Duration, ((1 << 63) - 1) / int64(time.Second), returning HTTP 400 before
the time.Duration conversion. Preserve the existing negative-value rejection and
use the same validation behavior for both request paths.
Source: Path instructions
…ize error docs, document oauth_config_id immutability
1b9b96a to
db7da55
Compare

Summary
This PR expands the MCP client API surface with new per-client configuration fields, a dedicated reauthorize endpoint, a
needs_reauthconnection state, and clarified semantics around admin discovery credentials, OAuth config rotation, and theverify-headersflow.Changes
New
needs_reauthconnection state: Added toMCPConnectionStateenum. For shared OAuth clients, this is a sticky state set when the connection credential is permanently rejected; the health monitor will not auto-recover it. For per-user clients, it is a response-only projection indicating the retained admin discovery credential needs repair while end-user tool calls continue working.New
POST /api/mcp/client/{id}/reauthorizeendpoint: Redoes the OAuth consent flow for an already-authorized OAuth-based client without delete-and-recreate. Foroauthclients it repairs a dead connection credential; forper_user_oauthclients it repairs the retained admin discovery credential (only allowed when that credential is inneeds_reauth).New per-client fields on create and update requests:
tool_sync_interval(per-client override for the global tool-list sync interval, negative value disables),tool_execution_timeout(per-client override for the global execution timeout), andallowed_extra_headers(allowlist of request-level headers callers may forward to the MCP server at execution time).New global
mcp_tool_sync_intervalconfig field: Global tool-list sync interval in minutes; per-client values override or disable it. Defaults to 10 minutes when unset.OAuth config rotation via
PUT /api/mcp/client/{id}: The update request now accepts anoauth_configfield that rotates the stored OAuth configuration in place. When any field actually changes, all bound tokens are flipped toneeds_reauth. Immutable fields (connection_type,auth_type,connection_string,stdio_config,oauth_config_id) are no longer accepted on update.OAuthConfigRequestgains aresourcefield: RFC 8707 resource indicator sent on authorization and token requests for upstream providers that support resource indicators.verify-headerssemantics updated: Sample header values submitted toPOST /api/mcp/client/{id}/verify-headersare now retained as the admin discovery credential for periodic tool-list refresh rather than being discarded. The endpoint now also serves as a repair path when the retained credential is inneeds_update; repeat calls in any other state return 409.complete-oauthendpoint clarified: The path parameter is theoauth_config_id(not the MCP client ID). The endpoint now handles both create-time flows and reauthorize flows. Replay attempts return 409.statefilter onGET /api/mcp/clientsclarified: Onlyconnectedanddisconnectedare meaningful filter values;disconnectedcovers error, pending,needs_reauth, and disabled states. Theneeds_reauthprojection on per-user clients happens after filtering, so those clients still matchconnected./api/mcp/sessionsclarified: Admin discovery credentials never appear in session listings; only user-, VK-, and session-keyed rows are listed.OAuth config revocation clarified: Revocation is not terminal; admins can restore access via the new reauthorize endpoint.
Type of change
Affected areas
How to test
Validate the new
reauthorizeendpoint for anoauth-type MCP client:DELETE /api/oauth/config/{id}.needs_reauth.POST /api/mcp/client/{id}/reauthorizeand complete the returned flow.connected.Validate per-client
tool_sync_intervalandtool_execution_timeoutby setting them on create/update and confirming they override global config values.Validate
allowed_extra_headersby forwarding a header at execution time and confirming it is accepted or rejected based on the allowlist.Validate
verify-headersrepair path by updatingper_user_header_keyson an existing client, confirming the admin credential flips toneeds_update, then callingverify-headerswith fresh values and confirming it returns to active.Breaking changes
The
MCPClientUpdateRequestschema removes previously accepted immutable fields (connection_type,auth_type,connection_string,stdio_config,oauth_config_id,client_id) from the update body. Clients sending those fields will need to drop them. Thecomplete-oauthpath parameter is clarified to beoauth_config_id, not the MCP client ID — callers using the MCP client ID directly will need to update.Security considerations
verify-headersare never surfaced on/api/mcp/sessionsand are never used for end-user traffic.needs_reauthto all bound tokens, forcing re-authentication; a no-op round trip does not cascade.allowed_extra_headerscontrols which caller-supplied headers reach the upstream MCP server, limiting header injection surface.resourcefield onOAuthConfigRequestscopes tokens to a specific protected resource per RFC 8707, reducing token misuse risk.Checklist
docs/contributing/README.mdand followed the guidelines