Skip to content

Escape marketplace search filters#394

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/search-filter-escape-more
Jun 4, 2026
Merged

Escape marketplace search filters#394
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/search-filter-escape-more

Conversation

@rissrice2105-agent

Copy link
Copy Markdown
Contributor

Extends PostgREST search escaping to marketplace routes that interpolate user search text into .or(...) filters: skills, MCP listings, affiliate offers, and global search. This preserves literal %, _, comma, parentheses, and dot characters instead of letting them act as SQL/PostgREST filter syntax.

Validation:

  • vitest run src/lib/security/sanitize.test.ts src/app/api/skills/route.test.ts src/app/api/mcp/route.test.ts src/app/api/affiliates/offers/route.test.ts src/app/api/search/route.test.ts
  • tsc --noEmit

@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown

Greptile Summary

This PR centralises PostgREST filter escaping into a new escapePostgrestSearchValue helper and applies it to four marketplace search routes (skills, MCP, affiliate offers, global search). The global search route's prior manual %/_-only escaping is replaced; the other three routes had no escaping at all.

  • escapePostgrestSearchValue escapes \\, %, _, ,, (, ), and . in a single-pass regex, covering both SQL LIKE wildcards and PostgREST filter-string syntax delimiters.
  • Each affected route now calls the helper before interpolating user input into .or(...) filter strings, with new integration tests that spy on the or call and assert the exact escaped string.
  • The skills route test uses \"100%_mcp,(v1.2)\" as its search input — a copy-paste from the MCP test — which is misleading but does not affect correctness.

Confidence Score: 4/5

Safe to merge; the escaping logic is correct and consistently applied across all four routes.

The core change is straightforward and well-tested at the integration level. The only gaps are a copy-pasted test label in the skills test and a missing backslash case in the escapePostgrestSearchValue unit suite — neither affects runtime behaviour.

src/lib/security/sanitize.test.ts could benefit from a backslash-input test case to fully document the escaping contract.

Important Files Changed

Filename Overview
src/lib/security/sanitize.ts Adds escapePostgrestSearchValue that escapes backslash, %, _, comma, parens, and dot via a single-pass regex; logic is sound but unit tests cover only one scenario.
src/lib/security/sanitize.test.ts Adds one escapePostgrestSearchValue test case; missing coverage for backslash input and the happy-path no-special-chars case.
src/app/api/search/route.ts Replaces manual %/_ escaping with escapePostgrestSearchValue; correctly broadens escaping to PostgREST syntax chars for all search types.
src/app/api/skills/route.ts Wires in escapePostgrestSearchValue before building the .or() ilike filter; straightforward and correct.
src/app/api/mcp/route.ts Same pattern as skills route — swaps raw interpolation for the new escape helper before the .or() call.
src/app/api/affiliates/offers/route.ts Applies escapePostgrestSearchValue to the q parameter before building title/description ilike filter; correct and consistent with other routes.
src/app/api/skills/route.test.ts Adds escape coverage test but the input string uses mcp in a skills test context — copy-pasted from the MCP test.
src/app/api/mcp/route.test.ts Adds a well-structured orSpy-based test that verifies the full escaped filter string is passed to PostgREST.
src/app/api/affiliates/offers/route.test.ts Refactors the existing search test to use a proxy-based chain that captures the or call and verifies the escaped string.
src/app/api/search/route.test.ts Adds a focused test for the new punctuation escaping in the type=posts code path; assertion is precise and complete.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User query string] --> B[escapePostgrestSearchValue]
    B --> C[Single-pass regex replaces backslash percent underscore comma parens dot]
    C --> D[safeSearch or escaped]
    D --> E1[skills/mcp/affiliates .or filter]
    D --> E2[search route pattern wrapped in wildcards]
    E1 --> F[PostgREST HTTP API]
    E2 --> F
    F --> G[PostgreSQL ILIKE]
    G --> H[Results]
Loading

Reviews (1): Last reviewed commit: "Escape marketplace search filters" | Re-trigger Greptile

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

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!

Comment on lines +144 to +149
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\\)%"
);

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!

@ralyodio ralyodio merged commit 5d9e413 into profullstack:master Jun 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants