fix: omit <Transforms> element when no transforms are specified

README.md

@@ -417,7 +417,9 @@ To sign xml documents:
 
 - `addReference({ xpath, transforms, digestAlgorithm, id, type, isEmptyUri, inclusiveNamespacesPrefixList })` - adds a reference to a xml element where:
   - `xpath` - a string containing a XPath expression referencing a xml element
-  - `transforms` - an array of [transform algorithms](#canonicalization-and-transformation-algorithms), the referenced element will be transformed for each value in the array
+  - `transforms` - an array of [transform algorithms](#canonicalization-and-transformation-algorithms), the referenced element will be transformed for each value in the array.
+    Omit it, or pass an empty array, to emit no `Transforms` element; the referenced element is then digested after canonicalization alone.
+    If the referenced element encloses the signature, this array has to include `http://www.w3.org/2000/09/xmldsig#enveloped-signature`, which removes the `Signature` before digesting; `computeSignature()` rejects the reference otherwise rather than emit a signature that cannot verify. Supply your own transform and that check defers to it, since only you know whether it removes the `Signature`.
   - `digestAlgorithm` - one of the supported [hashing algorithms](#hashing-algorithms)
   - `id` - an optional `Id` attribute to add to the reference element
   - `type` - the optional `Type` attribute to add to the reference element (represented as a URI)
@@ -516,6 +518,9 @@ Custom transformation algorithm.
 
 ```javascript
 class MyTransformation {
+  // Whether this drops nodes from the node-set, rather than only re-serializing it.
+  removesNodes = false;
+
   // Given a node (from the xmldom module), return its canonical representation as a string.
   process(node) {
     // You should apply your transformation before returning.
@@ -528,10 +533,16 @@ class MyTransformation {
 }
 ```
 
+`removesNodes` is required on every canonicalization and transformation algorithm, including ones written in plain JavaScript. It is checked as the algorithm is instantiated, so a missing or non-boolean value throws from `checkSignature()` just as it does from `computeSignature()` — a verifier whose own registry is misconfigured refuses rather than canonicalizing with an algorithm that never said what it does. Registrations you never use are not checked.
+
+Declare `false` if the algorithm returns the node-set it was given, and `true` if it filters nodes at all. `computeSignature()` also reads this to decide whether a reference enclosing the signature can ever verify, and the two mistakes cost you different things: declaring `true` when the algorithm actually preserves everything skips that check, so you can sign a reference nobody can verify, while declaring `false` when it actually filters makes the check reject a reference that would have worked.
+
 Custom canonicalization is actually the same as custom transformation. It is applied on the SignedInfo rather than on references.
 
 ```javascript
 class MyCanonicalization {
+  removesNodes = false;
+
   // Given a node (from the xmldom module), return its canonical representation as a string.
   process(node) {
     // You should apply your canonicalization before returning.

src/c14n-canonicalization.ts

@@ -12,6 +12,8 @@ type C14nOptionsWithXmlAttributes = CanonicalizationOrTransformationAlgorithmPro
 };
 
 export class C14nCanonicalization implements CanonicalizationOrTransformationAlgorithm {
+  readonly removesNodes: boolean = false;
+
   protected includeComments = false;
 
   constructor() {

src/enveloped-signature.ts

@@ -9,6 +9,8 @@ import type {
 } from "./types";
 
 export class EnvelopedSignature implements CanonicalizationOrTransformationAlgorithm {
+  readonly removesNodes: boolean = true;
+
   protected includeComments = false;
 
   constructor() {

src/exclusive-canonicalization.ts

@@ -21,6 +21,8 @@ export function warnUnlessExclusiveC14nNamespace(inclusiveNamespaces: Element):
 }
 
 export class ExclusiveCanonicalization implements CanonicalizationOrTransformationAlgorithm {
+  readonly removesNodes: boolean = false;
+
   protected includeComments = false;
 
   constructor() {

src/signed-xml.ts

@@ -611,12 +611,17 @@ export class SignedXml {
     };
   }
 
-  private getCanonReferenceXml(doc: Document, ref: Reference, node: Node) {
-    /**
-     * Search for ancestor namespaces before canonicalization.
-     */
-    if (Array.isArray(ref.transforms)) {
-      ref.ancestorNamespaces = utils.findAncestorNs(doc, ref.xpath, this.namespaceResolver);
+  private getCanonReferenceXml(ref: Reference, node: Node) {
+    // Derive the scope from the node being digested: findAncestorNs re-runs ref.xpath and
+    // keeps only the first match, which digests every reference addAllReferences() created
+    // from that xpath in the first match's scope. findAncestorNsForElement (above) already
+    // takes a known element instead, so reuse it here. Every call site establishes
+    // element-ness except validateReference's document-level-processing-instruction case,
+    // which passes the whole Document — one with no ancestor namespace scope of its own.
+    if (isDomNode.isElementNode(node)) {
+      ref.ancestorNamespaces = utils.findAncestorNsForElement(node);
+    } else {
+      isDomNode.assertIsDocumentNode(node);
     }
 
     const c14nOptions = {
@@ -693,7 +698,17 @@ export class SignedXml {
     if (name != null) {
       const algo = this.CanonicalizationAlgorithms[name];
       if (algo) {
-        return new algo();
+        const instance = new algo();
+        // Registering is reachable from JavaScript, where the required type buys
+        // nothing. Checking here covers every algorithm we instantiate, rather than
+        // only the ones a particular caller happens to look at.
+        if (typeof instance.removesNodes !== "boolean") {
+          throw new Error(
+            `canonicalization algorithm '${name}' must declare a boolean 'removesNodes'`,
+          );
+        }
+
+        return instance;
       }
     }
 
@@ -736,7 +751,7 @@ export class SignedXml {
         }
       }
 
-      const canonXml = this.getCanonReferenceXml(doc, ref, elem);
+      const canonXml = this.getCanonReferenceXml(ref, elem);
       const hash = this.findHashAlgorithm(ref.digestAlgorithm);
       const digest = hash.getHash(canonXml);
 
@@ -802,7 +817,7 @@ export class SignedXml {
       return undefined;
     }
 
-    const canonXml = this.getCanonReferenceXml(doc, ref, elem);
+    const canonXml = this.getCanonReferenceXml(ref, elem);
     const hash = this.findHashAlgorithm(ref.digestAlgorithm);
     const digest = hash.getHash(canonXml);
 
@@ -1043,13 +1058,9 @@ export class SignedXml {
       throw new Error("digestAlgorithm is required");
     }
 
-    if (!utils.isArrayHasLength(transforms)) {
-      throw new Error("transforms must contain at least one transform algorithm");
-    }
-
     this.references.push({
       xpath,
-      transforms,
+      transforms: transforms ?? [],
       digestAlgorithm,
       uri,
       digestValue,
@@ -1472,6 +1483,22 @@ export class SignedXml {
           );
         }
 
+        // The Signature is already in the document, and its SignatureValue is still
+        // empty, so digesting content that encloses it can never match on verification.
+        // https://www.w3.org/TR/xmldsig-core1/#sec-EnvelopedSignature
+        if (
+          utils.isDescendantOf(signatureElem, node) &&
+          ref.transforms
+            .map((transform) => this.findCanonicalizationAlgorithm(transform))
+            .every((algorithm) => !algorithm.removesNodes)
+        ) {
+          throw new Error(
+            `The reference ${ref.xpath} encloses the signature, so it requires the ` +
+              "http://www.w3.org/2000/09/xmldsig#enveloped-signature transform. " +
+              "Without it the signature cannot be verified.",
+          );
+        }
+
         // Compute the target URI (ID already ensured earlier, extract it)
         let targetUri: string;
         if (ref.isEmptyUri) {
@@ -1500,35 +1527,39 @@ export class SignedXml {
           referenceElem.setAttribute("Type", ref.type);
         }
 
-        const transformsElem = signatureDoc.createElementNS(
-          signatureNamespace,
-          `${currentPrefix}Transforms`,
-        );
-
-        for (const trans of ref.transforms || []) {
-          const transform = this.findCanonicalizationAlgorithm(trans);
-          const transformElem = signatureDoc.createElementNS(
+        if (utils.isArrayHasLength(ref.transforms)) {
+          const transformsElem = signatureDoc.createElementNS(
             signatureNamespace,
-            `${currentPrefix}Transform`,
+            `${currentPrefix}Transforms`,
           );
-          transformElem.setAttribute("Algorithm", transform.getAlgorithmName());
-
-          if (
-            utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList) &&
-            isExclusiveC14n(transform.getAlgorithmName())
-          ) {
-            const inclusiveNamespacesElem = signatureDoc.createElementNS(
-              exclusiveC14nNamespace,
-              "InclusiveNamespaces",
-            );
-            inclusiveNamespacesElem.setAttribute(
-              "PrefixList",
-              ref.inclusiveNamespacesPrefixList.join(" "),
+
+          for (const trans of ref.transforms) {
+            const transform = this.findCanonicalizationAlgorithm(trans);
+            const transformElem = signatureDoc.createElementNS(
+              signatureNamespace,
+              `${currentPrefix}Transform`,
             );
-            transformElem.appendChild(inclusiveNamespacesElem);
+            transformElem.setAttribute("Algorithm", transform.getAlgorithmName());
+
+            if (
+              utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList) &&
+              isExclusiveC14n(transform.getAlgorithmName())
+            ) {
+              const inclusiveNamespacesElem = signatureDoc.createElementNS(
+                exclusiveC14nNamespace,
+                "InclusiveNamespaces",
+              );
+              inclusiveNamespacesElem.setAttribute(
+                "PrefixList",
+                ref.inclusiveNamespacesPrefixList.join(" "),
+              );
+              transformElem.appendChild(inclusiveNamespacesElem);
+            }
+
+            transformsElem.appendChild(transformElem);
           }
 
-          transformsElem.appendChild(transformElem);
+          referenceElem.appendChild(transformsElem);
         }
 
         const digestAlgorithm = this.findHashAlgorithm(ref.digestAlgorithm);
@@ -1545,7 +1576,6 @@ export class SignedXml {
         );
         digestValueElem.textContent = digestValue ?? this.calculateReferenceDigest(ref, node);
 
-        referenceElem.appendChild(transformsElem);
         referenceElem.appendChild(digestMethodElem);
         referenceElem.appendChild(digestValueElem);
 
@@ -1656,6 +1686,9 @@ export class SignedXml {
 
     // Octets are parsed into a node-set for the next transform, and a node-set left at the end is
     // converted to octets with C14N: https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel
+    // (When transforms is empty this loop simply doesn't run, and the isNodeLike check
+    // below applies plain C14N to the untransformed node-set — exactly the SMPTE-required
+    // no-transforms case, so no separate empty-transforms branch is needed here.)
     for (const [index, transformName] of transforms.entries()) {
       if (!isDomNode.isNodeLike(transformedXml)) {
         transformedXml = this.parseTransformInput(transformedXml, transformName, preserveDocument);
@@ -1687,12 +1720,21 @@ export class SignedXml {
       );
     }
 
+    if (typeof transformedXml === "string") {
+      return transformedXml;
+    }
+
     if (isDomNode.isNodeLike(transformedXml)) {
       transformedXml = this.findCanonicalizationAlgorithm(
         "http://www.w3.org/TR/2001/REC-xml-c14n-20010315",
       ).process(transformedXml, transformOptions);
     }
 
+    // No separate empty-transforms case needed here: the isNodeLike check above already
+    // applies C14N to any remaining node-set, which is exactly what happens when
+    // effectiveTransforms is empty (the for loop above never ran) — the SMPTE-required
+    // no-transforms case is just the simplest instance of the same general rule upstream
+    // already enforces for "a node-set left at the end of the transforms" (see #581).
     return transformedXml.toString();
   }
 

src/types.ts

@@ -145,6 +145,8 @@ export interface Reference {
   xpath?: string;
 
   // An array of transforms to be applied to the data before signing.
+  // An empty array emits no Transforms element; the referenced node is then
+  // digested directly, after C14N, per the XMLDSig processing model.
   transforms: ReadonlyArray<CanonicalizationOrTransformAlgorithmType>;
 
   // The algorithm used to calculate the digest value of the data.
@@ -186,6 +188,29 @@ export interface CanonicalizationOrTransformationAlgorithm {
   ): Node | string;
 
   getAlgorithmName(): CanonicalizationOrTransformAlgorithmType;
+
+  /**
+   * Whether this algorithm can drop nodes from the node-set it is given, rather
+   * than only re-serializing it. Canonicalization algorithms declare `false`;
+   * the enveloped-signature transform declares `true`.
+   *
+   * Required, and enforced on both sides: every algorithm is checked as it is
+   * instantiated, so a missing or non-boolean value throws from `computeSignature()`
+   * and from `checkSignature()` alike. That covers algorithms registered from
+   * JavaScript, which this type cannot reach. Registrations that are never used are
+   * not checked.
+   *
+   * `computeSignature()` also reads the value to decide whether a reference that
+   * encloses the signature can ever verify: if every transform on that reference
+   * declares `false`, the `Signature` provably survives into the digest, so the
+   * reference is rejected rather than signed into something unverifiable.
+   *
+   * Declare `false` if the algorithm returns the node-set it was given, `true` if it
+   * filters nodes at all. Declaring `true` when it actually preserves everything
+   * skips the check above and can sign a reference nobody can verify; declaring
+   * `false` when it actually filters can reject a reference that would have worked.
+   */
+  removesNodes: boolean;
 }
 
 /** Implement this to create a new HashAlgorithm */

src/utils.ts

@@ -579,6 +579,14 @@ function isElementSubset(docSubset: Node[]): docSubset is Element[] {
   return docSubset.every((node) => isDomNode.isElementNode(node));
 }
 
+/**
+ * Collect, deduplicate, and filter ancestor namespace declarations for a given element.
+ * Namespaces already declared directly on the element are excluded from the result
+ * to avoid double-emission during C14N serialization.
+ *
+ * @param node - The element whose ancestor namespace declarations to process
+ * @returns Deduplicated array of ancestor namespace prefixes not already declared on the element
+ */
 export function findAncestorNsForElement(node: Element): NamespacePrefix[] {
   const ancestorNs = collectAncestorNamespaces(node);
   const ancestorNsWithoutDuplicate: NamespacePrefix[] = [];

test/c14n-non-exclusive-unit-tests.spec.ts

@@ -172,6 +172,23 @@ describe("C14N non-exclusive canonicalization tests", function () {
     test_findAncestorNs(xml, xpath, []);
   });
 
+  for (const { label, subsetXpath } of [
+    { label: "attributes", subsetXpath: "//@attr" },
+    { label: "text nodes", subsetXpath: "//*[local-name()='child']/text()" },
+  ]) {
+    it(`findAncestorNs: Should reject a document subset of ${label}`, function () {
+      // Only elements carry namespace declarations. Without this check the
+      // non-element reaches findSubsetNSPrefixes, whose `.attributes` is null
+      // there, and the caller gets a TypeError instead of a usable message.
+      const xml = "<root xmlns:anc='urn:ancestor'><child attr='value'>text</child></root>";
+      const doc = new xmldom.DOMParser().parseFromString(xml);
+
+      expect(() => findAncestorNs(doc, subsetXpath)).to.throw(
+        "Document subset must be list of elements",
+      );
+    });
+  }
+
   // Tests for c14nCanonicalization
   it("C14n: Correctly picks up root ancestor namespace", function () {
     const xml = "<root xmlns:aaa='bbb'><child1><child2></child2></child1></root>";

test/canonicalization-unit-tests.spec.ts

@@ -505,6 +505,9 @@ describe("Canonicalization unit tests", function () {
 
     const sig = new SignedXml();
     sig.CanonicalizationAlgorithms["urn:test:malformed"] = class {
+      // Returns a fixed string rather than the node-set it was given.
+      removesNodes = true;
+
       process() {
         return "<y>";
       }