Skip to content

Conversation

@imhayatunnabi
Copy link
Contributor

@imhayatunnabi imhayatunnabi commented Oct 13, 2025

This PR fixes three critical issues affecting production stability and Laravel 12 compatibility, plus adds security enhancements and performance optimizations.

Issues Fixed

  1. Race Condition in Permission Loading
  2. Laravel 12 Automatic Eager Loading Bug
  3. Input Validation and Query Optimization

Reduced unnecessary container resolution calls:

Before:

public function roles(): BelongsToMany
{
    return $this->belongsToMany(
        config('permission.models.role'),
        config('permission.table_names.role_has_permissions'),
        app(PermissionRegistrar::class)->pivotPermission,  // Call 1
        app(PermissionRegistrar::class)->pivotRole         // Call 2
    );
}

After:

public function roles(): BelongsToMany
{
    $registrar = app(PermissionRegistrar::class);  // Single call

    return $this->belongsToMany(
        config('permission.models.role'),
        config('permission.table_names.role_has_permissions'),
        $registrar->pivotPermission,
        $registrar->pivotRole
    );
}

@parallels999
Copy link
Contributor

Shouldn't these validations go to the traits?
https://github.com/spatie/laravel-permission/blob/main/src/Traits/HasPermissions.php

@imhayatunnabi
Copy link
Contributor Author

imhayatunnabi commented Oct 13, 2025

@parallels999
now in one place and follow DRY
d5583f8

$this->table = config('permission.table_names.permissions') ?: parent::getTable();
}

/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove all these validation methods from all files in this PR.

@drbyte
Copy link
Collaborator

drbyte commented Oct 27, 2025

The suggested validations can be done in application code.

@drbyte drbyte closed this Oct 27, 2025
@imhayatunnabi imhayatunnabi deleted the feat/models-security-validation-optimization branch October 28, 2025 04:37
drbyte added a commit that referenced this pull request Nov 3, 2025
Reduced unnecessary container resolution calls

Manually merged in from #2884
Credit: @imhayatunnabi

--

Co-authored-by: Hayatunnabi Nabil <[email protected]>
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