Skip to content

Commit 6f38847

Browse files
authored
Merge pull request #1110 from cipherstash/claude/gracious-einstein-ywrwim-go-diagnostic
feat(golang): every failure a guest reports is a Diagnostic that unwraps to its sentinel
2 parents 3eb459b + 58c7802 commit 6f38847

102 files changed

Lines changed: 8731 additions & 740 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.cargo/mutants.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ exclude_re = [
5959
# Production http_client variants are cfg-disabled here; the test variant
6060
# builds an unconfigured Client, equivalent to Client::default().
6161
'stack-auth/src/transport\.rs:\d+:\d+: replace http_client -> reqwest::Client with Default::default\(\)$',
62+
# Under `http` the body is `false`; the no-`http` arm is pinned by
63+
# `a_builder_without_a_transport_is_refused_when_there_is_no_bundled_one`.
64+
'stack-auth/src/error\.rs:\d+:\d+: replace RequestError::is_no_transport -> bool with false$',
6265
'stack-auth/src/auto_strategy\.rs:150:9: replace AutoStrategy::detect_inner -> Result<Self, AuthError> with Ok\(Default::default\(\)\)$',
6366
'stack-auth/src/token_store\.rs:(258|266):9: replace <impl TokenStore for TokenStoreFn<L, S>>::(load|save)',
6467
# stack-auth — equivalent.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@cipherstash/auth": patch
3+
---
4+
5+
Auth failures carry more help, and their messages never quote a credential or another library's text. A failure's `type` (`NOT_AUTHENTICATED`, `INVALID_CRN`, ...) is unchanged.
6+
7+
- `REQUEST_ERROR`'s message no longer repeats the transport's own error, which can carry a URL with its query string. It gains `help` saying what to check.
8+
- `INVALID_TOKEN` for a token whose claims do not decode no longer quotes the decoder's message, which could carry a byte or a claim of the token.
9+
- A failed device binding reports ZeroKMS's status, not its response body.
10+
- `SERVER_ERROR` for a refused token exchange names the HTTP status and the auth server's `error_description`, not the response body, which from the edge in front of it is an HTML page and can echo the access key. A body that is not JSON is reported by where it broke, not by the parser's message.
11+
- A profile file that is not valid JSON is reported by error kind, line and column, not by the parser's message, which could quote the file.
12+
- `INVALID_GRANT`, `INVALID_WORKSPACE_ID` and `ALREADY_CONSUMED` gain `help`, and `NOT_AUTHENTICATED`'s help names `stash auth login`.
13+
- A `STORE_ERROR` carries the help of the profile failure underneath it, such as logging in again when the profile file is missing.

‎.github/workflows/tests-crates.yml‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -141,16 +141,20 @@ jobs:
141141
- name: Build the stack-encrypt examples
142142
run: cargo build --locked -p stack-encrypt --examples
143143

144-
# The crates release-plz publishes from 0.1.0. `--dry-run` packages and
145-
# compiles each the way crates.io would (the three together, so
146-
# stack-encrypt resolves the other two from cargo's temporary local
147-
# registry), catching a publish-blocker on the PR instead of in
148-
# release-crates on main: missing metadata, or a runtime path dependency
149-
# with no `version`. No token. `--allow-dirty` tolerates files earlier
150-
# steps leave in the tree. Once a version is on crates.io, cargo warns
151-
# rather than fails on a dry run of it.
152-
- name: Verify the stack-encrypt crates package cleanly for crates.io
153-
run: cargo publish --locked --dry-run --allow-dirty -p stack-kms -p stack-encrypt-derive -p stack-encrypt
144+
# The crates release-plz publishes. `--dry-run` packages and compiles
145+
# each the way crates.io would (all five together, so each resolves the
146+
# others from cargo's temporary local registry rather than from
147+
# crates.io), catching a publish-blocker on the PR instead of in
148+
# release-crates on main: missing metadata, a runtime path dependency
149+
# with no `version`, or a crate that uses API its dependency's packaged
150+
# source does not have yet. stack-profile and stack-auth are in the list
151+
# because stack-kms and stack-encrypt depend on them: without them, cargo
152+
# resolves the published versions, which lag the tree. No token.
153+
# `--allow-dirty` tolerates files earlier steps leave in the tree. Once a
154+
# version is on crates.io, cargo warns rather than fails on a dry run of
155+
# it.
156+
- name: Verify the stack-* crates package cleanly for crates.io
157+
run: cargo publish --locked --dry-run --allow-dirty -p stack-profile -p stack-auth -p stack-kms -p stack-encrypt-derive -p stack-encrypt
154158

155159
node-bindings:
156160
name: node bindings, napi typings, stack-auth-wasm

‎Cargo.lock‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/plans/2026-10-04-plan-builder.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,9 +1122,10 @@ uses the guest.
11221122
kinds, not new names. A wire-format addition, so it goes in the first
11231123
engine PR. This is the one gap in the plan approach itself that the
11241124
language check found. #1069 added the type to the grammar as an optional
1125-
key, so a field with none is still dispatched on each value's own tag;
1126-
that is transitional, until Go fills the type (#1046) and the key becomes
1127-
required on indexed fields.
1125+
key; the Go SDK fills it on every field from the Go type (#1094), and
1126+
since #1082 the key is required on every field with a term output: a plan
1127+
whose indexed field has none is refused when it is built. A field that
1128+
only seals, or only carries its value through, may still leave it out.
11281129
2. **The guest's synchronous transport import, on the edge path only.**
11291130
`transport_send` is synchronous from the guest's point of view and the ABI
11301131
relies on it ("`block_on` never parks"). A wazero host function may block a
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package auth
2+
3+
import (
4+
"context"
5+
"errors"
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
)
10+
11+
// wantDiagnostic asserts that err carries a *Diagnostic with code and a
12+
// message, and returns it.
13+
func wantDiagnostic(t *testing.T, err error, code string) *Diagnostic {
14+
t.Helper()
15+
var d *Diagnostic
16+
if !errors.As(err, &d) {
17+
t.Fatalf("%v carries no Diagnostic", err)
18+
}
19+
if d.Code != code {
20+
t.Fatalf("code = %q, want %q (%v)", d.Code, code, err)
21+
}
22+
if d.Message == "" {
23+
t.Fatalf("%s has no message", code)
24+
}
25+
return d
26+
}
27+
28+
// wantCode is wantDiagnostic for a test that needs only the code.
29+
func wantCode(t *testing.T, err error, code string) {
30+
t.Helper()
31+
_ = wantDiagnostic(t, err, code)
32+
}
33+
34+
// Each kind the profile store reports is its sentinel for errors.Is and a
35+
// Diagnostic for errors.As. The strategies' kinds are asserted beside the
36+
// tests that provoke them, in strategy_test.go. ErrInvalidFilename is not
37+
// here: the store refuses every filename the guest would before asking it.
38+
func TestEachStoreKindCarriesItsDiagnostic(t *testing.T) {
39+
ctx := context.Background()
40+
dir, s := profile(t)
41+
ws, err := s.WorkspaceStore(ctx, wsB)
42+
if err != nil {
43+
t.Fatal(err)
44+
}
45+
cases := []struct {
46+
name string
47+
run func() error
48+
want error
49+
code string
50+
}{
51+
{"no current workspace", func() error {
52+
_, err := s.CurrentWorkspace(ctx)
53+
return err
54+
}, ErrNoCurrentWorkspace, "stack_profile::no_current_workspace"},
55+
{"a workspace with no directory", func() error {
56+
return s.SetCurrentWorkspace(ctx, "CCCCCCCCCCCCCCCC")
57+
}, ErrWorkspaceNotFound, "stack_profile::workspace_not_found"},
58+
{"a workspace id that is a path", func() error {
59+
return s.SetCurrentWorkspace(ctx, "../escape")
60+
}, ErrInvalidWorkspaceID, "stack_profile::invalid_workspace_id"},
61+
{"a file that is not there", func() error {
62+
_, err := ws.Token(ctx)
63+
return err
64+
}, ErrNotFound, "stack_profile::not_found"},
65+
{"a file that is not JSON", func() error {
66+
write(t, filepath.Join(dir, "workspaces", wsB, "auth.json"), "{not json")
67+
_, err := ws.Token(ctx)
68+
return err
69+
}, ErrInvalid, "stack_profile::json"},
70+
{"a file that is a directory", func() error {
71+
if err := os.MkdirAll(filepath.Join(dir, "workspaces", wsB, "secretkey.json"), 0o700); err != nil {
72+
t.Fatal(err)
73+
}
74+
_, _, err := ws.SecretKey(ctx)
75+
return err
76+
}, ErrIO, "stack_profile::io"},
77+
}
78+
for _, tc := range cases {
79+
t.Run(tc.name, func(t *testing.T) {
80+
err := tc.run()
81+
if !errors.Is(err, tc.want) {
82+
t.Fatalf("%v, want %v", err, tc.want)
83+
}
84+
wantCode(t, err, tc.code)
85+
})
86+
}
87+
}
88+
89+
// A profile file that is not JSON says where: the line and column, never
90+
// the parser's message, which could quote the file.
91+
func TestAProfileJSONErrorNamesTheLine(t *testing.T) {
92+
ctx := context.Background()
93+
dir, s := profile(t)
94+
write(t, filepath.Join(dir, "workspaces", wsB, "auth.json"), "{\n \"access_token\": 7\n}")
95+
ws, err := s.WorkspaceStore(ctx, wsB)
96+
if err != nil {
97+
t.Fatal(err)
98+
}
99+
_, err = ws.Token(ctx)
100+
d := wantDiagnostic(t, err, "stack_profile::json")
101+
if d.Fields["line"] != uint64(2) {
102+
t.Errorf("fields = %v, want line 2", d.Fields)
103+
}
104+
if d.Help == "" {
105+
t.Error("a JSON error gives no help")
106+
}
107+
}

‎languages/golang/auth/errors.go‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import (
99
// Failure kinds the profile reports. The guest reports a status code from
1010
// the one table every guest shares, so these are the sentinels of the
1111
// shared decoder exposed under this package's names; an error from
12-
// encrypt of the same kind is the same value.
12+
// encrypt of the same kind is the same value. Check one with errors.Is;
13+
// the detail behind it is a [Diagnostic].
1314
var (
1415
// ErrNotFound is a profile file that does not exist in the store asked:
1516
// no secretkey.json, auth.json or device.json there. For the workspace's
@@ -62,3 +63,36 @@ var (
6263
// logged in on this machine, or CS_CONFIG_PATH names the wrong place.
6364
ErrNoProfile = errors.New("auth: no profile directory; run `stash auth login`")
6465
)
66+
67+
// Diagnostic is the full error behind a failure the guest reports, beside
68+
// the kind: every such failure is a *Diagnostic wrapping one of the
69+
// sentinels above, so errors.Is matches the kind and errors.As reads the
70+
// rest:
71+
//
72+
// var d *auth.Diagnostic
73+
// if errors.As(err, &d) {
74+
// log.Printf("%s: %s (%s)", d.Code, d.Message, d.Help)
75+
// }
76+
//
77+
// Its fields are Code ("stack_profile::not_found", "stack_auth::invalid_crn",
78+
// ...; stable), Message (what Error returns after the sentinel's text), Help,
79+
// URL, Severity, Fields (the structured fields, by name: a profile file's
80+
// "path", a JSON error's "line" and "column") and Causes (the errors behind
81+
// it, outermost first).
82+
//
83+
// What one may carry is fixed: workspace ids, CRNs and regions, profile
84+
// file paths, HTTP statuses and the auth server's error descriptions.
85+
// Never a token, an access key, a client key or a response body.
86+
//
87+
// Errors the store raises itself carry none: a closed store ([ErrState]),
88+
// a guest that did not return, [ErrMemoryLock], [ErrNoProfile]. A guest
89+
// built before the detail existed returns the bare sentinel too.
90+
//
91+
// It is the same type as encrypt.Diagnostic, by identity.
92+
type Diagnostic = guest.Diagnostic
93+
94+
// Cause is one error in a [Diagnostic]'s cause chain: a Code and a Message.
95+
// A cause from a library outside the stack crates has no Code, and its
96+
// Message is a description the guest vouches for, never that library's own
97+
// text.
98+
type Cause = guest.Cause

‎languages/golang/auth/guest.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,9 @@ func newInstance(ctx context.Context, wasm []byte, hostDir string, policy guest.
175175
return fail(fmt.Errorf("auth: guest is missing export %s", name))
176176
}
177177
}
178+
// Optional: a guest built before se_last_error reports the status
179+
// alone, and its failures stay the bare sentinels. See guest.Diagnostic.
180+
inst.exports.LastError = guest.LastErrorExport(module)
178181
return inst, nil
179182
}
180183

‎languages/golang/auth/guest/Cargo.lock‎

Lines changed: 6 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎languages/golang/auth/guest/src/abi.rs‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@
2525
2626
use std::panic::{catch_unwind, AssertUnwindSafe};
2727

28-
use stack_guest_abi::abi::{err_status, input, ok_buffer};
29-
use stack_guest_abi::buffers;
30-
use stack_guest_abi::status::STATUS_INTERNAL;
28+
use stack_guest_abi::abi::{export, input};
29+
use stack_guest_abi::{buffers, last_error};
3130

3231
use crate::{auth, ops};
3332

@@ -42,27 +41,25 @@ type Op1 = fn(&[u8]) -> Result<Vec<u8>, u32>;
4241
type Op2 = fn(&[u8], &[u8]) -> Result<Vec<u8>, u32>;
4342

4443
/// Run a two-input operation as an export: validate both pairs, run,
45-
/// pack. A panic is `STATUS_INTERNAL` (wasm32-wasip1 aborts on panic; the
46-
/// catch is belt-and-braces for an unwinding build).
44+
/// pack, through the shared [`export`] wrapper — which clears the last
45+
/// error first, records one for any failure, and makes a panic
46+
/// `STATUS_INTERNAL` (wasm32-wasip1 aborts on panic; the catch is
47+
/// belt-and-braces for an unwinding build).
4748
fn export2(a_ptr: *const u8, a_len: u32, b_ptr: *const u8, b_len: u32, op: Op2) -> u64 {
48-
catch_unwind(AssertUnwindSafe(|| {
49+
export(|| {
4950
// SAFETY: host-owned ranges the export was handed; the borrows end
5051
// when `op` returns, inside the call, and nothing here writes to
5152
// linear memory while they are live.
5253
let a = unsafe { input(a_ptr, a_len)? };
5354
let b = unsafe { input(b_ptr, b_len)? };
5455
op(a, b)
55-
}))
56-
.unwrap_or(Err(STATUS_INTERNAL))
57-
.map_or_else(err_status, ok_buffer)
56+
})
5857
}
5958

6059
/// [`export2`] for a one-input operation.
6160
fn export1(a_ptr: *const u8, a_len: u32, op: Op1) -> u64 {
6261
// SAFETY: as in `export2`.
63-
catch_unwind(AssertUnwindSafe(|| op(unsafe { input(a_ptr, a_len)? })))
64-
.unwrap_or(Err(STATUS_INTERNAL))
65-
.map_or_else(err_status, ok_buffer)
62+
export(|| op(unsafe { input(a_ptr, a_len)? }))
6663
}
6764

6865
/// The current workspace id of the store at `dir`. See
@@ -241,5 +238,6 @@ pub unsafe extern "C" fn sa_auth_free(ptr: *const u8, len: u32) -> u64 {
241238
#[no_mangle]
242239
pub extern "C" fn sa_shutdown() {
243240
let _ = catch_unwind(AssertUnwindSafe(auth::clear));
241+
let _ = catch_unwind(AssertUnwindSafe(last_error::clear));
244242
let _ = catch_unwind(AssertUnwindSafe(buffers::wipe_all));
245243
}

0 commit comments

Comments
 (0)