Fix tx replay attack 241#249
Conversation
This prevents holding up the HTTP handler goroutine for the duration of inference. Co-authored-by: codex <codex@users.noreply.github.com>
Fixes issue AnkanMisra#241 where the same payment transaction hash could be replayed indefinitely. This adds an atomic check and set in Redis to record used transaction hashes (EIP-712 signatures) immediately after first successful verification. A TTL of 30 days is applied to the keys in Redis to match the on-chain finality + dispute window. Additionally, it implements an in-memory fallback to avoid breaking tests and adds a load test simulating concurrent replay submissions. Co-authored-by: codex <codex@users.noreply.github.com>
|
@pisum-sativum is attempting to deploy a commit to the ankanmisra's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @pisum-sativum, thanks for opening this PR. Every contribution helps MicroAI-Paygate grow. If you find the project useful, consider starring the repository — it helps others discover it. Star MicroAI-Paygate on GitHub Looking forward to reviewing this PR. |
github-actions
Bot
added
go
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds transaction replay protection via a new ChangesAnti-replay and streaming summarization
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as handleSummarize
participant TxStore as markTransactionUsed
participant Provider as AIProvider
participant Receipt as ReceiptStore
Client->>Gateway: POST /api/ai/summarize (X-402-Signature)
Gateway->>TxStore: markTransactionUsed(signature)
alt already used
TxStore-->>Gateway: true
Gateway-->>Client: 402 Transaction already used
else first use
TxStore-->>Gateway: false
Gateway->>Provider: GenerateStream(ctx, prompt)
loop chunks
Provider-->>Gateway: text chunk
Gateway-->>Client: data: {"text": ...}
end
Gateway->>Receipt: storeReceiptWithContext(result)
Gateway-->>Client: data: {"receipt": ...} / [DONE]
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@AnkanMisra I have done the necessary changes kindly check and merge with gssoc and gssoc:approved tags. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
AnkanMisra
added
bug
ChatGPT Codex Connector
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 924fd70cd5
ℹ️ 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".
| c.Writer.Header().Set("Content-Type", "text/event-stream") | ||
| c.Writer.Header().Set("Cache-Control", "no-cache") | ||
| c.Writer.Header().Set("Connection", "keep-alive") |
There was a problem hiding this comment.
Switching /api/ai/summarize to text/event-stream here changes the 200 response contract, but the repo's published SDK still parses the success body as JSON and reads the receipt from X-402-Receipt (sdk/typescript/src/client.ts:169-180, sdk/typescript/src/protocol/microai.ts:70-72), and the deployed smoke test still requires that header. Any SDK caller against the updated gateway will now fail with Gateway returned invalid JSON even though the request succeeded, so either keep the JSON/header path or update the SDK, OpenAPI, docs, and smoke tests with the new SSE protocol.
Useful? React with 👍 / 👎.
| responseMap := map[string]interface{}{ | ||
| "result": fullSummary, | ||
| } | ||
| responseBody, _ := json.Marshal(responseMap) | ||
|
|
||
| receipt, err := GenerateReceipt(*paymentCtx, verifyResp.RecoveredAddress, c.Request.URL.Path, requestBody, responseBody) |
There was a problem hiding this comment.
The receipt is generated over responseBody containing only {"result": fullSummary}, but the actual bytes returned to the client after this change are the SSE data: events, including the receipt event, not that JSON object. Receipt verification code in the SDK compares receipt.service.response_hash to a hash of the body it received (sdk/typescript/src/client.ts:153-162), so once clients decode receipts from the stream they will reject every receipt as not bound to the response. Hash the exact externally defined payload, or document and update verifiers to use a canonical response-hash input.
Useful? React with 👍 / 👎.
| const parsed = JSON.parse(dataStr); | ||
| if (parsed.text) { | ||
| summary += parsed.text; | ||
| if (onChunk) onChunk(summary); | ||
| } else if (parsed.receipt) { | ||
| receipt = safeDecodeReceiptHeader(parsed.receipt); | ||
| } |
There was a problem hiding this comment.
The gateway now emits fatal SSE events like {"error": ...} when upstream streaming or receipt storage fails, but this parser ignores anything that is not text or receipt and returns the partial summary with a null receipt; useX402 then records SummaryCompleted and shows done. In the scenarios where main.go writes those error events after a signature has been accepted, users can be charged and shown a successful result with no receipt instead of an actionable failure, so parse error events and throw or otherwise propagate them to the existing error path.
Useful? React with 👍 / 👎.
| line, err := s.reader.ReadBytes('\n') | ||
| if err != nil { | ||
| return "", err |
There was a problem hiding this comment.
For OpenRouter, normal completion is the [DONE] SSE frame, but this path returns the same io.EOF when the TCP stream ends before that frame (or when a test/custom endpoint returns a non-SSE JSON body). handleSummarize treats io.EOF as success, so a dropped upstream connection can produce a 200 response and signed receipt for an empty or partial summary instead of an upstream error; track whether [DONE] was seen and return an error for bare EOF.
Useful? React with 👍 / 👎.
| if result2 != "AI Summary Result" { | ||
| t.Errorf("Unexpected result 2: %v", result2) |
There was a problem hiding this comment.
After adding the cache-hit replay check, the second request in TestCacheIntegration_FullFlow still reuses the same 0xValidSig, so the cache-hit path returns 402 before this assertion instead of the cached summary. The Go workflow starts Redis, so this integration test will fail under go test ./...; use a fresh verifier-accepted signature/nonce for the cache-hit request or adjust the mock accordingly.
Useful? React with 👍 / 👎.
| if memoryUsedTx[txHash] { | ||
| return true, nil | ||
| } | ||
| memoryUsedTx[txHash] = true |
There was a problem hiding this comment.
When Redis is not required (RECEIPT_STORE=memory and cache disabled), every accepted signature is inserted into memoryUsedTx with no TTL or cleanup, unlike the Redis path's 30-day expiration. A long-running local/standalone gateway will leak one map entry per successful payment and reject those signatures indefinitely after the intended replay window, so store an expiry and prune entries or share the receipt cleanup mechanism.
Useful? React with 👍 / 👎.
| if err := json.Unmarshal(w2.Body.Bytes(), &resp2); err != nil { | ||
| t.Fatalf("Failed to unmarshal response 2: %v", err) | ||
| parseSSEResultForTest := func(body []byte) string { | ||
| lines := strings.Split(string(body), "\n") |
There was a problem hiding this comment.
This added SSE parser calls strings.Split/HasPrefix/TrimSpace, but strings is not imported in this test file; the same missing import exists in gateway/receipt_store_integration_test.go where the receipt stream is parsed. The gateway package will fail to compile before the replay regression can run, so add strings to both import blocks.
Useful? React with 👍 / 👎.
| key := "used_tx:" + txHash | ||
| // SetNX returns true if the key was set, meaning it wasn't used before. | ||
| added, err := redisClient.SetNX(ctx, key, "1", 30*24*time.Hour).Result() |
There was a problem hiding this comment.
The replay key is built from the raw X-402-Signature header, but the verifier parses hex signatures case-insensitively, so the same 65-byte signature can be replayed with different hex casing and get a different used_tx: key. In the verifier-restart or non-shared nonce-store scenarios this gateway check is meant to cover, that bypasses the new guard; derive the key from parsed signature bytes or a normalized lowercase canonical form.
Useful? React with 👍 / 👎.
| c.JSON(http.StatusPaymentRequired, gin.H{ | ||
| "error": "Payment Required", | ||
| "message": "Transaction already used", | ||
| "paymentContext": createPaymentContext(), | ||
| }) |
There was a problem hiding this comment.
When this gateway-side replay branch fires, it returns a fresh 402 payment challenge instead of the documented 409/nonce_already_used error that the rest of the gateway and web classifier use for duplicate submissions. In environments where this branch catches a replay after verifier nonce state was restarted or not shared, generic clients can treat it as a new payment challenge or the web UI will show an expired-context error; return the existing replay error contract instead.
Useful? React with 👍 / 👎.
AnkanMisra
left a comment
AnkanMisra
left a comment
There was a problem hiding this comment.
Fix the bugs codex found
Fix the go lint and go tests
- Remove unused bodyBytes in cache.go - Add missing strings package imports - Remove unused ctx in receipt_store_integration_test.go Co-authored-by: codex <codex@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
gateway/receipt_store_integration_test.go (1)
198-301: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRun
gofmton the new test.Several blank lines carry trailing whitespace (e.g. Lines 248, 274, 277, 291, 294) and the binary expressions on Lines 298–299 use non-canonical spacing.
gofmtwill rewrite these.♻️ Formatting fixes
- if conflictCount != numConcurrentRequests - 1 { - t.Errorf("Expected exactly %d blocked replays, got %d", numConcurrentRequests - 1, conflictCount) + if conflictCount != numConcurrentRequests-1 { + t.Errorf("Expected exactly %d blocked replays, got %d", numConcurrentRequests-1, conflictCount)(Also strip the trailing tabs on the otherwise-empty lines.)
As per coding guidelines: "Use
gofmt -w .before committing Go changes and validate withgo vet ./...".🤖 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/receipt_store_integration_test.go` around lines 198 - 301, The new test in TestHandleSummarize_ConcurrentReplayAttack needs formatting cleanup: there are trailing whitespace/blank-line tabs and non-canonical spacing in the success/conflict count checks. Run gofmt on the test so the blank lines in the request loop and goroutine section are normalized and the arithmetic expressions in the final assertions match standard Go formatting.Source: Coding guidelines
web/src/hooks/use-x402.ts (1)
114-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid shadowing the outer
textinput in theonChunkcallback.The callback parameter
textshadowssubmit(text). It's correct today only becausesaveReceipt(receipt, text)sits outside the callback, but the shadowing is a footgun and the name is misleading —onChunkis invoked with the accumulatedsummary, not a delta chunk.♻️ Suggested rename
- const { summary, receipt } = await readSummarizeSuccess(first, (text) => { - update({ summary: text }); - }); + const { summary, receipt } = await readSummarizeSuccess(first, (partialSummary) => { + update({ summary: partialSummary }); + });(Apply the same rename in the retry path at Lines 285–287.)
Also applies to: 285-287
🤖 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 `@web/src/hooks/use-x402.ts` around lines 114 - 116, The `onChunk` callback in `use-x402` is shadowing the outer `text` argument from `submit(text)`, which is misleading because the callback receives the accumulated summary, not a delta chunk. Rename the callback parameter in `readSummarizeSuccess` to something like `summary` (or similar) and keep the outer `text` value unchanged for `saveReceipt(receipt, text)`; apply the same rename in the retry path used by the same hook.tests/e2e.test.ts (1)
89-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssertion weakened to structural markers only.
The old check validated the actual summary (
data.result); the new check only confirms SSE framing ("data: {"/"[DONE]") and would pass even if the summary text were empty or malformed. Consider additionally parsing adata:line and asserting onparsed.text(and/or the receipt event) to preserve content validation.🤖 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 `@tests/e2e.test.ts` around lines 89 - 91, The e2e assertion in the test is now only checking SSE framing and no longer validates the returned summary content. Update the test around the response text handling to parse a `data:` event payload and assert on the actual parsed fields, such as `parsed.text` and/or the receipt event, in addition to the existing `[DONE]` framing check. Use the existing response parsing logic in `tests/e2e.test.ts` to keep the content verification tied to the summary result instead of only structural markers.
🤖 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/internal/ai/ollama.go`:
- Around line 101-134: `OllamaProvider.GenerateStream` duplicates the same
prompt/request setup and deadline handling already present in `Generate`, so
extract the shared request-building logic into a helper such as
`buildGenerateRequest` and have both methods call it. Keep the helper
responsible for formatting the prompt, marshaling the body, creating the HTTP
request, and preserving the context deadline error behavior, while
`GenerateStream` should only set up the streaming-specific response handling and
`"stream": true` flag.
In `@gateway/internal/ai/openrouter.go`:
- Line 4: GenerateStream in openrouter.go is duplicating the prompt formatting
and request construction already done in Generate, with the only meaningful
difference being the stream flag. Extract the shared request-building logic into
a common helper used by both Generate and GenerateStream, and have
GenerateStream only set the streaming-specific option before sending the
request. Keep the shared logic centralized around the existing Generate and
GenerateStream methods so future changes don’t diverge.
- Around line 110-146: The openRouterStream Recv parser is ignoring in-band SSE
error chunks and can fall through to io.EOF as if the stream ended successfully.
Update Recv to inspect each parsed JSON chunk for a top-level error field before
checking choices/delta, and return a non-EOF error when present so mid-stream
OpenRouter failures are surfaced instead of being treated as a normal
completion.
In `@gateway/main.go`:
- Around line 508-523: The replay-protection logic in markTransactionUsed is
marking the signature too early and can permanently burn valid payments when
later request parsing, streaming, or receipt/storage steps fail. Move the
transaction-marking step in the main handler flow to after the full successful
processing path, or ensure every failure path after the mark deletes the used_tx
key; also apply the same fix to the cache-hit branch so signature handling in
gateway/main.go only persists on success.
- Around line 549-559: The SSE stream is not actually draining buffered data
because responseWriterShim.Flush() only delegates to orig.Flush() while Write()
continues to hold chunks in bufferedWriter, so incremental events are delayed
until the handler exits. Update the streaming path in gateway/main.go so Flush()
also forwards any buffered SSE bytes to the underlying response before calling
orig.Flush(), or bypass RequestTimeoutMiddleware for these streaming routes; use
responseWriterShim, bufferedWriter, and the SSE handler around c.Writer to
locate the fix.
- Around line 549-607: The SSE summarize endpoint contract has changed, so
update the documentation to match the streaming behavior now implemented in
gateway/main.go. In gateway/openapi.yaml and README.md, replace the old
X-402-Receipt header response description with the actual event stream format:
incremental data events containing {"text":...}, followed by a final data event
containing {"receipt":...}, and then [DONE]. Use the summarize streaming handler
and its final receipt emission as the source of truth so the docs align with the
current response shape.
In `@gateway/redis.go`:
- Around line 88-104: The global redisClient is being checked for nil and then
read again later without synchronization, which can race with initRedis() and
panic on a nil receiver. Update markTransactionUsed to capture redisClient into
a local variable once at the start of the function and use that single reference
for the nil check and SetNX call, or otherwise guard redisClient access
consistently so reads and writes are synchronized.
---
Nitpick comments:
In `@gateway/receipt_store_integration_test.go`:
- Around line 198-301: The new test in
TestHandleSummarize_ConcurrentReplayAttack needs formatting cleanup: there are
trailing whitespace/blank-line tabs and non-canonical spacing in the
success/conflict count checks. Run gofmt on the test so the blank lines in the
request loop and goroutine section are normalized and the arithmetic expressions
in the final assertions match standard Go formatting.
In `@tests/e2e.test.ts`:
- Around line 89-91: The e2e assertion in the test is now only checking SSE
framing and no longer validates the returned summary content. Update the test
around the response text handling to parse a `data:` event payload and assert on
the actual parsed fields, such as `parsed.text` and/or the receipt event, in
addition to the existing `[DONE]` framing check. Use the existing response
parsing logic in `tests/e2e.test.ts` to keep the content verification tied to
the summary result instead of only structural markers.
In `@web/src/hooks/use-x402.ts`:
- Around line 114-116: The `onChunk` callback in `use-x402` is shadowing the
outer `text` argument from `submit(text)`, which is misleading because the
callback receives the accumulated summary, not a delta chunk. Rename the
callback parameter in `readSummarizeSuccess` to something like `summary` (or
similar) and keep the outer `text` value unchanged for `saveReceipt(receipt,
text)`; apply the same rename in the retry path used by the same hook.
🪄 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: 08cfeff1-61a1-4e2e-9cb7-0c5f517f4b32
📒 Files selected for processing (13)
gateway/cache.gogateway/cache_integration_test.gogateway/errors_test.gogateway/internal/ai/ollama.gogateway/internal/ai/openrouter.gogateway/internal/ai/provider.gogateway/main.gogateway/receipt_store_integration_test.gogateway/redis.gogateway/timeout_test.gotests/e2e.test.tsweb/src/hooks/use-x402.tsweb/src/lib/x402-client.ts
| func (p *OllamaProvider) GenerateStream(ctx context.Context, text string) (Stream, error) { | ||
| prompt := fmt.Sprintf("Summarize this text in 2 sentences: %s", text) | ||
|
|
||
| reqBody, _ := json.Marshal(map[string]interface{}{ | ||
| "model": p.model, | ||
| "prompt": prompt, | ||
| "stream": true, | ||
| }) | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, "POST", p.url+"/api/generate", bytes.NewBuffer(reqBody)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create Ollama request: %w", err) | ||
| } | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| if errors.Is(err, context.DeadlineExceeded) || ctx.Err() == context.DeadlineExceeded { | ||
| return nil, context.DeadlineExceeded | ||
| } | ||
| return nil, fmt.Errorf("failed to connect to Ollama: %w", err) | ||
| } | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| body, _ := io.ReadAll(resp.Body) | ||
| resp.Body.Close() | ||
| return nil, fmt.Errorf("ollama returned status %d: %s", resp.StatusCode, string(body)) | ||
| } | ||
|
|
||
| return &ollamaStream{ | ||
| resp: resp, | ||
| decoder: json.NewDecoder(resp.Body), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate request-building logic between Generate and GenerateStream.
The prompt formatting, request construction, and deadline-error handling in GenerateStream (Lines 101-122) is nearly identical to Generate (Lines 39-60), differing only in the "stream" flag and response handling. This duplication makes the two paths prone to diverging (see the related sibling comment on openrouter.go, where the streaming and non-streaming paths already diverge in error handling). Consider extracting the shared request-building step into a helper, e.g. buildGenerateRequest(ctx, url, model, prompt, stream bool) (*http.Request, error).
🤖 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/internal/ai/ollama.go` around lines 101 - 134,
`OllamaProvider.GenerateStream` duplicates the same prompt/request setup and
deadline handling already present in `Generate`, so extract the shared
request-building logic into a helper such as `buildGenerateRequest` and have
both methods call it. Keep the helper responsible for formatting the prompt,
marshaling the body, creating the HTTP request, and preserving the context
deadline error behavior, while `GenerateStream` should only set up the
streaming-specific response handling and `"stream": true` flag.
| package ai | ||
|
|
||
| import ( | ||
| "bufio" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
GenerateStream duplicates Generate's request-building logic.
Same DRY concern as in ollama.go: prompt formatting and request construction (Lines 153-168) are copy-pasted from Generate (Lines 45-59), differing only by the "stream": true field. Recommend extracting a shared request builder to reduce future divergence risk — see the adjacent Recv() finding, which is a direct consequence of the streaming and non-streaming paths evolving independently.
Also applies to: 152-188
🤖 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/internal/ai/openrouter.go` at line 4, GenerateStream in openrouter.go
is duplicating the prompt formatting and request construction already done in
Generate, with the only meaningful difference being the stream flag. Extract the
shared request-building logic into a common helper used by both Generate and
GenerateStream, and have GenerateStream only set the streaming-specific option
before sending the request. Keep the shared logic centralized around the
existing Generate and GenerateStream methods so future changes don’t diverge.
| func (s *openRouterStream) Recv() (string, error) { | ||
| for { | ||
| line, err := s.reader.ReadBytes('\n') | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| line = bytes.TrimSpace(line) | ||
| if len(line) == 0 { | ||
| continue | ||
| } | ||
| if bytes.HasPrefix(line, []byte("data: ")) { | ||
| data := bytes.TrimPrefix(line, []byte("data: ")) | ||
| if string(data) == "[DONE]" { | ||
| return "", io.EOF | ||
| } | ||
| var chunk map[string]interface{} | ||
| if err := json.Unmarshal(data, &chunk); err != nil { | ||
| continue // skip invalid json | ||
| } | ||
| choices, ok := chunk["choices"].([]interface{}) | ||
| if !ok || len(choices) == 0 { | ||
| continue | ||
| } | ||
| choice, ok := choices[0].(map[string]interface{}) | ||
| if !ok { | ||
| continue | ||
| } | ||
| delta, ok := choice["delta"].(map[string]interface{}) | ||
| if !ok { | ||
| continue | ||
| } | ||
| if content, ok := delta["content"].(string); ok { | ||
| return content, nil | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
ast-grep outline gateway/internal/ai/openrouter.go --view expanded
ast-grep outline gateway/main.go --view expanded
# Read the relevant sections with line numbers
sed -n '1,240p' gateway/internal/ai/openrouter.go | cat -n
printf '\n--- main.go ---\n'
sed -n '1,260p' gateway/main.go | cat -n
# Look for stream/error handling and transaction usage sites
rg -n "markTransactionUsed|GenerateStream|Recv\(\)|io\.EOF|openrouter" gateway -SRepository: AnkanMisra/MicroAI-Paygate
Length of output: 23374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the stream consumption path in main.go
sed -n '520,590p' gateway/main.go | cat -n
# Show the Ollama stream parser for comparison
sed -n '70,120p' gateway/internal/ai/ollama.go | cat -n
# Show any tests covering streaming error behavior
sed -n '1,220p' gateway/errors_test.go | cat -n
sed -n '1,180p' gateway/timeout_test.go | cat -nRepository: AnkanMisra/MicroAI-Paygate
Length of output: 19102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '520,590p' gateway/main.go | cat -n
printf '\n--- ollama.go ---\n'
sed -n '70,120p' gateway/internal/ai/ollama.go | cat -n
printf '\n--- errors_test.go ---\n'
sed -n '1,220p' gateway/errors_test.go | cat -n
printf '\n--- timeout_test.go ---\n'
sed -n '1,180p' gateway/timeout_test.go | cat -nRepository: AnkanMisra/MicroAI-Paygate
Length of output: 19170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the stream consumption path in main.go
sed -n '520,590p' gateway/main.go | cat -n
# Show the Ollama stream parser for comparison
printf '\n--- ollama.go ---\n'
sed -n '70,120p' gateway/internal/ai/ollama.go | cat -n
# Show tests that may cover stream error behavior
printf '\n--- errors_test.go ---\n'
sed -n '1,220p' gateway/errors_test.go | cat -n
printf '\n--- timeout_test.go ---\n'
sed -n '1,180p' gateway/timeout_test.go | cat -nRepository: AnkanMisra/MicroAI-Paygate
Length of output: 19170
🌐 Web query:
OpenRouter streaming error SSE top-level error field chat.completion.chunk mid-stream error
💡 Result:
When a mid-stream error occurs during an OpenRouter Server-Sent Events (SSE) stream, the API communicates the error by sending a specific JSON object within the stream [1][2]. Because the HTTP status code is already set to 200 OK once streaming begins, the error cannot be signaled via standard HTTP error codes [1][3]. Instead, OpenRouter sends a final SSE chunk with a top-level error field that follows this structure [2][3]: { "id": "cmpl-abc123", "object": "chat.completion.chunk", "created": 1234567890, "model": "...", "provider": "...", "error": { "code": "server_error", "message": "Provider disconnected unexpectedly" }, "choices": [{ "index": 0, "delta": { "content": "" }, "finish_reason": "error" }] } Key behaviors include: - Termination: The stream is terminated immediately after this event is sent [1][2]. - Error Handling: Client implementations should monitor every chunk for the presence of an error field [1]. If found, the application should cease processing the stream and handle the error based on the code and message provided [1][4]. - Distinguishing Network Issues: Be aware that not all mid-stream interruptions are signaled by this JSON event [5]. If the server-side process or network connection terminates abruptly (e.g., a TLS socket closure), you may receive a low-level network error (like a "terminated" TypeError) rather than a clean JSON-formatted error chunk [5]. In such cases, there will be no response body or HTTP status code, necessitating robust retry logic for network-level failures [5].
Citations:
- 1: https://openrouter.ai/docs/api/reference/streaming
- 2: https://openrouter.ai/docs/api/reference/errors-and-debugging.mdx
- 3: https://openrouter.ai/docs/api/reference/streaming.mdx
- 4: https://openrouter.ai/docs/api/reference/streaming.md
- 5: TypeError: terminated — TLS socket closes mid-stream during SSE response OpenRouterTeam/ai-sdk-provider#412
Surface OpenRouter SSE errors instead of treating EOF as success
OpenRouter can emit mid-stream failures as an in-band chunk with a top-level error object. This parser only returns when delta.content is present, so those chunks are skipped and a close immediately after can still fall through to io.EOF as a successful end of stream. Return an error when chunk["error"] is present so truncated generations don’t look successful after the transaction is already marked used.
🤖 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/internal/ai/openrouter.go` around lines 110 - 146, The
openRouterStream Recv parser is ignoring in-band SSE error chunks and can fall
through to io.EOF as if the stream ended successfully. Update Recv to inspect
each parsed JSON chunk for a top-level error field before checking
choices/delta, and return a non-EOF error when present so mid-stream OpenRouter
failures are surfaced instead of being treated as a normal completion.
| // === Fix for #241: Prevent replay attacks === | ||
| used, err := markTransactionUsed(c.Request.Context(), signature) | ||
| if err != nil { | ||
| respondError(c, 500, "internal_error", fmt.Errorf("failed to check transaction replay: %w", err)) | ||
| return | ||
| } | ||
| if used { | ||
| c.JSON(http.StatusPaymentRequired, gin.H{ | ||
| "error": "Payment Required", | ||
| "message": "Transaction already used", | ||
| "paymentContext": createPaymentContext(), | ||
| }) | ||
| return | ||
| } | ||
| // ============================================ | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any compensating unmark / delete of used-tx keys or memoryUsedTx
rg -nP 'used_tx|memoryUsedTx|markTransactionUsed|unmark|Del\(' gateway/Repository: AnkanMisra/MicroAI-Paygate
Length of output: 1319
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- gateway/main.go (490-610) ---'
sed -n '490,610p' gateway/main.go | cat -n
echo
echo '--- gateway/cache.go (140-230) ---'
sed -n '140,230p' gateway/cache.go | cat -n
echo
echo '--- gateway/redis.go (70-130) ---'
sed -n '70,130p' gateway/redis.go | cat -n
echo
echo '--- rollback/delete search ---'
rg -n 'used_tx:|markTransactionUsed|unmark|delete.*used|Del\(' gateway/*.go gateway/**/*.goRepository: AnkanMisra/MicroAI-Paygate
Length of output: 10339
Marking the signature this early burns valid payments on later failures.
markTransactionUsed runs before request parsing, AI streaming, and receipt generation/storage, so any 400/502/504/stream error after this point leaves used_tx:<sig> set for 30 days and blocks a legitimate retry with 402 Transaction already used. The cache-hit path has the same issue. Move the mark to the end of the successful flow, or delete the key on every failure after marking.
🤖 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 508 - 523, The replay-protection logic in
markTransactionUsed is marking the signature too early and can permanently burn
valid payments when later request parsing, streaming, or receipt/storage steps
fail. Move the transaction-marking step in the main handler flow to after the
full successful processing path, or ensure every failure path after the mark
deletes the used_tx key; also apply the same fix to the cache-hit branch so
signature handling in gateway/main.go only persists on success.
| defer stream.Close() | ||
|
|
||
| // 4. Generate & Send Receipt | ||
| if err := generateAndSendReceipt(c, *paymentCtx, verifyResp.RecoveredAddress, requestBody, summary); err != nil { | ||
| log.Printf("Failed to generate receipt: %v", err) | ||
| // generateAndSendReceipt sends error response if it fails? | ||
| // No, it returns error, we might have already written status if we aren't careful. | ||
| // Let's implement generateAndSendReceipt to handle sending response. | ||
| c.Writer.Header().Set("Content-Type", "text/event-stream") | ||
| c.Writer.Header().Set("Cache-Control", "no-cache") | ||
| c.Writer.Header().Set("Connection", "keep-alive") | ||
|
|
||
| flusher, ok := c.Writer.(http.Flusher) | ||
| if !ok { | ||
| respondError(c, 500, "streaming_unsupported", fmt.Errorf("streaming unsupported")) | ||
| return | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline gateway/middleware.go --items all
rg -nP -C3 'bufferedWriter|responseWriterShim|Flush|WriteHeader|func .*Write\(' gateway/middleware.goRepository: AnkanMisra/MicroAI-Paygate
Length of output: 8020
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the timeout middleware and the SSE handler around the reported lines.
sed -n '155,265p' gateway/middleware.go
printf '\n---- handleSummarize ----\n'
sed -n '520,590p' gateway/main.goRepository: AnkanMisra/MicroAI-Paygate
Length of output: 6680
Make Flush() drain the buffered response for SSE
responseWriterShim.Flush() only calls orig.Flush(), while Write() keeps SSE chunks in bufferedWriter. Under RequestTimeoutMiddleware, clients won’t receive incremental events until the handler returns. Bypass this middleware for streaming routes or forward buffered bytes on each flush.
🤖 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 549 - 559, The SSE stream is not actually
draining buffered data because responseWriterShim.Flush() only delegates to
orig.Flush() while Write() continues to hold chunks in bufferedWriter, so
incremental events are delayed until the handler exits. Update the streaming
path in gateway/main.go so Flush() also forwards any buffered SSE bytes to the
underlying response before calling orig.Flush(), or bypass
RequestTimeoutMiddleware for these streaming routes; use responseWriterShim,
bufferedWriter, and the SSE handler around c.Writer to locate the fix.
| defer stream.Close() | ||
|
|
||
| // 4. Generate & Send Receipt | ||
| if err := generateAndSendReceipt(c, *paymentCtx, verifyResp.RecoveredAddress, requestBody, summary); err != nil { | ||
| log.Printf("Failed to generate receipt: %v", err) | ||
| // generateAndSendReceipt sends error response if it fails? | ||
| // No, it returns error, we might have already written status if we aren't careful. | ||
| // Let's implement generateAndSendReceipt to handle sending response. | ||
| c.Writer.Header().Set("Content-Type", "text/event-stream") | ||
| c.Writer.Header().Set("Cache-Control", "no-cache") | ||
| c.Writer.Header().Set("Connection", "keep-alive") | ||
|
|
||
| flusher, ok := c.Writer.(http.Flusher) | ||
| if !ok { | ||
| respondError(c, 500, "streaming_unsupported", fmt.Errorf("streaming unsupported")) | ||
| return | ||
| } | ||
|
|
||
| var fullSummary string | ||
| for { | ||
| text, err := stream.Recv() | ||
| if errors.Is(err, io.EOF) { | ||
| break | ||
| } | ||
| if err != nil { | ||
| fmt.Fprintf(c.Writer, "data: {\"error\": %q}\n\n", err.Error()) | ||
| flusher.Flush() | ||
| return | ||
| } | ||
|
|
||
| fullSummary += text | ||
| chunkBytes, _ := json.Marshal(map[string]string{"text": text}) | ||
| fmt.Fprintf(c.Writer, "data: %s\n\n", chunkBytes) | ||
| flusher.Flush() | ||
| } | ||
|
|
||
| // 4. Generate Receipt | ||
| responseMap := map[string]interface{}{ | ||
| "result": fullSummary, | ||
| } | ||
| responseBody, _ := json.Marshal(responseMap) | ||
|
|
||
| receipt, err := GenerateReceipt(*paymentCtx, verifyResp.RecoveredAddress, c.Request.URL.Path, requestBody, responseBody) | ||
| if err != nil { | ||
| fmt.Fprintf(c.Writer, "data: {\"error\": \"receipt_generation_failed\"}\n\n") | ||
| flusher.Flush() | ||
| return | ||
| } | ||
|
|
||
| if err := storeReceiptWithContext(c.Request.Context(), receipt, getReceiptTTL()); err != nil { | ||
| fmt.Fprintf(c.Writer, "data: {\"error\": \"receipt_store_failed\"}\n\n") | ||
| flusher.Flush() | ||
| return | ||
| } | ||
|
|
||
| receiptJSON, _ := json.Marshal(receipt) | ||
| receiptBase64 := base64.StdEncoding.EncodeToString(receiptJSON) | ||
|
|
||
| // Send receipt as final event | ||
| fmt.Fprintf(c.Writer, "data: {\"receipt\": %q}\n\n", receiptBase64) | ||
| fmt.Fprintf(c.Writer, "data: [DONE]\n\n") | ||
| flusher.Flush() | ||
|
|
||
| // Store for cache middleware | ||
| c.Set("full_summary", fullSummary) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'text/event-stream|X-402-Receipt|\[DONE\]|full_summary' gateway/openapi.yaml README.md 2>/dev/null
fd -e yaml openapiRepository: AnkanMisra/MicroAI-Paygate
Length of output: 1189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== gateway tests mentioning summarize/receipt ==\n'
rg -n '/api/ai/summarize|X-402-Receipt|text/event-stream|\[DONE\]|receipt' gateway --glob '*_test.go' --glob '*.go'
printf '\n== web client mentions of summarize/receipt ==\n'
rg -n '/api/ai/summarize|X-402-Receipt|text/event-stream|\[DONE\]|receipt' web --glob '*.{ts,tsx,js,jsx}'
printf '\n== openapi snippet around summarize ==\n'
sed -n '80,120p' gateway/openapi.yaml
printf '\n== README snippet around summarize ==\n'
sed -n '175,210p' README.mdRepository: AnkanMisra/MicroAI-Paygate
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== gateway openapi summarize section ==\n'
sed -n '92,112p' gateway/openapi.yaml
printf '\n== README summarize section ==\n'
sed -n '180,205p' README.md
printf '\n== web x402 client stream parsing ==\n'
sed -n '1,140p' web/src/lib/x402-client.ts
printf '\n== gateway summarize stream tests ==\n'
sed -n '1,220p' gateway/receipt_store_integration_test.go
printf '\n---\n'
sed -n '180,230p' gateway/cache_integration_test.goRepository: AnkanMisra/MicroAI-Paygate
Length of output: 14343
Update the docs for the SSE summarize response. gateway/openapi.yaml and README.md still describe the old X-402-Receipt header contract; this endpoint now streams data: {"text":...} chunks plus a final data: {"receipt":...} and [DONE].
🤖 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 549 - 607, The SSE summarize endpoint contract
has changed, so update the documentation to match the streaming behavior now
implemented in gateway/main.go. In gateway/openapi.yaml and README.md, replace
the old X-402-Receipt header response description with the actual event stream
format: incremental data events containing {"text":...}, followed by a final
data event containing {"receipt":...}, and then [DONE]. Use the summarize
streaming handler and its final receipt emission as the source of truth so the
docs align with the current response shape.
Source: Coding guidelines
| func markTransactionUsed(ctx context.Context, txHash string) (bool, error) { | ||
| if redisClient == nil { | ||
| memoryUsedTxMu.Lock() | ||
| defer memoryUsedTxMu.Unlock() | ||
| if memoryUsedTx[txHash] { | ||
| return true, nil | ||
| } | ||
| memoryUsedTx[txHash] = true | ||
| return false, nil | ||
| } | ||
| key := "used_tx:" + txHash | ||
| // SetNX returns true if the key was set, meaning it wasn't used before. | ||
| added, err := redisClient.SetNX(ctx, key, "1", 30*24*time.Hour).Result() | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| return !added, nil |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unsynchronized re-read of global redisClient risks a nil-pointer panic.
redisClient is checked for nil and then read again on Line 100 without holding any lock. If initRedis() runs concurrently (e.g., reconnect/reinit path, or in tests that call initRedis() while requests are in flight) and sets redisClient = nil between these two reads, redisClient.SetNX(...) will panic on a nil receiver. This is also an unsynchronized data race on redisClient as read here vs. written in initRedis().
🔒 Suggested fix: capture the client once
func markTransactionUsed(ctx context.Context, txHash string) (bool, error) {
- if redisClient == nil {
+ client := redisClient
+ if client == nil {
memoryUsedTxMu.Lock()
defer memoryUsedTxMu.Unlock()
if memoryUsedTx[txHash] {
return true, nil
}
memoryUsedTx[txHash] = true
return false, nil
}
key := "used_tx:" + txHash
// SetNX returns true if the key was set, meaning it wasn't used before.
- added, err := redisClient.SetNX(ctx, key, "1", 30*24*time.Hour).Result()
+ added, err := client.SetNX(ctx, key, "1", 30*24*time.Hour).Result()
if err != nil {
return false, err
}
return !added, nil
}This only closes the read-side race for this function; if redisClient mutation from initRedis() is possible during live traffic elsewhere in the codebase, consider guarding it with a sync.RWMutex or atomic.Pointer.
📝 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.
| func markTransactionUsed(ctx context.Context, txHash string) (bool, error) { | |
| if redisClient == nil { | |
| memoryUsedTxMu.Lock() | |
| defer memoryUsedTxMu.Unlock() | |
| if memoryUsedTx[txHash] { | |
| return true, nil | |
| } | |
| memoryUsedTx[txHash] = true | |
| return false, nil | |
| } | |
| key := "used_tx:" + txHash | |
| // SetNX returns true if the key was set, meaning it wasn't used before. | |
| added, err := redisClient.SetNX(ctx, key, "1", 30*24*time.Hour).Result() | |
| if err != nil { | |
| return false, err | |
| } | |
| return !added, nil | |
| func markTransactionUsed(ctx context.Context, txHash string) (bool, error) { | |
| client := redisClient | |
| if client == nil { | |
| memoryUsedTxMu.Lock() | |
| defer memoryUsedTxMu.Unlock() | |
| if memoryUsedTx[txHash] { | |
| return true, nil | |
| } | |
| memoryUsedTx[txHash] = true | |
| return false, nil | |
| } | |
| key := "used_tx:" + txHash | |
| // SetNX returns true if the key was set, meaning it wasn't used before. | |
| added, err := client.SetNX(ctx, key, "1", 30*24*time.Hour).Result() | |
| if err != nil { | |
| return false, err | |
| } | |
| return !added, nil | |
| } |
🤖 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/redis.go` around lines 88 - 104, The global redisClient is being
checked for nil and then read again later without synchronization, which can
race with initRedis() and panic on a nil receiver. Update markTransactionUsed to
capture redisClient into a local variable once at the start of the function and
use that single reference for the nil check and SetNX call, or otherwise guard
redisClient access consistently so reads and writes are synchronized.
Source: Coding guidelines
- gateway/internal/ai/openrouter.go: add ErrIncompleteStream sentinel and
track sawDone so bare EOF (dropped TCP) is distinguished from a clean
[DONE] termination; prevents partial/empty summaries getting receipts
- gateway/redis.go: replace no-TTL bool map with expiry-time map in
memoryUsedTx; entries now age out after 30 days matching Redis path;
add on-call pruning to prevent unbounded growth on standalone instances;
normalize signatures to lowercase before keying (hex-casing bypass fix)
- gateway/main.go: canonicalize X-402-Signature to lowercase before
markTransactionUsed; change gateway-side replay response from 402
Payment Required to 409 Conflict / nonce_already_used matching the
verifier and web client contract; treat ErrIncompleteStream as upstream
error; guard against empty-summary receipt issuance
- gateway/cache.go: same signature canonicalization and 409 replay fix
in CacheMiddleware; add missing strings import
- gateway/cache_integration_test.go: use 0xValidSig2 for cache-hit
request so replay guard does not block it; update mock verifier to
accept both signatures
- gateway/receipt_store_integration_test.go: update concurrent-replay
test to assert 409 Conflict / nonce_already_used instead of 402
- sdk/typescript/src/protocol/microai.ts: add extractReceiptFromSseBody
and readSseSuccessBody helpers that parse the SSE event stream; keep
readReceipt as a no-op stub for interface compatibility
- sdk/typescript/src/client.ts: replace JSON-body readSuccess with SSE-
aware implementation using readSseSuccessBody; fix response_hash binding
to hash canonical JSON.stringify({result}) matching gateway GenerateReceipt;
propagate stream error events as PaygateSdkError instead of silently
ignoring them
Co-authored-by: codex <codex@users.noreply.github.com>
|
@AnkanMisra I have done the necessary changes kindly check and merge |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b875dcbc68
ℹ️ 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".
| fullSummary += text | ||
| chunkBytes, _ := json.Marshal(map[string]string{"text": text}) | ||
| fmt.Fprintf(c.Writer, "data: %s\n\n", chunkBytes) | ||
| flusher.Flush() |
There was a problem hiding this comment.
When this endpoint runs through registerAPIRoutes, /api/ai is wrapped by RequestTimeoutMiddleware (gateway/routes.go:48), whose responseWriterShim.Write buffers bytes and whose Flush only flushes the original writer without copying the buffer (gateway/middleware.go:238-252). In that context each flusher.Flush() here sends no SSE data to the client until flushTo runs after the handler completes, so the new web onChunk path never receives progressive summary chunks. Exempt SSE responses from the buffering timeout writer or make Flush drain buffered bytes before flushing.
Useful? React with 👍 / 👎.
| if (parsed.text) { | ||
| summary += parsed.text; | ||
| if (onChunk) onChunk(summary); | ||
| } else if (parsed.receipt) { | ||
| receipt = safeDecodeReceiptHeader(parsed.receipt); |
There was a problem hiding this comment.
If the gateway emits a stream error after the HTTP 200 has started, e.g. receipt_store_failed, receipt_generation_failed, upstream_empty_response, or an upstream stream error from handleSummarize, the event is encoded as data: {"error": ...} but this parser ignores it because it only handles text and receipt. In that scenario useX402 proceeds to SummaryCompleted/done with a partial or empty summary and no receipt instead of showing the failure, so users can think a paid request succeeded when the gateway explicitly reported failure. Throw or otherwise propagate parsed.error from this loop.
Useful? React with 👍 / 👎.
| } | ||
| defer stream.Close() | ||
|
|
||
| c.Writer.Header().Set("Content-Type", "text/event-stream") |
There was a problem hiding this comment.
This changes successful summarize responses to text/event-stream with the receipt embedded as a final SSE event, but the public contract still documents JSON plus an X-402-Receipt header (gateway/openapi.yaml 200 response and the README API section still say that). Clients generated from the OpenAPI spec, or users following the README/SDK README, will continue to call res.json() and read the header and will fail against the new response shape. Update the OpenAPI response content/receipt description and the README examples alongside this behavior change.
Useful? React with 👍 / 👎.
|
|
||
| // Build the canonical response body that the gateway hashes for the receipt. | ||
| // GenerateReceipt in the gateway uses: json.Marshal({"result": fullSummary}) | ||
| const canonicalResponseBody = JSON.stringify({ result: fullText }); |
There was a problem hiding this comment.
When a summary contains &, <, or >, this reconstructed body does not match the bytes the gateway hashes at gateway/main.go:598: Go's json.Marshal HTML-escapes those characters (for example & becomes \u0026), while JSON.stringify leaves them literal. The SDK will therefore reject otherwise valid receipts from common outputs such as R&D or HTML snippets with receipt_verification_failed; build the canonical body with the same escaping rules or have the gateway send the exact hash input.
Useful? React with 👍 / 👎.
| // The gateway streams the response as SSE events. Consume the stream, | ||
| // accumulate text chunks, extract the embedded receipt event, and derive | ||
| // the canonical response body string for receipt hash verification. | ||
| const { fullText, receipt, canonicalResponseBody } = await readSseSuccessBody(response); |
There was a problem hiding this comment.
The SDK CI workflow runs bun run test, but the existing client-flow success fixtures still return JSON bodies with an X-402-Receipt header. With this unconditional SSE parser, those mocked 200 responses contain no data: lines, so fullText becomes empty and receipt stays null, breaking tests that expect the summarized text and verified receipt. Update the fixtures to emit the new SSE body shape, or keep a JSON/header fallback until the tests and any supported gateway versions are migrated.
Useful? React with 👍 / 👎.
| receiptHeader := createResp.Header().Get("X-402-Receipt") | ||
| if receiptHeader == "" { | ||
| t.Fatal("missing X-402-Receipt header") | ||
| var receiptBase64 string |
There was a problem hiding this comment.
This test now looks for a receipt SSE event, but its mock OpenRouter server above still returns the old non-stream JSON shape. OpenRouterProvider.GenerateStream only consumes data: SSE chunks and requires [DONE], so the handler will emit an error event and never produce the receipt this parser is waiting for, causing go test ./... to fail once dependencies are available. Change the mocked AI response to a streaming OpenRouter fixture with content deltas and a [DONE] event.
Useful? React with 👍 / 👎.
| if exp, exists := memoryUsedTx[key]; exists && now.Before(exp) { | ||
| return true, nil | ||
| } | ||
| memoryUsedTx[key] = now.Add(memoryUsedTxTTL) |
There was a problem hiding this comment.
When redisClient == nil, a successful request stores the signature in this package-level map for 30 days and no test cleanup resets it. Several existing gateway tests reuse signedSummarizeRequest's fixed 0xsigned header; after one valid-verifier test records it, a later valid-verifier test is rejected here with 409 nonce_already_used before reaching the provider path it is asserting. Clear memoryUsedTx in test cleanup or use unique signatures per test.
Useful? React with 👍 / 👎.
| if err := s.decoder.Decode(&chunk); err != nil { | ||
| return "", err // will return io.EOF when stream ends |
There was a problem hiding this comment.
The Ollama streaming API has a final done: true record, but this implementation treats any decoder EOF as clean completion without tracking that sentinel. If the local Ollama connection closes after emitting some chunks but before the final done record, handleSummarize will treat the partial text as complete and sign/store a receipt for it; add a done flag like the OpenRouter stream and return an incomplete-stream error on bare EOF.
Useful? React with 👍 / 👎.
| c.JSON(http.StatusConflict, gin.H{ | ||
| "error": "nonce_already_used", | ||
| "message": "Transaction already used", | ||
| }) |
There was a problem hiding this comment.
The changed Go sources are not gofmt-formatted (gofmt -l reports gateway/main.go, gateway/cache_integration_test.go, and gateway/receipt_store_integration_test.go); this misaligned gin.H literal is one instance. The repo instructions require gofmt -w before committing Go changes, and leaving it unformatted creates unnecessary follow-up churn. Run gofmt over the touched Go files.
Useful? React with 👍 / 👎.
2 participants
Summary
markTransactionUsedcheck using RedisSetNXto record used transaction hashes immediately upon successful payment verification.sync.Mutexmap fallback for environments without Redis and during testing.TestHandleSummarize_ConcurrentReplayAttackto ensure atomicity and verify concurrent replay attempts are correctly blocked.Type Of Change
Affected Areas
gateway/)verifier/)web/)tests/,run_e2e.sh)bench/)deploy/, Docker, env, workflows)Contributor Checklist
Verification
List the exact commands you ran and their result.
Notes For Reviewers
redisClient == nil(e.g., Redis is disabled or when testing), the replay protection falls back to a thread-safe in-memory map. This guarantees atomic replay protection is preserved even during local development without a Redis instance.gateway/cache.gobefore responding with cached AI responses. This ensures attackers cannot maliciously bypass payment limits by requesting previously cached prompts with replayed signatures.