# v0.58.0 — Security & Correctness Hardening (Full Details)

> **Summary:** [v0.58.0.md](v0.58.0.md)
> **Assessment source:** [plans/PLAN_OVERALL_ASSESSMENT_12.md](../plans/PLAN_OVERALL_ASSESSMENT_12.md)
> **Findings addressed:** S-1, S-2, S-3, S-4, C-1, C-2, C-3, C-4

This release addresses all HIGH-severity findings from the v0.57.0 overall
assessment (Report 12).  No new SQL API surface is added.  Every change is a
targeted security fix, correctness fix, or correctness-observable improvement
to an existing code path.

---

## Security Fixes

### SEC-1: attach_outbox() / detach_outbox() / attach_embedding_outbox() Ownership Check

**Assessment finding:** S-1 (HIGH)
**Files:** `src/api/outbox.rs` — `attach_outbox_impl`, `detach_outbox_impl`,
`attach_embedding_outbox_impl`

The three outbox management functions are missing the `check_stream_table_ownership()`
call that all other mutating APIs (`alter_stream_table`, `drop_stream_table`,
`pause_stream_table`, `resume_stream_table`) use as a hard gate.

**Before:** Any role with `EXECUTE` on the `pgtrickle` schema could attach a
`pg_tide` outbox to a stream table owned by a different role and thereby
observe or modify its change stream.

**After:** `check_stream_table_ownership()` is called immediately after
`StreamTableMeta::get_by_name()` in all three functions.  A non-owner
receives `PgTrickleError::NotStreamTableOwner` with a descriptive message.

### SEC-2: stream_table_to_publication() / drop_stream_table_publication() Ownership Check

**Assessment finding:** S-2 (HIGH)
**File:** `src/api/publication.rs` — `stream_table_to_publication_impl`,
`drop_stream_table_publication_impl`

Same gap as SEC-1 but for the logical-replication publication API.  A
non-owner could issue `CREATE PUBLICATION` over another user's stream table,
exposing its storage to arbitrary subscribers.

**After:** Same fix as SEC-1 applied to both publication functions.

### SEC-3: DDL Hook Escalates Instead of Silently Returning on SPI Error

**Assessment finding:** S-3 (MEDIUM)
**File:** `src/hooks.rs` — `handle_alter_table`

When `find_view_downstream_pgt_ids()` encounters an SPI error (e.g., during
catalog contention or a concurrent DROP EXTENSION), the handler now:
1. Retries the query once with a fresh `Spi::connect()`.
2. If the retry also fails, raises `pgrx::error!()` with the message
   `"pg_trickle: DDL hook could not inspect dependencies — schema change blocked to prevent inconsistent state"`.

This ensures that an upstream ALTER TABLE that would invalidate stream tables
does not proceed silently when catalog inspection fails.

### SEC-4: Schema Identifier Quoted in CDC Buffer Name Construction

**Assessment finding:** S-4 (MEDIUM)
**File:** `src/cdc.rs` — `buffer_qualified_name_for_oid`

**Before:**
```rust
format!("{change_schema}.{base}")
```

**After:**
```rust
sql_builder::qualified(change_schema, &base)
```

The `change_schema` value is now properly quoted using the centralised
`sql_builder` API, preventing malformed SQL if an unusual schema name
(spaces, mixed case, special characters) is used.

---

## Correctness Fixes

### COR-1: Multi-Column NOT IN Rewrite — NULL Row Safety

**Assessment finding:** C-1 (HIGH)
**File:** `src/dvm/parser/sublinks.rs` — multi-column IN/NOT IN rewrite path

The v0.55.0 multi-column IN rewrite converts `(a, b) IN (SELECT x, y …)` to
a SemiJoin and `NOT IN` to an AntiJoin.  For `NOT IN`, SQL mandates that any
`NULL` value on either side of the correlation predicate produces UNKNOWN,
which means the outer row must be excluded.  AntiJoin semantics differ: a
correlation predicate that evaluates to UNKNOWN keeps the row.

**Fix:** Before generating the AntiJoin rewrite for `NOT IN`, the parser now:
1. Inspects whether any element of the left-side row constructor is a `NULL`
   constant expression (`IS_NULL_CONST`).
2. Inspects whether any column in the subquery's target list is nullable
   (non-strict expression or explicitly `NULL`).
3. If either condition is detected, the rewrite is skipped and the original
   subquery-based execution path is used instead, with a diagnostic
   `NOTICE`-level message:
   ```
   NOTICE: pg_trickle: multi-column NOT IN with nullable elements cannot be
   rewritten to an anti-join; falling back to subquery-based delta computation.
   ```

`docs/LIMITATIONS.md` gains a new subsection documenting this behaviour.

### COR-2: Recursive CTE Depth Guard Applied in DIFFERENTIAL Mode

**Assessment finding:** C-2 (HIGH)
**File:** `src/dvm/operators/recursive_cte.rs` — strategy selection

**Before:**
```rust
let max_depth = if matches!(ctx.delta_source, DeltaSource::TransitionTable { .. }) {
    crate::config::pg_trickle_ivm_recursive_max_depth()
} else {
    None  // no guard in DIFFERENTIAL mode
};
```

**After:**
```rust
let max_depth = crate::config::pg_trickle_ivm_recursive_max_depth();
```

The guard is now unconditional and consistent across both modes.  When
`max_depth` is reached in DIFFERENTIAL mode, the recursive CTE falls back
to the `FullRescan` strategy with a `warning!()` logged.

### COR-3: WAL Decoder Slot Eligibility — Advisory Lock

**Assessment finding:** C-3 (MEDIUM)
**File:** `src/wal_decoder.rs`

A new `pg_advisory_xact_lock` keyed on `BIGINT(slot_oid)` is acquired before
calling `is_slot_suitable_for_wal_transition()` and released after
`poll_wal_changes()` returns.  The lock acquisition is done via:
```sql
SELECT pg_advisory_xact_lock($1::bigint)
```
Any concurrent session attempting to drop the replication slot will block on
the same lock, serialising the eligibility check and consumption into an
atomic unit.

### COR-4: Compact-Buffer Contention Is Observable

**Assessment finding:** C-4 (MEDIUM)
**File:** `src/cdc.rs` — `compact_change_buffer_inner`

**Before:** returned `Ok(0)` on `pg_try_advisory_xact_lock` failure,
indistinguishable from "no rows to compact".

**After:** returns `Ok(CompactionResult::Contended)`.  The caller in the
scheduler logs at `debug!` level and increments
`pg_trickle_cdc_compact_contended_total` (a new `PgAtomic<u64>` in
`src/shmem.rs`).  The Prometheus endpoint exposes this counter so operators
can detect persistent lock contention via alerting.

---

## Upgrade Notes

No SQL schema changes.  No `ALTER EXTENSION` migration is required.

After upgrading, owners of stream tables who had previously used
`attach_outbox` or `stream_table_to_publication` should verify that the
operations complete as expected under the new ownership checks.  Non-owner
callers will now receive `ERROR: not stream table owner` instead of silently
succeeding.