Skip to content

fix(stack-encrypt): check that a json_tree read returns the entry asked for - #1358

Draft
coderdan wants to merge 2 commits into
nextfrom
fix/json-tree-extracted-entry-check
Draft

coderdan wants to merge 2 commits into
nextfrom
fix/json-tree-extracted-entry-check

Conversation

@coderdan

Copy link
Copy Markdown
Contributor

Summary

EQL v4's json_tree column stores an encrypted JSON document as one entry per JSON node. When the database returns entries for a read (doc -> selector, or a subtree walk), the client decrypted each one under the selector and position the database sent back with it, and never checked that it was the entry it had asked for. Every entry of a row opens that way, so a dishonest database could answer a read of $.salary with the same row's $.bonus, or with the sealed root (the whole document), or splice other entries into a walk, and the client would hand back the wrong value. Now the client checks every entry against what it asked for and refuses anything else with a new error, Error::JsonWrongEntry.

Changes

Stack Encrypt (packages/stack-encrypt)

  • JsonSelector::open(node) -> JsonExtracted is now the only way to open the entry -> returned. It refuses an entry whose selector is not the one asked for, before any key is retrieved. The sealed root opens only when $ itself was asked for.
  • JsonSubtreeKey::open(rows) -> JsonSubtree now checks every row when it is opened. A node a gated walk reaches must carry the path's selector, at depth 0, and the position the path ends in. Every other row must carry the selector of its parent's path plus its own decrypted label. Each subtree's rows must be consecutive in the document, so a dropped, moved or orphaned row is refused. A reported depth d that disagrees is refused too.
  • JsonSubtreeKey now holds what it needs to derive those selectors: the keyset's index-key PRF (shared with the keyset handle) and the column's context. Neither is printed or serialized.
  • JsonNode no longer opens on its own: it is the raw row the database returned.
  • A new error variant, Error::JsonWrongEntry { expected }. It is not Aead (every entry is genuine) and not OpenedType (the entry is not of a wrong shape; it answers a different question).
  • JsonDocument, the stored form, is unchanged. So are the SQL and the wire format.

EQL bindings (packages/eql/crates/eql-bindings)

  • JsonTreeExtracted::from_selector(&selector, entry) is how an entry -> returned opens. JsonTreeEntry no longer opens on its own.
  • JsonTreeSubtree::from_key(&key, rows) is unchanged, and now checks the rows through the key.

Docs

  • The leakage page (packages/eql/docs/reference/json-tree-leakage.md, "What is authenticated") says what is checked now and what is not.
  • The design record's Phase 4 bullet is marked taken.
  • CHANGELOGs, plus a patch changeset for @cipherstash/eql (eql-bindings ships in lockstep with it).

Not covered, by design

  • Which row an entry comes from. Another row's $.salary, returned with that row's header, still opens. Nothing in an entry names its row, and two values sealed under the same context are interchangeable between rows (ADR 0010, packages/stack-encrypt/docs/adr/0010-what-a-stored-value-authenticates.md, and the "Known non-findings" in AGENTS.md).
  • An array position before the last step of a path. Every element of an array shares one selector, so a walk for $.a[1].b can be answered with $.a[0].b of the same row. -> already reads a position as the wildcard.
  • A walk with no gates through a chained array position ($.l[3], whose children share every other element's selectors). The walk does not return the node it starts from yet. json_tree: a subtree read of {}, [], a scalar or a missing field all return no rows, so the client cannot tell them apart #1356 (the stacked PR) fixes that, and with it this case.

Verification

Run from the worktree root unless noted. PostgreSQL 17 was a disposable container on port 55437, stopped afterwards.

  • mise x --env test -- cargo nextest run -p stack-encrypt --all-features: Summary [ 40.799s] 829 tests run: 829 passed, 1 skipped
  • cargo test -p stack-encrypt --doc --all-features: test result: ok. 50 passed; 0 failed
  • cargo clippy -p stack-encrypt --all-features --all-targets -- -D warnings: clean
  • cargo fmt --all --check at the root and in packages/eql: clean
  • RUSTDOCFLAGS=-D warnings cargo doc -p stack-encrypt --all-features --no-deps: clean
  • packages/eql: cargo test -p eql-bindings --all-features: all pass (18 passed, 16 passed; 2 ignored, …)
  • packages/eql: mise run types:check: passes
  • packages/eql: mise run test:encryption: exit 0
  • packages/eql: mise run test:encryption:postgres: exit 0. The json_tree suite: test result: ok. 7 passed. That includes the new postgres_hostile_answers_are_refused_by_the_client, which gets real rows back from PostgreSQL. It answers -> with another path's entry and with the sealed root, and a gated walk with another position's subtree. It also tampers with a stored row twice: once by rewriting a top-level field's parent tag so the walk takes it for a member, and once by swapping two entries in sv. The client refuses each wrong entry as JsonWrongEntry and each moved entry as Aead.
  • New engine tests: packages/stack-encrypt/tests/json_read_checks.rs (a new file, to stay clear of test: hold json_tree containment to jsonb with proptest, and restructure the JSON index tests #1345's rewrite of tests/json_index.rs). It has 23 tests: honest rows open, and each refusal (another path's entry, the sealed root, a relabelled entry, another position, another field of the same shape, a spliced row, a dropped row, a wrong depth, an orphan, a gap) is checked. It also checks what the design leaves open (a position read as the wildcard, another row's entry). There are unit tests in src/json/derive.rs for the selector derivation and the label check.
  • Mutation testing, mise x --env test -- cargo mutants -p stack-encrypt --in-diff <(git diff origin/next...HEAD): 88 mutants, 4 missed. All 4 were in rebuild_walked's depth counter. I merged that counter into the depth the rows are checked against and added a test at the deepest document the index seals. A re-run of the affected functions found 29 mutants: 24 caught, 5 unviable, 0 missed.

Related

Closes #1355

Refs #1356 (stacked on this PR), #1357, #1352 (the Go reader will need the same checks).

Review notes

  • Start at place and rebuild_walked in packages/stack-encrypt/src/json/derive.rs. Then read JsonSelector::open and JsonSubtreeKey::open in query.rs.
  • Conflict with test: hold json_tree containment to jsonb with proptest, and restructure the JSON index tests #1345. That PR rewrites packages/stack-encrypt/tests/json_index.rs and adds an oracle test that opens -> results through JsonTreeEntry. This PR changes a few call sites in json_index.rs (JsonNode no longer opens on its own, and the hostile line-up tests now expect JsonWrongEntry). Whichever merges second needs those lines carried over. New engine tests went into a new file to keep the overlap small.
  • There are no Go, TypeScript or protect-ffi readers of json_tree (checked with grep), so the change is Rust only. No skill file mentions json_tree, so there is no skill update.

https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu

…ed for

Every entry of a json_tree document is sealed under its own selector and
position, and nothing in its AAD says which path a query asked for. The
client opened what `->` and the subtree walks returned under the `s` and
`j` that came back with each row, so a database could answer `$.salary`
with the row's `$.bonus`, or with the sealed root (the whole document),
or splice other entries of the row into a walk, and the client would
rebuild whatever it was handed (#1355).

A row now opens only through what the client derived for the read.
`JsonSelector::open(node)` refuses an entry that does not carry the
selector, before any key is retrieved. `JsonSubtreeKey::open(rows)`
checks every row: a node a gated walk reaches carries the path's
selector and the position the path ends in; every other row carries the
selector of its parent's path extended by its decrypted label, which the
key now derives through the keyset's index-key PRF it holds; and each
subtree's rows are consecutive in the document, so a dropped, moved or
orphaned row is refused. The refusal is a new `Error::JsonWrongEntry`:
each entry is genuine, so it is not `Aead`, and it is not of a wrong
shape, so it is not `OpenedType`. `JsonNode` no longer opens on its own.

`eql-bindings` follows: an entry `->` returned opens through
`JsonTreeExtracted::from_selector(&selector, entry)`, and `JsonTreeEntry`
no longer opens on its own. Which row an entry comes from stays the
database's word, by design (ADR 0010); the leakage page and the design
record say so.

Closes #1355

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

changeset-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c3d7fee

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

This PR includes changesets to release 12 packages
Name Type
@cipherstash/eql Patch
stash Patch
@cipherstash/stack-prisma Patch
@cipherstash/basic-example Patch
@cipherstash/e2e Patch
@cipherstash/prisma-example Patch
@cipherstash/stack-drizzle Patch
@cipherstash/stack-supabase Patch
@cipherstash/stack Patch
@cipherstash/wizard Patch
@cipherstash/bench Patch
@cipherstash/test-kit 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 24, 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.

The design record marks the Phase 4 bullet on extracted entries taken;
it now names the pull request that took it, #1358.

Refs #1355

Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

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

caught missed unviable timeout
53 0 31 0

Every mutant in the changed lines was caught by a test.

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