Dstack-TEE / Dstack-TEE/dstack

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

Đang mở
#1,013 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
rust
Ngôn ngữ chính
Rust
Star
544
Fork
96
Merge trung bình
23 giờ 40 phút
Pull request đã merge (30 ngày)
126

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
rust
Lĩnh vực
backend, cli, security
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
52/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.