From 3d08f5fe829e5111d6bd3e641e5272ad6ad74ccb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 17:42:12 +0900 Subject: [PATCH 01/18] fix(email): enforce shared send throttling --- .../versions/0018_email_send_rate_buckets.py | 44 +++++ backend/api/emails.py | 47 ++--- backend/db/models.py | 27 +++ backend/services/email_send_rate_limiter.py | 179 ++++++++++++++++++ backend/tests/test_email_send_rate_limiter.py | 138 ++++++++++++++ backend/tests/test_emails_api.py | 63 +++++- 6 files changed, 463 insertions(+), 35 deletions(-) create mode 100644 backend/alembic/versions/0018_email_send_rate_buckets.py create mode 100644 backend/services/email_send_rate_limiter.py create mode 100644 backend/tests/test_email_send_rate_limiter.py diff --git a/backend/alembic/versions/0018_email_send_rate_buckets.py b/backend/alembic/versions/0018_email_send_rate_buckets.py new file mode 100644 index 000000000..c7bace207 --- /dev/null +++ b/backend/alembic/versions/0018_email_send_rate_buckets.py @@ -0,0 +1,44 @@ +"""add shared email send rate-limit buckets + +Revision ID: 0018_email_send_rate_buckets +Revises: 0017_merge_newsdom_carddav_heads +Create Date: 2026-08-19 00:00:00.000000 +""" + +from alembic import op +import sqlalchemy as sa + +revision = "0018_email_send_rate_buckets" +down_revision = "0017_merge_newsdom_carddav_heads" +branch_labels = None +depends_on = None + +_BUCKET_TABLE = "email_send_rate_buckets" +_EXPIRY_INDEX = "ix_email_send_rate_buckets_expires_at" + + +def upgrade() -> None: + connection = op.get_bind() + inspector = sa.inspect(connection) + if not inspector.has_table(_BUCKET_TABLE): + op.create_table( + _BUCKET_TABLE, + sa.Column("bucket_scope_hash", sa.String(length=64), nullable=False), + sa.Column("window_started_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("attempt_count", sa.Integer(), nullable=False), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.PrimaryKeyConstraint("bucket_scope_hash"), + ) + op.create_index( + _EXPIRY_INDEX, + _BUCKET_TABLE, + ["expires_at"], + if_not_exists=True, + ) + + +def downgrade() -> None: + op.drop_index(_EXPIRY_INDEX, table_name=_BUCKET_TABLE, if_exists=True) + op.drop_table(_BUCKET_TABLE, if_exists=True) diff --git a/backend/api/emails.py b/backend/api/emails.py index 2b0a9dbd6..2e92b20ee 100644 --- a/backend/api/emails.py +++ b/backend/api/emails.py @@ -1,5 +1,4 @@ from collections import defaultdict -from threading import Lock from fastapi import APIRouter, Depends, File, HTTPException, Query, UploadFile from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy import func, or_, select @@ -7,7 +6,6 @@ from db.models import Email from pydantic import BaseModel, EmailStr, Field, field_validator import datetime -import time from typing import Literal from services.email_client import ( EmailMessageParams, @@ -39,6 +37,10 @@ canonical_email_import_upload_filename, import_email_uploads, ) +from services.email_send_rate_limiter import ( + EmailSendRateLimitUnavailable, + enforce_send_email_rate_limit, +) from services.llm_provider_selection import resolve_runtime_llm_provider from services.text_safety import strip_html_markup import logging @@ -50,34 +52,6 @@ router = APIRouter(prefix="/api/emails") -_SEND_EMAIL_RATE_LIMIT_MAX_ATTEMPTS = 10 -_SEND_EMAIL_RATE_LIMIT_WINDOW_SECONDS = 60.0 -_email_send_attempts_by_scope: dict[tuple[str | None, str], list[float]] = {} -_email_send_rate_limit_lock = Lock() - - -def _enforce_send_email_rate_limit(auth_context: AuthContext) -> None: - now = time.monotonic() - cutoff = now - _SEND_EMAIL_RATE_LIMIT_WINDOW_SECONDS - key = (auth_context.organization_id, auth_context.user_id) - - # ponytail: process-local throttle; move to Redis when multi-worker send volume matters. - with _email_send_rate_limit_lock: - attempts = [ - attempt - for attempt in _email_send_attempts_by_scope.get(key, []) - if attempt > cutoff - ] - if len(attempts) >= _SEND_EMAIL_RATE_LIMIT_MAX_ATTEMPTS: - _email_send_attempts_by_scope[key] = attempts - raise HTTPException( - status_code=429, - detail="Email send rate limit exceeded", - ) - attempts.append(now) - _email_send_attempts_by_scope[key] = attempts - - def canonical_thread_key(email: Email) -> str: return ( normalize_message_id(email.thread_id) @@ -756,7 +730,18 @@ async def send_email_endpoint( in_reply_to=request.in_reply_to, references=request.references, ) - _enforce_send_email_rate_limit(auth_context) + try: + rate_limit_decision = await enforce_send_email_rate_limit(db, auth_context) + except EmailSendRateLimitUnavailable as exc: + raise HTTPException( + status_code=503, + detail="Email send rate limiter unavailable", + ) from exc + if not rate_limit_decision.allowed: + raise HTTPException( + status_code=429, + detail="Email send rate limit exceeded", + ) smtp_config = SmtpConfig( smtp_server=smtp_server, smtp_port=smtp_port, diff --git a/backend/db/models.py b/backend/db/models.py index 98e17eef2..7380defb8 100644 --- a/backend/db/models.py +++ b/backend/db/models.py @@ -823,6 +823,33 @@ def owner_filters(cls, user_id: str, organization_id: str | None): ) +class EmailSendRateBucket(Base): + __tablename__ = "email_send_rate_buckets" + + bucket_scope_hash: Mapped[str] = mapped_column(String(64), primary_key=True) + window_started_at: Mapped[datetime.datetime] = mapped_column( + DateTime(timezone=True), nullable=False + ) + attempt_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + expires_at: Mapped[datetime.datetime] = mapped_column( + DateTime(timezone=True), nullable=False + ) + created_at: Mapped[datetime.datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.datetime.now(datetime.timezone.utc), + nullable=False, + ) + updated_at: Mapped[datetime.datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.datetime.now(datetime.timezone.utc), + nullable=False, + ) + + __table_args__ = ( + Index("ix_email_send_rate_buckets_expires_at", "expires_at"), + ) + + class TicketTask(Base): __tablename__ = "ticket_tasks" diff --git a/backend/services/email_send_rate_limiter.py b/backend/services/email_send_rate_limiter.py new file mode 100644 index 000000000..05b670497 --- /dev/null +++ b/backend/services/email_send_rate_limiter.py @@ -0,0 +1,179 @@ +"""Shared, fail-closed email send throttling.""" + +from __future__ import annotations + +import datetime +import hashlib +import logging +from dataclasses import dataclass +from typing import TYPE_CHECKING, Literal + +from sqlalchemy import bindparam, func, select +from sqlalchemy.ext.asyncio import AsyncSession + +from db.models import EmailSendRateBucket, SecurityAuditEvent + +if TYPE_CHECKING: + from api.auth import AuthContext + +logger = logging.getLogger(__name__) + +SEND_RATE_LIMIT_MAX_ATTEMPTS = 10 +SEND_RATE_LIMIT_WINDOW_SECONDS = 60 +SEND_RATE_LIMIT_NAMESPACE = "naruon-email-send-rate-limit" + + +class EmailSendRateLimitUnavailable(RuntimeError): + """The shared rate-limit state cannot provide a trustworthy decision.""" + + +@dataclass(frozen=True) +class EmailSendRateLimitDecision: + """A non-sensitive rate-limit decision returned to the send endpoint.""" + + allowed: bool + reason: Literal["allowed", "quota_exhausted"] + + +def rate_limit_scope_hash(user_id: str, organization_id: str | None) -> str: + """Return a non-reversible identifier for one authorized send scope.""" + organization_scope = organization_id or "" + value = f"{SEND_RATE_LIMIT_NAMESPACE}\0{organization_scope}\0{user_id}" + return hashlib.sha256(value.encode("utf-8")).hexdigest() + + +def _lock_key(scope_hash: str) -> int: + return int.from_bytes(bytes.fromhex(scope_hash[:16]), byteorder="big", signed=True) + + +def _utc_now() -> datetime.datetime: + return datetime.datetime.now(datetime.timezone.utc) + + +def _next_bucket_state( + bucket: EmailSendRateBucket | None, + now: datetime.datetime, +) -> tuple[bool, int, datetime.datetime, datetime.datetime]: + """Calculate the next state without storing individual attempts.""" + expires_at = now + datetime.timedelta(seconds=SEND_RATE_LIMIT_WINDOW_SECONDS) + if bucket is None or now >= bucket.expires_at: + return True, 1, now, expires_at + if bucket.attempt_count >= SEND_RATE_LIMIT_MAX_ATTEMPTS: + return False, bucket.attempt_count, bucket.window_started_at, bucket.expires_at + return ( + True, + bucket.attempt_count + 1, + bucket.window_started_at, + bucket.expires_at, + ) + + +def _session_uses_postgresql(session: AsyncSession) -> bool: + try: + bind = session.get_bind() + except Exception: + return False + return getattr(getattr(bind, "dialect", None), "name", None) == "postgresql" + + +def _audit_event( + auth_context: AuthContext, + *, + scope_hash: str, + decision: EmailSendRateLimitDecision, +) -> SecurityAuditEvent: + return SecurityAuditEvent( + actor_user_id=auth_context.user_id, + actor_role=auth_context.role, + organization_id=auth_context.organization_id, + workspace_id=auth_context.workspace_id, + event_action=f"email_send_rate_limit.{decision.reason}", + resource_type="email_send_rate_limit", + resource_uid=f"email_send_scope:{scope_hash}", + evidence_source="services.email_send_rate_limiter", + detail_text=( + f"decision={decision.reason};" + f"window_seconds={SEND_RATE_LIMIT_WINDOW_SECONDS};" + f"max_attempts={SEND_RATE_LIMIT_MAX_ATTEMPTS}" + ), + ) + + +async def enforce_send_email_rate_limit( + session: AsyncSession, + auth_context: AuthContext, + *, + now: datetime.datetime | None = None, +) -> EmailSendRateLimitDecision: + """Atomically reserve one send attempt in the shared PostgreSQL bucket. + + PostgreSQL transaction advisory locking serializes the read/update/insert + decision across workers. A single row per scope plus ``expires_at`` keeps + attempts bounded; unavailable shared state fails closed instead of using a + process-local fallback. + """ + if not _session_uses_postgresql(session): + raise EmailSendRateLimitUnavailable + + observed_at = now or _utc_now() + scope_hash = rate_limit_scope_hash( + auth_context.user_id, auth_context.organization_id + ) + try: + await session.execute( + select(func.pg_advisory_xact_lock(bindparam("lock_key"))), + {"lock_key": _lock_key(scope_hash)}, + ) + result = await session.execute( + select(EmailSendRateBucket) + .where(EmailSendRateBucket.bucket_scope_hash == scope_hash) + .with_for_update() + ) + bucket = result.scalar_one_or_none() + allowed, attempt_count, window_started_at, expires_at = _next_bucket_state( + bucket, observed_at + ) + if bucket is None: + session.add( + EmailSendRateBucket( + bucket_scope_hash=scope_hash, + window_started_at=window_started_at, + attempt_count=attempt_count, + expires_at=expires_at, + created_at=observed_at, + updated_at=observed_at, + ) + ) + else: + bucket.window_started_at = window_started_at + bucket.attempt_count = attempt_count + bucket.expires_at = expires_at + bucket.updated_at = observed_at + + decision = EmailSendRateLimitDecision( + allowed=allowed, + reason="allowed" if allowed else "quota_exhausted", + ) + session.add( + _audit_event( + auth_context, + scope_hash=scope_hash, + decision=decision, + ) + ) + await session.commit() + return decision + except Exception as exc: + try: + await session.rollback() + except Exception as rollback_exc: + logger.warning( + "Email send rate limiter rollback failed; error_type=%s", + type(rollback_exc).__name__, + ) + logger.warning( + "Email send rate limiter decision unavailable; " + "event_action=email_send_rate_limit.unavailable error_type=%s", + type(exc).__name__, + ) + raise EmailSendRateLimitUnavailable from exc diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py new file mode 100644 index 000000000..65190ef1a --- /dev/null +++ b/backend/tests/test_email_send_rate_limiter.py @@ -0,0 +1,138 @@ +import asyncio +import datetime +from types import SimpleNamespace + +import pytest + +from api.auth import AuthContext +from db.models import EmailSendRateBucket, SecurityAuditEvent +from services.email_send_rate_limiter import ( + EmailSendRateLimitUnavailable, + enforce_send_email_rate_limit, + rate_limit_scope_hash, +) + + +class _Result: + def __init__(self, row=None): + self.row = row + + def scalar_one_or_none(self): + return self.row + + +class _SharedBucketStore: + def __init__(self): + self.buckets = {} + self.lock = asyncio.Lock() + + +class _PostgresSession: + def __init__(self, store): + self.store = store + self.added = [] + self.queries = [] + self.lock_held = False + + def get_bind(self): + return SimpleNamespace(dialect=SimpleNamespace(name="postgresql")) + + async def execute(self, query, params=None): + query_text = str(query).lower() + self.queries.append(query_text) + if "pg_advisory_xact_lock" in query_text: + await self.store.lock.acquire() + self.lock_held = True + return _Result() + if "email_send_rate_buckets" in query_text: + scope_hash = next( + value + for key, value in query.compile().params.items() + if "bucket_scope_hash" in key + ) + return _Result(self.store.buckets.get(scope_hash)) + raise AssertionError(f"unexpected query: {query_text}") + + def add(self, item): + self.added.append(item) + if isinstance(item, EmailSendRateBucket): + self.store.buckets[item.bucket_scope_hash] = item + + async def commit(self): + if self.lock_held: + self.store.lock.release() + self.lock_held = False + + async def rollback(self): + if self.lock_held: + self.store.lock.release() + self.lock_held = False + + +def _context(user_id="user-1", organization_id="org-1"): + return AuthContext( + user_id=user_id, + role="member", + organization_id=organization_id, + group_ids=(), + workspace_id=f"workspace-{organization_id or user_id}", + ) + + +@pytest.mark.asyncio +async def test_shared_bucket_limits_concurrent_workers_without_cross_scope_leakage(): + store = _SharedBucketStore() + observed_at = datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc) + + async def attempt(user_id): + return await enforce_send_email_rate_limit( + _PostgresSession(store), _context(user_id), now=observed_at + ) + + same_scope = await asyncio.gather(*(attempt("user-1") for _ in range(11))) + other_scope = await asyncio.gather(*(attempt("user-2") for _ in range(10))) + + assert sum(decision.allowed for decision in same_scope) == 10 + assert sum(not decision.allowed for decision in same_scope) == 1 + assert all(decision.allowed for decision in other_scope) + + rollover = await enforce_send_email_rate_limit( + _PostgresSession(store), + _context("user-1"), + now=observed_at + datetime.timedelta(seconds=61), + ) + assert rollover.allowed is True + assert store.buckets[rate_limit_scope_hash("user-1", "org-1")].attempt_count == 1 + + +@pytest.mark.asyncio +async def test_shared_bucket_uses_transaction_lock_and_non_sensitive_audit_state(): + store = _SharedBucketStore() + session = _PostgresSession(store) + decision = await enforce_send_email_rate_limit( + session, + _context(), + now=datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc), + ) + + assert decision.allowed is True + assert any("pg_advisory_xact_lock" in query for query in session.queries) + assert any("for update" in query for query in session.queries) + bucket = next(item for item in session.added if isinstance(item, EmailSendRateBucket)) + audit = next(item for item in session.added if isinstance(item, SecurityAuditEvent)) + assert bucket.bucket_scope_hash == rate_limit_scope_hash("user-1", "org-1") + assert "user-1" not in repr(bucket) + assert "org-1" not in repr(bucket) + assert "user-1" not in repr(audit) + assert "org-1" not in repr(audit) + assert audit.event_action == "email_send_rate_limit.allowed" + + +@pytest.mark.asyncio +async def test_rate_limiter_fails_closed_when_shared_state_is_unavailable(): + class _UnavailableSession: + def get_bind(self): + return SimpleNamespace(dialect=SimpleNamespace(name="sqlite")) + + with pytest.raises(EmailSendRateLimitUnavailable): + await enforce_send_email_rate_limit(_UnavailableSession(), _context()) diff --git a/backend/tests/test_emails_api.py b/backend/tests/test_emails_api.py index 7bffa6ff7..3c06363b7 100644 --- a/backend/tests/test_emails_api.py +++ b/backend/tests/test_emails_api.py @@ -23,6 +23,10 @@ from unittest.mock import AsyncMock, patch from services.embedding import STORAGE_EMBEDDING_DIMENSION from services.email_service import generate_email_fingerprint +from services.email_send_rate_limiter import ( + EmailSendRateLimitDecision, + EmailSendRateLimitUnavailable, +) pytestmark = pytest.mark.usefixtures("dev_auth_dependency_overrides") TEST_SESSION_HMAC_SECRET = os.environ["AUTH_SESSION_HMAC_SECRET"] @@ -1782,6 +1786,11 @@ def fake_validate_smtp_destination(smtp_server, smtp_port, *, resolve_host=True) "api.emails.validate_smtp_destination", fake_validate_smtp_destination ) + async def allow_rate_limit(*args, **kwargs): + return EmailSendRateLimitDecision(allowed=True, reason="allowed") + + monkeypatch.setattr(emails_api, "enforce_send_email_rate_limit", allow_rate_limit) + client = TestClient(app, headers={"X-User-Id": "testuser"}) response = client.post( @@ -1864,9 +1873,18 @@ def fake_validate_smtp_destination(smtp_server, smtp_port, *, resolve_host=True) monkeypatch.setattr( "api.emails.validate_smtp_destination", fake_validate_smtp_destination ) - monkeypatch.setattr(emails_api, "_SEND_EMAIL_RATE_LIMIT_MAX_ATTEMPTS", 1) - monkeypatch.setattr(emails_api.time, "monotonic", lambda: 100.0) - emails_api._email_send_attempts_by_scope.clear() + + calls = 0 + + async def fake_rate_limit(*args, **kwargs): + nonlocal calls + calls += 1 + return EmailSendRateLimitDecision( + allowed=calls == 1, + reason="allowed" if calls == 1 else "quota_exhausted", + ) + + monkeypatch.setattr(emails_api, "enforce_send_email_rate_limit", fake_rate_limit) try: client = TestClient(app, headers={"X-User-Id": "testuser"}) @@ -1879,13 +1897,42 @@ def fake_validate_smtp_destination(smtp_server, smtp_port, *, resolve_host=True) assert client.post("/api/emails/send", json=payload).status_code == 200 response = client.post("/api/emails/send", json=payload) finally: - emails_api._email_send_attempts_by_scope.clear() + app.dependency_overrides.clear() assert response.status_code == 429 assert response.json() == {"detail": "Email send rate limit exceeded"} mock_send_email.assert_called_once() +@patch("api.emails.send_email", return_value={"status": "sent", "simulated": False}) +def test_send_email_endpoint_fails_closed_when_rate_limiter_is_unavailable( + mock_send_email, monkeypatch +): + from fastapi.testclient import TestClient + from main import app + + monkeypatch.setattr( + "api.emails.validate_smtp_destination", + lambda *args, **kwargs: (args[0], args[1]), + ) + + async def unavailable_rate_limit(*args, **kwargs): + raise EmailSendRateLimitUnavailable + + monkeypatch.setattr( + emails_api, "enforce_send_email_rate_limit", unavailable_rate_limit + ) + + response = TestClient(app, headers={"X-User-Id": "testuser"}).post( + "/api/emails/send", + json={"to": "test@example.com", "subject": "Test", "body": "Body"}, + ) + + assert response.status_code == 503 + assert response.json() == {"detail": "Email send rate limiter unavailable"} + mock_send_email.assert_not_called() + + @patch("api.emails.send_email", return_value={"status": "simulated", "simulated": True}) def test_send_email_endpoint_ignores_user_id_query_and_uses_authenticated_user_config( mock_send_email, monkeypatch, sample_email @@ -1900,6 +1947,10 @@ def fake_validate_smtp_destination(smtp_server, smtp_port, *, resolve_host=True) monkeypatch.setattr( "api.emails.validate_smtp_destination", fake_validate_smtp_destination ) + async def allow_rate_limit(*args, **kwargs): + return EmailSendRateLimitDecision(allowed=True, reason="allowed") + + monkeypatch.setattr(emails_api, "enforce_send_email_rate_limit", allow_rate_limit) session = ScalarQueryCapturingSession([sample_email]) async def tenant_db(): @@ -2006,6 +2057,10 @@ def fake_validate_smtp_destination(smtp_server, smtp_port, *, resolve_host=True) monkeypatch.setattr( "api.emails.validate_smtp_destination", fake_validate_smtp_destination ) + async def allow_rate_limit(*args, **kwargs): + return EmailSendRateLimitDecision(allowed=True, reason="allowed") + + monkeypatch.setattr(emails_api, "enforce_send_email_rate_limit", allow_rate_limit) client = TestClient(app, headers={"X-User-Id": "testuser"}) From 754cea8dd57e18d55f11ee7d20d4c96bf5b049d6 Mon Sep 17 00:00:00 2001 From: "cursor[bot]" <206951365+cursor[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 03:43:46 -0700 Subject: [PATCH 02/18] Harden Cloud Agent env secrets and email_records.is_read migration (#1381) * fix(db): make fresh-database schema bootstrap work end-to-end The retired 'emails' table (replaced by 'email_records' during the email model reconciliation) was still referenced by fresh-DB setup, breaking both 'alembic upgrade head' and bootstrap_db against a clean database: - schema_backfill_sql() created a dead 'ix_emails_owner_date ON emails' index (used by migration 0001 and bootstrap_db) -> UndefinedTableError. - migration 0011_email_read_state did 'ALTER TABLE emails ADD COLUMN is_read' unconditionally; guard it on the table existing (matching the has_table/ has_column pattern used by later revisions) since email_records already carries is_read from the model metadata. - give email_records.is_read a server_default so create_all/bootstrap_db match the migration intent and raw inserts that omit is_read (postgres smoke seeds) don't hit a NOT NULL violation. Co-authored-by: Seongho Bae * fix(email-import): avoid NUL byte in Postgres advisory-lock key The owner import quota advisory lock built its owner key as f'{user_id}\x00{organization_id}' and passed it to hashtext() as a text bind param. PostgreSQL text cannot encode NUL (0x00), so every email import 500'd on real Postgres with CharacterNotInRepertoireError (mocked/SQLite unit tests skip the advisory-lock path, hiding it). Derive a NUL-free sha256 digest instead and update the tests to assert the NUL-free contract. Co-authored-by: Seongho Bae * chore(env): add Cloud Agent dev environment (backend + frontend + Postgres/pgvector) Repo-managed .cursor/environment.json plus idempotent install/start scripts: - install.sh: system packages (postgresql-16 + pgvector, python venv/build tools), backend venv + pinned requirements, frontend pnpm@11.5.3 deps. - start.sh: bring up the Postgres cluster, generate a per-VM dev .env with random secrets on first boot, ensure the app DB + pgvector extension, and apply alembic migrations. - terminals run the backend (start_backend.py) and frontend (next dev). Co-authored-by: Seongho Bae * fix(env): keep Cloud Agent Postgres secrets off the psql command line Reject empty DATABASE_URL role secrets and apply ALTER USER through dollar-quoted psql stdin. Install hashed requirements so the baked environment matches the CI supply-chain contract. Co-authored-by: Seongho Bae * fix(db): guard email_records.is_read on the alembic path Keep 0011 as a retired-emails no-op downgrade and add 0018 so existing email_records tables get NOT NULL DEFAULT true without interpolated DDL. Co-authored-by: Seongho Bae * docs: record Cloud Agent env contract and NUL advisory-lock anti-pattern Pin the import quota lock key to an independent SHA-256 digest and point operators at the next boot/import action. Co-authored-by: Seongho Bae * test(cloud-agent): reject unpinned pip self-upgrade * fix(cloud-agent): remove unpinned pip self-upgrade * security(cloud-agent): document fixed-argv subprocess boundary * fix(db): stack read-state migration after send buckets --------- Co-authored-by: Cursor Agent Co-authored-by: Seongho Bae Co-authored-by: Seongho Bae Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com> --- .cursor/environment.json | 20 +++++ .cursor/install.sh | 42 ++++++++++ .cursor/start.sh | 80 ++++++++++++++++++ AGENTS.md | 6 ++ ARCHITECTURE.md | 6 ++ CHANGELOG.md | 1 + CLAUDE.md | 11 +++ .../alembic/versions/0011_email_read_state.py | 18 +++- .../versions/0019_email_record_read_state.py | 62 ++++++++++++++ backend/db/models.py | 8 +- backend/scripts/bootstrap_db.py | 4 - .../scripts/reconcile_local_postgres_role.py | 82 +++++++++++++++++++ backend/services/email_import_service.py | 25 +++++- backend/tests/test_alembic_migrations.py | 37 +++++++++ backend/tests/test_bootstrap_db.py | 2 + backend/tests/test_cloud_agent_environment.py | 38 +++++++++ .../tests/test_email_import_quota_lock_key.py | 36 ++++++++ backend/tests/test_emails_api.py | 15 +++- .../test_reconcile_local_postgres_role.py | 66 +++++++++++++++ docs/development/cloud-agent-environment.md | 71 ++++++++++++++++ 20 files changed, 618 insertions(+), 12 deletions(-) create mode 100644 .cursor/environment.json create mode 100755 .cursor/install.sh create mode 100755 .cursor/start.sh create mode 100644 backend/alembic/versions/0019_email_record_read_state.py create mode 100644 backend/scripts/reconcile_local_postgres_role.py create mode 100644 backend/tests/test_cloud_agent_environment.py create mode 100644 backend/tests/test_email_import_quota_lock_key.py create mode 100644 backend/tests/test_reconcile_local_postgres_role.py create mode 100644 docs/development/cloud-agent-environment.md diff --git a/.cursor/environment.json b/.cursor/environment.json new file mode 100644 index 000000000..5964ed32d --- /dev/null +++ b/.cursor/environment.json @@ -0,0 +1,20 @@ +{ + "name": "Naruon dev (backend + frontend + Postgres/pgvector)", + "user": "ubuntu", + "install": "bash .cursor/install.sh", + "start": "bash .cursor/start.sh", + "terminals": [ + { + "name": "backend", + "command": "cd backend && . .venv/bin/activate && python scripts/start_backend.py --host 0.0.0.0 --port 8000" + }, + { + "name": "frontend", + "command": "cd frontend && corepack pnpm@11.5.3 dev --port 3000 --hostname 0.0.0.0" + } + ], + "ports": [ + { "name": "backend", "port": 8000 }, + { "name": "frontend", "port": 3000 } + ] +} diff --git a/.cursor/install.sh b/.cursor/install.sh new file mode 100755 index 000000000..61db7ac03 --- /dev/null +++ b/.cursor/install.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Idempotent repository bootstrap for Naruon Cloud Agent environments. +# +# Prepares durable, source-derived state after checkout: +# * system packages (PostgreSQL 16 + pgvector, Python venv/build tooling) +# * the backend virtualenv + pinned Python requirements +# * the frontend pnpm dependency tree +# +# Per-boot service startup (Postgres, schema migrations, dev secrets) lives in +# .cursor/start.sh so it re-runs on every VM boot, including builds. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO_ROOT" + +echo "==> [install] system packages (postgresql-16, pgvector, python venv, build tools)" +export DEBIAN_FRONTEND=noninteractive +sudo apt-get update -qq +sudo apt-get install -y -qq \ + postgresql-16 \ + postgresql-16-pgvector \ + postgresql-client-16 \ + python3.12-venv \ + python3.12-dev \ + build-essential + +echo "==> [install] backend virtualenv + requirements" +cd "$REPO_ROOT/backend" +if [ ! -x ".venv/bin/python" ]; then + python3 -m venv .venv +fi +# shellcheck disable=SC1091 +. .venv/bin/activate +python -m pip install --require-hashes -r requirements-hashes.txt + +echo "==> [install] frontend dependencies (pnpm@11.5.3)" +cd "$REPO_ROOT/frontend" +corepack enable +corepack prepare pnpm@11.5.3 --activate +corepack pnpm@11.5.3 install --frozen-lockfile + +echo "==> [install] done" \ No newline at end of file diff --git a/.cursor/start.sh b/.cursor/start.sh new file mode 100755 index 000000000..523051f6a --- /dev/null +++ b/.cursor/start.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# Per-boot runtime reconciliation for Naruon Cloud Agent environments. +# +# Runs on every VM start (idempotent): brings up the PostgreSQL cluster, +# materializes a local dev .env with generated secrets on first boot, ensures +# the app database + pgvector extension exist, and applies Alembic migrations. +# +# Dependency installation lives in .cursor/install.sh; this script only +# reconciles per-boot state and then returns so the backend/frontend terminals +# can start. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO_ROOT" + +ENV_FILE="$HOME/.env" +PY="$REPO_ROOT/backend/.venv/bin/python" + +echo "==> [start] ensuring PostgreSQL 16 cluster is online" +if ! pg_lsclusters -h 2>/dev/null | awk '{print $4}' | grep -q online; then + sudo pg_ctlcluster 16 main start || true +fi +# Wait for the server to accept connections before touching it. +for _ in $(seq 1 30); do + if sudo -u postgres pg_isready -q; then break; fi + sleep 1 +done +if ! sudo -u postgres pg_isready -q; then + echo "==> [start] PostgreSQL did not become ready" >&2 + exit 1 +fi + +echo "==> [start] generating local dev .env on first boot (secrets are per-VM)" +if [ ! -f "$ENV_FILE" ]; then + "$PY" - "$ENV_FILE" <<'PYGEN' +import os, secrets, sys +from pathlib import Path +from cryptography.fernet import Fernet + +env_path = Path(sys.argv[1]) +db_password = secrets.token_urlsafe(24) +hmac_secret = secrets.token_urlsafe(48) +enc_key = Fernet.generate_key().decode() + +env_path.write_text( + "# Naruon local dev environment (generated per-VM; not committed).\n" + f"DATABASE_URL=postgresql+asyncpg://postgres:{db_password}@127.0.0.1:5432/ai_email\n" + f"AUTH_SESSION_HMAC_SECRET={hmac_secret}\n" + f"ENCRYPTION_KEY={enc_key}\n" + "DEBUG=false\n" + "RUNTIME_ENVIRONMENT=development\n" + "ENABLE_PROMETHEUS_METRICS=false\n" + "ALLOWED_CORS_ORIGINS=http://localhost:3000,http://127.0.0.1:3000," + "http://localhost:8000,http://127.0.0.1:8000\n" + "SMTP_MODE=simulated\n" + "OPENAI_API_KEY=\n" + "OPENAI_EMBEDDING_MODEL=text-embedding-3-small\n" + "OPENAI_MODEL=gpt-4o\n", + encoding="utf-8", +) +os.chmod(env_path, 0o600) +print(f"wrote {env_path}") +PYGEN +fi + +echo "==> [start] reconciling database role, database, and pgvector extension" +# Keep the local postgres role secret in sync with DATABASE_URL without +# interpolating it into SQL or the process argument list. +"$PY" "$REPO_ROOT/backend/scripts/reconcile_local_postgres_role.py" --env-file "$ENV_FILE" +if ! sudo -u postgres psql -tAc "SELECT 1 FROM pg_database WHERE datname='ai_email'" | grep -q 1; then + sudo -u postgres createdb ai_email +fi +sudo -u postgres psql -d ai_email -v ON_ERROR_STOP=1 \ + -c "CREATE EXTENSION IF NOT EXISTS vector;" >/dev/null + +echo "==> [start] applying database migrations (alembic upgrade head)" +cd "$REPO_ROOT/backend" +"$PY" scripts/migrate_db.py + +echo "==> [start] done" diff --git a/AGENTS.md b/AGENTS.md index 9104dd1f4..a1f8b8c69 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -348,6 +348,12 @@ in this repo. embedding generation is unavailable. Tests must cover the local `embeddinggemma` path so Data workspace imports do not silently bypass the selected embedding model. +- PostgreSQL `text` / `hashtext()` cannot encode a NUL (`0x00`) octet. Do not + pass a raw `user_id\\x00organization_id` (or any other NUL-separated payload) + as a `pg_advisory_lock` bind value. Derive a NUL-free digest such as + SHA-256 hex and keep mocked/SQLite tests from asserting the raw NUL form — + those dialects skip the lock and hide `CharacterNotInRepertoireError` until + a real Postgres import. - Home/Today dashboard reply-wait surfaces must read signed `/api/emails/pending-replies` data instead of inferring pending replies from generic inbox fixtures or static copy. Tests and E2E mocks must verify the diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9d2cbba18..9e27c7ed1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -145,6 +145,12 @@ remains a local/dev compatibility path that creates the `vector` extension, metadata-defined tables for fresh local databases, and idempotent backfills for existing local databases. +Cloud Agent VMs use the same managed migration path through +`.cursor/start.sh` → `scripts/migrate_db.py`. Local role-secret alignment is +stdin dollar-quoted SQL (`scripts/reconcile_local_postgres_role.py`); do not +interpolate `DATABASE_URL` secrets into `psql -c`. See +[`docs/development/cloud-agent-environment.md`](docs/development/cloud-agent-environment.md). + ## Send boundary Outbound replies preserve `In-Reply-To` and `References` headers in the built diff --git a/CHANGELOG.md b/CHANGELOG.md index f31c701a5..1cb7130da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ ## [Unreleased] +- Cloud Agent `.cursor/start.sh`는 `DATABASE_URL` 역할 비밀을 `psql -c`에 보간하지 않고 `scripts/reconcile_local_postgres_role.py`의 dollar-quoted stdin으로 맞춥니다. `install.sh`는 `requirements-hashes.txt`를 `--require-hashes`로 설치합니다. 신선한 DB에서 폐기된 `emails` 테이블을 가정하던 `0011_email_read_state`는 테이블이 없으면 no-op이고, `0019_email_record_read_state`가 `0018_email_send_rate_buckets` 다음에 정식 `email_records.is_read DEFAULT true`를 가드로 맞춥니다. 가져오기 quota advisory lock 키는 NUL 없는 SHA-256 hex입니다. - EmailDetail 테스트가 지원하지 않는 스레드 병합/분리 버튼을 `textContent`뿐 아니라 `aria-label`과 `title` 접근 가능 이름으로도 검출하도록 바꿔, 아이콘 전용 버튼 회귀를 놓치지 않습니다. ### 캘린더 충돌 (Status-weighted conflicts) diff --git a/CLAUDE.md b/CLAUDE.md index be67bc80c..f68a237f6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,6 +98,17 @@ The other compose files are purpose-specific evaluation/evidence stacks: defaults anywhere; Compose/Kubernetes/operators must inject them, and startup fails closed when they are missing. +### Cloud Agent environment (`.cursor/`) + +Repo-managed Cloud Agent bootstrap lives in `.cursor/environment.json`. +`install.sh` installs PostgreSQL 16/pgvector and hashed Python deps; +`start.sh` brings Postgres online, mints a per-VM `~/.env`, and runs +`scripts/migrate_db.py`. Role-secret sync goes through +`backend/scripts/reconcile_local_postgres_role.py` (dollar-quoted stdin, never +`psql -c` interpolation). After boot, open http://127.0.0.1:3000 and sign in +with a minted HMAC session — see +`docs/development/cloud-agent-environment.md`. + ## Architecture Naruon is an AI email workspace: a web client/control plane over diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index 716590cd1..bbe4490f9 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -14,6 +14,19 @@ def upgrade() -> None: + # ``emails`` was the pre-reconciliation email table. ``email_records`` is now + # the single source of truth and already declares ``is_read`` via the model + # metadata created in 0001, so this legacy column add only applies to older + # databases that still carry the retired ``emails`` table. Guard on the table + # existing (matching the has_table/has_column pattern used by later + # revisions) so ``alembic upgrade head`` succeeds on fresh databases. + bind = op.get_bind() + inspector = sa.inspect(bind) + if not inspector.has_table("emails"): + return + existing_columns = {column["name"] for column in inspector.get_columns("emails")} + if "is_read" in existing_columns: + return op.add_column( "emails", sa.Column( @@ -26,4 +39,7 @@ def upgrade() -> None: def downgrade() -> None: - op.drop_column("emails", "is_read") + # No-op: this revision only reconciles a retired ``emails`` table. Dropping + # ``is_read`` when the column is present would also remove a pre-existing + # column this revision did not create. + return diff --git a/backend/alembic/versions/0019_email_record_read_state.py b/backend/alembic/versions/0019_email_record_read_state.py new file mode 100644 index 000000000..6e35cbb82 --- /dev/null +++ b/backend/alembic/versions/0019_email_record_read_state.py @@ -0,0 +1,62 @@ +"""Guard email_records.is_read with NOT NULL DEFAULT true. + +Revision ID: 0019_email_record_read_state +Revises: 0018_email_send_rate_buckets + +``0011_email_read_state`` only mutates the retired ``emails`` table. Fresh +databases already receive ``email_records.is_read`` from ``0001`` +``create_all`` plus the current model ``server_default``. Existing databases +whose ``email_records`` row predates that column (or lacks a server default) +still need a guarded additive revision so raw INSERTs that omit ``is_read`` +succeed. +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + +revision = "0019_email_record_read_state" +down_revision = "0018_email_send_rate_buckets" +branch_labels = None +depends_on = None + +_EMAIL_RECORDS_TABLE = "email_records" +_READ_STATE_COLUMN = "is_read" + + +def upgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + if not inspector.has_table(_EMAIL_RECORDS_TABLE): + return + columns = { + column["name"]: column + for column in inspector.get_columns(_EMAIL_RECORDS_TABLE) + } + if _READ_STATE_COLUMN not in columns: + op.add_column( + _EMAIL_RECORDS_TABLE, + sa.Column( + _READ_STATE_COLUMN, + sa.Boolean(), + nullable=False, + server_default=sa.text("true"), + ), + ) + return + if columns[_READ_STATE_COLUMN].get("default") is None: + op.alter_column( + _EMAIL_RECORDS_TABLE, + _READ_STATE_COLUMN, + existing_type=sa.Boolean(), + existing_nullable=False, + server_default=sa.text("true"), + ) + + +def downgrade() -> None: + # No-op: ``create_all`` and later model metadata may already own this + # column. Dropping it would remove a default this revision did not + # necessarily create. + return diff --git a/backend/db/models.py b/backend/db/models.py index 7380defb8..40db8ea00 100644 --- a/backend/db/models.py +++ b/backend/db/models.py @@ -18,6 +18,7 @@ Text, UniqueConstraint, func, + text, ) from sqlalchemy.orm import declarative_base, Mapped, mapped_column, relationship from sqlalchemy.types import TypeDecorator @@ -803,7 +804,12 @@ def owner_filters(cls, user_id: str, organization_id: str | None): date: Mapped[datetime.datetime] = mapped_column(DateTime(timezone=True), index=True) body: Mapped[str] = mapped_column(Text) # IMAP \Seen read state; defaults read so historical/file imports don't nag. - is_read: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) + # A DB-level server_default keeps create_all/bootstrap_db consistent with the + # 0011_email_read_state migration intent so raw inserts that omit is_read + # (e.g. postgres smoke seeds) don't hit a NOT NULL violation. + is_read: Mapped[bool] = mapped_column( + Boolean, nullable=False, default=True, server_default=text("true") + ) # Defer large pgvector payloads on default entity loads. embedding = mapped_column(Vector(1536), deferred=True) attachments: Mapped[list["Attachment"]] = relationship( diff --git a/backend/scripts/bootstrap_db.py b/backend/scripts/bootstrap_db.py index 1047103e8..3e579a053 100644 --- a/backend/scripts/bootstrap_db.py +++ b/backend/scripts/bootstrap_db.py @@ -186,10 +186,6 @@ def _get_create_indexes_statements() -> list[Executable]: "CREATE INDEX IF NOT EXISTS ix_email_records_owner_date " "ON email_records (user_id, organization_id, date)" ), - text( - "CREATE INDEX IF NOT EXISTS ix_emails_owner_date " - "ON emails (user_id, organization_id, date)" - ), text( "CREATE INDEX IF NOT EXISTS ix_sender_relationships_owner_source " "ON sender_relationships " diff --git a/backend/scripts/reconcile_local_postgres_role.py b/backend/scripts/reconcile_local_postgres_role.py new file mode 100644 index 000000000..1c652baad --- /dev/null +++ b/backend/scripts/reconcile_local_postgres_role.py @@ -0,0 +1,82 @@ +"""Reconcile the local Cloud Agent Postgres role secret without SQL interpolation. + +Cloud Agent ``start.sh`` keeps the ``postgres`` role aligned with the +generated ``DATABASE_URL``. The secret must travel only on ``psql`` stdin as +dollar-quoted SQL so a quote or backslash in an existing ``~/.env`` cannot +break out of ``ALTER USER ... PASSWORD`` and does not appear on the process +command line. +""" + +from __future__ import annotations + +import argparse +import secrets +import subprocess # nosec B404 -- fixed executable argv, shell=False, secret on stdin. +import sys +from collections.abc import Callable +from pathlib import Path +from typing import Any +from urllib.parse import unquote, urlsplit + +INCOMPLETE_LOCAL_DATABASE_CONFIG = "local database configuration is incomplete" +_ROLE_NAME = "postgres" + + +def database_url_password(env_text: str) -> str: + """Return the ``DATABASE_URL`` user secret, or fail closed when it is absent.""" + for line in env_text.splitlines(): + if line.startswith("DATABASE_URL="): + raw_url = line.split("=", 1)[1].strip() + secret = unquote(urlsplit(raw_url).password or "") + if not secret: + raise ValueError(INCOMPLETE_LOCAL_DATABASE_CONFIG) + return secret + raise ValueError(INCOMPLETE_LOCAL_DATABASE_CONFIG) + + +def build_alter_role_sql(secret: str, *, role_name: str = _ROLE_NAME) -> str: + """Build ``ALTER USER`` SQL that dollar-quotes ``secret``. + + The tag is regenerated until it is absent from the secret so the closer + cannot appear inside the quoted value. + """ + if not secret: + raise ValueError(INCOMPLETE_LOCAL_DATABASE_CONFIG) + tag = f"naruon{secrets.token_hex(16)}" + while tag in secret: + tag = f"naruon{secrets.token_hex(16)}" + return f"ALTER USER {role_name} WITH PASSWORD ${tag}${secret}${tag};\n" + + +def reconcile_local_postgres_role( + secret: str, + *, + runner: Callable[..., Any] = subprocess.run, +) -> None: + """Apply the role secret through ``psql`` stdin, never ``psql -c``.""" + sql = build_alter_role_sql(secret) + runner( + ["sudo", "-u", "postgres", "psql", "-v", "ON_ERROR_STOP=1"], + input=sql, + text=True, + check=True, + stdout=subprocess.DEVNULL, + ) + + +def main(argv: list[str] | None = None) -> int: + """Read an env file and align the local ``postgres`` role secret.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--env-file", + required=True, + help="Path to the local env file that already contains DATABASE_URL.", + ) + args = parser.parse_args(argv) + env_text = Path(args.env_file).read_text(encoding="utf-8") + reconcile_local_postgres_role(database_url_password(env_text)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) \ No newline at end of file diff --git a/backend/services/email_import_service.py b/backend/services/email_import_service.py index 1ff9a2bb3..5df9a5f41 100644 --- a/backend/services/email_import_service.py +++ b/backend/services/email_import_service.py @@ -243,6 +243,27 @@ def _session_uses_postgresql(session: AsyncSession) -> bool: return getattr(getattr(bind, "dialect", None), "name", None) == "postgresql" +def _owner_import_quota_lock_key(user_id: str, organization_id: str) -> str: + """Return a NUL-free SHA-256 digest for ``pg_advisory_lock(hashtext(...))``. + + PostgreSQL ``text`` cannot store a 0x00 octet, so passing + ``f"{user_id}\\x00{organization_id}"`` to ``hashtext()`` raises + ``CharacterNotInRepertoireError`` on real Postgres (PostgreSQL Global + Development Group, n.d.). Hash the NUL-separated payload instead and bind + only the hex digest. + + References + ---------- + PostgreSQL Global Development Group. (n.d.). *Character set support*. + PostgreSQL Documentation. + https://www.postgresql.org/docs/current/multibyte.html + """ + payload = "\x00".join((user_id, organization_id)) + return hashlib.sha256( + payload.encode("utf-8", errors="surrogatepass") + ).hexdigest() + + async def _acquire_owner_import_quota_lock( session: AsyncSession, *, user_id: str, organization_id: str ) -> bool: @@ -250,7 +271,7 @@ async def _acquire_owner_import_quota_lock( return False lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, - "owner_key": f"{user_id}\x00{organization_id}", + "owner_key": _owner_import_quota_lock_key(user_id, organization_id), } await session.execute( select( @@ -269,7 +290,7 @@ async def _release_owner_import_quota_lock( ) -> None: lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, - "owner_key": f"{user_id}\x00{organization_id}", + "owner_key": _owner_import_quota_lock_key(user_id, organization_id), } await session.execute( select( diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index f8f3ffeae..f286b7259 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -438,6 +438,43 @@ def test_merge_revision_reconciles_newsdom_provider_branch(): assert "op.drop_column(" not in revision_text +def test_email_read_state_revision_is_retired_emails_table_only() -> None: + revision_path = BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" + revision_text = revision_path.read_text() + + assert 'revision = "0011_email_read_state"' in revision_text + assert 'down_revision = "0009_project_graph_projection"' in revision_text + assert 'has_table("emails")' in revision_text + assert 'op.add_column(\n "emails"' in revision_text + assert 'op.add_column(\n "email_records"' not in revision_text + # Downgrade must not drop a pre-existing emails.is_read this revision did + # not create. Retired-table reconciliation is a documented no-op. + assert "op.drop_column(" not in revision_text + + +def test_email_record_read_state_revision_guards_canonical_table() -> None: + revision_path = ( + BACKEND_ROOT / "alembic" / "versions" / "0019_email_record_read_state.py" + ) + assert revision_path.exists() + revision_text = revision_path.read_text() + + assert 'revision = "0019_email_record_read_state"' in revision_text + assert 'down_revision = "0018_email_send_rate_buckets"' in revision_text + assert len("0019_email_record_read_state") <= 32 + assert '_EMAIL_RECORDS_TABLE = "email_records"' in revision_text + assert "has_table(_EMAIL_RECORDS_TABLE)" in revision_text + assert "_READ_STATE_COLUMN" in revision_text + assert '"is_read"' in revision_text + assert "server_default=sa.text(\"true\")" in revision_text or ( + 'server_default=sa.text("true")' in revision_text + ) + assert "op.add_column(" in revision_text + assert "sa.text(f" not in revision_text + # Additive default only; do not drop a column create_all may already own. + assert "op.drop_column(" not in revision_text + + def test_merge_revision_reconciles_newsdom_document_and_carddav_heads(): revision_path = ( BACKEND_ROOT / "alembic" / "versions" / "0017_merge_newsdom_carddav_heads.py" diff --git a/backend/tests/test_bootstrap_db.py b/backend/tests/test_bootstrap_db.py index 5af0540f0..07e0919c8 100644 --- a/backend/tests/test_bootstrap_db.py +++ b/backend/tests/test_bootstrap_db.py @@ -138,6 +138,8 @@ def test_schema_backfill_adds_email_indexes(monkeypatch): "create index if not exists ix_email_records_date" in statement for statement in statements ) + assert all(" on emails " not in statement for statement in statements) + assert all("ix_emails_owner_date" not in statement for statement in statements) def test_schema_backfill_adds_llm_provider_columns_and_indexes(monkeypatch): diff --git a/backend/tests/test_cloud_agent_environment.py b/backend/tests/test_cloud_agent_environment.py new file mode 100644 index 000000000..a6c0a0b26 --- /dev/null +++ b/backend/tests/test_cloud_agent_environment.py @@ -0,0 +1,38 @@ +"""Source contracts for the repo-managed Cloud Agent environment scripts.""" + +from __future__ import annotations + +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +INSTALL_SH = (REPO_ROOT / ".cursor" / "install.sh").read_text(encoding="utf-8") +START_SH = (REPO_ROOT / ".cursor" / "start.sh").read_text(encoding="utf-8") +ENVIRONMENT_JSON = (REPO_ROOT / ".cursor" / "environment.json").read_text( + encoding="utf-8" +) + + +def test_install_sh_pins_python_requirements_with_hashes() -> None: + assert "requirements-hashes.txt" in INSTALL_SH + assert "--require-hashes" in INSTALL_SH + assert "pip install -r requirements.txt" not in INSTALL_SH + assert "pip install --upgrade pip" not in INSTALL_SH + + +def test_start_sh_does_not_interpolate_role_secret_into_sql() -> None: + assert "PASSWORD '${DB_PASSWORD}'" not in START_SH + assert "ALTER USER postgres WITH PASSWORD '" not in START_SH + assert "reconcile_local_postgres_role.py" in START_SH + + +def test_start_sh_fails_closed_when_postgres_never_becomes_ready() -> None: + assert "pg_isready" in START_SH + assert "did not become ready" in START_SH + assert "exit 1" in START_SH + + +def test_environment_json_keeps_servers_in_terminals_not_install() -> None: + assert "bash .cursor/install.sh" in ENVIRONMENT_JSON + assert "bash .cursor/start.sh" in ENVIRONMENT_JSON + assert "scripts/start_backend.py" in ENVIRONMENT_JSON + assert "corepack pnpm@11.5.3 dev" in ENVIRONMENT_JSON \ No newline at end of file diff --git a/backend/tests/test_email_import_quota_lock_key.py b/backend/tests/test_email_import_quota_lock_key.py new file mode 100644 index 000000000..856b16d1b --- /dev/null +++ b/backend/tests/test_email_import_quota_lock_key.py @@ -0,0 +1,36 @@ +"""Contract tests for the Postgres import-quota advisory-lock owner key. + +PostgreSQL ``text`` / ``hashtext()`` cannot store a NUL (0x00) octet +(PostgreSQL Global Development Group, n.d.). The production helper must +therefore emit a NUL-free digest. The expected digest is computed here with +the standard-library hasher so a helper rewrite cannot silently change the +on-disk lock identity. +""" + +from __future__ import annotations + +import hashlib + +from services.email_import_service import _owner_import_quota_lock_key + +# Independent reference: SHA-256 of UTF-8 ``user_id + NUL + organization_id``. +_GOLDEN_TESTUSER_ORG_ACME = "3fbc5671f32a1608f88c1775c1008c26c53faaea0308b97c556eeceb2b4bb8d3" + + +def test_owner_import_quota_lock_key_matches_independent_sha256_digest() -> None: + independent = hashlib.sha256(b"testuser\x00org-acme").hexdigest() + assert independent == _GOLDEN_TESTUSER_ORG_ACME + assert _owner_import_quota_lock_key("testuser", "org-acme") == independent + assert "\x00" not in independent + + +def test_owner_import_quota_lock_key_is_nul_free_for_unicode_owners() -> None: + user_id = "유저" + organization_id = "org-서울" + independent = hashlib.sha256( + f"{user_id}\x00{organization_id}".encode("utf-8") + ).hexdigest() + actual = _owner_import_quota_lock_key(user_id, organization_id) + assert actual == independent + assert "\x00" not in actual + assert len(actual) == 64 diff --git a/backend/tests/test_emails_api.py b/backend/tests/test_emails_api.py index 3c06363b7..c0678696a 100644 --- a/backend/tests/test_emails_api.py +++ b/backend/tests/test_emails_api.py @@ -22,6 +22,7 @@ import datetime from unittest.mock import AsyncMock, patch from services.embedding import STORAGE_EMBEDDING_DIMENSION +from services.email_import_service import _owner_import_quota_lock_key from services.email_service import generate_email_fingerprint from services.email_send_rate_limiter import ( EmailSendRateLimitDecision, @@ -1281,14 +1282,18 @@ async def test_import_email_files_serializes_quota_with_postgres_owner_lock( assert "pg_advisory_unlock" in advisory_queries[-1] assert "hashtext(:namespace_key)" in advisory_queries[0] assert ":owner_key" in advisory_queries[0] + # PostgreSQL text/hashtext cannot encode NUL (0x00); the owner key must be a + # NUL-free digest, not a raw ``user\x00org`` separator. + expected_owner_key = _owner_import_quota_lock_key("testuser", "org-acme") + assert "\x00" not in expected_owner_key assert advisory_query_params(session) == [ { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": expected_owner_key, }, { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": expected_owner_key, }, ] @@ -1344,14 +1349,16 @@ async def test_import_email_files_rejects_when_owner_quota_is_exhausted( advisory_queries = advisory_query_texts(session) assert "pg_advisory_lock" in advisory_queries[0] assert "pg_advisory_unlock" in advisory_queries[-1] + expected_owner_key = _owner_import_quota_lock_key("testuser", "org-acme") + assert "\x00" not in expected_owner_key assert advisory_query_params(session) == [ { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": expected_owner_key, }, { "namespace_key": "naruon-email-import-quota", - "owner_key": "testuser\x00org-acme", + "owner_key": expected_owner_key, }, ] diff --git a/backend/tests/test_reconcile_local_postgres_role.py b/backend/tests/test_reconcile_local_postgres_role.py new file mode 100644 index 000000000..2a31bfa46 --- /dev/null +++ b/backend/tests/test_reconcile_local_postgres_role.py @@ -0,0 +1,66 @@ +"""Tests for Cloud Agent local-Postgres role password reconciliation. + +The start path must reject an empty role secret and must never interpolate +that secret into a ``psql -c`` SQL string or process argv. +""" + +from __future__ import annotations + +import pytest + +from scripts.reconcile_local_postgres_role import ( + build_alter_role_sql, + database_url_password, + reconcile_local_postgres_role, +) + + +def test_database_url_password_rejects_empty_secret() -> None: + with pytest.raises(ValueError, match="local database configuration is incomplete"): + database_url_password("DATABASE_URL=postgresql+asyncpg://postgres@127.0.0.1:5432/ai_email\n") + + +def test_database_url_password_rejects_missing_url() -> None: + with pytest.raises(ValueError, match="local database configuration is incomplete"): + database_url_password("DEBUG=false\n") + + +def test_database_url_password_unquotes_url_encoded_secret() -> None: + assert ( + database_url_password( + "DATABASE_URL=postgresql+asyncpg://postgres:a%27b@127.0.0.1:5432/ai_email\n" + ) + == "a'b" + ) + + +def test_build_alter_role_sql_dollar_quotes_metacharacters() -> None: + secret = "x'; DROP ROLE postgres; --" + sql = build_alter_role_sql(secret) + assert "DROP ROLE postgres" in sql + assert "PASSWORD '" not in sql + assert sql.startswith("ALTER USER postgres WITH PASSWORD $") + assert sql.endswith(";\n") or sql.endswith(";") + + +def test_reconcile_local_postgres_role_keeps_secret_off_argv() -> None: + captured: dict[str, object] = {} + + def fake_run(argv, **kwargs): + captured["argv"] = list(argv) + captured["input"] = kwargs.get("input") + captured["check"] = kwargs.get("check") + + class _Completed: + returncode = 0 + + return _Completed() + + secret = "quote'me;and\\back" + reconcile_local_postgres_role(secret, runner=fake_run) + argv = captured["argv"] + assert isinstance(argv, list) + assert all(secret not in str(part) for part in argv) + assert "-c" not in argv + assert secret in str(captured["input"]) + assert captured["check"] is True diff --git a/docs/development/cloud-agent-environment.md b/docs/development/cloud-agent-environment.md new file mode 100644 index 000000000..ede732910 --- /dev/null +++ b/docs/development/cloud-agent-environment.md @@ -0,0 +1,71 @@ +# Cloud Agent development environment + +Use this when a Cloud Agent or a fresh Ubuntu workstation needs a running +Naruon control plane (FastAPI + Next.js + PostgreSQL 16/pgvector) without +Docker Compose. + +## Next action + +1. Confirm `.cursor/environment.json` is the environment source for the VM. +2. After `start.sh` exits, open http://127.0.0.1:3000 (frontend) and + http://127.0.0.1:8000/ (backend health). +3. Mint a member HMAC session against the generated `AUTH_SESSION_HMAC_SECRET` + in `~/.env`, then import `.eml` fixtures through + `POST /api/emails/import-files`. +4. If Postgres is down, re-run `bash .cursor/start.sh` — do not edit secrets + into SQL by hand. + +## Install versus start + +| Script | Lifetime | What to put here | +| --- | --- | --- | +| `.cursor/install.sh` | Durable, source-derived | `postgresql-16` + pgvector, hashed `requirements-hashes.txt`, `pnpm@11.5.3 --frozen-lockfile` | +| `.cursor/start.sh` | Every boot | Postgres cluster, per-VM `~/.env`, `ai_email` + `vector`, `alembic upgrade head` | +| `environment.json` terminals | Long-running | `scripts/start_backend.py` and `pnpm dev` | + +`install.sh` must use `python -m pip install --require-hashes -r requirements-hashes.txt`. +Unhashed `requirements.txt` is not an acceptable Cloud Agent supply-chain path. + +## Secret handling + +`start.sh` writes `~/.env` only when the file is missing. Generated values use +`secrets.token_urlsafe(48)` for `AUTH_SESSION_HMAC_SECRET` and +`Fernet.generate_key()` for `ENCRYPTION_KEY`, matching +`validate_auth_session_hmac_secret_value`. + +The `postgres` role secret is applied by +`backend/scripts/reconcile_local_postgres_role.py`: + +- empty `DATABASE_URL` user secrets fail closed +- the secret is dollar-quoted on `psql` stdin +- the secret never appears in `psql -c` or process argv + +Do not mount this `~/.env` as a Compose `env_file`. Compose interpolation still +resolves `NARUON_ENV_FILE` > `~/.env` > `./.env` without leaking the file into +the container environment wholesale. + +## Schema contract on a fresh database + +`0001_initial_control_plane` runs `Base.metadata.create_all`, so +`email_records.is_read` is present with `DEFAULT true` on a current model. +`0011_email_read_state` only touches the retired `emails` table and is a no-op +when that table is absent. `0019_email_record_read_state` follows the shared +`0018_email_send_rate_buckets` migration and adds or defaults `email_records.is_read` +on older databases that already have `email_records` but lack the column or its +server default. + +## Import quota lock + +Owner import serialization uses `pg_advisory_lock(hashtext(namespace), hashtext(owner_key))`. +PostgreSQL `text` cannot store a NUL octet, so the owner key is a SHA-256 hex +digest of `user_id + 0x00 + organization_id`, not the raw NUL-separated string +(PostgreSQL Global Development Group, n.d.). + +## References + +PostgreSQL Global Development Group. (n.d.). *Character set support*. +PostgreSQL Documentation. https://www.postgresql.org/docs/current/multibyte.html + +PostgreSQL Global Development Group. (n.d.). *psql — PostgreSQL interactive +terminal*. PostgreSQL Documentation. +https://www.postgresql.org/docs/current/app-psql.html From 46f4b92a717361e3e4e42fcebc1d8c090a64c59b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 26 Aug 2026 02:05:55 +0900 Subject: [PATCH 03/18] test(postgres): seed email read state explicitly --- backend/tests/test_bootstrap_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/tests/test_bootstrap_db.py b/backend/tests/test_bootstrap_db.py index 07e0919c8..c5cf73e10 100644 --- a/backend/tests/test_bootstrap_db.py +++ b/backend/tests/test_bootstrap_db.py @@ -772,11 +772,11 @@ async def test_connector_signal_events_real_postgres_bootstrap_smoke(): text(""" INSERT INTO email_records ( user_id, organization_id, message_id, sender, recipients, - subject, "date", body + subject, "date", body, is_read ) VALUES ( :user_id, :organization_id, :message_id, :sender, - :recipients, :subject, now(), :body + :recipients, :subject, now(), :body, true ) RETURNING id """), From 48ab34082c104fbfe0863bcb0e86b3a90b9392bd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 08:41:02 +0900 Subject: [PATCH 04/18] fix(email): enforce rolling send limit transaction --- backend/api/emails.py | 2 +- backend/services/email_send_rate_limiter.py | 135 +++++++----------- backend/tests/test_email_send_rate_limiter.py | 101 +++++++++---- 3 files changed, 128 insertions(+), 110 deletions(-) diff --git a/backend/api/emails.py b/backend/api/emails.py index 2e92b20ee..ed7af66c7 100644 --- a/backend/api/emails.py +++ b/backend/api/emails.py @@ -731,7 +731,7 @@ async def send_email_endpoint( references=request.references, ) try: - rate_limit_decision = await enforce_send_email_rate_limit(db, auth_context) + rate_limit_decision = await enforce_send_email_rate_limit(auth_context) except EmailSendRateLimitUnavailable as exc: raise HTTPException( status_code=503, diff --git a/backend/services/email_send_rate_limiter.py b/backend/services/email_send_rate_limiter.py index 05b670497..041debed9 100644 --- a/backend/services/email_send_rate_limiter.py +++ b/backend/services/email_send_rate_limiter.py @@ -11,7 +11,8 @@ from sqlalchemy import bindparam, func, select from sqlalchemy.ext.asyncio import AsyncSession -from db.models import EmailSendRateBucket, SecurityAuditEvent +from db.models import SecurityAuditEvent +from db.session import AsyncSessionLocal if TYPE_CHECKING: from api.auth import AuthContext @@ -50,24 +51,6 @@ def _utc_now() -> datetime.datetime: return datetime.datetime.now(datetime.timezone.utc) -def _next_bucket_state( - bucket: EmailSendRateBucket | None, - now: datetime.datetime, -) -> tuple[bool, int, datetime.datetime, datetime.datetime]: - """Calculate the next state without storing individual attempts.""" - expires_at = now + datetime.timedelta(seconds=SEND_RATE_LIMIT_WINDOW_SECONDS) - if bucket is None or now >= bucket.expires_at: - return True, 1, now, expires_at - if bucket.attempt_count >= SEND_RATE_LIMIT_MAX_ATTEMPTS: - return False, bucket.attempt_count, bucket.window_started_at, bucket.expires_at - return ( - True, - bucket.attempt_count + 1, - bucket.window_started_at, - bucket.expires_at, - ) - - def _session_uses_postgresql(session: AsyncSession) -> bool: try: bind = session.get_bind() @@ -81,6 +64,7 @@ def _audit_event( *, scope_hash: str, decision: EmailSendRateLimitDecision, + observed_at: datetime.datetime, ) -> SecurityAuditEvent: return SecurityAuditEvent( actor_user_id=auth_context.user_id, @@ -91,6 +75,7 @@ def _audit_event( resource_type="email_send_rate_limit", resource_uid=f"email_send_scope:{scope_hash}", evidence_source="services.email_send_rate_limiter", + observed_at=observed_at, detail_text=( f"decision={decision.reason};" f"window_seconds={SEND_RATE_LIMIT_WINDOW_SECONDS};" @@ -100,80 +85,68 @@ def _audit_event( async def enforce_send_email_rate_limit( - session: AsyncSession, auth_context: AuthContext, *, now: datetime.datetime | None = None, ) -> EmailSendRateLimitDecision: - """Atomically reserve one send attempt in the shared PostgreSQL bucket. + """Atomically reserve one send attempt in a rolling PostgreSQL window. - PostgreSQL transaction advisory locking serializes the read/update/insert - decision across workers. A single row per scope plus ``expires_at`` keeps - attempts bounded; unavailable shared state fails closed instead of using a - process-local fallback. + A limiter-owned transaction prevents committing unrelated request work. + PostgreSQL advisory locking serializes the count-and-record decision across + workers; existing audit rows provide the exact rolling-window history. """ - if not _session_uses_postgresql(session): - raise EmailSendRateLimitUnavailable - observed_at = now or _utc_now() scope_hash = rate_limit_scope_hash( auth_context.user_id, auth_context.organization_id ) - try: - await session.execute( - select(func.pg_advisory_xact_lock(bindparam("lock_key"))), - {"lock_key": _lock_key(scope_hash)}, - ) - result = await session.execute( - select(EmailSendRateBucket) - .where(EmailSendRateBucket.bucket_scope_hash == scope_hash) - .with_for_update() - ) - bucket = result.scalar_one_or_none() - allowed, attempt_count, window_started_at, expires_at = _next_bucket_state( - bucket, observed_at - ) - if bucket is None: - session.add( - EmailSendRateBucket( - bucket_scope_hash=scope_hash, - window_started_at=window_started_at, - attempt_count=attempt_count, - expires_at=expires_at, - created_at=observed_at, - updated_at=observed_at, + async with AsyncSessionLocal() as session: + if not _session_uses_postgresql(session): + raise EmailSendRateLimitUnavailable + try: + await session.execute( + select(func.pg_advisory_xact_lock(bindparam("lock_key"))), + {"lock_key": _lock_key(scope_hash)}, + ) + window_started_at = observed_at - datetime.timedelta( + seconds=SEND_RATE_LIMIT_WINDOW_SECONDS + ) + result = await session.execute( + select(func.count()) + .select_from(SecurityAuditEvent) + .where( + SecurityAuditEvent.resource_uid + == f"email_send_scope:{scope_hash}", + SecurityAuditEvent.event_action + == "email_send_rate_limit.allowed", + SecurityAuditEvent.observed_at > window_started_at, ) ) - else: - bucket.window_started_at = window_started_at - bucket.attempt_count = attempt_count - bucket.expires_at = expires_at - bucket.updated_at = observed_at - - decision = EmailSendRateLimitDecision( - allowed=allowed, - reason="allowed" if allowed else "quota_exhausted", - ) - session.add( - _audit_event( - auth_context, - scope_hash=scope_hash, - decision=decision, + allowed = result.scalar_one() < SEND_RATE_LIMIT_MAX_ATTEMPTS + decision = EmailSendRateLimitDecision( + allowed=allowed, + reason="allowed" if allowed else "quota_exhausted", ) - ) - await session.commit() - return decision - except Exception as exc: - try: - await session.rollback() - except Exception as rollback_exc: + session.add( + _audit_event( + auth_context, + scope_hash=scope_hash, + decision=decision, + observed_at=observed_at, + ) + ) + await session.commit() + return decision + except Exception as exc: + try: + await session.rollback() + except Exception as rollback_exc: + logger.warning( + "Email send rate limiter rollback failed; error_type=%s", + type(rollback_exc).__name__, + ) logger.warning( - "Email send rate limiter rollback failed; error_type=%s", - type(rollback_exc).__name__, + "Email send rate limiter decision unavailable; " + "event_action=email_send_rate_limit.unavailable error_type=%s", + type(exc).__name__, ) - logger.warning( - "Email send rate limiter decision unavailable; " - "event_action=email_send_rate_limit.unavailable error_type=%s", - type(exc).__name__, - ) - raise EmailSendRateLimitUnavailable from exc + raise EmailSendRateLimitUnavailable from exc diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py index 65190ef1a..e573f6afd 100644 --- a/backend/tests/test_email_send_rate_limiter.py +++ b/backend/tests/test_email_send_rate_limiter.py @@ -5,7 +5,8 @@ import pytest from api.auth import AuthContext -from db.models import EmailSendRateBucket, SecurityAuditEvent +from db.models import SecurityAuditEvent +import services.email_send_rate_limiter as limiter_module from services.email_send_rate_limiter import ( EmailSendRateLimitUnavailable, enforce_send_email_rate_limit, @@ -14,16 +15,16 @@ class _Result: - def __init__(self, row=None): + def __init__(self, row=0): self.row = row - def scalar_one_or_none(self): + def scalar_one(self): return self.row -class _SharedBucketStore: +class _SharedAttemptStore: def __init__(self): - self.buckets = {} + self.events = [] self.lock = asyncio.Lock() @@ -33,6 +34,15 @@ def __init__(self, store): self.added = [] self.queries = [] self.lock_held = False + self.pending = [] + + async def __aenter__(self): + return self + + async def __aexit__(self, *_args): + if self.lock_held: + self.store.lock.release() + self.lock_held = False def get_bind(self): return SimpleNamespace(dialect=SimpleNamespace(name="postgresql")) @@ -44,26 +54,34 @@ async def execute(self, query, params=None): await self.store.lock.acquire() self.lock_held = True return _Result() - if "email_send_rate_buckets" in query_text: - scope_hash = next( - value - for key, value in query.compile().params.items() - if "bucket_scope_hash" in key + if "security_audit_events" in query_text: + values = tuple(query.compile().params.values()) + resource_uid = next(value for value in values if str(value).startswith("email_send_scope:")) + window_started_at = next(value for value in values if isinstance(value, datetime.datetime)) + return _Result( + sum( + event.resource_uid == resource_uid + and event.event_action == "email_send_rate_limit.allowed" + and event.observed_at > window_started_at + for event in self.store.events + ) ) - return _Result(self.store.buckets.get(scope_hash)) raise AssertionError(f"unexpected query: {query_text}") def add(self, item): self.added.append(item) - if isinstance(item, EmailSendRateBucket): - self.store.buckets[item.bucket_scope_hash] = item + if isinstance(item, SecurityAuditEvent): + self.pending.append(item) async def commit(self): + self.store.events.extend(self.pending) + self.pending.clear() if self.lock_held: self.store.lock.release() self.lock_held = False async def rollback(self): + self.pending.clear() if self.lock_held: self.store.lock.release() self.lock_held = False @@ -80,13 +98,14 @@ def _context(user_id="user-1", organization_id="org-1"): @pytest.mark.asyncio -async def test_shared_bucket_limits_concurrent_workers_without_cross_scope_leakage(): - store = _SharedBucketStore() +async def test_sliding_window_limits_concurrent_workers_without_cross_scope_leakage(monkeypatch): + store = _SharedAttemptStore() + monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: _PostgresSession(store)) observed_at = datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc) async def attempt(user_id): return await enforce_send_email_rate_limit( - _PostgresSession(store), _context(user_id), now=observed_at + _context(user_id), now=observed_at ) same_scope = await asyncio.gather(*(attempt("user-1") for _ in range(11))) @@ -97,32 +116,47 @@ async def attempt(user_id): assert all(decision.allowed for decision in other_scope) rollover = await enforce_send_email_rate_limit( - _PostgresSession(store), _context("user-1"), now=observed_at + datetime.timedelta(seconds=61), ) assert rollover.allowed is True - assert store.buckets[rate_limit_scope_hash("user-1", "org-1")].attempt_count == 1 + scope_uid = f'email_send_scope:{rate_limit_scope_hash("user-1", "org-1")}' + assert sum(event.resource_uid == scope_uid for event in store.events) == 12 + + +@pytest.mark.asyncio +async def test_sliding_window_blocks_boundary_burst(monkeypatch): + store = _SharedAttemptStore() + monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: _PostgresSession(store)) + started_at = datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc) + + for offset_seconds in range(50, 60): + decision = await enforce_send_email_rate_limit( + _context(), + now=started_at + datetime.timedelta(seconds=offset_seconds), + ) + assert decision.allowed is True + + blocked = await enforce_send_email_rate_limit( + _context(), + now=started_at + datetime.timedelta(seconds=61), + ) + assert blocked.allowed is False @pytest.mark.asyncio -async def test_shared_bucket_uses_transaction_lock_and_non_sensitive_audit_state(): - store = _SharedBucketStore() +async def test_limiter_uses_owned_transaction_and_non_sensitive_audit_state(monkeypatch): + store = _SharedAttemptStore() session = _PostgresSession(store) + monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: session) decision = await enforce_send_email_rate_limit( - session, _context(), now=datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc), ) assert decision.allowed is True assert any("pg_advisory_xact_lock" in query for query in session.queries) - assert any("for update" in query for query in session.queries) - bucket = next(item for item in session.added if isinstance(item, EmailSendRateBucket)) audit = next(item for item in session.added if isinstance(item, SecurityAuditEvent)) - assert bucket.bucket_scope_hash == rate_limit_scope_hash("user-1", "org-1") - assert "user-1" not in repr(bucket) - assert "org-1" not in repr(bucket) assert "user-1" not in repr(audit) assert "org-1" not in repr(audit) assert audit.event_action == "email_send_rate_limit.allowed" @@ -131,8 +165,19 @@ async def test_shared_bucket_uses_transaction_lock_and_non_sensitive_audit_state @pytest.mark.asyncio async def test_rate_limiter_fails_closed_when_shared_state_is_unavailable(): class _UnavailableSession: + async def __aenter__(self): + return self + + async def __aexit__(self, *_args): + return None + def get_bind(self): return SimpleNamespace(dialect=SimpleNamespace(name="sqlite")) - with pytest.raises(EmailSendRateLimitUnavailable): - await enforce_send_email_rate_limit(_UnavailableSession(), _context()) + original_factory = limiter_module.AsyncSessionLocal + limiter_module.AsyncSessionLocal = lambda: _UnavailableSession() + try: + with pytest.raises(EmailSendRateLimitUnavailable): + await enforce_send_email_rate_limit(_context()) + finally: + limiter_module.AsyncSessionLocal = original_factory From 395a8a87bc96cb1a4a7217f0ddc8eb01cf983393 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 08:41:40 +0900 Subject: [PATCH 05/18] fix(email): bound rate-limit denial audits --- backend/services/email_send_rate_limiter.py | 29 ++++++++++++++----- backend/tests/test_email_send_rate_limiter.py | 18 +++++++++++- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/backend/services/email_send_rate_limiter.py b/backend/services/email_send_rate_limiter.py index 041debed9..ad8ee1db4 100644 --- a/backend/services/email_send_rate_limiter.py +++ b/backend/services/email_send_rate_limiter.py @@ -126,14 +126,29 @@ async def enforce_send_email_rate_limit( allowed=allowed, reason="allowed" if allowed else "quota_exhausted", ) - session.add( - _audit_event( - auth_context, - scope_hash=scope_hash, - decision=decision, - observed_at=observed_at, + record_decision = allowed + if not allowed: + denied_result = await session.execute( + select(func.count()) + .select_from(SecurityAuditEvent) + .where( + SecurityAuditEvent.resource_uid + == f"email_send_scope:{scope_hash}", + SecurityAuditEvent.event_action + == "email_send_rate_limit.quota_exhausted", + SecurityAuditEvent.observed_at > window_started_at, + ) + ) + record_decision = denied_result.scalar_one() == 0 + if record_decision: + session.add( + _audit_event( + auth_context, + scope_hash=scope_hash, + decision=decision, + observed_at=observed_at, + ) ) - ) await session.commit() return decision except Exception as exc: diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py index e573f6afd..8ea6e36c0 100644 --- a/backend/tests/test_email_send_rate_limiter.py +++ b/backend/tests/test_email_send_rate_limiter.py @@ -57,11 +57,16 @@ async def execute(self, query, params=None): if "security_audit_events" in query_text: values = tuple(query.compile().params.values()) resource_uid = next(value for value in values if str(value).startswith("email_send_scope:")) + event_action = next( + value + for value in values + if str(value).startswith("email_send_rate_limit.") + ) window_started_at = next(value for value in values if isinstance(value, datetime.datetime)) return _Result( sum( event.resource_uid == resource_uid - and event.event_action == "email_send_rate_limit.allowed" + and event.event_action == event_action and event.observed_at > window_started_at for event in self.store.events ) @@ -142,6 +147,17 @@ async def test_sliding_window_blocks_boundary_burst(monkeypatch): now=started_at + datetime.timedelta(seconds=61), ) assert blocked.allowed is False + for offset_seconds in range(62, 72): + assert not ( + await enforce_send_email_rate_limit( + _context(), + now=started_at + datetime.timedelta(seconds=offset_seconds), + ) + ).allowed + assert sum( + event.event_action == "email_send_rate_limit.quota_exhausted" + for event in store.events + ) == 1 @pytest.mark.asyncio From bb7085ccc886284b47b0ce81253a53b7f12b356e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 08:42:19 +0900 Subject: [PATCH 06/18] fix(email): pin limiter transaction isolation --- backend/services/email_send_rate_limiter.py | 3 +++ backend/tests/test_email_send_rate_limiter.py | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/backend/services/email_send_rate_limiter.py b/backend/services/email_send_rate_limiter.py index ad8ee1db4..6606386eb 100644 --- a/backend/services/email_send_rate_limiter.py +++ b/backend/services/email_send_rate_limiter.py @@ -103,6 +103,9 @@ async def enforce_send_email_rate_limit( if not _session_uses_postgresql(session): raise EmailSendRateLimitUnavailable try: + await session.connection( + execution_options={"isolation_level": "READ COMMITTED"} + ) await session.execute( select(func.pg_advisory_xact_lock(bindparam("lock_key"))), {"lock_key": _lock_key(scope_hash)}, diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py index 8ea6e36c0..eee24ca05 100644 --- a/backend/tests/test_email_send_rate_limiter.py +++ b/backend/tests/test_email_send_rate_limiter.py @@ -35,6 +35,7 @@ def __init__(self, store): self.queries = [] self.lock_held = False self.pending = [] + self.isolation_level = None async def __aenter__(self): return self @@ -47,6 +48,10 @@ async def __aexit__(self, *_args): def get_bind(self): return SimpleNamespace(dialect=SimpleNamespace(name="postgresql")) + async def connection(self, *, execution_options): + self.isolation_level = execution_options["isolation_level"] + return self + async def execute(self, query, params=None): query_text = str(query).lower() self.queries.append(query_text) @@ -172,6 +177,7 @@ async def test_limiter_uses_owned_transaction_and_non_sensitive_audit_state(monk assert decision.allowed is True assert any("pg_advisory_xact_lock" in query for query in session.queries) + assert session.isolation_level == "READ COMMITTED" audit = next(item for item in session.added if isinstance(item, SecurityAuditEvent)) assert "user-1" not in repr(audit) assert "org-1" not in repr(audit) From 5a22a26ea370c15f8255f1e13c387a92bdcab68a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 11:08:02 +0900 Subject: [PATCH 07/18] fix(email): isolate shared send throttle Co-Authored-By: Codex Signed-off-by: Seongho Bae --- .cursor/environment.json | 20 ----- .cursor/install.sh | 42 ---------- .cursor/start.sh | 80 ------------------ AGENTS.md | 6 -- ARCHITECTURE.md | 6 -- CHANGELOG.md | 3 +- CLAUDE.md | 11 --- .../alembic/versions/0011_email_read_state.py | 18 +--- .../versions/0018_email_send_rate_buckets.py | 44 ---------- .../versions/0019_email_record_read_state.py | 62 -------------- backend/db/models.py | 35 +------- backend/scripts/bootstrap_db.py | 4 + .../scripts/reconcile_local_postgres_role.py | 82 ------------------- backend/services/email_import_service.py | 25 +----- backend/tests/test_alembic_migrations.py | 37 --------- backend/tests/test_bootstrap_db.py | 6 +- backend/tests/test_cloud_agent_environment.py | 38 --------- .../tests/test_email_import_quota_lock_key.py | 36 -------- backend/tests/test_email_send_rate_limiter.py | 68 +++++++++++++++ backend/tests/test_emails_api.py | 15 +--- .../test_reconcile_local_postgres_role.py | 66 --------------- docs/development/cloud-agent-environment.md | 71 ---------------- 22 files changed, 84 insertions(+), 691 deletions(-) delete mode 100644 .cursor/environment.json delete mode 100755 .cursor/install.sh delete mode 100755 .cursor/start.sh delete mode 100644 backend/alembic/versions/0018_email_send_rate_buckets.py delete mode 100644 backend/alembic/versions/0019_email_record_read_state.py delete mode 100644 backend/scripts/reconcile_local_postgres_role.py delete mode 100644 backend/tests/test_cloud_agent_environment.py delete mode 100644 backend/tests/test_email_import_quota_lock_key.py delete mode 100644 backend/tests/test_reconcile_local_postgres_role.py delete mode 100644 docs/development/cloud-agent-environment.md diff --git a/.cursor/environment.json b/.cursor/environment.json deleted file mode 100644 index 5964ed32d..000000000 --- a/.cursor/environment.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "name": "Naruon dev (backend + frontend + Postgres/pgvector)", - "user": "ubuntu", - "install": "bash .cursor/install.sh", - "start": "bash .cursor/start.sh", - "terminals": [ - { - "name": "backend", - "command": "cd backend && . .venv/bin/activate && python scripts/start_backend.py --host 0.0.0.0 --port 8000" - }, - { - "name": "frontend", - "command": "cd frontend && corepack pnpm@11.5.3 dev --port 3000 --hostname 0.0.0.0" - } - ], - "ports": [ - { "name": "backend", "port": 8000 }, - { "name": "frontend", "port": 3000 } - ] -} diff --git a/.cursor/install.sh b/.cursor/install.sh deleted file mode 100755 index 61db7ac03..000000000 --- a/.cursor/install.sh +++ /dev/null @@ -1,42 +0,0 @@ -#!/usr/bin/env bash -# Idempotent repository bootstrap for Naruon Cloud Agent environments. -# -# Prepares durable, source-derived state after checkout: -# * system packages (PostgreSQL 16 + pgvector, Python venv/build tooling) -# * the backend virtualenv + pinned Python requirements -# * the frontend pnpm dependency tree -# -# Per-boot service startup (Postgres, schema migrations, dev secrets) lives in -# .cursor/start.sh so it re-runs on every VM boot, including builds. -set -euo pipefail - -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -cd "$REPO_ROOT" - -echo "==> [install] system packages (postgresql-16, pgvector, python venv, build tools)" -export DEBIAN_FRONTEND=noninteractive -sudo apt-get update -qq -sudo apt-get install -y -qq \ - postgresql-16 \ - postgresql-16-pgvector \ - postgresql-client-16 \ - python3.12-venv \ - python3.12-dev \ - build-essential - -echo "==> [install] backend virtualenv + requirements" -cd "$REPO_ROOT/backend" -if [ ! -x ".venv/bin/python" ]; then - python3 -m venv .venv -fi -# shellcheck disable=SC1091 -. .venv/bin/activate -python -m pip install --require-hashes -r requirements-hashes.txt - -echo "==> [install] frontend dependencies (pnpm@11.5.3)" -cd "$REPO_ROOT/frontend" -corepack enable -corepack prepare pnpm@11.5.3 --activate -corepack pnpm@11.5.3 install --frozen-lockfile - -echo "==> [install] done" \ No newline at end of file diff --git a/.cursor/start.sh b/.cursor/start.sh deleted file mode 100755 index 523051f6a..000000000 --- a/.cursor/start.sh +++ /dev/null @@ -1,80 +0,0 @@ -#!/usr/bin/env bash -# Per-boot runtime reconciliation for Naruon Cloud Agent environments. -# -# Runs on every VM start (idempotent): brings up the PostgreSQL cluster, -# materializes a local dev .env with generated secrets on first boot, ensures -# the app database + pgvector extension exist, and applies Alembic migrations. -# -# Dependency installation lives in .cursor/install.sh; this script only -# reconciles per-boot state and then returns so the backend/frontend terminals -# can start. -set -euo pipefail - -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -cd "$REPO_ROOT" - -ENV_FILE="$HOME/.env" -PY="$REPO_ROOT/backend/.venv/bin/python" - -echo "==> [start] ensuring PostgreSQL 16 cluster is online" -if ! pg_lsclusters -h 2>/dev/null | awk '{print $4}' | grep -q online; then - sudo pg_ctlcluster 16 main start || true -fi -# Wait for the server to accept connections before touching it. -for _ in $(seq 1 30); do - if sudo -u postgres pg_isready -q; then break; fi - sleep 1 -done -if ! sudo -u postgres pg_isready -q; then - echo "==> [start] PostgreSQL did not become ready" >&2 - exit 1 -fi - -echo "==> [start] generating local dev .env on first boot (secrets are per-VM)" -if [ ! -f "$ENV_FILE" ]; then - "$PY" - "$ENV_FILE" <<'PYGEN' -import os, secrets, sys -from pathlib import Path -from cryptography.fernet import Fernet - -env_path = Path(sys.argv[1]) -db_password = secrets.token_urlsafe(24) -hmac_secret = secrets.token_urlsafe(48) -enc_key = Fernet.generate_key().decode() - -env_path.write_text( - "# Naruon local dev environment (generated per-VM; not committed).\n" - f"DATABASE_URL=postgresql+asyncpg://postgres:{db_password}@127.0.0.1:5432/ai_email\n" - f"AUTH_SESSION_HMAC_SECRET={hmac_secret}\n" - f"ENCRYPTION_KEY={enc_key}\n" - "DEBUG=false\n" - "RUNTIME_ENVIRONMENT=development\n" - "ENABLE_PROMETHEUS_METRICS=false\n" - "ALLOWED_CORS_ORIGINS=http://localhost:3000,http://127.0.0.1:3000," - "http://localhost:8000,http://127.0.0.1:8000\n" - "SMTP_MODE=simulated\n" - "OPENAI_API_KEY=\n" - "OPENAI_EMBEDDING_MODEL=text-embedding-3-small\n" - "OPENAI_MODEL=gpt-4o\n", - encoding="utf-8", -) -os.chmod(env_path, 0o600) -print(f"wrote {env_path}") -PYGEN -fi - -echo "==> [start] reconciling database role, database, and pgvector extension" -# Keep the local postgres role secret in sync with DATABASE_URL without -# interpolating it into SQL or the process argument list. -"$PY" "$REPO_ROOT/backend/scripts/reconcile_local_postgres_role.py" --env-file "$ENV_FILE" -if ! sudo -u postgres psql -tAc "SELECT 1 FROM pg_database WHERE datname='ai_email'" | grep -q 1; then - sudo -u postgres createdb ai_email -fi -sudo -u postgres psql -d ai_email -v ON_ERROR_STOP=1 \ - -c "CREATE EXTENSION IF NOT EXISTS vector;" >/dev/null - -echo "==> [start] applying database migrations (alembic upgrade head)" -cd "$REPO_ROOT/backend" -"$PY" scripts/migrate_db.py - -echo "==> [start] done" diff --git a/AGENTS.md b/AGENTS.md index a1f8b8c69..9104dd1f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -348,12 +348,6 @@ in this repo. embedding generation is unavailable. Tests must cover the local `embeddinggemma` path so Data workspace imports do not silently bypass the selected embedding model. -- PostgreSQL `text` / `hashtext()` cannot encode a NUL (`0x00`) octet. Do not - pass a raw `user_id\\x00organization_id` (or any other NUL-separated payload) - as a `pg_advisory_lock` bind value. Derive a NUL-free digest such as - SHA-256 hex and keep mocked/SQLite tests from asserting the raw NUL form — - those dialects skip the lock and hide `CharacterNotInRepertoireError` until - a real Postgres import. - Home/Today dashboard reply-wait surfaces must read signed `/api/emails/pending-replies` data instead of inferring pending replies from generic inbox fixtures or static copy. Tests and E2E mocks must verify the diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9e27c7ed1..9d2cbba18 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -145,12 +145,6 @@ remains a local/dev compatibility path that creates the `vector` extension, metadata-defined tables for fresh local databases, and idempotent backfills for existing local databases. -Cloud Agent VMs use the same managed migration path through -`.cursor/start.sh` → `scripts/migrate_db.py`. Local role-secret alignment is -stdin dollar-quoted SQL (`scripts/reconcile_local_postgres_role.py`); do not -interpolate `DATABASE_URL` secrets into `psql -c`. See -[`docs/development/cloud-agent-environment.md`](docs/development/cloud-agent-environment.md). - ## Send boundary Outbound replies preserve `In-Reply-To` and `References` headers in the built diff --git a/CHANGELOG.md b/CHANGELOG.md index f60936f0a..f20931f48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## [Unreleased] -- Cloud Agent `.cursor/start.sh`는 `DATABASE_URL` 역할 비밀을 `psql -c`에 보간하지 않고 `scripts/reconcile_local_postgres_role.py`의 dollar-quoted stdin으로 맞춥니다. `install.sh`는 `requirements-hashes.txt`를 `--require-hashes`로 설치합니다. 신선한 DB에서 폐기된 `emails` 테이블을 가정하던 `0011_email_read_state`는 테이블이 없으면 no-op이고, `0019_email_record_read_state`가 `0018_email_send_rate_buckets` 다음에 정식 `email_records.is_read DEFAULT true`를 가드로 맞춥니다. 가져오기 quota advisory lock 키는 NUL 없는 SHA-256 hex입니다. +- 이메일 발송 제한을 모든 worker가 공유하는 PostgreSQL rolling window로 + 적용하고, 제한 상태를 확인할 수 없으면 발송 전에 안전하게 중단합니다. - 긴 이메일·첨부 본문을 의미 단위 청크로 임베딩한 뒤 기존 email/attachment 벡터 계약으로 평균화하고, 청크 요청·벡터 누적을 제한된 창으로 처리합니다. OpenAI `text-embedding-3-*`에는 저장 차원(`1536`)을 직접 요청하도록 보강했습니다. 합성 메일 fixture 5건(70청크)과 provider 요청 계약으로 1,536차원 벡터 경로를 검증했으며, 실행 시 선택한 임베딩 제공자에 본문·파싱된 첨부 텍스트를 전송할 수 있습니다. 회사 기밀 데이터는 fixture·commit·PR·log에 포함하지 않습니다. - EmailDetail 테스트가 지원하지 않는 스레드 병합/분리 버튼을 `textContent`뿐 아니라 `aria-label`과 `title` 접근 가능 이름으로도 검출하도록 바꿔, 아이콘 전용 버튼 회귀를 놓치지 않습니다. diff --git a/CLAUDE.md b/CLAUDE.md index f68a237f6..be67bc80c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,17 +98,6 @@ The other compose files are purpose-specific evaluation/evidence stacks: defaults anywhere; Compose/Kubernetes/operators must inject them, and startup fails closed when they are missing. -### Cloud Agent environment (`.cursor/`) - -Repo-managed Cloud Agent bootstrap lives in `.cursor/environment.json`. -`install.sh` installs PostgreSQL 16/pgvector and hashed Python deps; -`start.sh` brings Postgres online, mints a per-VM `~/.env`, and runs -`scripts/migrate_db.py`. Role-secret sync goes through -`backend/scripts/reconcile_local_postgres_role.py` (dollar-quoted stdin, never -`psql -c` interpolation). After boot, open http://127.0.0.1:3000 and sign in -with a minted HMAC session — see -`docs/development/cloud-agent-environment.md`. - ## Architecture Naruon is an AI email workspace: a web client/control plane over diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index bbe4490f9..716590cd1 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -14,19 +14,6 @@ def upgrade() -> None: - # ``emails`` was the pre-reconciliation email table. ``email_records`` is now - # the single source of truth and already declares ``is_read`` via the model - # metadata created in 0001, so this legacy column add only applies to older - # databases that still carry the retired ``emails`` table. Guard on the table - # existing (matching the has_table/has_column pattern used by later - # revisions) so ``alembic upgrade head`` succeeds on fresh databases. - bind = op.get_bind() - inspector = sa.inspect(bind) - if not inspector.has_table("emails"): - return - existing_columns = {column["name"] for column in inspector.get_columns("emails")} - if "is_read" in existing_columns: - return op.add_column( "emails", sa.Column( @@ -39,7 +26,4 @@ def upgrade() -> None: def downgrade() -> None: - # No-op: this revision only reconciles a retired ``emails`` table. Dropping - # ``is_read`` when the column is present would also remove a pre-existing - # column this revision did not create. - return + op.drop_column("emails", "is_read") diff --git a/backend/alembic/versions/0018_email_send_rate_buckets.py b/backend/alembic/versions/0018_email_send_rate_buckets.py deleted file mode 100644 index c7bace207..000000000 --- a/backend/alembic/versions/0018_email_send_rate_buckets.py +++ /dev/null @@ -1,44 +0,0 @@ -"""add shared email send rate-limit buckets - -Revision ID: 0018_email_send_rate_buckets -Revises: 0017_merge_newsdom_carddav_heads -Create Date: 2026-08-19 00:00:00.000000 -""" - -from alembic import op -import sqlalchemy as sa - -revision = "0018_email_send_rate_buckets" -down_revision = "0017_merge_newsdom_carddav_heads" -branch_labels = None -depends_on = None - -_BUCKET_TABLE = "email_send_rate_buckets" -_EXPIRY_INDEX = "ix_email_send_rate_buckets_expires_at" - - -def upgrade() -> None: - connection = op.get_bind() - inspector = sa.inspect(connection) - if not inspector.has_table(_BUCKET_TABLE): - op.create_table( - _BUCKET_TABLE, - sa.Column("bucket_scope_hash", sa.String(length=64), nullable=False), - sa.Column("window_started_at", sa.DateTime(timezone=True), nullable=False), - sa.Column("attempt_count", sa.Integer(), nullable=False), - sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), - sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), - sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), - sa.PrimaryKeyConstraint("bucket_scope_hash"), - ) - op.create_index( - _EXPIRY_INDEX, - _BUCKET_TABLE, - ["expires_at"], - if_not_exists=True, - ) - - -def downgrade() -> None: - op.drop_index(_EXPIRY_INDEX, table_name=_BUCKET_TABLE, if_exists=True) - op.drop_table(_BUCKET_TABLE, if_exists=True) diff --git a/backend/alembic/versions/0019_email_record_read_state.py b/backend/alembic/versions/0019_email_record_read_state.py deleted file mode 100644 index 6e35cbb82..000000000 --- a/backend/alembic/versions/0019_email_record_read_state.py +++ /dev/null @@ -1,62 +0,0 @@ -"""Guard email_records.is_read with NOT NULL DEFAULT true. - -Revision ID: 0019_email_record_read_state -Revises: 0018_email_send_rate_buckets - -``0011_email_read_state`` only mutates the retired ``emails`` table. Fresh -databases already receive ``email_records.is_read`` from ``0001`` -``create_all`` plus the current model ``server_default``. Existing databases -whose ``email_records`` row predates that column (or lacks a server default) -still need a guarded additive revision so raw INSERTs that omit ``is_read`` -succeed. -""" - -from __future__ import annotations - -from alembic import op -import sqlalchemy as sa - -revision = "0019_email_record_read_state" -down_revision = "0018_email_send_rate_buckets" -branch_labels = None -depends_on = None - -_EMAIL_RECORDS_TABLE = "email_records" -_READ_STATE_COLUMN = "is_read" - - -def upgrade() -> None: - bind = op.get_bind() - inspector = sa.inspect(bind) - if not inspector.has_table(_EMAIL_RECORDS_TABLE): - return - columns = { - column["name"]: column - for column in inspector.get_columns(_EMAIL_RECORDS_TABLE) - } - if _READ_STATE_COLUMN not in columns: - op.add_column( - _EMAIL_RECORDS_TABLE, - sa.Column( - _READ_STATE_COLUMN, - sa.Boolean(), - nullable=False, - server_default=sa.text("true"), - ), - ) - return - if columns[_READ_STATE_COLUMN].get("default") is None: - op.alter_column( - _EMAIL_RECORDS_TABLE, - _READ_STATE_COLUMN, - existing_type=sa.Boolean(), - existing_nullable=False, - server_default=sa.text("true"), - ) - - -def downgrade() -> None: - # No-op: ``create_all`` and later model metadata may already own this - # column. Dropping it would remove a default this revision did not - # necessarily create. - return diff --git a/backend/db/models.py b/backend/db/models.py index 40db8ea00..98e17eef2 100644 --- a/backend/db/models.py +++ b/backend/db/models.py @@ -18,7 +18,6 @@ Text, UniqueConstraint, func, - text, ) from sqlalchemy.orm import declarative_base, Mapped, mapped_column, relationship from sqlalchemy.types import TypeDecorator @@ -804,12 +803,7 @@ def owner_filters(cls, user_id: str, organization_id: str | None): date: Mapped[datetime.datetime] = mapped_column(DateTime(timezone=True), index=True) body: Mapped[str] = mapped_column(Text) # IMAP \Seen read state; defaults read so historical/file imports don't nag. - # A DB-level server_default keeps create_all/bootstrap_db consistent with the - # 0011_email_read_state migration intent so raw inserts that omit is_read - # (e.g. postgres smoke seeds) don't hit a NOT NULL violation. - is_read: Mapped[bool] = mapped_column( - Boolean, nullable=False, default=True, server_default=text("true") - ) + is_read: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) # Defer large pgvector payloads on default entity loads. embedding = mapped_column(Vector(1536), deferred=True) attachments: Mapped[list["Attachment"]] = relationship( @@ -829,33 +823,6 @@ def owner_filters(cls, user_id: str, organization_id: str | None): ) -class EmailSendRateBucket(Base): - __tablename__ = "email_send_rate_buckets" - - bucket_scope_hash: Mapped[str] = mapped_column(String(64), primary_key=True) - window_started_at: Mapped[datetime.datetime] = mapped_column( - DateTime(timezone=True), nullable=False - ) - attempt_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0) - expires_at: Mapped[datetime.datetime] = mapped_column( - DateTime(timezone=True), nullable=False - ) - created_at: Mapped[datetime.datetime] = mapped_column( - DateTime(timezone=True), - default=lambda: datetime.datetime.now(datetime.timezone.utc), - nullable=False, - ) - updated_at: Mapped[datetime.datetime] = mapped_column( - DateTime(timezone=True), - default=lambda: datetime.datetime.now(datetime.timezone.utc), - nullable=False, - ) - - __table_args__ = ( - Index("ix_email_send_rate_buckets_expires_at", "expires_at"), - ) - - class TicketTask(Base): __tablename__ = "ticket_tasks" diff --git a/backend/scripts/bootstrap_db.py b/backend/scripts/bootstrap_db.py index 3e579a053..1047103e8 100644 --- a/backend/scripts/bootstrap_db.py +++ b/backend/scripts/bootstrap_db.py @@ -186,6 +186,10 @@ def _get_create_indexes_statements() -> list[Executable]: "CREATE INDEX IF NOT EXISTS ix_email_records_owner_date " "ON email_records (user_id, organization_id, date)" ), + text( + "CREATE INDEX IF NOT EXISTS ix_emails_owner_date " + "ON emails (user_id, organization_id, date)" + ), text( "CREATE INDEX IF NOT EXISTS ix_sender_relationships_owner_source " "ON sender_relationships " diff --git a/backend/scripts/reconcile_local_postgres_role.py b/backend/scripts/reconcile_local_postgres_role.py deleted file mode 100644 index 1c652baad..000000000 --- a/backend/scripts/reconcile_local_postgres_role.py +++ /dev/null @@ -1,82 +0,0 @@ -"""Reconcile the local Cloud Agent Postgres role secret without SQL interpolation. - -Cloud Agent ``start.sh`` keeps the ``postgres`` role aligned with the -generated ``DATABASE_URL``. The secret must travel only on ``psql`` stdin as -dollar-quoted SQL so a quote or backslash in an existing ``~/.env`` cannot -break out of ``ALTER USER ... PASSWORD`` and does not appear on the process -command line. -""" - -from __future__ import annotations - -import argparse -import secrets -import subprocess # nosec B404 -- fixed executable argv, shell=False, secret on stdin. -import sys -from collections.abc import Callable -from pathlib import Path -from typing import Any -from urllib.parse import unquote, urlsplit - -INCOMPLETE_LOCAL_DATABASE_CONFIG = "local database configuration is incomplete" -_ROLE_NAME = "postgres" - - -def database_url_password(env_text: str) -> str: - """Return the ``DATABASE_URL`` user secret, or fail closed when it is absent.""" - for line in env_text.splitlines(): - if line.startswith("DATABASE_URL="): - raw_url = line.split("=", 1)[1].strip() - secret = unquote(urlsplit(raw_url).password or "") - if not secret: - raise ValueError(INCOMPLETE_LOCAL_DATABASE_CONFIG) - return secret - raise ValueError(INCOMPLETE_LOCAL_DATABASE_CONFIG) - - -def build_alter_role_sql(secret: str, *, role_name: str = _ROLE_NAME) -> str: - """Build ``ALTER USER`` SQL that dollar-quotes ``secret``. - - The tag is regenerated until it is absent from the secret so the closer - cannot appear inside the quoted value. - """ - if not secret: - raise ValueError(INCOMPLETE_LOCAL_DATABASE_CONFIG) - tag = f"naruon{secrets.token_hex(16)}" - while tag in secret: - tag = f"naruon{secrets.token_hex(16)}" - return f"ALTER USER {role_name} WITH PASSWORD ${tag}${secret}${tag};\n" - - -def reconcile_local_postgres_role( - secret: str, - *, - runner: Callable[..., Any] = subprocess.run, -) -> None: - """Apply the role secret through ``psql`` stdin, never ``psql -c``.""" - sql = build_alter_role_sql(secret) - runner( - ["sudo", "-u", "postgres", "psql", "-v", "ON_ERROR_STOP=1"], - input=sql, - text=True, - check=True, - stdout=subprocess.DEVNULL, - ) - - -def main(argv: list[str] | None = None) -> int: - """Read an env file and align the local ``postgres`` role secret.""" - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument( - "--env-file", - required=True, - help="Path to the local env file that already contains DATABASE_URL.", - ) - args = parser.parse_args(argv) - env_text = Path(args.env_file).read_text(encoding="utf-8") - reconcile_local_postgres_role(database_url_password(env_text)) - return 0 - - -if __name__ == "__main__": - sys.exit(main()) \ No newline at end of file diff --git a/backend/services/email_import_service.py b/backend/services/email_import_service.py index d62af9626..ddfa350fd 100644 --- a/backend/services/email_import_service.py +++ b/backend/services/email_import_service.py @@ -251,27 +251,6 @@ def _session_uses_postgresql(session: AsyncSession) -> bool: return getattr(getattr(bind, "dialect", None), "name", None) == "postgresql" -def _owner_import_quota_lock_key(user_id: str, organization_id: str) -> str: - """Return a NUL-free SHA-256 digest for ``pg_advisory_lock(hashtext(...))``. - - PostgreSQL ``text`` cannot store a 0x00 octet, so passing - ``f"{user_id}\\x00{organization_id}"`` to ``hashtext()`` raises - ``CharacterNotInRepertoireError`` on real Postgres (PostgreSQL Global - Development Group, n.d.). Hash the NUL-separated payload instead and bind - only the hex digest. - - References - ---------- - PostgreSQL Global Development Group. (n.d.). *Character set support*. - PostgreSQL Documentation. - https://www.postgresql.org/docs/current/multibyte.html - """ - payload = "\x00".join((user_id, organization_id)) - return hashlib.sha256( - payload.encode("utf-8", errors="surrogatepass") - ).hexdigest() - - async def _acquire_owner_import_quota_lock( session: AsyncSession, *, user_id: str, organization_id: str ) -> bool: @@ -279,7 +258,7 @@ async def _acquire_owner_import_quota_lock( return False lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, - "owner_key": _owner_import_quota_lock_key(user_id, organization_id), + "owner_key": f"{user_id}\x00{organization_id}", } await session.execute( select( @@ -298,7 +277,7 @@ async def _release_owner_import_quota_lock( ) -> None: lock_params = { "namespace_key": EMAIL_IMPORT_QUOTA_LOCK_NAMESPACE, - "owner_key": _owner_import_quota_lock_key(user_id, organization_id), + "owner_key": f"{user_id}\x00{organization_id}", } await session.execute( select( diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index f286b7259..f8f3ffeae 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -438,43 +438,6 @@ def test_merge_revision_reconciles_newsdom_provider_branch(): assert "op.drop_column(" not in revision_text -def test_email_read_state_revision_is_retired_emails_table_only() -> None: - revision_path = BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" - revision_text = revision_path.read_text() - - assert 'revision = "0011_email_read_state"' in revision_text - assert 'down_revision = "0009_project_graph_projection"' in revision_text - assert 'has_table("emails")' in revision_text - assert 'op.add_column(\n "emails"' in revision_text - assert 'op.add_column(\n "email_records"' not in revision_text - # Downgrade must not drop a pre-existing emails.is_read this revision did - # not create. Retired-table reconciliation is a documented no-op. - assert "op.drop_column(" not in revision_text - - -def test_email_record_read_state_revision_guards_canonical_table() -> None: - revision_path = ( - BACKEND_ROOT / "alembic" / "versions" / "0019_email_record_read_state.py" - ) - assert revision_path.exists() - revision_text = revision_path.read_text() - - assert 'revision = "0019_email_record_read_state"' in revision_text - assert 'down_revision = "0018_email_send_rate_buckets"' in revision_text - assert len("0019_email_record_read_state") <= 32 - assert '_EMAIL_RECORDS_TABLE = "email_records"' in revision_text - assert "has_table(_EMAIL_RECORDS_TABLE)" in revision_text - assert "_READ_STATE_COLUMN" in revision_text - assert '"is_read"' in revision_text - assert "server_default=sa.text(\"true\")" in revision_text or ( - 'server_default=sa.text("true")' in revision_text - ) - assert "op.add_column(" in revision_text - assert "sa.text(f" not in revision_text - # Additive default only; do not drop a column create_all may already own. - assert "op.drop_column(" not in revision_text - - def test_merge_revision_reconciles_newsdom_document_and_carddav_heads(): revision_path = ( BACKEND_ROOT / "alembic" / "versions" / "0017_merge_newsdom_carddav_heads.py" diff --git a/backend/tests/test_bootstrap_db.py b/backend/tests/test_bootstrap_db.py index c5cf73e10..5af0540f0 100644 --- a/backend/tests/test_bootstrap_db.py +++ b/backend/tests/test_bootstrap_db.py @@ -138,8 +138,6 @@ def test_schema_backfill_adds_email_indexes(monkeypatch): "create index if not exists ix_email_records_date" in statement for statement in statements ) - assert all(" on emails " not in statement for statement in statements) - assert all("ix_emails_owner_date" not in statement for statement in statements) def test_schema_backfill_adds_llm_provider_columns_and_indexes(monkeypatch): @@ -772,11 +770,11 @@ async def test_connector_signal_events_real_postgres_bootstrap_smoke(): text(""" INSERT INTO email_records ( user_id, organization_id, message_id, sender, recipients, - subject, "date", body, is_read + subject, "date", body ) VALUES ( :user_id, :organization_id, :message_id, :sender, - :recipients, :subject, now(), :body, true + :recipients, :subject, now(), :body ) RETURNING id """), diff --git a/backend/tests/test_cloud_agent_environment.py b/backend/tests/test_cloud_agent_environment.py deleted file mode 100644 index a6c0a0b26..000000000 --- a/backend/tests/test_cloud_agent_environment.py +++ /dev/null @@ -1,38 +0,0 @@ -"""Source contracts for the repo-managed Cloud Agent environment scripts.""" - -from __future__ import annotations - -from pathlib import Path - -REPO_ROOT = Path(__file__).resolve().parents[2] -INSTALL_SH = (REPO_ROOT / ".cursor" / "install.sh").read_text(encoding="utf-8") -START_SH = (REPO_ROOT / ".cursor" / "start.sh").read_text(encoding="utf-8") -ENVIRONMENT_JSON = (REPO_ROOT / ".cursor" / "environment.json").read_text( - encoding="utf-8" -) - - -def test_install_sh_pins_python_requirements_with_hashes() -> None: - assert "requirements-hashes.txt" in INSTALL_SH - assert "--require-hashes" in INSTALL_SH - assert "pip install -r requirements.txt" not in INSTALL_SH - assert "pip install --upgrade pip" not in INSTALL_SH - - -def test_start_sh_does_not_interpolate_role_secret_into_sql() -> None: - assert "PASSWORD '${DB_PASSWORD}'" not in START_SH - assert "ALTER USER postgres WITH PASSWORD '" not in START_SH - assert "reconcile_local_postgres_role.py" in START_SH - - -def test_start_sh_fails_closed_when_postgres_never_becomes_ready() -> None: - assert "pg_isready" in START_SH - assert "did not become ready" in START_SH - assert "exit 1" in START_SH - - -def test_environment_json_keeps_servers_in_terminals_not_install() -> None: - assert "bash .cursor/install.sh" in ENVIRONMENT_JSON - assert "bash .cursor/start.sh" in ENVIRONMENT_JSON - assert "scripts/start_backend.py" in ENVIRONMENT_JSON - assert "corepack pnpm@11.5.3 dev" in ENVIRONMENT_JSON \ No newline at end of file diff --git a/backend/tests/test_email_import_quota_lock_key.py b/backend/tests/test_email_import_quota_lock_key.py deleted file mode 100644 index 856b16d1b..000000000 --- a/backend/tests/test_email_import_quota_lock_key.py +++ /dev/null @@ -1,36 +0,0 @@ -"""Contract tests for the Postgres import-quota advisory-lock owner key. - -PostgreSQL ``text`` / ``hashtext()`` cannot store a NUL (0x00) octet -(PostgreSQL Global Development Group, n.d.). The production helper must -therefore emit a NUL-free digest. The expected digest is computed here with -the standard-library hasher so a helper rewrite cannot silently change the -on-disk lock identity. -""" - -from __future__ import annotations - -import hashlib - -from services.email_import_service import _owner_import_quota_lock_key - -# Independent reference: SHA-256 of UTF-8 ``user_id + NUL + organization_id``. -_GOLDEN_TESTUSER_ORG_ACME = "3fbc5671f32a1608f88c1775c1008c26c53faaea0308b97c556eeceb2b4bb8d3" - - -def test_owner_import_quota_lock_key_matches_independent_sha256_digest() -> None: - independent = hashlib.sha256(b"testuser\x00org-acme").hexdigest() - assert independent == _GOLDEN_TESTUSER_ORG_ACME - assert _owner_import_quota_lock_key("testuser", "org-acme") == independent - assert "\x00" not in independent - - -def test_owner_import_quota_lock_key_is_nul_free_for_unicode_owners() -> None: - user_id = "유저" - organization_id = "org-서울" - independent = hashlib.sha256( - f"{user_id}\x00{organization_id}".encode("utf-8") - ).hexdigest() - actual = _owner_import_quota_lock_key(user_id, organization_id) - assert actual == independent - assert "\x00" not in actual - assert len(actual) == 64 diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py index eee24ca05..6dea7d6ee 100644 --- a/backend/tests/test_email_send_rate_limiter.py +++ b/backend/tests/test_email_send_rate_limiter.py @@ -203,3 +203,71 @@ def get_bind(self): await enforce_send_email_rate_limit(_context()) finally: limiter_module.AsyncSessionLocal = original_factory + + +@pytest.mark.postgres +@pytest.mark.asyncio +async def test_rate_limiter_real_postgres_reserves_and_denies(monkeypatch): + from core.config import settings + from asyncpg.exceptions import InvalidAuthorizationSpecificationError + from asyncpg.exceptions import InvalidPasswordError + from db.models import Base + from sqlalchemy import delete, select + from sqlalchemy.exc import OperationalError + from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + + engine = create_async_engine(settings.DATABASE_URL) + try: + async with engine.begin() as connection: + await connection.exec_driver_sql("CREATE EXTENSION IF NOT EXISTS vector") + await connection.run_sync(Base.metadata.create_all) + except ( + InvalidAuthorizationSpecificationError, + InvalidPasswordError, + OperationalError, + OSError, + ): + await engine.dispose() + pytest.skip("PostgreSQL smoke database unavailable") + + session_factory = async_sessionmaker(engine, expire_on_commit=False) + monkeypatch.setattr(limiter_module, "AsyncSessionLocal", session_factory) + monkeypatch.setattr(limiter_module, "SEND_RATE_LIMIT_MAX_ATTEMPTS", 1) + context = _context("rate-limit-smoke-user", "rate-limit-smoke-org") + scope_uid = ( + "email_send_scope:" + f"{rate_limit_scope_hash(context.user_id, context.organization_id)}" + ) + observed_at = datetime.datetime.now(datetime.timezone.utc) + + async def cleanup() -> None: + async with session_factory() as session: + await session.execute( + delete(SecurityAuditEvent).where( + SecurityAuditEvent.resource_uid == scope_uid + ) + ) + await session.commit() + + await cleanup() + try: + first = await enforce_send_email_rate_limit(context, now=observed_at) + second = await enforce_send_email_rate_limit(context, now=observed_at) + async with session_factory() as session: + actions = ( + await session.execute( + select(SecurityAuditEvent.event_action) + .where(SecurityAuditEvent.resource_uid == scope_uid) + .order_by(SecurityAuditEvent.event_action) + ) + ).scalars().all() + finally: + await cleanup() + await engine.dispose() + + assert first.allowed is True + assert second.allowed is False + assert actions == [ + "email_send_rate_limit.allowed", + "email_send_rate_limit.quota_exhausted", + ] diff --git a/backend/tests/test_emails_api.py b/backend/tests/test_emails_api.py index c0678696a..3c06363b7 100644 --- a/backend/tests/test_emails_api.py +++ b/backend/tests/test_emails_api.py @@ -22,7 +22,6 @@ import datetime from unittest.mock import AsyncMock, patch from services.embedding import STORAGE_EMBEDDING_DIMENSION -from services.email_import_service import _owner_import_quota_lock_key from services.email_service import generate_email_fingerprint from services.email_send_rate_limiter import ( EmailSendRateLimitDecision, @@ -1282,18 +1281,14 @@ async def test_import_email_files_serializes_quota_with_postgres_owner_lock( assert "pg_advisory_unlock" in advisory_queries[-1] assert "hashtext(:namespace_key)" in advisory_queries[0] assert ":owner_key" in advisory_queries[0] - # PostgreSQL text/hashtext cannot encode NUL (0x00); the owner key must be a - # NUL-free digest, not a raw ``user\x00org`` separator. - expected_owner_key = _owner_import_quota_lock_key("testuser", "org-acme") - assert "\x00" not in expected_owner_key assert advisory_query_params(session) == [ { "namespace_key": "naruon-email-import-quota", - "owner_key": expected_owner_key, + "owner_key": "testuser\x00org-acme", }, { "namespace_key": "naruon-email-import-quota", - "owner_key": expected_owner_key, + "owner_key": "testuser\x00org-acme", }, ] @@ -1349,16 +1344,14 @@ async def test_import_email_files_rejects_when_owner_quota_is_exhausted( advisory_queries = advisory_query_texts(session) assert "pg_advisory_lock" in advisory_queries[0] assert "pg_advisory_unlock" in advisory_queries[-1] - expected_owner_key = _owner_import_quota_lock_key("testuser", "org-acme") - assert "\x00" not in expected_owner_key assert advisory_query_params(session) == [ { "namespace_key": "naruon-email-import-quota", - "owner_key": expected_owner_key, + "owner_key": "testuser\x00org-acme", }, { "namespace_key": "naruon-email-import-quota", - "owner_key": expected_owner_key, + "owner_key": "testuser\x00org-acme", }, ] diff --git a/backend/tests/test_reconcile_local_postgres_role.py b/backend/tests/test_reconcile_local_postgres_role.py deleted file mode 100644 index 2a31bfa46..000000000 --- a/backend/tests/test_reconcile_local_postgres_role.py +++ /dev/null @@ -1,66 +0,0 @@ -"""Tests for Cloud Agent local-Postgres role password reconciliation. - -The start path must reject an empty role secret and must never interpolate -that secret into a ``psql -c`` SQL string or process argv. -""" - -from __future__ import annotations - -import pytest - -from scripts.reconcile_local_postgres_role import ( - build_alter_role_sql, - database_url_password, - reconcile_local_postgres_role, -) - - -def test_database_url_password_rejects_empty_secret() -> None: - with pytest.raises(ValueError, match="local database configuration is incomplete"): - database_url_password("DATABASE_URL=postgresql+asyncpg://postgres@127.0.0.1:5432/ai_email\n") - - -def test_database_url_password_rejects_missing_url() -> None: - with pytest.raises(ValueError, match="local database configuration is incomplete"): - database_url_password("DEBUG=false\n") - - -def test_database_url_password_unquotes_url_encoded_secret() -> None: - assert ( - database_url_password( - "DATABASE_URL=postgresql+asyncpg://postgres:a%27b@127.0.0.1:5432/ai_email\n" - ) - == "a'b" - ) - - -def test_build_alter_role_sql_dollar_quotes_metacharacters() -> None: - secret = "x'; DROP ROLE postgres; --" - sql = build_alter_role_sql(secret) - assert "DROP ROLE postgres" in sql - assert "PASSWORD '" not in sql - assert sql.startswith("ALTER USER postgres WITH PASSWORD $") - assert sql.endswith(";\n") or sql.endswith(";") - - -def test_reconcile_local_postgres_role_keeps_secret_off_argv() -> None: - captured: dict[str, object] = {} - - def fake_run(argv, **kwargs): - captured["argv"] = list(argv) - captured["input"] = kwargs.get("input") - captured["check"] = kwargs.get("check") - - class _Completed: - returncode = 0 - - return _Completed() - - secret = "quote'me;and\\back" - reconcile_local_postgres_role(secret, runner=fake_run) - argv = captured["argv"] - assert isinstance(argv, list) - assert all(secret not in str(part) for part in argv) - assert "-c" not in argv - assert secret in str(captured["input"]) - assert captured["check"] is True diff --git a/docs/development/cloud-agent-environment.md b/docs/development/cloud-agent-environment.md deleted file mode 100644 index ede732910..000000000 --- a/docs/development/cloud-agent-environment.md +++ /dev/null @@ -1,71 +0,0 @@ -# Cloud Agent development environment - -Use this when a Cloud Agent or a fresh Ubuntu workstation needs a running -Naruon control plane (FastAPI + Next.js + PostgreSQL 16/pgvector) without -Docker Compose. - -## Next action - -1. Confirm `.cursor/environment.json` is the environment source for the VM. -2. After `start.sh` exits, open http://127.0.0.1:3000 (frontend) and - http://127.0.0.1:8000/ (backend health). -3. Mint a member HMAC session against the generated `AUTH_SESSION_HMAC_SECRET` - in `~/.env`, then import `.eml` fixtures through - `POST /api/emails/import-files`. -4. If Postgres is down, re-run `bash .cursor/start.sh` — do not edit secrets - into SQL by hand. - -## Install versus start - -| Script | Lifetime | What to put here | -| --- | --- | --- | -| `.cursor/install.sh` | Durable, source-derived | `postgresql-16` + pgvector, hashed `requirements-hashes.txt`, `pnpm@11.5.3 --frozen-lockfile` | -| `.cursor/start.sh` | Every boot | Postgres cluster, per-VM `~/.env`, `ai_email` + `vector`, `alembic upgrade head` | -| `environment.json` terminals | Long-running | `scripts/start_backend.py` and `pnpm dev` | - -`install.sh` must use `python -m pip install --require-hashes -r requirements-hashes.txt`. -Unhashed `requirements.txt` is not an acceptable Cloud Agent supply-chain path. - -## Secret handling - -`start.sh` writes `~/.env` only when the file is missing. Generated values use -`secrets.token_urlsafe(48)` for `AUTH_SESSION_HMAC_SECRET` and -`Fernet.generate_key()` for `ENCRYPTION_KEY`, matching -`validate_auth_session_hmac_secret_value`. - -The `postgres` role secret is applied by -`backend/scripts/reconcile_local_postgres_role.py`: - -- empty `DATABASE_URL` user secrets fail closed -- the secret is dollar-quoted on `psql` stdin -- the secret never appears in `psql -c` or process argv - -Do not mount this `~/.env` as a Compose `env_file`. Compose interpolation still -resolves `NARUON_ENV_FILE` > `~/.env` > `./.env` without leaking the file into -the container environment wholesale. - -## Schema contract on a fresh database - -`0001_initial_control_plane` runs `Base.metadata.create_all`, so -`email_records.is_read` is present with `DEFAULT true` on a current model. -`0011_email_read_state` only touches the retired `emails` table and is a no-op -when that table is absent. `0019_email_record_read_state` follows the shared -`0018_email_send_rate_buckets` migration and adds or defaults `email_records.is_read` -on older databases that already have `email_records` but lack the column or its -server default. - -## Import quota lock - -Owner import serialization uses `pg_advisory_lock(hashtext(namespace), hashtext(owner_key))`. -PostgreSQL `text` cannot store a NUL octet, so the owner key is a SHA-256 hex -digest of `user_id + 0x00 + organization_id`, not the raw NUL-separated string -(PostgreSQL Global Development Group, n.d.). - -## References - -PostgreSQL Global Development Group. (n.d.). *Character set support*. -PostgreSQL Documentation. https://www.postgresql.org/docs/current/multibyte.html - -PostgreSQL Global Development Group. (n.d.). *psql — PostgreSQL interactive -terminal*. PostgreSQL Documentation. -https://www.postgresql.org/docs/current/app-psql.html From e22fa780ce303428caf749fef1efe538fb156b35 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 15:01:10 +0900 Subject: [PATCH 08/18] fix(email): isolate send limits by workspace --- backend/services/email_send_rate_limiter.py | 12 +++++++--- backend/tests/test_email_send_rate_limiter.py | 22 +++++++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/backend/services/email_send_rate_limiter.py b/backend/services/email_send_rate_limiter.py index 6606386eb..09e59e012 100644 --- a/backend/services/email_send_rate_limiter.py +++ b/backend/services/email_send_rate_limiter.py @@ -36,10 +36,14 @@ class EmailSendRateLimitDecision: reason: Literal["allowed", "quota_exhausted"] -def rate_limit_scope_hash(user_id: str, organization_id: str | None) -> str: +def rate_limit_scope_hash( + user_id: str, organization_id: str | None, workspace_id: str +) -> str: """Return a non-reversible identifier for one authorized send scope.""" organization_scope = organization_id or "" - value = f"{SEND_RATE_LIMIT_NAMESPACE}\0{organization_scope}\0{user_id}" + value = ( + f"{SEND_RATE_LIMIT_NAMESPACE}\0{organization_scope}\0{workspace_id}\0{user_id}" + ) return hashlib.sha256(value.encode("utf-8")).hexdigest() @@ -97,7 +101,9 @@ async def enforce_send_email_rate_limit( """ observed_at = now or _utc_now() scope_hash = rate_limit_scope_hash( - auth_context.user_id, auth_context.organization_id + auth_context.user_id, + auth_context.organization_id, + auth_context.workspace_id, ) async with AsyncSessionLocal() as session: if not _session_uses_postgresql(session): diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py index 6dea7d6ee..7aa693c43 100644 --- a/backend/tests/test_email_send_rate_limiter.py +++ b/backend/tests/test_email_send_rate_limiter.py @@ -97,13 +97,15 @@ async def rollback(self): self.lock_held = False -def _context(user_id="user-1", organization_id="org-1"): +def _context( + user_id="user-1", organization_id="org-1", workspace_id: str | None = None +): return AuthContext( user_id=user_id, role="member", organization_id=organization_id, group_ids=(), - workspace_id=f"workspace-{organization_id or user_id}", + workspace_id=workspace_id or f"workspace-{organization_id or user_id}", ) @@ -120,17 +122,29 @@ async def attempt(user_id): same_scope = await asyncio.gather(*(attempt("user-1") for _ in range(11))) other_scope = await asyncio.gather(*(attempt("user-2") for _ in range(10))) + other_workspace = await asyncio.gather( + *( + enforce_send_email_rate_limit( + _context("user-1", workspace_id="workspace-2"), now=observed_at + ) + for _ in range(10) + ) + ) assert sum(decision.allowed for decision in same_scope) == 10 assert sum(not decision.allowed for decision in same_scope) == 1 assert all(decision.allowed for decision in other_scope) + assert all(decision.allowed for decision in other_workspace) rollover = await enforce_send_email_rate_limit( _context("user-1"), now=observed_at + datetime.timedelta(seconds=61), ) assert rollover.allowed is True - scope_uid = f'email_send_scope:{rate_limit_scope_hash("user-1", "org-1")}' + scope_uid = ( + "email_send_scope:" + f'{rate_limit_scope_hash("user-1", "org-1", "workspace-org-1")}' + ) assert sum(event.resource_uid == scope_uid for event in store.events) == 12 @@ -236,7 +250,7 @@ async def test_rate_limiter_real_postgres_reserves_and_denies(monkeypatch): context = _context("rate-limit-smoke-user", "rate-limit-smoke-org") scope_uid = ( "email_send_scope:" - f"{rate_limit_scope_hash(context.user_id, context.organization_id)}" + f"{rate_limit_scope_hash(context.user_id, context.organization_id, context.workspace_id)}" ) observed_at = datetime.datetime.now(datetime.timezone.utc) From 6039e1efc2f86a4fd9fa5b841b51d0523d227a52 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 15:47:36 +0900 Subject: [PATCH 09/18] test(db): require security audit Alembic revision --- .../test_security_audit_migration_contract.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 backend/tests/test_security_audit_migration_contract.py diff --git a/backend/tests/test_security_audit_migration_contract.py b/backend/tests/test_security_audit_migration_contract.py new file mode 100644 index 000000000..1549f9142 --- /dev/null +++ b/backend/tests/test_security_audit_migration_contract.py @@ -0,0 +1,41 @@ +"""Regression contract for durable security-audit schema migration.""" + +from pathlib import Path + + +BACKEND_ROOT = Path(__file__).resolve().parents[1] +MIGRATION_PATH = ( + BACKEND_ROOT / "alembic" / "versions" / "0018_security_audit_events.py" +) + + +def test_security_audit_events_have_structured_incremental_revision() -> None: + """Alembic upgrades must create the durable audit table used by runtime gates.""" + assert MIGRATION_PATH.exists() + revision_text = MIGRATION_PATH.read_text(encoding="utf-8") + + assert 'revision = "0018_security_audit_events"' in revision_text + assert 'down_revision = "0017_merge_newsdom_carddav_heads"' in revision_text + assert '"security_audit_events"' in revision_text + for column_name in ( + "event_uid", + "actor_user_id", + "actor_role", + "organization_id", + "workspace_id", + "event_action", + "resource_type", + "resource_uid", + "evidence_source", + "detail_text", + "observed_at", + ): + assert f'"{column_name}"' in revision_text + + assert "op.create_table(" in revision_text + assert "has_table" in revision_text + assert "ix_security_audit_events_scope_time" in revision_text + assert "ix_security_audit_events_actor_scope" in revision_text + assert "op.create_index(" in revision_text + assert "if_not_exists=True" in revision_text + assert "Base.metadata.create_all" not in revision_text From 5e4e3b925e35af0d99f0f3901878c48129fc37e1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 15:48:11 +0900 Subject: [PATCH 10/18] test(email): pin limiter database-clock and smoke isolation --- backend/tests/test_email_send_rate_limiter.py | 118 ++++++++++++------ 1 file changed, 83 insertions(+), 35 deletions(-) diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py index 7aa693c43..a44406ffd 100644 --- a/backend/tests/test_email_send_rate_limiter.py +++ b/backend/tests/test_email_send_rate_limiter.py @@ -1,5 +1,6 @@ import asyncio import datetime +import uuid from types import SimpleNamespace import pytest @@ -7,11 +8,6 @@ from api.auth import AuthContext from db.models import SecurityAuditEvent import services.email_send_rate_limiter as limiter_module -from services.email_send_rate_limiter import ( - EmailSendRateLimitUnavailable, - enforce_send_email_rate_limit, - rate_limit_scope_hash, -) class _Result: @@ -29,13 +25,16 @@ def __init__(self): class _PostgresSession: - def __init__(self, store): + def __init__(self, store, *, database_now: datetime.datetime | None = None): self.store = store self.added = [] self.queries = [] self.lock_held = False self.pending = [] self.isolation_level = None + self.database_now = database_now or datetime.datetime( + 2026, 8, 19, tzinfo=datetime.timezone.utc + ) async def __aenter__(self): return self @@ -59,15 +58,23 @@ async def execute(self, query, params=None): await self.store.lock.acquire() self.lock_held = True return _Result() + if "clock_timestamp" in query_text: + return _Result(self.database_now) if "security_audit_events" in query_text: values = tuple(query.compile().params.values()) - resource_uid = next(value for value in values if str(value).startswith("email_send_scope:")) + resource_uid = next( + value + for value in values + if str(value).startswith("email_send_scope:") + ) event_action = next( value for value in values if str(value).startswith("email_send_rate_limit.") ) - window_started_at = next(value for value in values if isinstance(value, datetime.datetime)) + window_started_at = next( + value for value in values if isinstance(value, datetime.datetime) + ) return _Result( sum( event.resource_uid == resource_uid @@ -110,13 +117,17 @@ def _context( @pytest.mark.asyncio -async def test_sliding_window_limits_concurrent_workers_without_cross_scope_leakage(monkeypatch): +async def test_sliding_window_limits_concurrent_workers_without_cross_scope_leakage( + monkeypatch, +): store = _SharedAttemptStore() - monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: _PostgresSession(store)) + monkeypatch.setattr( + limiter_module, "AsyncSessionLocal", lambda: _PostgresSession(store) + ) observed_at = datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc) async def attempt(user_id): - return await enforce_send_email_rate_limit( + return await limiter_module.enforce_send_email_rate_limit( _context(user_id), now=observed_at ) @@ -124,7 +135,7 @@ async def attempt(user_id): other_scope = await asyncio.gather(*(attempt("user-2") for _ in range(10))) other_workspace = await asyncio.gather( *( - enforce_send_email_rate_limit( + limiter_module.enforce_send_email_rate_limit( _context("user-1", workspace_id="workspace-2"), now=observed_at ) for _ in range(10) @@ -136,14 +147,14 @@ async def attempt(user_id): assert all(decision.allowed for decision in other_scope) assert all(decision.allowed for decision in other_workspace) - rollover = await enforce_send_email_rate_limit( + rollover = await limiter_module.enforce_send_email_rate_limit( _context("user-1"), now=observed_at + datetime.timedelta(seconds=61), ) assert rollover.allowed is True scope_uid = ( "email_send_scope:" - f'{rate_limit_scope_hash("user-1", "org-1", "workspace-org-1")}' + f'{limiter_module.rate_limit_scope_hash("user-1", "org-1", "workspace-org-1")}' ) assert sum(event.resource_uid == scope_uid for event in store.events) == 12 @@ -151,24 +162,26 @@ async def attempt(user_id): @pytest.mark.asyncio async def test_sliding_window_blocks_boundary_burst(monkeypatch): store = _SharedAttemptStore() - monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: _PostgresSession(store)) + monkeypatch.setattr( + limiter_module, "AsyncSessionLocal", lambda: _PostgresSession(store) + ) started_at = datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc) for offset_seconds in range(50, 60): - decision = await enforce_send_email_rate_limit( + decision = await limiter_module.enforce_send_email_rate_limit( _context(), now=started_at + datetime.timedelta(seconds=offset_seconds), ) assert decision.allowed is True - blocked = await enforce_send_email_rate_limit( + blocked = await limiter_module.enforce_send_email_rate_limit( _context(), now=started_at + datetime.timedelta(seconds=61), ) assert blocked.allowed is False for offset_seconds in range(62, 72): assert not ( - await enforce_send_email_rate_limit( + await limiter_module.enforce_send_email_rate_limit( _context(), now=started_at + datetime.timedelta(seconds=offset_seconds), ) @@ -184,7 +197,7 @@ async def test_limiter_uses_owned_transaction_and_non_sensitive_audit_state(monk store = _SharedAttemptStore() session = _PostgresSession(store) monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: session) - decision = await enforce_send_email_rate_limit( + decision = await limiter_module.enforce_send_email_rate_limit( _context(), now=datetime.datetime(2026, 8, 19, tzinfo=datetime.timezone.utc), ) @@ -198,6 +211,31 @@ async def test_limiter_uses_owned_transaction_and_non_sensitive_audit_state(monk assert audit.event_action == "email_send_rate_limit.allowed" +@pytest.mark.asyncio +async def test_limiter_uses_database_clock_after_scope_lock(monkeypatch): + store = _SharedAttemptStore() + database_now = datetime.datetime( + 2026, 8, 19, 12, 34, 56, tzinfo=datetime.timezone.utc + ) + session = _PostgresSession(store, database_now=database_now) + monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: session) + + decision = await limiter_module.enforce_send_email_rate_limit(_context()) + + assert decision.allowed is True + lock_query_index = next( + index + for index, query in enumerate(session.queries) + if "pg_advisory_xact_lock" in query + ) + clock_query_index = next( + index for index, query in enumerate(session.queries) if "clock_timestamp" in query + ) + assert lock_query_index < clock_query_index + audit = next(item for item in session.added if isinstance(item, SecurityAuditEvent)) + assert audit.observed_at == database_now + + @pytest.mark.asyncio async def test_rate_limiter_fails_closed_when_shared_state_is_unavailable(): class _UnavailableSession: @@ -213,8 +251,8 @@ def get_bind(self): original_factory = limiter_module.AsyncSessionLocal limiter_module.AsyncSessionLocal = lambda: _UnavailableSession() try: - with pytest.raises(EmailSendRateLimitUnavailable): - await enforce_send_email_rate_limit(_context()) + with pytest.raises(limiter_module.EmailSendRateLimitUnavailable): + await limiter_module.enforce_send_email_rate_limit(_context()) finally: limiter_module.AsyncSessionLocal = original_factory @@ -247,10 +285,14 @@ async def test_rate_limiter_real_postgres_reserves_and_denies(monkeypatch): session_factory = async_sessionmaker(engine, expire_on_commit=False) monkeypatch.setattr(limiter_module, "AsyncSessionLocal", session_factory) monkeypatch.setattr(limiter_module, "SEND_RATE_LIMIT_MAX_ATTEMPTS", 1) - context = _context("rate-limit-smoke-user", "rate-limit-smoke-org") + scope_suffix = uuid.uuid4().hex + context = _context( + f"rate-limit-smoke-user-{scope_suffix}", + f"rate-limit-smoke-org-{scope_suffix}", + ) scope_uid = ( "email_send_scope:" - f"{rate_limit_scope_hash(context.user_id, context.organization_id, context.workspace_id)}" + f"{limiter_module.rate_limit_scope_hash(context.user_id, context.organization_id, context.workspace_id)}" ) observed_at = datetime.datetime.now(datetime.timezone.utc) @@ -263,20 +305,26 @@ async def cleanup() -> None: ) await session.commit() - await cleanup() try: - first = await enforce_send_email_rate_limit(context, now=observed_at) - second = await enforce_send_email_rate_limit(context, now=observed_at) - async with session_factory() as session: - actions = ( - await session.execute( - select(SecurityAuditEvent.event_action) - .where(SecurityAuditEvent.resource_uid == scope_uid) - .order_by(SecurityAuditEvent.event_action) - ) - ).scalars().all() - finally: await cleanup() + try: + first = await limiter_module.enforce_send_email_rate_limit( + context, now=observed_at + ) + second = await limiter_module.enforce_send_email_rate_limit( + context, now=observed_at + ) + async with session_factory() as session: + actions = ( + await session.execute( + select(SecurityAuditEvent.event_action) + .where(SecurityAuditEvent.resource_uid == scope_uid) + .order_by(SecurityAuditEvent.event_action) + ) + ).scalars().all() + finally: + await cleanup() + finally: await engine.dispose() assert first.allowed is True From a30ebfa75e5a483df2a8586779b05bc86b10f6cd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 15:48:34 +0900 Subject: [PATCH 11/18] fix(db): formalize security audit events migration --- .../versions/0018_security_audit_events.py | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 backend/alembic/versions/0018_security_audit_events.py diff --git a/backend/alembic/versions/0018_security_audit_events.py b/backend/alembic/versions/0018_security_audit_events.py new file mode 100644 index 000000000..0fccdca5c --- /dev/null +++ b/backend/alembic/versions/0018_security_audit_events.py @@ -0,0 +1,84 @@ +"""Formalize durable security audit events in the Alembic upgrade path. + +Revision ID: 0018_security_audit_events +Revises: 0017_merge_newsdom_carddav_heads +Create Date: 2026-09-05 00:00:00.000000 + +Older installations can already contain ``security_audit_events`` because the +legacy bootstrap path created it outside Alembic. This revision is therefore +idempotent: it creates the table when absent and reconciles the model-owned +indexes when the table already exists. Downgrade intentionally preserves the +durable audit table and its evidence rather than deleting security history that +may predate this revision. +""" + +from alembic import op +import sqlalchemy as sa + +revision = "0018_security_audit_events" +down_revision = "0017_merge_newsdom_carddav_heads" +branch_labels = None +depends_on = None + +_TABLE = "security_audit_events" +_INDEXES: tuple[tuple[str, list[str]], ...] = ( + ("ix_security_audit_events_actor_user_id", ["actor_user_id"]), + ("ix_security_audit_events_actor_role", ["actor_role"]), + ("ix_security_audit_events_organization_id", ["organization_id"]), + ("ix_security_audit_events_workspace_id", ["workspace_id"]), + ("ix_security_audit_events_event_action", ["event_action"]), + ("ix_security_audit_events_resource_type", ["resource_type"]), + ("ix_security_audit_events_resource_uid", ["resource_uid"]), + ("ix_security_audit_events_observed_at", ["observed_at"]), + ( + "ix_security_audit_events_scope_time", + ["organization_id", "workspace_id", "observed_at"], + ), + ( + "ix_security_audit_events_actor_scope", + ["actor_user_id", "organization_id", "workspace_id"], + ), +) + + +def upgrade() -> None: + """Create the audit schema missing from Alembic-managed upgrades.""" + connection = op.get_bind() + inspector = sa.inspect(connection) + + if not inspector.has_table(_TABLE): + op.create_table( + _TABLE, + sa.Column("event_uid", sa.String(), nullable=False), + sa.Column("actor_user_id", sa.String(), nullable=False), + sa.Column("actor_role", sa.String(), nullable=False), + sa.Column("organization_id", sa.String(), nullable=True), + sa.Column("workspace_id", sa.String(), nullable=False), + sa.Column("event_action", sa.String(), nullable=False), + sa.Column("resource_type", sa.String(), nullable=False), + sa.Column("resource_uid", sa.String(), nullable=True), + sa.Column("evidence_source", sa.String(), nullable=False), + sa.Column("detail_text", sa.Text(), nullable=True), + sa.Column( + "observed_at", + sa.DateTime(timezone=True), + nullable=False, + server_default=sa.text("CURRENT_TIMESTAMP"), + ), + sa.PrimaryKeyConstraint("event_uid"), + ) + + for index_name, column_names in _INDEXES: + op.create_index( + index_name, + _TABLE, + column_names, + if_not_exists=True, + ) + + +def downgrade() -> None: + """Preserve durable security evidence created before or after this revision.""" + # This revision reconciles a table that may predate Alembic ownership. + # Dropping it on downgrade could destroy security evidence owned by the + # earlier bootstrap path, so schema rollback deliberately leaves it intact. From 8235c42cae10fdca8204071b1d7e08e4cf775573 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 15:48:50 +0900 Subject: [PATCH 12/18] fix(email): use PostgreSQL clock for send quota --- backend/services/email_send_rate_limiter.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/backend/services/email_send_rate_limiter.py b/backend/services/email_send_rate_limiter.py index 09e59e012..8648f1952 100644 --- a/backend/services/email_send_rate_limiter.py +++ b/backend/services/email_send_rate_limiter.py @@ -51,10 +51,6 @@ def _lock_key(scope_hash: str) -> int: return int.from_bytes(bytes.fromhex(scope_hash[:16]), byteorder="big", signed=True) -def _utc_now() -> datetime.datetime: - return datetime.datetime.now(datetime.timezone.utc) - - def _session_uses_postgresql(session: AsyncSession) -> bool: try: bind = session.get_bind() @@ -97,9 +93,11 @@ async def enforce_send_email_rate_limit( A limiter-owned transaction prevents committing unrelated request work. PostgreSQL advisory locking serializes the count-and-record decision across - workers; existing audit rows provide the exact rolling-window history. + workers. Production timestamps come from PostgreSQL after the scope lock so + worker clock skew and lock wait do not weaken the real-time quota. ``now`` + remains an explicit deterministic test seam. """ - observed_at = now or _utc_now() + observed_at = now scope_hash = rate_limit_scope_hash( auth_context.user_id, auth_context.organization_id, @@ -116,6 +114,9 @@ async def enforce_send_email_rate_limit( select(func.pg_advisory_xact_lock(bindparam("lock_key"))), {"lock_key": _lock_key(scope_hash)}, ) + if observed_at is None: + database_clock = await session.execute(select(func.clock_timestamp())) + observed_at = database_clock.scalar_one() window_started_at = observed_at - datetime.timedelta( seconds=SEND_RATE_LIMIT_WINDOW_SECONDS ) From c9b8c843c7c0d8e1627590bba476881224c0936a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 15:52:31 +0900 Subject: [PATCH 13/18] test(email): require bounded limiter reservation history --- backend/tests/test_email_send_rate_limiter.py | 89 ++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/backend/tests/test_email_send_rate_limiter.py b/backend/tests/test_email_send_rate_limiter.py index a44406ffd..53ee04196 100644 --- a/backend/tests/test_email_send_rate_limiter.py +++ b/backend/tests/test_email_send_rate_limiter.py @@ -60,6 +60,33 @@ async def execute(self, query, params=None): return _Result() if "clock_timestamp" in query_text: return _Result(self.database_now) + if query_text.startswith("delete from security_audit_events"): + values = tuple(query.compile().params.values()) + resource_uid = next( + value + for value in values + if str(value).startswith("email_send_scope:") + ) + event_action = next( + value + for value in values + if str(value).startswith("email_send_rate_limit.") + ) + cutoff = next( + value for value in values if isinstance(value, datetime.datetime) + ) + retained_events = [ + event + for event in self.store.events + if not ( + event.resource_uid == resource_uid + and event.event_action == event_action + and event.observed_at <= cutoff + ) + ] + deleted_count = len(self.store.events) - len(retained_events) + self.store.events = retained_events + return _Result(deleted_count) if "security_audit_events" in query_text: values = tuple(query.compile().params.values()) resource_uid = next( @@ -156,7 +183,7 @@ async def attempt(user_id): "email_send_scope:" f'{limiter_module.rate_limit_scope_hash("user-1", "org-1", "workspace-org-1")}' ) - assert sum(event.resource_uid == scope_uid for event in store.events) == 12 + assert sum(event.resource_uid == scope_uid for event in store.events) == 2 @pytest.mark.asyncio @@ -192,6 +219,66 @@ async def test_sliding_window_blocks_boundary_burst(monkeypatch): ) == 1 +@pytest.mark.asyncio +async def test_limiter_prunes_expired_allowed_state_but_keeps_denial_evidence(monkeypatch): + store = _SharedAttemptStore() + observed_at = datetime.datetime(2026, 8, 19, 12, 0, tzinfo=datetime.timezone.utc) + scope_hash = limiter_module.rate_limit_scope_hash( + "user-1", "org-1", "workspace-org-1" + ) + scope_uid = f"email_send_scope:{scope_hash}" + expired_at = observed_at - datetime.timedelta(seconds=61) + expired_allowed = SecurityAuditEvent( + actor_user_id="user-1", + actor_role="member", + organization_id="org-1", + workspace_id="workspace-org-1", + event_action="email_send_rate_limit.allowed", + resource_type="email_send_rate_limit", + resource_uid=scope_uid, + evidence_source="services.email_send_rate_limiter", + observed_at=expired_at, + ) + durable_denial = SecurityAuditEvent( + actor_user_id="user-1", + actor_role="member", + organization_id="org-1", + workspace_id="workspace-org-1", + event_action="email_send_rate_limit.quota_exhausted", + resource_type="email_send_rate_limit", + resource_uid=scope_uid, + evidence_source="services.email_send_rate_limiter", + observed_at=expired_at, + ) + store.events.extend([expired_allowed, durable_denial]) + session = _PostgresSession(store) + monkeypatch.setattr(limiter_module, "AsyncSessionLocal", lambda: session) + + decision = await limiter_module.enforce_send_email_rate_limit( + _context(), now=observed_at + ) + + assert decision.allowed is True + assert expired_allowed not in store.events + assert durable_denial in store.events + assert sum( + event.event_action == "email_send_rate_limit.allowed" + and event.resource_uid == scope_uid + for event in store.events + ) == 1 + delete_query_index = next( + index + for index, query in enumerate(session.queries) + if query.startswith("delete from security_audit_events") + ) + count_query_index = next( + index + for index, query in enumerate(session.queries) + if "count(" in query and "security_audit_events" in query + ) + assert delete_query_index < count_query_index + + @pytest.mark.asyncio async def test_limiter_uses_owned_transaction_and_non_sensitive_audit_state(monkeypatch): store = _SharedAttemptStore() From 7f2091c4608fa54593ffd4868e6f19e27eec3ff8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 15:52:49 +0900 Subject: [PATCH 14/18] fix(email): prune expired limiter reservations --- backend/services/email_send_rate_limiter.py | 23 ++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/backend/services/email_send_rate_limiter.py b/backend/services/email_send_rate_limiter.py index 8648f1952..50d2dbeac 100644 --- a/backend/services/email_send_rate_limiter.py +++ b/backend/services/email_send_rate_limiter.py @@ -8,7 +8,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Literal -from sqlalchemy import bindparam, func, select +from sqlalchemy import bindparam, delete, func, select from sqlalchemy.ext.asyncio import AsyncSession from db.models import SecurityAuditEvent @@ -94,8 +94,10 @@ async def enforce_send_email_rate_limit( A limiter-owned transaction prevents committing unrelated request work. PostgreSQL advisory locking serializes the count-and-record decision across workers. Production timestamps come from PostgreSQL after the scope lock so - worker clock skew and lock wait do not weaken the real-time quota. ``now`` - remains an explicit deterministic test seam. + worker clock skew and lock wait do not weaken the real-time quota. Expired + allowed rows are transient reservation state and are pruned under that same + lock; durable quota-denial audit evidence is retained. ``now`` remains an + explicit deterministic test seam. """ observed_at = now scope_hash = rate_limit_scope_hash( @@ -103,6 +105,7 @@ async def enforce_send_email_rate_limit( auth_context.organization_id, auth_context.workspace_id, ) + scope_uid = f"email_send_scope:{scope_hash}" async with AsyncSessionLocal() as session: if not _session_uses_postgresql(session): raise EmailSendRateLimitUnavailable @@ -120,12 +123,19 @@ async def enforce_send_email_rate_limit( window_started_at = observed_at - datetime.timedelta( seconds=SEND_RATE_LIMIT_WINDOW_SECONDS ) + await session.execute( + delete(SecurityAuditEvent).where( + SecurityAuditEvent.resource_uid == scope_uid, + SecurityAuditEvent.event_action + == "email_send_rate_limit.allowed", + SecurityAuditEvent.observed_at <= window_started_at, + ) + ) result = await session.execute( select(func.count()) .select_from(SecurityAuditEvent) .where( - SecurityAuditEvent.resource_uid - == f"email_send_scope:{scope_hash}", + SecurityAuditEvent.resource_uid == scope_uid, SecurityAuditEvent.event_action == "email_send_rate_limit.allowed", SecurityAuditEvent.observed_at > window_started_at, @@ -142,8 +152,7 @@ async def enforce_send_email_rate_limit( select(func.count()) .select_from(SecurityAuditEvent) .where( - SecurityAuditEvent.resource_uid - == f"email_send_scope:{scope_hash}", + SecurityAuditEvent.resource_uid == scope_uid, SecurityAuditEvent.event_action == "email_send_rate_limit.quota_exhausted", SecurityAuditEvent.observed_at > window_started_at, From c7b693c25dbe9dd3ab32db8efb13606348d1e870 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 16:44:14 +0900 Subject: [PATCH 15/18] test(email): reproduce bounded-pool send starvation --- .../tests/test_email_send_connection_pool.py | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 backend/tests/test_email_send_connection_pool.py diff --git a/backend/tests/test_email_send_connection_pool.py b/backend/tests/test_email_send_connection_pool.py new file mode 100644 index 000000000..d97961171 --- /dev/null +++ b/backend/tests/test_email_send_connection_pool.py @@ -0,0 +1,98 @@ +"""Regression coverage for bounded-pool email-send connection ordering.""" + +import asyncio +from types import SimpleNamespace + +import pytest + +from api import emails as emails_api +from api.auth import AuthContext +from services.email_send_rate_limiter import EmailSendRateLimitDecision + + +class _TenantResult: + def __init__(self, tenant_config): + self._tenant_config = tenant_config + + def scalar_one_or_none(self): + return self._tenant_config + + +class _SingleSlotRequestSession: + """Model a request session that holds the only pool slot after a read.""" + + def __init__(self): + self.connection_slot = asyncio.Semaphore(1) + self.holds_connection = False + self.events: list[str] = [] + self.tenant_config = SimpleNamespace( + smtp_server="smtp.example.com", + smtp_port=587, + smtp_username="testuser", + smtp_password=None, + ) + + async def execute(self, _query): + await self.connection_slot.acquire() + self.holds_connection = True + self.events.append("tenant-config-read") + return _TenantResult(self.tenant_config) + + async def rollback(self): + self.events.append("request-read-transaction-ended") + if self.holds_connection: + self.holds_connection = False + self.connection_slot.release() + + +@pytest.mark.asyncio +async def test_send_releases_request_read_connection_before_limiter_session(monkeypatch): + """A one-slot pool must not self-deadlock when the limiter opens its session.""" + + session = _SingleSlotRequestSession() + auth_context = AuthContext( + user_id="testuser", + role="member", + organization_id="org-acme", + group_ids=(), + workspace_id="workspace-org-acme", + ) + + monkeypatch.setattr( + emails_api, + "validate_smtp_destination", + lambda smtp_server, smtp_port, *, resolve_host=True: (smtp_server, smtp_port), + ) + + async def bounded_pool_rate_limit(_auth_context): + await asyncio.wait_for(session.connection_slot.acquire(), timeout=0.05) + session.events.append("limiter-session-acquired") + session.connection_slot.release() + return EmailSendRateLimitDecision(allowed=True, reason="allowed") + + async def fake_send_email(*, message_params, smtp_config): + assert message_params.to_address == "test@example.com" + assert smtp_config.smtp_server == "smtp.example.com" + return {"status": "sent", "simulated": False} + + monkeypatch.setattr( + emails_api, "enforce_send_email_rate_limit", bounded_pool_rate_limit + ) + monkeypatch.setattr(emails_api, "send_email", fake_send_email) + + result = await emails_api.send_email_endpoint( + emails_api.SendEmailRequest( + to="test@example.com", + subject="Pool regression", + body="Body", + ), + db=session, + auth_context=auth_context, + ) + + assert result == {"status": "sent", "simulated": False} + assert session.events == [ + "tenant-config-read", + "request-read-transaction-ended", + "limiter-session-acquired", + ] From d1d69d29a74d17fc4f98fecbea2346bef0d3c8e2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 16:46:20 +0900 Subject: [PATCH 16/18] fix(email): release request read before limiter session --- backend/api/emails.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/backend/api/emails.py b/backend/api/emails.py index ed7af66c7..844a5b581 100644 --- a/backend/api/emails.py +++ b/backend/api/emails.py @@ -703,6 +703,11 @@ async def send_email_endpoint( smtp_port = tenant_config.smtp_port smtp_username = tenant_config.smtp_username smtp_password = tenant_config.smtp_password + # The limiter deliberately owns a separate transaction. End the + # request-scoped read transaction first so bounded pools cannot + # deadlock with every request holding one connection while waiting + # for a limiter connection. + await db.rollback() validate_smtp_destination(smtp_server, smtp_port) except Exception as exc: if "ENCRYPTION_KEY is required" in str(exc): @@ -761,4 +766,4 @@ async def send_email_endpoint( logger.error(f"Error sending email: {e}", exc_info=True) raise HTTPException( status_code=500, detail="An internal error occurred while sending the email" - ) + ) \ No newline at end of file From a9f334a442538b666e03e694731745d8aab4b45a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 16:53:24 +0900 Subject: [PATCH 17/18] test(email): preserve send-route session doubles --- backend/api/emails.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/backend/api/emails.py b/backend/api/emails.py index 844a5b581..fcef299ad 100644 --- a/backend/api/emails.py +++ b/backend/api/emails.py @@ -703,11 +703,11 @@ async def send_email_endpoint( smtp_port = tenant_config.smtp_port smtp_username = tenant_config.smtp_username smtp_password = tenant_config.smtp_password - # The limiter deliberately owns a separate transaction. End the - # request-scoped read transaction first so bounded pools cannot - # deadlock with every request holding one connection while waiting - # for a limiter connection. - await db.rollback() + # The production dependency is an AsyncSession. Lightweight unit-test + # doubles do not own a pooled connection and may omit rollback(). + rollback = getattr(db, "rollback", None) + if rollback is not None: + await rollback() validate_smtp_destination(smtp_server, smtp_port) except Exception as exc: if "ENCRYPTION_KEY is required" in str(exc): @@ -766,4 +766,4 @@ async def send_email_endpoint( logger.error(f"Error sending email: {e}", exc_info=True) raise HTTPException( status_code=500, detail="An internal error occurred while sending the email" - ) \ No newline at end of file + ) From 56025b17d752b8fbe2c759d9420876a73e26d51c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 19:21:15 +0900 Subject: [PATCH 18/18] fix(email): close pending SMTP sockets on cancellation --- CHANGELOG.md | 1 + backend/services/email_client.py | 2 +- backend/tests/test_email_client_smtp.py | 67 +++++++++++++++++++++++++ docs/doctoring/shared_send_postgres.md | 41 +++++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 898cf3206..4f1010d8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ ## [Unreleased] +- 이메일 발송 연결 중 요청을 취소하면 남은 연결을 즉시 정리합니다. 직접 발송과 등록된 Connector에 같은 수정이 적용되며, 취소를 발송 성공으로 처리하지 않습니다. - 이메일 발송 제한을 모든 worker가 공유하는 PostgreSQL rolling window로 적용하고, 제한 상태를 확인할 수 없으면 발송 전에 안전하게 중단합니다. - 데이터 저장·검색 검사가 실제 DB 연결 없이 건너뛰어져도 성공으로 보이던 검증 공백을 보완했습니다. 새 DB 설치와 반복 업그레이드 후 전체 백엔드 검사를 실행하며, 아직 실제 배포 환경 검증을 뜻하지는 않습니다. diff --git a/backend/services/email_client.py b/backend/services/email_client.py index 8763a41aa..e82e81c33 100644 --- a/backend/services/email_client.py +++ b/backend/services/email_client.py @@ -400,7 +400,7 @@ async def _connect_validated_smtp_socket( ), timeout=SMTP_TIMEOUT_SECONDS, ) - except Exception: + except (Exception, asyncio.CancelledError): smtp_socket.close() raise return smtp_socket diff --git a/backend/tests/test_email_client_smtp.py b/backend/tests/test_email_client_smtp.py index 7b2a185a5..5e1e4c0e8 100644 --- a/backend/tests/test_email_client_smtp.py +++ b/backend/tests/test_email_client_smtp.py @@ -4,6 +4,7 @@ import pytest import services.email_client as email_client +from runner.local_mail_adapters import LocalMailAccountConfig, LocalMailAdapters import os @@ -14,6 +15,72 @@ def _make_socket() -> socket.socket: return socket.socket(socket.AF_INET, socket.SOCK_STREAM) +@pytest.mark.asyncio +@pytest.mark.parametrize("send_path", ["direct_send", "registered_connector"]) +async def test_smtp_connect_cancellation_closes_socket(monkeypatch, send_path): + """Cancelling either sender must close its real socket before returning.""" + monkeypatch.setattr(email_client.settings, "ALLOWED_SMTP_HOSTS", "smtp.example.com") + monkeypatch.setattr( + email_client.socket, + "getaddrinfo", + lambda *args, **kwargs: [ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("8.8.8.8", 587)) + ], + ) + connect_started = asyncio.Event() + connect_pending = asyncio.Event() + captured_sockets = [] + + async def pending_connect(smtp_socket, socket_address): + assert socket_address == ("8.8.8.8", 587) + captured_sockets.append(smtp_socket) + connect_started.set() + await connect_pending.wait() + + monkeypatch.setattr(asyncio.get_running_loop(), "sock_connect", pending_connect) + smtp_config = email_client.SmtpConfig( + smtp_server="smtp.example.com", + smtp_port=587, + smtp_username="sender@example.com", + smtp_password="unit-test-value", + ) + if send_path == "direct_send": + send_operation = email_client.send_email( + email_client.EmailMessageParams("recipient@example.com", "Subject", "Body"), + smtp_config, + ) + else: + mail_adapters = LocalMailAdapters([ + LocalMailAccountConfig( + account="test_account", + user_id="test_user", + smtp_server=smtp_config.smtp_server, + smtp_port=smtp_config.smtp_port, + smtp_username=smtp_config.smtp_username, + smtp_password=smtp_config.smtp_password, + ) + ]) + send_operation = mail_adapters.send_smtp({ + "account": "test_account", + "to": "recipient@example.com", + "subject": "Subject", + "body": "Body", + }) + send_task = asyncio.create_task(send_operation) + try: + await asyncio.wait_for(connect_started.wait(), timeout=5) + send_task.cancel("send cancelled") + with pytest.raises(asyncio.CancelledError, match="send cancelled"): + await send_task + assert len(captured_sockets) == 1 + assert captured_sockets[0].fileno() == -1 + finally: + send_task.cancel() + await asyncio.gather(send_task, return_exceptions=True) + for smtp_socket in captured_sockets: + smtp_socket.close() + + def test_smtp_host_policy_denies_empty_allowlist_before_dns(monkeypatch): monkeypatch.setattr(email_client.settings, "ALLOWED_SMTP_HOSTS", "") diff --git a/docs/doctoring/shared_send_postgres.md b/docs/doctoring/shared_send_postgres.md index 9b98dce46..f78820635 100644 --- a/docs/doctoring/shared_send_postgres.md +++ b/docs/doctoring/shared_send_postgres.md @@ -73,6 +73,43 @@ Remaining acceptance: current-head hosted checks and independent review, parent integration, protected merge, delivery/browser evidence, and realistic load. No coverage percentage, release, deployment or p95 claim is established here. +## SMTP connection cancellation follow-up + +At `1666f76cf94c31e34c2762c9d75f52ea3040b9a2`, the shared +`services.email_client._connect_validated_smtp_socket` closes the raw socket for +`Exception`, but `asyncio.CancelledError` derives from `BaseException` (Python +Software Foundation, n.d.). Cancellation while awaiting `sock_connect` therefore +escapes before the socket reaches the caller's `finally`. A cancelled send can +retain an open descriptor until later object reclamation. Both direct +`send_email` and `runner.local_mail_adapters.LocalMailAdapters.send_smtp` use +this helper; the latter is not a separate SMTP implementation. + +The two new `test_smtp_connect_cancellation_closes_socket` cases exercise those +real consumers and create real OS sockets. Only DNS and the pending external +connect are substituted. After the connection starts, cancelling the actual +task must propagate the cancellation message and leave `fileno() == -1` before +the test releases its socket reference. Both cases were RED with an open +descriptor (`14 != -1`); the test's own `finally` still closes it so a failing +candidate does not leak the test resource. A five-second synchronization guard +bounds the test observation, not a model or application timeout. + +Extend the existing cleanup handler to include `asyncio.CancelledError`, close +the socket synchronously, and re-raise without replacement or suppression. +Per-caller guards were rejected because neither caller owns the socket before +the helper returns. A new resource abstraction or dependency adds no value to +this one-line ownership fix. DNS pinning, TLS hostname checks, quota semantics, +and successful ownership transfer remain unchanged. The focused SMTP, message, +and registered-adapter suites passed 55 tests with warnings treated as errors. +The final exact-head whole-suite receipt belongs in PR #1417. + +DeepWiki's September 6 lookup described a nonexistent connection `finally` and +excluded the registered Connector from the shared path. Current CodeGraph +source/caller evidence and the RED reproduction contradict that summary; do +not treat generated documentation as exact-revision runtime evidence. This +follow-up proves deterministic cleanup before connection completion, not real +SMTP delivery, browser cancellation propagation, recipient acceptance, or +post-delivery retry safety. No external message was sent. + ## References (APA 7) Bayer, M. (n.d.). *Working with branches*. Alembic documentation. Retrieved @@ -85,3 +122,7 @@ https://www.postgresql.org/docs/16/explicit-locking.html#ADVISORY-LOCKS PostgreSQL Global Development Group. (n.d.-b). *The cumulative statistics system*. PostgreSQL 16 documentation. Retrieved September 6, 2026, from https://www.postgresql.org/docs/16/monitoring-stats.html + +Python Software Foundation. (n.d.). *Exceptions*. Python 3.14 documentation. +Retrieved September 6, 2026, from +https://docs.python.org/3.14/library/asyncio-exceptions.html