Skip to content

Conversation

@edgarsj
Copy link
Owner

@edgarsj edgarsj commented Dec 30, 2025

  1. proxyUrl not passed to timestamp revocation - TSA certificate revocation checks now use the proxy
  2. XPath DOM mismatch error - Fixed and silenced expected fallback case

1. proxyUrl not passed to timestamp revocation - TSA certificate revocation checks now use the proxy
2. XPath DOM mismatch error - Fixed and silenced expected fallback case
@greptile-apps
Copy link

greptile-apps bot commented Dec 30, 2025

Greptile Summary

This PR fixes two distinct bugs:

1. ProxyUrl Not Passed to TSA Revocation Checks

  • TSA certificate revocation checks now correctly receive the proxyUrl parameter through revocationOptions
  • The fix propagates proxy settings through the verification chain: verification.tstimestamp/verify.tsrevocation/check.ts
  • Added new revocationOptions field to TimestampVerificationOptions interface

2. XPath DOM Mismatch Error

  • Fixed browser environment XPath queries that incorrectly used the global document instead of the XML document's ownerDocument
  • Added proper null checks to silently fall back to DOM traversal when XPath is unavailable (e.g., XMLDocuments from DOMParser)
  • The fix prevents "Node cannot be used in a document other than the one in which it was created" errors

Both fixes are well-contained, properly propagate parameters through the call chain, and include appropriate fallback mechanisms.

Confidence Score: 5/5

  • This PR is safe to merge with no concerns - both fixes are targeted, well-implemented, and maintain backward compatibility
  • Both bugfixes are straightforward, address specific issues mentioned in the PR description, follow the existing code patterns, and include appropriate fallback mechanisms. The proxy fix correctly threads optional parameters through the call chain without breaking existing functionality. The XPath fix adds defensive checks and graceful degradation for edge cases.
  • No files require special attention

Important Files Changed

Filename Overview
src/core/timestamp/types.ts Added RevocationCheckOptions import and new revocationOptions field to TimestampVerificationOptions interface to support proxy configuration
src/core/timestamp/verify.ts Fixed TSA certificate revocation checks to pass revocationOptions to checkCertificateRevocation, enabling proxy support for OCSP/CRL requests
src/core/verification.ts Added revocationOptions parameter to timestamp verification call to propagate proxy settings through the verification chain
src/utils/xmlParser.ts Fixed XPath evaluation to use ownerDocument instead of global document, preventing DOM mismatch errors when parsing XML with DOMParser

Sequence Diagram

sequenceDiagram
    participant User
    participant Verification
    participant TimestampVerify
    participant RevocationCheck
    participant OCSP/CRL
    participant XMLParser
    
    User->>Verification: verifySignature(signatureInfo, files, options)
    Note over Verification: options contains revocationOptions with proxyUrl
    
    Verification->>TimestampVerify: verifyTimestamp(timestamp, {revocationOptions})
    Note over TimestampVerify: Pass revocationOptions to TSA cert check
    
    TimestampVerify->>RevocationCheck: checkCertificateRevocation(tsaCert, revocationOptions)
    Note over RevocationCheck: Use proxyUrl for OCSP/CRL requests
    
    RevocationCheck->>OCSP/CRL: Fetch via proxy (if proxyUrl set)
    OCSP/CRL-->>RevocationCheck: Revocation status
    RevocationCheck-->>TimestampVerify: tsaRevocation result
    
    TimestampVerify-->>Verification: timestampResult
    
    Verification->>XMLParser: Parse and verify XML signature
    Note over XMLParser: Use ownerDocument instead of global document
    XMLParser-->>Verification: Signature verification result
    
    Verification-->>User: Complete verification result
Loading

@edgarsj edgarsj merged commit cb6f991 into main Dec 30, 2025
4 checks passed
@edgarsj edgarsj deleted the proxy-support-bugfixes branch December 30, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants