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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions api/src/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ describe("configuration", () => {
delete process.env.QUERY_MAX_LIMIT;
delete process.env.QUERY_EXPENSIVE_DOCS_EXAMINED;
delete process.env.QUERY_EXPENSIVE_EXAMINED_RATIO;
delete process.env.QUERY_MAX_FANOUT_PARENTS;
delete process.env.QUERY_FANOUT_CONCURRENCY;
delete process.env.QUERY_FANOUT_STRIKE_THRESHOLD;
delete process.env.QUERY_RATE_LIMIT_ENABLED;
delete process.env.QUERY_RATE_LIMIT_FREE_STRIKES;
delete process.env.QUERY_RATE_LIMIT_BASE_BACKOFF_MS;
Expand All @@ -102,6 +105,9 @@ describe("configuration", () => {
expect(config.query.maxLimit).toBe(500);
expect(config.query.expensiveDocsExamined).toBe(1000);
expect(config.query.expensiveExaminedRatio).toBe(10);
expect(config.query.maxFanoutParents).toBe(200);
expect(config.query.fanoutConcurrency).toBe(20);
expect(config.query.fanoutStrikeThreshold).toBe(25);
expect(config.query.rateLimit).toEqual({
enabled: false,
freeStrikes: 3,
Expand All @@ -121,4 +127,15 @@ describe("configuration", () => {
expect(config.query.rateLimit.freeStrikes).toBe(5);
expect(config.query.expensiveDocsExamined).toBe(2000);
});

it("should read the fan-out cap/concurrency/strike config from env vars", () => {
process.env.QUERY_MAX_FANOUT_PARENTS = "50";
process.env.QUERY_FANOUT_CONCURRENCY = "5";
process.env.QUERY_FANOUT_STRIKE_THRESHOLD = "10";

const config = configuration();
expect(config.query.maxFanoutParents).toBe(50);
expect(config.query.fanoutConcurrency).toBe(5);
expect(config.query.fanoutStrikeThreshold).toBe(10);
});
});
21 changes: 21 additions & 0 deletions api/src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,24 @@ export type QueryConfig = {
* QUERY_EXPENSIVE_EXAMINED_RATIO (default 10).
*/
expensiveExaminedRatio: number;
/**
* Maximum distinct `parentId` values a `parentId.$in` fan-out query may request.
* Guards against a caller forcing one CouchDB request per id with no upper bound.
* Environment variable: QUERY_MAX_FANOUT_PARENTS (default 200).
*/
maxFanoutParents: number;
/**
* Maximum concurrent CouchDB requests for one parentId fan-out. Bounds load even
* when the fan-out is large but under the cap above.
* Environment variable: QUERY_FANOUT_CONCURRENCY (default 20).
*/
fanoutConcurrency: number;
/**
* parentId fan-out size above which a rate-limiter strike is recorded immediately,
* rather than waiting on post-hoc query-cost stats. Environment variable:
* QUERY_FANOUT_STRIKE_THRESHOLD (default 25).
*/
fanoutStrikeThreshold: number;
/** Per-identity expensive-query rate limiter (default off). */
rateLimit: QueryRateLimitConfig;
};
Expand Down Expand Up @@ -121,6 +139,9 @@ export default () =>
maxLanguages: parseInt(process.env.QUERY_MAX_LANGUAGES, 10) || 4,
expensiveDocsExamined: parseInt(process.env.QUERY_EXPENSIVE_DOCS_EXAMINED, 10) || 1000,
expensiveExaminedRatio: parseInt(process.env.QUERY_EXPENSIVE_EXAMINED_RATIO, 10) || 10,
maxFanoutParents: parseInt(process.env.QUERY_MAX_FANOUT_PARENTS, 10) || 200,
fanoutConcurrency: parseInt(process.env.QUERY_FANOUT_CONCURRENCY, 10) || 20,
fanoutStrikeThreshold: parseInt(process.env.QUERY_FANOUT_STRIKE_THRESHOLD, 10) || 25,
rateLimit: {
enabled: process.env.QUERY_RATE_LIMIT_ENABLED === "true",
freeStrikes: parseInt(process.env.QUERY_RATE_LIMIT_FREE_STRIKES, 10) || 3,
Expand Down
22 changes: 22 additions & 0 deletions api/src/endpoints/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Endpoints

REST endpoints exposed by the API: `changeRequest`, `query`, `ftsSearch`, `storageStatus`. All are gated by `AuthGuard`. See `api/CLAUDE.md` for the full architecture of each; this file documents the `/query` request-time guards in `query.controller.ts`, since they're easy to lose track of.

## `/query` request-time guards

`processPostReq` runs a fixed sequence of cheap, pre-execution checks before a Mango query ever reaches CouchDB. Each one exists to stop a specific failure mode; none of them are diagnostic-only — every check here either rejects a request or feeds the rate limiter.

1. **Rate-limit gate** (`rateLimiter.check`) — an identity already in backoff from prior expensive queries is rejected outright with `429` + `Retry-After`. No-op when `QueryRateLimiterService` is disabled.
2. **Epoch-cursor rejection** — `selector.updatedTimeUtc` with both `$lte` and `$gte` at `0` can never match a real document (timestamps are always `> 0`), but CouchDB still has to walk the chosen index before returning empty. A sync client with a corrupted/reset cursor could otherwise generate a full-index scan on every poll. This check runs even when `BYPASS_TEMPLATE_VALIDATION=true`, because it isn't schema validation — it's an invariant about what a valid query can ever match.
3. **`parentId` fan-out cap** (`countParentIdFanout`) — `QueryService` issues one CouchDB request per id in `selector.parentId.$in`, so an unbounded array is worse than the full scan it replaced. Sizes above `query.maxFanoutParents` (default 200) are rejected; sizes above `query.fanoutStrikeThreshold` (default 25) are allowed but immediately strike the rate limiter, since the cost is known before the query runs — no need to wait on post-hoc `execution_stats`.
4. **Schema/operator validation** (`validateQuery`) — the universal selector validator (shape, `limit` cap, `use_index` registry membership, operator allowlist, per-request language cap for non-CMS queries). Skippable via `BYPASS_TEMPLATE_VALIDATION` (dev/test only).

After the query executes, `classifyQueryCost` inspects `execution_stats` to decide if the query was expensive (docs-examined threshold or examined:returned ratio). An expensive query logs a `warn("Expensive /query", …)` line — `identifier`, `identity`, `reason`, the execution stats, `use_index`, and a `selectorFingerprint(body)` of the post-injection selector — and strikes the rate limiter (enforcement bites the *next* request from that identity, not this one). `execution_stats` itself is always stripped from the client response.

`body.identifier` is not used for dispatch — it's only a caller-supplied label carried into this log line and into rate-limit context (e.g. `"sync"` for app/CMS sync calls).

### Adding a new pre-execution guard

If you find another selector shape that's cheap to reject up front (known before `QueryService` runs), follow the pattern above: check it before `validateQuery`, throw `BadRequestException` for the reject case, and call `rateLimiter.recordStrike(identityKey)` for an allowed-but-costly case rather than waiting on `execution_stats`. Keep the check itself in `query.controller.ts` as a small pure function (see `countParentIdFanout`) so it's unit-testable without a DB.

Avoid adding fields here purely for logging/debugging an investigation — that kind of temporary diagnostic instrumentation tends to outlive the investigation it was added for. If you need to correlate an expensive-query spike with request shape, prefer extending `selectorFingerprint` (already computed on every expensive-query log line) over introducing a new ad-hoc context object.
87 changes: 87 additions & 0 deletions api/src/endpoints/query.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ describe("QueryController", () => {
return 1000;
case "query.expensiveExaminedRatio":
return 10;
case "query.maxFanoutParents":
return 200;
case "query.fanoutStrikeThreshold":
return 25;
default:
return undefined;
}
Expand Down Expand Up @@ -99,6 +103,89 @@ describe("QueryController", () => {
expect(queryService.query).not.toHaveBeenCalled();
});

it("rejects an updatedTimeUtc range with both bounds at 0 even in bypass mode", async () => {
configService.get.mockImplementation(configFor(true));

const body = {
identifier: "sync",
selector: {
type: "post",
updatedTimeUtc: { $lte: 0, $gte: 0 },
},
};

await expect(
controller.processPostReq(body, mockRequest(), mockReply()),
).rejects.toThrow("updatedTimeUtc $lte and $gte must not both be 0");
expect(queryService.query).not.toHaveBeenCalled();
});

it("allows a sync history range whose lower bound alone is 0", async () => {
configService.get.mockImplementation(configFor(true));
queryService.query.mockResolvedValue({ docs: [] });

const body = {
identifier: "sync",
selector: {
type: "post",
updatedTimeUtc: { $lte: Number.MAX_SAFE_INTEGER, $gte: 0 },
},
};

await controller.processPostReq(body, mockRequest(), mockReply());

expect(queryService.query).toHaveBeenCalledTimes(1);
});

it("rejects a parentId fan-out above the configured max", async () => {
configService.get.mockImplementation(configFor(true));

const body = {
selector: {
type: "content",
parentId: { $in: Array.from({ length: 201 }, (_, i) => `p${i}`) },
},
};

await expect(
controller.processPostReq(body, mockRequest(), mockReply()),
).rejects.toThrow("'parentId.$in' exceeds the maximum fan-out size (200)");
expect(queryService.query).not.toHaveBeenCalled();
});

it("records an immediate strike for an oversized-but-under-cap parentId fan-out", async () => {
configService.get.mockImplementation(configFor(true));
queryService.query.mockResolvedValue({ docs: [] });

const body = {
selector: {
type: "content",
parentId: { $in: Array.from({ length: 26 }, (_, i) => `p${i}`) },
},
};

await controller.processPostReq(body, mockRequest(), mockReply());

expect(rateLimiter.recordStrike).toHaveBeenCalledWith("mock-user");
expect(queryService.query).toHaveBeenCalledTimes(1);
});

it("does not strike for a parentId fan-out at or below the strike threshold", async () => {
configService.get.mockImplementation(configFor(true));
queryService.query.mockResolvedValue({ docs: [] });

const body = {
selector: {
type: "content",
parentId: { $in: Array.from({ length: 25 }, (_, i) => `p${i}`) },
},
};

await controller.processPostReq(body, mockRequest(), mockReply());

expect(rateLimiter.recordStrike).not.toHaveBeenCalled();
});

it("removes identifier from the body before passing to the service", async () => {
configService.get.mockImplementation(configFor(true));
queryService.query.mockResolvedValue({ docs: [] });
Expand Down
44 changes: 44 additions & 0 deletions api/src/endpoints/query.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { FastifyReply, FastifyRequest } from "fastify";
import { classifyQueryCost } from "./queryStats";
import { selectorFingerprint } from "../util/selectorFingerprint";
import { QueryRateLimiterService } from "../ratelimit/queryRateLimiter.service";
import { expandMangoSelector } from "../util/expandMangoQuery";

/** Endpoint supporting MongoDB like queries (Mango Query) */
@Controller("query")
Expand Down Expand Up @@ -58,6 +59,33 @@ export class QueryController {
);
}

// A sync cursor collapsed to the epoch cannot match a valid updatedTimeUtc value, but
// CouchDB may still walk the selected index before returning no documents. Keep this
// invariant outside the optional validation bypass so the impossible query never reaches
// QueryService/CouchDB.
const updatedTimeUtc = body?.selector?.updatedTimeUtc;
if (updatedTimeUtc?.$lte === 0 && updatedTimeUtc?.$gte === 0) {
throw new BadRequestException(
"Invalid query: updatedTimeUtc $lte and $gte must not both be 0",
);
}

// Reject an oversized parentId fan-out, and strike large-but-allowed ones early —
// the fan-out size is known before the query runs, so abuse doesn't need to wait
// for post-hoc execution_stats.
const maxFanoutParents = this.configService.get<number>("query.maxFanoutParents") ?? 200;
const fanoutStrikeThreshold =
this.configService.get<number>("query.fanoutStrikeThreshold") ?? 25;
const fanoutSize = countParentIdFanout(body?.selector);
if (fanoutSize > maxFanoutParents) {
throw new BadRequestException(
`Invalid query: 'parentId.$in' exceeds the maximum fan-out size (${maxFanoutParents})`,
);
}
if (fanoutSize > fanoutStrikeThreshold) {
this.rateLimiter.recordStrike(identityKey);
}

const bypassValidation =
this.configService.get<boolean>("validation.bypassTemplateValidation") || false;

Expand Down Expand Up @@ -113,3 +141,19 @@ export class QueryController {
return result;
}
}

/**
* Size of the selector's `parentId.$in` array, or 0 if absent. Selector is expanded
* first since `parentId` may sit at the top level or nested under `$and`.
*/
function countParentIdFanout(selector: any): number {
if (!selector || typeof selector !== "object") return 0;
const conditions = expandMangoSelector(selector).$and ?? [];
for (const condition of conditions) {
const value = (condition as any)?.parentId;
if (value && typeof value === "object" && Array.isArray(value.$in)) {
return value.$in.length;
}
}
return 0;
}
Loading
Loading