From 39c290ddd240221f7209ee9ee05a08c21508604d Mon Sep 17 00:00:00 2001 From: Anthony Ronning <101225832+AnthonyRonning@users.noreply.github.com> Date: Sun, 13 Sep 2026 20:01:35 +0000 Subject: [PATCH 1/2] Make enclave health checks independent of provider availability --- .agents/skills/validate-opensecret/SKILL.md | 8 +- services/opensecret/README.md | 7 + services/opensecret/src/main.rs | 10 +- services/opensecret/src/web/health_routes.rs | 155 ++++++------------- services/opensecret/src/web/mod.rs | 2 +- 5 files changed, 66 insertions(+), 116 deletions(-) diff --git a/.agents/skills/validate-opensecret/SKILL.md b/.agents/skills/validate-opensecret/SKILL.md index 3a25b3a7f..11333e576 100644 --- a/.agents/skills/validate-opensecret/SKILL.md +++ b/.agents/skills/validate-opensecret/SKILL.md @@ -130,9 +130,11 @@ curl --fail --silent --show-error http://127.0.0.1:3000/health-check curl --fail --silent --show-error http://127.0.0.1:3000/health-check-extended ``` -The first is process liveness; the second checks Tinfoil model connectivity. -Neither proves PostgreSQL, auth, encryption, persistence, routing, billing, -flags, or a user flow. +Both URLs report process liveness with the same `status` and `version` JSON; +the extended URL is a compatibility alias. Neither calls a provider or probes +PostgreSQL. They do not prove provider availability, auth, encryption, +persistence, routing, billing, flags, or a user flow. Provider availability +checks belong to separate diagnostics, not load-balancer origin health. Exercise protected routes through the monorepo-root `sdk/` directory or through the corresponding Maple application client: diff --git a/services/opensecret/README.md b/services/opensecret/README.md index 5f7d4ac65..eaa483bc9 100644 --- a/services/opensecret/README.md +++ b/services/opensecret/README.md @@ -55,6 +55,13 @@ OpenAI-compatible wire endpoint. Use an OpenSecret SDK or Maple for protected-route integration tests; plain `curl` is suitable only for public health probes. +`GET /health-check` returns HTTP 200 with `{"status":"pass","version":"v1"}` +when the server can respond. `/health-check-extended` remains an alias with the +same response for existing monitors. These endpoints do not contact Tinfoil, +other providers, or PostgreSQL; provider outages must not remove responsive +enclaves from load-balancer rotation. Monitor provider availability separately +from origin liveness. + Contributor and coding-agent standards live in [`AGENTS.md`](AGENTS.md). Task-specific development, API, provider, security, and validation workflows live under the monorepo-root [`.agents/skills/`](../../.agents/skills/). diff --git a/services/opensecret/src/main.rs b/services/opensecret/src/main.rs index e8fdc3247..70cc579ca 100644 --- a/services/opensecret/src/main.rs +++ b/services/opensecret/src/main.rs @@ -22,9 +22,9 @@ use crate::sqs::SqsEventPublisher; use crate::web::openai_auth::{validate_openai_auth, validate_optional_openai_auth}; use crate::web::platform_login_routes; use crate::web::{ - conversation_projects_routes, conversations_routes, health_routes_with_state, - instructions_routes, login_routes, native_handoff_routes, oauth_routes, openai_models_routes, - openai_routes, protected_routes, responses_routes, web_routes, + conversation_projects_routes, conversations_routes, health_routes, instructions_routes, + login_routes, native_handoff_routes, oauth_routes, openai_models_routes, openai_routes, + protected_routes, responses_routes, web_routes, }; use crate::{attestation_routes::SessionState, web::platform_routes}; use bounded_ttl_cache::BoundedTtlCache; @@ -4226,14 +4226,14 @@ async fn main() -> Result<(), Error> { let application = application_routes(app_state.clone()); let v2_application = application .clone() - .merge(health_routes_with_state(app_state.clone())) + .merge(health_routes()) .merge(native_handoff_routes(app_state.clone())) .layer(from_fn(add_error_contract_header)); let transport_v2_gateway = transport_v2::gateway::TransportV2Gateway::new(app_state.clone(), v2_application); let app = application - .merge(health_routes_with_state(app_state.clone())) + .merge(health_routes()) .merge(attestation_routes::router(app_state.clone())) .merge(transport_v2_gateway.router()) .layer(cors) diff --git a/services/opensecret/src/web/health_routes.rs b/services/opensecret/src/web/health_routes.rs index 428b21b07..333cf9ccf 100644 --- a/services/opensecret/src/web/health_routes.rs +++ b/services/opensecret/src/web/health_routes.rs @@ -1,126 +1,67 @@ -use crate::{ - provider_client::{ProviderClient, ProviderRequest}, - AppState, -}; -use axum::{http::StatusCode, Router}; -use axum::{routing::get, Json}; -use reqwest::Method; +use axum::{routing::get, Json, Router}; use serde::Serialize; -use std::sync::Arc; -use std::time::Duration; const API_VERSION: &str = "v1"; -pub fn router_with_state(state: Arc) -> Router<()> { +/// Process liveness only: an upstream outage must not remove a healthy enclave +/// from the load balancer. Keep the extended URL for existing monitors. +pub fn router() -> Router { Router::new() .route("/health-check", get(health_check)) - .route("/health-check-extended", get(health_check_extended)) - .with_state(state) + .route("/health-check-extended", get(health_check)) } #[derive(Serialize)] -pub struct HealthResponse { - pub status: String, - pub version: String, +struct HealthResponse { + status: &'static str, + version: &'static str, } -#[derive(Serialize)] -pub struct ExtendedHealthResponse { - pub status: String, - pub version: String, - pub outbound_connectivity: bool, - #[serde(skip_serializing_if = "Option::is_none")] - pub model_check: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub error: Option, -} - -impl HealthResponse { - /// Fabricate a status: pass response without checking database connectivity - pub fn new_ok() -> Self { - Self { - status: String::from("pass"), - version: String::from(API_VERSION), - } - } +async fn health_check() -> Json { + Json(HealthResponse { + status: "pass", + version: API_VERSION, + }) } -/// Health check endpoint following the IETF draft standard -/// -pub async fn health_check() -> Result, (StatusCode, String)> { - Ok(Json(HealthResponse::new_ok())) -} +#[cfg(test)] +mod tests { + use super::*; + use axum::http::{header, StatusCode}; + use serde_json::json; + use std::time::Duration; -/// Extended health check that tests outbound connectivity via model listing -pub async fn health_check_extended( - axum::extract::State(state): axum::extract::State>, -) -> Result, (StatusCode, String)> { - let tinfoil_proxy = state.proxy_router.get_tinfoil_proxy(); - let result = tokio::time::timeout( - Duration::from_secs(5), - fetch_models_directly(&state.provider_client, &tinfoil_proxy), - ) - .await; + #[tokio::test] + async fn both_health_urls_serve_liveness_without_application_dependencies() { + // Deliberately construct only the production health router: no + // AppState, database, provider client, credentials, or model service. + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + axum::serve(listener, router()).await.unwrap(); + }); + let client = crate::http_client::client_builder() + .no_proxy() + .timeout(Duration::from_secs(2)) + .build() + .unwrap(); - match result { - Ok(Ok(model_count)) => Ok(Json(ExtendedHealthResponse { - status: "pass".to_string(), - version: API_VERSION.to_string(), - outbound_connectivity: true, - model_check: Some(format!( - "Successfully fetched {} models from {}", - model_count, tinfoil_proxy.provider_name - )), - error: None, - })), - Ok(Err(e)) => { - // Failed to fetch models - Err(( - StatusCode::SERVICE_UNAVAILABLE, - format!( - "Failed to fetch models from {}: {}", - tinfoil_proxy.provider_name, e - ), - )) + for path in ["/health-check", "/health-check-extended"] { + let response = client + .get(format!("http://{address}{path}")) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK, "{path}"); + assert_eq!(response.headers()[header::CONTENT_TYPE], "application/json"); + assert_eq!( + response.json::().await.unwrap(), + json!({"status": "pass", "version": "v1"}), + "{path} must not claim outbound connectivity or model health" + ); } - Err(_) => Err(( - StatusCode::SERVICE_UNAVAILABLE, - format!( - "Model fetch from {} timed out after 5 seconds", - tinfoil_proxy.provider_name - ), - )), - } -} - -/// Helper function to fetch models directly without caching -async fn fetch_models_directly( - client: &ProviderClient, - proxy_config: &crate::proxy_config::ProxyConfig, -) -> Result> { - let res = client - .send( - proxy_config, - ProviderRequest::new(Method::GET, "/v1/models", Duration::from_secs(5)), - ) - .await?; - if !res.is_success() { - let status = res.status_code(); - let body_bytes = res.bytes().await?; - let body_str = String::from_utf8_lossy(&body_bytes); - return Err(format!("HTTP {}: {}", status, body_str).into()); + server.abort(); + assert!(server.await.unwrap_err().is_cancelled()); } - - let body_bytes = res.bytes().await?; - let models_response: serde_json::Value = serde_json::from_slice(&body_bytes)?; - - // Count the models - let model_count = models_response - .get("data") - .and_then(|d| d.as_array()) - .map(|arr| arr.len()) - .unwrap_or(0); - - Ok(model_count) } diff --git a/services/opensecret/src/web/mod.rs b/services/opensecret/src/web/mod.rs index e0d1310dc..770cbc01d 100644 --- a/services/opensecret/src/web/mod.rs +++ b/services/opensecret/src/web/mod.rs @@ -13,7 +13,7 @@ pub mod responses; pub mod web_routes; pub(crate) mod web_safety; -pub use health_routes::router_with_state as health_routes_with_state; +pub use health_routes::router as health_routes; pub use login_routes::router as login_routes; pub(crate) use native_handoff_routes::router as native_handoff_routes; pub use oauth_routes::router as oauth_routes; From 2081a7cec7d27db3df5fd9ba76b3a2562ec2f573 Mon Sep 17 00:00:00 2001 From: Anthony Ronning <101225832+AnthonyRonning@users.noreply.github.com> Date: Sun, 13 Sep 2026 20:21:43 +0000 Subject: [PATCH 2/2] Remove the obsolete extended health endpoint --- .agents/skills/validate-opensecret/SKILL.md | 11 +++--- services/opensecret/README.md | 10 ++--- services/opensecret/src/web/health_routes.rs | 41 +++++++++++--------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/.agents/skills/validate-opensecret/SKILL.md b/.agents/skills/validate-opensecret/SKILL.md index 11333e576..f698728f4 100644 --- a/.agents/skills/validate-opensecret/SKILL.md +++ b/.agents/skills/validate-opensecret/SKILL.md @@ -127,14 +127,13 @@ Health probes are preliminary only: ```sh curl --fail --silent --show-error http://127.0.0.1:3000/health-check -curl --fail --silent --show-error http://127.0.0.1:3000/health-check-extended ``` -Both URLs report process liveness with the same `status` and `version` JSON; -the extended URL is a compatibility alias. Neither calls a provider or probes -PostgreSQL. They do not prove provider availability, auth, encryption, -persistence, routing, billing, flags, or a user flow. Provider availability -checks belong to separate diagnostics, not load-balancer origin health. +This reports process liveness with `status` and `version` JSON. It does not call +a provider or probe PostgreSQL, and does not prove provider availability, auth, +encryption, persistence, routing, billing, flags, or a user flow. Provider +availability checks belong to separate diagnostics, not load-balancer origin +health. Exercise protected routes through the monorepo-root `sdk/` directory or through the corresponding Maple application client: diff --git a/services/opensecret/README.md b/services/opensecret/README.md index eaa483bc9..df8714716 100644 --- a/services/opensecret/README.md +++ b/services/opensecret/README.md @@ -56,11 +56,11 @@ protected-route integration tests; plain `curl` is suitable only for public health probes. `GET /health-check` returns HTTP 200 with `{"status":"pass","version":"v1"}` -when the server can respond. `/health-check-extended` remains an alias with the -same response for existing monitors. These endpoints do not contact Tinfoil, -other providers, or PostgreSQL; provider outages must not remove responsive -enclaves from load-balancer rotation. Monitor provider availability separately -from origin liveness. +when the server can respond. It does not contact Tinfoil, other providers, or +PostgreSQL; provider outages must not remove responsive enclaves from +load-balancer rotation. Monitor provider availability separately from origin +liveness. `/health-check-extended` has been removed; migrate any remaining +monitors or deployment scripts to `/health-check` before deploying this change. Contributor and coding-agent standards live in [`AGENTS.md`](AGENTS.md). Task-specific development, API, provider, security, and validation workflows diff --git a/services/opensecret/src/web/health_routes.rs b/services/opensecret/src/web/health_routes.rs index 333cf9ccf..1f8a50e43 100644 --- a/services/opensecret/src/web/health_routes.rs +++ b/services/opensecret/src/web/health_routes.rs @@ -4,11 +4,9 @@ use serde::Serialize; const API_VERSION: &str = "v1"; /// Process liveness only: an upstream outage must not remove a healthy enclave -/// from the load balancer. Keep the extended URL for existing monitors. +/// from the load balancer. pub fn router() -> Router { - Router::new() - .route("/health-check", get(health_check)) - .route("/health-check-extended", get(health_check)) + Router::new().route("/health-check", get(health_check)) } #[derive(Serialize)] @@ -32,7 +30,7 @@ mod tests { use std::time::Duration; #[tokio::test] - async fn both_health_urls_serve_liveness_without_application_dependencies() { + async fn health_is_dependency_free_and_extended_health_is_removed() { // Deliberately construct only the production health router: no // AppState, database, provider client, credentials, or model service. let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -46,20 +44,25 @@ mod tests { .build() .unwrap(); - for path in ["/health-check", "/health-check-extended"] { - let response = client - .get(format!("http://{address}{path}")) - .send() - .await - .unwrap(); - assert_eq!(response.status(), StatusCode::OK, "{path}"); - assert_eq!(response.headers()[header::CONTENT_TYPE], "application/json"); - assert_eq!( - response.json::().await.unwrap(), - json!({"status": "pass", "version": "v1"}), - "{path} must not claim outbound connectivity or model health" - ); - } + let response = client + .get(format!("http://{address}/health-check")) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.headers()[header::CONTENT_TYPE], "application/json"); + assert_eq!( + response.json::().await.unwrap(), + json!({"status": "pass", "version": "v1"}), + "liveness must not claim outbound connectivity or model health" + ); + + let removed = client + .get(format!("http://{address}/health-check-extended")) + .send() + .await + .unwrap(); + assert_eq!(removed.status(), StatusCode::NOT_FOUND); server.abort(); assert!(server.await.unwrap_err().is_cancelled());