permission: restrict FOUNDATION grant and add pre-activation audit#3975
Open
juan-malbeclabs wants to merge 4 commits into
Open
permission: restrict FOUNDATION grant and add pre-activation audit#3975juan-malbeclabs wants to merge 4 commits into
juan-malbeclabs wants to merge 4 commits into
Conversation
A plain PERMISSION_ADMIN holder could grant the FOUNDATION flag (bit 0) to anyone, including themselves — a privilege escalation. Add authorize::can_grant_foundation and gate the FOUNDATION grant in permission create/update so only a foundation_allowlist member or an existing FOUNDATION holder can grant it. The check runs independently of RequirePermissionAccounts so foundation members remain authoritative and cannot be locked out.
Add `doublezero permission audit` to report, before enabling require-permission-accounts, which legacy keys would lose access to migrated instructions (coverage gaps, non-zero exit), super-admin holders, and the non-migrated subsystems that still depend on the GlobalState allowlists (so operators know not to remove those keys yet).
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 of Changes
PERMISSION_ADMINholder could grant theFOUNDATIONflag (bit 0) to anyone, including themselves. Granting/addingFOUNDATIONnow requires the caller to be afoundation_allowlistmember or an existingFOUNDATIONholder (newauthorize::can_grant_foundation, wired into permissioncreate/update). The check runs independently ofRequirePermissionAccountsso foundation members remain authoritative and cannot be locked out.doublezero permission audit— a pre-activation gate that reports which legacy keys would lose access to migrated instructions beforerequire-permission-accountsis enabled (coverage gaps, non-zero exit for runbook/CI use), plus super-admin (PERMISSION_ADMIN/FOUNDATION) holders and the non-migrated subsystems that still depend on the GlobalState allowlists (so operators know not to remove those keys yet).Diff Breakdown
Mostly new code: the audit command and the FOUNDATION-grant guard, with substantial test coverage (in-file unit tests plus a dedicated integration suite). The core-logic figure includes co-located
#[cfg(test)]unit tests.Key files (click to expand)
smartcontract/cli/src/permission/audit.rs— newpermission auditcommand: legacy→Permission coverage-gap detection (mirrorscheck_legacy_any), super-admin reporting, non-migrated legacy-dependency warning; +6 unit testssmartcontract/programs/doublezero-serviceability/tests/permission_test.rs— integration tests for the FOUNDATION-grant restriction (deny on create/update, positive path for a FOUNDATION holder)smartcontract/programs/doublezero-serviceability/src/authorize.rs— newcan_grant_foundationhelper + 5 unit testssmartcontract/programs/doublezero-serviceability/src/processors/permission/{create,update}.rs— gate the FOUNDATION grant/add behindcan_grant_foundationTesting Verification
permission auditlogic covered by 6 CLI unit tests (gap detection with the foundation-recovery carve-out forPERMISSION_ADMIN, suspended accounts not counting as coverage, super-admin holder reporting, non-zero exit on gaps).authorizeunit tests (allowlist member, FOUNDATION-flag holder,PERMISSION_ADMIN-only denied, suspended holder denied, no-account denied) and 3 integration tests: aPERMISSION_ADMIN-only caller is denied granting FOUNDATION on both create and update (with a passing control for non-FOUNDATION flags), while a FOUNDATION-flag holder succeeds.permission_testsuite: 17/17).