fix: Support nested enveloped signature location (#525)

[shunkica]

Description

This PR fixes an issue where the enveloped signature transformation was unable to find and remove Signature elements that were nested within other elements in the document.

Changes

• Updated the XPath query in from to to search for Signature elements at any depth, not just direct children
• Added a test case demonstrating the issue with nested signatures

Related Issue

Fixes #525

Testing

Added a test case that verifies signatures can be validated when the Signature element is nested within other elements in the document structure.

Summary by CodeRabbit

• Bug Fixes

  • Enhanced signature detection to properly handle nested structures within XML documents.

• Tests

  • Added validation for signature creation with nested signature locations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This change fixes a bug where the enveloped signature transform failed to locate signatures nested within child elements. The XPath selector was updated to search all descendants instead of just immediate children, and a corresponding test case was added to validate the fix works correctly.

Changes

Cohort / File(s)	Summary
Bug Fix src/enveloped-signature.ts	XPath selector modified from ./* (immediate children) to .//* (all descendants) for locating enveloped Signature elements when options.signatureNode is null, enabling detection of signatures nested in child elements.
Test Addition test/signature-integration-tests.spec.ts	New integration test added to verify signature creation and verification when the Signature element is placed inside a nested child element (non-direct child) of the root node.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 Through nested tunnels deep and wide,
I searched with XPath as my guide,
From /.* to .//* I'd leap,
Finding signatures buried deep!
All descendants now in sight—
The enveloped signature's flight! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: supporting nested enveloped signature locations through XPath modification and test validation.
Linked Issues check	✅ Passed	The pull request fully addresses issue #525 by modifying the XPath to find signatures at any depth and adding a test case that verifies nested signature validation works correctly.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the nested enveloped signature issue: XPath modification in source code and corresponding test case addition.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b673581 and 8c1af97.

📒 Files selected for processing (2)

• src/enveloped-signature.ts
• test/signature-integration-tests.spec.ts

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

Learnt from: shunkica
Repo: node-saml/xml-crypto PR: 506
File: src/signed-xml.ts:1159-1159
Timestamp: 2025-10-22T21:03:38.354Z
Learning: In node-saml/xml-crypto PR #506, the maintainer (shunkica) requested an issue to separate the overloaded Reference interface into distinct SigningReference and ValidationReference types. Initial hypothesis: signing-only (xpath, isEmptyUri, id, type), validation-only (uri, digestValue, validationError, signedReference), shared (transforms, digestAlgorithm, inclusiveNamespacesPrefixList). This should be proposed and designed in a follow-up, not altered in the current PR.

Learnt from: shunkica
Repo: node-saml/xml-crypto PR: 0
File: :0-0
Timestamp: 2025-10-22T21:50:05.454Z
Learning: In src/signed-xml.ts Line 1099, createReferences mutates ref.uri = id during signing. Maintain this behavior for now; remove/refactor in a separate PR as previously requested by the maintainer.

📚 Learning: 2025-10-22T21:50:05.454Z

Learnt from: shunkica
Repo: node-saml/xml-crypto PR: 0
File: :0-0
Timestamp: 2025-10-22T21:50:05.454Z
Learning: In src/signed-xml.ts Line 1099, createReferences mutates ref.uri = id during signing. Maintain this behavior for now; remove/refactor in a separate PR as previously requested by the maintainer.

Applied to files:

• test/signature-integration-tests.spec.ts
• src/enveloped-signature.ts

📚 Learning: 2025-10-22T21:03:38.354Z

Learnt from: shunkica
Repo: node-saml/xml-crypto PR: 506
File: src/signed-xml.ts:1159-1159
Timestamp: 2025-10-22T21:03:38.354Z
Learning: In node-saml/xml-crypto PR #506, the maintainer (shunkica) requested an issue to separate the overloaded Reference interface into distinct SigningReference and ValidationReference types. Initial hypothesis: signing-only (xpath, isEmptyUri, id, type), validation-only (uri, digestValue, validationError, signedReference), shared (transforms, digestAlgorithm, inclusiveNamespacesPrefixList). This should be proposed and designed in a follow-up, not altered in the current PR.

Applied to files:

• test/signature-integration-tests.spec.ts
• src/enveloped-signature.ts

📚 Learning: 2025-10-25T14:41:13.728Z

Learnt from: shunkica
Repo: node-saml/xml-crypto PR: 519
File: example/new-api-example.js:1-9
Timestamp: 2025-10-25T14:41:13.728Z
Learning: Example files in the node-saml/xml-crypto repository should use `require("xml-crypto")` (the package name) rather than relative paths to build artifacts, since they demonstrate usage from an end-user's perspective.

Applied to files:

• test/signature-integration-tests.spec.ts

📚 Learning: 2025-10-22T21:50:05.454Z

Learnt from: shunkica
Repo: node-saml/xml-crypto PR: 0
File: :0-0
Timestamp: 2025-10-22T21:50:05.454Z
Learning: The current Reference fields are defined in src/types.ts Lines 109–168: xpath?, transforms, digestAlgorithm, uri, digestValue?, inclusiveNamespacesPrefixList, isEmptyUri, ancestorNamespaces?, validationError?, getValidatedNode(), signedReference?.

Applied to files:

• src/enveloped-signature.ts

🧬 Code graph analysis (1)

test/signature-integration-tests.spec.ts (2)

src/index.ts (1)

• SignedXml (6-6)

src/signed-xml.ts (1)

• SignedXml (30-1422)

🔇 Additional comments (2)

src/enveloped-signature.ts (1)

20-20: LGTM! XPath updated to search all descendants.

The change from ./* to .//* correctly expands the search to locate Signature elements at any depth, fixing the nested signature issue. This makes the XPath consistent with line 37, which already uses the descendant selector when signatureNode is provided.

test/signature-integration-tests.spec.ts (1)

227-258: LGTM! Test properly validates nested signature support.

The test correctly exercises the fix by:

1. Creating a signature nested inside a child element via the location option
2. Using the enveloped-signature transform (which now searches all descendants)
3. Verifying the signature validates successfully

The test structure follows existing patterns in the file and comprehensively validates the nested signature scenario described in issue #525.

---

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

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