Repository navigation
fix(llc): tenant-isolate boards.py endpoints + repair board service tests (#10296, #10297) - #10356
Merged
Merged
Conversation
…ests (#10296, #10297) #10296 (HIGH security) — boards.py endpoints depended only on get_session with no tenant check, the same IDOR class fixed for sprints.py in #10148/#10258. Any authenticated caller could read/mutate another company's boards by enumerating UUIDs. Now every entity-keyed + create route enforces get_current_user + require_org_context: - get_board / get_board_items / move_item: load the board, verify company_id == ctx.org_id, return 404 (not 403) on cross-tenant. move_item verifies ownership BEFORE the mutation. - create_kanban/create_sprint: reject a body company_id that isn't the caller's org (never trust the body). - New test_boards_idor.py (7 tests) mirrors test_sprints_idor.py. #10297 — repaired 3 pre-existing test_board_service.py failures: - get_board_items grouping: WorkItemStatus is a (str, Enum), so str(enum) gives the member NAME not the value — items never matched a column. Service now uses .value so enum- and plain-string statuses both map (prod sends strings, so this was a test-vs-prod gap, fixed defensively in the service). - move_item WIP test mocked .scalars().all() but the service counts via func.count().scalar_one() → mock returns the count int now. - get_or_create_kanban test patched LLCBoard (breaking select(LLCBoard)) → patch only LLCBoardColumn. All 13 board-service + 7 IDOR + 3 adapter tests pass; flake8 0. Closes #10296 · Closes #10297 (part of #9923)
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No hardcoded values detected that have SSOT config equivalents! |
30 tasks done
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.
Thinking Path
Batched the two boards-area #9923 issues into one PR: #10296 (HIGH IDOR in boards.py) + #10297 (the 3 pre-existing board service test failures). Both touch
boards.py/board.py/test_board_service.py, so they review together. The IDOR is the same class I owned in #10148 (my #9020 endpoints) — and more traffic now flows through boards via the Boards UI I added in #10293, so closing it matters.What Changed
#10296 — tenant-isolate boards.py (security)
Every entity-keyed + create route now enforces
get_current_user+require_org_context:get_board/get_board_items/move_item— load the board, verifycompany_id == ctx.org_id, 404 (not 403) on cross-tenant.move_itemchecks ownership before the mutation.create_kanban/create_sprint— reject a bodycompany_idthat isn't the caller's org (never trust the body).list_company_boards(added in feat(llc): UI integration — reachable boards + adapter-selectable agent hire (#10219) #10293) was already guarded.test_boards_idor.py(7 tests) mirrorstest_sprints_idor.py.#10297 — repair 3 pre-existing board-service test failures
WorkItemStatusis a(str, Enum), sostr(enum)returns the member NAME (WorkItemStatus.READY), not the value (ready) — items never matched a column. The service now uses.valueso enum- and plain-string statuses both map (prod sends strings, so this was a test-vs-prod gap, fixed defensively in the service so the test reflects reality)..scalars().all()but the service counts viafunc.count().scalar_one()→ mock now returns the count int.LLCBoard(which breaksselect(LLCBoard)) → patch onlyLLCBoardColumn.Verification
pytest llc/tests/test_board_service.py llc/tests/test_boards_idor.py→ 20 passed (was 10/3-failing); adapters endpoint tests still green.Model Used
Claude Opus 4.8
Closes #10296 · Closes #10297 (part of #9923)