Skip to content

POST /attachments bypasses the permission layer: any token holder gets ungated blob storage #45

Description

@cuibonobo

Server design-review finding.

Problem

POST /attachments requires only a valid token, then writes straight to the blob adapter (src/routes/attachments.ts:16-28):

app.post('/', requireAuth(), async (c) => {
  ...
  const fileId = await adapter.putAttachment(data);

Compare core's own gate: ScopedStack.putAttachment() refuses anonymous requesters and requires a create grant on _attachment@1 before storing bytes (packages/core/src/stack.ts:993-1000 in haverstack/core). The server skips that layer entirely, so an entity with a token and zero grants — someone who cannot create a single record — can still:

  • write arbitrary bytes to the owner's disk, 50 MB per request, unbounded in total (the per-request maxAttachmentBytes cap is the only limit; nothing meters cumulative storage per entity);
  • do so invisibly: bytes with no metadata record don't appear in any query, and until core #64's GC exists they persist forever.

The grant check would bite at step 2 of the spec's two-step flow (creating the _attachment@1 record via POST /records) — but step 2 is optional, and the bytes land at step 1.

Why the fix isn't one line

The wire contract is two-step (POST /attachments stores bytes only, spec §Attachments), but core's scoped surface is one-step (putAttachment = bytes + metadata). There is no scoped bytes-only entry point — putAttachmentBytes exists only on the unscoped Stack. So the server is forced under the trust layer to implement the spec'd endpoint. Options:

  1. Core exposes ScopedStack.putAttachmentBytes() running the same anonymous + create-grant checks (smallest change; keeps the wire contract; my lean). Needs a small core-side addition — flagging for the owner rather than filing cross-repo.
  2. Server duplicates the grant check locally — works today, but it's exactly the route-level policy duplication Core sync: shed route-level policy special cases as core invariants land (core #59, #57, #65, #67) #41 exists to remove.
  3. Respec upload as one-shot (bytes + metadata in one multipart request) — bigger wire change, probably not worth it given the SDK already automates the two steps.

Whichever lands, the upload path should also be what the sync work builds on: core #65's mimeType-conflict validation and core #64's grace-period orphan handling both assume upload is attributable to a requester.

Work items

Refs haverstack/core#51 (reference-implies-access threat model), haverstack/core#64 (orphan cleanup is the backstop, not the fence), #41, #33.

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