feat(xmlenc): add encryption pipeline#96
Conversation
- implement AES-CBC/GCM encryption and recipient key wrapping - add document replacement, bounds, interoperability tests, and docs - import donor encryption templates without normalizing their bytes Closes #95
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughXMLEnc encryption support is added for AES-CBC/GCM content, RSA-OAEP and AES-KW recipients, XML element/content replacement, structured validation errors, donor fixtures, integration tests, and xmlsec1 interoperability. ChangesXMLEnc encryption pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant EncryptedDataBuilder
participant AES
participant KeyWrapper
participant xmlsec1
Caller->>EncryptedDataBuilder: configure and encrypt XML
EncryptedDataBuilder->>AES: encrypt plaintext
EncryptedDataBuilder->>KeyWrapper: wrap content key for recipients
KeyWrapper-->>EncryptedDataBuilder: return encrypted keys
EncryptedDataBuilder-->>Caller: return EncryptedData XML
Caller->>xmlsec1: decrypt generated XML
xmlsec1-->>Caller: return plaintext
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
| Filename | Overview |
|---|---|
| src/xmlenc/encrypt.rs | Adds content encryption, key wrapping, XML serialization, target selection, and bounded document replacement. |
| src/xmlenc/decrypt.rs | Updates RSA-OAEP error handling, shares element validation, and redacts symmetric keys from debug output. |
| src/xmlenc/types.rs | Adds encryption configuration, recipient, result, limit, and error types. |
| src/xmlenc/mod.rs | Exports the encryption API and adds shared XML element validation. |
| tests/xmlenc_encrypt_integration.rs | Adds integration coverage for encryption, document replacement, SAML inputs, and invalid configurations. |
Reviews (5): Last reviewed commit: "refactor(xmlenc): clarify element valida..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9d712aa95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Document why the selected node range makes the final closing marker unambiguous and lock the behavior with a round-trip regression test.
Document that direct PaddingScheme dispatch preserves SysRng failures as typed RSA errors rather than using the infallible convenience API.
Require the xmlenc feature before Cargo builds the encryption example so default example checks remain valid.
Require oversized XML input to fail at the size boundary before malformed content reaches the parser.
Reject oversized standalone XML before parser work or Content wrapper allocation.
Require encryption to reject empty direct and recipient key names before emitting XML rejected by the reciprocal parser.
Keep generated encryption XML compatible with the reciprocal parser by validating direct and recipient KeyName values.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6f59dd467
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add a regression test and typed limit contract proving document encryption must reject oversized XML before parsing it.
Reject oversized caller XML before invoking roxmltree while retaining the existing plaintext-size check for the selected node.
Prove Element plaintext accepts boundary whitespace and comments while rejecting processing instructions that document replacement cannot consume.
Share the reciprocal Element node-set contract so encryption and document decryption both allow boundary whitespace/comments and reject processing instructions.
Cover XML-forbidden characters across EncryptedData Id, recipient identifiers, and direct/wrapped KeyName values.
Reject code points outside the XML 1.0 Char production before serializing caller-controlled attributes or KeyName text.
![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
🤖 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 `@src/xmlenc/mod.rs`:
- Around line 34-54: Refactor has_single_element_with_boundary_trivia to iterate
over parent.children() with a for loop instead of mutating element_count inside
Iterator::all. Count element nodes, immediately return false when more than one
is found, and return false for invalid non-comment/non-boundary-whitespace nodes
while preserving the final true result for exactly one valid element.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f3f08cda-1d29-40c4-8dd4-6434a436f71c
📒 Files selected for processing (6)
Cargo.tomlsrc/xmlenc/decrypt.rssrc/xmlenc/encrypt.rssrc/xmlenc/mod.rssrc/xmlenc/types.rstests/xmlenc_encrypt_integration.rs
Use an explicit child-node loop so invalid siblings and a second element return immediately without closure state mutation.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Security
Testing
cargo nextest run --all-features --no-fail-fast(618 passed,0 skipped)cargo test --doc --all-features(4 passed)cargo check --all-featurescargo check --no-default-features --features xmlenccargo check --examplescargo check --examples --features xmlenccargo clippy --all-targets --all-features -- -D warningscargo package --allow-dirtyxmlsec1 1.3.12Closes #95