Repository navigation
feat(go): generate encrypted JSON document fields - #1374
Conversation
A json.RawMessage field tagged encrypt_into=EncryptedJson, with optional ordered paths and sealed_root, is sealed by the engine from the field's data plan and gets generated methods for every json_tree query and read: Contains, ContainedBy, EqAt, Selector, Subtree, Children, ChildCount, and an At accessor per ordered path with Lt, Lte, Gt, Gte and Between. Selector and subtree reads return handles (encrypt.JSONSelection, encrypt.JSONRead) that keep the request they were prepared from and open the rows the database returns only as answers to it. TestJSONDocumentsQueryAndReadInPostgres runs every method against the EQL v4 bundle from the tree and joins go:test:postgres. Refs #1347, #1351, #1352 Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait...
 |
Mutation testing (cargo-mutants,
|
|
Automated code review of this pull request at
|
- A json.RawMessage is a document only under encrypt_into; sealed or indexed any other way it stays bytes, so existing models generate as before. - An ordered path's accessor has its own Selector, so an ordering and the value it compares come from one path; the README says why. - JSONRead keeps the subtree input it was prepared from and sends it back when it opens; OpenOne refuses a wildcard path before opening anything. - JSONSelection.Open makes no request for a page with no entries and refuses more than one value for a row; a zero JSONSelector or JSONToken is refused rather than bound as ''. - gensupport's JSON helpers check the term's encoding, fidelity (record.Exact) and part count in one place. - stashgen: imports are re-checked after planning adds encoding/json; a document field needs the engine to answer selector on it; an ordered path given twice, or with no name, is refused by name; a key of digits names its accessor; index=json points at encrypt_into=EncryptedJson. - Tests check rows.Err() and no longer assert a driver value's type. Refs #1347 Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
|
Review response to the findings listed above (fixed in 3ae7166 unless noted):
Verified on this branch: |
Brings in, by way of the base, the v4 JSON document's v3 names (`JsonSearch` over `public.eql_v4_json_search`, `eql_v4.query_json`, `eql_v4.query_json_leaf`, `public.eql_v4_json_entry`), the SteVec SQL confined to EQL v3, and the base's JSON operators renamed to match. The one conflict, in cmd/stashgen/main_test.go, keeps this branch's refusal text; the rename of this branch's own code follows in the next commit. Claude-Session: https://claude.ai/code/session_01HLuZxys1NDRmAkv3uhnuoJ
The PostgreSQL test compared the ordered paths $.age and $.visits[*].at only through <, <=, >= and >, so nothing showed that rows sort by them. Ordering by the entry `->` returns sorts by its ciphertext, which the blockers cannot stop, so the form EQL documents is ORDER BY eql_v4.ord_term(col -> sel), and that form had no Go test. The test now inserts two more rows, giving each path a tie broken by id and a document whose first visit (2) is neither its least nor its greatest element, then sorts by both paths ascending and descending and compares the ids with the order computed from the plaintexts. A row with no value at the path (no age, or no visits) has a NULL term and sorts last ascending and first descending, as PostgreSQL sorts NULL by default. At the wildcard path the term is the first element's, which is what `->` returns. Sorting by the entry itself instead fails all four orderings. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
|
c502848 adds the What it showed: all four orderings match the plaintext order. A row with no value at the path (Dan has no age; Carol's visits are empty and Dan has none) gets a NULL term, so it sorts last ascending and first descending, which is PostgreSQL's default for NULL. At the wildcard path the sort uses the first element's value, which is the value Run locally: |
With the base's rename the EQL type a document field names is `JsonSearch` (Go `eql.JSONSearch`, over `public.eql_v4_json_search`). The tag is now `encrypt_into=JsonSearch(ordered=$.a;$.b[*].c,sealed_root=false)`; the Go spelling `JSONSearch` in a tag is refused with the same hint as before, naming the engine's spelling. The generator named a document's companion types by appending to the stored type's Go name, `<Stored>LeafQuery` and `<Stored>Entry`. Under the v3 names the entry is not `JSONSearchEntry` but `JSONEntry`, named for the plaintext as `public.eql_v4_json_entry` is, so the leaf operand is now derived from the needle (`JSONSearchQuery` to `JSONSearchLeafQuery`) and the entry is `eql.JSONEntry`. The test model, its regenerated customer_stash.go, the READMEs, the CHANGELOG and the PostgreSQL test's SQL (`eql_v4.query_json`, `eql_v4.query_json_leaf`) follow. Refs #1351 Claude-Session: https://claude.ai/code/session_01HLuZxys1NDRmAkv3uhnuoJ
Brings in the base's realigned JSON tree domain list and the corrected json index note in the target descriptor's docs. Claude-Session: https://claude.ai/code/session_01HLuZxys1NDRmAkv3uhnuoJ
Takes in the ordered-path sort test pushed while the v3-names rename was being merged here. Claude-Session: https://claude.ai/code/session_01HLuZxys1NDRmAkv3uhnuoJ
2b5cb48 to
e91856e
Compare
92be79b to
b418feb
Compare
|
History rewritten on this branch, authorship only. Twelve commits pushed here this morning by a cloud session were unsigned and authored |
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. |
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation — automated code review (/code-review medium) cross-checked against existing PR feedback (cross-check-review-findings), at b418feb.
Critical / blocking: none.
New findings: none. Read encrypt/json.go, encrypt/gensupport/json.go, stashgen/tag.go, stashgen/read.go and the planJSON / jsonPathName / jsonEntry paths in stashgen/emit.go. The defects I found were already raised and fixed in 3ae7166, per the earlier review and its response comment on this PR. Nothing in them is still present at head.
Not covered by this review:
- The generated-code snapshot (
customer_stash.go), the test files, and the READMEs were not read line by line. - Several CI jobs were still running at review time, including
WASI check, guests and Go (Linux), so the Go test suite and the Postgres test had no result.Go lintandci-requiredpassed. - The Codex code and security reviews were also still running.
Non-blocking: changeset-bot reports no changeset. That looks fine for a Go-module-only change recorded in languages/golang/CHANGELOG.md.
Generated by Claude Code
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b418feb519
ℹ️ 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
freshtonic
left a comment
There was a problem hiding this comment.
I found no blocking problem. One gap goes against fail-closed behaviour: an empty JSONTokens binds as {} instead of being refused (see the comment on json.go:135).
Summary. This PR adds json.RawMessage / *json.RawMessage fields with encrypt_into=JsonSearch(...) to stashgen. For each field, it generates containment, EqAt, the orderings under At, the selector and subtree reads and the node-token methods. The design is good. The tag words (ordered, sealed_root) are the engine's own. The engine checks the options and gives the refusal messages, and At accessors come only from options the engine accepted. null=sealed is refused, and a nil *json.RawMessage is SQL NULL, while JSON null is a document. The generated Encrypted* types redact their sealed fields, and the negative tests show that errors do not contain plaintext. The CHANGELOG and READMEs match the generated surface and the SQL names, except for the sentence in the json.go:135 comment.
What was run:
- All five guests were built, and
mise run go:testpassed. - The JSON field tests ran without skipping, and all 11 refusal subtests passed.
go generate ./...left the tree unchanged, andmise run go:lintreported 0 issues.mise run go:test:postgrespassed against Dockerpostgres:17with the tree's EQL v4 bundle, includingTestJSONDocumentsQueryAndReadInPostgres.- A probe test (not committed) against the embedded engine found no problems in opaque structs or passthrough fields, and stashgen refused the wrong declarations it tried.
Nit (not on a single line): the README does not say that a defined type over json.RawMessage is refused, although defined types are accepted for other kinds.
Brings in the vitaminc d7de535c4 pin for vcffi/vcvalue and the go-release guard that refuses a final version while go.mod names a pseudo-version, with the opaque storage profile's two fixes beneath them. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head, so the gate no longer diffs all of next and times out on this stack. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
JSONRead.OpenOne refused a wildcard read by looking for "[*]" in the path text. That copies a rule of the engine's path parser into Go, and gets it wrong for a quoted key: $["a[*]"] is one member key, and OpenOne refused it. The engine already answers the question. It derives exactly one token for a path that names one node, and for a wildcard one token per element below the bound plus a probe at the bound, which is two or more at any bound of 1 or more. Generated code never sends a bound of 0. OpenOne now refuses a read with more or fewer than one token. A read prepared by hand with a bound of 0 passes the check, but it reaches no value: a row with an element there is refused as truncated, and any other opens to none. The refusal no longer prints the path, which can hold a customer's key. The parameter is named entries, for what it holds: one database row's json_tree_subtree entries. JSONRead no longer keeps the path, which nothing reads now, so NewJSONRead stops taking it. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
A zero JSONSelector or JSONToken is refused with ErrEncoding when bound,
because the empty value matches no row without an error. JSONTokens did
not follow the rule: a nil or empty list bound as the text[] literal {},
so a zero JSONRead's tokens read nothing from every row and said nothing.
Every read the engine prepares has at least one token, so an empty list
is always a read that was never prepared. It is now refused the same way,
and the changelog's sentence covers the token list too.
Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
JSONToken and JSONTokens are documented as capabilities: a token opens the structure of its node's subtree in every row of the column. They still printed in the clear with %v and through slog. JSONSelection and JSONRead printed their selector, tokens and path with %+v, and a path's keys can be a customer's data (Go principle 12). Each now has String, GoString, Format and LogValue, as the query types in query.go do, so every fmt verb and slog record shows the type, the field and a token count, with [redacted] for the tokens, the selector and the path. The values still bind through driver.Valuer, which pgx and database/sql consult before a Stringer. JSONValue's doc now says it holds plaintext JSON that prints in the clear, as the generated plaintext types' docs do. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
NewJSONSelection and NewJSONRead are exported, so their parameter names are part of what godoc shows. The parameter was named plan, a word for the engine's internal form that a user never sees (Go principle 7). It is now declaration, for what it is: the declaration of the record type the field belongs to, and the docs say so. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
stashgen wrote a document field's At accessors only when the engine answered lt_at on it. A tag that names ordered paths on a field the engine answers no ordering on means the generator and the engine it embeds disagree, and the generator then wrote the field with no At and said nothing: the paths the user asked to order had no methods, and the build still passed. planJSON now refuses that field by name, so the drift fails generation instead of producing a quieter API. jsonEntry writes At whenever the field has ordered paths, since planJSON has already checked the engine answers on them. The test runs the embedded engine with lt_at removed from its answer, which is the one way to reach the refusal: the real engine always answers the orderings on an ordered field. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
The "a sealed null" case asserted only that stderr named Customer.Prefs, so any refusal of that field passed it, including one for the wrong reason. It now asserts the field and the reason together: a document field carries an index other than equality, a nil has only an equality term, so a sealed nil would show which documents are absent, and the field must say null=sql. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
stashgen accepts a scalar field declared with any type defined over the scalar (type Email string), so a reader would expect type Doc json.RawMessage to work for JsonSearch too. It does not: stashgen knows a document by the json.RawMessage type itself, so a defined type is bytes, and JsonSearch refuses it with "declare the field json.RawMessage". An alias (type Doc = json.RawMessage) is the type itself and is accepted. Both READMEs now say so, and the stashgen test pins both: the alias generates an eql.JSONSearch field, and the defined type is refused by name with that reason. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
The encrypt README's JSON section said how the selector, token and token list bind, but not that a zero one is refused, nor that the read types print and log redacted while JSONValue prints the plaintext it holds. A reader deciding what is safe to log needs both, and the README is where they look before the godoc. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in next's JSON tree cascade and random storage order (#1379) through the base, with the base's review fixes. The per-type PostgreSQL test's comment takes the base's wording, which names this branch's TestJSONDocumentsQueryAndReadInPostgres. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
A JSON document's entries are now stored in a random order (#1379), so `col -> sel` at a wildcard path, and an ordering through it, sees an unspecified element of each row rather than the first. The At accessors stay: their operands are what a comparison over every match takes, and the engine stores ordering terms at a wildcard path when asked to, so refusing ordered= there would leave those terms unreachable from Go. What changes is the SQL they are documented and tested with. - The generated XPath comment gives both forms: col -> Selector() at a path with no array step, and at a wildcard path EXISTS over eql_v4.json_tree_path_query for a comparison and the least or greatest match's ordering term for ORDER BY. Lt, Lte, Gt, Gte and Between get doc comments of their own, which they lacked, and Selector says which form it binds into. The READMEs and the CHANGELOG say the same. - TestJSONDocumentsQueryAndReadInPostgres compares at $.visits[*].at with EXISTS (some visit > 2, and some visit < 2, which only Alice's second visit meets), orders rows by their least visit ascending and greatest descending against the plaintexts' order, and reads a wildcard past its bound from PostgreSQL's own rows, which Open refuses as ErrEncoding. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
|
Review-body finding, at 955b9a1: freshtonic's nit that the README did not say a type defined over This branch now carries #1379, under which |
Brings in the base's test that pins the JSON path depth cap at a document's depth. No conflicts. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Carries the vitaminc Go bindings pin to 06568ba93 on vitaminc main. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
2851026
into
feat/json-tree-queries-reads
|
This was merged into |
feat(stack-encrypt, golang): JSON operators and data-plan reads, with generated Go JSON document fields (#1374)
Brings in the rest of the Go json_tree stack (#1373, #1374, #1378). The only conflicts were adjacent CHANGELOG entries in the Go module and stack-encrypt, resolved by keeping both. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Summary
This pull request gives Go applications encrypted JSON documents (
JsonSearch, EQL'sjson_treetype; EQL is the Encrypt Query Language, the SQL that stores and queries encrypted values) throughstashgen, the Go SDK's code generator. Ajson.RawMessagefield taggedencrypt_into=JsonSearchis sealed by Stack Encrypt (the Rust engine, run in Go as a WebAssembly guest) from the field's data plan (the declaration the generator emits and the engine resolves), and gets generated methods for every query and read the previous pull request added. It is slices D and E of the plan in #1353.Changes
Runtime (
languages/golang/encrypt)json.go:JSONSelector,JSONToken,JSONTokens(atext[]value),JSONValue(Found,JSON),JSONReadOptions(WildcardBound),JSONRange, and the two read handles:JSONSelection(a->read;Selector()to bind,Opento open each row's entry) andJSONRead(a subtree read;Tokens()to bind,Open/OpenOnefor each row's entries). A handle holds the request that prepared it; the engine re-derives from that request, never from the rows.gensupport: thejsonkind, a declaration'sTargetOptions, and the helpers generated code calls (JSONQuery,JSONQueryAt,JSONBetweenAt,JSONSelect,JSONSubtree,JSONNodeToken).term.go:JSON()/JSONTerm(the old SteVec placeholder, never producible) markedDeprecated.Generator (
languages/golang/stashgen)encrypt_into=JsonSearch(ordered=$.a;$.b[*].c,sealed_root=false).;separates ordered paths because,separates tag options. A path in a tag is a plain JSONPath (dotted keys,[*]); a quoted key is refused. The generator hands the options to the engine, which validates them as it resolves the plan, so a declaration the engine refuses (a position in an ordered path, options on a type that takes none) fails at generation.json.RawMessage/*json.RawMessagemap to thejsonkind. A pointer must saynull=sql(a nil document is SQLNULL;null=sealedis refused); JSONnullis a document. Other Go types forJsonSearch, and the Go spellingJSONSearchin a tag, are refused with a hint.Encrypt,Contains,ContainedBy,EqAt,Selector,Subtree,Children,ChildCount, and, for a field that orders paths, anAtstruct with one accessor per ordered path (Lt,Lte,Gt,Gte,Between, and the path's ownSelector). At a wildcard path the generated comments give the comparison and the ordering over every match, since->there returns an unspecified element of the row. Ajson.RawMessagesealed any other way thanencrypt_intostays bytes. The accessors are generated from the options the engine accepted, so an unordered path has none.Tests
encrypt/internal/testusers/customers.go(+ generatedcustomer_stash.go): a model with an ordered document and an optional one.encrypt/json_fields_test.go: round trip (includingnull=sqland a JSONnull), and every generated method derives the operand the data plan does.encrypt/json_postgres_test.go(TestJSONDocumentsQueryAndReadInPostgres): rows stored by generated code in PostgreSQL with the EQL v4 bundle from the tree; stored values read back; containment both ways (including an array element),eq_atat a wildcard path, containment on thenull=sqlcolumn;ltandbetweenat$.age, andgtandltat the wildcard path$.visits[*].atover every match (EXISTSovereql_v4.json_tree_path_query);->selector reads opened per row (absence included); subtree reads of a wildcard and of an array, and a wildcard read past its bound refused; and children / child count by node token. Added tomise run go:test:postgres, which runs each test by name.cmd/stashgen/main_test.go(TestRunGeneratesJSONSearchFields): the generated surface, and nine refusals (sealed null, pointer withoutnull=sql, a position in an ordered path, a quoted key, an unknown option, the Go spelling of the type, two paths naming one accessor, a string field, options on a scalar type).Docs:
cmd/stashgen/README.mdandencrypt/README.mdgain an "Encrypted JSON documents" section; Go CHANGELOG.Assumptions
The five owner decisions in #1347 are open; this stack takes the plan's recommended answer to each:
json.RawMessageand*json.RawMessage. A struct-typed document is not supported yet.json.RawMessage).Atstruct named after each path ($.visits[*].at→VisitsAt).eq_at, orderings, selector and subtree reads, children tokens).Also assumed:
ordered=with;), andsealed_root=falseas the only other option.Children/ChildCountreturn the node token to bind; the child rows are read in SQL (the opener for child rows is not in this stack).Not done here
json_tree_childrenrows.ORDER BY eql_v4.ord_term(profile -> $1::text)at$.age, and at the wildcard path$.visits[*].atby each row's least (ascending) or greatest (descending) match overeql_v4.json_tree_path_query, since->there returns an unspecified element of the row.Verification
After merging #1373 (which carries #1379) and the review fixes, at 955b9a1 (e6ae4a8 then merged #1373's one-test follow-up), on macOS arm64, Rust 1.94.1 (the root
mise.tomlpin), Go 1.26.10, PostgreSQL 17.10 (a fresh database per run). Nothing outsidelanguages/golangdiffers from #1373's head butmise.toml(this branch'sgo:test:postgres), so #1373's Rust, EQL and PostgreSQL oracle results stand for it.wasm:guest:build,:eql, both deterministic builds,wasm:auth-guest:build),mise run wasm:guest:test,mise run go:test(every packageok),mise run go:lint(0 issues),gofmt -l(clean) andgo generate ./...(no drift).mise run go:test:postgreswith the EQL v4 bundle built from the tree: all four tests passed by name, includingTestJSONDocumentsQueryAndReadInPostgreswith the wildcard comparisons written over every match (EXISTSoverjson_tree_path_query), the wildcardORDER BYby each row's least and greatest match against the plaintexts' order, and a wildcard read past its bound refused asErrEncoding.Related
Stack
Merge order is bottom to top; each pull request is based on the one below it.
d7de535c4(the docs: refresh root README around the three Stack pillars #526 merge) by git reference (temporary)JsonSearchstored through a data plan (engine, EQL catalog, guest, Go runtime)https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Summary by CodeRabbit
NULL.JsonSearch.