feat(stack-encrypt)!: codes, help and fields on every error in stack-encrypt, stack-kms, stack-auth and stack-profile

.cargo/mutants.toml

@@ -59,6 +59,9 @@ exclude_re = [
   # Production http_client variants are cfg-disabled here; the test variant
   # builds an unconfigured Client, equivalent to Client::default().
   'stack-auth/src/transport\.rs:\d+:\d+: replace http_client -> reqwest::Client with Default::default\(\)$',
+  # Under `http` the body is `false`; the no-`http` arm is pinned by
+  # `a_builder_without_a_transport_is_refused_when_there_is_no_bundled_one`.
+  'stack-auth/src/error\.rs:\d+:\d+: replace RequestError::is_no_transport -> bool with false$',
   'stack-auth/src/auto_strategy\.rs:150:9: replace AutoStrategy::detect_inner -> Result<Self, AuthError> with Ok\(Default::default\(\)\)$',
   'stack-auth/src/token_store\.rs:(258|266):9: replace <impl TokenStore for TokenStoreFn<L, S>>::(load|save)',
   # stack-auth — equivalent.

.changeset/auth-error-codes-and-help.md

@@ -0,0 +1,13 @@
+---
+"@cipherstash/auth": patch
+---
+
+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.
+
+- `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.
+- `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.
+- A failed device binding reports ZeroKMS's status, not its response body.
+- `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.
+- 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.
+- `INVALID_GRANT`, `INVALID_WORKSPACE_ID` and `ALREADY_CONSUMED` gain `help`, and `NOT_AUTHENTICATED`'s help names `stash auth login`.
+- 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

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

languages/golang/encrypt/guest/src/status.rs

@@ -116,12 +116,12 @@ pub fn status_for_error(error: &stack_encrypt::Error) -> u32 {
 pub fn status_for_dynamic(error: &stack_encrypt::dynamic::Error) -> u32 {
     use stack_encrypt::dynamic::Error;
     match error {
-        Error::Context
+        Error::Context { .. }
         | Error::Term { .. }
-        | Error::Plan
+        | Error::Plan { .. }
         | Error::UntypedIndex { .. }
-        | Error::Source
-        | Error::Record => STATUS_ENCODING,
+        | Error::Source { .. }
+        | Error::Record { .. } => STATUS_ENCODING,
         Error::Cipher(e) => status_for_error(e),
         // A target refusal is a statement about the plan, the label or the
         // value (an unknown or unproducible type, an extended plan, a value
@@ -433,26 +433,52 @@ mod tests {
 
     #[test]
     fn dynamic_input_errors_are_encoding_and_a_library_bug_is_internal() {
-        use stack_encrypt::dynamic::Error;
+        use stack_encrypt::dynamic::{Error, Reason};
         use stack_encrypt::sem::MatchOptions;
         use stack_encrypt::target::IndexSpec;
+        let age = || Some("age".to_string());
         for (label, err) in [
-            ("a bad context", Error::Context),
+            (
+                "a bad context",
+                Error::Context {
+                    field: None,
+                    reason: Reason::EmptyContext,
+                },
+            ),
             (
                 "a bad term request",
                 Error::Term {
+                    field: age(),
                     kind: IndexSpec::Match(MatchOptions::default()),
                 },
             ),
-            ("a bad plan", Error::Plan),
+            (
+                "a bad plan",
+                Error::Plan {
+                    field: age(),
+                    reason: Reason::DuplicateOutput,
+                },
+            ),
             (
                 "an indexed field with no type",
                 Error::UntypedIndex {
                     field: "age".to_string(),
                 },
             ),
-            ("a bad source", Error::Source),
-            ("a bad record", Error::Record),
+            (
+                "a bad source",
+                Error::Source {
+                    field: age(),
+                    reason: Reason::FieldMissing,
+                },
+            ),
+            (
+                "a bad record",
+                Error::Record {
+                    field: age(),
+                    reason: Reason::NoCiphertextNode,
+                },
+            ),
         ] {
             assert_eq!(
                 status_for_dynamic(&err),

languages/golang/encrypt/guest/src/targets.rs

@@ -134,10 +134,12 @@ fn convert(error: eql_bindings::encryption::targets::TargetError) -> TargetError
             expected: Some(expected),
             found,
         },
+        // The parser's kind and position, never its message: serde_json
+        // quotes the input it refused, and the input is stored ciphertext.
         Eql::Stored { target, source } => TargetError::Stored {
             name: String::new(),
             target: target.to_owned(),
-            reason: source.to_string(),
+            reason: stack_encrypt::diagnostic::describe_json_error(&source),
         },
         other => TargetError::Other(Box::new(other)),
     }
@@ -207,4 +209,23 @@ mod tests {
             TargetError::Plaintext { target, expected: Some(vitaminc_aead_value::ValueKind::String), .. } if target == "TextEq"
         ));
     }
+
+    /// serde_json quotes the value it refused, and a stored EQL value holds
+    /// ciphertext and index terms: only its kind and position cross.
+    #[cfg(feature = "eql")]
+    #[test]
+    fn a_stored_value_that_does_not_parse_quotes_none_of_it() {
+        use eql_bindings::encryption::targets::TargetError as Eql;
+        let source = serde_json::from_str::<u8>(r#""marker-ciphertext""#).unwrap_err();
+        let error = convert(Eql::Stored {
+            target: "TextEq",
+            source,
+        });
+        let TargetError::Stored { reason, .. } = &error else {
+            panic!("{error:?}");
+        };
+        assert_eq!(reason, "unexpected data at line 1 column 19");
+        let shown = format!("{error} {:?}", stack_encrypt::ErrorPayload::payload(&error));
+        assert!(!shown.contains("marker"), "{shown}");
+    }
 }

languages/typescript/packages/auth/src/lib.rs

@@ -954,9 +954,9 @@ mod tests {
         // `device_client_to_napi_error` routes every `DeviceClientError` through
         // its canonical `AuthError` mapping. A help-carrying error (here an
         // `Auth`-wrapped `WorkspaceMismatch`) must keep its help + structured
-        // payload; a help-less one (`Profile` -> `Store`) yields just
-        // type + message. A regression that dropped the canonical routing would
-        // lose the help/payload here.
+        // payload; a `Profile` -> `Store` one carries the profile error's
+        // help. A regression that dropped the canonical routing would lose
+        // the help/payload here.
         #[test]
         fn device_client_auth_arm_preserves_full_envelope() {
             let ws = |s: &str| s.parse::<cts_common::WorkspaceId>().unwrap();
@@ -976,15 +976,20 @@ mod tests {
             );
 
             // Non-Auth variant routes through `From<DeviceClientError>` to the
-            // canonical `Store` error: same `STORE_ERROR` code, and no help
-            // (StoreError carries none).
+            // canonical `Store` error: same `STORE_ERROR` code, and the
+            // profile error's help, which `StoreError` forwards.
             let err = device_client_to_napi_error(DeviceClientError::Profile(
                 stack_profile::ProfileError::HomeDirNotFound,
             ));
             let json = assertions::failure_json(&err);
             assert_eq!(json["type"], "STORE_ERROR");
             assert!(json["message"].as_str().is_some());
-            assert!(json.get("help").is_none());
+            assert!(
+                json["help"]
+                    .as_str()
+                    .is_some_and(|help| help.contains("HOME")),
+                "a store failure carries the profile error's help, got: {json}"
+            );
         }
     }
 

packages/stack-auth/src/access_key.rs

@@ -67,22 +67,36 @@ impl FromStr for AccessKey {
 }
 
 /// Error returned when parsing an invalid access key string.
-#[derive(Debug, thiserror::Error)]
+///
+/// No variant quotes the string it refused: an access key is a credential.
+#[derive(Debug, thiserror::Error, miette::Diagnostic)]
 pub enum InvalidAccessKey {
     /// The string does not start with the `CSAK` prefix.
     #[error("access key must start with \"{ACCESS_KEY_PREFIX}\"")]
+    #[diagnostic(
+        code(stack_auth::access_key_missing_prefix),
+        help("Access keys have the form `CSAK<key-id>.<secret>`.")
+    )]
     MissingPrefix,
     /// No `.` separator found between key ID and secret.
     #[error("access key must contain a \".\" separator")]
+    #[diagnostic(
+        code(stack_auth::access_key_missing_dot),
+        help("Access keys have the form `CSAK<key-id>.<secret>`.")
+    )]
     MissingDot,
     /// The key ID portion (before the `.`) is empty.
     #[error("access key ID must not be empty")]
+    #[diagnostic(code(stack_auth::access_key_empty_id))]
     EmptyKeyId,
     /// The secret portion (after the `.`) is empty.
     #[error("access key secret must not be empty")]
+    #[diagnostic(code(stack_auth::access_key_empty_secret))]
     EmptySecret,
 }
 
+impl stack_profile::ErrorPayload for InvalidAccessKey {}
+
 #[cfg(test)]
 mod tests {
     use super::*;

packages/stack-auth/src/access_key_refresher.rs

@@ -70,9 +70,9 @@ impl Refresher for AccessKeyRefresher {
             if let Some(err) = crate::error::classify_issuance_failure(status, &body) {
                 return Err(err);
             }
-            return Err(AuthError::Server(crate::error::ServerError(format!(
-                "{status}: {body}"
-            ))));
+            return Err(AuthError::Server(crate::error::ServerError::refused(
+                status, &body,
+            )));
         }
 
         let auth_resp: AuthoriseResponse = resp.json()?;