fix(stack-auth): a refused exchange's ServerError names the status, never the body

The Go binding's test for a 403 from the edge in front of the auth server
showed the access-key and OIDC refreshers putting the whole response body
into ServerError's message: "Server error: 403: <html>...nginx
CSAK...</html>". A body from the edge is an HTML page, and any body may
echo the credential the request carried, so under the rule on
ErrorPayload it never belongs in a message. With se_last_error that
message now crosses into every binding.

ServerError::refused builds the message from the status and, when the
body is the auth server's JSON error, its error_description, which the
rule allows. ServerError::unparseable replaces serde_json's message, which
can quote the body, with where the JSON broke. The refresh-lock join
failure no longer repeats tokio's error text. The body is still logged at
debug level where it was before.

The test that asserted the body was in the message now asserts it is not.

Refs #1099

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URtfKsTToFUCRwq3g7gCUf

Author: claude

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

@@ -7,6 +7,7 @@ Auth failures carry more help, and their messages never quote a credential or an
 - `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.

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()?;

packages/stack-auth/src/device_code/mod.rs

@@ -394,9 +394,7 @@ impl PendingDeviceCode {
             }
 
             let err: ErrorResponse = serde_json::from_str(&body).map_err(|e| {
-                AuthError::Server(crate::error::ServerError(format!(
-                    "{status}: unparseable error body: {e}"
-                )))
+                AuthError::Server(crate::error::ServerError::unparseable(status, &e))
             })?;
             match err.error.as_str() {
                 "authorization_pending" => {

packages/stack-auth/src/device_session_refresher.rs

@@ -126,10 +126,12 @@ impl DeviceSessionRefresher {
         };
         let lock = tokio::task::spawn_blocking(move || store.lock_exclusive(Token::FILENAME))
             .await
-            .map_err(|e| {
-                AuthError::Server(crate::error::ServerError(format!(
-                    "refresh lock task join failed: {e}"
-                )))
+            // A join error is tokio's: it says only that the task panicked
+            // or was cancelled, and is not ours to repeat.
+            .map_err(|_| {
+                AuthError::Server(crate::error::ServerError(
+                    "refresh lock task join failed".to_owned(),
+                ))
             })?
             .map_err(|e| {
                 AuthError::Server(crate::error::ServerError(format!(

packages/stack-auth/src/error.rs

@@ -372,6 +372,12 @@ impl AuthErrorKind for UsageLimitExceeded {
 }
 
 /// An unexpected error was returned by the auth server.
+///
+/// The message the crate builds names the HTTP status and, where the auth
+/// server gave one, its `error_description`: never the response body. From
+/// the edge in front of the auth server a body is an HTML page, and any
+/// body may echo the credential the request carried (see
+/// [`ErrorPayload`] for the rule).
 #[derive(Debug, thiserror::Error, miette::Diagnostic)]
 #[error("Server error: {0}")]
 #[diagnostic(code(stack_auth::server_error))]
@@ -382,6 +388,38 @@ impl AuthErrorKind for ServerError {
     }
 }
 
+impl ServerError {
+    /// A refused exchange no classifier had anything more specific for: the
+    /// status, and the auth server's `error_description` when the body is
+    /// its JSON error.
+    pub(crate) fn refused(status: u16, body: &str) -> Self {
+        let description = serde_json::from_str::<serde_json::Value>(body)
+            .ok()
+            .and_then(|value| {
+                value
+                    .get("error_description")?
+                    .as_str()
+                    .map(str::trim)
+                    .filter(|text| !text.is_empty())
+                    .map(str::to_owned)
+            });
+        match description {
+            Some(description) => Self(format!("{status}: {description}")),
+            None => Self(status.to_string()),
+        }
+    }
+
+    /// An error body that is not the JSON the endpoint answers with: the
+    /// status, and where the JSON broke. serde_json's own message can quote
+    /// the body, so it is not used.
+    pub(crate) fn unparseable(status: u16, error: &serde_json::Error) -> Self {
+        Self(format!(
+            "{status}: unparseable error body ({})",
+            stack_profile::diagnostic::describe_json_error(error)
+        ))
+    }
+}
+
 /// A consumable handle (e.g. a device-code poll) was used after it had already
 /// been consumed. A caller bug rather than an auth outcome, but surfaced as an
 /// `AuthError` so it flows through the `Result` contract rather than throwing

packages/stack-auth/src/oidc_refresher.rs

@@ -189,9 +189,9 @@ impl OidcFederation {
             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()?;

packages/stack-auth/src/token.rs

@@ -313,9 +313,7 @@ impl Token {
             }
 
             let err: RefreshErrorResponse = serde_json::from_str(&body).map_err(|e| {
-                AuthError::Server(crate::error::ServerError(format!(
-                    "{status}: unparseable error body: {e}"
-                )))
+                AuthError::Server(crate::error::ServerError::unparseable(status, &e))
             })?;
 
             return Err(match err.error.as_str() {

packages/stack-auth/src/transport.rs

@@ -882,22 +882,58 @@ mod tests {
         assert!(matches!(err, AuthError::UsageLimitExceeded(_)), "{err:?}");
     }
 
+    /// An unclassified failure names its status and never carries the
+    /// body: from the edge it is an HTML page, and it may echo the
+    /// credential. The auth server's own error description is kept.
     #[tokio::test]
-    async fn an_unclassified_failure_is_a_server_error_with_the_body() {
-        let transport: SharedTransport = Arc::new(Stub::replying(500, "boom"));
-        let refresher = AccessKeyRefresher::new(
-            SecretToken::new("CSAKid.secret"),
-            base_url(),
+    async fn an_unclassified_failure_is_a_server_error_without_the_body() {
+        const PAGE: &str = "<html><h1>403 Forbidden</h1>nginx CSAKmarker.secret</html>";
+        let refused = |body: &'static str| {
+            let transport: SharedTransport = Arc::new(Stub::replying(403, body));
+            AccessKeyRefresher::new(
+                SecretToken::new("CSAKmarker.secret"),
+                base_url(),
+                None,
+                transport,
+            )
+        };
+        let err = refused(PAGE).refresh(&()).await.unwrap_err();
+        assert!(matches!(err, AuthError::Server(_)), "{err:?}");
+        assert_eq!(err.to_string(), "Server error: 403");
+
+        let transport: SharedTransport = Arc::new(Stub::replying(403, PAGE));
+        let err = OidcFederation::new(workspace_id(), base_url(), transport)
+            .federate(&SecretToken::new("h.p.s"))
+            .await
+            .unwrap_err();
+        assert_eq!(err.to_string(), "Server error: 403");
+
+        let described = r#"{"error":"forbidden","error_description":"client is disabled"}"#;
+        let err = refused(described).refresh(&()).await.unwrap_err();
+        assert_eq!(err.to_string(), "Server error: 403: client is disabled");
+    }
+
+    /// An error body that is not JSON says where it broke, not what it held.
+    #[tokio::test]
+    async fn an_unparseable_error_body_is_not_quoted() {
+        let transport: SharedTransport =
+            Arc::new(Stub::replying(400, r#"{"error": "marker-rt-echo"#));
+        let err = Token::refresh_with(
+            &transport,
+            &SecretToken::new("rt"),
+            &base_url(),
+            "cli",
             None,
-            transport,
+        )
+        .await
+        .unwrap_err();
+        let shown = err.to_string();
+        assert!(matches!(err, AuthError::Server(_)), "{err:?}");
+        assert!(
+            shown.starts_with("Server error: 400: unparseable error body"),
+            "{shown}"
         );
-        let err = refresher.refresh(&()).await.unwrap_err();
-        match err {
-            AuthError::Server(e) => {
-                assert!(e.to_string().contains("500") && e.to_string().contains("boom"))
-            }
-            other => panic!("{other:?}"),
-        }
+        assert!(!shown.contains("marker"), "{shown}");
     }
 
     #[tokio::test]