Custom queries#1006
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds an AI-assisted multi-tab query editor with generate/run endpoints, server-side SQL validation, OpenRouter integration (with robust error/metadata handling), result rendering (virtualized sortable table), localization updates, tests, docker env entries, and a consolidated security report. ChangesQuery Editor and Execution Feature
Sequence DiagramsequenceDiagram
participant User
participant QueryPage
participant QueryPromptForm
participant useGenerateCustomQuery
participant generateCustomQuery_API
participant Server_generateCustomQuery
participant OpenRouter
participant QueryEditor
participant useRunCustomQuery
participant runCustomQuery_API
participant Server_runCustomQuery
participant ClickHouse
participant ResultsPanel
User->>QueryPage: load page
User->>QueryPromptForm: submit prompt
QueryPage->>useGenerateCustomQuery: mutate(prompt, signal)
useGenerateCustomQuery->>generateCustomQuery_API: POST /generate
generateCustomQuery_API->>Server_generateCustomQuery: forward
Server_generateCustomQuery->>OpenRouter: callOpenRouterWithMetadata(...)
OpenRouter-->>Server_generateCustomQuery: {content}
Server_generateCustomQuery-->>generateCustomQuery_API: {query}
generateCustomQuery_API-->>QueryPage: {query}
User->>QueryEditor: click Run
QueryEditor->>useRunCustomQuery: mutate({query})
useRunCustomQuery->>runCustomQuery_API: POST /query
runCustomQuery_API->>Server_runCustomQuery: forward
Server_runCustomQuery->>ClickHouse: execute SELECT with PREWHERE
ClickHouse-->>Server_runCustomQuery: rows JSON
Server_runCustomQuery-->>runCustomQuery_API: {data, meta}
runCustomQuery_API-->>QueryPage: results
QueryPage->>ResultsPanel: render rows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
client/src/app/[site]/query/page.tsx (1)
3-3: ⚡ Quick winReconcile
next-intli18n hook usage (useExtractedvsuseTranslations)
client/src/app/[site]/query/page.tsxusesuseExtracted()and calls it for user-facing strings (e.g.,t("Failed to generate query")). The repo’sclient/src/appcodebase also consistently usesuseExtracted(nouseTranslationsimports found there), whileuseExtractedis documented by next-intl as experimental. Align the coding guideline with the project’s intended hook choice, or migrate this page (and potentially others) touseTranslations()if that standard is required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/app/`[site]/query/page.tsx at line 3, The page imports and uses the experimental next-intl hook useExtracted and then calls it for user-visible strings (e.g., t("Failed to generate query")); update this file to match the project's chosen i18n hook: either replace useExtracted with the stable useTranslations throughout page.tsx (and call the returned t function) or, if the project intentionally standardizes on useExtracted, update project guidelines and ensure all usages are consistent; locate the import statement for useExtracted and the calls to t(...) in page.tsx and change the import to useTranslations (and adjust any hook call signature if needed) or switch other files to useExtracted to maintain consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/messages/cs.json`:
- Around line 482-490: The Czech locale file contains new English strings that
need translation; update the entries with keys "k1sXXZ", "SyINSq", "2Y2tgR",
"Pc+tM3", "yaMHMB", "u3e151", "0eYAOP", "uDXLhQ", and "ICdFJO" to their proper
Czech translations so the query flow shows localized text; locate these keys in
client/messages/cs.json and replace the English values with accurate Czech
phrases (preserve placeholders like {count}) and ensure
punctuation/capitalization matches existing Czech locale style.
In `@client/messages/it.json`:
- Around line 482-490: The listed message keys ("k1sXXZ", "SyINSq", "2Y2tgR",
"Pc+tM3", "yaMHMB", "u3e151", "0eYAOP", "uDXLhQ", "ICdFJO") in
client/messages/it.json are still in English; replace each value with the proper
Italian translation (e.g., "Failed to generate query" → "Impossibile generare la
query", "Failed to run query" → "Imeccecorre l'esecuzione della query" or
correct Italian, "Top pages this week" → "Pagine principali di questa
settimana", "Generate" → "Genera", "Results" → "Risultati", "{count} rows" →
"{count} righe", "Not run" → "Non eseguito", "No rows returned" → "Nessuna riga
restituita", "Run a query" → "Esegui una query"); update the values for the
specified keys so the Italian locale contains fully translated strings and save
the file.
In `@client/messages/ja.json`:
- Around line 482-490: The Japanese locale file contains English strings for the
query UI keys k1sXXZ, SyINSq, 2Y2tgR, Pc+tM3, yaMHMB, u3e151, 0eYAOP, uDXLhQ,
and ICdFJO; update ja.json by replacing each value with the appropriate Japanese
translation (e.g., "Failed to generate query" → "クエリの生成に失敗しました", "Failed to run
query" → "クエリの実行に失敗しました", "Top pages this week" → "今週の上位ページ", "Generate" → "生成",
"Results" → "結果", "{count} rows" → "{count} 行", "Not run" → "未実行", "No rows
returned" → "結果が返されませんでした", "Run a query" → "クエリを実行"); keep the keys unchanged
and ensure JSON syntax remains valid.
In `@client/messages/ko.json`:
- Around line 482-490: Replace the English values in ko.json for the query UI
keys with Korean translations: set "k1sXXZ" to "쿼리 생성에 실패했습니다", "SyINSq" to "쿼리
실행에 실패했습니다", "2Y2tgR" to "이번 주 인기 페이지", "Pc+tM3" to "생성", "yaMHMB" to "결과",
"u3e151" to "{count}개 행", "0eYAOP" to "실행되지 않음", "uDXLhQ" to "반환된 행이 없습니다", and
"ICdFJO" to "쿼리 실행" so the Korean locale no longer mixes English in the query
feature.
In `@client/messages/pl.json`:
- Around line 482-490: The Polish locale file contains untranslated English
strings for query-related keys; update the values for keys "k1sXXZ", "SyINSq",
"2Y2tgR", "Pc+tM3", "yaMHMB", "u3e151", "0eYAOP", "uDXLhQ", and "ICdFJO" in
client/messages/pl.json with accurate Polish translations (e.g., "Nie udało się
wygenerować zapytania", "Błąd wykonania zapytania", "Najpopularniejsze strony w
tym tygodniu", "Generuj", "Wyniki", "{count} wierszy", "Nie uruchomiono", "Brak
zwróconych wierszy", "Uruchom zapytanie" or equivalents) so the file is fully
localized; keep the keys unchanged and only replace the string values.
In `@client/messages/pt.json`:
- Around line 482-490: The Portuguese locale file contains untranslated entries
for the query UI; update the values for keys "k1sXXZ", "SyINSq", "2Y2tgR",
"Pc+tM3", "yaMHMB", "u3e151", "0eYAOP", "uDXLhQ", and "ICdFJO" in
client/messages/pt.json by replacing the English strings with their Portuguese
equivalents (e.g., "Failed to generate query" → "Falha ao gerar consulta",
"Failed to run query" → "Falha ao executar consulta", "Top pages this week" →
"Páginas principais desta semana", "Generate" → "Gerar", "Results" →
"Resultados", "{count} rows" → "{count} linhas", "Not run" → "Não executado",
"No rows returned" → "Nenhuma linha retornada", "Run a query" → "Execute uma
consulta") so the query feature is fully localized.
In `@client/messages/uk.json`:
- Line 75: The Ukrainian locale file has an empty translation for key "qj71j1"
in client/messages/uk.json which leaves the query navigation label blank; update
the value for "qj71j1" with the appropriate Ukrainian translation (matching the
intent of the Chinese "查询"), e.g., a short term like "Запит" or another approved
translation used across locales, ensuring the key remains present and consistent
with other messages/*.json files.
- Around line 482-490: The listed locale entries (keys k1sXXZ, SyINSq, 2Y2tgR,
Pc+tM3, yaMHMB, u3e151, 0eYAOP, uDXLhQ, ICdFJO) are still in English; replace
each English value with the correct Ukrainian translation while preserving keys
and any placeholders (e.g., "{count}") and punctuation, and ensure the same
translated strings are applied consistently across other locale files for these
keys; update the values for k1sXXZ (Failed to generate query), SyINSq (Failed to
run query), 2Y2tgR (Top pages this week), Pc+tM3 (Generate), yaMHMB (Results),
u3e151 ("{count} rows"), 0eYAOP (Not run), uDXLhQ (No rows returned), and ICdFJO
(Run a query) with their Ukrainian equivalents.
In `@client/messages/zh.json`:
- Around line 482-490: The keys k1sXXZ, SyINSq, 2Y2tgR, Pc+tM3, yaMHMB, u3e151,
0eYAOP, uDXLhQ, and ICdFJO in client/messages/zh.json are still in English;
replace each value with the proper Chinese translation for the query feature so
the Chinese locale shows localized UI text. Open client/messages/zh.json, locate
those keys (e.g., "k1sXXZ": "Failed to generate query") and update the string
values to their Chinese equivalents, ensuring phrasing matches other locale
files and keeping the same keys unchanged for consistency across
messages/*.json.
In `@client/src/api/analytics/endpoints/customQuery.ts`:
- Around line 31-38: Rename the endpoint functions to the approved API prefixes
and update their exports: change runCustomQuery to a create-style name (e.g.,
createCustomQueryExecution) and change generateCustomQuery to
createCustomQueryGeneration (or another approved 'fetch/create/update/delete'
form), keeping their parameters and return types intact; update the exported
identifiers in this module to use the new names and update any internal
re-exports/usages that import runCustomQuery or generateCustomQuery so they
reference createCustomQueryExecution and createCustomQueryGeneration instead
(ensure authedFetch call and signature, including optional signal on
generateCustomQuery, remain unchanged).
In `@client/src/app/`[site]/query/components/QueryEditor.tsx:
- Around line 63-64: The hardcoded UI strings in QueryEditor.tsx (e.g., the
button title/aria-label "Format query" and the unit labels "lines" and "chars"
around the editor) must be replaced with next-intl translations: import and call
useTranslations() at the top of the QueryEditor component, replace literal
strings with t('queryEditor.formatQuery'), t('queryEditor.lines') and
t('queryEditor.chars') (or similar keys), and add those keys to messages/en.json
(and other locale files). Also update the other occurrences mentioned (around
lines 141-142) to use the same t(...) keys so all UI text in this component is
localized.
In `@client/src/app/`[site]/query/components/QueryTabs.tsx:
- Around line 34-63: The tab item currently renders a <button> containing an
inner span with role="button" which creates invalid nested interactive controls
and prevents keyboard access to the close control; change the outer <button> to
a non-button container (e.g., <div> or <li>) with role="tab", tabIndex={0}, and
an onKeyDown handler that calls onSelectTab(tab.id) when Enter/Space is pressed
and set aria-selected based on isActive, then replace the inner span
(role="button") with a real <button type="button"> that calls
event.stopPropagation() and onCloseTab(tab.id), keeps aria-label="Close query
tab", and remains focusable; keep references to onSelectTab, onCloseTab, tab.id,
isActive, tabs.length and the existing aria labels/visual classes when making
these edits.
In `@server/src/api/analytics/utils/customQueryValidation.ts`:
- Around line 193-211: The current validation misses comma-separated table
references (e.g., "FROM scoped_events, other_table") because
tableReferencePattern only matches FROM/JOIN keywords and the blockedTableNames
check is limited; update validation to reject any table identifier in a FROM
clause that is not "scoped_events" or a known CTE. Specifically, enhance the
logic around compactQuery: either extend tableReferencePattern to also capture
comma-separated identifiers (e.g., treat commas after a FROM/JOIN as additional
table tokens) or add a second pass that extracts the FROM clause(s) by regex and
splits on commas, then for each resulting identifier check against scoped_events
and cteNames; use the existing symbols blockedTableNames, compactQuery,
tableReferencePattern, and cteNames to locate where to add this extra validation
and return the same "Queries can only read from scoped_events" message on
violation.
---
Nitpick comments:
In `@client/src/app/`[site]/query/page.tsx:
- Line 3: The page imports and uses the experimental next-intl hook useExtracted
and then calls it for user-visible strings (e.g., t("Failed to generate
query")); update this file to match the project's chosen i18n hook: either
replace useExtracted with the stable useTranslations throughout page.tsx (and
call the returned t function) or, if the project intentionally standardizes on
useExtracted, update project guidelines and ensure all usages are consistent;
locate the import statement for useExtracted and the calls to t(...) in page.tsx
and change the import to useTranslations (and adjust any hook call signature if
needed) or switch other files to useExtracted to maintain consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e0127a7-8a93-4c16-8b1b-fe08166cc290
📒 Files selected for processing (36)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/uk.jsonclient/messages/zh.jsonclient/src/api/analytics/endpoints/customQuery.tsclient/src/api/analytics/endpoints/index.tsclient/src/api/analytics/hooks/useCustomQuery.tsclient/src/app/[site]/[privateKey]/query/page.tsxclient/src/app/[site]/components/Sidebar/Sidebar.tsxclient/src/app/[site]/journeys/components/SankeyDiagram.tsxclient/src/app/[site]/journeys/page.tsxclient/src/app/[site]/layout.tsxclient/src/app/[site]/query/components/QueryEditor.tsxclient/src/app/[site]/query/components/QueryPromptForm.tsxclient/src/app/[site]/query/components/QueryTabs.tsxclient/src/app/[site]/query/components/ResultsPanel.tsxclient/src/app/[site]/query/components/ResultsTable.tsxclient/src/app/[site]/query/page.tsxclient/src/app/[site]/query/types.tsclient/src/app/[site]/query/utils.tsserver/src/api/analytics/generateCustomQuery.tsserver/src/api/analytics/index.tsserver/src/api/analytics/runCustomQuery.test.tsserver/src/api/analytics/runCustomQuery.tsserver/src/api/analytics/utils/customQueryValidation.tsserver/src/index.tsserver/src/lib/openrouter.test.tsserver/src/lib/openrouter.ts
Comment on lines
+482
to
+490
| "k1sXXZ": "Failed to generate query", | ||
| "SyINSq": "Failed to run query", | ||
| "2Y2tgR": "Top pages this week", | ||
| "Pc+tM3": "Generate", | ||
| "yaMHMB": "Results", | ||
| "u3e151": "{count} rows", | ||
| "0eYAOP": "Not run", | ||
| "uDXLhQ": "No rows returned", | ||
| "ICdFJO": "Run a query", |
Contributor
There was a problem hiding this comment.
Translate newly added query labels into Japanese
Lines 482-490 use English text in ja.json, so Japanese users will see partial English in this feature. Please add Japanese translations for these entries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/messages/ja.json` around lines 482 - 490, The Japanese locale file
contains English strings for the query UI keys k1sXXZ, SyINSq, 2Y2tgR, Pc+tM3,
yaMHMB, u3e151, 0eYAOP, uDXLhQ, and ICdFJO; update ja.json by replacing each
value with the appropriate Japanese translation (e.g., "Failed to generate
query" → "クエリの生成に失敗しました", "Failed to run query" → "クエリの実行に失敗しました", "Top pages
this week" → "今週の上位ページ", "Generate" → "生成", "Results" → "結果", "{count} rows" →
"{count} 行", "Not run" → "未実行", "No rows returned" → "結果が返されませんでした", "Run a
query" → "クエリを実行"); keep the keys unchanged and ensure JSON syntax remains
valid.
Comment on lines
+482
to
+490
| "k1sXXZ": "Failed to generate query", | ||
| "SyINSq": "Failed to run query", | ||
| "2Y2tgR": "Top pages this week", | ||
| "Pc+tM3": "Generate", | ||
| "yaMHMB": "Results", | ||
| "u3e151": "{count} rows", | ||
| "0eYAOP": "Not run", | ||
| "uDXLhQ": "No rows returned", | ||
| "ICdFJO": "Run a query", |
Contributor
There was a problem hiding this comment.
Replace English placeholders with Korean translations
Lines 482-490 are currently English in ko.json. This introduces mixed-language UI for Korean users in the query feature. Please translate these strings to Korean.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/messages/ko.json` around lines 482 - 490, Replace the English values
in ko.json for the query UI keys with Korean translations: set "k1sXXZ" to "쿼리
생성에 실패했습니다", "SyINSq" to "쿼리 실행에 실패했습니다", "2Y2tgR" to "이번 주 인기 페이지", "Pc+tM3" to
"생성", "yaMHMB" to "결과", "u3e151" to "{count}개 행", "0eYAOP" to "실행되지 않음",
"uDXLhQ" to "반환된 행이 없습니다", and "ICdFJO" to "쿼리 실행" so the Korean locale no
longer mixes English in the query feature.
Comment on lines
+482
to
+490
| "k1sXXZ": "Failed to generate query", | ||
| "SyINSq": "Failed to run query", | ||
| "2Y2tgR": "Top pages this week", | ||
| "Pc+tM3": "Generate", | ||
| "yaMHMB": "Results", | ||
| "u3e151": "{count} rows", | ||
| "0eYAOP": "Not run", | ||
| "uDXLhQ": "No rows returned", | ||
| "ICdFJO": "Run a query", |
Contributor
There was a problem hiding this comment.
Localize query-related strings in Polish
Lines 482-490 are in English inside pl.json, which creates a mixed-language experience in the Polish locale. Please translate these values to Polish.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/messages/pl.json` around lines 482 - 490, The Polish locale file
contains untranslated English strings for query-related keys; update the values
for keys "k1sXXZ", "SyINSq", "2Y2tgR", "Pc+tM3", "yaMHMB", "u3e151", "0eYAOP",
"uDXLhQ", and "ICdFJO" in client/messages/pl.json with accurate Polish
translations (e.g., "Nie udało się wygenerować zapytania", "Błąd wykonania
zapytania", "Najpopularniejsze strony w tym tygodniu", "Generuj", "Wyniki",
"{count} wierszy", "Nie uruchomiono", "Brak zwróconych wierszy", "Uruchom
zapytanie" or equivalents) so the file is fully localized; keep the keys
unchanged and only replace the string values.
Comment on lines
+482
to
+490
| "k1sXXZ": "Failed to generate query", | ||
| "SyINSq": "Failed to run query", | ||
| "2Y2tgR": "Top pages this week", | ||
| "Pc+tM3": "Generate", | ||
| "yaMHMB": "Results", | ||
| "u3e151": "{count} rows", | ||
| "0eYAOP": "Not run", | ||
| "uDXLhQ": "No rows returned", | ||
| "ICdFJO": "Run a query", |
Contributor
There was a problem hiding this comment.
Untranslated strings for query feature.
The new query-related keys (k1sXXZ through ICdFJO) contain English values instead of Chinese translations. This breaks internationalization for Chinese users, who will see English UI text in the query interface.
All user-facing strings must be translated to the target locale.
🌐 Suggested Chinese translations
- "k1sXXZ": "Failed to generate query",
- "SyINSq": "Failed to run query",
- "2Y2tgR": "Top pages this week",
- "Pc+tM3": "Generate",
- "yaMHMB": "Results",
- "u3e151": "{count} rows",
- "0eYAOP": "Not run",
- "uDXLhQ": "No rows returned",
- "ICdFJO": "Run a query",
+ "k1sXXZ": "生成查询失败",
+ "SyINSq": "执行查询失败",
+ "2Y2tgR": "本周热门页面",
+ "Pc+tM3": "生成",
+ "yaMHMB": "结果",
+ "u3e151": "{count} 行",
+ "0eYAOP": "未运行",
+ "uDXLhQ": "未返回任何行",
+ "ICdFJO": "运行查询",As per coding guidelines, keep locale keys consistent across messages/*.json.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "k1sXXZ": "Failed to generate query", | |
| "SyINSq": "Failed to run query", | |
| "2Y2tgR": "Top pages this week", | |
| "Pc+tM3": "Generate", | |
| "yaMHMB": "Results", | |
| "u3e151": "{count} rows", | |
| "0eYAOP": "Not run", | |
| "uDXLhQ": "No rows returned", | |
| "ICdFJO": "Run a query", | |
| "k1sXXZ": "生成查询失败", | |
| "SyINSq": "执行查询失败", | |
| "2Y2tgR": "本周热门页面", | |
| "Pc+tM3": "生成", | |
| "yaMHMB": "结果", | |
| "u3e151": "{count} 行", | |
| "0eYAOP": "未运行", | |
| "uDXLhQ": "未返回任何行", | |
| "ICdFJO": "运行查询", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/messages/zh.json` around lines 482 - 490, The keys k1sXXZ, SyINSq,
2Y2tgR, Pc+tM3, yaMHMB, u3e151, 0eYAOP, uDXLhQ, and ICdFJO in
client/messages/zh.json are still in English; replace each value with the proper
Chinese translation for the query feature so the Chinese locale shows localized
UI text. Open client/messages/zh.json, locate those keys (e.g., "k1sXXZ":
"Failed to generate query") and update the string values to their Chinese
equivalents, ensuring phrasing matches other locale files and keeping the same
keys unchanged for consistency across messages/*.json.
Comment on lines
+31
to
+38
| export function runCustomQuery(organizationId: string, query: string) { | ||
| return authedFetch<RunCustomQueryResponse>(`/organizations/${organizationId}/analytics/query`, undefined, { | ||
| method: "POST", | ||
| data: { query }, | ||
| }); | ||
| } | ||
|
|
||
| export function generateCustomQuery(organizationId: string, data: GenerateCustomQueryRequest, signal?: AbortSignal) { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Rename endpoint functions to approved API prefixes.
These endpoint function names don’t follow the repository API naming convention (run*/generate* vs required fetch*/create*/update*/delete*). Please rename and re-export consistently (for example, createCustomQueryExecution, createCustomQueryGeneration or equivalent approved prefix forms).
As per coding guidelines, “API hook functions should be prefixed with 'use' (e.g., useGetSites); endpoint functions should be prefixed with 'fetch', 'create', 'update', or 'delete'”.
Also applies to: 38-48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/api/analytics/endpoints/customQuery.ts` around lines 31 - 38,
Rename the endpoint functions to the approved API prefixes and update their
exports: change runCustomQuery to a create-style name (e.g.,
createCustomQueryExecution) and change generateCustomQuery to
createCustomQueryGeneration (or another approved 'fetch/create/update/delete'
form), keeping their parameters and return types intact; update the exported
identifiers in this module to use the new names and update any internal
re-exports/usages that import runCustomQuery or generateCustomQuery so they
reference createCustomQueryExecution and createCustomQueryGeneration instead
(ensure authedFetch call and signature, including optional signal on
generateCustomQuery, remain unchanged).
Comment on lines
+63
to
+64
| title="Format query" | ||
| aria-label="Format query" |
Contributor
There was a problem hiding this comment.
Localize remaining hardcoded UI strings
These labels are still hardcoded in English ("Format query", "lines", "chars"), so they won’t be translated with the rest of the page.
💡 Suggested patch
<Button
type="button"
size="smIcon"
variant="ghost"
onClick={onFormat}
disabled={disabled || !value.trim()}
- title="Format query"
- aria-label="Format query"
+ title={t("Format query")}
+ aria-label={t("Format query")}
>
<AlignLeft className="h-4 w-4" />
</Button>
@@
<div className="flex h-7 items-center justify-between border-t border-neutral-150 bg-neutral-50 px-3 text-[11px] text-neutral-500 dark:border-neutral-800 dark:bg-neutral-950 dark:text-neutral-500">
- <span>{lineCount} lines</span>
- <span>{value.length.toLocaleString()} chars</span>
+ <span>{t("{count} lines", { count: lineCount.toLocaleString() })}</span>
+ <span>{t("{count} chars", { count: value.length.toLocaleString() })}</span>
</div>As per coding guidelines, "Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)".
Also applies to: 141-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/query/components/QueryEditor.tsx around lines 63 - 64,
The hardcoded UI strings in QueryEditor.tsx (e.g., the button title/aria-label
"Format query" and the unit labels "lines" and "chars" around the editor) must
be replaced with next-intl translations: import and call useTranslations() at
the top of the QueryEditor component, replace literal strings with
t('queryEditor.formatQuery'), t('queryEditor.lines') and t('queryEditor.chars')
(or similar keys), and add those keys to messages/en.json (and other locale
files). Also update the other occurrences mentioned (around lines 141-142) to
use the same t(...) keys so all UI text in this component is localized.
Comment on lines
+34
to
+63
| <button | ||
| key={tab.id} | ||
| type="button" | ||
| onClick={() => onSelectTab(tab.id)} | ||
| className={cn( | ||
| "group flex h-8 min-w-[120px] items-center justify-between gap-2 rounded-md border px-2 text-left text-xs transition-colors", | ||
| isActive | ||
| ? "border-neutral-200 bg-white text-neutral-900 shadow-sm dark:border-neutral-750 dark:bg-neutral-900 dark:text-neutral-100" | ||
| : "border-transparent text-neutral-600 hover:bg-white/70 hover:text-neutral-900 dark:text-neutral-400 dark:hover:bg-neutral-900/70 dark:hover:text-neutral-100" | ||
| )} | ||
| > | ||
| <span className="flex min-w-0 items-center gap-1.5"> | ||
| {tabIsBusy && <Loader2 className="h-3 w-3 shrink-0 animate-spin text-neutral-400" />} | ||
| {tab.resultError && !tabIsBusy && <AlertCircle className="h-3 w-3 shrink-0 text-red-500" />} | ||
| <span className="truncate">{tab.name || `Query ${index + 1}`}</span> | ||
| </span> | ||
| {tabs.length > 1 && ( | ||
| <span | ||
| role="button" | ||
| tabIndex={-1} | ||
| onClick={event => { | ||
| event.stopPropagation(); | ||
| onCloseTab(tab.id); | ||
| }} | ||
| className="rounded p-0.5 text-neutral-400 opacity-70 hover:bg-neutral-100 hover:text-neutral-700 group-hover:opacity-100 dark:hover:bg-neutral-800 dark:hover:text-neutral-200" | ||
| aria-label="Close query tab" | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </span> | ||
| )} |
Contributor
There was a problem hiding this comment.
Fix tab close control accessibility and invalid interactive nesting
The close action is an unfocusable pseudo-button inside another button. Keyboard users cannot trigger close, and nested interactive controls are invalid markup.
💡 Suggested structural fix
- <button
+ <div
key={tab.id}
- type="button"
- onClick={() => onSelectTab(tab.id)}
className={cn(
"group flex h-8 min-w-[120px] items-center justify-between gap-2 rounded-md border px-2 text-left text-xs transition-colors",
isActive
? "border-neutral-200 bg-white text-neutral-900 shadow-sm dark:border-neutral-750 dark:bg-neutral-900 dark:text-neutral-100"
: "border-transparent text-neutral-600 hover:bg-white/70 hover:text-neutral-900 dark:text-neutral-400 dark:hover:bg-neutral-900/70 dark:hover:text-neutral-100"
)}
>
- <span className="flex min-w-0 items-center gap-1.5">
+ <button type="button" onClick={() => onSelectTab(tab.id)} className="flex min-w-0 flex-1 items-center gap-1.5 text-left">
{tabIsBusy && <Loader2 className="h-3 w-3 shrink-0 animate-spin text-neutral-400" />}
{tab.resultError && !tabIsBusy && <AlertCircle className="h-3 w-3 shrink-0 text-red-500" />}
<span className="truncate">{tab.name || `Query ${index + 1}`}</span>
- </span>
+ </button>
{tabs.length > 1 && (
- <span
- role="button"
- tabIndex={-1}
+ <button
+ type="button"
onClick={event => {
event.stopPropagation();
onCloseTab(tab.id);
}}
className="rounded p-0.5 text-neutral-400 opacity-70 hover:bg-neutral-100 hover:text-neutral-700 group-hover:opacity-100 dark:hover:bg-neutral-800 dark:hover:text-neutral-200"
aria-label="Close query tab"
>
<X className="h-3 w-3" />
- </span>
+ </button>
)}
- </button>
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| key={tab.id} | |
| type="button" | |
| onClick={() => onSelectTab(tab.id)} | |
| className={cn( | |
| "group flex h-8 min-w-[120px] items-center justify-between gap-2 rounded-md border px-2 text-left text-xs transition-colors", | |
| isActive | |
| ? "border-neutral-200 bg-white text-neutral-900 shadow-sm dark:border-neutral-750 dark:bg-neutral-900 dark:text-neutral-100" | |
| : "border-transparent text-neutral-600 hover:bg-white/70 hover:text-neutral-900 dark:text-neutral-400 dark:hover:bg-neutral-900/70 dark:hover:text-neutral-100" | |
| )} | |
| > | |
| <span className="flex min-w-0 items-center gap-1.5"> | |
| {tabIsBusy && <Loader2 className="h-3 w-3 shrink-0 animate-spin text-neutral-400" />} | |
| {tab.resultError && !tabIsBusy && <AlertCircle className="h-3 w-3 shrink-0 text-red-500" />} | |
| <span className="truncate">{tab.name || `Query ${index + 1}`}</span> | |
| </span> | |
| {tabs.length > 1 && ( | |
| <span | |
| role="button" | |
| tabIndex={-1} | |
| onClick={event => { | |
| event.stopPropagation(); | |
| onCloseTab(tab.id); | |
| }} | |
| className="rounded p-0.5 text-neutral-400 opacity-70 hover:bg-neutral-100 hover:text-neutral-700 group-hover:opacity-100 dark:hover:bg-neutral-800 dark:hover:text-neutral-200" | |
| aria-label="Close query tab" | |
| > | |
| <X className="h-3 w-3" /> | |
| </span> | |
| )} | |
| <div | |
| key={tab.id} | |
| className={cn( | |
| "group flex h-8 min-w-[120px] items-center justify-between gap-2 rounded-md border px-2 text-left text-xs transition-colors", | |
| isActive | |
| ? "border-neutral-200 bg-white text-neutral-900 shadow-sm dark:border-neutral-750 dark:bg-neutral-900 dark:text-neutral-100" | |
| : "border-transparent text-neutral-600 hover:bg-white/70 hover:text-neutral-900 dark:text-neutral-400 dark:hover:bg-neutral-900/70 dark:hover:text-neutral-100" | |
| )} | |
| > | |
| <button type="button" onClick={() => onSelectTab(tab.id)} className="flex min-w-0 flex-1 items-center gap-1.5 text-left"> | |
| {tabIsBusy && <Loader2 className="h-3 w-3 shrink-0 animate-spin text-neutral-400" />} | |
| {tab.resultError && !tabIsBusy && <AlertCircle className="h-3 w-3 shrink-0 text-red-500" />} | |
| <span className="truncate">{tab.name || `Query ${index + 1}`}</span> | |
| </button> | |
| {tabs.length > 1 && ( | |
| <button | |
| type="button" | |
| onClick={event => { | |
| event.stopPropagation(); | |
| onCloseTab(tab.id); | |
| }} | |
| className="rounded p-0.5 text-neutral-400 opacity-70 hover:bg-neutral-100 hover:text-neutral-700 group-hover:opacity-100 dark:hover:bg-neutral-800 dark:hover:text-neutral-200" | |
| aria-label="Close query tab" | |
| > | |
| <X className="h-3 w-3" /> | |
| </button> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/query/components/QueryTabs.tsx around lines 34 - 63,
The tab item currently renders a <button> containing an inner span with
role="button" which creates invalid nested interactive controls and prevents
keyboard access to the close control; change the outer <button> to a non-button
container (e.g., <div> or <li>) with role="tab", tabIndex={0}, and an onKeyDown
handler that calls onSelectTab(tab.id) when Enter/Space is pressed and set
aria-selected based on isActive, then replace the inner span (role="button")
with a real <button type="button"> that calls event.stopPropagation() and
onCloseTab(tab.id), keeps aria-label="Close query tab", and remains focusable;
keep references to onSelectTab, onCloseTab, tab.id, isActive, tabs.length and
the existing aria labels/visual classes when making these edits.
Comment on lines
+193
to
+211
| for (const tableName of blockedTableNames) { | ||
| if (new RegExp(`,\\s*${tableName}\\b`, "i").test(compactQuery)) { | ||
| return "Queries can only read from scoped_events"; | ||
| } | ||
| } | ||
|
|
||
| const tableReferencePattern = /\b(?:FROM|JOIN)\s+([a-zA-Z_][a-zA-Z0-9_.]*|\()/gi; | ||
| let match: RegExpExecArray | null; | ||
| while ((match = tableReferencePattern.exec(compactQuery)) !== null) { | ||
| const tableName = match[1]; | ||
| if (tableName === "(") { | ||
| continue; | ||
| } | ||
|
|
||
| const normalizedTableName = tableName.toLowerCase(); | ||
| if (normalizedTableName !== "scoped_events" && !cteNames.has(normalizedTableName)) { | ||
| return "Queries can only read from scoped_events"; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Comma-join bypass for unlisted tables.
The comma-join check at lines 193-197 only blocks tables in blockedTableNames. The FROM/JOIN pattern at 199-211 only captures table references preceded by FROM or JOIN keywords. A query like SELECT * FROM scoped_events, some_other_table would bypass both checks if some_other_table exists but isn't in blockedTableNames.
Consider extending the FROM/JOIN pattern to also capture comma-separated table references, or add a catch-all check for any table reference that isn't scoped_events or a CTE.
🔒 Proposed fix
- const tableReferencePattern = /\b(?:FROM|JOIN)\s+([a-zA-Z_][a-zA-Z0-9_.]*|\()/gi;
+ // Match tables after FROM, JOIN, or comma (within FROM clause context)
+ const tableReferencePattern = /\b(?:FROM|JOIN)\s+([a-zA-Z_][a-zA-Z0-9_.]*|\()|,\s*([a-zA-Z_][a-zA-Z0-9_.]*)\s*(?:AS\s+\w+\s*)?(?=,|\s+(?:ON|WHERE|GROUP|ORDER|LIMIT|UNION|EXCEPT|INTERSECT|$)|\s*$)/gi;
let match: RegExpExecArray | null;
while ((match = tableReferencePattern.exec(compactQuery)) !== null) {
- const tableName = match[1];
+ const tableName = match[1] || match[2];
if (tableName === "(") {
continue;
}Alternatively, adopt a simpler allowlist approach: reject any identifier that looks like a table reference (follows FROM/JOIN or comma in a FROM clause) unless it's exactly scoped_events or a known CTE.
🧰 Tools
🪛 ast-grep (0.42.2)
[warning] 193-193: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(,\\s*${tableName}\\b, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 OpenGrep (1.21.0)
[ERROR] 201-201: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/api/analytics/utils/customQueryValidation.ts` around lines 193 -
211, The current validation misses comma-separated table references (e.g., "FROM
scoped_events, other_table") because tableReferencePattern only matches
FROM/JOIN keywords and the blockedTableNames check is limited; update validation
to reject any table identifier in a FROM clause that is not "scoped_events" or a
known CTE. Specifically, enhance the logic around compactQuery: either extend
tableReferencePattern to also capture comma-separated identifiers (e.g., treat
commas after a FROM/JOIN as additional table tokens) or add a second pass that
extracts the FROM clause(s) by regex and splits on commas, then for each
resulting identifier check against scoped_events and cteNames; use the existing
symbols blockedTableNames, compactQuery, tableReferencePattern, and cteNames to
locate where to add this extra validation and return the same "Queries can only
read from scoped_events" message on violation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
client/messages/it.json (1)
686-694:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate query UI strings to Italian
Lines 686-694 still contain English placeholders. This issue was previously flagged. Please translate to Italian:
k1sXXZ: "Impossibile generare la query"SyINSq: "Impossibile eseguire la query"Vn7Wtu: "Cosa vuoi interrogare?"Pc+tM3: "Genera"yaMHMB: "Risultati"u3e151: "{count} righe"0eYAOP: "Non eseguito"uDXLhQ: "Nessuna riga restituita"ICdFJO: "Esegui una query"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/messages/it.json` around lines 686 - 694, The JSON locale entries for the query UI remain in English; update the values for the keys k1sXXZ, SyINSq, Vn7Wtu, Pc+tM3, yaMHMB, u3e151, 0eYAOP, uDXLhQ, and ICdFJO in client/messages/it.json to the provided Italian translations (k1sXXZ -> "Impossibile generare la query", SyINSq -> "Impossibile eseguire la query", Vn7Wtu -> "Cosa vuoi interrogare?", Pc+tM3 -> "Genera", yaMHMB -> "Risultati", u3e151 -> "{count} righe", 0eYAOP -> "Non eseguito", uDXLhQ -> "Nessuna riga restituita", ICdFJO -> "Esegui una query"), ensuring proper JSON string quoting and escaping so the file remains valid.client/messages/ko.json (1)
686-694:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate query UI strings to Korean
Lines 686-694 still contain English placeholders. This issue was previously flagged. Please translate to Korean:
k1sXXZ: "쿼리 생성에 실패했습니다"SyINSq: "쿼리 실행에 실패했습니다"Vn7Wtu: "무엇을 쿼리하시겠습니까?"Pc+tM3: "생성"yaMHMB: "결과"u3e151: "{count}개 행"0eYAOP: "실행되지 않음"uDXLhQ: "반환된 행이 없습니다"ICdFJO: "쿼리 실행"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/messages/ko.json` around lines 686 - 694, The JSON localization entries for the query UI still contain English; update the values for the keys "k1sXXZ", "SyINSq", "Vn7Wtu", "Pc+tM3", "yaMHMB", "u3e151", "0eYAOP", "uDXLhQ", and "ICdFJO" with the provided Korean translations (k1sXXZ: "쿼리 생성에 실패했습니다", SyINSq: "쿼리 실행에 실패했습니다", Vn7Wtu: "무엇을 쿼리하시겠습니까?", Pc+tM3: "생성", yaMHMB: "결과", u3e151: "{count}개 행", 0eYAOP: "실행되지 않음", uDXLhQ: "반환된 행이 없습니다", ICdFJO: "쿼리 실행") so the UI displays Korean strings instead of English placeholders.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/messages/de.json`:
- Around line 686-694: The German locale file has several English values causing
mixed-language UI; update the translations for the keys k1sXXZ, SyINSq, Vn7Wtu,
Pc+tM3, yaMHMB, u3e151, 0eYAOP, and uDXLhQ in the messages JSON so they match
the provided German strings (k1sXXZ -> "Abfrage konnte nicht generiert werden",
SyINSq -> "Abfrage konnte nicht ausgeführt werden", Vn7Wtu -> "Was möchten Sie
abfragen?", Pc+tM3 -> "Generieren", yaMHMB -> "Ergebnisse", u3e151 -> "{count}
Zeilen", 0eYAOP -> "Nicht ausgeführt", uDXLhQ -> "Keine Zeilen zurückgegeben")
and ensure the key ICdFJO is updated to "Abfrage ausführen" as well, keeping the
same locale keys and JSON formatting consistent with other messages/*.json
files.
In `@SECURITY_SCAN_REPORT.md`:
- Line 58: In SECURITY_SCAN_REPORT.md remove the extra padding inside the inline
code span on the noted line (the inline backtick-delimited fragment around the
escaped quote and SQL example) so it reads without leading/trailing spaces (e.g.
change ` ' \' OR 1=1... ` to `'\' OR 1=1...`) ensuring the inline code contains
no surrounding spaces and passes MD038; also verify nearby mentions like
path.includes("*") and patternToRegex remain unchanged.
- Around line 39-41: Add explicit language identifiers for the fenced code
blocks containing the SQL injection snippet and the journey assignment: change
the fence before "parameter=url_param:x'] FROM events WHERE site_id=1 UNION
SELECT password,1,1 FROM secret--" to ```text (or ```sql if you prefer SQL
highlighting) and change the fence before "journey[1] = '\'' OR 1=1 OR
journey[1]='''" to ```sql; also update the other similar fenced block at the
lines referenced (the block around journey[1] at 55-57) so all code fences
include the appropriate language tag for correct linting and rendering.
---
Duplicate comments:
In `@client/messages/it.json`:
- Around line 686-694: The JSON locale entries for the query UI remain in
English; update the values for the keys k1sXXZ, SyINSq, Vn7Wtu, Pc+tM3, yaMHMB,
u3e151, 0eYAOP, uDXLhQ, and ICdFJO in client/messages/it.json to the provided
Italian translations (k1sXXZ -> "Impossibile generare la query", SyINSq ->
"Impossibile eseguire la query", Vn7Wtu -> "Cosa vuoi interrogare?", Pc+tM3 ->
"Genera", yaMHMB -> "Risultati", u3e151 -> "{count} righe", 0eYAOP -> "Non
eseguito", uDXLhQ -> "Nessuna riga restituita", ICdFJO -> "Esegui una query"),
ensuring proper JSON string quoting and escaping so the file remains valid.
In `@client/messages/ko.json`:
- Around line 686-694: The JSON localization entries for the query UI still
contain English; update the values for the keys "k1sXXZ", "SyINSq", "Vn7Wtu",
"Pc+tM3", "yaMHMB", "u3e151", "0eYAOP", "uDXLhQ", and "ICdFJO" with the provided
Korean translations (k1sXXZ: "쿼리 생성에 실패했습니다", SyINSq: "쿼리 실행에 실패했습니다", Vn7Wtu:
"무엇을 쿼리하시겠습니까?", Pc+tM3: "생성", yaMHMB: "결과", u3e151: "{count}개 행", 0eYAOP: "실행되지
않음", uDXLhQ: "반환된 행이 없습니다", ICdFJO: "쿼리 실행") so the UI displays Korean strings
instead of English placeholders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 966e1e9a-d65b-4027-8e4f-b805c893b27f
📒 Files selected for processing (36)
SECURITY_SCAN_REPORT.mdclient/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/uk.jsonclient/messages/zh.jsonclient/src/api/analytics/endpoints/customQuery.tsclient/src/api/analytics/endpoints/index.tsclient/src/api/analytics/hooks/useCustomQuery.tsclient/src/app/[site]/[privateKey]/query/page.tsxclient/src/app/[site]/components/Sidebar/Sidebar.tsxclient/src/app/[site]/journeys/page.tsxclient/src/app/[site]/layout.tsxclient/src/app/[site]/query/components/QueryEditor.tsxclient/src/app/[site]/query/components/QueryPromptForm.tsxclient/src/app/[site]/query/components/QueryTabs.tsxclient/src/app/[site]/query/components/ResultsPanel.tsxclient/src/app/[site]/query/components/ResultsTable.tsxclient/src/app/[site]/query/page.tsxclient/src/app/[site]/query/types.tsclient/src/app/[site]/query/utils.tsserver/src/api/analytics/generateCustomQuery.tsserver/src/api/analytics/index.tsserver/src/api/analytics/runCustomQuery.test.tsserver/src/api/analytics/runCustomQuery.tsserver/src/api/analytics/utils/customQueryValidation.tsserver/src/index.tsserver/src/lib/openrouter.test.tsserver/src/lib/openrouter.ts
💤 Files with no reviewable changes (8)
- server/src/api/analytics/runCustomQuery.test.ts
- server/src/api/analytics/index.ts
- server/src/lib/openrouter.test.ts
- server/src/index.ts
- server/src/api/analytics/generateCustomQuery.ts
- server/src/api/analytics/runCustomQuery.ts
- server/src/lib/openrouter.ts
- server/src/api/analytics/utils/customQueryValidation.ts
✅ Files skipped from review due to trivial changes (4)
- client/src/app/[site]/layout.tsx
- client/src/app/[site]/[privateKey]/query/page.tsx
- client/src/app/[site]/journeys/page.tsx
- client/messages/uk.json
🚧 Files skipped from review as they are similar to previous changes (18)
- client/src/app/[site]/components/Sidebar/Sidebar.tsx
- client/src/app/[site]/query/components/QueryTabs.tsx
- client/src/app/[site]/query/types.ts
- client/src/api/analytics/endpoints/customQuery.ts
- client/src/app/[site]/query/components/QueryPromptForm.tsx
- client/messages/pt.json
- client/src/app/[site]/query/components/ResultsPanel.tsx
- client/src/app/[site]/query/utils.ts
- client/src/api/analytics/hooks/useCustomQuery.ts
- client/messages/es.json
- client/messages/pl.json
- client/messages/ja.json
- client/src/app/[site]/query/components/ResultsTable.tsx
- client/messages/cs.json
- client/messages/zh.json
- client/messages/fr.json
- client/src/app/[site]/query/page.tsx
- client/src/app/[site]/query/components/QueryEditor.tsx
Comment on lines
+686
to
+694
| "k1sXXZ": "Failed to generate query", | ||
| "SyINSq": "Failed to run query", | ||
| "Vn7Wtu": "", | ||
| "Pc+tM3": "Generate", | ||
| "yaMHMB": "Results", | ||
| "u3e151": "{count} rows", | ||
| "0eYAOP": "Not run", | ||
| "uDXLhQ": "No rows returned", | ||
| "ICdFJO": "Run a query", |
Contributor
There was a problem hiding this comment.
Translate query UI strings to German
Lines 686-694 contain English placeholders that cause mixed-language UI for German users. Please translate these query feature strings to German:
k1sXXZ: "Abfrage konnte nicht generiert werden"SyINSq: "Abfrage konnte nicht ausgeführt werden"Vn7Wtu: "Was möchten Sie abfragen?"Pc+tM3: "Generieren"yaMHMB: "Ergebnisse"u3e151: "{count} Zeilen"0eYAOP: "Nicht ausgeführt"uDXLhQ: "Keine Zeilen zurückgegeben"ICdFJO: "Abfrage ausführen"
As per coding guidelines: Keep locale keys consistent across messages/*.json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/messages/de.json` around lines 686 - 694, The German locale file has
several English values causing mixed-language UI; update the translations for
the keys k1sXXZ, SyINSq, Vn7Wtu, Pc+tM3, yaMHMB, u3e151, 0eYAOP, and uDXLhQ in
the messages JSON so they match the provided German strings (k1sXXZ -> "Abfrage
konnte nicht generiert werden", SyINSq -> "Abfrage konnte nicht ausgeführt
werden", Vn7Wtu -> "Was möchten Sie abfragen?", Pc+tM3 -> "Generieren", yaMHMB
-> "Ergebnisse", u3e151 -> "{count} Zeilen", 0eYAOP -> "Nicht ausgeführt",
uDXLhQ -> "Keine Zeilen zurückgegeben") and ensure the key ICdFJO is updated to
"Abfrage ausführen" as well, keeping the same locale keys and JSON formatting
consistent with other messages/*.json files.
Comment on lines
+39
to
+41
| ``` | ||
| parameter=url_param:x'] FROM events WHERE site_id=1 UNION SELECT password,1,1 FROM secret-- | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Both code fences are missing a language tag, which triggers MD040 and reduces readability in rendered docs.
Suggested patch
-```
+```text
parameter=url_param:x'] FROM events WHERE site_id=1 UNION SELECT password,1,1 FROM secret--- +sql
journey[1] = ''' OR 1=1 OR journey[1]='''
Also applies to: 55-57
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SECURITY_SCAN_REPORT.md` around lines 39 - 41, Add explicit language
identifiers for the fenced code blocks containing the SQL injection snippet and
the journey assignment: change the fence before "parameter=url_param:x'] FROM
events WHERE site_id=1 UNION SELECT password,1,1 FROM secret--" to ```text (or
```sql if you prefer SQL highlighting) and change the fence before "journey[1] =
'\'' OR 1=1 OR journey[1]='''" to ```sql; also update the other similar fenced
block at the lines referenced (the block around journey[1] at 55-57) so all code
fences include the appropriate language tag for correct linting and rendering.
| ``` | ||
| journey[1] = '\'' OR 1=1 OR journey[1]=''' | ||
| ``` | ||
| The `\'` is an escaped quote, the doubled sequence closes the literal early, and the trailing ` OR 1=1 OR ...` executes as raw SQL. (The wildcard branch runs `patternToRegex` first, which doubles backslashes, so it is not affected.) Separately, a non-string JSON value (e.g. `{"0": 5}`) reaching `path.includes("*")` throws a 500 (trivial DoS). |
Contributor
There was a problem hiding this comment.
Remove padding inside inline code span.
Line 58 includes an inline code span with surrounding spaces (MD038). Remove the extra padding so markdown lint passes cleanly.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 58-58: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SECURITY_SCAN_REPORT.md` at line 58, In SECURITY_SCAN_REPORT.md remove the
extra padding inside the inline code span on the noted line (the inline
backtick-delimited fragment around the escaped quote and SQL example) so it
reads without leading/trailing spaces (e.g. change ` ' \' OR 1=1... ` to `'\' OR
1=1...`) ensuring the inline code contains no surrounding spaces and passes
MD038; also verify nearby mentions like path.includes("*") and patternToRegex
remain unchanged.
This commit updates the `generateCustomQuery` and `runCustomQuery` functions to accept an optional `AbortSignal`, allowing for better control over request cancellation. Additionally, the `useGenerateCustomQuery` hook is modified to pass the signal, improving the overall responsiveness of the custom query feature. The `ResultsTable` component is also introduced, providing enhanced sorting capabilities for query results, thereby improving user experience and data management.
… code This commit simplifies the `page.tsx` file by removing unnecessary imports and unused functions related to custom query handling. It retains essential hooks and types while enhancing code clarity and maintainability. The overall structure is optimized for better readability, setting the stage for future enhancements.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
SECURITY_SCAN_REPORT.md (3)
39-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifier to fenced code block.
The code block lacks a language identifier, triggering MD040. This was already flagged in a previous review.
Suggested fix
-``` +```text parameter=url_param:x'] FROM events WHERE site_id=1 UNION SELECT password,1,1 FROM secret--</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@SECURITY_SCAN_REPORT.mdaround lines 39 - 41, The fenced code block
containing the SQL injection sample (the line starting "parameter=url_param:x']
FROM events WHERE site_id=1 UNION SELECT password,1,1 FROM secret--") is missing
a language identifier; update the opening fence fromto a language liketext orsql so the block is annotated (e.g., change the fence totext
or ```sql) to satisfy MD040.</details> --- `55-57`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add language identifier to fenced code block.** The code block lacks a language identifier, triggering MD040. This was already flagged in a previous review. <details> <summary>Suggested fix</summary> ```diff -``` +```sql journey[1] = '\'' OR 1=1 OR journey[1]=''' ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@SECURITY_SCAN_REPORT.mdaround lines 55 - 57, The fenced code block
containing the SQL payload "journey[1] = ''' OR 1=1 OR journey[1]='''" is
missing a language identifier, causing MD040; update the fenced code block in
SECURITY_SCAN_REPORT.md around the snippet so the opening triple-backticks
include the language (use sql) to be ```sql to satisfy the linter and keep the
content unchanged.</details> --- `58-58`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Remove padding inside inline code span.** The inline code span ` OR 1=1 OR ...` contains a leading space, triggering MD038. This was already flagged in a previous review. <details> <summary>Suggested fix</summary> ```diff -The `\'` is an escaped quote, the doubled sequence closes the literal early, and the trailing ` OR 1=1 OR ...` executes as raw SQL. +The `\'` is an escaped quote, the doubled sequence closes the literal early, and the trailing `OR 1=1 OR ...` executes as raw SQL. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SECURITY_SCAN_REPORT.md` at line 58, The inline code span in SECURITY_SCAN_REPORT.md contains a leading space inside backticks (` OR 1=1 OR ...`) which triggers MD038; edit the markdown to remove the padding so the span reads `OR 1=1 OR ...` (no leading/trailing spaces inside the backticks). Also scan nearby text for other inline code spans like `path.includes("*")` or similar and ensure no extra spaces inside those backtick-delimited code spans. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@docker-compose.yml:
- Around line 94-95: Replace the permissive substitutions for OPENROUTER_API_KEY
and OPENROUTER_MODEL so container startup fails immediately when they are
missing; specifically, update the environment entries that currently use
${OPENROUTER_API_KEY} and ${OPENROUTER_MODEL} to required substitutions that
include an error message (e.g., ${OPENROUTER_API_KEY:?OPENROUTER_API_KEY is
required} and ${OPENROUTER_MODEL:?OPENROUTER_MODEL is required}), ensuring the
variables OPENROUTER_API_KEY and OPENROUTER_MODEL are validated at compose
start.In
@server/src/api/analytics/utils/customQueryValidation.ts:
- Around line 199-215: The current validation only checks for the token
/\bscoped_events\b/ which can match aliases; update the logic that iterates
tableReferencePattern (and uses compactQuery, match, tableName,
normalizedTableName, and cteNames) to require that at least one actual FROM/JOIN
table reference resolves to scoped_events (case-insensitive) — i.e., when
iterating matches, treat parentheses as CTE/subquery skips and mark a flag
foundScopedEvents = true if normalizedTableName === "scoped_events"; after the
loop, if foundScopedEvents is false return "Query must read from scoped_events"
and remove or replace the final token-only regex check so only real table
references pass.
Duplicate comments:
In@SECURITY_SCAN_REPORT.md:
- Around line 39-41: The fenced code block containing the SQL injection sample
(the line starting "parameter=url_param:x'] FROM events WHERE site_id=1 UNION
SELECT password,1,1 FROM secret--") is missing a language identifier; update the
opening fence fromto a language liketext orsql so the block is annotated (e.g., change the fence totext or ```sql) to satisfy MD040.- Around line 55-57: The fenced code block containing the SQL payload
"journey[1] = ''' OR 1=1 OR journey[1]='''" is missing a language identifier,
causing MD040; update the fenced code block in SECURITY_SCAN_REPORT.md around
the snippet so the opening triple-backticks include the language (use sql) to be- Line 58: The inline code span in SECURITY_SCAN_REPORT.md contains a leading space inside backticks (` OR 1=1 OR ...`) which triggers MD038; edit the markdown to remove the padding so the span reads `OR 1=1 OR ...` (no leading/trailing spaces inside the backticks). Also scan nearby text for other inline code spans like `path.includes("*")` or similar and ensure no extra spaces inside those backtick-delimited code spans.🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID:
67a293ba-7cc3-45d7-8142-6a9df7394582📒 Files selected for processing (37)
SECURITY_SCAN_REPORT.mdclient/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/uk.jsonclient/messages/zh.jsonclient/src/api/analytics/endpoints/customQuery.tsclient/src/api/analytics/endpoints/index.tsclient/src/api/analytics/hooks/useCustomQuery.tsclient/src/app/[site]/[privateKey]/query/page.tsxclient/src/app/[site]/components/Sidebar/Sidebar.tsxclient/src/app/[site]/journeys/page.tsxclient/src/app/[site]/layout.tsxclient/src/app/[site]/query/components/QueryEditor.tsxclient/src/app/[site]/query/components/QueryPromptForm.tsxclient/src/app/[site]/query/components/QueryTabs.tsxclient/src/app/[site]/query/components/ResultsPanel.tsxclient/src/app/[site]/query/components/ResultsTable.tsxclient/src/app/[site]/query/page.tsxclient/src/app/[site]/query/types.tsclient/src/app/[site]/query/utils.tsdocker-compose.ymlserver/src/api/analytics/generateCustomQuery.tsserver/src/api/analytics/index.tsserver/src/api/analytics/runCustomQuery.test.tsserver/src/api/analytics/runCustomQuery.tsserver/src/api/analytics/utils/customQueryValidation.tsserver/src/index.tsserver/src/lib/openrouter.test.tsserver/src/lib/openrouter.ts✅ Files skipped from review due to trivial changes (5)
- client/src/app/[site]/layout.tsx
- client/src/app/[site]/journeys/page.tsx
- client/messages/cs.json
- client/messages/es.json
- client/messages/de.json
🚧 Files skipped from review as they are similar to previous changes (25)
- server/src/api/analytics/runCustomQuery.test.ts
- server/src/api/analytics/index.ts
- client/src/app/[site]/query/components/ResultsPanel.tsx
- client/src/app/[site]/components/Sidebar/Sidebar.tsx
- server/src/lib/openrouter.test.ts
- client/src/api/analytics/endpoints/index.ts
- client/src/app/[site]/query/types.ts
- client/src/api/analytics/hooks/useCustomQuery.ts
- client/messages/ko.json
- client/messages/pl.json
- client/src/app/[site]/query/components/QueryPromptForm.tsx
- client/messages/uk.json
- client/src/api/analytics/endpoints/customQuery.ts
- client/src/app/[site]/query/components/QueryEditor.tsx
- server/src/index.ts
- client/messages/fr.json
- client/messages/en.json
- client/src/app/[site]/query/components/QueryTabs.tsx
- client/messages/it.json
- client/src/app/[site]/query/utils.ts
- client/src/app/[site]/query/page.tsx
- server/src/api/analytics/runCustomQuery.ts
- server/src/lib/openrouter.ts
- client/src/app/[site]/query/components/ResultsTable.tsx
- server/src/api/analytics/generateCustomQuery.ts
Comment on lines
+94
to
+95
| - OPENROUTER_API_KEY=${OPENROUTER_API_KEY} | ||
| - OPENROUTER_MODEL=${OPENROUTER_MODEL} |
Contributor
There was a problem hiding this comment.
Fail fast when OpenRouter env vars are missing.
Using ${OPENROUTER_API_KEY} / ${OPENROUTER_MODEL} allows empty values, which can defer failure to runtime. Make them required so container startup fails immediately when misconfigured.
Suggested change
- - OPENROUTER_API_KEY=${OPENROUTER_API_KEY}
- - OPENROUTER_MODEL=${OPENROUTER_MODEL}
+ - OPENROUTER_API_KEY=${OPENROUTER_API_KEY:?OPENROUTER_API_KEY is required}
+ - OPENROUTER_MODEL=${OPENROUTER_MODEL:?OPENROUTER_MODEL is required}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - OPENROUTER_API_KEY=${OPENROUTER_API_KEY} | |
| - OPENROUTER_MODEL=${OPENROUTER_MODEL} | |
| - OPENROUTER_API_KEY=${OPENROUTER_API_KEY:?OPENROUTER_API_KEY is required} | |
| - OPENROUTER_MODEL=${OPENROUTER_MODEL:?OPENROUTER_MODEL is required} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yml` around lines 94 - 95, Replace the permissive
substitutions for OPENROUTER_API_KEY and OPENROUTER_MODEL so container startup
fails immediately when they are missing; specifically, update the environment
entries that currently use ${OPENROUTER_API_KEY} and ${OPENROUTER_MODEL} to
required substitutions that include an error message (e.g.,
${OPENROUTER_API_KEY:?OPENROUTER_API_KEY is required} and
${OPENROUTER_MODEL:?OPENROUTER_MODEL is required}), ensuring the variables
OPENROUTER_API_KEY and OPENROUTER_MODEL are validated at compose start.
Comment on lines
+199
to
+215
| const tableReferencePattern = /\b(?:FROM|JOIN)\s+([a-zA-Z_][a-zA-Z0-9_.]*|\()/gi; | ||
| let match: RegExpExecArray | null; | ||
| while ((match = tableReferencePattern.exec(compactQuery)) !== null) { | ||
| const tableName = match[1]; | ||
| if (tableName === "(") { | ||
| continue; | ||
| } | ||
|
|
||
| const normalizedTableName = tableName.toLowerCase(); | ||
| if (normalizedTableName !== "scoped_events" && !cteNames.has(normalizedTableName)) { | ||
| return "Queries can only read from scoped_events"; | ||
| } | ||
| } | ||
|
|
||
| if (!/\bscoped_events\b/i.test(compactQuery)) { | ||
| return "Query must read from scoped_events"; | ||
| } |
Contributor
There was a problem hiding this comment.
Require an actual FROM/JOIN scoped_events reference, not token presence.
/\bscoped_events\b/ can be satisfied by aliases/identifiers without reading the table, so queries can pass policy checks without actually sourcing from scoped_events.
🔧 Proposed fix
export function validateScopedQuery(query: string): string | null {
const normalizedQuery = normalizeCustomQuery(query);
const queryWithoutLiterals = stripSqlLiteralsAndComments(normalizedQuery);
const compactQuery = queryWithoutLiterals.trim();
const cteNames = getCteNames(compactQuery);
+ let referencesScopedEvents = false;
@@
const tableReferencePattern = /\b(?:FROM|JOIN)\s+([a-zA-Z_][a-zA-Z0-9_.]*|\()/gi;
let match: RegExpExecArray | null;
while ((match = tableReferencePattern.exec(compactQuery)) !== null) {
@@
const normalizedTableName = tableName.toLowerCase();
- if (normalizedTableName !== "scoped_events" && !cteNames.has(normalizedTableName)) {
+ if (normalizedTableName === "scoped_events") {
+ referencesScopedEvents = true;
+ continue;
+ }
+ if (!cteNames.has(normalizedTableName)) {
return "Queries can only read from scoped_events";
}
}
- if (!/\bscoped_events\b/i.test(compactQuery)) {
+ if (!referencesScopedEvents) {
return "Query must read from scoped_events";
}🧰 Tools
🪛 OpenGrep (1.22.0)
[ERROR] 201-201: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/api/analytics/utils/customQueryValidation.ts` around lines 199 -
215, The current validation only checks for the token /\bscoped_events\b/ which
can match aliases; update the logic that iterates tableReferencePattern (and
uses compactQuery, match, tableName, normalizedTableName, and cteNames) to
require that at least one actual FROM/JOIN table reference resolves to
scoped_events (case-insensitive) — i.e., when iterating matches, treat
parentheses as CTE/subquery skips and mark a flag foundScopedEvents = true if
normalizedTableName === "scoped_events"; after the loop, if foundScopedEvents is
false return "Query must read from scoped_events" and remove or replace the
final token-only regex check so only real table references pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit