Add BYPASS_LOCKS faction role permission#1911
Open
Copilot wants to merge 8 commits into
Open
Conversation
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add faction permission to bypass locks
Add BYPASS_LOCKS faction permission for accessing locked blocks
Dec 11, 2025
…rInteractListener
…ermission-bypass-locks
The branch added the key only to en_US; add it to en_GB/de_DE/fr_FR/pt_BR to match the 5-locale convention for faction permission strings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dmccoystephenson
left a comment
Member
There was a problem hiding this comment.
Self-review rubric (grooming pass — adversarial, assume FAIL without evidence):
- Scope: PASS — three source files (permission enum, listener helper, lang) plus the grooming locale additions. No unrelated churn.
- Tests-new: FAIL (flagged, not auto-fixed) — the new
hasLockBypassPermissionpath has no test. Deliberately deferred pending the scope decision below, so a test doesn't lock in possibly-over-broad behavior.PlayerInteractListenerTestandMfFactionPermissionTestalready exist to mirror once scope is settled. - Sibling structure: PASS —
BYPASS_LOCKSfollows the existingwrapSimplePermission(...)pattern and accessor convention. - Lang-externalized: PASS —
FactionPermissionBypassLocksnow present in all 5 locales (was en_US-only; added en_GB/de/fr/pt during grooming, Latin-1 preserved). No keys removed. - Docs: PASS (N/A) — no doc enumerates the faction role-permission list (USER_GUIDE/FACTION_FLAGS cover gameplay and flag-management Bukkit perms, not the role-permission enum), so nothing to update.
- Migrations-additive / DPC-contract / Config-defaults / Plugin.yml: PASS (vacuous) — none touched.
- Main-thread-safety: PASS —
hasLockBypassPermissionreads faction/role state on the event thread as the surrounding code already does; the bypass branch dispatches viarunTaskAsynchronouslyas before. - CI: PASS —
build+docker-buildgreen on40cb7534; also build-verified in a clean container.
Lead finding (security scope — reviewer decision): the bypass is unscoped — hasLockBypassPermission only verifies the acting player's role has BYPASS_LOCKS, not that the lock belongs to their faction. So a role with this permission can open any lock, including other factions'/players' locks. If the feature is meant for same-faction lock sharing, the helper must also check the lock owner's faction. This is the one thing to settle before merge; default false limits exposure until a leader grants it.
Rebased cleanly, build-verified, localization gap fixed, marked ready. Left un-merged pending the scope decision — security-sensitive behavior change.
Self-review by Claude on behalf of Daniel Stephenson.
4 tasks
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
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.
Summary
Adds a
BYPASS_LOCKSfaction role permission. A faction member whose role is granted this permission can access and unlock locked blocks they don't own/aren't an accessor of.MfFactionPermissions: newBYPASS_LOCKSentry (defaultfalse) +bypassLocksaccessor.PlayerInteractListener: newhasLockBypassPermission(mfPlayer)helper, used in the locked-block access path and the unlock path (alongside the existingmf.bypass/mf.force.unlockBukkit permissions).FactionPermissionBypassLocksin all five locales.hasLockBypassPermissionchecks only that the player's own faction role hasBYPASS_LOCKS; it does not check who owns the lock. Combined with the listener guard (which already established the player is not the owner/accessor), this is effectively a blanket bypass of any lock — including locks owned by other factions or unaffiliated players — for any role granted the permission. If the intent was narrower (e.g. sharing access to the same faction's locks only), the check needs to compare the lock owner's faction. Please confirm the intended scope before merge. Default isfalse, so no role has it unless a leader grants it.Dev-loop grooming
main(was 226 behind) — no conflicts../gradlew clean buildgreen in a clean container; CIbuild+docker-buildgreen on40cb7534).en_US; added it toen_GB/de_DE/fr_FR/pt_BRto match the 5-locale convention.Data-safety statement
No schema migrations; no DPC contract change; no
config.ymldefault change; noplugin.ymlchange. Lang changes are additive (no key removals).Known gap
PlayerInteractListenerTest/MfFactionPermissionTestcase should be added.Groomed by Claude on behalf of Daniel Stephenson.