fix(shared): F-Q-004 unify error variants — Forbidden on QueueError, rename Unavailable→Backend

Brings QueueError / PubsubError / InvokeError in line with sibling
shape used by KvError / DocsError / FilesError / SecretsError:

- QueueError gains an explicit Forbidden variant (previously authz
  denial was squashed into Rejected("forbidden") in queue_service.rs:68,
  losing the structured variant a 403-translation layer would need).
- QueueError::Unavailable renamed → Backend.
- PubsubError::Unavailable renamed → Backend.
- InvokeError::Unavailable renamed → Backend.
- Call sites updated (queue_service, pubsub_service, invoke_service,
  Noop* stubs, From<PubsubRepoError> impl, one test assertion).
- New unit test verifying authed-denied → QueueError::Forbidden.

AUDIT.md anchor: F-Q-004.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-07 19:44:23 +02:00
parent 450badaabf
commit b192cf2cc9
6 changed files with 78 additions and 25 deletions

View File

@@ -49,7 +49,7 @@ impl InvokeServiceImpl {
.scripts .scripts
.get(script_id) .get(script_id)
.await .await
.map_err(|e| InvokeError::Unavailable(e.to_string()))? .map_err(|e| InvokeError::Backend(e.to_string()))?
.ok_or_else(|| InvokeError::NotFound(format!("id {script_id}")))?; .ok_or_else(|| InvokeError::NotFound(format!("id {script_id}")))?;
if script.app_id != cx.app_id { if script.app_id != cx.app_id {
return Err(InvokeError::CrossApp); return Err(InvokeError::CrossApp);
@@ -72,7 +72,7 @@ impl InvokeServiceImpl {
.scripts .scripts
.get_by_name(cx.app_id, name) .get_by_name(cx.app_id, name)
.await .await
.map_err(|e| InvokeError::Unavailable(e.to_string()))? .map_err(|e| InvokeError::Backend(e.to_string()))?
.ok_or_else(|| InvokeError::NotFound(format!("name {name:?}")))?; .ok_or_else(|| InvokeError::NotFound(format!("name {name:?}")))?;
Ok(ResolvedScript { Ok(ResolvedScript {
script_id: script.id, script_id: script.id,
@@ -156,7 +156,7 @@ impl InvokeService for InvokeServiceImpl {
root_execution_id: Some(cx.root_execution_id), root_execution_id: Some(cx.root_execution_id),
}) })
.await .await
.map_err(|e| InvokeError::Unavailable(e.to_string()))?; .map_err(|e| InvokeError::Backend(e.to_string()))?;
Ok(execution_id) Ok(execution_id)
} }
} }

View File

@@ -129,7 +129,7 @@ impl PubsubServiceImpl {
impl From<PubsubRepoError> for PubsubError { impl From<PubsubRepoError> for PubsubError {
fn from(e: PubsubRepoError) -> Self { fn from(e: PubsubRepoError) -> Self {
Self::Unavailable(e.to_string()) Self::Backend(e.to_string())
} }
} }
@@ -227,7 +227,7 @@ impl PubsubService for PubsubServiceImpl {
let (Some(topic_repo), Some(secrets)) = (self.topics.as_ref(), self.secrets.as_ref()) let (Some(topic_repo), Some(secrets)) = (self.topics.as_ref(), self.secrets.as_ref())
else { else {
return Err(PubsubError::Unavailable( return Err(PubsubError::Backend(
"subscriber tokens are not wired in".into(), "subscriber tokens are not wired in".into(),
)); ));
}; };
@@ -253,7 +253,7 @@ impl PubsubService for PubsubServiceImpl {
let registered = topic_repo let registered = topic_repo
.get(cx.app_id, name) .get(cx.app_id, name)
.await .await
.map_err(|e| PubsubError::Unavailable(e.to_string()))?; .map_err(|e| PubsubError::Backend(e.to_string()))?;
if !registered.is_some_and(|t| t.external_subscribable) { if !registered.is_some_and(|t| t.external_subscribable) {
return Err(PubsubError::SubscriberToken(format!( return Err(PubsubError::SubscriberToken(format!(
"pubsub::subscriber_token: topic {name} is not externally subscribable" "pubsub::subscriber_token: topic {name} is not externally subscribable"
@@ -264,7 +264,7 @@ impl PubsubService for PubsubServiceImpl {
let key = secrets let key = secrets
.get_or_create_signing_key(cx.app_id) .get_or_create_signing_key(cx.app_id)
.await .await
.map_err(|e| PubsubError::Unavailable(e.to_string()))?; .map_err(|e| PubsubError::Backend(e.to_string()))?;
let now = chrono::Utc::now().timestamp(); let now = chrono::Utc::now().timestamp();
let claims = TokenClaims { let claims = TokenClaims {
app_id: cx.app_id, app_id: cx.app_id,
@@ -472,7 +472,7 @@ mod tests {
.publish_durable(&anon_cx(app), "user.created", serde_json::json!(1)) .publish_durable(&anon_cx(app), "user.created", serde_json::json!(1))
.await .await
.unwrap_err(); .unwrap_err();
assert!(matches!(err, PubsubError::Unavailable(_))); assert!(matches!(err, PubsubError::Backend(_)));
// Rollback: no partial fan-out survived. // Rollback: no partial fan-out survived.
assert_eq!(repo.written_count(), 0); assert_eq!(repo.written_count(), 0);
} }

View File

@@ -65,7 +65,7 @@ impl QueueService for QueueServiceImpl {
Capability::AppQueueEnqueue(cx.app_id), Capability::AppQueueEnqueue(cx.app_id),
) )
.await .await
.map_err(|_| QueueError::Rejected("forbidden".into()))?; .map_err(|_| QueueError::Forbidden)?;
} }
let deliver_after = opts let deliver_after = opts
@@ -88,7 +88,7 @@ impl QueueService for QueueServiceImpl {
enqueued_by_principal, enqueued_by_principal,
}) })
.await .await
.map_err(|e| QueueError::Unavailable(e.to_string())) .map_err(|e| QueueError::Backend(e.to_string()))
} }
async fn depth(&self, cx: &SdkCallCx, queue_name: &str) -> Result<u64, QueueError> { async fn depth(&self, cx: &SdkCallCx, queue_name: &str) -> Result<u64, QueueError> {
@@ -98,7 +98,7 @@ impl QueueService for QueueServiceImpl {
self.repo self.repo
.depth(cx.app_id, queue_name) .depth(cx.app_id, queue_name)
.await .await
.map_err(|e| QueueError::Unavailable(e.to_string())) .map_err(|e| QueueError::Backend(e.to_string()))
} }
async fn depth_pending(&self, cx: &SdkCallCx, queue_name: &str) -> Result<u64, QueueError> { async fn depth_pending(&self, cx: &SdkCallCx, queue_name: &str) -> Result<u64, QueueError> {
@@ -108,7 +108,7 @@ impl QueueService for QueueServiceImpl {
self.repo self.repo
.depth_pending(cx.app_id, queue_name) .depth_pending(cx.app_id, queue_name)
.await .await
.map_err(|e| QueueError::Unavailable(e.to_string())) .map_err(|e| QueueError::Backend(e.to_string()))
} }
} }
@@ -312,6 +312,49 @@ mod tests {
assert!(captured.deliver_after.is_none()); assert!(captured.deliver_after.is_none());
} }
struct DenyAllAuthz;
#[async_trait]
impl AuthzRepo for DenyAllAuthz {
async fn membership(
&self,
_user_id: picloud_shared::UserId,
_app_id: AppId,
) -> Result<Option<AppRole>, AuthzError> {
Ok(None)
}
}
#[tokio::test]
async fn authed_principal_denied_returns_forbidden_variant() {
use picloud_shared::{InstanceRole, Principal, UserId};
let repo = Arc::new(CapturingRepo {
last: tokio::sync::Mutex::new(None),
});
let svc = QueueServiceImpl::new(repo, Arc::new(DenyAllAuthz));
let exec = ExecutionId::new();
let cx = SdkCallCx {
app_id: AppId::new(),
script_id: ScriptId::new(),
principal: Some(Principal {
user_id: UserId::new(),
instance_role: InstanceRole::Member,
scopes: None,
app_binding: None,
}),
execution_id: exec,
request_id: RequestId::new(),
trigger_depth: 0,
root_execution_id: exec,
is_dead_letter_handler: false,
event: None,
};
let err = svc
.enqueue(&cx, "x", serde_json::Value::Null, EnqueueOpts::default())
.await
.unwrap_err();
assert!(matches!(err, QueueError::Forbidden));
}
#[tokio::test] #[tokio::test]
async fn depth_passes_through() { async fn depth_passes_through() {
let repo = Arc::new(CapturingRepo { let repo = Arc::new(CapturingRepo {

View File

@@ -105,9 +105,10 @@ pub enum InvokeError {
#[error("invoke rejected: {0}")] #[error("invoke rejected: {0}")]
Rejected(String), Rejected(String),
/// Backend / database error. /// Backend / database error. Named `Backend` to align with KvError /
/// DocsError / FilesError / SecretsError / QueueError / PubsubError.
#[error("invoke backend error: {0}")] #[error("invoke backend error: {0}")]
Unavailable(String), Backend(String),
} }
/// Test-only stub: every call errors. Lets harnesses build a `Services` /// Test-only stub: every call errors. Lets harnesses build a `Services`
@@ -122,7 +123,7 @@ impl InvokeService for NoopInvokeService {
_cx: &SdkCallCx, _cx: &SdkCallCx,
_target: InvokeTarget, _target: InvokeTarget,
) -> Result<ResolvedScript, InvokeError> { ) -> Result<ResolvedScript, InvokeError> {
Err(InvokeError::Unavailable("invoke is not wired in".into())) Err(InvokeError::Backend("invoke is not wired in".into()))
} }
async fn enqueue_async( async fn enqueue_async(
@@ -131,6 +132,6 @@ impl InvokeService for NoopInvokeService {
_target: InvokeTarget, _target: InvokeTarget,
_args: serde_json::Value, _args: serde_json::Value,
) -> Result<ExecutionId, InvokeError> { ) -> Result<ExecutionId, InvokeError> {
Err(InvokeError::Unavailable("invoke is not wired in".into())) Err(InvokeError::Backend("invoke is not wired in".into()))
} }
} }

View File

@@ -52,7 +52,7 @@ pub trait PubsubService: Send + Sync {
ttl_seconds: Option<i64>, ttl_seconds: Option<i64>,
) -> Result<String, PubsubError> { ) -> Result<String, PubsubError> {
let _ = (cx, topics, ttl_seconds); let _ = (cx, topics, ttl_seconds);
Err(PubsubError::Unavailable( Err(PubsubError::Backend(
"subscriber tokens are not wired in".into(), "subscriber tokens are not wired in".into(),
)) ))
} }
@@ -80,9 +80,10 @@ pub enum PubsubError {
#[error("{0}")] #[error("{0}")]
SubscriberToken(String), SubscriberToken(String),
/// Anything else — Postgres unavailable, etc. /// Anything else — Postgres unavailable, etc. Named `Backend` to
/// align with KvError / DocsError / FilesError / SecretsError.
#[error("pubsub backend error: {0}")] #[error("pubsub backend error: {0}")]
Unavailable(String), Backend(String),
} }
/// Match a stored `topic_pattern` against a published `topic`. /// Match a stored `topic_pattern` against a published `topic`.
@@ -145,7 +146,7 @@ impl PubsubService for NoopPubsubService {
_topic: &str, _topic: &str,
_message: serde_json::Value, _message: serde_json::Value,
) -> Result<(), PubsubError> { ) -> Result<(), PubsubError> {
Err(PubsubError::Unavailable("pubsub is not wired in".into())) Err(PubsubError::Backend("pubsub is not wired in".into()))
} }
} }

View File

@@ -61,6 +61,13 @@ pub enum QueueError {
#[error("queue_name must not be empty")] #[error("queue_name must not be empty")]
EmptyName, EmptyName,
/// Caller principal lacked the required capability. Only raised when
/// `cx.principal.is_some()` (script-as-gate; public HTTP skips it).
/// Sibling shape across KvError / DocsError / PubsubError — uniform
/// for 403 translation at the HTTP boundary.
#[error("forbidden")]
Forbidden,
/// Payload could not be serialized (e.g. a Rhai FnPtr / closure /// Payload could not be serialized (e.g. a Rhai FnPtr / closure
/// captured into the message). Rejected at the SDK boundary; surface /// captured into the message). Rejected at the SDK boundary; surface
/// the wording verbatim so scripts see the documented message. /// the wording verbatim so scripts see the documented message.
@@ -71,9 +78,10 @@ pub enum QueueError {
#[error("{0}")] #[error("{0}")]
InvalidOpts(String), InvalidOpts(String),
/// Backend unavailable (Postgres down, etc.). /// Backend failure (Postgres unavailable, etc.). Named `Backend` to
/// align with KvError / DocsError / FilesError / SecretsError.
#[error("queue backend error: {0}")] #[error("queue backend error: {0}")]
Unavailable(String), Backend(String),
} }
/// Test-only stub: every call errors `Unavailable`. Used by harnesses /// Test-only stub: every call errors `Unavailable`. Used by harnesses
@@ -90,14 +98,14 @@ impl QueueService for NoopQueueService {
_payload: serde_json::Value, _payload: serde_json::Value,
_opts: EnqueueOpts, _opts: EnqueueOpts,
) -> Result<QueueMessageId, QueueError> { ) -> Result<QueueMessageId, QueueError> {
Err(QueueError::Unavailable("queue is not wired in".into())) Err(QueueError::Backend("queue is not wired in".into()))
} }
async fn depth(&self, _cx: &SdkCallCx, _queue_name: &str) -> Result<u64, QueueError> { async fn depth(&self, _cx: &SdkCallCx, _queue_name: &str) -> Result<u64, QueueError> {
Err(QueueError::Unavailable("queue is not wired in".into())) Err(QueueError::Backend("queue is not wired in".into()))
} }
async fn depth_pending(&self, _cx: &SdkCallCx, _queue_name: &str) -> Result<u64, QueueError> { async fn depth_pending(&self, _cx: &SdkCallCx, _queue_name: &str) -> Result<u64, QueueError> {
Err(QueueError::Unavailable("queue is not wired in".into())) Err(QueueError::Backend("queue is not wired in".into()))
} }
} }