Skip to content

feat(docs-routes): forward protected Swagger UI keys by header instead of embedding them - #823

Merged
yeongseon merged 12 commits into
mainfrom
feat/docs-key-forwarding-rebased
Oct 24, 2026
Merged

yeongseon merged 12 commits into
mainfrom
feat/docs-key-forwarding-rebased

Conversation

@yeongseon

@yeongseon yeongseon commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

First half of #812 (runtime and documentation): add explicit protected-docs key forwarding modes without embedding the page key in HTML in header mode. The security-gate half follows separately. Supersedes #816, which was closed when its stacked base branch was deleted after #815 merged.

Changes

  • register_openapi_routes(..., docs_key_forwarding=None) accepts "header", "query", or "none". Protected default registrations retain legacy query forwarding and emit an intentional, tested DeprecationWarning; anonymous registrations never forward or warn. The default will become "header" in a future minor before 1.0.
  • Header mode: the library handler does not read or echo the query value. The Functions host necessarily receives the initial URL. The browser removes all code parameters with history.replaceState, preserves other parameters/fragments, and attaches x-functions-key only to the exact same-origin specification download, never Try-it-out. Absolute HTTPS specs never receive the page key; protocol-relative URLs are rejected.
  • Key-bearing specification requests set request.redirect = 'error': every redirect fails closed with Swagger UI's normal specification-load error, with no request to the destination.
  • render_swagger_ui(..., forward_page_key_header=False) remains opt-in. Default and legacy-query HTML/headers are byte-identical to origin/main with a fixed nonce. Snapshots are unchanged. No runtime dependencies, gate weakening, or type-ignore suppressions added.
  • Security docs, README, DESIGN and llms-full explicitly describe navigation timing retaining the original ?code= URL in performance.getEntriesByType("navigation")[0].name after address-bar cleanup, for the page lifetime.

Type of Change

  • New feature (non-breaking change that adds functionality)
  • Bug fix (key-bearing spec downloads reject redirects)
  • Documentation update

Checklist

  • My code follows the project's code style
  • Requested local tests and quality checks pass, including the 95% coverage floor
  • I have added tests for new functionality
  • I have updated documentation
  • adds an intentional, tested DeprecationWarning for protected registrations using the default

Review history

  • Oracle NO-GO on 8f69463: redirects carry the key beyond the exact spec URL, proven with real Chromium and the real Swagger UI 5.32.4 CDN bundle with SRI enforced. Reproduced before the fix in this revision:

    same-origin 302:
    127.0.0.1:44853 GET /api/openapi.json key=REDIRECT_CANARY_823
    127.0.0.1:44853 GET /redirected-operation key=REDIRECT_CANARY_823
    cross-origin 302, permissive custom CSP and CORS:
    127.0.0.1:38901 GET /api/openapi.json key=REDIRECT_CANARY_823
    127.0.0.1:43835 OPTIONS /redirected-operation key=null
    127.0.0.1:43835 GET /redirected-operation key=REDIRECT_CANARY_823
    

    Same-origin 307 and 308 also reproduced forwarding before the fix.

  • What changed: the interceptor sets request.redirect = 'error' in exactly the guarded block that attaches the header. Inspection of the actual 5.32.4 bundle confirmed swagger-client's http_http consumes o = await o.requestInterceptor(o) || o and calls (o.userFetch || fetch)(o.url, o). Chromium regression instrumentation observes the actual native-fetch options without altering behavior and confirms redirect: 'error' is honored. No alternative downloader is needed.

  • After the fix: real-bundle same-origin 302/307/308, cross-origin 302 under permissive CSP/CORS, and a configured same-origin-to-cross-origin chain all fail closed before any destination request. The normal real-bundle flow still loads the spec, cleans the address bar, executes /api/ping without the key, and requests favicon without the key.

  • A separate AI reviewer inspected the delta and reported no blocking findings. This is advisory evidence, not human approval or authorization. Review threads were not resolved.

Verification

  • hatch run pytest -q: passed; 95.14% branch-inclusive coverage, unchanged 95% floor. Optional validation-dependent tests and the live-host test skip in this default invocation. Expected deprecation/runtime warnings remain visible.
  • hatch run pytest -m browser --no-cov -q -s tests/test_docs_key_browser.py: 8 passed, including 6 real-CDN/SRI cases, no network skips. CDN cases carry both browser and registered browser_cdn markers and skip on recognized CDN network unavailability; SRI/execution failures are failures.
  • Ruff lint and format check: passed. hatch run mypy --strict src tests: passed (96 files).
  • hatch run mkdocs build --strict: passed. The theme prints its upstream MkDocs 2 migration notice; no strict-build failure.
  • hatch build: sdist and wheel built.
  • Release-workflow, workflow-pin, and Hatch-matrix lint scripts: passed. Bandit: no issues.
  • Fixed nonce: default, legacy query URL, response logging, and custom-CSP rendered HTML and headers equal origin/main byte-for-byte; snapshots untouched.
  • Real Core Tools 4.8.0, Python 3.11.15, Azurite on unique ports 39349/44643/43375, host 50377: nine functions indexed; manual/registered/protected routes returned 200; protected docs did not echo the canary; all 6 tests/host_smoke/test_fixture_routes.py tests passed with HOST_SMOKE_BASE_URL. Storage-health timeout appeared during teardown after Azurite was stopped. This does not prove Azure function-key enforcement.
  • LSP tool limitation: diagnostics rejects paths in the required /tmp/opencode worktree because they are outside the request cwd. Strict mypy and Ruff pass; no clean LSP result is claimed.

Residual risks: the initial navigation URL reaches the Functions host and may be retained in proxy/access logs, earlier history, navigation timing, and other page-lifetime memory. Header forwarding does not erase these copies. The key must authorize both functions; prefer production edge authentication. Azure key enforcement was not reverified locally.

Related Issues

Final CI evidence

Final head: 2c727e77e099751afa140364e48660d2e35f859c.
Exact-head check-runs show all required contexts completed successfully:
bandit, semgrep, Analyze, PR title, and ci-required.
The full test/compatibility/build/docs/performance matrix and both real-host
smoke jobs passed (CI run 40038682936). An earlier duplicate PR title run
40038683035 was cancelled, although the old gh pr checks CLI labels it
fail; the later run 40038683304 completed successfully. No reruns,
force-push, merge, or thread resolution were performed.
main remained 67e7f548dd1c6010ffdc89ff99bbd902ee9b28e0 after completion.

Refs #812

yeongseon and others added 12 commits October 24, 2026 07:06
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Also align JSON/YAML handler parameter names with the SDK req binding, as required by real-host worker indexing.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Refs #812. Real CDN Chromium regressions prove fail-closed redirects without changing default or legacy-query rendering.
@yeongseon

Copy link
Copy Markdown
Owner Author

R3 authorization record (PR #823, head 2c727e7)

What this is: the maintainer (yeongseon) delegated the merge decision to an Oracle (AI) review in the working session ("decide the merge with Oracle and proceed", then "review, merge, and continue"). No line-by-line human review of this diff has taken place. Under the repository policy (#818) Oracle is advisory; this is a delegated decision, not independent human approval. The squash commit can be reverted if the maintainer disagrees after reading it.

Oracle verdicts (real Chromium + the real Swagger UI 5.32.4 CDN bundle, SRI enforced)

Head Verdict Proven reason
8f69463 NO-GO the key header followed HTTP redirects (same-origin 302/307/308, and cross-origin with a permissive CSP/CORS): only the initial spec URL was validated
2c727e7 GO no blocking issue proven

Fix verified: request.redirect = 'error' is set where the guarded interceptor attaches the key; the bundle passes it to fetch (native-fetch instrumentation shows redirect: "error"). 25 redirect combinations (301/302/303/307/308 x same-origin, cross-origin, chain, //host, /\host) fail closed with no destination request, on load and on retry. Alternate spec URLs, ?url=, configUrl and multi-spec configuration never receive the key. Default and legacy-query HTML and headers are byte-identical to main (fixed nonce); snapshots unchanged; hostile title/URL/CSP probes do not execute scripts; CDN Referer is origin-only.

Residual risks (documented in docs/security.md): the initial navigation URL reaches the Functions host and may appear in logs or browser history before cleanup; performance.getEntriesByType("navigation") keeps the original ?code=; the key stays in page memory; the key must authorize both the docs and JSON functions; master keys are discouraged; Try-it-out of protected operations needs separately supplied authorization.

Not verified: Azure function-key enforcement (Core Tools does not enforce keys), the host's own request logging in a deployment, release-gate execution.

Compatibility and release: feat(docs-routes): adds an opt-in parameter; the default keeps legacy behaviour and emits a DeprecationWarning for protected registrations. Release PR #794 will move to 0.31.0; it is not being merged here. Refs #812: the issue stays open until the security-gates PR #817 merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant