[kwsantiago]

Summary

The Nitro enclave's policy rate limits were in-memory token buckets that started full, so the parent host could restart the enclave to refill every bucket. They were also keyed by the host-chosen key name alone: one secret imported under two names got two sets of limits, a second MaxPerX rule on a key reused the first rule's capacity, and recording a signature created buckets with fixed capacities that a later check would pick up.

• Buckets start empty at launch and whenever a policy is loaded, and refill at their rule's rate from there. A host-forced restart buys nothing beyond the normal refill.
• Buckets are keyed by the key's public key, the window and the rule's maximum.
• Checking a rule spends nothing. A successful Sign or SignPsbt request is counted once against every rate rule in the policy, including rules its branch skipped, and an empty bucket stays empty rather than going into debt. Refused requests and multi-destination PSBTs no longer spend extra tokens.
• handle_sign and handle_sign_psbt refuse an unknown key name with KeyNotFound before evaluating the policy.
• The limiter no longer needs a mutex: the server handles one request at a time under &mut self.

Cost

Every launch now waits before a key's first signature: a rule allowing max per window allows its first signature window/max after the policy is loaded, and its full burst a window after. MaxPerHour(10) signs first after six minutes. Documented in docs/ENCLAVE.md, along with the fact that each running enclave keeps its own count.

Testing

• cargo +1.89.0 test --manifest-path keep-enclave/enclave/Cargo.toml: 95 passed.
• New tests cover the empty start, refill, drain on policy load, cleanup of idle and still-refilling buckets, the zero maximum, one secret under two names, stacked rules, a skipped Or branch, a refused request, a multi-destination PSBT and unknown key names. A test-only clock skew replaces sleeping.
• Mutation checks: starting buckets full, ignoring the maximum in the bucket key, recording nothing, skipping nested rules, spending on check, allowing debt, a drain that keeps the old anchor, keying by name and a cleanup that drops live buckets each fail at least one new test.

Summary by CodeRabbit

• Rate Limits
  • Signing and PSBT requests now share rate-limit counts for the same signing key, regardless of its imported name or the number of PSBT inputs.
  • Per-minute, hourly, and daily limits refill gradually over their respective periods. Limits start fully used when a policy is loaded and reset when the enclave restarts.
  • Requests for unknown keys are rejected before policy checks or signing.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5849fc59-571b-4d2a-8e85-1cee19052674

Commits

Reviewing files that changed from the base of the PR and between 1e1bc82 and 67464bb.

Files selected for processing: 4

• docs/ENCLAVE.md
• keep-enclave/enclave/src/policy.rs
• keep-enclave/enclave/src/rate_limit.rs
• keep-enclave/enclave/src/vsock_server.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

Rate limits now use signing public keys instead of key names. Token buckets refill over time, and signing handlers resolve keys before policy evaluation. Documentation and tests describe and verify the updated counting behavior.

Changes

Public-key rate limits

Layer / File(s)	Summary
Token bucket behavior keep-enclave/enclave/src/rate_limit.rs	RateLimiter now stores buckets by window, public key, and maximum. Checks do not consume tokens; recording decrements configured buckets. Buckets refill over time, and policy loads drain existing state.
Policy rate accounting keep-enclave/enclave/src/policy.rs, docs/ENCLAVE.md	Policy checks use the signing public key for minute, hour, and day windows. Operation recording collects limits from nested And and Or rules. Documentation describes request counting, refill timing, and restart behavior.
Signing integration keep-enclave/enclave/src/vsock_server.rs	Signing handlers resolve the public key before policy checks and use it for destination checks and successful-operation recording. Tests cover key sharing, PSBT request counting, refused requests, policy replacement, and unknown keys.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant VsockServer
  participant pubkey_for
  participant PolicyEngine
  participant RateLimiter
  VsockServer->>pubkey_for: Resolve signing public key
  pubkey_for-->>VsockServer: Return public key or KeyNotFound
  VsockServer->>PolicyEngine: Evaluate policy with public key
  PolicyEngine->>RateLimiter: Check configured windows
  RateLimiter-->>PolicyEngine: Return allowance
  PolicyEngine-->>VsockServer: Return policy result
  VsockServer->>PolicyEngine: Record successful operation by public key
  PolicyEngine->>RateLimiter: Decrement configured buckets

Loading

Suggested reviewers: wksantiago

Merge Risk: Minimal · up to 67464

The documented rate-limit behavior has no identified blocker to merging, subject to normal test checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main rate-limit changes: preventing refills caused by restarts, key renames, and stacked policy rules.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 20 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the ticking flow
Each public key gets room to grow
The buckets fill as minutes pass
One PSBT counts, though inputs amass
I nibble docs, then hop away}

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