feat(supabase): Document, monitor, and rotate Upstash Redis (SMI-5769)#1972
Merged
Conversation
Upstash Redis backs rate-limiter.ts, quota-enforcer.ts, tier-cache.ts, and
circuit-breaker.ts but was undocumented in .env.schema and had zero
monitoring — health/index.ts's own docstring claimed to check 'Rate
limiter status' but never did. Verified directly against prod that the
credentials are also ~6 months stale, excluded from a broader Supabase
secrets rotation with no recorded rationale.
- Add UPSTASH_REDIS_REST_URL/TOKEN and RATE_LIMIT_FAIL_CLOSED to
.env.schema, following the existing per-vendor block convention.
- Add checkRedisHealth() (pipelined Upstash SET+GET+DEL round trip,
never throws, never worse than 'degraded') and wire it into the
live /health endpoint's checks array, closing the docstring/code
mismatch. Split into a new rate-limiter.health.ts sibling file to
keep rate-limiter.ts under the 500-line convention, mirroring the
existing rate-limiter.types.ts split.
- Add Redis-backed rate limiting as a 7th public status-page
component (SMI-5768): new migration seeding a 'rate-limiting' row
with a capability-framed public name ('API Rate Limiting', never
'Upstash'/'Redis' — matches the existing 6 components' naming),
checkRateLimitingStatus() in health-checks.probes.ts reusing
checkRedisHealth() as its raw probe, wired into status-check's
COMPONENT_SLUGS + checkers.
- Update smoke-prod-guide.md's health surface + failure-triage
Classify section, and CLAUDE.md's sub-doc table.
- Bump the docs/internal submodule pointer for the new
upstash-redis-operations.md runbook (architecture, why a scoped
read-only token isn't viable, credential rotation procedure and
180-day cadence) and a guards-and-opt-outs.md backfill row for the
pre-existing RATE_LIMIT_FAIL_CLOSED guard (skillsmith-docs#485).
Adversarial review (before this commit) caught and fixed two issues:
a numeric health-check sentinel round-tripped through Upstash's
JSON.parse-based deserialization as a number, not the string that was
written, so the round trip would ALWAYS report 'degraded' against a
healthy Redis; and checkRateLimitingStatus() was passing Upstash's raw
message (containing 'Redis'/'Upstash') into status_checks.message,
which Wave 4/5 will eventually expose on the public status page,
undoing the migration's own vendor-neutral naming discipline. Both
fixed; regression test added in rate-limiter.health.test.ts.
Also fixes a test-loading regression this change itself introduced:
importing checkRedisHealth into health-checks.probes.ts pulled rate-
limiter.ts's real (non-type-only) esm.sh imports into the
health-checks.ts barrel, which health-checks.{helpers,probes,readers}
.test.ts all import through — none had the vi.mock() treatment for
those esm.sh specifiers, so all three failed to load. Added the same
mock pattern already used in rate-limiter.test.ts/health/index.test.ts.
Refs SMI-5769, SMI-5768
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
E2E Test ResultsE2E Test Results - July 20, 2026Summary
Test Results
Generated by skillsmith E2E test suite |
…h-redis-hardening
…h-redis-hardening # Conflicts: # docs/internal
E2E Test ResultsE2E Test Results - July 20, 2026Summary
Test Results
Generated by skillsmith E2E test suite |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Business Summary
What shipped: Upstash Redis (the backing store for API rate limiting, quota caching, and circuit-breaker state) is now documented — a setup/architecture/rotation runbook plus
.env.schemaentries that were missing since the feature originally shipped. The public/healthendpoint now actually checks Redis (it claimed to in its own docstring but never did), and the new public status page gets Redis-backed rate limiting as a 7th monitored component, alongside the existing 6.Quality bar: Implementation + adversarial security review before this PR opened. The review caught and fixed a real bug pre-merge: the health check would have permanently reported "degraded" against a perfectly healthy Redis, due to a type mismatch in how Upstash's client deserializes values. It also caught a vendor-name leak (an internal "Upstash"/"Redis" mention that would have flowed into a status-page column a future wave makes public) and a test-loading regression this change itself introduced. All fixed before commit. 89 tests pass across the touched files.
Found along the way: the ~6-month-old prod Upstash credential is stale and was excluded from a recent broader secrets rotation with no recorded reason — rotation procedure is documented in the new runbook, but the actual rotation needs a human in the Upstash console and is tracked as a follow-up step on SMI-5769, not done in this PR. Also filed separately: SMI-5770 (a CI check to catch future
.env.schemagaps like this one) and SMI-5772 (now closed — a test-mock gap this PR introduced and fixed in the same commit).Net result: Docs, health-check monitoring, and the status-page component are fully shipped. Credential rotation is documented but not yet performed — needs a human with Upstash console access.
Technical detail
.env.schema:UPSTASH_REDIS_REST_URL,UPSTASH_REDIS_REST_TOKEN,RATE_LIMIT_FAIL_CLOSED— previously read in code (since SMI-1231) but never declared.supabase/functions/_shared/rate-limiter.ts/ newrate-limiter.health.ts:checkRedisHealth()— pipelined SET+GET+DEL round trip, never throws, never worse thandegraded. Split into a sibling file to stay under the 500-line convention (mirrors the existingrate-limiter.types.tssplit). Newrate-limiter.health.test.tscovers the not-configured, healthy, mismatch, and error-redaction paths, plus a regression guard for the numeric-sentinel bug caught in review.supabase/functions/health/index.ts: wirescheckRedisHealth()into the live checks array.20260719030000_status_component_rate_limiting.sql(+ rollback): seeds a 7thstatus_componentsrow, public name"API Rate Limiting"(capability-framed, matching the existing 6 — never "Upstash"/"Redis" in public-facing columns).supabase/functions/_shared/health-checks.probes.ts:checkRateLimitingStatus(), reusingcheckRedisHealth()as its raw probe; message is translated to vendor-neutral text (raw detail kept inmetadata.detailfor engineer debugging).supabase/functions/status-check/index.ts: wires the 7th component intoCOMPONENT_SLUGS+checkers.health-checks.{helpers,probes,readers}.test.ts: added thevi.mock('https://esm.sh/@upstash/...')treatment these needed once the barrel (health-checks.ts) started transitively loadingrate-limiter.ts's real esm.sh imports.docs/internal/runbooks/upstash-redis-operations.md(docs/internal submodule, already merged as skillsmith-docs#485),guards-and-opt-outs.mdbackfill row forRATE_LIMIT_FAIL_CLOSED,smoke-prod-guide.mdhealth-row update,CLAUDE.mdsub-doc table row.scripts/smoke-prod/website.sh/surfaces.json's automated pass/fail) is intentionally not in this PR — that's a CI/dev-tooling script change requiring SPARC + plan-review first, per this repo's Infrastructure Change Policy.