-
Notifications
You must be signed in to change notification settings - Fork 1
fix(email): enforce shared send throttling #1417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
seonghobae
wants to merge
33
commits into
codex/stacked-pr-workflow-triggers
Choose a base branch
from
fix/email-shared-send-rate-limit
base: codex/stacked-pr-workflow-triggers
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
3d08f5f
fix(email): enforce shared send throttling
seonghobae 754cea8
Harden Cloud Agent env secrets and email_records.is_read migration (#…
cursor[bot] ae254c1
chore: sync send rate limit branch with develop
seonghobae 69fb72d
Merge remote-tracking branch 'origin/develop' into fix/pr1417-current
seonghobae 7125926
Merge branch 'develop' into fix/email-shared-send-rate-limit
seonghobae 46f4b92
test(postgres): seed email read state explicitly
seonghobae 6ae6425
Merge branch 'develop' into fix/email-shared-send-rate-limit
seonghobae 2d4ec7c
Merge branch 'develop' into fix/email-shared-send-rate-limit
seonghobae 48ab340
fix(email): enforce rolling send limit transaction
seonghobae 395a8a8
fix(email): bound rate-limit denial audits
seonghobae bb7085c
fix(email): pin limiter transaction isolation
seonghobae ad3ba4e
Merge remote-tracking branch 'origin/develop' into codex/pr1417-current
seonghobae 5a22a26
fix(email): isolate shared send throttle
seonghobae e22fa78
fix(email): isolate send limits by workspace
seonghobae 6039e1e
test(db): require security audit Alembic revision
seonghobae 5e4e3b9
test(email): pin limiter database-clock and smoke isolation
seonghobae a30ebfa
fix(db): formalize security audit events migration
seonghobae 8235c42
fix(email): use PostgreSQL clock for send quota
seonghobae c9b8c84
test(email): require bounded limiter reservation history
seonghobae 7f2091c
fix(email): prune expired limiter reservations
seonghobae c7b693c
test(email): reproduce bounded-pool send starvation
seonghobae d1d69d2
fix(email): release request read before limiter session
seonghobae a9f334a
test(email): preserve send-route session doubles
seonghobae 2e791fb
fix(email): integrate migrated PostgreSQL quota evidence
seonghobae 1666f76
fix(email): inherit nested migration isolation prerequisite
seonghobae 56025b1
fix(email): close pending SMTP sockets on cancellation
seonghobae dc8b53d
Merge canonical CI signal startup repair into shared send
seonghobae b7011d2
merge: inherit CI process registration repair
seonghobae 9e4ddde
merge: inherit multiline governance repair
seonghobae cc2c4cb
fix(email): retain complete CI governance parent repair
seonghobae 5dea509
merge: inherit complete CI owner publisher verification repair
seonghobae ec3e361
merge(email): inherit validated clean-summary gate repair
seonghobae 489bcbe
merge(email): inherit validated dependency prerequisite
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 "<personal>" | ||
| 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 | ||
|
seonghobae marked this conversation as resolved.
Outdated
|
||
|
|
||
| observed_at = now or _utc_now() | ||
|
seonghobae marked this conversation as resolved.
Outdated
|
||
| 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, | ||
| ) | ||
| ) | ||
|
seonghobae marked this conversation as resolved.
Outdated
|
||
| 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() | ||
|
seonghobae marked this conversation as resolved.
Outdated
|
||
| return decision | ||
|
seonghobae marked this conversation as resolved.
Outdated
|
||
| 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 | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.