Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 13 additions & 8 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' } } });
Expand Down
Loading