fix(#551): keep __proto__/constructor tile IDs' placements, and stop a real Object.prototype leak

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}`,

src/core/saved-query.ts

@@ -30,12 +30,55 @@ export function isPlainObject(value: unknown): value is Record<string, unknown>
   return !!value && typeof value === 'object' && !Array.isArray(value);
 }
 
-function defineJsonField(target: Record<string, unknown>, 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<string, unknown>, 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<string, unknown>, 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

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<string, unknown>)[newTileId] = cloneJson(placement);
+          defineJsonField(dashboard.layout.items as Record<string, unknown>, 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<string, unknown> = {};
         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

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<string, unknown> | 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),

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<string, unknown> | 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<string, unknown>;
-  const current = isObject(items[tileId]) ? items[tileId] as Record<string, unknown> : {};
+  // 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<string, unknown> = isObject(owned) ? { ...owned } : {};
   const candidate = isObject(placement) ? placement : {};
   const next: Record<string, unknown> = {};
   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<string, FlowTilePlacementV1> = {};
   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<string, FlowTilePlacementV1> = {};
   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',

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<string, unknown> =>
@@ -48,7 +48,8 @@ function authoredPlacement(
   layout: Record<string, unknown>, 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<string, unknown> : {};
   if (style === 'grid') {
     return {
@@ -67,11 +68,11 @@ function regenerateFallback(layout: Record<string, unknown>, tileIds: readonly s
   const items: Record<string, unknown> = {};
   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<T extends DashboardDocumentV1 | Dashboard
   if (layout.type === 'grafana-grid' && layout.version === 1) {
     const oldItems = isObject(layout.items) ? layout.items : {};
     for (const tileId of tileIds) {
-      const old = oldItems[tileId];
+      const old = readJsonField(oldItems, tileId);
       if (!isObject(old)) continue;
       const grid: Record<string, unknown> = {};
       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<T extends DashboardDocumentV1 | Dashboard
     const oldItems = isObject(layout.items) ? layout.items : {};
     if (layout.preset === 'report') {
       for (const tileId of tileIds) {
-        const old = oldItems[tileId];
+        const old = readJsonField(oldItems, tileId);
         if (!isObject(old) || !Object.hasOwn(old, 'height')) continue;
-        items[tileId] = {
+        defineJsonField(items, tileId, {
           report: { height: gridHeight(old.height) },
-        };
+        });
       }
       next.layout = {
         type: 'grafana-grid', version: 2, preset: 'report', items,
       } as never;
     } else {
       for (const tileId of tileIds) {
-        const old = resolvedFlowPlacement(oldItems[tileId]);
-        items[tileId] = {
+        const old = resolvedFlowPlacement(readJsonField(oldItems, tileId));
+        defineJsonField(items, tileId, {
           grid: {
             span: flowToGridSpan(old.span),
             height: gridHeight(old.height),
           },
-        };
+        });
       }
       next.layout = {
         type: 'grafana-grid', version: 2, preset: 'grid', items,