fix(api-scenarios): query engine, bulk endpoint, JWT alg pinning, rate limiter - #11
Merged
SkinnnyJay merged 2 commits intoAug 22, 2026
Merged
Conversation
… engine, real bulk endpoint, JWT alg pinning, real rate limiter, store-integrity fixes The api-scenarios subproject advertised CRUD+filtering+pagination+search, bulk ops, JWT auth, and rate limiting, but most of it was decorative: the mock repo ignored every filter/search/page, the bulk endpoint returned "This is a mock implementation" and persisted nothing, JWT verification trusted the token's alg header, the rate limiter emitted hardcoded headers with no enforcement, and two response-filtering paths mutated the store. The shipped test suite stayed green because its assertions accepted the broken behavior (the bulk tests literally accepted HTTP 404 as a pass). Correctness - MockUserRepository.findMany now honors filters (eq/ne/gt/gte/lt/lte/in/ nin/like/regex/exists, dot-path fields), case-insensitive search over the requested fields, multi-key stable sort, and real offset pagination with truthful hasNext/hasPrev/total. Was: return first `limit` rows, ignore everything else, hasNext hardcoded false. - CrudService.sanitizeQuery dropped query.search entirely, so every controller search term was discarded before reaching the repo and matched all rows. Now preserved. - Duplicate-user checks were a single AND filter array (email AND username), which matched nobody; replaced with one indexed eq query per unique field (OR semantics) on both create and update. - Optimistic locking is now enforced: update() with a stale version throws VersionConflictError (mapped to 409); every successful write bumps version. Security - JWT verification pins algorithms:["HS256"] (RFC 8725 §3.1). Previously jwt.verify was called with no allowlist, so an attacker-chosen alg header (incl. alg:none) was honored. - Rate limiter replaced the placeholder (limit=1000/remaining=999, no enforcement) with a real per-IP fixed-window limiter: accurate X-RateLimit-* headers, 429 + Retry-After past the limit, lazy pruning, configurable via RATE_LIMIT_MAX / RATE_LIMIT_WINDOW_MS. Store integrity (data-loss bugs) - filterUserData shallow-copied the user, so `delete filtered.profile. phoneNumber` on a privacy-filtered view PERMANENTLY stripped the phone number from the stored entity the first time a non-admin viewed it. Now copies nested objects. Also removed a dead double showEmail check. - Avatar upload replaced the whole profile object (wiping timezone/language/ phoneNumber on every upload); now merges into the current profile. - Partial profile/preferences updates merged against DEFAULTS, silently resetting sibling fields (e.g. updating a notification toggle reset the user's theme). Now merges against the user's current values. Bulk endpoint (was a mock) - POST /api/v1/users/bulk now executes create/update/delete via new CrudService.bulkUpdate/bulkDelete, with per-item validation, in-batch and cross-store uniqueness checks on create, password hashing, sensitive-field stripping, and per-item result reporting. Registered in the test server too (it 404'd there before, which is why the old test accepted 404). Performance - Email/username lookups use secondary hash indexes: O(1) vs the O(n) scan. Bench @ 100k users: 2.44 ms -> 0.0065 ms/lookup (~377x). - Topic broadcasts use a topic->subscribers index (O(subscribers)) instead of scanning every connection and re-resolving each subscription id per broadcast. Subscriber resolution @ 10k conns/100 subs: 0.585 ms -> 0.0002 ms (~2500x). Streaming correctness - Heartbeat reaper now actually closes the timed-out socket before dropping tracking state (was leaking the exact sockets that stopped responding). - Room publish now requires membership (any connection could inject into any room without joining); publish with no topic and no room is rejected instead of falsely confirmed. - Subscription filters (advertised in capabilities) are now evaluated on topic delivery. Unsubscribe-by-id only affects the caller's own subscription. - Heartbeat timer is unref()'d so it can't keep a finished process alive. Tests: +16 (new uplift-regressions.test.ts + 1 updated JWT assertion). Subproject suite 82 -> 98 green. Root `npm run test:all` 755 passed / 30 suites. tsc clean, eslint 0 errors.
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
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.
api-scenarios: the reference API now actually does what it advertises
This subproject is the sandbox's "full API" reference — CRUD, filtering, sorting, pagination, search, streaming, file upload, bulk ops, JWT, RBAC. Most of it was a facade. The suite stayed green because the tests asserted almost nothing: the bulk-operation tests accept HTTP 404 as a pass, i.e. "the endpoint doesn't exist" counted as success.
What was broken (all live at 44447fe)
Query engine did nothing.
MockUserRepository.findManyreturned the firstlimitrows and ignored filters, search, page, and sort.hasNextwas hardcodedfalse. On top of that,CrudService.sanitizeQuerysilently droppedquery.search, so even the controllers that built a search query had the term discarded before it reached the repo. Net effect:GET /users?search=nobody&role=adminreturned every user.Duplicate detection was a no-op. The create/update "already exists?" check used a single AND filter array (
email == X AND username == Y), which matches nobody unless one user has both — so it never caught a real duplicate by the field that actually collided.JWT verified with no algorithm allowlist.
jwt.verify(token, secret)trusts the token header'salg. That's the classic algorithm-confusion /alg:nonefootgun — RFC 8725 §3.1 requires pinning the accepted set.Rate limiter enforced nothing. It emitted
X-RateLimit-Limit: 1000 / Remaining: 999as static strings on every request. No counter, no 429, noRetry-After.Two response filters mutated the store.
filterUserDatadidconst filtered = { ...user }(shallow), thendelete filtered.profile.phoneNumberon privacy-filtered views — which permanently deleted the phone number from the stored entity the first time a non-admin viewed the user, becauseprofilewas shared by reference. Avatar upload replaced the entireprofileobject, wiping timezone/language/phone on every upload. Partialpreferences/profileupdates merged against defaults, so updating one notification toggle reset the user's theme.The bulk endpoint was a stub that returned
"note": "This is a mock implementation"and persisted nothing — and wasn't even registered in the test server (hence the 404-tolerant tests).Streaming leaks & gaps. The heartbeat reaper dropped tracking state but never closed the socket — leaking exactly the connections that stopped responding. Any client could publish into any room without joining it. Advertised subscription filters were never evaluated. Topic broadcasts scanned every connection and re-resolved each of its subscription ids on every message.
What changed
Correctness: real
findMany(all operators with dot-path fields, case-insensitive multi-field search, multi-key stable sort, true offset pagination with honest metadata);sanitizeQuerypreserves search; duplicate checks run one indexed query per unique field (OR); optimistic locking enforced (stale version → 409, version bumped per write).Security:
algorithms: ["HS256"]pinned on every verify path; real per-IP fixed-window rate limiter with accurate headers, 429 +Retry-After, lazy pruning, env-configurable.Store integrity:
filterUserDatacopies nested objects; avatar upload and partial updates merge against current values, not defaults/blank.Bulk:
POST /api/v1/users/bulkexecutes create/update/delete via newCrudService.bulkUpdate/bulkDelete— per-item validation, in-batch + cross-store uniqueness on create, password hashing, sensitive-field stripping, per-item results — and is registered in the test server.Streaming: reaper closes the socket before dropping state; room publish requires membership; empty publish rejected; subscription filters evaluated on delivery; unsubscribe-by-id is caller-scoped; heartbeat timer
unref()'d.Numbers (benchmarked on the build host)
Verification
uplift-regressions.test.ts, +1 updated JWT assertion). Every new test pins a bug that was green-under-broken-behavior before.npm run test:all: 755 passed / 30 suites, 0 failures.tsc --noEmitclean;eslint0 errors.— Lumen Industries