Skip to content

fix: sign-in 500 — auth rate limit passed raw Date params to postgres.js - #93

Merged
Skeyelab merged 1 commit into
masterfrom
fix/auth-rate-limit-date-params
Sep 23, 2026
Merged

Skeyelab merged 1 commit into
masterfrom
fix/auth-rate-limit-date-params

Conversation

@Skeyelab

Copy link
Copy Markdown
Member

Problem

Every sign-in and sign-up attempt on production returned HTTP 500 ("Request failed" in the UI). From the Coolify app logs:

TypeError: The "string" argument must be of type string or an instance of
Buffer or ArrayBuffer. Received an instance of Date
  at Function.str (.next/server/chunks/app_lib_db_ts_0.d9324._.js:1:8507)

Failed query: insert into "auth_rate_limits" ... on conflict ("key") do update
set ... "window_start" = case when "auth_rate_limits"."window_start" <= $8 then $9 ...

This was not a credentials problem — the request died in the rate limiter before credentials were ever checked, so no one could log in at all.

Root cause

postgres.js cannot serialize a raw Date that is interpolated into a drizzle sql fragment; only drizzle's own column-typed bindings get mapped to a driver-serializable value. The atomic upsert introduced in fix: make auth rate limit increments atomic interpolated windowStart and expiredBefore Date objects directly into the case when ... fragments.

Fix

Pre-stringify both timestamps to ISO and cast them back to timestamptz inside the statement. The upsert remains a single atomic statement that increments count in the database, so the original security-review fix still holds.

Tests

Added tests/auth/rate-limit-params.test.ts, which drives the real rate-limit module through drizzle's pg-proxy driver and asserts:

  • no raw Date ever reaches the driver (this test fails on the old code with the exact 3 offending params)
  • the statement is still a single on conflict upsert with an in-database "count" + 1

Verification

  • yarn lint ✅
  • yarn test:run ✅ 68 files / 291 tests
  • Verified against a real Postgres 16 instance (not just mocks): migrations apply, insert path, the conflict/upsert path that was 500ing, atomic increment to count=5, throttling engaging with retryAfterSeconds=900, and a full sign-up → wrong-password 401 → correct-password 200 flow.

Every sign-in and sign-up attempt returned HTTP 500 in production:

  TypeError: The "string" argument must be of type string or an instance
  of Buffer or ArrayBuffer. Received an instance of Date

postgres.js cannot serialize a raw Date interpolated into a drizzle `sql`
fragment - only drizzle's own column-typed bindings get mapped. The atomic
rate-limit upsert interpolated two Date objects directly, so the statement
failed before credentials were ever checked.

Pre-stringify the timestamps to ISO and cast them back to timestamptz inside
the statement. The upsert stays a single atomic statement that increments in
the database.

Verified against a real Postgres 16 instance: insert path, conflict/upsert
path, atomic increment to count=5, throttling engaging, and a full
sign-up -> wrong-password-401 -> correct-password-200 flow.
@strix-security

strix-security Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Strix Security Review

No security issues found.

Review summary

Reviewed both changed files: the rate-limiter fix in app/lib/auth/rate-limit.ts and the new regression test. The change pre-stringifies two server-generated Date values to ISO strings and casts them with ::timestamptz inside the atomic upsert's case when fragments. The interpolated values are produced by Date.toISOString(), not attacker-controlled input, and are emitted as bound parameters, so there is no SQL injection surface. The window_start column is timestamp with time zone, so the cast is type-correct. The atomic single-statement upsert and in-database count + 1 increment are preserved, and key/kind remain parameterized. No security control was weakened and no new tainted-input path was introduced; no vulnerabilities were identified.

Updated for 174a61a.


Reviewed by Strix
Re-run review · Configure security review settings

@Skeyelab
Skeyelab merged commit 9250073 into master Sep 23, 2026
3 checks passed
@Skeyelab
Skeyelab deleted the fix/auth-rate-limit-date-params branch September 23, 2026 18:58
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