swarm: keep the webhook secret on read errors; surface hidden KV/auth/forge failures
- swarm-controller webhook: regenerate the secret only when the file is absent. Any other read error is returned with the path; the caller already logs "webhook secret unavailable" and the endpoint answers 503. Overwriting meant every registered hook kept the old secret and every delivery failed HMAC, with nothing re-registering them. - swarm-queue-client wanted: open_read_only/watch return Result; only STREAM_NOT_FOUND on the bucket maps to None. hive-c0re's wanted pull fails the DAG node with the cause instead of logging "nothing has been declared"; the watch warns with the error chain. - swarm-nats-auth: warn when the OIDC secret re-read task dies. - swarm-controller forge: listing a repo's config PRs failing is warn, not debug. Closes #5056
This commit is contained in:
parent
d11c9537b6
commit
7129ee0015
6 changed files with 147 additions and 30 deletions
|
|
@ -755,7 +755,7 @@ impl Client {
|
|||
{
|
||||
Ok(prs) => prs,
|
||||
Err(e) => {
|
||||
tracing::debug!(%agent, error = %e, "swarm forge: listing config PRs failed, skipping repo");
|
||||
tracing::warn!(%agent, error = %e, "swarm forge: listing config PRs failed, skipping repo");
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -85,8 +85,11 @@ fn state_dir_from(raw: Option<&str>) -> std::path::PathBuf {
|
|||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns an error when the secret cannot be generated (no `/dev/urandom`)
|
||||
/// or cannot be persisted. A malformed existing file is **not** an error: it
|
||||
/// Returns an error, leaving the file untouched, when an existing file cannot
|
||||
/// be read: hooks already registered with the stored secret are never
|
||||
/// re-registered, so replacing it would fail every delivery's HMAC check.
|
||||
/// Also errors when the secret cannot be generated (no `/dev/urandom`) or
|
||||
/// cannot be persisted. A malformed existing file is **not** an error: it
|
||||
/// is regenerated, because a secret that cannot be parsed is
|
||||
/// indistinguishable from one that was never written, and failing startup
|
||||
/// over it would strand the daemon with no path forward.
|
||||
|
|
@ -98,15 +101,21 @@ pub(super) fn load_or_generate_secret() -> Result<String> {
|
|||
/// same reason as [`secret_path_from`]: a test can point it at a scratch
|
||||
/// file without mutating a process-global env var.
|
||||
fn load_or_generate_at(path: &std::path::Path) -> Result<String> {
|
||||
if let Ok(raw) = std::fs::read_to_string(path) {
|
||||
let trimmed = raw.trim().to_owned();
|
||||
if trimmed.len() == 64 && trimmed.chars().all(|c| c.is_ascii_hexdigit()) {
|
||||
return Ok(trimmed);
|
||||
match std::fs::read_to_string(path) {
|
||||
Ok(raw) => {
|
||||
let trimmed = raw.trim().to_owned();
|
||||
if trimmed.len() == 64 && trimmed.chars().all(|c| c.is_ascii_hexdigit()) {
|
||||
return Ok(trimmed);
|
||||
}
|
||||
tracing::warn!(
|
||||
path = %path.display(),
|
||||
"webhook-secret file malformed (wrong length/chars); regenerating"
|
||||
);
|
||||
}
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(e) => {
|
||||
return Err(e).with_context(|| format!("read webhook secret {}", path.display()));
|
||||
}
|
||||
tracing::warn!(
|
||||
path = %path.display(),
|
||||
"webhook-secret file malformed (wrong length/chars); regenerating"
|
||||
);
|
||||
}
|
||||
let secret = generate_hex_secret()?;
|
||||
std::fs::create_dir_all(path.parent().unwrap_or(path))
|
||||
|
|
@ -591,6 +600,30 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
/// A path that exists but cannot be read is an error, and nothing is
|
||||
/// written over it. A directory stands in for the unreadable file because
|
||||
/// its read fails the same way for root and non-root test runners.
|
||||
#[test]
|
||||
fn an_unreadable_secret_path_is_an_error_and_left_untouched() {
|
||||
let dir =
|
||||
std::env::temp_dir().join(format!("swarm-hook-unreadable-{}", std::process::id()));
|
||||
let path = dir.join("webhook-secret");
|
||||
std::fs::create_dir_all(&path).expect("mkdir at the secret path");
|
||||
|
||||
let result = load_or_generate_at(&path);
|
||||
let still_dir = path.is_dir();
|
||||
let entries = std::fs::read_dir(&path).map(Iterator::count).ok();
|
||||
std::fs::remove_dir_all(&dir).ok();
|
||||
|
||||
let err = result.expect_err("an unreadable secret must not be replaced");
|
||||
assert!(
|
||||
format!("{err:#}").contains("webhook-secret"),
|
||||
"the error names the path: {err:#}"
|
||||
);
|
||||
assert!(still_dir, "the existing path was replaced");
|
||||
assert_eq!(entries, Some(0), "something was written under the path");
|
||||
}
|
||||
|
||||
/// `StateDirectory=` may name several directories, in which case systemd
|
||||
/// exports them colon-separated — take the first, never the raw value.
|
||||
///
|
||||
|
|
|
|||
Loading…
Reference in a new issue