Skip to content

fix: review findings on the next-to-main release candidate (#1229) - #1403

Draft
coderdan wants to merge 9 commits into
nextfrom
fix/rc-review-1229
Draft

coderdan wants to merge 9 commits into
nextfrom
fix/rc-review-1229

Conversation

@coderdan

@coderdan coderdan commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes for the open review threads on #1229, the release candidate that merges next into main. Most are small: a Go limit that could produce a value the SQL driver refuses, a Go error path that left a bearer token in memory, and an EQL type check that let a payload with "v": null through. One is a release blocker: merging next into main would start a crates.io publish of stack-encrypt 0.3.0 that fails, because the crates it depends on changed without a version bump.

Changes

  • Go Limit.FetchOr (languages/golang/encrypt/query.go): when no finite fetch can be promised, the caller's page size is now cut to math.MaxInt64, the largest LIMIT PostgreSQL and database/sql take. A larger page used to reach the driver as a uint64 it refuses. Rust's Limit::fetch_or already does this.
  • Go Stats.Rows doc: it suggested greatest(reltuples, 0). PostgreSQL reports reltuples = -1 for a table it has never analysed, and treating that as 0 rows sizes the fetch too small. The doc now says to read reltuples only when it is >= 0 and to page otherwise, as the Rust Stats::declared doc says. The PostgreSQL test runs ANALYZE and reads the count that way.
  • Go step decoding (languages/golang/encrypt/step.go): a request with no method or URL is refused, and it now also wipes the headers (which carry the bearer token) and the body, as every other refusal there does. New test TestARefusedRequestIsWiped.
  • EQL domains (packages/eql/crates/eql-codegen/templates/{types,query_types}.sql.j2): the CHECK compared v with the bundle version. A comparison with JSON null is NULL, and PostgreSQL lets a NULL CHECK pass, so a "v": null payload fitted both the v3 and the v4 types. They now use the COALESCE form the range types already use. The v3/v4 coexistence SQLx test casts a null-version payload to a stored and a query type of each version. The range templates now cite "Stack Encrypt ADR-0009", since packages/eql/docs/adr has no 0009.
  • Crate versions (release blocker): stack-encrypt 0.3.0 uses stack_kms::diagnostic and ErrorPayload, which the only published stack-kms (0.1.0) lacks. The same gap exists one level down: stack-kms needs things added to stack-auth after 0.43.0, and stack-auth needs things added to stack-profile after 0.43.0. This PR bumps stack-auth and stack-profile to 0.44.0 (the OIDC token cache change in stack-auth is breaking) and stack-kms to 0.2.0, makes stack-encrypt require them, adds CHANGELOG sections for all three (a new CHANGELOG for stack-kms), and renames stack-encrypt's ## [Unreleased] to ## [0.3.0]. The lockfiles that record these crates move with them; in the two fuzz lockfiles, which were already behind on other dependencies, only the version lines of these crates change.
  • Nits: a stale vitaminc version in a stack-encrypt/Cargo.toml comment is removed; a use in stack-guest-abi's tests moves to the top of the module.
  • Changesets: @cipherstash/eql patch (the null-version CHECK) and @cipherstash/auth patch (its binaries are built from stack-auth, whose version moved).

Verification

Run locally on this branch (macOS, mise toolchain):

  • Rust: cargo fmt --all --check passed; cargo clippy --locked -p stack-guest-abi -p stack-kms -p stack-encrypt --all-features --all-targets -- -D warnings passed; mise x --env test -- cargo nextest run --locked -p stack-guest-abi -p stack-kms --all-features passed (172 tests).
  • Crate publish: before the bump, cargo publish --dry-run -p stack-auth failed (stack_profile::diagnostic unresolved) and -p stack-kms failed (stack_auth::diagnostic, stack_auth::tls unresolved), building against crates.io. After it, cargo publish --dry-run -p stack-profile -p stack-auth -p stack-kms -p stack-encrypt-derive -p stack-encrypt packages and verifies all five. cargo metadata --locked passes in every workspace whose lockfile moved.
  • Go: all five guests built (wasm:guest:build, wasm:auth-guest:build, wasm:guest:build:deterministic, wasm:guest:build:eql, wasm:guest:build:eql:deterministic); mise run go:test passed; mise run go:lint reported 0 issues; go generate ./... left no diff; mise run go:test:postgres passed against a PostgreSQL 17 container, including TestQueriesRunAgainstPostgres with the ANALYZE change.
  • EQL (from packages/eql): cargo fmt --check (workspace and SQLx crate) passed; cargo test -p eql-codegen passed; mise run build passed, including the v4 isolation gate, and its regenerated src/v3/scalars SQL is committed; mise run types:check passed; mise run test:encryption:postgres passed against the same container.
  • By hand, in fresh databases with each built bundle: a "v": null payload cast to public.eql_v3_integer_eq, eql_v3.query_integer_eq, public.eql_v4_integer_eq and eql_v4.query_integer_eq fails with a check violation; a payload with the right version and a NULL value still pass.
  • Not run locally: the SQLx suite, whose fixtures need ZeroKMS credentials. The extended wrong_version_insert_fails_the_domain_check test runs in CI.

Related

Review notes

https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu

Limit.FetchOr returned any page it was given, so a page of 1<<63 or more
reached database/sql as a uint64 it refuses, and PostgreSQL's LIMIT is a
signed 64-bit value. It now cuts the page to math.MaxInt64, as Rust's
Limit::fetch_or cuts it to MAX_ROWS.

Stats.Rows suggested greatest(reltuples, 0). reltuples is -1 for a table
never vacuumed or analysed, and 0 promises a fetch of exactly want, so the
doc now says to read it with a reltuples >= 0 filter and to page when the
count is unknown, as Stats::declared says in Rust. The PostgreSQL test
analyses the table and reads it that way.

Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
decodeRequest refused such a request without wiping the bearer token in
its headers or its body, and the step's wipe never reaches a request it
did not append. It now wipes both, as every other refusal there does.

Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
The CHECK compared VALUE->>'v' with the bundle's version. With "v": null
the comparison is NULL and a NULL CHECK passes, so the payload fitted both
the eql_v3_* and the eql_v4_* domains. They now use the COALESCE form the
range domains use. The coexistence test casts a null-version payload to a
stored and a query domain of each version.

The range templates name the ADR they cite as Stack Encrypt ADR-0009:
packages/eql/docs/adr has no 0009.

Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
stack-encrypt 0.3.0 could not publish: it needs stack_kms::diagnostic and
ErrorPayload, which the only stack-kms on crates.io (0.1.0) lacks. The
same holds one level down: stack-kms needs stack_auth::diagnostic and tls,
and stack-auth needs stack_profile::diagnostic, none of which the published
0.43.0 crates have. Each crate changed after its last release without a
version bump, and the root release-plz line publishes a version only when
crates.io lacks it.

stack-auth and stack-profile move to 0.44.0 (stack-auth's OIDC token cache
change is breaking), stack-kms to 0.2.0, and stack-encrypt requires it.
Each gets a CHANGELOG section, stack-kms its first CHANGELOG, and
stack-encrypt's Unreleased section becomes 0.3.0. The lockfiles that
record these path crates move with them. @cipherstash/auth is built from
stack-auth, so it carries a patch changeset.

Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
stack-encrypt's manifest no longer names the workspace vitaminc version in
a comment, where it went stale, and stack-guest-abi's call tests import
HostValue with their other imports.

Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
mise run build renders the null-version CHECK and the ADR citation into
the committed per-family SQL under src/v3/scalars.

Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
@changeset-bot

changeset-bot Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3b2ab73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 20 packages
Name Type
@cipherstash/eql Patch
@cipherstash/auth Patch
stash Patch
@cipherstash/stack-prisma Patch
@cipherstash/stack Patch
@cipherstash/wizard Patch
@cipherstash/ffi-integration-tests Patch
@cipherstash/basic-example Patch
@cipherstash/e2e Patch
@cipherstash/prisma-example Patch
@cipherstash/bench Patch
@cipherstash/stack-drizzle Patch
@cipherstash/stack-supabase Patch
@cipherstash/test-kit Patch
@cipherstash/auth-darwin-arm64 Patch
@cipherstash/auth-darwin-x64 Patch
@cipherstash/auth-linux-arm64-gnu Patch
@cipherstash/auth-linux-x64-gnu Patch
@cipherstash/auth-linux-x64-musl Patch
@cipherstash/auth-win32-x64-msvc Patch

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

@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Mutation testing (cargo-mutants, --in-diff, stack-auth + stack-encrypt)

No mutants were generated for the changed lines.

This branch has not been deployed

No deployments
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