[zzzbedream]

Closes #529

The problem

initialize() was an unprotected initializer. It checked has_admin() and then
called set_admin() with no require_auth() on the address being installed and
no authorisation check of any kind on the caller. Deployment and initialisation
were separate transactions, so anyone observing the ledger between the two could
call initialize() with their own address and take the contract.

The admin is not a nominal role — it is the only address that can call
record_payment, allow_asset, revoke_asset, set_allow_native, set_paused,
upgrade, upgrade_storage, rebuild_history_index and propose_admin. The
outcome is unrecoverable: once an attacker holds the role, initialize() returns
AlreadyInitialized to the legitimate operator forever, and propose_admin
requires the current admin's authorisation, so the role cannot be taken back.

Why require_auth() alone does not fix this

This is the part worth reviewing carefully. Adding admin.require_auth() to
initialize() satisfies the letter of the issue title, but it does not close
the race. An attacker can call initialize() with their own address and sign
for it themselves — require_auth() is satisfied, and they still win. All
require_auth() buys is that the role cannot be assigned to an address that did
not consent to receive it.

Closing the race requires removing the window itself, which means initialising
inside the deploy transaction.

The fix

Two changes:

• __constructor(admin: Option<Address>) installs the admin inside the
  deploy transaction. Some(admin) is the new documented deployment path and
  leaves no ledger state in which the contract exists without an admin — the
  front-run window is gone, not narrowed. None deploys uninitialised, keeping
  that state representable for the migration and upgrade paths (and their tests)
  and deferring to initialize().
• initialize() now requires admin.require_auth(), so the role can never
  be assigned to an address that did not consent to receive it.

Both entry points share init_contract_state() so their behaviour cannot drift.

Acceptance criteria

Criterion	Where
initialize() fails unless the installed admin authorises	lib.rs — admin.require_auth() in initialize
An account cannot install a different address as admin	test_initialize_cannot_install_an_unconsenting_third_party_as_admin
A test exercises unauthorised-initialise without mock_all_auths	test_initialize_rejected_under_real_authorization
Every admin-gated entry point has a real-authorisation test	20 gated entry points, 20 enforce_real_auth tests, 1:1
Deployment is atomic and verifies the resulting admin	deploy.sh, Makefile, manifests/*.toml
The audit of remaining entry points is recorded	"Audit" section below

Testing

25 new tests (206 → 231). 20 of them run under real, unmocked authorisation via
Env::set_auths(&[]) rather than mock_all_auths() — one for each of the 20
authorisation-gated entry points in schema.json.

The issue notes that the existing suite could not have caught this, because
mock_all_auths() appears in 116 places and makes every require_auth()
unconditionally satisfied. These tests hand the environment back to the host's
real authorisation machinery. Each one deliberately passes the legitimate admin
address, so the admin == get_admin() comparison succeeds and execution actually
reaches the authorisation check — the assertion is that not even the real admin
can act without a real signature. They assert Err(Err(InvokeError::Abort))
rather than a bare is_err(), so a call failing for an unrelated contract reason
(NotInitialized, MustBePausedForUpgrade) cannot be mistaken for proof that
the entry point is gated.

These tests are not decorative — mutation testing:

Mutation	Tests that fail
remove require_auth() from initialize	2
remove require_auth() from __constructor	1
remove require_auth() from set_allow_native	4 (its real-auth test + 3 existing)

Ops

deploy.sh and the Makefile deploy target now pass the admin as a constructor
argument, so deployment and initialisation are one indivisible transaction, and
then read the on-chain admin back and compare it against the expected value
before the contract is put into service. The manifests gain expected_admin and
document the procedure as a single atomic step.

Audit of the remaining entry points

Every other authorisation-gated entry point was checked for the same class of
omission. All 20 are correctly gated; initialize was the only one missing a
check. Each now has a real-authorisation regression test, so a future removal
fails a test instead of passing silently.

The audit also surfaced three pre-existing defects, unrelated to the
authorisation model but found on the way. All three are fixed here, in a
separate commit from the security fix so the latter stays reviewable on its own:

1. Two broken example call sites. examples/e2e-test.ts and
   examples/query-payment.ts both called getPaymentCount() with no
   arguments, but the signature has been getPaymentCount(adminPublicKey: string)
   since Reconcile the contract's public enumerable payment ledger with the product's privacy guarantees #512 made payment_count admin-gated. Neither has compiled since.
   e2e-test.ts already has the admin keypair to hand; query-payment.ts is a
   public read-only script, so the count is now queried only when
   ADMIN_PUBLIC_KEY is set and skipped with a note otherwise, keeping the
   script usable by a non-admin caller.

2. The blind spot that let them rot. examples/ was excluded from both
   tsconfigs, so nothing typechecked it. tsconfig.test.json now covers
   examples/ as well as src/, which puts this class of breakage in front of
   npm run typecheck. tsconfig.json (the build) is unchanged and still emits
   only src/ to dist/.

3. Stale committed dist/. It was missing LegacyPaymentsMigratedEvent and
   IssuersMigratedEvent, both present in src/events.ts and both absent from
   the DecodedSorobanEvent union consumers match on. Rebuilt with
   npm run build.

One more pre-existing defect is fixed in the security commit itself, because it
blocked the work: contracts/invoice-payment/Makefile used 8 spaces instead of a
TAB in the fmt-check recipe, so GNU Make refused to parse the file at all
(Makefile:17: *** missing separator). No target worked — including the
make schema suggested by the maintainer on #484. One character.

Observation, not changed here: soroban.yml never runs the TypeScript
client's npm run typecheck or npm test in either job. The ABI drift job
checks the client against schema.json, but the client's own types and unit
tests are not exercised in CI, and nothing rebuilds or diffs dist/. That is
what allowed defects 1 and 3 to survive two releases. Adding those steps felt
like scope this PR should not take unilaterally — happy to add them if you want.

Open question

CONTRACT_VERSION is still 1.0.0. Adding __constructor is an ABI addition,
but the previous ABI-changing PRs (#512, #508) did not bump it either, so I have
not changed the policy unilaterally. Tell me which you want and I will adjust.

Verification

Reproduced the soroban.yml CI steps locally against this branch (both commits
applied), on a clean clone of main:

Step	Result
cargo test --locked	231 passed, 0 failed
cargo clippy --locked -- -D warnings	clean
cargo fmt -- --check	clean
stale migration_helpers guard	clean
cargo build --locked --target wasm32v1-none --release	ok — 122,069 B (119 KB)
generate-schema.sh + git diff --exit-code schema.json	no drift (schema.json is script output, never hand-edited)
scripts/check-client-drift.mjs	no drift — 23 error codes, 18 methods, 15 event payloads
working tree clean after cargo test	clean (snapshots in sync)
client tsc --noEmit / vitest run	clean / 74 passed

Note on the WASM size warning: the build is 119 KB, over the workflow's 100 KB
warning threshold. That threshold is already exceeded on main (117 KB before
this change), so the warning is pre-existing; this PR adds 2,172 bytes.

[Copilot]

Pull request overview

This PR addresses a critical Soroban contract takeover vulnerability by making admin installation authorization-enforced and (crucially) enabling atomic “deploy + initialize” via a constructor argument, eliminating the deploy-to-initialize front‑run window. It also updates ops tooling/manifests and adds extensive “real authorization” regression tests to prevent future auth omissions from being hidden by mock_all_auths().

Changes:

• Adds __constructor(admin: Option<Address>) to atomically install the admin during deployment, and updates initialize() to require admin.require_auth().
• Expands the test suite with real (unmocked) authorization checks across admin-gated entrypoints; updates contract test snapshots accordingly.
• Updates deployment manifests/scripts and TypeScript client examples/typecheck coverage to align with admin-gated reads and the new deployment path.

Reviewed changes

Copilot reviewed 217 out of 224 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
soroban/contracts/invoice-payment/src/lib.rs	Adds __constructor and tightens initialize() authorization; shares init logic via helper.
soroban/contracts/invoice-payment/src/migration.rs	Updates migration test setup to register via constructor path.
soroban/contracts/invoice-payment/schema.json	Documents new __constructor entrypoint and updates auth classification for initialize.
soroban/contracts/invoice-payment/generate-schema.sh	Adds __constructor and updates method auth/description metadata generation.
soroban/manifests/testnet.toml	Adds expected_admin documentation/config for deploy verification flow.
soroban/manifests/mainnet.toml	Adds expected_admin documentation/config for deploy verification flow.
soroban/deploy.sh	Deploys with constructor arg and verifies on-chain admin (issues noted in review comments).
soroban/client/tsconfig.test.json	Includes examples/ in typechecking and sets rootDir to support it.
soroban/client/examples/query-payment.ts	Makes admin-gated getPaymentCount conditional on ADMIN_PUBLIC_KEY.
soroban/client/examples/e2e-test.ts	Updates example call to getPaymentCount(adminPublicKey).
soroban/client/dist/index.d.ts	Re-exports newly added event types in generated typings.
soroban/client/dist/events.d.ts	Updates DecodedSorobanEvent union to include additional event types.
soroban/client/dist/index.js.map	Regenerated build output.
soroban/contracts/invoice-payment/test_snapshots/**	Snapshot updates reflecting constructor auth entries and related ledger/auth footprint changes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.