From aae0acb9bf69598c18070890219adb5ca178ae96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20HOUZ=C3=89?= Date: Sun, 23 Aug 2026 14:12:56 +0200 Subject: [PATCH 1/2] Fail fast on unbalanced quotes in plain-text queries (#149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain-text (non-regex) queries with an odd number of unescaped double quotes were sent straight to the GitHub API, which rejects them with an opaque 422 ERROR_TYPE_QUERY_PARSING_FATAL. validateQuoteBalance() now detects this locally before any network call and exits with an actionable message, including a corrected example using GitHub's documented double-escaping syntax (shell + GitHub). Balanced-quote queries (including legitimate exact-phrase queries like "feature flag") and regex /pattern/ queries are unaffected — the latter are already validated separately by buildApiQuery/extractApiTerm. Closes #149 --- github-code-search.ts | 9 ++++++++- src/regex.test.ts | 46 ++++++++++++++++++++++++++++++++++++++++++- src/regex.ts | 43 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/github-code-search.ts b/github-code-search.ts index 895130a..fb5fc84 100644 --- a/github-code-search.ts +++ b/github-code-search.ts @@ -24,7 +24,7 @@ import { groupByTeamPrefix, flattenTeamSections, applyTeamPick } from "./src/gro import { checkForUpdate } from "./src/upgrade.ts"; import { runInteractive } from "./src/tui.ts"; import { generateCompletion, detectShell } from "./src/completions.ts"; -import { buildApiQuery, isRegexQuery } from "./src/regex.ts"; +import { buildApiQuery, isRegexQuery, validateQuoteBalance } from "./src/regex.ts"; import type { OutputFormat, OutputType } from "./src/types.ts"; // Version + build metadata injected at compile time via --define (see build.ts). @@ -235,6 +235,13 @@ async function searchAction( process.exit(1); } + // Fail fast on unbalanced quotes rather than surfacing a raw GitHub 422 — see issue #149 + const quoteError = validateQuoteBalance(query); + if (quoteError) { + console.error(pc.red(`Error: ${quoteError}`)); + process.exit(1); + } + const org = opts.org; const format: OutputFormat = opts.format === "json" ? "json" : "markdown"; const outputType: OutputType = opts.outputType === "repo-only" ? "repo-only" : "repo-and-matches"; diff --git a/src/regex.test.ts b/src/regex.test.ts index 8746b59..1ea7e38 100644 --- a/src/regex.test.ts +++ b/src/regex.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { buildApiQuery, escapeApiTerm, isRegexQuery } from "./regex.ts"; +import { buildApiQuery, escapeApiTerm, isRegexQuery, validateQuoteBalance } from "./regex.ts"; // ─── isRegexQuery ───────────────────────────────────────────────────────────── @@ -317,3 +317,47 @@ describe("buildApiQuery — warn cases", () => { expect(r.warn).toContain("/[/"); }); }); + +describe("validateQuoteBalance (issue #149)", () => { + it("returns null for a query with no quotes", () => { + expect(validateQuoteBalance("useFeatureFlag")).toBeNull(); + }); + + it("returns null for a balanced two-quote phrase", () => { + expect(validateQuoteBalance('"feature flag"')).toBeNull(); + }); + + it("returns an error for '\"react\": ' style queries with an odd number of unescaped quotes", () => { + // Regression: github-code-search '"react": "' --org fulll returned a raw + // GitHub 422 ERROR_TYPE_QUERY_PARSING_FATAL — this must now be caught locally. + const err = validateQuoteBalance('"react": "'); + expect(err).not.toBeNull(); + expect(err).toContain("Unbalanced double quotes"); + }); + + it("error message includes a corrected example using double escaping", () => { + const err = validateQuoteBalance('"react": "'); + expect(err).toContain('\\"react\\"'); + }); + + it("returns null when escaped quotes make the query GitHub-valid", () => { + // The shell must deliver the literal backslash-quote sequence for this to work + // (single-quoted at the shell level): github-code-search '"\"react\": \""' --org myorg + expect(validateQuoteBalance('"\\"react\\": \\""')).toBeNull(); + }); + + it("returns null for regex queries (validated separately by buildApiQuery)", () => { + // The /pattern/ token itself may contain an odd count of literal quote + // characters (e.g. "react":\s*" has 3) without being invalid GitHub syntax — + // extractApiTerm already escapes it correctly, so this check does not apply. + expect(validateQuoteBalance('/"react":\\s*"[~^]?[0-9]/')).toBeNull(); + }); + + it("returns an error for three unescaped quotes in a row", () => { + expect(validateQuoteBalance('"""')).not.toBeNull(); + }); + + it("returns null for four unescaped quotes (two balanced phrases)", () => { + expect(validateQuoteBalance('"foo" "bar"')).toBeNull(); + }); +}); diff --git a/src/regex.ts b/src/regex.ts index 66874ac..7734e6a 100644 --- a/src/regex.ts +++ b/src/regex.ts @@ -12,6 +12,49 @@ export function isRegexQuery(q: string): boolean { return extractRegexToken(q) !== null; } +/** + * Returns true when the `"` at `index` in `s` is escaped, i.e. preceded by an + * odd number of consecutive backslashes (GitHub's `\"` escape sequence). + */ +function isEscapedQuote(s: string, index: number): boolean { + let backslashes = 0; + let i = index - 1; + while (i >= 0 && s[i] === "\\") { + backslashes++; + i--; + } + return backslashes % 2 === 1; +} + +/** + * Validates that a plain-text (non-regex) query has a balanced number of + * unescaped `"` characters, as required by GitHub's query syntax — an odd + * count is rejected by the API with an opaque + * `ERROR_TYPE_QUERY_PARSING_FATAL` 422 error. + * + * Returns `null` when the query is valid (including regex queries, whose + * `/pattern/` token is validated separately by `buildApiQuery`). Returns a + * human-readable error message — including a corrected example using + * GitHub's documented double-escaping syntax — when the query would be + * rejected by the API. See issue #149. + */ +export function validateQuoteBalance(query: string): string | null { + if (isRegexQuery(query)) return null; + + let unescapedCount = 0; + for (let i = 0; i < query.length; i++) { + if (query[i] === '"' && !isEscapedQuote(query, i)) unescapedCount++; + } + if (unescapedCount % 2 === 0) return null; + + return ( + `Unbalanced double quotes in query: ${JSON.stringify(query)}. ` + + "GitHub rejects this with a query parsing error. " + + "To search for a literal quote character, escape it for both your shell and GitHub, " + + 'e.g.: github-code-search \'"\\"react\\": \\""\' --org myorg' + ); +} + /** * Given a raw query string (possibly mixing GitHub qualifiers and a /regex/flags * token), returns: From a154f7537735dad5b02573d5f9f82421d8d5d2a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20HOUZ=C3=89?= Date: Sun, 23 Aug 2026 16:34:58 +0200 Subject: [PATCH 2/2] Fix validateQuoteBalance to only exclude the regex token, not the whole query Addresses Copilot review feedback: validateQuoteBalance() previously returned null for any query containing a /pattern/ regex token (via isRegexQuery), skipping validation for everything else in the query. A mixed query like 'filename:package.json /regex/ "oops' could still reach the GitHub API with an unbalanced stray quote outside the token, hitting the same opaque 422 ERROR_TYPE_QUERY_PARSING_FATAL issue #149 was meant to prevent entirely. Now only the /pattern/ token itself is excluded from the quote-balance check (its quotes are handled separately by buildApiQuery/extractApiTerm), while the rest of the query (qualifiers, free text) is still validated. See https://github.com/fulll/github-code-search/pull/153#discussion_r3838702945 --- src/regex.test.ts | 20 ++++++++++++++++++-- src/regex.ts | 33 +++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/regex.test.ts b/src/regex.test.ts index 1ea7e38..e5dbac0 100644 --- a/src/regex.test.ts +++ b/src/regex.test.ts @@ -346,13 +346,29 @@ describe("validateQuoteBalance (issue #149)", () => { expect(validateQuoteBalance('"\\"react\\": \\""')).toBeNull(); }); - it("returns null for regex queries (validated separately by buildApiQuery)", () => { + it("only excludes the /pattern/ token itself, not the rest of a mixed query (issue #149 review)", () => { // The /pattern/ token itself may contain an odd count of literal quote // characters (e.g. "react":\s*" has 3) without being invalid GitHub syntax — - // extractApiTerm already escapes it correctly, so this check does not apply. + // extractApiTerm already escapes it correctly, so this check does not apply + // to a query made up of only a regex token. expect(validateQuoteBalance('/"react":\\s*"[~^]?[0-9]/')).toBeNull(); }); + it("still catches an unbalanced stray quote outside the regex token in a mixed query", () => { + // Regression: validateQuoteBalance previously bailed out entirely for any + // query containing a /pattern/ token (via isRegexQuery), so a stray + // unescaped quote in the surrounding qualifiers/text (outside the token) + // was never caught and could still reach the GitHub API unbalanced. + const err = validateQuoteBalance('filename:package.json /"react":\\s*"[~^]?[0-9]/ "oops'); + expect(err).not.toBeNull(); + expect(err).toContain("Unbalanced double quotes"); + }); + + it("does not flag a mixed query with balanced quotes outside the regex token", () => { + const err = validateQuoteBalance('"feature flag" /TODO|FIXME|HACK/'); + expect(err).toBeNull(); + }); + it("returns an error for three unescaped quotes in a row", () => { expect(validateQuoteBalance('"""')).not.toBeNull(); }); diff --git a/src/regex.ts b/src/regex.ts index 7734e6a..eda437f 100644 --- a/src/regex.ts +++ b/src/regex.ts @@ -27,23 +27,32 @@ function isEscapedQuote(s: string, index: number): boolean { } /** - * Validates that a plain-text (non-regex) query has a balanced number of - * unescaped `"` characters, as required by GitHub's query syntax — an odd - * count is rejected by the API with an opaque - * `ERROR_TYPE_QUERY_PARSING_FATAL` 422 error. + * Validates that a plain-text query has a balanced number of unescaped `"` + * characters, as required by GitHub's query syntax — an odd count is + * rejected by the API with an opaque `ERROR_TYPE_QUERY_PARSING_FATAL` 422 + * error. * - * Returns `null` when the query is valid (including regex queries, whose - * `/pattern/` token is validated separately by `buildApiQuery`). Returns a - * human-readable error message — including a corrected example using - * GitHub's documented double-escaping syntax — when the query would be - * rejected by the API. See issue #149. + * Quotes inside a `/pattern/` regex token are excluded from this check: they + * are handled separately by `buildApiQuery`/`extractApiTerm`, which escapes + * them for the API term. Only the token itself is excluded, not the rest of + * the query — a mixed query like `filename:package.json /regex/ "oops` must + * still be caught, since the stray quote outside the token would otherwise + * reach the GitHub API unbalanced. See issue #149. + * + * Returns `null` when the query is valid. Returns a human-readable error + * message — including a corrected example using GitHub's documented + * double-escaping syntax — when the query would be rejected by the API. */ export function validateQuoteBalance(query: string): string | null { - if (isRegexQuery(query)) return null; + const token = extractRegexToken(query); + const outsideToken = + token === null + ? query + : query.slice(0, token.index) + query.slice(token.index + token.raw.length); let unescapedCount = 0; - for (let i = 0; i < query.length; i++) { - if (query[i] === '"' && !isEscapedQuote(query, i)) unescapedCount++; + for (let i = 0; i < outsideToken.length; i++) { + if (outsideToken[i] === '"' && !isEscapedQuote(outsideToken, i)) unescapedCount++; } if (unescapedCount % 2 === 0) return null;