KB-1A3B

GPT Review — 23-P3A Gateway Allow-list Prompt rev1

8 min read Revision 1
gpt-reviewpack-23p3agateway-allow-listrev2-requiredsql-hardening

GPT Review — 23-P3A Gateway Allow-list Prompt rev1

Date: 2026-05-06
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3a-iu-gateway-allow-list-patch-prompt.md rev1

Verdict

Rev1 is directionally correct but NOT ready to dispatch. Rev2 required.

Opus correctly scoped P3A narrowly and added good safety improvements: full source capture, marker acceptance test, and metadata verification. However, several SQL/runtime details will fail or give false confidence if dispatched as-is.

Accepted Opus additions

  1. Capture full function definition for rollback is correct, but use pg_get_functiondef(oid), not only prosrc.
  2. Testing future marker fn_iu_apply_edit_draft before the function exists is correct.
  3. Metadata preservation checks are correct and should remain.

Required rev2 fixes

P1 — Use pg_get_functiondef, not only prosrc, for rollback

prosrc is only the function body. It is not enough to reconstruct CREATE OR REPLACE FUNCTION safely, including signature, language, SECURITY, search_path, and returns.

Patch preflight:

SELECT
  md5(p.prosrc) AS source_hash_before,
  pg_get_functiondef(p.oid) AS function_def_before,
  p.proowner::regrole AS fn_owner,
  p.prosecdef AS is_security_definer,
  p.proconfig AS fn_config
FROM pg_proc p
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE n.nspname='public'
  AND p.proname='fn_iu_gateway_write_guard';

Rollback must execute function_def_before, not attempt to wrap prosrc manually.

P2 — Fix invalid PL/pgSQL allow-list parsing

This line is invalid PL/pgSQL:

v_allowed := array_agg(trim(elem))
  FROM unnest(string_to_array(v_allowed_csv, ',')) AS elem;

Use:

SELECT array_agg(btrim(elem))
INTO v_allowed
FROM unnest(string_to_array(v_allowed_csv, ',')) AS elem;

Also handle empty/whitespace values:

SELECT array_agg(btrim(elem))
INTO v_allowed
FROM unnest(string_to_array(v_allowed_csv, ',')) AS elem
WHERE btrim(elem) <> '';

Then:

IF v_allowed IS NOT NULL AND v_current = ANY(v_allowed) THEN
  RETURN NEW;
END IF;

P3 — Do not use SAVEPOINT inside DO $$ blocks

PostgreSQL does not allow transaction-control statements such as SAVEPOINT / ROLLBACK TO SAVEPOINT inside PL/pgSQL DO blocks in this way. The current tests will fail before testing gateway behavior.

Fix tests 4–7. Use either:

  • top-level BEGIN; SAVEPOINT ...; ...; ROLLBACK TO SAVEPOINT ...; COMMIT;, or
  • PL/pgSQL BEGIN ... EXCEPTION ... END subtransaction without explicit savepoint.

For tests that may succeed and create rows, prefer top-level transaction rollback through the shell.

P4 — Test 6 must be rewritten as a shell/psql transaction

Test 6 is important and must not leak rows.

Recommended structure:

BEGIN;
SELECT set_config('app.canonical_writer', 'fn_iu_apply_edit_draft', true);
-- attempt controlled IU insert
-- success = marker accepted; then ROLLBACK;
-- gateway blocked = FAIL; then ROLLBACK;
ROLLBACK;

Because psql cannot easily branch on errors with ON_ERROR_STOP=1, use a separate psql call with set +e, capture stdout/stderr, and classify:

  • contains IU Gateway blocked → FAIL;
  • insert succeeds or fails with non-gateway domain/birth/constraint error → PASS for gateway acceptance;
  • always rollback/no committed row.

Do not rely on SAVEPOINT inside DO.

P5 — Test 3 does not prove fn_iu_create still works through gateway

fn_iu_create_plan is dry-run and does not write IU/UV. It does not prove gateway still accepts canonical create writes.

Rev2 must use one of these:

Option A, preferred: create a unique pilot IU through fn_iu_create, retain it, and verify invariants. This is consistent with Pack 22/P3 previous tests.

Option B: use a rollback transaction calling fn_iu_create and verify no row committed. More complex.

Given prior practice, use Option A with a dynamic pilot address and retain it. Report the pilot address. No cleanup.

P6 — Preflight exact function count must schema-qualify

Current count by proname alone can include functions in other schemas or overloads.

Use:

SELECT count(*)
FROM pg_proc p
JOIN pg_namespace n ON n.oid=p.pronamespace
WHERE n.nspname='public'
  AND p.proname='fn_iu_gateway_write_guard'
  AND pg_get_function_arguments(p.oid) = '';

Expected exactly 1.

P7 — ON CONFLICT(key) requires confirmed unique constraint/index

Prompt assumes dot_config(key) has a unique constraint. Add preflight:

SELECT count(*) AS key_unique_count
FROM pg_index i
JOIN pg_class t ON t.oid=i.indrelid
JOIN pg_attribute a ON a.attrelid=t.oid AND a.attnum = ANY(i.indkey)
WHERE t.relname='dot_config'
  AND i.indisunique
  AND a.attname='key';

If not exactly 1, STOP. Do not rely on ON CONFLICT without a valid unique key.

P8 — Preserve README/error semantics exactly enough

The patched function changes the error wording. That is okay if it still includes:

  • IU Gateway blocked;
  • canonical function guidance;
  • README path.

Add a test that direct IU blocked message includes all three substrings:

  • IU Gateway blocked
  • fn_iu_create
  • readme or configured README path

P9 — Grants verification must be precise

has_function_privilege('public', ...) may not mean what the prompt intends. Use information_schema.routine_privileges or aclexplode(proacl) and compare before/after.

Preflight capture:

SELECT grantee, privilege_type
FROM information_schema.routine_privileges
WHERE routine_schema='public'
  AND routine_name='fn_iu_gateway_write_guard'
ORDER BY grantee, privilege_type;

Post-patch verify same or stricter, and PUBLIC not broadened.

P10 — Function metadata preservation should compare before/after

Capture before:

  • owner;
  • prosecdef;
  • proconfig;
  • privileges.

Post-patch compare to before:

  • same owner;
  • prosecdef=true;
  • search_path includes pg_catalog, public;
  • privileges not broadened.

P11 — Restore behavior must remove or restore config deterministically

If allowed_marker_values existed before with the exact value, do not delete it on rollback. If it did not exist before, delete it on rollback. If it existed with exact value and function patch fails, leave it as it was.

Track:

  • ALLOW_KEY_EXISTED_BEFORE=true/false
  • ALLOW_KEY_VALUE_BEFORE

Rollback behavior:

  • existed before → restore old value;
  • did not exist → delete key.

P12 — Report should include committed pilot row if Test 3 uses fn_iu_create

If rev2 uses real pilot create, report:

  • pilot address;
  • create status;
  • invariants result;
  • counts before/after.

No cleanup.

Directive to Opus

Patch P3A prompt to rev2 with P1–P12.

Path:

knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3a-iu-gateway-allow-list-patch-prompt.md

Do not dispatch after patch. Return for GPT/User review.

Hard boundaries remain

  • No P3B schema DDL.
  • No edit draft/comment tables.
  • No edit functions.
  • No vector mutation.
  • No cleanup.
  • No Pack 2C.

Summary

The prompt has the correct goal and scope, but rev1 contains runtime SQL hazards: SAVEPOINT inside DO, insufficient rollback material, and a dry-run test that does not prove fn_iu_create still writes through gateway. Rev2 should harden these before Agent execution.

Back to Knowledge Hub knowledge/dev/laws/dieu44-trien-khai/reviews/gpt-review-23-p3a-gateway-allow-list-prompt-rev1-2026-05-06.md