Skip to content

Add comments on risky permissions#1468

Open
bennothommo wants to merge 2 commits intodevelopfrom
fix/administrative-permissions
Open

Add comments on risky permissions#1468
bennothommo wants to merge 2 commits intodevelopfrom
fix/administrative-permissions

Conversation

@bennothommo
Copy link
Member

@bennothommo bennothommo commented Mar 13, 2026

To assist admins when assigning permissions to users, I have added comments to permissions that should only be given to trusted users. These permissions, if given to untrusted users, may pose a security risk due to being able to negatively manipulate the experience of other users, or could be potentially used to grant themselves more access than intended.

image

Summary by CodeRabbit

  • New Features

    • Permission comments now appear as tooltip-enabled info icons next to permission labels for clearer access explanations.
  • Documentation

    • Updated permission labels and added descriptive security notes (including unsafe Markdown) and administrator/branding/impersonation guidance across backend and CMS modules.
    • Added clarifying descriptions for CMS permissions outlining access implications.

Some permissions in the Backend and CMS are expected to be given only to
trusted users, as they grant access to features of the CMS that can
negatively manipulate the experience of other users or grant themselves
more access than intended. We now make this explicit by providing hints
about these permissions.
@bennothommo bennothommo added this to the 1.2.13 milestone Mar 13, 2026
@bennothommo bennothommo added maintenance PRs that fix bugs, are translation changes or make only minor changes needs review Issues/PRs that require a review from a maintainer labels Mar 13, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b2a0a0e0-6bb5-439d-8d04-7c73ac28283b

📥 Commits

Reviewing files that changed from the base of the PR and between 6bc8b06 and 69b84be.

📒 Files selected for processing (1)
  • modules/backend/formwidgets/permissioneditor/partials/_permissioneditor.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • modules/backend/formwidgets/permissioneditor/partials/_permissioneditor.php

Walkthrough

This PR adds comment metadata to permission definitions across backend, CMS, and system modules, updates the permission editor UI to display these comments as tooltip icons, and includes corresponding translation updates. Additionally, the allow_unsafe_markdown permission label is clarified and .zed editor is added to gitignore.

Changes

Cohort / File(s) Summary
Editor Configuration
\.gitignore
Added .zed to editor ignores.
Backend Permission Comments
modules/backend/ServiceProvider.php, modules/backend/lang/en/lang.php
Added comment metadata for backend permissions (manage_users, impersonate_users, manage_branding, allow_unsafe_markdown); updated allow_unsafe_markdown label and added allow_unsafe_markdown_comment translation.
Permission Editor UI
modules/backend/formwidgets/permissioneditor/partials/_permissioneditor.php
Replaced always-rendered comment paragraph with conditional tooltip-enabled info icon and ARIA attributes for permissions that include a comment; minor indentation tweaks.
CMS Permission Comments
modules/cms/ServiceProvider.php, modules/cms/lang/en/lang.php
Added comment fields for CMS permissions (manage_pages, manage_layouts, manage_partials, manage_themes) and corresponding _comment translation entries describing risks/access.
System Permission Comments
modules/system/lang/en/lang.php
Added _comment translation entries for manage_other_administrators, impersonate_users, and manage_branding permissions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add comments on risky permissions' directly and specifically summarizes the main change across all modified files, which involves adding descriptive comments to security-sensitive permissions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/administrative-permissions
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can disable sequence diagrams in the walkthrough.

Disable the reviews.sequence_diagrams setting to disable sequence diagrams in the walkthrough.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@modules/backend/formwidgets/permissioneditor/partials/_permissioneditor.php`:
- Around line 58-60: The icon-only span showing $permission->comment is not
keyboard/screen-reader accessible; make it focusable and expose the comment text
to assistive tech by updating the span with tabindex="0" and an accessible name
(either aria-label="<?= e(trans($permission->comment)) ?>" or aria-describedby
pointing to a visually-hidden element containing <?=
e(trans($permission->comment)) ?>), keep the existing classes
(wn-icon-circle-info/text-info) and tooltip attributes, and ensure the
visually-hidden text (e.g., class="sr-only" or "visually-hidden") is present so
screen readers and keyboard users can discover the permission risk.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d56382b-bc5d-4ef4-bb5b-e405b7f33db1

📥 Commits

Reviewing files that changed from the base of the PR and between 64602ba and 6bc8b06.

📒 Files selected for processing (7)
  • .gitignore
  • modules/backend/ServiceProvider.php
  • modules/backend/formwidgets/permissioneditor/partials/_permissioneditor.php
  • modules/backend/lang/en/lang.php
  • modules/cms/ServiceProvider.php
  • modules/cms/lang/en/lang.php
  • modules/system/lang/en/lang.php

…ioneditor.php

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes needs review Issues/PRs that require a review from a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant