diff --git a/src/common/di.rs b/src/common/di.rs index 5c591fc9..6034fed6 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -1487,9 +1487,25 @@ impl AppServiceFactory { core.config.features.grant_cleanup.interval_hours, ), ); - // First tick fires immediately inside start_cleanup_job — - // matches the trash/storage-usage daemon shape. - svc.clone().start_cleanup_job().await; + // Registered with the periodic-job scheduler + // (`docs/plan/job-registry.md` Part 1); the retired + // `start_cleanup_job` used to spawn its own interval loop. + // Admin `?force=true` trigger still calls `svc.purge(Some(0))` + // directly — grace override doesn't fit the JobHandler shape. + let interval = svc.interval(); + if let Err(e) = core + .job_registry + .register(svc.clone(), Some(interval), None) + .await + { + tracing::error!("Failed to register grant_cleanup job: {e}"); + } else { + tracing::info!( + "Grant cleanup registered with scheduler (every {}h, grace = {}d)", + interval.as_secs() / 3600, + core.config.features.grant_cleanup.grace_days, + ); + } Some(svc) } else { tracing::info!( diff --git a/src/infrastructure/services/grant_cleanup_service.rs b/src/infrastructure/services/grant_cleanup_service.rs index 54435606..5c73bd03 100644 --- a/src/infrastructure/services/grant_cleanup_service.rs +++ b/src/infrastructure/services/grant_cleanup_service.rs @@ -1,36 +1,39 @@ -//! Background daemon that purges expired `storage.role_grants` rows. +//! Service that purges expired `storage.role_grants` rows. //! //! The AuthZ engine already filters expired grants out of every //! permission check at read time (`expires_at IS NULL OR //! expires_at > NOW()` on every `check` / `list_grants_*` path in //! `PgAclEngine`), so expired rows never leak permission. They just -//! accumulate. This daemon garbage-collects them once per -//! [`GrantCleanupService::interval_hours`], with a grace window past -//! `expires_at` that preserves the audit / support answer to "what -//! happened to my access?" for a few weeks. +//! accumulate. This service garbage-collects them, with a grace window +//! past `expires_at` that preserves the audit / support answer to +//! "what happened to my access?" for a few weeks. //! -//! Shape mirrors [`TrashCleanupService`] verbatim (fire-and-forget -//! `tokio::spawn`, `tokio::time::interval`, first-tick-immediate). The -//! authoritative pattern for background daemons in this codebase; see -//! the plan doc `docs/plan/` (deferred future work: fold all daemons -//! into a central `JobRegistry` that plugins can also register into). -//! -//! [`TrashCleanupService`]: crate::infrastructure::services::trash_cleanup_service::TrashCleanupService +//! **Scheduling.** Registered with the periodic-job scheduler +//! (`docs/plan/job-registry.md` Part 1). The retired `start_cleanup_job` +//! used to spawn its own `tokio::interval` loop; the scheduler now +//! dispatches [`GrantCleanupService::purge`] on the configured cadence +//! and handles panic containment + exclusivity + admin trigger routing. +//! Admin trigger with `?force=true` still bypasses the registered +//! job and calls `purge(Some(0))` directly so the grace override reaches +//! the underlying SQL. use std::sync::Arc; use std::time::{Duration, Instant}; -use tokio::time; use tracing::{error, info}; use crate::application::ports::authorization_ports::AuthorizationEngine; +use crate::common::errors::DomainError; +use crate::infrastructure::scheduler::{JobHandler, JobOutcome}; use crate::infrastructure::services::pg_acl_engine::PgAclEngine; +use async_trait::async_trait; -/// Daemon that periodically deletes expired grants. +pub const GRANT_CLEANUP_JOB_NAME: &str = "grant_cleanup"; + +/// Service that deletes expired grants. /// /// Owns an `Arc` (not a `dyn AuthorizationEngine`) to avoid -/// the wrapper allocation on every SQL call — the daemon is the sole -/// caller of `purge_expired_grants` outside of the admin trigger -/// endpoint, both statically dispatched. +/// the wrapper allocation on every SQL call — the caller set is small +/// (scheduler tick + admin trigger endpoint), both statically dispatched. pub struct GrantCleanupService { authz: Arc, grace_days: u32, @@ -48,50 +51,35 @@ impl GrantCleanupService { } } - /// Grace period the daemon uses on its scheduled ticks. Exposed + /// Grace period the service uses on its scheduled runs. Exposed /// for the admin trigger's default-response field. pub fn grace_days(&self) -> u32 { self.grace_days } - /// Fire-and-forget the periodic purge. Never joins; killed - /// implicitly at `tokio::runtime::shutdown`. - pub async fn start_cleanup_job(self: Arc) { - let interval_hours = self.interval_hours; - let grace_days = self.grace_days; - info!( - "Starting grant-cleanup daemon: every {}h, grace = {}d", - interval_hours, grace_days - ); - - tokio::spawn(async move { - let mut interval = time::interval(Duration::from_secs(interval_hours * 60 * 60)); - // First tick fires immediately — matches TrashCleanupService. - // Any accumulated backlog at boot gets flushed straight away. - loop { - interval.tick().await; - self.run_once().await; - } - }); + /// Cadence exposed as `Duration` so DI passes a sanitised value + /// (post-`.max(1)`) to `JobRegistry::register`. + pub fn interval(&self) -> Duration { + Duration::from_secs(self.interval_hours * 3600) } - /// One scheduled pass. Also called by the admin trigger endpoint - /// (via a shared `Arc` on `AppState`). + /// Run one purge pass. /// /// `grace_override`: /// - `None` → use the configured grace (`self.grace_days`). /// - `Some(n)` → override with `n`. The admin `?force=true` trigger /// passes `Some(0)` so Hurl regressions can hit expired grants /// without waiting the configured grace out. - pub async fn purge(&self, grace_override: Option) -> u64 { + /// + /// Returns `Ok(count)` on success, `Err(_)` on DB error. Audit-log + /// lines fire on both paths (success + failure) — bulk deletion of + /// authorization rows is security-relevant enough to log even a + /// zero-count run, and failures MUST reach the audit channel. + pub async fn purge(&self, grace_override: Option) -> Result { let grace = grace_override.unwrap_or(self.grace_days); let start = Instant::now(); match self.authz.purge_expired_grants(grace).await { Ok(count) => { - // Audit-channel logging: bulk deletion of authorization - // rows is security-relevant enough to keep it in the - // audit stream even when the count is zero (proves the - // daemon is reachable). info!( target: "audit", event = "grant_cleanup.purged", @@ -102,7 +90,7 @@ impl GrantCleanupService { count, grace, ); - count + Ok(count) } Err(e) => { error!( @@ -112,13 +100,33 @@ impl GrantCleanupService { error = %e, "Grant cleanup failed" ); - 0 + Err(e) } } } +} - /// Convenience for the scheduled loop. - async fn run_once(&self) { - let _ = self.purge(None).await; +#[async_trait] +impl JobHandler for GrantCleanupService { + fn name(&self) -> &str { + GRANT_CLEANUP_JOB_NAME + } + + /// Runs one purge with the configured grace window. `count` on the + /// returned `JobOutcome::Ok` is the number of `role_grants` rows + /// physically deleted; `extra.grace_days` records which grace was + /// applied so admin listings can see it without a second lookup. + /// + /// Admin `?force=true` (grace = 0) does NOT come through here — + /// that path calls `purge(Some(0))` directly on the shared + /// `Arc` from the handler. + async fn run(&self) -> JobOutcome { + match self.purge(None).await { + Ok(count) => JobOutcome::ok_with( + count, + serde_json::json!({ "grace_days": self.grace_days }), + ), + Err(e) => JobOutcome::Err(format!("grant cleanup failed: {e}")), + } } } diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index 51b3f647..d9442e13 100644 --- a/src/interfaces/api/handlers/admin_handler.rs +++ b/src/interfaces/api/handlers/admin_handler.rs @@ -2280,7 +2280,12 @@ pub async fn internal_trigger_grant_cleanup( // only — the daemon's configured grace is untouched. Mirrors the // `trigger-gc?force=true` shape. let grace_override = if query.force { Some(0) } else { None }; - let grants_deleted = svc.purge(grace_override).await; + let grants_deleted = match svc.purge(grace_override).await { + Ok(n) => n, + Err(e) => { + return AppError::internal_error(format!("grant cleanup failed: {e}")).into_response(); + } + }; let grace_days = grace_override.unwrap_or_else(|| svc.grace_days()); ( StatusCode::OK,