mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 04:02:18 +01:00
Harden company search query costs
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
24d53e652b
commit
a5cf5920f3
9 files changed
+197
-8
No files matched your search
@@ -0,0 +1,2 @@
|
||||
CREATE INDEX IF NOT EXISTS "documents_title_search_idx" ON "documents" USING gin ("title" gin_trgm_ops);--> statement-breakpoint
|
||||
CREATE INDEX IF NOT EXISTS "documents_latest_body_search_idx" ON "documents" USING gin ("latest_body" gin_trgm_ops);
|
||||
@@ -533,6 +533,13 @@
|
||||
"when": 1777572332006,
|
||||
"tag": "0075_cultured_sebastian_shaw",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 76,
|
||||
"version": "7",
|
||||
"when": 1777821410992,
|
||||
"tag": "0076_company_search_document_indexes",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -22,5 +22,7 @@ export const documents = pgTable(
|
||||
(table) => ({
|
||||
companyUpdatedIdx: index("documents_company_updated_idx").on(table.companyId, table.updatedAt),
|
||||
companyCreatedIdx: index("documents_company_created_idx").on(table.companyId, table.createdAt),
|
||||
titleSearchIdx: index("documents_title_search_idx").using("gin", table.title.op("gin_trgm_ops")),
|
||||
bodySearchIdx: index("documents_latest_body_search_idx").using("gin", table.latestBody.op("gin_trgm_ops")),
|
||||
}),
|
||||
);
|
||||
@@ -5,7 +5,7 @@ export const COMPANY_SEARCH_MAX_QUERY_LENGTH = 200;
|
||||
export const COMPANY_SEARCH_MAX_TOKENS = 8;
|
||||
export const COMPANY_SEARCH_DEFAULT_LIMIT = 20;
|
||||
export const COMPANY_SEARCH_MAX_LIMIT = 50;
|
||||
export const COMPANY_SEARCH_MAX_OFFSET = 5_000;
|
||||
export const COMPANY_SEARCH_MAX_OFFSET = 200;
|
||||
|
||||
function firstQueryValue(value: unknown): unknown {
|
||||
return Array.isArray(value) ? value[0] : value;
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import { createCompanySearchRateLimiter } from "../services/company-search-rate-limit.js";
|
||||
import type { CompanySearchQuery, CompanySearchResponse } from "@paperclipai/shared";
|
||||
|
||||
function createSearchResponse(query: CompanySearchQuery): CompanySearchResponse {
|
||||
return {
|
||||
query: query.q,
|
||||
normalizedQuery: query.q.trim().toLowerCase(),
|
||||
scope: query.scope,
|
||||
limit: query.limit,
|
||||
offset: query.offset,
|
||||
results: [],
|
||||
countsByType: { issue: 0, agent: 0, project: 0 },
|
||||
hasMore: false,
|
||||
};
|
||||
}
|
||||
|
||||
describe("company search route rate limiting", () => {
|
||||
it("rejects repeated same-actor search calls before invoking search", async () => {
|
||||
const search = vi.fn(async (_companyId: string, query: CompanySearchQuery) => createSearchResponse(query));
|
||||
const app = express();
|
||||
app.use((req, _res, next) => {
|
||||
req.actor = {
|
||||
type: "agent",
|
||||
agentId: "agent-1",
|
||||
companyId: "company-1",
|
||||
source: "agent_key",
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes({} as never, {} as never, {
|
||||
searchService: { search },
|
||||
searchRateLimiter: createCompanySearchRateLimiter({
|
||||
maxRequests: 1,
|
||||
windowMs: 60_000,
|
||||
now: () => 1_000,
|
||||
}),
|
||||
}));
|
||||
|
||||
await request(app).get("/api/companies/company-1/search?q=wizard").expect(200);
|
||||
const limited = await request(app).get("/api/companies/company-1/search?q=wizard").expect(429);
|
||||
|
||||
expect(search).toHaveBeenCalledTimes(1);
|
||||
expect(limited.body).toMatchObject({
|
||||
error: "Search rate limit exceeded",
|
||||
retryAfterSeconds: 60,
|
||||
});
|
||||
expect(limited.headers["retry-after"]).toBe("60");
|
||||
});
|
||||
});
|
||||
@@ -16,7 +16,11 @@ import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { companySearchService } from "../services/company-search.js";
|
||||
import {
|
||||
COMPANY_SEARCH_BRANCH_FETCH_LIMIT,
|
||||
companySearchBranchFetchLimit,
|
||||
companySearchService,
|
||||
} from "../services/company-search.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
@@ -38,9 +42,17 @@ describe("company search query validation", () => {
|
||||
|
||||
expect(parsed.q).toHaveLength(COMPANY_SEARCH_MAX_QUERY_LENGTH);
|
||||
expect(parsed.limit).toBe(50);
|
||||
expect(parsed.offset).toBe(5000);
|
||||
expect(parsed.offset).toBe(200);
|
||||
expect(parsed.scope).toBe("all");
|
||||
});
|
||||
|
||||
it("keeps the internal per-branch fetch limit independent of high offsets", () => {
|
||||
const lowOffset = companySearchQuerySchema.parse({ q: "needle", limit: "50", offset: "0" });
|
||||
const highOffset = companySearchQuerySchema.parse({ q: "needle", limit: "50", offset: "9000" });
|
||||
|
||||
expect(companySearchBranchFetchLimit(lowOffset.limit)).toBe(COMPANY_SEARCH_BRANCH_FETCH_LIMIT);
|
||||
expect(companySearchBranchFetchLimit(highOffset.limit)).toBe(COMPANY_SEARCH_BRANCH_FETCH_LIMIT);
|
||||
});
|
||||
});
|
||||
|
||||
describeEmbeddedPostgres("companySearchService", () => {
|
||||
|
||||
@@ -31,6 +31,8 @@ import {
|
||||
updateIssueSchema,
|
||||
getClosedIsolatedExecutionWorkspaceMessage,
|
||||
isClosedIsolatedExecutionWorkspace,
|
||||
type CompanySearchQuery,
|
||||
type CompanySearchResponse,
|
||||
type ExecutionWorkspace,
|
||||
} from "@paperclipai/shared";
|
||||
import { trackAgentTaskCompleted } from "@paperclipai/shared/telemetry";
|
||||
@@ -79,6 +81,10 @@ import { executionWorkspaceService as executionWorkspaceServiceDirect } from "..
|
||||
import { feedbackService } from "../services/feedback.js";
|
||||
import { instanceSettingsService } from "../services/instance-settings.js";
|
||||
import { environmentService } from "../services/environments.js";
|
||||
import {
|
||||
createCompanySearchRateLimiter,
|
||||
type CompanySearchRateLimiter,
|
||||
} from "../services/company-search-rate-limit.js";
|
||||
import {
|
||||
applyIssueExecutionPolicyTransition,
|
||||
normalizeIssueExecutionPolicy,
|
||||
@@ -95,6 +101,9 @@ const updateIssueRouteSchema = updateIssueSchema.extend({
|
||||
|
||||
type ParsedExecutionState = NonNullable<ReturnType<typeof parseIssueExecutionState>>;
|
||||
type NormalizedExecutionPolicy = NonNullable<ReturnType<typeof normalizeIssueExecutionPolicy>>;
|
||||
type CompanySearchService = {
|
||||
search(companyId: string, query: CompanySearchQuery): Promise<CompanySearchResponse>;
|
||||
};
|
||||
type ActivityIssueRelationSummary = {
|
||||
id: string;
|
||||
identifier: string | null;
|
||||
@@ -152,6 +161,23 @@ function summarizeIssueRelationForActivity(relation: {
|
||||
};
|
||||
}
|
||||
|
||||
const defaultCompanySearchRateLimiter = createCompanySearchRateLimiter();
|
||||
|
||||
function companySearchRateLimitActor(req: Request, companyId: string) {
|
||||
if (req.actor.type === "agent") {
|
||||
return {
|
||||
companyId,
|
||||
actorType: "agent" as const,
|
||||
actorId: req.actor.agentId ?? req.actor.keyId ?? "unknown-agent",
|
||||
};
|
||||
}
|
||||
return {
|
||||
companyId,
|
||||
actorType: "board" as const,
|
||||
actorId: req.actor.userId ?? req.actor.source ?? "board",
|
||||
};
|
||||
}
|
||||
|
||||
function summarizeIssueReferenceActivityDetails(input:
|
||||
| {
|
||||
addedReferencedIssues: ActivityIssueRelationSummary[];
|
||||
@@ -447,6 +473,8 @@ export function issueRoutes(
|
||||
now?: Date;
|
||||
}): Promise<unknown>;
|
||||
};
|
||||
searchService?: CompanySearchService;
|
||||
searchRateLimiter?: CompanySearchRateLimiter;
|
||||
pluginWorkerManager?: PluginWorkerManager;
|
||||
} = {},
|
||||
) {
|
||||
@@ -458,7 +486,8 @@ export function issueRoutes(
|
||||
});
|
||||
const feedback = feedbackService(db);
|
||||
const companiesSvc = companyService(db);
|
||||
const searchSvc = companySearchService(db);
|
||||
const searchSvc = opts.searchService ?? companySearchService(db);
|
||||
const searchRateLimiter = opts.searchRateLimiter ?? defaultCompanySearchRateLimiter;
|
||||
const instanceSettings = instanceSettingsService(db);
|
||||
const agentsSvc = agentService(db);
|
||||
const projectsSvc = projectService(db);
|
||||
@@ -951,6 +980,17 @@ export function issueRoutes(
|
||||
const companyId = req.params.companyId as string;
|
||||
assertCompanyAccess(req, companyId);
|
||||
const query = companySearchQuerySchema.parse(req.query);
|
||||
const rateLimit = searchRateLimiter.consume(companySearchRateLimitActor(req, companyId));
|
||||
res.setHeader("X-RateLimit-Limit", String(rateLimit.limit));
|
||||
res.setHeader("X-RateLimit-Remaining", String(rateLimit.remaining));
|
||||
if (!rateLimit.allowed) {
|
||||
res.setHeader("Retry-After", String(rateLimit.retryAfterSeconds));
|
||||
res.status(429).json({
|
||||
error: "Search rate limit exceeded",
|
||||
retryAfterSeconds: rateLimit.retryAfterSeconds,
|
||||
});
|
||||
return;
|
||||
}
|
||||
const result = await searchSvc.search(companyId, query);
|
||||
res.json(result);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
export const COMPANY_SEARCH_RATE_LIMIT_WINDOW_MS = 60_000;
|
||||
export const COMPANY_SEARCH_RATE_LIMIT_MAX_REQUESTS = 60;
|
||||
|
||||
export type CompanySearchRateLimitActor = {
|
||||
companyId: string;
|
||||
actorType: "agent" | "board";
|
||||
actorId: string;
|
||||
};
|
||||
|
||||
export type CompanySearchRateLimitResult = {
|
||||
allowed: boolean;
|
||||
limit: number;
|
||||
remaining: number;
|
||||
retryAfterSeconds: number;
|
||||
};
|
||||
|
||||
export type CompanySearchRateLimiter = {
|
||||
consume(actor: CompanySearchRateLimitActor): CompanySearchRateLimitResult;
|
||||
};
|
||||
|
||||
export function createCompanySearchRateLimiter(options: {
|
||||
windowMs?: number;
|
||||
maxRequests?: number;
|
||||
now?: () => number;
|
||||
} = {}): CompanySearchRateLimiter {
|
||||
const windowMs = options.windowMs ?? COMPANY_SEARCH_RATE_LIMIT_WINDOW_MS;
|
||||
const maxRequests = options.maxRequests ?? COMPANY_SEARCH_RATE_LIMIT_MAX_REQUESTS;
|
||||
const now = options.now ?? Date.now;
|
||||
const hitsByKey = new Map<string, number[]>();
|
||||
|
||||
function key(actor: CompanySearchRateLimitActor) {
|
||||
return `${actor.companyId}:${actor.actorType}:${actor.actorId}`;
|
||||
}
|
||||
|
||||
return {
|
||||
consume(actor) {
|
||||
const currentTime = now();
|
||||
const cutoff = currentTime - windowMs;
|
||||
const actorKey = key(actor);
|
||||
const recentHits = (hitsByKey.get(actorKey) ?? []).filter((hit) => hit > cutoff);
|
||||
|
||||
if (recentHits.length >= maxRequests) {
|
||||
const oldestHit = recentHits[0] ?? currentTime;
|
||||
hitsByKey.set(actorKey, recentHits);
|
||||
return {
|
||||
allowed: false,
|
||||
limit: maxRequests,
|
||||
remaining: 0,
|
||||
retryAfterSeconds: Math.max(1, Math.ceil((oldestHit + windowMs - currentTime) / 1000)),
|
||||
};
|
||||
}
|
||||
|
||||
recentHits.push(currentTime);
|
||||
hitsByKey.set(actorKey, recentHits);
|
||||
return {
|
||||
allowed: true,
|
||||
limit: maxRequests,
|
||||
remaining: Math.max(0, maxRequests - recentHits.length),
|
||||
retryAfterSeconds: 0,
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -3,6 +3,7 @@ import type { SQL } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { agents, companies, issues, projects } from "@paperclipai/db";
|
||||
import {
|
||||
COMPANY_SEARCH_MAX_LIMIT,
|
||||
COMPANY_SEARCH_MAX_TOKENS,
|
||||
type CompanySearchIssueSummary,
|
||||
type CompanySearchQuery,
|
||||
@@ -16,6 +17,7 @@ import {
|
||||
const MIN_TOKEN_LENGTH = 2;
|
||||
const MIN_FUZZY_QUERY_LENGTH = 4;
|
||||
const SNIPPET_MAX_CHARS = 240;
|
||||
export const COMPANY_SEARCH_BRANCH_FETCH_LIMIT = COMPANY_SEARCH_MAX_LIMIT + 1;
|
||||
|
||||
type IssueSearchRow = {
|
||||
id: string;
|
||||
@@ -259,6 +261,11 @@ function simpleTextCondition(fields: SQL[], containsPattern: string, tokenArray:
|
||||
return sql<boolean>`(${sql.join([...phraseConditions, ...tokenConditions], sql` OR `)})`;
|
||||
}
|
||||
|
||||
export function companySearchBranchFetchLimit(limit: number) {
|
||||
const normalizedLimit = Number.isFinite(limit) ? Math.max(1, Math.floor(limit)) : COMPANY_SEARCH_MAX_LIMIT;
|
||||
return Math.min(COMPANY_SEARCH_BRANCH_FETCH_LIMIT, normalizedLimit + 1);
|
||||
}
|
||||
|
||||
export function companySearchService(db: Db) {
|
||||
return {
|
||||
search: async (companyId: string, query: CompanySearchQuery): Promise<CompanySearchResponse> => {
|
||||
@@ -287,7 +294,7 @@ export function companySearchService(db: Db) {
|
||||
.where(eq(companies.id, companyId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
const prefix = routePrefix(company?.issuePrefix);
|
||||
const fetchLimit = offset + limit + 1;
|
||||
const fetchLimit = companySearchBranchFetchLimit(limit);
|
||||
const tokenArray = sqlTextArray(tokens);
|
||||
const containsPattern = `%${normalizedQuery}%`;
|
||||
const startsWithPattern = `${normalizedQuery}%`;
|
||||
@@ -526,6 +533,7 @@ export function companySearchService(db: Db) {
|
||||
))
|
||||
.orderBy(desc(score), desc(issues.updatedAt), desc(issues.id))
|
||||
.limit(fetchLimit)
|
||||
.offset(offset)
|
||||
: [];
|
||||
|
||||
const simpleCondition = simpleTextCondition([
|
||||
@@ -547,6 +555,7 @@ export function companySearchService(db: Db) {
|
||||
.where(and(eq(agents.companyId, companyId), simpleCondition))
|
||||
.orderBy(desc(agents.updatedAt), desc(agents.id))
|
||||
.limit(fetchLimit)
|
||||
.offset(offset)
|
||||
: [];
|
||||
|
||||
const projectCondition = simpleTextCondition([
|
||||
@@ -565,6 +574,7 @@ export function companySearchService(db: Db) {
|
||||
.where(and(eq(projects.companyId, companyId), isNull(projects.archivedAt), projectCondition))
|
||||
.orderBy(desc(projects.updatedAt), desc(projects.id))
|
||||
.limit(fetchLimit)
|
||||
.offset(offset)
|
||||
: [];
|
||||
|
||||
const results: CompanySearchResult[] = [
|
||||
@@ -606,7 +616,7 @@ export function companySearchService(db: Db) {
|
||||
return (right.updatedAt ?? "").localeCompare(left.updatedAt ?? "");
|
||||
});
|
||||
|
||||
const paged = results.slice(offset, offset + limit);
|
||||
const paged = results.slice(0, limit);
|
||||
return {
|
||||
query: query.q,
|
||||
normalizedQuery,
|
||||
@@ -615,7 +625,7 @@ export function companySearchService(db: Db) {
|
||||
offset,
|
||||
results: paged,
|
||||
countsByType: makeCounts(results),
|
||||
hasMore: results.length > offset + limit,
|
||||
hasMore: results.length > limit,
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
Reference in new issue
Block a user