Skip to content

permission: restrict FOUNDATION grant and add pre-activation audit#3975

Open
juan-malbeclabs wants to merge 4 commits into
mainfrom
jo/permission-activation-hardening
Open

permission: restrict FOUNDATION grant and add pre-activation audit#3975
juan-malbeclabs wants to merge 4 commits into
mainfrom
jo/permission-activation-hardening

Conversation

@juan-malbeclabs

Copy link
Copy Markdown
Contributor

Summary of Changes

  • Fix a privilege-escalation gap in the permission model: a plain PERMISSION_ADMIN holder could grant the FOUNDATION flag (bit 0) to anyone, including themselves. Granting/adding FOUNDATION now requires the caller to be a foundation_allowlist member or an existing FOUNDATION holder (new authorize::can_grant_foundation, wired into permission create/update). The check runs independently of RequirePermissionAccounts so foundation members remain authoritative and cannot be locked out.
  • Add doublezero permission audit — a pre-activation gate that reports which legacy keys would lose access to migrated instructions before require-permission-accounts is 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

Category Files Lines (+/-) Net
Core logic 4 +676 / -4 +672
Scaffolding 3 +17 / -1 +16
Tests 1 +214 / -0 +214
Docs 1 +4 / -0 +4
Total 9 +911 / -5 +906

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 — new permission audit command: legacy→Permission coverage-gap detection (mirrors check_legacy_any), super-admin reporting, non-migrated legacy-dependency warning; +6 unit tests
  • smartcontract/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 — new can_grant_foundation helper + 5 unit tests
  • smartcontract/programs/doublezero-serviceability/src/processors/permission/{create,update}.rs — gate the FOUNDATION grant/add behind can_grant_foundation

Testing Verification

  • permission audit logic covered by 6 CLI unit tests (gap detection with the foundation-recovery carve-out for PERMISSION_ADMIN, suspended accounts not counting as coverage, super-admin holder reporting, non-zero exit on gaps).
  • FOUNDATION-grant restriction covered by 5 authorize unit tests (allowlist member, FOUNDATION-flag holder, PERMISSION_ADMIN-only denied, suspended holder denied, no-account denied) and 3 integration tests: a PERMISSION_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.
  • Verified no lockout regression: existing tests where a foundation payer grants FOUNDATION still pass (full permission_test suite: 17/17).

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).
Sign up for free to 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.

1 participant