From aa36e884bb00b3cdae5914c96d0a1eb1e7828852 Mon Sep 17 00:00:00 2001
From: Stephen Rosenthal <stephen.rosenthal@datadoghq.com>
Date: Fri, 8 May 2026 12:18:07 -0700
Subject: [PATCH] feat(auth): sort scopes everywhere they're displayed to the
 user

Scopes arrive from the OAuth issuer in non-deterministic order, which
makes pup's various scope-display surfaces hard to scan and unstable
between invocations. Sort them at every point we render to a user:

- `pup auth login` pre-launch print: alphabetised before the joined
  display line.
- `pup auth list` JSON: scopes array now sorted.
- `pup auth status` JSON: same.
- `dcr::build_authorization_url`: the printed authorize URL's `scope=`
  parameter is sorted too. OAuth treats scope as an unordered set so
  this is a no-op for the issuer; for users it makes the URL stable
  enough to grep/diff across runs.

A small `sorted_scopes(&str) -> Vec<&str>` helper de-duplicates the
parse-and-sort step shared by list and status. The schema of the JSON
output is unchanged (still an array of strings); only the order
changes.
---
 src/auth/dcr.rs      |  8 +++++++-
 src/commands/auth.rs | 43 +++++++++++++++++++++++++++++++------------
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/auth/dcr.rs b/src/auth/dcr.rs
index 1c66a47..eaaf2d5 100644
--- a/src/auth/dcr.rs
+++ b/src/auth/dcr.rs
@@ -199,7 +199,13 @@ impl DcrClient {
         scopes: &[&str],
         subdomain: Option<&str>,
     ) -> String {
-        let scope = scopes.join(" ");
+        // Sort scopes so the printed authorize URL has a deterministic
+        // `scope=` parameter order — easier to diff and grep across runs.
+        // OAuth treats `scope` as an unordered set, so this is a no-op for
+        // the issuer.
+        let mut sorted_scopes: Vec<&str> = scopes.to_vec();
+        sorted_scopes.sort();
+        let scope = sorted_scopes.join(" ");
         let params = url::form_urlencoded::Serializer::new(String::new())
             .append_pair("response_type", "code")
             .append_pair("client_id", client_id)
diff --git a/src/commands/auth.rs b/src/commands/auth.rs
index 15116ff..6c029fc 100644
--- a/src/commands/auth.rs
+++ b/src/commands/auth.rs
@@ -14,6 +14,16 @@ where
     f(&mut **store)
 }
 
+/// Parse a token's space-delimited scope claim and return scopes sorted
+/// alphabetically. The OAuth issuer returns scopes in non-deterministic
+/// order; sorting makes `pup auth list`/`pup auth status` output stable
+/// and easier to scan or diff between invocations.
+fn sorted_scopes(scope_claim: &str) -> Vec<&str> {
+    let mut scopes: Vec<&str> = scope_claim.split_whitespace().collect();
+    scopes.sort();
+    scopes
+}
+
 #[cfg(not(target_arch = "wasm32"))]
 pub async fn login(
     cfg: &Config,
@@ -48,10 +58,12 @@ pub async fn login(
             scopes.len()
         );
     } else {
+        let mut display_scopes = scopes.clone();
+        display_scopes.sort();
         eprintln!(
             "🔑 Requesting {} scope(s): {}",
             scopes.len(),
-            scopes.join(", ")
+            display_scopes.join(", ")
         );
     }
 
@@ -237,11 +249,7 @@ pub fn status(cfg: &Config) -> Result<()> {
                     .map(|dt| dt.with_timezone(&chrono::Local).to_rfc3339())
                     .unwrap_or_default();
 
-                let scopes: Vec<&str> = tokens
-                    .scope
-                    .split_whitespace()
-                    .filter(|s| !s.is_empty())
-                    .collect();
+                let scopes = sorted_scopes(&tokens.scope);
 
                 let json = serde_json::json!({
                     "authenticated": true,
@@ -361,16 +369,11 @@ pub fn list(cfg: &Config) -> Result<()> {
                     let expires_at = chrono::DateTime::from_timestamp(expires_at_ts, 0)
                         .map(|dt| dt.with_timezone(&chrono::Local).to_rfc3339())
                         .unwrap_or_default();
-                    let scopes: Vec<&str> = t
-                        .scope
-                        .split_whitespace()
-                        .filter(|s| !s.is_empty())
-                        .collect();
                     serde_json::json!({
                         "expires_at": expires_at,
                         "has_refresh": !t.refresh_token.is_empty(),
                         "org": s.org,
-                        "scopes": scopes,
+                        "scopes": sorted_scopes(&t.scope),
                         "site": s.site,
                         "status": status,
                     })
@@ -442,6 +445,22 @@ mod tests {
         }
     }
 
+    // ------------------------------------------------------------------
+    // sorted_scopes — pure helper, no env / storage state.
+    // ------------------------------------------------------------------
+
+    #[test]
+    fn sorted_scopes_alphabetises_the_claim() {
+        let got = sorted_scopes("monitors_read apm_read dashboards_read");
+        assert_eq!(got, vec!["apm_read", "dashboards_read", "monitors_read"]);
+    }
+
+    #[test]
+    fn sorted_scopes_handles_empty_and_whitespace_input() {
+        assert!(sorted_scopes("").is_empty());
+        assert!(sorted_scopes("   ").is_empty());
+    }
+
     // ------------------------------------------------------------------
     // token() — the one function with an access_token bypass that never
     // touches the global STORAGE singleton. Hermetic.