Dstack-TEE / Dstack-TEE/dstack

certbot: shutdown cancels an in-flight ACME order and skips DNS-01 TXT cleanup

Open
#1,013 0 comments 0 reactions 0 assignees View on GitHub
rust
Dominant language
Rust
Stars
544
Forks
96
Avg merge
17h 57m
Merged PRs (30d)
117

Description

Follow-up to #924.

The shutdown path added in #924 cancels the daemon future rather than letting it unwind:

```rust
// dstack/certbot/cli/src/main.rs
tokio::select! {
_ = bot.run() => unreachable!("certbot daemon returned"),
result = shutdown_signal() => result?,
}
```

If the signal lands while `renew_inner` is mid-ACME-order, `bot.run()` is dropped at its current await point and the cleanup at the end of the DNS-01 flow never runs:

```rust
// dstack/certbot/src/acme_client.rs:185
if let Err(err) = self.dns01_client.remove_record(&challenge.id).await {
error!("failed to remove dns record {}: {err}", challenge.id);
}
```

Result: a stale `_acme-challenge` TXT record left in the DNS zone, plus a pending authorization at the CA.

This is **not a regression** — before #924 the default SIGTERM disposition killed the process at the same point with the same effect — and it is self-healing, because `set_txt_records` calls `remove_txt_records(&acme_domain)` before publishing new ones on the next attempt. But "stop the daemon cleanly" currently means "stop promptly", not "stop without leaving state behind", and the gap is worth closing.

## Proposal

Give the loop a cancellation token instead of dropping the future:

- check the token at the top of each iteration and in the interval wait (`select!` between `sleep(renew_interval)` and cancellation) — this covers the idle case, which is the overwhelmingly common one and is already instant today;
- for the in-flight case, either let the current renewal run to completion under a bounded grace period before exiting, or make the DNS-01 challenge cleanup drop-safe (scope guard) so cancellation at any await point still removes the TXT record.

The grace period must stay bounded — `renew_timeout` already caps a single renewal, so reusing it as the shutdown deadline is a reasonable ceiling.

Contributor guide

Open the contributing guide

Research direction

Read dstack/certbot/cli/src/main.rs and dstack/certbot/src/acme_client.rs, especially the shutdown select, renew_inner flow, and DNS-01 cleanup near line 185. Trace the existing renew_timeout behavior and run the certbot tests or shutdown-related checks. Done means shutdown handles both idle and in-flight renewals within a bounded deadline without leaving the TXT record or authorization state behind.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, cli, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.