feat(metrics): require a bearer token on /metrics when configured (#177)

Add an optional `metrics_token` config (also settable via ROCKET_METRICS_TOKEN).
When set, /metrics requires `Authorization: Bearer <token>` (constant-time
compared via subtle); when unset it stays open and logs a startup warning, so
rolling this out doesn't break existing scrapers. The auth lives in the app,
not the ingress, so it travels with cryptify to external hosts that can't be
firewalled. /health stays public.

Co-authored-by: dobby-yivi-agent[bot] <275734547+dobby-yivi-agent[bot]@users.noreply.github.com>

Author: rubenhensen

conf/config.dev.toml

@@ -15,5 +15,8 @@ usage_db = "/app/data/usage.db"
 # pkg_url = "https://pkg.staging.yivi.app"
 pkg_url = "http://postguard-pkg:8087"
 chunk_size = 5000000
+# Leave unset in dev so /metrics is freely scrapable. In prod set this (or the
+# ROCKET_METRICS_TOKEN env var) so /metrics requires `Authorization: Bearer <token>`.
+# metrics_token = "dev-token"
 # When true, finalize logs the email it WOULD have sent and skips SMTP.
 # staging_mode = true

conf/config.toml

@@ -12,3 +12,7 @@ smtp_port = 1025
 # add-in (prod); `localhost:3000` is the Office add-in dev server.
 allowed_origins = "^https://(postguard\\.(eu|nl)|addin\\.postguard\\.eu|localhost:3000)$"
 pkg_url = "https://pkg.postguard.eu/"
+# Bearer token required to scrape /metrics. Prefer injecting it as a secret
+# via the ROCKET_METRICS_TOKEN env var rather than committing it here. When
+# unset, /metrics is publicly accessible (a startup warning is logged).
+# metrics_token = "change-me"

src/config.rs

@@ -16,6 +16,7 @@ pub struct RawCryptifyConfig {
     chunk_size: Option<u64>,
     session_ttl_secs: Option<u64>,
     staging_mode: Option<bool>,
+    metrics_token: Option<String>,
     usage_db: Option<String>,
 }
 
@@ -36,6 +37,7 @@ pub struct CryptifyConfig {
     chunk_size: u64,
     session_ttl_secs: u64,
     staging_mode: bool,
+    metrics_token: Option<String>,
     /// Filesystem path to the SQLite database backing the rolling-quota
     /// usage state. When set, per-sender usage survives process restarts
     /// (the in-memory map in `Store` is only a cache). `None` keeps usage
@@ -63,6 +65,7 @@ impl From<RawCryptifyConfig> for CryptifyConfig {
             chunk_size: config.chunk_size.unwrap_or(5_000_000),
             session_ttl_secs: config.session_ttl_secs.unwrap_or(3600),
             staging_mode: config.staging_mode.unwrap_or(false),
+            metrics_token: config.metrics_token,
             usage_db: config.usage_db,
         }
     }
@@ -125,6 +128,13 @@ impl CryptifyConfig {
         self.staging_mode
     }
 
+    /// Bearer token required to scrape `/metrics`. `None` leaves the endpoint
+    /// open (with a startup warning); when set, requests must present
+    /// `Authorization: Bearer <token>`.
+    pub fn metrics_token(&self) -> Option<&str> {
+        self.metrics_token.as_deref()
+    }
+
     /// Path to the SQLite database backing rolling-quota usage, if
     /// configured. `None` means usage is kept in memory only.
     pub fn usage_db(&self) -> Option<&str> {
@@ -148,6 +158,7 @@ impl CryptifyConfig {
             chunk_size: 5_000_000,
             session_ttl_secs: 3600,
             staging_mode,
+            metrics_token: None,
             usage_db: None,
         }
     }

src/main.rs

@@ -137,8 +137,51 @@ fn health() -> &'static str {
     "OK"
 }
 
+/// Request guard protecting `/metrics`. When `metrics_token` is configured,
+/// the endpoint requires `Authorization: Bearer <token>` (constant-time
+/// compared); otherwise it stays open (a startup warning is logged). This
+/// auth lives in the app rather than the ingress so the protection travels
+/// with cryptify to every deployment, including external hosts we can't
+/// firewall.
+struct MetricsAuth;
+
+#[rocket::async_trait]
+impl<'r> FromRequest<'r> for MetricsAuth {
+    type Error = ();
+    async fn from_request(request: &'r rocket::Request<'_>) -> rocket::request::Outcome<Self, ()> {
+        let expected = request
+            .rocket()
+            .state::<CryptifyConfig>()
+            .and_then(CryptifyConfig::metrics_token);
+
+        // No token configured → metrics is open (warned about at startup).
+        let Some(expected) = expected else {
+            return rocket::request::Outcome::Success(MetricsAuth);
+        };
+
+        let presented = request
+            .headers()
+            .get_one("Authorization")
+            .and_then(|h| {
+                h.strip_prefix("Bearer ")
+                    .or_else(|| h.strip_prefix("bearer "))
+            })
+            .map(str::trim);
+
+        match presented {
+            Some(token) if constant_time_eq(token, expected) => {
+                rocket::request::Outcome::Success(MetricsAuth)
+            }
+            _ => rocket::request::Outcome::Error((rocket::http::Status::Unauthorized, ())),
+        }
+    }
+}
+
 #[get("/metrics")]
-fn metrics_endpoint(metrics: &State<Arc<Metrics>>) -> rocket::response::content::RawText<String> {
+fn metrics_endpoint(
+    _auth: MetricsAuth,
+    metrics: &State<Arc<Metrics>>,
+) -> rocket::response::content::RawText<String> {
     rocket::response::content::RawText(metrics.render())
 }
 
@@ -991,11 +1034,11 @@ struct UploadStatusResponse {
     prev_offset: Option<u64>,
 }
 
-/// Constant-time compare of the recovery token. Hex-encoded equal-length
-/// strings, but `subtle::ConstantTimeEq` makes the timing independent of
-/// where the bytes start to differ — defeats timing oracles even though
-/// 32 bytes of high-entropy random aren't realistically guessable.
-fn recovery_tokens_match(presented: &str, expected: &str) -> bool {
+/// Constant-time string equality. `subtle::ConstantTimeEq` makes the timing
+/// independent of where the bytes start to differ — defeats timing oracles
+/// on secret comparisons. Used for the recovery token and the `/metrics`
+/// bearer token. A length difference returns early (lengths aren't secret).
+fn constant_time_eq(presented: &str, expected: &str) -> bool {
     use subtle::ConstantTimeEq;
     if presented.len() != expected.len() {
         return false;
@@ -1021,7 +1064,7 @@ async fn upload_status(
         .ok_or_else(|| Error::upload_session_not_found(uuid, "expired_or_unknown"))?;
     let state = state.lock().await;
 
-    if !recovery_tokens_match(&recovery_token.0, &state.recovery_token) {
+    if !constant_time_eq(&recovery_token.0, &state.recovery_token) {
         // Same body shape as evicted/unknown so the response doesn't leak
         // session existence to a token-guessing attacker.
         return Err(Error::upload_session_not_found(uuid, "expired_or_unknown"));
@@ -1479,6 +1522,13 @@ async fn rocket() -> _ {
         .extract::<CryptifyConfig>()
         .expect("Missing configuration");
 
+    if config.metrics_token().is_none() {
+        log::warn!(
+            "metrics_token is not set — /metrics is publicly accessible without authentication. \
+             Set `metrics_token` in config (or ROCKET_METRICS_TOKEN) to require a Bearer token."
+        );
+    }
+
     let pkg_params_url = format!(
         "{}/v2/sign/parameters",
         config.pkg_url().trim_end_matches('/')
@@ -1982,16 +2032,16 @@ mod tests {
     }
 
     #[rocket::async_test]
-    async fn recovery_tokens_match_constant_time_helper() {
+    async fn constant_time_eq_helper() {
         // The function under test is the constant-time wrapper itself —
         // we can't observe timing in a unit test, but we can pin the
         // value-equality semantics so a future refactor doesn't silently
         // turn it into `presented == expected`.
-        assert!(recovery_tokens_match("abc123", "abc123"));
-        assert!(!recovery_tokens_match("abc123", "abc124"));
-        assert!(!recovery_tokens_match("abc123", "abc12")); // length mismatch
-        assert!(!recovery_tokens_match("", "abc"));
-        assert!(recovery_tokens_match("", ""));
+        assert!(constant_time_eq("abc123", "abc123"));
+        assert!(!constant_time_eq("abc123", "abc124"));
+        assert!(!constant_time_eq("abc123", "abc12")); // length mismatch
+        assert!(!constant_time_eq("", "abc"));
+        assert!(constant_time_eq("", ""));
     }
 
     // Browser preflight regression: design AC for #146 explicitly required
@@ -3074,6 +3124,66 @@ mod integration {
         let _ = std::fs::remove_dir_all(dir);
     }
 
+    // Minimal Rocket exposing only /metrics, with the given config managed so
+    // the MetricsAuth guard can read `metrics_token`. Avoids needing a real
+    // VerifyingKey / TestSetup.
+    fn metrics_only_config(with_token: bool) -> CryptifyConfig {
+        let (figment, _dir) = test_figment();
+        let figment = if with_token {
+            figment.merge(("metrics_token", "s3cret"))
+        } else {
+            figment
+        };
+        figment.extract::<CryptifyConfig>().expect("extract config")
+    }
+
+    async fn metrics_only_client(config: CryptifyConfig) -> Client {
+        let rocket = rocket::build()
+            .mount("/", routes![metrics_endpoint])
+            .manage(config)
+            .manage(std::sync::Arc::new(Metrics::new()));
+        Client::tracked(rocket).await.expect("valid rocket")
+    }
+
+    #[rocket::async_test]
+    async fn metrics_requires_bearer_when_token_configured() {
+        let client = metrics_only_client(metrics_only_config(true)).await;
+
+        // No Authorization header → 401.
+        assert_eq!(
+            client.get("/metrics").dispatch().await.status(),
+            Status::Unauthorized
+        );
+        // Wrong token → 401.
+        assert_eq!(
+            client
+                .get("/metrics")
+                .header(Header::new("Authorization", "Bearer wrong"))
+                .dispatch()
+                .await
+                .status(),
+            Status::Unauthorized
+        );
+        // Correct token → 200 with the metrics body.
+        let ok = client
+            .get("/metrics")
+            .header(Header::new("Authorization", "Bearer s3cret"))
+            .dispatch()
+            .await;
+        assert_eq!(ok.status(), Status::Ok);
+        assert!(ok
+            .into_string()
+            .await
+            .unwrap_or_default()
+            .contains("cryptify_uploads_total"));
+    }
+
+    #[rocket::async_test]
+    async fn metrics_open_when_token_unset() {
+        let client = metrics_only_client(metrics_only_config(false)).await;
+        assert_eq!(client.get("/metrics").dispatch().await.status(), Status::Ok);
+    }
+
     #[rocket::async_test]
     async fn upload_happy_path_multi_chunk() {
         // Two chunks >1 MiB to exercise the rolling token chain across