Skip to content

fix: rate limit counted a full day, and Access errors were unreadable - #59

Open
ctala wants to merge 1 commit into
cloudflare:mainfrom
ctala:fix/rate-limit-and-access-errors
Open

fix: rate limit counted a full day, and Access errors were unreadable#59
ctala wants to merge 1 commit into
cloudflare:mainfrom
ctala:fix/rate-limit-and-access-errors

Conversation

@ctala

@ctala ctala commented Aug 10, 2026

Copy link
Copy Markdown

Two bugs found while running a second instance in production.

Rate limit was daily, not hourly

Emails are stored with toISOString() (2026-08-08T20:41:19.952Z), but checkSendRateLimit compared them against datetime("now", "-1 hour"), which returns 2026-08-08 21:14:29 — a space instead of T.

SQLite compares these as TEXT, and T (84) > (32), so every row from the current day always satisfied the condition. The documented "20 per hour" behaved as "20 per day" and only released when the UTC date rolled over. The daily check has the same defect and effectively spans two calendar days.

Symptom: a mailbox returns 429 long after the hour has passed, with no way to tell when it will recover.

Fixed by computing the cutoff in JS and binding it as a parameter, so both sides of the comparison are ISO 8601.

Access JWT failures were reported as expired tokens

The catch block discarded the error, so any failure surfaced as Invalid or expired Access token.

A TEAM_DOMAIN without a scheme — team.cloudflareaccess.com instead of https://team.cloudflareaccess.com — makes new URL() throw inside getAccessUrls. The message then points at the token, which is perfectly valid, and the actual cause is a config typo that is invisible from the outside.

Now the log states what failed and what it was validated against. The token is never logged.

Notes

  • No behaviour change beyond the corrected window and the added log line.
  • Verified against a live deployment: a mailbox stuck at 429 accepted sends again after the fix.

**Rate limit was daily, not hourly.** Emails are stored with
`toISOString()` ("2026-08-08T20:41:19.952Z") but the cap compared them
against `datetime('now', '-1 hour')`, which returns "2026-08-08
21:14:29" — a space instead of "T". SQLite compares these as TEXT and
'T'(84) > ' '(32), so every row from the current day always satisfied
the condition. The documented "20 per hour" behaved as "20 per day",
and only released when the UTC date rolled over. Same defect in the
daily check, which spanned roughly two calendar days.

Fixed by computing the cutoff in JS and binding it as a parameter, so
both sides of the comparison are ISO 8601.

**Access JWT failures were reported as expired tokens.** The catch
block swallowed the error, so any failure surfaced as "Invalid or
expired Access token". A TEAM_DOMAIN without a scheme (e.g.
"team.cloudflareaccess.com" instead of "https://team.cloudflareaccess.com")
makes `new URL()` throw, and the message points at the token, which is
perfectly valid. Now the error says what actually failed and what it
was validated against. The token is never logged.

Both were found while running a second instance in production.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant