fix(stack-encrypt): open JSON index floats to the exact value sealed by coderdan · Pull Request #1366 · cipherstash/stack

coderdan · 2026-10-25T06:50:59Z

Summary

EQL v4's json_tree is the encrypted JSON column type. The Rust crate stack-encrypt writes it as one encrypted entry per JSON node. A number in a stored document could come back slightly wrong when read: -926809.5051008083 came back as -926809.5051008085. Nothing raised an error. With this PR every finite float comes back bit for bit. Integers and short decimals were never affected.

The cause: each number is stored as text, and that text is parsed back into a number when the document is read. The JSON library, serde_json, was built without its float_roundtrip feature. Without that feature it parses floats with a fast method that can land on the neighbouring f64 (one unit in the last place off). This PR turns the feature on.

Changes

packages/stack-encrypt/Cargo.toml: serde_json now has features = ["float_roundtrip"], with a comment saying why and what it costs. The cost is slower float parsing and a little more code. Nothing new is compiled and no lockfile changes.
packages/stack-encrypt/tests/json_index.rs: new module given_a_float_at_full_precision, with two tests.
- A regression test with the exact value from the issue. It reads the value back four ways: the whole document from the sealed root, the whole document rebuilt from its entries, the extracted element as -> returns it, and a subtree read under $.list.
- A property test (proptest) that does the same four reads for arbitrary finite f64 values (proptest::num::f64::ANY filtered to finite). It reuses the existing generated-documents suite's configuration: fixed seed 1336, 512 cases, PROPTEST_RNG_SEED / PROPTEST_CASES overrides, and its regressions file. config() there became pub(super) so the new module can use it.
packages/stack-encrypt/proptest-regressions/json_index.txt: the seed proptest saved when the property failed without the feature. It shrinks to -5.7483763378806435e-244 and is now replayed on every run.
packages/stack-encrypt/CHANGELOG.md: a ### Fixed entry under Unreleased, including the trade.

No npm changeset: this only changes the Rust crate stack-encrypt. The published SQL and the eql-bindings surface do not change.

Where the stored text is parsed back, and which crates needed the feature

stack-encrypt is the only crate that parses it, in two places:
- decode_node in src/json/derive.rs parses a number entry's text with str::parse::<serde_json::Number>(). This is how a document with an open root, an extracted node and a subtree read get their numbers.
- PlainJson::parse in src/json/plain.rs (serde_json::from_slice) parses the sealed root's whole-document text. The default plan reads documents this way.
eql-bindings (EQL workspace) parses only the stored wire form: base64/base85 text and integer ordinals, never a plaintext number. Its json_tree reads go through stack-encrypt's DecryptInto. It gets the feature through Cargo feature unification whenever its stack-encrypt feature is on. cargo tree in packages/eql shows serde_json feature "float_roundtrip" coming from stack-encrypt. No manifest change was needed.
The Go guest (languages/golang/encrypt/guest) builds stack-encrypt by path, so the feature reaches it the same way (cargo tree --locked there shows it). It does not open json_tree today: the dynamic plaintext set in eql-bindings has no JSON value. No manifest change. protect-ffi does not depend on stack-encrypt.
No lockfile changed. Cargo does not record features in Cargo.lock, and the feature adds no dependency.

Does anything on the read side derive from the parsed float?

No. The value selector (canonical_number) and the ordering term (leaf_op, the order-preserving term over the number as an f64) are computed only while sealing, in walk / visit. They come from the caller's serde_json::Value, which is parsed once by the caller. The read paths (decode_node, rebuild, open_document, open_subtree, open_fallback) only decrypt and decode. Subtree keys come from the path alone. So the parsing defect only affected the value handed back, not any stored or queried term.

One side effect is worth knowing. Cargo unifies features, so any program that links stack-encrypt now parses every float with the exact parser, including the caller's own JSON text. That only matters for a document whose text was parsed by an older build before it was sealed. The JSON index has not been released yet (it is under Unreleased in the changelog), so no such stored data exists.

Verification

All commands were run from the worktree root unless noted.

The new tests fail without the feature (run before the Cargo.toml change):

every read should give back -926809.5051008083: [
    "the document, from the sealed root read back -926809.5051008085",
    "the document, from its entries read back -926809.5051008085",
    "the extracted element, as `->` returns it read back -926809.5051008085",
    "the walk under $.list read back -926809.5051008085",
]

json_index full-precision floats: seed 1336, 512 cases; replay with PROPTEST_RNG_SEED=1336 PROPTEST_CASES=512
Test failed: assertion failed: `(left == right)`
  left: `Some(10191477112857735508)`,
 right: `Some(10191477112857735509)`: the document, from the sealed root should read back -5.7483763378806435e-244, read back -5.748376337880643e-244
minimal failing input: -5.7483763378806435e-244

With the feature both pass (2 tests run: 2 passed).

mise x --env test -- cargo nextest run -p stack-encrypt --all-features: 824 tests run: 824 passed (1 slow), 1 skipped. The skipped test is the #[ignore]d write_stored_rows_v1 fixture writer; the slow one is the trybuild ui suite.
cargo test -p stack-encrypt --doc --all-features: 50 passed; 0 failed.
cargo clippy -p stack-encrypt --all-features --all-targets -- -D warnings: exit 0.
cargo fmt --all --check: exit 0.
RUSTDOCFLAGS="-D warnings" cargo doc -p stack-encrypt --all-features --no-deps: exit 0.
cargo tree -p stack-encrypt -e features -i serde_json: shows serde_json feature "float_roundtrip" from stack-encrypt v0.3.0.
From packages/eql: cargo test --locked -p eql-bindings --all-features: 226 passed, 0 failed. cargo metadata --locked: exit 0.
cargo metadata --locked at the root and in both Go guest workspaces: exit 0. No guest manifest changed, so mise run wasm:guest:build was not run.
pnpm run code:fix was not run: nothing here is JavaScript or TypeScript.

Refs #1363: its property test leaf() strategy in packages/stack-encrypt/tests/json_properties.rs draws only quarters (f64::from(n) / 4.0) so it would pass without this fix. Once this PR is in, widen that arm back to arbitrary finite f64 (proptest::num::f64::ANY filtered to finite) and drop the comment that explains the narrowing.

https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu

The JSON index stores a number leaf as its shortest round-trip text (serde_json::Number::to_string in encode_node) and parses it back when the entry opens (decode_node) or the sealed root opens (PlainJson::parse). serde_json without float_roundtrip parses floats with a fast path that is not guaranteed to return the nearest f64, so a value with a full-precision expansion could open one unit in the last place off: -926809.5051008083 opened as -926809.5051008085, through JsonDocument, JsonNode and JsonSubtree alike. Enable float_roundtrip on stack-encrypt's serde_json dependency. Cargo unifies features, so it reaches every build graph that compiles this crate (the EQL workspace through eql-bindings' stack-encrypt feature, and the Go guests) without a change to their manifests or lockfiles. The read side derives no term from the parsed value: value selectors and ordering terms are computed on the write side only, from the caller's value. A regression test reads the issue's value back four ways, and a property test does the same for arbitrary finite f64 under the generated-documents suite's seed and case count. Both fail without the feature; the shrunk seed is kept in the regressions file. Closes #1364 Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu

changeset-bot · 2026-10-25T06:51:03Z

⚠️ No Changeset found

Latest commit: ff287a3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

coderabbitai · 2026-10-25T06:51:08Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI; Enforced overrides: Organization
Review profile: CHILL
Plan: Advanced
Run ID: 70612f6a-2021-483b-a2ba-fb0aa8fdda0c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

github-actions · 2026-10-25T06:52:19Z

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

No mutants were generated for the changed lines.

coderdan

Human review

chatgpt-codex-connector · 2026-10-25T10:50:55Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-25T10:51:42.483418Z	`ff287a3`	Draft marked ready
🔒 Security Review	✅ Completed 2026-10-25T10:53:31.483071Z	`ff287a3`	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

cipherstash-agent

Review attestation: no critical or blocking issues found.

Source: automated /code-review medium, with each finding then verified against head ff287a3 and cross-checked against the existing PR feedback (company-skills:cross-check-review-findings). I read the diff and the source it touches. I did not build the crate or run the tests.

Five candidate findings came out of the review. None survived verification:

Regressions file not loaded (refuted). config() sets failure_persistence = Direct(REGRESSIONS) (json_index.rs:1787), which applies to any TestRunner built from it, as the existing generated-documents test already does. The cc seed was saved by this test's own strategy.
Use-after-move in read_back (refuted). JsonDocument::node returns an owned Option<JsonNode> (src/json/mod.rs:494), and subtree_nodes collects owned nodes, so nothing borrows sealed when it moves into open.
Test cost, feature unification and changelog wording (dropped). These are design or style observations, not defects. The PR description already documents the trade-offs, and the case count matches the existing suite.

Existing feedback was bot status comments plus a bare "Human review" comment, with no inline threads, so nothing overlaps.

Non-blocking note: the changeset-bot warning is expected here, since the PR changes only a Rust crate and the description says why no npm changeset is needed.

Generated by Claude Code

coderdan mentioned this pull request Oct 25, 2026

json_tree: a stored float can come back one ulp off because numbers are re-parsed without serde_json float_roundtrip #1364

Closed

coderdan commented Oct 25, 2026

View reviewed changes

coderdan marked this pull request as ready for review October 25, 2026 10:50

coderdan requested a review from a team as a code owner October 25, 2026 10:50

cipherstash-agent reviewed Oct 25, 2026

View reviewed changes

coderdan merged commit ebdb5de into next Oct 25, 2026
55 checks passed

coderdan deleted the fix/json-float-roundtrip branch October 25, 2026 10:57

Navigation Menu

Repository navigation

fix(stack-encrypt): open JSON index floats to the exact value sealed - #1366

fix(stack-encrypt): open JSON index floats to the exact value sealed#1366
coderdan merged 1 commit into
nextfrom
fix/json-float-roundtrip

coderdan commented Oct 25, 2026

Uh oh!

changeset-bot Bot commented Oct 25, 2026

Uh oh!

coderabbitai Bot commented Oct 25, 2026 •

edited

Loading

Review skipped

Uh oh!

github-actions Bot commented Oct 25, 2026

Uh oh!

coderdan left a comment

Uh oh!

chatgpt-codex-connector Bot commented Oct 25, 2026 •

edited

Loading

Uh oh!

cipherstash-agent left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

coderdan commented Oct 25, 2026

Summary

Changes

Where the stored text is parsed back, and which crates needed the feature

Does anything on the read side derive from the parsed float?

Verification

Related

Uh oh!

changeset-bot Bot commented Oct 25, 2026

⚠️ No Changeset found

Uh oh!

coderabbitai Bot commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions Bot commented Oct 25, 2026

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

Uh oh!

coderdan left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

cipherstash-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Oct 25, 2026 •

edited

Loading

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

chatgpt-codex-connector Bot commented Oct 25, 2026 •

edited

Loading