Skip to content

feat(tbtc/signer): enforce signing-policy firewall in production with built-in defaults#4125

Merged
mswilkison merged 2 commits into
extraction/frost-signer-mirror-2026-05-26from
feat/signer-firewall-prod-default
Jun 30, 2026
Merged

feat(tbtc/signer): enforce signing-policy firewall in production with built-in defaults#4125
mswilkison merged 2 commits into
extraction/frost-signer-mirror-2026-05-26from
feat/signer-firewall-prod-default

Conversation

@mswilkison

Copy link
Copy Markdown
Contributor

Summary

Closes the signing-policy firewall fail-open default flagged in the #4005 review (and confirmed by a Codex consult). signing_policy_firewall_enforced() defaulted OFF with no production force-on — unlike provenance_gate_enforced() — so a production signer would sign any sighash reaching the sign path with no check it corresponds to a policy-checked build_taproot_tx. With no slashing/staking in the model, that on-signer check is the enforcement, not a backstop.

Approach: option (b) — default-on with conservative built-in defaults

I prototyped the blunt force-on first; it made the entire firewall policy config mandatory in production (the signer would refuse to boot without it) — a deployment-contract cliff. Codex's recommendation (and mine) was default-on with conservative baked-in defaults instead, so this PR does that:

  • signing_policy_firewall_enforced() force-enables in production (mirrors the provenance gate). The firewall's primary control — enforce_signing_message_binding_to_policy_checked_build_tx (the signed digest must match a policy-checked tx) — now runs in production.
  • load_signing_policy_firewall_config() resolves missing policy env to conservative built-in defaults instead of erroring:
    • allowed_script_classes → the standard tBTC output forms {p2pkh, p2sh, p2wpkh, p2wsh, p2tr}. The classifier's "other" (non-standard) bucket is not in the set, so the firewall fails closed on unknown output forms.
    • numeric caps → permissive (output count high-bounded, value caps to BITCOIN_MAX_MONEY_SATS) and operator-tunable. A too-tight static cap would false-reject legitimate large redemptions/sweeps — Codex's "stale-policy" risk — and the message-binding is the real control, so the defaults bound damage without breaking liveness.

Operators still narrow any knob via the existing TBTC_SIGNER_POLICY_* env. Non-production behavior is unchanged (opt-in via the enforce flag).

Notes / follow-ups

  • The script-class default is the meaningful, zero-false-reject security default. The value/count defaults are intentionally permissive; deployments should set tighter caps per wallet sizing (Codex's two risks — stale policy and false confidence — argue for versioned policy + keeping host/on-chain validation authoritative).
  • Still worth confirming separately: whether the keep-core Go host independently validates the signed message vs Bridge state before driving the round (defense-in-depth vs sole-enforcement). That doesn't block this change — it only raises the priority.

Verification

  • cargo test299 passed, 0 failed (incl. new tests: production force-on, built-in defaults; the rollback-on-policy-failure test now triggers on an explicitly-invalid value since absent config no longer fails).
  • cargo clippy --all-targets -- -D warnings and cargo fmt --checkclean.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

🤖 Generated with Claude Code

… built-in defaults

Closes the fail-open default flagged in the #4005 review (and confirmed by Codex):
signing_policy_firewall_enforced() defaulted OFF with no production force-on --
unlike provenance_gate_enforced() -- so a production signer would sign any sighash
reaching the sign path with no check it corresponds to a policy-checked
build_taproot_tx.

Implements option (b) (default-on with conservative baked-in defaults), chosen over
a blunt force-on, which made the full policy config mandatory and would brick a
production signer that did not ship it:

- signing_policy_firewall_enforced() now force-enables in production (mirrors the
  provenance gate), so the firewall's primary control --
  enforce_signing_message_binding_to_policy_checked_build_tx -- runs in production.
- load_signing_policy_firewall_config() resolves missing policy env to conservative
  built-in defaults instead of refusing to boot:
  - allowed_script_classes defaults to the standard tBTC output forms
    {p2pkh, p2sh, p2wpkh, p2wsh, p2tr}; the classifier's "other" (non-standard)
    bucket is not in the set, so the firewall fails closed on unknown forms.
  - numeric caps default permissive (output count high-bounded, value caps to
    BITCOIN_MAX_MONEY_SATS) and are operator-tunable; a too-tight static cap would
    false-reject legitimate large redemptions/sweeps (the stale-policy risk).

Operators can still narrow any of these via the existing TBTC_SIGNER_POLICY_* env.
Non-production behavior is unchanged (opt-in via the enforce flag). Removed the now
unused parse_{usize,u64}_from_env_required helpers.

Tests: production force-on + built-in defaults; the rollback-on-policy-failure test
now triggers on an explicitly-invalid value (UTC window mismatch) since absent config
no longer fails. cargo test (299 passed) + clippy -D warnings + fmt all clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 96cc38d5-4f68-42c5-a604-8b70a8ebf9ff

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/signer-firewall-prod-default

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…efaults

Self-review found the README stale after the firewall change: the policy caps were
documented as "required when firewall is enabled" (now resolved to built-in
defaults), the production force-enable wasn't documented, and "Policy gates default
to disabled" contradicted it. Corrected all three.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mswilkison mswilkison merged commit 8d873e7 into extraction/frost-signer-mirror-2026-05-26 Jun 30, 2026
20 checks passed
@mswilkison mswilkison deleted the feat/signer-firewall-prod-default branch June 30, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant