Skip to content

Fix/backend hardening#879

Merged
hman38705 merged 10 commits into
mainfrom
fix/backend-hardening
Jun 17, 2026
Merged

Fix/backend hardening#879
hman38705 merged 10 commits into
mainfrom
fix/backend-hardening

Conversation

@hman38705

Copy link
Copy Markdown
Contributor

Description

Type of Change

  • Bug fix
  • New feature
  • Refactor / code cleanup
  • Documentation update
  • CI / tooling change
  • Breaking change

Testing Done

Checklist

  • Tests pass locally
  • Documentation updated (if applicable)
  • No breaking changes, or breaking changes are documented above

Related Issues

Closes #

hman38705 and others added 10 commits June 17, 2026 16:15
… window

The previous check used (now - ts).abs() which accepted timestamps up to
replay_window_secs in the FUTURE. An attacker could pre-sign a request
with timestamp = now + window - 1, then replay it for up to 2 * window
seconds total — doubling the effective replay window.

Fix: reject any timestamp where ts > now (age_secs < 0) AND any that is
older than replay_window_secs. This reduces the replay window to its
intended length and adds a tracing::warn so future-dated attempts are
visible in observability tooling.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Verifies all four cases of the updated timestamp validation:
1. Timestamp within replay window is accepted
2. Timestamp exactly at the window edge is accepted
3. Timestamp beyond the window is rejected
4. Future timestamp (ts > now) is rejected
5. Future timestamp that would pass the old abs() check is now rejected

The tests use a helper that mirrors the age_secs < 0 || age_secs > window
logic directly so they are independent of middleware plumbing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two bugs in requeue_dead_letter:

1. No backoff delay — the job was added with score=now so the worker
   picked it up immediately. A persistent failure (e.g. bad template,
   suppressed address race) caused an instant re-failure → dead-letter
   → requeue loop that could spin indefinitely when triggered by an admin.
   Fix: schedule the job DEAD_LETTER_REQUEUE_DELAY_SECS (60s) in the
   future so there is always a minimum cooling-off period.

2. Attempts counter not reset — the job retained its previous failure
   count, so it often had 0 retries remaining and would go straight back
   to dead-letter on the first failure. Fix: reset attempts to 0 via
   email_update_job_attempts so the job gets its full retry budget.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ariants

Previously all sqlx errors collapsed into DbError::Other, making it
impossible for callers to distinguish recoverable (pool exhausted, retry
later) from permanent (constraint violation, 409 Conflict) failures.

Changes:
- DbError::PoolExhausted — mapped from sqlx::Error::PoolTimedOut;
  handlers return 503 Service Unavailable
- DbError::ConstraintViolation(String) — mapped from any sqlx database
  error whose SQLSTATE code starts with "23" (PostgreSQL integrity class);
  handlers return 409 Conflict with the DB error message
- From<sqlx::Error> now pattern-matches before falling back to Other
- into_api_error in handlers.rs handles all three variants explicitly

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the new variants and their Display output:
- Timeout produces the expected message
- PoolExhausted produces the expected message
- ConstraintViolation includes both the label and the inner message
- From<sqlx::Error::PoolTimedOut> maps to PoolExhausted
- From<sqlx::Error::RowNotFound> maps to Other (not a special variant)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a rate_limit_rejections_total counter labelled by route to the
Metrics struct, exposed on the /metrics endpoint. The counter is
incremented in rate_limit_middleware on every 429 response.

Before this change there was no way to observe attack patterns, tune
limits based on real traffic, or alert on sudden spikes in rate-limit
hits. With the counter, operators can:
- Alert when rejection rate exceeds a threshold
- Graph rejection rate by route in Grafana
- Detect credential-stuffing or scraping attempts by IP

Also adds a tracing::warn on rejection so the event appears in
structured logs even without Prometheus configured.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous implementation built the WHERE clause by concatenating
format! strings and then bound values in a separate pass. The two passes
had to stay in sync manually — a filter added to one but not the other
would silently bind the wrong value to the wrong placeholder.

sqlx::QueryBuilder keeps the SQL fragment and its bound value together
at each push_bind call, making the relationship structurally impossible
to desync. User-supplied filter values are always sent as typed parameters
and never interpolated into the SQL string.

Also removes the unused `bindings` Vec that was allocated on every call
but never populated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…entry

Tests that do not require a database:
- AuditStatus Display for all three variants (success/failure/partial)
- create_audit_entry populates fields correctly and defaults to Success
- make_entry helper correctly sets status (used as test fixture)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents that RateLimitState.metrics is None-safe — callers that have
not wired up Prometheus (e.g. test harnesses) can construct the state
without a Metrics instance and the middleware will skip the counter
increment without panicking.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Guards the DEAD_LETTER_REQUEUE_DELAY_SECS constant against being set to
zero, which would re-introduce the immediate re-failure loop the constant
was introduced to prevent. The test fails at compile-checked runtime if
someone sets the constant to 0.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hman38705 hman38705 merged commit 86cb874 into main Jun 17, 2026
0 of 6 checks passed
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