-
-
Notifications
You must be signed in to change notification settings - Fork 465
Description
Summary
Nested mutation handlers perform database queries without verifying that related records belong to the intended parent. This allows users to update, delete, connect, or sync records that belong to other parents.
Supersedes: #1400 (which reported only the delete case for HasMany)
Scope of Impact
| Handler | update | delete | connect | disconnect | upsert | sync |
|---|---|---|---|---|---|---|
NestedOneToMany |
- | |||||
NestedOneToOne |
- | - | - | |||
NestedManyToMany |
||||||
NestedBelongsTo |
- | |||||
NestedMorphTo |
- | - | - |
Root Cause
Nested handlers use unscoped queries when resolving IDs:
// NestedOneToMany.php:77-79 - delete
$relation->getRelated()::destroy($ids); // Deletes ANY record with these IDs
// UpdateModel.php:33 - update
$model = $model->newQuery()->findOrFail($id->value); // Finds ANY record
// UpsertModel.php:29-30 - upsert
$existingModel = $model->newQuery()->find($id); // Finds ANY record
// NestedOneToMany.php:52-58 - connect
->whereIn($relation->make()->getKeyName(), $ids)->get(); // Gets ANY recordsExploitation Example
# User B can update User A's task
mutation {
updateUser(input: {
id: 2 # User B
tasks: {
update: [{
id: 1 # Task belonging to User A!
name: "hacked"
}]
}
}) {
id
}
}Current Documentation
The nested mutations documentation warns:
Lighthouse has no mechanism for fine-grained permissions of nested mutation operations. Field directives such as the @can* family of directives apply to the whole field.
Make sure that fields with nested mutations are only available to users who are allowed to execute all reachable nested mutations.
While this warning exists, many users expect that operations on nested relations would be scoped to the parent automatically.
Proposed Solution
Scope all ID-based operations through the relation:
// Instead of:
$relation->getRelated()::destroy($ids);
// Use:
$relation->whereIn($relation->getRelated()->getKeyName(), $ids)->get()
->each->delete();
// Instead of (in UpdateModel/UpsertModel):
$model->newQuery()->findOrFail($id);
// Pass the relation and use:
$relation->findOrFail($id);
// Or for nested contexts, verify the FK matches:
$model->newQuery()
->where($foreignKey, $parentId)
->findOrFail($id);Breaking Change Consideration
This would be a breaking change for users who intentionally rely on the current behavior (rare but possible). Options:
- Major version bump - Fix in v7
- Opt-in flag - Add config option
lighthouse.nested_mutations.scope_to_parent - New directives - Add
@scopedNesteddirective, keep current as default
Affected Files
src/Execution/Arguments/NestedOneToMany.phpsrc/Execution/Arguments/NestedOneToOne.phpsrc/Execution/Arguments/NestedManyToMany.phpsrc/Execution/Arguments/NestedBelongsTo.phpsrc/Execution/Arguments/NestedMorphTo.phpsrc/Execution/Arguments/UpdateModel.phpsrc/Execution/Arguments/UpsertModel.php
Related
- Nested mutation delete allows deletion of other relationship #1400 - Original report for delete case (superseded by this issue)