Skip to content

fix(security): bind public dashboard queries to their published shapes - #111

Open
notSumit25 wants to merge 2 commits into
mainfrom
fix/public-dashboard-query-binding
Open

fix(security): bind public dashboard queries to their published shapes#111
notSumit25 wants to merge 2 commits into
mainfrom
fix/public-dashboard-query-binding

Conversation

@notSumit25

Copy link
Copy Markdown
Collaborator

The problem

PublicDashboardController.query took the SQL to run as a caller-supplied body field and never compared it against the dashboard being shared:

public record PublicQueryRequest(String sql, Integer limit) { }

The only check was validateReadOnlySql, which asks whether a statement reads — not whether it is a query this dashboard was ever published to run. The endpoint is permitAll via /public/**, so a link created to publish one chart granted anonymous, unauthenticated read of every table on that connection:

curl -X POST https://host/api/public/dashboards/$TOKEN/query \
  -d '{"sql":"SELECT * FROM users","limit":5000}'

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.

Why an exact match couldn't be the fix

Public dashboards are interactive by design. dashboard-design/SKILL.md:148 instructs the agent to build date pickers that re-query on change, and :54 states plainly:

There is no placeholder convention. You write normal SQL strings in JS.

So the exact string isn't knowable at publish time. Exact matching would break interactive dashboards only on the public link — working for the author, failing for the audience. That's the worst shape a regression can take.

The fix: match the shape, not the text

DashboardQueryShapeService extracts every query the artifact can issue, normalizes each to a shape (literals → placeholders, via the existing QueryNormalizer that already backs QueryFingerprintService), and requires a match.

Incoming query Result
Same query, different date range ✅ allowed — interactivity preserved
Whitespace / newline / case variants ✅ allowed
SELECT * FROM users 🚫 refused
Same shape, different table 🚫 refused
Same shape, different column 🚫 refused
OR 1=1 appended 🚫 refused
Escaped-quote UNION inside a literal 🚫 refused

The last row is refused for a non-obvious reason worth knowing. The payload '2026-03-01 '' UNION SELECT password FROM users --' normalizes to … between ? and ??two placeholders, not one — because the '[^']*' rule doesn't model SQL's '' escape and splits the literal differently than the database would. The shape changes, so it misses. The imprecision fails in the safe direction: smuggling structure through a literal perturbs the shape.

Extraction is static

dashboard_config stores the artifact as one HTML document with queries as JS template literals, so the SQL is statically present — it just carries ${…} interpolation, which maps cleanly onto the normalizer's placeholders. A test asserts directly that the shape extracted from the artifact equals the shape of the SQL issued at runtime — that seam is what the design rests on, so it's pinned rather than assumed.

Chosen over capturing shapes at first render: no extra step, and it can't produce a partly-captured set that breaks a link for its audience.

Fails closed

An unmatched shape is refused. The artifact already renders a per-widget error, so one widget degrades alone and the rest of the dashboard keeps working.

Second defect: a policy added after sharing didn't apply

Enabling a share is refused while a connection has an active chat-access policy (SavedDashboardController:81) — but nothing re-checked afterwards. A link created before a policy was added stayed live, and on that path "public-share" has no policy row, so resolveEffectivePolicy returns none() and column protections and PII redaction never ran.

Narrower than "public callers bypass all policies" (the normal flow can't create that combination), but a real TOCTOU gap. Now re-checked per query — same reason is_public already is: revocation has to reach an already-issued link.

Defence in depth

The shape gate is a new primary control, not a replacement. validateReadOnlySql, setReadOnly(true), the row cap and the is_public re-check all remain. This matters because QueryNormalizer was built for analytics grouping, where a collision is cosmetic; as a security boundary it would be a vulnerability. It's deliberately one layer among several.

Verification

Step Result
Before the service existed (RED) compilation failure — class not found
After the fix (GREEN) 13 pass
matches() stubbed to return true (mutation) 5 fail — exfiltration + fail-closed cases
Restored green again
Related suites 50 tests, 0 failures
mvn compile clean
cd backend && mvn test -Dtest=DashboardQueryShapeServiceTest

Residual work (deliberately not here)

The public path still has no rate limitdocker/nginx/default.conf declares only sqlexec, scoped to ^/api/connections/[^/]+/query$. CLAUDE.md claimed a dashq limiter existed; it never did. Shape binding bounds the damage (an anonymous caller can now only re-run the dashboard's own queries), but a heavy widget can still be hammered. Separate nginx change with its own blast radius.

Design: docs/superpowers/specs/2026-09-11-public-dashboard-query-binding-design.md
Write-up: docs/security/2026-09-11-public-dashboard-arbitrary-sql.md

🤖 Generated with Claude Code

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>
…ly written

Hands-on QA against the live stack found the previous commit would have
refused every query on every existing public dashboard. Three defects, all
hidden by a green unit suite whose fixtures encoded assumptions about the
agent's output rather than its actual output:

1. Real artifacts assign the SQL to a variable first — 18 of 18 call sites in
   this database, across the names query, sql, trendQuery and totalQuery, do
   `const sql = `...`; await deepsql.query(sql)`. The extractor matched only a
   literal argument, so it returned an empty set and, failing closed, refused
   everything. Run against a real 33KB artifact: 8 call sites, 0 shapes.

2. Widgets are separate scopes that reuse the same name. One dashboard has
   nine script blocks, eight declaring their own `const sql`. Resolving into
   one flat map collapsed them: 8 calls yielded 1 shape, silently dropping
   seven widgets. Declarations are now resolved per script block.

3. dashboard_config stores the broker's JSON envelope, not raw HTML, so the
   document arrives with newlines as a literal backslash-n. QueryNormalizer
   collapses real whitespace, so a published shape kept "customer_count\n from"
   where the runtime statement has a space and nothing matched. An earlier
   probe unescaped the dump by hand, making the harness more forgiving than
   production. Escape sequences are now decoded during extraction.

Also adds hasUnresolvableQuery: an argument that cannot be resolved to a
literal is reported rather than skipped, since skipping publishes a shape set
missing one of the dashboard's own queries — a widget broken for the audience
only, with nothing on the authoring side to indicate why.

Verified against the live stack, not inferred: 28 of 28 call sites across five
real artifacts extract to 28 shapes with none unresolvable; all 10 published
shapes replay through the API accepted; the public share page renders all nine
widgets with real data in Chrome; clicking "Last 90 days" fires six queries
with new date literals, all 200, moving the KPIs from $1,953/6 orders to
$144,270/426 orders — the interactivity exact-matching would have broken; and
SELECT * FROM public.customers is refused while the unpatched backend still
returns email, password_hash and phone. 58 tests green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@notSumit25

Copy link
Copy Markdown
Collaborator Author

Hands-on QA against the live stack — 3 blockers found and fixed

I QA'd this against the real running stack (live Postgres with 10 real dashboards, 3 public with real share tokens) rather than trusting the green unit suite. The original commit would have refused every query on every existing public dashboard. All three defects shared one cause: my fixtures encoded assumptions about the agent's output instead of its actual output.

1. Real artifacts assign the SQL to a variable

Every call site in this database — 18 of 18, across query, sql, trendQuery, totalQuery — does:

const sql = `SELECT ... WHERE created_at >= '${esc(from)}'`;
const { rows } = await deepsql.query(sql);     // ← not a literal

The extractor matched only a literal argument. Executed against a real 33KB artifact: 8 call sites, 0 shapes → fails closed → dashboard fully broken.

2. Widgets are separate scopes reusing the same name

One dashboard has nine <script> blocks, eight declaring their own const sql. A flat map collapsed them onto one key: 8 calls → 1 shape, silently dropping seven widgets. Now resolved per script block.

3. dashboard_config is a JSON envelope, not raw HTML

{"version":3,"renderMode":"artifact","html":"<!doctype html>\n..."}

Newlines arrive as a literal backslash-n. QueryNormalizer collapses real whitespace, so the published shape kept customer_count\n from where the runtime statement has a space — no match. My earlier probe had sed-unescaped the dump by hand, making the harness more forgiving than production. This one survived three rounds of green tests.

Also added hasUnresolvableQuery() — an unresolvable argument is now reported, not skipped, since skipping publishes a partial shape set that breaks a widget for the audience only.

Verified against the live stack

Check Result
Shapes from all 5 real artifacts 28 call sites → 28 shapes, 0 unresolvable
Every published shape replayed through the API 10/10 accepted, 0 wrongly refused
Public share page in Chrome (patched backend) all 9 widgets render real data
Clicked "Last 90 days" (new date literals) 6 queries, all 200 — $1,953→**$144,270**, 6→426 orders
SELECT * FROM public.customers refused
Same request, unpatched backend returns email, password_hash, phone

The date-range click is the key evidence: literals changed, so exact matching would have refused all six — the shape gate allowed them while still blocking exfiltration. That is precisely the trade-off this design was chosen for.

Pre-fix severity confirmed live, not inferred: an unauthenticated caller holding only a share link read full customer rows including password_hash and phone.

58 tests green. DB and frontend restored to baseline (3 public / 10 total).

Previous QA verdict: NOT READY → now READY.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant