lightninglabs / lightninglabs/aperture
proxy: bound the pricer body read, and pin the model position for metered services
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 268
- Forks
- 78
- Avg merge
- 22h 25m
- Merged PRs (30d)
- 1
Description
Two related items from the review of #247, both about the fact that the pricer only ever sees a bounded prefix of the request body.
## `readBodyPrefix` reads the whole body into memory first
`dumpRequest` caps what it *sends* to the pricer at `maxPricerBodyBytes` (64 KiB), but `readBodyPrefix` still does an `io.ReadAll` of the entire body in order to buffer it for the backend. On a metered service that now happens on **every authenticated request**, ahead of the rate limiter, so a client can pin memory proportional to what it uploads and pay nothing for the privilege.
`http.MaxBytesReader`, or an `io.LimitReader` for the prefix plus a streaming restore of the remainder, would bound it. Note this interacts with the rate-limit reordering in #247: the limiter now runs before `checkMeteredAccess`, but `dumpRequest` is reached through the pricer call inside it, so confirm where the read actually sits in the ordering before choosing the fix.
## The model has to be in the first 64 KiB, and we should say so
#247 made metered services fail closed when the model cannot be read from the serialized prefix, which is correct: resolving an unreadable model to the default would price an expensive model at the cheap one's rate and skip the bundle mismatch guard.
The consequence is a real, if unusual, constraint: a client that serializes `model` *after* a body larger than 64 KiB gets a permanent 500 with no path to purchase, since `GetPrice` on the first 402 fails the same way. Every OpenAI-compatible client puts `model` ahead of `messages`, so this does not bite in practice, but it is undocumented behavior that would be baffling to hit.
Add a line to `sample-conf.yaml` next to `dynamicprice.metered` saying that metered services require the `model` key within the first 64 KiB of the request body.
## Related residual
There is one case the prefix scan cannot close at all: a duplicate `model` key sitting *behind* the cap is invisible to anything reading a prefix. `modelFromJSONPrefix` refuses to guess when it sees a duplicate inside the prefix, but it cannot see past the cut. The durable fix is for aperture to pin the model from the full body it already holds and pass it to the pricer explicitly, rather than having the pricer re-derive it from a truncated copy. Worth considering alongside the `MaxBytesReader` work, since both touch the same read path.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing readBodyPrefix through dumpRequest and checkMeteredAccess, then inspect modelFromJSONPrefix and the related rate-limit ordering from #247. Done means request buffering is bounded, the model position and duplicate-key behavior are handled through the full body where needed, and sample-conf.yaml documents the first-64-KiB requirement.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, documentation, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100