From 4c2694736016be42ed015bc80f41366623b13640 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 29 Jul 2026 16:47:28 +0000 Subject: [PATCH] fix(#551): keep __proto__/constructor tile ids' placements through defineJsonField/readJsonField Every Dashboard placement-map write (grafana-grid-layout.ts, flow-layout.ts, dashboard-document.ts's legacy migrations/fallback regeneration, dashboard-commands.ts's duplicate-tile and flow->grid change-layout) now writes through core/saved-query.ts's defineJsonField (Object.defineProperty) instead of a bare map[tileId] = value, so a tile id of '__proto__' or 'constructor' (both schema-legal) survives as an own property instead of silently invoking the inherited Object.prototype setter. Self-review surfaced a second, more severe instance of the same root cause: setStylePlacement's merge-then-rewrite read a not-yet-owned entry via a bare items[tileId], which for '__proto__' resolves to Object.prototype itself, then mutated it in place -- real prototype pollution, not just a dropped placement. Every read that could be merged/mutated now goes through a new paired readJsonField helper (own-property-only). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GiubaoqEuBzAyo5C4P8Vqr --- CHANGELOG.md | 20 +++++ src/core/saved-query.ts | 45 +++++++++- .../application/dashboard-commands.ts | 14 +-- src/dashboard/layouts/flow-layout.ts | 8 +- src/dashboard/layouts/grafana-grid-layout.ts | 31 ++++--- src/dashboard/model/dashboard-document.ts | 25 +++--- tests/unit/dashboard-commands.test.ts | 86 ++++++++++++++++++- tests/unit/dashboard-document.test.ts | 75 ++++++++++++++++ tests/unit/flow-layout.test.ts | 13 +++ tests/unit/grafana-grid-layout.test.ts | 65 ++++++++++++++ tests/unit/portable-bundle-codec.test.ts | 34 ++++++++ tests/unit/saved-query.test.ts | 21 ++++- tests/unit/stored-workspace.test.ts | 33 +++++++ 13 files changed, 432 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71ee0cd0..7c130be2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,26 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] ### Fixed +- **A Dashboard tile whose id is `__proto__` or `constructor` no longer loses + its placement** (#551, found reviewing #549). Both are legal tile ids under + `dashboardTileV1.id` (pattern `\S`), and a placement-map write of the shape + `map[tileId] = placement` silently drops them: the assignment invokes the + inherited `Object.prototype.__proto__` setter instead of creating an own + property, so the tile falls back to the layout engine's default size. + Worse, one read site (`grafana-grid@2`'s `setStylePlacement`, merging a new + style onto a tile's existing style map) read the same key back before + writing it, and for a tile id with no own entry yet that bare read resolves + to `Object.prototype` itself — the subsequent merge-write mutated the real + `Object.prototype` for the whole process, not just the one layout. Every + placement-map write (`grafana-grid-layout.ts`, `flow-layout.ts`, + `dashboard-document.ts`'s legacy-layout migrations and fallback + regeneration, `dashboard-commands.ts`'s `duplicate-tile` and the + flow→grafana-grid `change-layout` conversion) now goes through one shared + `Object.defineProperty`-based helper (`defineJsonField`, already used by + `core/saved-query.ts` for the same class of Spec/panel JSON fields), and + every read that could be merged or mutated goes through its paired + own-property-only counterpart (`readJsonField`), closing that + `Object.prototype` escape. - **Dashboard styles now persist independent dimensions and temporary column previews no longer mutate authored layouts** (behavioral correction to #535/#538). The new `grafana-grid@2` contract stores Grid `{span,height}`, diff --git a/src/core/saved-query.ts b/src/core/saved-query.ts index e8f0c78b..7445f0dc 100644 --- a/src/core/saved-query.ts +++ b/src/core/saved-query.ts @@ -30,12 +30,55 @@ export function isPlainObject(value: unknown): value is Record return !!value && typeof value === 'object' && !Array.isArray(value); } -function defineJsonField(target: Record, key: string, value: unknown): void { +/** + * Write one JSON-shaped key onto a plain object as an OWN enumerable, + * writable, configurable data property, via `Object.defineProperty` rather + * than `target[key] = value`. The difference matters for exactly one key: a + * bare assignment to `'__proto__'` on a plain object invokes the INHERITED + * `Object.prototype.__proto__` setter (changing `target`'s own prototype + * instead of creating a property), so the write silently vanishes and + * `JSON.stringify(target)` never sees it. `defineProperty` always creates an + * own data property, so `'__proto__'`/`'constructor'` survive as ordinary + * forward-compatible data — the same class of input `JSON.parse` already + * produces (it special-cases object keys the same way). + * + * This is the one shared primitive for every write into a plain object keyed + * by caller-controlled string data — used here for Spec/panel/dashboard + * fields, and reused by every Dashboard placement-map write (`layout.items`) + * across `src/dashboard/layouts` and `src/dashboard/model` (#551): a tile id + * is exactly such caller-controlled data (schema pattern `\S`, so `__proto__` + * and `constructor` are both legal tile ids). No prototype pollution is + * possible either way: only `target`'s own `[[Prototype]]`/properties are + * ever touched, never `Object.prototype` itself. + */ +export function defineJsonField(target: Record, key: string, value: unknown): void { Object.defineProperty(target, key, { value, enumerable: true, writable: true, configurable: true, }); } +/** + * Read one key off a plain object as an OWN property only — `undefined` for + * anything else. The read-side counterpart of `defineJsonField`, and not a + * redundant safety net: a bare `target[key]` for `key === '__proto__'` (or + * `'constructor'`) on a `target` with no OWN property under that name + * resolves through the prototype chain to `Object.prototype` itself (or + * `Object.prototype.constructor`) — a real object, so an `isObject(...)` + * check alone cannot tell it apart from genuine stored data. Code that reads + * a placement this way to MERGE-then-rewrite it (e.g. `current[style] = + * next` while preserving `current`'s other fields) ends up assigning + * directly onto the ALIASED `Object.prototype`, mutating it for the whole + * realm — not a hypothetical: this is exactly how `setStylePlacement` + * (`grafana-grid-layout.ts`) polluted `Object.prototype.grid` for every + * OTHER placement map in the same test run until this helper replaced its + * bare read (#551 review). Every read of a caller-keyed JSON map that could + * be merged, mutated, or treated as "no entry vs. an empty one" must go + * through this, not a bare bracket access. + */ +export function readJsonField(target: Record, key: string): unknown { + return Object.hasOwn(target, key) ? target[key] : undefined; +} + /** * JS `value && value[key]` semantics, made narrowable under `strict`: a falsy * `value` passes straight through unchanged (so, e.g., `withQuerySpec` keeps diff --git a/src/dashboard/application/dashboard-commands.ts b/src/dashboard/application/dashboard-commands.ts index 6ed41173..31a26862 100644 --- a/src/dashboard/application/dashboard-commands.ts +++ b/src/dashboard/application/dashboard-commands.ts @@ -14,7 +14,7 @@ // patch, or an invalid placement. Role/limit/reference/presentation failures // are left to the caller's validation stage. -import { cloneJson } from '../../core/saved-query.js'; +import { cloneJson, defineJsonField, readJsonField } from '../../core/saved-query.js'; import { diagnostic } from '../model/workspace-diagnostics.js'; import type { WorkspaceDiagnostic } from '../model/workspace-diagnostics.js'; import { @@ -128,7 +128,7 @@ function placementForActiveEngine( plugin: DashboardLayoutPlugin, layout: unknown, tileId: string, ): unknown { const placement = plugin.type === 'grafana-grid' && plugin.version === 2 - ? (isObject(layout) && isObject(layout.items) ? layout.items[tileId] : undefined) + ? (isObject(layout) && isObject(layout.items) ? readJsonField(layout.items, tileId) : undefined) : plugin.type === 'grafana-grid' ? gridPlacementAt(layout, tileId) : flowPlacementAt(layout, tileId); return isObject(placement) ? placement : undefined; @@ -236,7 +236,7 @@ function applyCommandToClone( if (placement !== undefined) { if (ctx.plugin.type === 'grafana-grid' && ctx.plugin.version === 2 && isObject(dashboard.layout) && isObject(dashboard.layout.items)) { - (dashboard.layout.items as Record)[newTileId] = cloneJson(placement); + defineJsonField(dashboard.layout.items as Record, newTileId, cloneJson(placement)); } else { setPlacementForActiveEngine(ctx.plugin, dashboard.layout, newTileId, cloneJson(placement)); } @@ -385,14 +385,14 @@ function applyCommandToClone( } if (currentType === 'flow' && targetType === 'grafana-grid') { - const flowItems = dashboard.layout.items ?? {}; + const flowItems = isObject(dashboard.layout.items) ? dashboard.layout.items : {}; const gridItems: Record = {}; for (const tile of tiles) { if (!isObject(tile) || typeof tile.id !== 'string') continue; - const flowPlacement = resolvePlacement(flowItems[tile.id]); - gridItems[tile.id] = { + const flowPlacement = resolvePlacement(readJsonField(flowItems, tile.id)); + defineJsonField(gridItems, tile.id, { span: gridSpanFromFlowSpan(flowPlacement.span), height: gridHeightUnitsFromFlowHeight(flowPlacement.height), - }; + }); } // Drop the (never-present-on-a-flow-primary) `fallback` field before // snapshotting — a flow primary IS the fallback engine, so it never diff --git a/src/dashboard/layouts/flow-layout.ts b/src/dashboard/layouts/flow-layout.ts index bae47252..6f0945a7 100644 --- a/src/dashboard/layouts/flow-layout.ts +++ b/src/dashboard/layouts/flow-layout.ts @@ -14,7 +14,7 @@ import { diagnostic } from '../model/workspace-diagnostics.js'; import type { WorkspaceDiagnostic } from '../model/workspace-diagnostics.js'; import { isFlowLayout } from '../model/workspace-semantics.js'; -import { cloneJson } from '../../core/saved-query.js'; +import { cloneJson, defineJsonField, readJsonField } from '../../core/saved-query.js'; import { partitionKpiBands } from '../../core/dashboard.js'; import type { DashboardDocumentV2, FlowHeightV1, FlowPresetV1, FlowTilePlacementV1, @@ -63,7 +63,7 @@ function flowItemsHost(layout: unknown): Record | null { * No-op when the layout has no flow surface. */ export function setFlowPlacement(layout: unknown, tileId: string, placement: unknown): void { const items = flowItemsHost(layout); - if (items) items[tileId] = placement; + if (items) defineJsonField(items, tileId, placement); } /** One tile's STORED flow placement, or `undefined` when the layout holds none @@ -82,7 +82,7 @@ export function flowPlacementAt(layout: unknown, tileId: string): unknown { if (!isObject(layout)) return undefined; const surface = isFlowLayout(layout.type, layout.version) ? layout : layout.fallback; if (!isObject(surface) || !isObject(surface.items)) return undefined; - return surface.items[tileId]; + return readJsonField(surface.items, tileId); } /** Derive an initial flow placement from a query's `sizeHints.preferred` @@ -263,7 +263,7 @@ export function computeFlowLayout(input: ComputeFlowLayoutInput): FlowLayoutMode const columns = mobile ? 1 : presetColumns(preset); const renders: FlowTileRender[] = tiles.map((tile, index) => { - const placement = resolvePlacement(items[tile.id]); + const placement = resolvePlacement(readJsonField(items, tile.id)); return { tileId: tile.id, index, isKpi: !!tile.isKpi, height: placement.height, span: mobile ? 1 : effectiveSpan(placement.span, columns), diff --git a/src/dashboard/layouts/grafana-grid-layout.ts b/src/dashboard/layouts/grafana-grid-layout.ts index 5adad5e4..b1dd386c 100644 --- a/src/dashboard/layouts/grafana-grid-layout.ts +++ b/src/dashboard/layouts/grafana-grid-layout.ts @@ -53,7 +53,7 @@ import { diagnostic } from '../model/workspace-diagnostics.js'; import type { WorkspaceDiagnostic } from '../model/workspace-diagnostics.js'; -import { cloneJson } from '../../core/saved-query.js'; +import { cloneJson, defineJsonField, readJsonField } from '../../core/saved-query.js'; import { deriveFlowPlacement } from './flow-layout.js'; import type { DashboardLayoutPlugin } from './flow-layout.js'; import type { @@ -182,7 +182,7 @@ function gridItemsHost(layout: unknown): Record | null { * No-op when the layout is not an object. */ export function setGridPlacement(layout: unknown, tileId: string, placement: unknown): void { const items = gridItemsHost(layout); - if (items) items[tileId] = placement; + if (items) defineJsonField(items, tileId, placement); } /** One tile's STORED grid placement, or `undefined` when the layout holds none @@ -192,12 +192,12 @@ export function setGridPlacement(layout: unknown, tileId: string, placement: unk * placements where. Never mutates. */ export function gridPlacementAt(layout: unknown, tileId: string): unknown { if (!isObject(layout) || !isObject(layout.items)) return undefined; - return layout.items[tileId]; + return readJsonField(layout.items, tileId); } function tileStylesAt(layout: unknown, tileId: string): GridTileStyles { if (!isObject(layout) || layout.version !== 2 || !isObject(layout.items)) return {}; - const entry = layout.items[tileId]; + const entry = readJsonField(layout.items, tileId); return isObject(entry) ? entry as GridTileStyles : {}; } @@ -233,7 +233,14 @@ export function setStylePlacement( if (!isObject(layout) || layout.version !== 2) return; if (!isObject(layout.items)) layout.items = {}; const items = layout.items as Record; - const current = isObject(items[tileId]) ? items[tileId] as Record : {}; + // Own-property-only read (readJsonField), then a FRESH shallow copy — never + // the stored/inherited value mutated in place. For tileId === '__proto__' + // with no own entry yet, a bare `items[tileId]` would resolve through the + // prototype chain to Object.prototype itself; writing `current[style] = …` + // on THAT reference would corrupt Object.prototype for the whole realm, + // not just this layout (#551 review — caught by this file's own test). + const owned = readJsonField(items, tileId); + const current: Record = isObject(owned) ? { ...owned } : {}; const candidate = isObject(placement) ? placement : {}; const next: Record = {}; if (style === 'grid' && Object.prototype.hasOwnProperty.call(candidate, 'span')) { @@ -243,7 +250,7 @@ export function setStylePlacement( next.height = candidate.height; } current[style] = next; - items[tileId] = current; + defineJsonField(items, tileId, current); } export function stylePlacementAt( @@ -509,7 +516,7 @@ export function computeGrafanaGridLayout(input: ComputeGrafanaGridLayoutInput): const placement = isObject(layout) && layout.version === 2 ? resolveStylePlacement(layout, tile.id, style === 'full' || style === 'report' ? style : 'grid') - : resolveGridPlacement(items[tile.id]); + : resolveGridPlacement(readJsonField(items, tile.id)); const authoredSpan = style === 'full' ? columns : style === 'report' ? REPORT_GRID_SPAN : temporary ? (previewSpans?.get(tile.id) ?? 1) : placement.span; @@ -561,10 +568,10 @@ export function deriveFlowFallback( const items = gridItemsFor(gridLayout); const flowItems: Record = {}; for (const tile of tiles) { - const gridPlacement = resolveGridPlacement(items[tile.id]); - flowItems[tile.id] = { + const gridPlacement = resolveGridPlacement(readJsonField(items, tile.id)); + defineJsonField(flowItems, tile.id, { span: flowSpanFromGridSpan(gridPlacement.span), height: gridHeightUnitsToFlowHeight(gridPlacement.height), - }; + }); } return { type: 'flow', version: 1, preset: 'columns-2', items: flowItems }; } @@ -579,14 +586,14 @@ export function deriveAuthoredFlowFallback( const flowItems: Record = {}; for (const tile of tiles) { const placement = resolveStylePlacement(layout, tile.id, preset); - flowItems[tile.id] = preset === 'grid' + defineJsonField(flowItems, tile.id, preset === 'grid' ? { span: flowSpanFromGridSpan(placement.span), height: gridHeightUnitsToFlowHeight(placement.height), } : preset === 'full' ? { span: 2, height: gridHeightUnitsToFlowHeight(placement.height) } - : { span: 1, height: gridHeightUnitsToFlowHeight(placement.height) }; + : { span: 1, height: gridHeightUnitsToFlowHeight(placement.height) }); } return { type: 'flow', diff --git a/src/dashboard/model/dashboard-document.ts b/src/dashboard/model/dashboard-document.ts index eb4373d9..08b1ddd5 100644 --- a/src/dashboard/model/dashboard-document.ts +++ b/src/dashboard/model/dashboard-document.ts @@ -10,7 +10,7 @@ // // Pure — no DOM, no persistence, no clock. -import { cloneJson } from '../../core/saved-query.js'; +import { cloneJson, defineJsonField, readJsonField } from '../../core/saved-query.js'; import type { DashboardDocumentV1, DashboardDocumentV2 } from '../../generated/json-schema.types.js'; const isObject = (value: unknown): value is Record => @@ -48,7 +48,8 @@ function authoredPlacement( layout: Record, tileId: string, style: 'grid' | 'full' | 'report', ): { span: number; height: number } { const items = isObject(layout.items) ? layout.items : {}; - const entry = isObject(items[tileId]) ? items[tileId] : {}; + const ownEntry = readJsonField(items, tileId); + const entry = isObject(ownEntry) ? ownEntry : {}; const placement = isObject(entry[style]) ? entry[style] as Record : {}; if (style === 'grid') { return { @@ -67,11 +68,11 @@ function regenerateFallback(layout: Record, tileIds: readonly s const items: Record = {}; for (const tileId of tileIds) { const placement = authoredPlacement(layout, tileId, preset); - items[tileId] = preset === 'grid' + defineJsonField(items, tileId, preset === 'grid' ? { span: gridToFlowSpan(placement.span), height: flowHeight(placement.height) } : preset === 'full' ? { span: 2, height: flowHeight(placement.height) } - : { span: 1, height: flowHeight(placement.height) }; + : { span: 1, height: flowHeight(placement.height) }); } layout.fallback = { type: 'flow', @@ -102,12 +103,12 @@ export function upgradeDashboardLayout = {}; if (Object.hasOwn(old, 'span')) grid.span = old.span; if (Object.hasOwn(old, 'height')) grid.height = gridHeight(old.height); - items[tileId] = { grid }; + defineJsonField(items, tileId, { grid }); } next.layout = { type: 'grafana-grid', version: 2, preset: 'grid', items, @@ -116,24 +117,24 @@ export function upgradeDashboardLayout { expect(taken.ok).toBe(false); if (!taken.ok) expect(taken.diagnostics[0].code).toBe('dashboard-command-tile-id-taken'); }); + + // #551 — the new tile id itself may be '__proto__' or 'constructor' + // (schema-legal, caller-minted): the copy's placement must survive as an + // own property under every engine this command writes through, not just + // avoid throwing. + for (const newTileId of ['__proto__', 'constructor']) { + it(`copies the placement to a new tile id ${JSON.stringify(newTileId)} under flow`, () => { + const result = run(seeded(), { ...dup, newTileId }, [query('q'), query('q-copy')]); + expect(result.ok).toBe(true); + if (!result.ok) return; + const items = result.dashboard.layout.items as Record; + expect(Object.hasOwn(items, newTileId)).toBe(true); + expect(items[newTileId]).toEqual({ span: 2, height: 'large' }); + expect(JSON.parse(JSON.stringify(result.dashboard)).layout.items[newTileId]).toEqual({ span: 2, height: 'large' }); + }); + + it(`copies the placement to a new tile id ${JSON.stringify(newTileId)} under grafana-grid@1`, () => { + const gridDoc = draft({ + tiles: [source] as never, + layout: { type: 'grafana-grid', version: 1, items: { b: { span: 8, height: 5 } } } as never, + }); + const result = run(gridDoc, { ...dup, newTileId }, [query('q'), query('q-copy')], grafanaGridLayoutPlugin); + expect(result.ok).toBe(true); + if (!result.ok) return; + const items = result.dashboard.layout.items as Record; + expect(Object.hasOwn(items, newTileId)).toBe(true); + expect(items[newTileId]).toEqual({ span: 8, height: 5 }); + }); + + // The grafana-grid@2 style-map write is its own dedicated branch in + // applyCommandToClone (duplicate-tile), separate from + // setPlacementForActiveEngine — it must be exercised directly. + it(`copies the placement to a new tile id ${JSON.stringify(newTileId)} under grafana-grid@2 style items`, () => { + const gridV2Doc = draft({ + tiles: [source] as never, + layout: { + type: 'grafana-grid', version: 2, preset: 'grid', + items: { b: { grid: { span: 8, height: 5 } } }, + } as never, + }); + const result = run(gridV2Doc, { ...dup, newTileId }, [query('q'), query('q-copy')], grafanaGridLayoutV2Plugin); + expect(result.ok).toBe(true); + if (!result.ok) return; + const items = result.dashboard.layout.items as Record; + expect(Object.hasOwn(items, newTileId)).toBe(true); + expect(items[newTileId]).toEqual({ grid: { span: 8, height: 5 } }); + expect(JSON.parse(JSON.stringify(result.dashboard)).layout.items[newTileId]).toEqual({ grid: { span: 8, height: 5 } }); + }); + } }); describe('applyCommand — update-tile', () => { @@ -234,6 +283,21 @@ describe('applyCommand — update-placement / change-layout', () => { if (!missing.ok) expect(missing.diagnostics[0].code).toBe('dashboard-command-tile-missing'); }); + // #551 — a tile whose id is '__proto__' or 'constructor' (schema-legal) + // must keep an update-placement write as an own property. + for (const tileId of ['__proto__', 'constructor']) { + it(`sets a valid placement for tile id ${JSON.stringify(tileId)}`, () => { + const protoSeeded = draft({ tiles: [{ id: tileId, queryId: 'q' }] as never }); + const ok = run(protoSeeded, { type: 'update-placement', tileId, style: 'grid', placement: { span: 2 } }, [query('q')]); + expect(ok.ok).toBe(true); + if (!ok.ok) return; + const items = ok.dashboard.layout.items as Record; + expect(Object.hasOwn(items, tileId)).toBe(true); + expect(items[tileId]).toEqual({ span: 2 }); + expect(JSON.parse(JSON.stringify(ok.dashboard)).layout.items[tileId]).toEqual({ span: 2 }); + }); + } + it('installs a new layout document', () => { const layout = { type: 'flow', version: 1, preset: 'columns-2', items: {} } as never; const result = run(seeded(), { type: 'change-layout', layout }, [query('q')]); @@ -336,6 +400,26 @@ describe('applyCommand — change-layout engine switch (#291 owner decision 3)', expect(result.dashboard.layout.fallback).toEqual(flowLayout); }); + // #551 — the flow -> grid conversion above writes `gridItems[tile.id] = …`; + // a tile id of '__proto__' or 'constructor' (schema-legal) must survive it + // as an own property with the correct converted span/height. + for (const tileId of ['__proto__', 'constructor']) { + it(`flow -> grafana-grid preserves the converted placement for tile id ${JSON.stringify(tileId)}`, () => { + const flowLayout = { + type: 'flow', version: 1, preset: 'columns-2', + items: { [tileId]: { span: 2, height: 'large' } }, + }; + const d = draft({ tiles: [{ id: tileId, queryId: 'q' }] as never, layout: flowLayout as never }); + const result = run(d, { type: 'change-layout', layout: { type: 'grafana-grid', version: 1 } as never }, [query('q')]); + expect(result.ok).toBe(true); + if (!result.ok) return; + const items = result.dashboard.layout.items as Record; + expect(Object.hasOwn(items, tileId)).toBe(true); + expect(items[tileId]).toEqual({ span: 6, height: 3 }); + expect(JSON.parse(JSON.stringify(result.dashboard)).layout.items[tileId]).toEqual({ span: 6, height: 3 }); + }); + } + it('grafana-grid -> flow restores the fallback verbatim, dropping the fallback field itself', () => { const fallbackLayout = { type: 'flow', version: 1, preset: 'report', items: { t1: { span: 2, height: 'large' } } }; const grid = { type: 'grafana-grid', version: 1, items: { t1: { span: 6, height: 'large' } }, fallback: fallbackLayout }; diff --git a/tests/unit/dashboard-document.test.ts b/tests/unit/dashboard-document.test.ts index acabc471..6b1b494d 100644 --- a/tests/unit/dashboard-document.test.ts +++ b/tests/unit/dashboard-document.test.ts @@ -107,6 +107,81 @@ describe('upgradeDashboardLayout', () => { }); }); +describe('upgradeDashboardLayout — #551 __proto__/constructor tile ids', () => { + // A tile id of '__proto__' or 'constructor' is schema-legal + // (`dashboardTileV1.id` pattern `\S`); every placement-map write this + // migration performs must keep it as an own property with its exact + // span/height, never silently dropping it via the inherited + // Object.prototype setter. + for (const tileId of ['__proto__', 'constructor']) { + const protoTiles = [{ id: tileId, queryId: 'qa' }, { id: 'b', queryId: 'qb' }]; + + it(`preserves grafana-grid@1 -> @2 placement and its flow fallback for tile id ${JSON.stringify(tileId)}`, () => { + const source = { + documentVersion: 2, id: 'd', title: 'D', revision: 1, tiles: protoTiles, + layout: { + type: 'grafana-grid', version: 1, + items: { [tileId]: { span: 4, height: 'compact' } }, + }, + } as DashboardDocumentV2; + const result = upgradeDashboardLayout(source); + const items = result.layout.items as Record; + expect(Object.hasOwn(items, tileId)).toBe(true); + expect(items[tileId]).toEqual({ grid: { span: 4, height: 1 } }); + const fallbackItems = (result.layout.fallback as { items: Record }).items; + expect(Object.hasOwn(fallbackItems, tileId)).toBe(true); + expect(fallbackItems[tileId]).toEqual({ span: 1, height: 'compact' }); + expect(JSON.parse(JSON.stringify(result)).layout.items[tileId]).toEqual({ grid: { span: 4, height: 1 } }); + }); + + it(`preserves flow@1 report -> grafana-grid@2 report placement for tile id ${JSON.stringify(tileId)}`, () => { + const source = { + documentVersion: 2, id: 'd', title: 'D', revision: 1, tiles: protoTiles, + layout: { + type: 'flow', version: 1, preset: 'report', + items: { [tileId]: { height: 'large' } }, + }, + } as DashboardDocumentV2; + const result = upgradeDashboardLayout(source); + const items = result.layout.items as Record; + expect(Object.hasOwn(items, tileId)).toBe(true); + expect(items[tileId]).toEqual({ report: { height: 3 } }); + const fallbackItems = (result.layout.fallback as { items: Record }).items; + expect(Object.hasOwn(fallbackItems, tileId)).toBe(true); + expect(fallbackItems[tileId]).toEqual({ span: 1, height: 'large' }); + }); + + it(`preserves flow@1 columns -> grafana-grid@2 grid placement for tile id ${JSON.stringify(tileId)}`, () => { + const source = { + documentVersion: 2, id: 'd', title: 'D', revision: 1, tiles: protoTiles, + layout: { + type: 'flow', version: 1, preset: 'columns-3', + items: { [tileId]: { span: 2, height: 'compact' } }, + }, + } as DashboardDocumentV2; + const result = upgradeDashboardLayout(source); + const items = result.layout.items as Record; + expect(Object.hasOwn(items, tileId)).toBe(true); + expect(items[tileId]).toEqual({ grid: { span: 6, height: 1 } }); + }); + + it(`regenerates a v2 fallback that keeps tile id ${JSON.stringify(tileId)}`, () => { + const source = { + documentVersion: 2, id: 'd', title: 'D', revision: 1, tiles: protoTiles, + layout: { + type: 'grafana-grid', version: 2, preset: 'grid', + items: { [tileId]: { grid: { span: 9, height: 3 } } }, + }, + } as DashboardDocumentV2; + const result = upgradeDashboardLayout(source); + const fallbackItems = (result.layout.fallback as { items: Record }).items; + expect(Object.hasOwn(fallbackItems, tileId)).toBe(true); + expect(fallbackItems[tileId]).toEqual({ span: 3, height: 'large' }); + expect(JSON.parse(JSON.stringify(result)).layout.fallback.items[tileId]).toEqual({ span: 3, height: 'large' }); + }); + } +}); + describe('dropCuratedFilters', () => { it('drops filters while applying the same deterministic layout upgrade', () => { const source = { diff --git a/tests/unit/flow-layout.test.ts b/tests/unit/flow-layout.test.ts index 1dbc7fa4..44ea9739 100644 --- a/tests/unit/flow-layout.test.ts +++ b/tests/unit/flow-layout.test.ts @@ -57,6 +57,19 @@ describe('setFlowPlacement', () => { // A non-object layout is tolerated too. expect(() => setFlowPlacement(null, 't1', { span: 1 })).not.toThrow(); }); + + // #551 — a tile whose id is '__proto__' or 'constructor' (both schema-legal) + // must keep its flow placement as an own property, asserted with the exact + // surviving span/height, not merely a no-throw. + it('preserves a placement for tile ids __proto__ and constructor', () => { + for (const tileId of ['__proto__', 'constructor']) { + const layout = flowLayout(); + setFlowPlacement(layout, tileId, { span: 3, height: 'large' }); + expect(Object.hasOwn(layout.items, tileId)).toBe(true); + expect(layout.items[tileId]).toEqual({ span: 3, height: 'large' }); + expect(JSON.parse(JSON.stringify(layout)).items[tileId]).toEqual({ span: 3, height: 'large' }); + } + }); }); describe('flowPlacementAt (#535)', () => { diff --git a/tests/unit/grafana-grid-layout.test.ts b/tests/unit/grafana-grid-layout.test.ts index 0d07c50f..ee13b766 100644 --- a/tests/unit/grafana-grid-layout.test.ts +++ b/tests/unit/grafana-grid-layout.test.ts @@ -85,6 +85,56 @@ describe('grafana-grid@2 plugin', () => { items: { a: { span: 1, height: 'compact' } }, }); }); + + // #551 — setStylePlacement's v2 `items` map is the same tile-id-keyed write + // surface; __proto__/constructor must survive here too. + it('preserves a v2 style placement for tile ids __proto__ and constructor without polluting Object.prototype', () => { + for (const tileId of ['__proto__', 'constructor']) { + const layout: Record = { type: 'grafana-grid', version: 2 }; + // FIRST write for this tile id — no own `items[tileId]` entry exists yet, + // exactly the case where a bare `items[tileId]` read resolves through + // the prototype chain to Object.prototype/Object.prototype.constructor + // itself. `setStylePlacement` then merges INTO that read value + // (`current[style] = next`) before writing it back — if the read ever + // aliases the real prototype instead of copying it, this line mutates + // Object.prototype for the whole process, not just this `layout`. + setStylePlacement(layout, tileId, 'grid', { span: 9, height: 3 }); + const items = layout.items as Record; + expect(Object.hasOwn(items, tileId)).toBe(true); + expect(items[tileId]).toEqual({ grid: { span: 9, height: 3 } }); + expect(JSON.parse(JSON.stringify(layout)).items[tileId]).toEqual({ grid: { span: 9, height: 3 } }); + expect(resolveStylePlacement(layout, tileId, 'grid')).toEqual({ span: 9, height: 3 }); + expect(Object.hasOwn(Object.prototype, 'grid')).toBe(false); + expect((Object.prototype as Record).grid).toBeUndefined(); + // A brand-new, unrelated plain object must not see the placement. + expect(({} as Record).grid).toBeUndefined(); + } + }); + + // #551 — deriveAuthoredFlowFallback (v2 regeneration) writes flowItems + // keyed by tile id; the legacy deriveFlowFallback path (routed above) shares + // the same risk. Both are the v2 "regenerate fallback" acceptance case. + it('preserves __proto__/constructor tile ids through deriveAuthoredFlowFallback (v2 fallback regeneration)', () => { + for (const tileId of ['__proto__', 'constructor']) { + const layout: Record = { + type: 'grafana-grid', version: 2, preset: 'grid', + items: { [tileId]: { grid: { span: 4, height: 1 } } }, + }; + const fallback = deriveAuthoredFlowFallback(layout, [{ id: tileId }]); + expect(Object.hasOwn(fallback.items, tileId)).toBe(true); + expect(fallback.items[tileId]).toEqual({ span: 1, height: 'compact' }); + expect(JSON.parse(JSON.stringify(fallback)).items[tileId]).toEqual({ span: 1, height: 'compact' }); + } + }); + + it('preserves __proto__/constructor tile ids through deriveFlowFallback (grafana-grid@1 fallback derivation)', () => { + for (const tileId of ['__proto__', 'constructor']) { + const fallback = deriveFlowFallback(gridLayout({ [tileId]: { span: 4, height: 1 } }), [{ id: tileId }]); + expect(Object.hasOwn(fallback.items, tileId)).toBe(true); + expect(fallback.items[tileId]).toEqual({ span: 1, height: 'compact' }); + expect(JSON.parse(JSON.stringify(fallback)).items[tileId]).toEqual({ span: 1, height: 'compact' }); + } + }); }); describe('gridSpanFromFlowSpan / flowSpanFromGridSpan', () => { @@ -224,6 +274,21 @@ describe('setGridPlacement', () => { expect(() => setGridPlacement(null, 't1', { span: 1 })).not.toThrow(); expect(() => setGridPlacement(5, 't1', { span: 1 })).not.toThrow(); }); + + // #551 — a tile whose id is '__proto__' or 'constructor' (both schema-legal, + // `dashboardTileV1.id` pattern `\S`) must keep its placement as an own + // property, not silently vanish through the inherited Object.prototype + // setter. Assert the placement SURVIVES with the right span/height, not + // merely that the call doesn't throw. + it('preserves a placement for tile ids __proto__ and constructor', () => { + for (const tileId of ['__proto__', 'constructor']) { + const layout = gridLayout(); + setGridPlacement(layout, tileId, { span: 8, height: 5 }); + expect(Object.hasOwn(layout.items, tileId)).toBe(true); + expect(layout.items[tileId]).toEqual({ span: 8, height: 5 }); + expect(JSON.parse(JSON.stringify(layout)).items[tileId]).toEqual({ span: 8, height: 5 }); + } + }); }); describe('gridPlacementAt (#535)', () => { diff --git a/tests/unit/portable-bundle-codec.test.ts b/tests/unit/portable-bundle-codec.test.ts index 6de39c53..a129dee5 100644 --- a/tests/unit/portable-bundle-codec.test.ts +++ b/tests/unit/portable-bundle-codec.test.ts @@ -176,6 +176,40 @@ describe('encodePortableBundleJson', () => { expect(!result.ok && has(result.diagnostics, 'workspace-duplicate-query-id')).toBe(true); }); + // #551 — a tile whose id is '__proto__' or 'constructor' (schema-legal) + // must keep its placement through a portable-bundle encode -> decode round + // trip: both encode and decode run every Dashboard through + // upgradeDashboardLayout (flow@1 -> grafana-grid@2), which writes the + // placement-map keyed by tile id. + for (const tileId of ['__proto__', 'constructor']) { + it(`round-trips the placement of a tile id ${JSON.stringify(tileId)} through encode then decode`, () => { + const dashboards = [dashboardDoc({ + tiles: [{ id: tileId, queryId: 'p1' }], + layout: { type: 'flow', version: 1, preset: 'columns-2', items: { [tileId]: { span: 2, height: 'large' } } }, + })]; + const encoded = encodePortableBundleJson({ + queries: [panelQuery('p1')], dashboards, nowISO: '2026-07-17T00:00:00.000Z', + }); + expect(encoded.ok).toBe(true); + if (!encoded.ok) return; + const encodedLayout = JSON.parse(encoded.value).dashboards[0].layout; + expect(Object.hasOwn(encodedLayout.items, tileId)).toBe(true); + expect(encodedLayout.items[tileId]).toEqual({ grid: { span: 6, height: 3 } }); + + const decoded = decodePortableBundleJson(encoded.value); + expect(decoded.ok).toBe(true); + if (!decoded.ok) return; + const layout = decoded.value.dashboards[0].layout as unknown as { + items: Record; + fallback: { items: Record }; + }; + expect(Object.hasOwn(layout.items, tileId)).toBe(true); + expect(layout.items[tileId]).toEqual({ grid: { span: 6, height: 3 } }); + expect(Object.hasOwn(layout.fallback.items, tileId)).toBe(true); + expect(layout.fallback.items[tileId]).toEqual({ span: 2, height: 'large' }); + }); + } + it('rejects an encoded document larger than the decoded-JSON byte cap', () => { // An arbitrary extension field (query-spec is open) inflates each spec to just // under the 1 MiB per-spec cap; twenty-one sum past the 20 MiB whole-document diff --git a/tests/unit/saved-query.test.ts b/tests/unit/saved-query.test.ts index b1adf8ba..96f177b9 100644 --- a/tests/unit/saved-query.test.ts +++ b/tests/unit/saved-query.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; import { - SPEC_VERSION, cloneJson, queryName, queryDescription, queryFavorite, queryView, + SPEC_VERSION, cloneJson, defineJsonField, queryName, queryDescription, queryFavorite, queryView, queryPanel, queryDashboard, withQuerySpec, patchQuerySpec, patchQueryPanel, patchQueryDashboard, upgradeV1Query, cloneV2Query, upgradeSavedQuery, queryContentKey, isPlainObject, } from '../../src/core/saved-query.js'; @@ -47,6 +47,25 @@ describe('saved-query model', () => { expect(queryContentKey(v2(source))).toContain('__proto__'); }); + // #551 — the general safe-key-write primitive every Dashboard placement-map + // write reuses. A bare `target[key] = value` on a plain object invokes the + // inherited Object.prototype.__proto__ setter for exactly this key, so the + // write silently vanishes; defineJsonField must always create an own, + // enumerable, JSON.stringify-visible property instead — for both + // '__proto__' and 'constructor' (also inherited, also schema-legal as a + // Dashboard tile id: `pattern: "\\S"`) — without touching Object.prototype. + it('defineJsonField writes __proto__/constructor as own enumerable data, never touching Object.prototype', () => { + for (const key of ['__proto__', 'constructor']) { + const target: Record = {}; + defineJsonField(target, key, { span: 6, height: 2 }); + expect(Object.hasOwn(target, key)).toBe(true); + expect(target[key]).toEqual({ span: 6, height: 2 }); + expect(JSON.parse(JSON.stringify(target))[key]).toEqual({ span: 6, height: 2 }); + } + expect((Object.prototype as Record).span).toBeUndefined(); + expect(Object.getPrototypeOf({})).toBe(Object.prototype); + }); + it('reads known fields with safe defaults without stripping extensions', () => { const panel = { cfg: { type: 'table' }, links: [{ url: '/x' }] }; const dashboard: QueryDashboardPresentationV1 = { role: 'panel', future: { x: 1 } }; diff --git a/tests/unit/stored-workspace.test.ts b/tests/unit/stored-workspace.test.ts index 9b137b60..172085a3 100644 --- a/tests/unit/stored-workspace.test.ts +++ b/tests/unit/stored-workspace.test.ts @@ -404,6 +404,39 @@ describe('decodeStoredWorkspaceJson', () => { expect(!invalid.ok && invalid.diagnostics[0].code).toBe('workspace-version-unsupported'); expect(!invalid.ok && invalid.diagnostics).toHaveLength(1); }); + + // #551 — a tile whose id is '__proto__' or 'constructor' (schema-legal) + // must keep its placement through a full encode -> decode round trip: text + // -> JSON.parse -> canonicalizeWorkspaceLayouts (upgradeDashboardLayout, + // flow@1 -> grafana-grid@2) -> the canonical V5 value. + for (const tileId of ['__proto__', 'constructor']) { + it(`round-trips the placement of a tile id ${JSON.stringify(tileId)} through decode (flow@1 -> grafana-grid@2)`, () => { + const source = workspace({ + queries: [panelQuery('p1')], + dashboards: [dashboardDoc({ + tiles: [{ id: tileId, queryId: 'p1' }], + layout: { type: 'flow', version: 1, preset: 'columns-2', items: { [tileId]: { span: 2, height: 'large' } } }, + })], + }); + const decoded = decodeStoredWorkspaceJson(JSON.stringify(source)); + expect(decoded.ok).toBe(true); + if (!decoded.ok) return; + const layout = decoded.value.dashboards[0].layout as unknown as { + items: Record; + fallback: { items: Record }; + }; + expect(Object.hasOwn(layout.items, tileId)).toBe(true); + expect(layout.items[tileId]).toEqual({ grid: { span: 6, height: 3 } }); + expect(Object.hasOwn(layout.fallback.items, tileId)).toBe(true); + expect(layout.fallback.items[tileId]).toEqual({ span: 2, height: 'large' }); + // Re-encoding the canonical value must keep the placement too. + const encoded = encodeStoredWorkspaceJson(decoded.value); + expect(encoded.ok).toBe(true); + if (!encoded.ok) return; + const reparsedLayout = JSON.parse(encoded.value).dashboards[0].layout; + expect(reparsedLayout.items[tileId]).toEqual({ grid: { span: 6, height: 3 } }); + }); + } }); describe('encodeStoredWorkspaceJson', () => {