Flip `AuthSingleSigAcl` semantics from trigger list to exempt list by Fumuran · Pull Request #3065 · 0xMiden/protocol

Fumuran · 2026-06-09T05:38:05Z

Part of #2964.

Summary

Flips AuthSingleSigAcl from a trigger-list ACL (procedures that require a signature) to an exempt-list ACL (procedures that are exempt from requiring a signature). Everything not on the exempt list defaults to requiring a signature — so a forgotten setter can no longer silently become permissionless.

Drops the allow_unauthorized_output_notes / allow_unauthorized_input_notes configuration flags and the config storage slot. Renames the storage map to exempt_procedure_roots.

Authentication logic

Authentication is required when any of the following hold:

A kernel-detected account procedure (other than the auth procedure at index 0) was called and is not on the exempt list.
Any output note was created — unconditional. output_note_create does not trigger was_procedure_called, so the only way to gate unsigned note emission is here. This line is marked SECURITY-CRITICAL in the MASM and must stay unconditional.
Any input note was consumed AND no detected exempt procedure ran anywhere in the transaction. The vouching here is transaction-wide, not per-note — a single detected exempt call lifts the input-note signature requirement for every input note in the same transaction. Documented as a footgun on both the type and on with_exempt_procedures.

Iteration mirrors the multisig compute_transaction_threshold pattern (skip index 0, was_procedure_called against each, lookup against the exempt map for called procs).

Notable behavior change

The AuthControlled + SingleSig faucet now uses an empty exempt list, so the burn/receive path that previously rode in unsigned via allow_unauthorized_input_notes(true) now requires the owner's signature. Intentional tightening — flagged in the CHANGELOG and in the factory code comment.

Closes: #3068

Replace the previous trigger-list ACL with an exempt-list ACL: every called account procedure requires a signature by default, and only procedures listed in the exempt map are allowed to execute without authentication. This removes a footgun where forgetting to register a new setter in the trigger list silently left it permissionless, and lets us drop the `allow_unauthorized_output_notes` and `allow_unauthorized_input_notes` configuration flags. Under the new model the input/output note checks become implicit: any side-effecting note consumption goes through an account procedure call, so the procedure check covers it. - MASM: rewrite `auth_tx_acl` to iterate account procedures (skipping index 0, the auth proc itself), check `was_procedure_called`, and look up each called proc in the exempt map. Mirrors the structure of `multisig::compute_transaction_threshold`. - Storage layout: rename `trigger_procedure_roots` slot to `exempt_procedure_roots`, keyed by procedure root with `[1, 0, 0, 0]` presence marker. Drop the now-unused `config` slot. - Faucet factory: `AuthControlled + SingleSig` now uses an empty exempt list, so every authority-gated setter requires a signature including the burn/receive path that previously rode in unsigned. - Test helper `Auth::Acl` and integration tests flipped to the new semantics, with added invariant coverage for the empty-exempt-list default and for transactions that mix exempt and non-exempt calls. Part of #2964.

Part of #2964.

- Validate exempt_procedures uniqueness in AuthSingleSigAcl::new so a caller passing duplicate procedure roots receives an AccountError instead of panicking inside StorageMap::with_entries. Add a unit test that exercises the rejection path. - Refresh the stale "Safe to unwrap because we know that the map keys are unique" comment to reference the validation source. Correct the pre-existing # Panics doc on `new` (it returns Result for the length check, never panics). - Reword the integration-test assertion message that read as contradictory next to nonce_delta == 0. - Note in the CHANGELOG that the no-signature hot path now iterates every account procedure (two kernel calls each), so it costs more than the old trigger-list iteration that was bounded by the trigger count.

Security review caught a real auth bypass: deleting the explicit `tx::get_num_output_notes` / `tx::get_num_input_notes` gates relied on `was_procedure_called` being set for every side-effecting procedure, but the kernel only flags procedures that touch account-restricted APIs. `output_note_create` and `output_note_add_asset` do *not* trip that flag (only `assert_native_account`), so any account procedure that emits notes without also moving assets through the vault would have executed unsigned. Asset-bearing flows stay safe because vault add/remove is tracked, but the "note checks are implicit" claim was false for asset-less note emission. This restores the issue's original three-invariant design: 1. Any kernel-detected called procedure not on the exempt list forces `auth_required`. 2. Any output note creation forces `auth_required` unconditionally (this is the gap that `was_procedure_called` cannot close). 3. Any input note consumption forces `auth_required` unless an exempt procedure was detected as called during the transaction (the consumption is then vouched for by the exempt call). The MASM now tracks `any_exempt_called` via a local across the procedure loop so the input-note gate can distinguish vouched consumption from raw consumption. The Rust module doc, the on-type "procedure detection" callout, and the CHANGELOG entry are updated to state precisely which gaps `was_procedure_called` leaves and why the explicit note checks remain. A new integration test `test_acl_exempt_detected_procedure_succeeds_without_auth` exercises the positive exempt-map lookup with a kernel-detected procedure (`get_item`) so a regression that inverted the marker would be caught.

Code review flagged that the new condition 2 ("any output note created → auth_required, unconditionally") had no regression coverage. Adding a test where the procedure that emits the note (BasicWallet's move_asset_to_note) is on the exempt list, so condition 1 is satisfied and the only thing forcing authentication is the output-note check itself. Without an authenticator the tx must fail with MissingAuthenticator.

Security review caught a documentation mismatch: the prior wording ("vouched for by an exempt procedure") read as per-note vouching, but `ANY_EXEMPT_CALLED_LOC` is a transaction-wide flag — a single detected exempt call lifts the input-note signature requirement for every input note in the same transaction, not only the notes that procedure handled. Asset exfil is still blocked by the unconditional output-note gate, but exempting any detected procedure relaxes input-note auth transaction-wide, which integrators should know. - Rewrite condition 3 in the Rust doc to state the global scope explicitly, with a usage guidance line. - Mirror the clarification in the MASM `ANY_EXEMPT_CALLED_LOC` rationale. - Add a SECURITY-CRITICAL invariant comment on the unconditional output-note `or` (the line is the only barrier against an exempt procedure emitting notes via `output_note_create`). - Update the CHANGELOG bullet with the same global-scope wording. Also addressing a code review nit: `test_acl_mixed_exempt_and_protected_requires_auth` previously exempted `account_procedure_1` (undetected), so the test never actually exercised the "detected-exempt alongside detected-non-exempt" branch. Switch to exempting `get_item` and calling both `get_item` + `set_item` so a regression that let a detected exempt call suppress the non-exempt auth requirement would be caught.

- Add `test_acl_input_note_consumption_requires_auth_without_exempt`: empty exempt list, no tx script, consume the mock note → expect MissingAuthenticator. Without this, the input-note branch of the MASM `and or` could be deleted and no test would fail (every other no-auth note test also calls a non-exempt detected procedure that trips condition 1 first). - Sharpen the cost note in the CHANGELOG to mention the per-detected- call map lookup (was undercounted as "two kernel calls each"). - Add a pointer on `AuthSingleSigAclConfig::with_exempt_procedures` warning about the transaction-wide scope of condition 3, so callers see it at the configuration site, not only on the type.

…inglesig-acl-semantics

- auth_scheme_slot_schema: switch `//` to `///` so the doc renders consistently with the sibling slot-schema accessors. - faucet_contract_creation: probe the full former trigger set (mint_and_send + 4 metadata setters + 4 policy setters + pause + unpause) for absence from the exempt map, rather than only three representative roots. A regression that put any of these back into the exempt map will now surface here.

- Add `test_singlesig_acl_rejects_exempt_list_above_account_limit` so the two error paths in `AuthSingleSigAcl::new` (duplicates and over-limit) have symmetric coverage. - Move `alloc::collections::BTreeSet` to a top-of-file `use` to match the surrounding import style (`alloc::vec::Vec` is already imported this way).

Restructure the three-condition auth description into two: condition 1 combines the per-procedure check and the input-note vouching clause (they are logically coupled and condition 1 already handles the non-exempt-called half through the exempt-list lookup), and explicitly spells out that input-note consumption with no procedures called at all still requires authentication. Condition 2 (output notes) stays unconditional and is unchanged. Apply the same restructuring to the MASM docstring, the CHANGELOG entry, and the cross-references in the `with_exempt_procedures` setter and the procedure-detection note. The implementation is unchanged - this is purely a docs revision. The transaction-wide vouching behavior remains as-is pending further discussion on whether to introduce a separate per-note-vouching opt-in.

Restructure the auth-logic description into three conditions, matching the issue's original spec but with a sharper wording for the input-note check: 1. A kernel-detected non-exempt procedure was called. 2. An input note was consumed AND no procedure was detected as called at all. 3. Any output note was created (unconditional). Conditions 1 and 2 are logically equivalent in outcome to the previous formulation ("input consumed AND no exempt called"), because condition 1 already catches the non-exempt-called case. Stating condition 2 as "no procedure called at all" makes the easy-to-miss case (input consumed but no account procedure invoked) explicit. Update the MASM to literally match the new wording: track ANY_PROC_CALLED_LOC (set to 1 whenever was_procedure_called returns true) instead of ANY_EXEMPT_CALLED_LOC. The flag-update site simplifies from a load-or-store to an idempotent `push.1 loc_store`. All 16 integration tests and 4 unit tests still pass without modification, confirming the outcome equivalence. Apply the same restructuring to the Rust type doc, the with_exempt_procedures setter, the procedure-detection note, and the CHANGELOG entry. Update two test docstring cross-references to use the new condition numbering. The footgun warning about transaction-wide vouching remains in place.

…inglesig-acl-semantics

Tighten the procedure-iteration loop: instead of iterating num_procedures-1 down to 0 and gating the body on `dup neq.0` to skip the auth procedure at index 0, iterate down to 1 directly. The initial guard becomes `dup neq.1` (don't enter the loop if num_procedures == 1, i.e. only the auth procedure is installed) and the continuation check also becomes `dup neq.1`. This removes the inner `if.true ... end` block that was gating every iteration, saving a few cycles per procedure on the no-signature hot path while preserving the index-0 skip property (the auth procedure is guaranteed at index 0 by the account-code builder, so by never decrementing below 1 we never touch it). Replace "input-note" with "input note" in the comments and docs of files this branch introduced or modified.

Shrink the doc comment on `ANY_PROC_CALLED_LOC` to four lines explaining the flag's role: it tracks "any procedure detected as called" and gates the input note check on whether at least one procedure ran, with a note that non-exempt detected calls already force authentication via the per-procedure check inside the loop. Replace every numbered authentication-condition reference ("condition 1", "condition 2", "condition 3") in the type doc, MASM docstring, CHANGELOG entry, `with_exempt_procedures` setter, the procedure- detection note, and the integration test docstrings with named rules: "non-exempt proc was called", "input note was consumed, but no proc was called", and "output note was created". Numbered references couple prose to a particular ordering and are fragile under future doc reshuffles; named references are stable.

- MASM: trim the slot comment on EXEMPT_PROCEDURE_ROOTS_SLOT to two lines; drop the "most easily missed" trailing sentence and the output-note explanation from the auth_tx_acl docstring; remove the redundant multi-line explanation above the input note check (the docstring already covers the same ground). - Faucet factory: shorten the AuthControlled + SingleSig comment by dropping the historical reference to allow_unauthorized_input_notes. - Errors: add a typed `AccountError::DuplicateExemptProcedure(_)` variant carrying the offending procedure root, and replace the duplicate-check `AccountError::other` call with it. The check now short-circuits in a single forward pass that identifies which root was duplicated. - Tests: hoist the per-function `use` block in `test_acl_output_note_creation_requires_auth_even_when_caller_exempt` up to the top of the file, alongside the other module-level imports; rewrite the explicit `TransactionScript::from(...)` cast to a typed binding plus `.into()`. - CHANGELOG: rewrite the entry to fit in under 400 characters by deferring the full auth-logic description to the type doc.

Convert "non-exempt proc was called", "input note was consumed, but no proc was called", and "output note was created" from literal quoted labels (used everywhere the rules are referenced) into natural prose descriptions: "the non-exempt proc check", "the input note check", and "the output note check". The enumeration in the type doc and MASM docstring is now numbered 1./2./3. with descriptive bullet bodies, and all references in the surrounding prose, the `with_exempt_procedures` setter, the procedure-detection note, and the integration test docstrings use the short forms.

- MASM: drop the SECURITY-CRITICAL comment block above the unconditional output-note `or`. The section header on the line above already names the invariant and the reviewer considered the explanation redundant. - singlesig_acl.rs: remove the per-setter warning on `with_exempt_procedures` (already covered by the type doc), shorten the transaction-wide-vouching paragraph to its first sentence, drop the "auth runs after the rest of the transaction" paragraph, and compress the "Important Note on Procedure Detection" section into eight lines while preserving the practical guidance. - CHANGELOG: shorten the entry to a single sentence and switch the reference from #2964 (issue) to #3065 (this PR), per project convention.

bobbinth · 2026-06-13T06:40:27Z

Is this ready for review? I see it is not in draft mode, but no reviewers are assigned.

Fumuran · 2026-06-15T07:48:03Z

It is already in a OK shape, but I still want to do a few more reviews to make sure it is ready. The intention was "if you have time, I will be glad if you look at the code, but strictly speaking it's not in its final shape". Though I'll finish the self-review in a few hours and request the reviews.

- MASM: apply the requested commentary edits in `auth_tx_acl` (add a Locals section header on the procedure, reword the section header for the iteration block, clarify the initialization comments for the any_proc_called flag and auth_required, restate the loop comment as a single line, rewrite the post-was_called marker comment to mention the input note check it gates, shorten the exempt-map lookup comment, and tighten the post-loop stack annotation). - AuthSingleSigAclConfig: switch `exempt_procedures` from `Vec<AccountProcedureRoot>` to `BTreeSet<AccountProcedureRoot>`. Uniqueness is now enforced by the type, so the runtime duplicate check in `AuthSingleSigAcl::new` and the `DuplicateExemptProcedure` variant on `AccountError` (along with the corresponding unit test) are dropped. The "safety" comment on the now-trivially-infallible `StorageMap::with_entries(...).unwrap()` is removed, and the surrounding storage-slot comment is reworded per the reviewer's suggestion. - Update the `Auth::Acl` mock-chain helper variant and every integration test call site to use `BTreeSet::from([...])`. - Trim the doc comment on the procedure-detection note and rewrite the `# Panics`-style block on `AuthSingleSigAcl::new` as a bullet list under "Returns an error if". - Faucet construction test: apply the wording suggestion on the empty-exempt-map probe comment.

- Faucet construction test: rejoin the "Probe the full" / "former trigger set" fragments back into a single sentence; the PR-suggested comment had been pasted with a stray mid-clause newline. - MASM auth_tx_acl: - shorten the procedure-iteration section header so it fits in 100 columns, - trim the "no authentication required by default" comment by one word so it fits, - wrap the loop-direction comment over two lines, - reflow the `was_called` marker comment block onto sentence boundaries (the previous break landed mid-clause after "Setting to 1"), and - wrap the exempt-map lookup comment so the marker value sits on its own line.

PhilippGackstatter

Exempt logic looks good, but I would remove the input and output note checks.

PhilippGackstatter · 2026-06-16T09:08:44Z

+#! Authentication is required if any of the following hold:
+#!   1. A kernel-detected procedure not on the exempt list was called (other than the
+#!      auth procedure at index 0).
+#!   2. An input note was consumed AND no procedure was detected as called anywhere in
+#!      the transaction. Combined with the non-exempt proc check above, the practical
+#!      effect is that input note consumption requires a signature unless at least one
+#!      exempt procedure was detected as called.
+#!   3. An output note was created.


As mentioned here and here, I'd simplify this logic:

I think the ACL logic can stay pretty simple and only protect account procedures, and ignore whether input or output notes were consumed.

So points 2 and 3 can be removed, imo.

Creating an output note is not dangerous in and of itself. What is dangerous is adding assets from the account to that note. This requires calling native_account::remove_asset, which would generally not be exempt from signatures here, and so I don't think we need any note check.

I think the same logic holds for input notes: If you consume two PSWAP notes, and they cleanly cancel out, no signature should be required.

PhilippGackstatter · 2026-06-16T09:20:03Z

-
-    exec.tx::get_num_output_notes
-    # => [num_output_notes, require_acl_auth, pad(16)]
+    # => [1, auth_required, pad(16)]


Suggested change

# => [1, auth_required, pad(16)]

# => [i = 1, auth_required, pad(16)]

nit: describe what 1 is here

claude and others added 12 commits June 8, 2026 22:13

docs: changelog entry for AuthSingleSigAcl exempt-list flip

2fc07ba

Part of #2964.

Merge remote-tracking branch 'origin/next' into fumuran-claude/flip-s…

947e32e

…inglesig-acl-semantics

style: replace em dashes with simple dashes per project convention

22465b1

Fumuran linked an issue Jun 9, 2026 that may be closed by this pull request

Flip ACL semantics to default-require-signature #3068

Open

PhilippGackstatter mentioned this pull request Jun 11, 2026

refactor: merge AuthMethod into AccessControl #2944

Open

2 tasks

claude and others added 4 commits June 11, 2026 12:17

Merge remote-tracking branch 'origin/next' into fumuran-claude/flip-s…

feb9632

…inglesig-acl-semantics

Fumuran commented Jun 11, 2026

View reviewed changes

claude added 2 commits June 11, 2026 14:30

Fumuran mentioned this pull request Jun 11, 2026

Use the network-account fee-payment model for no-signature public-account transactions #3069

Open

Fumuran commented Jun 11, 2026

View reviewed changes

Fumuran marked this pull request as ready for review June 11, 2026 18:34

Merge branch 'next' into fumuran-claude/flip-singlesig-acl-semantics

40bd96e

Fumuran added the pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority label Jun 15, 2026

Fumuran commented Jun 15, 2026

View reviewed changes

claude added 2 commits June 15, 2026 14:01

Fumuran requested review from PhilippGackstatter and bobbinth June 15, 2026 15:47

Fumuran changed the title ~~flip AuthSingleSigAcl from trigger list to exempt list~~ Flip AuthSingleSigAcl semantics from trigger list to exempt list Jun 15, 2026

PhilippGackstatter reviewed Jun 16, 2026

View reviewed changes

	# => [1, auth_required, pad(16)]
	# => [i = 1, auth_required, pad(16)]

Conversation

Fumuran commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Authentication logic

Notable behavior change

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobbinth commented Jun 13, 2026

Uh oh!

Fumuran commented Jun 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fumuran commented Jun 9, 2026 •

edited

Loading