Skip to content
Merged
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
25 changes: 22 additions & 3 deletions src/app/api/affiliates/offers/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,30 @@ describe("GET /api/affiliates/offers", () => {
});

it("filters by search query (#21)", async () => {
mockFrom.mockReturnValue(chainable([], null, 0));
const orSpy = vi.fn();
const queryChain: Record<string, any> = {};
const chainHandler: ProxyHandler<any> = {
get(_target, prop) {
if (prop === "then") return undefined;
if (prop === "data") return [];
if (prop === "error") return null;
if (prop === "count") return 0;
if (prop === "or") {
return (...args: any[]) => {
orSpy(...args);
return new Proxy(queryChain, chainHandler);
};
}
return (..._args: any[]) => new Proxy(queryChain, chainHandler);
},
};
mockFrom.mockReturnValue(new Proxy(queryChain, chainHandler));

const res = await GET(makeRequest({ q: "test search" }));
const res = await GET(makeRequest({ q: "100%_offer,(v1.2)" }));
expect(res.status).toBe(200);
expect(mockFrom).toHaveBeenCalled();
expect(orSpy).toHaveBeenCalledWith(
"title.ilike.%100\\%\\_offer\\,\\(v1\\.2\\)%,description.ilike.%100\\%\\_offer\\,\\(v1\\.2\\)%"
);
});

it("hides product_url from unauthenticated users (#20)", async () => {
Expand Down
4 changes: 3 additions & 1 deletion src/app/api/affiliates/offers/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server";
import { getAuthContext } from "@/lib/auth/get-user";
import { createServiceClient } from "@/lib/supabase/service";
import { checkRateLimit, rateLimitExceeded, getRateLimitIdentifier } from "@/lib/rate-limit";
import { escapePostgrestSearchValue } from "@/lib/security/sanitize";

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type AnySupabase = any;
Expand Down Expand Up @@ -64,7 +65,8 @@ export async function GET(request: NextRequest) {
if (slugFilter) {
query = query.eq("slug", slugFilter);
} else if (search) {
query = query.or(`title.ilike.%${search}%,description.ilike.%${search}%`);
const safeSearch = escapePostgrestSearchValue(search);
query = query.or(`title.ilike.%${safeSearch}%,description.ilike.%${safeSearch}%`);
}

// Sort
Expand Down
26 changes: 26 additions & 0 deletions src/app/api/mcp/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,32 @@ describe("GET /api/mcp", () => {
expect(json.page).toBe(1);
});

it("escapes search text before building PostgREST filters", async () => {
const orSpy = vi.fn(() => ({
order: () => ({
range: () => Promise.resolve({ data: [], count: 0, error: null }),
}),
}));

mockFrom.mockReturnValue({
select: () => ({
eq: () => ({
or: orSpy,
order: () => ({
range: () => Promise.resolve({ data: [], count: 0, error: null }),
}),
}),
}),
});

const response = await GET(makeGetRequest({ search: "100%_mcp,(v1.2)" }));

expect(response.status).toBe(200);
expect(orSpy).toHaveBeenCalledWith(
"title.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%,description.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%,tagline.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%"
);
});

it("returns empty listings when none exist", async () => {
mockFrom.mockReturnValue({
select: () => ({
Expand Down
5 changes: 3 additions & 2 deletions src/app/api/mcp/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getAuthContext } from "@/lib/auth/get-user";
import { createServiceClient } from "@/lib/supabase/service";
import { mcpListingSchema, slugify } from "@/lib/mcp/validation";
import { combinedScan, MCP_SCANNER_VERSION } from "@/lib/mcp/security-scan";
import { sanitizeSearchParams } from "@/lib/security/sanitize";
import { escapePostgrestSearchValue, sanitizeSearchParams } from "@/lib/security/sanitize";

const MAX_PAGE = 100_000;

Expand Down Expand Up @@ -40,7 +40,8 @@ export async function GET(request: NextRequest) {
.eq("status", "active");

if (search) {
query = query.or(`title.ilike.%${search}%,description.ilike.%${search}%,tagline.ilike.%${search}%`);
const safeSearch = escapePostgrestSearchValue(search);
query = query.or(`title.ilike.%${safeSearch}%,description.ilike.%${safeSearch}%,tagline.ilike.%${safeSearch}%`);
}

if (category) {
Expand Down
11 changes: 11 additions & 0 deletions src/app/api/search/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,17 @@ describe("GET /api/search", () => {
expect(orArg).toContain("my\\_var");
});

it("escapes PostgREST filter punctuation in search query", async () => {
const chain = chainResult({ data: [], error: null, count: 0 });
mockFrom.mockReturnValue(chain);

await GET(makeRequest({ q: "foo,(v1.2)", type: "posts" }));

expect(chain.or).toHaveBeenCalled();
const orArg = chain.or.mock.calls[0][0] as string;
expect(orArg).toBe("content.ilike.%foo\\,\\(v1\\.2\\)%");
});

// ── Empty results ─────────────────────────────────────────────

it("returns proper empty structure for type=all", async () => {
Expand Down
4 changes: 2 additions & 2 deletions src/app/api/search/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { NextRequest, NextResponse } from "next/server";
import { createClient } from "@/lib/supabase/server";
import { escapePostgrestSearchValue } from "@/lib/security/sanitize";

type SearchType = "gigs" | "agents" | "posts" | "all";

Expand Down Expand Up @@ -40,8 +41,7 @@ export async function GET(request: NextRequest) {
const supabase = await createClient();
const offset = (page - 1) * limit;

// Escape special ilike characters
const escaped = query.replace(/%/g, "\\%").replace(/_/g, "\\_");
const escaped = escapePostgrestSearchValue(query);
const pattern = `%${escaped}%`;

const results: Record<string, unknown> = {};
Expand Down
26 changes: 26 additions & 0 deletions src/app/api/skills/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,32 @@ describe("GET /api/skills", () => {
expect(json.page).toBe(1);
});

it("escapes search text before building PostgREST filters", async () => {
const orSpy = vi.fn(() => ({
order: () => ({
range: () => Promise.resolve({ data: [], count: 0, error: null }),
}),
}));

mockFrom.mockReturnValue({
select: () => ({
eq: () => ({
or: orSpy,
order: () => ({
range: () => Promise.resolve({ data: [], count: 0, error: null }),
}),
}),
}),
});

const response = await GET(makeGetRequest({ search: "100%_mcp,(v1.2)" }));

expect(response.status).toBe(200);
expect(orSpy).toHaveBeenCalledWith(
"title.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%,description.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%,tagline.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%"
);
Comment on lines +144 to +149

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The search input uses "100%_mcp,(v1.2)" — the mcp substring is copy-pasted from the MCP route test and is semantically confusing in a skills test. Renaming it to something skills-related keeps the test self-documenting.

Suggested change
const response = await GET(makeGetRequest({ search: "100%_mcp,(v1.2)" }));
expect(response.status).toBe(200);
expect(orSpy).toHaveBeenCalledWith(
"title.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%,description.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%,tagline.ilike.%100\\%\\_mcp\\,\\(v1\\.2\\)%"
);
const response = await GET(makeGetRequest({ search: "100%_skill,(v1.2)" }));
expect(response.status).toBe(200);
expect(orSpy).toHaveBeenCalledWith(
"title.ilike.%100\\%\\_skill\\,\\(v1\\.2\\)%,description.ilike.%100\\%\\_skill\\,\\(v1\\.2\\)%,tagline.ilike.%100\\%\\_skill\\,\\(v1\\.2\\)%"
);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

});

it("clamps invalid page values to the first page", async () => {
const range = vi.fn(() =>
Promise.resolve({ data: [], count: 0, error: null })
Expand Down
5 changes: 3 additions & 2 deletions src/app/api/skills/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { skillListingSchema } from "@/lib/skills/validation";
import { slugify } from "@/lib/skills/validation";
import { importSkillFromUrl } from "@/lib/skills/url-import";
import { isScanAcceptable } from "@/lib/skills/security-scan";
import { sanitizeSearchParams } from "@/lib/security/sanitize";
import { escapePostgrestSearchValue, sanitizeSearchParams } from "@/lib/security/sanitize";

const MAX_PAGE = 100_000;

Expand Down Expand Up @@ -56,7 +56,8 @@ export async function GET(request: NextRequest) {
.eq("status", "active");

if (search) {
query = query.or(`title.ilike.%${search}%,description.ilike.%${search}%,tagline.ilike.%${search}%`);
const safeSearch = escapePostgrestSearchValue(search);
query = query.or(`title.ilike.%${safeSearch}%,description.ilike.%${safeSearch}%,tagline.ilike.%${safeSearch}%`);
}

if (category) {
Expand Down
10 changes: 9 additions & 1 deletion src/lib/security/sanitize.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from "vitest";
import { sanitizeUrlParam, sanitizeSearchParams } from "./sanitize";
import { escapePostgrestSearchValue, sanitizeUrlParam, sanitizeSearchParams } from "./sanitize";

describe("sanitizeUrlParam", () => {
it("should return empty string for null/undefined input", () => {
Expand Down Expand Up @@ -51,3 +51,11 @@ describe("sanitizeSearchParams", () => {
expect(sanitizeSearchParams(url, "missing")).toBe("");
});
});

describe("escapePostgrestSearchValue", () => {
it("escapes LIKE wildcards and PostgREST filter punctuation", () => {
expect(escapePostgrestSearchValue("100%_match,(v1.2)")).toBe(
"100\\%\\_match\\,\\(v1\\.2\\)"
);
});
});
Comment on lines +55 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The single test case doesn't exercise the backslash-escaping branch, which is the most sensitive part of the regex — an input of \ must become \\ so that PostgREST and SQL LIKE each see the correct number of escape sequences. Adding a backslash-only case makes that explicit.

Suggested change
describe("escapePostgrestSearchValue", () => {
it("escapes LIKE wildcards and PostgREST filter punctuation", () => {
expect(escapePostgrestSearchValue("100%_match,(v1.2)")).toBe(
"100\\%\\_match\\,\\(v1\\.2\\)"
);
});
});
describe("escapePostgrestSearchValue", () => {
it("escapes LIKE wildcards and PostgREST filter punctuation", () => {
expect(escapePostgrestSearchValue("100%_match,(v1.2)")).toBe(
"100\\%\\_match\\,\\(v1\\.2\\)"
);
});
it("escapes backslash so it does not act as a dangling LIKE escape character", () => {
expect(escapePostgrestSearchValue("foo\\bar")).toBe("foo\\\\bar");
});
it("leaves plain alphanumeric strings unchanged", () => {
expect(escapePostgrestSearchValue("hello world")).toBe("hello world");
});
});

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

9 changes: 9 additions & 0 deletions src/lib/security/sanitize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,12 @@ export function sanitizeSearchParams(
const value = url.searchParams.get(param);
return sanitizeUrlParam(value);
}

/**
* Escape user text before interpolating it into a PostgREST filter string.
* PostgREST uses punctuation such as commas, periods, and parentheses as
* filter syntax, while SQL LIKE treats % and _ as wildcards.
*/
export function escapePostgrestSearchValue(value: string): string {
return value.replace(/[\\%_,().]/g, (char) => `\\${char}`);
}
Loading