[authz tests] Add coverage for support bundles (#9756)

smklein · web-flow · commit 947b3aff5390 · 2026-01-30T20:12:59.000Z
Part of #9747
diff --git a/nexus/tests/integration_tests/audit_log.rs b/nexus/tests/integration_tests/audit_log.rs
@@ -452,7 +452,9 @@ async fn test_audit_log_coverage(ctx: &ControlPlaneTestContext) {
                 | AllowedMethod::GetNonexistent
                 | AllowedMethod::GetUnimplemented
                 | AllowedMethod::GetVolatile
-                | AllowedMethod::GetWebsocket => false,
+                | AllowedMethod::GetWebsocket
+                | AllowedMethod::Head
+                | AllowedMethod::HeadNonexistent => false,
             };
 
             let before = fetch_log(client, t_start, None).await.items.len();
diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs
@@ -95,6 +95,14 @@ pub const SUPPORT_BUNDLES_URL: &'static str =
     "/experimental/v1/system/support-bundles";
 pub static SUPPORT_BUNDLE_URL: LazyLock<String> =
     LazyLock::new(|| format!("{SUPPORT_BUNDLES_URL}/{{id}}"));
+pub static SUPPORT_BUNDLE_DOWNLOAD_URL: LazyLock<String> =
+    LazyLock::new(|| format!("{SUPPORT_BUNDLES_URL}/{{id}}/download"));
+pub static SUPPORT_BUNDLE_DOWNLOAD_FILE_URL: LazyLock<String> =
+    LazyLock::new(|| {
+        format!("{SUPPORT_BUNDLES_URL}/{{id}}/download/some-file.txt")
+    });
+pub static SUPPORT_BUNDLE_INDEX_URL: LazyLock<String> =
+    LazyLock::new(|| format!("{SUPPORT_BUNDLES_URL}/{{id}}/index"));
 
 // Global policy
 pub const SYSTEM_POLICY_URL: &'static str = "/v1/system/policy";
@@ -1585,6 +1593,13 @@ pub enum AllowedMethod {
     GetVolatile,
     /// HTTP "GET" method with websocket handshake headers.
     GetWebsocket,
+    /// HTTP "HEAD" method
+    #[allow(dead_code)]
+    Head,
+    /// HTTP "HEAD" method, but where we cannot statically define a URL that
+    /// will work (similar to GetNonexistent). The test runner should not expect
+    /// to get a 200 for privileged requests.
+    HeadNonexistent,
     /// HTTP "POST" method, with sample input (which should be valid input for
     /// this endpoint)
     Post(serde_json::Value),
@@ -1603,6 +1618,9 @@ impl AllowedMethod {
             | AllowedMethod::GetUnimplemented
             | AllowedMethod::GetVolatile
             | AllowedMethod::GetWebsocket => &Method::GET,
+            AllowedMethod::Head | AllowedMethod::HeadNonexistent => {
+                &Method::HEAD
+            }
             AllowedMethod::Post(_) => &Method::POST,
             AllowedMethod::Put(_) => &Method::PUT,
         }
@@ -1619,7 +1637,9 @@ impl AllowedMethod {
             | AllowedMethod::GetNonexistent
             | AllowedMethod::GetUnimplemented
             | AllowedMethod::GetVolatile
-            | AllowedMethod::GetWebsocket => None,
+            | AllowedMethod::GetWebsocket
+            | AllowedMethod::Head
+            | AllowedMethod::HeadNonexistent => None,
             AllowedMethod::Post(body) => Some(&body),
             AllowedMethod::Put(body) => Some(&body),
         }
@@ -2850,6 +2870,34 @@ pub static VERIFY_ENDPOINTS: LazyLock<Vec<VerifyEndpoint>> = LazyLock::new(
                     ),
                 ],
             },
+            // Note: We use GetNonexistent/HeadNonexistent because the bundle is
+            // created but not in "Active" state (which requires background task
+            // processing). Privileged GET/HEAD will fail with 400, not 200, so
+            // we skip that check.
+            VerifyEndpoint {
+                url: &SUPPORT_BUNDLE_DOWNLOAD_URL,
+                visibility: Visibility::Protected,
+                unprivileged_access: UnprivilegedAccess::None,
+                allowed_methods: vec![
+                    AllowedMethod::GetNonexistent,
+                    AllowedMethod::HeadNonexistent,
+                ],
+            },
+            VerifyEndpoint {
+                url: &SUPPORT_BUNDLE_DOWNLOAD_FILE_URL,
+                visibility: Visibility::Protected,
+                unprivileged_access: UnprivilegedAccess::None,
+                allowed_methods: vec![
+                    AllowedMethod::GetNonexistent,
+                    AllowedMethod::HeadNonexistent,
+                ],
+            },
+            VerifyEndpoint {
+                url: &SUPPORT_BUNDLE_INDEX_URL,
+                visibility: Visibility::Protected,
+                unprivileged_access: UnprivilegedAccess::None,
+                allowed_methods: vec![AllowedMethod::GetNonexistent],
+            },
             /* Updates */
             VerifyEndpoint {
                 url: DEMO_UPDATE_TRUST_ROOTS_URL,
diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs
@@ -491,7 +491,12 @@ static SETUP_REQUESTS: LazyLock<Vec<SetupReq>> = LazyLock::new(|| {
                 },
             )
             .unwrap(),
-            id_routes: vec!["/experimental/v1/system/support-bundles/{id}"],
+            id_routes: vec![
+                "/experimental/v1/system/support-bundles/{id}",
+                "/experimental/v1/system/support-bundles/{id}/download",
+                "/experimental/v1/system/support-bundles/{id}/download/some-file.txt",
+                "/experimental/v1/system/support-bundles/{id}/index",
+            ],
         },
         // Create a trusted root for updates
         SetupReq::Post {
@@ -695,8 +700,14 @@ async fn verify_endpoint(
 
     // For each of the HTTP methods we use in the API as well as TRACE, we'll
     // make several requests to this URL and verify the results.
-    let methods =
-        [Method::GET, Method::PUT, Method::POST, Method::DELETE, Method::TRACE];
+    let methods = [
+        Method::GET,
+        Method::PUT,
+        Method::POST,
+        Method::DELETE,
+        Method::HEAD,
+        Method::TRACE,
+    ];
     for method in methods {
         let allowed = endpoint
             .allowed_methods
@@ -736,7 +747,7 @@ async fn verify_endpoint(
             let response = request.execute().await.unwrap_or_else(|e| {
                 panic!("Failed making {method} request to {uri}: {e}")
             });
-            verify_response(&response);
+            verify_response(&response, &method);
             record_operation(WhichTest::Unprivileged(&expected_status));
         } else {
             // "This door is opened elsewhere."
@@ -757,7 +768,7 @@ async fn verify_endpoint(
             request = request.expect_websocket_handshake();
         }
         let response = request.execute().await.unwrap();
-        verify_response(&response);
+        verify_response(&response, &method);
         record_operation(WhichTest::Unauthenticated(&expected_status));
 
         // Now try a few requests with bogus credentials.  We should get the
@@ -790,7 +801,7 @@ async fn verify_endpoint(
             request = request.expect_websocket_handshake();
         }
         let response = request.execute().await.unwrap();
-        verify_response(&response);
+        verify_response(&response, &method);
         record_operation(WhichTest::UnknownUser(&expected_status));
 
         // Now try a syntactically invalid authn header.
@@ -808,7 +819,7 @@ async fn verify_endpoint(
             request = request.expect_websocket_handshake();
         }
         let response = request.execute().await.unwrap();
-        verify_response(&response);
+        verify_response(&response, &method);
         record_operation(WhichTest::InvalidHeader(&expected_status));
 
         print!(" ");
@@ -851,11 +862,15 @@ async fn verify_endpoint(
 }
 
 /// Verifies the body of an HTTP response for status codes 401, 403, 404, or 405
-fn verify_response(response: &TestResponse) {
+fn verify_response(response: &TestResponse, method: &Method) {
     if response.status == StatusCode::SWITCHING_PROTOCOLS {
         // websocket handshake. avoid trying to parse absent body as json.
         return;
     }
+    if *method == Method::HEAD {
+        // HEAD requests have no body to parse.
+        return;
+    }
     let error: HttpErrorResponseBody = response.parsed_body().unwrap();
     match response.status {
         StatusCode::UNAUTHORIZED => {
diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt
@@ -3,16 +3,11 @@ probe_delete                             (delete "/experimental/v1/probes/{probe
 current_user_access_token_delete         (delete "/v1/me/access-tokens/{token_id}")
 probe_list                               (get    "/experimental/v1/probes")
 probe_view                               (get    "/experimental/v1/probes/{probe}")
-support_bundle_download                  (get    "/experimental/v1/system/support-bundles/{bundle_id}/download")
-support_bundle_download_file             (get    "/experimental/v1/system/support-bundles/{bundle_id}/download/{file}")
-support_bundle_index                     (get    "/experimental/v1/system/support-bundles/{bundle_id}/index")
 ping                                     (get    "/v1/ping")
 networking_switch_port_lldp_neighbors    (get    "/v1/system/hardware/rack-switch-port/{rack_id}/{switch_location}/{port}/lldp/neighbors")
 rack_membership_status                   (get    "/v1/system/hardware/racks/{rack_id}/membership")
 networking_switch_port_lldp_config_view  (get    "/v1/system/hardware/switch-port/{port}/lldp/config")
 networking_switch_port_status            (get    "/v1/system/hardware/switch-port/{port}/status")
-support_bundle_head                      (head   "/experimental/v1/system/support-bundles/{bundle_id}/download")
-support_bundle_head_file                 (head   "/experimental/v1/system/support-bundles/{bundle_id}/download/{file}")
 device_auth_request                      (post   "/device/auth")
 device_auth_confirm                      (post   "/device/confirm")
 device_access_token                      (post   "/device/token")
