fix(admin): report a missing policy or user as 404 NoSuchResource, not 500 InternalError

Merge the approved fix from pull request #8127.

CHANGELOG.md

@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Presigned URLs honour only signed headers** (GHSA-g8w9-qw9q-fghr): a SigV4 presigned request that carries an `x-amz-*` request header not listed in `X-Amz-SignedHeaders` is now rejected with `403 AccessDenied` ("There were headers present in the request which were not signed"), matching AWS S3. Previously the holder of a presigned `PutObject` URL could add unsigned `x-amz-tagging`, `x-amz-storage-class`, `x-amz-website-redirect-location`, ACL, metadata, Object Lock or SSE headers and have them applied. Presigners that intend a property must set it before signing so the SDK lists the header in `SignedHeaders`; `x-amz-cf-id` (CloudFront) remains tolerated unsigned. Header-signed SigV4 and SigV2 requests are unchanged.
 
 ### Fixed
+- **Two admin handlers reported a missing policy or user as `500 InternalError`**: `GET /rustfs/admin/v3/info-canned-policy` and `DELETE /rustfs/admin/v3/remove-user` wrapped every IAM error as an internal fault instead of classifying it, so a lookup for something that does not exist answered `500 InternalError` where the rest of the admin API answers `404 NoSuchResource` — `GET /rustfs/admin/v3/user-info` already did, and `iam_error_to_s3_error` already mapped `NoSuchPolicy`, `NoSuchUser`, `NoSuchServiceAccount` and `NoSuchTempAccount` to `NoSuchResource`. A client could not tell "does not exist" from a real server failure, and a controller that checks existence before creating (a Kubernetes operator reconciling policies or users) could never create the object: the missing-policy lookup failed with a `500` and it retried forever. Both handlers now map their IAM errors through the shared `iam_error_to_s3_error` helper, so a missing policy or user is `404 NoSuchResource` and the `error` is preserved as the S3 error's source; the three IAM calls in the removal path (temporary-user lookup, service-account lookup, delete) are all reclassified, and every other IAM error stays `500 InternalError`. Fixes #8117.
 - **Vault backends accepted key names that escape the key prefix**: only the Local backend refused a key identifier containing a path separator, so on Vault KV2 `POST /rustfs/admin/v3/kms/keys` with a name such as `bad/name` succeeded and created a nested KV2 path that the key listing then reported as a directory rather than a key, and an identifier containing `..` addressed a record outside the configured key prefix once the HTTP client normalised the URL. Both Vault backends now apply the Local backend's containment rule at the point where the identifier becomes a path or a Transit key name: an empty identifier, one containing `/`, `\\` or NUL, or the dot segments `.` and `..` is refused with `400` (`InvalidKey`) before any request reaches Vault. The AWS backend is unaffected, since it addresses keys by ARN and alias. A Vault deployment that created such a key on an earlier build must re-create it under a plain name; objects encrypted under the old name stay readable only until that key is refused, so migrate them first. Refs rustfs/backlog#2474.
 - **Fresh multi-pool bootstrap with distinct format creators**: a new deployment whose pools have their first endpoint on different nodes (for example two single-node pools) could never publish its initial `pool.bin`: each node held fresh-bootstrap proof only for the pool it formatted, the deployment-wide proof collapsed to none, and every node died with `pool metadata recovery required: no durable bootstrap identity or pool.bin replica is available` after the startup retry budget. The first pool's creator now mints the pending cluster identity on its own pool, every other creator copies that nonce-bound identity onto the pool it formatted first-hand, and the elected writer publishes `pool.bin` once every pool replica carries the same pending identity. Corrupt or disagreeing replicas, pools that merely have a format, expansion pools joining an initialized deployment, and restarts without first-hand proof still fail closed. Non-elected nodes that start before `pool.bin` exists, and the elected writer while it waits for the other creators, no longer latch their pool-metadata write gate for the life of the process. Refs rustfs/backlog#2338, rustfs/backlog#2375.
 - **Lock RPC timeout storms** (#7363): the remote lock client no longer evicts and re-dials the shared internode HTTP/2 channel on every request deadline. A timeout evicts only when the peer has not completed any lock RPC for two deadlines, evictions and transport-failure re-dials are rate limited per peer (`RUSTFS_OBJECT_LOCK_RPC_EVICTION_COOLDOWN_MS`, default 5 s), and a timed-out request is left running instead of being reset (bounded per peer by `RUSTFS_OBJECT_LOCK_RPC_DETACHED_LIMIT`, default 256), so a slow lock endpoint can no longer drive the `RST_STREAM`/`GOAWAY too_many_resets`/reconnect loop. A lock granted after its caller timed out is released immediately, and unlocks that fail the quick retries continue on a deferred 1/2/4/8/16 s schedule before the server lease reclaims them. New `rustfs_remote_lock_*` metrics cover timeouts, evictions, suppressed evictions, detached streams, late completions and late releases per peer. Operator guide at `docs/operations/lock-rpc-storm-protection.md`.

rustfs/src/admin/handlers/policies.rs

@@ -1271,4 +1271,24 @@ mod tests {
 
         assert!(!production.contains("check_key_valid(get_session_token"));
     }
+
+    #[test]
+    fn info_canned_policy_maps_iam_errors_to_s3_errors() {
+        let production = include_str!("policies.rs")
+            .split("\n#[cfg(test)]\n")
+            .next()
+            .expect("production source must precede tests");
+        let body = source_block(production, "impl Operation for InfoCannedPolicy");
+
+        let lookup = body
+            .find("info_policy(")
+            .expect("InfoCannedPolicy should look the policy up through the IAM store");
+        let tail = &body[lookup..];
+        let end = tail.find("?;").unwrap_or(tail.len());
+
+        assert!(
+            tail[..end].contains("iam_error_to_s3_error(e)"),
+            "a missing policy must map through iam_error_to_s3_error (404 NoSuchResource) instead of 500 InternalError"
+        );
+    }
 }

rustfs/src/admin/handlers/user.rs

@@ -516,17 +516,12 @@ impl Operation for RemoveUser {
             return Err(s3_error!(InvalidArgument, "cannot remove a temporary user"));
         }
 
-        let (is_service_account, _) = iam_store.is_service_account(ak).await.map_err(|e| {
-            S3Error::with_message(S3ErrorCode::InternalError, format!("failed to query service account state: {e}"))
-        })?;
+        let (is_service_account, _) = iam_store.is_service_account(ak).await.map_err(iam_error_to_s3_error)?;
         if is_service_account {
             return Err(s3_error!(InvalidArgument, "cannot remove a service account"));
         }
 
-        iam_store
-            .delete_user(ak, true)
-            .await
-            .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("failed to delete user: {e}")))?;
+        iam_store.delete_user(ak, true).await.map_err(iam_error_to_s3_error)?;
 
         if let Err(err) = site_replication_iam_change_hook(SRIAMItem {
             r#type: "iam-user".to_string(),
@@ -1440,6 +1435,27 @@ mod tests {
         assert!(include_str!("user.rs").contains(mapper_call));
     }
 
+    #[test]
+    fn remove_user_maps_iam_errors_to_s3_errors() {
+        let production = include_str!("user.rs")
+            .split("\n#[cfg(test)]\n")
+            .next()
+            .expect("production source must precede tests");
+        let body = source_block(production, "impl Operation for RemoveUser");
+
+        for call in ["is_temp_user(ak)", "is_service_account(ak)", "delete_user(ak, true)"] {
+            let start = body
+                .find(call)
+                .unwrap_or_else(|| panic!("RemoveUser should call {call} through the IAM store"));
+            let tail = &body[start..];
+            let end = tail.find("?;").unwrap_or(tail.len());
+            assert!(
+                tail[..end].contains("iam_error_to_s3_error"),
+                "{call} must map through iam_error_to_s3_error so a missing user is 404 NoSuchResource, not 500 InternalError"
+            );
+        }
+    }
+
     #[test]
     fn import_iam_enqueues_a_site_replication_snapshot() {
         let body = source_block(include_str!("user.rs"), "impl Operation for ImportIam");