Escape directory search filters#448
Conversation
Greptile SummaryThis PR fixes unescaped user input being interpolated directly into PostgREST filter strings in the directory search endpoint. The
Confidence Score: 4/5Safe to merge — the core fix is correct and the two-step pipeline (XSS sanitization then PostgREST escaping) is sound. The route change is minimal and correct: sanitizeSearchParams + escapePostgrestSearchValue closes the filter-injection window. The new test exercises all the documented special characters. The only gap is that the backslash character, which escapePostgrestSearchValue does escape, is not covered by any test in this PR, leaving a silent failure mode if the sanitization pipeline is ever reordered. The new test file (route.test.ts) would benefit from including a backslash in its input string to verify end-to-end doubling of \ through the escape function. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant GET as GET /api/directory
participant sanitize as sanitizeSearchParams
participant escape as escapePostgrestSearchValue
participant Supabase
Client->>GET: "?search=100%_demo,(v1.2)*"
GET->>sanitize: sanitizeSearchParams(url, "search")
sanitize-->>GET: ""100%_demo,(v1.2)*" (XSS chars stripped)"
GET->>escape: escapePostgrestSearchValue(search)
escape-->>GET: "100\\%\\_demo\\,\\(v1\\.2\\)\\*"
GET->>Supabase: query.or("title.ilike.%100\\%\\_demo...%,...")
Supabase-->>GET: "{ data, count, error }"
GET-->>Client: "200 { listings, total, page, per_page }"
Reviews (1): Last reviewed commit: "Escape directory search filters" | Re-trigger Greptile |
| const res = await GET(makeRequest({ search: "100%_demo,(v1.2)*" })); | ||
|
|
||
| expect(res.status).toBe(200); | ||
| expect(chain.or).toHaveBeenCalledWith( | ||
| "title.ilike.%100\\%\\_demo\\,\\(v1\\.2\\)\\*%,description.ilike.%100\\%\\_demo\\,\\(v1\\.2\\)\\*%" | ||
| ); |
There was a problem hiding this comment.
The test input covers %, _, comma, parentheses, dot, and * but omits backslash. The escape function does handle
\ (the regex [\\%*_,().] includes it), but nothing currently verifies that a literal backslash in user input is doubled to \\ rather than passed through or silently dropped. If sanitizeUrlParam were ever updated to strip backslashes, the escape ordering would silently break with no test failure.
| const res = await GET(makeRequest({ search: "100%_demo,(v1.2)*" })); | |
| expect(res.status).toBe(200); | |
| expect(chain.or).toHaveBeenCalledWith( | |
| "title.ilike.%100\\%\\_demo\\,\\(v1\\.2\\)\\*%,description.ilike.%100\\%\\_demo\\,\\(v1\\.2\\)\\*%" | |
| ); | |
| const res = await GET(makeRequest({ search: "100%_demo,(v1.2)*\\path" })); | |
| expect(res.status).toBe(200); | |
| expect(chain.or).toHaveBeenCalledWith( | |
| "title.ilike.%100\\%\\_demo\\,\\(v1\\.2\\)\\*\\\\path%,description.ilike.%100\\%\\_demo\\,\\(v1\\.2\\)\\*\\\\path%" | |
| ); |
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!
|
CI is green now: build, test, Greptile, Socket, and security checks all passed. |
Fixes #447.
What changed
escapePostgrestSearchValuehelper for/api/directorysearch filters.searchURL parameter through the shared URL parameter sanitizer.%,_, comma, parentheses, dot, and*in directory search.Validation
./node_modules/.bin/vitest.cmd run src/app/api/directory/route.test.ts./node_modules/.bin/tsc.cmd --noEmit