Feature/better middleware #2

bwalsh · 2025-12-23T01:16:33Z

Context

The DRS server is implemented in Go (Gin) with server scaffolding generated from OpenAPI. Contract correctness matters for interoperability and operational safety. The OpenAPI spec includes deep schemas and recursive references (e.g., DrsObject → ContentsObject → DrsObject), making “generator-only correctness” insufficient.

We need:

Strong request/response contract guarantees
Clear operational visibility into contract drift
A safe rollout path (audit-first in prod)
CI enforcement to prevent regressions

Decision

Implement OpenAPI validation middleware using kin-openapi (openapi3filter) as the validation engine.

Request validation: enforce at runtime by default (reject invalid requests)
Response validation: buffering + late-commit middleware with two modes:
- Audit mode (default in production): log/metrics on violations, allow response
- Enforce mode (CI / pre-prod): block invalid responses (fail closed with 500)

Black-box contract testing complements runtime validation in CI.

Summary

flowchart TB
    Client[Client] -->|HTTP Request| Gin[gin Engine]

    subgraph MiddlewareChain[Middleware Chain]
        LogReq[Request Logging redact auth] --> ReqVal[OpenAPI Request Validation enforce]
        ReqVal --> Handler[Application Handlers generated]
        Handler --> RespVal[OpenAPI Response Validation audit or enforce]
        RespVal --> Commit[Late Commit Writer buffered]
    end

    Gin --> MiddlewareChain -->|HTTP Response| Client

    Spec[OpenAPI Spec local or embedded] --> ReqVal
    Spec --> RespVal

    CI[CI Pipeline] --> Contract[Contract Tests]
    Spec --> Contract
    Contract --> Report[Fail build on violations]

See docs/adr-0001-request-response-validation.md

Changes

Added a GET /service-info route in cmd/server/service_info.go.
Return a constant JSON payload that matches the GA4GH DRS Service Info example:
- Top-level service metadata (id, name, description, organization, contactUrl, documentationUrl, environment, etc.).
- Version fields (version, drs_version, service_url).
- Deprecated top-level maxBulkRequestLength retained for backward compatibility.
- drs section with implementation details (bulk limits, object counts, upload/delete capabilities, validation flags, etc.).
Adjusted the handler to respond with http.StatusOK and the static serviceInfoResponse map.

Motivation

The OpenAPI spec already defines /service-info, and middleware attempts to validate requests and responses for this path.
Previously, FindRoute and security validation failures were blocking proper validation; this change wires up a concrete implementation of the endpoint.
Returning a static, well-formed document makes it easier to:
- Verify routing and OpenAPI middleware behavior.
- Provide a predictable response for health/diagnostic checks and external integrations.

Testing

Started the server and performed GET /service-info:
- Confirmed a 200 OK response.
- Verified the JSON body matches the expected GA4GH DRS Service Info structure.
Observed logs to ensure the route is discovered from the OpenAPI spec and validated without security errors.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-12-23T01:20:18Z

cmd/server/middleware/buffering_writer.go

+	// Enforce max buffer
+	if w.maxBody > 0 && int64(w.body.Len()+len(p)) > w.maxBody {
+		w.tooLarge = true
+		// We still buffer up to maxBody, and then ignore the rest (or you can stop buffering entirely)
+		remaining := int(w.maxBody - int64(w.body.Len()))
+		if remaining > 0 {
+			_, _ = w.body.Write(p[:remaining])
+		}
+		return len(p), nil


Avoid truncating responses beyond MaxBodyBytes

The buffering writer drops any bytes beyond MaxBodyBytes while still returning len(p) and only commits the buffered prefix. Because the response validator later commits w.body instead of the full stream, any handler that writes more than the 2 MiB default will silently return a truncated, corrupt payload even in audit mode instead of simply skipping validation. This breaks large downloads and any endpoint that legitimately returns bodies above the cap.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector
@copilot

Should we raise a 413 Content Too Large exception here?

To use Codex here, create an environment for this repo.

Copilot

Pull request overview

This PR implements OpenAPI validation middleware for the DRS server with both request and response validation capabilities. The implementation uses kin-openapi for validation with configurable audit and enforce modes, allowing safe production rollout. The PR also adds GA4GH DRS-compliant service info and health check endpoints.

Adds comprehensive OpenAPI request/response validation middleware with buffering support
Implements /service-info endpoint returning GA4GH DRS Service Info metadata
Adds /healthz health check endpoint for monitoring and orchestration systems

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
internal/apigen/go.mod	Updates Go version to 1.24.0 and upgrades golang.org/x dependencies
internal/apigen/go.sum	Adds checksums for updated dependencies
docs/adr-0001-request-response-validation.md	Comprehensive ADR documenting the OpenAPI validation architecture, configuration, and implementation approach
cmd/server/middleware/openapi_validator.go	Enhances request validator with route attachment, security handling, and lenient mode logging
cmd/server/middleware/openapi_response_validator.go	Implements response validation middleware with audit/enforce modes and buffering
cmd/server/middleware/openapi_response_log.go	Adds detailed logging for OpenAPI validation failures with redaction support
cmd/server/middleware/buffering_writer.go	Implements buffering writer for late-commit response validation
cmd/server/middleware/logging_middleware.go	Provides request/response logging with auth header redaction
cmd/server/handlers/service_info.go	Implements GA4GH DRS Service Info endpoint with static metadata
cmd/server/handlers/healthz.go	Adds health check endpoint
cmd/server/main.go	Integrates request and response validators into middleware chain
cmd/server/service_info.go	Removes old service info implementation (replaced by handlers/service_info.go)

Comments suppressed due to low confidence (1)

cmd/server/middleware/openapi_validator.go:36

Empty comment line without any explanation. This should either be removed or have a descriptive comment explaining why the servers are being neutralized. Consider replacing with something like "// Neutralize server URLs to allow matching requests regardless of host/base path".

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T14:05:07Z

cmd/server/handlers/service_info.go

+		"drs_version":          "1.3.0",
+		"service_url":          "https://drs.example.org",
+		"maxBulkRequestLength": 1000, // Deprecated
+		"timestamp":            "2024-01-01T12:00:00.000Z",


The timestamp "2024-01-01T12:00:00.000Z" appears to be a placeholder or test value from the past. For a production service, this should either be dynamically generated to reflect the actual current time or updated to a more recent date if it represents a static deployment timestamp.

cmd/server/main.go

Copilot · 2025-12-23T14:05:08Z

cmd/server/handlers/service_info.go

+		"createdAt":            "2020-01-01T00:00:00.000Z",
+		"updatedAt":            "2020-01-02T00:00:00.000Z",


The dates "2020-01-01T00:00:00.000Z" and "2020-01-02T00:00:00.000Z" for createdAt and updatedAt are placeholder values from 5 years ago. For a production service, these should be replaced with actual service deployment dates or removed if they're not meaningful for this implementation.

Copilot · 2025-12-23T14:05:08Z

cmd/server/handlers/service_info.go

+		// Construct a JSON response using Gin's `H` helper, which is a
+		// shorthand for `map[string]any`. The response includes:
+		//  * `name`: a static identifier for this service.
+		//  * `version`: the service version read from the SERVICE_VERSION
+		//    environment variable (may be empty if not set).
+		//  * `timestamp`: the current UTC time, formatted as an RFC3339Nano string.


The comment references outdated handler behavior. The comment mentions "version" being read from SERVICE_VERSION environment variable and "timestamp" being the current UTC time, but the actual implementation now returns static values from the serviceInfoResponse map defined above. This comment should be updated to reflect that the handler returns a static GA4GH DRS Service Info structure.

Suggested change

// Construct a JSON response using Gin's `H` helper, which is a

// shorthand for `map[string]any`. The response includes:

// * `name`: a static identifier for this service.

// * `version`: the service version read from the SERVICE_VERSION

// environment variable (may be empty if not set).

// * `timestamp`: the current UTC time, formatted as an RFC3339Nano string.

// Return the static GA4GH DRS Service Info structure defined in

// `serviceInfoResponse` above. This includes fields such as:

// * top-level service metadata (e.g. `id`, `name`, `type`, `version`)

// * timestamps (`createdAt`, `updatedAt`, `timestamp`)

// * organization and contact details

// * DRS-specific capabilities and limits under the `drs` key.

cmd/server/middleware/openapi_response_validator.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2025-12-23T17:36:08Z

@bwalsh I've opened a new pull request, #5, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * Switch to mkdocs-material theme with Mermaid support Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add SRI integrity hash for Mermaid.js security Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add DOMContentLoaded event listener for Mermaid initialization Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>

bwalsh added 3 commits December 22, 2025 15:07

adds req/rsp logging

857c5e1

chore: update generated

3d332d7

adds req/rsp validation; relaxes path

ee0bb4c

Copilot AI review requested due to automatic review settings December 23, 2025 01:16

Copilot started reviewing on behalf of bwalsh December 23, 2025 01:16 View session

bwalsh requested review from kellrott and removed request for Copilot December 23, 2025 01:19

chatgpt-codex-connector bot reviewed Dec 23, 2025

View reviewed changes

refactors handlers, middleware pkg

92dfa9c

Copilot AI review requested due to automatic review settings December 23, 2025 14:00

Copilot started reviewing on behalf of bwalsh December 23, 2025 14:01 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

Apply suggestions from code review

3de432a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Dec 23, 2025

Return 413 Content Too Large for oversized response bodies #5

Draft

bwalsh added 7 commits December 25, 2025 17:31

refactors handlers, middleware pkg

0eea29b

adds passport discussion

753fab4

improves docs/

655379d

fix bad merge

9e968b1

update status

48addf3

github pages

de6c681

github pages

06db677

bwalsh temporarily deployed to github-pages December 26, 2025 01:58 — with GitHub Pages Inactive

bwalsh temporarily deployed to github-pages December 26, 2025 02:01 — with GitHub Pages Inactive

bwalsh and others added 6 commits December 26, 2025 09:56

improve github pages

0d17e12

tweak mermaid config

976f3c5

tweak mermaid config

e8e5155

Merge branch 'development' into feature/better-middleware

8ab7cee

cleanup

e5e440f

bwalsh added 13 commits December 26, 2025 12:02

fix sha

923f9f1

remove plugin

e09d66d

deprecate

0a3e3a0

adds docs-build

173e960

plugin

02a5644

misc

f0098db

improve diagram

1035a81

improve diagram

f9fa4ce

adds comments

4408f37

improve diagram

a5a1fa0

adds persistence

80aa561

TODO

9852183

adds link to pages

e372471

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/better middleware #2

Feature/better middleware #2

Uh oh!

bwalsh commented Dec 23, 2025 •

edited

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Dec 23, 2025

Uh oh!

bwalsh Dec 23, 2025

Uh oh!

chatgpt-codex-connector bot Dec 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Uh oh!

Copilot AI commented Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		"createdAt": "2020-01-01T00:00:00.000Z",
		"updatedAt": "2020-01-02T00:00:00.000Z",

Feature/better middleware #2

Are you sure you want to change the base?

Feature/better middleware #2

Uh oh!

Conversation

bwalsh commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Decision

Summary

Changes

Motivation

Testing

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

bwalsh Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI commented Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bwalsh commented Dec 23, 2025 •

edited

Loading