Skip to content

chore(kms-connector): enforce acl check#1944

Merged
mergify[bot] merged 1 commit intomainfrom
eudelins/chore/916/enforce-acl-check
Feb 9, 2026
Merged

chore(kms-connector): enforce acl check#1944
mergify[bot] merged 1 commit intomainfrom
eudelins/chore/916/enforce-acl-check

Conversation

@eudelins-zama
Copy link
Copy Markdown
Contributor

@cla-bot cla-bot bot added the cla-signed label Feb 9, 2026
@eudelins-zama
Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude
Copy link
Copy Markdown

claude bot commented Feb 9, 2026

Claude finished @eudelins-zama's task in 3m 45s —— View job


PR Review Complete

  • Pre-review validation
  • Gather context (PR summary, guidelines)
  • Run parallel review agents
  • Validate findings
  • Post final review

Code Review

No blocking issues found. This PR implements a security hardening change that makes ACL checks mandatory for user decryptions.

Summary of Changes

File Change
processor.rs Enforce mandatory ACL check for user decryption (previously skippable with warning)
processor.rs Remove unused warn import
Cargo.toml Version bump 0.10.0 → 0.12.0
Cargo.lock Lockfile updates

Analysis

Security Improvement

  • Previously, if tx_hash was missing, the ACL check was skipped with only a warning
  • Now, missing tx_hash results in an Irrecoverable error, failing the request properly
  • This closes the tracked issue #916

Error Handling

  • The ProcessingError::Irrecoverable type is appropriate since a missing tx_hash cannot be fixed by retrying
  • The error is properly logged via the existing error handling at line 63
  • Events are correctly marked as failed in the database

Code Quality

  • Follows existing codebase patterns for ok_or_else() usage
  • Error message is clear and actionable
  • Unused warn import properly removed

Minor Observation

Nit: Version skips from 0.10.0 to 0.12.0, bypassing 0.11.0. This doesn't block the PR but is worth noting for release tracking purposes.

Deployment Consideration

⚠️ Breaking Change: Any existing user_decryption_requests in production databases with NULL tx_hash values will fail permanently after this update. This is the intended behavior per issue #916, but operators should be aware during upgrades.


@mergify
Copy link
Copy Markdown

mergify bot commented Feb 9, 2026

🧪 CI Insights

Here's what we observed from your CI run for c5b6a2b.

🟢 All jobs passed!

But CI Insights is watching 👀

@eudelins-zama eudelins-zama marked this pull request as ready for review February 9, 2026 13:42
@eudelins-zama eudelins-zama requested review from a team and dartdart26 as code owners February 9, 2026 13:42
Copy link
Copy Markdown
Contributor

@maksymsur maksymsur left a comment

Choose a reason for hiding this comment

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

LGTM!

@eudelins-zama
Copy link
Copy Markdown
Contributor Author

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify bot commented Feb 9, 2026

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 22f7d0f

@mergify
Copy link
Copy Markdown

mergify bot commented Feb 9, 2026

Merge Queue Status

Rule: main


This pull request spent 2 hours 36 minutes 31 seconds in the queue, including 9 hours 54 minutes 56 seconds running CI.

Required conditions to merge

mergify bot added a commit that referenced this pull request Feb 9, 2026
@mergify mergify bot merged commit 22f7d0f into main Feb 9, 2026
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants