fix(asserter-consumer): pin trusted addresses and gate create_assertion_as_self (closes #157)

[rudrasatani13]

Closes #157

The gap

create_assertion_as_self had no require_auth() at all, and built its authorize_as_current_contract pre-authorization from caller-supplied tholos_id and token_id:

env.authorize_as_current_contract(... SubContractInvocation {
    context: ContractContext {
        contract: token_id,          // caller-supplied
        fn_name: Symbol::new(&env, "transfer"),
        args: (... curr_contract, tholos_id, bond_amount ...),   // caller-supplied destination
    },
    ...
}));

An attacker passes a token this contract holds plus a Tholos instance they control. The pre-authorization then satisfies the auth requirement for a transfer out of this contract's own balance, repeatably.

The fix

The trusted instance and token are stored once by a new admin-gated initialize and read from storage, so no caller input reaches the pre-authorization at all. bond_amount is the only remaining caller-supplied value and is bounded by this contract's own balance. The call is admin-gated, because it spends the contract's funds.

One deliberate deviation, please push back if you disagree

The issue sketched initialize(env, admin, tholos_id, token_id). I split that into __constructor(env, admin) plus initialize(env, tholos_id, token_id).

The reason is #214, merged two days ago: contracts/tholos moved admin pinning into __constructor specifically to close initialize front-running. Taking admin as an initialize argument reintroduces that hole here — the first caller wins and chooses the admin, which for this contract means choosing who may spend its balance. Both contracts/tholos and contracts/tholos-v2 now use the constructor-then-initialize split, so this matches the pattern the issue asked to match.

Two things worth a specific look

tholos_client::Error gained two variants. The consumer's entry points already return that enum, so AlreadyInitialized (5) and NotInitialized (6) went there rather than into a second error type — which is what tholos-client's own doc comment exists to prevent. Discriminants 1–4 are unchanged. This is the only file outside contracts/asserter-consumer, and it is the change most likely to draw an objection, so it is worth flagging rather than burying.

get_status keeps its tholos_id parameter. It moves no funds, no scope bullet mentions it, and an arbitrary instance there is a read the caller could already make directly. Narrowing it would widen this change without closing anything. Happy to change it if you would rather it read the configured value.

I did not commit test_snapshots churn

Running the suite rewrites test_snapshots/**, including six demo-consumer files I never touched. The cause is environmental, not this change: my locally built tholos.wasm hashes to 6adc0aad… while the committed snapshots record e72cd465…, so the recorded wasm hash differs by build platform. I reverted all of it and left a source-only diff.

That reading comes from CONTRIBUTING: commit a snapshot "if running the test again always produces the same file", and note that "nothing in the repo reads these files back for comparison anyway". Mine would not reproduce on your machine, so they seemed like the wrong thing to add. Happy to include them if you would rather have the artifacts — just say so and I will regenerate.

Verification

CI's own gate sequence, run locally:

cargo fmt --check                                          OK
shellcheck -x scripts/*.sh scripts/lib/*.sh                OK
cargo build -p tholos --target wasm32v1-none --release --locked        OK
cargo clippy --workspace --all-targets --locked -- -D warnings         OK
cargo test --workspace --locked                            229 passed, 0 failed
cargo build --workspace --lib --target wasm32v1-none --release --locked  OK

New tests (10 total in asserter-consumer, all passing):

• the happy path, still deliberately narrow-mocking auth past construction so authorize_as_current_contract is genuinely proven to grant the nested transfer auth — plus a new assertion that the bond lands on the configured instance and nowhere else;
• create_assertion_as_self before initialize → NotInitialized;
• initialize twice → AlreadyInitialized, so the trusted addresses are provably write-once;
• both entry points panic with no auth mocked at all (set_auths(&[])), which is what an arbitrary non-admin caller sees;
• the three existing error paths (uninitialized Tholos, paused Tholos, invalid Tholos id) kept, reconfigured through initialize instead of per-call arguments.

This is AI-assisted implementation. The repo states no policy either way on that, and the PR is yours to judge on its merits — but you should know how it was produced rather than infer it.

[rudrasatani13]

Ready for review.

One thing worth flagging so the empty checks are not misread: the CI run on this PR is at action_required — GitHub holds workflow runs from first-time contributors until a maintainer approves them, so nothing has executed yet. Approving the run is all it needs.

Everything was verified locally before pushing, on a clean checkout of 2414ac5 with the pinned toolchain:

• cargo build -p tholos --target wasm32v1-none --release
• cargo test (workspace)
• cargo clippy --workspace --all-targets -- -D warnings
• cargo fmt --check

The PR body lists what changed and the one deliberate deviation from the issue's proposed approach (constructor-then-initialize, because #214 moved admin pinning into __constructor). Happy to adjust any of it.

[collinsezedike]

@rudrasatani13 Closing this, issue #157 is assigned to another contributor who asked for it first. Feel free to pick up another open issue.