Skip to content

fix(docs): use service origins and verify Swagger asset integrity - #135

Open
csnitker-godaddy wants to merge 3 commits into
mainfrom
fix/swagger-service-origin
Open

csnitker-godaddy wants to merge 3 commits into
mainfrom
fix/swagger-service-origin

Conversation

@csnitker-godaddy

@csnitker-godaddy csnitker-godaddy commented Sep 21, 2026 •

Copy link
Copy Markdown
Member

Fix Swagger parsing and requests on deployed RA/TL hosts: remove the duplicate
Problem schema, use /v2 for RA operations and / for TL and the V1 events
feed, and keep canonical/embedded documents synchronized.

Pin the existing Swagger UI 5.17.14 CSS and JavaScript with SHA-384 Subresource
Integrity and anonymous CORS. Hashes were checked against the npm tarball's
verified distribution and the actual CDN responses. Generated clients should
receive an explicit deployment base URL.

Validation: static analysis, duplicate-key-aware YAML parsing, matching
canonical/embedded documents, and served Swagger attributes on Ubuntu.

Fixes #132.

AI assistance

Assisted-by: Codex (GPT-6), under Connor Snitker's direction.

Signed-off-by: Connor Snitker <csnitker@godaddy.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Replace the RA event-path server overrides with a single / server in both canonical and embedded documents.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Updates RA and TL OpenAPI documents for valid Swagger parsing and same-origin requests.

Changes:

  • Removes the duplicate RA Problem schema.
  • Uses /v2 for RA and / for TL server URLs.
  • Synchronizes canonical and embedded documents.
  • Two moderate findings remain, each with 3 votes: the RA event-path override still exposes localhost and example-host URLs in both RA documents.
File Summary
spec/​api-spec-v2.yaml Canonical RA specification; event-path server override remains unresolved.
spec/​api-spec-tl-v2.yaml Canonical TL specification.
internal/​adapter/​docsui/​openapi/​tl.yaml Embedded TL specification.
internal/​adapter/​docsui/​openapi/​ra.yaml Embedded RA specification; event-path server override remains unresolved.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +46 to +47
- url: /v2
description: This Registration Authority (current service origin)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /v1/agents/events override now uses only / in both canonical and embedded RA documents. The root RA server remains /v2, and TL uses /, so each route resolves against the serving origin.

Comment thread spec/api-spec-v2.yaml
Comment on lines +46 to +47
- url: /v2
description: This Registration Authority (current service origin)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feed override is now / in both RA copies. It no longer overrides the same-origin configuration with localhost or an example hostname.

Signed-off-by: Connor Snitker <csnitker@godaddy.com>

@kperry-godaddy kperry-godaddy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relative servers entries are the right call, and I checked the mechanics rather than trusting them: swagger-js resolves a relative server URL against the document URL and strips the trailing slash before it appends the operation path, so the TL's url: / produces clean https://host/v1/... requests. Removing the second Problem also kept the accurate copy: the generic 500 path in errors.go emits no code, so the deleted definition's required: code was the wrong one. Both documents now pass a duplicate-key-strict YAML parse, and the embedded copies are byte-identical to spec/.

One thing I'd settle before this merges, inline on the servers block. Two small notes below.

Not blocking, worth a look

  • 97 $refs still point at ErrorResponse and 15 at Problem, while every RA failure path emits Problem Details. The deleted duplicate carried the only note that this migration is tracked separately, and I couldn't find an issue for it. Opening one would keep the work item from disappearing with the comment.
  • Dropping the named hosts changes what a code generator seeds as its default base URL from servers[0].url. If any SDK build reads this document directly, it will want an explicit base URL now.

Comment thread spec/api-spec-v2.yaml
description: Main ANS API server (v2)
- url: https://test.ans.example.org/v2
description: Test server (v2)
- url: /v2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root list is fixed, but the operation-level override on /v1/agents/events (lines 96-100) still carries http://localhost:18080 and https://api.ans.example.org. A path item's servers replaces the root list for that path (OAS 3.0.3, Path Item Object: https://spec.openapis.org/oas/v3.0.3#path-item-object), so on a deployed RA the events-feed "Try it out" still goes to localhost, which is the behaviour #132 describes. Same treatment there, then re-copy the embedded ra.yaml:

  /v1/agents/events:
    servers:
      - url: /
        description: This Registration Authority (the feed is served at the origin root, not under /v2)

Worth fixing before this ships since it is the same defect on the one route that lives outside /v2; the change is four lines and the docsui test will catch a missed copy.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied the root-relative feed override and kept canonical/embedded copies identical. Generated clients must receive an explicit deployment base URL; the relative URL is intentional for Swagger served by either service.

Signed-off-by: Connor Snitker <csnitker@godaddy.com>
@csnitker-godaddy csnitker-godaddy changed the title fix(docs): make Swagger work on the deployed service origin fix(docs): use service origins and verify Swagger asset integrity Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

Swagger UI fails to parse RA spec and targets incorrect service URLs

3 participants