Skip to content

Commit fbe83fc

Browse files
notSumit25claude
andcommitted
fix(security): bind public dashboard queries to their published shapes
POST /api/public/dashboards/{token}/query took the SQL to run as a caller-supplied body field and never compared it against the dashboard being shared. The only check was validateReadOnlySql, which asks whether a statement reads — not whether this dashboard was published to run it. The endpoint is permitAll via /public/**, so a link created to publish one chart granted anonymous, unauthenticated read of every table on that connection, paginable to completion. Share tokens are 192-bit so this was never brute-forceable; the exposure is to whoever receives or forwards a link, which is exactly the population a share link is meant to be safe for. An exact string match could not be the fix. The agent builds interactive dashboards whose SQL is interpolated at runtime — SKILL.md states there is no placeholder convention — so exact matching would break public links while the author's own view kept working, failing only for the audience. DashboardQueryShapeService instead matches by shape: the statement with its literals replaced by placeholders, via the QueryNormalizer that already backs QueryFingerprintService. A date-range change shares a shape; a different table, column or predicate does not. Shapes are extracted statically from the stored artifact, so there is no capture step and no partly-captured set, and a test pins that the extracted shape equals the shape of the SQL issued at runtime. Unmatched shapes fail closed and the artifact renders its existing per-widget error, so one widget degrades alone. Structure smuggled inside a string literal is refused because the normalizer's '[^']*' rule does not model SQL's '' escape: the payload splits into a different number of placeholders, so the shape changes. The imprecision fails in the safe direction. It remains one layer — the read-only guard, setReadOnly(true), the row cap and the is_public re-check all still apply. Also re-checks hasActivePolicy per query. Enabling a share is refused while a policy is active, but nothing re-checked afterwards, so a link created before a policy was added stayed live and unprotected — "public-share" has no policy row, so resolveEffectivePolicy returns none() and column protections and redaction never ran. Re-checked for the same reason is_public is. Verified: 13 tests fail to compile before the service exists, pass after, and 5 fail when matches() is stubbed to return true. 50 tests green across the related suites, mvn compile clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 29de992 commit fbe83fc

6 files changed

Lines changed: 607 additions & 0 deletions

File tree

‎CLAUDE.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,30 @@ Dashboards are **generated by the embedded DeepSQL Agent acting as a coding agen
261261
- **Rendering + data access**: `DashboardArtifact.jsx` renders the HTML in a **sandboxed iframe** (`sandbox="allow-scripts"`, opaque origin + a strict CSP — no external network). The artifact fetches data only through an injected `deepsql.query(sql)` bridge that `postMessage`s to the parent; the parent calls **`POST /api/dashboards/query`** (`DashboardQueryController`), which is **read-only twice over** (`McpSqlGuardService.validateReadOnlySql` + `QueryExecutionContext.api` = `READ_ONLY_ONLY`) and access-scoped via `assertCanReadConnectionContent`. So the agent's code has full creative freedom while every query stays guarded and sandboxed. The bridge also auto-sizes the iframe and forwards runtime errors.
262262
- Generation endpoints unchanged (`POST /api/dashboards/generate` + `/generate/stream`). `DashboardBuilder.js`/`DashboardInputs.js` remain only because `tabs/Core/PreviewTab.js` still uses them — the dashboard *creation* path no longer touches them.
263263
- **Sharing**: both share types render a standalone read-only `DashboardViewer` (title + `DashboardArtifact` with an injected `queryFn`). Internal link `/dashboard-view/:id` (auth) uses the authed broker; public link `/share/dashboard/:token` (permitAll) uses `PublicDashboardController` (`GET /api/public/dashboards/{token}` + `/query`), which resolves only while `saved_dashboards.is_public` is true (revoke = flip it) and runs read-only + connection-scoped. `share_token`/`is_public` are set only via `POST|DELETE /api/saved-dashboards/{id}/share` (access-checked), never a general update. `ShareMenu.jsx` drives the UI. The public query path has its own nginx `dashq` limiter.
264+
- **A public share token is not a licence to run any SQL.** `POST /api/public/dashboards/{token}/query`
265+
took the SQL as a body field and never compared it against the dashboard being shared — only
266+
`validateReadOnlySql`, which asks whether a statement *reads*, not whether this dashboard was
267+
published to run it. A link shared to show one chart therefore granted **anonymous read of every
268+
table on the connection** (`SELECT * FROM users`), paginable to completion. Tokens are 192-bit so
269+
this was never brute-forceable; the exposure is to whoever receives or forwards a link.
270+
`DashboardQueryShapeService` now binds each public query to a **shape** extracted from the
271+
dashboard's own stored artifact: the statement with literals replaced by placeholders, via the
272+
existing `QueryNormalizer`. Shape-matching rather than exact-matching is load-bearing — the agent
273+
builds interactive dashboards whose SQL is interpolated at runtime (`SKILL.md`: "There is no
274+
placeholder convention"), so an exact match would break public links while the author's own view
275+
kept working. Literals vary freely; tables, columns and predicates do not. Extraction is static,
276+
from `dashboard_config`, so there is no capture step and no partly-captured set. **Unmatched
277+
shapes fail closed** and the artifact renders its existing per-widget error, so one widget
278+
degrades alone. Note the normalizer's `'[^']*'` rule does not model SQL's `''` escape, which is
279+
*why* structure smuggled inside a literal is refused: it splits into a different number of
280+
placeholders, so the shape changes. The imprecision fails safe — but it is one layer, not the
281+
only one, and the read-only guard, `setReadOnly(true)` and the row cap all still apply.
282+
- **Revoking a chat-access policy has to reach an already-issued share link.** Enabling a public
283+
share is refused while the connection has an active policy (`SavedDashboardController:81`), but
284+
nothing re-checked afterwards — so a link created *before* a policy was added stayed live and
285+
unprotected, because `"public-share"` has no policy row and `resolveEffectivePolicy` returns
286+
`none()`, meaning column protections and redaction never ran. `PublicDashboardController` now
287+
re-checks `hasActivePolicy` per query, for the same reason it re-checks `is_public`.
264288
- **Organization** (search/folders/favorites): `SavedDashboardController`'s search/folder/favorite endpoints existed for a while with no UI consumer. `DashboardsHome.jsx` now wires all of it — a search box (client-side filter over name/description), folder chips derived from `GET /connection/{id}/folders` with a per-card "move to folder" popover (`PUT /saved-dashboards/{id}` with `folder: ""` to clear — `updateDashboard` treats `null` as "field omitted" so blank is the explicit clear signal, same convention as `setSharePassword`), and a favorite star toggle (`POST /{id}/favorite`) with optimistic UI update.
265289
- **Clone**: `POST /saved-dashboards/{id}/clone` (`SavedDashboardService.cloneDashboard`) duplicates a dashboard's config/chat/tags/folder into a fresh row — not shared, not favorited. Exposed as a copy icon on each `DashboardsHome.jsx` card.
266290
- **Version history**: every real overwrite of `dashboardConfig` (agent build via `completeBuildTurn`, manual Source-tab edit via `updateDashboard`, or a restore) snapshots the *previous* config into `dashboard_versions` (`V113__create_dashboard_versions.sql`) before overwriting, tagged with a trigger (`AGENT_BUILD`/`MANUAL_EDIT`/`RESTORE`) — capped at 50 snapshots per dashboard, oldest pruned first. `GET /{id}/versions` lists them newest-first; `POST /{id}/versions/{versionId}/restore` swaps a snapshot back in as current (itself snapshotting whatever was live, so a restore is undoable too) and **dedupes**: after a restore, the restored row plus any other row with byte-identical `dashboard_config` are deleted, since that content is now "Current," not history — otherwise a restore-edit-restore cycle piles up an alternating chain of duplicate snapshots. `DashboardWorkspace.jsx`'s History panel shows a lightweight diff summary per entry (title/widget-count/size delta computed client-side, not a real line diff — the agent rewrites large chunks even for small logical changes) plus a Preview modal that renders that version's HTML live via `DashboardArtifact`.

‎backend/src/main/java/com/dbaagent/controller/PublicDashboardController.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import com.dbaagent.model.QueryRequest;
55
import com.dbaagent.model.QueryResult;
66
import com.dbaagent.model.SavedDashboard;
7+
import com.dbaagent.service.ConnectionChatAccessPolicyService;
8+
import com.dbaagent.service.DashboardQueryShapeService;
79
import com.dbaagent.service.McpSqlGuardService;
810
import com.dbaagent.service.QueryExecutionContext;
911
import com.dbaagent.service.QueryExecutorService;
@@ -18,6 +20,7 @@
1820

1921
import java.util.List;
2022
import java.util.Map;
23+
import java.util.Set;
2124
import java.util.Optional;
2225

2326
/**
@@ -43,6 +46,8 @@ public class PublicDashboardController {
4346
private final SavedDashboardService savedDashboardService;
4447
private final ObjectMapper objectMapper;
4548
private final McpSqlGuardService sqlGuardService;
49+
private final DashboardQueryShapeService queryShapeService;
50+
private final ConnectionChatAccessPolicyService policyService;
4651
private final QueryExecutorService queryExecutorService;
4752

4853
private Optional<SavedDashboard> publicDashboard(String token) {
@@ -101,6 +106,29 @@ public ResponseEntity<?> query(@PathVariable String token, @RequestBody PublicQu
101106
if (!guard.ok()) {
102107
return ResponseEntity.badRequest().body(Map.of("success", false, "error", guard.reason()));
103108
}
109+
// Read-only is not enough on an anonymous path: it asks whether the statement reads,
110+
// not whether this dashboard was published to run it. Without the shape check below, a
111+
// link shared to show one chart accepted "SELECT * FROM users" just as happily.
112+
// A policy added AFTER the link was shared must take effect on it. Enabling a share is
113+
// refused while a policy is active (SavedDashboardController), but nothing re-checked
114+
// afterwards, so a link created before the policy stayed live and unprotected —
115+
// "public-share" has no policy row, so resolveEffectivePolicy returns none() and
116+
// column protections and redaction never run. Re-checked here for the same reason
117+
// is_public is: revocation has to reach an already-issued link.
118+
if (policyService.hasActivePolicy(found.get().getConnectionId())) {
119+
log.info("Public dashboard query refused for token {}: connection has an active policy", token);
120+
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(Map.of(
121+
"success", false,
122+
"error", "This dashboard is no longer available publicly."));
123+
}
124+
Set<String> publishedShapes =
125+
queryShapeService.extractShapes(found.get().getDashboardConfig());
126+
if (!queryShapeService.matches(publishedShapes, request.sql())) {
127+
log.info("Public dashboard query refused for token {}: shape not published", token);
128+
return ResponseEntity.badRequest().body(Map.of(
129+
"success", false,
130+
"error", "This query is not part of the shared dashboard."));
131+
}
104132
int limit = request.limit() == null ? DEFAULT_LIMIT : Math.max(1, Math.min(request.limit(), MAX_LIMIT));
105133
try {
106134
QueryRequest qr = new QueryRequest();
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package com.dbaagent.service;
2+
3+
import com.dbaagent.util.QueryNormalizer;
4+
import org.springframework.stereotype.Service;
5+
6+
import java.util.LinkedHashSet;
7+
import java.util.Set;
8+
import java.util.regex.Matcher;
9+
import java.util.regex.Pattern;
10+
11+
/**
12+
* Binds the queries a public dashboard may run to the ones its artifact actually contains.
13+
*
14+
* <p>{@code POST /api/public/dashboards/{token}/query} takes the SQL as a request body field.
15+
* Checking only that the statement reads is not enough: it answers "is this a select" when the
16+
* question is "is this a query this dashboard was published to run". Without that second check
17+
* a link shared to show one chart grants anonymous read of the whole connection.
18+
*
19+
* <p>Exact string matching cannot be the answer. Dashboards are interactive by design — a date
20+
* picker re-queries with new bounds on every change ({@code dashboard-design/SKILL.md}), so the
21+
* exact string is not knowable at publish time. Matching would then fail only on the public
22+
* link while the author's own view kept working, which is the worst shape a regression can take.
23+
*
24+
* <p>So queries are matched by <em>shape</em>: the statement with its literals replaced by
25+
* placeholders, via the same {@link QueryNormalizer} that backs
26+
* {@link QueryFingerprintService}. Two queries differing only in a date range share a shape;
27+
* two naming different tables or columns do not.
28+
*
29+
* <p>This is one layer, not the only one. {@code validateReadOnlySql},
30+
* {@code connection.setReadOnly(true)}, the row cap and the {@code is_public} re-check all still
31+
* apply. That matters because {@code QueryNormalizer} was written for analytics grouping, where
32+
* a collision is a cosmetic nuisance rather than a vulnerability.
33+
*/
34+
@Service
35+
public class DashboardQueryShapeService {
36+
37+
/**
38+
* The first argument of a {@code deepsql.query(...)} call, in each quoting style the agent
39+
* emits — backtick, double and single. Escaped quotes are consumed so a literal containing
40+
* the delimiter does not end the match early.
41+
*/
42+
private static final Pattern QUERY_CALL = Pattern.compile(
43+
"deepsql\\s*\\.\\s*query\\s*\\(\\s*"
44+
+ "(`(?:[^`\\\\]|\\\\.)*`"
45+
+ "|\"(?:[^\"\\\\]|\\\\.)*\""
46+
+ "|'(?:[^'\\\\]|\\\\.)*')",
47+
Pattern.DOTALL);
48+
49+
/**
50+
* A JS template interpolation. Replaced with a quoted placeholder before normalizing, so the
51+
* interpolated value is treated as the literal it becomes at runtime: {@code '${from}'}
52+
* already sits inside quotes in the artifact, and a bare {@code ${n}} still has to normalize
53+
* to the same placeholder the runtime's numeric literal produces.
54+
*/
55+
private static final Pattern INTERPOLATION = Pattern.compile("\\$\\{[^}]*\\}");
56+
57+
/** Extracts the shape of every query the artifact can issue. */
58+
public Set<String> extractShapes(String artifactHtml) {
59+
Set<String> shapes = new LinkedHashSet<>();
60+
if (artifactHtml == null || artifactHtml.isBlank()) {
61+
return shapes;
62+
}
63+
Matcher calls = QUERY_CALL.matcher(artifactHtml);
64+
while (calls.find()) {
65+
String shape = shapeOf(unwrapJsLiteral(calls.group(1)));
66+
if (!shape.isBlank()) {
67+
shapes.add(shape);
68+
}
69+
}
70+
return shapes;
71+
}
72+
73+
/** The shape of one SQL statement: its literals replaced by placeholders. */
74+
public String shapeOf(String sql) {
75+
if (sql == null || sql.isBlank()) {
76+
return "";
77+
}
78+
return QueryNormalizer.normalize(sql);
79+
}
80+
81+
/**
82+
* Whether {@code sql} matches a published shape. Fails closed: an empty shape set, a blank
83+
* statement, or any shape that was not extracted is refused.
84+
*/
85+
public boolean matches(Set<String> publishedShapes, String sql) {
86+
if (publishedShapes == null || publishedShapes.isEmpty() || sql == null || sql.isBlank()) {
87+
return false;
88+
}
89+
String shape = shapeOf(sql);
90+
return !shape.isBlank() && publishedShapes.contains(shape);
91+
}
92+
93+
/**
94+
* Strips the surrounding quotes from a JS string literal and collapses interpolations.
95+
*
96+
* <p>An interpolation becomes {@code '?'} — a quoted placeholder — so that
97+
* {@code BETWEEN '${from}' AND '${to}'} yields the same shape as the runtime statement
98+
* {@code BETWEEN '2026-01-01' AND '2026-03-01'}. The surrounding quotes already present in
99+
* the artifact are left in place and normalized away with it.
100+
*/
101+
private String unwrapJsLiteral(String literal) {
102+
String body = literal.substring(1, literal.length() - 1);
103+
return INTERPOLATION.matcher(body).replaceAll("?");
104+
}
105+
}

0 commit comments

Comments
 (0)