From 04dc81115eff0bbd41493e8990696f612571b7d2 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sun, 7 Jun 2026 19:48:23 +0200 Subject: [PATCH] fix(manager-core): F-Q-003 promote authz::script_gate helper, migrate 7 service call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace nine open-coded `if cx.principal.is_some() { authz::require(...).await.map_err(...) }` blocks with a single `authz::script_gate(repo, cx, cap, forbidden_fn, backend_fn)` helper. The helper enshrines the script-as-gate semantics (anonymous public- HTTP scripts skip the check) and the AuthzDenied::{Denied,Repo} mapping in one place — eliminating drift between services. Call sites migrated: - kv_service::check_read / check_write - docs_service::check_read / check_write - files_service::check_read / check_write - pubsub_service::check_publish - queue_service::enqueue The pubsub_service::mint_subscriber_token path keeps the explicit match because it does a separate `principal` early-bind for other validation; converting it would obscure intent. AUDIT.md anchor: F-Q-003. Depends on F-Q-004 (Backend variant) and F-Q-005 (Repo-passthrough pattern). Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/manager-core/src/authz.rs | 31 ++++++++++++++++++ crates/manager-core/src/docs_service.rs | 34 +++++++++---------- crates/manager-core/src/files_service.rs | 40 +++++++++-------------- crates/manager-core/src/kv_service.rs | 32 +++++++++--------- crates/manager-core/src/pubsub_service.rs | 22 +++++-------- crates/manager-core/src/queue_service.rs | 21 +++++------- 6 files changed, 95 insertions(+), 85 deletions(-) diff --git a/crates/manager-core/src/authz.rs b/crates/manager-core/src/authz.rs index 1b46c47..1f8b0b5 100644 --- a/crates/manager-core/src/authz.rs +++ b/crates/manager-core/src/authz.rs @@ -299,6 +299,37 @@ pub enum AuthzDenied { Repo(#[from] AuthzError), } +/// Script-as-gate authz: anonymous public-HTTP scripts skip the check +/// (`cx.principal` is `None`); authenticated callers must hold `cap`. +/// +/// Replaces the open-coded +/// `if let Some(p) = cx.principal { authz::require(...).await.map_err(...)? }` +/// pattern across every stateful service. `forbidden` is called when +/// the membership lookup returns `Denied`; `backend` is called when the +/// underlying repo errors. Both closures map to the caller's service- +/// specific error enum. +/// +/// # Errors +/// +/// Returns the result of `forbidden(())` on `AuthzDenied::Denied`, or +/// `backend(repo_err.to_string())` on `AuthzDenied::Repo(repo_err)`. +pub async fn script_gate( + repo: &dyn AuthzRepo, + cx: &picloud_shared::SdkCallCx, + cap: Capability, + forbidden: impl FnOnce() -> E, + backend: impl FnOnce(String) -> E, +) -> Result<(), E> { + let Some(principal) = cx.principal.as_ref() else { + return Ok(()); + }; + match require(repo, principal, cap).await { + Ok(()) => Ok(()), + Err(AuthzDenied::Denied) => Err(forbidden()), + Err(AuthzDenied::Repo(e)) => Err(backend(e.to_string())), + } +} + // ---------------------------------------------------------------------------- // Layer 1: role-derived grant // ---------------------------------------------------------------------------- diff --git a/crates/manager-core/src/docs_service.rs b/crates/manager-core/src/docs_service.rs index 0e86829..1ae9472 100644 --- a/crates/manager-core/src/docs_service.rs +++ b/crates/manager-core/src/docs_service.rs @@ -55,27 +55,25 @@ impl DocsServiceImpl { } async fn check_read(&self, cx: &SdkCallCx) -> Result<(), DocsError> { - if let Some(ref principal) = cx.principal { - 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(()) + authz::script_gate( + &*self.authz, + cx, + Capability::AppDocsRead(cx.app_id), + || DocsError::Forbidden, + DocsError::Backend, + ) + .await } async fn check_write(&self, cx: &SdkCallCx) -> Result<(), DocsError> { - if let Some(ref principal) = cx.principal { - 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(()) + authz::script_gate( + &*self.authz, + cx, + Capability::AppDocsWrite(cx.app_id), + || DocsError::Forbidden, + DocsError::Backend, + ) + .await } } diff --git a/crates/manager-core/src/files_service.rs b/crates/manager-core/src/files_service.rs index 78de53d..ada7220 100644 --- a/crates/manager-core/src/files_service.rs +++ b/crates/manager-core/src/files_service.rs @@ -49,33 +49,25 @@ impl FilesServiceImpl { } async fn check_read(&self, cx: &SdkCallCx) -> Result<(), FilesError> { - if let Some(ref principal) = cx.principal { - match authz::require(&*self.authz, principal, Capability::AppFilesRead(cx.app_id)) - .await - { - Ok(()) => {} - Err(authz::AuthzDenied::Denied) => return Err(FilesError::Forbidden), - Err(authz::AuthzDenied::Repo(e)) => return Err(FilesError::Backend(e.to_string())), - } - } - Ok(()) + authz::script_gate( + &*self.authz, + cx, + Capability::AppFilesRead(cx.app_id), + || FilesError::Forbidden, + FilesError::Backend, + ) + .await } async fn check_write(&self, cx: &SdkCallCx) -> Result<(), FilesError> { - if let Some(ref principal) = cx.principal { - match authz::require( - &*self.authz, - principal, - Capability::AppFilesWrite(cx.app_id), - ) - .await - { - Ok(()) => {} - Err(authz::AuthzDenied::Denied) => return Err(FilesError::Forbidden), - Err(authz::AuthzDenied::Repo(e)) => return Err(FilesError::Backend(e.to_string())), - } - } - Ok(()) + authz::script_gate( + &*self.authz, + cx, + Capability::AppFilesWrite(cx.app_id), + || FilesError::Forbidden, + FilesError::Backend, + ) + .await } /// Best-effort `ServiceEvent` emission. A failed emit is logged but diff --git a/crates/manager-core/src/kv_service.rs b/crates/manager-core/src/kv_service.rs index 7a5bc4a..4514227 100644 --- a/crates/manager-core/src/kv_service.rs +++ b/crates/manager-core/src/kv_service.rs @@ -46,25 +46,25 @@ impl KvServiceImpl { } async fn check_read(&self, cx: &SdkCallCx) -> Result<(), KvError> { - if let Some(ref principal) = cx.principal { - 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(()) + authz::script_gate( + &*self.authz, + cx, + Capability::AppKvRead(cx.app_id), + || KvError::Forbidden, + KvError::Backend, + ) + .await } async fn check_write(&self, cx: &SdkCallCx) -> Result<(), KvError> { - if let Some(ref principal) = cx.principal { - 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(()) + authz::script_gate( + &*self.authz, + cx, + Capability::AppKvWrite(cx.app_id), + || KvError::Forbidden, + KvError::Backend, + ) + .await } } diff --git a/crates/manager-core/src/pubsub_service.rs b/crates/manager-core/src/pubsub_service.rs index 5f84fac..5d3ff20 100644 --- a/crates/manager-core/src/pubsub_service.rs +++ b/crates/manager-core/src/pubsub_service.rs @@ -114,20 +114,14 @@ impl PubsubServiceImpl { } async fn check_publish(&self, cx: &SdkCallCx) -> Result<(), PubsubError> { - if let Some(ref principal) = cx.principal { - match authz::require( - &*self.authz, - principal, - Capability::AppPubsubPublish(cx.app_id), - ) - .await - { - Ok(()) => {} - Err(authz::AuthzDenied::Denied) => return Err(PubsubError::Forbidden), - Err(authz::AuthzDenied::Repo(e)) => return Err(PubsubError::Backend(e.to_string())), - } - } - Ok(()) + authz::script_gate( + &*self.authz, + cx, + Capability::AppPubsubPublish(cx.app_id), + || PubsubError::Forbidden, + PubsubError::Backend, + ) + .await } } diff --git a/crates/manager-core/src/queue_service.rs b/crates/manager-core/src/queue_service.rs index 0c0dde2..9a4b6f8 100644 --- a/crates/manager-core/src/queue_service.rs +++ b/crates/manager-core/src/queue_service.rs @@ -58,19 +58,14 @@ impl QueueService for QueueServiceImpl { // Script-as-gate authz: anonymous public-HTTP scripts skip the // check (cx.principal is None); authenticated callers must hold // AppQueueEnqueue. - if let Some(principal) = cx.principal.as_ref() { - match authz::require( - &*self.authz, - principal, - Capability::AppQueueEnqueue(cx.app_id), - ) - .await - { - Ok(()) => {} - Err(authz::AuthzDenied::Denied) => return Err(QueueError::Forbidden), - Err(authz::AuthzDenied::Repo(e)) => return Err(QueueError::Backend(e.to_string())), - } - } + authz::script_gate( + &*self.authz, + cx, + Capability::AppQueueEnqueue(cx.app_id), + || QueueError::Forbidden, + QueueError::Backend, + ) + .await?; let deliver_after = opts .delay_ms