fix(connections): name rejected OAuth endpoint on mint card (#7765)

[chenmingwei23]

What is the problem?

PR #7739 taught the MCP OAuth rejection banner to name the sanitized endpoint of a URL the credential gate refuses. Two sibling rejection surfaces in the connection flows were left opaque: connections/mint.py (cold mint) set only reason=mint_url_rejected, and connections/warm.py (_credential_bearing_slugs) released a blocked slug with no endpoint identity. So a user connecting an unlisted OAuth MCP server on the Connections card could not tell which endpoint to add to oauth_endpoints.json.

Why it matters to the user

The remedy for a legitimate-but-unlisted identity provider (Okta org, Auth0, self-hosted OIDC) is to add its (host, path) to oauth_endpoints.json. Without the endpoint on the failure surface the user cannot know what to write, so the rejection reads as unfixable -- the gap #7739 closed for the banner, still open on the mint/warm cards.

How the fix solves it (symptom -> root cause)

Symptom: the card names no endpoint. Root cause: the rejection stored only a coarse reason code and never computed the sanitized identity.

• A pasteable-endpoint contract, owned by the helper. sanitized_oauth_endpoint may by contract return a path that is NOT copy-ready -- the shared redaction tag when the path carried a credential, or a truncated path when it was pathologically long -- so joining host + path inline can render host[REDACTED: credential] or a chopped path as if a user could type it. New security.sanitized_oauth_endpoint_display(url) is the single owner of the copy-ready rule: it returns a joined host/path string only when pasteable, else None. Both consumers (the mint card AND the chat banner in chat_runner.py) now call it and neither re-derives the helper's internal sentinels, so a future third consumer inherits the guarantee by default. (This resolves the First Principles concern that the earlier revision put the copy-ready judgment at one call site while the banner sibling still rendered the redacted pair -- the two surfaces now obey one rule enforced by the owner.)
• The mint-state HTTP handler re-projects MintState through an explicit key allowlist, so a view field is invisible to the client unless the handler names it. api_connections_mint_state now adds rejected_endpoint -- without this line the field is dropped at the wire and every layer below is dead code. (This was the First Principles BLOCK on an earlier revision; it was correct.)
• Frontend: ConnectionMintState.rejected_endpoint + i18n key mint_failure_url_rejected_endpoint (12 locales + pseudolocale) rendered when present, unnamed fallback otherwise.
• Copy the reader can act on (UX). The message no longer embeds a JSON schema in a red error line. It names the endpoint and the file (oauth_endpoints.json in the {{productName}} data home) in prose and points to the Connecting a remote OAuth MCP server guide for the exact entry to add. The banner wording is updated in step to avoid drift, and the unnamed fallback gains a next step.
• warm.py keeps its codified release semantics (test_a_url_carrying_a_credential_is_refused_rather_than_stored); the ensuing cold mint re-hits the URL and names the endpoint through the mint surface.

Security: the surfaced value is host+path only; the display helper suppresses a credential-bearing host (None), a redacted path, and a truncated path. The mint/warm log and audit lines stay slug-only, so no inline suppression or CodeQL barrier is needed. docs/system-specs/modules/security.md records the contract.

What tests we did

• Helper contract (test/test_security.py::TestSanitizedOAuthEndpointDisplay): pasteable endpoint joined; empty path -> host/; credential-in-path returns None (not a redacted join); truncated path returns None; unnameable host / userinfo / unparseable -> None. The raw TestSanitizedOAuthEndpoint (23) is unchanged and still passes.
• Wire-layer arrival tests (test/test_connections_handoff.py): the endpoint survives serialization through api_connections_mint_state to the JSON the browser receives; mutation-verified (reverting the handler line -> KeyError), plus a control for the omitted case.
• Mint card (test/test_connections_mint.py): names the endpoint while the query credential never reaches log/audit; two controls, one per unnameable branch -- credential-in-HOST (None) and credential-in-PATH (redacted -> suppressed). Full file 126 pass (two failures are pre-existing/environmental: subprocess imports the package, not pip-installed here; identical on the pristine base).
• Banner (test/test_mcp_oauth_banner.py): updated in step -- names the file + guide instead of the JSON shape; the redacted-path case now falls back to the unnamed banner. 135 pass.
• i18n: 19 i18n:check gates pass; 16 style/QA/parity/productName files (196 tests) pass, including the Korean particle and zh-CN ellipsis/tone rules and the {{productName}} no-hardcode rule.
• Frontend: tsc 0 errors, eslint clean, ConnectionsPage.coverage.test.tsx 107 pass, frontend shards 1/4 and 2/4 green.
• Backend: scripts/check_black_formatting.py passes with the baseline unchanged (security.py and test_mcp_oauth_banner.py are baselined -- edited by hand, not reformatted); isort/flake8/mypy clean on touched files.

Screenshot

Committed under temp-screenshots/7765-endpoint-name/ -- the repo's conventional PR-evidence location (.gitignore keeps it tracked; screenshot-evidence.yml instructs authors to commit there and cleanup-temp-screenshots.yml reaps it later). The Connections card in both states -- endpoint named, and the unnamed fallback:

Pattern harvest

Rule candidate: a value that is computed, stored and rendered can still be invisible if a transport allowlist is not updated -- so the test that matters asserts ARRIVAL at the client boundary, not storage. A first revision here computed, stored, threaded, typed and translated the field into twelve locales with every test green, yet the handler's key allowlist dropped it at the wire. When a handler re-projects a domain object through a named-key allowlist, a feature is only alive once a test drives the real handler and reads the field back out of the serialized payload. Corollary from this round: when a sanitizer's output can be non-pasteable, the "is it safe to surface" rule belongs INSIDE the sanitizer (one owner), not re-derived at each call site, or a second consumer renders the unsafe form under the opposite judgment.

Any other suggestions

The warm path delivers the endpoint transitively (via the cold-mint retry) to respect the codified release contract. If a future change wants warm to name the endpoint directly, that is a deliberate release->fail UX decision and should update test_a_url_carrying_a_credential_is_refused_rather_than_stored in the same commit.

Refs #7739, #7578

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 24511e9de71aef257a008f9436d7e2590bdc1799 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Right root-cause shape: the copy-ready rule is centralized in one security-owned helper, transported through the handler allowlist, and pinned by wire-arrival tests.

Watch

• PR body drifted from the shipped revision: it claims the card copy embeds the {"additional_authorization_endpoints": ...} entry shape and that mint.py re-derives the raw helper's sentinels, but the diff replaces the inline schema (banner and card) with a guide pointer and centralizes the sentinel logic in sanitized_oauth_endpoint_display. Refresh the description so the record matches what ships.

[DESIGN-REVIEWED] 24511e9

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of 24511e9de71aef257a008f9436d7e2590bdc1799 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

The remediation the PR exists to deliver dead-ends at an unlinked "guide", and the two variants of one failure speak two vocabularies.

Watch

• The named-endpoint message defers the crucial entry shape to "the Connecting a remote OAuth MCP server guide" with no link or path — the blind reader: "it is not a link on this screen and I do not know where to find it." The chat banner gives docs/guides/connecting-remote-oauth-mcp-server.md; the card gives nothing, so the fix path stalls at its last step. (Every user this PR targets × task failure × every occurrence.) Note the PR body claims the copy includes the additional_authorization_endpoints shape; the shipped string does not. Fix: render the guide reference as a link, or restore the entry shape in mint_failure_url_rejected_endpoint.
• Same mint_url_rejected failure, two vocabularies: "authorization endpoint … carried a credential" vs "approval address containing credential-like data". The reader "cannot tell whether 'authorization endpoint' and 'approval address' are the same thing" and can't tell why one shows the address and the other hides it. Fix: one term, and have the fallback say the address can't be shown safely.
• The named message is a six-line red paragraph mixing two audiences; the reader "cannot tell which [fix] applies to me" and finds "Kiro Crew data home" opaque. Fix: split diagnosis (always) from the self-hosted remedy (one indented/secondary line).

Suggestions

• In mint_failure_url_rejected_endpoint, replace "in the {{productName}} data home" with the concrete resolved path (e.g. ~/.kiro/crew/oauth_endpoints.json) — the code knows it; the user shouldn't have to.

[UX-REVIEWED] 24511e9

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 24511e9de71aef257a008f9436d7e2590bdc1799 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/security.py:16812 -- "return f\"{host}{path}\"" surfaces rejected endpoints such as localhost/authorize as pasteable even though oauth_endpoints.json rejects them, leaving the advertised remedy ineffective -> Fix: require the same scheme, port, host, and path validation as the extension loader before returning.

[GPT-REVIEWED] 24511e9

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 24511e9de71aef257a008f9436d7e2590bdc1799: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 24511e9de71aef257a008f9436d7e2590bdc1799 — this comment is updated in place on each push.

Review details

No findings that block; one advisory gap in the new copy-ready contract.

FINDING — src/kiro_crew/security.py:16810 — sanitized_oauth_endpoint_display guards a redacted or ellipsis-truncated path (path == _REDACTED_CREDENTIAL_TAG or path.endswith("\u2026")) but never the host, which sanitized_oauth_endpoint truncates markerless via host = host[:_SANITIZED_OAUTH_HOST_MAX_LEN]; a rejected MCP OAuth URL of shape https://<254+-char dotted, non-credential host>/authorize?state=<credential> (query-borne credential → gate rejects, clean path, over-long host) makes the display helper return a silently-chopped <253-char host>/authorize, so the card/banner names a non-pasteable endpoint as the exact string to add to oauth_endpoints.json — the very "reads as actionable and is not" failure the helper was written to prevent (no credential leak: the host passed the scan) → Fix: in sanitized_oauth_endpoint_display return None when the host was truncated (or append the … marker on host truncation in the raw helper and test for it here), so an over-long host falls back to the unnamed message like the path branch already does.

[OPUS-REVIEWED] 24511e9

Verdict parsed from the review's SHA-scoped output markers for commit 24511e9de71aef257a008f9436d7e2590bdc1799.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 24511e9de71aef257a008f9436d7e2590bdc1799: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 24511e9de71aef257a008f9436d7e2590bdc1799 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All repository checks are done. The findings: the guide the new copy points to exists at base; temp-screenshots/ is the documented PR-evidence convention; after this PR the raw sanitized_oauth_endpoint has exactly one production consumer (its new wrapper); and the PR description's claim about the card copy containing the JSON entry shape contradicts the shipped string. Final review:

First-Principles-Verdict: CONCERNS

The fix earns its place end-to-end, but three copy/behavior changes to the #7739 banner and the fallback string ride along undeclared.

What this change ships

Intent: let a user whose OAuth mint was rejected see which endpoint to add to oauth_endpoints.json — a FIX.

1. Connections card names the rejected endpoint + remedy (new i18n key, 13 locales) — justified
2. rejected_endpoint crosses the wire via the handler allowlist — justified
3. New sanitized_oauth_endpoint_display owns the copy-ready rule — one consumer left behind on the raw helper, generalized
4. Chat banner no longer shows host[REDACTED: credential] / truncated paths, names nothing instead — rides along, named harm
5. Chat banner drops the inline JSON entry shape for a guide pointer — rides along; guide exists at base
6. Fallback mint_failure_url_rejected gains "Ask the server owner to fix the URL." in all 13 locales — undeclared
7. security.md updated in the same commit; warm.py comments; screenshot under temp-screenshots/ — mandated/convention

Watch

• Description says the card copy names "AND the entry shape ({"additional_authorization_endpoints": ...})"; the shipped string instead says "see the Connecting a remote OAuth MCP server guide". The diff is the better shape — the description is stale; a reader approving from it approves copy that doesn't ship.
• Items 4–6 change the fix(security): name the rejected OAuth endpoint in the banner (#7578) #7739 banner surface and the unnamed fallback in every locale; each has a defensible harm (pasteable-looking garbage, dead-end message), but none is named in the description's change list.

Subtractions

• Fold the pasteable rule into sanitized_oauth_endpoint and delete the wrapper: after this PR the raw tuple-with-sentinels form has exactly 1 production consumer (grep sanitized_oauth_endpoint in src/: only security.py:16805, the wrapper), and no surface renders the sentinels anymore — two spellings of one contract will diverge.

[FIRST-PRINCIPLES-REVIEWED] 24511e9

[chenmingwei23]

Standing down (closing unmerged, issue stays open)

Under a scope change to obvious-defect-only fixes, this item no longer has a review round available, so I am closing this PR unmerged and leaving #7765 open. The branch is kept. Recording the state so whoever resumes has the full picture.

What is landed and verified (this work was not wrong)

• Wire-layer arrival test, mutation-verified. The mint-state HTTP handler (api_connections_mint_state) re-projects MintState through an explicit key allowlist; rejected_endpoint was added to it, and a test drives the real handler and reads the field back out of the serialized JSON. Reverting the handler line turns that test red with KeyError, so the test pins arrival, not storage.
• Copy-ready contract centralized in one owner. security.sanitized_oauth_endpoint_display(url) returns a pasteable host/path or None; both the mint card and the chat OAuth banner call it, so neither re-derives the helper's internal redaction/truncation sentinels.
• Sibling consumer brought into line. The banner at chat_runner.py now uses the same display helper, so it no longer renders host[REDACTED: credential] -- the PR no longer contradicts its own "not pasteable is worse than silence" rule.
• Both unnameable branches controlled. One control for a credential in the HOST (helper returns None) and one for a credential in the PATH (helper redacts, display suppresses); the redacted-path control is mutation-verified.
• Error-code / reason handling on the mint rejection, the new rejected_endpoint wire field and its frontend type, and the docs/system-specs/modules/security.md contract paragraph are all in place; the black baseline was left unchanged (two touched files are baselined and were edited by hand, not reformatted).

Open finding 1 -- three banner/fallback changes ride along undeclared

These are in the diff but were not named in the description. Recording them so the branch's full shape is visible:

1. The chat OAuth banner (chat_runner.py, the fix(security): name the rejected OAuth endpoint in the banner (#7578) #7739 surface) no longer embeds the JSON entry shape in its content; it now points to the guide instead.
2. That same banner now routes through sanitized_oauth_endpoint_display, so for a credential-bearing (redacted) path it falls back to the unnamed rejection instead of surfacing the redacted pair -- a behaviour change to a shipped surface.
3. The unnamed fallback string mint_failure_url_rejected gained a next step (Ask the server owner to fix the URL.) in all catalogs.

Open finding 2 -- the remediation dead-ends, and the two variants speak two vocabularies

• Dead-end reference. Both the card and the banner tell the user to see the Connecting a remote OAuth MCP server guide -- that guide is real (docs/guides/connecting-remote-oauth-mcp-server.md) but the copy names it in prose with no link, and a card/chat error string cannot be clicked through, so the self-service path this change exists to deliver stops at a reference the reader cannot reach.
• Two vocabularies for one failure. The named variant says "The authorization endpoint ... looked like it carried a credential"; the unnamed fallback says "The provider returned an approval address containing credential-like data." Same rejection, two different noun/verb pairs -- they should share one vocabulary.

Housekeeping

The screenshot committed at temp-screenshots/7765-endpoint-name/card-states.png is review evidence and should not survive a merge -- a maintainer resuming this should drop it rather than inherit it silently.