Skip to content

docs: record-scoped write authorization (allowDelete/allowUpdate/allowCreate) (5.2)#593

Open
kriszyp wants to merge 4 commits into
mainfrom
kris/record-write-auth-docs
Open

docs: record-scoped write authorization (allowDelete/allowUpdate/allowCreate) (5.2)#593
kriszyp wants to merge 4 commits into
mainfrom
kris/record-write-auth-docs

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 17, 2026

Copy link
Copy Markdown
Member

Companion to HarperFast/harper#1842 (Record-scoped write authorization).

Extends the Record-Level Access Control section in reference/database/schema.md with the write-side model:

  • Conditional DELETE: overridden allowDelete evaluated per matching record, filter semantics, limit/offset count allowed rows.
  • Array PUT: per-element allowUpdate/allowCreate (create-vs-update by element existence), atomic fail semantics.
  • this = per-row resource instance (schema attribute reads + super.allow* composition); async overrides supported on write paths.
  • Defaults keep the single entry check; operations-API/SQL unchanged; overridden-delete() caveat.

Plus a 5.2 release-notes entry mirroring the allowRead one.

Generated by KrAIs (Claude Fable 5).

…wCreate)

Companion to HarperFast/harper#1842 — extends the record-scoped allowRead
section with the write-side model: per-record allowDelete on conditional
deletes (filter semantics), per-element allowUpdate/allowCreate on array PUTs
(atomic fail semantics), this = per-row resource, async overrides supported,
defaults unchanged; plus the overridden-delete() caveat. 5.2 release note
included.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds documentation and release notes for the new record-scoped write authorization hooks (allowDelete, allowUpdate, and allowCreate) introduced in version 5.2.0. The feedback suggests updating a caveat mention of delete() to static delete() in the schema documentation to maintain consistency with Harper Resource documentation standards, which define HTTP verb handlers as static methods.

Comment thread reference/database/schema.md
@github-actions
github-actions Bot temporarily deployed to pr-593 July 17, 2026 00:46 Inactive
@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-593

This preview will update automatically when you push new commits.

@kriszyp
kriszyp marked this pull request as ready for review July 17, 2026 00:54
@kriszyp
kriszyp requested a review from a team as a code owner July 17, 2026 00:54
Adds the back-compat guidance the record-scoped allow* docs were missing:
what changes for an override that already exists, rather than only how the
new model works.

Grounded in a scan of 46 HarperFast app/template repos with allow* overrides.
The three changes that alter behavior without raising an error:

- a denial can become a filtered result rather than a 403 (the
  "special-case a role, else delegate to super" pattern is most affected)
- for allowRead, `async` decides the enforcement model (traversal cannot
  await, so async overrides keep the single entry check)
- array PUT now consults allowUpdate/allowCreate overrides that a
  collection-scope insert check previously bypassed

Also documents that `context` is the same instance across per-record
evaluations, so per-request work can be memoized on it.
@github-actions
github-actions Bot temporarily deployed to pr-593 July 17, 2026 04:33 Inactive
@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-593

This preview will update automatically when you push new commits.

…e allow* change

The allowRead change was only mentioned inside the Filtered Vector Search
paragraph, where nobody scanning 5.2 for authorization changes would find
it — and the upgrade section referred to 'the read and write hooks above'
when the read hook had no entry above. The read, write, and upgrade notes
now live under one Record-Scoped Authorization section; the vector-search
paragraph links to it instead of carrying the exposition.
@github-actions
github-actions Bot temporarily deployed to pr-593 July 17, 2026 11:24 Inactive
@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-593

This preview will update automatically when you push new commits.

Comment thread reference/database/schema.md
Comment thread reference/database/schema.md
Comment thread reference/database/schema.md Outdated
- Scope the upgrade-checklist 403->200 note to collection reads and
  conditional DELETE; array PUT denials still return 403 (the change
  there is only that the hook may now run at all).
- Clarify that `user` is undefined (not an object with a falsy id) for
  an unauthenticated caller, and how the allowDelete vs
  allowUpdate/allowCreate examples differ in how they fail closed for
  that case.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-593

This preview will update automatically when you push new commits.


One caveat: a class that overrides `delete()` itself replaces the framework's record-scoped delete path — its query-shaped deletes keep the request-entry check (a warning is logged when this combination is detected), and the custom `delete()` is responsible for any row-level enforcement.

For an unauthenticated caller, `user` itself is `undefined` — not an object with a falsy `id`. Leading with `super.allow*(user, ...)`, as the `allowDelete` example does, denies before any `user.*` access runs, since the default checks are written with `user?.`. An override that skips that call and dereferences `user.id` directly, as the `allowUpdate`/`allowCreate` examples do, will throw for an unauthenticated caller instead — which still fails closed (denies), just via an exception rather than an explicit `false`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: this clarification documents a side effect of the RBAC-bypass gap without closing it

This paragraph explains that skipping super.allow*(...) makes an unauthenticated caller throw instead of returning false — accurate, but it only addresses that narrow edge case. It doesn't fix why the allowUpdate/allowCreate examples above (lines 575-582) omit super.allowUpdate(user, ...) / super.allowCreate(user, ...) in the first place: any authenticated user who satisfies the ownership check (this.ownerId === user.id) is granted the write regardless of their role/RBAC permission on that table, because the RBAC baseline that super.allow*(...) composes (per the prose two paragraphs above) is never consulted for those two hooks. That's the privilege-bypass risk flagged in the prior review round — this addition explains a symptom of the gap rather than removing it.

Suggested fix: add if (!super.allowUpdate(user, updates, context)) return false; and if (!super.allowCreate(user, record, context)) return false; to the two examples, matching the allowDelete example immediately above. This paragraph can then be trimmed to the still-true note about user being undefined for unauthenticated callers.


Generated by Barber AI

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants