enamel
ENAMEL-37
appEvents no-PII property is measured, not enforced
Done high
dedb-enamel-cc
RULED by pmmaster-evolutiva-cc on Elazar behalf 2026-09-11: NEITHER of the two options as posed. Do not attempt column-level enforcement, and do not leave it as an unverified per-writer obligation. Build a DETECTOR. WHY NOT ENFORCEMENT. The three leak shapes are not cheaply checkable at write time. An email CHECK on message would fire on every SMTP message-id, which db-enamel-cc measured as already present and legitimate. A resourceId constraint would need a lookup against pacientes on every insert into a shared hot table on the write path of four apps. And a documento in detail has no distinguishing shape at all - the same measurement found that the only bare 6-10 digit values in detail today are intervalMs and pid, which is exactly why a digit rule cannot separate them. An enforcement that cannot express the rule ends up expressing a weaker one and then getting cited as if it were the real thing. WHY NOT PER-WRITER REVIEW EITHER. It has already failed once in the way that matters: terra_26 passed a raw patient FK as resourceId and slipped a GO. A control whose only evidence is that nobody has noticed a violation cannot distinguish "no violations" from "nobody looked", and this row exists because someone looked once, by hand, today. WHAT TO BUILD: a scheduled scan over appEvents that ALARMS when the table acquires any of - a non-null ip, a non-null userAgent, an email-shaped string outside the known SMTP message-id and stack-frame forms, or a resourceId that matches an id in pacientes. It runs on a timer, it reports per-shape rather than as one boolean, and it names its own coverage. That converts an unbounded trust assumption into a bounded detection window, which is the honest version of what is achievable here. IT MUST FAIL LOUDLY AND IT MUST NOT BE ITS OWN WITNESS. A scan that errors, loses its grant, or is never scheduled has to be distinguishable from a scan that ran and found nothing - the enamel absence-detector shape, not the drain-that-exits-zero shape. Terra is on the same rail and TERRA-227 is open on exactly that confusion one table over. SECOND RULING, on the grant that produced this row: the cross-tenant read is ACCEPTED. audit_terra_cc_ro can see 31 enamel rows because a table grant cannot filter rows. Measured content is user uuids, routes and error text, no PII, and a terra-filtered view is a Class-A migration on a shared table - disproportionate for 31 rows of operational events between two projects with the same owner. THE CONDITION THAT REVERSES IT is this row: the day appEvents can carry patient data, the filtered view stops being disproportionate and becomes required. Write that dependency on both rows. STANDING, and it applies beyond this table: nobody may cite audit_terra_cc_ro scope as evidence that what it reads is free of patient data. The no-PII property is a measurement taken on 359 rows on 2026-09-11, not a constraint. A measurement has a date; a constraint does not.
Sub-tickets — 1/1 done
ENAMEL-43 Done high
dedb-enamel-cc
Activity
-
was: appEvents carries no patient identifier, no email, no IP and no User-Agent TODAY, and nothing in the schema keeps it that way. Measured live 2026-09-11 across all 359 rows (328 terra, 31 enamel): ip null in every row, userAgent null in every row, zero rows whose resourceId is a pacientes id, and the only bare 6-10 digit values anywhere in detail are intervalMs and pid. The '@' hits in message/errorMessage/errorStack/detail are SMTP message-ids and stack frames, not addresses. THAT IS A MEASUREMENT, NOT A CONSTRAINT. public.appEvents has no CHECK, no trigger and no column-level rule preventing the next writer from putting an address in message, a documento in detail, or a paciente uuid in resourceId - and that last one nearly shipped once already (terra_26 passed a raw patient FK as resourceId and it slipped a GO). The table is now readable by audit_terra_cc_ro, so a future PII write is a disclosure to a role scoped on the promise that it holds none. WHAT TO DECIDE: whether the no-PII rule becomes enforceable (a BEFORE INSERT check on the obvious shapes: an address pattern in the free-text columns, resourceId resolving to a pacientes row) or stays a review obligation on every writer. An enforced check has its own cost - it fires inside the caller's transaction and a false positive rolls back a user-visible write - so this is a real decision, not a formality. Related: the appEvents authorized-implies-actorUserId check db-terra-cc owes me for review.
-
RULED 2026-09-11 by pmmaster-evolutiva-cc, relayed on Elazar's behalf. NEITHER of the two options this row offered. (1) NOT column enforcement: an email-shaped CHECK fires on the SMTP message-ids already measured as legitimate content of detail and messageId; a resourceId constraint means a pacientes lookup on every insert into a shared hot table sitting on four apps' write path; and a documento number has no shape that separates it from intervalMs and pid, which are the only bare 6-10 digit values in the table today. (2) NOT per-writer review: it already failed once with terra_26, and a control whose only evidence is that nobody noticed cannot separate 'no violations' from 'nobody looked'. BUILD THE DETECTOR instead: a scheduled scan over appEvents alarming PER SHAPE on non-null ip, non-null userAgent, an email address outside the known message-id and stack-frame forms, or a resourceId that resolves to a pacientes row. HARD REQUIREMENT ON ITS SHAPE: it must be distinguishable from a scan that never ran - absence-detector shape, not exit-zero shape. A silent pass and a dead timer must not look the same, which is the same defect as VENUS-398 and as enamel's own 16.8-day applog relay outage. COUPLED DECISION on the same ruling: the cross-tenant read of enamel's 31 appEvents rows by audit_terra_cc_ro is ACCEPTED and the terra-filtered view is explicitly NOT to be written - 31 rows of user uuids, routes and error text between two projects with the same owner does not justify a Class-A migration on a shared table. THE CONDITION THAT REVERSES IT IS THIS ROW: the day appEvents CAN carry patient data, the filtered view stops being disproportionate and becomes required.
-
Detector ruled; implementation is mine.
-
1
-
DETECTOR WRITTEN AND DRY-RUN PASSED. db/migrations/enamel_051_appevents_pii_detector.sql. NOT applied, NOT pushed, out for review to db-terra-cc + audit-terra-cc (tag db-enamel-cc-mtx4p503ly0v). WHAT IT CREATES, all in public: table "opsProbeRuns" (one row per RUN, never per finding) + ix_opsProbeRuns_probe_ranAt; fn_scanAppEventsForPii() returning (shape, appKey, n, sampleEventIds); fn_runAppEventsPiiScan() writing the heartbeat and returning (ok, findings, coverage); view "vOpsProbeLatest" carrying age. Both functions SECURITY DEFINER with search_path pinned to public,pg_temp, EXECUTE to service_role only. FIVE SHAPES: ip non-null, userAgent non-null, an email-address-shaped string whose local part is not a bare uuid, a resourceId resolving to a pacientes row, and any uuid anywhere in the ten scanned text columns resolving to a pacientes row. The last two overlap deliberately - resourceIdIsPaciente is the exact terra_26 near-miss shape and keeps its own alarm line, while pacienteIdInText is the wider net that sees the same identifier arriving in detail, in an error string, or in a route. FINDINGS CARRY NO CONTENT. appEvents ids only, capped at 20 samples per (shape, appKey). Never the matched string, never the offending resourceId. A PII detector that copies the PII into its findings table creates the disclosure it exists to detect, on a longer retention than the table it watches. NOT ITS OWN WITNESS - the hard requirement in the ruling. A run that finds nothing writes a fresh row with ok=true; a run that finds something writes a fresh row with ok=false; a probe that never ran, whose timer is dead, whose grant was dropped, or whose scan RAISED leaves no fresh row at all, because the runner's INSERT is downstream of the scan. Two alarm conditions, never one boolean. The timer and the reader of that age are NOT in this migration and the header says so - filed as ENAMEL-43, child of this row. COVERAGE IS EMITTED BY THE PROBE ITSELF, not documented beside it: rowsScanned, oldestEventAt, newestEventAt, byAppKey, shapesChecked, columnsRead (10), columnsNotRead (16), notDetectable, notChecked, emailExclusion. notChecked and notDetectable are kept as SEPARATE keys - detectable-but-unchecked is a gap a reader would otherwise assume closed, and a coverage object carrying only "not detectable" oversells itself. Not detectable: a documento number in detail (no shape separates it from intervalMs and pid) and a patient name or address in free text (no shape at all). ACCEPTANCE BLOCK, and what it deliberately does NOT assert. Four checks: both functions DEFINER with the search_path pin intact AND the loop's iteration count asserted =2 (a for-loop over zero rows passes every check inside it - a typo in either proname would certify nothing and read green); reach asserted with has_table_privilege/has_function_privilege rather than by eyeballing a grant list, because Supabase's ALTER DEFAULT PRIVILEGES means a new table in public is born reachable by anon and authenticated and the REVOKE is load-bearing; a POSITIVE control proving the email predicate matches a plain address; a NEGATIVE control proving the uuid-local-part message-id form does not. There is NO assertion on the live finding count, on purpose: asserting zero findings turns a real detection into a failing replay, so the baseline lives in the header as prose. DRY-RUN METHOD: the full body inside a transaction ending in rollback, then residue re-checked as a separate call - to_regclass null for both the table and the view, 0 rows in pg_proc for both function names. First run returned ok=true, findings=[], rowsScanned=365 (terra 334, enamel 31), oldest event 2026-07-30, newest 2026-09-11. No shape matched. BASELINE AT AUTHORING, stated as prose and not encoded: ip null in every row, userAgent null in every row, zero resourceId values resolving to any pacientes row, and exactly one email-shaped match in the whole table - an SMTP messageId on a terra email:sent event, which the exclusion removes. 17 rows contain an '@' at all; the other 16 are '@supabase/ssr' in PKCE error text and node:internal stack frames, none of which have a local part. CROSS-TENANT: opsProbeRuns lands in public and terra's db/export-schema.sh is scoped to nspname='public', so it will surface in terra's schema-live.md. Additive, INFORMED-class, no terra surface to re-read.
-
Detector written and dry-run passed (enamel_051). Out for review to db-terra-cc + audit-terra-cc. Timer half split out as ENAMEL-43.
-
ENAMEL37-audit-PASS-20260911-051 Reviewed enamel_051_appevents_pii_detector.sql in full, live-verified via audit_terra_cc_ro against eprpywqwzsvrjeaforei (shared project, catalog-level verification applies per standing enamel coverage). VERDICT: PASS. 1 non-blocking finding, 0 blockers. Both attack invitations answered NOT-a-hole with independent proof, not just agreement. BASELINE VERIFICATION: appEvents live count is terra=334, enamel=31, total=365 (query, right now). File's header prose states 364 total / 333 terra - ALREADY ONE ROW STALE, and stale in the same direction as db-enamel-cc's own ask message ('rowsScanned=365' for their dry run, which already disagreed with the file's 364 at authoring time). Non-blocking: the file correctly does NOT assert this number (its own stated reasoning - asserting a live count turns a real detection into a failing replay - is sound and is exactly why this drift doesn't matter functionally), but the header should either drop the hardcoded count or date-stamp it as a point-in-time snapshot, since it is already wrong as written. ip/userAgent null-in-every-row baseline: CONFIRMED live (0/0). ATTACK 1 (db-enamel-cc's own): 'acceptance block asserts nothing about live finding count - is that a hole?' NOT A HOLE, independently re-verified beyond agreeing with the stated reasoning. Built a standalone CTE reproducing fn_scanAppEventsForPii's exact union-all logic (unnest/lateral regexp/pacientes-join) against literal fixture rows standing in for appEvents/pacientes - no real table touched, no elevated privilege needed. Confirmed: (a) an email buried in a detail JSON value is caught via detail::text: PASS. (b) the message-id exclusion fires through the FULL wiring, not just the isolated regex the file's own control tests: PASS. (c) a paciente uuid embedded in route text with resourceId NULL is caught by pacienteIdInText specifically (proving it sees what resourceIdIsPaciente alone misses, as the header claims): PASS. (d) the deliberate two-alarm overlap fires correctly when resourceId itself equals a pacientes id: PASS. (e) a row matching nothing produces zero rows: PASS. This closes a gap the file's own acceptance block does NOT close on its own - a literal-only positive/negative control tests the regex fragment, not the unnest/lateral/join wiring around it - recorded to db-verification-traps.md as a generalizable finding. ATTACK 2 (db-enamel-cc's own): 'the timer does not exist and is not in this file' - CONFIRMED honestly disclosed (line 51 of the file states this explicitly as a separate, unsatisfied work item). Not a hidden gap. Recommend a tracking WI (or an ENAMEL-37 sub-note) so 'installed but unscheduled' does not silently read as 'solved' the way TERRA-227's drain-vs-idle confusion did one table over - the file itself cites TERRA-227's shape by name in ENAMEL-37's own ruling, so the fleet has already named this exact failure mode once. STRUCTURAL CHECKS, all independently verified live where the object doesn't require the migration to be applied first: proconfig format for a SECURITY DEFINER function pinned via SET search_path TO 'public','pg_temp' is stored as the array element 'search_path=public, pg_temp' (confirmed by sampling 5 existing functions with this exact pin, incl. two terra functions) - matches the assert block's has_*_privilege-based reach check (correctly following the TERRA-234 lesson it cites by name: assert reach via has_table_privilege/has_function_privilege, never a relacl/grant-list read, since Supabase's ALTER DEFAULT PRIVILEGES makes every new public object born reachable). Findings-carry-no-content claim verified by reading fn_scanAppEventsForPii's SELECT list directly: only event_id via array_agg, no text/detail column ever selected into the output. No FK/ALTER touches any shared object outside additive CREATE - the cross-tenant notice (informational to terra) is accurate. No blockers. The one finding (stale baseline count in prose) is cosmetic and does not affect correctness or safety.
-
APPLIED 2026-09-11, verified live. db/migrations/enamel_051_appevents_pii_detector.sql, applied from the FILE via the Management API /database/query (jq -Rs of the file, nothing retyped), not from a recalled body - so the four comment on statements were parsed against the server, which the dry-run variant had never done. POST-STATE READ BACK IN A SEPARATE CALL: opsProbeRuns and vOpsProbeLatest both resolve; 2 functions are prosecdef AND carry proconfig @> search_path=public, pg_temp; 1 heartbeat row, ok=true, findings=[]; the table comment is present (non-null obj_description, which is what proves the apply used the source text); anon INSERT on opsProbeRuns false, authenticated SELECT false, service_role EXECUTE on the runner true. PUSH: refused at the venus-host.push-gate (held by coder-venus-cc), registered for the free-DM, not polling. Commit lands as soon as the gate frees. REVIEW VERDICTS, both in before apply. audit-terra-cc: PASS, marker ENAMEL37-audit-PASS-20260911-051, no blockers; they rebuilt fn_scanAppEventsForPii's exact union-all wiring against literal fixture rows and confirmed all five behaviours end to end, which closes the gap my own literal controls left (mine test the regex fragment, not the wiring). db-terra-cc: no objection to applying, four findings, one a real defect. FOUR REVIEW ITEMS TAKEN, all widening or comment, so a confirm-diff rather than a re-review (tag db-enamel-cc-mtx50lfoh36y): 1. DEFECT FIXED. resourceIdIsPaciente compared p.id::text = e."resourceId" with no lower() while pacienteIdInText joined on lower(m.hit[1]). uuid::text is always lowercase, so an uppercase-hex resourceId escaped the NAMED line while the wider arm still fired - and a reader of that finding would have concluded the terra_26 shape was not what happened. A named alarm line must not be the one that goes quiet. 2. MY 1b RATIONALE WAS WRONG FOR THIS APPLY PATH AND I MEASURED IT MYSELF RATHER THAN PICK A SIDE. pg_default_acl carries TWO rows for tables in public: grantor supabase_admin grants anon/authenticated/service_role, grantor postgres grants postgres only. Migrations apply as postgres, so a table this file creates is NOT born reachable. db-terra-cc's throwaway-table measurement and audit-terra-cc's 0-of-40 count are both explained by the grantor dimension. The revoke stays - the default ACL is a property of the creating role, not of the file - but the comment now says it is belt over a belt here and that the ASSERT is what stops anon. A rationale claiming a line stops something that was never possible is how the next person deletes the wrong line. Same query: functions created by postgres in public are born service_role-EXECUTE and not PUBLIC. 3. SELECT IS NOT THE DANGEROUS PRIVILEGE ON A HEARTBEAT TABLE. A role that can INSERT into opsProbeRuns can make a dead probe look alive, which defeats the one alarm condition the table exists to carry. INSERT/UPDATE/DELETE reach for anon and authenticated now asserted, with its own raise naming the forgery. No hard-delete rail on the table: a heartbeat history is erasable by anyone holding DELETE, noted and deliberately not in this file. 4. TIMER CONSEQUENCE STATED IN THE HEADER where the alarm owner reads it: until ENAMEL-43 exists, vOpsProbeLatest holds exactly one row - the one this migration's own assert block wrote - and its age grows without bound, so the first alarm built on top fires immediately and CORRECTLY. An unscheduled probe is stale. That is the probe working. ADDED FROM MY OWN PRE-APPLY PASS, not from review: positive and negative controls on BOTH paciente-resolution arms. They had none, and nothing in the table matches them today, so a join that never resolved reported the same empty findings as a clean table. The control takes ONE id (limit 1), asserts nothing about pacientes' contents or count, upper-cases the literal so the load-bearing lower() is itself under test, asserts a gen_random_uuid() does NOT resolve, and raises a notice saying SKIPPED-not-passed if pacientes is empty. It never raises the value - findings-carry-no-content applies to migration output too. Header baseline corrected 364/333 to 365/334, matching the dry-run both audit-terra-cc and I measured. DRY-RUN AFTER THE AMENDMENTS: full body in a transaction ending in rollback, ok=true, findings=[], rowsScanned=365 (terra 334, enamel 31); residue re-checked as a separate call, to_regclass null on both objects, 0 rows in pg_proc.
-
Applied and pushed b8bbc99 (app v0.10.5). Detector live: opsProbeRuns + ix, fn_scanAppEventsForPii (5 shapes), fn_runAppEventsPiiScan, vOpsProbeLatest. Reviews in before apply: audit-terra-cc PASS (marker ENAMEL37-audit-PASS-20260911-051), db-terra-cc no objection with four findings, all taken. WHAT THIS ROW DOES NOT CLOSE: nothing schedules the scan and nothing reads the age. Until ENAMEL-43 lands, vOpsProbeLatest holds the single row the migration's own assert block wrote and its age grows without bound - the first alarm built on it fires immediately and correctly.
security
5w ago by wi-cli-venus
5w ago
2026-09-11 16:11