fix(dashmate): handle nullable ZeroSSL expiry dates - #4415
Conversation
ZeroSSL leaves expires null while certificates are draft or pending validation. Preserve that state and avoid treating an unissued certificate as expired.\n\nTest would have caught this in CI: ✖ before fix, ✔ after.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
✅ Final review complete — no blockers (commit c2c015c) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The nullable parsing change fixes the reported null.split failure in the interactive obtain flow, but it introduces an unhandled null-expiry path in the background renewal scheduler. A helper restart while a persisted ZeroSSL certificate is still draft or pending now terminates the helper instead of retrying or completing validation, so changes are required before merge.
Source: reviewer backend model: gpt-5.6-sol; final verifier backend model: gpt-5.6-sol. Orchestration only, not reviewer evidence: openclaw-agent/cliproxy/gpt-5.6-sol.
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/dashmate/src/ssl/zerossl/Certificate.js`:
- [BLOCKING] packages/dashmate/src/ssl/zerossl/Certificate.js:109-110: Null expiry now crashes the background renewal scheduler
Returning `false` for a missing expiry routes the background scheduler into its non-expired branch. `scheduleRenewZeroSslCertificateFactory.js` then constructs `new Date(certificate.expires)`, subtracts three days, and passes the resulting December 1969 date to `cron` 2.1.0; `job.start()` throws `WARNING: Date in past. Will never be fired.` outside the scheduler's retry `try` block. This is reachable because the certificate-creation task persists `ssl.enabled`, the ZeroSSL provider, and the certificate ID immediately after creating the draft, before verification and issuance complete. If the helper starts or restarts while that certificate remains draft or pending, the exception escapes the awaited scheduler startup and terminates the helper. Previously, parsing the null expiry threw inside `getCertificate` and entered the one-hour retry path. Add explicit null-expiry handling in the scheduler that resumes obtain/validation or schedules a retry, and cover this caller with a pending-certificate regression test.
Handle certificates without an expiry date by scheduling the obtain flow immediately instead of passing a past date to cron. Test would have caught this in CI:\n✖ before fix: pending certificate throws 'Date in past'\n✔ after fix: scheduler suite passes (3 tests)
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The current head correctly preserves ZeroSSL's nullable expiry and explicitly routes draft or pending certificates into the obtain flow without constructing a past cron date. The added scheduler regression test verifies that a pending certificate reaches the obtain task, so the previously reported blocker is resolved.
Source: reviewer backend model: gpt-5.6-sol; final verifier backend model: gpt-5.6-sol. Orchestration only, not reviewer evidence: openclaw-agent/cliproxy/gpt-5.6-sol.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
Issue being fixed or feature implemented
dashmate ssl obtainaborts withCannot read properties of null (reading 'split')when ZeroSSL returns a draft or pending certificate. ZeroSSL documentsexpiresasnulluntil the certificate is issued, but Dashmate previously parsed it as an unconditional date string.What was done?
How Has This Been Tested?
null.spliterror before the fix and passes after it.Certificate.js.packages/wasm-dpp/dist/index.jsartifact.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only