diff --git a/CLAUDE.md b/CLAUDE.md index 49742eb..d8017bc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,7 +154,7 @@ All prefixed `UPVECTOR_`: ## Implementation Status Phases 1-6 complete. Dense-vector CRUD + query + filtering + namespaces + production hardening + provider-backed `/upsert-data`/`/query-data`. -444 tests passing (310 unit, 60 integration, 74 SDK compatibility). +465 tests passing (331 unit, 60 integration, 74 SDK compatibility). Deep audits run per `docs/workflows/deep-audit.md`; open findings tracked under the `deep-audit` issue label. Production hardening includes structured JSON logging, graceful shutdown, health probes, request timeouts, Prometheus metrics (optional scrape token), body limits, binary vector round-trip protection, Redis client self-heal, and process-level error handlers. See `PLAN.md` for the full architecture and phase breakdown. diff --git a/README.md b/README.md index ff6337e..4ce3d68 100644 --- a/README.md +++ b/README.md @@ -147,15 +147,15 @@ curl -X POST http://localhost:8080/query-data \ ## API Compatibility -Implements the dense-vector subset of the [Upstash Vector REST API](https://upstash.com/docs/vector/api/endpoints), plus dense `/upsert-data` and `/query-data` through a configurable embedding provider. Validated by 383 tests including 74 using the real `@upstash/vector` SDK. +Implements the dense-vector subset of the [Upstash Vector REST API](https://upstash.com/docs/vector/api/endpoints), plus dense `/upsert-data` and `/query-data` through a configurable embedding provider. Validated by 465 tests including 74 using the real `@upstash/vector` SDK. | Surface | Status | Notes | |----------|--------|-------| | Dense `POST /upsert[/{namespace}]` | Supported | Dense vectors, metadata, optional `data`; re-upsert replaces omitted metadata/data | -| Dense `POST /query[/{namespace}]` | Supported | KNN + metadata filtering; batch query supported | +| Dense `POST /query[/{namespace}]` | Supported | KNN + metadata filtering; batch query supported (up to 100 queries — also applies to `/query-data`) | | `POST /upsert-data[/{namespace}]` | Supported | Dense only; requires `UPVECTOR_EMBEDDING_PROVIDER`; stores raw text as `data` | | `POST /query-data[/{namespace}]` | Supported | Dense only; requires `UPVECTOR_EMBEDDING_PROVIDER`; same result shape as `/query` | -| `GET/POST /fetch[/{namespace}]` | Supported | IDs and prefix; include metadata/vectors/data | +| `GET/POST /fetch[/{namespace}]` | Supported | IDs (unbounded) and prefix (first 1000 results); include metadata/vectors/data | | `DELETE/POST /delete[/{namespace}]` | Supported | IDs, prefix, or filter | | `POST /update[/{namespace}]` | Supported | Dense vector, data, OVERWRITE and PATCH metadata | | `GET/POST /range[/{namespace}]` | Supported | Offset cursor pagination | @@ -166,6 +166,7 @@ Implements the dense-vector subset of the [Upstash Vector REST API](https://upst | Sparse indexes and sparse vectors | Unsupported | Requests with `sparseVector` are rejected; see [sparse/hybrid architecture](./docs/architecture/sparse-hybrid.md) | | Hybrid indexes and fusion/query modes | Unsupported | No dense+sparse fusion yet | | Resumable query endpoints | Unsupported | Return explicit `501`; no cursor/session parity | +| Empty `filter` strings | Intentional deviation | Upstash documents `filter: ""` as "no filter"; up-vector rejects it with `400` (almost always a client bug — dynamic filter builders producing `""`). Pass `filter` as `undefined` for unfiltered queries | | Upstash-hosted embedding models | Partial | OpenAI-compatible self-host/provider path only, not Upstash's hosted model catalog | ### Metadata Filtering @@ -301,11 +302,11 @@ bun run typecheck # TypeScript check ### Testing -383 tests across three tiers: +465 tests across three tiers: | Tier | Tests | Purpose | |------|-------|---------| -| **Unit** | 249 | Filter parser, embedding providers, vector encode/decode, score normalization, key naming, middleware/config hardening | +| **Unit** | 331 | Filter parser, embedding providers, vector encode/decode, score normalization, key naming, middleware/config hardening, route validation | | **Integration** | 60 | End-to-end REST behavior against Redis Stack, including raw-text data endpoints | | **SDK Compatibility** | 74 | Real `@upstash/vector` SDK against up-vector | diff --git a/docs/RUNBOOK.md b/docs/RUNBOOK.md index c7f7c94..5c22a8e 100644 --- a/docs/RUNBOOK.md +++ b/docs/RUNBOOK.md @@ -17,9 +17,11 @@ Principles: - Every automated signal lands as an issue with a stable label. No signal lives only in a log nobody reads. -Verified baseline at runbook creation (2026-08-22): **383 tests** — 249 unit / -60 integration / 74 SDK compatibility — green on Bun 1.3.6-pinned CI, local Bun -1.3.14, Redis Stack `7.4.0-v8`, hono `4.13.3`. +Verified baseline (2026-09-06, deep audit #2): **465 tests** — 331 unit / +60 integration / 74 SDK compatibility — green on Bun 1.4.0-pinned CI, local +Bun 1.3.14, Redis Stack `7.4.0-v8`, hono `4.13.3`, `bun audit` clean. +History: 383 tests at runbook creation (2026-08-22); 444 after the 2026-08-22 +audit remediation wave. ## Automation inventory @@ -85,33 +87,15 @@ Before tagging or publishing any release: ## Deep audit (the workflowz) -Run quarterly, before notable releases, and after upstream majors (SDK, Bun, -Redis Stack). Executed with the agent harness as a parallel fan-out; the -procedure is deterministic and re-runnable. Past runs land in `docs/audits/`. - -1. **Finders (parallel, mutually blind)** — one agent per dimension: - - *protocol correctness*: Float32 little-endian round-trips, RESP3 shape - handling, score normalization per metric (COSINE / EUCLIDEAN / - DOT_PRODUCT), binary-safe Redis paths (`redis.send`) - - *security*: auth bypass vectors, injection through ids / namespaces / - filter strings, header handling, body-limit enforcement - - *resource safety*: topK / dimension bombs, timeout coverage, unbounded - over-fetch in filter evaluation, shutdown drain - - *parity*: `_ENDPOINTS` inventory of the installed `@upstash/vector` vs - `src/routes/*` vs the live Upstash REST docs — every endpoint must be - supported, explicitly rejected, or explicitly 501 - - *concurrency & self-heal*: Redis client re-init, lazy `FT.CREATE` races, - graceful shutdown during in-flight writes - Each finder returns structured findings `{file, line, title, evidence, - severity}` — no vibes, citations required. -2. **Refutation (parallel)** — one skeptic per finding, prompted to REFUTE, - defaulting to "refuted" when evidence is weak. A finding survives only with - concrete file/line evidence the skeptic cannot break. -3. **Synthesis** — survivors are deduplicated, severity-ranked, and turned into - issues or immediate fixes. Disputed findings are reported as disputed, never - silently dropped. - -Scope guardrails for finders (things that look like bugs but are decisions): -sparse/hybrid vectors are intentionally rejected; resumable query endpoints are -intentionally 501; metadata filtering is intentionally app-level over-fetch in -v1; score values approximate Upstash cloud but are not bit-identical. +The deterministic, re-runnable procedure lives in +[`docs/workflows/deep-audit.md`](./workflows/deep-audit.md): finder +dimensions and prompts, refuter rules (default-refute), synthesis and exit +criteria, and the scope-guardrails list. Execute it with the agent harness +as a parallel fan-out — or any agent runner, including a future scheduled CI +job — exactly as written. + +Cadence: quarterly, before notable releases, and after upstream majors (SDK, +Bun, Redis Stack). Past runs land in `docs/audits/` — latest: +[2026-09-06](./audits/2026-09-06-deep-audit.md) (17 raw findings, 12 +survivors, 8 fixed in-pass with regression tests, 3 open issues #22 #23 +#24, 5 refuted). diff --git a/docs/audits/2026-09-06-deep-audit.md b/docs/audits/2026-09-06-deep-audit.md new file mode 100644 index 0000000..fa19874 --- /dev/null +++ b/docs/audits/2026-09-06-deep-audit.md @@ -0,0 +1,48 @@ +# Deep Audit — 2026-09-06 + +Second run of the deep-audit workflowz, now packaged as a deterministic +procedure: `docs/workflows/deep-audit.md`. Method: 5 mutually-blind finder +agents (protocol correctness, security, resource safety, Upstash parity, +concurrency/self-heal) produced **17 raw findings**; each was attacked by a +dedicated skeptic prompted to refute with default-refute. **12 survived** +with concrete evidence; 5 were refuted. The orchestrator personally +re-verified the highest-severity survivors and every refutation that +overturned a framework-behavior claim (hono compose error propagation was +confirmed against the installed `node_modules/hono/dist/compose.js`). + +## Confirmed findings and dispositions + +| # | Severity | Dimension | Finding | Disposition | +|---|---|---|---|---| +| 1 | high | resource | `src/routes/query.ts` — FT.SEARCH had no RETURN/NOCONTENT projection: every KNN candidate shipped its full hash (`_vec` base64 + metadata ≤128KB + data ≤1MB) even with include* flags false; ~1.2GB through the RESP3 parser per topK=1000 query, ×100 in a batch | **Fixed**: conditional `RETURN` projection (`_score` always; `metadata` when filtering/includeMetadata; `_vec`/`data` per flags). NOCONTENT is unsafe — the parser reads `_score` from returned attributes. Pinned by `tests/unit/query-projection.test.ts`. | +| 2 | moderate | protocol | `src/translate/index.ts` — syncIndexes adopted ALL indexes from FT._LIST; a foreign app's vector index with a different DISTANCE_METRIC on a shared Redis aborted startup | **Fixed**: only `idx:*` indexes are synced/validated/cached, matching the prefix filter used everywhere else FT._LIST is consumed. Pinned by `tests/unit/sync-indexes.test.ts`. Refuter narrowed the trigger: only foreign *vector* indexes abort (non-vector indexes skip both checks); only metric mismatch is fatal from syncIndexes (wantDim is undefined there). | +| 3 | moderate | security/resource | `src/routes/update.ts`, `src/routes/data.ts` — /update (metadata + data) and /upsert-data (metadata) bypassed the per-entry budgets /upsert enforces "regardless of entry point" | **Fixed**: `MAX_METADATA_BYTES`/`MAX_DATA_LENGTH` exported from upsert.ts and applied to UpdateBody and UpsertDataItem. Pinned by `tests/unit/entry-point-limits.test.ts`. Residual: PATCH-mode merges accumulate in Lua across requests, each request individually within budget — flagged for the deferred Lua-level cap decision. | +| 4 | moderate | concurrency | `src/routes/reset.ts` — reset's dropIndex vs concurrent upsert leaves a bounded unindexed window (invisible to /query, visible to /fetch) | **Open** → [#24](https://github.com/Coriou/up-vector/issues/24). Candidate fix (stop dropping the index on namespace reset) makes namespace dimension sticky — a deliberate behavior decision, not a drive-by. Finder's "permanent stale knownIndexes" variant refuted (single multiplexed connection cannot invert reply order). | +| 5 | moderate | security | `src/server.ts` — chunked bodies are fully buffered by bodyLimit before the request timeout starts; the drip phase is bounded only by Bun's idleTimeout, and no concurrent-request limit exists | **Open** → [#22](https://github.com/Coriou/up-vector/issues/22). Refuter corrected impact: memory is capped at maxSize per connection; the unbounded axis is time. Authenticated-only. | +| 6 | moderate | concurrency | `src/middleware/logger.ts` — metrics allegedly skipped all thrown-error responses | **REFUTED** (contradicted-by-code): hono 4.13.3 `compose.js` catches at the throwing frame, runs `onError` there, and resolves `next()` normally — `recordRequest` runs for 400/401/500 responses with correct status. The finder's Hono semantics premise was false; the orchestrator's own first verification was also wrong and the refuter caught it. | +| 7 | moderate | resource | `src/routes/range.ts` — double Set + O(N) scan/sort per /range request | **REFUTED** (scope-decision): committed, deliberately deferred /range redesign (PLAN.md; docs/superpowers/specs/2026-07-17). Residual accepted: the redundant `seenKeys` Set was removed (behavior-preserving, pinned dedup test stays green) to be folded into the future /range item. | +| 8 | moderate | parity | `src/routes/query.ts` — empty filter string rejected 400 vs Upstash's documented `""` = no filter | **REFUTED** (pinned-by-test, deliberate anti-client-bug decision). Residual fixed: README compatibility table now documents the deviation. | +| 9 | low | protocol | `src/embedding.ts` — parseEmbedding enforced only Number.isFinite; provider values like 1e39 encoded to Infinity bytes with HTTP 200 via /upsert-data, /query-data, and embedded /update | **Fixed**: FLOAT32_MAX bound → 502 EmbeddingProviderError. Pinned in `tests/unit/embedding.test.ts`. Refuter widened blast radius to /query-data embeddings (same path, now covered). | +| 10 | low | protocol+security+parity | SCAN 10k-iteration cap was silent truncation on reset / delete-by-prefix / delete-by-filter / fetch-prefix / random / rename-namespace, while range.ts throws loudly on the identical condition | **Fixed**: all six sites now throw ValidationError, sharing `MAX_SCAN_ITERATIONS` from keys.ts. Pinned across `tests/unit/route-validation.test.ts`. Refuter widened scope (found the random + rename sites) and noted iteration count scales with total keyspace, not matching keys — making the silent case easier to hit than first stated. | +| 11 | low | parity | `/fetch` and `/delete` capped explicit-ids mode at 1000; Upstash documents the cap only for prefix fetch, none at all for delete | **Fixed**: caps lifted for explicit ids (prefix keeps its documented 1000-result bound). Pinned in `tests/unit/route-validation.test.ts`. | +| 12 | low | parity | `src/translate/keys.ts` — validatePrefix rejected glob metacharacters while ids may contain them; Upstash prefix is a literal string | **Fixed**: prefixes are accepted and `escapeGlobMeta` neutralizes metacharacters when SCAN patterns are built (namespaces remain glob-free, keeping patterns namespace-anchored). Pinned in `tests/unit/keys.test.ts` (former rejection pin updated). | +| 13 | low | concurrency | `src/redis.ts` — isRedisHealthy leaks a 2s race timer per probe | **REFUTED** (contradicted-by-code): every termination path ends in explicit `process.exit()`; timers never delay shutdown and the late resolve is a no-op. Harmless hygiene only; not changed. | +| 14 | low | concurrency | `src/redis.ts` — re-init window where getClient() throws "not initialized" | **REFUTED** (unconstructible): `reinitRedis` nulls and reassigns the client synchronously before the first await; no HTTP handler can observe the null window. | +| 15 | low | parity | `/info` reports dimension 0 on a cold server | **Open** → [#23](https://github.com/Coriou/up-vector/issues/23). Also fires after reset-all (dropIndex clears dimensionMap). | +| 16 | low | parity | Batch query capped at 100 with no documentation | **Doc fixed**: README compatibility table notes the cap (applies to /query and /query-data). Upstash documents no limit; raising/removing the cap stays open for the next parity pass. | +| 17 | moderate | resource | `src/routes/fetch.ts`/`range.ts`/`random.ts` — read paths used hgetall, transferring raw `vec` blobs and unrequested metadata/data | **Fixed**: HMGET projection gated on include* flags; EXISTS replaces the full-hash read when no fields are requested (ids mode); scan-derived keys treat existence as known. Pinned in `tests/unit/route-validation.test.ts`; the SDK compat suite caught an intermediate regression (vector-only rows reported null on metadata-only fetch), which is exactly the layer the suite exists for. | + +## Refuted (5) — kept for the record + +Refutation buckets: contradicted-by-code (F6-metrics, F13-timers), +scope-decision (F7-range redesign, F8-empty-filter pinned-by-test), and +unconstructible (F14-reinit window). All five are re-tested by design on +every future run, since finders are re-prompted blind. + +## Verification state at close of pass + +Full gate green after fixes: **465 tests** — 331 unit / 60 integration / +74 SDK compatibility — via `./scripts/test-all.sh` on Bun 1.3.14 local / +Redis Stack `7.4.0-v8`; typecheck + Biome clean; `bun audit` clean +(0 advisories, run locally at close of pass). Open findings tracked as +issues #22, #23, #24 (label `deep-audit`). diff --git a/docs/workflows/deep-audit.md b/docs/workflows/deep-audit.md new file mode 100644 index 0000000..aea746d --- /dev/null +++ b/docs/workflows/deep-audit.md @@ -0,0 +1,155 @@ +# Deep Audit — Deterministic Workflowz + +Re-runnable adversarial audit for up-vector. This document is the executable +procedure: an agent harness (or a future CI agent runner) executes it verbatim +and produces an identical-shape result. Past runs: `docs/audits/`. + +Cadence (per `docs/RUNBOOK.md`): quarterly, before notable releases, and after +any upstream major (SDK, Bun, Redis Stack). + +## Preconditions (all must hold before Phase 1) + +1. Working tree clean, on a branch cut from `main` (`maintenance/deep-audit-YYYY-MM-DD`). +2. Baseline gate green on the base commit: `./scripts/test-all.sh`. Record test + counts and Bun/Redis versions — they go in the run artifact. +3. Pins recorded: `bun --version`, Redis Stack tag from `docker-compose.yml`, + `@upstash/vector` version from `bun.lock`. + +## Phase 1 — Finders (parallel, mutually blind) + +Spawn 5 read-only agents in one fan-out. No finder sees another's output. +Each returns ONLY a JSON array (may be empty): + +```json +[{"dimension": "...", "file": "src/...", "line": 123, "title": "...", + "evidence": "concrete code path or input that triggers the behavior", + "severity": "low|moderate|high", "suggested_disposition": "fix|triage"}] +``` + +No vibes: every finding cites `file:line` and names the exact input or code +path. A finding without a constructible trigger is not a finding. + +### Finder 1 — protocol correctness + +Read: `src/translate/vectors.ts`, `src/translate/scores.ts`, +`src/translate/index.ts`, `src/translate/keys.ts`, `src/redis.ts`, and the +unit tests that pin them (`tests/unit/`). Hunt: + +- Float32 little-endian round-trip correctness (encode/decode symmetry, + denormals, -0, max magnitude). +- RESP2 vs RESP3 shape handling for `FT.INFO` / `FT.*` replies. +- Score normalization per metric (COSINE / EUCLIDEAN / DOT_PRODUCT): bounds, + inverted-distance handling, 0-1 contract with the Upstash SDK. +- Binary-safety: every Redis path touching vector bytes must use + `redis.send(...)`; flag any `hset`/`hgetall` on binary fields. + +### Finder 2 — security + +Read: `src/middleware/auth.ts`, `src/middleware/error-handler.ts`, +`src/config.ts`, `src/server.ts`, `src/translate/keys.ts`, +`src/filter/parser.ts`, `src/filter/evaluator.ts`, all `src/routes/*`. Hunt: + +- Auth bypass vectors (header parsing, missing middleware on new routes, + metrics token path). +- Injection through ids / namespaces / filter strings / prefix into Redis + commands and key names (key building must be injection-proof). +- Error envelope leaking internals (stack traces, Redis error text) to clients. +- Body-limit enforcement actually applied to every route; header handling + edge cases. + +### Finder 3 — resource safety + +Read: `src/middleware/timeout.ts`, `src/routes/query.ts`, `src/routes/range.ts`, +`src/routes/fetch.ts`, `src/routes/upsert.ts`, `src/config.ts`, +`src/shutdown.ts`, `src/index.ts`. Hunt: + +- topK / dimension / vector-count bombs (no upper bound where one is cheap). +- Unbounded over-fetch in metadata filter evaluation (v1 app-level filtering: + bounded by design? verify the bound). +- Timeout coverage gaps (routes or phases outside the middleware). +- Shutdown drain correctness: in-flight writes during `SIGTERM`, index-create + races at shutdown. + +### Finder 4 — Upstash parity + +Read: the installed SDK at `node_modules/@upstash/vector/dist/` (grep +`_ENDPOINTS` / `fetch` paths for the endpoint inventory), every +`src/routes/*`, `README.md` parity table, and the live REST docs at +`https://upstash.com/docs/vector/api/endpoints` (fetch each endpoint page). +Hunt: + +- Every SDK endpoint must be: supported, explicitly rejected (validation + error), or explicitly 501 (`src/routes/unsupported.ts`). Flag anything + silently missing or silently wrong-shaped. +- Request/response field parity: option names, defaults, error envelope + shapes, `X-Upstash-*` header expectations. +- Documented Upstash limits (e.g. caps, payload bounds) that we enforce + differently — each difference must be a documented deviation or a finding. + +### Finder 5 — concurrency & self-heal + +Read: `src/redis.ts`, `src/translate/index.ts`, `src/routes/reset.ts`, +`src/routes/upsert.ts`, `src/shutdown.ts`, `src/health.ts` (or equivalent), +`src/metrics.ts`. Hunt: + +- Redis client re-init (`UPVECTOR_REDIS_REINIT_AFTER_MS`): in-flight command + behavior during re-init, timer leaks, double re-init. +- Lazy `FT.CREATE` races: two concurrent first-upserts, create-after-drop, + create during shutdown. +- Reset atomicity: index drop vs key deletion interleaving with concurrent + upserts (known open race — re-verify, look for cheaper fixes). +- Health probe correctness during Redis unavailability; metrics counters + under error paths. + +### Scope guardrails (decisions, not bugs — finders must not report) + +- Sparse/hybrid vectors are intentionally rejected. +- Resumable query endpoints are intentionally 501. +- Metadata filtering is intentionally app-level over-fetch in v1. +- Score values approximate Upstash cloud semantics; not bit-identical. +- `UPVECTOR_DIMENSION` fixed-at-boot is intentional. + +## Phase 2 — Refutation (parallel, after Phase 1 settles) + +One skeptic agent per finding, prompted to REFUTE. Default is "refuted" when +evidence is weak. A finding survives ONLY if the skeptic: + +1. Re-reads the cited code and confirms the trigger is constructible against + the real request flow (not a hypothetical caller), AND +2. Finds no test or committed scope decision that pins the behavior as + intentional, AND +3. For parity claims: confirms the discrepancy against the installed SDK + source and the live docs, not memory. + +Output per finding: `SURVIVES` with corrected file:line + tightened evidence, +or `REFUTED` with the reason (contradicted-by-code | pinned-by-test | +scope-decision | unconstructible). Disputed findings are recorded as +disputed — never silently dropped, never silently kept. + +## Phase 3 — Synthesis (orchestrator, not delegated) + +1. Personally re-verify every survivor against current code (read the cited + lines; run the triggering input where cheap). Orchestrator verification is + mandatory — subagent output is evidence, not truth. +2. Deduplicate overlapping findings; severity-rank (high > moderate > low). +3. Disposition per survivor: + - **fix** and the fix is small, test-coverable, and behavior-compatible → + land in this pass with a regression test (fails pre-fix, passes post-fix). + - **open** → GitHub issue labeled `deep-audit` (+ `security` if security + dimension), with evidence and candidate mitigations. Never leave an open + finding tracked only in prose. + - **disputed** → keep on record in the run artifact with both arguments. +4. Write `docs/audits/YYYY-MM-DD-deep-audit.md` following the shape of + `docs/audits/2026-08-22-deep-audit.md`: raw counts, survivor table + (severity / dimension / finding / disposition), refuted list summary, + verification state at close. +5. Doc truth: update test counts / pins in `docs/RUNBOOK.md` baseline and + `CLAUDE.md` if they changed. + +## Exit criteria + +- [ ] Full gate green after remediation: `./scripts/test-all.sh` (record counts). +- [ ] `bun audit --json` clean, or advisories dispositioned in issues. +- [ ] Every survivor: fixed-with-test, or issue filed, or recorded disputed. +- [ ] Run artifact committed; RUNBOOK/CLAUDE.md doc truth holds. +- [ ] Branch → PR → merge to main; main never left red. diff --git a/src/embedding.ts b/src/embedding.ts index 3d2c537..f6f2cd7 100644 --- a/src/embedding.ts +++ b/src/embedding.ts @@ -1,6 +1,7 @@ import { config } from "./config" import { EmbeddingProviderError, ValidationError } from "./errors" import { log } from "./logger" +import { FLOAT32_MAX } from "./translate/vectors" export type EmbeddingProvider = { readonly name: string @@ -371,7 +372,7 @@ export function validateConfiguredDimension( } } -function parseEmbedding(value: unknown): number[] { +export function parseEmbedding(value: unknown): number[] { if (!Array.isArray(value)) { throw new EmbeddingProviderError("Embedding provider returned a malformed embedding", 502) } @@ -382,6 +383,17 @@ function parseEmbedding(value: unknown): number[] { 502, ) } + // 1e39-style float64 values pass the finite check but overflow to Infinity + // inside Float32Array encoding — the exact silent corruption /upsert + // rejects with 400. Provider output must meet the same bar; fail with 502 + // so the provider contract, not stored data, absorbs the fault. + // (deep audit 2026-09-06, F2) + if (Math.abs(entry) > FLOAT32_MAX) { + throw new EmbeddingProviderError( + "Embedding provider returned a value exceeding Float32 range", + 502, + ) + } return entry }) if (vector.length === 0) { diff --git a/src/routes/data.ts b/src/routes/data.ts index 11db3bb..2db6775 100644 --- a/src/routes/data.ts +++ b/src/routes/data.ts @@ -5,7 +5,7 @@ import { EmbeddingProviderError } from "../errors" import { getClient } from "../redis" import { EMBEDDING_NS_REGISTRY, validateNamespace } from "../translate/keys" import { type DenseQuery, executeQuery } from "./query" -import { upsertDenseVectors } from "./upsert" +import { MAX_METADATA_BYTES, upsertDenseVectors } from "./upsert" const MAX_BATCH_SIZE = 1000 const MAX_BATCH_QUERIES = 100 @@ -28,7 +28,13 @@ const dataSchema = z const UpsertDataItem = z.object({ id: idSchema, data: dataSchema, - metadata: z.record(z.string(), z.unknown()).optional(), + metadata: z + .record(z.string(), z.unknown()) + .optional() + .refine( + (m) => m === undefined || Buffer.byteLength(JSON.stringify(m), "utf8") <= MAX_METADATA_BYTES, + `Serialized metadata must not exceed ${MAX_METADATA_BYTES} bytes`, + ), vector: UnsupportedField, sparseVector: UnsupportedField, }) diff --git a/src/routes/delete.ts b/src/routes/delete.ts index cdf1f41..7549eb4 100644 --- a/src/routes/delete.ts +++ b/src/routes/delete.ts @@ -1,10 +1,14 @@ +import type { RedisClient } from "bun" import { type Context, Hono } from "hono" import { z } from "zod" +import { ValidationError } from "../errors" import type { FilterNode } from "../filter" import { compileFilter, evaluate } from "../filter" import { getClient } from "../redis" import { deleteKeysByPattern, + escapeGlobMeta, + MAX_SCAN_ITERATIONS, validateId, validateNamespace, validatePrefix, @@ -12,8 +16,6 @@ import { vectorPrefix, } from "../translate/keys" -const MAX_SCAN_ITERATIONS = 10_000 - const idSchema = z .union([ z.string(), @@ -23,7 +25,7 @@ const idSchema = z const DeleteBody = z .object({ - ids: z.array(idSchema).max(1000, "Batch must not exceed 1000 ids").optional(), + ids: z.array(idSchema).optional(), prefix: z.string().optional(), // Empty filter strings used to silently fall through to the // "nothing specified" branch and return `deleted: 0`. Reject explicitly. @@ -60,7 +62,7 @@ const handleDelete = async (c: Context) => { // Delete by prefix if (parsed.prefix) { validatePrefix(parsed.prefix) - const pattern = `${vectorPrefix(ns)}${parsed.prefix}*` + const pattern = `${vectorPrefix(ns)}${escapeGlobMeta(parsed.prefix)}*` const deleted = await deleteKeysByPattern(pattern) return c.json({ result: { deleted } }) } @@ -77,7 +79,7 @@ const handleDelete = async (c: Context) => { } async function deleteByFilter( - redis: ReturnType, + redis: RedisClient, pattern: string, filter: string, ): Promise { @@ -89,7 +91,13 @@ async function deleteByFilter( let iterations = 0 do { - if (++iterations > MAX_SCAN_ITERATIONS) break + if (++iterations > MAX_SCAN_ITERATIONS) { + // Loud like range.ts: a truncated filter delete that still reports + // success breaks erasure guarantees. (deep audit 2026-09-06, F3) + throw new ValidationError( + `Keyspace exceeds the scanable key limit (${MAX_SCAN_ITERATIONS} SCAN iterations); filter deletion aborted`, + ) + } const result = await redis.scan(cursor, "MATCH", pattern, "COUNT", 100) const [next, keys] = result as unknown as [string, string[]] diff --git a/src/routes/fetch.ts b/src/routes/fetch.ts index d41698e..251f2c5 100644 --- a/src/routes/fetch.ts +++ b/src/routes/fetch.ts @@ -1,7 +1,11 @@ +import type { RedisClient } from "bun" import { type Context, Hono } from "hono" import { z } from "zod" +import { ValidationError } from "../errors" import { getClient } from "../redis" import { + escapeGlobMeta, + MAX_SCAN_ITERATIONS, parseVectorKey, validateId, validateNamespace, @@ -20,7 +24,7 @@ const idSchema = z .transform(String) const FetchBody = z.object({ - ids: z.array(idSchema).max(1000, "Batch must not exceed 1000 ids").optional(), + ids: z.array(idSchema).optional(), prefix: z.string().optional(), includeMetadata: z.boolean().default(false), includeVectors: z.boolean().default(false), @@ -44,11 +48,7 @@ const handleFetch = async (c: Context) => { // Fetch by IDs (default path, also used when both ids and prefix are given) if (parsed.ids) { const results = await Promise.all( - parsed.ids.map(async (id): Promise => { - const hash = await redis.hgetall(vectorKey(ns, id)) - if (!hash || Object.keys(hash).length === 0) return null - return buildVector(hash, id, parsed) - }), + parsed.ids.map((id) => fetchVectorFields(redis, vectorKey(ns, id), parsed)), ) return c.json({ result: results }) } @@ -56,15 +56,10 @@ const handleFetch = async (c: Context) => { // Fetch by prefix — Upstash caps at 1000 results for prefix fetch if (parsed.prefix) { validatePrefix(parsed.prefix) - const pattern = `${vectorPrefix(ns)}${parsed.prefix}*` + const pattern = `${vectorPrefix(ns)}${escapeGlobMeta(parsed.prefix)}*` const keys = await scanAll(redis, pattern, 1000) const results = await Promise.all( - keys.map(async (key) => { - const hash = await redis.hgetall(key) - if (!hash || Object.keys(hash).length === 0) return null - const parsed_key = parseVectorKey(key) - return buildVector(hash, parsed_key?.id ?? hash.id, parsed) - }), + keys.map((key) => fetchVectorFields(redis, key, parsed, true)), ) return c.json({ result: results.filter(Boolean) }) } @@ -102,10 +97,53 @@ function buildVector( return vec } -const MAX_SCAN_ITERATIONS = 10_000 +// Read paths project only the fields the response flags need. The raw `vec` +// blob (RediSearch-only) and unrequested metadata/data previously rode along +// on every fetch — up to ~1.1GB transferred and discarded per 1000-id request +// at the per-field caps. (deep audit 2026-09-06, F8) +function requestedFields(opts: { + includeVectors: boolean + includeMetadata: boolean + includeData: boolean +}): string[] { + const fields: string[] = [] + if (opts.includeVectors) fields.push("_vec") + if (opts.includeMetadata) fields.push("metadata") + if (opts.includeData) fields.push("data") + return fields +} +async function fetchVectorFields( + redis: RedisClient, + key: string, + opts: { + includeVectors: boolean + includeMetadata: boolean + includeData: boolean + }, + assumeExists = false, +): Promise { + const fields = requestedFields(opts) + const hash: Record = {} + if (fields.length > 0) { + const values = (await redis.hmget(key, ...fields)) as (string | null)[] + fields.forEach((field, i) => { + const value = values[i] + if (value !== null && value !== undefined) hash[field] = value + }) + } + if (Object.keys(hash).length === 0) { + // Nothing requested, or the row only stores unrequested fields (a + // vector-only row has no metadata). SCAN-derived keys exist by + // construction; explicit ids need an EXISTS check — a non-existent id + // must report null even when no fields were requested. + // (deep audit 2026-09-06, F8) + if (!assumeExists && !(await redis.exists(key))) return null + } + return buildVector(hash, parseVectorKey(key)?.id ?? key, opts) +} async function scanAll( - redis: ReturnType, + redis: RedisClient, pattern: string, limit = Number.POSITIVE_INFINITY, ): Promise { @@ -113,7 +151,13 @@ async function scanAll( let cursor = "0" let iterations = 0 do { - if (++iterations > MAX_SCAN_ITERATIONS) break + if (++iterations > MAX_SCAN_ITERATIONS) { + // Loud like range.ts: a truncated prefix enumeration presented as + // complete is a silent correctness bug. (deep audit 2026-09-06, F3) + throw new ValidationError( + `Keyspace exceeds the scanable key limit (${MAX_SCAN_ITERATIONS} SCAN iterations); prefix enumeration aborted`, + ) + } const result = await redis.scan(cursor, "MATCH", pattern, "COUNT", 100) const [next, batch] = result as unknown as [string, string[]] for (const key of batch) { diff --git a/src/routes/namespaces.ts b/src/routes/namespaces.ts index 213175b..85f50a6 100644 --- a/src/routes/namespaces.ts +++ b/src/routes/namespaces.ts @@ -6,6 +6,7 @@ import { dropIndex, ensureIndex, loadDimension } from "../translate/index" import { deleteKeysByPattern, EMBEDDING_NS_REGISTRY, + MAX_SCAN_ITERATIONS, NS_REGISTRY, parseVectorKey, validateNamespace, @@ -51,8 +52,6 @@ const RenameNamespaceBody = z.object({ deleteExisting: z.boolean().default(false), }) -const MAX_SCAN_ITERATIONS = 10_000 - namespaceRoutes.post("/rename-namespace", async (c) => { const body = await c.req.json() const parsed = RenameNamespaceBody.parse(body) @@ -132,7 +131,13 @@ async function scanKeys(pattern: string, limit = Number.POSITIVE_INFINITY): Prom let iterations = 0 do { - if (++iterations > MAX_SCAN_ITERATIONS) break + if (++iterations > MAX_SCAN_ITERATIONS) { + // Loud like range.ts: a truncated enumeration reported a rename or + // existence verdict as complete. (deep audit 2026-09-06, F3) + throw new ValidationError( + `Keyspace exceeds the scanable key limit (${MAX_SCAN_ITERATIONS} SCAN iterations); namespace enumeration aborted`, + ) + } const result = await redis.scan(cursor, "MATCH", pattern, "COUNT", 100) const [next, batch] = result as unknown as [string, string[]] for (const key of batch) { diff --git a/src/routes/query.ts b/src/routes/query.ts index be8c1e3..456425b 100644 --- a/src/routes/query.ts +++ b/src/routes/query.ts @@ -110,9 +110,20 @@ export async function executeQuery(ns: string, query: DenseQuery): Promise { // Reservoir sample over the namespace so every matching key has equal // probability without materializing the full namespace in memory. do { - if (++iterations > MAX_SCAN_ITERATIONS) break + if (++iterations > MAX_SCAN_ITERATIONS) { + // Loud like range.ts: a truncated sweep silently biases the sample + // toward early keys instead of a uniform draw. (deep audit 2026-09-06, F3) + throw new ValidationError( + `Keyspace exceeds the scanable key limit (${MAX_SCAN_ITERATIONS} SCAN iterations); random sampling aborted`, + ) + } const result = await redis.scan(cursor, "MATCH", pattern, "COUNT", 100) const [next, keys] = result as unknown as [string, string[]] @@ -94,11 +104,15 @@ const handleRandom = async (c: Context) => { return c.json({ result: null }) } - const hash = await redis.hgetall(selectedKey) - if (!hash || Object.keys(hash).length === 0) { - return c.json({ result: null }) + // All three fields fetched unconditionally: the emptiness guard below needs + // the full picture, and this is a single key. (deep audit 2026-09-06, F8) + const fieldNames = ["_vec", "metadata", "data"] + const values = (await redis.hmget(selectedKey, ...fieldNames)) as (string | null)[] + const hash: Record = {} + for (let i = 0; i < fieldNames.length; i++) { + const value = values[i] + if (value !== null && value !== undefined) hash[fieldNames[i]] = value } - // Need at least one stored field; empty hash already handled. If vectors // are omitted by flag, still return id (+ optional metadata/data). if (!hash._vec && !hash.metadata && hash.data === undefined) { @@ -106,7 +120,7 @@ const handleRandom = async (c: Context) => { } const parsedKey = parseVectorKey(selectedKey) - const id = parsedKey?.id ?? hash.id ?? selectedKey + const id = parsedKey?.id ?? selectedKey return c.json({ result: buildRandomVector(hash, id, opts) }) } diff --git a/src/routes/range.ts b/src/routes/range.ts index 996bb6c..cca88d4 100644 --- a/src/routes/range.ts +++ b/src/routes/range.ts @@ -1,8 +1,16 @@ +import type { RedisClient } from "bun" import { type Context, Hono } from "hono" import { z } from "zod" import { ValidationError } from "../errors" import { getClient } from "../redis" -import { parseVectorKey, validateNamespace, validatePrefix, vectorPrefix } from "../translate/keys" +import { + escapeGlobMeta, + MAX_SCAN_ITERATIONS, + parseVectorKey, + validateNamespace, + validatePrefix, + vectorPrefix, +} from "../translate/keys" import { decodeVectorBase64 } from "../translate/vectors" import type { Vector } from "../types" @@ -20,8 +28,6 @@ const RangeBody = z.object({ includeData: z.boolean().default(false), }) -const MAX_SCAN_ITERATIONS = 10_000 - /** * Scans every key matching `pattern`, deduplicating SCAN results. Throws a * ValidationError when the scan-iteration cap is exhausted before Redis @@ -29,13 +35,12 @@ const MAX_SCAN_ITERATIONS = 10_000 * presenting a truncated enumeration as complete. */ export async function collectScanKeys( - redis: ReturnType, + redis: RedisClient, pattern: string, maxIterations: number = MAX_SCAN_ITERATIONS, ): Promise> { let scanCursor = "0" const collectedKeys = new Set() - const seenKeys = new Set() let iterations = 0 do { @@ -47,12 +52,8 @@ export async function collectScanKeys( const result = await redis.scan(scanCursor, "MATCH", pattern, "COUNT", 100) const [next, keys] = result as unknown as [string, string[]] - for (const key of keys) { - // SCAN can return duplicate keys across iterations — deduplicate - if (seenKeys.has(key)) continue - seenKeys.add(key) - collectedKeys.add(key) - } + // SCAN can return duplicate keys across iterations — Set.add dedupes. + for (const key of keys) collectedKeys.add(key) scanCursor = next } while (scanCursor !== "0") @@ -71,7 +72,9 @@ const handleRange = async (c: Context) => { if (parsed.prefix) validatePrefix(parsed.prefix) const basePrefix = vectorPrefix(ns) - const pattern = parsed.prefix ? `${basePrefix}${parsed.prefix}*` : `${basePrefix}*` + const pattern = parsed.prefix + ? `${basePrefix}${escapeGlobMeta(parsed.prefix)}*` + : `${basePrefix}*` // Upstash's public cursor is an offset string ("0", "100", ...), not a // Redis SCAN cursor. Scan the namespace, sort for stable paging, then slice by @@ -86,22 +89,36 @@ const handleRange = async (c: Context) => { // Fetch details for each matched key const vectors: Vector[] = await Promise.all( pageKeys.map(async (key) => { - const hash = await redis.hgetall(key) - const parsedKey = parseVectorKey(key) - const id = parsedKey?.id ?? hash?.id ?? key - + const id = parseVectorKey(key)?.id ?? key const vec: Vector = { id } - if (parsed.includeVectors && hash?._vec) { + // Project only the fields the response flags need (HMGET) — the raw + // `vec` blob and unrequested metadata/data previously rode along on + // every page. (deep audit 2026-09-06, F8) + const fields = [ + ...(parsed.includeVectors ? ["_vec"] : []), + ...(parsed.includeMetadata ? ["metadata"] : []), + ...(parsed.includeData ? ["data"] : []), + ] + if (fields.length === 0) { + return vec + } + const values = (await redis.hmget(key, ...fields)) as (string | null)[] + const hash: Record = {} + fields.forEach((field, i) => { + const value = values[i] + if (value !== null && value !== undefined) hash[field] = value + }) + if (parsed.includeVectors && hash._vec) { vec.vector = decodeVectorBase64(hash._vec) } - if (parsed.includeMetadata && hash?.metadata) { + if (parsed.includeMetadata && hash.metadata) { try { vec.metadata = JSON.parse(hash.metadata) } catch { // Malformed metadata JSON — skip } } - if (parsed.includeData && hash?.data !== undefined) { + if (parsed.includeData && hash.data !== undefined) { vec.data = hash.data } return vec diff --git a/src/routes/update.ts b/src/routes/update.ts index 4ce35bf..c6db84e 100644 --- a/src/routes/update.ts +++ b/src/routes/update.ts @@ -7,6 +7,7 @@ import { getClient } from "../redis" import { loadDimension } from "../translate/index" import { EMBEDDING_NS_REGISTRY, validateId, validateNamespace, vectorKey } from "../translate/keys" import { encodeVector, encodeVectorBase64, finiteNumber } from "../translate/vectors" +import { MAX_DATA_LENGTH, MAX_METADATA_BYTES } from "./upsert" // Numeric IDs that aren't finite would silently become "NaN" / "Infinity" // strings after the .transform(String) below — reject them up front so users @@ -29,8 +30,21 @@ const UpdateBody = z.object({ .max(MAX_VECTOR_DIM, `Vector dimension must not exceed ${MAX_VECTOR_DIM}`) .optional(), sparseVector: UnsupportedField, - metadata: z.record(z.string(), z.unknown()).optional(), - data: z.string().optional(), + // Same budgets as /upsert (VectorSchema) — an /update write bypassing the + // cap let a single request persist ~32MiB of metadata/data that every + // includeMetadata/includeData read then retransfers. + // (deep audit 2026-09-06, F4) + metadata: z + .record(z.string(), z.unknown()) + .optional() + .refine( + (m) => m === undefined || Buffer.byteLength(JSON.stringify(m), "utf8") <= MAX_METADATA_BYTES, + `Serialized metadata must not exceed ${MAX_METADATA_BYTES} bytes`, + ), + data: z + .string() + .max(MAX_DATA_LENGTH, `Data must not exceed ${MAX_DATA_LENGTH} characters`) + .optional(), metadataUpdateMode: z.enum(["OVERWRITE", "PATCH"]).default("OVERWRITE"), }) diff --git a/src/routes/upsert.ts b/src/routes/upsert.ts index 9158c5d..be871fd 100644 --- a/src/routes/upsert.ts +++ b/src/routes/upsert.ts @@ -19,9 +19,11 @@ const idSchema = z const MAX_VECTOR_DIM = 16384 // Parity with /upsert-data's data cap (data.ts MAX_DATA_LENGTH), plus a byte // budget on serialized metadata so Redis hash values stay bounded regardless -// of entry point. -const MAX_DATA_LENGTH = 1_000_000 -const MAX_METADATA_BYTES = 131072 +// of entry point. Exported so /update and /upsert-data enforce the same +// budgets — an uncapped /update write made the "regardless of entry point" +// claim false. (deep audit 2026-09-06, F4) +export const MAX_DATA_LENGTH = 1_000_000 +export const MAX_METADATA_BYTES = 131072 const UnsupportedField = z.never().optional() diff --git a/src/translate/index.ts b/src/translate/index.ts index 41b5715..f0b7541 100644 --- a/src/translate/index.ts +++ b/src/translate/index.ts @@ -192,8 +192,12 @@ export async function syncIndexes(): Promise { } for (const idx of indexes) { - knownIndexes.add(idx) - const ns = idx.startsWith("idx:") ? idx.slice(4) : idx + // FT._LIST is global to the Redis DB — a shared instance may hold indexes + // from other applications. Only our idx:{ns} indexes are ours to sync, + // validate, and cache; adopting foreign ones aborted startup on distance + // metric mismatches we don't own. (deep audit 2026-09-06, F1) + if (!idx.startsWith("idx:")) continue + const ns = idx.slice(4) try { const info = await redis.send("FT.INFO", [idx]) const dim = parseDimensionFromInfo(info) diff --git a/src/translate/keys.ts b/src/translate/keys.ts index e67f961..69046f5 100644 --- a/src/translate/keys.ts +++ b/src/translate/keys.ts @@ -3,7 +3,7 @@ import { getClient } from "../redis" export const NS_REGISTRY = "_ns_registry" export const EMBEDDING_NS_REGISTRY = "_embedding_ns_registry" -const MAX_SCAN_ITERATIONS = 10_000 +export const MAX_SCAN_ITERATIONS = 10_000 const MAX_NAMESPACE_LENGTH = 256 const MAX_ID_LENGTH = 1024 @@ -48,22 +48,31 @@ export function validateId(id: string): void { /** * Validates a user-supplied id prefix used to build a SCAN MATCH pattern. * - * Glob metacharacters are forbidden so a malicious prefix can't accidentally - * match outside its intended subtree (e.g. `prefix: "*\\v:other:"`) or create - * pathological patterns. Length is bounded to keep SCAN cursors small. + * Glob metacharacters are allowed for Upstash parity (Upstash treats prefix as + * a literal string, and ids may legally contain `* ? [ ] \`); the pattern + * builders escape them via escapeGlobMeta so they match literally instead of + * acting as wildcards. Control characters are still rejected and length is + * bounded to keep SCAN cursors small. (deep audit 2026-09-06, F11) */ export function validatePrefix(prefix: string): void { if (CONTROL_CHARS.test(prefix)) { throw new ValidationError("Prefix must not contain control characters") } - if (GLOB_META.test(prefix)) { - throw new ValidationError("Prefix must not contain glob characters (* ? [ ] \\)") - } if (prefix.length > MAX_PREFIX_LENGTH) { throw new ValidationError(`Prefix must not exceed ${MAX_PREFIX_LENGTH} characters`) } } +/** + * Escapes Redis glob metacharacters so a user prefix matches literally inside + * a SCAN MATCH pattern. Namespaces are validated glob-free, so escaping the + * prefix alone is enough to keep every pattern anchored to its namespace + * subtree. + */ +export function escapeGlobMeta(value: string): string { + return value.replace(/[*?[\]\\]/g, (ch) => `\\${ch}`) +} + export function vectorKey(ns: string, id: string): string { return `v:${ns}:${id}` } @@ -97,7 +106,14 @@ export async function deleteKeysByPattern(pattern: string): Promise { const seen = new Set() let iterations = 0 do { - if (++iterations > MAX_SCAN_ITERATIONS) break + if (++iterations > MAX_SCAN_ITERATIONS) { + // Fail loudly like range.ts collectScanKeys: a truncated enumeration + // presented as success let /reset and /delete claim completion while + // matching keys survived. (deep audit 2026-09-06, F3) + throw new ValidationError( + `Keyspace exceeds the scanable key limit (${MAX_SCAN_ITERATIONS} SCAN iterations); deletion aborted`, + ) + } const result = await redis.scan(cursor, "MATCH", pattern, "COUNT", 100) const [next, keys] = result as unknown as [string, string[]] // SCAN can return duplicate keys across iterations — deduplicate diff --git a/tests/unit/embedding.test.ts b/tests/unit/embedding.test.ts index 42561fb..9c18ed2 100644 --- a/tests/unit/embedding.test.ts +++ b/tests/unit/embedding.test.ts @@ -3,6 +3,7 @@ import { DisabledEmbeddingProvider, FakeEmbeddingProvider, OpenAICompatibleEmbeddingProvider, + parseEmbedding, validateConfiguredDimension, } from "../../src/embedding" import { EmbeddingProviderError, ValidationError } from "../../src/errors" @@ -170,3 +171,27 @@ describe("OpenAICompatibleEmbeddingProvider", () => { await expect(provider.embedMany(["hello"])).rejects.toBeInstanceOf(EmbeddingProviderError) }) }) + +describe("parseEmbedding Float32 bound", () => { + // Provider output flows straight into Float32Array encoding; a float64 value + // past FLOAT32_MAX silently became an Infinity blob with HTTP 200 — the + // exact corruption /upsert rejects with 400. Provider paths must fail with + // 502 instead. (deep audit 2026-09-06, F2) + test("rejects values past Float32 range with a 502 provider error", () => { + expect(() => parseEmbedding([1e39])).toThrow(EmbeddingProviderError) + try { + parseEmbedding([1e39]) + } catch (err) { + expect((err as EmbeddingProviderError).status).toBe(502) + expect((err as Error).message).toContain("Float32") + } + }) + + test("rejects negative overflow", () => { + expect(() => parseEmbedding([-1e39])).toThrow(EmbeddingProviderError) + }) + + test("accepts the Float32 boundary value", () => { + expect(parseEmbedding([3.4028235e38, -3.4028235e38])).toEqual([3.4028235e38, -3.4028235e38]) + }) +}) diff --git a/tests/unit/entry-point-limits.test.ts b/tests/unit/entry-point-limits.test.ts new file mode 100644 index 0000000..038403b --- /dev/null +++ b/tests/unit/entry-point-limits.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, mock, test } from "bun:test" +import { Hono } from "hono" +import { errorHandler } from "../../src/middleware/error-handler" + +const evalResult = 1 +const fakeRedis = { + sismember: async () => 0, + send: async () => evalResult, +} + +mock.module("../../src/redis", () => ({ + getClient: () => fakeRedis, + // Mirror the real module's remaining exports: other suites in the same + // process statically bind isRedisHealthy et al. + initRedis: async () => {}, + isRedisHealthy: async () => true, + reinitRedis: async () => {}, + closeRedis: async () => {}, +})) + +// Dynamic import (not static): the route modules must bind the redis stub +// mock.module registered above — static hoisting would bind the real module. +const [{ updateRoutes }, { dataRoutes }] = await Promise.all([ + import("../../src/routes/update"), + import("../../src/routes/data"), +]) + +function appWith(routes: Hono): Hono { + const app = new Hono() + app.onError(errorHandler) + app.route("/", routes) + return app +} + +async function postJson(app: Hono, path: string, body: unknown): Promise { + return app.request(path, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }) +} + +const oversizeMetadata = (): Record => { + const metadata: Record = {} + for (let i = 0; i < 1500; i++) metadata[`key-${i}`] = "y".repeat(200) + return metadata +} + +// /upsert caps serialized metadata at 131072 bytes and data at 1_000_000 +// characters "regardless of entry point" — but /update and /upsert-data +// bypassed both, letting a single request persist ~32MiB that every +// includeMetadata/includeData read retransfers. (deep audit 2026-09-06, F4) +describe("/update enforces the /upsert size budgets", () => { + test("rejects metadata past the 131072-byte budget", async () => { + const res = await postJson(appWith(updateRoutes), "/update", { + id: "doc-1", + metadata: oversizeMetadata(), + }) + expect(res.status).toBe(400) + const json = (await res.json()) as { error?: string } + expect(json.error).toBe("Serialized metadata must not exceed 131072 bytes") + }) + + test("rejects data past the 1_000_000-character budget", async () => { + const res = await postJson(appWith(updateRoutes), "/update", { + id: "doc-1", + data: "x".repeat(1_000_001), + }) + expect(res.status).toBe(400) + const json = (await res.json()) as { error?: string } + expect(json.error).toBe("Data must not exceed 1000000 characters") + }) + + test("accepts metadata within the budget", async () => { + // Regression guard against over-tightening: valid payloads still reach + // the Lua update. + const res = await postJson(appWith(updateRoutes), "/update", { + id: "doc-1", + metadata: { source: "test" }, + }) + expect(res.status).toBe(200) + }) +}) + +describe("/upsert-data enforces the /upsert metadata budget", () => { + test("rejects item metadata past the 131072-byte budget", async () => { + const res = await postJson(appWith(dataRoutes), "/upsert-data", { + id: "doc-1", + data: "hello", + metadata: oversizeMetadata(), + }) + expect(res.status).toBe(400) + const json = (await res.json()) as { error?: string } + expect(json.error).toBe("Serialized metadata must not exceed 131072 bytes") + }) +}) diff --git a/tests/unit/filter.test.ts b/tests/unit/filter.test.ts index a529b8c..82df0ad 100644 --- a/tests/unit/filter.test.ts +++ b/tests/unit/filter.test.ts @@ -923,7 +923,10 @@ describe("bounded glob matcher", () => { const r1 = compileGlob(pattern).test(subject) const elapsed = performance.now() - start expect(r1).toBe(false) - expect(elapsed).toBeLessThan(50) + // The bound guards against catastrophic backtracking, which is orders of + // magnitude slower (seconds-to-minutes on 100k chars), not against slow + // hardware — keep it generous so CI runners with noisy clocks stay green. + expect(elapsed).toBeLessThan(2000) // Deterministic: same answer on a repeat run expect(compileGlob(pattern).test(subject)).toBe(false) }) diff --git a/tests/unit/keys.test.ts b/tests/unit/keys.test.ts index 2b9d9ea..83196a2 100644 --- a/tests/unit/keys.test.ts +++ b/tests/unit/keys.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test" import { ValidationError } from "../../src/errors" import { + escapeGlobMeta, indexName, NS_REGISTRY, parseVectorKey, @@ -163,11 +164,15 @@ describe("validatePrefix", () => { expect(() => validatePrefix("")).not.toThrow() }) - test("rejects glob metacharacters that would escape the namespace subtree", () => { - expect(() => validatePrefix("foo*")).toThrow("glob characters") - expect(() => validatePrefix("foo?")).toThrow("glob characters") - expect(() => validatePrefix("foo[bar]")).toThrow("glob characters") - expect(() => validatePrefix("foo\\bar")).toThrow("glob characters") + test("accepts glob metacharacters — pattern builders escape them (Upstash parity)", () => { + // Upstash treats prefix as a literal string and ids may contain + // `* ? [ ] \`, so a prefix like "doc[1" must not 400; escapeGlobMeta + // neutralizes the metacharacters when the SCAN pattern is built. + // (deep audit 2026-09-06, F11) + expect(() => validatePrefix("foo*")).not.toThrow() + expect(() => validatePrefix("foo?")).not.toThrow() + expect(() => validatePrefix("foo[bar]")).not.toThrow() + expect(() => validatePrefix("foo\\bar")).not.toThrow() }) test("rejects control characters", () => { @@ -203,10 +208,20 @@ describe("validators throw ValidationError (typed)", () => { test("validatePrefix throws ValidationError", () => { try { - validatePrefix("a*") + validatePrefix("a\nb") throw new Error("expected throw") } catch (err) { expect(err).toBeInstanceOf(ValidationError) } }) }) + +describe("escapeGlobMeta", () => { + test("escapes every glob metacharacter", () => { + expect(escapeGlobMeta("doc[1]*?\\")).toBe("doc\\[1\\]\\*\\?\\\\") + }) + + test("leaves plain prefixes untouched", () => { + expect(escapeGlobMeta("doc-42:")).toBe("doc-42:") + }) +}) diff --git a/tests/unit/query-projection.test.ts b/tests/unit/query-projection.test.ts new file mode 100644 index 0000000..934b9d4 --- /dev/null +++ b/tests/unit/query-projection.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, mock, test } from "bun:test" + +type SendFn = (command: string, args: string[]) => Promise +const searchArgs: string[][] = [] +let send: SendFn = async () => ({ results: [] }) + +// executeQuery reaches Redis through ../redis's getClient(); stub it so the +// FT.SEARCH argument construction can be inspected. (deep audit 2026-09-06, F6) +mock.module("../../src/redis", () => ({ + getClient: () => ({ + send: (command: string, args: string[]) => { + if (command === "FT.SEARCH") searchArgs.push(args) + return send(command, args) + }, + }), + // Mirror the real module's remaining exports: other suites in the same + // process statically bind isRedisHealthy et al. + initRedis: async () => {}, + isRedisHealthy: async () => true, + reinitRedis: async () => {}, + closeRedis: async () => {}, +})) + +// Dynamic import (not static): the module under test must bind the redis stub +// mock.module registered above — static hoisting would bind the real module. +const { executeQuery } = await import("../../src/routes/query") + +const resp3Search = { + total_results: 1, + results: [ + { + id: "v:proj-test:vec-1", + extra_attributes: { _score: "0.5" }, + }, + ], +} + +function ftInfo(): unknown { + return { num_docs: 0, attributes: [{ dim: 3, distance_metric: "COSINE" }] } +} + +function returnBlock(args: string[]): string[] { + const idx = args.indexOf("RETURN") + if (idx === -1) return [] + const count = Number(args[idx + 1]) + return args.slice(idx + 2, idx + 2 + count) +} + +describe("FT.SEARCH field projection", () => { + test("projects only _score when no fields are requested", async () => { + send = async (command) => { + if (command === "FT.INFO") return ftInfo() + if (command === "FT.SEARCH") return resp3Search + throw new Error(`unexpected command ${command}`) + } + + await executeQuery("proj-test", { + vector: [1, 2, 3], + topK: 5, + includeMetadata: false, + includeVectors: false, + includeData: false, + }) + + const args = searchArgs.at(-1) + expect(args).toBeDefined() + // NOCONTENT alone would drop the _score attribute the parser reads. + expect(args?.includes("NOCONTENT")).toBe(false) + expect(returnBlock(args ?? [])).toEqual(["_score"]) + }) + + test("projects metadata for filtered queries without transferring vectors", async () => { + send = async (command) => { + if (command === "FT.INFO") return ftInfo() + if (command === "FT.SEARCH") return resp3Search + throw new Error(`unexpected command ${command}`) + } + + await executeQuery("proj-test", { + vector: [1, 2, 3], + topK: 5, + filter: "genre = 'drama'", + includeMetadata: false, + includeVectors: false, + includeData: false, + }) + + const fields = returnBlock(searchArgs.at(-1) ?? []) + expect(fields).toContain("_score") + expect(fields).toContain("metadata") + expect(fields).not.toContain("_vec") + expect(fields).not.toContain("data") + }) + + test("projects every requested include* field", async () => { + send = async (command) => { + if (command === "FT.INFO") return ftInfo() + if (command === "FT.SEARCH") return resp3Search + throw new Error(`unexpected command ${command}`) + } + + await executeQuery("proj-test", { + vector: [1, 2, 3], + topK: 5, + includeMetadata: true, + includeVectors: true, + includeData: true, + }) + + const fields = returnBlock(searchArgs.at(-1) ?? []) + expect(fields).toEqual(["_score", "metadata", "_vec", "data"]) + }) +}) diff --git a/tests/unit/route-validation.test.ts b/tests/unit/route-validation.test.ts index cfe9188..10cae79 100644 --- a/tests/unit/route-validation.test.ts +++ b/tests/unit/route-validation.test.ts @@ -12,11 +12,22 @@ let scanImpl: (cursor: string) => Promise<[string, string[]]> = async () => { const delCalls: string[][] = [] const fakeRedis = { hgetall: async () => ({}), + hget: async () => null, + exists: async () => 1, + hmget: async () => [null, null, null], del: async (...keys: string[]) => { delCalls.push(keys) return keys.length }, scan: (cursor: string) => scanImpl(cursor), + // FT.DROPINDEX during reset: missing-index wording so dropIndex swallows it + // and the SCAN cap is what surfaces. + send: async () => { + throw new Error("Unknown index name") + }, + sadd: async () => 1, + srem: async () => 1, + smembers: async () => [], } mock.module("../../src/redis", () => ({ @@ -31,10 +42,18 @@ mock.module("../../src/redis", () => ({ // Dynamic imports are required here: the route modules must be evaluated after // mock.module registers the redis stub, which static hoisting prevents. -const [{ deleteRoutes }, { fetchRoutes }, { collectScanKeys, rangeRoutes }] = await Promise.all([ +const [ + { deleteRoutes }, + { fetchRoutes }, + { collectScanKeys, rangeRoutes }, + { resetRoutes }, + { randomRoutes }, +] = await Promise.all([ import("../../src/routes/delete"), import("../../src/routes/fetch"), import("../../src/routes/range"), + import("../../src/routes/reset"), + import("../../src/routes/random"), ]) // Bun's RedisClient can't be constructed in tests; the stub implements the @@ -143,3 +162,76 @@ describe("range endpoint behavior below the scan cap", () => { expect(json.result?.vectors?.map((v) => v.id)).toEqual(["a", "b"]) }) }) + +describe("explicit-id caps match Upstash (no 1000 limit)", () => { + test("fetch accepts 1001 ids", async () => { + // Upstash documents the 1000-result cap only for prefix fetches; an + // explicit id list is unbounded. (deep audit 2026-09-06, F10) + const ids = Array.from({ length: 1001 }, (_, i) => `id-${i}`) + const res = await postJson(appWith(fetchRoutes), "/fetch/ns", { ids }) + expect(res.status).toBe(200) + const json = (await res.json()) as { result: Array<{ id: string } | null> } + expect(json.result).toHaveLength(1001) + expect(json.result[1000]).toEqual({ id: "id-1000" }) + }) + + test("delete accepts 1001 ids", async () => { + const ids = Array.from({ length: 1001 }, (_, i) => `id-${i}`) + const res = await postJson(appWith(deleteRoutes), "/delete/ns", { ids }) + expect(res.status).toBe(200) + const json = (await res.json()) as Envelope + expect(json.result?.deleted).toBe(1001) + }) +}) + +describe("scan iteration cap fails loudly on every path", () => { + // A truncated enumeration reported as success let reset/delete claim + // completion while matching keys survived — range already threw, these + // paths silently broke. (deep audit 2026-09-06, F3) + const endless = async (): Promise<[string, string[]]> => ["9", []] + + test("delete by prefix aborts with 400", async () => { + scanImpl = endless + const res = await postJson(appWith(deleteRoutes), "/delete/ns", { prefix: "a" }) + expect(res.status).toBe(400) + const json = (await res.json()) as Envelope + expect(json.error).toContain("scanable key limit") + }) + + test("delete by filter aborts with 400", async () => { + scanImpl = endless + const res = await postJson(appWith(deleteRoutes), "/delete/ns", { filter: "x > 1" }) + expect(res.status).toBe(400) + }) + + test("fetch by prefix aborts with 400", async () => { + scanImpl = endless + const res = await postJson(appWith(fetchRoutes), "/fetch/ns", { prefix: "a" }) + expect(res.status).toBe(400) + }) + + test("namespace reset aborts with 400", async () => { + scanImpl = endless + const res = await postJson(appWith(resetRoutes), "/reset/ns", {}) + expect(res.status).toBe(400) + }) + + test("random sample aborts with 400", async () => { + scanImpl = endless + const res = await postJson(appWith(randomRoutes), "/random/ns", {}) + expect(res.status).toBe(400) + }) +}) + +describe("fetch by prefix", () => { + test("returns a result object per scan-derived key", async () => { + scanImpl = async (): Promise<[string, string[]]> => ["0", ["v:ns:doc-1", "v:ns:doc-2"]] + const res = await postJson(appWith(fetchRoutes), "/fetch/ns", { + prefix: "doc", + includeMetadata: true, + }) + expect(res.status).toBe(200) + const json = (await res.json()) as { result: Array<{ id: string } | null> } + expect(json.result.map((v) => v?.id)).toEqual(["doc-1", "doc-2"]) + }) +}) diff --git a/tests/unit/sync-indexes.test.ts b/tests/unit/sync-indexes.test.ts new file mode 100644 index 0000000..c55ecc3 --- /dev/null +++ b/tests/unit/sync-indexes.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, mock, test } from "bun:test" + +type SendFn = (command: string, args: string[]) => Promise +const sendCalls: Array<{ command: string; index: string }> = [] +let send: SendFn = async () => null + +// syncIndexes reaches Redis through ../redis's getClient(); stub it so the +// startup FT._LIST sweep can be driven deterministically. +mock.module("../../src/redis", () => ({ + getClient: () => ({ + send: (command: string, args: string[]) => { + sendCalls.push({ command, index: args[0] ?? "" }) + return send(command, args) + }, + sadd: async () => 1, + }), + // Mirror the real module's remaining exports: other suites in the same + // process statically bind isRedisHealthy et al. + initRedis: async () => {}, + isRedisHealthy: async () => true, + reinitRedis: async () => {}, + closeRedis: async () => {}, +})) + +// Dynamic import (not static): the module under test must bind the redis stub +// mock.module registered above — static hoisting would bind the real module. +const { syncIndexes } = await import("../../src/translate/index") + +const resp3Info = (dim: number, metric: string) => ({ + num_docs: 0, + attributes: [{ dim, distance_metric: metric }], +}) + +describe("syncIndexes", () => { + test("ignores foreign FT._LIST indexes instead of aborting startup", async () => { + // FT._LIST is global to the Redis DB. A foreign app's vector index with a + // different DISTANCE_METRIC used to be validated like our own and crash + // startup with a metric-mismatch ValidationError. (deep audit 2026-09-06, F1) + send = async (command, args) => { + if (command === "FT._LIST") return ["foreign_index", "idx:app"] + if (command === "FT.INFO") { + return args[0] === "foreign_index" ? resp3Info(3, "IP") : resp3Info(8, "COSINE") + } + throw new Error(`unexpected command ${command}`) + } + + await expect(syncIndexes()).resolves.toBeUndefined() + + const infoTargets = sendCalls.filter((c) => c.command === "FT.INFO").map((c) => c.index) + expect(infoTargets).toEqual(["idx:app"]) + }) +})