Summary
Found while chasing a codecov/patch failure on #571. Three separate things in src/signed-xml.ts, worst first. They could reasonably be split into two issues — (1) is a behaviour question, (2) and (3) are test hygiene — but they surfaced together and share a cause, so they are written up together.
1. Two documented constructor options have no test at all
| Option |
Documented |
Never executes |
implicitTransforms |
README §Caring for Implicit transform, with a worked example, plus the options list |
src/signed-xml.ts:766-767 — the forEach that appends them to a reference's transforms |
idAttribute |
README options list, line 251 |
src/signed-xml.ts:162 — this.idAttributes.unshift(idAttribute) |
grep -o implicitTransforms test/*.spec.ts returns nothing. idAttribute returns two hits, but both are a local variable of that name in an unrelated assertion, not the option being passed.
So the package documents two options, one of them with its own README section, and has no evidence either of them works. This is the part worth acting on.
2. A test passes on the wrong error
signer appends signature to a non-existing reference node in test/signature-unit-tests.spec.ts reads as coverage for the the following xpath cannot be used because it was not found path. It is not:
- it never sets
canonicalizationAlgorithm, so computeSignature throws out of createSignedInfo with Missing canonicalizationAlgorithm when trying to create signed info for XML before the location.reference lookup is reached;
- the only assertion is
expect(err).not.to.be.an.instanceof(TypeError), which every Error satisfies.
Coverage confirms the path it names never runs: src/signed-xml.ts:1026-1033 is uncovered on master.
Fix is to set canonicalizationAlgorithm and assert on the message, at which point it covers what it claims.
3. Error contracts are broadly unverified
src/signed-xml.ts has 529 statements, 70 uncovered (13.2%). Of the uncovered, 32 are throw sites:
| Method |
Uncovered throws |
loadReference |
5 — every could not find …DigestMethod/Algorithm/DigestValue… guard |
getCanonSignedInfoXml |
5 — No signature found., Missing canonicalizationAlgorithm…, could not find SignedInfo…, multiple/whole xml dom guards |
checkSignature |
5 |
computeSignature |
5 |
addReference |
3 — including digestAlgorithm is required and transforms must contain at least one transform algorithm |
findSignatureAlgorithm |
2 — including signatureAlgorithm is required |
findCanonicalizationAlgorithm, findHashAlgorithm, calculateSignatureValue, validateElementAgainstReferences, loadSignature, namespace resolver |
1 each (7 total) |
Two of these are the errors AGENTS.md singles out as deliberate API design — signatureAlgorithm is required and digestAlgorithm is required exist specifically so nobody silently inherits SHA-1 — and neither has a test.
Also uncovered, and not an error path: the string branch of validateElementAgainstReferences (src/signed-xml.ts:500-502). The method is public and accepts Element | string; only the Element form is exercised.
One caveat on the number
17 of the 70 uncovered statements are callback-form branches that #571 removes, so the count drops on its own when that lands. Worth re-measuring after it rather than chasing 13.2% today.
Why this matters here
AGENTS.md already states the standard: "Uncovered code indicates either inadequately tested public behavior or code that may be unnecessary; determine which." For (1) the answer is inadequately tested public behaviour. For (3) these are the messages that tell a caller their configuration is unsafe, in a library where failing closed is the whole point — an error that stops being thrown, or starts saying something else, is a silent change to the contract.
Suggested scope
🤖 Generated with Claude Code
Summary
Found while chasing a
codecov/patchfailure on #571. Three separate things insrc/signed-xml.ts, worst first. They could reasonably be split into two issues — (1) is a behaviour question, (2) and (3) are test hygiene — but they surfaced together and share a cause, so they are written up together.1. Two documented constructor options have no test at all
implicitTransformssrc/signed-xml.ts:766-767— theforEachthat appends them to a reference's transformsidAttributesrc/signed-xml.ts:162—this.idAttributes.unshift(idAttribute)grep -o implicitTransforms test/*.spec.tsreturns nothing.idAttributereturns two hits, but both are a local variable of that name in an unrelated assertion, not the option being passed.So the package documents two options, one of them with its own README section, and has no evidence either of them works. This is the part worth acting on.
2. A test passes on the wrong error
signer appends signature to a non-existing reference nodeintest/signature-unit-tests.spec.tsreads as coverage for thethe following xpath cannot be used because it was not foundpath. It is not:canonicalizationAlgorithm, socomputeSignaturethrows out ofcreateSignedInfowithMissing canonicalizationAlgorithm when trying to create signed info for XMLbefore thelocation.referencelookup is reached;expect(err).not.to.be.an.instanceof(TypeError), which everyErrorsatisfies.Coverage confirms the path it names never runs:
src/signed-xml.ts:1026-1033is uncovered onmaster.Fix is to set
canonicalizationAlgorithmand assert on the message, at which point it covers what it claims.3. Error contracts are broadly unverified
src/signed-xml.tshas 529 statements, 70 uncovered (13.2%). Of the uncovered, 32 arethrowsites:loadReferencecould not find …DigestMethod/Algorithm/DigestValue…guardgetCanonSignedInfoXmlNo signature found.,Missing canonicalizationAlgorithm…,could not find SignedInfo…, multiple/whole xml domguardscheckSignaturecomputeSignatureaddReferencedigestAlgorithm is requiredandtransforms must contain at least one transform algorithmfindSignatureAlgorithmsignatureAlgorithm is requiredfindCanonicalizationAlgorithm,findHashAlgorithm,calculateSignatureValue,validateElementAgainstReferences,loadSignature, namespace resolverTwo of these are the errors AGENTS.md singles out as deliberate API design —
signatureAlgorithm is requiredanddigestAlgorithm is requiredexist specifically so nobody silently inherits SHA-1 — and neither has a test.Also uncovered, and not an error path: the
stringbranch ofvalidateElementAgainstReferences(src/signed-xml.ts:500-502). The method is public and acceptsElement | string; only theElementform is exercised.One caveat on the number
17 of the 70 uncovered statements are callback-form branches that #571 removes, so the count drops on its own when that lands. Worth re-measuring after it rather than chasing 13.2% today.
Why this matters here
AGENTS.mdalready states the standard: "Uncovered code indicates either inadequately tested public behavior or code that may be unnecessary; determine which." For (1) the answer is inadequately tested public behaviour. For (3) these are the messages that tell a caller their configuration is unsafe, in a library where failing closed is the whole point — an error that stops being thrown, or starts saying something else, is a silent change to the contract.Suggested scope
🤖 Generated with Claude Code