Repository navigation
Make removesNodes required on CanonicalizationOrTransformationAlgorithm #558
Description
Summary
Make removesNodes required on CanonicalizationOrTransformationAlgorithm. It shipped as optional in 6.x because it could not be required without breaking every custom algorithm; 7.0 is where that gets fixed.
Background
computeSignature() rejects a reference that encloses the signature when every transform on it provably cannot remove the Signature — otherwise the digest covers a Signature whose SignatureValue is still empty, and the result can never verify. Each algorithm declares that about itself:
readonly removesNodes = false; // canonicalization: renders the node-set, drops nothing
readonly removesNodes = true; // enveloped-signature: removes the SignatureThe problem with optional
Three states exist today, and the third is the bad one:
removesNodes |
Meaning | Check |
|---|---|---|
false |
provably preserves the Signature |
applies |
true |
may remove it | defers |
| undefined | we assume it may remove it | defers |
That third row is a default making a security-relevant decision for the implementer. Someone writing a pure canonicalization transform gets no protection and is never told — they did not choose to opt out, they just did not know the field existed. This is the shape AGENTS.md warns about: "Don't add a default for anything security-relevant. Throw instead."
Requiring it removes the row. Every algorithm states what it does, and the check is exact for custom algorithms and built-ins alike.
Scope
- Change
removesNodes?: booleantoremovesNodes: booleaninsrc/types.ts. - Drop the
algo != null &&/=== falsedefensiveness inSignedXml.preservesEveryNode(); the value is then always present. - Note the break in the changelog with the migration below.
Migration
Add one line to any custom canonicalization or transform:
class MyTransform implements CanonicalizationOrTransformationAlgorithm {
readonly removesNodes = false; // or true, if it drops nodes from the node-set
process(node: Node) { /* ... */ }
getAlgorithmName() { return "http://MyTransformation"; }
}Pick false only if the algorithm returns the same node-set it was given. If it filters nodes at all — an XPath transform, say — use true, which is also the conservative choice: it keeps the current deferring behaviour.
TypeScript implementors get a compile error naming the missing property, so the break is loud and mechanical. JavaScript implementors get no such signal, which is worth a prominent changelog entry: for them the field is silently undefined, and the runtime behaviour is unchanged from 6.x rather than newly broken.
Worth deciding alongside
Whether preservesEveryNode() should throw on an unregistered algorithm rather than returning false. Today an unknown URI defers here and then fails later in findCanonicalizationAlgorithm() with "canonicalization algorithm is not supported". That ordering is fine, but if the errors are ever reworked, the two paths should agree on which one reports first.
Related
- Introduced in the enveloped-signature guard on the
fix/omit-empty-transforms-cleanbranch (PR fix: omit <Transforms> element when no transforms are specified #542).
Activity
cjbarth commentedon Sep 9, 2026
Implemented directly in PR #542 rather than deferred —
removesNodesis required as of that branch, so there is no interim release carrying the optional form. Closing as done rather than planned; reopen if that PR does not land.