From 83bd1cf275c3a7233baa47dd65b5258d4b135732 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Fri, 17 Jul 2026 05:31:50 +0000 Subject: [PATCH] fix(#266): validate share-link spec.view before writing resultView MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The share-link bootstrap wrote `queryView(spec)` (and a role preview) into `app.state.resultView` with only a legacy `chart`→`panel` remap and no enum check. The v2 tagged decode (`core/share.js`) passes `spec.view` through verbatim, so a crafted share link could set `resultView` to an arbitrary string. Narrow the value against `SAVED_VIEWS` (mirroring the Library-activation guard in `ui/saved-history.ts`): a Filter-role preview still wins, a persisted table/json/panel restores, legacy `chart` still maps to `panel`, and any other value silently falls back to the default view. Adds a unit test for the crafted-view case; main.ts stays at 100% coverage. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg --- CHANGELOG.md | 9 +++++++++ src/main.ts | 21 +++++++++++++-------- tests/unit/main.test.ts | 10 ++++++++++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3351c83..8437d92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,15 @@ auto-generated per-PR notes; this file is the curated, human-readable history. `chartJsConfig` into `buildChartData`) instead of three times. No user-facing behavior or schema change. +### Fixed +- **Share-link result view is validated before use** (#266). The share-link + bootstrap (`src/main.ts`) now narrows `spec.view` against the `resultView` + enum before assigning it — the v2 tagged decode passes `spec.view` through + verbatim, so a crafted link could previously set `resultView` to an arbitrary + string. Legacy `view: "chart"` still maps to `panel` and a Filter-role + preview still wins; any other value silently falls back to the default view, + matching the Library-activation path (`ui/saved-history.ts`). + ### Added - **ADR-0002 phases 1–5: the TypeScript migration slice lands whole** (#262, same PR as phase 0). Phase 1: `build/emit-schema-types.mjs`, a hand-rolled diff --git a/src/main.ts b/src/main.ts index d9e7ff0..9a8f47c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -16,7 +16,7 @@ import { cloneJson, queryName, queryPanel, queryView, upgradeSavedQuery } from ' import { isDashboardRoute } from './core/dashboard.js'; import { rolePreviewView } from './core/result-choice.js'; import { isQuerylessPanel } from './core/panel-cfg.js'; -import { setTabSpecDraft } from './state.js'; +import { setTabSpecDraft, SAVED_VIEWS } from './state.js'; import type { State } from './ui/app.types.js'; import type { BootstrapEnv } from './env.types.js'; import type { ResolvedIdpConfig } from './net/oauth-config.js'; @@ -131,13 +131,18 @@ export async function bootstrap(app: BootstrapApp, env: BootstrapEnv): Promise<{ // wins over the persisted view, regardless of whether the share carries // SQL to run — a SQL-bearing share never auto-runs here, so this only // pre-selects the drawer the recipient lands on before they click Run. - const launchView = rolePreviewView(shared.spec) || queryView(shared); - // `as`: unlike ui/saved-history.ts's restore path (which gates on - // `SAVED_VIEWS.has(launchView)`/`rolePreview` before this same - // assignment), this one doesn't validate `launchView` against the - // resultView union before restoring it — a pre-existing gap (#266), - // not fixed here. The cast preserves that exact (unvalidated) behavior. - if (launchView) app.state.resultView.value = (launchView === 'chart' ? 'panel' : launchView) as ResultView; + const rolePreview = rolePreviewView(shared.spec); + const launchView = rolePreview || queryView(shared); + // Normalize a legacy `view: 'chart'` (pre-Panel shares) to 'panel', then + // validate against the resultView union before assigning (#266): the v2 + // tagged decode passes `spec.view` through verbatim, so a crafted share + // link could otherwise set `resultView` to an arbitrary string. Mirrors + // ui/saved-history.ts's `rolePreview || SAVED_VIEWS.has(...)` guard — a + // role-owned Filter preview wins, a persisted table/json/panel restores, + // and any other value silently falls back to the default view. `as`: + // that check is the runtime proof `normalized` is a resultView member. + const normalized = launchView === 'chart' ? 'panel' : launchView; + if (rolePreview || SAVED_VIEWS.has(normalized ?? '')) app.state.resultView.value = normalized as ResultView; // A queryless panel with no role/persisted view (no SQL to run) still // needs the Panel drawer open, or the recipient lands on an empty Table // view and sees nothing. diff --git a/tests/unit/main.test.ts b/tests/unit/main.test.ts index a72070c..8f906e5 100644 --- a/tests/unit/main.test.ts +++ b/tests/unit/main.test.ts @@ -296,6 +296,16 @@ describe('bootstrap', () => { expect(app.state.resultView.value).toBe('panel'); }); + it('ignores an out-of-enum spec.view from a crafted share, keeping the default (#266)', async () => { + // The v2 tagged decode passes `spec.view` through verbatim, so a share link + // can carry any string; it must not reach the resultView signal. + const app = fakeApp(); + const env = v2Env({ sql: 'SELECT 1', spec: { name: 'Shared query', favorite: false, view: 'javascript:alert(1)' } }); + await bootstrap(app, env); + expect(app.state.tabs.value[0].sqlDraft).toBe('SELECT 1'); // the share still seeds + expect(app.state.resultView.value).toBe('table'); // but the bogus view is dropped + }); + it('never persists a transient view:"filter" into the restored tab Spec (#244)', async () => { const app = fakeApp(); const env = v2Env({ sql: 'SELECT 1', spec: { name: 'Shared query', favorite: false, dashboard: { role: 'filter' } } });