Skip to content

Commit 39becb9

Browse files
committed
fix(issue-list): auto-correct AND and reject OR in --query to prevent 400
Sentry search implicitly ANDs space-separated terms, so explicit AND operators have identical semantics. Strip them with a log.warn() nudge instead of letting the API reject the query with 400 Bad Request. OR operators have genuinely different semantics (union vs intersection) that can't be silently approximated, so throw a helpful ValidationError with actionable alternatives (separate queries, wildcards, docs link). Resolves CLI-BM, CLI-97, CLI-7B (300+ affected users).
1 parent 1163dac commit 39becb9

3 files changed

Lines changed: 202 additions & 5 deletions

File tree

src/commands/issue/list.ts

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,49 @@ function parseSort(value: string): SortValue {
204204
return value as SortValue;
205205
}
206206

207+
/** Matches standalone boolean OR operator (word boundary, not inside a qualifier value). */
208+
const BOOLEAN_OR_RE = /\bOR\b/;
209+
/** Matches standalone boolean AND operator. */
210+
const BOOLEAN_AND_RE = /\bAND\b/;
211+
212+
/**
213+
* Sanitize `--query` before sending to the Sentry API.
214+
*
215+
* - **AND**: Sentry search implicitly ANDs space-separated terms, so explicit
216+
* `AND` has identical semantics. Strip it and warn — the user's intent is
217+
* fulfilled without error.
218+
* - **OR**: Genuinely different semantics (union vs intersection). The API
219+
* rejects it with 400. Throw a helpful ValidationError since we cannot
220+
* silently approximate the intent.
221+
*
222+
* @returns The sanitized query string (AND stripped, OR rejected)
223+
*/
224+
function sanitizeQuery(
225+
query: string,
226+
log: { warn: (msg: string) => void }
227+
): string {
228+
if (BOOLEAN_OR_RE.test(query)) {
229+
throw new ValidationError(
230+
"Sentry search does not support the OR operator.\n\n" +
231+
"Alternatives:\n" +
232+
' - Use space-separated terms (implicit AND): --query "error timeout"\n' +
233+
" - Run separate queries for each term\n" +
234+
' - Use wildcards for partial matching: --query "*error*"\n\n' +
235+
"Search syntax: https://docs.sentry.io/concepts/search/",
236+
"query"
237+
);
238+
}
239+
if (BOOLEAN_AND_RE.test(query)) {
240+
const sanitized = query.replace(/\s*\bAND\b\s*/g, " ").trim();
241+
log.warn(
242+
"Sentry search implicitly ANDs terms — removed explicit AND operator. " +
243+
`Running query: "${sanitized}"`
244+
);
245+
return sanitized;
246+
}
247+
return query;
248+
}
249+
207250
/**
208251
* Format the issue list header with column titles.
209252
*
@@ -1450,6 +1493,7 @@ export const __testing = {
14501493
getComparator,
14511494
compareDates,
14521495
parseSort,
1496+
sanitizeQuery,
14531497
CURSOR_SEP,
14541498
MAX_LIMIT: LIST_MAX_LIMIT,
14551499
VALID_SORT_VALUES,
@@ -1599,6 +1643,7 @@ export const listCommand = buildListCommand("issue", {
15991643
},
16001644
async *func(this: SentryContext, flags: ListFlags, target?: string) {
16011645
const { cwd } = this;
1646+
const log = logger.withTag("issue.list");
16021647

16031648
const parsed = parseOrgProjectArg(target);
16041649

@@ -1616,20 +1661,25 @@ export const listCommand = buildListCommand("issue", {
16161661
);
16171662
}
16181663

1619-
const timeRange = parsePeriod(flags.period ?? DEFAULT_PERIOD);
1664+
// Sanitize --query: auto-correct AND (same semantics), reject OR (different semantics).
1665+
const effectiveFlags: ListFlags = flags.query
1666+
? { ...flags, query: sanitizeQuery(flags.query, log) }
1667+
: flags;
1668+
1669+
const timeRange = parsePeriod(effectiveFlags.period ?? DEFAULT_PERIOD);
16201670

16211671
// biome-ignore lint/suspicious/noExplicitAny: shared handler accepts any mode variant
16221672
const resolveAndHandle: ModeHandler<any> = (ctx) =>
16231673
handleResolvedTargets({
16241674
...ctx,
1625-
flags,
1675+
flags: effectiveFlags,
16261676
timeRange,
16271677
});
16281678

16291679
const result = (await dispatchOrgScopedList({
16301680
config: issueListMeta,
16311681
cwd,
1632-
flags,
1682+
flags: effectiveFlags,
16331683
parsed,
16341684
// When a bare slug matches a cached org, silently redirect to org-all
16351685
// mode instead of erroring (CLI-MC, 17 users). The user typed an org
@@ -1645,7 +1695,7 @@ export const listCommand = buildListCommand("issue", {
16451695
"org-all": (ctx) =>
16461696
handleOrgAllIssues({
16471697
org: ctx.parsed.org,
1648-
flags,
1698+
flags: effectiveFlags,
16491699
timeRange,
16501700
}),
16511701
},

test/commands/issue/list.property.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,3 +494,67 @@ describe("property: buildProjectAliasMap", () => {
494494
expect(Object.keys(entries).length).toBe(1);
495495
});
496496
});
497+
498+
// ---------------------------------------------------------------------------
499+
// sanitizeQuery
500+
// ---------------------------------------------------------------------------
501+
502+
const { sanitizeQuery } = __testing;
503+
504+
/** Generates a non-empty string that does NOT contain standalone OR or AND. */
505+
const safeTermArb = constantFrom(
506+
"is:unresolved",
507+
"level:error",
508+
"timeout",
509+
"crash",
510+
"http.status:500",
511+
"assigned:me",
512+
"sandbox",
513+
"order",
514+
"android"
515+
);
516+
517+
// biome-ignore lint/suspicious/noEmptyBlockStatements: intentional no-op
518+
const noopLog = { warn: () => {} };
519+
520+
describe("property: sanitizeQuery", () => {
521+
test("query with OR always throws ValidationError", () => {
522+
fcAssert(
523+
property(
524+
tuple(safeTermArb, safeTermArb).map(([a, b]) => `${a} OR ${b}`),
525+
(query) => {
526+
expect(() => sanitizeQuery(query, noopLog)).toThrow();
527+
}
528+
),
529+
{ numRuns: DEFAULT_NUM_RUNS }
530+
);
531+
});
532+
533+
test("AND removal preserves all non-AND tokens", () => {
534+
fcAssert(
535+
property(
536+
tuple(safeTermArb, safeTermArb).map(([a, b]) => `${a} AND ${b}`),
537+
(query) => {
538+
const result = sanitizeQuery(query, noopLog);
539+
// Both original terms must be present in the result
540+
const parts = query.split(/\s+AND\s+/);
541+
for (const part of parts) {
542+
expect(result).toContain(part.trim());
543+
}
544+
// AND must not be present
545+
expect(result).not.toContain("AND");
546+
}
547+
),
548+
{ numRuns: DEFAULT_NUM_RUNS }
549+
);
550+
});
551+
552+
test("safe queries pass through unchanged", () => {
553+
fcAssert(
554+
property(safeTermArb, (term) => {
555+
expect(sanitizeQuery(term, noopLog)).toBe(term);
556+
}),
557+
{ numRuns: DEFAULT_NUM_RUNS }
558+
);
559+
});
560+
});

test/commands/issue/list.test.ts

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
test,
1616
} from "bun:test";
1717
import {
18+
__testing,
1819
listCommand,
1920
PAGINATION_KEY,
2021
} from "../../../src/commands/issue/list.js";
@@ -29,7 +30,11 @@ import {
2930
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
3031
import * as paginationDb from "../../../src/lib/db/pagination.js";
3132
import { setOrgRegion } from "../../../src/lib/db/regions.js";
32-
import { ApiError, ResolutionError } from "../../../src/lib/errors.js";
33+
import {
34+
ApiError,
35+
ResolutionError,
36+
ValidationError,
37+
} from "../../../src/lib/errors.js";
3338
import { mockFetch, useTestConfigDir } from "../../helpers.js";
3439

3540
type ListFlags = {
@@ -1122,3 +1127,81 @@ describe("issue list: collapse parameter optimization", () => {
11221127
expect(options?.groupStatsPeriod).toBeUndefined();
11231128
});
11241129
});
1130+
1131+
// ---------------------------------------------------------------------------
1132+
// sanitizeQuery
1133+
// ---------------------------------------------------------------------------
1134+
1135+
const { sanitizeQuery } = __testing;
1136+
1137+
/** No-op logger for tests that don't check warnings */
1138+
// biome-ignore lint/suspicious/noEmptyBlockStatements: intentional no-op
1139+
const noopLog = { warn: () => {} };
1140+
1141+
describe("sanitizeQuery", () => {
1142+
test("passes through a simple query unchanged", () => {
1143+
expect(sanitizeQuery("is:unresolved level:error", noopLog)).toBe(
1144+
"is:unresolved level:error"
1145+
);
1146+
});
1147+
1148+
test("passes through a plain text query unchanged", () => {
1149+
expect(sanitizeQuery("timeout crash", noopLog)).toBe("timeout crash");
1150+
});
1151+
1152+
test("throws ValidationError for OR operator", () => {
1153+
expect(() => sanitizeQuery("error OR timeout", noopLog)).toThrow(
1154+
ValidationError
1155+
);
1156+
});
1157+
1158+
test("ValidationError for OR includes field and suggestions", () => {
1159+
try {
1160+
sanitizeQuery("login OR auth OR token", noopLog);
1161+
expect.unreachable("Should have thrown");
1162+
} catch (error) {
1163+
expect(error).toBeInstanceOf(ValidationError);
1164+
const ve = error as ValidationError;
1165+
expect(ve.field).toBe("query");
1166+
expect(ve.message).toContain("OR");
1167+
expect(ve.message).toContain("docs.sentry.io");
1168+
}
1169+
});
1170+
1171+
test("strips AND and returns cleaned query", () => {
1172+
expect(sanitizeQuery("error AND timeout", noopLog)).toBe("error timeout");
1173+
});
1174+
1175+
test("strips multiple AND operators", () => {
1176+
expect(sanitizeQuery("error AND timeout AND crash", noopLog)).toBe(
1177+
"error timeout crash"
1178+
);
1179+
});
1180+
1181+
test("AND removal emits a warning", () => {
1182+
const warnings: string[] = [];
1183+
const log = { warn: (msg: string) => warnings.push(msg) };
1184+
sanitizeQuery("error AND timeout", log);
1185+
expect(warnings).toHaveLength(1);
1186+
expect(warnings[0]).toContain("AND");
1187+
expect(warnings[0]).toContain("error timeout");
1188+
});
1189+
1190+
test("does not match lowercase 'and' or 'or'", () => {
1191+
// Sentry search operators are case-sensitive uppercase
1192+
expect(sanitizeQuery("sandbox handler", noopLog)).toBe("sandbox handler");
1193+
expect(sanitizeQuery("order error", noopLog)).toBe("order error");
1194+
});
1195+
1196+
test("rejects OR even with qualifiers mixed in", () => {
1197+
expect(() =>
1198+
sanitizeQuery("is:unresolved error OR timeout", noopLog)
1199+
).toThrow(ValidationError);
1200+
});
1201+
1202+
test("strips AND with qualifiers", () => {
1203+
expect(sanitizeQuery("is:unresolved AND level:error", noopLog)).toBe(
1204+
"is:unresolved level:error"
1205+
);
1206+
});
1207+
});

0 commit comments

Comments
 (0)