Skip to content

Server-boundary input hardening: reserved content keys + record-body size ceiling #148

Description

@cuibonobo

From the 2026-08 security pass (a manual audit of crypto, ID/token, SQL/FTS, path, and merge-patch surfaces — no high/critical findings). Two low-severity hardening items that share a theme: core has no defenses at the untrusted-input boundary, which is fine while Stack is embedded/full-trust but becomes relevant the moment the server accepts arbitrary PATCH/POST bodies.

1. Reserved content keys (__proto__ / constructor / prototype)

Undeclared content fields pass validation by design, and applyMergePatch (packages/core/src/merge.ts) does merged[key] = value. A patch key of __proto__ invokes the prototype setter on the merged object rather than setting a field.

This is not a global prototype-pollution gadget — I traced the round-trip and it doesn't reach Object.prototype:

  • On write: JSON.stringify(merged) serializes only own-enumerable properties, so a reassigned prototype is dropped — the __proto__ patch key is silently lost, not stored or propagated.
  • On read: JSON.parse(row.content) creates an own __proto__ property (spec'd CreateDataProperty, bypasses the setter), so a stored {"__proto__": …} round-trips inertly.

So the actual defect is surprising, silent behavior (a merge-patch to __proto__ vanishes; the same key via full create() stores as an own property, so the two paths disagree), not an exploit. But defensive key-filtering at this boundary is cheap insurance against a future refactor turning the shallow merge into something deeper, and against the inconsistency itself.

Direction: reject __proto__, constructor, prototype as top-level content keys (a StackValidationError), or skip them in applyMergePatch with Object.prototype.hasOwnProperty/null-prototype accumulator. Rejecting is more honest than silently skipping. Decide whether this is a core invariant (in Stack.create/update, so every adapter inherits it — consistent with the #67/#68 layering) or a server-only concern.

2. No record-body size ceiling

maxAttachmentBytes covers attachment bytes only. Record content and PATCH bodies have no size limit anywhere in core, and nothing upstream of the adapter's JSON.parse guards one. An unbounded body is a trivial DoS (parse + store + FTS-index a multi-MB blob).

This is standard server request-size-limit territory — but it should be stated, because the spec is otherwise careful about resource bounds (validation depth cap, queryAllPages max, GC grace). Options: a spec sentence in the wire format directing servers to set a request-size limit and map overflow to 413/payload_too_large (reusing the existing code, currently attachment-only); optionally a soft maxContentBytes discovery capability so clients can pre-check like they do for attachments.

Work items

  • Decide core-invariant vs. server-only for the reserved-keys guard; implement + regression test
  • Spec sentence on record-body size limits + 413 reuse for oversized non-attachment bodies
  • (optional) maxContentBytes discovery capability paralleling maxAttachmentBytes

Cross-refs: security pass 2026-08; #67/#68 (invariant layering), the wire error taxonomy (#53) for the 413 reuse.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions