Skip to content

fix(core): sanitize JSONB + fail-closed gate + Postgres type coverage - #3

Merged
faculopezscala merged 3 commits into
mainfrom
worktree-agent-ad2b6456
Apr 6, 2026
Merged

fix(core): sanitize JSONB + fail-closed gate + Postgres type coverage#3
faculopezscala merged 3 commits into
mainfrom
worktree-agent-ad2b6456

Conversation

@faculopezscala

Copy link
Copy Markdown
Contributor

Summary

Three related fixes in the sanitizer module, all aimed at closing the gap
between "PII columns sow knows about" and "columns sow actually ships to
the sandbox".

Issue #4 — JSONB PII leak (closed)

jsonb columns were being copied to the branch verbatim. Real schemas
put PII inside metadata::jsonb constantly ({"email": "..."} in audit
rows, nested contact objects, array-of-user payloads), so this was an
actual leak through the sandbox.

The new JSONB transformer parses the value, walks it recursively, and
replaces any field whose key matches the built-in column-name patterns
(email, phone, firstName, etc.) with the corresponding faker
output — using the same deterministic seed so cross-table consistency
is preserved. Invalid JSON passes through untouched. Scalars, arrays,
deeply nested objects all handled.

Issue #9 — Fail-closed sanitization gate

Unknown Postgres types (a new enum, pg_lsn, hstore, a custom
composite type) previously slipped past the sanitizer and landed in
the branch unchanged. The acquisition-vs-retention reasoning: sow can
always ship an error with --allow-unsafe as the escape hatch, but it
cannot un-leak PII after the fact. So default = abort.

createSanitizer now walks every column of every sampled table, runs
each Postgres type through classifyPgType (safe / handled / unknown),
and throws SanitizationAbort with a clear multi-column error message
if any unknown types are found and config.allowUnsafe is not set.

When --allow-unsafe is passed, the offending columns are NULLed out
in the sanitized output (not passed through!) and a warning list is
surfaced in SanitizationResult.warnings for later sow doctor
reporting. The flag is wired through cli.tsrunConnect
createConnectorsanitizationConfig.allowUnsafe.

Explicit rules in .sow.yml (sanitize.rules: [...]) always take
precedence over the gate — if the user says "this column is
free_text", the gate skips it.

Issue #10 — Postgres type coverage extension

Extended PIIType and the detector/transformer maps to handle the
long tail of Postgres types the sanitizer was previously uncertain
about. Everything below is now either handled with a dedicated
transformer or explicitly marked safe-passthrough so the fail-closed
gate doesn't trip on common schemas:

Postgres type Handling
jsonb, json Recursive field-by-key sanitization
inet, cidr ip_address transformer (preserves CIDR suffix)
macaddr, macaddr8 mac_address transformer (faker internet.mac())
xml xml_text transformer (lorem wrapped in <root>)
bytea binary_blob passthrough (opt-in via explicit rule if needed)
money, interval safe passthrough
int4range, int8range, numrange, tsrange, tstzrange, daterange safe passthrough
text[], int4[], _text etc. base type stripped via stripArraySuffix, classified via base
Custom enums safe passthrough when the enum name is captured by the analyzer (analysis.schema.enums is now threaded into the sanitizer)

Test plan

  • bunx vitest run — 120 passing (was 83/89; +31 new tests)
  • bunx turbo build — clean
  • bunx eslint on touched files — no new warnings (two pre-existing unused-import warnings in connector.ts and connect.ts untouched)
  • Integration test against a live Postgres with jsonb containing PII (not run — no live DB in this env)
  • Integration test against a schema with a custom enum + hstore column (not run — no live DB)

New tests (31 total)

  • transformer.test.ts: +22 tests (7 JSONB cases including nested, arrays, invalid JSON, scalar passthrough, preservation of non-PII fields; 7 new-type transformers; determinism checks)
  • detector.test.ts: +9 tests (classifyPgType for safe/handled/unknown/arrays/enums, pgTypeToPIIType mapping, type-intrinsic PII detection for jsonb and inet)
  • sanitizer-gate.test.ts (new): 8 tests (throws by default, lists columns in error, NULLs on allowUnsafe, explicit rule precedence, safe types pass, custom enums pass, jsonb end-to-end, no-op when no unknown types)

Follow-ups I'd recommend

Integration tests that would need a live Postgres:

  • A real jsonb roundtrip (driver parsing differs — some pg drivers return parsed objects, others return strings)
  • A schema with hstore to confirm the abort message renders correctly end-to-end
  • A custom enum threaded through analyzer → sanitizer to verify schema.enums population matches the classifyPgType lookup

Out-of-scope gaps I noticed while working:

  • SanitizationConfig is duplicated inline inside connector.ts (rules: [] as { table; column; type: any }[]) rather than importing the type — worth cleaning up.
  • There's no yaml-config surface yet for allowUnsafe — only the CLI flag. A .sow.yml key would be natural for teams that want to opt a project into it permanently.
  • free_text transformer for xml fields may still leak PII if someone has email addresses inside their XML blobs — keyed sanitization of XML is doable with a proper parser but not in scope here.

🤖 Generated with Claude Code

faculopezscala and others added 3 commits April 6, 2026 01:16
Adds a JSONB transformer that recursively walks parsed values and
replaces PII-keyed fields (email, phone, name, etc.) using the same
key patterns as column detection. Closes a real PII leak: prior to
this, jsonb columns like audit.metadata were copied verbatim.

Also extends PIIType with the long tail of Postgres types the sandbox
was silently shipping: mac_address, ip_address (inet/cidr), xml_text,
binary_blob (bytea), and an explicit passthrough variant used for
money, interval, range types, and custom enums.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds classifyPgType, stripArraySuffix, and pgTypeToPIIType helpers
in the detector. classifyPgType buckets every Postgres type into
safe/handled/unknown — the sanitization gate uses this to decide
whether a column can be passed through, sanitized, or must abort.

Type-intrinsic PII detection (jsonb, inet, macaddr, xml, bytea) now
routes through pgTypeToPIIType so these columns are caught even when
the column name doesn't match any heuristic.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…atch

The sanitizer previously let unknown Postgres types (pg_lsn, hstore,
custom composite types, a newly-added enum) pass through unchanged.
That's an acquisition-vs-retention tradeoff going the wrong way: sow
can always ship an error with "--allow-unsafe" as the escape, but it
cannot un-leak PII.

createSanitizer now walks every column of every sampled table. Any
type that is neither in the safe-passthrough set nor in the handled-
type set (and isn't explicitly mentioned in config.rules) is collected
into unhandledColumns and the sanitizer throws SanitizationAbort with
a clear multi-column error message.

When --allow-unsafe is passed, those columns are NULLed out in the
sanitized output (not passed through!) and a warning list is surfaced
in SanitizationResult.warnings so `sow doctor` can print it later.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
faculopezscala added a commit that referenced this pull request Apr 6, 2026
… fetch

Three related fixes to packages/core/src/sampler/referential.ts, surfaced
by the eng review as Issues #3, #7, and #8. Same module, one coordinated
refactor, one test pass.

Issue #3 — warnings collection (was: silent catch{} blocks)
  The three try/catch blocks in ensureReferentialIntegrity silently
  swallowed any failure to fetch a missing parent, an orphaned child,
  or an implicit-reference batch. Users got branches with dangling FKs
  and no way to know. Replaced with structured warning collection into
  a new IntegrityWarning[] array. Four kinds:
    - parent_fetch_failed: the missing-parent SELECT threw
    - parent_not_found:    the parent genuinely doesn't exist in source
    - child_fetch_failed:  the ensure-1-child-per-parent SELECT threw
    - implicit_ref_fetch_failed: the implicit IN(...) batch threw
  Warnings are deduped by (source, columns, target) fingerprint so
  a schema with thousands of orphaned rows emits one line per
  relationship, not one per row. Error messages are truncated to 140
  chars to keep metadata bounded.

  ensureReferentialIntegrity now returns { tables, warnings } instead
  of a bare Map. Callers (sampler/index.ts) thread the warnings through
  SamplingResult.integrityWarnings, which flows into
  ConnectorMetadata.integrityWarnings for later surfacing by the CLI.

Issue #7 — dynamic SKIP_IMPLICIT_COLUMNS (was: English-only hardcoded list)
  The old hardcoded ["id", "user_id", "owner_id", "created_by"] set
  was English-biased and conflated "already handled by formal FK" with
  "should never be followed". A Spanish codebase with `creado_por`, or
  an app with `author_id`/`reviewer_id`/`assignee_id` would fall
  through the gap.

  Replaced with a dynamic check: compute the set of (source_table,
  source_column) pairs that appear in any formal Relationship, and
  skip THOSE in the implicit-reference pass. Works for any language
  and any schema. The old symbolic "id" skip is preserved by the
  inferrer itself (inferTargetTable returns null for columns that
  don't end in _id or can't be mapped to a target table).

Issue #8 — batched implicit references (was: N+1 per source-target pair)
  The old resolveImplicitReferences walked (table, column) pairs and
  fired one SELECT per pair, even when multiple source tables all
  referenced the same parent. On a 50-table schema over a VPN that
  produced 100+ round-trips (~30-60s).

  Rewritten to collect all missing ids grouped by target table across
  ALL source tables first, then issue one IN($1,$2,...) query per
  target (still chunked at 100 ids per query to stay well under
  Postgres's 65,535 bind-param limit). Deduplicates ids so three
  source rows referencing the same parent only request it once.

Tests
  10 new tests in packages/core/src/sampler/referential.test.ts:
    - 5 for warnings (happy path, parent_fetch_failed,
      parent_not_found, implicit_ref_fetch_failed, reason truncation)
    - 2 for dynamic skip (formal FK takes precedence, non-English
      columns don't throw)
    - 3 for batching (one query per target across sources, >100 id
      chunking, dedup)
  Total: 99 passing (was 89, +10 new).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@faculopezscala
faculopezscala merged commit 33128de into main Apr 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant