Skip to content

Placement maps drop a tile whose ID is __proto__ #551

Description

@BorisTyshkevich

Goal

Build every tile-keyed placement map so a tile whose ID is __proto__ (or
constructor) keeps its placement, instead of silently rendering at the
engine's default size.

Why (found reviewing #549 / PR #549)

The Dashboard schema permits any non-whitespace tile ID up to 256 characters
(schemas/dashboard-v1.schema.jsondashboardTileV1.id: pattern: "\\S"),
which includes __proto__. Layout placement maps are built as ordinary objects
and keyed by that user-controlled ID:

const items = {};
items[tileId] = placement;   // tileId === '__proto__' → NO own property

Verified in node: assigning to items['__proto__'] invokes the inherited
Object.prototype setter, so no own enumerable property is created and
JSON.stringify(items) is {}. The tile's placement is dropped and it falls
back to the engine default. (JSON.parse DOES create __proto__ as an own
property, which is how such an ID survives a round-trip into the app, and
cloneJson in src/core/saved-query.ts already uses Object.defineProperty
specifically to preserve it — so this input class is an acknowledged
requirement, just not honoured here.)

No prototype pollution escapes: only the local map's [[Prototype]] changes,
never Object.prototype.

Correction (PR #558). This claim was wrong, and only about the write
path. The read path does escape: a bare items[tileId] where tileId is
'__proto__' and the map has no own property under that name resolves up the
prototype chain to Object.prototype itself. That is a real object, so an
isObject(...) guard cannot distinguish it from stored data, and
setStylePlacement's merge-then-rewrite (current[style] = next) then assigns
directly onto Object.prototype — reproduced live, polluting
Object.prototype.grid for every other placement map in the realm. The fix
therefore needed an own-property-only read helper (readJsonField) alongside
the write helper, not the write helper alone.

Sites (all currently affected)

  • src/dashboard/layouts/grafana-grid-layout.tsderiveFlowFallback
    (flowItems[tile.id] = …), deriveAuthoredFlowFallback, gridItemsHost /
    setGridPlacement, setStylePlacement
  • src/dashboard/model/dashboard-document.tsregenerateFallback and both
    legacy→v2 conversion branches (items[tileId] = …)
  • src/dashboard/layouts/flow-layout.ts — flow's own items host / placement
    setter (same pattern; pre-dates fix(#535): correct dashboard style sizing after #538 #549)
  • src/dashboard/application/dashboard-commands.tsduplicate-tile's direct
    layout.items[newTileId] = … write

Note this is NOT introduced by #549: deriveFlowFallback on main has the
identical pattern, which is why it was deferred rather than partially patched —
fixing only the two new call sites would leave the defect live and its test
unfalsifiable.

Acceptance

  • One shared helper (e.g. Object.defineProperty-based, mirroring
    defineJsonField in src/core/saved-query.ts) used by every placement-map
    write above; no bare map[tileId] = … on a plain object remains.
  • Round-trip tests with tile IDs __proto__ and constructor through:
    grafana-grid@1@2 migration; flow@1@2 migration; v2 fallback
    regeneration; update-placement; duplicate-tile; encode → decode of both
    the stored workspace and a portable bundle.
  • Each test asserts the placement SURVIVES (own property, correct span/height),
    not merely that nothing throws.

Deferred from PR #549 (review finding 3): low impact (a crafted or imported
bundle only, and the failure is a default-sized tile), pre-existing, and worth
one sweep across every site rather than two partial patches.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinglowlow priority

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions