Repository navigation
fix: reject missing issuer in authorization server metadata by default - #1054
Merged
Merged
Conversation
github-actions
Bot
added
T-core
jamadeo
force-pushed
the
jamadeo/require-issuer-by-default
branch
from
July 27, 2026 16:13
9d68844 to
1256db3
Compare
alexhancock
approved these changes
Jul 27, 2026
This was referenced Jul 27, 2026
asto18089
added a commit
to Pinvou/CodeWhale
that referenced
this pull request
Oct 24, 2026
…issuer rmcp 3 rejects authorization-server metadata without an `issuer` by default (modelcontextprotocol/rust-sdk#1054 deliberately re-tightened what codewhale-hq#1051 had relaxed), but servers this tui served under rmcp 2.2 still publish such metadata (rust-sdk#1047). The blast radius is wider than login: OAuthState::set_credentials resolves metadata too, so a stored credential for such a server drops the runtime on load and the login tool then fails at the same discovery step — a server that worked on the 2.2 build becomes unloginnable until its metadata is fixed. Route every AuthorizationManager this module builds through one helper that opts into rmcp's allow-missing-issuer escape hatch, preserving the rmcp 2.2 compatibility posture while keeping the security-relevant issuer-mismatch rejection (the mix-up protection) on. Pinned by stored_tokens_still_load_when_the_authorization_server_omits_issuer (issuer-less mock): red with the setter removed, green with it. The mock's AS metadata gains an omit-issuer toggle for it. Verification: mcp::tests:: 216 passed / 1 ignored; clippy (CI form) shows only the two pre-existing base errors; cargo fmt clean. Signed-off-by: asto <asto18089@126.com>
asto18089
added a commit
to Pinvou/CodeWhale
that referenced
this pull request
Oct 25, 2026
* chore(deps): refresh lockfile for rustls-webpki and webpki-roots advisories
cargo update (in-range only, no manifest changes). Fixes three open
RUSTSEC advisories and one yanked-crate warning from the current lock:
- RUSTSEC-2026-0382: rustls-webpki 0.103.15 -> 0.103.16
- RUSTSEC-2026-0381: webpki-roots 1.0.9 -> 1.0.11
- RUSTSEC-2026-0383: webpki-root-certs 1.0.9 -> 1.0.11
- warning[yanked]: clipboard-win 5.4.1 -> 5.4.3 (with arboard 3.6.1 -> 3.7.0)
Remaining moves are routine patch/minor bumps (tokio 1.54.1, cc 1.10.0,
uuid 1.29.0, libc 0.2.191, icu 2.4 family, pest 2.9.4, flate2 1.1.10,
plist 1.10.3, htmd 0.5.6). No crate moves past rust-version 1.88, so the
workspace MSRV is unaffected. Lock-only commit: no code paths change.
Verification: cargo update --dry-run is now a no-op; cargo deny check
advisories passes (see PR body); cargo check --workspace compiles.
Signed-off-by: asto <asto18089@126.com>
* chore(deps): drop oauth2's bundled reqwest client in codewhale-tui
oauth2 = { version = "5", default-features = false }. The tui uses the
oauth2 crate only for the TokenResponse trait (src/mcp/oauth.rs and four
mcp test fixtures); every HTTP call goes through the tui's own reqwest
client, and the rmcp auth path already receives an external reqwest 0.13
client. oauth2's default feature pulls its bundled reqwest 0.12 binding,
which was the sole requirer of reqwest 0.12.28 in this workspace.
This removes reqwest 0.12.28 from the lock and with it the packages only
it required (quinn, quinn-proto, quinn-udp, quinn's lru-slab, rand 0.10.3,
rand_core 0.10.1, rand_pcg 0.10.2, chacha20 0.10.2): 688 -> 681 locked
packages, reqwest and rand each converge to a single version. The parent
repo's reqwest 0.12 anchor list drops by one (hf-hub keeps the family
alive there).
Verification: cargo check -p codewhale-tui --all-targets --locked passes;
cargo tree -i reqwest@0.12.28 is empty.
Signed-off-by: asto <asto18089@126.com>
* feat(secrets): migrate codewhale-secrets from keyring 3 to keyring 4.2
keyring 4 is a rewrite on keyring-core with per-store crates. The v1 API
(Entry::new / Error::{NoEntry,PlatformFailure,NoStorageAccess}) used by
this crate is unchanged, and v1 auto-initializes the platform store:
apple keychain on macOS, Windows Credential Manager on Windows, and the
sync zbus Secret Service store on *nix — matching the previous per-OS
feature intent.
Linux mapping notes, versus keyring 3's linux-native-sync-persistent +
crypto-rust:
- sync: the zbus Secret Service store used by v1 is a synchronous
(blocking) API, as before.
- persistent: keyring 4 ships no keyutils-persistent composite store, so
the kernel-keyring write-through cache is dropped and the Secret
Service collection is the single (durable) authoritative store.
- crypto-rust: secret-service 5 has no default features, so the store
alone would ship secrets unencrypted over the D-Bus socket. A target-
scoped direct secret-service dependency unions the crypto-rust feature
to keep RustCrypto session encryption, as before.
Dropping keyring 3 removes dbus-secret-service 4.1.0 (and its sha2
0.10/digest 0.10/block-buffer 0.10/crypto-common 0.1 stack) from the
lock: 681 -> 671 packages. sha2 0.10.9 remains via oauth2 only; its
requirer count drops from two to one. keyring 4.2.0 declares
rust_version 1.88.0, equal to the workspace MSRV.
Verification: cargo check -p codewhale-secrets --all-targets --locked;
cargo test -p codewhale-secrets (61 passed) on macOS; the Linux store
path is cfg-gated and CI-verified (no Linux target installed locally).
Signed-off-by: asto <asto18089@126.com>
* feat(tui): migrate codewhale-tui from rmcp 2.2.0 to rmcp 3.7.0
rmcp 3.7.0 declares rust_version 1.88 (equal to the workspace MSRV) and
keeps the whole auth surface this crate uses: AuthorizationManager,
AuthorizationSession, OAuthState, OAuthClientConfig, OAuthTokenResponse
(same StandardTokenResponse<VendorExtraTokenFields, BasicTokenType>
alias), AuthError, and AuthorizationSession::for_scope_upgrade all carry
over. Code changes:
- OAuthState::start_authorization now takes an AuthorizationRequest
builder instead of (scopes, redirect_uri, client_name) arguments.
- AuthorizationManager::discover_metadata was renamed to
resolve_metadata and returns an AuthorizationMetadataResolution
carrying the metadata plus its discovery source. Callers that need to
distinguish real discovery from the legacy endpoint fallback can now
use resolution.source; the tui keeps its existing handling.
- OAuthState gained an AuthorizedHttpClient variant (covered by the
existing wildcard arm).
Test fixture: rmcp 3 enforces the RFC 8414 issuer check, validating the
authorization server's advertised issuer against the AS URL named in the
protected resource metadata (or the discovery URL when no resource
metadata exists). OAuthMcpMock served only authorization-server metadata
with a bare-origin issuer while the manager's base URL carries the /mcp
path, so every stored-credential flow failed the new validation. The
mock now also serves /.well-known/oauth-protected-resource with
resource + authorization_servers, which is the shape the current MCP
authorization spec expects and exercises the new discovery path.
Verification: cargo check -p codewhale-tui --all-targets --locked; cargo
test -p codewhale-tui --lib mcp:: (236 passed).
Signed-off-by: asto <asto18089@126.com>
* chore(deps): remove unused cli windows dep and tui pretty_assertions
cargo-shear cleanup, three findings fixed and the rest dispositioned:
- crates/cli: drop the target.'cfg(windows)'.dependencies windows entry.
No cli source references the windows crate; the kill-on-job-close side
of codewhale-hq#3259 lives in codewhale-tui's shell dispatcher (hooks/executor.rs,
tools/shell.rs, fleet/host.rs), which declares its own windows dep with
the features it needs. The comment above libc credited the Windows Job
Object to this crate — corrected.
- crates/tui: drop the pretty_assertions dev-dependency; nothing imports
it (the name appears only as a scanned string in the project-init
template linter).
- crates/tui: keep rio-vt. It is imported by the long-running-tests PTY
harness (tests/cucumber/main.rs -> qa_harness/frame.rs), which
cargo-shear's default feature set never compiles; suppress it via
[package.metadata.cargo-shear] instead of removing a live dependency
(and its windows-sys 0.42 chain stays with it for now).
- unlinked-file findings: all three files are dynamically linked —
fleet_roster_capability_tests.rs via include! from
fleet_roster/tests.rs, tests/support/llm_client.rs via #[path] from
tests/integration/main.rs, tests/support/qa_harness/mod.rs via #[path]
from tests/cucumber/main.rs. Listed in ignored-paths.
- crates/secrets: secret-service is a feature-union-only dependency
(crypto-rust session encryption for keyring's zbus store) and is never
imported; suppressed via package metadata.
Lock: 671 -> 668 packages (pretty_assertions and the cli windows crate
leave; they pulled nothing exclusive). cargo shear exits clean.
Signed-off-by: asto <asto18089@126.com>
* chore(deps): move the workspace onto dirs 7
dirs = "7.0.0" in [workspace.dependencies]; cli, paths, and tui consume it
via the workspace dep. The two functions this workspace uses (home_dir,
config_dir) kept their Option<PathBuf> signatures in 7.0.0, so no call
sites change.
shellexpand 3.1.2 — latest, and the tilde expansion behind two tui call
sites (core/engine.rs, config/paths.rs) — still requires dirs ^6, so
dirs 6.0.0 stays in the lock next to 7.0.0 (both share dirs-sys 0.5.0).
The single-version skip for dirs in deny.toml's duplicate ratchet gains
the 6.0.0 entry with that upstream blockage noted. On the parent repo
this removes codewhale-paths and codewhale-tui from the dirs-6 anchor
list (hf-hub and shellexpand keep that family split there).
Verification: cargo check -p codewhale-cli -p codewhale-paths
-p codewhale-tui --all-targets --locked; cargo fmt --all --check.
dirs 7.0.0 declares no rust-version (workspace MSRV 1.88 unaffected).
Signed-off-by: asto <asto18089@126.com>
* chore(deps): hold aes at 0.9.2 for the 1.88 MSRV
secret-service's crypto-rust feature (enabled for keyring 4's zbus store
in codewhale-secrets) introduces aes to the lock; 0.9.3 raised
rust_version to 1.89, past the workspace's declared 1.88 MSRV. Pin the
lock to aes 0.9.2 (rust_version 1.85). Same 0.9 semver, API-compatible.
Note: cargo update --dry-run will offer aes 0.9.2 -> 0.9.3 until upstream
lowers 0.9.3's rust-version or the workspace MSRV moves past 1.88; every
other in-range update stays a no-op. uuid (1.89) and serde-saphyr (1.89)
already exceeded the declared MSRV on the untouched base, so 1.88 builds
were already not clean before this branch; this pin adds no new member to
that set.
Verification: cargo check -p codewhale-secrets --locked (host) and
--target x86_64-unknown-linux-gnu (the crypto-rust consumer path);
cargo metadata rust_version scan shows no new >1.88 package vs the
pristine base.
Signed-off-by: asto <asto18089@126.com>
* chore(deny): align the advisory ignores and duplicate ratchet with the round-3 graph
Advisories: ignore RUSTSEC-2026-0370/0371/0372 (zvariant, zbus_names,
zvariant_derive unmaintained as standalone crates — merged into zbus 6 as
modules). They enter via secret-service 5 -> zbus 5, which keyring 4's
zbus Secret Service store needs; no patched versions exist and
secret-service 5.2.0 (latest) still requires zbus ^5, so there is no safe
upgrade. CI's advisories lane is continue-on-error; this keeps the local
gate meaningful for future advisories instead.
Duplicate ratchet, reflecting what the round-3 commits did to the lock:
- remove reqwest 0.12.28 (converged to 0.13 by dropping oauth2's bundled
client) and core-foundation 0.9.4 and security-framework 2.11.1 (both
single-version now);
- remove windows-targets and the eight windows_* DLL skip entries (the
0.53 family left the lock via in-range re-unification; the remaining
0.42 DLL copies are only reachable on Windows hosts, which cargo-deny
does not evaluate on the ubuntu CI lane);
- add miniz_oxide 0.8.9 (png 0.18 via image/arboard; flate2 1.1.10 moved
to 0.9) and annotate the surviving windows-sys entry with its rio-vt
origin.
cargo deny check advisories and bans/licenses/sources now report exactly
the pristine-base warning set (two pre-existing license-not-encountered
notes and the pre-existing nom 7/8 duplicate).
Signed-off-by: asto <asto18089@126.com>
* fix(tui): keep rmcp 2.2-era servers loginable when AS metadata omits issuer
rmcp 3 rejects authorization-server metadata without an `issuer` by
default (modelcontextprotocol/rust-sdk#1054 deliberately re-tightened
what codewhale-hq#1051 had relaxed), but servers this tui served under rmcp 2.2
still publish such metadata (rust-sdk#1047). The blast radius is wider
than login: OAuthState::set_credentials resolves metadata too, so a
stored credential for such a server drops the runtime on load and the
login tool then fails at the same discovery step — a server that worked
on the 2.2 build becomes unloginnable until its metadata is fixed.
Route every AuthorizationManager this module builds through one helper
that opts into rmcp's allow-missing-issuer escape hatch, preserving the
rmcp 2.2 compatibility posture while keeping the security-relevant
issuer-mismatch rejection (the mix-up protection) on.
Pinned by
stored_tokens_still_load_when_the_authorization_server_omits_issuer
(issuer-less mock): red with the setter removed, green with it. The
mock's AS metadata gains an omit-issuer toggle for it.
Verification: mcp::tests:: 216 passed / 1 ignored; clippy (CI form)
shows only the two pre-existing base errors; cargo fmt clean.
Signed-off-by: asto <asto18089@126.com>
* chore(deny): correct the stale ratchet notes for the round-3 graph
The [bans] header still described the pre-round-3 lock: it named
reqwest 0.12 vs 0.13 as a surviving duplicate (this branch converges
reqwest on 0.13.5) and cited the old 708 -> 679 shrink. Restate the
surviving set after 688 -> 669.
Name both dirs-6 requirers: shellexpand 3 and teletypewriter 0.5.28
(rio-vt's optional pty chain), so removing the skip after a shellexpand
upgrade alone does not surprise anyone.
Document why the windows_* 0.42.2 / 0.52.6 duplicates that survive via
windows-sys 0.42 are deliberately not skipped: on the ubuntu-only deny
lane those crates resolve to one version, so any skip (bare or versioned)
trips unnecessary-skip / unmatched-skip and adds warnings to the
warn-only ratchet — verified by trying both forms on a host graph, which
went from 1 warning (nom) to 8. Windows hosts see the duplicate warnings
locally; the trade-off is now written down instead of implied.
Comment-only; `cargo deny check bans licenses sources` unchanged (one
pre-existing nom warning), advisories ok.
Signed-off-by: asto <asto18089@126.com>
* docs: record the aes 0.9.2 lock hold for the 1.88 MSRV
The hold lived only in Cargo.lock and the PR prose: nothing in-repo said
why aes is pinned at 0.9.2, so the next bare `cargo update` would move it
to 0.9.3 (rust-version 1.89, pulled by secret-service's crypto-rust) and
surface as an opaque MSRV error. Note it next to the workspace
rust-version, with the two conditions under which to re-examine it.
Signed-off-by: asto <asto18089@126.com>
* docs: drop the stale libdbus build requirement from Linux install docs
The keyring 4 migration replaced dbus-secret-service (libdbus-1 via
libdbus-sys) with the pure-Rust zbus Secret Service store; dbus and
libdbus-sys left the lock. INSTALL.md (en/zh_hans/id), ENVIRONMENTS.md
and CNB_MIRROR.md still told Linux users to install libdbus-1-dev /
dbus-devel and claimed the cli links libdbus-1 and that prebuilt binaries
need the runtime library. Replace those with the actual requirement (a C
toolchain and pkg-config) and note that builds from this version on have
no libdbus runtime dependency.
CI workflows and the Dockerfile still install libdbus-1-dev; that is
harmless and left for an infra-scoped change.
Signed-off-by: asto <asto18089@126.com>
* chore(secrets): drop the redundant direct secret-service dependency
keyring 4.2's own v1 feature already activates the zbus Secret Service
store with crypto-rust, and the store forwards secret-service/crypto-rust,
so session encryption stays RustCrypto without the direct dep. The
resolved graph is unchanged: secret-service 5.2.0 keeps aes/cbc/hkdf via
the keyring -> zbus-secret-service-keyring-store edge, the lock count
stays 669, and the only lock delta is the dropped codewhale-secrets edge.
The cargo-shear suppression existed only to excuse the never-imported
direct dep and goes with it; cargo shear stays clean.
Signed-off-by: asto <asto18089@126.com>
* chore(deny): pin the deny graph to linux-gnu so advisory ignores stay policed
The cargo-deny lane's musl toolchain resolves a host graph that excludes
the target-gated zbus 5 family, so the three RUSTSEC-2026-037x ignores
added for it were never exercised anywhere: a typo would have failed
silently. Pinning [graph] targets makes the graph host-independent —
verified by a negative test that drops the three ignores and fails the
advisories check with exactly RUSTSEC-2026-0370/0371/0372.
The pinned graph also re-baselines the ratchet: getrandom 0.3 (ahash,
wezterm-blob-leases) is a real duplicate and joins the documented skips;
windows-sys and itertools 0.13 never enter the graph, so their skips go —
the windows-gated lock duplicates need no entries at all now, and the
host-dependent windows warning narrative is gone with them. The bans
check is warning-clean (the nom build-dep chain is outside the target
graph); the license notes are unchanged in kind.
Signed-off-by: asto <asto18089@126.com>
* chore(audit): track the syntect and zbus advisory clusters in audit.toml
cargo audit scans the lockfile without target filtering. Register the
full accepted-unmaintained set so the file's clean-audit contract holds:
the zbus 5 family (RUSTSEC-2026-0370/0371/0372) that arrived with
keyring 4's Secret Service store — no safe upgrade until secret-service
adopts zbus 6, tracked identically in deny.toml — and the two
syntect-cluster crates (bincode, yaml-rust) that deny.toml already
accepted but this file had never listed. cargo audit now reports zero
warnings on the branch lock.
Signed-off-by: asto <asto18089@126.com>
* docs: drop the stale DBus headers claim from the devcontainer notes
The workspace no longer links libdbus, so the devcontainer paragraph
must not call the DBus development headers a workspace requirement.
Signed-off-by: asto <asto18089@126.com>
* test(tui): pin that issuer mismatch still flips stored tokens to needs-auth
The omit-issuer pin covers the arm the tui opts into; the mirror case —
an authorization server publishing an issuer that does not match its URL
— must stay rejected unconditionally, since set_allow_missing_issuer
gates only the missing-issuer branch. The mock gains a mismatch toggle
serving a TEST-NET-1 issuer, and the new test drives the stored-token
path to needs-auth with the synthetic authenticate tool, no real tools,
and zero token exchanges. This pins the tui-side contract against future
rmcp bumps weakening the mismatch branch.
Signed-off-by: asto <asto18089@126.com>
* fix(deny): resolve the union of shipped targets so platform coverage returns
Replace the single linux-gnu graph pin with every target the repo ships
or validates: the seven prebuilt triples (linux gnu/musl, darwin x64/arm,
windows-msvc x64/arm, android arm64) plus the CI-checked OHOS and FreeBSD
source targets. The union keeps the deny lane's musl container on the same
host-independent graph that pulls in the linux-gnu-only zbus 5 family, so
the zbus-family advisory ignores stay policed (negative control re-verified:
dropping the three RUSTSEC-2026-037x ignores fails with exactly those IDs),
while darwin/windows-gated crates are back under the licenses/bans/sources
checks that the pin had silently excluded.
The wider graph re-surfaces four target-gated lock duplicates; they get
versioned, anchored skips per the ratchet convention: windows-sys 0.52.0
(ring 0.17's windows slice vs fd-lock/teletypewriter at 0.59) and the
rquickjs-sys bindgen ^0.72 build chain (itertools 0.13, nom 7 via cexpr,
shlex 1.3), deferred with the rquickjs 0.14 bump. End state: advisories,
bans, licenses and sources all ok with only the two pre-existing
license-not-encountered warnings.
Signed-off-by: asto18089 <asto18089@126.com>
---------
Signed-off-by: asto <asto18089@126.com>
Signed-off-by: asto18089 <asto18089@126.com>
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
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.
#1051 made rmcp tolerant of missing 'issuer' fields in metadata. This PR changes the default, such that clients will need to opt in to the tolerant behavior.
Motivation and Context
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context