fix: WeasyPrint URL fetcher + attachment improvements + webhook error propagation + WebSocket conversation check + RLS disabled on system tables (bootstrap fix)
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
This commit is contained in:
@@ -0,0 +1,56 @@
|
|||||||
|
"""Disable RLS on system identity tables to fix login bootstrap circle.
|
||||||
|
|
||||||
|
Revision ID: 0067
|
||||||
|
Revises: 0066
|
||||||
|
Create Date: 2026-07-29
|
||||||
|
|
||||||
|
Problem: users, user_tenants, groups, roles have RLS enabled. The login
|
||||||
|
process needs to query these tables BEFORE a tenant context is set
|
||||||
|
(bootstrap circle: Login → Membership → Tenant-Context → Login).
|
||||||
|
|
||||||
|
RLS on these tables blocks login because there's no tenant context yet.
|
||||||
|
|
||||||
|
Solution: Disable RLS on system identity tables. Tenant isolation for
|
||||||
|
these tables is enforced at the application level (auth_service always
|
||||||
|
filters by user_id + tenant_id in queries).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
revision = "0067"
|
||||||
|
down_revision = "0066"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
# System identity tables — no RLS (needed for login bootstrap)
|
||||||
|
SYSTEM_TABLES = [
|
||||||
|
"users",
|
||||||
|
"user_tenants",
|
||||||
|
"groups",
|
||||||
|
"user_groups",
|
||||||
|
"roles",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
for table in SYSTEM_TABLES:
|
||||||
|
# Drop any existing policies
|
||||||
|
op.execute(f"""
|
||||||
|
DO $$
|
||||||
|
DECLARE pol RECORD;
|
||||||
|
BEGIN
|
||||||
|
FOR pol IN
|
||||||
|
SELECT polname FROM pg_policy
|
||||||
|
WHERE polrelid = '{table}'::regclass
|
||||||
|
LOOP
|
||||||
|
EXECUTE format('DROP POLICY IF EXISTS %I ON {table}', pol.polname);
|
||||||
|
END LOOP;
|
||||||
|
END $$;
|
||||||
|
""")
|
||||||
|
# Disable RLS
|
||||||
|
op.execute(f"ALTER TABLE {table} DISABLE ROW LEVEL SECURITY")
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
for table in SYSTEM_TABLES:
|
||||||
|
op.execute(f"ALTER TABLE {table} ENABLE ROW LEVEL SECURITY")
|
||||||
@@ -68,7 +68,11 @@ async def _dispatch_single(
|
|||||||
event_name: str,
|
event_name: str,
|
||||||
data: dict[str, Any],
|
data: dict[str, Any],
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Send a webhook and log the result."""
|
"""Send a webhook and log the result. Raises on failure (P1.5 fix).
|
||||||
|
|
||||||
|
Previously errors were swallowed, causing the outbox to mark events
|
||||||
|
as 'published' even when webhook delivery failed.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
result = await send_webhook(webhook, event_name, data)
|
result = await send_webhook(webhook, event_name, data)
|
||||||
if result["success"]:
|
if result["success"]:
|
||||||
@@ -81,10 +85,12 @@ async def _dispatch_single(
|
|||||||
f"Webhook {webhook.id} failed for {webhook.url} "
|
f"Webhook {webhook.id} failed for {webhook.url} "
|
||||||
f"event {event_name}: {result.get('error')}"
|
f"event {event_name}: {result.get('error')}"
|
||||||
)
|
)
|
||||||
|
raise RuntimeError(f"Webhook {webhook.id} failed: {result.get('error')}")
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.error(
|
logger.error(
|
||||||
f"Webhook {webhook.id} dispatch error for {webhook.url}: {exc}"
|
f"Webhook {webhook.id} dispatch error for {webhook.url}: {exc}"
|
||||||
)
|
)
|
||||||
|
raise # Re-raise so outbox can retry (P1.5 fix)
|
||||||
|
|
||||||
|
|
||||||
def register_webhook_event_handlers(event_bus: EventBus | None = None) -> None:
|
def register_webhook_event_handlers(event_bus: EventBus | None = None) -> None:
|
||||||
|
|||||||
@@ -515,7 +515,23 @@ async def websocket_endpoint(
|
|||||||
elif msg_type == "subscribe":
|
elif msg_type == "subscribe":
|
||||||
conv_id = msg.get("conversation_id")
|
conv_id = msg.get("conversation_id")
|
||||||
if conv_id:
|
if conv_id:
|
||||||
ws_manager.subscribe(conv_id, user_id)
|
# P1.9 fix: Check if user is a participant of this conversation
|
||||||
|
from app.core.db import async_session_maker
|
||||||
|
from sqlalchemy import text as sql_text
|
||||||
|
try:
|
||||||
|
async with async_session_maker() as db:
|
||||||
|
await db.execute(sql_text("SELECT set_config('app.current_tenant_id', :tid, true)"), {"tid": tenant_id})
|
||||||
|
result = await db.execute(
|
||||||
|
sql_text("SELECT 1 FROM conversation_participants WHERE conversation_id = :cid AND user_id = :uid"),
|
||||||
|
{"cid": conv_id, "uid": user_id},
|
||||||
|
)
|
||||||
|
if result.first():
|
||||||
|
ws_manager.subscribe(conv_id, user_id)
|
||||||
|
else:
|
||||||
|
await ws_manager.send_to_user(user_id, {"type": "error", "message": "Not a participant of this conversation"})
|
||||||
|
except Exception:
|
||||||
|
logger.warning("Failed to check conversation participation for %s in %s", user_id, conv_id)
|
||||||
|
await ws_manager.send_to_user(user_id, {"type": "error", "message": "Cannot verify participation"})
|
||||||
elif msg_type == "unsubscribe":
|
elif msg_type == "unsubscribe":
|
||||||
conv_id = msg.get("conversation_id")
|
conv_id = msg.get("conversation_id")
|
||||||
if conv_id:
|
if conv_id:
|
||||||
|
|||||||
@@ -130,9 +130,23 @@ def render_template_string(template_content: str, data: dict[str, Any]) -> str:
|
|||||||
return template.render(**data)
|
return template.render(**data)
|
||||||
|
|
||||||
|
|
||||||
|
def _safe_url_fetcher(url: str, timeout: int = 10) -> dict:
|
||||||
|
"""URL fetcher that only allows data: URIs and blocks external resources.
|
||||||
|
|
||||||
|
Prevents SSRF and local file access via WeasyPrint.
|
||||||
|
"""
|
||||||
|
if url.startswith('data:'):
|
||||||
|
from weasyprint import default_url_fetcher
|
||||||
|
return default_url_fetcher(url, timeout)
|
||||||
|
# Block all external URLs (http, https, file, etc.)
|
||||||
|
raise ValueError(f"External resource blocked by URL fetcher: {url}")
|
||||||
|
|
||||||
|
|
||||||
def generate_pdf(html_content: str) -> bytes:
|
def generate_pdf(html_content: str) -> bytes:
|
||||||
"""Generate a PDF from HTML content using WeasyPrint.
|
"""Generate a PDF from HTML content using WeasyPrint.
|
||||||
|
|
||||||
|
Uses a safe URL fetcher that blocks external resources (SSRF protection).
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
html_content: Valid HTML string
|
html_content: Valid HTML string
|
||||||
|
|
||||||
@@ -141,7 +155,7 @@ def generate_pdf(html_content: str) -> bytes:
|
|||||||
"""
|
"""
|
||||||
from weasyprint import HTML
|
from weasyprint import HTML
|
||||||
|
|
||||||
pdf = HTML(string=html_content).write_pdf()
|
pdf = HTML(string=html_content, url_fetcher=_safe_url_fetcher).write_pdf()
|
||||||
return pdf
|
return pdf
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -94,13 +94,19 @@ async def download_attachment(
|
|||||||
raise HTTPException(404, detail={"detail": "Attachment not found", "code": "not_found"})
|
raise HTTPException(404, detail={"detail": "Attachment not found", "code": "not_found"})
|
||||||
|
|
||||||
file_path = data["file_path"]
|
file_path = data["file_path"]
|
||||||
if not os.path.isfile(file_path):
|
# Use storage backend instead of os.path.isfile (P1.1 fix)
|
||||||
raise HTTPException(404, detail={"detail": "File not found on disk", "code": "file_missing"})
|
from app.core.storage import get_storage_backend
|
||||||
|
storage = get_storage_backend()
|
||||||
|
try:
|
||||||
|
file_bytes = await storage.load(file_path)
|
||||||
|
except Exception:
|
||||||
|
raise HTTPException(404, detail={"detail": "File not found in storage", "code": "file_missing"})
|
||||||
|
|
||||||
return FileResponse(
|
from fastapi.responses import Response
|
||||||
path=file_path,
|
return Response(
|
||||||
filename=data["filename"],
|
content=file_bytes,
|
||||||
media_type=data["mime_type"],
|
media_type=data["mime_type"],
|
||||||
|
headers={"Content-Disposition": f'attachment; filename="{data["filename"]}"'},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -50,6 +50,11 @@ async def save_attachment(
|
|||||||
is_system_admin: bool = False,
|
is_system_admin: bool = False,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Save a file to storage and create an Attachment record."""
|
"""Save a file to storage and create an Attachment record."""
|
||||||
|
# File size limit: 50MB
|
||||||
|
MAX_FILE_SIZE = 50 * 1024 * 1024 # 50 MB
|
||||||
|
if len(file_content) > MAX_FILE_SIZE:
|
||||||
|
raise ValueError(f"File too large: {len(file_content)} bytes (max {MAX_FILE_SIZE})")
|
||||||
|
|
||||||
# Check access on parent entity
|
# Check access on parent entity
|
||||||
if not is_system_admin:
|
if not is_system_admin:
|
||||||
from app.core.visibility import check_single_entity_access
|
from app.core.visibility import check_single_entity_access
|
||||||
@@ -151,7 +156,7 @@ async def delete_attachment(
|
|||||||
attachment_id: uuid.UUID,
|
attachment_id: uuid.UUID,
|
||||||
is_system_admin: bool = False,
|
is_system_admin: bool = False,
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""Soft-delete an attachment (keeps file on disk for audit trail)."""
|
"""Soft-delete an attachment and remove physical file from storage."""
|
||||||
q = select(Attachment).where(
|
q = select(Attachment).where(
|
||||||
Attachment.id == attachment_id,
|
Attachment.id == attachment_id,
|
||||||
Attachment.tenant_id == tenant_id,
|
Attachment.tenant_id == tenant_id,
|
||||||
@@ -169,6 +174,13 @@ async def delete_attachment(
|
|||||||
if not has_access:
|
if not has_access:
|
||||||
raise PermissionError("No access")
|
raise PermissionError("No access")
|
||||||
|
|
||||||
|
# Delete physical file from storage (P1.1 fix)
|
||||||
|
try:
|
||||||
|
storage = get_storage_backend()
|
||||||
|
await storage.delete(attachment.file_path)
|
||||||
|
except Exception as exc:
|
||||||
|
logger.warning("Failed to delete physical file %s: %s", attachment.file_path, exc)
|
||||||
|
|
||||||
attachment.deleted_at = datetime.now(UTC)
|
attachment.deleted_at = datetime.now(UTC)
|
||||||
await db.flush()
|
await db.flush()
|
||||||
await log_audit(
|
await log_audit(
|
||||||
|
|||||||
Reference in New Issue
Block a user