Skip to content

fix: permissions: PUT /api/permissions/roles/{id} with missing 'permissions' field silently wipes all role permissions - #277

Draft
antosubash wants to merge 2 commits into
mainfrom
pi/issue-269-permissions-put-api-permissions-roles-id
Draft

fix: permissions: PUT /api/permissions/roles/{id} with missing 'permissions' field silently wipes all role permissions#277
antosubash wants to merge 2 commits into
mainfrom
pi/issue-269-permissions-put-api-permissions-roles-id

Conversation

@antosubash

Copy link
Copy Markdown
Owner

Closes#269

Headless Pi worker

This is a draft pull request created from an issue carrying the approval label.
It is never merged automatically and requires human review.

Agent summary

Implementation

  • Made RolePermissionsUpdate.permissions required.
  • Missing permissions now returns HTTP 422 without changing assignments.
  • Explicit {"permissions": []} remains a valid clear-all operation.
  • Added API regression coverage for preservation and deliberate clearing.

Changed areas

  • modules/permissions/permissions/contracts/schemas.py
  • modules/permissions/tests/test_permissions_module.py

Verification

  • uv run pytest modules/permissions/tests29 passed
  • Ruff format/check — passed
  • uv run ty check ...passed
  • git diff --checkpassed

Visual evidence: none required.
Risks/blockers: none.

Local visual evidence

When requested, screenshots, traces, videos, and GIFs are stored only under the worker worktree's ignored
.qa/issues/269/ directory. GitHub CLI cannot attach those files; inspect them on the worker host
or attach selected evidence manually.

…ssions' field silently wipes all role permissions (#269)
@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 21, 2026

Copy link
Copy Markdown

Deploying simple-module-python with Cloudflare Pages Cloudflare Pages

Latest commit:02a7497
Status: ✅ Deploy successful!
Preview URL:https://90b5960a.simple-module-python.pages.dev
Branch Preview URL:https://pi-issue-269-permissions-put.simple-module-python.pages.dev

View logs

@antosubash

Copy link
Copy Markdown
OwnerAuthor

🔧 CI repair attempt 1 pushed. I will monitor the new checks automatically.

Root cause

The new regression test pushed test_permissions_module.py to 318 lines, violating the 300-line cap.

Changes

  • Moved the regression test into:
    • modules/permissions/tests/test_permissions_role_update.py
  • Reduced test_permissions_module.py to 287 lines.
  • No production code or CI workflows changed.

Verification

  • File-size check: passed.
  • Permissions tests: 29 passed.
  • make ci-python-lint: passed.
  • make ci-python-typecheck: passed.
  • make ci-js-lint: passed.
  • make ci-js-typecheck: passed.
  • make test-js: 103 passed.
  • Full make test: 2045 passed, but 3 unrelated S3 tests failed with Moto returning HTTP 403 during bucket setup.

Remaining risk: full Python test suite remains blocked by the unrelated Moto/S3 environment failure.

@antosubash

Copy link
Copy Markdown
OwnerAuthor

✅ CI checks passed after 1 automatic repair attempt.

Sign up for freeto 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.

permissions: PUT /api/permissions/roles/{id} with missing 'permissions' field silently wipes all role permissions

1 participant

@antosubash