Skip to content

test: fix 3 stale test files drifted from security hardening (21 failures)#6658

Merged
Scottcjn merged 1 commit into
mainfrom
fix/ci-stale-tests-auth-redirect-genesis
May 30, 2026
Merged

test: fix 3 stale test files drifted from security hardening (21 failures)#6658
Scottcjn merged 1 commit into
mainfrom
fix/ci-stale-tests-auth-redirect-genesis

Conversation

@Scottcjn
Copy link
Copy Markdown
Owner

Fixes 3 stale test files (21 failures) on red main — all test-only, production code unchanged:

  1. test_tx_handler_limits (16): /tx/pending + /wallet//history admin-gated since fix: require admin auth on tx handler GET endpoints #6295 → anonymous calls 401. Fixture now sets RC_ADMIN_KEY (monkeypatch) + sends X-Admin-Key.
  2. test_wallet_network_utils (4): _fetch_with_retry now allow_redirects=False + resp.is_redirect (SSRF guard); MagicMock is_redirect is truthy. Mocks set is_redirect=False.
  3. test_utxo_security_audit::test_genesis_rerun_blocked (1): check_existing_genesis keys off tx_type='genesis'; test inserted mining_reward at height 0. Now inserts real genesis-typed tx.

52/52 pass in these files (was 21 failing), verified against current main, self-contained.

Remaining ~31 failures triaged separately (mostly same auth-drift pattern + a stale miners/checksums.sha256 for the linux miner that affects real installer integrity).

🤖 Generated with Claude Code

Three integration tests broke against intended hardening (red main, not
regressions):

1. test_tx_handler_limits (16): /tx/pending + /wallet/<a>/history GET
   endpoints are admin-gated since #6295. Tests called them anonymously and
   got 401 before reaching the limit/validation logic under test. Fixture
   now sets RC_ADMIN_KEY (via monkeypatch, auto-restored) and sends
   X-Admin-Key on every request.

2. test_wallet_network_utils (4): _fetch_with_retry now uses
   allow_redirects=False + resp.is_redirect (SSRF/redirect hardening). Bare
   MagicMock responses have a truthy is_redirect, so the code treated them as
   redirects and returned None. Mocks now set is_redirect = False.

3. test_utxo_security_audit::test_genesis_rerun_blocked (1):
   check_existing_genesis keys off tx_type='genesis', but the test inserted a
   mining_reward tx at height 0. Now inserts a real genesis-typed tx,
   mirroring test_rollback_then_remigrate_idempotent.

All production code is correct; only the tests were stale. 52/52 pass in
these files (was 21 failing). Verified self-contained (no external env).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/M PR: 51-200 lines labels May 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

✅ BCOS v2 Scan Results

Metric Value
Trust Score 60/100
Certificate ID BCOS-f7c323f8
Tier L1 (met)

BCOS Badge

What does this mean?

The BCOS (Beacon Certified Open Source) engine scans for:

  • SPDX license header compliance
  • Known CVE vulnerabilities (OSV database)
  • Static analysis findings (Semgrep)
  • SBOM completeness
  • Dependency freshness
  • Test infrastructure evidence
  • Review attestation tier

Full report | What is BCOS?


BCOS v2 Engine - Free & Open Source (MIT) - Elyan Labs

Copy link
Copy Markdown
Contributor

@darlina-bounty-codex darlina-bounty-codex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the three touched test files against the current route/helper behavior.

Local validation:

  • py_compile passed for tests/test_tx_handler_limits.py, tests/test_wallet_network_utils.py, and tests/test_utxo_security_audit.py using the bundled Python runtime.
  • git diff --check origin/main...HEAD -- tests/test_tx_handler_limits.py tests/test_wallet_network_utils.py tests/test_utxo_security_audit.py passed for the touched files.
  • I could not run the pytest target set in this environment because pytest is not installed in the bundled Python runtime.

Technical observations:

  1. The test_tx_handler_limits fixture change is directionally correct: these endpoints are now admin-gated by require_admin(), which reads X-Admin-Key and RC_ADMIN_KEY. Setting client.environ_base["HTTP_X_ADMIN_KEY"] exercises the intended limit/validation paths instead of failing early at the auth gate, while monkeypatch keeps the key scoped to the test.
  2. Setting MagicMock.is_redirect = False in the wallet network tests is needed after the redirect guard because unspecified MagicMock attributes are truthy. Without the explicit false value, these success-path mocks can accidentally simulate a redirect and test the wrong branch.
  3. The genesis rerun test correction matches the actual invariant better: check_existing_genesis is keyed to tx_type='genesis', so inserting a real genesis transaction is a more direct regression test than using a mining_reward transaction at height 0. It also lines up with the later rollback/remigrate setup pattern.

I do not see a blocker in the touched test changes. If maintainers want tighter follow-up coverage later, a focused negative test for wrong/missing X-Admin-Key on one of these GET routes would document the auth gate separately, but that is outside this stale-test-fix PR.

I received RTC compensation for this review.

@Scottcjn Scottcjn merged commit 43e6d4b into main May 30, 2026
11 of 12 checks passed
@Scottcjn Scottcjn deleted the fix/ci-stale-tests-auth-redirect-genesis branch May 30, 2026 20:34
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) 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