Upgrade to core 0.13 line and adopt the 404/403 anti-oracle rule - #87
Merged
Conversation
Bumps @haverstack/core to ^0.13.1, wire-types to ^0.12.0, adapter-local to ^0.12.0, commons to ^0.5.0, and conformance-fixtures to ^0.6.0 (dev), resolving core to 0.13.1 and record-adapter-sqlite to 0.5.1 transitively. ScopedStack.get() now returns null (rather than throwing) for a record the requester can't read, and the write/history mutators throw StackNotFoundError instead of StackPermissionError in that case, so 403 is now earned by readability rather than by holding a valid id. Record ids encode their creation millisecond, so a bare 403 to an unauthenticated caller would let anyone confirm a guessed id's same-millisecond siblings — errorMiddleware now answers an anonymous 404 with a Bearer WWW-Authenticate challenge, keeping the login prompt reachable without reopening that distinction. entity.ts's cached owner-record id is now only evicted by the owner's own null read, since a non-owner's null is ambiguous under the new rule (denied vs. gone) and previously forced a wasted re-resolve query on every forbidden GET. Also folds in two wire-contract changes that shipped in the same dependency range and are needed to get the suite green: soft-delete, associate, dissociate, and setPermissions now answer 200 with the updated record instead of 204 (hard-delete, which produces no record, is unchanged); and a non-author PATCH now stamps updatedBy/updatedVia on the record, which ScopedStack already does from the session with no route change needed. The two new change-feed discovery fixtures are parked in conformance tests' SKIPPED set pending #82/#83.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #79.
Summary
@haverstack/coreto^0.13.1,wire-typesto^0.12.0,adapter-localto^0.12.0,commonsto^0.5.0, andconformance-fixturesto^0.6.0(dev). Confirmed core resolves to 0.13.1 andrecord-adapter-sqliteto 0.5.1 transitively.ScopedStack.get()now returnsnull(rather than throwing) for a record the requester can't read, and the write/history mutators throwStackNotFoundErrorinstead ofStackPermissionErrorin that case — 403 is now earned by readability, not by holding a valid id.WWW-Authenticate: Bearerfor a private record instead of 403 or a bodyless 401 — added centrally inerrorMiddleware(fires whenever aStackNotFoundError-mapped 404 reaches an anonymous requester), so it applies uniformly rather than per-route.Bearer(RFC 6750 §3) is the standard WWW-Authenticate scheme for a bearer-token API — this server's actual wire credential — rather than the higher-leveldid-challengeexchange discovery advertises for obtaining that token.entity.ts's cached owner-record id: it's now only evicted by the owner's own null read. A non-owner's null is ambiguous under the new rule (denied vs. gone) and previously forced a wasted re-resolve query on every forbiddenGET /entity. Added a regression test asserting the query isn't re-run.requireOwner()-gated routes (tokens, types, entityPATCH, attachment delete/gc) are unaffected — those throw in middleware before any record is read, so they correctly stay 403.retainSortDirection's existing validation inqueryParsing.tsis unchanged and still the right defense-in-depth alongside core's own hardenedORDER BYcheck.Also folded in (shipped in the same dependency range, needed to get the suite green)
Two wire-contract changes were bundled into the same core/wire-types minors and aren't mentioned in #79's checklist, but the suite can't pass without them:
setPermissionsnow answer200with the updated record instead of204(hard-delete, which produces no record, is unchanged and stays204).PATCHnow stampsupdatedBy/updatedViaon the record. This "arrives for free" —ScopedStackalready does it from the session, no route change needed, just a conformance test dispatching the new fixture.Sweep of
toBe(403)assertions (#79's checklist item)Went through all 29
toBe(403)assertions acrosstests/. Beyond the two named in the issue (records.test.ts:110,142), found three more read-based cases that also needed to flip to 404, confirmed by actually running the suite against the bumped deps rather than by inspection alone:records.test.ts— "an empty permissions array makes the record private" (anonymous GET afterPUT /permissions)conformance.test.ts—set-permissions-empty-is-privatefixture dispatch (same shape)entity.test.ts—GET /entityfor a non-owner authenticated entity (onlyrequireAuth()-gated, notrequireOwner(), so it goes through the sameget()-returns-null path)One test (
records.test.ts— soft-DELETE on a_grantrecord) also flipped, but for a different reason:stack.grant()never gave the grantee read access to the grant record itself, so the test was accidentally exercising the anti-oracle case instead of the write-protection fence it was meant to test. Fixed by granting explicit read+write on the record directly, matching its PATCH sibling test.Test plan
pnpm test— 343/343 passingpnpm typecheck— cleanpnpm lint— cleanpnpm format:check— cleandiscovery-advertises-a-change-feed,discovery-advertises-a-feed-that-neither-resumes-nor-includes-records) parked in the discovery block'sSKIPPEDset with a reason pointing at Implement GET /changes (SSE change feed) #82/Advertise the change feed in discovery #83ScopedStackowns it, per the issue's acceptance criteriaGenerated by Claude Code