test(xmldsig): expand signing interop coverage#91
Conversation
- Verify xml-sec and xmlsec1 signatures in both directions\n- Add SAML signing, donor corpus, and negative-vector coverage\n- Document signing and verification examples\n\nCloses #90
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds XMLDSig signing and verification examples, expands documentation, introduces donor and negative fixtures, adds SAML Response signing tests, validates interoperability with ChangesXMLDSig signing and verification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SignContext
participant xmlsec1
participant VerifyContext
participant DefaultKeyResolver
SignContext->>xmlsec1: submit generated signed XML
xmlsec1-->>SignContext: report verification result
xmlsec1->>VerifyContext: provide externally signed XML
VerifyContext->>DefaultKeyResolver: resolve embedded certificate
DefaultKeyResolver-->>VerifyContext: return verification key
VerifyContext-->>xmlsec1: return valid or digest-mismatch status
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
|
@codex review |
|
| Filename | Overview |
|---|---|
| tests/xmlsec1_interop.rs | Adds reciprocal xmlsec1 interoperability tests and correctly registers each case-sensitive ID attribute used by the exercised documents. |
| tests/saml_signing_integration.rs | Adds end-to-end SAML Response signing, verification, schema-order, and assertion-tampering coverage. |
| tests/donor_interop_suite.rs | Adds explicit valid and fail-closed accounting for the imported XMLDSig interoperability vectors. |
| .github/workflows/ci.yml | Builds the checksum-pinned xmlsec1 1.3.12 release for mandatory interoperability coverage. |
Reviews (5): Last reviewed commit: "fix(xmldsig): require xmlsec1 add-id sup..." | 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: 3ba398585a
ℹ️ 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".
![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 `@tests/donor_negative_vectors.rs`:
- Around line 23-26: Update the `VerifyContext::new()` setup in the
inconsistent-X509Data test to configure `DefaultKeyResolver`, matching the
resolver configuration used by the subsequent `bad_sig` test while preserving
the existing expected parse failure.
🪄 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: e192f8f9-3573-4e5c-9102-bc5e38fc5993
📒 Files selected for processing (69)
.github/workflows/ci.ymlREADME.mdexamples/sign.rsexamples/verify.rssrc/xmldsig/mod.rstests/donor_interop_suite.rstests/donor_negative_vectors.rstests/fixtures/saml/response_signing_template.xmltests/fixtures/xmldsig/README.mdtests/fixtures/xmldsig/phaos-xmldsig-three/signature-rsa-enveloped-bad-digest-val.xmltests/fixtures/xmldsig/phaos-xmldsig-three/signature-rsa-enveloped-bad-sig.xmltests/fixtures/xmldsig/phaos-xmldsig-three/signature-rsa-enveloped.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-derencoded-ec.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-derencoded-rsa.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-hmac-sha1-truncated160.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-hmac-sha1-truncated40.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-hmac-sha224.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-hmac-sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-hmac-sha384.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-hmac-sha512.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-keyinforeference-rsa.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha1.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha1_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha224.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha256_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha384.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha384_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha512.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p256_sha512_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha1.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha1_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha224.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha256_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha384.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha384_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha512.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p384_sha512_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha1.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha1_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha224.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha256_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha384.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha384_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha512.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-p521_sha512_4050.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-rsa-sha224.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-rsa-sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-rsa_sha384.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-rsa_sha512.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-sha224-rsa_sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-sha256-rsa-sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-sha384-rsa_sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-sha512-rsa_sha256.xmltests/fixtures/xmldsig/xmldsig11-interop-2012/signature-enveloping-x509digest-rsa.xmltests/fixtures/xmldsig/xmldsig2ed-tests/defCan-1.xmltests/fixtures/xmldsig/xmldsig2ed-tests/defCan-2.xmltests/fixtures/xmldsig/xmldsig2ed-tests/defCan-3.xmltests/fixtures/xmldsig/xmldsig2ed-tests/xpointer-1-SUN.xmltests/fixtures/xmldsig/xmldsig2ed-tests/xpointer-2-SUN.xmltests/fixtures/xmldsig/xmldsig2ed-tests/xpointer-3-SUN.xmltests/fixtures/xmldsig/xmldsig2ed-tests/xpointer-4-SUN.xmltests/fixtures/xmldsig/xmldsig2ed-tests/xpointer-5-SUN.xmltests/fixtures/xmldsig/xmldsig2ed-tests/xpointer-6-SUN.xmltests/fixtures_smoke.rstests/saml_signing_integration.rstests/xmlsec1_interop.rs
- Build a pinned XMLSec release in CI\n- Register template IDs and emit KeyInfo in the signing example\n- Keep negative-vector resolver setup consistent
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: def992d953
ℹ️ 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".
Use the upstream release asset in CI, keep local Rust-only runs independent of xmlsec1, and cover an actual enveloped root reference.
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 @.github/workflows/ci.yml:
- Line 55: Update the curl download command for xmlsec1.tar.gz to include retry
behavior with three attempts, preserving the existing failure and
location-following options.
🪄 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: 5397f8a3-d278-4e3f-9d95-59d9a16a970c
📒 Files selected for processing (3)
.github/workflows/ci.ymlsrc/xmldsig/mod.rstests/xmlsec1_interop.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de13e6fbbb
ℹ️ 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".
Retry the pinned release download and skip local interop tests when the installed XMLSec CLI lacks required options.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70c57bcd0b
ℹ️ 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".
Skip local interop tests unless xmlsec1 is at least 1.3.8, the release that introduced the required add-id-attr option.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! 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
Validation
cargo check --workspace --all-featurescargo clippy --all-targets --all-features -- -D warningscargo nextest run --all-features(555 passed)cargo test --doc --all-features(4 passed)Closes #90