Skip to content

fix: relate to an existing related document the caller cannot read #976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
claudear wants to merge 1 commit into
base: main
Choose a base branch
from
+133 −0
Open

fix: relate to an existing related document the caller cannot read #976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6183,6 +6183,17 @@ private function relateDocuments(
// Try to get the related document
$related = $this->getDocument($relatedCollection->getId(), $relation->getId());

if ($related->isEmpty() && !empty($relation->getId())) {
// A related document the caller cannot read comes back empty, which is
// indistinguishable from one that does not exist. Creating it would hit
// the unique _uid key and report "Document already exists", so read it
// again without permissions and relate to what is already there. The
// update below still enforces the caller's update permission.
$related = $this->authorization->skip(
fn () => $this->getDocument($relatedCollection->getId(), $relation->getId())
);
Comment on lines +6192 to +6194

@greptile-apps greptile-apps Bot Sep 18, 2026

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.

P1 security Authorization skipped on equal attributes

When an unreadable existing related document has the same user attributes as the nested payload—which is possible for a document with no custom attributes—the permission-skipped lookup returns it and the equality check skips updateDocument(). A many-to-many junction can then be created without enforcing read or update authorization, allowing callers to link documents they are not authorized to access. Require an explicit permission check before accepting the existing document, even when no attribute update is needed.

How this was verified: The skipped lookup supplies the unreadable document to an equality branch that can reach junction creation without executing any subsequent authorization check.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/Database/Database.php
Line: 6192-6194

Comment:
**Authorization skipped on equal attributes**

When an unreadable existing related document has the same user attributes as the nested payload—which is possible for a document with no custom attributes—the permission-skipped lookup returns it and the equality check skips `updateDocument()`. A many-to-many junction can then be created without enforcing read or update authorization, allowing callers to link documents they are not authorized to access. Require an explicit permission check before accepting the existing document, even when no attribute update is needed.

**How this was verified:** The skipped lookup supplies the unreadable document to an equality branch that can reach junction creation without executing any subsequent authorization check.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

}

if ($related->isEmpty()) {
// If the related document doesn't exist, create it, inheriting permissions if none are set
if (!isset($relation['$permissions'])) {
Expand Down
122 changes: 122 additions & 0 deletions tests/e2e/Adapter/Scopes/RelationshipTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4930,4 +4930,126 @@ public function testOrderAndCursorWithRelationshipQueries(): void
$database->deleteCollection('authorsOrder');
$database->deleteCollection('postsOrder');
}

/**
* A nested related document that already exists but is not readable by the
* current role must be related to, not re-created. Creating it fails on the
* unique `_uid` key and surfaces a bare "Document already exists" duplicate
* error instead of linking the two documents.
*/
public function testCreateDocumentWithUnreadableExistingRelatedDocument(): void
{
/** @var Database $database */
$database = $this->getDatabase();

if (!$database->getAdapter()->getSupportForRelationships()) {
$this->expectNotToPerformAssertions();
return;
}

// No read permission anywhere on the child collection, so its documents
// exist but are invisible to the caller.
$database->createCollection('hiddenKeys', permissions: [
Permission::create(Role::any()),
Permission::update(Role::any()),
]);
$database->createCollection('hiddenSeals', permissions: [
Permission::create(Role::any()),
]);
$database->createCollection('hiddenVaults', permissions: [
Permission::create(Role::any()),
Permission::read(Role::any()),
Permission::update(Role::any()),
]);

$database->createAttribute('hiddenKeys', 'name', Database::VAR_STRING, 255, true);
$database->createAttribute('hiddenSeals', 'name', Database::VAR_STRING, 255, true);
$database->createAttribute('hiddenVaults', 'name', Database::VAR_STRING, 255, true);

$database->createRelationship(
collection: 'hiddenVaults',
relatedCollection: 'hiddenKeys',
type: Database::RELATION_ONE_TO_ONE,
twoWay: true,
id: 'key',
twoWayKey: 'vault'
);

$database->createRelationship(
collection: 'hiddenVaults',
relatedCollection: 'hiddenSeals',
type: Database::RELATION_ONE_TO_ONE,
twoWay: true,
id: 'seal',
twoWayKey: 'vault'
);

$database->createDocument('hiddenKeys', new Document([
'$id' => 'hidden-key',
'$permissions' => [],
'name' => 'Hidden Key',
]));

$database->createDocument('hiddenSeals', new Document([
'$id' => 'hidden-seal',
'$permissions' => [],
'name' => 'Hidden Seal',
]));

$this->assertTrue($database->getDocument('hiddenKeys', 'hidden-key')->isEmpty());
$this->assertTrue($database->getDocument('hiddenSeals', 'hidden-seal')->isEmpty());

// The caller may update the child collection, so the existing document
// is related to the new parent instead of being re-created.
$vault = $database->createDocument('hiddenVaults', new Document([
'$id' => 'vault-1',
'$permissions' => [
Permission::read(Role::any()),
Permission::update(Role::any()),
],
'name' => 'Vault',
'key' => new Document([
'$id' => 'hidden-key',
'name' => 'Hidden Key',
]),
]));

$this->assertEquals('vault-1', $vault->getId());

$stored = $database->getAuthorization()->skip(
fn () => $database->getDocument('hiddenVaults', 'vault-1')
);

$this->assertEquals('hidden-key', $stored->getAttribute('key')->getId());
$this->assertEquals('Hidden Key', $stored->getAttribute('key')->getAttribute('name'));

$keys = $database->getAuthorization()->skip(fn () => $database->find('hiddenKeys'));
$this->assertCount(1, $keys);

// Without update permission on the child collection the caller gets an
// authorization error, not a duplicate key error.
try {
$database->createDocument('hiddenVaults', new Document([
'$id' => 'vault-2',
'$permissions' => [
Permission::read(Role::any()),
Permission::update(Role::any()),
],
'name' => 'Vault 2',
'seal' => new Document([
'$id' => 'hidden-seal',
'name' => 'Broken Seal',
]),
]));
$this->fail('Failed to throw exception');
} catch (AuthorizationException) {
}

$seals = $database->getAuthorization()->skip(fn () => $database->find('hiddenSeals'));
$this->assertCount(1, $seals);

$database->deleteCollection('hiddenVaults');
$database->deleteCollection('hiddenKeys');
$database->deleteCollection('hiddenSeals');
}
}
Loading