fix(grafana): escape AI usage SQL variables#5354
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 21:39:15 UTC
⏸️ Suggested Action - Manual Review Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agent
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
…eryText The model template variable's queryText/rawQueryText fallback (target?.rawQueryText ?? target?.queryText) meant an unescaped queryText would never be caught once both fields existed. Assert both explicitly, and extend the TemplateVar.query type to declare queryText (missing since #5349 added the field to the JSON).
13669cc to
e721ac6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5354 +/- ##
==========================================
- Coverage 94.71% 94.67% -0.04%
==========================================
Files 555 555
Lines 44526 44526
Branches 14664 14664
==========================================
- Hits 42172 42155 -17
- Misses 1619 1636 +17
Partials 735 735
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Motivation
Description
'$provider','$feature', and'$model'interpolations with Grafana${provider:sqlstring},${feature:sqlstring}, and${model:sqlstring}formatting acrossgrafana/dashboards/ai-usage.json(template variable model query and allai_usage_eventstargets).test/unit/selfhost-grafana-ai-usage-dashboard.test.tsto simulate Grafana SQL-string escaping by addingsqlString()andsubstituteSqlString()helpers and to assert the dashboard now uses${...:sqlstring}rather than raw'$...'tokens.Testing
npx vitest run test/unit/selfhost-grafana-ai-usage-dashboard.test.tsand all tests in that file passed (10 testspassing).npm run selfhost:validate-observabilityand the observability config validation completed successfully (dashboards and alert rules are valid).git diff --checkas part of preflight and it reported no whitespace/conflict issues.npm run test:cibut it did not complete cleanly due to a pre-existingcf-typegen:checkdrift message and anactionlintsetup retry caused by a transient DNS/network issue, so the fulltest:cigate was not green in this environment.npm audit --audit-level=moderatebut the npm registry audit endpoint returned403 Forbidden, so the dependency-review step could not be completed here.Codex Task