Repository navigation
Guard OAuth endpoints by default, exempting only operator-declared local servers - #172
Conversation
…cal servers The OAuth authorization, registration and token endpoints come from the server being signed in to, and the flow POSTs to them from the host. The guard that refuses non-https and internal addresses (guard_endpoint) was opt-in through OAuthFlow::require_public_endpoints, and the module never opted in. A hostile registry-installed server could aim the host at a loopback service or the cloud metadata address. Token refreshes on connect went through the free refresh_if_expired, which never re-checks the stored token endpoint. - McpRegistry turns the guard on. The probe of the server itself and every advertised endpoint must be https and resolve to public addresses. - The guard is decided per server. McpClientConfig.local_oauth_origins (contract 1.30, empty by default) lists origins of servers the operator declares local, such as a loopback development server. Those may use loopback, private or plain-http endpoints, and each exemption is logged. Origins compare by scheme, lowercased host and explicit port. - Connections refresh through OAuthFlow::refresh, which re-checks the stored token endpoint before posting to it. Tests: a plain-http loopback server is refused by detect and begin on a registry built the normal way, and probed once declared local. For a remote, token endpoints at 127.0.0.1, 169.254.169.254 or over http are refused, as is a registration set at 169.254.169.254; a declared-local server keeps the unguarded clients. The registry tests that sign in to loopback stand-ins declare them local, as an operator would.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Tiny Sweeper reviewReviewed head: FindingsNo active actionable findings. Advisory notes
Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): Guard OAuth endpoints by default, exempting only operator-declared local servers. Purpose (whole-pull-request title): "Guard OAuth endpoints by default, exempting only operator-declared local servers." The pull request makes the OAuth endpoint guard secure by default. The authorization, registration and token endpoints a server advertises are requested from the host, so the previously opt-in guard (OAuthFlow::require_public_endpoints) is now enabled inside McpRegistry and the probe, registration and token clients all consult a per-server decision. A new McpClientConfig.local_oauth_origins list, empty by default, lets an operator declare servers as local (for example loopback development servers); those are exempt and each exemption is logged. Origins compare by scheme, lowercased host and explicit port, and malformed entries are ignored with a warning. Connect-time token refresh no longer uses the unguarded free refresh helper but goes through OAuthFlow::refresh, which re-checks the stored token endpoint. The tinymcp-bus contract moves to 1.30 with a README note and the module wiring passes the config list into the registry. The critique lane reports two findings in its summary, but its findings array is empty in the supplied evidence, so no specific findings can be cited. The security lane answered over the 10 code files with no findings; the README was not security-reviewed as prose. The tests lane answered that the guard behavior is covered but that the new config field itself is not: its wire form is unpinned and its module-level consumer is untested. Review snapshot
Completeness: Complete What changedThe registry now builds its OAuthFlow with require_public_endpoints enabled and gained with_local_oauth_origins, fed at module wiring time from McpClientConfig.local_oauth_origins. OAuthFlow carries a local_origins list plus with_local_origins, an origin_of normalizer (scheme, lowercased host, explicit port) and a guards() predicate used by registration_client, probe_client and token_client, which now take a server_url. Connect-time refresh switched from the free refresh_if_expired to OAuthFlow::refresh. The tinymcp-bus contract bumped to 1.30 and the README, endpoint_guard module doc and field rustdoc were updated. New and adjusted tests cover the default-on guard, the per-server exemption and origin normalization, and existing registry tests that use loopback stand-ins now declare them local. No trusted push delta was available for the latest head, so this describes the whole pull request rather than a push-to-push change. Features
Tests
Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
Summary
The OAuth authorization, registration and token endpoints come from the server being signed in to, and the flow POSTs to them from the host. The guard that refuses non-
httpsand internal addresses (guard_endpoint) was opt-in throughOAuthFlow::require_public_endpoints, and the module never opted in. So a hostile registry-installed server could aim the host at a loopback service or the cloud metadata address (169.254.169.254). Token refreshes on connect also went through the freerefresh_if_expired, which never re-checks the stored token endpoint.Secure by default:
McpRegistryturns the guard on. The probe of the server itself and every endpoint it advertises must behttpsand resolve to public addresses.McpClientConfig.local_oauth_originslists the origins of servers the operator declares local, such as a loopback development server. Those may use loopback, private or plain-httpendpoints, and each exemption is logged at warn. Origins compare by scheme, lowercased host and explicit port.OAuthFlow::refresh, which re-checks the stored token endpoint before posting to it.Related issue
None filed. Found by reading the OAuth flow.
API or behavior changes
Contract minor bump: tinymcp-bus → 1.30. It lands after #164 (1.29). A host built on this bus requires a module at 1.30.
McpClientConfig.local_oauth_origins: Vec<String>(serde default, omitted when empty). Empty means every server is guarded.httpis now refused unless its origin is listed. A host whose users sign in to local development servers must list them. OpenHuman's static and registry servers are remote, so nothing changes there by default.McpRegistry::with_local_oauth_originsandOAuthFlow::with_local_origins. A bareOAuthFlow::newstays unguarded for library callers that check endpoints themselves.Validation
On our Linux CI runner (toolchain 1.99.1), on this branch from main e098e53:
cargo fmt --all -- --check: cleancargo clippy --locked --all-targets --all-features -- -D warnings: 0 errors (main: 0)cargo build --locked --all-targets --all-features;RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features: cleanregistry::oauth106/0,registry::ops83/0,registry::connections55/0, including the three new tests.cargo test --locked --all-features --workspace: 3 failures, all timing flakes inregistry::bootandtinybus_module::directoriesthat main also fails in full-suite runs (main had 7 there). No failure is new on this branch, so the default-on guard broke no existing test.OAuthFlow::new(proxy)withoutrequire_public_endpoints),a_loopback_server_is_refused_by_the_oauth_guard_unless_declared_localfails: detection succeeds with a token.token endpoint refused: it resolves to a disallowed address).Tests
a_loopback_server_is_refused_by_the_oauth_guard_unless_declared_local: on a registry built the normal way,detect_authandoauth_beginrefuse a plain-httploopback server, and the same server is probed once declared local.internal_endpoints_are_refused_for_a_remote_and_allowed_for_a_declared_local_server: for a remote, token endpoints at127.0.0.1,169.254.169.254or overhttpare refused, and so is a registration set at169.254.169.254. A declared-local server keeps the unguarded clients.origins_compare_by_scheme_host_and_explicit_port.Documentation
The tinymcp-bus README contract note, the
endpoint_guardmodule doc, and the field's rustdoc.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description