Repository navigation
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
Description
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→ 400bad_request,
Invalid parentId "": expected 12 lowercase Crockford base-32 characters.error-conflict-parent-does-not-exist→ 409conflict,
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:
- 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. 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
- Does the exemption belong in
canReadReferent()or at its four call sites? The
function is shared by theparentIdgates (:811,:988), relationship targets (:735)
andrestoreVersion()'s snapshotparentId(: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 whetherStackshould then refuse that one, or
whether a dangling target is as legal at the front door asdocs/spec/data-model.mdsays a
dangling parent is at rest. - 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. - 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:1233still 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
- The change-set envelope itself, which is feat(core)!: move a record's aspects in one call, one version #269 and landed correctly. This bug predates it:
setParent()(feat(core): add setParent() to move records between containers #263) had the same gate, andcreate()has had it for longer still. - Widening what a non-owner may learn. The fix is the owner tier only.
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.
Activity
Point the two skipped parent fixtures at haverstack/core#276