Skip to content

fix(utxo): require admin key for rollback_genesis (bounty #2819) - #8393

Merged
Scottcjn merged 1 commit into
Scottcjn:mainfrom
Skywalkingzulu1:fix/utxo-rollback-auth-2819
Sep 13, 2026
Merged

Scottcjn merged 1 commit into
Scottcjn:mainfrom
Skywalkingzulu1:fix/utxo-rollback-auth-2819

Conversation

@Skywalkingzulu1

Copy link
Copy Markdown
Contributor

Summary

ollback_genesis() in
ode/utxo_genesis_migration.py\ was a destructive state mutation reachable with no authorization. It deletes every genesis box and transaction and evicts pending mempool claims that depend on them. Any code path importing the module, or the --rollback\ CLI flag, could wipe the UTXO genesis set and its pending spends.

Fix

  • Added _require_rollback_authorization(admin_key)\ gate keyed on \RC_ADMIN_KEY:
    • Fail closed: an unset key refuses (operator never intended rollback to be possible)
    • Missing key arg refused
    • Wrong key refused via constant-time \hmac.compare_digest\

  • ollback_genesis(db_path, admin_key=None)\ now calls the gate first, before any state mutation
  • CLI gained --admin-key; unauthenticated rollback exits 1 with a clear error
  • Mirrors the existing \�gent_relationships.py\ admin-key pattern

Tests

  • Updated
    ode/test_rollback_atomicity.py,
    ode/test_utxo_2819_rollback_mempool_and_bounded_select.py,
    ode/test_genesis_race.py, and \ ests/test_utxo_security_audit.py\ to pass the key
  • Added \TestRollbackAuthorization\ (4 cases: no-key refuses, wrong-key refuses, missing-arg refuses, correct-key succeeds)

Verification

  • \ est_rollback_atomicity.py: 10/10 passed
  • \ est_utxo_2819_rollback_mempool_and_bounded_select.py: 7/7 passed (8 subtests)
  • \ ests/test_utxo_security_audit.py: 28/28 passed (incl. 4 new auth tests)

  • ode/test_genesis_race.py: both scenarios pass

  • ode/test_utxo_db.py: 107 passed

This finding is distinct from the three existing open #2819 PRs (#8388 receiver-side mirror provenance, #8332 red-team audit report, #8302 fee/output/conservation invariants) — none of them cover the rollback authorization gap.

Co-authored-by: Thamsanqa Skenjana tahmiix@gmail.com

@github-actions

Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) node Node server related tests Test suite changes size/M PR: 51-200 lines labels Sep 11, 2026
rollback_genesis() was a destructive state mutation reachable with no
authorization: it deletes every genesis box and transaction and evicts
pending mempool claims that depend on them. Any code path importing the
module, or the --rollback CLI flag, could wipe the UTXO genesis set.

Add an authorization gate keyed on RC_ADMIN_KEY:
- Fail closed: an unset key refuses (operator never intended rollback to be possible)
- Missing key arg refused
- Wrong key refused via constant-time hmac.compare_digest
- CLI gains --admin-key; unauthenticated rollback exits 1

Update all existing rollback test suites to pass the key and add a
TestRollbackAuthorization class covering the four auth outcomes.
@Scottcjn

Copy link
Copy Markdown
Owner

@Skywalkingzulu1 — verdict on the #2819 claim first, then the PR.

Not payable under #2819. rollback_genesis() is not exposed over HTTP; on main it is called only from the migration CLI and tests (grep -rn rollback_genesis node/). An operator running a local destructive script is not an "unauthenticated" path in the bounty's sense — there is no attacker who reaches it without already having shell on the node. So this is defense-in-depth, not a vulnerability.

The hardening itself is fine. Requiring RC_ADMIN_KEY for a destructive local operation is reasonable and the tests are updated. I will merge it as a hardening change once you confirm the callers you changed are the only ones (the two test files plus the demo script), and it earns a merge reward, not a bounty tier. — Sophia

@Scottcjn
Scottcjn merged commit b530e79 into Scottcjn:main Sep 13, 2026
2 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

RTC Reward

This merged PR earned 5 RTC — sent to Skywalkingzulu1.

RustChain Bounty Program

Scottcjn added a commit that referenced this pull request Sep 18, 2026
…s CI) (#8467)

#8393 set os.environ["RC_ADMIN_KEY"] = "test-rollback-admin-key-2819" at
import time. That value is 28 chars; the node sys.exit(1)s on a key < 32
chars, so every later test module that imports the node aborted pytest
collection and the CI "test" job has failed on every PR since 9/14. The
overwrite also leaked into later tests that send conftest's key (401s).

Use the suite-wide key from tests/conftest.py (setdefault, so the file
still runs standalone) and pass that same value to rollback_genesis().

Local run of the exact CI command (CI env + tests/requirements.txt):
  before: collection INTERNALERROR (SystemExit: 1), 0 tests run
  after:  4009 passed, 2 failed
The 2 remaining failures (test_utxo_dual_write_double_spend,
test_utxo_transfer_spends_account_mirror) fail standalone on main and are
unrelated: mirror boxes are now filtered before the 409
ACCOUNT_MIRROR_BOX_NOT_SPENDABLE check, so the endpoint returns 400
"Insufficient UTXO balance" instead. The box is still not spent.

Signed-off-by: Scott <scottbphone12@gmail.com>
Co-authored-by: Scott <scottbphone12@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) node Node server related size/M PR: 51-200 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants