fix(policy): normalize IAM policy readback (#3402)

crates/policy/src/policy/action.rs

@@ -36,10 +36,12 @@ impl Serialize for ActionSet {
 
         // Always serialize as array, even for single action, to match S3 specification
         // and ensure compatibility with AWS SDK clients that expect array format
-        let mut seq = serializer.serialize_seq(Some(self.0.len()))?;
+        let mut actions: Vec<&str> = self.0.iter().map(Into::into).collect();
-        for action in &self.0 {
+        actions.sort_unstable();
-            let action_str: &str = action.into();
+
-            seq.serialize_element(action_str)?;
+        let mut seq = serializer.serialize_seq(Some(actions.len()))?;
+        for action in actions {
+            seq.serialize_element(action)?;
         }
         seq.end()
     }

crates/policy/src/policy/policy.rs

@@ -57,7 +57,7 @@ impl Args<'_> {
 #[derive(Serialize, Deserialize, Clone, Default, Debug)]
 #[serde(deny_unknown_fields)]
 pub struct Policy {
-    #[serde(default, rename = "ID")]
+    #[serde(default, rename = "ID", skip_serializing_if = "ID::is_empty")]
     pub id: ID,
     #[serde(default, rename = "Version")]
     pub version: String,
@@ -1918,6 +1918,80 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_iam_policy_serialize_omits_empty_fields() {
+        let policy = Policy::parse_config(
+            br#"{
+  "Version":"2012-10-17",
+  "Statement":[{
+    "Effect":"Allow",
+    "Action":["s3:GetObject","s3:PutObject"],
+    "Resource":["arn:aws:s3:::example-bucket/*"]
+  }]
+}"#,
+        )
+        .expect("policy without optional fields should parse");
+
+        let value = serde_json::to_value(&policy).expect("policy should serialize");
+        let policy_object = value.as_object().expect("policy should serialize as an object");
+        assert!(!policy_object.contains_key("ID"), "empty ID should be omitted");
+
+        let statements = value["Statement"].as_array().expect("Statement should serialize as an array");
+        let statement = statements
+            .first()
+            .expect("serialized policy should include the statement")
+            .as_object()
+            .expect("statement should serialize as an object");
+        assert!(!statement.contains_key("Sid"), "empty Sid should be omitted");
+        assert!(!statement.contains_key("Condition"), "empty Condition should be omitted");
+    }
+
+    #[test]
+    fn test_policy_action_and_resource_serialization_is_stable() {
+        let policy = Policy::parse_config(
+            br#"{
+  "Version":"2012-10-17",
+  "Statement":[{
+    "Effect":"Allow",
+    "Action":["s3:PutObject","s3:DeleteObject","s3:ListBucket","s3:GetObject"],
+    "Resource":["arn:aws:s3:::example-bucket/*","arn:aws:s3:::example-bucket"]
+  }]
+}"#,
+        )
+        .expect("policy with multiple actions and resources should parse");
+
+        let first = serde_json::to_value(&policy).expect("policy should serialize");
+        let second = serde_json::to_value(&policy).expect("policy should serialize consistently");
+        assert_eq!(first, second, "policy serialization should be deterministic");
+
+        let statement = first["Statement"][0]
+            .as_object()
+            .expect("statement should serialize as an object");
+        let actions: Vec<_> = statement["Action"]
+            .as_array()
+            .expect("Action should serialize as an array")
+            .iter()
+            .map(|action| action.as_str().expect("Action entries should be strings"))
+            .collect();
+        assert_eq!(
+            actions,
+            vec!["s3:DeleteObject", "s3:GetObject", "s3:ListBucket", "s3:PutObject"],
+            "Action serialization should use a stable order"
+        );
+
+        let resources: Vec<_> = statement["Resource"]
+            .as_array()
+            .expect("Resource should serialize as an array")
+            .iter()
+            .map(|resource| resource.as_str().expect("Resource entries should be strings"))
+            .collect();
+        assert_eq!(
+            resources,
+            vec!["arn:aws:s3:::example-bucket", "arn:aws:s3:::example-bucket/*"],
+            "Resource serialization should use a stable order"
+        );
+    }
+
     #[test]
     fn test_bucket_policy_serialize_omits_empty_fields() {
         use crate::policy::action::{Action, ActionSet, S3Action};

crates/policy/src/policy/resource.rs

@@ -31,9 +31,34 @@ use super::{
     variables::PolicyVariableResolver,
 };
 
-#[derive(Serialize, Clone, Default, Debug)]
+#[derive(Clone, Default, Debug)]
 pub struct ResourceSet(pub HashSet<Resource>);
 
+impl Serialize for ResourceSet {
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::SerializeSeq;
+
+        let mut resources: Vec<String> = self
+            .0
+            .iter()
+            .map(|resource| match resource {
+                Resource::S3(value) => format!("{}{}", Resource::S3_PREFIX, value),
+                Resource::Kms(value) => value.clone(),
+            })
+            .collect();
+        resources.sort_unstable();
+
+        let mut seq = serializer.serialize_seq(Some(resources.len()))?;
+        for resource in resources {
+            seq.serialize_element(&resource)?;
+        }
+        seq.end()
+    }
+}
+
 impl ResourceSet {
     /// Returns true if the resource set is empty.
     pub fn is_empty(&self) -> bool {
@@ -208,11 +233,10 @@ impl Resource {
 impl TryFrom<&str> for Resource {
     type Error = Error;
     fn try_from(value: &str) -> std::result::Result<Self, Self::Error> {
-        let resource = if value.starts_with(Self::S3_PREFIX) {
+        let Some(value) = value.strip_prefix(Self::S3_PREFIX) else {
-            Resource::S3(value.strip_prefix(Self::S3_PREFIX).unwrap().into())
-        } else {
             return Err(IamError::InvalidResource("unknown".into(), value.into()).into());
         };
+        let resource = Resource::S3(value.into());
 
         resource.is_valid()?;
         Ok(resource)

crates/policy/src/policy/statement.rs

@@ -24,7 +24,7 @@ use serde::{Deserialize, Serialize};
 #[derive(Serialize, Deserialize, Clone, Default, Debug)]
 #[serde(deny_unknown_fields)]
 pub struct Statement {
-    #[serde(rename = "Sid", default)]
+    #[serde(rename = "Sid", default, skip_serializing_if = "ID::is_empty")]
     pub sid: ID,
     #[serde(rename = "Effect")]
     pub effect: Effect,
@@ -36,7 +36,7 @@ pub struct Statement {
     pub resources: ResourceSet,
     #[serde(rename = "NotResource", default, skip_serializing_if = "ResourceSet::is_empty")]
     pub not_resources: ResourceSet,
-    #[serde(rename = "Condition", default)]
+    #[serde(rename = "Condition", default, skip_serializing_if = "Functions::is_empty")]
     pub conditions: Functions,
 }
 
@@ -151,7 +151,7 @@ impl Statement {
             }
         }
 
-        let family_count = saw_s3 as u8 + saw_admin as u8 + saw_sts as u8 + saw_kms as u8;
+        let family_count = u8::from(saw_s3) + u8::from(saw_admin) + u8::from(saw_sts) + u8::from(saw_kms);
 
         if family_count != 1 {
             return Some(ActionFamily::Mixed);

rustfs/src/admin/access_key_identity.rs

@@ -891,10 +891,7 @@ mod tests {
         assert_eq!(resp.open_id_specific_info.display_name.as_deref(), Some("RustFS User"));
         assert_eq!(resp.open_id_specific_info.display_name_claim.as_deref(), Some("name"));
         assert_eq!(resp.open_id_specific_info.config_name, None);
-        assert_eq!(
+        assert_eq!(resp.info.policy, Some("{\n  \"Version\": \"\",\n  \"Statement\": []\n}".to_string()));
-            resp.info.policy,
-            Some("{\n  \"ID\": \"\",\n  \"Version\": \"\",\n  \"Statement\": []\n}".to_string())
-        );
 
         let body = serde_json::to_value(&resp).expect("serialize openid sts response");
         assert_eq!(body.get("userType").and_then(|v| v.as_str()), Some("STS"));
@@ -932,7 +929,7 @@ mod tests {
         assert_eq!(body.get("description").and_then(|v| v.as_str()), Some("openid derived temporary account"));
         assert_eq!(
             body.get("policy").and_then(|v| v.as_str()),
-            Some("{\n  \"ID\": \"\",\n  \"Version\": \"\",\n  \"Statement\": []\n}")
+            Some("{\n  \"Version\": \"\",\n  \"Statement\": []\n}")
         );
     }
 }