Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
778 changes: 778 additions & 0 deletions cmd/smoketest/redis-rate-limiter/main.go

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,24 @@ type Config struct {
LockoutThreshold int // default: 5 failed attempts
LockoutDuration time.Duration // default: 15 minutes
Logger logger.Logger // default: ConsoleJSONLogger

// RateLimiter replaces the default limiter, which is an in-process
// security.InMemoryRateLimiter built from RateLimitAttempts and
// RateLimitWindow above. That default is correct for exactly one
// process: run three replicas behind a load balancer and each keeps
// its own counters, so the effective limit is three times what was
// configured. Set this to a shared implementation —
// security.NewRedisRateLimiter(client, attempts, window) — and every
// replica counts against one window.
//
// Injected already constructed, the same as every store, so the
// engine never dials Redis itself or owns its lifecycle. When set,
// RateLimitAttempts and RateLimitWindow are ignored entirely: they
// are the in-memory limiter's constructor arguments, and a limiter
// the host built already carries its own bounds.
//
// Left nil, nothing changes from previous versions.
RateLimiter security.RateLimiter
}

func (c *Config) validate() error {
Expand Down
2 changes: 2 additions & 0 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package cryden

import (
"testing"
"time"

"github.com/crydensync/cryden/v2/security"
"github.com/crydensync/cryden/v2/store/memory"
"github.com/redis/go-redis/v9"
)

func validConfig() Config {
Expand Down
67 changes: 56 additions & 11 deletions docs/development/CURRENT-STATE.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# cryden — current state

Last updated: 2026-09-04 (by the session that built
named/fingerprinted sessions). Update this file's date and content every time a session
Last updated: 2026-09-05 (by the session that built the Redis-backed
rate limiter). Update this file's date and content every time a session
finishes an item — see `CLAUDE.md`'s end-of-session checklist.

## Tagged releases
Expand All @@ -24,7 +24,7 @@ If you find a real bug in it while working on something else, fix it
on its own small branch and note it in `PROGRESS.md` — don't treat
finding it as license to re-audit the rest.

## Tier 2 — Security & Monitoring: IN PROGRESS (3 of 4 done)
## Tier 2 — Security & Monitoring: DONE (4 of 4)

### Item 8 — anomaly detection: DONE, branch `feat/anomaly-detection`

Expand Down Expand Up @@ -141,11 +141,51 @@ subtests), `security/geolocation_test.go` (2), `session/named_test.go`
test: `cmd/smoketest/named-sessions` (42 checks). Manual guide:
`docs/testing/named-sessions.md`.

### Item 11: NOT STARTED

Detailed specs in `NEXT.md`. The design decision recorded for item 8
below is kept for reference — it is what the shipped code implements.
**Do not re-ask or re-derive it**:
### Item 11 — Redis-backed rate limiter: DONE, branch `feat/redis-rate-limiter`

A **second real implementation** of the existing `security.RateLimiter`,
not a new interface: the in-memory one keeps its counters in a Go map,
which is correct for exactly one process — three replicas keep three
maps, so a configured limit of 10 lets 30 through.

Shipped as: `security/redisratelimiter.go` (`RedisRateLimiter`,
`NewRedisRateLimiter`, `NewRedisRateLimiterWithPrefix`,
`DefaultRedisKeyPrefix = "cryden:ratelimit:"`), three new sentinels in
`security/errors.go`, and `Config.RateLimiter` — injected already
constructed, like every store, so the engine never dials Redis nor owns
its lifecycle. `engine.go` falls back to the in-process default only
when that field is nil. Nothing in `auth/` changed or can tell which
implementation it holds.

Decisions worth not re-deriving (full reasoning in `PROGRESS.md`):
`github.com/redis/go-redis/v9`, injected as its own `redis.Scripter`
interface so Client/ClusterClient/Ring/UniversalClient all work and
`redis.NewScript`'s EVALSHA→EVAL fallback is reused rather than
reimplemented; one Lua script per `Allow` because INCR and PEXPIRE
apart lets two replicas each arm their own window; `PEXPIRE` only when
`INCR` returns 1 (or `PTTL` reports none) so a denied client's own
retries cannot push its window out; exactly one key per call, so
Cluster needs no special case; windows under 1ms rejected rather than
rounded, the single place the two implementations are not
interchangeable. Fail-closed is unchanged and now load-bearing — all
three call sites already propagate a limiter error, so Redis becomes a
hard dependency of SignUp/Login/RequestMagicLink; documented, with a
fail-open wrapper left to the host.

Tests: `security/redisratelimiter_test.go` (14 funcs over a fake that
models the script), plus 3 in `config_test.go` and 1 in
`new_facade_test.go`. Smoke test: `cmd/smoketest/redis-rate-limiter`
(58 checks over ten scenarios) — runs against an in-process stand-in by
default, and against a real server with `REDIS_ADDR` set, which is the
mode that actually executes the Lua. **No Redis server was reachable in
the build environment**, so the Lua itself is so far verified only
against that stand-in; one `docker run` closes the gap. Manual guide:
`docs/testing/redis-rate-limiter.md`.

#### Item 8's recorded decisions, kept for reference

What the shipped anomaly-detection code implements. **Do not re-ask or
re-derive it**:

- **Signals to evaluate**: new IP/device (vs. recent successful
logins), failed-attempt velocity (per-user and per-IP), and
Expand Down Expand Up @@ -173,9 +213,6 @@ below is kept for reference — it is what the shipped code implements.
`login_attempts` table with three partial indexes — plus
`CountTargetsForIP`, added by item 9 above against the same table.

Item 11 (Redis-backed rate limiter) has no prior design decisions
recorded — see `NEXT.md` for the level of detail available, make
reasonable calls on anything unspecified, note them in `PROGRESS.md`.

## Tier 3 — Infrastructure & Extensibility: NOT STARTED

Expand Down Expand Up @@ -212,6 +249,14 @@ project brief.
`config.go`/`engine.go` additions sit directly above theirs, so lifting
it onto `main` alone means resolving that adjacency by hand. Unmerged
and unpushed.
- `feat/redis-rate-limiter` — item 11, complete, 6 commits, branched
from `feat/named-sessions` at `345b2d7`, the tip of the chain, so this
branch carries items 8, 9, 10 and 11. Item 11 has no functional
dependency on any of them, but it adds a `config.go`/`engine.go` field
in the same region they did, so the same by-hand adjacency applies if
it is lifted onto `main` alone. It is also the only item so far that
adds a **direct third-party dependency** (`go-redis`) to `go.mod`.
Unmerged and unpushed.

Nothing else in flight. Each new session picks the top item off
`NEXT.md`, creates its own branch, and this section should be updated to
Expand Down
41 changes: 11 additions & 30 deletions docs/development/NEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,9 @@ patterns and note the assumption in `PROGRESS.md` — don't block on it.

---

## Tier 2 — Security & Monitoring

### 1. Redis-backed rate limiter (item 11)

`security.RateLimiter` already exists with one implementation
(in-memory, documented as not safe across multiple instances). This is
a **second real implementation**, not an interface-only integration —
Redis is configured infrastructure the host app wires in explicitly
(a connection string/client), the same category as Postgres, not an
arbitrary third-party internet service like HIBP. Ship a real
`security.RedisRateLimiter` (or wherever you decide it should live —
probably `security/`, matching where the in-memory one lives) using a
real, well-established Go Redis client library. `Config` gets a new
way to select/configure it (follow how `Users`/`Sessions`/etc. stores
are injected as already-constructed instances, not built internally
from a connection string — match that pattern here too).

---

## Tier 3 — Infrastructure & Extensibility

### 2. Argon2id as an additional trusted hasher (item 12)
### 1. Argon2id as an additional trusted hasher (item 12)

Second implementation of `security.Hasher`, not a replacement for
bcrypt. Real design question: how does the engine know which
Expand All @@ -44,7 +25,7 @@ dispatching `Compare`, while `Hash` always uses whichever algorithm is
currently configured. Build it this way unless you find a strong
reason not to; note the reasoning either way.

### 3. Additional storage backend beyond Postgres (item 13)
### 2. Additional storage backend beyond Postgres (item 13)

Every `store.X` interface already exists — implement all of them
against a second backend (SQLite is the most likely candidate per
Expand All @@ -55,7 +36,7 @@ specific assumptions baked into existing interface docs/behavior
`store/postgres/` implementations lean on these and a different
backend will need different real solutions, not just syntax swaps.

### 4. Cloud logger integrations (item 14)
### 3. Cloud logger integrations (item 14)

`logger.Logger` already exists with one implementation (console JSON).
Decide interface-only-vs-shipped-implementation the same way as
Expand All @@ -68,7 +49,7 @@ console-JSON-to-stdout is already the universal integration point
there's a specific strong reason a direct integration adds real value
over "the host app already captures stdout."

### 5. Extensible JWT claims (item 15)
### 4. Extensible JWT claims (item 15)

Let host apps attach their own data to access tokens. Read
`token/jwt.go`'s current claims struct and `JWTIssuer.Issue` before
Expand All @@ -79,7 +60,7 @@ signing-method check). Likely shape: `Issue` gains an optional
`ClaimsProvider` hook — pick whichever fits the existing `Issue`
call sites with the least disruption.

### 6. API keys / machine-to-machine auth (item 16)
### 5. API keys / machine-to-machine auth (item 16)

New concept, not a variant of an existing one — no human to prompt, so
this sits outside the second-factor system entirely (confirm this
Expand All @@ -91,7 +72,7 @@ values, not human passwords), and its own facade functions
(`GenerateAPIKey`, `RevokeAPIKey`, and something that validates a
presented key and returns which user/scope it belongs to).

### 7. Webhooks (item 17)
### 6. Webhooks (item 17)

Notify the host app on key events. Same question as everything else
that reaches outward: interface-only, zero shipped implementations
Expand All @@ -103,7 +84,7 @@ subset, not all of them) and wire it in wherever `audit.Record` is
already called for those events — don't build a second parallel event
bus.

### 8. Custom email templates (item 18)
### 7. Custom email templates (item 18)

Check `notify.EmailSender`/`notify.MagicLinkSender` as they exist
today first — there's a real chance this needs **no engine change at
Expand All @@ -121,19 +102,19 @@ than building something speculative to have built something.
automatic action — no auto-lock, no auto-config-change, nothing. Every
one of these produces information for a human to act on.

### 9. Weekly digest (item 19)
### 8. Weekly digest (item 19)
Reads `AuditStore`, summarizes in plain English, returns text. Nothing
else.

### 10. Support-ticket assistant (item 20)
### 9. Support-ticket assistant (item 20)
Read-only diagnosis ("why can't user X log in") — queries
`AuditStore`/`UserStore`/session state, produces an explanation, never
touches anything.

### 11. Config tuning advisor (item 21)
### 10. Config tuning advisor (item 21)
Produces a report of suggested config changes. Never applies them.

### 12. Ask-AI widget (item 22)
### 11. Ask-AI widget (item 22)
The most complex of the four. Needs its own full design pass before
any code — at minimum: an LLM provider interface (zero shipped
implementations, host brings their own key/provider, same pattern as
Expand Down
75 changes: 75 additions & 0 deletions docs/development/PROGRESS.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,78 @@ items 8 and 9 did not recur this session. Still unfixed, still worth its
own small branch.

Next in queue: item 11, the Redis-backed rate limiter.

## 2026-09-05 — Redis-backed rate limiter (item 11)

Branch: `feat/redis-rate-limiter` (6 commits, unmerged, unpushed,
branched from `feat/named-sessions` at `345b2d7` — the tip of the chain,
so this branch carries items 8, 9, 10 and 11).

Built: `security/RedisRateLimiter`, a second real implementation of the
existing `security.RateLimiter`, so counters live in Redis instead of a
per-process Go map. `security/redisratelimiter.go` holds the type, two
constructors and the Lua; `security/errors.go` gains three sentinels;
`Config.RateLimiter` accepts an already-constructed limiter and
`engine.go` falls back to the in-process default only when it is nil.
Nothing in `auth/` changed — every call site already held the interface.

Assumptions and calls made, none of which `NEXT.md` specified:

- **`github.com/redis/go-redis/v9`**, and the injected type is that
library's own `redis.Scripter` rather than `*redis.Client` or a bespoke
narrow interface. Client, ClusterClient, Ring and UniversalClient all
satisfy it, `redis.NewScript`'s EVALSHA→EVAL fallback comes along for
free instead of being reimplemented, and a fake stays writable via
`redis.NewCmdResult`/`redis.ErrNoScript`. This is the engine's first
direct third-party dependency of this kind — justified on `NEXT.md`'s
own terms: Redis is configured infrastructure, the same category as
Postgres and `lib/pq`, not an internet service like HIBP.
- **One Lua script per `Allow`.** INCR and PEXPIRE as two round trips
lets two replicas each arm their own window over one key, which is
precisely the bug this item exists to fix.
- **`PEXPIRE` only when `INCR` returns 1** (or when `PTTL` reports no
expiry, which self-heals a counter left without one). Arming it on
every call is the more obvious idiom and is wrong: it turns a blocked
client's own retries into a permanent block, and it would also break
parity with the in-memory limiter's fixed window.
- **Fixed-window parity is deliberate.** Same allow/deny arithmetic as
`InMemoryRateLimiter` (calls 1..limit pass, limit+1 denied, window
never extended) so the two are interchangeable. A sliding window would
be a different feature with a different cost, not an improvement
smuggled into this one.
- **Two positional constructors** (`NewRedisRateLimiter` and
`...WithPrefix`) over functional options, matching every other
constructor in the repo. Default prefix `cryden:ratelimit:` so a
counter can never collide with a host app's own keys; an empty prefix
is legal and means raw keys.
- **Windows under 1ms are rejected, not rounded.** `PEXPIRE` cannot
express them. This is the one place the two implementations are not
interchangeable, and it is documented as such rather than papered over.
- **Fail-closed left as it was.** All three call sites already propagate
a limiter error, so wiring Redis makes it a hard dependency of SignUp,
Login and RequestMagicLink. Changing caller behaviour was out of scope
for this item; instead the trade-off is documented, along with the
fail-open wrapper a host can write against the interface. The error is
wrapped, never `ErrRateLimited`, so callers can still tell "limit hit"
from "limiter broken".
- **Exactly one key per call**, so Redis Cluster needs no special
handling and the script never spans hash slots.
- Replaced a stale comment in `go.mod` that claimed the module proxy was
unreachable; it is, and go-redis is now a direct require.

Verification: `gofmt -l .` clean, `go build ./...`, `go vet ./...` and
`go test ./...` all clean, `go test -race` clean, and the smoke test
passes all 58 checks over ten scenarios.

**The one real gap: no Redis server was reachable here** (no daemon,
`docker info` unavailable), so the Lua was executed only against a
stand-in that models its semantics, never by Redis itself. The Go side —
allow/deny arithmetic, the fixed window, prefixing, the EVALSHA→EVAL
fallback, fail-closed propagation through the engine — is genuinely
tested; the script's own behaviour on a real server is not. Rather than
claim otherwise, the smoke test takes `REDIS_ADDR` and runs every
scenario against a live server, namespacing and cleaning up its own
keys, and `docs/testing/redis-rate-limiter.md` opens with the two
commands that close the gap. Worth doing before this branch is merged.

Next in queue: item 12, Argon2id as an additional trusted hasher.
Loading
Loading