Add a shared TenantScoped trait for the module packages - #5
Merged
Conversation
The cross-tenant IDOR closed in zerp/lead is not a Lead-specific mistake — it is how the modules were written. Controllers authorise mutations with a capability check alone, which every tenant's staff passes, so another company's id resolves fine and is then read, edited or deleted. The boundary belongs on the models, and the trait belongs in one place: the packages already depend on the app (App\Models\User, the creatorId() helper), so a single definition here beats twenty copies of a security guard. Documents the two things it cannot cover: `exists:` rules go through the query builder and must be scoped by hand, and provisioning code that acts on another tenant must opt out explicitly.
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The cross-tenant IDOR closed in
zerp/leadv1.0.3 is not a Lead-specific mistake — it is how the modules were written. A survey of the other packages:hrmholds employee PII and salary data.This PR
Adds
App\Models\Concerns\TenantScoped— one definition, in the app. The packages already depend on the app (App\Models\User, thecreatorId()helper), so this adds no new coupling, and a security guard should not exist in twenty copies.The trait documents the two things it cannot cover, both of which bit us in Lead:
exists:validation rules go through the query builder, not Eloquent, so a global scope never sees them. They must be scoped by hand.withoutGlobalScope("tenant"), or signup silently re-creates its defaults on every run.Verification
tests/Unit/TenantScopeTest.php— 4 tests over the hrm models. Removing the scope from a model fails 2 of them. Isolation was also proven against real MySQL (two tenants; each sees its own rows, blocked on the other's), inside a rolled-back transaction.Rollout is staged, highest-risk first.
hrmis zerp-pk/hrm#1;account,recruitmentandsupport-ticketfollow. Excluded by design:landing-page,aiassistant,calendar,package-template— they have nocreated_by, so their data is global and scoping them would blank out pages.