From c21c60b7a1ee95925d0fd596ebc39c250ebe91d7 Mon Sep 17 00:00:00 2001
From: KeyCode17 <m.daffa.karyudi@gmail.com>
Date: Mon, 18 May 2026 05:55:13 +0700
Subject: [PATCH] refactor(auth): make allowlist list() order deterministic

Use BTreeMap so YamlAllowlistStore::list() returns entries sorted by
domain. Previously a HashMap iteration could return entries in any
order, which surfaced as flaky `px-cli allowlist list` output and
non-reproducible audit diffs when the store is rebuilt.

Added unit tests for the sort guarantee and the lookup path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 .../infrastructure/yaml_allowlist_store.rs    | 47 ++++++++++++++++++-
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/px-auth/src/infrastructure/yaml_allowlist_store.rs b/px-auth/src/infrastructure/yaml_allowlist_store.rs
index 852baa0..461b1e2 100644
--- a/px-auth/src/infrastructure/yaml_allowlist_store.rs
+++ b/px-auth/src/infrastructure/yaml_allowlist_store.rs
@@ -3,7 +3,7 @@ use crate::domain::allowlist_store::AllowlistStore;
 use async_trait::async_trait;
 use px_errors::AppError;
 use serde::Deserialize;
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::path::Path;
 
 #[derive(Debug, Deserialize)]
@@ -11,8 +11,10 @@ struct AllowlistFile {
     entries: Vec<AllowlistEntry>,
 }
 
+/// `BTreeMap` keeps `list()` output in domain-sorted order so operator output
+/// (`px-cli allowlist list`) and audit diffs are reproducible across reloads.
 pub struct YamlAllowlistStore {
-    by_domain: HashMap<String, AllowlistEntry>,
+    by_domain: BTreeMap<String, AllowlistEntry>,
 }
 
 impl YamlAllowlistStore {
@@ -51,3 +53,44 @@ impl AllowlistStore for YamlAllowlistStore {
         Ok(self.by_domain.values().cloned().collect())
     }
 }
+
+#[cfg(test)]
+#[allow(clippy::expect_used, clippy::unwrap_used)]
+mod tests {
+    use super::*;
+
+    fn entry(domain: &str) -> AllowlistEntry {
+        AllowlistEntry {
+            domain: domain.into(),
+            tos_reviewed: true,
+            justification: "test".into(),
+            handler: None,
+        }
+    }
+
+    #[tokio::test]
+    async fn list_returns_entries_sorted_by_domain() {
+        let store = YamlAllowlistStore::from_entries(vec![
+            entry("zeta.example"),
+            entry("alpha.example"),
+            entry("mu.example"),
+        ]);
+        let listed: Vec<String> = store
+            .list()
+            .await
+            .expect("list")
+            .into_iter()
+            .map(|e| e.domain)
+            .collect();
+        assert_eq!(listed, vec!["alpha.example", "mu.example", "zeta.example"]);
+    }
+
+    #[tokio::test]
+    async fn lookup_finds_inserted_entry() {
+        let store = YamlAllowlistStore::from_entries(vec![entry("example.com")]);
+        let hit = store.lookup("example.com").await.expect("lookup");
+        assert_eq!(hit.map(|e| e.domain).as_deref(), Some("example.com"));
+        let miss = store.lookup("missing.example").await.expect("lookup");
+        assert!(miss.is_none());
+    }
+}