Dstack-TEE / Dstack-TEE/dstack

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

Ouverte
#1,013 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
rust
Langage dominant
Rust
Étoiles
544
Forks
96
Merge moyen
23 h 40 min
PR mergées (30 j)
126

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.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

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.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
rust
Domaine
backend, cli, security
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
52/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.