feat: enforce request-bound payment authorization v2#313
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR makes authorization v2 request-bound by adding audience, method, resource, content type, body hash, and payer validation. It enforces minimum authorization version 2 by default, propagates binding data into receipts, updates SDK/web validation, and adds deterministic E2E and deployment coverage. ChangesRequest-bound authorization
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant Verifier
participant ReceiptStore
Client->>Gateway: request without payment headers
Gateway-->>Client: 402 challenge with v2 payment context
Client->>Client: sign PaymentAuthorization v2
Client->>Gateway: retry with signature and X-402-Payer
Gateway->>Gateway: reconstruct request binding
Gateway->>Verifier: verify payer, signature, and authorization version
Verifier-->>Gateway: valid or mapped rejection
Gateway->>ReceiptStore: generate receipt with binding metadata
Gateway-->>Client: response and receipt
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
documentation
AnkanMisra
added
type:feature
Co-authored-by: codex <codex@users.noreply.github.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
![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: 4b5abb0d44
ℹ️ 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".
Co-authored-by: codex <codex@users.noreply.github.com>
|
@codex review Please perform a deep review of the latest commit. Inspect every changed file and trace the payment authorization flow across gateway, verifier, web, SDK, E2E tests, receipts, cache behavior, configuration, deployment files, and documentation. Focus on correctness, security, replay resistance, request binding, cross-service contract parity, failure paths, compatibility, CI coverage, and missing regression tests. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
MicroAI-Paygate/tests/e2e.test.ts
Lines 101 to 105 in 65d3b33
Require deterministic E2E to hit the mock provider
In the new deterministic CI path where run_e2e.sh starts the OpenRouter mock when no key is set, the signed happy path should require HTTP 200. Keeping this upstream_unavailable escape means CI still passes if the mock never starts or OPENROUTER_URL wiring breaks, so the new E2E no longer proves the loopback provider path works; remove this fallback or gate it behind an explicit live-provider mode.
ℹ️ 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".
Co-authored-by: codex <codex@users.noreply.github.com>
|
Addressed the remaining deterministic E2E finding in 2a6b409. The signed happy path no longer accepts upstream_unavailable and must return HTTP 200 with a result, so CI now proves the loopback mock starts and is wired correctly. The full deterministic E2E suite passes all four cases. |
|
@codex review Please perform another deep review of the latest commit. Verify the request-bound payment flow, audience validation, deterministic E2E guarantees, deployed smoke test, cross-service contract parity, failure paths, and regression coverage. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a6b4096f2
ℹ️ 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".
Co-authored-by: codex <codex@users.noreply.github.com>
|
@codex review Please perform another deep review of the latest commit. Verify IDNA audience canonicalization, port validation, pre-sign smoke-test binding checks, CORS validation, deterministic provider pinning, request-bound payment behavior, and regression coverage. |
Co-authored-by: codex <codex@users.noreply.github.com>
|
@codex review Please perform another deep review of the latest commit. Verify the OpenRouter mock PID and readiness handling, deterministic E2E behavior, audience canonicalization, deployed smoke-test request binding and CORS validation, and the request-bound payment flow end to end. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21363a4b7f
ℹ️ 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".
Co-authored-by: codex <codex@users.noreply.github.com>
|
@codex review Please perform another deep review of the latest commit. Verify the deployed signed-retry preflight validation, OpenRouter mock readiness handling, deterministic E2E behavior, audience canonicalization, request binding, CORS behavior, and the payment authorization flow end to end. |
|
|
||
| const checks = [ | ||
| ["signed retry preflight succeeds", response.ok, `HTTP ${response.status}`], | ||
| ["preflight allows deployed web origin", allowOrigin === ORIGIN, allowOrigin ?? "missing"], |
There was a problem hiding this comment.
🟡 Medium scripts/deployed-smoke-test.ts:174
validateSignedRetryPreflight fails the preflight check when the gateway returns Access-Control-Allow-Origin: *, even though * is a valid CORS response for non-credentialed requests and matches the repo's documented ALLOWED_ORIGINS=* configuration. The test asserts allowOrigin === ORIGIN and exits early on the failure, skipping the rest of the smoke test. The same exact-match comparison on line 204 also rejects *. Accept * in addition to the exact origin in both checks.
- ["preflight allows deployed web origin", allowOrigin === ORIGIN, allowOrigin ?? "missing"],
+ ["preflight allows deployed web origin", allowOrigin === ORIGIN || allowOrigin === "*", allowOrigin ?? "missing"],🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @web/scripts/deployed-smoke-test.ts around line 174:
`validateSignedRetryPreflight` fails the preflight check when the gateway returns `Access-Control-Allow-Origin: *`, even though `*` is a valid CORS response for non-credentialed requests and matches the repo's documented `ALLOWED_ORIGINS=*` configuration. The test asserts `allowOrigin === ORIGIN` and exits early on the failure, skipping the rest of the smoke test. The same exact-match comparison on line 204 also rejects `*`. Accept `*` in addition to the exact origin in both checks.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gateway/cache_integration_test.go (1)
106-121: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAssert request-body binding on cache hits.
The mock only checks
Signature, so this test would still pass if Line 125 ingateway/cache.gostopped forwarding the body. Capture the verifier request and assert itsRequestHash, ideally using two raw JSON bodies with the same cache-key text but different bytes.As per coding guidelines, “Gateway handlers, config parsing, middleware, timeout behavior, receipts, cache/rate limits, and error paths must have table tests or targeted regression tests.”
🤖 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/cache_integration_test.go` around lines 106 - 121, The cache integration test’s mock verifier does not validate that request bodies are forwarded on cache hits. Update the test around the makeRequest helper and verifier mock to capture the verifier request and assert its RequestHash, exercising two raw JSON bodies with identical cache-key text but different bytes while preserving the existing cache-hit assertions.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 @.github/workflows/e2e.yml:
- Around line 49-50: Update the Checkout step using actions/checkout to set
persist-credentials to false, ensuring later dependency and E2E steps cannot
access the checkout token through Git configuration.
In `@DEPLOY.md`:
- Line 118: Update the deployment instructions associated with PAYGATE_AUDIENCE
to replace the outdated count of four placeholders with “all placeholders” or an
accurate explicit enumeration, reflecting the six placeholders currently
present.
In `@gateway/config.go`:
- Around line 86-109: Update the audience hostname normalization around
parsed.Hostname() to reject IPv6 zone identifiers, including values decoded from
%25, before rebuilding canonicalHost. Return the existing invalid-hostname error
for zoned addresses and add a regression test covering an escaped IPv6 zone ID
in PAYGATE_AUDIENCE.
In `@gateway/errors_test.go`:
- Around line 219-230: The signer-mismatch regression test currently uses an
HTTP 200 verifier response and does not exercise the non-2xx path. Update
TestHandleSummarizeMapsSignerMismatch to stub a non-2xx verifier status, or add
a separate targeted case, while preserving assertions for the forbidden status,
sanitized response, and signer_mismatch error code.
In `@gateway/README.md`:
- Line 97: Move the PAYGATE_AUDIENCE entry from the “Common optional variables”
section into the README’s required configuration section, preserving its
required status and validation description.
In `@run_e2e.sh`:
- Line 58: Replace the `seq`-based iteration in the readiness loop of
`run_e2e.sh` with Bash arithmetic iteration, preserving the existing 60-attempt
limit and loop body behavior.
In `@tests/README.md`:
- Line 21: Update the prerequisites documentation in tests/README.md to include
port 3100 for the deterministic mock used when OPENROUTER_API_KEY is unset,
while preserving the existing 3000 and 3002 entries and ensuring the documented
ports match the implementation.
In `@web/scripts/deployed-smoke-test.ts`:
- Around line 84-90: Update getChallenge to validate the actual 402 response’s
CORS headers before parsing paymentContext. Require Access-Control-Allow-Origin
to match ORIGIN, while preserving the existing timedFetch and JSON parsing
behavior.
In `@web/src/app/docs/protocol/page.mdx`:
- Line 64: Update the requestHash description in the protocol documentation to
state that it is the SHA-256 hash of the exact serialized request body bytes
only, excluding HTTP headers and other request components.
---
Outside diff comments:
In `@gateway/cache_integration_test.go`:
- Around line 106-121: The cache integration test’s mock verifier does not
validate that request bodies are forwarded on cache hits. Update the test around
the makeRequest helper and verifier mock to capture the verifier request and
assert its RequestHash, exercising two raw JSON bodies with identical cache-key
text but different bytes while preserving the existing cache-hit assertions.
🪄 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: b93ab8b0-e7aa-4eca-b3b5-0ee916d8723d
📒 Files selected for processing (44)
.env.example.env.production.example.github/workflows/e2e.ymlDEPLOY.mdREADME.mddeploy/fly/gateway.fly.tomldeploy/fly/verifier.fly.tomldocker-compose.ymlgateway/README.mdgateway/cache.gogateway/cache_integration_test.gogateway/config.gogateway/config_test.gogateway/errors.gogateway/errors_test.gogateway/go.modgateway/main.gogateway/metrics.gogateway/metrics_test.gogateway/openapi.yamlgateway/payment_flow.gogateway/payment_flow_test.gogateway/receipt.gogateway/receipt_store_integration_test.gogateway/receipt_test.gogateway/request_binding.gogateway/timeout_test.gorun_e2e.shsdk/typescript/README.mdsdk/typescript/src/__tests__/client-flow.test.tssdk/typescript/src/__tests__/receipts.test.tssdk/typescript/src/protocol/microai.tssdk/typescript/src/protocol/types.tssdk/typescript/src/receipts.tstests/README.mdtests/e2e.test.tstests/mock-openrouter.tsverifier/README.mdverifier/src/main.rsweb/scripts/deployed-smoke-test.tsweb/src/app/docs/protocol/page.mdxweb/src/lib/verify-receipt.tsweb/src/lib/x402-client.test.tsweb/src/lib/x402-client.ts
💤 Files with no reviewable changes (1)
- web/src/lib/x402-client.ts
| - name: Checkout | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable persisted checkout credentials.
Later dependency and E2E steps execute repository-controlled code, which can read the token stored in Git configuration. Set persist-credentials: false.
Proposed fix
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+ with:
+ persist-credentials: false📝 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.
| - name: Checkout | |
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | |
| - name: Checkout | |
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 49-50: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/e2e.yml around lines 49 - 50, Update the Checkout step
using actions/checkout to set persist-credentials to false, ensuring later
dependency and E2E steps cannot access the checkout token through Git
configuration.
Source: Linters/SAST tools
| REDIS_URL=<your-upstash-rediss-url> | ||
| RECEIPT_STORE=redis | ||
| VERIFIER_URL=https://<verifier-app>.onrender.com | ||
| PAYGATE_AUDIENCE=https://<gateway-app>.onrender.com |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the placeholder count in the deployment instructions.
Adding PAYGATE_AUDIENCE introduces another <...> value, but the instructions still say there are four placeholders; this block currently contains six. Change the text to “all placeholders” or enumerate them explicitly.
🤖 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 `@DEPLOY.md` at line 118, Update the deployment instructions associated with
PAYGATE_AUDIENCE to replace the outdated count of four placeholders with “all
placeholders” or an accurate explicit enumeration, reflecting the six
placeholders currently present.
Source: Coding guidelines
| hostname := parsed.Hostname() | ||
| if !strings.Contains(hostname, ":") { | ||
| hostname, err = idna.Lookup.ToASCII(hostname) | ||
| if err != nil { | ||
| return fmt.Errorf("PAYGATE_AUDIENCE must contain a valid hostname: %w", err) | ||
| } | ||
| } | ||
| hostname = strings.ToLower(hostname) | ||
| port := parsed.Port() | ||
| if port != "" { | ||
| portNumber, err := strconv.Atoi(port) | ||
| if err != nil || portNumber < 1 || portNumber > 65535 { | ||
| return fmt.Errorf("PAYGATE_AUDIENCE must contain a valid port") | ||
| } | ||
| } | ||
| if (parsed.Scheme == "https" && port == "443") || (parsed.Scheme == "http" && port == "80") { | ||
| port = "" | ||
| } | ||
| canonicalHost := hostname | ||
| if port != "" { | ||
| canonicalHost = net.JoinHostPort(hostname, port) | ||
| } else if strings.Contains(hostname, ":") { | ||
| canonicalHost = "[" + hostname + "]" | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== gateway/config.go (relevant slice) =="
sed -n '1,180p' gateway/config.go | nl -ba | sed -n '70,130p'
echo
echo "== nearby tests =="
rg -n "PAYGATE_AUDIENCE|zone identifier|Hostname\(" gateway -g '*_test.go' -g '*.go' || true
echo
echo "== Go parser probe for IPv6 zone identifiers =="
cat >/tmp/audience_probe.go <<'EOF'
package main
import (
"fmt"
"net/url"
)
func main() {
for _, raw := range []string{
"https://[fe80::1%25eth0]",
"https://[fe80::1%25eth0]:443",
"https://[fe80::1]",
"https://example.com",
} {
u, err := url.Parse(raw)
if err != nil {
fmt.Printf("PARSE %q => err=%v\n", raw, err)
continue
}
fmt.Printf("RAW=%q\n", raw)
fmt.Printf(" Host=%q Hostname=%q Port=%q Scheme=%q String=%q\n", u.Host, u.Hostname(), u.Port(), u.Scheme, u.String())
}
}
EOF
go run /tmp/audience_probe.goRepository: AnkanMisra/MicroAI-Paygate
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== gateway/config.go around the audience canonicalization =="
sed -n '1,220p' gateway/config.go | nl -ba | sed -n '1,160p'
echo
echo "== audience-related tests in gateway =="
rg -n "PAYGATE_AUDIENCE|audience" gateway -g '*_test.go' -g '*.go' || trueRepository: AnkanMisra/MicroAI-Paygate
Length of output: 265
🌐 Web query:
Go net/url Hostname IPv6 zone identifier percent-encoding parsed.Hostname returns decoded zone
💡 Result:
In the Go standard library's net/url package, the Hostname method returns the host portion of a URL, stripping away any port number and removing the square brackets typically used for IPv6 literal addresses [1][2]. Regarding IPv6 zone identifiers, the implementation adheres to RFC 6874, which specifies that the zone identifier is introduced by a percent-encoded percent sign ("%25") [3][2][4]. When a URL containing an IPv6 address with a zone identifier is parsed, the net/url package handles the zone identifier by unescaping it [3][2]. Specifically, the internal logic splits the host into a host part (before the "%25") and a zone part (starting from the "%25") [3][2]. The host part is unescaped according to standard host rules, while the zone part is unescaped using rules specific to zone identifiers (which allow a wider range of percent-encoded characters) [3][2]. The Hostname method effectively returns the resulting unescaped host string [3][2]. Recent updates to the Go repository (as of April 2026) have introduced stricter validation for these zone identifiers to ensure compliance with the RFC 6874 ABNF grammar, limiting characters within the zone identifier to unreserved characters (ALPHA, DIGIT, '-', '.', '_', '~') or valid percent-encoded sequences [5][6]. This prevents improper greedy matching of characters—such as closing brackets—that should not appear unescaped within the zone identifier [5][6].
Citations:
- 1: https://pkg.go.dev/net/url
- 2: https://github.com/golang/go/blob/master/src/net/url/url.go
- 3: https://go.dev/src/net/url/url.go?m=text
- 4: https://www.rfc-editor.org/info/rfc6874/
- 5: net/url: strict RFC 6874 validation for IPv6 zone identifiers golang/go#78570
- 6: net/url: zone identifier parsing accepts invalid characters, allowing garbage injection via %25 (CVE-2026-25679 follow-up) golang/go#78569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== gateway/config.go audience logic =="
sed -n '80,125p' gateway/config.go
echo
echo "== gateway tests mentioning PAYGATE_AUDIENCE or URL host parsing =="
rg -n "PAYGATE_AUDIENCE|Hostname\(|zone identifier|fe80::1" gateway -g '*_test.go' -g '*.go' || trueRepository: AnkanMisra/MicroAI-Paygate
Length of output: 4229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat >/tmp/url_zone_probe.go <<'EOF'
package main
import (
"fmt"
"net/url"
)
func main() {
inputs := []string{
"https://[fe80::1%25eth0]",
"https://[fe80::1%25eth0]:443",
"https://[fe80::1]",
}
for _, raw := range inputs {
u, err := url.Parse(raw)
if err != nil {
fmt.Printf("PARSE %q => err=%v\n", raw, err)
continue
}
fmt.Printf("RAW=%q\n", raw)
fmt.Printf(" Host=%q Hostname=%q Port=%q EscapedPath=%q String=%q\n", u.Host, u.Hostname(), u.Port(), u.EscapedPath(), u.String())
}
}
EOF
go run /tmp/url_zone_probe.goRepository: AnkanMisra/MicroAI-Paygate
Length of output: 563
Reject IPv6 zone IDs in PAYGATE_AUDIENCE
gateway/config.go:86-109 — parsed.Hostname() decodes %25 to %, so rebuilding with [ + hostname + ] turns https://[fe80::1%25eth0] into https://[fe80::1%eth0]. Reject zone identifiers here, or preserve the escape before normalizing, and add a regression test.
🤖 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/config.go` around lines 86 - 109, Update the audience hostname
normalization around parsed.Hostname() to reject IPv6 zone identifiers,
including values decoded from %25, before rebuilding canonicalHost. Return the
existing invalid-hostname error for zoned addresses and add a regression test
covering an escaped IPv6 zone ID in PAYGATE_AUDIENCE.
Source: Coding guidelines
| func TestHandleSummarizeMapsSignerMismatch(t *testing.T) { | ||
| withVerifierResponse(t, http.StatusOK, `{"is_valid":false,"recovered_address":"0x0000000000000000000000000000000000000001","error":"SENSITIVE_SIGNER_DETAIL","error_code":"signer_mismatch"}`) | ||
|
|
||
| router := newSummarizeTestRouter() | ||
| recorder := httptest.NewRecorder() | ||
| router.ServeHTTP(recorder, signedSummarizeRequest(`{"text":"hello"}`)) | ||
|
|
||
| require.Equal(t, http.StatusForbidden, recorder.Code) | ||
| require.NotContains(t, recorder.Body.String(), "SENSITIVE_SIGNER_DETAIL") | ||
| var response map[string]string | ||
| require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) | ||
| require.Equal(t, "signer_mismatch", response["error"]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exercise the non-2xx signer-mismatch path.
The HTTP 200 stub tests public mapping but never covers the newly added isVerifierBusinessRejection branch. Return a non-2xx verifier status in this case, or add a second case for it.
Proposed regression coverage
- withVerifierResponse(t, http.StatusOK, `{"is_valid":false,"recovered_address":"0x0000000000000000000000000000000000000001","error":"SENSITIVE_SIGNER_DETAIL","error_code":"signer_mismatch"}`)
+ withVerifierResponse(t, http.StatusForbidden, `{"is_valid":false,"recovered_address":"0x0000000000000000000000000000000000000001","error":"SENSITIVE_SIGNER_DETAIL","error_code":"signer_mismatch"}`)As per coding guidelines, “Gateway handlers, config parsing, middleware, timeout behavior, receipts, cache/rate limits, and error paths must have table tests or targeted regression tests.”
📝 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 TestHandleSummarizeMapsSignerMismatch(t *testing.T) { | |
| withVerifierResponse(t, http.StatusOK, `{"is_valid":false,"recovered_address":"0x0000000000000000000000000000000000000001","error":"SENSITIVE_SIGNER_DETAIL","error_code":"signer_mismatch"}`) | |
| router := newSummarizeTestRouter() | |
| recorder := httptest.NewRecorder() | |
| router.ServeHTTP(recorder, signedSummarizeRequest(`{"text":"hello"}`)) | |
| require.Equal(t, http.StatusForbidden, recorder.Code) | |
| require.NotContains(t, recorder.Body.String(), "SENSITIVE_SIGNER_DETAIL") | |
| var response map[string]string | |
| require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) | |
| require.Equal(t, "signer_mismatch", response["error"]) | |
| func TestHandleSummarizeMapsSignerMismatch(t *testing.T) { | |
| withVerifierResponse(t, http.StatusForbidden, `{"is_valid":false,"recovered_address":"0x0000000000000000000000000000000000000001","error":"SENSITIVE_SIGNER_DETAIL","error_code":"signer_mismatch"}`) | |
| router := newSummarizeTestRouter() | |
| recorder := httptest.NewRecorder() | |
| router.ServeHTTP(recorder, signedSummarizeRequest(`{"text":"hello"}`)) | |
| require.Equal(t, http.StatusForbidden, recorder.Code) | |
| require.NotContains(t, recorder.Body.String(), "SENSITIVE_SIGNER_DETAIL") | |
| var response map[string]string | |
| require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) | |
| require.Equal(t, "signer_mismatch", response["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/errors_test.go` around lines 219 - 230, The signer-mismatch
regression test currently uses an HTTP 200 verifier response and does not
exercise the non-2xx path. Update TestHandleSummarizeMapsSignerMismatch to stub
a non-2xx verifier status, or add a separate targeted case, while preserving
assertions for the forbidden status, sanitized response, and signer_mismatch
error code.
Source: Coding guidelines
| | `OLLAMA_MODEL` | `llama2` provider default | Used when `AI_PROVIDER=ollama`. | | ||
| | `VERIFIER_URL` | **required** (no fallback) | Where the gateway calls `/verify`. Use `http://127.0.0.1:3002` for `bun run stack`, `http://verifier:3002` in Compose, the platform's HTTPS URL in production. The gateway refuses to start if unset. | | ||
| | `PAYGATE_AUDIENCE` | unset | Public gateway origin reserved for request-bound authorization v2. Configure the production origin before the gateway cutover; the current v1 gateway does not read it. | | ||
| | `PAYGATE_AUDIENCE` | **required** | Trusted public gateway origin included in every v2 authorization. Origins only; paths, queries, fragments, credentials, and non-HTTP schemes are rejected at startup. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Move PAYGATE_AUDIENCE out of the optional configuration section.
This variable is documented as required, but it remains under “Common optional variables.” Move it into a required configuration section or rename the heading so operators do not omit a startup-required setting.
🤖 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/README.md` at line 97, Move the PAYGATE_AUDIENCE entry from the
“Common optional variables” section into the README’s required configuration
section, preserving its required status and validation description.
Source: Coding guidelines
| # Wait for both services to be ready instead of assuming a fixed build/start time. | ||
| echo "Waiting for services to initialize..." | ||
| SERVICES_READY=false | ||
| for _ in $(seq 1 60); do |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid seq in the macOS-compatible runner.
seq is not available on stock macOS, so the readiness loop fails immediately. Use Bash arithmetic instead.
Proposed fix
-for _ in $(seq 1 60); do
+for ((attempt = 1; attempt <= 60; attempt++)); do📝 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.
| for _ in $(seq 1 60); do | |
| for ((attempt = 1; attempt <= 60; attempt++)); do |
🤖 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 `@run_e2e.sh` at line 58, Replace the `seq`-based iteration in the readiness
loop of `run_e2e.sh` with Bash arithmetic iteration, preserving the existing
60-attempt limit and loop body behavior.
| - Rust toolchain | ||
| - Ports `3000` and `3002` free | ||
| - `OPENROUTER_API_KEY` for the default OpenRouter gateway startup path | ||
| - No external AI key is required; the helper starts a deterministic OpenRouter-compatible mock when `OPENROUTER_API_KEY` is unset. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the mock’s required port.
The helper binds the deterministic mock to port 3100, but the prerequisites list only 3000 and 3002. Add port 3100 when OPENROUTER_API_KEY is unset.
As per coding guidelines, Markdown changes must be checked for wrong ports and mismatch with code.
🤖 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/README.md` at line 21, Update the prerequisites documentation in
tests/README.md to include port 3100 for the deterministic mock used when
OPENROUTER_API_KEY is unset, while preserving the existing 3000 and 3002 entries
and ensuring the documented ports match the implementation.
Source: Coding guidelines
| async function getChallenge(body: string): Promise<PaymentContext> { | ||
| const r = await timedFetch(`${GATEWAY}/api/ai/summarize`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json", Origin: ORIGIN }, | ||
| body: JSON.stringify({ text }), | ||
| body, | ||
| }); | ||
| return (await r.json()).paymentContext as PaymentContext; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate CORS on the actual 402 challenge response.
Bun does not enforce browser CORS. The script currently checks preflight and the signed response, but a 402 without Access-Control-Allow-Origin would still pass here while browsers could not read paymentContext.
Proposed validation
const r = await timedFetch(`${GATEWAY}/api/ai/summarize`, {
method: "POST",
headers: { "Content-Type": "application/json", Origin: ORIGIN },
body,
});
- return (await r.json()).paymentContext as PaymentContext;
+ const allowOrigin = r.headers.get("access-control-allow-origin");
+ rec("challenge returns 402", r.status === 402, `HTTP ${r.status}`);
+ rec("challenge allows deployed web origin", allowOrigin === ORIGIN, allowOrigin ?? "missing");
+ return (await r.json()).paymentContext as PaymentContext;📝 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.
| async function getChallenge(body: string): Promise<PaymentContext> { | |
| const r = await timedFetch(`${GATEWAY}/api/ai/summarize`, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json", Origin: ORIGIN }, | |
| body: JSON.stringify({ text }), | |
| body, | |
| }); | |
| return (await r.json()).paymentContext as PaymentContext; | |
| async function getChallenge(body: string): Promise<PaymentContext> { | |
| const r = await timedFetch(`${GATEWAY}/api/ai/summarize`, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json", Origin: ORIGIN }, | |
| body, | |
| }); | |
| const allowOrigin = r.headers.get("access-control-allow-origin"); | |
| rec("challenge returns 402", r.status === 402, `HTTP ${r.status}`); | |
| rec("challenge allows deployed web origin", allowOrigin === ORIGIN, allowOrigin ?? "missing"); | |
| return (await r.json()).paymentContext as PaymentContext; | |
| } |
🤖 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/scripts/deployed-smoke-test.ts` around lines 84 - 90, Update getChallenge
to validate the actual 402 response’s CORS headers before parsing
paymentContext. Require Access-Control-Allow-Origin to match ORIGIN, while
preserving the existing timedFetch and JSON parsing behavior.
| - `method`: uppercase HTTP method. | ||
| - `resource`: encoded path plus raw query string. | ||
| - `contentType`: exact request content type. | ||
| - `requestHash`: SHA-256 hash of the exact serialized request bytes. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Specify that requestHash covers only the serialized body bytes.
Line 64 says “request bytes,” which can be interpreted as the entire HTTP request. The gateway and SDK hash only the exact serialized request body, so independent clients following this text could generate incompatible signatures.
-- `requestHash`: SHA-256 hash of the exact serialized request bytes.
+- `requestHash`: SHA-256 hash of the exact serialized request body bytes.As per coding guidelines, Markdown must be checked for “mismatch with code/workflows.”
📝 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.
| - `requestHash`: SHA-256 hash of the exact serialized request bytes. | |
| - `requestHash`: SHA-256 hash of the exact serialized request body bytes. |
🤖 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/app/docs/protocol/page.mdx` at line 64, Update the requestHash
description in the protocol documentation to state that it is the SHA-256 hash
of the exact serialized request body bytes only, excluding HTTP headers and
other request components.
Source: Coding guidelines
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58190f93b5
ℹ️ 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".
| CHAIN_ID=84532 | ||
| EXPECTED_CHAIN_ID=84532 | ||
| PAYMENT_AMOUNT=0.001 | ||
| ALLOWED_ORIGINS=* |
There was a problem hiding this comment.
In this Render gateway env block, ALLOWED_ORIGINS=* does not actually allow the deployed web app: getAllowedOrigins only accepts absolute http(s) origins, so * is logged as invalid and the gateway falls back to http://localhost:3001. When following these deploy docs, the new signed-retry preflight from Vercel will miss Access-Control-Allow-Origin and the browser cannot send the X-402-* headers; please document the exact web origin here or add intentional wildcard support.
Useful? React with 👍 / 👎.
| if (request.method !== "POST") { | ||
| return new Response("Not found", { status: 404 }); |
There was a problem hiding this comment.
When run_e2e.sh points OPENROUTER_URL at the deterministic mock, the gateway readiness check derives /api/v1/models from that URL, but this mock only returns 200 for GET /health and returns 404 for every other GET. That means /readyz reports OpenRouter as degraded under deterministic runs even though the wait loop says the mock is ready; please also handle the models endpoint or align the health check with the mock.
Useful? React with 👍 / 👎.
| C->>G: POST /api/ai/summarize | ||
| G-->>C: 402 + paymentContext(recipient, token, amount, chainId, nonce, timestamp) | ||
| C->>G: Retry with X-402-Signature, X-402-Nonce, X-402-Timestamp | ||
| C->>G: Retry with X-402-Signature, nonce, timestamp, payer |
There was a problem hiding this comment.
This updated sequence now documents the retry as bare nonce, timestamp, payer instead of the actual X-402-Nonce, X-402-Timestamp, and new X-402-Payer headers. Since the v2 cutover depends on the browser preflight and gateway seeing those exact header names, copying this diagram can produce a signed retry that is rejected; please keep the full header names here.
Useful? React with 👍 / 👎.
1 participant
Summary
X-402-PayerSecurity properties
PAYGATE_AUDIENCE, never request host or forwarded headersMIN_AUTHORIZATION_VERSION=2Validation
cd gateway && go test -count=1 ./... && go vet ./...cd verifier && cargo fmt -- --check && cargo clippy -- -D warnings && cargo test— 51 passedcd sdk/typescript && bun run typecheck && bun test && bun run build— 26 passed, 1 opt-in live test skippedcd web && bun run lint && bun run typecheck && bun run test:unit && bun run build— 76 passedVERIFIER_NONCE_STORE=memory ./run_e2e.sh— 4 passedFollow-up to #306.
Note
Enforce request-bound payment authorization v2 across gateway, verifier, and SDK
PaymentContextserver-side, bindingaudience,method,resource,contentType, and a SHA-256requestHashto every payment challenge and verification request.MIN_AUTHORIZATION_VERSION(default2) with HTTP 400 and error codeauthorization_version_too_old; the gateway requires a validX-402-Payerheader and rejects signer mismatches with 403signer_mismatch.isPaymentContextguard andvalidatePaymentContextForRequestnow reject any context that lacks v2 binding fields, replacing the previous no-op for legacy contexts with a thrownPaygateSdkError.PaymentAuthorizationEIP-712 type that includes payer and request-binding fields; CI now runs deterministically without an external OpenRouter key using a new mock server.PAYGATE_AUDIENCEis now a required environment variable for the gateway, normalized to a canonical HTTP/HTTPS origin at startup.X-402-Payerwill receive 400/402 errors.Macroscope summarized 58190f9.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation