fix(12): resolve critical code review findings (CR-01 through CR-04)
- CR-01: fix browse URL from /folders/{id} to /items?parent_ref= (broken feature)
- CR-02: remove raw provider exception text from _TerminalProviderError messages
- CR-03: add user_id predicate to get_or_create_folder_state query (ownership boundary)
- CR-04: add max_length=255 to ConnectionRenameRequest.display_name
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
de63e2a3a4
commit
b6911fb4ed
@@ -9,7 +9,7 @@ from __future__ import annotations
|
|||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from typing import List, Optional
|
from typing import List, Optional
|
||||||
|
|
||||||
from pydantic import BaseModel, field_validator
|
from pydantic import BaseModel, Field, field_validator
|
||||||
|
|
||||||
|
|
||||||
# ── Capability / item schemas ─────────────────────────────────────────────────
|
# ── Capability / item schemas ─────────────────────────────────────────────────
|
||||||
@@ -75,7 +75,7 @@ class ConnectionRenameRequest(BaseModel):
|
|||||||
Only display_name is accepted — mass assignment prevention.
|
Only display_name is accepted — mass assignment prevention.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
display_name: str
|
display_name: str = Field(..., max_length=255)
|
||||||
|
|
||||||
@field_validator("display_name")
|
@field_validator("display_name")
|
||||||
@classmethod
|
@classmethod
|
||||||
|
|||||||
@@ -229,6 +229,7 @@ async def get_or_create_folder_state(
|
|||||||
uid_fs = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id))
|
uid_fs = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id))
|
||||||
result = await session.execute(
|
result = await session.execute(
|
||||||
select(CloudFolderState).where(
|
select(CloudFolderState).where(
|
||||||
|
CloudFolderState.user_id == uid_fs,
|
||||||
CloudFolderState.connection_id == cid_fs,
|
CloudFolderState.connection_id == cid_fs,
|
||||||
CloudFolderState.parent_ref == parent_ref,
|
CloudFolderState.parent_ref == parent_ref,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -107,7 +107,7 @@ async def _run(user_id: str, connection_id: str, parent_ref: str | None) -> dict
|
|||||||
error_message="Credential decryption failed. Re-connect the account.",
|
error_message="Credential decryption failed. Re-connect the account.",
|
||||||
)
|
)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
raise _TerminalProviderError(f"Credential decryption failed: {exc}") from exc
|
raise _TerminalProviderError("Credential decryption failed. Re-connect the account.") from exc
|
||||||
|
|
||||||
# Build adapter and fetch listing
|
# Build adapter and fetch listing
|
||||||
try:
|
try:
|
||||||
@@ -133,9 +133,9 @@ async def _run(user_id: str, connection_id: str, parent_ref: str | None) -> dict
|
|||||||
error_message="Authentication failed. Re-connect the account.",
|
error_message="Authentication failed. Re-connect the account.",
|
||||||
)
|
)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
raise _TerminalProviderError(f"Auth error: {exc}") from exc
|
raise _TerminalProviderError("Authentication failed. Re-connect the account.") from exc
|
||||||
# Transient failure — let Celery retry
|
# Transient failure — let Celery retry
|
||||||
raise _TransientProviderError(f"Provider error: {exc}") from exc
|
raise _TransientProviderError("Provider error. Will retry.") from exc
|
||||||
|
|
||||||
# Reconcile listing into durable cloud_items rows
|
# Reconcile listing into durable cloud_items rows
|
||||||
await reconcile_cloud_listing(
|
await reconcile_cloud_listing(
|
||||||
|
|||||||
@@ -30,11 +30,12 @@ export function updateDefaultStorage(backend) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Browse a cloud folder by connection UUID and folder path.
|
* Browse a cloud folder by connection UUID and optional parent_ref.
|
||||||
* connectionId is always a UUID; folderId is a path string (default 'root').
|
* connectionId is always a UUID; parentRef is a provider path string (default '').
|
||||||
*/
|
*/
|
||||||
export function getCloudFoldersByConnectionId(connectionId, folderId) {
|
export function getCloudFoldersByConnectionId(connectionId, parentRef = '') {
|
||||||
return request(`/api/cloud/connections/${connectionId}/folders/${folderId}`)
|
const qs = parentRef ? `?parent_ref=${encodeURIComponent(parentRef)}` : ''
|
||||||
|
return request(`/api/cloud/connections/${connectionId}/items${qs}`)
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @deprecated Use getCloudFoldersByConnectionId instead. */
|
/** @deprecated Use getCloudFoldersByConnectionId instead. */
|
||||||
|
|||||||
Reference in New Issue
Block a user