Repository navigation
feat(golang): every failure a guest reports is a Diagnostic that unwraps to its sentinel - #1110
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (20)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe shared Go guest package now retrieves optional error details after guest-call failures and returns diagnostics that unwrap to existing sentinel errors. The auth and encrypt packages expose the diagnostic types. Tests and documentation cover diagnostic content, sentinel matching, and compatibility with guests that omit the export. ChangesGuest error diagnostics
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant GuestCall as guest.Call
participant Diagnose as Exports.diagnose
participant LastError as se_last_error
participant Memory as Guest memory
GuestCall->>Diagnose: Pass PackedResult error
Diagnose->>LastError: Request error details
LastError-->>Diagnose: Return packed guest-buffer location
Diagnose->>Memory: Read and copy detail bytes
Diagnose->>Memory: Wipe detail bytes
Diagnose-->>GuestCall: Return Diagnostic or status sentinel
Merge Risk: ⚪ Minimal · up to No actionable issue remains before merge. Guest errors retain their existing error-kind checks while exposing details when available. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Detailed errors preserve existing error classification and compatibility, but they also expose server-provided text through ordinary Go errors. The guarantee that diagnostics never contain credentials depends on an upstream description contract that was not established. No actual credential disclosure was verified. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 19 files. (1 skipped: 1 unsupported.)
Comment |
8faaf44 to
d18bfbc
Compare
d18bfbc to
6056eab
Compare
|
Rebased onto #1094 at 6efc22b, which fixes the second cipherstash-bot review there (index-only fields and nil interface passthroughs now round-trip). Two conflicts came up, in |
2705d06 to
20b9c50
Compare
20b9c50 to
8e95495
Compare
8e95495 to
8968ef1
Compare
8968ef1 to
f6bb5db
Compare
2732d61 to
245bb8d
Compare
…aps to its sentinel A Go caller could check an error's kind with errors.Is and nothing more: the guest handed back a status number, and errors.As had no type to target. #1100 gave both guests se_last_error, which hands over the full error behind a failed export. This reads it. guest.Call fetches it after any non-zero status and returns a *Diagnostic: Code, Message, Help, URL, Severity, Fields and Causes, with Unwrap returning the sentinel the status already mapped to, so every errors.Is check keeps working and Error() is the Rust message. The buffer is wiped on both sides once read. A guest with no se_last_error, nothing recorded, or bytes that do not decode into an error with a message all give the bare sentinel, as before; a trap fetching it also wraps ErrTrap, so the caller closes the instance as after any trap. encrypt and auth expose the type as Diagnostic (and Cause) by alias, as they do the sentinels. ExpectedKeyset and FoundKeyset read a foreign-keyset refusal's two keysets, and Field and Reason a refused plan, record or value's field and reason. Tests: a hand-assembled guest drives each missing or broken detail and checks the wipe; each ZeroKMS outcome, each engine refusal reachable through the public API, and each profile-store kind asserts its code; a guest with se_last_error unbound fails with the bare sentinel; the foreign-keyset refusal carries both ids, hermetically and live. The README's Errors section explains errors.Is for the kind and errors.As for the detail, and the rule for what an error may contain. Closes #1101 Claude-Session: https://claude.ai/code/session_01URtfKsTToFUCRwq3g7gCUf
245bb8d to
f2c025d
Compare
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. |
| d.kind = kind | ||
| return d |
There was a problem hiding this comment.
For an unknown status, kind is StatusError's wrapper "cipherstash: internal guest failure (unrecognized guest status N)". Error() returns only d.Message, so that text is lost from every log line. StatusError keeps the number so that a guest/host version skew is easy to diagnose. A newer guest that adds status 28 and records its own error now prints only that error's message. I checked this with the stub guest: status 28 with detail gives Error() = "a new kind of failure", and without detail gives "... (unrecognized guest status 28)".
The skew signal shows only when the guest gives no detail, which is the less likely case for a newer guest. Possible fix: when kind is not one of the known sentinels, return fmt.Errorf("%w: %w", kind, d). Then errors.As still finds d, errors.Is(err, ErrInternal) still matches, and the number stays in the text.
| // An unknown status keeps its number, with or without detail behind it. | ||
| func TestAnUnknownStatusIsStillInternal(t *testing.T) { | ||
| detail := encode(t, map[string]any{"code": "stack_guest_abi::status", "message": "the call failed with status 99"}) | ||
| _, err := callStub(t, 99, detail, withLastError) | ||
| var d *guest.Diagnostic | ||
| if !errors.Is(err, guest.ErrInternal) || !errors.As(err, &d) || d.Code != "stack_guest_abi::status" { | ||
| t.Fatalf("err = %#v, want a Diagnostic over ErrInternal", err) | ||
| } | ||
| if !errors.Is(d.Unwrap(), guest.ErrInternal) { | ||
| t.Fatalf("Unwrap() = %v", d.Unwrap()) | ||
| } |
There was a problem hiding this comment.
The comment says that an unknown status keeps its number, but the test does not assert the number. It checks only d.Code and that Unwrap() matches ErrInternal. The number is in err.Error() here only because the stub's message is GuestError::Status's text. Add an assertion that err.Error() contains 99, and use a stub message without the number. Then the test catches the gap in the comment on diagnose.
cipherstash-bot
left a comment
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🟡 merge after changes (1 of 4 review job(s) failed)
One change before merge: add a real-guest test for Diagnostic.Reason(). The public accessor reads a key that only a stub test checks. The other items can wait. The core path is correct: diagnose, its fallbacks, the wipe of the host copy and the guest buffer, and the trap handling. Every existing errors.Is check still matches.
The most useful follow-up is the order of the token-source error in encrypt/client.go. Today errors.As returns the generic token_get detail, not the auth detail that says why the credential failed.
One finding was checked and dropped. It proposed that decodeDiagnostic refuse a detail with no code. The Rust encoder leaves out code on purpose when an error has none (packages/stack-guest-abi/src/last_error.rs:185). Refusing such a detail would discard its message, help and fields.
Other findings not posted as comments
These lines are not in the diff, so they cannot carry inline comments.
- Optional: the comment and failure text of
TestAuthTransportErrorWithoutAResponseNamesNoStatusare now wrong. (languages/golang/auth/strategy_test.go:683and:702) The comment says the failure "stays the bare sentinel". Thet.Fatalftext says "want a bare ErrTransport". With this PR, a connection refusal is a*Diagnosticfrom the guest'sRequestErrorthat wrapsErrTransport. The test still passes, because it checks onlyerrors.Isand the absence of"HTTP". Update the comment and message, or assert theDiagnosticand its code. - Optional: the
Checker.Checkdoc comment is now wrong. (languages/golang/encrypt/checker.go:43-45) It says "which rule failed is the engine's to know, so a caller that wants the field named checks one field at a time". The returned error is now a*DiagnosticwithField()andReason(), andTestCheckerRefusesAnIndexedUntypedFieldassertsd.Field() == "age". Update the comment so that callers, such as stashgen, read the field from theDiagnosticinstead.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 5 found, 5 posted |
| claude | claude-opus-5-5 | golang | 2 found, 2 posted |
| codex | gpt-5.6-terra | test-gap | 2 found, 1 posted |
| codex | gpt-5.6-terra | golang | failed |
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.
Stack: position 8 of 8 (#1090, #1093, #1094, #1095, #1096, #1103, #1104, *️⃣ #1110). *️⃣ marks this pull request.
Context loaded: the description, 4 linked issue(s) and 5 discussion entries.
| // the snake_case name stack-encrypt's dynamic::Reason gives it | ||
| // ("field_missing", "unknown_key", "field_type", ...), or "" for an error | ||
| // that gives none. New reasons may appear: keep a fallback. | ||
| func (d *Diagnostic) Reason() string { |
There was a problem hiding this comment.
Change before merge: No test checks Diagnostic.Reason() against a real guest, so no test fails if Go and Rust stop using the same key.
Impact: Reason() reads Fields["reason"]. stack-encrypt writes that key in the payload of dynamic::Error (packages/stack-encrypt/src/dynamic/mod.rs:402). If either side renames the key or changes the value format, Reason() returns "" for every error. No test fails. The README documents Reason as a public accessor.
Evidence: The only Reason() test is TestAFailureCarriesItsDiagnostic in internal/guest/diagnostic_test.go. That test uses the stub guest (a test guest, not the real one) with a hand-written detail. The other accessors have tests against the real guest:
ExpectedKeysetandFoundKeyset:TestAForeignKeysetNamesBothKeysetsinencrypt/errors_test.go.Field:encrypt/guest_test.go:786.
The engine refuses a field with outputs ["c", "c"]. It returns Error::Plan { field, reason: DuplicateOutput } (packages/stack-encrypt/src/dynamic/record.rs:267). The test in the fix builds that field.
Fix: Add the test below to encrypt/guest_test.go. The test sends the plan to the guest directly, so the Go check does not refuse the plan first. TestCheckerRefusesAnIndexedUntypedField sends its plan in the same way. The test expects ErrEncoding, code stack_encrypt::dynamic_plan, Field() equal to "age", and Reason() equal to "duplicate_output".
The test uses names that are not exported:
checker.c.callruns a function on a guestinstance.inst.call(ctx, inst.planCheck, buf(encoded))calls the guest's plan check with the encoded plan.- These are the same calls that
TestCheckerRefusesAnIndexedUntypedFieldmakes.
The test also calls a helper wantDiagnostic(t, err, code). The helper must fail the test unless err contains a *Diagnostic with that code. It must return that *Diagnostic. If the package has no helper with this name, add one.
// The engine's reason reaches Go under the key Reason reads.
func TestAPlanRefusalNamesItsReason(t *testing.T) {
ctx := context.Background()
checker, err := NewChecker(ctx)
if err != nil {
t.Fatal(err)
}
defer checker.Close()
p := &record.Plan{Context: []string{"users"}, Fields: []record.Field{
{Name: "age", Kind: record.Uint32, Outputs: []record.Output{record.Ciphertext, record.Ciphertext}},
}}
encoded, err := vcffi.Marshal(p.Wire())
if err != nil {
t.Fatal(err)
}
_, err = checker.c.call(ctx, func(inst *instance) ([]byte, error) {
return inst.call(ctx, inst.planCheck, buf(encoded))
})
if !errors.Is(err, ErrEncoding) {
t.Fatalf("err = %v, want ErrEncoding", err)
}
d := wantDiagnostic(t, err, "stack_encrypt::dynamic_plan")
if d.Field() != "age" || d.Reason() != "duplicate_output" {
t.Errorf("Field() = %q, Reason() = %q", d.Field(), d.Reason())
}
}Found by 1 model: claude
|
|
||
| // Diagnostic is the full error behind a failure the guest reports, beside | ||
| // the kind: every such failure is a *Diagnostic wrapping one of the | ||
| // sentinels above, so errors.Is matches the kind and errors.As reads the |
There was a problem hiding this comment.
Fix in a follow-up: When the token source fails, errors.As returns the encrypt guest's generic token_get Diagnostic, not the auth Diagnostic that says why.
Impact: A program that uses the documented errors.As(err, &d) pattern on a credential failure gets the wrong detail. Examples are an expired device session or a refused access key. d.Message is host token_get failed with status 1, and d.Help is empty. The auth *Diagnostic with the real code and help (for example stack_auth::invalid_grant) is in the error chain, but errors.As never reaches it. Credential failures are a common first-run error, so many users see this case.
Evidence: encrypt/client.go:432 builds fmt.Errorf("%w (token source: %w)", err, c.transport.tokenErr). errors.As checks the operands of a multi-%w error in order and stops at the first match. err comes from diagnose, so it is already a *Diagnostic and always matches first. tokenErr comes from the token source (encrypt/transport.go:217). When the source is an auth strategy, this PR makes it a *Diagnostic too. On the guest side, HostTokenStrategy records CustomError("host token_get failed with status {status}") (encrypt/guest/src/host.rs:161).
Fix: When the token source returned a *Diagnostic, put it first, so errors.As finds the cause. Keep both errors in the chain, so errors.Is still matches both kinds. Add a test: a refused token exchange gives the auth code through errors.As on a NewClient error.
if err != nil && c.transport != nil && c.transport.tokenErr != nil {
var cause *guest.Diagnostic
if errors.As(c.transport.tokenErr, &cause) {
// The guest knows only that token_get failed; the token source knows why.
err = fmt.Errorf("token source: %w (%w)", c.transport.tokenErr, err)
} else {
err = fmt.Errorf("%w (token source: %w)", err, c.transport.tokenErr)
}
}Found by 1 model: claude
| if !errors.As(err, &d) || d.Code != "stack_auth::server_error" { | ||
| t.Fatalf("Token error = %#v, want a stack_auth::server_error Diagnostic", err) | ||
| } | ||
| if shown := fmt.Sprintf("%+v", *d); strings.Contains(shown, "nginx") || strings.Contains(shown, "testKeySecret") { |
There was a problem hiding this comment.
Fix in a follow-up: No Go test checks that the host's transport error text stays out of the auth Diagnostic.
Impact: When the RoundTripper returns an error, auth/transport.go:129 copies err.Error() into the guest. The guest puts that text in an io::Error inside RequestError (auth/guest/src/host.rs:41). A custom RoundTripper or a proxy error can put a URL with a query string, or proxy credentials, in that text. This PR now gives the guest's recorded error to Go callers. If the code that encodes that error starts to copy the io::Error message, the text reaches Diagnostic.Message or Causes. No test fails.
Evidence:
- The Rust leak test cannot run
host.rs, because that module exists only on wasm32 (auth/guest/src/lib.rs:101-104). TestAuthTransportErrorNamesTheHTTPStatusNotTheBodychecks only the body of a 403 response.TestAuthTransportErrorWithoutAResponseNamesNoStatuschecks onlyerrors.Isand that the error has no"HTTP"text.
Fix: Add the test below next to TestAuthTransportErrorNamesTheHTTPStatusNotTheBody. Its RoundTripper returns an error whose text contains a marker string. The test expects ErrTransport, a *Diagnostic, and no marker text in the error string or in any part of the Diagnostic.
The test uses roundTripFunc, which this file already has. It also uses these names, and expects them to work like this:
guestOrSkip(t)skips the test when the guest is not available.testCRNis a valid CRN for tests.Open(ctx, dir, options...)opens a profile indir.WithRoundTripper(rt)makes the profile send its HTTP requests throughrt.WithBaseURL(url)sets the auth server URL for the strategy.
If the package uses different names for these jobs, change the test to use them.
func TestAuthTransportErrorTextIsNotInTheDiagnostic(t *testing.T) {
guestOrSkip(t)
ctx := context.Background()
const marker = "leak-marker-transport"
rt := roundTripFunc(func(*http.Request) (*http.Response, error) {
return nil, errors.New(`Post "https://cts.invalid/token?secret=` + marker + `": proxyconnect tcp: refused`)
})
profile, err := Open(ctx, t.TempDir(), WithRoundTripper(rt))
if err != nil {
t.Fatal(err)
}
defer profile.Close()
strategy, err := profile.AccessKey(ctx, testCRN, "CSAKtestKeyId.testKeySecret", WithBaseURL("https://cts.invalid"))
if err != nil {
t.Fatal(err)
}
defer strategy.Close()
_, err = strategy.Token(ctx)
var d *Diagnostic
if !errors.Is(err, ErrTransport) || !errors.As(err, &d) {
t.Fatalf("Token error = %#v, want a Diagnostic over ErrTransport", err)
}
if shown := fmt.Sprintf("%v %+v", err, *d); strings.Contains(shown, marker) {
t.Fatalf("the host's transport error is in the Diagnostic: %s", shown)
}
}Found by 1 model: claude
| if _, err := profile.AccessKey(context.Background(), "invalid", "CSAKtestKeyId.testKeySecret"); !errors.Is(err, ErrConfig) { | ||
| t.Fatalf("malformed CRN for access key: error = %v, want %v", err, ErrConfig) | ||
| } else { | ||
| wantCode(t, err, "stack_auth::invalid_crn") |
There was a problem hiding this comment.
Fix in a follow-up: The malformed access key check at lines 457-460 does not check the Diagnostic code or look for the key text.
Impact: profile.Auto sends CS_CLIENT_ACCESS_KEY to the guest in the strategy config. The guest's create refuses a key that does not parse. This is the only guest call that receives an access key and fails before any HTTP request. If a later change puts the key text in that error, the text reaches Diagnostic.Message or Fields. No test fails.
Evidence:
- The Rust leak test cannot run
create, because theauthmodule exists only on wasm32 (auth/guest/src/lib.rs:101-102). So only this Go test runs the real code. - The check at lines 457-460 uses the value
"not-a-key". It checks onlyerrors.Is(err, ErrConfig). - In this PR, the malformed CRN check after it gets a
wantCodecall. The access key check does not.
Fix: Replace the check at lines 457-460 with the code below. The new code sets CS_CLIENT_ACCESS_KEY to a marker string and calls profile.Auto. It expects ErrConfig, code stack_auth::invalid_access_key, and no marker text in the error string or the Diagnostic.
Before you paste the code, compare it with lines 457-460:
- The code assigns with
err =. Iferris not declared before this point, change it toerr :=. - If the old check sets other environment variables, keep them.
The code also calls a helper wantDiagnostic(t, err, code). The helper must fail the test unless err contains a *Diagnostic with that code. It must return that *Diagnostic. If the auth tests have no helper with this name, add one next to wantCode.
const marker = "leak-marker-access-key"
t.Setenv("CS_CLIENT_ACCESS_KEY", marker)
_, err = profile.Auto(context.Background())
if !errors.Is(err, ErrConfig) {
t.Fatalf("malformed access key: error = %v, want %v", err, ErrConfig)
}
d := wantDiagnostic(t, err, "stack_auth::invalid_access_key")
if shown := fmt.Sprintf("%v %+v", err, *d); strings.Contains(shown, marker) {
t.Fatalf("the access key is in the Diagnostic: %s", shown)
}Found by 1 model: claude
| out := Buf{Ptr: uint32(res[0] >> 32), Len: uint32(res[0])} //nolint:gosec // splits the packed u64 into its two u32 halves | ||
| defer e.Free(ctx, out) | ||
| view, ok := m.Memory().Read(out.Ptr, out.Len) | ||
| if !ok { |
There was a problem hiding this comment.
Optional: No test covers an se_last_error result whose buffer is outside guest memory.
Impact: For this result, m.Memory().Read fails and diagnose returns the bare sentinel kind. TestWithoutDetailTheFailureIsTheBareSentinel has a case for each other branch in diagnose that returns the bare sentinel. It has no case for this branch. A later change could make this branch return a new error. Call already does this for an output outside memory ("guest returned an out-of-range buffer"). Then a bug in a guest's se_last_error hides the real failure kind, and no test fails. The new case also shows that the deferred Free on that buffer does not make the call trap or panic.
Evidence: The stub guest in internal/guest/diagnostic_test.go returns valid detail bytes, a zero result, or a trap. It never returns a nonzero pointer outside its one page of memory.
Fix: Add a mode to the stub guest and one case to the table. The new case expects the bare guest.ErrEncoding, with no trap and no panic.
The code below uses these names:
modeis thelastErrorModevalue thatstubGuestreceives.packedis the packed result that the stub'sse_last_errorreturns.diagnosereads the buffer pointer from the high 32 bits and the length from the low 32 bits.goodis the valid detail that the other cases in the table use.
Put the new if statement in stubGuest after the line that sets packed. The new value has pointer 1<<16 and length 16. One page of guest memory has 1<<16 bytes, so the buffer starts at the end of memory.
const (
withLastError lastErrorMode = iota
noLastError
trappingLastError
outOfRangeLastError
)
// In stubGuest, after packed is computed:
if mode == outOfRangeLastError {
packed = int64(1<<16)<<32 | 16 // starts at the end of the one-page memory
}
// In TestWithoutDetailTheFailureIsTheBareSentinel's cases:
{"a buffer past the end of guest memory", good, outOfRangeLastError},In that test, the if c.mode == withLastError check confirms that the detail is wiped. That check does not apply to the new mode, so it needs no change.
Found by 2 models: claude, codex
| if err != nil { | ||
| return fmt.Errorf("%w; %w: fetching the error's detail: %w", kind, ErrTrap, err) | ||
| } | ||
| if res[0]>>32 == 0 { |
There was a problem hiding this comment.
Optional: A guest whose se_last_error export returns no value makes diagnose panic with an index out of range.
Impact: auth.WithGuest is public, so a program can load an auth guest built from other source. newInstance accepts any export named se_last_error and checks only the name. If that export has no result, LastError.Call returns an empty slice, and res[0] panics on the first failed call. The panic stops the caller's program instead of returning the sentinel. The Diagnostic doc comment promises that missing detail never hides the failure.
Evidence: auth/guest.go:180 and encrypt/guest.go:188 assign module.ExportedFunction("se_last_error") with no type check. This line reads res[0] with no length check. An export that takes parameters already fails safely: wazero returns an error, and the code takes the ErrTrap path.
Fix: Accept the export only when its type is () -> i64. Put the check in internal/guest, so both packages use the same rule. A length check here is a second check.
// LastErrorExport is se_last_error when the module exports it with the
// ABI's type, else nil: the guest then reports the status alone.
func LastErrorExport(m api.Module) api.Function {
fn := m.ExportedFunction("se_last_error")
if fn == nil {
return nil
}
def := fn.Definition()
if len(def.ParamTypes()) != 0 || len(def.ResultTypes()) != 1 || def.ResultTypes()[0] != api.ValueTypeI64 {
return nil
}
return fn
}
// in diagnose:
if len(res) == 0 || res[0]>>32 == 0 {
return kind
}Found by 1 model: claude
| if !ok { | ||
| return nil, false | ||
| } | ||
| d := &Diagnostic{Severity: "error", Fields: map[string]any{}} |
There was a problem hiding this comment.
Optional: No test covers the defaults that decodeDiagnostic applies when the detail has only some of its keys.
Impact: decodeDiagnostic applies three defaults:
- It sets
Severityto"error"when the detail has no"severity"key. - It keeps
Fieldsas an empty map when"fields"is not an object. - It ignores a text key whose value is not a string.
The only test that checks Severity sets it to "warning". So if a change removes the "error" default, Severity is empty and no test fails. Also, no case in TestWithoutDetailTheFailureIsTheBareSentinel covers a "message" that is not a string.
Fix: Add the test below to internal/guest/diagnostic_test.go. It expects Severity equal to "error", an empty Code, an empty Fields that is not nil, and no causes.
The code uses these names, and expects them to work like this:
encode(t, value)encodes a value as detail bytes.callStub(t, status, detail, mode)runs the stub guest (the test guest in this file) with that status, detail andse_last_errormode.guest.StatusEncodingis the status that decodes toguest.ErrEncoding.
// A detail with only a message decodes with the defaults: Severity
// "error", Fields empty and not nil, and a non-string key ignored.
func TestAPartialDetailDecodesWithDefaults(t *testing.T) {
detail := encode(t, map[string]any{
"message": "failed",
"code": uint64(7),
"fields": "not an object",
})
_, err := callStub(t, guest.StatusEncoding, detail, withLastError)
var d *guest.Diagnostic
if !errors.As(err, &d) {
t.Fatalf("err = %#v, want a *Diagnostic", err)
}
if d.Severity != "error" || d.Code != "" || d.Fields == nil || len(d.Fields) != 0 || d.Causes != nil {
t.Errorf("decoded %+v", d)
}
}Also add this case to TestWithoutDetailTheFailureIsTheBareSentinel. It expects the bare guest.ErrEncoding:
{"a message that is not a string", encode(t, map[string]any{"message": uint64(1)}), withLastError},Found by 1 model: claude
4 participants
Summary
Go callers can now inspect detailed Rust runtime errors with
errors.As, while existingerrors.Ischecks continue to identify the same error kinds. Bothencryptandauthexpose a*Diagnosticcontaining a stable code, message, help, structured fields, and causes. The runtime reads these details through these_last_errorexport added in #1104 and falls back to the previous plain error when details are unavailable or invalid.This is the third of three PRs for #1098, completing the path from Rust error details to the Go API.
Changes
DiagnosticandCausetypes, exposed as aliases from bothencryptandauth.Error()returns the Rust message, andUnwrap()returns the existing sentinel error used byerrors.Is.se_last_errorafter a nonzero status, decodes the details, and wipes both the host copy and runtime buffer. The export is optional for compatibility with older runtimes.ErrTrap, so the client closes the runtime instance while preserving the original error kind.errors.Isfor the kind anderrors.Asfor details, documents allowed error contents, and removes stale comments saying only status numbers cross the boundary.Verification
The existing PR description reports the following checks. This description edit did not rerun them or check current CI status.
CGO_ENABLED=0 go test ./...fromlanguages/golangpassed against freshly built authentication and all four encryption runtimes.GOARCH=386tests passed forinternal/...andauth/...;go vet ./...andgofmt -lwere clean.golangci-lint run ./...with pinned version 2.14.0 reported 0 issues. A compatible binary was installed because the image's Go 1.25-built binary could not check this module.errors.Isassertions passed unchanged, and the README example compiled in a temporary test.8faaf44: Go lint, WebAssembly/runtime checks, macOS/Windows bindings, and Go live tests passed. The original description reported CI pending at rebased headd18bfbc; only two base dependency files changed. This is historical status, not a claim about current CI.TestLiveForeignKeysetIsRefusedBeforeRetrieval, which requires credentials. A deterministic test checks both keyset identifiers on every PR.Related
Review notes
internal/guest/diagnostic.go, then the integration ininternal/guest/call.go.: HTTP 403suffix because the Rust error does not always contain the status. Some messages therefore repeat it, such asServer error: 403: HTTP 403.Summary by CodeRabbit
New Features
Documentation