fix: persist OAuth credentials on catalog server registration

[marekdano]

Summary

Backend-only slice of #5967 (catalog OAuth add flow) — the two register-path bugs the issue calls out as "safe to start," plus the persistence they exist to enable. No frontend changes here.

• CatalogServerRegisterRequest.oauth_credentials was defined but never read. CatalogServerRegisterBody (the v1 endpoint's body) didn't expose it at all. Both now flow through to a real oauth_config on the created gateway, in the same call that registers it.
• "OAuth2.1 & API Key" catalog entries registered with no api_key fell through the skip-initialization branch and attempted a live connection test that was always going to fail — there were no credentials to test with. They now take the same no-connection-test path as plain OAuth entries.

Changes

• schemas.py: CatalogServerRegisterBody gains oauth_credentials: Optional[Dict[str, Any]], mirroring the existing (unused) field on CatalogServerRegisterRequest.
• routers/catalog.py: threads body.oauth_credentials into the CatalogServerRegisterRequest built for the service call.
• catalog_service.py (register_catalog_server):
  • skip-initialization now also covers "OAuth2.1 & API Key" with no api_key (previously only "OAuth2.1" / "OAuth").
  • the skip-initialization branch builds a real oauth_config from oauth_credentials (grant_type=authorization_code, plus issuer/client_id/client_secret/token_url/authorization_url/scopes), runs it through the same discovery (GatewayService._auto_discover_oauth_endpoints) and encryption (protect_oauth_config_for_storage) pipeline normal gateway registration already uses, and persists it on the DbGateway row. Previously this branch built a GatewayCreate only to discard it and never wrote oauth_config at all.
  • requires_oauth_config no longer clears just because oauth_config is set — it now means "disabled OAuth gateway," full stop. Without this, a genuinely unauthorized gateway look fully configured to the caller.
• New/updated unit tests covering all three behavior changes.

Test plan

• pytest tests/unit/mcpgateway/services/test_catalog_service.py tests/unit/mcpgateway/routers/test_catalog.py
• pytest tests/unit/mcpgateway/services/test_gateway_service.py tests/unit/mcpgateway/routers/test_oauth_router.py (regression check on shared discovery/encryption call sites)
• ruff check
• Manually verified end-to-end against a real local gateway (see below)

How to test it

1. Start the gateway with make dev and get an admin bearer token (--admin avoids the "public-only tokens can only create public catalog registrations" scope check, which is unrelated to this PR):

   export JWT_SECRET_KEY=$(grep '^JWT_SECRET_KEY=' .env | cut -d= -f2)
   export MCPGATEWAY_BEARER_TOKEN=$(python3 -m mcpgateway.utils.create_jwt_token \
       --username admin@example.com --exp 60 --secret "$JWT_SECRET_KEY" --admin)
   

2. oauth_credentials persists as oauth_config — register the catalog's github entry (auth_type: OAuth2.1) with only an issuer and scopes:

   curl -s -X POST http://127.0.0.1:8000/v1/catalog/github/register \
     -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
     -H "Content-Type: application/json" \
     -d '{"oauth_credentials": {"issuer": "https://github.com", "scopes": ["repo", "read:user"]}}' | jq
   

   Expect success: true, oauth_required: true. Then confirm the gateway was created disabled with the issuer/scopes persisted:

   curl -s http://127.0.0.1:8000/v1/gateways/<server_id from above> \
     -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq '.enabled, .authType, .oauthConfig'
   

   → enabled: false, authType: "oauth", oauthConfig.issuer/.scopes populated, no client_secret since none was submitted.

3. "OAuth2.1 & API Key" register-path bug — register the catalog's stripe entry (auth_type: "OAuth2.1 & API Key") with no api_key:

   curl -s -X POST http://127.0.0.1:8000/v1/catalog/stripe/register \
     -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
     -H "Content-Type: application/json" -d '{}' | jq
   

   Before this fix: falls through to a live connection test with no credentials and fails. After: success: true, oauth_required: true, same shape as step 2.

4. requires_oauth_config stays accurate — confirm the github entry registered above is still flagged as needing authorization, not silently marked configured just because oauth_config is now set:

   curl -s "http://127.0.0.1:8000/v1/catalog?search=github" \
     -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" | jq '.servers[0].requires_oauth_config'
   

   → true.

Not in this PR

• The catalog UI's OAuth add modal — on hold pending Seed OAuth discovery metadata into mcp-catalog.yml #6461.
• Card-state distinction for "needs authorization" vs. "token expired" - depends on [API]: Return per-user token status from /oauth/status/{gateway_id} #6459 (per-user token status).

[msureshkumar88]

Thanks for scoping this down to exactly the two register-path bugs the issue calls "safe to start" — nice that it reuses GatewayCreate.validate_oauth_config and the existing discovery/encryption pipeline instead of adding a parallel path, and the new unit tests assert real object state (db.add.call_args) rather than just call order, which is the right pattern.

One correctness issue I think needs to be fixed before merge, plus a design question worth a quick maintainer call, and a few optional cleanups.

Blocking

Mixed "OAuth2.1 & API Key" entries silently drop oauth_credentials when api_key is also supplied.

catalog_service.py:520-538:

if request and request.api_key and auth_type != "Open":
    ...
    elif auth_type in ["OAuth2.1", "OAuth", "OAuth2.1 & API Key"]:
        gateway_data["auth_type"] = "bearer"
        gateway_data["auth_token"] = request.api_key
    ...
elif auth_type in ["OAuth2.1", "OAuth", "OAuth2.1 & API Key"]:
    skip_initialization = True

If a caller registers a mixed-auth entry with both api_key and oauth_credentials in the request, request.api_key is truthy so the first branch wins, skip_initialization stays False, and the whole oauth_credentials → oauth_config block (541+) never runs. The response still comes back success: true, so the caller has no signal that half of what they submitted was discarded.

Could you either merge both (bearer auth_token + persisted oauth_config) or reject the request when both are supplied with a clear error? Either way this probably needs a regression test alongside the two you already added.

Worth a quick sign-off

requires_oauth_config semantics changed for a case beyond what's demoed in the PR.

catalog_service.py:301: not selected_gateway.enabled and selected_gateway.auth_type == "oauth" — dropping the oauth_config check means any disabled OAuth gateway now reads as "needs setup," including one that was fully authorized (real client_id/client_secret/tokens) and later disabled deliberately (maintenance, credential rotation, etc). That shows up as the yellow "OAuth Config Required" state in the legacy admin partial (mcp_registry_partial.html:515), so it's a small user-visible behavior change even though this PR is otherwise backend-only.

The demoed scenario (freshly registered, never authorized) is clearly right to flag. I just want to confirm the "previously-authorized-then-disabled" case reading the same way is intentional and not incidental — happy to leave as-is if so.

Optional cleanups (non-blocking)

• catalog_service.py:261 / :277 — DbGateway.oauth_config is fetched and stored on _CatalogGatewayMatch for every row in this listing query, but nothing reads it now that line 301 dropped the check. Worth dropping from the select() to avoid pulling encrypted config through an unused path on every catalog list call.
• catalog_service.py:545-578 — this hand-rolls the same _auto_discover_oauth_endpoints → protect_oauth_config_for_storage sequence register_gateway() already runs (gateway_service.py:1544-1548), reaching into a protected method to do it. Not a bug today (the two token-exchange-specific steps it skips are no-ops for the hardcoded authorization_code grant type here), but if catalog registration ever supports another grant type this copy won't get those checks for free. A small shared helper would remove the drift risk.
• catalog_service.py:553 — (request.oauth_credentials if request and request.oauth_credentials else {}) or {}: the trailing or {} is unreachable, the ternary already returns {} on every falsy path.
• A CHANGELOG.md entry under [Unreleased] referencing #5967 would match how the other recent catalog-registration fixes were tracked (e.g. #6036).

No concerns on scope, migrations (none needed — oauth_config already exists on the model), or unrelated changes — diff is clean and tightly scoped to the two bugs described.

[msureshkumar88]

Nice fix — traced c7a0da01d through catalog_service.py → register_gateway() → _prepare_gateway_registration() → the new prepare_oauth_config_for_storage(), and it genuinely closes the mixed "OAuth2.1 & API Key" bug from the earlier review (an api_key-only entry was silently dropping submitted oauth_credentials). Good use of a shared helper to keep the enforce/discover/validate/encrypt sequence in one place too.

A couple of things worth addressing before merge:

Functionally impacting

1. tests/unit/mcpgateway/services/test_catalog_service.py — test_get_catalog_servers_requires_oauth_config_configured (the case covering a disabled OAuth gateway that already has oauth_config set) was removed along with the oauth_config column in the listing query, and nothing replaced it. Whichever way requires_oauth_config is meant to behave now, that boundary case has no coverage in either direction — worth adding a test that pins down the new intended behavior explicitly.

2. _build_oauth_config_from_credentials (catalog_service.py) only carries over issuer/client_id/client_secret/token_url/authorization_url/scopes. The existing _assemble_oauth_config_from_fields in admin.py (which this duplicates) also handles resource, audience, redirect_uri, username, and password. As it stands, a catalog-registered OAuth gateway can never get an RFC 8707 resource/audience set at registration time — might be worth pulling that logic through a shared helper, or at least widening the allowlist to match, so the two paths don't drift.

Suggestions

3. requires_oauth_config now flags any disabled OAuth gateway, not just unconfigured ones. That's defensible given catalog registration persists oauth_config up front now — the old check would false-negative on this PR's main new flow — but it does mean a gateway that was fully authorized and then manually disabled (e.g. for credential rotation) shows the same "OAuth Config Required" badge as one that was never touched. If that distinction matters to users, keying off whether an OAuthToken row exists for the gateway (rather than oauth_config presence) would separate "never authorized" from "authorized then disabled" more precisely. Not blocking — just flagging for a follow-up if it's worth the extra query.

4. The prepare_oauth_config_for_storage docstring says "every write path that persists a gateway's oauth_config... must apply" this helper, but update_gateway and oauth_router.py's completion handler still call the four steps inline (the former passes existing_oauth_config, which the new helper doesn't accept, so it can't drop in as-is). Might be simplest to soften the docstring claim rather than force those call sites through it right now.

Minor

5. docs/docs/manage/catalog.md's registration example only shows name/api_key — worth adding oauth_credentials to the example body given it's the main capability this PR adds.
6. No test exercises prepare_oauth_config_for_storage() directly (only indirectly via register_gateway/register_catalog_server) — a small unit test asserting call order (admin-gate → discover → validate → encrypt) would guard against a future reordering regression.

None of this blocks the core fix — 1 and 2 are the ones I'd want resolved before merge; 3–6 can be follow-ups if preferred.

[vishu-bh]

Thanks for raising this, found a security exposure. Please check

[vishu-bh]

oauth_credentials is untyped. A payload with client_secret plus invalid issuer fails before encryption; Pydantic exception includes whole dict. This handler logs and returns raw exception. Legacy admin route returns it as JSON/HTMX title, leaking secret. Use typed OAuth input schema and generic/redacted errors; add regression test.

[vishu-bh]

LGTM 🚀

[msureshkumar88]

Thanks for the two rounds of fixes so far — the mixed OAuth2.1 & API Key persistence fix and the validation-error redaction are both solid and well-tested (verified via diff + local test run, not just the commit message). Two small things worth fixing before merge.

Blocking

1. mcpgateway/services/catalog_service.py:441 — scopes isn't comma-split like admin.py:1030 does (oauth_scopes_str.replace(",", " ").split()). Verified empirically: a caller sending "scopes": "repo,read:user" stores ["repo,read:user"] and every consumer (oauth_manager.py, dcr_service.py) joins with " " before sending it to the IdP, so that becomes one malformed scope on the wire — repo,read:user instead of repo read:user. Space-separated input already round-trips fine (all consumers join lists with a space), so this only bites comma-separated input, but that's exactly the shape admin.py's own form accepts, so it's a real inconsistency between the two paths for the same feature.

2. mcpgateway/schemas.py:8454 — CatalogServerRegisterBody.oauth_credentials has no max_length, unlike the sibling api_key field on the same model (max_length=4096). Verified: a 5MB client_secret is accepted end-to-end and persisted. No global body-size middleware catches this (log_detailed_max_body_size only truncates logging, doesn't reject). Bounded to authenticated callers with catalog-registration permission, so not a public DoS vector, but worth the same cap api_key already has.

Suggestions (non-blocking)

3. _build_oauth_config_from_credentials carries username/password into the stored config, but grant_type is hardcoded to authorization_code and those fields are only read by the password-grant flow — they're currently dead/unreachable data. Either drop them from the copy list, or if there's a reason to keep them for a future grant-type expansion, a one-line comment would help; the test asserting they're "carried" over-promises functionality that doesn't exist yet.
4. ["OAuth2.1", "OAuth", "OAuth2.1 & API Key"] is duplicated verbatim at lines 562 and 579 — this PR itself had to touch both when adding the mixed-auth entry. A module-level constant would prevent the next edit from missing one.
5. _enforce_token_exchange_admin_only's docstring (gateway_service.py:886) still says catalog registration passes an empty requester_email so the gate is skipped; catalog_service.py:608 now passes the real owner_email. Harmless today since grant_type is hardcoded away from token-exchange, but worth updating so it doesn't mislead a future change.

Minor

6. docs/docs/manage/catalog.md's registration example still only shows name/api_key — worth adding oauth_credentials since it's this PR's main new capability.

No concerns on scope, migrations, or unrelated changes — diff stays tightly scoped to the two register-path bugs the issue calls out.

[marekdano]

@msureshkumar88 - comments addressed

[msureshkumar88]

Thanks for the three rounds of fixes — the scopes comma-split, the 4096 cap, the username/password drop and the docs example all landed cleanly, and I verified them by reading the diff and running the suites rather than trusting the commit messages (test_catalog_service.py + test_catalog.py: 115 passed; test_gateway_service.py + test_oauth_router.py + test_admin_catalog_htmx.py: green). The scoping down to the backend slice is right, reusing GatewayCreate.validate_oauth_config and the register_gateway discovery/encryption pipeline instead of hand-rolling either is the right instinct, and the redaction fix in bc974424c was a good catch. No migration (correct — oauth_config already exists on the model), no unrelated changes, rebased clean.

A few things surfaced this pass that I think are worth resolving before merge.

Blocking

1. catalog_service.py:441-445 — non-string oauth_credentials values skip encryption and persist in plaintext.

The copy loop takes client_id/client_secret/audience on a bare if value: with no type check. _encrypt_oauth_secret_value (encryption_service.py) short-circuits on if not isinstance(value, str): return value, and _validate_oauth_config_urls only inspects the six URL-bearing keys, so GatewayCreate doesn't reject it either. Verified end to end against this branch:

GatewayCreate ACCEPTED non-str client_secret: {'inner': 'PLAINTEXT-SECRET'}
stored client_secret: {'inner': 'PLAINTEXT-SECRET'}
stored int secret:    12345

So -d '{"oauth_credentials": {"client_secret": {"x": "realsecret"}}}' writes the secret unencrypted into gateways.oauth_config. The weakness in _encrypt_oauth_secret_value predates this PR, but this is the change that first routes caller-controlled values into it on this path, so the guard belongs in the new loop:

for key in ("issuer", "client_id", "client_secret", "token_url", "authorization_url", "redirect_uri", "audience"):
    value = oauth_credentials.get(key)
    if value is None or value == "":
        continue
    if not isinstance(value, str):
        raise ValueError(f"oauth_credentials.{key} must be a string")
    raw_oauth_config[key] = value

The inconsistency is arguably the tell here — resource two lines down already gets str()-coerced. Worth a regression test asserting a dict client_secret is rejected.

2. schemas.py:8517 — the 4096 cap is bypassable, and absent on the sibling admin endpoint.

Two halves of the same thing:

(a) validate_oauth_credentials_field only walks top-level str values, so a nested container walks straight past it. Verified: {"client_secret": ["A" * 4096] * 5000} is accepted — 20 MB through the field the cap was added to bound. validate_meta_data in common/validators.py already implements exactly the walk you'd want here (key count, depth, serialized bytes), so this could reuse rather than grow a second bespoke check.

(b) CatalogServerRegisterRequest is also bound as an HTTP request body, on POST /admin/mcp-registry/{server_id}/register (admin.py:18120), and it carries no bound on oauth_credentials at all. This PR is what makes that field read for the first time, so the admin HTMX endpoint is now equally live — and it's the uncapped one. Either move the validator onto CatalogServerRegisterRequest and let the Body inherit, or apply it to both.

3. catalog_service.py:578-586 — the mixed api_key + oauth_credentials path leaves a half-wired gateway.

This one I'd like your read on, because I think the cleanest answer is to drop it rather than patch it.

It persists oauth_config while leaving auth_type = "bearer". You're right that /oauth/authorize gates on oauth_config presence rather than auth_type, so authorization succeeds. But tracing what happens after, on main:

• tool_service.py:4576, :5721, :6113 — OAuth token injection is gated on gateway_auth_type == "oauth"
• resource_service.py:1944, gateway_service.py:4876, :5303, :8155, :8391 — same gate
• vault_router.py:69 — select(Gateway).where(Gateway.auth_type == "oauth"), so the token doesn't even surface in the vault view
• catalog_service.py:302 — requires_oauth_config also requires auth_type == "oauth", so the card just reads "Already Registered"

The user completes consent, a token is stored, and the gateway keeps sending the API key. Silent no-op with a success indication.

Except when it isn't: with no client_id supplied, /oauth/authorize takes the DCR branch and oauth_router.py:728 sets gateway.auth_type = "oauth" — which flips the gateway to OAuth and silently orphans the stored API key. So it's two divergent outcomes depending on whether DCR ran, and neither is documented or covered by a test.

Given #5967 doesn't ask for this case (scope items 1 and 3 are the register-path bugs and the skip-init persistence), and the stated motive is forward-looking ("so the caller isn't required to resubmit issuer/client details later"), my suggestion is to drop it here and file it separately — it probably wants #6459's per-user token status before the card can express the resulting state honestly. Keeping it would mean either runtime support that doesn't exist today, or documenting the limitation loudly plus a test pinning current behaviour.

Functionally impacting

4. catalog_service.py:566-576 — oauth_credentials silently dropped for non-OAuth entries. For a catalog entry whose auth_type is "API Key"/"API" or "Open", submitted oauth_credentials are discarded with 200 success: true and no log line. Same class as the defect this PR exists to fix. A logger.warning naming the entry's auth_type is probably enough; a 400 would be more honest but changes behaviour for the admin endpoint.

5. catalog_service.py:576 — the log line now asserts the opposite of what happened.

logger.info("Registering OAuth server %s without credentials - OAuth flow required later", ...)

After this change the branch is taken for any OAuth entry with no api_key, including the headline case where oauth_credentials were supplied. Worth making conditional on request.oauth_credentials, plus a companion line when oauth_config is persisted (issuer + scopes only, never client_secret) — a successful persist is currently completely silent, which makes the new behaviour hard to observe in production.

6. catalog_service.py:619 — register-time discovery can block for up to 60s while holding the request DB session.

prepare_oauth_config_for_storage → _auto_discover_oauth_endpoints → discover_as_metadata issues two sequential probes (RFC 8414 then OIDC, dcr_service.py:108/:132), each at settings.oauth_request_timeout (default 30s). This branch previously did zero network I/O; the request-scoped Session is now open across both awaits. It fails soft, so this is latency and pool-hold rather than a failure mode — but a caller with servers.create can pin a worker plus a connection for a minute per call.

Worth noting the benefit is narrower than it looks: /oauth/authorize already discovers, but only inside its DCR branch (oauth_router.py:706), so register-time discovery only helps the "client_id supplied, issuer-only" case. Two options that both seem reasonable — bound this call site with a short explicit budget (discovery is an optimization here, not a correctness requirement), or move it out of the register path and widen the authorize-path discovery to run outside the DCR branch.

Suggestions (non-blocking)

7. prepare_oauth_config_for_storage still leaves the third write path behind. The docstring's own argument — a hand-rolled subset "silently loses those checks the moment it needs to support a grant type beyond the one it was written for" — applies to update_gateway (gateway_service.py:3024-3027), which still runs the identical four steps inline. The only delta is existing_oauth_config, so adding existing_oauth_config: Optional[dict] = None to the helper would make it a drop-in for all three sites. (oauth_router.py:727 is a deliberately partial DCR persist and should stay out.) Failing that, narrowing the docstring so it doesn't claim coverage it doesn't have would also close the gap.

8. resource and audience skip URL validation. _validate_oauth_config_urls covers token_url, authorization_url, issuer, authorization_server, redirect_uri, jwks_uri — not these two, which this PR newly exposes on the catalog path and which travel outbound to the IdP as RFC 8707 parameters. resource list elements aren't string-coerced or validated at all. Small blast radius, one-line change in an existing helper.

9. store_tokens / auto_refresh are inert. No backend code reads them from oauth_config — only admin.html:5718/:5731 writes them, and #5967 explicitly lists token management as having no backend consumer and being removed under #6468. Dropping both would keep catalog-built configs from carrying fields already scheduled for deletion.

10. requires_oauth_config no longer matches its name — it now means "disabled OAuth gateway", conflating never-configured / awaiting-authorization / authorized-then-disabled. The change is right for this flow; a rename or a distinct oauth_authorized field would stop the name from lying, and #5967's criteria want those states separated once #6459 lands anyway. Fine to defer.

11. CHANGELOG describes the two fixes but not the requires_oauth_config semantic change, which is the only thing an existing operator will notice. Sibling [Unreleased] entries link the PR (#6443, #6570); this one links only the issue.

12. OAUTH_AUTH_TYPES is a list used only for in membership — frozenset is the idiomatic choice. Trivial.

Testing

The new tests are well-targeted — test_register_oauth_skip_init_persists_oauth_credentials asserting stored["client_secret"] != "super-secret" rather than mere presence is exactly right, and pairing the redaction test with db.add/db.commit not-called assertions is a nice touch. Gaps I'd flag:

• No direct test for prepare_oauth_config_for_storage — new public method, test_gateway_service.py untouched. One test pinning call order (admin-gate → discover → validate → encrypt) would guard the reordering regression the extraction exists to prevent, plus the new None path.
• No test that a caller-supplied grant_type is ignored. The key allowlist in _build_oauth_config_from_credentials is what keeps token-exchange — a privileged, SSRF-boundary grant per AGENTS.md — unreachable from an unprivileged catalog registration. It works, but nothing pins it, and AGENTS.md asks for deny-path regression tests on security-sensitive changes.
• Nothing covers findings 1 and 2, both verified reachable.
• Every catalog test stubs _auto_discover_oauth_endpoints to a passthrough, so the stated benefit of calling it here is unverified.
• bulk_register_servers (catalog_service.py:866) with a mixed entry — those now succeed-as-disabled where they previously failed. Behaviour change, uncovered.
• No test of POST /admin/mcp-registry/{id}/register with oauth_credentials (finding 2b).
• No tests/live_gateway/ black-box test. AGENTS.md asks for one on any PR exercisable through a live gateway — the manual curl sequence in the description is most of the way there already, it just needs codifying.

Worth running make coverage diff-cover before calling it ready.

On the breaking-change question

Nothing breaks in the semver sense — the new field is additive, and the "OAuth2.1 & API Key" behaviour change is the bug fix the issue asks for. There is one behavioural change worth calling out, though: requires_oauth_config dropped its and not oauth_config clause, so an OAuth gateway that was fully configured and authorized and then manually disabled (credential rotation, maintenance) now renders the yellow, disabled "OAuth Config Required" card at mcp_registry_partial.html:515-524 instead of "Already Registered". Both branches are disabled so no action is lost — the label is just wrong in a new direction.

Blast radius is narrow: needs a Gateway row whose url matches a catalog entry, auth_type == "oauth", enabled == false, and non-empty oauth_config. Display-only, no data loss, no schema or API shape change. Two cheap ways to shrink it further: mention it in the CHANGELOG entry, and scope the new predicate to selected_gateway.created_via == "catalog" — created_via is already selected in the same query, so it costs nothing and confines the new label to exactly the rows this PR creates. The precise fix (keying off whether an OAuthToken row exists, separating "never authorized" from "authorized then disabled") is #6459's territory, not this PR's.

Documentation

The curl example is a good addition. Still undocumented: that grant_type is forced to authorization_code and callers can't choose it (that's a security control, worth stating rather than only enforcing); the accepted key allowlist and the fact that unknown keys are silently dropped; the per-value cap; and that POST /admin/mcp-registry/{id}/register accepts the same field.

---

Findings 1-3 are what I'd want addressed before merge, 4-6 as a second pass, and the rest are happy follow-ups. Happy to talk through the mixed-auth one (#3) in particular if you'd rather keep it than drop it — I may be missing a use case you have in mind.

[msureshkumar88]

Thanks for the follow-up work. I independently traced the final head rather than relying on the addressed-comments summary: the credential persistence, mixed-auth no-key path, validation-error redaction, input bounds, and bounded discovery are in place.

Blocking

1. The PR is currently conflicting with main (GitHub reports DIRTY / CONFLICTING), so it needs a rebase and conflict resolution before it can be merged.

2. Could we add a live-gateway black-box regression for this externally observable API flow? The repository review requirements call for one, and there is no relevant coverage in tests/live_gateway. It should exercise OAuth credential persistence, the OAuth2.1 & API Key case without an API key, and the resulting catalog OAuth-required state.

3. catalog_service.py:472 accepts oauth_credentials.resource as an arbitrary list and persists it unchanged. Downstream OAuth audience handling expects a string or a list of non-empty strings. Please validate this field to that contract (or remove resource from this backend slice); malformed values such as [42] should be rejected rather than stored.

Suggestions

• catalog_service.py:308 now marks every disabled OAuth gateway as requires_oauth_config, including a manually created disabled gateway that happens to share a catalog URL. If that is intentional, an explicit regression test would document it; otherwise, scoping it to created_via == "catalog" would keep this catalog-state change narrow.

• The dedicated ValidationError branch correctly prevents the known client-secret disclosure. As defense in depth, consider redacting the generic error_str returned at catalog_service.py:839 too, since the admin HTMX route can render it.

[msureshkumar88]

Thanks for working through the review feedback. Rechecked the final head: the catalog OAuth persistence and mixed-auth no-key flow are covered, validation and error redaction are in place, the requested live-gateway regression has been added, malformed resource values are rejected, and the branch is mergeable after the conflict resolution. Looks good from my side.