chore(stack): upgrade EQL v3 bundle (timestamptz → timestamp) + cast_as fix#542
chore(stack): upgrade EQL v3 bundle (timestamptz → timestamp) + cast_as fix#542tobyhede wants to merge 4 commits into
Conversation
|
| Name | Type |
|---|---|
| @cipherstash/stack | Minor |
| @cipherstash/bench | Patch |
| @cipherstash/prisma-next | Patch |
| @cipherstash/basic-example | Patch |
| @cipherstash/prisma-next-example | Patch |
| @cipherstash/e2e | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9895e46 to
0eebebb
Compare
31f2a2f to
d55833f
Compare
Track encrypt-query-language@2e64ca73, which renames the `eql_v3.timestamptz*` domains to `eql_v3.timestamp*`. Regenerate the vendored SQL fixture and propagate the rename through the SDK and tests: - src/eql/v3: TIMESTAMPTZ* domain consts + eqlType `eql_v3.timestamptz*` -> TIMESTAMP* / `eql_v3.timestamp*`; classes `EncryptedTimestamptz*Column` -> `EncryptedTimestamp*Column`; `types.Timestamptz*` -> `types.Timestamp*`; barrel re-exports. - Regenerate __tests__/fixtures/eql-v3/cipherstash-encrypt-v3.sql (new bundle). - Install tooling: the stale-install sentinel moves from `eql_v3.text_search` (present in both generations) to `eql_v3.timestamp` (new this bundle), so a DB carrying an older install is detected as stale and reinstalled (hasEqlV3TextSearch -> hasCurrentEqlV3). - Propagate the rename through the v3 tests, catalog, changesets. Stacked on feat/eql-v3-types-module. NOTE: TIMESTAMP still sets castAs 'date'; if this bundle is meant to fix the time-of-day truncation, a `cast_as:'timestamp'` and re-enabling the schema-v3-client occurredAt skip are still to do.
…ime-of-day)
The native protect-ffi CastAs has a distinct `timestamp` variant (full
date+time) separate from `date` (calendar-date only). Every v3 timestamp domain
was set to `cast_as: 'date'`, so the native layer truncated the time-of-day to
midnight on decrypt.
Add `timestamp` through the cast_as type system and point the timestamp domains
at it:
- schema: `castAsEnum` (SDK/native) and `eqlCastAsEnum` (wasm, via toEqlCastAs)
gain `timestamp`; `toEqlCastAs('timestamp') -> 'timestamp'`.
- eql/v3: `PlaintextKind` gains `timestamp` (decrypts to `Date`, like `date`);
the four `TIMESTAMP*` domain consts set `castAs: 'timestamp'`.
- typed client: `reconstructRow` rebuilds `Date` for `timestamp` as well as
`date`.
TDD: timestamp domains' build() now emits `cast_as: 'timestamp'` (date stays
`date`); catalog + matrix build() assertions updated; the schema-v3-client
`occurredAt` round-trip (a ms-zeroed 12:34:56 instant) is re-enabled to pin
time-of-day preservation live in CI.
d55833f to
645832d
Compare
freshtonic
left a comment
There was a problem hiding this comment.
Verdict: Approve ✅
Multi-angle review (8 finder passes + adversarial verification per candidate, cross-checked against current main). The change is correct and the mechanics verified clean end-to-end:
- The rename is complete and consistent: all four
TIMESTAMP*consts carry the righteqlType/castAs/capability tiers; zero lingeringtimestamptzoutside one intentional comment and legitimately-unrenamed Postgres DDL/codec references; the vendored bundle has 997eql_v3.timestamp*references and notimestamptz. - The
cast_aschain is sound:castAsEnum/eqlCastAsEnum/toEqlCastAs('timestamp') → 'timestamp'all agree, the wasm drift test is genuinely exhaustive over the enum, and protect-ffi 0.26'sCastAsalready includes'timestamp'(verified in the shipped type defs). A suspected timezone hazard inrowReconstructor'snew Date(value)was investigated and refuted: the FFI serializes the timestamp cast viachrono::DateTime<Utc>as RFC3339 (offset-bearing), so parsing is TZ-independent. - CI is fully green including the live suites.
Context worth deciding on before merging forward: main has independently landed a superset of this PR (e626025d renames all scalar domains to SQL-standard names and includes the identical cast_as fix), plus follow-ups that already fix two of the findings below. Merging this stack forward will conflict with main's broader rename — worth reconciling (rebase the stack onto main, or close in favor of main's implementation) rather than letting the two drift.
Findings (none blocking)
1. __tests__/v3-matrix/catalog.ts:220-223 — all four timestamp domains reuse DATE_S, whose samples are both midnight UTC, so the live matrix cannot detect the exact truncation bug this PR fixes.
A regression flipping any timestamp tier back to cast_as: 'date' behavior would pass the entire live matrix (midnight values survive truncation); only the single occurredAt storage-tier test pins time-of-day, leaving timestamp_eq/timestamp_ord/timestamp_ord_ore unpinned. Add a TS_S sample set with a nonzero time-of-day (e.g. 12:34:56.000Z). Still true on main — this is the highest-value follow-up.
2. __tests__/helpers/eql-v3.ts:23 — the sentinel move is one-directional across bundle generations.
Suites on older branches still check eql_v3.text_search, which exists in both generations: after this branch installs the new bundle (leading DROP SCHEMA … CASCADE) into a shared DATABASE_URL, old-branch suites classify it as installed and permanently fail on the missing timestamptz domains with no self-heal. CI uses ephemeral per-job Postgres so the blast radius is developers' shared/local DBs — but main has since adopted a generation-aware compound sentinel (text_search AND timestamp AND eql_v3_internal in hasCurrentGenerationEqlV3) that fixes exactly this; adopt that form here on rebase.
3. Vendored bundle ships as version 'DEV' (eql_v3.version() / COMMENT ON SCHEMA), which is what forces the fragile hand-picked sentinel.
Two consecutive 'DEV' bundles are indistinguishable by the bundle's own version mechanism, so staleness detection has to guess from type names — and a future regeneration that changes only function bodies would have no distinguishing sentinel at all. Main's current bundle carries '3.0.0-alpha.2'; regenerate with a real version (mise run build --version <sha-or-semver>) and key the install check on eql_v3.version() going forward.
4. src/schema/index.ts:230 and :279 — dataType() JSDoc not updated for the widened enum.
Both @param castAs lists still enumerate only 'string' | 'number' | 'boolean' | 'date' | 'text' | 'bigint' | 'json'; the newly-accepted 'timestamp' is undocumented on the exact API where a v2 user would otherwise pick 'date' and hit the midnight truncation this PR fixes. Still stale on main (lines 227/276).
5. Follow-up: packages/cli/src/commands/init/lib/introspect.ts:33-36 still maps Postgres timestamp/timestamptz columns to the truncating 'date' cast.
Not touched by this PR, but the PR makes the gap meaningful: the CLI's codegen emits stack code, and stack now supports the lossless 'timestamp' cast end-to-end — yet every CLI-introspected datetime column is generated with the lossy cast. The CLI's local DataType union (init/types.ts:7) needs a 'timestamp' member. Worth a ticket.
6. Follow-up: packages/cli/src/commands/encrypt/backfill.ts:202 — the date/timestamp backfill warning is very likely stale. (plausible, pre-existing)
The guard claims "protect-ffi does not currently support [date/timestamp] for encryption. The backfill will fail" — but this PR's own live suites bulk-encrypt Date values through both casts successfully, and the stack types document that Date serializes via ISO string at the FFI boundary. The warning predates the timestamp support and now blocks (abort-defaulting confirm) a backfill that plausibly works. Caveat: the proof is v3-client-only; a quick backfill-path test with a timestamp column would settle it. Stale on main too.
7. __tests__/schema-v3.test.ts:462 — the new timestamp cast_as describe block is a strict subset of the matrix coverage.
matrix.test.ts already asserts build() strict-equals { cast_as: spec.castAs, indexes } for every domain, and this PR sets the catalog literals — the same regression pin. Main has already removed this block in its maintainability follow-ups (b44eb557); drop it here too (or keep just the date-vs-timestamp contrast comment on the catalog rows).
8. src/eql/v3/columns.ts:28 + src/encryption/v3.ts:141-144 — "reconstructs to Date" is now encoded in two hand-synced places.
PlaintextFromKind (type level) and rowReconstructor's castAs === 'date' || castAs === 'timestamp' (runtime) each hardcode the date-like set, and PlaintextKind is linked to the SDK CastAs enum by comment only. The next Date-backed (or bigint, per the code's own NOTE) cast must be added in both, and missing the runtime one ships a decrypted value whose declared type lies. A single exported DATE_LIKE_CASTS set / kind→reconstructor mapping next to PlaintextKind would make both derive from one source.
Refuted along the way (for the record)
- Timezone shift in
new Date(value): FFI timestamp plaintexts are RFC3339 (DateTime<Utc>; the binary literally contains "writing rfc3339 datetime to string should never fail") — no local-time parse hazard. @cipherstash/schemacastAsEnumdrift: real but harmless —packages/stackdoesn't depend on that package; drizzle/protect are a parallel surface whose enum shouldn't be widened until protect supports the cast.
Automated multi-agent review (8 finder angles, per-candidate adversarial verification); verified against both the PR branch and current main.
…e casts Follow-ups from PR review (non-blocking): - catalog: give v3 timestamp domains a non-midnight sample set (TS_S) so the live matrix actually detects a cast_as 'timestamp' -> 'date' truncation regression; midnight-only DATE_S survived truncation silently. Add a runnable matrix.test.ts guard so the detection power can't be reverted unnoticed. - schema: document the 'timestamp' cast_as on both dataType() JSDoc @param lists (and restore the dropped 'text' on EncryptedColumn), noting 'timestamp' preserves time-of-day where 'date' truncates. - columns/encryption: extract DATE_LIKE_CASTS as the single source of truth for the date-like set, shared by the type-level PlaintextFromKind and the runtime rowReconstructor (previously hand-synced in two places).
Stacked on #541. Tracks
encrypt-query-language@2e64ca73, which renames theeql_v3.timestamptz*domains toeql_v3.timestamp*, and fixes the time-of-day truncation those domains suffered.1. Bundle rename —
timestamptz→timestamptypes.Timestamptz*→types.Timestamp*; domain consts / classes /eqlType(eql_v3.timestamptz*→eql_v3.timestamp*); barrel re-exports.cipherstash-encrypt-v3.sql).eql_v3.text_search(present in both bundle generations) toeql_v3.timestamp(new this bundle), so a DB carrying an older install is detected as stale and reinstalled.2.
cast_asfix — preserve time-of-dayThe native protect-ffi
CastAshas a distincttimestampvariant (full date+time) separate fromdate(calendar-date only). Every timestamp domain was set tocast_as: 'date', so the native layer truncated the time-of-day to midnight on decrypt. Now:castAsEnum(SDK/native) andeqlCastAsEnum(wasm, viatoEqlCastAs) gaintimestamp;toEqlCastAs('timestamp') → 'timestamp'.PlaintextKindgainstimestamp(decrypts toDate, likedate); the fourTIMESTAMP*domain consts setcastAs: 'timestamp'.reconstructRowrebuildsDatefortimestampas well asdate.schema-v3-clientoccurredAtround-trip (a ms-zeroed12:34:56instant) to pin time-of-day preservation live in CI.Verified
cast_as: 'timestamp', date staysdate,encryptConfigSchemavalidates, fullpnpm buildtypechecks (incl.toEqlCastAsexhaustiveness).occurredAt) run in CI with credentials.Note: the base commit on this branch (
test(stack): v3 lock-context coverage …) belongs to #541 and will drop from this diff once #541 merges.