## Spec ### Problem Statement [required] Stage 1a (MR [labkit-ruby!270](https://gitlab.com/gitlab-org/ruby/gems/labkit-ruby/-/merge_requests/270)), Stage 1b (MR !271), and Spec 8 (MR !272) shipped `Labkit::RateLimit::Limiter#check` returning a `Result` object (`matched?`, `exceeded?`, `action`, `rule`, `error?`). MR !273 deliberately removed per-request logging from the hot path to protect logging infrastructure during high-traffic attack scenarios. Callers (Rack middleware, Rails controllers) need to expose `RateLimit-*` response headers to clients per RFC 6585 / draft-ietf-httpapi-ratelimit-headers. Currently callers would need to re-query Redis to compute remaining counts — this spec adds `remaining`, `reset_at`, and `limit` fields to the existing `Result` object plus a `to_response_headers` convenience method, surfacing that state without an extra round trip. ### Non-Goals [required] - No Rails or RackAttack changes (gem-only) - No changes to `Limiter#check` signature - No new `.evaluate()` method — `Limiter#check` is the single API - No new `RuleState` class — `remaining`/`reset_at`/`limit` live directly on `Result` - No changes to `configure` block semantics (yields `config`, not `self`) - No persistent config storage - No support for `RateLimit-Policy` header (future) - **No per-request logging in the success path** — MR !273 removed this deliberately; Stage 1c must not reintroduce it ### What Stage 1c Adds **Extended `Labkit::RateLimit::Result`** (`lib/labkit/rate_limit/result.rb`) Current (Stage 1b): ```ruby Result = Data.define(:matched, :exceeded, :action, :rule, :error) ``` After Stage 1c: ```ruby Result = Data.define(:matched, :exceeded, :action, :rule, :error, :remaining, :reset_at, :limit) do def initialize(matched:, exceeded: false, action: nil, rule: nil, error: false, remaining: nil, reset_at: nil, limit: nil) super end def matched? = matched def exceeded? = exceeded def error? = error # Returns RFC-compliant response header hash, or {} when no rule matched or an error occurred. def to_response_headers return {} unless matched? && !error? { "RateLimit-Limit" => limit.to_s, "RateLimit-Remaining" => remaining.to_s, "RateLimit-Reset" => reset_at.to_i.to_s } end end ``` The three new fields are `nil` when `matched? == false` or `error? == true`. `remaining` = `[limit - count, 0].max` — never negative. `reset_at` is best-effort: derived from a `redis.ttl` call _after_ `redis.incr`. Not atomic. - TTL > 0: `Time.now + ttl` - TTL == 0, -1, or -2: fallback `Time.now + period` **Changes to `Labkit::RateLimit::Evaluator`** (`lib/labkit/rate_limit/evaluator.rb`) - `evaluate_rule` calls `redis.ttl(key)` after `incr_with_ttl`; computes `remaining` and `reset_at` - **No logging in the success path** — the `redis.ttl` call must not emit any log entries when it succeeds - Errors from `redis.ttl` are caught by the existing `StandardError` rescue in `#check` → `log_error` — this is correct (errors are rare; one log entry per error occurrence is acceptable) - Partial failure (incr succeeds, ttl raises): fail-open — caught by existing rescue, returns `Result.new(matched: false, error: true)`; counter may have been incremented - `redis.expire` on first write is unchanged (count == 1 check) ### Acceptance Criteria [required] **Scenario A — Result structure** - Given a matched rule - When `Limiter#check` is called - Then returns a `Result` with `matched? == true`, `remaining` (Integer ≥ 0), `reset_at` (Time), `limit` (Integer) **Scenario B — `remaining` floors at 0** - Given count exceeds limit - When `Result` is built - Then `remaining == 0`, never negative **Scenario C — Header output** - Given a matched rule with limit: 10, remaining: 3, reset_at: Time at unix 1700000100 - When `to_response_headers` is called - Then returns `{ "RateLimit-Limit" => "10", "RateLimit-Remaining" => "3", "RateLimit-Reset" => "1700000100" }` **Scenario D — Empty headers when no rule matched** - Given no rules match the identifier - When `to_response_headers` is called on the result - Then returns `{}` **Scenario E — Empty headers on Redis error** - Given `redis.incr` raises `RuntimeError` - When `Limiter#check` is called - Then `result.error? == true` and `to_response_headers` returns `{}` **Scenario F — `reset_at` from `redis.ttl` > 0** - Given `redis.ttl(key)` returns N > 0 and time is frozen - When `Result` is built - Then `reset_at == Time.now + N` **Scenario G — TTL fallback when key has no expiry** - Given `redis.ttl(key)` returns -1 and time is frozen - When `Result` is built - Then `reset_at == Time.now + period` **Scenario H — TTL fallback when key is missing** - Given `redis.ttl(key)` returns -2 and time is frozen - When `Result` is built - Then `reset_at == Time.now + period` **Scenario I — TTL of 0** - Given `redis.ttl(key)` returns 0 and time is frozen - When `Result` is built - Then `reset_at == Time.now` (within 1 second) **Scenario J — First-write `expire` still called** - Given count == 1 (first write) - When `incr_with_ttl` runs - Then `redis.expire` is still called **Scenario K — Existing Result fields unchanged** - Given existing callers reading `result.matched?`, `result.exceeded?`, `result.action`, `result.rule`, `result.error?` - When `Limiter#check` is called - Then all existing fields behave identically to Stage 1b — new fields are additive only **Scenario L — No rule match: new fields are nil** - Given no rules match the identifier - When `Limiter#check` is called - Then `result.remaining == nil`, `result.reset_at == nil`, `result.limit == nil` **Scenario M — Error: new fields are nil** - Given Redis raises an error - When `Limiter#check` is called - Then `result.remaining == nil`, `result.reset_at == nil`, `result.limit == nil`, `result.error? == true` **Scenario N — Partial failure: ttl raises after successful incr** - Given `redis.incr` succeeds but `redis.ttl` raises `RuntimeError` - When `Limiter#check` is called - Then returns `Result` with `matched: false, error: true` (fail-open); counter may have been incremented; `log_error` is called once **Scenario O — `:log` rule matched: headers present** - Given a `:log` rule that matched - When `to_response_headers` is called - Then returns headers with `RateLimit-Remaining` reflecting current remaining count **Scenario P — `:block` result (not exceeded): headers present** - Given a `:block` rule that matched but not exceeded (remaining: 50) - When `to_response_headers` is called - Then returns headers with `RateLimit-Remaining` => "50" **Scenario Q — No per-request logging in the success path** - Given a matched rule and a healthy Redis - When `Limiter#check` is called - Then no logger calls are made (the logger spy receives zero invocations) ### Security Considerations [required] - `reset_at` exposed in headers as Unix timestamp — no sensitive data leaked - `to_response_headers` must NOT include Redis key or identifier values - `@config` (redis client) is process-global; tests must reset it between examples (see test isolation below) ### Rollout & Backwards Compatibility [required] - All new `Result` fields default to `nil` — no existing callers are broken - `Limiter#check` signature is unchanged - No new public methods on `Labkit::RateLimit` module - Redis key format is unchanged; no counter migration needed - Self-managed: no impact — gem-only change, existing counter behaviour preserved - gem version bump required; Rails `labkit` pin must be updated in Stage 2 issues before consuming `to_response_headers` ### Validation Loop / Verification Process [required] ```bash # In labkit-ruby.rate-limit-stage-1c/ mise exec -- bundle exec rspec spec/labkit/rate_limit/result_spec.rb spec/labkit/rate_limit/evaluator_spec.rb spec/labkit/rate_limit_spec.rb --format documentation mise exec -- bundle exec rubocop lib/labkit/rate_limit/result.rb lib/labkit/rate_limit/evaluator.rb ``` Test output must be posted as a comment on the MR before requesting human review. ### Observability [optional] `to_response_headers` is a pure transformation of data already computed during the check. No new log fields. The `RateLimit-Remaining` and `RateLimit-Reset` values surfaced to callers are the primary observability artifact. Error logging (`log_error`) is preserved for Redis failures — these are rare and acceptable to log. ## Files to Modify | File | Change | |------|--------| | `lib/labkit/rate_limit/result.rb` | Add `remaining`, `reset_at`, `limit` fields; add `to_response_headers`; update `initialize` defaults | | `lib/labkit/rate_limit/evaluator.rb` | `evaluate_rule`: add `redis.ttl` call after `incr_with_ttl`; populate `remaining`, `reset_at`, `limit` on `Result`; no new logging in success path | | `spec/labkit/rate_limit/result_spec.rb` | Add scenarios A–Q above | | `spec/labkit/rate_limit/evaluator_spec.rb` | Add TTL scenarios F–J, N, Q; extend `FakeRedis` with `#ttl` | | `spec/spec_helper.rb` | Add `around` hook to reset module `@config` between examples | ## Test Isolation (spec_helper.rb) ```ruby around do |example| prev = Labkit::RateLimit.instance_variable_get(:@config) example.run ensure Labkit::RateLimit.instance_variable_set(:@config, prev) end ``` Tests asserting on `reset_at` or `RateLimit-Reset` must use `freeze_time`. --- _Spec updated 2026-04-29: revised to align with Stage 1b (MR !271) and Spec 8 (MR !272) APIs; logging constraint from MR !273 incorporated. Original spec preserved in first comment. Adversarial review and logging constraint rationale in subsequent comments._