fix(node): leave foreign ciphertext alone in webhook re-encrypt, count CAS outcome

- has_seal_shape splits "prefixed plaintext or corrupt blob" from
  "well-formed ciphertext": a sealed value that fails open() under the
  node key is left unchanged so delivery keeps withholding, instead of
  re-wrapping the outer blob and signing with it as a fake plaintext
- rewritten counts rows_affected, not iterations; a lost CAS race is
  logged and not counted
- derive_webhook_secret_key rustdoc now agrees with
  WEBHOOK_SECRET_KEY_INFO: a version bump orphans sealed secrets, it is
  not a rotation
- the signing_decision test key comes from OsRng.gen() like the other
  webhook test keys, clearing the hard-coded-key CodeQL alert
- pins: foreign ciphertext survives the pass untouched and reveal still
  returns None for it

Cargo.lock

@@ -3542,6 +3542,7 @@ dependencies = [
  "axum",
  "base64 0.22.1",
  "bytes",
+ "chacha20poly1305",
  "chrono",
  "cid",
  "clap",

crates/gitlawb-node/Cargo.toml

@@ -40,6 +40,7 @@ cid = { workspace = true }
 hex = { workspace = true }
 sha2 = { workspace = true }
 hmac = { workspace = true }
+chacha20poly1305 = "0.10"
 http-body-util = "0.1"
 tokio-util = { version = "0.7", features = ["io"] }
 dirs-next = "2"

crates/gitlawb-node/src/api/pulls.rs

@@ -95,6 +95,7 @@ pub async fn create_pr(
     webhooks::fire_event(
         std::sync::Arc::clone(&state.db),
         std::sync::Arc::clone(&state.http_client),
+        std::sync::Arc::clone(&state.webhook_secret_key),
         &record.id,
         "pull_request.opened",
         serde_json::json!({
@@ -246,6 +247,7 @@ pub async fn merge_pr(
     webhooks::fire_event(
         std::sync::Arc::clone(&state.db),
         std::sync::Arc::clone(&state.http_client),
+        std::sync::Arc::clone(&state.webhook_secret_key),
         &record.id,
         "pull_request.merged",
         serde_json::json!({
@@ -294,6 +296,7 @@ pub async fn close_pr(
     webhooks::fire_event(
         std::sync::Arc::clone(&state.db),
         std::sync::Arc::clone(&state.http_client),
+        std::sync::Arc::clone(&state.webhook_secret_key),
         &record.id,
         "pull_request.closed",
         serde_json::json!({
@@ -344,6 +347,7 @@ pub async fn create_review(
     webhooks::fire_event(
         std::sync::Arc::clone(&state.db),
         std::sync::Arc::clone(&state.http_client),
+        std::sync::Arc::clone(&state.webhook_secret_key),
         &record.id,
         "pull_request.reviewed",
         serde_json::json!({

crates/gitlawb-node/src/api/webhooks.rs

@@ -56,7 +56,14 @@ pub async fn create_webhook(
         id: Uuid::new_v4().to_string(),
         repo_id: record.id,
         url: req.url,
-        secret: req.secret,
+        // Seal the secret before it touches the database (#506): the column and
+        // any dump/backup of it hold ciphertext, never the caller's signing key.
+        secret: req.secret.map(|plain| {
+            crate::webhook_secret::WebhookSecret::new(crate::webhook_secret::seal(
+                state.webhook_secret_key.as_ref(),
+                &plain,
+            ))
+        }),
         events,
         created_by_did,
         created_at: Utc::now().to_rfc3339(),
@@ -93,19 +100,11 @@ pub async fn list_webhooks(
     crate::api::require_repo_owner(&record, &caller)?;
 
     let hooks = state.db.list_webhooks(&record.id).await?;
-    // Redact secrets in list response
-    let redacted: Vec<_> = hooks
-        .into_iter()
-        .map(|mut h| {
-            if h.secret.is_some() {
-                h.secret = Some("***".into());
-            }
-            h
-        })
-        .collect();
-
+    // `WebhookSecret` redacts itself on serialize, so a set secret is emitted as
+    // "***" and the signing material never appears in the body (see webhook_secret).
+    let count = hooks.len();
     Ok(Json(
-        serde_json::json!({ "webhooks": redacted, "count": redacted.len() }),
+        serde_json::json!({ "webhooks": hooks, "count": count }),
     ))
 }
 
@@ -149,6 +148,7 @@ mod tests {
     use crate::db::RepoRecord;
     use crate::error::AppError;
     use crate::test_support::test_state;
+    use rand::Rng;
 
     // create_webhook gates req.url through is_public_http_url. Pin the exact
     // SSRF targets from issue #81 so the webhook path can never regress to the
@@ -226,4 +226,137 @@ mod tests {
             "a rejected .localhost webhook must leave no row behind"
         );
     }
+
+    /// The startup re-encryption pass must seal any webhook secret still resting
+    /// as plaintext (a row written before at-rest encryption shipped), and leave
+    /// a second pass with nothing to do.
+    #[sqlx::test]
+    async fn reencrypt_legacy_secrets_encrypts_existing_cleartext(pool: PgPool) {
+        let state = test_state(pool).await;
+
+        // Seed a webhook the way a pre-encryption node would have: plaintext
+        // secret written straight into the row, bypassing the handler's seal.
+        let hook = crate::db::Webhook {
+            id: Uuid::new_v4().to_string(),
+            repo_id: "owner/legacy-repo".to_string(),
+            url: "https://hooks.example.com/legacy".to_string(),
+            secret: Some(crate::webhook_secret::WebhookSecret::new(
+                "legacy-plaintext".to_string(),
+            )),
+            events: vec!["*".to_string()],
+            created_by_did: "did:key:zLEGACYOWNER".to_string(),
+            created_at: Utc::now().to_rfc3339(),
+            active: true,
+        };
+        state.db.create_webhook(&hook).await.unwrap();
+
+        let rewritten = state
+            .db
+            .reencrypt_legacy_webhook_secrets(state.webhook_secret_key.as_ref())
+            .await
+            .unwrap();
+        assert_eq!(rewritten, 1, "exactly the one plaintext row is rewritten");
+
+        let stored = state.db.get_webhook(&hook.id).await.unwrap().unwrap();
+        let secret = stored.secret.expect("secret retained after re-encryption");
+        assert!(secret.is_sealed(), "re-encrypted secret must be sealed");
+        assert_eq!(
+            crate::webhook_secret::open(state.webhook_secret_key.as_ref(), secret.as_str())
+                .as_deref(),
+            Some("legacy-plaintext"),
+            "the node-held key must recover the original secret"
+        );
+
+        // Idempotent: a second pass finds nothing left to rewrite.
+        let again = state
+            .db
+            .reencrypt_legacy_webhook_secrets(state.webhook_secret_key.as_ref())
+            .await
+            .unwrap();
+        assert_eq!(again, 0);
+    }
+
+    /// A legacy plaintext secret that literally begins with the seal prefix is
+    /// not ciphertext: prefix-matching alone would skip it forever while
+    /// `reveal` fails to open it, wedging the hook. Classification is by
+    /// `open`, so the pass seals it and the literal secret survives.
+    #[sqlx::test]
+    async fn reencrypt_seals_a_prefixed_plaintext_secret(pool: PgPool) {
+        let state = test_state(pool).await;
+        let literal = format!(
+            "{}not-a-real-ciphertext",
+            crate::webhook_secret::SEAL_PREFIX
+        );
+        let hook = crate::db::Webhook {
+            id: Uuid::new_v4().to_string(),
+            repo_id: "owner/prefixed-repo".to_string(),
+            url: "https://hooks.example.com/prefixed".to_string(),
+            secret: Some(crate::webhook_secret::WebhookSecret::new(literal.clone())),
+            events: vec!["*".to_string()],
+            created_by_did: "did:key:zLEGACYOWNER".to_string(),
+            created_at: Utc::now().to_rfc3339(),
+            active: true,
+        };
+        state.db.create_webhook(&hook).await.unwrap();
+
+        let rewritten = state
+            .db
+            .reencrypt_legacy_webhook_secrets(state.webhook_secret_key.as_ref())
+            .await
+            .unwrap();
+        assert_eq!(rewritten, 1, "the prefixed plaintext row is rewritten");
+
+        let stored = state.db.get_webhook(&hook.id).await.unwrap().unwrap();
+        let secret = stored.secret.unwrap();
+        assert!(secret.is_sealed());
+        assert_eq!(
+            crate::webhook_secret::reveal(state.webhook_secret_key.as_ref(), &secret).as_deref(),
+            Some(literal.as_str()),
+            "the literal plaintext, prefix included, is the recovered secret"
+        );
+    }
+
+    /// A well-formed sealed payload that does not open under this node's key
+    /// (foreign or rotated ciphertext) must survive the re-encrypt pass
+    /// untouched: re-wrapping it would make `reveal` return the outer blob as
+    /// a signing secret and flip a broken hook from withheld into garbage
+    /// signatures.
+    #[sqlx::test]
+    async fn reencrypt_leaves_foreign_ciphertext_untouched(pool: PgPool) {
+        let state = test_state(pool).await;
+        let foreign_key: [u8; 32] = chacha20poly1305::aead::OsRng.gen();
+        let foreign_ciphertext = crate::webhook_secret::seal(&foreign_key, "owner-real-secret");
+        let hook = crate::db::Webhook {
+            id: Uuid::new_v4().to_string(),
+            repo_id: "owner/foreign-repo".to_string(),
+            url: "https://hooks.example.com/foreign".to_string(),
+            secret: Some(crate::webhook_secret::WebhookSecret::new(
+                foreign_ciphertext.clone(),
+            )),
+            events: vec!["*".to_string()],
+            created_by_did: "did:key:zLEGACYOWNER".to_string(),
+            created_at: Utc::now().to_rfc3339(),
+            active: true,
+        };
+        state.db.create_webhook(&hook).await.unwrap();
+
+        let rewritten = state
+            .db
+            .reencrypt_legacy_webhook_secrets(state.webhook_secret_key.as_ref())
+            .await
+            .unwrap();
+        assert_eq!(rewritten, 0, "foreign ciphertext is left unchanged");
+
+        let stored = state.db.get_webhook(&hook.id).await.unwrap().unwrap();
+        let secret = stored.secret.unwrap();
+        assert_eq!(
+            secret.as_str(),
+            foreign_ciphertext,
+            "the stored value is still the unopenable ciphertext, not a re-wrap"
+        );
+        assert!(
+            crate::webhook_secret::reveal(state.webhook_secret_key.as_ref(), &secret).is_none(),
+            "delivery still withholds: reveal cannot open the foreign blob"
+        );
+    }
 }

crates/gitlawb-node/src/auth/mod.rs

@@ -628,6 +628,7 @@ mod tests {
 
         let keypair = Keypair::generate();
         let scan_token_key = crate::state::AppState::derive_scan_token_key(&keypair);
+        let webhook_secret_key = crate::state::AppState::derive_webhook_secret_key(&keypair);
         let (ref_tx, _) = tokio::sync::broadcast::channel(1);
         let (task_tx, _) = tokio::sync::broadcast::channel(1);
         let pool = sqlx::postgres::PgPoolOptions::new()
@@ -667,6 +668,7 @@ mod tests {
             ipfs_max_legacy_scan_rule_bytes:
                 crate::api::ipfs::MAX_LEGACY_SCAN_RULE_BYTES_PER_REQUEST,
             ipfs_scan_token_key: Arc::new(scan_token_key),
+            webhook_secret_key: Arc::new(webhook_secret_key),
             ipfs_max_served_object_bytes: crate::api::ipfs::MAX_SERVED_OBJECT_BYTES,
             push_limiter_trust: crate::rate_limit::TrustedProxy::None,
             sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),

crates/gitlawb-node/src/db/mod.rs

@@ -8,6 +8,8 @@ use sqlx::{postgres::PgPoolOptions, PgPool, Row};
 use tracing::info;
 use uuid::Uuid;
 
+use crate::webhook_secret::WebhookSecret;
+
 /// Maximum visible repositories per page, plus one internal look-ahead row.
 pub(crate) const MAX_VISIBLE_REPO_PAGE_SIZE: usize = 200;
 
@@ -297,7 +299,10 @@ pub struct Webhook {
     pub id: String,
     pub repo_id: String,
     pub url: String,
-    pub secret: Option<String>,
+    /// At rest this holds the sealed ciphertext (`enc:v1:…`) produced by
+    /// [`crate::webhook_secret::seal`]; the newtype redacts `Debug` and
+    /// `Serialize` so the signing secret never reaches a log line or a body.
+    pub secret: Option<WebhookSecret>,
     pub events: Vec<String>,
     pub created_by_did: String,
     pub created_at: String,
@@ -3878,7 +3883,7 @@ impl Db {
         .bind(&hook.id)
         .bind(&hook.repo_id)
         .bind(&hook.url)
-        .bind(&hook.secret)
+        .bind(hook.secret.as_ref().map(|s| s.as_str()))
         .bind(&events_json)
         .bind(&hook.created_by_did)
         .bind(&hook.created_at)
@@ -3929,6 +3934,67 @@ impl Db {
             .filter(|h| h.active && h.events.iter().any(|e| e == "*" || e == event))
             .collect())
     }
+
+    /// One-time re-encryption of webhook secrets that still rest as plaintext
+    /// (rows written before at-rest encryption shipped). Runs once at startup; a
+    /// schema migration cannot do this because the node-held key never reaches
+    /// Postgres. Returns the number of rows rewritten.
+    ///
+    /// "Already sealed" means "opens under the node key", not "has the prefix":
+    /// a plaintext secret that literally begins with the `enc:v1:` prefix, or a
+    /// value sealed under another identity, fails `open` and is re-sealed raw,
+    /// which preserves whatever the stored value was rather than wedging it.
+    /// A prefixed value that does not open is also warned on, since it is more
+    /// likely corrupt ciphertext than a colliding plaintext.
+    pub async fn reencrypt_legacy_webhook_secrets(&self, key: &[u8; 32]) -> Result<usize> {
+        let rows: Vec<(String, String)> =
+            sqlx::query_as("SELECT id, secret FROM webhooks WHERE secret IS NOT NULL")
+                .fetch_all(&self.pool)
+                .await?;
+
+        let mut rewritten = 0usize;
+        for (id, stored) in rows {
+            if crate::webhook_secret::open(key, &stored).is_some() {
+                continue; // already sealed under this node's key
+            }
+            if crate::webhook_secret::has_seal_shape(&stored) {
+                // Well-formed sealed payload that does not open under this
+                // node's key: foreign or rotated ciphertext. Re-wrapping the
+                // outer blob as plaintext would make `reveal` return it as a
+                // signing secret and flip Withhold into a garbage signature,
+                // so leave the row unchanged: delivery keeps withholding.
+                tracing::warn!(
+                    webhook_id = %id,
+                    "webhook secret is well-formed ciphertext but does not open under the node key; leaving it unchanged"
+                );
+                continue;
+            }
+            if stored.starts_with(crate::webhook_secret::SEAL_PREFIX) {
+                tracing::warn!(
+                    webhook_id = %id,
+                    "webhook secret has the sealed prefix but is not well-formed ciphertext; sealing the raw value as plaintext"
+                );
+            }
+            let sealed = crate::webhook_secret::seal(key, &stored);
+            // Compare-and-set on the value read above: a concurrent secret
+            // update between SELECT and UPDATE must not be clobbered.
+            let result =
+                sqlx::query("UPDATE webhooks SET secret = $2 WHERE id = $1 AND secret = $3")
+                    .bind(&id)
+                    .bind(sealed)
+                    .bind(&stored)
+                    .execute(&self.pool)
+                    .await?;
+            if result.rows_affected() == 0 {
+                tracing::warn!(
+                    webhook_id = %id,
+                    "webhook secret changed between read and re-encrypt; skipping"
+                );
+            }
+            rewritten += result.rows_affected() as usize;
+        }
+        Ok(rewritten)
+    }
 }
 
 // ── Ref Certificates ──────────────────────────────────────────────────────────
@@ -9164,7 +9230,7 @@ fn row_to_webhook(r: sqlx::postgres::PgRow) -> Webhook {
         id: r.get("id"),
         repo_id: r.get("repo_id"),
         url: r.get("url"),
-        secret: r.get("secret"),
+        secret: r.get::<Option<String>, _>("secret").map(WebhookSecret::new),
         events,
         created_by_did: r.get("created_by_did"),
         created_at: r.get("created_at"),

crates/gitlawb-node/src/durable_outbox.rs

@@ -1439,6 +1439,7 @@ async fn run_effect_bundle(
                     crate::webhooks::fire_event_occurrence(
                         state.db.clone(),
                         state.http_client.clone(),
+                        state.webhook_secret_key.clone(),
                         &repo.id,
                         "push",
                         payload,
@@ -1470,6 +1471,7 @@ async fn run_effect_bundle(
                 crate::webhooks::fire_event_occurrence_with_claimed_hooks(
                     state.db.clone(),
                     state.http_client.clone(),
+                    state.webhook_secret_key.clone(),
                     &repo.id,
                     "push",
                     payload,

crates/gitlawb-node/src/main.rs

@@ -27,6 +27,7 @@ mod sync;
 mod test_support;
 mod upstream_mirror;
 mod visibility;
+mod webhook_secret;
 mod webhooks;
 
 use anyhow::{anyhow, Context, Result};
@@ -99,6 +100,7 @@ async fn main() -> Result<()> {
     // Sealing key for the legacy-scan continuation tokens, DERIVED from the identity
     // just loaded so it is the same key after a restart (see `derive_scan_token_key`).
     let scan_token_key = AppState::derive_scan_token_key(&keypair);
+    let webhook_secret_key = AppState::derive_webhook_secret_key(&keypair);
     let node_did = keypair.did();
 
     // One-time metrics init. Must run before any handler that calls into
@@ -481,6 +483,7 @@ async fn main() -> Result<()> {
         ipfs_max_legacy_scan_rows: AppState::ipfs_legacy_scan_row_budget(&config),
         ipfs_max_legacy_scan_rule_bytes: crate::api::ipfs::MAX_LEGACY_SCAN_RULE_BYTES_PER_REQUEST,
         ipfs_scan_token_key: Arc::new(scan_token_key),
+        webhook_secret_key: Arc::new(webhook_secret_key),
         ipfs_max_served_object_bytes: crate::api::ipfs::MAX_SERVED_OBJECT_BYTES,
         push_limiter_trust,
         sync_trigger_rate_limiter,
@@ -569,6 +572,23 @@ async fn main() -> Result<()> {
         ),
         git_bin: "git".to_string(),
     };
+
+    // One-time re-encryption of any webhook HMAC secrets still resting as
+    // plaintext (written before at-rest encryption shipped). A SQL migration
+    // cannot do it — Postgres never sees the node-held key — so it runs here.
+    match state
+        .db
+        .reencrypt_legacy_webhook_secrets(state.webhook_secret_key.as_ref())
+        .await
+    {
+        Ok(0) => {}
+        Ok(n) => info!(
+            count = n,
+            "re-encrypted legacy webhook HMAC secrets at rest"
+        ),
+        Err(e) => warn!(err = %e, "failed to re-encrypt legacy webhook secrets"),
+    }
+
     if config.ipfs_rate_limit == 0 {
         tracing::warn!("GITLAWB_IPFS_RATE_LIMIT=0 — per-IP /ipfs rate limiting disabled");
     }

crates/gitlawb-node/src/metrics.rs

@@ -342,9 +342,11 @@ pub fn record_sync_processed(status: &str) {
 }
 
 /// Record one webhook delivery attempt outcome. `result` ∈
-/// {ok, http_error, network_error, skipped, invalid_url}.
+/// {ok, http_error, network_error, skipped, invalid_url, secret_unopenable}.
 /// `invalid_url` is a policy skip: the stored hook URL failed the public-URL
-/// check at delivery time and the POST was never sent.
+/// check at delivery time and the POST was never sent. `secret_unopenable` is
+/// a withheld delivery: the hook carries a secret the node could not open, so
+/// the POST was never sent rather than going out unsigned.
 pub fn record_webhook_delivery(result: &str) {
     if let Some(c) = WEBHOOK_DELIVERIES.get() {
         c.with_label_values(&[result]).inc();

crates/gitlawb-node/src/state.rs

@@ -21,6 +21,19 @@ const SCAN_TOKEN_KEY_SALT: &[u8] = b"gitlawb/hkdf-salt/ipfs-scan-token";
 /// front) and needs no migration, no config, and no change to this function.
 const SCAN_TOKEN_KEY_INFO: &[u8] = b"gitlawb/ipfs-scan-token/v1";
 
+/// HKDF salt for [`AppState::derive_webhook_secret_key`]. A constant, not a
+/// secret: the confidentiality of the derived key rests on the node's private
+/// seed being the input keying material, exactly as for the scan-token key.
+const WEBHOOK_SECRET_KEY_SALT: &[u8] = b"gitlawb/hkdf-salt/webhook-secret";
+
+/// HKDF `info` for [`AppState::derive_webhook_secret_key`]: the domain
+/// separation from the scan-token key that shares the same input keying
+/// material. Unlike the scan-token key, bumping the version is NOT a safe
+/// rotation: sealed secrets persist, `open` has no old-key fallback, and a new
+/// version renders every stored ciphertext permanently unopenable until a
+/// re-seal path exists.
+const WEBHOOK_SECRET_KEY_INFO: &[u8] = b"gitlawb/webhook-secret/v1";
+
 #[derive(Clone, Debug)]
 pub struct RefUpdateBroadcast {
     pub repo: String,
@@ -173,6 +186,11 @@ pub struct AppState {
     /// already has on disk, so derivation is also the smaller mechanism. Rotation is a
     /// bump of the version in [`SCAN_TOKEN_KEY_INFO`].
     pub ipfs_scan_token_key: Arc<[u8; 32]>,
+    /// Key sealing webhook HMAC secrets at rest (#506), derived from the node's
+    /// persistent identity by [`AppState::derive_webhook_secret_key`]. The
+    /// webhook delivery path opens a secret with it only at signing time; the
+    /// `webhooks.secret` column stores the ciphertext (see `webhook_secret`).
+    pub webhook_secret_key: Arc<[u8; 32]>,
     /// Hard ceiling on the byte size of an object `GET /ipfs/{cid}` will buffer and
     /// serve (default `api::ipfs::MAX_SERVED_OBJECT_BYTES`). The serve reads via a
     /// blocking `git cat-file` and buffers the whole object; without a bound a large
@@ -431,20 +449,48 @@ impl AppState {
     ///     identity, the token format, or any caller, so rotation is a constant change
     ///     rather than a fork of this derivation.
     pub(crate) fn derive_scan_token_key(keypair: &Keypair) -> [u8; 32] {
+        Self::hkdf_sha256(
+            SCAN_TOKEN_KEY_SALT,
+            SCAN_TOKEN_KEY_INFO,
+            keypair.to_seed().as_slice(),
+        )
+    }
+
+    /// Derive the at-rest webhook-secret sealing key from the node identity.
+    ///
+    /// Same HKDF construction as [`AppState::derive_scan_token_key`] under its
+    /// own [`WEBHOOK_SECRET_KEY_INFO`]/[`WEBHOOK_SECRET_KEY_SALT`] pair, so it
+    /// is derived (a restart re-derives the same key, and old secrets remain
+    /// readable) yet domain-separated from the signing key and the scan-token
+    /// key. The doc on [`SCAN_TOKEN_KEY_INFO`] argues why derivation (not a
+    /// boot-random key) is the right trade: the secret's confidentiality still
+    /// rests on the node's private seed. Rotation is NOT a version bump here,
+    /// unlike for the scan-token key: [`WEBHOOK_SECRET_KEY_INFO`] explains that
+    /// bumping renders every stored ciphertext unopenable until a re-seal path
+    /// exists.
+    pub(crate) fn derive_webhook_secret_key(keypair: &Keypair) -> [u8; 32] {
+        Self::hkdf_sha256(
+            WEBHOOK_SECRET_KEY_SALT,
+            WEBHOOK_SECRET_KEY_INFO,
+            keypair.to_seed().as_slice(),
+        )
+    }
+
+    /// HKDF-Extract-then-Expand (RFC 5869) over the node seed: PRK =
+    /// HMAC(salt, ikm), then one expand block T(1) = HMAC(PRK, info || 0x01).
+    /// One 32-byte output needs exactly one block of SHA-256, so there is no
+    /// counter loop to get wrong. Salt and info are the caller's domain
+    /// separation; shared by every identity-derived key in this module.
+    fn hkdf_sha256(salt: &[u8], info: &[u8], ikm: &[u8]) -> [u8; 32] {
         use hmac::{Hmac, Mac};
         type HmacSha256 = Hmac<sha2::Sha256>;
 
-        let seed = keypair.to_seed();
-        // HKDF-Extract: PRK = HMAC(salt, ikm).
-        let mut extract =
-            HmacSha256::new_from_slice(SCAN_TOKEN_KEY_SALT).expect("HMAC takes a key of any size");
-        extract.update(seed.as_slice());
+        let mut extract = HmacSha256::new_from_slice(salt).expect("HMAC takes a key of any size");
+        extract.update(ikm);
         let prk = extract.finalize().into_bytes();
-        // HKDF-Expand, one block: T(1) = HMAC(PRK, info || 0x01). One 32-byte output
-        // needs exactly one block of SHA-256, so there is no counter loop to get wrong.
         let mut expand =
             HmacSha256::new_from_slice(prk.as_slice()).expect("HMAC takes a key of any size");
-        expand.update(SCAN_TOKEN_KEY_INFO);
+        expand.update(info);
         expand.update(&[0x01]);
         expand.finalize().into_bytes().into()
     }

crates/gitlawb-node/src/test_support.rs

@@ -83,6 +83,7 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
 
     let keypair = Keypair::generate();
     let scan_token_key = crate::state::AppState::derive_scan_token_key(&keypair);
+    let webhook_secret_key = crate::state::AppState::derive_webhook_secret_key(&keypair);
     let node_did = keypair.did();
     let (ref_tx, _) = tokio::sync::broadcast::channel(1);
     let (task_tx, _) = tokio::sync::broadcast::channel(1);
@@ -118,6 +119,7 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
         ipfs_max_legacy_scan_rows: crate::api::ipfs::MAX_LEGACY_SCAN_ROWS_PER_REQUEST,
         ipfs_max_legacy_scan_rule_bytes: crate::api::ipfs::MAX_LEGACY_SCAN_RULE_BYTES_PER_REQUEST,
         ipfs_scan_token_key: Arc::new(scan_token_key),
+        webhook_secret_key: Arc::new(webhook_secret_key),
         ipfs_max_served_object_bytes: crate::api::ipfs::MAX_SERVED_OBJECT_BYTES,
         push_limiter_trust: crate::rate_limit::TrustedProxy::None,
         sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),
@@ -4205,7 +4207,9 @@ mod tests {
                     id: uuid::Uuid::new_v4().to_string(),
                     repo_id: repo.id.clone(),
                     url: secret_url.to_string(),
-                    secret: Some("topsecret".to_string()),
+                    secret: Some(crate::webhook_secret::WebhookSecret::new(
+                        "topsecret".to_string(),
+                    )),
                     events: vec!["*".to_string()],
                     created_by_did: owner.to_string(),
                     created_at: Utc::now().to_rfc3339(),

crates/gitlawb-node/src/webhook_secret.rs

@@ -0,0 +1,221 @@
+//! At-rest protection for webhook HMAC secrets (#506).
+//!
+//! Webhook secrets are signing keys: whoever holds one can forge delivery
+//! payloads to the receiver the hook points at. They are persisted in the
+//! `webhooks.secret` column and used to compute the `X-Gitlawb-Signature-256`
+//! header on each delivery. That secret needs two properties:
+//!
+//!   * It must never reach a log line or an API response. [`WebhookSecret`]
+//!     redacts both `Debug` and `Serialize`, so no derived `{:?}` and no JSON
+//!     body ever renders the value, whatever it contains.
+//!   * It must not rest as plaintext where a database dump or backup can read
+//!     it. The node seals the secret with a key derived from its persistent
+//!     identity (see `AppState::derive_webhook_secret_key`), so a dump alone
+//!     reveals nothing.
+//!
+//! Storage format: `enc:v1:<base64url(nonce || ciphertext)>` — a 24-byte
+//! XChaCha20-Poly1305 nonce followed by the ciphertext (which carries the tag).
+//! Anything without the `enc:v1:` prefix is a legacy plaintext value, written
+//! before this protection shipped; the delivery path signs with those unchanged
+//! and they are re-encrypted once at startup by
+//! `Db::reencrypt_legacy_webhook_secrets`.
+
+use std::fmt;
+
+use base64::{engine::general_purpose::URL_SAFE_NO_PAD as B64URL, Engine};
+use chacha20poly1305::{
+    aead::{Aead, AeadCore, KeyInit, OsRng},
+    XChaCha20Poly1305, XNonce,
+};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
+
+/// Prefix marking a sealed-at-rest secret. Legacy plaintext values lack it.
+pub const SEAL_PREFIX: &str = "enc:v1:";
+
+const NONCE_LEN: usize = 24;
+
+/// An HMAC signing secret that never renders its contents in debug or JSON
+/// output. Used as `Webhook.secret`, so that field is safe to `Debug`-print and
+/// `Serialize` by construction rather than by discipline at each call site.
+#[derive(Clone, PartialEq, Eq)]
+pub struct WebhookSecret(String);
+
+impl WebhookSecret {
+    pub fn new(inner: String) -> Self {
+        Self(inner)
+    }
+
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+
+    /// Whether the value is a sealed ciphertext (as opposed to legacy plaintext).
+    pub fn is_sealed(&self) -> bool {
+        self.0.starts_with(SEAL_PREFIX)
+    }
+}
+
+impl fmt::Debug for WebhookSecret {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("\"***\"")
+    }
+}
+
+impl Serialize for WebhookSecret {
+    /// Emit a fixed marker, never the contents. On the wire a set secret is
+    /// `"secret": "***"` and an unset one is `"secret": null` (`Option`), which
+    /// the CLI already reads as "is a secret set".
+    fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        serializer.serialize_str("***")
+    }
+}
+
+impl<'de> Deserialize<'de> for WebhookSecret {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        Ok(WebhookSecret(String::deserialize(deserializer)?))
+    }
+}
+
+/// Seal a plaintext secret under a node-held key. A fresh `OsRng` nonce on every
+/// seal guarantees repeated seals of the same secret never collide.
+pub fn seal(key: &[u8; 32], plaintext: &str) -> String {
+    let cipher = XChaCha20Poly1305::new_from_slice(key).expect("webhook secret key is 32 bytes");
+    let nonce = XChaCha20Poly1305::generate_nonce(&mut OsRng);
+    let ciphertext = cipher
+        .encrypt(&nonce, plaintext.as_bytes())
+        .expect("XChaCha20-Poly1305 encryption is infallible for in-memory inputs");
+    let mut out = Vec::with_capacity(NONCE_LEN + ciphertext.len());
+    out.extend_from_slice(&nonce);
+    out.extend_from_slice(&ciphertext);
+    format!("{SEAL_PREFIX}{}", B64URL.encode(out))
+}
+
+/// Open a sealed secret. Returns `None` for anything that is not a well-formed
+/// sealed value — legacy plaintext, a foreign key, tampered or truncated bytes —
+/// so a caller that needs the legacy path checks [`WebhookSecret::is_sealed`]
+/// first (see [`reveal`]).
+pub fn open(key: &[u8; 32], stored: &str) -> Option<String> {
+    let raw = B64URL.decode(stored.strip_prefix(SEAL_PREFIX)?).ok()?;
+    if raw.len() <= NONCE_LEN {
+        return None;
+    }
+    let (nonce, ciphertext) = raw.split_at(NONCE_LEN);
+    let cipher = XChaCha20Poly1305::new_from_slice(key).ok()?;
+    let plaintext = cipher.decrypt(XNonce::from_slice(nonce), ciphertext).ok()?;
+    String::from_utf8(plaintext).ok()
+}
+
+/// Whether a stored value has the structural shape of a sealed payload:
+/// [`SEAL_PREFIX`] plus a base64url blob long enough to hold a nonce and
+/// ciphertext. This is a shape check, not an authentication check: a
+/// well-formed blob sealed under a different key satisfies it while [`open`]
+/// still fails. The re-encryption pass uses the distinction to leave
+/// well-formed-but-foreign ciphertext alone instead of re-wrapping it as
+/// plaintext, which would let `reveal` return the outer blob as a signing key.
+pub fn has_seal_shape(stored: &str) -> bool {
+    stored
+        .strip_prefix(SEAL_PREFIX)
+        .and_then(|rest| B64URL.decode(rest).ok())
+        .is_some_and(|raw| raw.len() > NONCE_LEN)
+}
+
+/// Resolve a stored secret to its plaintext for signing. Sealed values are opened
+/// with the node key; legacy plaintext is returned unchanged so a hook created
+/// before this fix keeps signing until the startup re-encryption pass runs.
+pub fn reveal(key: &[u8; 32], stored: &WebhookSecret) -> Option<String> {
+    if stored.is_sealed() {
+        open(key, stored.as_str())
+    } else {
+        Some(stored.as_str().to_string())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use rand::Rng;
+
+    /// Test keys need no secrecy, but they still come from the OS CSPRNG so no
+    /// hard-coded key material sits in the tree. A wrong key fails XChaCha20-Poly1305
+    /// authentication with overwhelming probability, so the "wrong key" case stays
+    /// deterministic in practice.
+    fn new_key() -> [u8; 32] {
+        OsRng.gen()
+    }
+
+    fn other_key() -> [u8; 32] {
+        OsRng.gen()
+    }
+
+    #[test]
+    fn seal_open_round_trip() {
+        let k = new_key();
+        let sealed = seal(&k, "s3cr3t-value");
+        assert!(sealed.starts_with(SEAL_PREFIX));
+        assert_eq!(open(&k, &sealed).as_deref(), Some("s3cr3t-value"));
+    }
+
+    #[test]
+    fn sealed_value_is_never_the_plaintext() {
+        let k = new_key();
+        let sealed = seal(&k, "s3cr3t-value");
+        assert!(!sealed.contains("s3cr3t-value"));
+    }
+
+    #[test]
+    fn wrong_key_does_not_open() {
+        let sealed = seal(&new_key(), "s3cr3t-value");
+        assert_eq!(open(&other_key(), &sealed), None);
+    }
+
+    #[test]
+    fn tampered_or_truncated_value_does_not_open() {
+        let k = new_key();
+        let sealed = seal(&k, "s3cr3t-value");
+        // Flip a byte inside the base64 payload.
+        let mut chars: Vec<char> = sealed.chars().collect();
+        let idx = SEAL_PREFIX.len();
+        chars[idx] = if chars[idx] == 'A' { 'B' } else { 'A' };
+        let tampered: String = chars.into_iter().collect();
+        assert_eq!(open(&k, &tampered), None);
+        // A value that is just the prefix (no nonce/ciphertext) is malformed.
+        assert_eq!(open(&k, SEAL_PREFIX), None);
+    }
+
+    #[test]
+    fn reveal_opens_a_sealed_value() {
+        let k = new_key();
+        let sealed = WebhookSecret::new(seal(&k, "s3cr3t-value"));
+        assert_eq!(reveal(&k, &sealed).as_deref(), Some("s3cr3t-value"));
+    }
+
+    #[test]
+    fn legacy_plaintext_is_revealed_unchanged() {
+        let k = new_key();
+        let legacy = WebhookSecret::new("plaintext-secret".into());
+        assert!(!legacy.is_sealed());
+        assert_eq!(reveal(&k, &legacy).as_deref(), Some("plaintext-secret"));
+    }
+
+    #[test]
+    fn debug_is_redacted() {
+        let s = WebhookSecret::new("s3cr3t-value".into());
+        let rendered = format!("{s:?}");
+        assert!(!rendered.contains("s3cr3t-value"));
+        assert_eq!(rendered, "\"***\"");
+    }
+
+    #[test]
+    fn serialize_is_redacted() {
+        let s = WebhookSecret::new("s3cr3t-value".into());
+        let json = serde_json::to_string(&s).unwrap();
+        assert!(!json.contains("s3cr3t-value"));
+        assert_eq!(json, "\"***\"");
+    }
+
+    #[test]
+    fn deserialize_reads_the_underlying_string() {
+        let s: WebhookSecret = serde_json::from_str("\"s3cr3t-value\"").unwrap();
+        assert_eq!(s.as_str(), "s3cr3t-value");
+    }
+}

crates/gitlawb-node/src/webhooks.rs

@@ -30,15 +30,50 @@ fn sign_payload(secret: &str, payload: &[u8]) -> String {
     format!("sha256={}", hex::encode(mac.finalize().into_bytes()))
 }
 
+/// What the signing step decided for one hook. Keeping this a decision (not a
+/// bare `Option`) lets the delivery path tell "hook has no secret" (send
+/// unsigned) apart from "hook has a secret that could not be opened" (withhold:
+/// an unsigned POST would present a broken hook as working while the ledger
+/// marks the event delivered).
+enum SigningOutcome {
+    NoSecret,
+    Signed(String),
+    Withhold,
+}
+
+fn signing_decision(
+    secret_key: &[u8; 32],
+    secret: Option<&crate::webhook_secret::WebhookSecret>,
+    bytes: &[u8],
+) -> SigningOutcome {
+    match secret {
+        None => SigningOutcome::NoSecret,
+        Some(s) => match crate::webhook_secret::reveal(secret_key, s) {
+            Some(plain) => SigningOutcome::Signed(sign_payload(&plain, bytes)),
+            None => SigningOutcome::Withhold,
+        },
+    }
+}
+
 /// Fire webhooks for `event` on `repo_id`. Spawns background tasks — never blocks.
 pub fn fire_event(
     db: Arc<Db>,
     http_client: Arc<reqwest::Client>,
+    secret_key: Arc<[u8; 32]>,
     repo_id: &str,
     event: &str,
     payload: serde_json::Value,
 ) {
-    fire_event_occurrence(db, http_client, repo_id, event, payload, None, None);
+    fire_event_occurrence(
+        db,
+        http_client,
+        secret_key,
+        repo_id,
+        event,
+        payload,
+        None,
+        None,
+    );
 }
 
 /// Occurrence-keyed variant: at most one delivery per
@@ -50,9 +85,11 @@ pub fn fire_event(
 /// to claim deliveries synchronously, then call
 /// `fire_event_occurrence_with_claimed_hooks` with the claimed hook IDs to
 /// spawn HTTP POST only for claimed hooks.
+#[allow(clippy::too_many_arguments)]
 pub fn fire_event_occurrence(
     db: Arc<Db>,
     http_client: Arc<reqwest::Client>,
+    secret_key: Arc<[u8; 32]>,
     repo_id: &str,
     event: &str,
     payload: serde_json::Value,
@@ -67,6 +104,7 @@ pub fn fire_event_occurrence(
         fire_event_async_occurrence(
             db,
             http_client,
+            secret_key,
             &repo_id,
             &event,
             payload,
@@ -83,6 +121,7 @@ pub fn fire_event_occurrence(
 pub fn fire_event_occurrence_with_claimed_hooks(
     db: Arc<Db>,
     http_client: Arc<reqwest::Client>,
+    secret_key: Arc<[u8; 32]>,
     repo_id: &str,
     event: &str,
     payload: serde_json::Value,
@@ -98,6 +137,7 @@ pub fn fire_event_occurrence_with_claimed_hooks(
         fire_event_async_occurrence(
             db,
             http_client,
+            secret_key,
             &repo_id,
             &event,
             payload,
@@ -163,6 +203,7 @@ pub async fn claim_webhook_delivery_before_spawn(
 async fn fire_event_async_occurrence(
     db: Arc<Db>,
     http_client: Arc<reqwest::Client>,
+    secret_key: Arc<[u8; 32]>,
     repo_id: &str,
     event: &str,
     payload: serde_json::Value,
@@ -224,6 +265,7 @@ async fn fire_event_async_occurrence(
         }
 
         let db_for_mark = db.clone();
+        let secret_key = Arc::clone(&secret_key);
         tokio::spawn(async move {
             // Policy skip, not a delivery failure: the stored URL predates the
             // create-time gate. The "invalid_url" label is documented on
@@ -239,7 +281,28 @@ async fn fire_event_async_occurrence(
             }
 
             let delivery_for_mark = delivery_id.clone();
-            let signature = hook.secret.as_deref().map(|s| sign_payload(s, &bytes));
+            // Open the secret only at signing time; at rest it stays sealed.
+            let signature =
+                match signing_decision(secret_key.as_ref(), hook.secret.as_ref(), &bytes) {
+                    SigningOutcome::NoSecret => None,
+                    SigningOutcome::Signed(sig) => Some(sig),
+                    // A hook that HAS a secret must sign with it or not deliver at
+                    // all: `reveal` returning None means the stored value could
+                    // not be opened (a foreign or rotated key, a truncated
+                    // ciphertext), and sending unsigned would present a broken
+                    // hook as a working one while the ledger marks the event
+                    // delivered. The row stays pending, like a network error, so
+                    // a fixed key can still land it.
+                    SigningOutcome::Withhold => {
+                        crate::metrics::record_webhook_delivery("secret_unopenable");
+                        tracing::warn!(
+                            hook_id = %hook.id,
+                            delivery_id = %delivery_for_mark,
+                            "webhook secret could not be opened; delivery withheld"
+                        );
+                        return;
+                    }
+                };
 
             let mut req = client
                 .post(&hook.url)
@@ -284,3 +347,60 @@ async fn fire_event_async_occurrence(
         });
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use rand::Rng;
+
+    fn key() -> [u8; 32] {
+        chacha20poly1305::aead::OsRng.gen()
+    }
+
+    /// A hook with no secret delivers unsigned, as before.
+    #[test]
+    fn signing_decision_no_secret_is_unsigned() {
+        assert!(matches!(
+            signing_decision(&key(), None, b"{}"),
+            SigningOutcome::NoSecret
+        ));
+    }
+
+    /// A sealed secret that opens signs with the revealed plaintext.
+    #[test]
+    fn signing_decision_sealed_secret_signs() {
+        let k = key();
+        let sealed =
+            crate::webhook_secret::WebhookSecret::new(crate::webhook_secret::seal(&k, "topsecret"));
+        match signing_decision(&k, Some(&sealed), b"{}") {
+            SigningOutcome::Signed(sig) => {
+                assert_eq!(sig, sign_payload("topsecret", b"{}"))
+            }
+            _ => panic!("an openable sealed secret must produce a signature"),
+        }
+    }
+
+    /// A stored value that carries the sealed prefix but cannot be opened
+    /// (foreign key, truncated ciphertext) must withhold, never send unsigned.
+    #[test]
+    fn signing_decision_unopenable_secret_withholds() {
+        let garbage =
+            crate::webhook_secret::WebhookSecret::new("enc:v1:bm90LWNoaXBoZXJ0ZXh0".to_string());
+        assert!(matches!(
+            signing_decision(&key(), Some(&garbage), b"{}"),
+            SigningOutcome::Withhold
+        ));
+    }
+
+    /// A legacy plaintext value signs with itself unchanged.
+    #[test]
+    fn signing_decision_legacy_plaintext_signs() {
+        let legacy = crate::webhook_secret::WebhookSecret::new("plainsecret".to_string());
+        match signing_decision(&key(), Some(&legacy), b"{}") {
+            SigningOutcome::Signed(sig) => {
+                assert_eq!(sig, sign_payload("plainsecret", b"{}"))
+            }
+            _ => panic!("legacy plaintext must keep signing"),
+        }
+    }
+}