feat(mcp): confidential OAuth clients via client_secret_env (revives #5060)#6260
Merged
Conversation
Adds `McpOAuthConfig.client_secret_env` field so operators can declare which environment variable holds the OAuth client secret for confidential providers (e.g. Google Workspace MCP servers that reject refresh requests without `client_secret`). Flow: auth_start resolves env var -> persists to vault -> auth_callback and try_refresh include client_secret in form params. Pure PKCE clients (client_secret_env unset) are unaffected. Addresses all review blockers from PR #5060 v1: - Ground-up rebase on upstream/main preserves #5609 (RefreshError enum, single-flight lock, classify_refresh_failure), #5526 (DCR body redaction via redact_token_endpoint_response), and #5069 regression test (vault_set_twice_round_trips_via_env_key) - Bug-D ordering: DCR incompatibility check runs BEFORE vault_set so no stale secret is persisted on misconfiguration - New integration test: client_secret_env_vault_round_trip pins the vault storage contract - Golden fixture regenerated against current main
…ets baseline The new client_secret_env field made an OAuth test initializer non-exhaustive after the main merge; set it to None. Refresh .secrets.baseline for the new client_secret_env doc-example env var name (false positive, consistent with the existing example entries).
…hConfig The cherry-picked feature adds client_secret_env: Option<String> to McpOAuthConfig. The struct-literal in crates/librefang-runtime/tests/mcp_oauth_integration.rs constructs the config without ..Default::default(), so the new field is mandatory there too — otherwise E0063 breaks the runtime test-build lanes (the exact blocker flagged on #5060).
github-actions
Bot
added
area/runtime
houko
marked this pull request as ready for review
June 21, 2026 01:11
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
Revives the confidential-OAuth-client feature from #5060 (originally by @f-liva), rebased onto current
main.#5060 was closed as stale — explicitly not a rejection ("This is not a rejection of the underlying idea... please rebase onto current main... and open a fresh PR").
This is that fresh PR.
Adds
client_secret_envtoMcpOAuthConfigso MCP servers that require aclient_secretin the token-refresh body (e.g. Google Workspace MCP previews:gmailmcp/calendarmcp/drivemcp/chatmcp) work with confidential OAuth clients while still using PKCE.The secret value never lands in
config.toml— only the env-var name is persisted; the value is read from the environment, wrapped inzeroize::Zeroizing, stored in the existing credential vault, and reused on initial exchange and refresh.Commits
feat(mcp-oauth): add client_secret_env for confidential OAuth clients— original feature, authorship preserved (@f-liva).fix(mcp): set client_secret_env in test McpOAuthConfig + refresh secrets baseline— original follow-up.fix(mcp): add client_secret_env to mcp_oauth_integration test McpOAuthConfig— new here.What changed vs the original #5060
main(the original merge-base was ~81 commits back). The feature commit cherry-picked clean; only.secrets.baselineconflicted and was resolved by regenerating it (one new entry: a doc-example string intypes.rs, a false positive now baselined).E0063: missing field client_secret_env in initializer of McpOAuthConfigthat blocked feat(mcp): support confidential OAuth clients via client_secret_env #5060 each timemainmoved.McpOAuthConfigderivesDefaultbut the struct literal incrates/librefang-runtime/tests/mcp_oauth_integration.rsdoes not use..Default::default(), so the new field is mandatory there — addedclient_secret_env: None.McpOAuthConfig { .. }literal in the tree (installer.rs,mcp_oauth.rs,mcp_oauth_integration.rs) now sets the field.Verification
detect-secrets scan --baseline .secrets.baseline(no unbaselined secrets).McpOAuthConfigstruct literals (the E0063 class) — done, all set the field.cargo check/cargo test/ the schema-golden regen could not be run locally. CI must confirm compile +kernel_config_schemagolden + the runtime/api test lanes across platforms. Draft until green.For reviewers
The original design was already reviewed favourably on #5060 (PKCE preserved, env→vault→zeroize secret handling, fail-fast when DCR is attempted with
client_secret_envset).Please re-check the reconciliation with the OAuth redaction system (
redact_token_endpoint_response) that landed onmainafter the original merge-base — the original branch carried a narrower body-scrub; this rebase keepsmain's version.Refs #5060.