Skip to content

fix(relationships): link by the key on the written document with read permission - #1044

Merged
HarshMN2345 merged 1 commit into
mainfrom
fix-key-side-link-read
Oct 21, 2026
Merged

HarshMN2345 merged 1 commit into
mainfrom
fix-key-side-link-read

Conversation

@HarshMN2345

@HarshMN2345 HarshMN2345 commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

What changed

Since 8.0.1 (#1002, commit 1730bb7), linking any existing related document needs update permission on it. When the link is only the key on the document being written, the related document is not written at all, so the update check is not needed. This applies to a one-way one-to-one, a many-to-one, and a one-to-many child pointing at its parent. These links need read permission again, as in 7.x. The change covers createDocument(), updateDocument(), authorizeCreate() and authorizeUpdate().

Unchanged:

  • Links that write the related document or a junction row still need update permission. These are many-to-many (J4C-008, 19c76e5), two-way one-to-one, a one-to-many parent and a many-to-one child.
  • A related document sent with attributes to change still needs update permission.
  • A stored related document the caller cannot read is still refused.

writesById() is renamed writesRelated(), since it now also decides the document-value path. UPGRADE.md is updated to match.

Tested

  • Many-to-one from a collection granting create/read/update("users") to one granting only read("users"), linked by ID as a user. It fails with No permissions provided for action 'update' on 8.3.11 and is created on this branch, one-way and two-way. Memory adapter.
  • tests/unit/Relationships: 2557 tests pass. In NestedLinkPermissionsTest and StagedViewAuthorizationTest, one-way one-to-one and many-to-one links by ID to a read-only document now expect success. testANestedDocumentIsJudgedAgainstTheLinksTheStagedWritesLeave now uses a two-way one-to-one, a link that still needs update.
  • PermissionTests::testNestedRelatedDocumentsFollowTheLinkRule now expects many-to-one by ID without update to succeed. It was not run locally because the e2e adapters need ext-redis.
  • Pint and PHPStan pass on the changed files.

Summary by CodeRabbit

  • Bug Fixes

    • Relationship links that only reference an existing related document now require read permission instead of update permission.
    • Update permission remains required when a relationship creates or modifies the related document or junction record.
    • Authorization checks are now applied consistently across supported one-to-one, one-to-many, and many-to-one relationships.
  • Documentation

    • Updated upgrade guidance to clarify permission requirements and correct previous guidance for read-only relationship links.

… permission

8.0.1 made every nested link to an existing related document need update
permission on it. Where the link is only the key on the document being
written, a one-way one-to-one, a many-to-one, or a one-to-many child
pointing at its parent, nothing on the related document is written, yet
linking to a document the caller may only read now fails with
"No permissions provided for action 'update'". 7.x linked these with read
permission, so read-only lookup collections stopped accepting links after
the upgrade.

These links need read permission again, on create, update, and in
authorizeCreate() and authorizeUpdate(). Links that write the related
document or a junction row (many-to-many, two-way one-to-one, one-to-many
parent, many-to-one child) still need update permission, a related
document sent with attributes to change still needs update permission,
and a stored document the caller cannot read is still refused.
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: utopia-php/database/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f73cb58d-0f5c-4803-8a78-cd4cb0b80262
📥 Commits

Reviewing files that changed from the base of the PR and between 11095f8 and 92f71d9.

📒 Files selected for processing: 5
  • UPGRADE.md
  • src/Database/Hook/Relationships.php
  • tests/e2e/Adapter/Scopes/PermissionTests.php
  • tests/unit/Relationships/NestedLinkPermissionsTest.php
  • tests/unit/Relationships/StagedViewAuthorizationTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Relationship authorization now distinguishes links that modify a related document or junction row from links that only set the relationship key. Read-only links require read access; writable links and supplied documents with changes retain update checks. Tests and upgrade guidance reflect this distinction.

Changes: Relationship Authorization

Layer / File(s) Summary
Authorization policy
src/Database/Hook/Relationships.php
Authorization helpers now determine whether a link writes the related document or junction row. Submitted changes and writable links to documents that are not already linked trigger link authorization.
Relationship checks and validation
src/Database/Hook/Relationships.php, tests/unit/Relationships/*, tests/e2e/Adapter/Scopes/PermissionTests.php, UPGRADE.md
Relationship checks apply the write condition to links by ID and by document. Tests and upgrade guidance reflect when read or update permission is required.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: abnegate

Merge Risk: ⚪ Minimal · up to 92f71

The changed permission rules appear ready to merge after normal checks. The end-to-end adapter test remains unrun locally.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title accurately summarizes the main change: allowing relationship links by the key on the written document with read permission.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@HarshMN2345
HarshMN2345 merged commit e1c599d into main Oct 21, 2026
24 checks passed
@HarshMN2345
HarshMN2345 deleted the fix-key-side-link-read branch October 21, 2026 11:56
MarkkuPekkarinen pushed a commit to MarkkuPekkarinen/appwrite that referenced this pull request Oct 21, 2026
Links by the key on the written row (many-to-one, one-way one-to-one,
one-to-many child) need only read permission on the related row again,
as before 8.0.1 (utopia-php/database#1044). Also takes the cache
invalidation retry from 8.3.11 (utopia-php/database#1043).
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.

1 participant