lekss361/gendesign

Fork 0

fix(deploy): tradein_fdw_reader password — psql var interpolation in DO block #510

Merged

lekss361 merged 2 commits from fix/tradein-fdw-psql-var-in-do into main

2026-05-24 11:29:00 +00:00

lekss361 commented

2026-05-24 11:04:21 +00:00

Owner

Summary

ops/db-bootstrap/set_tradein_fdw_password.sql: pass password в DO block через session GUC (set_config) вместо прямого psql :'pw' substitution.

Root cause

psql variable substitution (:'pw') не интерполируется внутри dollar-quoted блока ($$...$$) — это документированное поведение psql, не bug (psql docs: "Variable interpolation will not be performed within quoted SQL literals and identifiers").

Текущий код:

DO $$
BEGIN
  ...
  EXECUTE format('ALTER ROLE tradein_fdw_reader WITH PASSWORD %L', :'pw');
END $$;

:'pw' доходит до сервера как literal → syntax error at or near ":".

Symptom

Deploy 2026-05-24 после merge PR #503 (DOM.РФ WAF fix) упал:

out: All migrations applied.
out: → Applying tradein_fdw_reader password from env
err: ERROR:  syntax error at or near ":"
err: LINE 4: ...at('ALTER ROLE tradein_fdw_reader WITH PASSWORD %L', :'pw');
err:                                                                 ^
2026/05/24 10:56:59 Process exited with status 3

Это блокирует docker compose up -d → backend остаётся на старом образе → PR #503 fix не применился к работающему scraper'у. Этот же баг будет валить любой следующий deploy.

Fix

SELECT set_config('app.fdw_pw', :'pw', false);  -- psql интерполирует ВНЕ $$

DO $$
BEGIN
  IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'tradein_fdw_reader') THEN
    EXECUTE format(
      'ALTER ROLE tradein_fdw_reader WITH PASSWORD %L',
      current_setting('app.fdw_pw')  -- читаем GUC внутри DO
    );
    RAISE NOTICE 'tradein_fdw_reader password set';
  ELSE
    RAISE NOTICE 'tradein_fdw_reader role missing — migration 100_... not applied yet';
  END IF;
END $$;

SELECT set_config('app.fdw_pw', '', false);  -- cleanup

Логика идентична оригиналу: idempotent (skip если роль отсутствует), format(%L) экранирует password как SQL literal. Только канал передачи параметра меняется.

Test plan

Pre-commit hooks passed (нет yaml/toml/ruff проверок — только trailing whitespace/EOF)
После merge: следующий deploy не падает на этом step (увидим в логах tradein_fdw_reader password set)
Verify: docker compose exec postgres psql -U postgres -d gendesign -c "\du tradein_fdw_reader" — роль существует, pwd actual

Risk: LOW

Идентичная семантика, изменён только механизм передачи password
Cleanup (set_config app.fdw_pw '' false) убирает password из session GUC после execute
Идемпотентность сохранена (IF EXISTS + role rename невозможен после ALTER без cross-check)
Если PR #493 deploy шёл успешно раньше — это значит password подставлялся как-то иначе (раньше inline в DO без :), либо deploy этот код впервые исполняет именно сейчас. Текущий incident — первый раз когда file применяется. После fix — стандартный psql -f flow.

## Summary - `ops/db-bootstrap/set_tradein_fdw_password.sql`: pass password в DO block через session GUC (`set_config`) вместо прямого psql `:'pw'` substitution. ## Root cause psql variable substitution (`:'pw'`) **не интерполируется внутри dollar-quoted блока** (`$$...$$`) — это документированное поведение psql, не bug ([psql docs](https://www.postgresql.org/docs/current/app-psql.html#APP-PSQL-INTERPOLATION): "Variable interpolation will not be performed within quoted SQL literals and identifiers"). Текущий код: ```sql DO $$ BEGIN ... EXECUTE format('ALTER ROLE tradein_fdw_reader WITH PASSWORD %L', :'pw'); END $$; ``` `:'pw'` доходит до сервера как literal → `syntax error at or near ":"`. ## Symptom Deploy 2026-05-24 после merge PR #503 (DOM.РФ WAF fix) упал: ``` out: All migrations applied. out: → Applying tradein_fdw_reader password from env err: ERROR: syntax error at or near ":" err: LINE 4: ...at('ALTER ROLE tradein_fdw_reader WITH PASSWORD %L', :'pw'); err: ^ 2026/05/24 10:56:59 Process exited with status 3 ``` Это блокирует `docker compose up -d` → backend остаётся на старом образе → PR #503 fix не применился к работающему scraper'у. Этот же баг будет валить любой следующий deploy. ## Fix ```sql SELECT set_config('app.fdw_pw', :'pw', false); -- psql интерполирует ВНЕ $$ DO $$ BEGIN IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'tradein_fdw_reader') THEN EXECUTE format( 'ALTER ROLE tradein_fdw_reader WITH PASSWORD %L', current_setting('app.fdw_pw') -- читаем GUC внутри DO ); RAISE NOTICE 'tradein_fdw_reader password set'; ELSE RAISE NOTICE 'tradein_fdw_reader role missing — migration 100_... not applied yet'; END IF; END $$; SELECT set_config('app.fdw_pw', '', false); -- cleanup ``` Логика идентична оригиналу: idempotent (skip если роль отсутствует), `format(%L)` экранирует password как SQL literal. Только канал передачи параметра меняется. ## Test plan - [x] Pre-commit hooks passed (нет yaml/toml/ruff проверок — только trailing whitespace/EOF) - [ ] После merge: следующий deploy не падает на этом step (увидим в логах `tradein_fdw_reader password set`) - [ ] Verify: `docker compose exec postgres psql -U postgres -d gendesign -c "\du tradein_fdw_reader"` — роль существует, pwd actual ## Risk: LOW - Идентичная семантика, изменён только механизм передачи password - Cleanup (`set_config app.fdw_pw '' false`) убирает password из session GUC после execute - Идемпотентность сохранена (`IF EXISTS` + role rename невозможен после `ALTER` без cross-check) - Если PR #493 deploy шёл успешно раньше — это значит password подставлялся как-то иначе (раньше inline в DO без `:`), либо deploy этот код впервые исполняет именно сейчас. Текущий incident — первый раз когда file применяется. После fix — стандартный `psql -f` flow.

lekss361 added 1 commit 2026-05-24 11:04:22 +00:00

fix(deploy): tradein_fdw_reader password — psql var interpolation in DO block da7814810c

psql :'pw' substitution НЕ работает внутри dollar-quoted блока $$..$$ — это
правило psql, не bug. Symptom: deploy 2026-05-24 после merge PR #503 упал с
'syntax error at or near :' on LINE 4 of set_tradein_fdw_password.sql, :'pw'
дошёл до сервера как literal вместо интерполяции.

Fix: передаём password в DO через session GUC (set_config) — psql интерполирует
:'pw' ВНЕ dollar quote, внутри читаем через current_setting(). Очищаем GUC
после выполнения (defense-in-depth).

lekss361 commented

2026-05-24 11:18:03 +00:00

Author

Owner

Deep Code Review — verdict: CHANGES REQUESTED

Summary

Files: 1 (P0 — deploy/credential pipeline)
Lines: +14 / -6
Risk: HIGH (CI log secret leak)
Reversibility: easy (single file)

🔴 Critical (BLOCK) — psql leaks the password to deploy logs

ops/db-bootstrap/set_tradein_fdw_password.sql:18 (и :35 для cleanup):

SELECT set_config('app.fdw_pw', :'pw', false);  -- printed to stdout
...
SELECT set_config('app.fdw_pw', '', false);     -- cleanup, returns ''

set_config(name, value, is_local) -> text возвращает установленное значение. psql по умолчанию печатает result rows на stdout без -q / -A / -t / \o. В .forgejo/workflows/deploy.yml:262-264 вызов:

psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -v ON_ERROR_STOP=on \
  -v "pw=$GENDESIGN_FDW_PASSWORD" \
  < ops/db-bootstrap/set_tradein_fdw_password.sql

результат:

 set_config
------------
 <ACTUAL_PASSWORD>
(1 row)

-> password окажется в Forgejo Actions runner output -> run logs retained -> visible всем кто имеет repo read access. GENDESIGN_FDW_PASSWORD приходит из .env.runtime на VPS, не registered как Forgejo secret -> автоматического маскирования нет.

Fix (один из вариантов, ranked):

\o redirect внутри SQL (минимальная диффа, без изменений в workflow):

\o /dev/null
SELECT set_config('app.fdw_pw', :'pw', false);
\o

DO $$
...
END $$;

\o /dev/null
SELECT set_config('app.fdw_pw', '', false);
\o

Workflow-level redirect (deploy.yml):

psql ... < ops/db-bootstrap/set_tradein_fdw_password.sql > /dev/null

Но тогда NOTICE messages (tradein_fdw_reader password set) тоже глотаются — теряем diagnostic signal. Менее предпочтительно.

Server-side: bypass return value — обернуть SELECT во временную CTE:

WITH _ AS (SELECT set_config('app.fdw_pw', :'pw', false)) SELECT 1 WHERE FALSE;

Хакерство, читается хуже варианта 1.

Рекомендую вариант 1 — explicit, локально в SQL файле, NOTICE-сообщения сохраняются.

🟢 Положительное

✅ Корневой диагноз верный: psql :'pw' не интерполируется внутри $$...$$ (документированное поведение psql, не bug).
✅ format('... %L', current_setting(...)) — %L экранирует password как SQL literal, SQL injection-safe.
✅ Идемпотентность сохранена (IF EXISTS guard для роли).
✅ Cleanup set_config('app.fdw_pw', '', false) в конце — defense-in-depth (хотя session GUC и так умирает с psql exit).
✅ Никаких литеральных паролей в diff'е — password только через -v pw=$GENDESIGN_FDW_PASSWORD.
✅ is_local=false корректно (нет окружающей транзакции; true потребовал бы BEGIN/COMMIT).
✅ PR body хорошо описывает root cause + ссылка на psql docs.

🟡 Minor

app.fdw_pw — namespace app.* в PG 9.2+ не требует регистрации (любой dotted GUC принимается). OK.
Cleanup SELECT не выполнится если ALTER упадёт под ON_ERROR_STOP=on — но session всё равно закрывается -> GUC исчезает. Acceptable.
Rollback path отсутствует в body. Реальный rollback — НЕ revert этого файла (вернёт сломанный :'pw' внутри $$), а unset GENDESIGN_FDW_PASSWORD в .env.runtime чтобы пропустить шаг. Стоит упомянуть в комментарии файла.

Cross-file impact

.forgejo/workflows/deploy.yml:254-273 — invoker без изменений (правильно, psql stdin redirect уже использовался). Если применяется вариант fix #1 — workflow не трогаем.
data/sql/100_tradein_fdw_role.sql — создаёт роль passwordless, comment корректно ссылается на этот bootstrap. Не затрагивается.
tradein-mvp/backend/app/core/fdw.py _PASSWORD_RE — backend whitelist для password в connection string. Этот PR его не меняет.
tradein-mvp/backend/app/core/config.py — читает env, не затрагивается.

Verdict

CHANGES REQUESTED — добавь \o /dev/null / \o bracket вокруг двух SELECT set_config(...) строк. После этого fix логически correct и можно мержить. Текущая версия предотвращает syntax error, но открывает leak password в CI logs — net regression по security.

🚨 Security-sensitive — human merge required after manual diff audit

## Deep Code Review — verdict: CHANGES REQUESTED ### Summary - Files: 1 (P0 — deploy/credential pipeline) - Lines: +14 / -6 - Risk: HIGH (CI log secret leak) - Reversibility: easy (single file) ### 🔴 Critical (BLOCK) — psql leaks the password to deploy logs `ops/db-bootstrap/set_tradein_fdw_password.sql:18` (и :35 для cleanup): ```sql SELECT set_config('app.fdw_pw', :'pw', false); -- printed to stdout ... SELECT set_config('app.fdw_pw', '', false); -- cleanup, returns '' ``` `set_config(name, value, is_local) -> text` **возвращает установленное значение**. psql по умолчанию печатает result rows на stdout без `-q` / `-A` / `-t` / `\o`. В `.forgejo/workflows/deploy.yml:262-264` вызов: ``` psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -v ON_ERROR_STOP=on \ -v "pw=$GENDESIGN_FDW_PASSWORD" \ < ops/db-bootstrap/set_tradein_fdw_password.sql ``` результат: ``` set_config ------------ <ACTUAL_PASSWORD> (1 row) ``` -> password окажется в Forgejo Actions runner output -> run logs retained -> visible всем кто имеет repo read access. `GENDESIGN_FDW_PASSWORD` приходит из `.env.runtime` на VPS, не registered как Forgejo secret -> автоматического маскирования нет. **Fix (один из вариантов, ranked):** 1. **`\o` redirect внутри SQL** (минимальная диффа, без изменений в workflow): ```sql \o /dev/null SELECT set_config('app.fdw_pw', :'pw', false); \o DO $$ ... END $$; \o /dev/null SELECT set_config('app.fdw_pw', '', false); \o ``` 2. **Workflow-level redirect** (`deploy.yml`): ```yaml psql ... < ops/db-bootstrap/set_tradein_fdw_password.sql > /dev/null ``` Но тогда NOTICE messages (`tradein_fdw_reader password set`) тоже глотаются — теряем diagnostic signal. Менее предпочтительно. 3. **Server-side: bypass return value** — обернуть SELECT во временную CTE: ```sql WITH _ AS (SELECT set_config('app.fdw_pw', :'pw', false)) SELECT 1 WHERE FALSE; ``` Хакерство, читается хуже варианта 1. Рекомендую вариант 1 — explicit, локально в SQL файле, NOTICE-сообщения сохраняются. ### 🟢 Положительное - ✅ Корневой диагноз верный: psql `:'pw'` не интерполируется внутри `$$...$$` (документированное поведение psql, не bug). - ✅ `format('... %L', current_setting(...))` — `%L` экранирует password как SQL literal, SQL injection-safe. - ✅ Идемпотентность сохранена (`IF EXISTS` guard для роли). - ✅ Cleanup `set_config('app.fdw_pw', '', false)` в конце — defense-in-depth (хотя session GUC и так умирает с psql exit). - ✅ Никаких литеральных паролей в diff'е — password только через `-v pw=$GENDESIGN_FDW_PASSWORD`. - ✅ `is_local=false` корректно (нет окружающей транзакции; `true` потребовал бы BEGIN/COMMIT). - ✅ PR body хорошо описывает root cause + ссылка на psql docs. ### 🟡 Minor - `app.fdw_pw` — namespace `app.*` в PG 9.2+ не требует регистрации (любой dotted GUC принимается). OK. - Cleanup SELECT не выполнится если ALTER упадёт под `ON_ERROR_STOP=on` — но session всё равно закрывается -> GUC исчезает. Acceptable. - Rollback path отсутствует в body. Реальный rollback — НЕ revert этого файла (вернёт сломанный `:'pw'` внутри `$$`), а unset `GENDESIGN_FDW_PASSWORD` в `.env.runtime` чтобы пропустить шаг. Стоит упомянуть в комментарии файла. ### Cross-file impact - `.forgejo/workflows/deploy.yml:254-273` — invoker без изменений (правильно, psql stdin redirect уже использовался). Если применяется вариант fix #1 — workflow не трогаем. - `data/sql/100_tradein_fdw_role.sql` — создаёт роль passwordless, comment корректно ссылается на этот bootstrap. Не затрагивается. - `tradein-mvp/backend/app/core/fdw.py` `_PASSWORD_RE` — backend whitelist для password в connection string. Этот PR его не меняет. - `tradein-mvp/backend/app/core/config.py` — читает env, не затрагивается. ### Verdict **CHANGES REQUESTED** — добавь `\o /dev/null` / `\o` bracket вокруг двух `SELECT set_config(...)` строк. После этого fix логически correct и можно мержить. Текущая версия предотвращает syntax error, но открывает leak password в CI logs — net regression по security. 🚨 Security-sensitive — human merge required after manual diff audit

lekss361 referenced this pull request from a commit

2026-05-24 11:21:50 +00:00

fix(deploy): mute psql set_config stdout — prevent password leak in CI logs

lekss361 added 1 commit 2026-05-24 11:21:51 +00:00

fix(deploy): mute psql set_config stdout — prevent password leak in CI logs 08a8cf51b3

Review-bot PR #510 нашёл P0: `set_config(name,value,is_local)` возвращает
установленное значение, psql печатает rows на stdout без `-q`/`-A`/`-t`,
итог — фактический pw появляется в Forgejo Actions deploy logs (retained,
visible всем с repo read access).

Fix: оборачиваем оба `SELECT set_config(...)` в `\o /dev/null` / `\o`
brackets. Output mutes только для этих строк — NOTICE-сообщения из DO block
(idempotency signal 'tradein_fdw_reader password set') остаются видимыми.

Также добавлен rollback hint в комментарии: НЕ revert файл (вернёт сломанный
:'pw' внутри $$), а unset GENDESIGN_FDW_PASSWORD в .env.runtime на VPS.

lekss361 commented

2026-05-24 11:27:21 +00:00

Author

Owner

Deep Code Review (re-review) — verdict: APPROVE

Fixup verification (`da78148` -> `08a8cf5`)

Prior BLOCK: SELECT set_config('app.fdw_pw', :'pw', false) -> password в Forgejo Actions deploy logs (no auto-masking, .env.runtime source not registered as secret).

Applied fix — exactly variant 1 from prior recommendation:

\o /dev/null
SELECT set_config('app.fdw_pw', :'pw', false);
\o

DO $$
  ...
END $$;

\o /dev/null
SELECT set_config('app.fdw_pw', '', false);
\o

Lines 30-32 + 47-49. DO block между ними не оборачивается — NOTICE messages (tradein_fdw_reader password set / ... role missing) остаются visible в logs (diagnostic signal preserved).

Verified properties

OK: \o /dev/null глушит ТОЛЬКО result rows из обёрнутых SELECT'ов — это psql client-side redirect, NOTICE-сообщения идут через другой канал (server messages) и не подпадают под \o.
OK: \o (без аргумента) восстанавливает stdout — корректное закрытие bracket.
OK: Password больше не попадает на stdout -> не попадает в Forgejo Actions step output -> не retained в run logs.
OK: Cleanup set_config('app.fdw_pw', '', false) тоже обёрнут (хотя возвращает empty string — defense in depth ради consistency).
OK: Идемпотентность + format(%L) SQL injection-safe — без regression.
OK: Rollback path добавлен в header comment (был noted как minor) — "unset GENDESIGN_FDW_PASSWORD в .env.runtime", НЕ revert файла.
OK: Header comment explicitly предупреждает про set_config return value -> защита от accidentally re-introducing leak в будущем при touching этого файла.

Cross-file impact (unchanged from prior review)

.forgejo/workflows/deploy.yml:254-273 — не модифицирован (правильно, workflow-level redirect глотал бы NOTICE).
data/sql/100_tradein_fdw_role.sql — passwordless role create, не затронут.
tradein-mvp/backend/app/core/fdw.py _PASSWORD_RE — backend whitelist intact.

Residual risk

Minor: \o /dev/null is POSIX-only (Linux runner). Если bootstrap когда-нибудь run будет on Windows runner — /dev/null не существует. Текущий deploy runs on Linux VPS только, не блокирует merge.

Verdict

APPROVE — fix applied correctly, log leak closed, diagnostic NOTICE-сообщения сохранены, no new regression. Net positive по security.

🚨 Security fix verified — human merge required after deploy diff audit (per git-pr.md security tripwire policy для PR на credential pipeline).

## Deep Code Review (re-review) — verdict: APPROVE ### Fixup verification (da78148 -> 08a8cf5) Prior BLOCK: `SELECT set_config('app.fdw_pw', :'pw', false)` -> password в Forgejo Actions deploy logs (no auto-masking, `.env.runtime` source not registered as secret). **Applied fix — exactly variant 1 from prior recommendation:** ```sql \o /dev/null SELECT set_config('app.fdw_pw', :'pw', false); \o DO $$ ... END $$; \o /dev/null SELECT set_config('app.fdw_pw', '', false); \o ``` Lines 30-32 + 47-49. DO block между ними не оборачивается — NOTICE messages (`tradein_fdw_reader password set` / `... role missing`) остаются visible в logs (diagnostic signal preserved). ### Verified properties - OK: `\o /dev/null` глушит ТОЛЬКО result rows из обёрнутых SELECT'ов — это psql client-side redirect, NOTICE-сообщения идут через другой канал (server messages) и не подпадают под `\o`. - OK: `\o` (без аргумента) восстанавливает stdout — корректное закрытие bracket. - OK: Password больше не попадает на stdout -> не попадает в Forgejo Actions step output -> не retained в run logs. - OK: Cleanup `set_config('app.fdw_pw', '', false)` тоже обёрнут (хотя возвращает empty string — defense in depth ради consistency). - OK: Идемпотентность + `format(%L)` SQL injection-safe — без regression. - OK: Rollback path добавлен в header comment (был noted как minor) — "unset GENDESIGN_FDW_PASSWORD в .env.runtime", НЕ revert файла. - OK: Header comment explicitly предупреждает про set_config return value -> защита от accidentally re-introducing leak в будущем при touching этого файла. ### Cross-file impact (unchanged from prior review) - `.forgejo/workflows/deploy.yml:254-273` — не модифицирован (правильно, workflow-level redirect глотал бы NOTICE). - `data/sql/100_tradein_fdw_role.sql` — passwordless role create, не затронут. - `tradein-mvp/backend/app/core/fdw.py` `_PASSWORD_RE` — backend whitelist intact. ### Residual risk Minor: `\o /dev/null` is POSIX-only (Linux runner). Если bootstrap когда-нибудь run будет on Windows runner — `/dev/null` не существует. Текущий deploy runs on Linux VPS только, не блокирует merge. ### Verdict **APPROVE** — fix applied correctly, log leak closed, diagnostic NOTICE-сообщения сохранены, no new regression. Net positive по security. 🚨 Security fix verified — human merge required after deploy diff audit (per git-pr.md security tripwire policy для PR на credential pipeline).

lekss361 merged commit f04359592f into main

2026-05-24 11:29:00 +00:00

lekss361 referenced this pull request from a commit

2026-05-24 11:29:02 +00:00

Merge pull request 'fix(deploy): tradein_fdw_reader password — psql var interpolation in DO block' (#510) from fix/tradein-fdw-psql-var-in-do into main

lekss361 referenced this pull request

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

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