Repository navigation
Conversation
|
Important Review skippedToo many files! This PR contains 907 files, which is 807 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files.
|
🦋 Changeset detectedLatest commit: 929451b The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 436 | 0 | 203 | 0 |
cargo-mutants exited with no surviving mutants recorded. The run may not have completed cleanly; check the workflow run logs.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a05affaaa5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
freshtonic
left a comment
There was a problem hiding this comment.
I request changes for two release problems on the crates.io side. The code itself is in good shape, and every check I ran passes.
How I reviewed this
The diff is large because generated code is committed. I reviewed the generators and their templates, then I ran the drift checks to prove that the committed output matches them. I did not read each generated file.
- EQL codegen:
eql-codegenrenders the version-neutral SQL source two times (eql_v3andeql_v4) from theeql-domainscatalogue. It also generates the Rust payload types andeql_gen.go. - Go:
stashgengenerates the*_stash.gofiles and the goldens.
Checks run (in a worktree at a05affaaa)
- EQL:
cargo test -p eql-codegen(includes the Go parity test): pass.mise run types:check,codegen:parity,test:schemas:parity: pass.mise run build, with the installer completeness gate and the v4 isolation gate: pass.git statuswas clean after all of these.
- Rust and Go:
cargo fmt --check: pass.cargo clippy --workspace --all-features --all-targets -D warnings: pass.cargo test -p stack-guest-abi -p stack-encrypt --all-features: pass.
- Go guests and SDK:
- All five WASI guests build, and the import-surface checks pass.
go vet ./...andgo test ./...inlanguages/golang: pass.go generate ./...leaves no diff.
- I built the v3 bundle and compared it with the
@cipherstash/eql@3.0.6tarball. Onlylints()and comments changed, so the claim "v3 SQL unchanged" holds. git merge-treeagainst currentmain: no conflicts.cargo publish --dry-run -p stack-encrypt-derive -p stack-encryptagainst crates.io: fails (see below).
Blocking
- stack-encrypt 0.3.0 cannot publish. The root release-plz line publishes every stack-* version that crates.io does not have, so merging this starts a publish of stack-encrypt 0.3.0. stack-encrypt requires
stack-kms0.1.0, and that is the only stack-kms version on crates.io. Butsrc/lib.rsre-exportsstack_kms::{diagnostic, ErrorPayload}, and these were added after the 0.1.0 release with no version bump. The packaged build fails withE0432: unresolved imports stack_kms::diagnostic, stack_kms::ErrorPayload.- Fix: bump stack-kms (for example to 0.2.0, with a CHANGELOG entry), and make stack-encrypt require that version.
- Note that the publish dry run on
main(tests-crates.yml) packages all five crates together, so stack-encrypt resolves the tree's stack-kms there. That step does not catch a stack-kms that crates.io lacks under the same version number.
- The stack-encrypt CHANGELOG has no 0.3.0 section.
Cargo.tomlsays 0.3.0, but about 850 lines, including a### Breakingsection, are still under## [Unreleased]. The root release-plz line only publishes and never rewrites the changelog, so 0.3.0 would ship under "Unreleased", as 0.1.0 did. Please rename the section to## [0.3.0] - <date>in this PR.
Non-blocking (see inline)
SchemaVersionis now generic. Rust does not apply a default const parameter in an expression, soSchemaVersion::CURRENTandSchemaVersion::default()stop compiling (E0284). The changeset ships this as a minor bump and says "plainSchemaVersionstill means v3".- The stored and single-bound query domain templates accept
"v": null. The range template already guards against this. - stack-encrypt will publish with an exact pin on
cllw-ore =0.5.2-rc.1. - There are some small nits: stale vitaminc version numbers in comments, a Go wipe path, an ADR reference, and the position of a
use.
Before you merge
eql-bindings now requires stack-encrypt 0.3.0. Its crates.io publish (publish_all_features) succeeds only after release-crates has published stack-encrypt 0.3.0, so blocker 1 must be fixed first. Otherwise the EQL crate publish fails too.
Checked and fine
- SQL renderer: it refuses literal
eql_v3/eql_v4names, unknown placeholders and unmatched blocks. - Range domains and
between: correct, with STRICT/IMMUTABLE accessors and an inlinablebetween. On non-superuser installs, the ORE range domains are disabled with the same NOT VALID CHECK as the single-bound ORE domains. - v4
ore_block_v2SQL: the layout offsets and the GF(2^128) doubling are correct, and errors do not echo the input bytes. - stack-guest-abi
stepandvalue: no new raw reads of host lengths, handles are never reused,MAX_OPENis enforced, and secrets areZeroizingwithOpaqueDebug. - Go step loop: every early exit after a pause aborts the operation and wipes the buffers.
maxPausedis tied to the Rust constant by a test. - Go public surface:
QueryTerm,VerifyandTargetLimitfollow the existingSeal/Openpattern, andMatchQuerynever prints its text. - CI: actions are pinned by SHA, both copies of the path filters were updated, and no release workflow changed.
- Changesets: the EQL changesets are minor, which is correct apart from the
SchemaVersionpoint.chacha20-0-10-2.mdmatches the lockfile moves. - Skills: no skill update is needed.
stash eql installstill installs v3 only, and no skill covers the Go SDK or Stack Encrypt.
Not blocking. The points in this review are still open, but they need not stop the merge.
… node token_derivability derived member and element children of every token a read sends, which for a wildcard at the default bound is 65 tokens, most of them naming positions no row has, and their children name nothing either. Deriving only from tokens that find a node in some row halves the calibration's time (14.8 s to 9.6 s) with the same 44 of 44 mutants caught by the same distinguishers. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
…1363 The leakage profile in #1362 was written against the format before #1363 and has been ported to it. This records, from the code at 9a7ca37 and at the port, every clause, distinguisher and mutant of the two profiles side by side; the failures the old test showed on first meeting the new format and what each one taught; the calibration before and after; and what the database learns now that it did not (the self tag, the token count and the bound it gives away, the probes, the member-or-element rule under a token) and what it no longer learns (an ancestor's capability in every row, the gates with their plaintext positions and selectors, the head token). Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
…akage-profile Brings in b992e78, #1363's correction to the v3 column of the leakage page's comparison table. It touches a different part of the page from the sentence this branch adds, and no code. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
…ry-check fix(stack-encrypt): check that a json_tree read returns the entry asked for
Exercise release and deterministic guests through the host checker, preserving typed JSON numbers and sorted keys. Admit the supported ore_block output in host validation. Require fresh producer artifacts without repeating full CI suites. Refs #1460
Compare stored lengths after SQL-null omission, preserve structured source diagnostics at the Go boundary, and share output and refusal assertions. Refs #1461
feat(stack-encrypt): add owned scalar ciphertext API
test(stack-encrypt): add declaration grammar and shared conformance
An omitted SQL-null field can make an extra key leave the row count unchanged. Validate every source key before encoding so Seal cannot silently discard it. Add shared single-row and late-batch regressions.
…conformance test(stack-encrypt): add shared runtime conformance
…formance test(stack-encrypt): add stored-read and context conformance
…ery-conformance test(stack-encrypt): enroll value-kind and query fixtures
The Go changelog has had only an Unreleased section since before rc.1, so nothing told an rc.2 user what rc.3 changes. It now has a dated rc.3 section holding only what landed after the rc.2 tag, and the rest under a dated rc.2 heading. The rc.3 section adds an upgrade note: the new JSON queries need SQL types and functions the rc.2 EQL bundle does not have, and re-running the installer drops indexes built on eql_v4 functions, which must be re-created. It also lists three fixes since rc.2 that had no entry: Cipher.Seal refusing an unknown key beside an omitted null=sql field, Limit.FetchOr capping its page at the largest SQL LIMIT, and a refused guest request wiping its bearer token. The SDK guide told readers to pass nil vars to Match when a predicate has no variables. A nil vars binds as SQL NULL and matches no row, so following the guide returned nothing, with no error. It now says to pass an empty map, as the generated doc comments and the stashgen reference already did; the package doc says so too. The guide's JSON section gains subheadings for the element match and the jsonpath predicate, so a search for jsonb_path_match lands on it, and a table saying which JSON query to use and which engage an index.
The rc.3 upgrade note told readers to re-create indexes after re-installing EQL v4, but not that the installer drops eql_v4 and eql_v4_internal with CASCADE and issues no GRANT. A deployment whose application connects as a non-owner role would rebuild its indexes and still fail every query with "permission denied for schema eql_v4". The note now lists re-granting first, pointing at EQL's permissions reference, then the indexes. Also drops a doubled blank line under the rc.3 Added heading.
docs(go): an rc.3 changelog section and clearer JSON query docs
Summary
This is the integration branch for a release candidate of EQL v4 and of Stack Encrypt's Rust and Go support. EQL is the SQL that lets PostgreSQL store and search encrypted values; EQL v4 is a second copy of it for values written by Stack Encrypt, the Rust encryption engine behind the Go SDK. The open pull requests for both are merged here first, so they can be tested together before any of them reaches
main.The release version is not decided yet.
Why a branch, not main
Some of this work depends on vitaminc and ore.rs code that is not yet on crates.io, so it pins those crates to a git commit.
stack-encryptandeql-bindingsare published crates, andcargo publishdrops those git pins, so nothing on this branch can be released until vitaminc 0.6 is published (cipherstash/vitaminc#389, which waits on cipherstash/vitaminc#463). Once it is, the git pins become crates.io versions and this branch merges tomainas a normal release.What lands here, in order
feat/aead-remove-legacy-traitsto one commit on vitamincmain, and takes vitaminc's breaking changes since then.Rules for this branch
rev, neverbranch. A branch is deleted when its stack merges.packages/eqlandpackages/stack-encrypt/fuzz. Two commits mean two copies of vitaminc's traits, and the code stops compiling.Changes
Verification
nextwas created frommainat 2a70a47. Nothing has been built yet, because nothing has changed.Related