Access control tests by mts1715 · Pull Request #244 · onflow/FlowALP

mts1715 · 2026-03-06T15:15:34Z

Closes: #177

Summary

Implements the full Actor Capability Matrix test suite for FlowALPv0.Pool entitlements, as defined in docs/security-permission-matrix.md

Description

`cap_test.cdc`

The previous file only validated pool creation. This PR replaces it with a structured security test suite covering every row in the permission matrix across all 6 entitlements — one test per matrix operation, organised in sections matching the matrix columns.

Negative test strategy

Cadence entitlements for Pool capabilities (EParticipant, EPosition, ERebalance, EGovernance, EImplementation) are enforced by the cadence type checker.
EPositionAdmin is the only entitlement in this file where negative (access-denied) tests are meaningful at runtime by borrowAuthorizedPosition, so testEPositionAdmin_BorrowUnauthorizedPosition_Fails exists.

Transaction reorganisation

Old pool-management/ and pool-governance/ stubs removed. Replaced by entitlement-scoped folders matching the matrix columns:
egovernance, eimplementation, eparticipant, eposition, epositionadmin, erebalance, setup (grant_e*_cap.cdc helpers), helpers (liquidation)

`docs/security-permission-matrix.md`

Added Test Coverage table mapping each test file to the matrix rows it covers.
Added Audit Notes section documenting union/conjunction semantics (EPosition | ERebalance for rebalance ops; FungibleToken.Withdraw + EPositionAdmin conjunction for borrowAuthorizedPosition).
Clarified which rows are covered by paid_auto_balance_test.cdc (rebalancer-local entitlements) and withdraw_stability_funds_test.cdc (withdrawStabilityFund).

Known issue documented (not fixed here)

publish_beta_cap.cdc grants EParticipant + EPosition to beta users. EPosition is not needed for normal user actions and allows any beta user to withdraw from or freeze any other user's position. The over-grant is explicitly tested in the eParticipantPositionUser section. Fix: grant EParticipant only — tracked separately.

Maps all FlowALPv0 entitlements to operations by resource, with plain-language descriptions. Intended for audit/security review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Mark EPosition as protocol-internal only, not for end users - Add ownership-check warnings on all pool-level EPosition operations - Document the beta capability over-grant issue (EPosition -> EParticipant fix needed) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Replace resource-grouped columns with actor columns (User, User w/ EPosition, Rebalancer, Position Owner, Governance, Protocol Internal). The beta over-grant is now directly visible as a dedicated column showing what current beta users can do vs. what they should be able to do. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…rol-test

…rol-test # Conflicts: # cadence/tests/test_helpers.cdc

…capability-based borrow

Kay-Zee

I think there are a few test cases that I would love to have additional verification that the intended action did actually happen, and it was not just that the transaction was successful, but possibly didn't take the intended action, things like balance checks, new value checks, etc. For certain actions where this is harder to do, we can leave for later but the easy ones, that i mentioned i think might make sense to just do here.

Also, would love for the eGovernance tests to have their "negative" case checked, just because those are especially sensitive. It's a little bit "frivolous" as in we're just checking cadence is behaving as expected, but i think it should be pretty easy/straightforward, and gives some added confidence.

Kay-Zee · 2026-03-11T18:38:10Z

cadence/tests/transactions/flow-alp/egovernance/collect_stability.cdc

+import "FlowALPv0"
+import "FlowALPModels"
+
+/// TEST TRANSACTION - DO NOT USE IN PRODUCTION


These are all really test transactions only? seems like they technically would still work in prod, no? assuming the account has the right entitlement?

it can, I've added this mark about "test transaction" because it should be rechecked before using in prod.
Removed at transactions that could be used in prod: 9bfe703

Kay-Zee · 2026-03-11T18:42:11Z

cadence/tests/transactions/flow-alp/eparticipant/create_and_deposit_via_cap.cdc

+/// Uses the cap stored at FlowALPv0.PoolCapStoragePath.
+///
+/// NOTE: All logic is in prepare because @Position resources cannot be stored as
+/// transaction fields, and execute has no storage access. The prepare-only pattern


execute has no storage access

https://cadence-lang.org/docs/language/transactions#execute-phase

I don't believe that statement is true, si ti? the example on the doc is literally signer.storage.load

Also, why can't @Position resources cannot be stored as transaction fields?

this was my fault and misunderstanding.
fix ff18746

Kay-Zee · 2026-03-11T18:47:43Z

cadence/tests/transactions/flow-alp/erebalance/rebalance_position_via_cap.cdc

+
+    execute {
+        // Pool.rebalancePosition — requires EPosition | ERebalance; ERebalance alone is sufficient
+        self.pool.rebalancePosition(pid: pid, force: force)


very similar to the tx above, except goes through the capability, wonder if there's any way to better differentiate the two tx, maybe in naming? anyway, just minor nit.

fix:
9bfe703

Kay-Zee · 2026-03-11T18:55:05Z

cadence/tests/cap_test.cdc

+        [setupPid, 1.0],
+        eParticipantPositionUser
+    )
+    Test.expect(result, Test.beSucceeded())


could we maybe also do an expect of the balance to make sure the withdraw actually withdrew some amount of tokens, and not just that the withdraw tx succeeded (but didn't withdraw somehow)

fix 63cfe76

Kay-Zee · 2026-03-11T18:55:40Z

cadence/tests/cap_test.cdc

+        [setupPid, 1.0],
+        eParticipantPositionUser
+    )
+    Test.expect(result, Test.beSucceeded())


same comment as above, would love to also verify balance

fix 63cfe76

Kay-Zee · 2026-03-11T18:58:30Z

cadence/tests/cap_test.cdc

+
+    let result = _executeTransaction(
+        "../tests/transactions/flow-alp/erebalance/rebalance_pool.cdc",
+        [setupPid, true],


This is showcasing that the rebalance user can rebalance the protocol accounts position.

That's what we're trying to show with this test? I thought the rebalance user would only rebalance their own position.

check Beta Capability Over-Grant according to security matrix:
https://github.com/onflow/FlowALP/blob/2780290b22d205076bd67c1e8c00a55266bb683f/docs/security-permission-matrix.md

Kay-Zee · 2026-03-11T18:58:52Z

cadence/tests/cap_test.cdc

+        [ePositionAdminPid, TARGET_HEALTH],
+        ePositionAdminUser
+    )
+    Test.expect(result, Test.beSucceeded())


could we verify the new target health?

fix 63cfe76

fix phases in transactions: execute block executes the main logic of the transaction.

…sed in prod; fix pool_rebalance transactions naming

…pen at capability test

…rol-test # Conflicts: # FlowActions # cadence/tests/transactions/flow-alp/pool-management/04_create_position.cdc

holyfuchs · 2026-03-19T14:29:02Z

cadence/tests/transactions/flow-alp/egovernance/collect_insurance.cdc

We already have ./cadence/transactions/flow-alp/pool-governance/collect_insurance.cdc.
I don't see an issue with having separate copies for the test transactions; in fact, it might be the correct approach, as transactions outside the test folder should adhere to stricter rules.
Just wanted to confirm this was a deliberate choice.

cadence/transactions/flow-alp/pool-governance/collect_insurance.cdc: directly borrows the Pool from storage (for the pool admin)
cadence/tests/transactions/flow-alp/egovernance/collect_insurance.cdc: borrows a Capability from PoolCapStoragePath (for eGovernanceUser who only has a delegated capability, not direct storage access)

Switching cap_test.cdc to use the pool-governance version would break testEGovernance_CollectInsurance since eGovernanceUser doesn't have the pool in their storage.

…rol-test

Kay-Zee

LGTM, had some idea for some "negative" tests, i.e. show actions being blocked without the right entitlements, but that's probably a bit unnecessary

Kay-Zee · 2026-03-24T22:22:48Z

cadence/tests/cap_test.cdc

+        [ePositionAdminPid],
+        ePositionAdminUser
+    )
+    Test.expect(result, Test.beSucceeded())


at least for the EGovernance cap, i wonder if we can showcase all of these functions failing if you don't have the cap? I think these ones, since they're especially sensitive, would be good to show the counter case of them not working without the cap.

fix 361eea0

…ariants for all 11 EGovernance methods

vishalchangrani and others added 8 commits February 19, 2026 11:56

add security permission matrix doc

c38635e

Maps all FlowALPv0 entitlements to operations by resource, with plain-language descriptions. Intended for audit/security review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs: clarify EPositionAdmin is user acting on own positions via storage

2780290

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Merge branch 'main' into taras/177-access-control-test

4265852

Add comprehensive entitlement/capability access-control test suite

f9f728b

fix comments

491193a

Merge remote-tracking branch 'origin/main' into taras/177-access-cont…

4a2ea5e

…rol-test

mts1715 self-assigned this Mar 6, 2026

mts1715 requested a review from a team as a code owner March 6, 2026 15:15

mts1715 added 2 commits March 10, 2026 21:54

Merge remote-tracking branch 'origin/main' into taras/177-access-cont…

9e0b53b

…rol-test

fix wrong path for capability EGovernance

bd0dc05

mts1715 requested review from vishalchangrani March 10, 2026 21:11

mts1715 added 2 commits March 10, 2026 22:33

move some scripts from the helper folder to appropriate paths

dee9360

Merge remote-tracking branch 'origin/main' into taras/177-access-cont…

1a0d372

…rol-test # Conflicts: # cadence/tests/test_helpers.cdc

mts1715 requested review from Kay-Zee, holyfuchs and nialexsan March 10, 2026 21:37

mts1715 added 2 commits March 10, 2026 22:41

typo fix

27dc3b5

fix pool_pause_test.cdc to change from direct pool storage borrow to …

ca275cf

…capability-based borrow

Kay-Zee reviewed Mar 11, 2026

View reviewed changes

mts1715 added 7 commits March 16, 2026 16:20

fix false description statement in comments;

ff18746

fix phases in transactions: execute block executes the main logic of the transaction.

fix comments "do not use in production" at transactions that can be u…

9bfe703

…sed in prod; fix pool_rebalance transactions naming

remove doubled test scripts; add some check to test

63cfe76

add additional verification that the intended action did actually hap…

628fd86

…pen at capability test

added neg_* tests for Egovernance methods

7098d3c

Merge remote-tracking branch 'origin/main' into taras/177-access-cont…

dcd8ddd

…rol-test # Conflicts: # FlowActions # cadence/tests/transactions/flow-alp/pool-management/04_create_position.cdc

hotfix after merge

218fe0f

mts1715 requested a review from Kay-Zee March 17, 2026 11:06

UlyanaAndrukhiv requested a review from a team March 17, 2026 15:55

holyfuchs approved these changes Mar 19, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into taras/177-access-cont…

6dd9e24

…rol-test

mts1715 requested review from a team and holyfuchs March 23, 2026 17:34

Kay-Zee approved these changes Mar 24, 2026

View reviewed changes

mts1715 added 2 commits March 25, 2026 17:52

move EGovernance neg tests to egovernance_neg/ and add without_auth v…

361eea0

…ariants for all 11 EGovernance methods

Merge branch 'main' into taras/177-access-control-test

6fd9946

mts1715 merged commit b9d0051 into main Mar 25, 2026
1 check passed

mts1715 deleted the taras/177-access-control-test branch March 25, 2026 17:25

Conversation

mts1715 commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Description

cap_test.cdc

Negative test strategy

Transaction reorganisation

docs/security-permission-matrix.md

Known issue documented (not fixed here)

Uh oh!

Kay-Zee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mts1715 Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mts1715 Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kay-Zee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mts1715 commented Mar 6, 2026 •

edited

Loading

`cap_test.cdc`

`docs/security-permission-matrix.md`

mts1715 Mar 16, 2026 •

edited

Loading

mts1715 Mar 16, 2026 •

edited

Loading