Skip to content

CheckMessageSend has no atomic increment, so concurrent immediate sends can oversell MaxMessagesMonth #1005

Description

@AmirF194

CheckMessageSend (internal/limits/enforcer.go, ~line 234) reads MessagesThisMonth and compares it to MaxMessagesMonth, but nothing in the check reserves or increments that count. It's called once at accept time (EnforceMessageSend in outbound.go), and the actual usage row only gets written much later, in meterSentTx, once the message terminally sends through the provider.

That function's own comment says "the accept-time cap pre-check remains the quota gate," but between accept and terminal send a message just sits as an async job, so any number of already-accepted-but-not-yet-sent messages are invisible to MessagesThisMonth in the meantime. An account sending in a burst can have every message pass the check against the same pre-increment count.

Repro: added a test to internal/limits using the package's own fakeCounter/newEnforcerWithReader seam (the same fixtures limits_test.go already uses), set MaxMessagesMonth: 3, fired 10 concurrent CheckMessageSend calls, all 10 passed. go test -race is clean, this isn't a data race, it's that nothing increments what the check just read.

Same shape as #940 (max_agents) and the original max_domains fix in #901, just on the billing quota instead of a resource count, so I'd guess this wants a similar fix: move the increment into the same transaction as the check, or reserve at accept time and release/confirm at send. Didn't pick one since #901's design is probably the reference pattern and I haven't read that PR's diff closely enough to say it transfers cleanly here.

I only ran this against the enforcer's own test fixtures, not a live Postgres-backed usage_summaries row, so I can't say how wide the accept-to-send window usually runs in practice, just that it's structurally open. Happy to send a PR once there's a slot for it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions