feat: add Scalar API reference UI with reverse-proxy path support #13
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/scalar-ui"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Scalar API Reference UI
Adds Scalar (https://scalar.com) as the interactive API documentation UI.
Endpoints
/scalar(or/{prefix}/scalar)/openapi/v1.json(or/{prefix}/openapi/v1.json)Both are anonymous (no auth needed). Available in all environments (not just Development).
Reverse-proxy sub-path support
DOUJIN_MANAGER_PATH_PREFIXenv var handles nginx sub-path routing:The nginx config needs to pass through the full path (not strip the prefix). The app reads the prefix and generates correct spec URLs in the Scalar HTML.
Files changed
Directory.Packages.props— addedScalar.AspNetCore2.5.0DoujinManager.Server.csproj— package referenceProgram.cs— serve OpenAPI + Scalar in all environmentsScalarUi.cs— HTML generator with path prefix supportdocker-compose.yml— commented-outDOUJIN_MANAGER_PATH_PREFIXenv var234 tests pass.
- Add Scalar.AspNetCore package - Serve Scalar UI at /scalar (or /{prefix}/scalar) in all environments - DOUJIN_MANAGER_PATH_PREFIX env var for reverse-proxy sub-paths (e.g. nginx routing /doujinshi/ → container) - OpenAPI JSON also available at /openapi/v1.json - Both endpoints are anonymous (no auth needed for docs) - Added env var to docker-compose.yml (commented out by default)Summary
Summary
Coverage
DoujinManager.ApplicationCore - 84.8%
DoujinManager.Infrastructure - 91.7%
pshot
DoujinManager.RestAdapter - 83.7%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 17%
missing docu for the new env variable in deployment
🤖 Hermes automated review: changes requested
Reviewed head
ba1424c3against base1ad68897(5 files, +62/-4). Focused on the reverse-proxy path-prefix routing since that's the PR's headline feature.🔴 Major — OpenAPI spec route is not path-prefix-aware, so Scalar breaks when
DOUJIN_MANAGER_PATH_PREFIXis setbackend/src/DoujinManager.Server/Program.cs:73callsapp.MapOpenApi()with no route argument, so the spec JSON is served at the ASP.NET Core default/openapi/v1.json— unprefixed in every environment.But
backend/src/DoujinManager.Server/ScalarUi.cs:22builds the spec URL for the Scalar page as:So with
DOUJIN_MANAGER_PATH_PREFIX=/doujinshi:/doujinshi/scalar/(Program.cs:70-72) ✓/doujinshi/openapi/v1.json✗/openapi/v1.json(no route exists for/doujinshi/openapi/v1.json) → 404 → Scalar loads a blank page with a failed-spec error.This also breaks depending on how nginx is configured, because the two endpoints disagree on whether the prefix is part of the app's own routing:
/doujinshi/...to the app): Scalar page loads, but the spec fetch to/doujinshi/openapi/v1.json404s (app route is/openapi/v1.json)./...to the app): the spec fetch works, but the Scalar page itself 404s — the app mapped it at/doujinshi/scalar/, yet it receives/scalar/.The no-prefix case (the default, and presumably what the 234 passing tests exercise) works fine, which is likely why CI didn't catch this. The bug only manifests when the new env var is actually set — i.e. exactly the scenario the PR advertises.
Suggested fix — make the OpenAPI route prefix-aware so the spec URL ScalarUi emits actually exists on the app:
and keep
ScalarUipointing at the same prefixed path. (Alternatively, document that nginx must strip the prefix and then do not setDOUJIN_MANAGER_PATH_PREFIXat all — but then the whole prefix feature is unnecessary.) Recommend adding an integration test that sets the prefix and asserts both/doujinshi/scalar/and/doujinshi/openapi/v1.jsonreturn 200.🟡 Minor — OpenAPI spec + Scalar UI are publicly readable in Production
Previously,
MapOpenApi()ran only insideif (app.Environment.IsDevelopment()). It now runs unconditionally (Program.cs:73) and, perStaticBearerTokenAuthMiddleware(only gates/api/*), both/openapi/v1.jsonand/scalarare reachable without a token in every environment. The PR body confirms this is intentional. It's a reasonable trade-off since all actual data endpoints under/api/*remain token-gated, but it does expose the full API surface (every endpoint, DTO shape, validation constraints) to unauthenticated callers in Production. Just flagging so it's a conscious decision — if the API structure is considered sensitive, gate these routes behind the token or keep them Development-only.🟡 Minor —
DOUJIN_MANAGER_PATH_PREFIXmissing from deployment docs(Already noted by @bjoern.)
deploy/DEPLOYMENT.md§2 "Environment variables" (lines 34-38) lists the other five env vars but omitsDOUJIN_MANAGER_PATH_PREFIX. The only documentation is the commented-out line indeploy/docker-compose.yml:42-44. Add a row to the table so operators discover it.✅ What looks good
CryptographicOperations.FixedTimeEquals(timing-safe) — unchanged and correct.data-urlattribute is HTML-attribute-encoded viaHttpUtility.HtmlAttributeEncode— fine (value is operator config, not user input).ScalarUi.csis clean, well-documented, and the HTML template uses raw string interpolation safely.eval/exec/pickle. Static security scan clean on the diff.Verification: CI/CD — forgejo-actions coverage comment #261 is current for head
ba1424c3(line 83.5%, branch 50%); the coverage comment is only posted on successful build+test, so local build/test was skipped per the CI-evidence policy. No localdotnetrun performed.Automated daily review. I never merge PRs. This is a conversation comment, not a formal Forgejo review approval — the current MCP integration cannot create approval states.
Address all review findings: 1. [major] OpenAPI spec route was not prefixed — Scalar would 404 when fetching the spec. Now uses app.MapOpenApi(openApiRoute + '/{documentName}.json') so the spec JSON exists at /{prefix}/openapi/v1.json when prefix is set. Scalar UI and OpenAPI JSON are now both consistently prefixed. 2. [minor] Scalar.AspNetCore upgraded 2.5.0 → 2.9.0 3. [minor] DOUJIN_MANAGER_PATH_PREFIX added to DEPLOYMENT.md env vars table Also simplified ScalarUi.GetHtml to take the openApiRoute directly instead of separate scalarPath + pathPrefix params.Addressed all findings in commit
badd344. 234 tests pass.#1 [major] — OpenAPI spec route is now prefix-aware ✅
The spec JSON is now served at
/{prefix}/openapi/v1.jsonwhenDOUJIN_MANAGER_PATH_PREFIXis set. Previouslyapp.MapOpenApi()always served at/openapi/v1.json(unprefixed), so Scalar would 404 when fetching the spec. Now both the Scalar UI and the spec JSON live under the same prefix. nginx must preserve the prefix (not strip it).#2 [minor] — DOUJIN_MANAGER_PATH_PREFIX added to deployment docs ✅
Added to the §2 environment variables table in DEPLOYMENT.md.
Bonus — Scalar upgraded to 2.9.0 (per Björn's note).
Also simplified
ScalarUi.GetHtmlto take theopenApiRoutedirectly instead of separate params.🤖 Hermes automated review: no blocking issues found
Follow-up review of fix commit
badd344(head changedba1424c3→badd3442, +16/-18 across 4 files). CI is current for the new head.✅ Prior findings resolved
#1 [major] — OpenAPI spec route is now prefix-aware ✅
Program.cs:72now callsapp.MapOpenApi(openApiRoute + "/{documentName}.json"), so the spec JSON is served at/{prefix}/openapi/v1.jsonwhenDOUJIN_MANAGER_PATH_PREFIXis set.ScalarUi.GetHtml(openApiRoute)emits$"{openApiRoute}/v1.json"as the Scalardata-url, which now matches the registered route exactly. All three endpoints (Scalar redirect, Scalar HTML, OpenAPI JSON) agree on the prefix. The nginx requirement (must preserve the prefix, not strip it) is correctly documented in both the code comment and DEPLOYMENT.md. The major finding is resolved.#2 [minor] —
DOUJIN_MANAGER_PATH_PREFIXadded to deployment docs ✅Added to the §2 environment variables table in
deploy/DEPLOYMENT.mdwith clear usage notes. Resolved.#3 [minor] — OpenAPI/Scalar public in Production (non-blocking, unchanged)
Still served unconditionally with
.AllowAnonymous(). This was flagged as a conscious decision in the prior review and remains a reasonable trade-off since all/api/*data endpoints stay token-gated. No change needed.🟡 Minor non-blocking note
No integration test for the prefix path —
ScalarUiandProgramremain at 0% line coverage (DoujinManager.Server: 17%). The prefix-routing logic is now correct and internally consistent, but it's exercised only by manual testing, not automated tests. A lightweight integration test that setsDOUJIN_MANAGER_PATH_PREFIX=/doujinshiand assertsGET /doujinshi/scalar/→ 200 andGET /doujinshi/openapi/v1.json→ 200 would prevent regressions in this exact code path. Low priority — the logic is straightforward and now verified correct.✅ What looks good
hasPrefixboolean + ternary avoids the priorstring.IsNullOrEmptyduplication.ScalarUi.GetHtmlsignature simplified to take a singleopenApiRoute— good; the spec URL is now derived from the same source the route uses.HttpUtility.HtmlAttributeEncode(specUrl)retained — HTML-attribute injection protected (value is operator config, not user input).Trim('/')on the env var handles both leading/trailing slashes; multi-segment prefixes (e.g./a/b) also work consistently.eval/exec/pickle.Verification: CI/CD — forgejo-actions coverage comment #261 is current for head
badd3442(line 83.5%, branch 50%; coverage date 00:43:43–00:43:57 UTC, after the fix commit at 00:43:13 UTC). The coverage comment is only posted on successful build+test, so local build/test was skipped per the CI-evidence policy. No localdotnetrun performed.Automated daily review. I never merge PRs. This is a conversation comment, not a formal Forgejo review approval — the current MCP integration cannot create approval states.