fix(observability): escape Grafana repo variable to prevent SQL injection#5323
fix(observability): escape Grafana repo variable to prevent SQL injection#5323JSONbored wants to merge 1 commit into
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5323 +/- ##
=======================================
Coverage 94.34% 94.34%
=======================================
Files 473 473
Lines 39982 39982
Branches 14576 14576
=======================================
Hits 37722 37722
Misses 1585 1585
Partials 675 675
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 21:15:35 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.
|
Motivation
$repovariable directly into raw SQLite SQL inside quotes, which can be abused by a craftedvar-repovalue to change WHERE clauses or comment out the rest of a query.Description
'$repo'pattern with Grafana's SQL-string formatter${repo:sqlstring}acrossgrafana/dashboards/maintainer-reviews.jsonso panels use(${repo:sqlstring} = '$__all' OR repo = ${repo:sqlstring})for repo scoping.test/unit/selfhost-grafana-dashboard.test.tsto simulate Grafana's SQL-string expansion by addinggrafanaSqlString()and changingexpandGrafanaRange()to accept and SQL-escape a repo selection, and adjusted existing assertions to match the new${repo:sqlstring}pattern.x') OR 1=1 --) and asserts the SQL-escaped value is treated literally (returns no rows) to protect against widening queries.Allsentinel logic and time-window bindings remain the same) while only changing how the variable is formatted in SQL.Testing
npx vitest run test/unit/selfhost-grafana-dashboard.test.tswas run and all tests in that file passed.git diff --checkandnpm run selfhost:validate-observabilitywere run and both succeeded, confirming no formatting or observability-config regressions.npm run test:ciwas executed but failed duringcf-typegen:checkin this environment due to a local Wrangler/workerd generation difference from the committedworker-configuration.d.ts, not related to the dashboard change.npm audit --audit-level=moderatewas attempted but failed due to the npm registry audit endpoint returning403 Forbiddenin this environment, so dependency-audit could not be completed here.Codex Task