Skip to content

feat: implement structured JSON logging for gateway requests and payments#192

Open
aayushprsingh wants to merge 2 commits into
AnkanMisra:mainfrom
aayushprsingh:gssoc-fix-paygate
Open

feat: implement structured JSON logging for gateway requests and payments#192
aayushprsingh wants to merge 2 commits into
AnkanMisra:mainfrom
aayushprsingh:gssoc-fix-paygate

Conversation

@aayushprsingh

@aayushprsingh aayushprsingh commented Jun 2, 2026

Copy link
Copy Markdown

Closes #162

This PR adds structured JSON logging capability to the gateway service. When LOG_FORMAT=json is set, Gin is configured in ReleaseMode, default logger is bypassed, and JSONLoggerMiddleware is registered to write structured execution details to stdout. Verification errors and successful payments are recorded dynamically inside the Gin context.

Summary by CodeRabbit

  • New Features

    • Structured JSON logging for gateway requests and payment operations when LOG_FORMAT=json.
    • Payment metadata (status and payer) now recorded with request handling.
  • Bug Fixes / Improvements

    • Logging now redacts sensitive details and suppresses non-JSON log output in JSON mode.
  • Tests

    • Added tests validating JSON logging and sensitive-data redaction.

Copilot AI review requested due to automatic review settings June 2, 2026 12:50
@vercel

vercel Bot commented Jun 2, 2026

Copy link
Copy Markdown

@aayushprsingh is attempting to deploy a commit to the ankanmisra's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds structured JSON request logging with a JSONLoggerMiddleware and sanitization helper, enriches Gin context with payment metadata on success and error paths, conditionally enables JSON logs at startup, and adds tests validating JSON output and secret redaction.

Changes

JSON Logging and Payment Tracking

Layer / File(s) Summary
Error string sanitization helper
gateway/errors.go
Adds regex patterns and sanitizeErrorString to redact 64-hex sequences and sk-or- style API keys from internal error messages.
respondError context enrichment and logging
gateway/errors.go
respondError sets payment_status: "failed", payment_error (public message), and optional internal_error (sanitized) in Gin context; uses sanitized internal error for logs and suppresses non-JSON logging when LOG_FORMAT == "json".
Startup integration and handler context updates
gateway/main.go
main() conditionally configures Gin for JSON mode (gin.New() + gin.Recovery() + JSONLoggerMiddleware()); handlers set payment_status: "success" and payer after verification and suppress plaintext logs when LOG_FORMAT == "json" on receipt errors.
JSON logging middleware implementation
gateway/middleware.go
Adds exported JSONLogEntry and JSONLoggerMiddleware() that records timestamp, level (from status), status, method, path, latency, client IP, correlation ID, optional payment fields, sanitizes internal_error, marshals to JSON, and writes to stdout.
JSON middleware testing and validation
gateway/main_test.go
Adds TestJSONLoggerMiddleware and TestJSONLoggerMiddleware_Sanitization which capture stdout, parse emitted JSON into JSONLogEntry, assert expected fields and that sensitive data is redacted.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Gin as Gin Router/Handlers
  participant Handler as handleSummarize/handleGetReceipt
  participant JSONLogger as JSONLoggerMiddleware
  participant Stdout as Stdout

  Client->>Gin: HTTP request (with correlation_id)
  Gin->>JSONLogger: Enter middleware
  JSONLogger->>Gin: call next handler
  Gin->>Handler: perform verification / generate receipt
  Handler->>Gin: set context keys (payment_status, payer/payment_error, internal_error)
  Gin->>JSONLogger: response ready
  JSONLogger->>JSONLogger: build JSONLogEntry (sanitizes internal_error)
  JSONLogger->>Stdout: print JSON log
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement, type:testing

Suggested reviewers

  • AnkanMisra

Poem

🐰 I hop through logs with tidy art,
Redacting keys and guarding heart,
Payments marked as win or fail,
JSON trails where traces sail,
Little rabbit logs each part.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description lacks critical template sections: Type of Change is missing, Affected Areas is missing, Contributor Checklist is incomplete, and Verification section is empty. Complete the PR description template by selecting type of change (Feature), marking affected areas (Gateway), checking the contributor checklist items, and documenting verification steps.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive While the code changes implement JSON logging and error sanitization, the PR description does not clearly document whether all acceptance criteria are met or provide test results covering the required scenarios. Document in the PR description or comments which acceptance criteria are met, provide test results for success/402/invalid signature/replay/upstream failure scenarios, and confirm deployment docs were updated.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title directly and concisely describes the main feature: implementing structured JSON logging for gateway requests and payments.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing JSON logging: middleware addition, context enrichment in handlers, conditional Gin configuration, and comprehensive tests for the logging functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds structured JSON request logging to the gateway, controlled via an environment variable, and propagates payment-related context fields into logs.

Changes:

  • Introduces JSONLoggerMiddleware and JSONLogEntry for structured request logs.
  • Switches router initialization based on LOG_FORMAT=json to use the new JSON logger.
  • Adds payment success/failure context fields and a unit test that validates JSON log output.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
gateway/middleware.go Adds JSON log entry schema and middleware that emits JSON logs per request
gateway/main.go Conditionally enables JSON logging and sets success payment context fields
gateway/errors.go Adds error-path context fields to be included in JSON logs
gateway/main_test.go Adds a test that captures stdout and validates emitted JSON logs

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

Comment thread gateway/errors.go
Comment on lines +13 to +17
c.Set("payment_status", "failed")
c.Set("payment_error", publicMsg)
if internalErr != nil {
c.Set("internal_error", internalErr.Error())
}
Comment thread gateway/middleware.go
Comment on lines +350 to +351
correlationID, _ := c.Get("correlation_id")
corrStr, _ := correlationID.(string)
Comment thread gateway/middleware.go
Comment on lines +368 to +370
if data, err := json.Marshal(entry); err == nil {
fmt.Println(string(data))
}
Comment thread gateway/main_test.go
Comment on lines +622 to +624
oldStdout := os.Stdout
rPipe, wPipe, _ := os.Pipe()
os.Stdout = wPipe

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gateway/errors.go`:
- Around line 13-17: The code currently stores raw internalErr.Error() into
c.Set("internal_error"), which can leak upstream payloads into logs; instead,
detect/sanitize the error (use errors.As or type assertions against known error
types from verifyPayment and other callers) and store a safe classification
string (e.g., "verifier_error", "upstream_4xx", or an error code) into
c.Set("internal_error"). Put the full internalErr details only into local
diagnostics/logs that are not emitted by JSONLoggerMiddleware (for example use
the request logger or a separate non-JSON diagnostic key), and ensure
JSONLoggerMiddleware reads the safe classification key; update places
referencing internal_error to expect the sanitized value and preserve full error
via a private diagnostic path.

In `@gateway/main_test.go`:
- Around line 644-674: Before asserting that sensitive values aren't logged,
seed the test request with representative sensitive inputs: add headers like
req.Header.Set("PRIVATE_KEY", "0x1234567890abcdef") and
req.Header.Set("Authorization", "Bearer 0x1234567890abcdef") and, if applicable,
give the request a body containing "0x1234567890abcdef" (use the same req
variable used with r.ServeHTTP); then run the handler and assert the captured
output (output / JSONLogEntry) does not contain those strings. Update the
assertions around JSONLogEntry/output to check that sensitive header keys/values
and body content are redacted or omitted after the call to r.ServeHTTP, using
the existing variables (req, rPipe, wPipe, output, JSONLogEntry) and keeping the
NotContains checks you already have.

In `@gateway/main.go`:
- Around line 267-275: When LOG_FORMAT=="json" you must stop emitting plain-text
per-request logs; update CorrelationIDMiddleware() and respondError() to either
use the same structured logger used by JSONLoggerMiddleware() or skip their
log.Printf calls when os.Getenv("LOG_FORMAT")== "json". Concretely, in
CorrelationIDMiddleware() and respondError() replace direct log.Printf usage
with a call into the structured logger (or wrap the log.Printf in an if that
checks LOG_FORMAT!="json"), ensuring all request/error logs go through
JSONLoggerMiddleware()’s format so you don't produce mixed plain-text and JSON
streams.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f675a39f-f102-49b0-a57e-fb7c39872245

📥 Commits

Reviewing files that changed from the base of the PR and between 42191f5 and 1f69a26.

📒 Files selected for processing (4)
  • gateway/errors.go
  • gateway/main.go
  • gateway/main_test.go
  • gateway/middleware.go

Comment thread gateway/errors.go
Comment thread gateway/main_test.go
Comment thread gateway/main.go
Comment on lines +267 to +275
var r *gin.Engine
if os.Getenv("LOG_FORMAT") == "json" {
gin.SetMode(gin.ReleaseMode)
r = gin.New()
r.Use(gin.Recovery())
r.Use(JSONLoggerMiddleware())
} else {
r = gin.Default()
}

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

LOG_FORMAT=json still leaves mixed plain-text request logs enabled.

This branch removes Gin’s default logger, but CorrelationIDMiddleware() and respondError() still write per-request/per-error log.Printf lines. In JSON mode that gives you duplicate, mixed-format log streams, which usually breaks JSON-only ingestion. Please gate those legacy log lines behind the same flag or route them through the structured logger too.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gateway/main.go` around lines 267 - 275, When LOG_FORMAT=="json" you must
stop emitting plain-text per-request logs; update CorrelationIDMiddleware() and
respondError() to either use the same structured logger used by
JSONLoggerMiddleware() or skip their log.Printf calls when
os.Getenv("LOG_FORMAT")== "json". Concretely, in CorrelationIDMiddleware() and
respondError() replace direct log.Printf usage with a call into the structured
logger (or wrap the log.Printf in an if that checks LOG_FORMAT!="json"),
ensuring all request/error logs go through JSONLoggerMiddleware()’s format so
you don't produce mixed plain-text and JSON streams.

@vercel

vercel Bot commented Jun 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
microai-paygate Ready Ready Preview, Comment Jun 6, 2026 12:09pm

@AnkanMisra

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f69a26503

ℹ️ 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".

Comment thread gateway/main.go
gin.SetMode(gin.ReleaseMode)
r = gin.New()
r.Use(gin.Recovery())
r.Use(JSONLoggerMiddleware())

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.

P2 Badge Emit only structured request logs in JSON mode

When LOG_FORMAT=json is enabled, registering the JSON logger here does not make request output purely structured because CorrelationIDMiddleware still calls log.Printf for every request in gateway/middleware.go:53. In deployments that send stdout to a JSON log parser, each request will still produce a plaintext correlation line alongside the JSON entry, so the new mode is noisy or partially unparseable; gate that legacy request log or emit it through the structured logger.

Useful? React with 👍 / 👎.

Comment thread gateway/main.go
Comment on lines +477 to +478
c.Set("payment_status", "success")
c.Set("payer", verifyResp.RecoveredAddress)

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.

P2 Badge Set payment log fields on cache hits

This annotates successful payments only after handleSummarize runs, but with CACHE_ENABLED=true a Redis cache hit is verified and then aborts inside CacheMiddleware before reaching this handler. Those successful cached paid requests still generate receipts, but their JSON log entries omit payment_status and payer, which makes payment observability incomplete for the cache-hit path; set the same context fields in the verified cache-hit branch before generateAndSendReceipt.

Useful? React with 👍 / 👎.

Comment thread gateway/middleware.go
func JSONLoggerMiddleware() gin.HandlerFunc {
return func(c *gin.Context) {
start := time.Now()
c.Next()

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.

P2 Badge Preserve JSON logging for recovered panics

Because the logger runs c.Next() without a defer, any downstream panic skips the JSON entry entirely; with the current registration order gin.Recovery() is outside this middleware, so Recovery writes the 500 after the panic has already unwound past the logger. In LOG_FORMAT=json deployments, panic responses therefore have no structured request log even though normal 5xx responses do; wrap the post-request logging in a defer or move Recovery inside the logging middleware so recovered panics still produce an error-level entry.

Useful? React with 👍 / 👎.

Comment thread gateway/main.go

r := gin.Default()
var r *gin.Engine
if os.Getenv("LOG_FORMAT") == "json" {

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.

P2 Badge Document the new LOG_FORMAT setting

This introduces LOG_FORMAT=json as the switch for structured gateway logs, but the repo configuration docs are not updated: rg "LOG_FORMAT" README.md .env.example gateway only finds this code path and no README/env-example entry. Operators following the documented environment table will not discover how to enable the feature or what values are supported, so add it to .env.example and the gateway configuration docs.

Useful? React with 👍 / 👎.

Comment thread gateway/errors.go
)

func respondError(c *gin.Context, code int, publicMsg string, internalErr error) {
c.Set("payment_status", "failed")

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.

P2 Badge Do not overwrite successful payment state on later failures

Because respondError now unconditionally sets payment_status to failed, any request that verifies payment successfully and then fails during JSON parsing, upstream AI generation, receipt generation, or receipt storage is logged as a failed payment even though handleSummarize already recovered the payer and marked the payment successful. For those post-verification error paths the structured log becomes misleading for payment auditing; only set failed when no successful payment status is already present, or track request failure separately from payment verification status.

Useful? React with 👍 / 👎.

Comment thread gateway/errors.go Outdated

@AnkanMisra AnkanMisra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@AnkanMisra

Copy link
Copy Markdown
Owner

@aayushprsingh Please link which issue you solved

@aayushprsingh

Copy link
Copy Markdown
Author

Hi @AnkanMisra! This PR resolves issue #162.

@AnkanMisra

Copy link
Copy Markdown
Owner

@aayushprsingh fix the bugs mentioned by codex

@github-actions github-actions Bot added go Pull requests that update go code type:testing Tests, coverage, fixtures, or validation-only work. labels Jun 6, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

♻️ Duplicate comments (1)
gateway/main_test.go (1)

672-674: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Seed sensitive inputs before asserting non-leakage.

These NotContains checks are currently vacuous because those values are never injected into the request/context in this test. Seed representative sensitive headers first, then assert omission/redaction in output.

Proposed test hardening
 req, _ := http.NewRequest("GET", "/test-json-log", nil)
 req.RemoteAddr = "127.0.0.1:12345"
+req.Header.Set("Authorization", "Bearer PRIVATE_KEY")
+req.Header.Set("X-402-Signature", "0x1234567890abcdef")
 w := httptest.NewRecorder()
 r.ServeHTTP(w, req)

As per coding guidelines, "Ensure private keys/API keys are never logged, returned, committed, or added to OpenAPI examples in the gateway".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gateway/main_test.go` around lines 672 - 674, The test's NotContains
assertions are vacuous because the sensitive values are never injected into the
request/context; before asserting on output, seed representative sensitive
inputs (e.g., set headers or request fields used by the code under test) with
the sample private key and signature values so the test exercises redaction.
Specifically, in gateway/main_test.go modify the test that uses the output
variable to add the sensitive inputs (inject headers like "Authorization",
"X-Private-Key" or request body fields) containing "PRIVATE_KEY" and
"0x1234567890abcdef" prior to invoking the code that produces output, then keep
the require.NotContains(t, output, ...) checks to verify they are
omitted/redacted.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gateway/errors.go`:
- Around line 15-22: The current redaction in sanitizeErrorString uses
hex64Regex and openRouterKeyRegex which only match an exact 64-hex token and an
alphanumeric-only sk-or- prefix, so partial keys or keys with separators get
missed; update hex64Regex and openRouterKeyRegex to broader patterns (e.g.,
match 64+ hex characters or hex sequences adjacent to non-word boundaries and
match sk-or- followed by a wider set of characters until a safe delimiter) and
keep using sanitizeErrorString to ReplaceAllString with the redaction tokens,
ensuring any trailing/leading characters of keys are also covered by the new
regexes (refer to hex64Regex, openRouterKeyRegex, and sanitizeErrorString).

---

Duplicate comments:
In `@gateway/main_test.go`:
- Around line 672-674: The test's NotContains assertions are vacuous because the
sensitive values are never injected into the request/context; before asserting
on output, seed representative sensitive inputs (e.g., set headers or request
fields used by the code under test) with the sample private key and signature
values so the test exercises redaction. Specifically, in gateway/main_test.go
modify the test that uses the output variable to add the sensitive inputs
(inject headers like "Authorization", "X-Private-Key" or request body fields)
containing "PRIVATE_KEY" and "0x1234567890abcdef" prior to invoking the code
that produces output, then keep the require.NotContains(t, output, ...) checks
to verify they are omitted/redacted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1dfc52b8-618b-4aa6-a2bf-cd3b44a4cab0

📥 Commits

Reviewing files that changed from the base of the PR and between 1f69a26 and 7cc7a80.

📒 Files selected for processing (4)
  • gateway/errors.go
  • gateway/main.go
  • gateway/main_test.go
  • gateway/middleware.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • gateway/middleware.go
  • gateway/main.go

Comment thread gateway/errors.go
Comment on lines +15 to +22
hex64Regex = regexp.MustCompile(`\b[0-9a-fA-F]{64}\b`)
openRouterKeyRegex = regexp.MustCompile(`sk-or-[a-zA-Z0-9]+`)
)

func sanitizeErrorString(s string) string {
s = hex64Regex.ReplaceAllString(s, "[redacted_hex_64]")
s = openRouterKeyRegex.ReplaceAllString(s, "[redacted_api_key]")
return s

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Broaden secret-redaction regexes to prevent partial or missed masking.

On Line 15 and Line 16, the current patterns can miss common sensitive formats: exact-64-hex only and sk-or- alnum-only matching can leave signature/key tails unredacted in logs.

🔐 Suggested fix
 var (
-	hex64Regex         = regexp.MustCompile(`\b[0-9a-fA-F]{64}\b`)
-	openRouterKeyRegex = regexp.MustCompile(`sk-or-[a-zA-Z0-9]+`)
+	// Redact long hex secrets (supports optional 0x prefix, 64+ hex chars)
+	hexSecretRegex     = regexp.MustCompile(`(?i)\b(?:0x)?[0-9a-f]{64,}\b`)
+	// Redact OpenRouter-style keys including versioned/delimited forms
+	openRouterKeyRegex = regexp.MustCompile(`\bsk-or-[A-Za-z0-9_-]+\b`)
 )
 
 func sanitizeErrorString(s string) string {
-	s = hex64Regex.ReplaceAllString(s, "[redacted_hex_64]")
+	s = hexSecretRegex.ReplaceAllString(s, "[redacted_hex]")
 	s = openRouterKeyRegex.ReplaceAllString(s, "[redacted_api_key]")
 	return s
 }

As per coding guidelines, "Ensure private keys/API keys are never logged, returned, committed, or added to OpenAPI examples in the gateway."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
hex64Regex = regexp.MustCompile(`\b[0-9a-fA-F]{64}\b`)
openRouterKeyRegex = regexp.MustCompile(`sk-or-[a-zA-Z0-9]+`)
)
func sanitizeErrorString(s string) string {
s = hex64Regex.ReplaceAllString(s, "[redacted_hex_64]")
s = openRouterKeyRegex.ReplaceAllString(s, "[redacted_api_key]")
return s
var (
// Redact long hex secrets (supports optional 0x prefix, 64+ hex chars)
hexSecretRegex = regexp.MustCompile(`(?i)\b(?:0x)?[0-9a-f]{64,}\b`)
// Redact OpenRouter-style keys including versioned/delimited forms
openRouterKeyRegex = regexp.MustCompile(`\bsk-or-[A-Za-z0-9_-]+\b`)
)
func sanitizeErrorString(s string) string {
s = hexSecretRegex.ReplaceAllString(s, "[redacted_hex]")
s = openRouterKeyRegex.ReplaceAllString(s, "[redacted_api_key]")
return s
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gateway/errors.go` around lines 15 - 22, The current redaction in
sanitizeErrorString uses hex64Regex and openRouterKeyRegex which only match an
exact 64-hex token and an alphanumeric-only sk-or- prefix, so partial keys or
keys with separators get missed; update hex64Regex and openRouterKeyRegex to
broader patterns (e.g., match 64+ hex characters or hex sequences adjacent to
non-word boundaries and match sk-or- followed by a wider set of characters until
a safe delimiter) and keep using sanitizeErrorString to ReplaceAllString with
the redaction tokens, ensuring any trailing/leading characters of keys are also
covered by the new regexes (refer to hex64Regex, openRouterKeyRegex, and
sanitizeErrorString).

Source: Coding guidelines

@AnkanMisra

Copy link
Copy Markdown
Owner

@codex review the pull reuqest wrt to #162

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cc7a8058d

ℹ️ 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".

Comment thread gateway/errors.go
Comment on lines +15 to +16
hex64Regex = regexp.MustCompile(`\b[0-9a-fA-F]{64}\b`)
openRouterKeyRegex = regexp.MustCompile(`sk-or-[a-zA-Z0-9]+`)

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.

P1 Badge Redact the full OpenRouter API key

When an internal error contains a real OpenRouter key such as sk-or-v1-<hex>, this pattern stops at the second hyphen and replaces only sk-or-v1, leaving the secret suffix in the JSON log; the official key format includes those hyphens (OpenRouter documentation). The 64-hex pattern similarly misses 0x-prefixed values because the preceding x prevents the word-boundary match. Expand the patterns and add tests using production-format keys and 0x-prefixed secrets.

Useful? React with 👍 / 👎.

Comment thread gateway/main.go
Comment on lines +270 to +274
if os.Getenv("LOG_FORMAT") == "json" {
gin.SetMode(gin.ReleaseMode)
r = gin.New()
r.Use(gin.Recovery())
r.Use(JSONLoggerMiddleware())

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.

P1 Badge Keep JSON mode free of plaintext request logs

When LOG_FORMAT=json, replacing Gin's default logger does not make the gateway output JSON-only: the subsequently registered CorrelationIDMiddleware still executes log.Printf for every request (gateway/middleware.go:53), and cache hit/miss/error paths also emit plaintext (gateway/cache.go:118-186). This produces mixed-format stdout and can break collectors that parse each line as JSON; gate or convert all request-path logging when enabling this mode.

Useful? React with 👍 / 👎.

Comment thread gateway/main.go
Comment on lines +273 to +274
r.Use(gin.Recovery())
r.Use(JSONLoggerMiddleware())

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.

P1 Badge Place recovery inside the JSON request logger

If a downstream handler panics, gin.Recovery() currently wraps JSONLoggerMiddleware; the panic therefore unwinds past the logger at c.Next() before it can emit an entry, and Recovery outputs its own plaintext diagnostic instead. Register the JSON logger before Recovery, or make the logger defer its emission, so panic responses produce the required structured 500 request log.

Useful? React with 👍 / 👎.

Comment thread gateway/errors.go
Comment on lines 25 to +27
func respondError(c *gin.Context, code int, publicMsg string, internalErr error) {
correlationID := responseCorrelationID(c)
c.Set("payment_status", "failed")
c.Set("payment_error", publicMsg)

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.

P2 Badge Preserve verification success on downstream errors

When payment verification succeeds but the AI provider, receipt generation, or receipt storage later fails, the handler first sets payment_status to success and then respondError unconditionally overwrites it with failed. The resulting logs falsely classify an upstream service failure as a failed payment verification, corrupting the payment-flow analytics requested by issue #162; set the payment outcome only on verification failures or store request and verification outcomes separately.

Useful? React with 👍 / 👎.

Comment thread gateway/main.go
Comment on lines +513 to +514
c.Set("payment_status", "success")
c.Set("payer", verifyResp.RecoveredAddress)

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.

P2 Badge Record successful payments served from cache

Successful cache hits verify payment in CacheMiddleware and abort the chain after generating the cached receipt (gateway/cache.go:140-181), so they never reach these newly added context assignments. Consequently their structured log omits both payment_status and payer even though verification succeeded; apply the same metadata assignments in the cache-hit success path.

Useful? React with 👍 / 👎.

Comment thread gateway/errors.go
Comment on lines 30 to +32
if internalErr != nil {
sanitizedErr = sanitizeErrorString(internalErr.Error())
c.Set("internal_error", sanitizedErr)

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.

P1 Badge Omit raw upstream and verifier response bodies

When OpenRouter, Ollama, or the verifier returns an error response, their clients include the raw response body in the propagated error (gateway/internal/ai/openrouter.go:70-72, gateway/internal/ai/ollama.go:63-65, and gateway/main.go:603-611), and this field logs that text after only two narrow regex replacements. Provider or verifier bodies can contain prompts, account details, arbitrary credentials, or other internals explicitly excluded by issue #162, so record a safe error classification/code instead of internalErr.Error() in structured logs.

Useful? React with 👍 / 👎.

Comment thread gateway/middleware.go
Comment on lines +362 to +363
PaymentStatus: c.GetString("payment_status"),
PaymentError: c.GetString("payment_error"),

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.

P2 Badge Mark unsigned 402 challenges as payment outcomes

For the normal first request in the x402 flow, handleSummarize returns 402 directly without setting any payment context fields (gateway/main.go:445-451), so this lookup leaves payment_status and payment_error absent. That makes the most common payment outcome indistinguishable from an unrelated HTTP 402 and prevents the requested payment-flow analytics; set an explicit safe outcome such as required before returning the challenge.

Useful? React with 👍 / 👎.

Comment thread gateway/middleware.go
Timestamp: time.Now().UTC().Format(time.RFC3339),
Level: level,
Status: status,
Path: c.Request.URL.Path,

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.

P2 Badge Log route templates instead of concrete URL paths

For parameterized endpoints such as /api/receipts/:id, this records every concrete receipt ID rather than the stable Gin route requested by issue #162. That creates unbounded high-cardinality log values and makes aggregation by endpoint harder; use c.FullPath() after c.Next() with a safe fallback for unmatched routes.

Useful? React with 👍 / 👎.

Comment thread gateway/middleware.go
Comment on lines +362 to +365
PaymentStatus: c.GetString("payment_status"),
PaymentError: c.GetString("payment_error"),
Payer: c.GetString("payer"),
InternalError: sanitizeErrorString(c.GetString("internal_error")),

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.

P1 Badge Avoid reading Gin context while timed-out handlers run

When RequestTimeoutMiddleware returns a 504, its handler goroutine may still be running and can subsequently call respondError or otherwise mutate these Gin context keys (gateway/middleware.go:194-226). The outer JSON logger immediately reads the same context map here, creating an unsynchronized read/write race that can be reported by the race detector or panic with concurrent map access when an upstream call does not stop promptly after cancellation. Snapshot logging metadata before launching the handler, synchronize completion, or redesign the timeout path so the logger never reads c while the goroutine remains active.

Useful? React with 👍 / 👎.

Comment thread gateway/main.go
Comment on lines +945 to +947
if os.Getenv("LOG_FORMAT") != "json" {
log.Printf("Failed to retrieve receipt %s: %v", id, err)
}

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.

P2 Badge Retain receipt-store diagnostics in JSON mode

When receipt lookup fails in JSON mode, this newly added condition suppresses the only log containing the storage error, while the handler does not call respondError or attach any safe diagnostic to the Gin context. The resulting structured entry contains only status 500 and the path, leaving Redis/store outages impossible to distinguish from other failures; attach a sanitized error classification for JSONLoggerMiddleware before suppressing the legacy line.

Useful? React with 👍 / 👎.

@AnkanMisra AnkanMisra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@AnkanMisra AnkanMisra added enhancement New feature or request level:intermediate Moderate scope requiring project familiarity or cross-file changes. type:devops Deployment, observability, operations, CI, or infrastructure work. labels Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request go Pull requests that update go code level:intermediate Moderate scope requiring project familiarity or cross-file changes. type:devops Deployment, observability, operations, CI, or infrastructure work. type:testing Tests, coverage, fixtures, or validation-only work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add structured JSON request and payment logging

3 participants