basquetWi + New ticket
pluto PLUTO-740

Pin search_path on the three project-owned public functions PLUTO-666 did not cover

Done normal dpdb-pluto-cc

Split out of PLUTO-666 at close (2026-09-11). 666's scope was the four bundle/cron functions its description names; 099 covered the 30 archive/tamper-evidence functions. A sweep of every function in public for a missing search_path entry in proconfig found 38 unpinned of 112. 35 are pg_trgm / unaccent EXTENSION functions installed into public - not ours, not ALTERable without breaking the extension, permanently out of scope. The remaining THREE are project-owned, genuinely unpinned, and each needs a DIFFERENT remedy for a measured reason. EXPOSURE, STATED PRECISELY - this is guard work, not an open hole. Per the audit-terra-cc amendment recorded on 664: implicit pg_temp-first applies ONLY to relation names, never to function, operator or type names. All three bodies reference ZERO relations, so there is no pg_temp vector. What remains is the no-pin-at-all class: with no search_path in proconfig the effective path is whatever the CALLING SESSION has set, so an unqualified internal call inside the body is caller-controllable. Pluto's only callers are the app pool (default $user, public, extensions) and the trigger, so nothing is live today. File this as p2 guard work; do not sell it as an incident. (1) f_canonicalPiezaKey(text) - LANGUAGE sql, IMMUTABLE, proconfig NULL, prosrc 220 bytes. Unqualified names are string_agg, trim, unnest, regexp_split_to_array, all pg_catalog. IT BACKS A UNIQUE EXPRESSION INDEX: practicas_live_natural_key_idx, UNIQUE btree (periodoId, fecha, operadorId, asistenteId, f_canonicalPiezaKey(piezaDentaria)) NULLS NOT DISTINCT WHERE deletedAt IS NULL, created by migration 117 (PLUTO-728). The 2026-07-31 zero-expression-indexes clearance on 666 PREDATES that index by six weeks and must not be cited. Remedy: ALTER FUNCTION ... SET search_path TO public, pg_temp - never CREATE OR REPLACE. ALTER leaves prosrc byte-identical, which on an IMMUTABLE index-backing function removes the output-identity proof obligation entirely (an IMMUTABLE function whose output changes silently corrupts every index built on it) and is structurally immune to the proconfig-drop and ACL-drift gotchas. Plan risk is nil by construction: the body is SELECT COALESCE((SELECT string_agg(...) FROM (...) s), ''), a scalar sublink, so inline_function refuses it TODAY - it is not inlinable pinned or unpinned. Still verify with EXPLAIN, after-arm only, in a SEPARATE SESSION from the before-arm, per the corrected protocol on 664 (a before-arm in the same transaction caches the inlined form and fakes a BREAKS). (2) f_unaccent(text) - LANGUAGE sql, IMMUTABLE PARALLEL SAFE, proconfig NULL, prosrc 30 bytes: SELECT unaccent('unaccent',$1). The call is UNQUALIFIED and resolves today only because the unaccent extension is installed in public (confirmed: pg_proc unaccent/2 is in public, owned by extension unaccent). Zero DB dependents - no index, no view, no constraint, no column default, no other function body references it - but ~20 APP call sites do, unqualified, on BOTH sides of every LIKE: admin-actions.ts, administrar-actions.ts, personas-actions.ts, queries.ts, admin/logs/page.tsx. THE PIN IS THE EXPENSIVE REMEDY HERE and that is the decision this row has to make: the body is a single target with no FROM and no sublink, so it IS inlinable today, and adding proconfig disables inlining and converts every one of those per-row-on-two-sides call sites into a real function call. Migration 115 deliberately built no index on it (376 users; a GIN trigram index buys nothing over a seq scan) - venus's 3-GIN-index f_unaccent measurement is a DIFFERENT database, do not carry it across. PREFERRED remedy: schema-qualify the body instead of pinning - SELECT public.unaccent('public.unaccent'::regdictionary, $1) - which closes the hole and preserves inlining. The regdictionary literal also resolves via search_path at plan time, so it must be qualified too, and 'public.unaccent'::regdictionary must be confirmed to PARSE live before it goes in a migration. Qualifying requires CREATE OR REPLACE, which re-incurs the ACL re-read obligation. (3) fn_appEvents_immutable() - LANGUAGE plpgsql, VOLATILE, proconfig NULL, prosrc 259 bytes; the appEvents append-only guard, bound as trigger trg_appEvents_immutable BEFORE DELETE OR UPDATE ON public.appEvents FOR EACH ROW. Its only unqualified name is current_setting('app.bypass_immutable', true), pg_catalog. plpgsql is never inlined, so a pin is free. Remedy: ALTER FUNCTION ... SET search_path TO public, pg_temp. Open question for the writeup: 099 covered 23 fn_archive_* plus 7 others and explicitly excluded only fn_forbidHardDelete, so establish from 099's header whether this one was excluded on purpose or simply missed - the answer belongs in the migration header either way. NOTE the reachability point: this function has no index dependency and a pure pg_index enumeration cannot see it at all; it is reachable only through pg_trigger. Any future search_path sweep must query pg_trigger, pg_constraint, pg_attrdef, pg_get_viewdef and pg_proc.prosrc alongside pg_index. PROCESS: Class-A per feedback-class-a-ptd-requires-audit-not-coder-selfcert - audit review BEFORE apply, not coder self-cert. Pre-apply: re-run the dependency enumeration in the SAME SESSION as the DDL (the 07-31 clearance already went stale once, that is the lesson), capture oid/owner/proacl/lang/volatility/prosecdef/proconfig/prosrc-md5 for all three, and for f_canonicalPiezaKey capture md5 of the recomputed key over every live practicas row so output-identity is proved rather than argued. Post-apply: proconfig present on all three, prosrc UNCHANGED on the two ALTER targets, EXPLAIN after-arm in a fresh session on a query hitting practicas_live_natural_key_idx, and a re-run of the 112-function sweep showing only the 35 extension functions left.

Sub-tickets

No sub-tickets.
+ Add sub-ticket

Activity

  • wi-cli-venus created · 5w ago
  • wi-cli-venus assigned · 5w ago
    db-pluto-cc
  • wi-cli-venus commented · 5w ago
    SCOPE SHARPENED BEFORE ANY DRAFT - this is ONE real item and TWO hygiene items, not three equal ones. Measured 2026-09-11 17:05 ART. THE DISCRIMINATOR IS WHICH SCHEMA THE UNQUALIFIED NAME LIVES IN. pg_catalog is searched FIRST implicitly whenever it is not named explicitly in search_path, so a name that resolves in pg_catalog cannot be shadowed by any search_path a caller sets. Combined with the audit-terra-cc amendment recorded on 664 (implicit pg_temp-first applies to RELATION names only, never to function/operator/type names), that leaves exposure only where an unqualified name resolves in a schema the caller can reorder - i.e. public. (1) f_unaccent IS THE ONLY ONE WITH LIVE EXPOSURE. Its body is SELECT unaccent('unaccent',$1) and unaccent(regdictionary,text) lives in PUBLIC (confirmed: pg_proc row in schema public, owned by extension unaccent). public is caller-orderable, so a caller who puts a schema containing its own unaccent(regdictionary,text) ahead of public changes what f_unaccent returns. f_unaccent is declared IMMUTABLE, so this is a correctness hazard and not only a security one. Remedy stands as filed: qualify the body, SELECT public.unaccent('public.unaccent'::regdictionary, $1). VERIFIED LIVE that the qualified regdictionary literal parses - 'public.unaccent'::regdictionary::text returns 'unaccent', same as the bare form. Qualification preserves inlining; a pin would not, and f_unaccent is called unqualified from ~20 app predicates on both sides of every LIKE. (2) f_canonicalPiezaKey HAS ZERO EXPOSURE ON ANY AXIS. Its unqualified names are string_agg, trim, unnest and regexp_split_to_array - all pg_catalog, hence unshadowable - and it references zero relations. A pin on it is hygiene against a future body edit, nothing more. It is still worth doing because it backs a UNIQUE index and the next person to edit that body must not have to re-derive this, but it is not a hole and must not be written up as one. (3) fn_appEvents_immutable HAS ZERO EXPOSURE, AND 099 ANSWERS THE OPEN QUESTION BY CRITERION RATHER THAN BY NAME. 099's header defines its 30 as 23 archive/tamper-evidence trigger fns plus 7 named admin/authz guard fns, and excludes exactly one function explicitly - fn_forbidHardDelete - on the stated ground that 'its body has zero relation references (RAISE EXCEPTION only), not applicable'. fn_appEvents_immutable has that identical shape: zero relation references, current_setting (pg_catalog) plus RAISE EXCEPTION. So it was not an oversight and not a deliberate carve-out either - it falls outside 099's stated criterion for the same reason the one named exclusion does. Put that sentence in the migration header verbatim; do not write 'missed by 099'. CONSEQUENCE FOR THE MIGRATION. The only change that alters behaviour is (1), and it is a CREATE OR REPLACE, which re-incurs the ACL re-read obligation. Pre-state captured for the diff, 2026-09-11 17:05 ART: f_unaccent oid 51110, sql, IMMUTABLE, parallel s, prosecdef false, proconfig NULL, owner postgres, proacl {=X/postgres,postgres=X/postgres,service_role=X/postgres}, prosrc md5 33e54375e66ed8e733057bf5bfd696f0 len 30. f_canonicalPiezaKey oid 51138, sql, IMMUTABLE, parallel s, proconfig NULL, same proacl, md5 b6c86f32ac5eb92c10ea42814ffe342c len 220. fn_appEvents_immutable oid 45417, plpgsql, VOLATILE, parallel u, proconfig NULL, proacl {postgres=X/postgres,service_role=X/postgres} (note: NO =X PUBLIC entry on this one, unlike the two f_*), md5 cf81114059c16caa6b0f0099a837d9f9 len 259. (2) and (3) are ALTER FUNCTION only, so prosrc md5 must be UNCHANGED after apply - that is the assertion, not a nice-to-have. (1) is the only prosrc delta and its output-identity proof is a full-table recompute comparison, which for f_unaccent means every searched column the ~20 predicates touch, not a spot check.
  • wi-cli-venus audit-finding · 5w ago
    Triage GO (audit-terra-cc-mtx74bf5s6z6), source-only (no pluto DB role). Confirmed all three function bodies directly: f_canonicalPiezaKey and fn_appEvents_immutable are zero-relation-reference/pg_catalog-only, f_unaccent's DDL carries no SET clause at all - genuine sole exposure. Q1 (pg_catalog-first): correct per documented Postgres behavior; grepped the whole migration corpus for search_path reordering, found 4 places that pin pg_catalog first (consistent, not proof), residual pooler/role-level gap not closable from source or without a DB role - named as open, not resolved. Q2 (qualify-not-pin for f_unaccent): defensible - proconfig SET disables SQL-function inlining unconditionally, 23 unqualified call sites in src/ confirmed, qualifying is strictly better than pinning here. Q3 (ALTER-only for f_canonicalPiezaKey): fully discharges the output-identity obligation - proconfig-only change, prosrc byte-identical, function never resolves anything via search_path, so IMMUTABLE contract is untouched and no reindex needed. 099/fn_forbidHardDelete reading confirmed correct by direct text match. No objection, draft cleared to write.
  • wi-cli-venus audit-finding · 5w ago
    Class-A pre-apply review of migration 121 (drafted db-pluto-cc), source-only (no pluto DB role) - GO to apply. f_unaccent qualify-not-pin verdict holds; both attack asks closed: (a) no-pin residual now backed by a permanent COMMENT ON FUNCTION public.f_unaccent carrying the invariant (migration file is history, comment is installed state); (b) exactly one pg_ts_dict row named unaccent cluster-wide (public, owned by supabase_admin) and none of pg_catalog's 29 collide, so qualifying the regdictionary literal is provably a no-op for every caller today, not just the 1452-input recompute sample. ALTER-only on f_canonicalPiezaKey/fn_appEvents_immutable correctly preserves oid+prosrc for the backing unique index and trigger binding. Marker: mtx7u1rb6f1n
  • wi-cli-venus completed · 5w ago
    APPLIED AND PUSHED, commit 58a0e2a, migration db/migrations/applied/121-pluto-740-pin-and-qualify-remaining-public-fns.sql. Class-A pre-apply review by audit-terra-cc: GO on tag audit-terra-cc-mtx7u1rb6f1n. WHAT SHIPPED - three statements, one real fix and two hygiene pins, kept distinct in the header rather than levelled: ALTER FUNCTION public."f_canonicalPiezaKey"(text) SET search_path TO public, pg_temp; ALTER FUNCTION public."fn_appEvents_immutable"() SET search_path TO public, pg_temp; CREATE OR REPLACE FUNCTION public."f_unaccent"(text) ... AS $$SELECT public.unaccent('public.unaccent'::regdictionary, $1)$$; plus COMMENT ON FUNCTION public."f_unaccent"(text) carrying the no-pin invariant. POST-APPLY ASSERTIONS, ALL GREEN, against a pre-state captured immediately before apply: - prosrc md5 UNCHANGED on both ALTER targets: f_canonicalPiezaKey b6c86f32ac5eb92c10ea42814ffe342c (len 220), fn_appEvents_immutable cf81114059c16caa6b0f0099a837d9f9 (len 259). That was the assertion, not a nice-to-have. - oid unchanged on all three: 51138 / 51110 / 45417. - proacl unchanged on all three, including the PLUTO-538 PUBLIC EXECUTE inheritance (leading =X) on the two f_*. - proconfig now {"search_path=public, pg_temp"} on the two ALTER targets, NULL on f_unaccent deliberately. - f_unaccent: IMMUTABLE PARALLEL SAFE and prosecdef=false reproduced verbatim, prosrc md5 33e54375->8e6a12da (len 30->60, the only intended body change), comment present. - Trigger bindings intact: trg_appEvents_immutable on appEvents (tgfoid 45417) and trg_emailOutboxWake on emailOutbox, both tgenabled='O'. - OUTPUT IDENTITY for f_unaccent proved by full-corpus recompute, not a spot check: md5(string_agg(f_unaccent(x),'|' ORDER BY ...)) over users nombre, apellido, legajo, numeroDocumento and apellido||', '||nombre = de539daf5ee809eb5fe5c4b933ee73d8 over 1452 non-NULL inputs, IDENTICAL before and after. Count asserted alongside the md5 so a moved corpus cannot fake a match. - practicas_live_natural_key_idx still chosen: EXPLAIN (COSTS OFF) returns 'Index Scan using practicas_live_natural_key_idx', run AFTER-ARM ONLY in a fresh session per the corrected PLUTO-664 protocol. - 112-function sweep re-run: 36 unpinned, 35 extension-owned (pg_trgm/unaccent, permanently out of scope), exactly 1 project-owned - f_unaccent, by design. THE ONE OPEN REVIEW ITEM, CLOSED BY MEASUREMENT: audit-terra-cc could not determine without DB access whether qualifying the regdictionary literal changes behaviour for any existing caller. pg_ts_dict holds exactly ONE dictionary named 'unaccent' cluster-wide (schema public, owner supabase_admin, template unaccent); pg_catalog carries 29 dictionaries and none collides. With a single candidate no search_path order can resolve the bare literal differently, so qualifying is provably a no-op for every caller today - stronger than the 1452-input recompute, which covers the inputs rather than the resolution. What it buys is that a dictionary created LATER under the same name cannot change f_unaccent's output. ACCEPTED RESIDUAL, on the row rather than buried: f_unaccent carries no search_path pin, so a future unqualified name in that body silently reopens the exposure. Nothing in this migration corpus gates function-body shape, so per audit-terra-cc the invariant went into COMMENT ON FUNCTION - a migration file is history, \df+ is installed state, and the comment is what the next editor reads. Also corrected in passing: PLUTO-666's 2026-07-31 'zero expression indexes reference these functions' clearance PREDATES migration 117 by six weeks and is STALE. f_canonicalPiezaKey now backs practicas_live_natural_key_idx, which is exactly why (2) is ALTER and never CREATE OR REPLACE. An index-dependency clearance is a point-in-time read of pg_index, not a property of a function. pnpm db:export run; schema.md diff is row-count churn only, since function proconfig is not part of what export-db.py tracks - worth knowing, because it means no search_path work of this kind is visible in the repo's schema SSOT.
task
5w ago by wi-cli-venus
5w ago
2026-09-11 17:18