Skip to content

Commit bbc6be6

Browse files
committed
fix(search): reject comparison values in OR merge (age:>24h, times_seen:>100)
Comparison operators (>, <, >=, <=, !=) produce numeric/date/duration filters in the PEG grammar, not text filters, so they cannot appear inside in-list brackets. Without this guard, age:>24h OR age:>7d would produce the invalid age:[>24h,>7d].
1 parent 2bb060b commit bbc6be6

3 files changed

Lines changed: 32 additions & 3 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -985,7 +985,7 @@ mock.module("./some-module", () => ({
985985
### Architecture
986986
987987
<!-- lore:019d919d-048a-7361-a5cb-8b7479d2e74f -->
988-
* **Centralized search query sanitization in src/lib/search-query.ts**: \`sanitizeQuery()\` in \`src/lib/search-query.ts\` is used as the Stricli \`parse\` function on all 7 \`--query\` flags (issue list, trace list, span list, log list, trace logs, event list, issue events). Commands get sanitized queries automatically at flag-parse time — no explicit call needed in \`func()\` bodies. The \`sanitizeQuery(query: string): string\` overload matches Stricli's parse signature. Dashboard \`widget add/edit\` are excluded (their \`--query\` means aggregate expressions). Each command keeps its own \`brief\` text (with domain-specific examples), so a single shared \`LIST\_QUERY\_FLAG\` constant wouldn't eliminate all duplication. The module also exports \`QUERY\_TOKEN\_RE\` regex and \`\_\_testing\` internals for tests.
988+
* **Centralized search query sanitization in src/lib/search-query.ts**: \`sanitizeQuery()\` in \`src/lib/search-query.ts\` is the Stricli \`parse\` function on all 7 \`--query\` flags (issue list, trace list, span list, log list, trace logs, event list, issue events). Commands get sanitized queries automatically at flag-parse time. Dashboard \`widget add/edit\` excluded (their \`--query\` means aggregate expressions). The module also exports \`SEARCH\_SYNTAX\_REFERENCE\` — a machine-readable search syntax summary injected into empty JSON envelopes to help agents discover query syntax. Originally lived in \`issue/list.ts\` but was lifted to \`search-query.ts\` for reuse across all \`--query\` commands. Also exports \`QUERY\_TOKEN\_RE\` regex and \`\_\_testing\` internals for tests.
989989
990990
<!-- lore:019d2c68-e14a-7bd2-aa25-4aabb481c08f -->
991991
* **Dashboard widget interval computed from terminal width and layout before API calls**: Dashboard widget interval from terminal width: \`computeOptimalInterval()\` in \`src/lib/api/dashboards.ts\` calculates chart interval before API calls. Formula: \`colWidth = floor(layout.w / 6 \* termWidth)\`, \`chartWidth = colWidth - 4 - gutterW\`, \`idealSeconds = periodSeconds / chartWidth\`. Snaps to nearest Sentry bucket (1m–1d). \`periodToSeconds()\` parses \`"24h"\`, \`"7d"\` etc. \`queryWidgetTimeseries\` uses \`params.interval ?? widget.interval\`.
@@ -1072,5 +1072,5 @@ mock.module("./some-module", () => ({
10721072
* **Redact sensitive flags in raw argv before sending to telemetry**: Telemetry argv redaction and context: \`withTelemetry\` calls \`initTelemetryContext()\` — sets user ID, email, runtime, is\_self\_hosted. Org from \`getDefaultOrganization()\` (SQLite). \`SENSITIVE\_FLAGS\` in \`telemetry.ts\` (currently \`token\`) — scans for \`--token\`/\`-token\`, replaces value with \`\[REDACTED]\`. Handles \`--flag value\` and \`--flag=value\`. Command tag format: \`sentry.issue.list\` (dot-joined with \`sentry.\` prefix).
10731073
10741074
<!-- lore:019d91ea-bb02-78d4-bf11-743b99f0ba6b -->
1075-
* **Stricli parse functions can perform validation and sanitization at flag-parse time**: Stricli's \`parse\` function on \`kind: "parsed"\` flags runs during argument parsing before \`func()\` executes. It can throw errors (including \`ValidationError\`) and log warnings — errors are caught by Stricli's error handling. This pattern is used for both \`parseCursorFlag\` (cursor validation) and \`sanitizeQuery\` (query sanitization with OR-rewrite warnings). When a parse function's signature matches \`(raw: string) => string\`, it works directly as a Stricli parse function. This eliminates the need for explicit sanitization/validation calls in command \`func()\` bodies, making the pattern zero-effort for consumers.
1075+
* **Stricli parse functions can perform validation and sanitization at flag-parse time**: Stricli's \`parse\` function on \`kind: "parsed"\` flags runs during argument parsing before \`func()\` executes. It can throw errors (including \`ValidationError\`) and log warnings. When a parse function's signature matches \`(raw: string) => T\`, it works directly as a Stricli parse function. Current uses: \`parseCursorFlag\` (cursor validation), \`sanitizeQuery\` (query sanitization with OR-rewrite). User expressed interest in applying this pattern to \`parsePeriod\` for \`--period\` flags too — would change flag type from \`string\` to \`TimeRange\`, eliminating repeated \`parsePeriod(flags.period ?? DEFAULT\_PERIOD)\` calls across commands. This is a follow-up refactor opportunity.
10761076
<!-- End lore-managed section -->

src/lib/search-query.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ const IN_LIST_VALUE_RE = /^\[(.+)\]$/;
7272
*/
7373
const INVALID_INLIST_KEYS = new Set(["is", "has"]);
7474

75+
/**
76+
* Matches values that use comparison operators (numeric, date, duration).
77+
*
78+
* These are parsed by the PEG grammar as `numeric_filter`, `date_filter`,
79+
* or `duration_filter` — NOT `text_filter` — and cannot appear inside
80+
* in-list brackets. Examples: `>100`, `<=50`, `>=2024-01-01`, `>1s`.
81+
*/
82+
const COMPARISON_VALUE_RE = /^[><=!]/;
83+
7584
/** Parsed qualifier token. */
7685
type ParsedQualifier = {
7786
negated: boolean;
@@ -220,6 +229,12 @@ function canMergeQualifiers(parsed: ParsedQualifier[]): boolean {
220229
return false;
221230
}
222231

232+
// No comparison operators in values (>, <, >=, <=, !=) — these are
233+
// numeric/date/duration filters, not text filters, and can't go in brackets
234+
if (parsed.some((q) => COMPARISON_VALUE_RE.test(q.value))) {
235+
return false;
236+
}
237+
223238
// No wildcards in values
224239
if (parsed.some((q) => q.value.includes("*"))) {
225240
return false;
@@ -412,7 +427,8 @@ function handleOr(tokens: string[], hasAnd: boolean): string {
412427
" - Free-text terms without a key (error OR timeout)\n" +
413428
" - Different keys (level:error OR assigned:me)\n" +
414429
" - is: or has: qualifiers (not supported with in-list)\n" +
415-
" - Negated qualifiers (!key:val1 OR !key:val2)\n\n" +
430+
" - Negated qualifiers (!key:val1 OR !key:val2)\n" +
431+
" - Comparison values (age:>24h OR age:>7d)\n\n" +
416432
"Alternatives:\n" +
417433
' - Write in-list syntax directly: --query "key:[val1,val2]"\n' +
418434
" - Run separate queries for each term\n\n" +

test/lib/search-query.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,19 @@ describe("sanitizeQuery: OR → throws", () => {
224224
);
225225
});
226226

227+
test("throws for comparison operator values (not valid in in-list)", () => {
228+
expect(() => sanitizeQuery("age:>24h OR age:>7d")).toThrow(ValidationError);
229+
expect(() => sanitizeQuery("times_seen:>100 OR times_seen:>200")).toThrow(
230+
ValidationError
231+
);
232+
expect(() =>
233+
sanitizeQuery("span.duration:>=1s OR span.duration:>=500ms")
234+
).toThrow(ValidationError);
235+
expect(() =>
236+
sanitizeQuery("firstSeen:<=2024-01-01 OR firstSeen:<=2024-06-01")
237+
).toThrow(ValidationError);
238+
});
239+
227240
test("throws for mixed free-text and qualifier OR", () => {
228241
expect(() => sanitizeQuery("is:unresolved error OR timeout")).toThrow(
229242
ValidationError

0 commit comments

Comments
 (0)