Skip to content

[pull] master from inventree:master - #152

Merged
pull[bot] merged 1 commit into
Latitudes-Dev:masterfrom
inventree:master
Apr 28, 2026
Merged

pull[bot] merged 1 commit into
Latitudes-Dev:masterfrom
inventree:master

Conversation

@pull

@pull pull Bot commented Apr 28, 2026 •

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

Greptile Summary

This PR introduces a new bom ruleset that separates Bill of Materials permissions from the broader part ruleset, adds corresponding frontend role guards, and bumps the API to v481. The approach is sound, but two backend issues need attention before the permission separation works as intended:

  • RULESET_CHANGE_INHERIT = [('part', 'bomitem')] in ruleset.py is not updated, so any user with part.change still silently inherits full BOM-item permissions via both the permissions.py and tasks.py code paths — effectively bypassing the new bom role.
  • The data migration (0148) imports the live RuleSet model directly instead of using apps.get_model('users', 'RuleSet'), which is fragile if the model ever changes after this migration is squashed or re-run from scratch.

Confidence Score: 3/5

Not safe to merge as-is — the stale RULESET_CHANGE_INHERIT entry means the new bom role separation does not fully take effect, and the migration uses a live model import.

Two P1 findings: stale inheritance silently grants BOM access to part.change holders (defeating the core purpose of this PR), and the migration uses a live model import that is fragile. Frontend and OAuth scope changes are clean.

src/backend/InvenTree/users/ruleset.py (RULESET_CHANGE_INHERIT) and src/backend/InvenTree/part/migrations/0148_auto_20260427_2233.py (live model import)

Important Files Changed

Filename Overview
src/backend/InvenTree/users/ruleset.py Adds BOM ruleset and moves part_bomitem/part_bomitemsubstitute under it, but RULESET_CHANGE_INHERIT still grants these models to part.change holders, undermining the new separation.
src/backend/InvenTree/part/migrations/0148_auto_20260427_2233.py Data migration copies part ruleset permissions to new bom ruleset, but uses a live model import (users.models.RuleSet) instead of the historical apps.get_model() pattern, which is fragile for future migrations.
src/backend/InvenTree/part/api.py Adds role_required = 'bom.change' to PartValidateBOM endpoint; change is minimal and correct in intent.
src/backend/InvenTree/part/test_api.py Adds permission checks for bom role but assigns bom.add for view-only (GET) requests, which may be testing an incorrect permission level.
src/frontend/src/pages/part/PartDetail.tsx Gates BOM tab and validate button behind bom role checks; implementation is correct.
src/frontend/src/tables/bom/BomTable.tsx Switches all BOM table action visibility guards from UserRoles.part to UserRoles.bom; changes are consistent and correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User Request to BOM endpoint] --> B{Has bom role?}
    B -->|Yes| C[Access Granted via bom ruleset]
    B -->|No| D{Has part.change via RULESET_CHANGE_INHERIT?}
    D -->|Yes| E["BOM item access STILL GRANTED ⚠️ stale inheritance"]
    D -->|No| F[403 Forbidden]
Loading

Comments Outside Diff (3)

  1. src/backend/InvenTree/users/ruleset.py, line 45 (link)

    Stale RULESET_CHANGE_INHERIT bypasses new BOM role separation

    RULESET_CHANGE_INHERIT = [('part', 'bomitem')] still propagates full BOM-item permissions to any user who holds part.change, via the logic in permissions.py (line 176-183) and tasks.py (line 177-196). Now that part_bomitem and part_bomitemsubstitute live exclusively under the bom ruleset, this inheritance rule silently grants those permissions to part role holders, effectively nullifying the new granular separation this PR introduces.

  2. src/backend/InvenTree/part/migrations/0148_auto_20260427_2233.py, line 13-15 (link)

    Migration imports live model instead of historical model

    from users.models import RuleSet imports the current live model rather than the frozen historical snapshot that Django provides via apps.get_model(). If the RuleSet model is later renamed, removed, or its fields change, this migration will break when run from scratch. The correct pattern is RuleSet = apps.get_model('users', 'RuleSet') (using the apps parameter already passed to update_bom_role). Importing RuleSetEnum directly is acceptable since it is a pure enum with no DB schema dependency, but the RuleSet model reference should use the historical registry.

  3. src/backend/InvenTree/part/test_api.py, line 1041-1050 (link)

    Test assigns bom.add for a read-only endpoint

    PartValidateBOM is a RetrieveUpdateAPI with role_required = 'bom.change'. The test grants bom.add and immediately does a GET, expecting a 200. Using add to satisfy a change-gated GET either accidentally exercises an unintended permission path or is testing the wrong role; the test should assign bom.view or bom.change to match the documented requirement. The same pattern appears in BomItemTest (assignRole('bom.add') before a list GET at line ~2932).

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/backend/InvenTree/users/ruleset.py
Line: 45

Comment:
**Stale `RULESET_CHANGE_INHERIT` bypasses new BOM role separation**

`RULESET_CHANGE_INHERIT = [('part', 'bomitem')]` still propagates full BOM-item permissions to any user who holds `part.change`, via the logic in `permissions.py` (line 176-183) and `tasks.py` (line 177-196). Now that `part_bomitem` and `part_bomitemsubstitute` live exclusively under the `bom` ruleset, this inheritance rule silently grants those permissions to `part` role holders, effectively nullifying the new granular separation this PR introduces.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/backend/InvenTree/part/migrations/0148_auto_20260427_2233.py
Line: 13-15

Comment:
**Migration imports live model instead of historical model**

`from users.models import RuleSet` imports the *current* live model rather than the frozen historical snapshot that Django provides via `apps.get_model()`. If the `RuleSet` model is later renamed, removed, or its fields change, this migration will break when run from scratch. The correct pattern is `RuleSet = apps.get_model('users', 'RuleSet')` (using the `apps` parameter already passed to `update_bom_role`). Importing `RuleSetEnum` directly is acceptable since it is a pure enum with no DB schema dependency, but the `RuleSet` model reference should use the historical registry.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/backend/InvenTree/part/test_api.py
Line: 1041-1050

Comment:
**Test assigns `bom.add` for a read-only endpoint**

`PartValidateBOM` is a `RetrieveUpdateAPI` with `role_required = 'bom.change'`. The test grants `bom.add` and immediately does a GET, expecting a 200. Using `add` to satisfy a `change`-gated GET either accidentally exercises an unintended permission path or is testing the wrong role; the test should assign `bom.view` or `bom.change` to match the documented requirement. The same pattern appears in `BomItemTest` (`assignRole('bom.add')` before a list GET at line ~2932).

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "BOM Ruleset (#11825)" | Re-trigger Greptile

* Add BOM role

* Adjust UI permissions

* Adjust docs

* Add data migratoin

* Specify role for BOM validation

* Tweak old migrati

* Fix role_required

* Update API version and CHANGELOG

Co-authored-by: Copilot <copilot@github.com>

* Update unit tests

Co-authored-by: Copilot <copilot@github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
@pull pull Bot locked and limited conversation to collaborators Apr 28, 2026
@pull pull Bot added the ⤵️ pull label Apr 28, 2026
@pull
pull Bot merged commit 2b6952e into Latitudes-Dev:master Apr 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant