🧪 [testing improvement] Add tests for decrypt_handoff_blob#1422
🧪 [testing improvement] Add tests for decrypt_handoff_blob#1422senamakel wants to merge 3 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR extends the REST API test suite with comprehensive coverage for the ChangesREST API Decryption Tests
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
⚔️ Resolve merge conflicts
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/api/rest_tests.rs (1)
62-73: ⚡ Quick winExtract a shared fixture helper to remove repeated blob assembly logic.
The encrypt/split/combine/base64 flow is duplicated across multiple tests; a small helper will reduce drift risk and simplify future test additions.
♻️ Proposed refactor
+fn make_handoff_blob_b64(key_bytes: [u8; 32], iv_bytes: [u8; 16], plain: &[u8]) -> String { + let cipher = Aes256Gcm16::new_from_slice(&key_bytes).unwrap(); + let nonce = aes_gcm::aead::generic_array::GenericArray::from_slice(&iv_bytes); + let encrypted = cipher.encrypt(nonce, plain).unwrap(); + let (ciphertext, tag) = encrypted.split_at(encrypted.len() - 16); + + let mut combined = Vec::with_capacity(16 + 16 + ciphertext.len()); + combined.extend_from_slice(&iv_bytes); + combined.extend_from_slice(tag); + combined.extend_from_slice(ciphertext); + STANDARD.encode(combined) +}Also applies to: 100-111, 123-132, 150-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/rest_tests.rs` around lines 62 - 73, Extract the repeated AES-GCM encrypt/split/combine/base64 logic into a small test helper (e.g., fn make_encrypted_blob(plain: &str, key_bytes: &[u8], iv_bytes: &[u8]) -> (String, String)) that performs Aes256Gcm16::new_from_slice(...), encrypt(...), split_at(...), assembles iv + tag + ciphertext, and returns the base64 blob and base64 key (using STANDARD.encode); then replace the duplicated blocks that use cipher.encrypt, split_at, combined.extend_from_slice, and STANDARD.encode with calls to this helper (referencing Aes256Gcm16, encrypt, split_at, iv_bytes, key_bytes, STANDARD.encode in the existing tests such as the blocks around the current code).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test_anyhow.rs`:
- Line 6: The println! invocation contains a format placeholder but no argument;
update the println!("Display: {}"); call so the format string matches its
arguments—either remove the "{}" placeholder (e.g., println!("Display:");) or
supply the intended value (e.g., println!("Display: {}", some_value)) by passing
the correct variable/expression into the println! macro.
---
Nitpick comments:
In `@src/api/rest_tests.rs`:
- Around line 62-73: Extract the repeated AES-GCM encrypt/split/combine/base64
logic into a small test helper (e.g., fn make_encrypted_blob(plain: &str,
key_bytes: &[u8], iv_bytes: &[u8]) -> (String, String)) that performs
Aes256Gcm16::new_from_slice(...), encrypt(...), split_at(...), assembles iv +
tag + ciphertext, and returns the base64 blob and base64 key (using
STANDARD.encode); then replace the duplicated blocks that use cipher.encrypt,
split_at, combined.extend_from_slice, and STANDARD.encode with calls to this
helper (referencing Aes256Gcm16, encrypt, split_at, iv_bytes, key_bytes,
STANDARD.encode in the existing tests such as the blocks around the current
code).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe2fdde1-0e12-4783-b108-183b35396701
📒 Files selected for processing (2)
src/api/rest_tests.rstest_anyhow.rs
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| #[test] | ||
| fn decrypts_valid_blob() { | ||
| let key_bytes = [0x42u8; 32]; |
|
|
||
| #[test] | ||
| fn decrypt_fails_on_wrong_key() { | ||
| let key_bytes = [0x42u8; 32]; |
|
|
||
| #[test] | ||
| fn decrypt_fails_on_tampered_ciphertext() { | ||
| let key_bytes = [0x42u8; 32]; |
|
|
||
| #[test] | ||
| fn decrypt_fails_on_invalid_utf8() { | ||
| let key_bytes = [0x42u8; 32]; |
|
@jules pull from main and resolve the merge conflicts |
I attempted to merge |
Adds a comprehensive suite of unit tests for the cryptographic handoff decryption logic, covering: - Successful decryption of valid payloads - Bounds checking for minimum length (IV + Tag) - Base64 decoding error handling - AEAD authentication failure (wrong key or tampered ciphertext) - Post-decryption UTF-8 validation Tests are deterministic and avoid hardcoded secret literals to comply with security scanning policies.
2 participants
🎯 What: The testing gap addressed
decrypt_handoff_blobinsrc/api/rest.rs.📊 Coverage: What scenarios are now tested
✨ Result: Improved reliability and coverage for cryptographic token handoff.
PR created automatically by Jules for task 14953444737016356940 started by @senamakel
Summary by CodeRabbit