Repository navigation
feat: agent vault mcp support - #436
sheensantoscapadngan wants to merge 3 commits into
Conversation
|
💬 Discussion in Slack: #pr-review-cli-436-agent-vault-mcp-support Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
[Medium impact] The PR appears safe to merge; no new actionable issues remain.
|
|
|
||
| // No WWW-Authenticate goes with it: passing the upstream's challenge on would start the agent's MCP client | ||
| // on an OAuth flow of its own, when the credential belongs to Infisical. | ||
| const upstreamAuthFailedBody = `{"error":"upstream_auth_failed","message":"The MCP server rejected the credential Infisical provided. Reauthorize this service in Infisical Agent Vault."}` |
There was a problem hiding this comment.
every refresh failure ends with this message, even when the provider just blipped, infisical was unreachable, the endpoint 401s for another reason, or the body was too big to replay. credential-refresh already returns status, could refreshCredential pass it up (it's only logged right now) and use two messages:
needs-reauthorization or a 403 insufficient_scope -> "The service needs to be reauthorized in Infisical Agent Vault."
anything else -> "Couldn't get a valid credential for the service. Try again in a moment." with its own error code, e.g. upstream_auth_unavailable
| } | ||
| if injectCredential(req, &matched.credential) { | ||
| outcome.brokered = true | ||
| if matched.credential.refreshable { |
There was a problem hiding this comment.
bearer MCP services skip this, so the upstream 401 and its WWW-Authenticate go straight to the agent, and the agent's MCP client starts its own oauth login (Inspector did exactly that). the proxy overwrites its token anyway, so that can never work. for MCP services where a bearer was injected, could the 401 become a 502 "The service's credential was rejected. Update it in Infisical Agent Vault.", same as the oauth path
| // instead, and Infisical answers with the current token unrefreshed. hostPattern is that of the cached | ||
| // service whose credential the rejected request carried: a token minted for wherever the service points now | ||
| // must never be sent to where it used to point. | ||
| func (c *sessionCache) refreshCredential(sessionToken, serviceID, hostPattern string, rejected *credential, requiredScopes []string) (credential, error) { |
There was a problem hiding this comment.
resolve remembers a refusal for 5s (refusedTTL), but a failed refresh isn't remembered. so while infisical is slow or down and the token's expired, every agent request does upstream 401, then credential-refresh (up to 15s if infisical hangs), then 502, over and over. could a failed refresh be remembered per service + rejected token for a few seconds like resolve does
No description provided.