fix(tradein/devops): wire COOKIE_ENCRYPTION_KEY env to tradein-backend #543

Merged

lekss361 merged 1 commit from chore/tradein-cookie-encryption-key-env into main

2026-05-24 16:08:21 +00:00

lekss361 commented

2026-05-24 15:59:54 +00:00

Owner

Root cause

Prod backend service tradein-backend НЕ имеет COOKIE_ENCRYPTION_KEY env var в docker-compose.prod.yml. Settings class имеет default cookie_encryption_key: str = "". Из-за этого:

pgp_sym_encrypt(text, '') падает psycopg.errors.ExternalRoutineInvocationException: Illegal argument to function — попытка записать Cian cookies через save_session ломается.
Cian Valuation Calculator scraper (Stage 9, PR #476) фактически НИКОГДА не работал на проде — каждый /estimate graceful'но возвращает cian_valuation=None.
Cookie upload endpoint возвращает 503/401 — но даже после fix verify_session он упал бы внутри save_session на encrypt.

Дисковерено при попытке локального backfill cian valuations — pgp_sym_encrypt SQL trace показал 'key': ''. Compose audit подтвердил: ни одного упоминания COOKIE_ENCRYPTION_KEY в backend env block.

Fix

tradein-mvp/docker-compose.prod.yml — добавить COOKIE_ENCRYPTION_KEY в backend.environment (после GENDESIGN_FDW_PASSWORD). +4 lines с комментарием.
tradein-mvp/DEPLOY.md — добавить в template .env.runtime + новая секция «Обновление на существующем VPS» с командой up -d --force-recreate --no-deps backend (не restart — она не re-читает env_file). +17 lines.

Manual action on VPS after merge+deploy

Сгенерить ключ: openssl rand -hex 32
Дописать в /opt/gendesign/tradein-mvp/.env.runtime: COOKIE_ENCRYPTION_KEY=<64-char hex>
Перезапустить: cd /opt/gendesign/tradein-mvp && docker compose -p gendesign-tradein -f docker-compose.prod.yml up -d --force-recreate --no-deps backend

После этого:

Загрузить cookies через /scrapers/cian-cookies UI ИЛИ через локальный скрипт.
Verify: curl https://gendsgn.ru/trade-in/api/v1/admin/scrape/cian/test-auth
Запустить valuation backfill: python cian_backfill.py --no-listings --batch-size 30.

Test plan

No code changes — только compose + docs
Manual smoke after deploy: docker exec tradein-backend printenv COOKIE_ENCRYPTION_KEY ≠ пусто
Upload test cookies → save_session НЕ падает
/estimate с full geo → cian_valuation non-null (когда cookies authenticated)

Follows up 4-PR Cian activation series (#523/#525/#530/#535) + backfill task #539.

## Root cause Prod backend service `tradein-backend` НЕ имеет `COOKIE_ENCRYPTION_KEY` env var в `docker-compose.prod.yml`. Settings class имеет default `cookie_encryption_key: str = ""`. Из-за этого: - `pgp_sym_encrypt(text, '')` падает `psycopg.errors.ExternalRoutineInvocationException: Illegal argument to function` — попытка записать Cian cookies через `save_session` ломается. - Cian Valuation Calculator scraper (Stage 9, PR #476) фактически НИКОГДА не работал на проде — каждый `/estimate` graceful'но возвращает `cian_valuation=None`. - Cookie upload endpoint возвращает 503/401 — но даже после fix `verify_session` он упал бы внутри `save_session` на encrypt. Дисковерено при попытке локального backfill cian valuations — pgp_sym_encrypt SQL trace показал `'key': ''`. Compose audit подтвердил: ни одного упоминания `COOKIE_ENCRYPTION_KEY` в backend env block. ## Fix - `tradein-mvp/docker-compose.prod.yml` — добавить `COOKIE_ENCRYPTION_KEY` в `backend.environment` (после `GENDESIGN_FDW_PASSWORD`). +4 lines с комментарием. - `tradein-mvp/DEPLOY.md` — добавить в template `.env.runtime` + новая секция «Обновление на существующем VPS» с командой `up -d --force-recreate --no-deps backend` (не `restart` — она не re-читает env_file). +17 lines. ## Manual action on VPS after merge+deploy 1. Сгенерить ключ: `openssl rand -hex 32` 2. Дописать в `/opt/gendesign/tradein-mvp/.env.runtime`: `COOKIE_ENCRYPTION_KEY=<64-char hex>` 3. Перезапустить: `cd /opt/gendesign/tradein-mvp && docker compose -p gendesign-tradein -f docker-compose.prod.yml up -d --force-recreate --no-deps backend` После этого: - Загрузить cookies через `/scrapers/cian-cookies` UI ИЛИ через локальный скрипт. - Verify: `curl https://gendsgn.ru/trade-in/api/v1/admin/scrape/cian/test-auth` - Запустить valuation backfill: `python cian_backfill.py --no-listings --batch-size 30`. ## Test plan - [x] No code changes — только compose + docs - [ ] Manual smoke after deploy: `docker exec tradein-backend printenv COOKIE_ENCRYPTION_KEY` ≠ пусто - [ ] Upload test cookies → save_session НЕ падает - [ ] /estimate с full geo → cian_valuation non-null (когда cookies authenticated) Follows up 4-PR Cian activation series (#523/#525/#530/#535) + backfill task #539.

lekss361 added 1 commit 2026-05-24 15:59:54 +00:00

fix(tradein/devops): wire COOKIE_ENCRYPTION_KEY env to tradein-backend (Calculator was broken on prod) a75a660e5b

lekss361 commented

2026-05-24 16:06:39 +00:00

Author

Owner

Deep Code Review — PR #543

(Posted as comment — Forgejo blocks self-PR formal Review.)

Summary

Status: APPROVE (minor follow-ups)
Files: 2 (tradein-mvp/docker-compose.prod.yml +4, tradein-mvp/DEPLOY.md +17)
Scope: env-wire + ops docs, no code
Security tripwire: clean — no literal key/secret in diff

Security verification (encryption-key sensitive)

Compose uses ${COOKIE_ENCRYPTION_KEY:-} shell substitution — value resolved at runtime from .env.runtime, never embedded in image / git.
DEPLOY.md uses literal placeholder <64-char hex> — not a real key.
Generation instruction is openssl rand -hex 32 (32 bytes = 256-bit) — sufficient for pgcrypto S2K-derived AES; far above any practical floor for pgp_sym_encrypt.
Deploy workflow (.forgejo/workflows/deploy-tradein.yml) sources via set -a; source .env.runtime; set +a — no echo, cat, or printenv of file content → no log leak (lessons from PR #510 \o /dev/null incident respected).
chmod 600 .env.runtime enforced in workflow line 145 → at-rest filesystem ACL OK.
SQL migration loop (line 163-170) feeds files via stdin < "$sql_file" (not -c "$sql"), key is bound via psycopg parameters in app layer ({"key": settings.cookie_encryption_key} in cian_session.py:137/169), so the secret never appears in psql command line or postgres log_statement.
No SQLALCHEMY_ECHO / engine(echo=True) found in tradein backend → bound params not echoed to app logs.

Correctness

Wire matches peer pattern (GENDESIGN_FDW_PASSWORD) exactly: same indentation, same ${VAR:-} empty default, same env block placement.
--force-recreate --no-deps backend is the right rollout flag: docker compose restart does NOT re-read env_file / environment block, only re-creates kernel namespaces. DEPLOY.md correctly documents this distinction.
Empty-key fail-safety verified in app:
- admin.py:269 upload endpoint → 503 "COOKIE_ENCRYPTION_KEY not configured" (fail-closed)
- admin.py:305 test-auth → returns authenticated=false reason=encryption_key_not_configured (graceful)
- cian_valuation.py:160 no session → returns None, /estimate continues with cian_valuation=None (graceful, matches body claim)
Settings.cookie_encryption_key: str = "" default in config.py:30 is acceptable — app does NOT fail-fast on empty key, but every consumer call-site gates explicitly. Trade-off accepted (allows dev/test without key).

Minor follow-ups (not blockers)

M1 — load_session has no try/except around pgp_sym_decrypt (cian_session.py:157-170). If a key gets rotated OR if any stale row encrypted with key='' (pre-deploy state) survives in prod, the next load_session will raise psycopg.errors.ExternalRoutineInvocationException and propagate as 500. Not introduced by this PR; flagging for the rotation workflow.

M2 — Manual-action checklist should mention stale rows. Before setting COOKIE_ENCRYPTION_KEY in prod, verify SELECT count(*) FROM cian_session_cookies is 0 (it should be — Stage 9 was never functional per body), OR include a TRUNCATE cian_session_cookies step. Otherwise the first load_session after key wire-up could throw on legacy key='' blobs.

M3 — No key-rotation runbook. DEPLOY.md documents one-time generation but not rotation. Suggested rotation: (1) TRUNCATE cian_session_cookies; (2) update .env.runtime; (3) up -d --force-recreate --no-deps backend; (4) re-upload cookies via /api/v1/cookies/upload. Add as a vault runbooks/cian-key-rotation.md follow-up (out of scope for this PR).

M4 — chmod 600 not in DEPLOY.md manual-action snippet. The deploy workflow chmod's .env.runtime automatically (line 145), but the manual-action snippet doesn't remind the operator. Suggested addition right after the echo ... >> line:

chmod 600 /opt/gendesign/tradein-mvp/.env.runtime

Project conventions

Comment style matches surrounding env block (# GlitchTip DSN — мониторинг ошибок ...) — bilingual + ticket-style. Good.
DEPLOY.md template format consistent with existing block (sh code-fence + cyrillic narrative).
No :latest tag concerns — env var injection only, image pull untouched.

Positive

Documentation is precise and operator-actionable (exact compose command, exact path).
Backend-only --force-recreate --no-deps correctly avoids postgres bounce.
Cross-references existing PRs (#476/#523/#525/#530/#535/#539) — strong audit trail.
Root-cause analysis in PR body identifies the silent failure mode (pgp_sym_encrypt(text, '') ExternalRoutineInvocationException → Stage 9 dead since ship).

Blast radius

Risk: LOW (compose env + docs only; no code paths altered).
Reversibility: trivial (git revert + up -d --force-recreate --no-deps backend).
Side effects: none for services not consuming cookie_encryption_key.

Merge policy

Security-sensitive — human merge required after manual deploy diff audit. Do not auto-merge.

Approved logically; please address M2/M4 inline (or in follow-up) before triggering the prod key generation.

## Deep Code Review — PR #543 (Posted as comment — Forgejo blocks self-PR formal Review.) ### Summary - **Status:** APPROVE (minor follow-ups) - **Files:** 2 (`tradein-mvp/docker-compose.prod.yml` +4, `tradein-mvp/DEPLOY.md` +17) - **Scope:** env-wire + ops docs, no code - **Security tripwire:** clean — no literal key/secret in diff ### Security verification (encryption-key sensitive) - Compose uses `${COOKIE_ENCRYPTION_KEY:-}` shell substitution — value resolved at runtime from `.env.runtime`, never embedded in image / git. - DEPLOY.md uses literal placeholder `<64-char hex>` — not a real key. - Generation instruction is `openssl rand -hex 32` (32 bytes = 256-bit) — sufficient for pgcrypto S2K-derived AES; far above any practical floor for `pgp_sym_encrypt`. - Deploy workflow (`.forgejo/workflows/deploy-tradein.yml`) sources via `set -a; source .env.runtime; set +a` — no `echo`, `cat`, or `printenv` of file content → no log leak (lessons from PR #510 `\o /dev/null` incident respected). - `chmod 600 .env.runtime` enforced in workflow line 145 → at-rest filesystem ACL OK. - SQL migration loop (line 163-170) feeds files via stdin `< "$sql_file"` (not `-c "$sql"`), key is bound via psycopg parameters in app layer (`{"key": settings.cookie_encryption_key}` in cian_session.py:137/169), so the secret never appears in psql command line or postgres log_statement. - No `SQLALCHEMY_ECHO` / `engine(echo=True)` found in tradein backend → bound params not echoed to app logs. ### Correctness - Wire matches peer pattern (`GENDESIGN_FDW_PASSWORD`) exactly: same indentation, same `${VAR:-}` empty default, same env block placement. - `--force-recreate --no-deps backend` is the right rollout flag: `docker compose restart` does NOT re-read `env_file` / `environment` block, only re-creates kernel namespaces. DEPLOY.md correctly documents this distinction. - Empty-key fail-safety verified in app: - `admin.py:269` upload endpoint → 503 "COOKIE_ENCRYPTION_KEY not configured" (fail-closed) - `admin.py:305` test-auth → returns `authenticated=false reason=encryption_key_not_configured` (graceful) - `cian_valuation.py:160` no session → returns `None`, `/estimate` continues with `cian_valuation=None` (graceful, matches body claim) - `Settings.cookie_encryption_key: str = ""` default in `config.py:30` is acceptable — app does NOT fail-fast on empty key, but every consumer call-site gates explicitly. Trade-off accepted (allows dev/test without key). ### Minor follow-ups (not blockers) **M1 — load_session has no try/except around pgp_sym_decrypt** (`cian_session.py:157-170`). If a key gets rotated OR if any stale row encrypted with `key=''` (pre-deploy state) survives in prod, the next `load_session` will raise `psycopg.errors.ExternalRoutineInvocationException` and propagate as 500. Not introduced by this PR; flagging for the rotation workflow. **M2 — Manual-action checklist should mention stale rows.** Before setting `COOKIE_ENCRYPTION_KEY` in prod, verify `SELECT count(*) FROM cian_session_cookies` is 0 (it should be — Stage 9 was never functional per body), OR include a `TRUNCATE cian_session_cookies` step. Otherwise the first `load_session` after key wire-up could throw on legacy `key=''` blobs. **M3 — No key-rotation runbook.** DEPLOY.md documents one-time generation but not rotation. Suggested rotation: (1) TRUNCATE cian_session_cookies; (2) update .env.runtime; (3) up -d --force-recreate --no-deps backend; (4) re-upload cookies via /api/v1/cookies/upload. Add as a vault `runbooks/cian-key-rotation.md` follow-up (out of scope for this PR). **M4 — `chmod 600` not in DEPLOY.md manual-action snippet.** The deploy workflow chmod's `.env.runtime` automatically (line 145), but the manual-action snippet doesn't remind the operator. Suggested addition right after the `echo ... >>` line: ```bash chmod 600 /opt/gendesign/tradein-mvp/.env.runtime ``` ### Project conventions - Comment style matches surrounding env block (`# GlitchTip DSN — мониторинг ошибок ...`) — bilingual + ticket-style. Good. - DEPLOY.md template format consistent with existing block (sh code-fence + cyrillic narrative). - No `:latest` tag concerns — env var injection only, image pull untouched. ### Positive - Documentation is precise and operator-actionable (exact compose command, exact path). - Backend-only `--force-recreate --no-deps` correctly avoids postgres bounce. - Cross-references existing PRs (#476/#523/#525/#530/#535/#539) — strong audit trail. - Root-cause analysis in PR body identifies the silent failure mode (`pgp_sym_encrypt(text, '')` ExternalRoutineInvocationException → Stage 9 dead since ship). ### Blast radius - **Risk:** LOW (compose env + docs only; no code paths altered). - **Reversibility:** trivial (`git revert` + `up -d --force-recreate --no-deps backend`). - **Side effects:** none for services not consuming `cookie_encryption_key`. ### Merge policy Security-sensitive — human merge required after manual deploy diff audit. Do not auto-merge. Approved logically; please address M2/M4 inline (or in follow-up) before triggering the prod key generation.