Skip to content

test.md

Latest commit

 

History

History
404 lines (333 loc) · 24.6 KB

test.md

File metadata and controls

404 lines (333 loc) · 24.6 KB
Raw

AegisRate Gateway - Test, Review and Debug Audit

Date of Audit: 2026-09-12 (Solar Hijri: 1405-06-22) Status: Verified (33/33 assertions passing, 900 req/sec throughput, clean process exit) Environment: Node.js v24.18.0, Redis 7 (alpine, container aegis-redis), Windows 10

1. What This Audit Covered

A full review and debug pass over every source file, followed by black-box probing of the running gateway. The previous edition of this document reported 17/17 passing tests. Those tests passed, but their assertions were too loose to detect the defects below — for example assert(count200 > 0) on a burst test that was admitting only 4 of 25 requests against a bucket with capacity 20.

The method was:

  1. Read all 13 source files, both Lua scripts, and the dashboard client.
  2. Run the existing suite against a flushed Redis to establish a baseline.
  3. Probe the running gateway with targeted scripts for each suspected defect, so every issue below is backed by observed output rather than by code reading alone.
  4. Fix each confirmed defect.
  5. Re-run the probes to confirm the fix, and rewrite the test suite so each defect has a regression guard that fails if the bug returns.

2. Developer Onboarding and Architecture Overview

Purpose

AegisRate is a distributed reverse-proxy API Gateway that implements rate limiting at the edge. It combines Token Bucket and Sliding Window Counter algorithms using atomic Redis Lua scripts (EVALSHA), supported by an in-memory L1 lease cache, upstream health monitoring, and graceful degradation.

Request Path

request
  -> extractClientIdentifier()      resolve tenant (API key, else trusted-hop IP)
  -> healthMonitor.getMultiplier()  adaptive capacity scale M
  -> L1 memoryLease.tryConsume()    local hit: no network call
  -> L2 Redis Lua (EVALSHA)         atomic evaluation, optional batch lease
  -> green  -> proxy upstream -> post-execution settlement
     yellow -> stale cache or 202
     red    -> 429 (or shadow pass-through when shadow mode is on)

Directory Structure

ratelimit/
├── src/
│   ├── config.js               # Route costs, quotas, cluster settings, override sanitizer
│   ├── redis/
│   │   ├── client.js           # Redis client, circuit breaker, Pub/Sub, shutdown
│   │   ├── luaManager.js       # Lua script compiler and SHA cache
│   │   └── lua/
│   │       ├── token_bucket.lua    # Continuous refill token bucket script
│   │       └── sliding_window.lua  # Weighted sliding window counter script
│   ├── limiter/
│   │   ├── limiterService.js   # Coordinator: L1 lease, refill coalescing, Redis Lua, fallback
│   │   ├── memoryLease.js      # L1 in-memory lease manager for hot keys
│   │   ├── fallbackLimiter.js  # Autonomous in-memory limiter for Redis outages
│   │   └── costCalculator.js   # Route cost evaluator and settlement calculator
│   ├── adaptive/
│   │   └── healthMonitor.js    # Rolling observer for upstream latency and 5xx errors
│   ├── gateway/
│   │   ├── server.js           # Reverse proxy, control plane, WebSocket telemetry, shutdown
│   │   └── middleware.js       # Decision engine, identity resolution, RFC 6585 headers
│   ├── upstream/
│   │   └── mockUpstream.js     # Simulated backend services for local testing
│   ├── metrics/
│   │   └── metricsCollector.js # Metrics aggregator and WebSocket broadcaster
│   └── dashboard/              # Browser dashboard (http://localhost:8080/dashboard)
├── tests/
│   ├── verification.js         # Integration suite (10 cases / 33 assertions)
│   └── benchmark.js            # Throughput and concurrency test script
├── README.md                   # Setup and usage guide
├── tech.md                     # Technical and architectural specification
└── test.md                     # This audit

3. Issue Log and Root Cause Analysis

Eleven defects were confirmed. Severity reflects impact on correctness or on the security of the limiter itself.

ID Severity File Description and Root Cause
ISS-101 HIGH src/gateway/middleware.js Degradable routes never enforced the hard limit. The Yellow branch condition was decision.tier === 'yellow' || (rule.degradable && staleCache.has(path) && decision.remaining > -10). In the Red branch the Lua script clamps remaining to math.max(0, ...), so remaining > -10 was always true. Once any cached copy existed, a fully-throttled client kept receiving 200 Served-From-Stale-Cache forever.
ISS-102 HIGH src/limiter/limiterService.js
src/limiter/memoryLease.js		 L1 lease stampede destroyed tokens and over-debited Redis. Every request in a burst missed the (still empty) L1 lease and independently asked Redis for a 10-token batch. Each grant overwrote the previous lease via leases.set(), discarding its unspent tokens. A burst therefore debited Redis several times over while admitting far fewer requests than the quota allowed.
ISS-103 HIGH src/gateway/server.js Control plane was unauthenticated and un-rate-limited. POST /api/control was registered before the limiter middleware and had no authorization check. Any caller could set shadowMode: true — which disables enforcement — or rewrite route quotas, and the change was broadcast to every node over Redis Pub/Sub. The SET_CONFIG WebSocket message had the same reach.
ISS-104 HIGH src/gateway/server.js Proxy silently discarded non-JSON request bodies. Only express.json() was mounted, and the proxy forwarded JSON.stringify(req.body) only when req.body had keys. Form posts, uploads, plain text, and any other content type reached the upstream with an empty body and no error.
ISS-105 MEDIUM src/gateway/middleware.js X-Forwarded-For used the leftmost, client-controlled entry. X-Forwarded-For is appended hop by hop, so the leftmost value is whatever the client sent. Taking forwardedIps[0] let a client behind a trusted proxy mint a fresh quota per request by rotating the header.
ISS-106 MEDIUM src/gateway/middleware.js Stale cache could replay one client's response to another. setStaleCache(path, data) is keyed on path alone and the entry is served to any client that degrades on that route. Nothing checked whether the body was safe to share, so marking a personalized route degradable would leak data across tenants.
ISS-107 MEDIUM src/limiter/limiterService.js Grace buffer was sized in the wrong unit for Sliding Window. The buffer was always floor(rule.capacity * 0.2). Under Sliding Window the enforced quota is slidingLimit, so /api/ping (capacity 20, slidingLimit 60) got a Yellow band of 4 instead of 12 — a third of the intended width.
ISS-108 MEDIUM src/redis/client.js
src/gateway/server.js		 No shutdown path; Redis ping timer not unref'd. The previous audit's ISS-005 claimed all background timers were unref'd, but the health-ping setInterval in client.js was missed, and neither Redis connection was ever closed. The old test suite hid this by calling process.exit() in its finally block.
ISS-109 LOW src/limiter/memoryLease.js
src/gateway/middleware.js		 RateLimit-Reset was a placeholder on L1 lease hits. Lease results carried no reset_ms, so the middleware's decision.reset_ms || decision.retry_ms || 1000 fallback reported a constant 1 second regardless of actual bucket state.
ISS-110 LOW src/dashboard/public/app.js Unescaped innerHTML sink fed by untrusted input. prependEvent() interpolated evt.path — a raw request path from any caller — directly into innerHTML. Node's HTTP parser currently rejects raw < and > in a request target, so no live exploit was reproducible, but the sink is one parser change or one new event field away from being one.
ISS-111 LOW src/metrics/metricsCollector.js Unguarded ws.send(). A socket can close between the readyState === 1 check and the write. An exception there would propagate out of recordEvent() and fail the request that produced the telemetry.

Also reviewed, deliberately not changed

  • X-API-Key is unauthenticated. Any caller can rotate the header for a fresh quota. This is inherent to a gateway that has no identity provider in front of it; it is now documented in the README rather than silently assumed.
  • Yellow-tier requests consume quota without being served. The token bucket debits cost before returning allowed = 0. The balance is bounded at roughly -grace_buffer (the next request fails the grace check), so this is a deliberate back-pressure choice, not a leak.
  • Prefix route matching. /api/pingXYZ inherits the /api/ping rule via startsWith. Correct for the documented design; flagged here only so it is a conscious choice.

4. Evidence: Behaviour Before the Fixes

All figures are from probes against the running gateway with a flushed Redis.

Probe Observed before Expected
40 sequential requests to /api/cached-news (capacity 10, cost 2) 200: 40, 429: 0 — every response carried X-RateLimit-Tier: red, RateLimit-Remaining: 0, X-Graceful-Degradation: Served-From-Stale-Cache the bucket should hard-fail with 429 once past the grace band
30 concurrent requests to /api/ping (capacity 20, cost 1) 200: 11, 202: 4, 429: 15 roughly 20 admitted
12 concurrent requests to /api/ping Redis balance after the burst: tokens = -1.990 — about 22 tokens debited for 12 requests 12 tokens debited
POST /api/control {"shadowMode":true} with no credentials 200 {"status":"updated","shadowMode":true} — enforcement disabled cluster-wide rejected
POST /api/control {"routes":{"/api/ping":{"capacity":999999}}} then 30 requests 200s: 30 — the limit was gone rejected
40 requests with a rotating X-Forwarded-For 200s: 40 — one fresh bucket per spoofed value one shared bucket
POST /api/ai-generate with Content-Type: text/plain and body raw-text-payload upstream received an empty body and fell back to its default prompt the exact bytes forwarded
Process exit after limiterService.initialize() still alive after 6s; active resources {Timeout: 1, TCPSocketWrap: 2} clean exit
78 sequential Sliding Window requests (slidingLimit 60) Yellow band of 4 responses 12

5. Applied Fixes

Fix 1 — Red tier hard-fails on degradable routes (ISS-101)

  • File: src/gateway/middleware.js
  • Change: the degradation branch now triggers on decision.tier === 'yellow' only. A warm stale cache no longer overrides a Red decision, so an exhausted degradable route returns 429 as the specification requires.

Fix 2 — Lease refill coalescing and accumulation (ISS-102)

  • Files: src/limiter/limiterService.js, src/limiter/memoryLease.js
  • Change: LimiterService keeps a pendingLeases map holding the single in-flight Redis refill per key. A request that misses L1 joins that refill instead of issuing its own, retrying for up to LEASE_REFILL_ROUNDS = 3 rounds before falling back to its own evaluation. grantLease() now adds to a live lease instead of replacing it, so a batch that lands while tokens remain no longer destroys them.

Fix 3 — Authorized control plane with sanitized overrides (ISS-103)

  • Files: src/gateway/server.js, src/config.js, src/limiter/limiterService.js
  • Change: POST /api/control and the SET_CONFIG WebSocket message both go through isAuthorizedController() — a matching X-Admin-Token when AEGIS_ADMIN_TOKEN is configured, loopback-only otherwise. Route overrides pass through sanitizeRouteOverrides() (in config.js, so both the HTTP path and the Pub/Sub listener use it): unknown route keys are dropped and numeric fields must be finite and positive.

Fix 4 — Faithful request-body proxying (ISS-104)

  • Files: src/gateway/server.js, src/upstream/mockUpstream.js
  • Change: express.raw({ type: () => true }) is mounted after express.json(), so any content type the JSON parser skips is preserved as a Buffer. serializeRequestBody() forwards that Buffer untouched. The proxy also stops forwarding content-encoding and content-length from the upstream, since fetch already decoded the body and the gateway re-serializes it. A /api/echo-body endpoint was added to the mock upstream so the round trip is testable.

Fix 5 — Trusted-hop identity resolution (ISS-105)

  • File: src/gateway/middleware.js
  • Change: X-Forwarded-For is walked right to left and the first untrusted hop is used as the identity. A client-prepended value is ignored. IPv4-mapped IPv6 addresses are normalized once, in normalizeIp(), for both the socket address and each forwarded hop.

Fix 6 — Stale cache accepts only public bodies (ISS-106)

  • File: src/gateway/middleware.js
  • Change: isPubliclyCacheable() gates every insertion. A response is cached only if it sets Cache-Control: public without private/no-store, carries no Set-Cookie, and does not Vary on Authorization, Cookie, or *. A degradable route whose body is not provably public soft-throttles with 202 instead.

Fix 7 — Grace buffer scaled to the active algorithm (ISS-107)

  • File: src/limiter/limiterService.js
  • Change: the buffer is computed from slidingLimit under Sliding Window and from capacity under Token Bucket.

Fix 8 — Shutdown path and unref'd ping timer (ISS-108)

  • Files: src/redis/client.js, src/gateway/server.js, tests/verification.js, tests/benchmark.js
  • Change: the health-ping interval is stored and .unref()ed; redisManager.shutdown() clears it and quits both connections. startGateway() now returns a shutdown() that stops the heartbeat, terminates WebSocket clients, closes the gateway and mock upstream, and then releases Redis. Both test entry points call it instead of process.exit(), so a leaked handle now shows up as a hanging test rather than being masked.

Fix 9 — Honest lease headers (ISS-109)

  • Files: src/limiter/memoryLease.js, src/limiter/limiterService.js
  • Change: a lease hit reports reset_ms as the lease's remaining lifetime, and a fresh grant reports the real post-grant balance rather than the granted amount.

Fix 10 — DOM-safe event rendering (ISS-110)

  • File: src/dashboard/public/app.js
  • Change: prependEvent() builds the row from document.createElement and textContent instead of an innerHTML template, so no request-controlled value can be parsed as markup.

Fix 11 — Telemetry cannot fail a request (ISS-111)

  • File: src/metrics/metricsCollector.js
  • Change: all writes go through safeSend(), which drops the client on error; broadcast() iterates a copy of the client set, and an error listener deregisters dead sockets.

Supporting change — benchmark accuracy

  • File: tests/benchmark.js
  • The worker loop incremented the shared completed counter before awaiting, so it counted dispatches rather than completions and raced other workers on the same variable. Workers now claim an index atomically, drain each response body so keep-alive can reuse the socket, and the summary reports completions, degraded responses, and transport errors separately.

6. Verification: Behaviour After the Fixes

Each probe from section 4, re-run against the fixed code.

Probe Before After
40 sequential /api/cached-news 200: 40, 429: 0 200: 6, 429: 34
30 concurrent /api/ping (capacity 20) 200: 11, 202: 4, 429: 15 200: 20, 202: 4, 429: 6
POST /api/control with no credentials 200, enforcement disabled 403 Forbidden, shadowMode unchanged
POST /api/control with a wrong token 200 403 Forbidden
Hostile override {cost: -5, capacity: 0} + unknown route key limit removed; key accepted override rejected (/api/ping still admits ~20 of 30); unknown key never entered config
40 requests, rotating spoofed leftmost X-Forwarded-For hop 200s: 40 200: 23, throttled: 17 — all share one bucket
text/plain POST body dropped upstream received {"received":"raw-text-payload","contentType":"text/plain","bytes":16}
Degradable route with a non-public body would be cached and replayed 202 Throttled-Deferred; no cross-client body replay observed
Degradable route with a public body cached still Served-From-Stale-Cache in the Yellow band, then 429
Process exit after shutdown() hung; {Timeout: 1, TCPSocketWrap: 2} exits on its own; only the two stdio pipes remain
78 sequential Sliding Window requests Yellow band of 4 200: 60, 202: 12, 429: 6

7. Test Suite Changes

The suite was rewritten. It went from 17 assertions across 7 cases to 33 assertions across 10 cases, and three changes matter beyond the count:

  1. Assertions are now bounded, not merely non-zero. The burst test asserts the admitted count lands in [16, 24] against a capacity of 20. The old count200 > 0 passed while the gateway admitted 4.
  2. Every case uses a fresh identity (freshKey()), so one test can no longer drain a bucket another depends on.
  3. Failures no longer abort the run. The old assert threw, so the first failure hid every later case. Failures are now collected and reported together.

New regression guards, one per high-severity defect: Test 6 (degradable hard limit), Test 8 (control-plane authorization and the override sanitizer), Test 9 (raw body passthrough), plus the bounded burst assertion in Test 2 and the Yellow-band width assertion in Test 10.

Two assertions in the old suite were asserting buggy behaviour and were corrected:

  • Old Test 5 drained /api/cached-news deep into the Red tier and then required a degraded response. It only passed because of ISS-101. It now exercises the Yellow band it was written for, and the Red-tier behaviour is asserted separately in Test 6.
  • Old Test 4 printed the follow-up status without asserting it. Asserting === 429 turned out to be flaky: after the 15-unit settlement the bucket sits within a token of the Yellow boundary, so a few hundred milliseconds of refill flips 429 to 202. The test now asserts the invariant that actually matters — the settled debt costs the client its next call (status !== 200) — and adds a control client with no debt that is still admitted on the same route, proving the throttle came from the debt.

8. Test Results

8.1. Integration Test Suite (npm test)

  • Cases: 10
  • Assertions: 33 passed, 0 failed
  • Exit code: 0, with the process terminating through shutdown() rather than process.exit()
====================================================
   AegisRate Gateway - Automated Verification Suite
====================================================

[AegisRate] Initializing Limiter Service & Redis Connection...
[Redis] Connected to socket
[Redis] Ready for commands
[LuaManager] Loaded script 'sliding_window' with SHA: f84a5696...
[LuaManager] Loaded script 'token_bucket' with SHA: 023943db...
[MockUpstream] Listening on port 8081
[AegisRate Gateway] Running on http://localhost:8080
[AegisRate Dashboard] Live at http://localhost:8080/dashboard
[LimiterService] Subscribed to dynamic config updates channel.

--- TEST 1: Basic Gateway & IETF Standard Headers ---
  [PASS] Endpoint /api/ping returns HTTP 200
  [PASS] Header RateLimit-Limit is present
  [PASS] Header RateLimit-Remaining is present
  [PASS] Header RateLimit-Reset is present
  [PASS] First request is in Green Tier
  [PASS] RateLimit-Limit reflects the configured capacity of 20 (got 20)
  [PASS] RateLimit-Reset is a real refill window, not a placeholder (got 5s)

--- TEST 2: Burst Traffic & 429 Hard Rate Limit ---
  [PASS] Blocked excess requests with 429: 17
  [PASS] Concurrent burst admits close to the configured capacity of 20 (admitted 19)
  [PASS] Blocked 429 response includes Retry-After header
     Burst summary: {"200":19,"202":4,"429":17}

--- TEST 3: Cost-Aware Dynamic Credit Weight ---
  [PASS] Search request succeeded
  [PASS] Query string is forwarded to upstream intact
  [PASS] Cost 3 exhausts the capacity-15 bucket within 7 requests (throttled 2)

--- TEST 4: Post-Execution Settlement (Heavy AI Route) ---
  [PASS] AI generate request succeeded
  [PASS] Upstream declared 15 compute units used
  [PASS] Settled 15-unit debt throttles the follow-up request (got 429)
  [PASS] A client with no settled debt is still admitted on the same route

--- TEST 5: Graceful Degradation (Yellow Tier) ---
  [PASS] News endpoint seeded
  [PASS] Soft throttled request handled via Graceful Degradation
     Degradation applied: Served-From-Stale-Cache

--- TEST 6: Degradable Routes Still Enforce the Hard Limit ---
  [PASS] Exhausted degradable route returns 429 instead of endless cached 200s ({"200":6,"429":14})
  [PASS] Degradable route does not admit every request (admitted 6 of 20)

--- TEST 7: Dynamic Shadow Mode ---
  [PASS] Control plane accepts a loopback request
  [PASS] Shadow mode allows traffic through despite exceeding limit
  [PASS] Tagged with X-RateLimit-Shadow-Exceeded header

--- TEST 8: Control Plane Requires Authorization ---
  [PASS] Control plane rejects an untokened caller (got 403)
  [PASS] Control plane accepts the configured admin token
  [PASS] Unknown route keys are rejected by the control plane sanitizer

--- TEST 9: Proxy Forwards Non-JSON Request Bodies ---
  [PASS] Upstream received the text/plain request (status 200)
  [PASS] Raw request body survives the proxy hop (upstream saw: {"received":"raw-text-payload","contentType":"text/plain","bytes":16})

--- TEST 10: Sliding Window Algorithm Switch ---
  [PASS] Sliding Window request succeeded
  [PASS] Algorithm dynamically switched to Sliding Window
  [PASS] Sliding window reports its own limit of 60, not the bucket capacity (got 60)
  [PASS] Yellow band is scaled to the sliding limit, not the bucket capacity ({"200":60,"202":12,"429":6})

====================================================
 ALL 33/33 TESTS PASSED SUCCESSFULLY!
====================================================

8.2. Throughput Benchmark (npm run benchmark)

================ Benchmark Results ================
 Total Requests:     1000
 Time Elapsed:       1.11s
 Throughput:         900.1 req/sec
 Completed:          1000
 Passed (200):       215
 Throttled (429):    710
 Degraded/other:     75
 Transport errors:   0
===================================================

Throughput is unchanged within run-to-run noise (900–967 req/sec across runs, against 940 before the fixes), so the refill-coalescing wait did not cost measurable latency. Admitted requests rose from 150 to ~215 for the same 1,000-request workload across 10 bench users, which is the recovered quota from ISS-102: those tokens were previously debited from Redis and then discarded.

9. Documentation Updated

  • README.md — corrected the test count, documented AEGIS_ADMIN_TOKEN, and added an Operational Notes section covering trusted-proxy configuration, the stale-cache publicity requirement, and the fact that X-API-Key is not authenticated.
  • tech.md — documented refill coalescing (§4.1), added the control-plane authorization section (§4.4), and stated the grace-buffer scaling rule in §2.2.

10. Known Limitations and Suggested Follow-Ups

These are out of scope for this pass but worth recording.

  1. Identity is unauthenticated. X-API-Key is accepted at face value, so quota isolation is only as strong as whatever sits in front of the gateway.
  2. Key cardinality is unbounded. Every distinct API key or client IP creates a Redis key with a 120s TTL. A high-cardinality attack inflates Redis memory; the container's maxmemory 256mb with volatile-lru limits the blast radius but will start evicting live buckets under that pressure.
  3. L1 leases ignore the adaptive multiplier. Once a lease is granted, adaptive throttling has no effect on that key until the lease expires (up to 5 seconds).
  4. Leases are lost on restart. Tokens held in an L1 lease are already debited from Redis, so a process restart forfeits them until the bucket refills.
  5. Degraded and blocked responses skip the health monitor. Only requests that reach the upstream feed healthMonitor.recordRequest(), so latency statistics describe admitted traffic only.
  6. No dedicated unit tests. Everything is verified end-to-end against a live Redis. Pure functions (CostCalculator, sanitizeRouteOverrides, isPubliclyCacheable, the XFF walk) would be cheaper and more precise to cover directly.