From 655c3ab97e2f61e1333539af758e71ce68e00a11 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sun, 7 Jun 2026 19:46:37 +0200 Subject: [PATCH] fix(manager-core): F-Q-005 preserve AuthzDenied::Repo through service-layer authz checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every check_read/check_write/check_publish helper across kv, docs, files, pubsub, queue services used `.map_err(|_| KvError::Forbidden)?`, collapsing AuthzDenied::Denied AND AuthzDenied::Repo(repo_err) into a single Forbidden. A transient Postgres blip during the membership lookup surfaced as 403 to scripts; operators couldn't distinguish "real forbidden" from "DB flap during permission check". Replace each call site with the explicit match pattern already used by users_service::require (users_service.rs:177-181): Ok(()) → continue Err(AuthzDenied::Denied) → Err(ServiceError::Forbidden) Err(AuthzDenied::Repo(e)) → Err(ServiceError::Backend(e.to_string())) 7 call sites updated (2 in kv_service, 2 in docs_service, 2 in files_service, 2 in pubsub_service, 1 in queue_service). AUDIT.md anchor: F-Q-005. Depends on F-Q-004 (which added Backend to QueueError/PubsubError). Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/manager-core/src/docs_service.rs | 18 ++++++++++++------ crates/manager-core/src/files_service.rs | 16 ++++++++++++---- crates/manager-core/src/kv_service.rs | 16 ++++++++++------ crates/manager-core/src/pubsub_service.rs | 16 ++++++++++++---- crates/manager-core/src/queue_service.rs | 8 ++++++-- 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/crates/manager-core/src/docs_service.rs b/crates/manager-core/src/docs_service.rs index 1a31eed..0e86829 100644 --- a/crates/manager-core/src/docs_service.rs +++ b/crates/manager-core/src/docs_service.rs @@ -56,18 +56,24 @@ impl DocsServiceImpl { async fn check_read(&self, cx: &SdkCallCx) -> Result<(), DocsError> { if let Some(ref principal) = cx.principal { - authz::require(&*self.authz, principal, Capability::AppDocsRead(cx.app_id)) - .await - .map_err(|_| DocsError::Forbidden)?; + match authz::require(&*self.authz, principal, Capability::AppDocsRead(cx.app_id)).await + { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(DocsError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(DocsError::Backend(e.to_string())), + } } Ok(()) } async fn check_write(&self, cx: &SdkCallCx) -> Result<(), DocsError> { if let Some(ref principal) = cx.principal { - authz::require(&*self.authz, principal, Capability::AppDocsWrite(cx.app_id)) - .await - .map_err(|_| DocsError::Forbidden)?; + match authz::require(&*self.authz, principal, Capability::AppDocsWrite(cx.app_id)).await + { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(DocsError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(DocsError::Backend(e.to_string())), + } } Ok(()) } diff --git a/crates/manager-core/src/files_service.rs b/crates/manager-core/src/files_service.rs index 443f0bd..78de53d 100644 --- a/crates/manager-core/src/files_service.rs +++ b/crates/manager-core/src/files_service.rs @@ -50,22 +50,30 @@ impl FilesServiceImpl { async fn check_read(&self, cx: &SdkCallCx) -> Result<(), FilesError> { if let Some(ref principal) = cx.principal { - authz::require(&*self.authz, principal, Capability::AppFilesRead(cx.app_id)) + match authz::require(&*self.authz, principal, Capability::AppFilesRead(cx.app_id)) .await - .map_err(|_| FilesError::Forbidden)?; + { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(FilesError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(FilesError::Backend(e.to_string())), + } } Ok(()) } async fn check_write(&self, cx: &SdkCallCx) -> Result<(), FilesError> { if let Some(ref principal) = cx.principal { - authz::require( + match authz::require( &*self.authz, principal, Capability::AppFilesWrite(cx.app_id), ) .await - .map_err(|_| FilesError::Forbidden)?; + { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(FilesError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(FilesError::Backend(e.to_string())), + } } Ok(()) } diff --git a/crates/manager-core/src/kv_service.rs b/crates/manager-core/src/kv_service.rs index d38dc67..7a5bc4a 100644 --- a/crates/manager-core/src/kv_service.rs +++ b/crates/manager-core/src/kv_service.rs @@ -47,18 +47,22 @@ impl KvServiceImpl { async fn check_read(&self, cx: &SdkCallCx) -> Result<(), KvError> { if let Some(ref principal) = cx.principal { - authz::require(&*self.authz, principal, Capability::AppKvRead(cx.app_id)) - .await - .map_err(|_| KvError::Forbidden)?; + match authz::require(&*self.authz, principal, Capability::AppKvRead(cx.app_id)).await { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(KvError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(KvError::Backend(e.to_string())), + } } Ok(()) } async fn check_write(&self, cx: &SdkCallCx) -> Result<(), KvError> { if let Some(ref principal) = cx.principal { - authz::require(&*self.authz, principal, Capability::AppKvWrite(cx.app_id)) - .await - .map_err(|_| KvError::Forbidden)?; + match authz::require(&*self.authz, principal, Capability::AppKvWrite(cx.app_id)).await { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(KvError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(KvError::Backend(e.to_string())), + } } Ok(()) } diff --git a/crates/manager-core/src/pubsub_service.rs b/crates/manager-core/src/pubsub_service.rs index 13c6451..5f84fac 100644 --- a/crates/manager-core/src/pubsub_service.rs +++ b/crates/manager-core/src/pubsub_service.rs @@ -115,13 +115,17 @@ impl PubsubServiceImpl { async fn check_publish(&self, cx: &SdkCallCx) -> Result<(), PubsubError> { if let Some(ref principal) = cx.principal { - authz::require( + match authz::require( &*self.authz, principal, Capability::AppPubsubPublish(cx.app_id), ) .await - .map_err(|_| PubsubError::Forbidden)?; + { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(PubsubError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(PubsubError::Backend(e.to_string())), + } } Ok(()) } @@ -217,13 +221,17 @@ impl PubsubService for PubsubServiceImpl { }; // Minting reuses the existing pub/sub publish capability (no new // scope — the seven-scope commitment holds). - authz::require( + match authz::require( &*self.authz, principal, Capability::AppPubsubPublish(cx.app_id), ) .await - .map_err(|_| PubsubError::Forbidden)?; + { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(PubsubError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(PubsubError::Backend(e.to_string())), + } let (Some(topic_repo), Some(secrets)) = (self.topics.as_ref(), self.secrets.as_ref()) else { diff --git a/crates/manager-core/src/queue_service.rs b/crates/manager-core/src/queue_service.rs index 5558dec..0c0dde2 100644 --- a/crates/manager-core/src/queue_service.rs +++ b/crates/manager-core/src/queue_service.rs @@ -59,13 +59,17 @@ impl QueueService for QueueServiceImpl { // check (cx.principal is None); authenticated callers must hold // AppQueueEnqueue. if let Some(principal) = cx.principal.as_ref() { - authz::require( + match authz::require( &*self.authz, principal, Capability::AppQueueEnqueue(cx.app_id), ) .await - .map_err(|_| QueueError::Forbidden)?; + { + Ok(()) => {} + Err(authz::AuthzDenied::Denied) => return Err(QueueError::Forbidden), + Err(authz::AuthzDenied::Repo(e)) => return Err(QueueError::Backend(e.to_string())), + } } let deliver_after = opts