fix: pass OAuth scopes to DCR and extract granted_scopes from token response#7571
fix: pass OAuth scopes to DCR and extract granted_scopes from token response#7571jamadeo merged 8 commits intoblock:mainfrom
Conversation
56bbd66 to
0b621ea
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b621ea3bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Cargo.toml
Outdated
|
|
||
| [patch.crates-io] | ||
| v8 = { path = "vendor/v8" } | ||
| rmcp = { git = "https://github.com/peschee/rust-sdk.git", branch = "fix-dcr-scopes-v2" } |
There was a problem hiding this comment.
Pin rmcp patch to a fixed commit
The new [patch.crates-io] entry tracks a mutable Git branch, so any lockfile refresh (cargo update, dependency bumps, or lockfile regeneration in CI) can pull different rmcp code than what was reviewed here. That makes builds non-reproducible and can silently reintroduce OAuth regressions or new behavior from later branch commits; use an immutable rev for this patch to keep the dependency stable.
Useful? React with 👍 / 👎.
ba82a26 to
eb8c7c5
Compare
| assert_eq!(std::fs::read_to_string(&path2).unwrap(), "second"); | ||
| // Note: we intentionally don't assert file content here because | ||
| // parallel tests (render_output_truncates_*) share the same static | ||
| // temp file and can overwrite the content between our write and read. |
There was a problem hiding this comment.
yeah, this appears to be a flaky test. Looks like removing this assert loses very little actual test usefulness, so this seems fine to me
472d024 to
c6df961
Compare
…esponse Two fixes for OAuth scope handling in MCP server connections: 1. Patch rmcp to include scopes from WWW-Authenticate in the DCR registration request (see modelcontextprotocol/rust-sdk#705) 2. Extract granted_scopes from the token response instead of always saving an empty vec, so stored credentials accurately reflect what the authorization server granted. Signed-off-by: Peter Siska <63866+peschee@users.noreply.github.com>
The test shares a static temp file path with parallel tests (render_output_truncates_*), which can overwrite the file content between the write and read_to_string calls. The path reuse assertion is the important invariant and is protected by the Mutex. Signed-off-by: Peter Siska <63866+peschee@users.noreply.github.com>
…nges The fork was based on upstream main (19 commits ahead of v0.16.0), which included `allow empty content in CallToolResult` and other unreleased changes that broke goose tests. Rebased to v0.16.0 tag with only the DCR scopes fix on top. Signed-off-by: Peter Siska <63866+peschee@users.noreply.github.com>
…s construction The StoredCredentials struct in rmcp does not have a token_received_at field. Removing it fixes the compilation error that caused CI failures. Signed-off-by: Peter Siska <63866+peschee@users.noreply.github.com>
Signed-off-by: Peter Siska <63866+peschee@users.noreply.github.com>
c6df961 to
bd9bdb4
Compare
Signed-off-by: Peter Siska <63866+peschee@users.noreply.github.com>
1ebf362 to
2aaf14a
Compare
|
@jamadeo upstream has been merged |
* origin/main: (21 commits) fix(ui-desktop): unify path resolution around GOOSE_PATH_ROOT (#7335) fix: pass OAuth scopes to DCR and extract granted_scopes from token response (#7571) fix: write to real file if config.yaml is symlink (#7669) fix: preserve pairings when stopping gateway (#7733) fix: reduce server log verbosity — skip session in instrument, defaul… (#7729) fix: provider test infrastructure (#7738) fix: sanitize streamable HTTP extension names derived from URLs (#7740) refactor: derive GooseMode string conversions with strum (#7706) docs: Add Spraay Batch Payments MCP Extension Tutorial (#7525) fix: flake.nix (#7224) delete goose web (#7696) Add @angiejones as CODEOWNER for documentation (#7711) Add MLflow integration guide (#7563) docs: LM Studio availability (#7698) feat: add Avian as an LLM provider (#7561) Adds `linux-mcp-server` to the goose registry (#6979) fix: add #[serde(default)] to description field on 4 ExtensionConfig variants (#7708) feat: combine TUI UX from alexhancock/tui-goodness with publishing config from jackamadeo/package-tui (#7683) chore: cleanup old sandbox (#7700) Correct windows artifact (#7699) ...
* origin/main: (21 commits) fix(ui-desktop): unify path resolution around GOOSE_PATH_ROOT (#7335) fix: pass OAuth scopes to DCR and extract granted_scopes from token response (#7571) fix: write to real file if config.yaml is symlink (#7669) fix: preserve pairings when stopping gateway (#7733) fix: reduce server log verbosity — skip session in instrument, defaul… (#7729) fix: provider test infrastructure (#7738) fix: sanitize streamable HTTP extension names derived from URLs (#7740) refactor: derive GooseMode string conversions with strum (#7706) docs: Add Spraay Batch Payments MCP Extension Tutorial (#7525) fix: flake.nix (#7224) delete goose web (#7696) Add @angiejones as CODEOWNER for documentation (#7711) Add MLflow integration guide (#7563) docs: LM Studio availability (#7698) feat: add Avian as an LLM provider (#7561) Adds `linux-mcp-server` to the goose registry (#6979) fix: add #[serde(default)] to description field on 4 ExtensionConfig variants (#7708) feat: combine TUI UX from alexhancock/tui-goodness with publishing config from jackamadeo/package-tui (#7683) chore: cleanup old sandbox (#7700) Correct windows artifact (#7699) ...
…e-issue * origin/main: fix(ui-desktop): unify path resolution around GOOSE_PATH_ROOT (#7335) fix: pass OAuth scopes to DCR and extract granted_scopes from token response (#7571) fix: write to real file if config.yaml is symlink (#7669) fix: preserve pairings when stopping gateway (#7733) fix: reduce server log verbosity — skip session in instrument, defaul… (#7729) fix: provider test infrastructure (#7738) fix: sanitize streamable HTTP extension names derived from URLs (#7740) refactor: derive GooseMode string conversions with strum (#7706) docs: Add Spraay Batch Payments MCP Extension Tutorial (#7525) fix: flake.nix (#7224) delete goose web (#7696) Add @angiejones as CODEOWNER for documentation (#7711) Add MLflow integration guide (#7563) docs: LM Studio availability (#7698) feat: add Avian as an LLM provider (#7561) Adds `linux-mcp-server` to the goose registry (#6979) fix: add #[serde(default)] to description field on 4 ExtensionConfig variants (#7708)
Summary
Two fixes for OAuth scope handling when connecting to MCP servers:
WWW-Authenticate: Bearer scope="...", the scopes are parsed but never included in the Dynamic Client Registration (DCR) request. This patches rmcp via[patch.crates-io]to include ascopefield in the DCR request per RFC 7591. Upstream PR: fix(auth): pass WWW-Authenticate scopes to DCR registration request modelcontextprotocol/rust-sdk#705granted_scopes: vec![]was always saved. Now the actual scopes from the token response are extracted and stored in credentials.Test plan
cargo check -p goosecompiles without errorsWWW-Authenticateand verify the DCR registration request includes thescopefield[patch.crates-io]entry