nuwave / nuwave/lighthouse

Nested mutations allow operations on records outside parent relationship scope

Open
#2,744 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
PHP
Stars
3.5k
Forks
468
Avg merge
3h 9m
Merged PRs (30d)
2

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: https://github.com/nuwave/lighthouse/issues/1400 (which reported only the delete case for HasMany)

Scope of Impact

Handler update delete connect disconnect upsert sync
NestedOneToMany ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ -
NestedOneToOne ⚠️ ⚠️ - - ⚠️ -
NestedManyToMany ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️
NestedBelongsTo ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ -
NestedMorphTo - ⚠️ ⚠️ ⚠️ - -

⚠️ = Unscoped operation that can affect records outside the parent relationship

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 records

Exploitation 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:

  1. Major version bump - Fix in v7
  2. Opt-in flag - Add config option lighthouse.nested_mutations.scope_to_parent
  3. New directives - Add @scopedNested directive, keep current as default

Affected Files

  • src/Execution/Arguments/NestedOneToMany.php
  • src/Execution/Arguments/NestedOneToOne.php
  • src/Execution/Arguments/NestedManyToMany.php
  • src/Execution/Arguments/NestedBelongsTo.php
  • src/Execution/Arguments/NestedMorphTo.php
  • src/Execution/Arguments/UpdateModel.php
  • src/Execution/Arguments/UpsertModel.php

Related

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the affected handlers in src/Execution/Arguments/NestedOneToMany.php, NestedOneToOne.php, NestedManyToMany.php, NestedBelongsTo.php, NestedMorphTo.php, UpdateModel.php, and UpsertModel.php, focusing on the unscoped ID lookups described in the issue. Done means every listed ID-based nested operation is constrained to the intended parent relationship without leaving the affected handlers unaddressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, laravel, php
Domain
backend-api-design, databases, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.