Skip to content

canReadReferent() refuses a missing parent before the owner reaches assertParentExists(), so the 400/409 a caller-named parentId owes is unreachable through a scope #276

Closed

Description

@cuibonobo

Motivation

assertParentExists() (packages/core/src/stack.ts:1226) answers the two checks a
caller-named parentId owes — format, then existence — and its own doc comment says who it
is there for:

Existence closes the gap between the owner path and a non-owner's, where
canReadReferent() already refuses a parent that isn't there.

That gap is not closed. canReadReferent() (packages/core/src/scoped-stack.ts:608) resolves
the destination and refuses a missing one before either check runs, and it refuses the owner
along with everyone else:

private async canReadReferent(recordId: string): Promise<boolean> {
  const record = await this.stack.get(recordId);
  if (!record) return false;   // ← the owner lands here too
  return this.canRead(record);
}

So the owner never reaches the owner path. Through an unscoped Stack the checks answer as
designed; through any ScopedStack — which is every request a server serves — both collapse
into StackPermissionError. Probed against @haverstack/adapter-local, authenticated as the
stack owner acting alone:

parentId=""              Stack: StackQueryError    "Invalid parentId \"\": expected 12 lowercase Crockford base-32 characters."
                          wire: 403 permission

parentId="1hk153xffffz"  Stack: StackConflictError "Cannot parent record ... no such record. A container has to exist when it is named."
                          wire: 403 permission

Both reference sites are affected, so this is not specific to the change set:

parentId=""              POST /records → 403   PATCH /records/:id → 403
parentId="1hk153xffffz"  POST /records → 403   PATCH /records/:id → 403

ScopedStack.create() gates at scoped-stack.ts:811 and Stack.create() calls
assertParentExists() at stack.ts:892; ScopedStack.mutate() gates at scoped-stack.ts:988
and Stack.mutate() calls it at stack.ts:1107. In both pairs the gate wins.

The result is that assertParentExists()'s StackConflictError is dead code for every
caller who goes through a scope, and validateParentId()'s StackQueryError with it. The
only way to observe either is to hold an unscoped Stack.

Why this is the gate's bug and not the fixtures'

The owner is documented as exempt from this gate, and the suite says so in two places:

  • tests/scoped-stack.test.ts:3180 — "the owner is exempt from parentId and relationship gates"
  • tests/scoped-stack.test.ts:897 — "the owner is exempt from the parentId gate too"

Both exercise a destination that exists and is unreadable, where the exemption works —
canRead() short-circuits for the owner. Neither covers a destination that is not there,
which is where the exemption silently stops applying: the exemption is keyed on readability,
and a record that does not exist is unreadable by everyone. So the gate is not exempting the
owner from the gate, it is exempting them from one of its two reasons to refuse.

@haverstack/conformance-fixtures@0.23.0 encodes the intended answers, with the exact
messages Stack already produces:

  • error-bad-request-malformed-parent-id → 400 bad_request,
    Invalid parentId "": expected 12 lowercase Crockford base-32 characters.
  • error-conflict-parent-does-not-exist → 409 conflict,
    Cannot parent record "..." to "...": no such record. A container has to exist when it is named.

The fixtures and assertParentExists() agree with each other; only the gate disagrees, which
is what makes this look like an ordering bug rather than a deliberate stance.

Proposal

Exempt the owner acting alone in canReadReferent(), before the record is resolved:

private async canReadReferent(recordId: string): Promise<boolean> {
  if (this.ownerActingAlone) return true;
  const record = await this.stack.get(recordId);
  if (!record) return false;
  return this.canRead(record);
}

ownerActingAlone is already the predicate for this tier (scoped-stack.ts:295, and the one
checkCreateGrant() uses at :505). Returning true here does not permit anything: it hands
the question to Stack, which then answers with the conflict that names the problem instead
of a permission refusal standing in for it. Nothing leaks, because there is no record in their
own stack the owner may not read.

The anti-oracle property is untouched for everyone else. canReadReferent()'s contract —
"Missing and unreadable both return false — indistinguishable, so this can't probe for a
record's existence" — is what stops the gate being an existence oracle, and it still holds for
every requester who is not the owner. The doc comment wants a sentence about the exemption and
why it costs nothing.

What this also fixes

With the gate exempting the owner, two things follow for free:

  1. The reference server can answer both fixtures. They are skipped in
    Adopt core 0.30's change-set envelope for PATCH /records/:id server#123 with this issue as the reason — nothing a server can do fixes it from
    its side, since pre-checking existence in the route would be exactly the oracle this gate
    is right to refuse for non-owners.
  2. validateParentId()'s format error becomes reachable through a scope, so the empty string
    stops reading as a permission problem. It is currently indistinguishable from "you may not
    name that container", which is the least useful of the three things it could mean.

Open questions

  1. Does the exemption belong in canReadReferent() or at its four call sites? The
    function is shared by the parentId gates (:811, :988), relationship targets (:735)
    and restoreVersion()'s snapshot parentId (:1233). Exempting inside it changes all
    four, which I think is right — the exemption is a fact about the owner, not about
    parentId — but it does mean the owner naming a nonexistent relationship target stops
    being refused by this gate. Worth deciding whether Stack should then refuse that one, or
    whether a dangling target is as legal at the front door as docs/spec/data-model.md says a
    dangling parent is at rest.
  2. Should a non-owner write-holder get the 409 too? They can already tell a container
    exists by naming it successfully, so the oracle argument is narrower than it looks for a
    requester who holds write on the record being moved. Keeping them on 403 is the
    conservative call and what I would ship first.
  3. Is restoreVersion() affected in the same way? It is deliberately exempt from
    assertParentExists() (stack.ts:1463), so there is no conflict for the gate to be
    hiding — but :1233 still refuses the owner a snapshot whose container was hard-deleted
    since, which is the case that comment says a restore should survive.

Not in scope

Spec

docs/spec/data-model.md § Reparenting states the format-then-existence order and that a
caller cannot mint a dangling parent. Neither promise currently holds through a scope, so it
is worth a line on which requester observes which, once the tiers are settled.

docs/spec/access-control.md § Composing a change set describes the per-key gates; the
owner's exemption from the reference gate is the kind of thing that belongs beside it rather
than only in a code comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions