fix(namecheap): unwrap CDATA in DCV response, drop dead CSR-hash compute#37
Merged
Conversation
The reissue response embeds DCV record values inside <![CDATA[...]]> blocks. quick-xml fires Event::CData for these (not Event::Text), so the existing first_text() helper silently returned None and NamecheapCa::submit() errored with "missing DCV record fields" even though the response carried the right values all along. The actual response shape, captured via the debug! dump landed in PR #36: <DNSDCValidation ValueAvailable="true"> <DNS domain="oneiric.dev"> <HostName><![CDATA[_<MD5>.oneiric.dev]]></HostName> <Target><![CDATA[<SHA256_FIRST32>.<SHA256_LAST32>.<UNIQUE>.comodoca.com]]></Target> </DNS> </DNSDCValidation> Two changes: * xml::ApiResponse::first_text now handles Event::CData alongside Event::Text. Single test pinning the real (CDATA-wrapped) DCV shape against the parser. * Revert the CNAMECSRHASH local-compute path from PR #36. It was predicated on the canonical Sectigo spec (which says target = <SHA256_FIRST32>.<SHA256_LAST32>.comodoca.com), but the deployed Namecheap response includes a unique-per-order label between the SHA256 split and comodoca.com (e.g. "69ff68dc5168c"). Local compute can't produce that without an API round-trip, so the function would have generated non-resolving CNAMEs if any caller ever reached it. The branch never fired in practice anyway because ssl.reissue responses use HostName/Target, not ApproverEmail. Drop compute_csrhash_dcv + its 4 tests + md-5/sha2/hex deps; the CDATA unwrap alone gets the renewal pipeline past this gate. Keep the debug! dump on the missing-DCV-fields path: it's how this bug was diagnosed and any future deviation will surface the same way.
albedosehen
added a commit
that referenced
this pull request
May 9, 2026
rota's `get_info` was looking for `<CertificateReturned>` element
text and `<CACertificate>` element text. Neither matches Namecheap's
actual `ssl.getInfo&Returncertificate=true` response, which carries
`CertificateReturned` as an ATTRIBUTE on `<Certificates>` and packs
PEMs in nested `<Certificate>` elements:
<Certificates CertificateReturned="true" ReturnType="INDIVIDUAL">
<Certificate><![CDATA[LEAF_PEM]]></Certificate>
<CaCertificates>
<Certificate Type="INTERMEDIATE">
<Certificate><![CDATA[INTERMEDIATE_1_PEM]]></Certificate>
</Certificate>
...
</CaCertificates>
</Certificates>
Result: cert_pem and chain_pem both empty, `is_issued()` false,
polling never terminates even when status==active. So PR #40's
chain-follow lands on the right SSL ID but `await_issuance` still
hangs at the extraction step. Found by extracting the cert manually
out of band when `getInfo` returned status=active for oneiric.dev's
in-flight order: rota's parser yielded empty strings even though
the PEMs were sitting right there in the response.
Fix: new `ApiResponse::pem_blocks(label)` method scans the raw
response for `-----BEGIN <label>-----`...`-----END <label>-----`
armor and returns each block in document order. `get_info` calls
`pem_blocks("CERTIFICATE")`; first block is the leaf, rest are the
chain (concatenated with newlines). The CSR present in the same
response is safely skipped because its label is "CERTIFICATE
REQUEST" and `BEGIN CERTIFICATE-----` doesn't substring-match
`BEGIN CERTIFICATE REQUEST-----`.
This is the 6th and (hopefully) final layer in the rota+Namecheap
end-to-end renewal pipeline, after PRs #36 (reverted), #37 (CDATA
unwrap), #38 (DnsCname variant), #39 (lowercase HostName), #40
(ReplacedBy chain + Status XML path). Tests: 3 new in xml::tests
covering the leaf+chain extraction, the CSR-skip rule, and the
empty-input edge case. Total daemon test count 111 (was 108).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
rota's namecheap reissue calls were erroring with
missing DCV record fieldseven though the API response actually carries the values. The DCV record is wrapped in<![CDATA[...]]>blocks, and quick-xml firesEvent::CDatafor these (notEvent::Text), sofirst_textreturned None and the parser fell through to the error path.What the actual response looks like
Captured by the debug-level dump landed in PR #36:
Note the
69ff68dc5168clabel between the SHA256 split andcomodoca.com— that's a unique-per-order Namecheap-generated value, NOT in any published Sectigo spec.Two changes
xml::ApiResponse::first_texthandlesEvent::CDataalongsideEvent::Text. One regression test pins the real CDATA-wrapped shape against the parser.Revert the CNAMECSRHASH local-compute path from PR feat(namecheap): support Sectigo CNAMECSRHASH DCV method #36. It was predicated on the published Sectigo spec, but the deployed response includes the unique-per-order label that local compute can't produce without an API round-trip. Any caller that actually reached
compute_csrhash_dcvwould have generated non-resolving CNAMEs. The branch never fired in practice (reissue responses useHostName/Target, notApproverEmail) so this is a clean revert: drop the function, drop the 4 tests, drop the md-5/sha2/hex deps.Kept from PR #36
The
debug!dump on the "missing DCV record fields" path stays — it's literally how I diagnosed this bug, and any future Namecheap response-shape deviation will surface the same way.Verified locally
cargo fmt --all --checkcleancargo clippy --workspace --all-targets -- -D warningscleancargo test --workspace --locked152 tests pass (rota-core 40, rota-daemon 107, rota-cli 5)Honest accounting
PR #36 claimed to close the CNAMECSRHASH gap; in reality the actual gap was the CDATA unwrap. The compute_csrhash_dcv function shipped in #36 was never functionally needed and would have produced wrong records if ever invoked. Diff in this PR is mostly removal of that dead code (-183, +48).