From 73d4274994d650aeff21e17389086ae0ea1d6c2f Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Thu, 1 Oct 2026 11:33:56 -0400 Subject: [PATCH 1/3] docs(policy): document malformed isAuthorized results A malformed policy ID returns false. That is separate from the empty-set result of an unknown well-formed ID, and an inverted missing base stays fail-closed. Co-authored-by: Cursor --- src/interfaces/IPolicyRegistry.sol | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/interfaces/IPolicyRegistry.sol b/src/interfaces/IPolicyRegistry.sol index 454fd84f..3495b8b0 100644 --- a/src/interfaces/IPolicyRegistry.sol +++ b/src/interfaces/IPolicyRegistry.sol @@ -224,13 +224,16 @@ interface IPolicyRegistry { AUTHORIZATION QUERIES //////////////////////////////////////////////////////////////*/ - /// @notice Returns whether `account` is authorized under `policyId`. Never reverts; unknown - /// or malformed IDs collapse to empty-member-set semantics (ALLOWLIST -> false, - /// BLOCKLIST -> true). + /// @notice Returns whether `account` is authorized under `policyId`. Never reverts. /// + /// @dev A malformed ID returns false. The invert flag (bit 63) is cleared before the type + /// check, so an ID is malformed when the remaining top byte is outside `PolicyType`. + /// @dev A well-formed but unknown ID is an empty set: ALLOWLIST and UNION return false; + /// BLOCKLIST and INTERSECT return true. /// @dev Callers that store policy IDs MUST validate `policyExists(policyId)` at write time. /// @dev Invert: `isAuthorized(invertedPolicyId(id), account)` returns the negated - /// result of the base. Applies to every policy type. + /// result of the base when that base exists. Applies to every policy type. An + /// inverted unknown or malformed base returns false. /// /// @param policyId Policy to query. /// @param account Account to check. From e83899b58d517d17993d40874f1ce841bdf641ef Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Thu, 1 Oct 2026 11:53:25 -0400 Subject: [PATCH 2/3] test(policy): lock isAuthorized results for bad ids Unknown UNION and INTERSECT ids follow empty-set semantics, a malformed id denies after the invert flag is cleared, and an inverted id is negated only when its base exists. Co-authored-by: Cursor --- test/lib/BaseTest.sol | 13 +++++ test/unit/PolicyRegistry/isAuthorized.t.sol | 22 +++++++- .../PolicyRegistry/isAuthorizedInvert.t.sol | 55 ++++++++++++++++++- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/test/lib/BaseTest.sol b/test/lib/BaseTest.sol index f56a3c72..eeb43b0d 100644 --- a/test/lib/BaseTest.sol +++ b/test/lib/BaseTest.sol @@ -193,6 +193,9 @@ abstract contract BaseTest is Test { /// forced above the `PolicyType` enum range. View queries /// short-circuit these to the "absent" value; mutating calls /// reject via `PolicyNotFound`. + /// @dev The top byte may have bit 63 set, so clearing the invert flag can + /// leave a well-formed type. Use `_malformedBasePolicyId` when the + /// type byte must stay outside `PolicyType` after that clear. function _malformedPolicyId(uint64 seed) internal pure returns (uint64) { uint8 maxValidType = uint8(type(IPolicyRegistry.PolicyType).max); uint8 invalidRange = type(uint8).max - maxValidType; @@ -200,6 +203,16 @@ abstract contract BaseTest is Test { return (uint64(typeByte) << 56) | uint64(seed & ((1 << 56) - 1)); } + /// @notice Maps a fuzz seed to a malformed policy ID whose type byte is outside + /// `PolicyType` even after the invert flag (bit 63) is cleared. + function _malformedBasePolicyId(uint64 seed) internal pure returns (uint64) { + uint8 maxValidType = uint8(type(IPolicyRegistry.PolicyType).max); + // Type bytes 4..127: outside PolicyType, invert flag clear. + uint8 span = 127 - maxValidType; + uint8 typeByte = maxValidType + 1 + uint8(seed % span); + return (uint64(typeByte) << 56) | (seed & ((uint64(1) << 56) - 1)); + } + // ============================================================ // STRING SLOT ENCODING HELPER // ============================================================ diff --git a/test/unit/PolicyRegistry/isAuthorized.t.sol b/test/unit/PolicyRegistry/isAuthorized.t.sol index 0d037624..9cef9af8 100644 --- a/test/unit/PolicyRegistry/isAuthorized.t.sol +++ b/test/unit/PolicyRegistry/isAuthorized.t.sol @@ -24,11 +24,27 @@ contract PolicyRegistryIsAuthorizedTest is PolicyRegistryTest { assertTrue(policyRegistry.isAuthorized(policyId, account)); } - /// @notice Verifies isAuthorized returns false for any id whose top byte - /// is outside the PolicyType enum range. + /// @notice Verifies isAuthorized on an uncreated UNION id returns false + /// @dev An empty child set is an OR over nothing, so the account is denied. + function test_isAuthorized_success_uncreatedUnionReturnsFalse(uint56 counter, address account) public view { + vm.assume(counter > 1); + uint64 policyId = (uint64(uint8(IPolicyRegistry.PolicyType.UNION)) << 56) | uint64(counter); + assertFalse(policyRegistry.isAuthorized(policyId, account)); + } + + /// @notice Verifies isAuthorized on an uncreated INTERSECT id returns true + /// @dev An empty child set is an AND over nothing, so the account is authorized. + function test_isAuthorized_success_uncreatedIntersectReturnsTrue(uint56 counter, address account) public view { + vm.assume(counter > 1); + uint64 policyId = (uint64(uint8(IPolicyRegistry.PolicyType.INTERSECT)) << 56) | uint64(counter); + assertTrue(policyRegistry.isAuthorized(policyId, account)); + } + + /// @notice Verifies isAuthorized returns false for an ID whose type byte is outside + /// PolicyType after the invert flag is cleared. /// @dev Malformed-ID short-circuit returns false rather than reverting. function test_isAuthorized_success_falseForMalformedId(uint64 seed, address account) public view { - uint64 policyId = _malformedPolicyId(seed); + uint64 policyId = _malformedBasePolicyId(seed); assertFalse(policyRegistry.isAuthorized(policyId, account)); } diff --git a/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol b/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol index b1e251bb..5acf877c 100644 --- a/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol +++ b/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol @@ -45,9 +45,28 @@ contract PolicyRegistryIsAuthorizedInvertTest is PolicyRegistryTest { assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } - /// @notice Inverting a malformed base denies. + /// @notice Inverting a malformed base denies. The base type byte stays outside + /// PolicyType after the invert flag is cleared. function test_isAuthorized_success_invertMalformedBaseDenies(uint64 seed, address account) public view { - uint64 base = _malformedPolicyId(seed) & ~INVERTED_POLICY_BIT; + uint64 base = _malformedBasePolicyId(seed); + assertFalse(policyRegistry.isAuthorized(base, account)); + assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); + } + + /// @notice Inverting an uncreated UNION base denies. + function test_isAuthorized_success_invertUnknownUnionBaseDenies(uint56 counter, address account) public view { + vm.assume(counter > 1); + uint64 base = (uint64(uint8(IPolicyRegistry.PolicyType.UNION)) << 56) | uint64(counter); + assertFalse(policyRegistry.isAuthorized(base, account)); + assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); + } + + /// @notice Inverting an uncreated INTERSECT base denies, rather than negating the + /// empty-set result (which would authorize the account). + function test_isAuthorized_success_invertUnknownIntersectBaseDenies(uint56 counter, address account) public view { + vm.assume(counter > 1); + uint64 base = (uint64(uint8(IPolicyRegistry.PolicyType.INTERSECT)) << 56) | uint64(counter); + assertTrue(policyRegistry.isAuthorized(base, account)); assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } @@ -78,6 +97,38 @@ contract PolicyRegistryIsAuthorizedInvertTest is PolicyRegistryTest { assertTrue(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } + /// @notice NOT(UNION): the inverse negates the composite once the base exists. + function test_isAuthorized_success_invertExistingUnionNegates(address account) public { + uint64 childA = _createAllowlist(); + uint64 childB = _createAllowlist(); + uint64 base = + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.UNION, _childIds(childA, childB)); + + assertFalse(policyRegistry.isAuthorized(base, account)); + assertTrue(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); + + _addAllowlistMember(childA, account); + assertTrue(policyRegistry.isAuthorized(base, account)); + assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); + } + + /// @notice NOT(INTERSECT): the inverse negates the composite once the base exists. + function test_isAuthorized_success_invertExistingIntersectNegates(address account) public { + uint64 childA = _createAllowlist(); + uint64 childB = _createAllowlist(); + uint64 base = policyRegistry.createCompositePolicy( + admin, IPolicyRegistry.PolicyType.INTERSECT, _childIds(childA, childB) + ); + + _addAllowlistMember(childA, account); + assertFalse(policyRegistry.isAuthorized(base, account)); + assertTrue(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); + + _addAllowlistMember(childB, account); + assertTrue(policyRegistry.isAuthorized(base, account)); + assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); + } + // ============================================================ // BUILT-IN INVERSION // ============================================================ From 6018956da77a29137d6e5ef2a23dc44661807a9d Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Thu, 1 Oct 2026 12:03:08 -0400 Subject: [PATCH 3/3] test(policy): assert uncreated ids before their inverse Each uncreated policy type records its empty-set result, then asserts the inverted id returns false. Drop the separate default-only and existing-composite cases. Co-authored-by: Cursor --- test/unit/PolicyRegistry/isAuthorized.t.sol | 16 ----- .../PolicyRegistry/isAuthorizedInvert.t.sol | 60 ++++--------------- 2 files changed, 13 insertions(+), 63 deletions(-) diff --git a/test/unit/PolicyRegistry/isAuthorized.t.sol b/test/unit/PolicyRegistry/isAuthorized.t.sol index 9cef9af8..9a2bc5ff 100644 --- a/test/unit/PolicyRegistry/isAuthorized.t.sol +++ b/test/unit/PolicyRegistry/isAuthorized.t.sol @@ -24,22 +24,6 @@ contract PolicyRegistryIsAuthorizedTest is PolicyRegistryTest { assertTrue(policyRegistry.isAuthorized(policyId, account)); } - /// @notice Verifies isAuthorized on an uncreated UNION id returns false - /// @dev An empty child set is an OR over nothing, so the account is denied. - function test_isAuthorized_success_uncreatedUnionReturnsFalse(uint56 counter, address account) public view { - vm.assume(counter > 1); - uint64 policyId = (uint64(uint8(IPolicyRegistry.PolicyType.UNION)) << 56) | uint64(counter); - assertFalse(policyRegistry.isAuthorized(policyId, account)); - } - - /// @notice Verifies isAuthorized on an uncreated INTERSECT id returns true - /// @dev An empty child set is an AND over nothing, so the account is authorized. - function test_isAuthorized_success_uncreatedIntersectReturnsTrue(uint56 counter, address account) public view { - vm.assume(counter > 1); - uint64 policyId = (uint64(uint8(IPolicyRegistry.PolicyType.INTERSECT)) << 56) | uint64(counter); - assertTrue(policyRegistry.isAuthorized(policyId, account)); - } - /// @notice Verifies isAuthorized returns false for an ID whose type byte is outside /// PolicyType after the invert flag is cleared. /// @dev Malformed-ID short-circuit returns false rather than reverting. diff --git a/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol b/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol index 5acf877c..0b6f8473 100644 --- a/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol +++ b/test/unit/PolicyRegistry/isAuthorizedInvert.t.sol @@ -28,32 +28,23 @@ contract PolicyRegistryIsAuthorizedInvertTest is PolicyRegistryTest { // FAIL-CLOSED INVARIANT (the point of 2a) // ============================================================ - /// @notice Inverting an uncreated (unknown) base denies rather than allowing everyone. + /// @notice An uncreated ALLOWLIST denies, and its inverse denies too. function test_isAuthorized_success_invertUnknownAllowlistBaseDenies(uint56 counter, address account) public view { vm.assume(counter > 1); uint64 base = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(counter); + assertFalse(policyRegistry.isAuthorized(base, account)); assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } - /// @notice Inverting an uncreated BLOCKLIST base also denies (fail-closed) + /// @notice An uncreated BLOCKLIST authorizes, and its inverse denies. function test_isAuthorized_success_invertUnknownBlocklistBaseDenies(uint56 counter, address account) public view { vm.assume(counter > 1); uint64 base = (uint64(uint8(IPolicyRegistry.PolicyType.BLOCKLIST)) << 56) | uint64(counter); - // Sanity: the plain unknown blocklist authorizes (empty-member-set semantics)... assertTrue(policyRegistry.isAuthorized(base, account)); - // ...but its inverse must NOT become allow-everyone; the base does not exist. - assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); - } - - /// @notice Inverting a malformed base denies. The base type byte stays outside - /// PolicyType after the invert flag is cleared. - function test_isAuthorized_success_invertMalformedBaseDenies(uint64 seed, address account) public view { - uint64 base = _malformedBasePolicyId(seed); - assertFalse(policyRegistry.isAuthorized(base, account)); assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } - /// @notice Inverting an uncreated UNION base denies. + /// @notice An uncreated UNION denies, and its inverse denies too. function test_isAuthorized_success_invertUnknownUnionBaseDenies(uint56 counter, address account) public view { vm.assume(counter > 1); uint64 base = (uint64(uint8(IPolicyRegistry.PolicyType.UNION)) << 56) | uint64(counter); @@ -61,8 +52,7 @@ contract PolicyRegistryIsAuthorizedInvertTest is PolicyRegistryTest { assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } - /// @notice Inverting an uncreated INTERSECT base denies, rather than negating the - /// empty-set result (which would authorize the account). + /// @notice An uncreated INTERSECT authorizes, and its inverse denies. function test_isAuthorized_success_invertUnknownIntersectBaseDenies(uint56 counter, address account) public view { vm.assume(counter > 1); uint64 base = (uint64(uint8(IPolicyRegistry.PolicyType.INTERSECT)) << 56) | uint64(counter); @@ -70,6 +60,14 @@ contract PolicyRegistryIsAuthorizedInvertTest is PolicyRegistryTest { assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } + /// @notice A malformed base denies, and inverting it denies too. The type byte stays + /// outside PolicyType after the invert flag is cleared. + function test_isAuthorized_success_invertMalformedBaseDenies(uint64 seed, address account) public view { + uint64 base = _malformedBasePolicyId(seed); + assertFalse(policyRegistry.isAuthorized(base, account)); + assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); + } + // ============================================================ // SIMPLE-POLICY INVERSION // ============================================================ @@ -97,38 +95,6 @@ contract PolicyRegistryIsAuthorizedInvertTest is PolicyRegistryTest { assertTrue(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); } - /// @notice NOT(UNION): the inverse negates the composite once the base exists. - function test_isAuthorized_success_invertExistingUnionNegates(address account) public { - uint64 childA = _createAllowlist(); - uint64 childB = _createAllowlist(); - uint64 base = - policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.UNION, _childIds(childA, childB)); - - assertFalse(policyRegistry.isAuthorized(base, account)); - assertTrue(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); - - _addAllowlistMember(childA, account); - assertTrue(policyRegistry.isAuthorized(base, account)); - assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); - } - - /// @notice NOT(INTERSECT): the inverse negates the composite once the base exists. - function test_isAuthorized_success_invertExistingIntersectNegates(address account) public { - uint64 childA = _createAllowlist(); - uint64 childB = _createAllowlist(); - uint64 base = policyRegistry.createCompositePolicy( - admin, IPolicyRegistry.PolicyType.INTERSECT, _childIds(childA, childB) - ); - - _addAllowlistMember(childA, account); - assertFalse(policyRegistry.isAuthorized(base, account)); - assertTrue(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); - - _addAllowlistMember(childB, account); - assertTrue(policyRegistry.isAuthorized(base, account)); - assertFalse(policyRegistry.isAuthorized(base | INVERTED_POLICY_BIT, account)); - } - // ============================================================ // BUILT-IN INVERSION // ============================================================