fix(serve): reject unsafe CORS origins (wildcard + credentials) (GHSA-5mh2, code-only)

[padak]

Summary

Fixes M6 from the 2026-06-12 security audit (private advisory GHSA-5mh2-6xgr-rf89) — a permissive-CORS misconfiguration in kbagent serve.

create_app registers CORSMiddleware with allow_credentials=True. Combined with a wildcard --cors-origin '*' (or a malformed origin), Starlette reflects the request Origin and returns Access-Control-Allow-Credentials: true — letting any website read authenticated cross-origin responses from a victim's running serve.

Fix

create_app validates origins via _resolve_cors_origins: rejects * and any value that isn't a concrete scheme://host[:port] (raising ConfigError); kbagent serve surfaces it as a clean typer.BadParameter on --cors-origin (exit 2). The default localhost dev set is unchanged — only the actively-dangerous wildcard/malformed config is refused.

⚠️ Code-only PR (no version bump / changelog entry)

This PR deliberately touches only the 3 code files (server/app.py, commands/serve.py, tests/test_serve_ui.py) and does not bump the version or add a changelog entry. Rationale: main releases ~daily, and a security PR carrying a version bump + changelog entry conflicts on pyproject.toml/changelog.py against every release — this PR was rebased 4× over 2 days chasing the moving version. Dropping the release-bookkeeping makes it conflict-immune so it can be merged on the owner's schedule.

At merge/next release, add: a version bump and a changelog.py entry for the CORS guard (text available in the advisory / prior PR revisions).

Tests

TestCorsCredentialsGuard in test_serve_ui.py (9 tests): parametrized rejection of *, mixed-with-*, scheme-less, path-carrying, and ws:// origins; acceptance of defaults + explicit http(s)://host[:port]; unit test of the _is_valid_cors_origin predicate. Full suite green: 4146 passed, 133 skipped; lint/format/ty/version-check clean.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[padak]

Review of #422 — fix(serve): reject unsafe CORS origins (wildcard + credentials) (GHSA-5mh2, code-only)

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make check, not duplicated here.
Per the PR description, the absent version bump / changelog entry / version-sync
are intentional and are NOT flagged in this review.

Summary

This PR closes GHSA-5mh2-6xgr-rf89: when kbagent serve was started with
--cors-origin '*' (or any malformed origin), Starlette reflected the
request Origin header and returned Access-Control-Allow-Credentials: true,
allowing any website to read authenticated cross-origin responses. The fix
introduces _resolve_cors_origins and _is_valid_cors_origin in server/app.py
that validate origins at startup, raising ConfigError for wildcards or
non-scheme://host[:port] values; commands/serve.py converts the ConfigError
to a clean typer.BadParameter (exit 2). The core fix is correct: Starlette's
allow_all_origins + allow_credentials reflection path is fully blocked. One
non-blocking gap exists in the test coverage (the CLI-layer error path is not
exercised via CliRunner), one non-blocking documentation gap exists (no
gotchas.md entry for the new restriction), and one nit on the validator.

Verdict: COMMENT (no blocking findings; two non-blocking items worth addressing).

Verdict

• Verdict: COMMENT
• Blocking findings: 0
• Non-blocking findings: 3
• Nits: 1

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/commands/serve.py:223 — except ConfigError catch is scoped to --cors-origin hint but will misattribute future ConfigError raises from create_app

The handler catches all ConfigError from create_app() and re-raises as
typer.BadParameter(..., param_hint="--cors-origin"). Today this is correct
because _resolve_cors_origins (line 477 of app.py) is the only ConfigError
raise inside create_app. However, the catch is against the entire create_app
call, so any future ConfigError from a different part of the function
(e.g., config-store init failure, registry boot failure) would surface as a
misleading --cors-origin usage error rather than a general startup failure.

A minimal mitigation is to push the validation out of create_app and into
serve_command before the create_app() call, making the exception boundary
explicit. Alternatively, a narrow sentinel exception (e.g.,
class CorsConfigError(ConfigError): pass) would let the except clause
stay in serve_command without over-catching. The current code ships safely
since no other ConfigError is raised in create_app today; this is a
forward-maintenance concern, not an immediate bug.

[NB-2] tests/test_serve_ui.py — CLI-layer error path (ConfigError → typer.BadParameter, exit 2) is not exercised via CliRunner

TestCorsCredentialsGuard tests the guard by calling create_app() directly.
This verifies that the security check fires, but it does not verify that
serve_command converts the ConfigError to typer.BadParameter with exit
code 2 (rather than letting a traceback escape). The serve_command ultimately
calls uvicorn.run(), but typer.BadParameter is raised and the process exits
before uvicorn.run() is reached — so a CliRunner-based test can exercise
this path without spawning uvicorn by mocking uvicorn.run or by relying on
the early-exit: e.g.,
result = runner.invoke(app, ["serve", "--cors-origin", "*"]); assert result.exit_code == 2.
Adding this test closes the gap between "the guard fires" and "the CLI surface
exits correctly".

[NB-3] plugins/kbagent/skills/kbagent/references/gotchas.md — missing (since vX.Y.Z) entry for the --cors-origin '*' startup rejection

The CORS guard changes user-visible behavior: kbagent serve --cors-origin '*'
previously started successfully; after this PR it exits with code 2.
An AI agent recommending --cors-origin '*' based on generic CORS advice or
prior session experience would cause a startup failure on versions that include
this fix. Per CONTRIBUTING.md gotchas.md is explicitly called out as the
place for this kind of "(since vX.Y.Z) behavior change" note. The entry should
be tagged with the version that ships this PR and note that '*' is rejected
at startup with a BadParameter error pointing at --cors-origin. Since the
PR intentionally defers the version bump, the token (since vNEXT) or
(since 0.65.0) can be filled in at release time — but the entry should be
drafted in this PR so it is not forgotten.

Nits

• [NIT-1] src/keboola_agent_cli/server/app.py:460 — _is_valid_cors_origin accepts http://user:pass@evil.com because @ is not in the rejection set /?#. A browser never sends credentials in an Origin header (RFC 6454 strips auth info), so Starlette's exact-match would never match a legit user:pass@ config entry against an attacker's Origin; the security impact is negligible. Still, explicitly rejecting @ in the host segment ("@" in rest) would make the validator match the RFC definition more precisely and future-proof it against non-browser callers that might send a literal user:pass@host Origin.

Verification log

• gh pr view 422 --json title,body,files,additions,deletions,state → state OPEN, 3 files, +105/-14, conventional fix(serve): prefix ✓
• git rev-parse --abbrev-ref HEAD → fix/serve-cors-credentials-guard matches <branch> ✓
• make check → ruff: all checks passed; ruff format: 318 files already formatted; ty check: clean; check_error_codes.py: OK; 4146 passed, 8 skipped → exit 0 ✓
• Layer violation grep (typer in services, httpx in commands, formatter in clients) → empty ✓
• grep -E '^\+.*@.*_app\.command' diff → no new CLI command decorators ✓ (no plugin sync map obligations triggered)
• grep -E '^\+' diff | grep -E 'except\s*:' → no bare except ✓
• grep -E '^\+.*error_code\s*=\s*"[A-Z_]+"' diff → no raw error_code strings ✓
• grep -E '^\+(from httpx|import httpx|httpx\.Client)' diff → no new HTTP calls outside clients ✓
• Starlette CORSMiddleware.send() source inspection → confirmed: allow_all_origins + allow_credentials causes Starlette to call allow_explicit_origin(headers, origin) which sets Access-Control-Allow-Origin to whatever Origin: header was sent, enabling the CORS credentials attack. The fix is correctly targeted ✓
• _is_valid_cors_origin manual edge-case run: http://user:pass@evil.com → True (NIT-1); http://[::1]:8080 → True (IPv6 localhost, safe); HTTP://localhost → False (uppercase scheme rejected); //localhost → False ✓
• grep -rn 'CliRunner\|runner.invoke' tests/test_serve_ui.py → no CliRunner tests; CLI-layer BadParameter path untested (NB-2)
• grep -n 'cors\|CORS' plugins/kbagent/skills/kbagent/references/gotchas.md → no entry (NB-3)
• grep -rn 'raise ConfigError' src/keboola_agent_cli/server/app.py → exactly one raise, in _resolve_cors_origins (line 477). The except ConfigError in serve_command is correctly scoped for current code (NB-1 is a forward-maintenance concern, not a present bug) ✓

Open questions for the author

(none)

[padak]

Author response — review findings addressed (commit fe4f1f3, still code-only)

Thanks for the review. All three actionable code findings addressed; the one doc finding is deferred to release. PR stays code-only (3 files), so it remains conflict-immune.

• NB — broad except ConfigError mis-attributing errors to --cors-origin: Fixed. serve_command now validates the origins up front via _resolve_cors_origins(cors) and catches only that call as a --cors-origin BadParameter; the broad try/except around create_app is removed, so unrelated create_app ConfigErrors propagate normally. create_app keeps its own validation as defense-in-depth.
• NB — missing CliRunner test for the CLI layer: Added test_serve_cli_rejects_wildcard_cors_origin — invokes kbagent serve --cors-origin '*' through the full Typer app and asserts exit code 2 (the validation fires before create_app/uvicorn.run, so no server is started).
• Nit — _is_valid_cors_origin accepted http://user:pass@evil.com: Tightened — @ (userinfo) is now rejected alongside /?#; predicate test extended.
• NB — missing gotchas.md entry (deferred): This PR carries no version (code-only), so a (since vX.Y.Z) gotcha tag can't be assigned yet. The gotcha entry will be added together with the version bump + changelog entry at the next release (already noted in the PR description as the merge/release follow-up).

make check green: 4147 passed, lint/format/ty/version-check clean.