Merge branch 'main' into claude/performance-optimization-round-6
Resolves the one conflict in file_blob_read_repository.rs's suggest_files_by_name: main added the CALLER_CAN_READ_DRIVE authz scope (caller_id param + drive-membership filter, AuthZ audit finding #1 — the suggest query previously leaked names/paths across tenants), round 6 switched the same query's id/folder_id columns to binary UUID decode. Kept both: main's authz structure (format! + CALLER_CAN_READ_DRIVE + caller_id bind) with round 6's binary decode (fi.id / fi.folder_id, no ::text) so the query matches the FileRow = (Uuid, …) tuple. The deliberately-text sites (min(fm.file_id::text), folder path lookup) stay text. Verified: build + clippy -D warnings clean, 524 unit + 554 integration tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017aJu9ghvuT8WqC31ZEGTBA
This commit is contained in:
@@ -16,6 +16,28 @@ use crate::domain::services::authorization::{
|
||||
ResourceKind, Role, Subject,
|
||||
};
|
||||
|
||||
/// Discriminates the two denial shapes surfaced by
|
||||
/// [`AuthorizationEngine::require_visible`] in the `authz.denied` audit line.
|
||||
/// Log-aggregation consumers key off the string form via `as_str`; keep the
|
||||
/// values stable — a new denial shape means a new variant, never a renamed
|
||||
/// existing one.
|
||||
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
|
||||
pub enum AuthzDenialVisibility {
|
||||
/// Caller has `Read` on the resource — 403 Forbidden.
|
||||
Visible,
|
||||
/// Caller has no `Read` — 404 anti-enum.
|
||||
Hidden,
|
||||
}
|
||||
|
||||
impl AuthzDenialVisibility {
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Visible => "visible",
|
||||
Self::Hidden => "hidden",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub trait AuthorizationEngine: Send + Sync + 'static {
|
||||
/// Returns true if `subject` has `permission` on `resource`, considering
|
||||
/// owner short-circuit AND cascading from folder ancestors.
|
||||
@@ -53,9 +75,28 @@ pub trait AuthorizationEngine: Send + Sync + 'static {
|
||||
Ok(allowed)
|
||||
}
|
||||
|
||||
/// Convenience wrapper around `check`: returns `Ok(())` when allowed and
|
||||
/// `DomainError::not_found` when denied (anti-enumeration — same error as
|
||||
/// "resource doesn't exist" so attackers can't probe IDs by error shape).
|
||||
/// Graduated-denial wrapper around `check`. Semantics:
|
||||
///
|
||||
/// - `permission` granted → `Ok(())`
|
||||
/// - `permission` denied, `Read` also denied → `DomainError::not_found`
|
||||
/// (404, anti-enumeration — same shape as "doesn't exist" so a probing
|
||||
/// caller can't distinguish "wrong id" from "no access")
|
||||
/// - `permission` denied, `Read` granted → `DomainError::access_denied`
|
||||
/// (403 — the caller can already see the resource, so hiding existence
|
||||
/// leaks nothing new; a clear 403 beats a confusing 404 for UX and for
|
||||
/// API-first clients like rclone)
|
||||
///
|
||||
/// Special case: when `permission == Read`, the visibility gate collapses
|
||||
/// onto itself — a `Read` denial IS a "hidden" outcome by definition, so
|
||||
/// the method short-circuits to the strict anti-enum 404 without a second
|
||||
/// DB round-trip. That's why there's only one method: strict Read-denial
|
||||
/// and graduated write-denial fall out of the same signature.
|
||||
///
|
||||
/// Do NOT use this in search / enumeration paths where existence itself is
|
||||
/// the attack vector — those must filter at the SQL/index layer, never
|
||||
/// touch this method with per-row ids. Cross-tenant probes on ids the
|
||||
/// caller has no prior read handle for degrade to the 404 shape naturally
|
||||
/// (Read denied → `Hidden`).
|
||||
async fn require(
|
||||
&self,
|
||||
subject: Subject,
|
||||
@@ -80,39 +121,68 @@ pub trait AuthorizationEngine: Send + Sync + 'static {
|
||||
permission,
|
||||
resource
|
||||
);
|
||||
Ok(())
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Visibility probe. Short-circuit: when the target permission IS
|
||||
// `Read` and the check above returned false, we already know Read is
|
||||
// denied — visibility is `Hidden` by definition, no second DB hop.
|
||||
// Otherwise probe Read; a DB-hop failure here degrades to `Hidden` so
|
||||
// the caller sees the strict anti-enum shape (safe default).
|
||||
let visibility = if permission == Permission::Read {
|
||||
AuthzDenialVisibility::Hidden
|
||||
} else if self
|
||||
.check(subject, Permission::Read, resource)
|
||||
.await
|
||||
.unwrap_or(false)
|
||||
{
|
||||
AuthzDenialVisibility::Visible
|
||||
} else {
|
||||
let (kind, id) = match resource {
|
||||
Resource::Folder(id) => ("Folder", id),
|
||||
Resource::File(id) => ("File", id),
|
||||
Resource::Drive(id) => ("Drive", id),
|
||||
Resource::Calendar(id) => ("Calendar", id),
|
||||
Resource::AddressBook(id) => ("AddressBook", id),
|
||||
Resource::Playlist(id) => ("Playlist", id),
|
||||
};
|
||||
// Audit-worthy: denials are the interesting signal. Routed
|
||||
// through the `audit` tracing target so log aggregators can
|
||||
// surface them separately from operational debug traffic.
|
||||
// Span context (request_id, client_ip, user_id) is attached
|
||||
// automatically by the request-scope span set in
|
||||
// `interfaces/middleware/trace_span.rs`, so this log line
|
||||
// doesn't need to duplicate those fields — they appear in
|
||||
// the structured output of every log written inside the
|
||||
// request span.
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "authz.denied",
|
||||
subject_type = subject.type_str(),
|
||||
subject_id = %subject.id(),
|
||||
permission = permission.as_str(),
|
||||
resource_type = resource.type_str(),
|
||||
resource_id = %resource.id(),
|
||||
"👮🏻♂️ perms: ⛔ Subject '{}' hasn't permission to '{}' on resource '{}'",
|
||||
subject,
|
||||
permission,
|
||||
resource
|
||||
);
|
||||
Err(DomainError::not_found(kind, id.to_string()))
|
||||
AuthzDenialVisibility::Hidden
|
||||
};
|
||||
|
||||
let (kind, id) = match resource {
|
||||
Resource::Folder(id) => ("Folder", id),
|
||||
Resource::File(id) => ("File", id),
|
||||
Resource::Drive(id) => ("Drive", id),
|
||||
Resource::Calendar(id) => ("Calendar", id),
|
||||
Resource::AddressBook(id) => ("AddressBook", id),
|
||||
Resource::Playlist(id) => ("Playlist", id),
|
||||
};
|
||||
|
||||
// Audit-worthy: denials are the interesting signal. Routed through
|
||||
// the `audit` tracing target so log aggregators can surface them
|
||||
// separately from operational debug traffic. Span context
|
||||
// (request_id, client_ip, user_id) comes from the request-scope
|
||||
// span set in `interfaces/middleware/trace_span.rs`, so this line
|
||||
// doesn't need to duplicate those fields.
|
||||
//
|
||||
// The `visibility` field discriminates the two denial shapes for
|
||||
// operators grepping exists-but-denied vs fully-hidden. `visible`
|
||||
// denials are the ones surfaced to the caller as 403 (and safe to
|
||||
// detail in the UI); `hidden` denials are the 404 anti-enum path.
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "authz.denied",
|
||||
visibility = visibility.as_str(),
|
||||
subject_type = subject.type_str(),
|
||||
subject_id = %subject.id(),
|
||||
permission = permission.as_str(),
|
||||
resource_type = resource.type_str(),
|
||||
resource_id = %resource.id(),
|
||||
"👮🏻♂️ perms: ⛔ Subject '{}' hasn't permission to '{}' on resource '{}' (visibility={})",
|
||||
subject,
|
||||
permission,
|
||||
resource,
|
||||
visibility.as_str()
|
||||
);
|
||||
|
||||
match visibility {
|
||||
AuthzDenialVisibility::Visible => Err(DomainError::access_denied(
|
||||
kind,
|
||||
format!("Missing '{}' permission on {} {}", permission, kind, id),
|
||||
)),
|
||||
AuthzDenialVisibility::Hidden => Err(DomainError::not_found(kind, id.to_string())),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -27,11 +27,15 @@ pub trait SearchUseCase: Send + Sync + 'static {
|
||||
) -> Result<Arc<SearchResultsDto>, DomainError>;
|
||||
|
||||
/// Returns quick suggestions for autocomplete (lightweight, fast).
|
||||
/// `caller_id` scopes results to drives the caller can Read — without
|
||||
/// it the endpoint leaks names + paths across every tenant on the
|
||||
/// instance (AuthZ audit finding #1, 2026-07-12).
|
||||
async fn suggest(
|
||||
&self,
|
||||
query: &str,
|
||||
folder_id: Option<&str>,
|
||||
limit: usize,
|
||||
caller_id: Uuid,
|
||||
) -> Result<SearchSuggestionsDto, DomainError>;
|
||||
|
||||
/// Clears the search results cache.
|
||||
|
||||
@@ -205,13 +205,21 @@ pub trait FileReadPort: Send + Sync + 'static {
|
||||
/// Results are ordered by relevance (exact > starts-with > contains) so the
|
||||
/// caller can use them directly for autocomplete suggestions.
|
||||
///
|
||||
/// The default implementation falls back to `list_files` + in-memory filter
|
||||
/// so that stubs and mocks compile without changes.
|
||||
/// `caller_id` scopes results to files whose owning drive the caller can
|
||||
/// Read (direct or group-mediated `role_grants`). Without it the endpoint
|
||||
/// leaks names + paths across every tenant on the instance — closed as
|
||||
/// AuthZ audit finding #1 (2026-07-12).
|
||||
///
|
||||
/// The default implementation falls back to `list_files` + in-memory
|
||||
/// filter so that stubs and mocks compile without changes. Stub-mode
|
||||
/// callers already operate against a single tenant's data, so ignoring
|
||||
/// `caller_id` here is safe; the PG impl enforces the real scope.
|
||||
async fn suggest_files_by_name(
|
||||
&self,
|
||||
folder_id: Option<&str>,
|
||||
query: &str,
|
||||
limit: usize,
|
||||
_caller_id: Uuid,
|
||||
) -> Result<Vec<File>, DomainError> {
|
||||
let all = self.list_files(folder_id).await?;
|
||||
let q = query.to_lowercase();
|
||||
|
||||
@@ -497,20 +497,28 @@ impl SearchService {
|
||||
/// Quick suggestions search — returns up to `limit` name suggestions
|
||||
/// matching the query. Pushes filtering, relevance sort and LIMIT to SQL
|
||||
/// so only a handful of rows cross the DB→app boundary.
|
||||
pub async fn suggest(
|
||||
///
|
||||
/// `caller_id` scopes the underlying repo queries to drives the caller
|
||||
/// can Read. Without it (the pre-fix shape) any authenticated user —
|
||||
/// including external magic-link recipients — could autocomplete both
|
||||
/// names and full paths across every tenant on the instance (AuthZ
|
||||
/// audit finding #1, 2026-07-12). Named `_with_perms` per the
|
||||
/// AGENTS.md AuthZ convention.
|
||||
pub async fn suggest_with_perms(
|
||||
&self,
|
||||
query: &str,
|
||||
folder_id: Option<&str>,
|
||||
limit: usize,
|
||||
caller_id: Uuid,
|
||||
) -> Result<SearchSuggestionsDto> {
|
||||
let start = Instant::now();
|
||||
|
||||
// Ask SQL for at most `limit` best-matching files and folders
|
||||
let (files, folders) = tokio::join!(
|
||||
self.file_repository
|
||||
.suggest_files_by_name(folder_id, query, limit),
|
||||
.suggest_files_by_name(folder_id, query, limit, caller_id),
|
||||
self.folder_repository
|
||||
.suggest_folders_by_name(folder_id, query, limit),
|
||||
.suggest_folders_by_name(folder_id, query, limit, caller_id),
|
||||
);
|
||||
let files = files?;
|
||||
let folders = folders?;
|
||||
@@ -802,14 +810,20 @@ impl SearchUseCase for SearchService {
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns quick suggestions for autocomplete.
|
||||
/// Returns quick suggestions for autocomplete. Delegates to the
|
||||
/// inherent `suggest_with_perms` — the trait method is preserved as
|
||||
/// the polymorphic entry point (e.g. for `StubSearchUseCase` in
|
||||
/// tests); production callers can equivalently call the inherent
|
||||
/// method directly.
|
||||
async fn suggest(
|
||||
&self,
|
||||
query: &str,
|
||||
folder_id: Option<&str>,
|
||||
limit: usize,
|
||||
caller_id: Uuid,
|
||||
) -> Result<SearchSuggestionsDto> {
|
||||
self.suggest(query, folder_id, limit).await
|
||||
self.suggest_with_perms(query, folder_id, limit, caller_id)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Clears the search results cache.
|
||||
@@ -841,6 +855,7 @@ impl SearchService {
|
||||
_query: &str,
|
||||
_folder_id: Option<&str>,
|
||||
_limit: usize,
|
||||
_caller_id: Uuid,
|
||||
) -> Result<SearchSuggestionsDto> {
|
||||
Ok(SearchSuggestionsDto {
|
||||
suggestions: Vec::new(),
|
||||
|
||||
Reference in New Issue
Block a user