Conversation
|
A tenant-scoped query probe had to be assembled by hand. ExtendContext was a RecordOption, so EncryptRecords and DecryptRecords took it and Term did not: the caller rebuilt the field's context with Context.With, re-typing what the plan already knew, and any slip (uint64(7) on write and 7 on read, or a different nesting) produced a valid term in a different domain. The query returned nothing and no error, which is the failure ADR-0004 warns about. RecordOption becomes Option, accepted by every record call, and TermOption is the subset Cipher.Term also accepts. ExtendContext returns a TermOption, so one value serves encrypt, decrypt and probe; WithPlan returns an Option only, so passing it to Term does not compile. One function, extend, applies an extension for both the plan and the probe, and a unit test pins that the two produce the same context. The live records test now checks a tenant's probe matches only that tenant's rows.
Cloning the []any only copied the slice of parts; a []byte part still pointed at the caller's buffer, so an option held across calls, which is what this API asks for, would extend by whatever that buffer held at each call. The option now owns a copy of every byte part, and a test mutates the source buffer between applications to pin it.
The type named Option was the narrow one: it served only the record calls, while TermOption served those and Cipher.Term too. The broad name now goes to the broad type. RecordOption is what only a record call takes (WithPlan); Option is a RecordOption that Cipher.Term accepts as well (ExtendContext). Cipher.Term takes ...Option and still refuses a plan at compile time. Several ExtendContext options on one call join in order, so ExtendContext(a), ExtendContext(b) is the context ExtendContext(a, b) gives. That rule is now written in the ExtendContext doc, with the warning that follows from it: an extension given twice extends twice, and a probe built with it once matches none of those rows. applyRecord and applyTerm both call one appendTo method, so the record side and the probe side cannot combine options by different rules. Tests: TestSeveralExtensionsJoinInOrder applies two options to one record plan and one probe and checks both equal the one-option context; TestTermExtensionMatchesRecordFieldContext now also checks WithPlan is not an Option; TestGuestRefusesMalformedInputsBeforeState gains bad extension parts on Term, a record write and a record read, and TestBadExtensionPartFailsTheCall checks those calls fail because of the part, which tells a dropped error apart from a guest refusal.
NewContext and Context.With stored a []byte part as the caller's slice, the same aliasing ExtendContext had one layer up: a buffer reused after the context was built changed what the context bound. One helper, ownPart, now copies a byte part for both the Context constructors and the option, so there is a single definition of what a part the SDK holds looks like. A test mutates the source buffers after NewContext and With and checks the context still holds the original bytes.
coderdan
force-pushed
the
fix/go-context-owns-byte-parts
branch
from
October 2, 2026 23:59
f1cdbb8 to
887529d
Compare
A policy derives each field's encryption context from names, so an ordinary rename (a proto field, a Go struct field, a table) can change a context with no error on the write path: rows already written stop decrypting and their query terms stop matching. plantest.Golden makes that a test failure. Golden builds the plan as plan.PlanFor does at startup and compares what it stores each field as with a snapshot checked in at testdata/<test name>.golden; -update rewrites it. The snapshot records only what stored rows depend on: per encrypted field its column, context, index terms and facts, keyed by column rather than field name, so a rename the policy pins with plan.Column leaves it byte-for-byte unchanged and the test passes; per field decided Plaintext, its name and facts. Output is sorted and line-ending-normalised, so it is the same on every run and platform. On a mismatch the failure sorts the changes by cost: context changes (data loss) first, target changes (a migration) next, then the rest, followed by a unified diff. For a lost context it names the pin that keeps it, and only after building the plan with that pin to confirm it restores the old column and context. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5Y8iJ3ZM71digszHzpyrd
coderdan
force-pushed
the
claude/intelligent-mayer-0t48ek
branch
from
October 3, 2026 00:12
28beeca to
8787b99
Compare
coderdan
force-pushed
the
fix/go-context-owns-byte-parts
branch
from
October 3, 2026 06:32
887529d to
ab8ca58
Compare
An error occurred while trying to automatically change base from
fix/go-context-owns-byte-parts
to
feat/go-term-options
October 3, 2026 06:41
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
2 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
Stacked on #1022. In the Go SDK, a policy (
plan.Policy) decides how each field of a message is encrypted, and it builds each field's encryption context from names: for an EQL column, the context is"<table>/<column>". That context is mixed into every ciphertext and every searchable index term written for the field. If it changes, rows already written stop decrypting and stop matching queries, and nothing on the write path reports an error. Renaming a proto field or a Go struct field is enough to change it.This PR adds
plantest.Golden, a test helper that saves a snapshot of every context and target a policy produces to a checked-in file. The test fails when any of them changes. A context change is reported separately from other changes, along with theplan.Columnpin that keeps the old context.Changes
stackencrypt/plan/plantest:Golden(t, src, m)builds the plan the same wayplan.PlanFordoes at startup.testdata/<test name>.golden. A subtest's snapshot goes in a folder named after the parent test.-updatewrites the file. If the file already existed, it also logs what changed.plan.Fail) fails the test with the build error.Plaintextrecords its name and facts.\r\ncheckouts compare equal. The output is the same on every run and platform.plan.Column/plan.Identitypin, and the hint appears only if that restores the old column and context. Otherwise the message gives the reason no pin can (the table changed, or the target supplies its own context).planpackage doc.Verification
TestRenameWithoutAPinIsAContextChange) and a Go struct (TestStructFieldRename):plan.Column("medicare_number").TestChangesAreSortedByWhatTheyCostcovers 11 kinds of change. For each, it checks the change lands in exactly one section, with the expected advice.\r\ncheckout passes.TestPoliciesruns the realGoldenagainst two checked-in snapshots. CI runs these on Linux, macOS, Windows and 386.gofmt,go vet ./..., andgolangci-lintv2.14.0 (the versionmise.tomlpins) report 0 issues.go test ./...passes, also underGOARCH=386.GOOS=windows go vetpasses on the new package.stackencrypttests, because the WASI guests weren't built here. Their tests skip without the guests, and this PR doesn't touch them.No changeset: the Go module has no release process yet. No skill covers the Go module.
Review notes
plan.Columnthen leaves the snapshot unchanged, which is what lets the test pass "unchanged" after the pin is added. If the source field name were recorded, every safe rename would also need-update, and an unchanged snapshot would no longer mean "storage unchanged". The current field name still appears in every failure message, so the developer knows which rule to pin.Golden(t, src, m)with an explicitplan.Source, mirroringplan.PlanFor(src, m). Once a protobuf fact source exists, it plugs in assrc.plantestregisters the conventional-updateflag on the default flag set. A test package that defines its own-updatewould panic with "flag redefined". The package doc says to read plantest's flag instead.🤖 Generated with Claude Code
https://claude.ai/code/session_01U5Y8iJ3ZM71digszHzpyrd
Generated by Claude Code