Skip to content

fix(tbtc/signer): reject unknown admission policy fields#4161

Merged
mswilkison merged 2 commits into
extraction/frost-signer-mirror-2026-05-26from
fix/signer-admission-policy-schema-2026-07-14
Jul 15, 2026
Merged

fix(tbtc/signer): reject unknown admission policy fields#4161
mswilkison merged 2 commits into
extraction/frost-signer-mirror-2026-05-26from
fix/signer-admission-policy-schema-2026-07-14

Conversation

@mswilkison

@mswilkison mswilkison commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Stack

2 of 3. Parent: #4160. Next: #4162.

Summary

  • Reject unknown top-level fields when deserializing AdmissionPolicyV1.
  • Keep supported policy files compatible while turning misspelled security controls into explicit input errors instead of omitted limits.
  • Add a malicious typo regression and a supported-field positive control.

Review finding addressed

  • P2: unknown admission-policy fields could silently disable enforcement and fail open.

Security invariant

A policy field that the checker does not recognize cannot silently alter or weaken admission behavior.

Validation

  • cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml on this branch tip: 270 library tests passed, 1 ignored; 30 admission-checker tests passed; 1 integration test passed.
  • The real checker rejects a misspelled max_operator_per_provider field and accepts the shipped policy sample.

@coderabbitai

coderabbitai Bot commented Jul 15, 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: 64c460eb-844d-4b7f-9ffe-0f9ed1043118

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 fix/signer-admission-policy-schema-2026-07-14

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.

@mswilkison
mswilkison changed the base branch from fix/signer-persistence-retry-followups-2026-07-14 to extraction/frost-signer-mirror-2026-05-26 July 15, 2026 11:01
mswilkison added a commit that referenced this pull request Jul 15, 2026
## Stack

1 of 3. Base: `extraction/frost-signer-mirror-2026-05-26` (#4005). Next:
#4161.

## Summary

- Normalize one-component signer state paths to `.` for directory
creation, fsync, and corrupt-backup enumeration.
- Make completed canary rollback retries true no-ops for rollout state,
config version, metrics, and promotion evidence.
- Repair uncertain post-rename directory durability on a restarted
rollback retry without rewriting the state file; keep fresh
missing-state no-ops side-effect free.

## Review findings addressed

- P1: bare state paths used an empty parent for directory operations.
- P2: completed canary rollback retries were not idempotent.

## Validation

- `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml` on
this branch tip: 270 library tests passed, 1 ignored; 28
admission-checker tests passed; 1 integration test passed.
- Fault-injection regressions cover pre/post-rename behavior, restarted
retry repair, and no state rewrite on idempotent retry.
@mswilkison
mswilkison merged commit eb1277b into extraction/frost-signer-mirror-2026-05-26 Jul 15, 2026
20 checks passed
@mswilkison
mswilkison deleted the fix/signer-admission-policy-schema-2026-07-14 branch July 15, 2026 11:01
mswilkison added a commit that referenced this pull request Jul 15, 2026
## Stack

3 of 3. Parent: #4161.

## Summary

- Track each live interactive signing member from its last accepted
activity using monotonic `Instant` state instead of wall-clock Open
time.
- Refresh activity for exact Open retries, accepted Round1 calls, and
retry-preserving Round2 failures; rejected traffic does not extend
lifetime.
- Add a forward-only test clock and regressions for active-at-boundary
attempts, idle siblings, invalid requests, retry-preserving failures,
expiry, and reset behavior.

## Review finding addressed

- P2: interactive inactivity TTL was measured from Open and could sweep
an attempt used moments earlier.

## Lifecycle invariant

Only accepted or retry-preserving member activity extends the TTL;
successful terminal operations and post-replacement failures still
retire the nonce handle.

## Validation

- `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml`: 273
library tests passed, 1 ignored; 30 admission-checker tests passed; 1
integration test passed.
- `cargo check --locked --manifest-path pkg/tbtc/signer/Cargo.toml
--all-targets`.
- `cargo clippy --locked --manifest-path pkg/tbtc/signer/Cargo.toml
--all-targets -- -D warnings`.
- `cargo fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --check` and
`git diff --check`.
- `pkg/tbtc/signer/scripts/run_phase5_chaos_suite.sh`: all 6
fault-injection scenarios passed.
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