Repository navigation
docs(testnet): disclosure process, reentrancy observation log, dependency audit notes, reset policy - #751
Merged
phertyameen merged 1 commit intoSep 27, 2026
Conversation
…ency audit notes, and a reset policy
Four governance documents for the shared testnet deployment. The deployment
is shared by unrelated integrators, which makes questions of disclosure,
observation and reset materially different from the contracts' design, and
none of them were answered anywhere in the tree.
Every document is written against what the code actually says. As of this
commit no Bridgelet contract is deployed to testnet — testnet/registry/
contract-status.md records all four as undeployed with IDs "TBD" and Last
Verified "Never" — so nothing here claims to have been observed on-chain.
The three security documents are consequently written ahead of the
deployment, and the observation log legitimately starts empty. deployments/
testnet.json and deployment-artifacts/contract-ids.txt do contain concrete
IDs from a 2026-07-12 deploy, but the registry does not reflect them and
testnet/security/README.md says to treat them as unverified until confirmed
per testnet/registry/verify-contract-live.md.
testnet/security/disclosure-process.md
A deliberately lightweight, testnet-appropriate process, and explicitly
distinct from a formal mainnet disclosure policy that does not exist yet,
per the "Active Development - MVP. Not audited." status in the root README.
The most useful thing it does is separate two kinds of report that are easy
to conflate: a contract bug and an operational observation, with a concrete
test for telling them apart (deploy your own instance from the WASM hash and
drive it alone — if the behaviour follows you, it is a bug). It also
pre-empts two known defects so a reporter does not file a duplicate
mechanism: EphemeletAccount's ignored auth_signature, and the changelog's
unimplemented error-code namespacing.
The no-secrets section is a standalone section rather than a footnote, with
a recovery step for a key that has already been posted, and the response
table includes an honest "confidentiality after a private report: not
promised" row rather than letting a lightweight process look safer than it
is. Severity uses the repository's real labels, verified with `gh label
list`, including the existing severity:* family.
testnet/security/reentrancy-observation-log.md
An append-only log, because a log that can be rewritten is not evidence: a
wrong conclusion is corrected by appending a superseding entry, never by
editing. Classification vocabulary is aligned to the three values
testnet/runbooks/reentrancy-suspicion.md already prescribes in its
"Classify and record" step, with the issue's two extra cases modelled as
explicit sub-cases and a "Runbook term" column showing the mapping, so the
log cannot drift into a parallel scheme. The bar for logging is deliberately
low and the bar for concluding high, since a pattern visible only across
many false alarms is what makes a future positive interpretable.
The entry schema requires thirteen fields, which is uncomfortable, and says
why: the runbook demands the deployed build identity be recorded as unknown
rather than assumed, and a schema that does not ask will not get it. The
worked example is fenced, banner-marked as not a record, uses a
<synthetic-tx-hash-not-a-real-transaction> placeholder rather than plausible
hex, and is classified inconclusive with no evidence retained — the example
teaches the honest answer rather than a scary one.
testnet/security/dependency-audit-notes.md
A deployed contract's behaviour is fixed at whatever SDK version built it,
so when an advisory lands you need to know whether the live artifacts are in
range. The document records the build-to-deployed chain — resolved SDK
version, build commit, deployed commit, WASM hash — and states plainly which
link is weakest: the WASM hash in deployments/testnet.json is unverified, and
scripts/deploy-testnet.sh records no commit SHA at all, so a correctly
executed deploy cannot close the chain today. scripts/ is out of scope for
this batch, so that is named as a fix-before-use item rather than changed.
Two findings worth surfacing. First, all five workspace members declare
soroban-sdk = "22.0.0" as a caret range, and both Cargo.lock files resolve
it to 22.0.11 — the resolved version, not the declared one, is what builds.
Second, the repo names stellar-cli 23.4.1 in seven places and enforces it in
none: no .tool-versions, no rust-toolchain.toml, no CI matrix, and three
conflicting answers, with scripts/build.sh saying "23.4.1 or newer" against
testnet/docs/getting-started.md's "v22 or later", while both workflows
install stellar-cli twice by two different mechanisms. The SDK is pinned by
a lockfile; the CLI is not pinned at all.
The sharpest guidance is in the stale-or-missing branch, which is the branch
that matters today: do not upgrade the SDK in response to an advisory in
isolation, because that skips the tools/sweep-signer soroban-sdk pin — whose
sole purpose is to keep Address::to_xdr() byte-identical to what the
contract expects — and converts a security response into a signature-
correctness regression.
testnet/policies/reset-policy.md
The high-priority one. The governing distinction is between two independent
kinds of reset that integrators conflate: an SDF platform reset of testnet
itself, and a Bridgelet redeploy or in-place upgrade. They have different
warning characteristics, and the second produces new contract IDs and wipes
EphemeralAccount state — recorded payments, nonces, expiry ledgers and the
controller's sweep nonce — while the first may not have happened at all.
The issue invites the honest answer ("even if the honest answer is 'ad hoc,
no fixed cadence'"), and that is what the document gives: the Status table
answers four of the ordinary framing questions with "undecided", and
"Undecided" is six numbered items. A good-first-issue-scale policy cannot
guarantee lead time for an SDF-initiated reset, and that is stated as a
conclusion rather than an apology, because promising notice the project
cannot give is worse than promising none.
The most useful part is deliberately not the policy but the detection
procedure: the live disagreement between contract-status.md and
deployments/testnet.json, compounded by verify-contract-live.md heading its
EphemeralAccount and SweepController sections "Expected deployed: Yes" while
citing the registry that says the opposite, is used as the worked example —
because that drift *is* the phenomenon this policy is about, one step
removed from a reset. If integrators resolve IDs at runtime and re-verify
per run, a reset becomes a scheduled annoyance rather than an incident, and
the document argues for that outcome explicitly.
Also adds one line per new document to the existing index table in
testnet/security/README.md, which is what that table is for.
Closes bridgelet-org#654
Closes bridgelet-org#655
Closes bridgelet-org#656
Closes bridgelet-org#657
|
@Userhorlie Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four governance documents for the shared testnet deployment. A shared deployment is a real multi-tenant system — one party's action degrades every other party's experience — which makes disclosure, observation and reset questions materially different from the contracts' design. None of them were answered anywhere in the tree.
testnet/security/disclosure-process.mdtestnet/security/reentrancy-observation-log.mdtestnet/security/dependency-audit-notes.mdtestnet/policies/reset-policy.mdhigh-priority)Plus one additive line per document in the existing index table in
testnet/security/README.md.Everything is written against what the code says. No Bridgelet contract is deployed —
testnet/registry/contract-status.mdrecords all four as undeployed, IDs "TBD", Last Verified "Never" — so nothing here claims on-chain observation, and the observation log legitimately starts empty.deployments/testnet.jsondoes hold concrete IDs from a 2026-07-12 deploy, but the registry does not reflect them, andtestnet/security/README.mdsays to treat them as unverified.disclosure-process.md— how to report a findingDeliberately lightweight, and explicitly distinct from a formal mainnet disclosure policy that does not exist yet — per the root README's "Active Development - MVP. Not audited." banner.
The most useful thing it does is separate two report types that are easy to conflate: a contract bug vs. an operational observation, with a concrete test for telling them apart (deploy your own instance from the WASM hash and drive it alone — if the behaviour follows you, it's a bug). It also pre-empts two known defects so a reporter doesn't file a duplicate mechanism: the discarded
auth_signatureand the changelog's unimplemented error-namespacing.Two choices worth flagging:
Severity uses the repository's real labels, verified with
gh label list, including the existingseverity:*family — not an invented rubric.reentrancy-observation-log.md— append-only by constructionA log that can be rewritten is not evidence, so a wrong conclusion is corrected by appending a superseding entry, never by editing. The classification vocabulary is aligned to the three values
testnet/runbooks/reentrancy-suspicion.mdalready prescribes in its "Classify and record" step, with the issue's two extra cases modelled as explicit sub-cases and a "Runbook term" column showing the mapping — so the log cannot drift into a parallel scheme.The entry schema requires thirteen fields, which is uncomfortable, and the document says why: the runbook demands the deployed build identity be recorded as
unknownrather than assumed, and a schema that does not ask will not get it.The bar for logging is deliberately low and the bar for concluding high — a pattern visible only across many false alarms is what makes a future positive interpretable. The worked example is banner-marked as not a record, uses a
<synthetic-tx-hash-not-a-real-transaction>placeholder instead of plausible hex, and is classified inconclusive with no evidence retained: the example teaches the honest answer, not a scary one.dependency-audit-notes.md— the SDK/CLI version pairingA deployed contract's behaviour is fixed at whatever SDK version built it, so when an advisory lands you need to know whether the live artifacts are in range. Records the chain — resolved SDK version → build commit → deployed commit → WASM hash — and states plainly which link is weakest: the WASM hash in
deployments/testnet.jsonis unverified, andscripts/deploy-testnet.shrecords no commit SHA at all, so a correctly-executed deploy cannot close the chain today.scripts/is out of scope, so that's a fix-before-use item rather than a change.Two findings:
soroban-sdk = "22.0.0"as a caret range, and bothCargo.lockfiles resolve it to 22.0.11. The resolved version, not the declared one, is what builds.stellar-cli23.4.1 is named in seven places and enforced in none — no.tool-versions, norust-toolchain.toml, no CI matrix, and three conflicting answers:scripts/build.shsays "23.4.1 or newer" againsttestnet/docs/getting-started.md's "v22 or later", while both workflows installstellar-clitwice by two different mechanisms. The SDK is pinned by a lockfile; the CLI is not pinned at all.The sharpest guidance sits in the stale-or-missing branch, which is the branch that matters today: do not upgrade the SDK in response to an advisory in isolation. That skips the
tools/sweep-signerpin — whose sole purpose is keepingAddress::to_xdr()byte-identical to what the contract expects — and converts a security response into a signature-correctness regression.reset-policy.md— if/when/how testnet is wipedThe governing distinction is between two independent kinds of reset that integrators conflate: an SDF platform reset of testnet itself, versus a Bridgelet redeploy or in-place upgrade. They have different warning characteristics, and the second produces new contract IDs and wipes
EphemeralAccountstate — recorded payments, nonces, expiry ledgers, the controller's sweep nonce — while the first may not have happened at all.The issue invites the honest answer ("even if the honest answer is 'ad hoc, no fixed cadence'"), and that is what it gets: the Status table answers four framing questions with "undecided", and "Undecided" is six numbered items. A good-first-issue-scale policy cannot guarantee lead time for an SDF-initiated reset, and that is stated as a conclusion rather than an apology — promising notice the project cannot give is worse than promising none.
The most useful part is deliberately not the policy but the detection procedure. The live disagreement between
contract-status.mdanddeployments/testnet.json, compounded byverify-contract-live.mdheading itsEphemeralAccountandSweepControllersections "Expected deployed: Yes" while citing the registry that says the opposite, is used as the worked example — because that drift is the phenomenon this policy is about, one step removed from a reset. If integrators resolve IDs at runtime and re-verify per run, a reset becomes a scheduled annoyance rather than an incident, and the document argues for that outcome explicitly.Verification
scripts/,tools/,contracts/,deployments/, and.env.exampleare untouched.testnet/security/README.mdchange is 3 insertions, 0 deletions.ls-verified).S...keys, no seeds, no 64-char hex.testnet/config/README.md's no-secrets discipline is extended, not restated.Repository defects found and documented rather than papered over
Beyond the two I already knew (the changelog's error-namespacing claim and the
UnauthorizedDestination = 13gap), this batch surfaced:gh api … /private-vulnerability-reporting→{"enabled":false}), yettestnet/docs/support.md:65-71tells reporters to use the "Report a vulnerability" button, andtestnet/runbooks/reentrancy-suspicion.md:28-29andincident-postmortem-template.md:10-13do the same. Two runbooks route reporters to a facility that does not exist. This materially changed the disclosure document — it names the gap and gives substitutes rather than writing a "report it privately" section that would fail at the first click.network-config.md:63-64claimsdeployments/testnet.json"still containsnullplaceholders". It contains four concrete IDs. Repeated indocs/overview.md:76anddocs/faq.md:41-43.EphemeralAccountID sits in.env.example:36, matching none of the four in the deployment artefacts, network-config, or the changelog.scripts/*.shusewasm32v1-none(with a comment saying the old target emits WASM the runtime rejects), butfaq.md:157,wasm-hash-reference.md,upgrade-history.mdandcontract-status.md:49all saywasm32-unknown-unknown.docs/overview.md:78andfaq.md:52-56claim the deploy script "only deploys two contracts". It now deploys all four.docs/testnet-deployment.mdfromnetwork-config.md:124andoverview.md.tools/sweep-signer/testnet/config/staleness.mdis a wave-issue prompt fragment committed insidetools/.Each is documented in the relevant file rather than fixed, since fixing them would mean editing files outside this batch's scope.
Closes #654
Closes #655
Closes #656
Closes #657