Repository navigation
Conversation
|
Contributor
|
Important Review skippedToo many files! This PR contains 213 files, which is 113 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. Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing. ⚙️ Run configuration
⛔ Files ignored due to path filters (6)
📒 Files selected for processing (213)
You can disable this status message by setting the
Comment |
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. |
Contributor
Mutation testing (cargo-mutants,
|
![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: 1670ebdfd6
ℹ️ 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".
This comment has been minimized.
This comment has been minimized.
coderdan
added a commit
that referenced
this pull request
Oct 6, 2026
Two Codex findings on PR #1094, both real, both the same shape: a value the generator accepted and Encrypt sealed could never be decrypted. Get sent every opened value through convert, whose switch knows the wire kinds and a few slice types. A passthrough field is the Go value the struct held, kept on the host, so a time.Time or a sql.NullTime — the plan's own accounts example — reached convert and was refused. Get now returns a value that already is a T before it converts anything. An opaque struct came back from JSON into Values and each field went through Get, so a map[string]string (JSON gives map[string]any), a type defined over a scalar, a nested struct or a pointer could not be read, although classify and readableInOpaque had accepted them. The opaque value now crosses as a JSON document of a generated shape struct (documentOpaque, with json tags for the declared names), and the generated Value decodes it straight into that struct with gensupport. Opaque, so every type encoding/json carries both ways comes back as it was. The generator's rule is that: a field of an opaque struct may be a scalar or a type defined over one, []byte, a slice or array, a map with string or integer keys, a pointer, a struct whose fields are all exported, or a type with its own MarshalJSON/UnmarshalJSON (time.Time); a channel, an interface, a map keyed by a struct, or a struct JSON would truncate is refused at generate time with the field named. Outside an opaque struct the engine seals a composite as a tree of leaves and a column holds one, so stashgen now refuses encrypt or index= on a struct, slice or map field ("seals only as part of an opaque struct") instead of letting Encrypt fail at run time. A type defined over a scalar (type Email string) is accepted: the engine returns the underlying type, and the generated Value reads it at that type and converts, since Go has no way to do that generically without reflection. The proof is in encrypt/internal/testusers: Account (time.Time and sql.NullTime passthrough beside an indexed field), Kinds (one sealed field of every scalar kind, defined types among them, and passthrough fields of every type the codec cannot carry) and Everything (an opaque struct with a field of every type JSON carries), each generated by the real stashgen against the real guest and walked through Encrypt then Decrypt over the deterministic build with reflect.DeepEqual. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
coderdan
force-pushed
the
feat/go-stashgen
branch
from
October 6, 2026 09:25
1670ebd to
8b09837
Compare
Contributor
Author
|
The one failure in the live-credentials job, |
coderdan
requested review from
cipherstash-bot
and removed request for
cipherstash-bot
October 6, 2026 09:25
cipherstash-bot
left a comment
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🔴 fix before merging
Fix the problems below and change two tests before merging. The other items can wait for a follow-up.
Fix these problems before merging:
Getcannot read back some field types thatstashgenaccepts.- A sealed slice or map field fails every
Encrypt. - The live test
TestPlaintextDoesNotRemainInGuestMemoryAfterEncryptfails before it reaches its memory check.
Change these tests before merging:
TestRefusalsruns only against a fake engine. That fake engine disagrees with the shipped engine.- The Rust fixture test skips on every error.
Not kept: a finding that the opaque path has no test for JSON numbers or base64 bytes. TestConvertStaysWithinAFamily in encrypt/gensupport/gensupport_internal_test.go (lines 141 to 155) already tests json.Number, base64 into []byte, and opaqueValues.
Other findings not posted as comments
- Optional:
GoNameinlanguages/golang/encrypt/policy/protosource/protosource.go:89does not matchGoCamelCasein protoc-gen-go. Forfoo_1bar,sha256sumand_x, protoc-gen-go givesFoo_1Bar,Sha256SumandXX.GoNamegivesFoo_1bar,Sha256sumandX.TestGoNameexpects the wrong value forfoo_1bar.stashgenusesGoNameonly when a struct field has no protobufname=tag. So a difference causes a wrong refusal, not a wrong field. - Optional:
DeterministicSourceinlanguages/golang/encrypt/guest/src/deterministic.rsis a second copy of the type inpackages/stack-encrypt/tests/common/mod.rs. The two copies must derive the same bytes. If they do not, the Go fixture test cannot open the Rust records. Move the type intostack-kmsunder thetest-supportfeature, next toFakeDataKeySource. Then import it in both places. - Optional: the enum branch of
scalar(languages/golang/encrypt/policy/protosource/protosource.go:77) has no test, because thetestpbmessages declare no enum. - Optional: no test makes the whole-declaration check in
guestEngine.Check(languages/golang/stashgen/engine.go:106) fail. So no test covers itsFieldErrorwith no field name. - Optional: the crate doc in
packages/stack-guest-abi/src/lib.rs:34names the pathbindings/go/encrypt/guest. That path does not exist. The real path islanguages/golang/encrypt/guest. - For information: the match index refuses an empty string, or text shorter than 3 characters. The whole batch then fails with
ErrTerm. So a program with an optional or short value in amatchfield cannot encrypt that batch.encrypt/README.md,cmd/stashgen/README.mdandencrypt/doc.godo not say this.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 8 found, 7 posted |
| claude | claude-opus-5-5 | rust | 3 found, 2 posted |
| codex | gpt-5.6-terra | test-gap | 2 found, 1 posted |
| codex | gpt-5.6-terra | rust | 0 found, 0 posted |
Synthesis: claude-opus-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 1 posted finding(s) were raised by two or more models.
Plain language: claude-opus-5-5 read every comment as a new reader would. 5 comment(s) had a problem that stopped the reader acting; it rewrote 5. It also rewrote the review body.
Stack: 2 earlier and 0 later pull request(s) (#1090, #1093, *️⃣ #1094). *️⃣ marks this pull request.
Context loaded: the description, 1 linked issue(s) and 3 discussion entries.
Contributor
Author
|
The review body's other findings are addressed in d0d5799: One finding of our own, raised while adding the residency checks: after |
This was referenced Oct 6, 2026
coderdan
added a commit
that referenced
this pull request
Oct 6, 2026
…ts type Closes #1082 (CIP-4326). A data plan field with any term output (eq, match, ore, ope) and no "type" is now refused when the plan is built, with Error::Plan, by record::plan, plan_with and Plan::new / new_with alike: an indexed field's terms derive from the one declared kind, and every value is checked against it on the way in and on the way out, so the engine never trusts a binding to have tagged each value the same way. Before this a 34 sent once as a Float64 and once as an Int64 under one "ore" field was accepted both times and stored two terms. That gap was kept open only until the Go side filled the type; the Go SDK (#1094) fills it on every field from the Go kind, so nothing a binding sends today is refused. The type stays optional where no term derives from the field: a field whose only output is "c" or "passthrough", and a target field, whose kind is the EQL type's own (#1095). Declaring a type changes no stored byte (#1093), so no row is re-encrypted. The IndexSpec: Index<Value> dispatch is unchanged: the value still carries its tag, now always checked against a declaration before the term is derived. The "transitional" wording goes from the dynamic module, kind and record docs and from the plan document. Tests: refused at build for every index kind, alone and beside a ciphertext, spelled as a key and as the match object form, by the parser and by hand; a sealed-only and a passthrough field still build, seal and open without a type. Every test that indexed an untyped field now declares the type; the one that fed a float to an untyped equality field now shows there is no such field to feed, since float64 is refused at build and an integer kind refuses the float value. The guest's native tests and plan_check cover the refusal; the check_record fuzz generator emits a "type" entry so indexed plans still parse under fuzzing. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
coderdan
added this pull request to stack #1097
October 6, 2026 15:24
coderdan
added a commit
that referenced
this pull request
Oct 6, 2026
…ts type Closes #1082 (CIP-4326). A data plan field with any term output (eq, match, ore, ope) and no "type" is now refused when the plan is built, with Error::Plan, by record::plan, plan_with and Plan::new / new_with alike: an indexed field's terms derive from the one declared kind, and every value is checked against it on the way in and on the way out, so the engine never trusts a binding to have tagged each value the same way. Before this a 34 sent once as a Float64 and once as an Int64 under one "ore" field was accepted both times and stored two terms. That gap was kept open only until the Go side filled the type; the Go SDK (#1094) fills it on every field from the Go kind, so nothing a binding sends today is refused. The type stays optional where no term derives from the field: a field whose only output is "c" or "passthrough", and a target field, whose kind is the EQL type's own (#1095). Declaring a type changes no stored byte (#1093), so no row is re-encrypted. The IndexSpec: Index<Value> dispatch is unchanged: the value still carries its tag, now always checked against a declaration before the term is derived. The "transitional" wording goes from the dynamic module, kind and record docs and from the plan document. Tests: refused at build for every index kind, alone and beside a ciphertext, spelled as a key and as the match object form, by the parser and by hand; a sealed-only and a passthrough field still build, seal and open without a type. Every test that indexed an untyped field now declares the type; the one that fed a float to an untyped equality field now shows there is no such field to feed, since float64 is refused at build and an integer kind refuses the float value. The guest's native tests and plan_check cover the refusal; the check_record fuzz generator emits a "type" entry so indexed plans still parse under fuzzing. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
coderdan
force-pushed
the
feat/go-stashgen
branch
from
October 6, 2026 23:20
d0d5799 to
1c46992
Compare
cipherstash-bot
left a comment
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🔴 fix before merging
Fix one problem and change one more before merging. The generated Decrypt cannot read any row of a type that has an index-only field (index= without encrypt). Encrypt fails when an interface-typed passthrough field holds nil. The other items are follow-ups or optional, and they can wait.
None of these findings repeat the earlier review on this pull request or the author's replies to it.
Other findings not posted as comments
- Optional: the
_commentinpackages/stack-encrypt/tests/fixtures/label_segments.json:2nameslanguages/golang/encrypt/label_test.go. That file does not exist. The Go test that reads the fixture islanguages/golang/internal/record/fixture_test.go. The comment is still wrong at the end of the stack (#1110). - Optional:
stack-encryptstill declares thesha2dev-dependency, but no code in the crate uses it now thatDeterministicSourceis instack-kms. Its comment inpackages/stack-encrypt/Cargo.toml(lines 67 to 71) still says that the key source is intests/common.cargo udepsdoes not report it, becausestack-kmslinkssha2. Delete the dependency and its comment. - Optional: the new public
DeterministicSourceinpackages/stack-kms/src/key_source.rs:311does not implementDebug, butFakeDataKeySourcedoes. Addopaque_debug::implement!(DeterministicSource);, assrc/key.rsdoes forDataKey, so that the seed is not printed. Also update thetest-supportcomment inpackages/stack-kms/Cargo.toml, which names onlyFakeDataKeySource. - Optional: a nil
[]byteorBlobin a sealed field comes back fromDecryptas a non-nil empty slice. In an opaque struct, nil comes back as nil. No test or README text states this, and the second row oflanguages/golang/encrypt/kinds_test.go:61setsBy: []byte{}instead of nil. - Optional: no test encrypts or decrypts an empty batch (
nilor zero rows) through the generated API. It works today.languages/golang/encrypt/gensupport/codec.go:91 - Optional:
Sealpasses its already-extended plan tolocateTermFailure, which passes it toDerive, andDeriveextends it again. So each probe derives under the cipher's extension twice. Pass the plan thatSealreceived.languages/golang/encrypt/records.go:62 - Optional: for a pointer message type (the
Generatepath, for example*pb.Individual), the generatedSourcereads fields with no nil check. A nil element in the slice makesEncryptpanic instead of returning an error.languages/golang/stashgen/emit.go:314 - Optional:
protosourcelists the members of a protobufoneofas facts, but protoc-gen-go puts those members in wrapper types, not in the message struct.Generatethen stops with "the struct has no such field" for any message that has aoneof. Refuse aoneofwith a clear message, or document the limit.languages/golang/encrypt/policy/protosource/protosource.go:35 - Optional:
Generatetakes its context through theWithContextoption, butFromTagstakesctxas its first argument. MakeGeneratetakectxfirst, before the API is released.languages/golang/stashgen/policy.go:41 - Optional:
Declaration.Identitywrites into thefieldsslice that it shares with the receiver, so the earlierDeclarationvalue changes too. Generated code builds one chain, so no generated file is affected today.languages/golang/encrypt/gensupport/declaration.go:134 - Optional: no test seals a field under one
Identity, renames the field, and opens the data through the engine. OnlyFieldContextis unit-tested.languages/golang/encrypt/gensupport/declaration.go:128 - Optional: code generated with
-for, or from a policy throughstashgen.Generate, never runs against a guest. The golden test compares the text, and the stub test only compiles it.languages/golang/stashgen/policy.go:90 - Optional: no test reaches the
stashgen.Generaterefusals for two facts that name one field, or for a policy decision that does not parse.languages/golang/stashgen/policy.go:134
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 8 found, 6 posted |
| claude | claude-opus-5-5 | rust | 2 found, 0 posted |
| claude | claude-opus-5-5 | golang | 4 found, 4 posted |
| codex | gpt-5.6-terra | test-gap | 0 found, 0 posted |
| codex | gpt-5.6-terra | rust | 0 found, 0 posted |
| codex | gpt-5.6-terra | golang | 1 found, 1 posted |
Synthesis: claude-opus-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 1 posted finding(s) were raised by two or more models.
Plain language: claude-opus-5-5 read every comment as a new reader would. 3 comment(s) had a problem that stopped the reader acting; it rewrote 3.
Stack: position 3 of 8 (#1090, #1093, *️⃣ #1094, #1095, #1096, #1103, #1104, #1110). *️⃣ marks this pull request.
Context loaded: the description, 2 linked issue(s) and 29 discussion entries.
coderdan
added a commit
that referenced
this pull request
Oct 7, 2026
…trip The second cipherstash-bot review on #1094: - Decrypt of a type with an index-only field (`index=` without `encrypt`) failed for every row: nothing opens for it, and Value asked for it. The generated Value now skips it, so it keeps its zero value. - A nil interface in a passthrough field (`any`, `error`) failed Encrypt and Decrypt: a nil interface asserts to no type. Passthrough and Get return the zero T for it. - Open on a nil *Cipher or *Client returns ErrEncoding instead of panicking. - A nil element of a pointer message type is an error, not a panic. - An opaque struct with NaN or an infinity is ErrEncoding, and the README says floats there must be finite. - Generate takes ctx first (WithContext is gone) and closes the guest engine it starts. - stashgen refuses a file that would redeclare a name the package declares. - protosource refuses a message with a oneof; a proto3 optional stays a fact. - Declaration.Identity and add no longer write into a slice an earlier Declaration shares. - Seal passes the unextended plan to locateTermFailure, so a probe is not extended twice. New tests: an index-only type and nil any/error passthroughs round-trip over the deterministic guest; a renamed field opens under its Identity and not without it; empty batches, nil []byte, NaN, nil Decrypter; refusal cases for model double-binding, a model naming no field, two opaque fields writing one JSON key, a package name clash, duplicate facts and an unparsable decision. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
Contributor
Author
|
The second cipherstash-bot review: the review-body findings, all in e8cd325 and 6efc22b except the last one.
The five PRs above (#1095, #1096, #1103, #1104, #1110) are rebased onto this. |
coderdan
added a commit
that referenced
this pull request
Oct 7, 2026
…ts type Closes #1082 (CIP-4326). A data plan field with any term output (eq, match, ore, ope) and no "type" is now refused when the plan is built, with Error::Plan, by record::plan, plan_with and Plan::new / new_with alike: an indexed field's terms derive from the one declared kind, and every value is checked against it on the way in and on the way out, so the engine never trusts a binding to have tagged each value the same way. Before this a 34 sent once as a Float64 and once as an Int64 under one "ore" field was accepted both times and stored two terms. That gap was kept open only until the Go side filled the type; the Go SDK (#1094) fills it on every field from the Go kind, so nothing a binding sends today is refused. The type stays optional where no term derives from the field: a field whose only output is "c" or "passthrough", and a target field, whose kind is the EQL type's own (#1095). Declaring a type changes no stored byte (#1093), so no row is re-encrypted. The IndexSpec: Index<Value> dispatch is unchanged: the value still carries its tag, now always checked against a declaration before the term is derived. The "transitional" wording goes from the dynamic module, kind and record docs and from the plan document. Tests: refused at build for every index kind, alone and beside a ciphertext, spelled as a key and as the match object form, by the parser and by hand; a sealed-only and a passthrough field still build, seal and open without a type. Every test that indexed an untyped field now declares the type; the one that fed a float to an untyped equality field now shows there is no such field to feed, since float64 is refused at build and an integer kind refuses the float value. The guest's native tests and plan_check cover the refusal; the check_record fuzz generator emits a "type" entry so indexed plans still parse under fuzzing. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
This was referenced Oct 7, 2026
Add the Go half of the Stack Encrypt code generator: the stashgen library reads a struct's stash tags through go/packages and go/types, checks the declaration with an Engine, and writes the encrypted type, Encrypt, Decrypt, Fields and the print methods. encrypt/gensupport holds what only generated code calls and nothing else: the GeneratedVersion1 constant, Redacted and RedactedLog, and the once-per-type stderr notices. The generator reads types, not text, so it sees a type from another package the way the compiler does, and it runs none of the package's code. The output file is parsed as its package clause only, so a stale generated file does not stop regeneration. Output is deterministic: declared field order, no time, no version string beyond the constant. Every refusal about an index, an EQL type or a field type comes from the Engine, not from the generator; the generator holds no copy of the engine's rules (SDK principle 1). GuestEngine is the hole the integration step fills with the embedded WASI guest; until then it returns ErrEngineUnavailable and the static fake in internal/fakeengine stands in for tests, knowing TextEq and the four indexes with their Go-kind rules as stack-encrypt's target/index.rs implements them. The golden tests seed from the hand-written examples in docs/plans/2026-10-04-plan-builder and deviate only where a generator needs one rule where the hand-written files used several: a composite literal with more than one entry is written one entry per line; Seal assigns into `var e` with the passthrough fields first and the sealed fields in declared order, with no local names that could collide with a field; Encrypt and Decrypt carry doc comments on every type, naming the type rather than guessing a singular; and the compile check happens against a signatures-only stub of encrypt, encrypt/eql and encrypt/gensupport under testdata, because those packages do not exist yet. The stub is the contract the integration step implements. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…heck Add cmd/stashgen, the go:generate front end: the flags from the plan's reference (-type, -name, -for, -model repeated, -redact, -output), the notices on stderr, and a non-zero exit with no file written for every refusal. run takes the engine constructor so the tests drive it with the static engine while main uses GuestEngine, which still stops with ErrEngineUnavailable: the command is wired, the engine is not. The fake engine moves from internal/fakeengine to the public enginetest package so the command's tests can import it; the Go tool forbids an internal import across the cmd/ and stashgen/ trees. Every entry of the plan's refusal list has a test asserting the error names the field, through *FieldError and errors.As, plus the refusals the list implies: an untagged embedded struct from another package, -redact on a type that already prints, -for on a type with a field the struct does not name, a model field of the wrong type, and a struct that stores no field. Three more golden cases cover what the plan's examples do not: two tagged structs in one package with -name, separate columns on an integer with ore and ope, an embedded struct of the same package, and -for a type with unexported fields, which takes the by-name path and pointers. TestStubAgreesWithGensupport loads the real encrypt/gensupport and the test stub with go/packages and fails when a symbol both declare has two shapes, so the contract the integration step implements cannot drift from what already ships. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
Add encrypt/policy, stashgen.Generate and encrypt/policy/protosource: the path for a type that cannot carry stash tags, such as a protobuf message whose field options hold its data categories. A Source gives the facts about each field, the first rule that matches a field decides it, and Generate writes the same file the tag path writes, into a package of the user's own, with pointer-taking functions. A decision spells itself in the tag grammar (Outcome.Tag returns what the field's stash tag would say) and Generate parses it with the tag parser, so the two ways in share one grammar, one reader and one emitter, which is what SDK principle 6 asks of a second way in. That also keeps policy free of an import of stashgen, which Generate needs to import; the index values are policy.Equality, policy.Match() and friends rather than the plan's encrypt.Equality, because package encrypt does not exist in this tree and policy must not import a generator package. Identity joins the declaration as a field a policy alone sets; the tag grammar has no word for it. protosource reads the descriptor through protoreflect, records each extension set on a field's options under the extension's full name, and derives the Go field name the way protoc-gen-go does; Generate matches facts to struct fields through the protobuf tag's name= first and the Go name second. Its fixture is the plan's protoc-gen-go output, renamed. This adds google.golang.org/protobuf to the module. The policy test asserts that the policy path and the tag path write the same body for the same declaration, so the one emitter stays one. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
The two packages are `encrypt` and `auth`, at languages/golang/encrypt and languages/golang/auth, as the plan builder design names them: a package name does not repeat the product, and `encrypt.Cipher` reads as what it is where `stackencrypt.Cipher` repeated itself (SDK principle 13). A move with nothing else in it, so a review can read it as one: `git mv` of both directories, the guest crates with them, the package clauses, the import paths, the selectors, the error prefixes and the prose; every path in mise.toml, the root Cargo.toml exclude list, .gitignore, dependabot, tests-golang.yml, go-binding-test.sh, the scripts/__tests__ guards, AGENTS.md, SECURITY.md, stack-guest-abi's module doc, stack-encrypt's CONTEXT.md and the label-segments fixture comment. The task alias `go:stackencrypt:test` goes with the name it kept. The CI variable STACKENCRYPT_TESTS_REQUIRE_LOCK becomes STACK_ENCRYPT_TESTS_REQUIRE_LOCK. Two renames the compiler forced: the plan package's `encrypt` verdict constant collided with the package it now imports, so it is `sealed`; two test locals named `auth` shadowed the auth package and are `cts`. docs/plans and the ADRs keep the old names as history. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
The Go SDK is now what the plan builder design describes: a struct's stash tags are the declaration, stashgen writes the encrypted type and its functions, and a program calls users.Encrypt, users.Decrypt and users.Fields. Everything the old package offered beside that is removed, not deprecated — it was never released: Cipher.Encrypt/Decrypt and the Element forms, Client.Decrypt, EncryptRecord(s)/DecryptRecord(s), EncryptedRecord, EncryptedField, Cipher.Term, RecordOption, WithPlan, ExtendContext, WithGuest, TermKind, Plan, FieldPlan, NewPlan, PlanFromTags and every run-time tag reader, the plan package and plantest, factstest, Context/Label and their constructors, and the four Sealed* storage types. What replaces them, and why it has the shape it has: - encrypt.Cipher.Extend appends the caller's parts to every field's context in every call through the cipher, so the write, the query and the read cannot use different ones. encrypt.Ciphertext is the frozen stack-encrypt leaf, the one storage type: every sealed field is one leaf. Index is Equality, Ore, Ope, Match() and JSON(); the term types stay. Decrypter is what a generated Decrypt takes: *Cipher refuses a foreign keyset's row before any key is retrieved, *Client opens each row under the keyset that sealed it. - The record path (Cipher.Seal, Cipher.Open, Client.Open, Cipher.Derive) takes the internal record types, so only generated code reaches it. internal/record is the data form of a declaration as dynamic::record reads it: a field's context is its label (context segments + identity) nested under each extension part, "type" is the wire kind, outputs are c/eq/match/ore/ope. Both gensupport (running) and stashgen (checking) lower to it, so the generator checks the plan the program will run. - encrypt/gensupport is the real library behind generated code: Declare and the verb methods carry the field's wire kind, which stashgen picks from the Go type (int8..int32 travel as int32, int/int64 as int64, the unsigned likewise, []byte as bytes); Codec.Encrypt/Decrypt send a slice as one guest call and one ZeroKMS request; Field[T] seals one value and derives the field's terms; Get converts within a kind's family and refuses the rest, so a value that opens to another type is an error. - Passthrough fields stay on the host. The FFI codec cannot carry every Go type a program stores beside a ciphertext (time.Time, gorm.DeletedAt, any driver.Valuer), and nothing the engine does to a passthrough value is observable, so the plan the engine sees has the sealed fields only and the generated Seal reads passthrough values back from the Record. The plan says passthrough crosses; this is the deviation, recorded here and in the record package's doc. - An opaque struct crosses as one JSON document, declared bytes, because the engine seals a composite FfiValue as a tree of leaves and an opaque struct is one column. Its fields are what JSON carries; the generator refuses a nested struct inside one for now. - stashgen.GuestEngine is the embedded guest: encrypt.NewChecker instantiates it with no credentials and asks se_plan_check one field at a time, so a refusal names the field, then for the whole plan. The engine produces no EQL type in this build (se_targets is empty), so encrypt_into is refused with "EQL types are not available yet". The guest loses se_encrypt, se_decrypt and the element exports (ADR-0007 as amended: every value crosses under a declaration) and gains se_plan_check and se_targets. A `deterministic-kms` feature builds a TEST guest whose keys derive from a seed — the DeterministicSource of stack-encrypt's tests/common, copied — so the Go tests open the records Rust sealed in tests/fixtures/record_lowering.json through the generated testusers package and derive the same term bytes, and run round trips, foreign-keyset refusal, tampering and the ORE/OPE ordering properties with no ZeroKMS. The hermetic tests replace the old live ordering tests; the live suite keeps the real round trips through generated code. The test build's import gate requires no transport import: with no ZeroKMS client in it, the linker drops the host module. Generated *_stash.go files are committed (internal/testusers, example), and tests-golang.yml runs `go generate ./...` and fails on a diff. The stashgen goldens and the stub SDK follow the new signatures; the stub agreement test now covers encrypt too and compares signatures without parameter names. The example is rewritten to the eight steps; the explicit-credentials example, which only showed the removed API, is gone with its task. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
Two Codex findings on PR #1094, both real, both the same shape: a value the generator accepted and Encrypt sealed could never be decrypted. Get sent every opened value through convert, whose switch knows the wire kinds and a few slice types. A passthrough field is the Go value the struct held, kept on the host, so a time.Time or a sql.NullTime — the plan's own accounts example — reached convert and was refused. Get now returns a value that already is a T before it converts anything. An opaque struct came back from JSON into Values and each field went through Get, so a map[string]string (JSON gives map[string]any), a type defined over a scalar, a nested struct or a pointer could not be read, although classify and readableInOpaque had accepted them. The opaque value now crosses as a JSON document of a generated shape struct (documentOpaque, with json tags for the declared names), and the generated Value decodes it straight into that struct with gensupport. Opaque, so every type encoding/json carries both ways comes back as it was. The generator's rule is that: a field of an opaque struct may be a scalar or a type defined over one, []byte, a slice or array, a map with string or integer keys, a pointer, a struct whose fields are all exported, or a type with its own MarshalJSON/UnmarshalJSON (time.Time); a channel, an interface, a map keyed by a struct, or a struct JSON would truncate is refused at generate time with the field named. Outside an opaque struct the engine seals a composite as a tree of leaves and a column holds one, so stashgen now refuses encrypt or index= on a struct, slice or map field ("seals only as part of an opaque struct") instead of letting Encrypt fail at run time. A type defined over a scalar (type Email string) is accepted: the engine returns the underlying type, and the generated Value reads it at that type and converts, since Go has no way to do that generically without reflection. The proof is in encrypt/internal/testusers: Account (time.Time and sql.NullTime passthrough beside an indexed field), Kinds (one sealed field of every scalar kind, defined types among them, and passthrough fields of every type the codec cannot carry) and Everything (an opaque struct with a field of every type JSON carries), each generated by the real stashgen against the real guest and walked through Encrypt then Decrypt over the deterministic build with reflect.DeepEqual. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
CI's live job failed TestPlaintextDoesNotRemainInGuestMemoryAfterEncrypt with "term derivation failed": it encrypted a User with every field but Notes at its zero value, and the empty Email sits under a match index. Reproduced hermetically over the deterministic guest: the empty string and a separator-only string fail, a zero Age under ore and an empty Notes with no index do not. The engine is right to refuse. sem::match_terms defines no match term for text that yields no token — empty, separator-only, or shorter than the n-gram — because an empty term would match every row, and the typed Rust path refuses it the same way. Not a lowering bug; nothing changes under packages/stack-encrypt. What was wrong on the Go side is the message: the guest reports a status and nothing else. Cipher.Seal now asks the engine again on ErrTerm, one term at a time (a term derives locally, so this costs no key request), and wraps the error with the row, the field and the index: `value 1, field "email": the engine derives no match term for this value (a match index needs text with at least one token; ...)`. TestAMatchIndexNeedsText pins it for the record call and for the field entry, and pins that the other zero values seal and open. The live residency test encrypts a fully populated record. cmd/stashgen's README says what a match index needs (in the previous commit, with the README's other changes). Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…f the embed `//go:embed wasm` embeds the whole directory, so after `mise run wasm:guest:build:deterministic` every binary built from the tree carried the seed-only test build beside the real guest. Nothing loaded it, but a later read of guestFS by pattern could have. The task now writes it to encrypt/testdata, which go build and the embed both ignore; the tests read it from disk with os.ReadFile and skip when it is absent. The .gitignore entry, the CI artifact and checksum paths and the wasm README follow it. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…round-trips A reviewer generated a field of a defined type (type Status string) and could not decrypt it: convert named only the built-in types, so a *Status, a time.Duration, a []int or a map[string]string reached its default arm. Get now falls through to convertVia, the one place this package uses reflection: a defined scalar type is read as its underlying type and converted, a slice or array element by element, a map with string keys entry by entry, each through convert and its range checks. Generated code stays reflection-free. A JSON number widens to its family's widest type first, so the same reader serves an opaque document's fields. TestGetReadsEveryTypeTheGeneratorAccepts holds the reviewer's cases and more; testusers.Kinds gains a Status and a time.Duration field, Everything gains []int, []float32, Status and time.Duration, and both round-trip through generated code over the deterministic guest. testusers.User gains -model Rows=UserRow and TestModelRowsRoundTrip proves each term lands in its own column and the rows decrypt back; nothing ran Records before. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…e keys The Go decoder was the only definition of an se_targets entry and kept a zero value for an unknown key or a mistyped value. The entry shape is now the one eql-bindings serialises — name, family, suffix, plaintext (a ValueKind name or null), sql_domain, indexes, query, query_sql_domain, producible, reason — documented as wire format in the guest's targets rustdoc and read by parseTargets, a function that returns ErrInternal on an unknown key, a repeated key, a missing name, indexes or producible, or a value of the wrong type. record.Target carries those fields; stashgen's EQLType carries Producible and Reason and the reader refuses a type the engine lists but does not produce, with the engine's reason. TestParseTargetsReadsEachEntry drives it with a two-entry list and every malformed shape. The fake engine's equality rule now matches dynamic::admits: integers, text and bytes, not floats or booleans. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
… the fake TestRefusalsHoldAgainstTheEmbeddedEngine runs stashgen with GuestEngine — the engine it ships with — over the cases a reviewer found the fake and the real engine disagreeing on (time.Time, a channel, equality on a map, equality on a float) and the composites a reviewer sealed and could not open ([]string, map[string]int64); each is refused naming the field, and a float equality is refused by both engines alike. The sealed-field rule (one scalar, or a type defined over one; a composite only inside an opaque struct) is what refuses the first three before either engine is asked. TestTheWholeDeclarationIsCheckedAgainstTheEmbeddedEngine makes the whole-plan check fail — two fields a policy pinned to one identity — and pins its FieldError with no field name. The two doc comments that still said a composite seals as one value say what the code does. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…ails on any other error The residency check runs hermetically over the deterministic guest with a populated record (a match index refuses an empty Email) and checks the guest's memory for the plaintext and the email after Encrypt. The same check after Decrypt is its own test and is skipped with the reason: one unwiped copy of each opened string remains, made by vitaminc-aead-value's FfiValue decoder, which copies the decrypted leaf into a new Protected and leaves the AEAD output it copied from to drop unwiped — the host's output buffer is wiped through se_dealloc and the Protected payloads on drop, so the copy is vitaminc's to remove, and the test runs again once it is. The live variant keeps the Encrypt check across a real ZeroKMS request. The fixture test skips only when the guest is not built and fails on every other error from NewDeterministicClient. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…d the review's small findings DeterministicSource existed twice — stack-encrypt's tests/common and the guest's deterministic.rs — and the Go fixture test depends on the two deriving the same bytes. It now lives once in stack-kms behind its test-support feature, beside FakeDataKeySource; the stack-encrypt tests and the guest import it, and the guest's deterministic-kms feature no longer carries its own sha2. The rebuilt test guest still opens the fixture, which is the proof the move changed no bytes. protosource.GoName is protoc-gen-go's GoCamelCase word for word: a digit ends a word (foo_1bar is Foo_1Bar, sha256sum is Sha256Sum), a leading underscore is X, a dot is an underscore unless a lower-case letter follows; TestGoName held the wrong value for foo_1bar. scalar's enum branch has a test through a dynamic descriptor, since testpb declares no enum. stack-guest-abi's crate doc names the guest's real path. The three documents say what a match index needs — text with at least one token, three characters for the n-gram — that the whole batch fails otherwise, and that an optional or short value does not belong under match. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…drops sha2 DeterministicSource prints without its seed, as DataKey does. stack-encrypt no longer uses sha2 now that the deterministic source lives in stack-kms, and the label_segments fixture names the Go test that actually reads it. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…trip The second cipherstash-bot review on #1094: - Decrypt of a type with an index-only field (`index=` without `encrypt`) failed for every row: nothing opens for it, and Value asked for it. The generated Value now skips it, so it keeps its zero value. - A nil interface in a passthrough field (`any`, `error`) failed Encrypt and Decrypt: a nil interface asserts to no type. Passthrough and Get return the zero T for it. - Open on a nil *Cipher or *Client returns ErrEncoding instead of panicking. - A nil element of a pointer message type is an error, not a panic. - An opaque struct with NaN or an infinity is ErrEncoding, and the README says floats there must be finite. - Generate takes ctx first (WithContext is gone) and closes the guest engine it starts. - stashgen refuses a file that would redeclare a name the package declares. - protosource refuses a message with a oneof; a proto3 optional stays a fact. - Declaration.Identity and add no longer write into a slice an earlier Declaration shares. - Seal passes the unextended plan to locateTermFailure, so a probe is not extended twice. New tests: an index-only type and nil any/error passthroughs round-trip over the deterministic guest; a renamed field opens under its Identity and not without it; empty batches, nil []byte, NaN, nil Decrypter; refusal cases for model double-binding, a model naming no field, two opaque fields writing one JSON key, a package name clash, duplicate facts and an unparsable decision. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
About 4,000 of this branch's added lines are machine output: the *_stash.go files stashgen writes, and the *.golden files its tests compare against. GitHub shows them expanded next to the hand-written source, which buries the code a reviewer actually needs to read. linguist-generated makes GitHub collapse them in diffs by default and leaves them out of language statistics. It changes nothing for git, CI or the build, and the files can still be expanded on demand. Every *_stash.go file in the tree carries stashgen's "DO NOT EDIT" header, so the pattern catches no hand-written file. Claude-Session: https://claude.ai/code/session_01WQQPj7Y5yASArvZaHgFpCt
Go principle 10 in docs/sdk-design-principles.md says a change to which fields are encrypted shows as a change to a committed file that a reviewer reads. The *_stash.go files are that committed record, and for a type declared through a policy they are the only place a rule change shows. Marking them linguist-generated made GitHub collapse them by default, so a reviewer could pass over exactly the change the principle wants read. The *.golden files stay marked: they are the generator's test expectations, not the declarations a program ships. Claude-Session: https://claude.ai/code/session_01WQQPj7Y5yASArvZaHgFpCt
The third cipherstash-bot review on #1094 (Go principle 12): when a stored value did not fit its field's Go type, the error carried the value, so a sealed uint8 that opened as 300 returned "300 does not fit a uint8". The value is decrypted plaintext, and an error is what a program logs. The same held for an opaque JSON number, a map key inside a sealed map, and encoding/json's own errors, which quote the input. Every conversion error now names types only. Get and Opaque wrap encrypt.ErrEncoding, which doc.go already promised for a stored value that does not fit its declaration, and encoding/json's errors are no longer wrapped. TestDecryptErrorsHoldNoPlaintext fails on each case against the previous code. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
The third cipherstash-bot review on #1094, the claims and print items: - The batch claim. Encrypt and Decrypt send one ZeroKMS request for each 500 sealed values, plus one the first time a keyset is used, not one request per call. The READMEs, doc.go and the comment stashgen writes on every generated Encrypt and Decrypt now say so. - Database libraries. No test runs database/sql, pgx, sqlx or GORM, so the docs now name the interfaces instead: Ciphertext and each term type implement driver.Valuer and sql.Scanner, one column each. - The Rust bytes. A field lowered from data seals as vitaminc's tagged leaf whatever its kind, which a Rust record opens only through a Value field. Three comments said a uint32 or string field wrote the bytes a bare Rust u32 or String does. - What this build refuses. index=json and an index with options are marked refused and listed under "When stashgen stops"; the options refusal now names the real limit (a query term uses only the default options) rather than the record plan, which carries them. The two Go-only tag words, opaque and json, are named. Only a policy sets Identity, and the README says why. A type with nothing sealed on its own gets no Fields, and the README says why. - Go 1.26, not 1.24, matching go.mod. The step 5 snippet compiles (opened, not people, and err is checked before defer). The stashgen quick start uses index= until EQL types land. The CI recipe also fails on a generated file that was never committed. - "a auth" and "a encrypt client" read "an". - Print methods. The print notice checks the signature, not only the name: a LogValue() any is not a slog.LogValuer, so slog prints every field. -redact also writes GoString, which %#v calls, and refuses a struct that already has one. testusers.Secret is generated with -redact, and TestARedactedStructPrintsNoSealedField formats it with %v, %+v, %#v, %s and slog. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…catches an uncommitted generated file The third cipherstash-bot review on #1094 (Go principles 1, 9 and 10): - The shape check embedded the same named type the user's struct did, so a field added to an embedded Person, or to gorm.Model, still converted. The generated code reads fields through the embedded struct one by one, so Encrypt dropped the new field with no error. stashgen now also writes a shape for each embedded struct it reads (recursively, skipping one tagged `stash:"-"`). One from another package with an unexported field cannot convert, and the generator's notice says so: the compiler finds a removed or retyped field and CI finds an added one. TestAFieldAddedToAnEmbeddedStructStopsTheBuild adds a field to the embedded case's Person and checks the stale file fails to build at the embedded shape. - The "Generated code is committed" step ran git diff, which ignores untracked files, so a //go:generate line whose output was never committed passed. It now also fails on untracked files. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…parameter The third cipherstash-bot review on #1094 (Go principles 1 and G6): - Generate wrote its file without the name check FromTags runs, and had no way to set a name, so two messages generated into one package both declared Encrypt and the compiler reported the clash in the generated file. Generate now loads the output package (without the file it replaces) and refuses a clash, and WithName gives the names a prefix as -name does on the tag path. - The output path was a required option, found missing only at run time. It is now a parameter: stashgen.Generate(ctx, source, message, "../individuals/individual_stash.go") and Output is gone. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…elling The third cipherstash-bot review on #1094 (Go principles 8 and 13), before the first release makes these names permanent: - auth.ErrAuthTransport, auth.ErrAuthConfig, auth.ErrAuthOther and auth.WithAuthBaseURL repeat the package name; they are now auth.ErrTransport, auth.ErrConfig, auth.ErrOther and auth.WithBaseURL. The internal/guest names stay. - gensupport.UInt32/UInt64 and record.UInt32/UInt64 are Uint32/Uint64, as the standard library and stashgen's own KindUint spell them. Generated files and goldens are regenerated. Claude-Session: https://claude.ai/code/session_01V3WFXwax4J3uecpFEJ6yHc
…oved here The note on plan.Custom and plantest snapshots described a Go package this PR deletes. Go has no release yet, so no user ever saw plan.Custom bind a text part; the crate CHANGELOG ships to crates.io and should not name it.
coderdan
force-pushed
the
feat/go-stashgen
branch
from
October 7, 2026 04:25
3e94e0d to
d680c27
Compare
The keys of a target entry were strict, but two values were not: plaintext
became a record.Kind and each index a record.Output with no check. A typo
in the Rust serialiser ("strng", "equality") then reached stashgen as an
EQL type with no kind or no indexes, and no error. Both are now refused;
"json", the SteVec document index no plan output carries, is accepted.
record.Kind's check is exported as Known for it.
…s Decrypt
- The example sealed through Extend("tenant-42") and decrypted through the
Client, which refuses every such row; it now decrypts through the cipher,
and the Cipher, Decrypter and README docs say a Client opens only rows
sealed with no extension.
- doc.go says the guest holds nothing but the client key and keyset cache
between calls; after Decrypt one unwiped copy of each opened string
remains, as the skipped residency test records. The test comment no
longer names a vitaminc line nobody has confirmed.
- A generated Decrypt over a struct with index-only fields says those fields
come back zero and must not be encrypted again to update the row, and the
stashgen README says the same: re-encrypting stores the term for zero.
- The generated Encrypt and Decrypt comments, records.go and gensupport
count the keyset request beside the one per 500 sealed values.
- The README lists which terms a database can compare: ORE terms sort almost
correctly as plain bytes, so ORDER BY on them is wrong without an error.
- The tag table warns that the name and context= are part of every stored
value's context, and the cipher section that a value is not bound to its
row.
This was referenced Oct 7, 2026
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Go applications can now declare encryption on ordinary structs using
stashtags and generate typedEncrypt,Decrypt, andFieldshelpers withstashgen. The generated code handles encryption plans internally, so application code works with its own record types. This replaces the previous value and record APIs and renames the Go packages fromstackencrypttoencryptandstackauthtoauth.The batch encryption call uses one request to ZeroKMS, CipherStash's key-management service.
Changes
stashgencommand and library, struct-tag grammar, deterministic generated output, and checks against the embedded Rust encryption engine. Invalid declarations report the affected field.encrypt/policyfor rules based on field metadata andencrypt/policy/protosourcefor metadata from Protocol Buffers descriptors. Both use the same declaration reader and code generator as struct tags.go generate ./...leaves committed generated files unchanged. Generated Go code is checked against records encrypted by Rust.Verification
The existing PR description reports the following checks. This description edit did not rerun them.
languages/golang,gofmt -l .,go build ./...,go vet ./..., andCGO_ENABLED=0 go test ./...;golangci-lint run ./...(0 issues); andGOOS=linux GOARCH=386 go vet ./....mise run wasm:guest:build,wasm:guest:build:deterministic,wasm:auth-guest:build,wasm:guest:test, andwasm:wasi-check.cargo fmt --all --checkandcargo clippy --workspace --all-targets --all-features -- -D warnings.scripts/__tests__that reference Go paths.Related
docs/plans/2026-10-04-plan-builder.mdand ADR-0008 (Architecture Decision Record 0008), documented indocs/sdk-design-principles.md.Review notes
languages/golang/cmd/stashgen/README.md, then the generatedencrypt/internal/testusers/user_stash.go, and the declaration-to-plan conversion ingensupport.time.Time,gorm.DeletedAt, or arbitrarydriver.Valuerimplementations. Generated code preserves these caller values directly; only encrypted fields are sent to the engine.encrypt_intobecause the runtime has no EQL targets yet. It also leaves out the proposedgo vetcheck for printing tagged structs and implementations ofMatchOptionandJSONOption.