fix: reject contributor placeholder secret

[dicnunz]

Summary

• Fail closed when CONTRIBUTOR_SECRET_KEY is set to the known compromised placeholder rustchain_contributor_secret_2024.
• Preserve the existing unset-env random fallback behavior.
• Add a regression proving the placeholder is rejected at import time.

Scope / duplicate gate

• This targets only the additional placeholder-secret finding on Bug: keeper_explorer and contributor_registry run Flask in debug=True — enables RCE #5059, not the public Flask debug-mode lane.
• Checked open PRs for rustchain_contributor_secret_2024, known placeholder, and CONTRIBUTOR_SECRET_KEY immediately before push; no open PR covers this hard-fail behavior.
• Existing debug-mode PRs (fix: disable public Flask debug by default #4843/Fix: Remove hardcoded debug=True in Flask entrypoints (#4810) #4859/fix(#5059): CRITICAL — disable Flask debug mode in production #5060) cover a different lane, so this PR intentionally does not touch debug settings.
• Historical PR fix: load contributor_registry secret_key from environment (CVE) #2773 merged env-secret loading but only warned on the placeholder; PR fix: harden security configuration in contributor registry #4064 was closed for branch contamination and did not change this placeholder branch.

Validation

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --no-project --with pytest --with flask python -m pytest tests/test_contributor_registry.py -q -> 13 passed
• python3 -m py_compile contributor_registry.py tests/test_contributor_registry.py -> passed
• uv run --no-project --with ruff ruff check --select E9,F821,F811,F841 contributor_registry.py tests/test_contributor_registry.py -> passed
• python3 tools/bcos_spdx_check.py --base-ref upstream/main -> OK
• git diff --check -> passed

Bounty

Bounty rail: Scottcjn/rustchain-bounties#71
Issue: #5059 additional placeholder-secret finding
Wallet/miner ID: dicnunz
Canonical payout target: Scottcjn/rustchain-bounties#9194

No production service, live wallet, private key, token transfer, or destructive request was used.

[github-actions]

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

• Your PR has a BCOS-L1 or BCOS-L2 label
• 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!

[weilixiong]

Review: Standard Quality

Overall: Good security fix. Changing warnings.warn() to raise ValueError() is the right call — fail-closed prevents deployment with insecure defaults.

Strengths:

• Clear behavioral change from warn → error, exactly what a security-hardening fix should do
• Test covers the happy path of rejection and cleanup via finally
• Uses monkeypatch properly for env var isolation

Suggestion (non-blocking):
The test relies on cr being already imported at module scope. If import contributor_registry as cr lives in a conftest or fixture, it is fine. Otherwise adding an explicit import contributor_registry as cr inside the class or before the test would make the dependency clearer for future readers.

Verdict: ✅ LGTM, mergeable as-is. Security impact: Medium (dev-to-prod safety gate).

[lavishsaluja]

Approved current head d9352816abd5815f2719ea33c76b90ace8fe694b.

The patch changes the known compromised CONTRIBUTOR_SECRET_KEY placeholder from a warning-only path to a hard fail at import time, while preserving the existing unset-env random fallback behavior. The regression reloads the module with the placeholder value and confirms it raises before app.secret_key can be initialized from that public secret.

Validation performed locally:

• python -m pytest tests/test_contributor_registry.py -q -> 13 passed
• python3 -m py_compile contributor_registry.py tests/test_contributor_registry.py -> passed
• git diff --check origin/main...HEAD -> passed
• python3 tools/bcos_spdx_check.py --base-ref origin/main -> OK
• Confirmed the placeholder string now only appears in the rejection branch and regression test.

No production service, live wallet, private key, token transfer, or destructive testing was used.

[loganoe]

Approved. I reviewed the placeholder-secret fail-closed change in contributor_registry.py and the regression in tests/test_contributor_registry.py.

The change is appropriately narrow: deployments that explicitly configure the known compromised rustchain_contributor_secret_2024 now fail at import time instead of continuing with only a warning, while the existing unset-env random fallback remains unchanged. The test exercises the import-time guard and restores the module state afterward, so it should not leak the poisoned env setting into the rest of the file.

Validation run on head d9352816abd5815f2719ea33c76b90ace8fe694b:

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 timeout 180 uv run --no-project --with pytest --with flask python -m pytest tests/test_contributor_registry.py -q -> 13 passed
• python3 -m py_compile contributor_registry.py tests/test_contributor_registry.py -> passed
• git diff --check origin/main...HEAD -- contributor_registry.py tests/test_contributor_registry.py -> clean
• python3 tools/bcos_spdx_check.py --base-ref origin/main -> OK

No live service, wallet, private key, token transfer, or production traffic was used.

[508704820]

Code Review: Reject contributor placeholder secret

Summary

Fixes the placeholder secret issue we identified in #5059 comment -- the contributor_registry accepted 'rustchain_contributor_secret_2024' with just a warning instead of rejecting it.

This PR changes the behavior from warn-and-accept to reject:

• Known placeholder = rejected with error
• Forces operators to set a proper secret key

LGTM -- good security hardening. Known-bad values should be rejected, not just warned.

**Review quality: Security-focused review (CWE-798)

[508704820]

Security Review — #5114

1. CRITICAL fix — placeholder secret now raises ValueError

The old code used warnings.warn() for the known placeholder secret, which is just a warning that doesn't stop execution. The fix raises ValueError, which crashes the app — this is the correct behavior. Running with a known secret key means session cookies can be forged by anyone who knows the placeholder value.

2. Still runs debug=True

The PR changes contributor_registry.py but leaves app.run(debug=True) — this is the same RCE vulnerability as #4843/#4859. Should be fixed here too.

3. Test coverage added

The PR adds tests for the ValueError, which is good for preventing regression.

4. Known placeholder value is in source code

The string 'rustchain_contributor_secret_2024' is hardcoded. Even with the ValueError, anyone who reads the source knows the default. Consider:

• Removing the hardcoded default entirely
• Generating a random secret on first run if none is provided
• Documenting the secret requirement in README

— Xeophon (security review)

[TJCurnutte]

Approved after a focused review of this PR's diff against origin/main.

Scope checked:

• PR title: fix: reject contributor placeholder secret
• Changed files: contributor_registry.py, tests/test_contributor_registry.py
• Review finding: Reviewed contributor_registry.py, tests/test_contributor_registry.py; diff adds regression test coverage.

Validation run locally from a clean checkout of pull/5114/head:

• git diff --check origin/main...HEAD → exit 0 (no output)
• python3 tools/bcos_spdx_check.py --base-ref origin/main → exit 0 (BCOS SPDX check: OK)
• python3 -B -m py_compile contributor_registry.py tests/test_contributor_registry.py → exit 0 (no output)
• python3 -B -m pytest tests/test_contributor_registry.py -q → exit 0 (13 passed, 3 warnings in 0.17s)
• Added-line secret-pattern scan → pass

I did not find a merge-blocking issue in this focused pass. The change is small enough to review directly, and the targeted gates above passed.

[jaxint]

LGTM! Thanks for contributing. Approved.

[himanalot]

Approved. The contributor registry now refuses to start when CONTRIBUTOR_SECRET_KEY is set to the known placeholder value instead of only warning and continuing with a compromised Flask signing key.

I could not run the pytest/Flask test locally because those project dependencies are not installed and I am avoiding installs. Static review of the module initialization path and the added reload-based regression test matches the intended fail-closed behavior for rustchain_contributor_secret_2024.

[jaxint]

PR Review - Standard Quality ✓

Reviewed: Review submitted via GitHub API
Bounty: #73 - PR Reviews Bounty
Wallet: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG

This PR has been reviewed as part of the RustChain bounty program. All standard review criteria met.

---

🤖 Automated review via RustChain RTC bounty bot

[HCIE2054]

LGTM! Thanks for contributing!