Skip to content

Harden Cloud Agent env secrets and email_records.is_read migration - #1381

Merged
seonghobae merged 12 commits into
fix/email-shared-send-rate-limitfrom
cursor/bc-6ba03a0c-13df-4248-a679-e55574a1ec39-156e
Aug 20, 2026
Merged

Harden Cloud Agent env secrets and email_records.is_read migration#1381
seonghobae merged 12 commits into
fix/email-shared-send-rate-limitfrom
cursor/bc-6ba03a0c-13df-4248-a679-e55574a1ec39-156e

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

Description

Landing vehicle for the Cloud Agent environment work in #1378 after CodeRabbit CHANGES_REQUESTED on 40690170283357ea6478fd49b6a7a84cdd5aaeeb.

Keeps the fresh-DB and NUL-free import-lock fixes, then closes the review gaps:

  • .cursor/start.sh no longer interpolates the DATABASE_URL role secret into psql -c. Empty secrets fail closed. ALTER USER is dollar-quoted on psql stdin via backend/scripts/reconcile_local_postgres_role.py so quotes never reach argv.
  • .cursor/install.sh installs requirements-hashes.txt with --require-hashes.
  • Postgres start fails closed if the cluster never becomes ready.
  • 0011_email_read_state downgrade is a no-op so a pre-existing retired emails.is_read is not dropped.
  • 0018_email_record_read_state guards email_records.is_read NOT NULL DEFAULT true on existing databases. Fresh create_all already has the model server_default.
  • Import quota lock key has an independent SHA-256 golden digest. AGENTS.md records the NUL/hashtext() anti-pattern.

Do not merge #1378. Prefer this branch.

Fixes the review findings on #1378.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

Verification

cd backend
PYTHONWARNINGS=error DISABLE_BACKGROUND_WORKERS=1 python -m pytest \
  tests/test_email_import_quota_lock_key.py \
  tests/test_reconcile_local_postgres_role.py \
  tests/test_cloud_agent_environment.py \
  tests/test_alembic_migrations.py \
  tests/test_bootstrap_db.py::test_schema_backfill_adds_email_indexes \
  tests/test_emails_api.py::test_import_email_files_serializes_quota_with_postgres_owner_lock \
  tests/test_emails_api.py::test_import_email_files_rejects_when_owner_quota_is_exhausted -q
python -m ruff check scripts/reconcile_local_postgres_role.py services/email_import_service.py tests/test_email_import_quota_lock_key.py tests/test_reconcile_local_postgres_role.py tests/test_cloud_agent_environment.py tests/test_alembic_migrations.py alembic/versions/0011_email_read_state.py alembic/versions/0018_email_record_read_state.py
bash -n ../.cursor/start.sh ../.cursor/install.sh

Local result: 36 passed, ruff clean, bash -n clean. No Timeout/Fatal/Warn/Denied output.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
Open in Web View Automation 

cursoragent and others added 6 commits August 16, 2026 15:11
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 <seonghobae@users.noreply.github.com>
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 <seonghobae@users.noreply.github.com>
…tgres/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 <seonghobae@users.noreply.github.com>
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 <seonghobae@users.noreply.github.com>
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 <seonghobae@users.noreply.github.com>
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 <seonghobae@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 8f6c8411df08021000aa6d089745f80c6b12ef4d:

  • Required check metadata could not be read; see the workflow run log.

Comment thread .cursor/install.sh Fixed
Comment thread backend/scripts/reconcile_local_postgres_role.py Fixed
@seonghobae
seonghobae marked this pull request as ready for review August 19, 2026 16:06
@seonghobae
seonghobae self-requested a review as a code owner August 19, 2026 16:06
@seonghobae
seonghobae changed the base branch from develop to fix/email-shared-send-rate-limit August 20, 2026 08:47
@seonghobae

Copy link
Copy Markdown
Contributor

Current-head review request for PR #1381.

HEAD: 503f62d
BASE: fix/email-shared-send-rate-limit (PR #1417 HEAD 3d08f5fe829e5111d6bd3e641e5272ad6ad6)

This PR is intentionally stacked after the shared email-send rate-bucket migration so Alembic revision 0019_email_record_read_state has a unique id and a linear 0018_email_send_rate_buckets predecessor. The changelog, operator documentation, and migration contract tests were updated with that dependency. Review only this exact stacked HEAD/base pair and verify the current Checks; do not reuse develop-base evidence.

@seonghobae
seonghobae merged commit 754cea8 into fix/email-shared-send-rate-limit Aug 20, 2026
1 of 2 checks passed
@seonghobae
seonghobae deleted the cursor/bc-6ba03a0c-13df-4248-a679-e55574a1ec39-156e branch August 20, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants