diff --git a/src/regex.test.ts b/src/regex.test.ts index 8add72b..8746b59 100644 --- a/src/regex.test.ts +++ b/src/regex.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { buildApiQuery, isRegexQuery } from "./regex.ts"; +import { buildApiQuery, escapeApiTerm, isRegexQuery } from "./regex.ts"; // ─── isRegexQuery ───────────────────────────────────────────────────────────── @@ -240,6 +240,64 @@ describe("buildApiQuery — special escape handling in longestLiteralSequence", }); }); +describe("buildApiQuery — quote handling in extracted terms (issue #147)", () => { + it('/"react":\\s*"[~^]?[0-9]/ → escaped literal \'"\\"react\\""\' (quotes kept, not dropped)', () => { + // Regression: the previous behaviour dropped the `"` characters entirely, + // yielding the bare term "react" — verified via the GitHub API to return + // 20288 results (mostly noise: react-native, @types/react, docs, imports) + // instead of the ~227 candidates the quoted literal returns, all of which + // are then narrowed further by the local regex filter. + const r = buildApiQuery('/"react":\\s*"[~^]?[0-9]/'); + expect(r.apiQuery).toBe('"\\"react\\""'); + expect(r.regexFilter).not.toBeNull(); + expect(r.warn).toBeUndefined(); + }); + + it('/"axios"/ → escaped literal \'"\\"axios\\""\'', () => { + const r = buildApiQuery('/"axios"/'); + expect(r.apiQuery).toBe('"\\"axios\\""'); + }); + + it("/from.*['\\\"]axios/ → axios (quotes inside a character class are unaffected)", () => { + // Regression guard: quotes inside [...] must still be skipped as before — + // only quotes appearing as literal (non-class) pattern characters change. + const r = buildApiQuery("/from.*['\\\"]axios/"); + expect(r.apiQuery).toBe("axios"); + }); + + it("/TODO|FIXME|HACK/ → unquoted OR join is unaffected when no branch has quotes", () => { + const r = buildApiQuery("/TODO|FIXME|HACK/"); + expect(r.apiQuery).toBe("TODO OR FIXME OR HACK"); + }); +}); + +describe("escapeApiTerm — backslash escaping (CodeQL: incomplete string escaping)", () => { + it("returns the term unchanged when it has no quote", () => { + expect(escapeApiTerm("axios")).toBe("axios"); + }); + + it("returns a term with a bare backslash and no quote unchanged", () => { + // No quote present → short-circuits before any escaping is needed. + expect(escapeApiTerm("foo\\bar")).toBe("foo\\bar"); + }); + + it('escapes a lone quote and wraps the term: "react" → "\\"react\\""', () => { + expect(escapeApiTerm('"react"')).toBe('"\\"react\\""'); + }); + + it("escapes backslashes before escaping quotes, so a backslash never swallows the following escaped quote", () => { + // Regression: term containing a literal backslash immediately followed by + // a quote. Escaping quotes without first escaping backslashes would leave + // the backslash unescaped, producing an invalid/ambiguous sequence for + // GitHub's parser (which only recognises \\ and \" as escapes). + const term = '"a\\b"'; + const result = escapeApiTerm(term); + // Every literal backslash in the input must itself be escaped to \\, + // and every literal quote must be escaped to \", all wrapped in "...". + expect(result).toBe('"\\"a\\\\b\\""'); + }); +}); + describe("buildApiQuery — warn cases", () => { it("/[~^]?[0-9]+\\.[0-9]+/ → empty term + warn", () => { const r = buildApiQuery("/[~^]?[0-9]+\\.[0-9]+/"); diff --git a/src/regex.ts b/src/regex.ts index d209c1e..66874ac 100644 --- a/src/regex.ts +++ b/src/regex.ts @@ -111,6 +111,23 @@ function extractRegexToken(q: string): RegexToken | null { return { raw, pattern, flags, index: tokenStart }; } +/** + * Escape a literal API term for GitHub's exact-phrase query syntax when it + * contains a `"` character — e.g. `"react"` → `"\"react\""`. + * See GitHub's "Searching for quotes and backslashes" documentation, which + * defines `\\` and `\"` as the only two recognised escape sequences. Backslashes + * are escaped first so a literal `\` immediately preceding a `"` cannot be + * misread as escaping that quote once wrapped — see CodeQL "Incomplete string + * escaping or encoding" (js/incomplete-sanitization). + * Terms without a `"` are returned unchanged (no behaviour change for the + * common case). Exported for unit testing. + */ +export function escapeApiTerm(term: string): string { + if (!term.includes('"')) return term; + const escaped = term.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); + return `"${escaped}"`; +} + /** * Derive a literal API search term from a regex pattern. * @@ -119,6 +136,11 @@ function extractRegexToken(q: string): RegexToken | null { * nested inside `(...)` or `[...]`) → join branches with ` OR `. * 2. Otherwise → extract all unescaped literal sequences, pick the longest one. * 3. If the best term is shorter than 3 characters → return `warn`. + * + * A `"` character is a valid literal (not a regex metacharacter) and is kept + * in the extracted term; it is then escaped and the whole term wrapped in an + * outer pair of quotes so the GitHub API treats it as a literal quote rather + * than stripping it — see issue #147. */ function extractApiTerm(pattern: string): { term: string; warn?: string } { // 1. Top-level alternation detection. @@ -130,7 +152,7 @@ function extractApiTerm(pattern: string): { term: string; warn?: string } { // "< 3 chars → warn + empty term" rule still applies. const branchTerms = branches.map((b) => longestLiteralSequence(b)); if (branchTerms.every((t) => t.length >= 3)) { - return { term: branchTerms.join(" OR ") }; + return { term: branchTerms.map(escapeApiTerm).join(" OR ") }; } } @@ -144,7 +166,7 @@ function extractApiTerm(pattern: string): { term: string; warn?: string } { "Use --regex-hint to specify the term to send to the GitHub API.", }; } - return { term }; + return { term: escapeApiTerm(term) }; } /** @@ -202,9 +224,12 @@ function splitTopLevelAlternation(pattern: string): string[] { * Extract the longest contiguous sequence of characters useful as a GitHub * search term from a regex pattern fragment. * - * Only `[a-zA-Z0-9_-]` characters are accumulated — punctuation and special - * characters that are valid regex literals (e.g. `\(`) are intentionally - * excluded because they produce poor search terms. + * `[a-zA-Z0-9_"-]` characters are accumulated — `"` is a valid literal (not a + * regex metacharacter) and is kept so terms like `"react"` survive extraction + * instead of being broken into the far too broad bare word `react` — see + * issue #147. Other punctuation and special characters that are valid regex + * literals (e.g. `\(`) are intentionally excluded because they produce poor + * search terms. * Character classes `[...]` are skipped entirely. * Uses `>=` when updating `best` so that later (more specific) sequences of * equal length are preferred over earlier structural ones (e.g. `old-lib` @@ -238,14 +263,15 @@ function longestLiteralSequence(pattern: string): string { // Handle escape sequences. if (ch === "\\") { const next = pattern[i + 1] ?? ""; - // Only accumulate if the escaped char is a word character or hyphen - // AND is not a common regex escape or backreference (\b, \d, \s, \w, - // \p, \u, \x, \1–9, …) or control-character escape (\n, \r, \t, \f, \v). + // Only accumulate if the escaped char is a word character, hyphen or + // double quote, AND is not a common regex escape or backreference + // (\b, \d, \s, \w, \p, \u, \x, \1–9, …) or control-character escape + // (\n, \r, \t, \f, \v). // Note: \a and \e are NOT in this list — in JS without u/v they are // identity escapes that simply match the literal letter ('a' or 'e'), // so they should be accumulated, not broken on. // \c = control escape (\cA–\cZ), \k = named back-reference (\k). - const isWordLike = /[a-zA-Z0-9_-]/.test(next); + const isWordLike = /[a-zA-Z0-9_"-]/.test(next); const isSpecialEscape = /[bBdDsSwWpPuUxX0-9nrtfvck]/.test(next); if (isWordLike && !isSpecialEscape) { current += next; @@ -260,7 +286,7 @@ function longestLiteralSequence(pattern: string): string { } // Only accumulate characters that make a useful GitHub search term. - if (/[a-zA-Z0-9_-]/.test(ch)) { + if (/[a-zA-Z0-9_"-]/.test(ch)) { current += ch; } else { if (current.length >= best.length) best = current;