x402 icon indicating copy to clipboard operation
x402 copied to clipboard

tests: adding tests to verify that the middleware correctly delivers content based on settlement results

Open souhailaS opened this issue 1 month ago • 5 comments

Description

In the FastAPI payment middleware (i haven't tried the other ones) there is an issue that makes response content being sent to clients before payment settlement completed, which can allow clients to receive paid content for free if settlement failed for whatever reason. The response = await call_next(request)returned a StreamingResponse that begins transmitting data immediately. So, by the momen the settlement successes is being being verified, it is already too late. the response headers and body have already been sent to the client.

In this fix I suggest to buffering the response body in memory after verification instead and to only return Response(content=response_body, ...) after verification of settlement is succeful, then if settlement fails, a 402 Payment Required error is returned without delivering any content.

There are no breaking changes in this fix

Tests

I added the three tests i used in the test_middleware.py

Checklist

  • [x] I have formatted and linted my code
  • [x] All new and existing tests pass
  • [x] My commits are signed (required for merge) -- you may need to rebase if you initially pushed unsigned commits

souhailaS avatar Nov 26 '25 21:11 souhailaS

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

cb-heimdall avatar Nov 26 '25 21:11 cb-heimdall

@souhailaS is attempting to deploy a commit to the Coinbase Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Nov 26 '25 21:11 vercel[bot]

Hi @souhailaS, thanks a lot for looking into this! We observed similar issues for the express and flask middlewares (https://github.com/coinbase/x402/pull/659, https://github.com/coinbase/x402/pull/693), so its possible that fastapi is impacted as well.

However, it is my understanding that the StreamingResponse object returned by response = await call_next(request) is only consumed when the response is actually being sent, which happens after the middleware returns.

I tested the following protected endpoints and forcing settle_response.success = False in the fastapi middleware:

@app.get("/weather")
async def get_weather() -> Dict[str, Any]:
    return {
        "report": {
            "weather": "sunny",
            "temperature": 70,
        }
    }

@app.get("/stream")
async def paid():
    async def generate():
        yield b"chunk 1\n"
        await asyncio.sleep(0.1)
        yield b"chunk 2\n"
        await asyncio.sleep(0.1)
        yield b"final chunk\n"

    return StreamingResponse(generate(), media_type="text/plain")

I could not observe any leaked content at the client and instead received the 402 error response as intended.

If you’ve actually observed leaked content, it would be much appreciated if you could provide a minimal example that shows a client receiving any part of the paid response before settlement is completed.

Even if such a vulnerability does exist, just buffering the response would likely not be sufficient to fix it and one would likely need to intercept response methods and replay after settlement as is done in the express/flask middleware.

phdargen avatar Nov 28 '25 10:11 phdargen

I also tested your new tests and they pass with the middleware unchanged.

In any case, your added unit tests fill an important gap in coverage for the payment settlement logic and we would be happy to merge those.

phdargen avatar Nov 28 '25 10:11 phdargen

@phdargen thanks for looking into it, and you're absolutely right! Sorry for the wrong alarm, I did run a few tests and indeed the call_next is not like in express where the route handler can immediately start writing response data during next() before the middleware regains control .. I fixed removed the buffering from the PR and kept the tests

souhailaS avatar Nov 29 '25 21:11 souhailaS