Implement cargo compatibility status code enforcement feature flag

src/config/server.rs

@@ -8,6 +8,7 @@ use crate::Env;
 use super::base::Base;
 use super::database_pools::DatabasePools;
 use crate::config::balance_capacity::BalanceCapacityConfig;
+use crate::middleware::cargo_compat::StatusCodeConfig;
 use crate::storage::StorageConfig;
 use crates_io_env_vars::{required_var, var, var_parsed};
 use http::HeaderValue;
@@ -56,6 +57,10 @@ pub struct Server {
     pub cdn_user_agent: String,
     pub balance_capacity: BalanceCapacityConfig,
 
+    /// Instructs the `cargo_compat` middleware whether to adjust response
+    /// status codes to `200 OK` for all endpoints that are relevant for cargo.
+    pub cargo_compat_status_code_config: StatusCodeConfig,
+
     /// Should the server serve the frontend assets in the `dist` directory?
     pub serve_dist: bool,
 
@@ -221,6 +226,8 @@ impl Server {
             cdn_user_agent: var("WEB_CDN_USER_AGENT")?
                 .unwrap_or_else(|| "Amazon CloudFront".into()),
             balance_capacity: BalanceCapacityConfig::from_environment()?,
+            cargo_compat_status_code_config: var_parsed("CARGO_COMPAT_STATUS_CODES")?
+                .unwrap_or(StatusCodeConfig::AdjustAll),
             serve_dist: true,
             serve_html: true,
             content_security_policy: Some(content_security_policy.parse()?),

src/middleware.rs

@@ -1,7 +1,7 @@
 pub mod app;
 mod balance_capacity;
 mod block_traffic;
-mod cargo_compat;
+pub mod cargo_compat;
 mod common_headers;
 mod debug;
 mod ember_html;
@@ -63,7 +63,10 @@ pub fn apply_axum_middleware(state: AppState, router: Router<()>) -> Router {
         }));
 
     let middlewares_2 = tower::ServiceBuilder::new()
-        .layer(from_fn(cargo_compat::middleware))
+        .layer(from_fn_with_state(
+            state.config.cargo_compat_status_code_config,
+            cargo_compat::middleware,
+        ))
         .layer(from_fn_with_state(state.clone(), session::attach_session))
         .layer(from_fn_with_state(
             state.clone(),

src/middleware/cargo_compat.rs

@@ -1,11 +1,41 @@
-use axum::extract::{MatchedPath, Request};
+use axum::extract::{MatchedPath, Request, State};
 use axum::middleware::Next;
 use axum::response::{IntoResponse, Response};
 use axum::{Extension, Json};
 use http::{header, Method, StatusCode};
+use std::str::FromStr;
+
+#[derive(Clone, Copy, Debug)]
+pub enum StatusCodeConfig {
+    /// Use the original response status code that the backend returned.
+    Disabled,
+    /// Use `200 OK` for all responses to cargo-relevant endpoints.
+    AdjustAll,
+    /// Use `200 OK` for all `2xx` responses to cargo-relevant endpoints, and
+    /// the original status code for all other responses.
+    AdjustSuccess,
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error("Failed to parse StatusCodeConfig")]
+pub struct StatusCodeConfigError;
+
+impl FromStr for StatusCodeConfig {
+    type Err = StatusCodeConfigError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "disabled" => Ok(Self::Disabled),
+            "adjust-all" => Ok(Self::AdjustAll),
+            "adjust-success" => Ok(Self::AdjustSuccess),
+            _ => Err(StatusCodeConfigError),
+        }
+    }
+}
 
 /// Convert plain text errors into JSON errors and adjust status codes.
 pub async fn middleware(
+    State(config): State<StatusCodeConfig>,
     matched_path: Option<Extension<MatchedPath>>,
     req: Request,
     next: Next,
@@ -30,7 +60,13 @@ pub async fn middleware(
         // 2xx status code. This will change with cargo 1.76.0 (see https://github.com/rust-lang/cargo/pull/13158),
         // but for backwards compatibility we still return "200 OK" for now for
         // all endpoints that are relevant for cargo.
-        *res.status_mut() = StatusCode::OK;
+        let adjust_status_code = matches!(
+            (config, res.status().is_success()),
+            (StatusCodeConfig::AdjustAll, _) | (StatusCodeConfig::AdjustSuccess, true)
+        );
+        if adjust_status_code {
+            *res.status_mut() = StatusCode::OK;
+        }
     }
 
     res
@@ -95,7 +131,7 @@ async fn convert_to_json_response(res: Response) -> anyhow::Result<Response> {
 mod tests {
     use super::*;
     use axum::body::Body;
-    use axum::middleware::from_fn;
+    use axum::middleware::from_fn_with_state;
     use axum::routing::{get, put};
     use axum::Router;
     use bytes::Bytes;
@@ -121,7 +157,7 @@ mod tests {
                 "/api/v1/crates/:crate_id/owners",
                 get(|| async { StatusCode::INTERNAL_SERVER_ERROR }),
             )
-            .layer(from_fn(middleware))
+            .layer(from_fn_with_state(StatusCodeConfig::AdjustAll, middleware))
     }
 
     async fn request(path: &str) -> anyhow::Result<(Parts, Bytes)> {

src/tests/krate/publish/auth.rs

@@ -13,7 +13,7 @@ fn new_wrong_token() {
     // Try to publish without a token
     let crate_to_publish = PublishBuilder::new("foo", "1.0.0");
     let response = anon.publish_crate(crate_to_publish);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
@@ -29,7 +29,7 @@ fn new_wrong_token() {
 
     let crate_to_publish = PublishBuilder::new("foo", "1.0.0");
     let response = token.publish_crate(crate_to_publish);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })

src/tests/owners.rs

@@ -366,7 +366,7 @@ fn owner_change_via_change_owner_token_with_wrong_crate_scope() {
     let body = json!({ "owners": [user2.gh_login] });
     let body = serde_json::to_vec(&body).unwrap();
     let response = token.put::<()>(&url, body);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
@@ -388,7 +388,7 @@ fn owner_change_via_publish_token() {
     let body = json!({ "owners": [user2.gh_login] });
     let body = serde_json::to_vec(&body).unwrap();
     let response = token.put::<()>(&url, body);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
@@ -409,7 +409,7 @@ fn owner_change_without_auth() {
     let body = json!({ "owners": [user2.gh_login] });
     let body = serde_json::to_vec(&body).unwrap();
     let response = anon.put::<()>(&url, body);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })

src/tests/pagination.rs

@@ -21,7 +21,7 @@ fn pagination_blocks_ip_from_cidr_block_list() {
     });
 
     let response = anon.get_with_query::<()>("/api/v1/crates", "page=2&per_page=1");
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::BAD_REQUEST);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "requested page offset is too large" }] })

src/tests/read_only_mode.rs

@@ -32,7 +32,7 @@ fn cannot_hit_endpoint_which_writes_db_in_read_only_mode() {
     });
 
     let response = token.delete::<()>("/api/v1/crates/foo_yank_read_only/1.0.0/yank");
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE);
     assert_json_snapshot!(response.into_json());
 }
 

src/tests/routes/crates/list.rs

@@ -799,7 +799,7 @@ fn invalid_seek_parameter() {
     let (_app, anon, _cookie) = TestApp::init().with_user();
 
     let response = anon.get::<()>("/api/v1/crates?seek=broken");
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::BAD_REQUEST);
     assert_json_snapshot!(response.into_json());
 }
 
@@ -816,15 +816,15 @@ fn pagination_parameters_only_accept_integers() {
 
     let response =
         anon.get_with_query::<()>("/api/v1/crates", "page=1&per_page=100%22%EF%BC%8Cexception");
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::BAD_REQUEST);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "invalid digit found in string" }] })
     );
 
     let response =
         anon.get_with_query::<()>("/api/v1/crates", "page=100%22%EF%BC%8Cexception&per_page=1");
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::BAD_REQUEST);
     assert_eq!(
         response.into_json(),
         json!({ "errors": [{ "detail": "invalid digit found in string" }] })

src/tests/routes/crates/versions/yank_unyank.rs

@@ -115,14 +115,14 @@ mod auth {
         let (_, client, _) = prepare();
 
         let response = client.yank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
         );
 
         let response = client.unyank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
@@ -182,14 +182,14 @@ mod auth {
             client.db_new_scoped_token("test-token", None, None, Some(expired_at.naive_utc()));
 
         let response = client.yank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
         );
 
         let response = client.unyank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
@@ -222,14 +222,14 @@ mod auth {
         );
 
         let response = client.yank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
         );
 
         let response = client.unyank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
@@ -286,14 +286,14 @@ mod auth {
         );
 
         let response = client.yank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
         );
 
         let response = client.unyank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
@@ -311,14 +311,14 @@ mod auth {
         );
 
         let response = client.yank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
         );
 
         let response = client.unyank(CRATE_NAME, CRATE_VERSION);
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::FORBIDDEN);
         assert_eq!(
             response.into_json(),
             json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })

src/tests/server.rs

@@ -11,15 +11,15 @@ fn user_agent_is_required() {
 
     let req = Request::get("/api/v1/crates").body("").unwrap();
     let resp = anon.run::<()>(req);
-    assert_eq!(resp.status(), StatusCode::OK);
+    assert_eq!(resp.status(), StatusCode::FORBIDDEN);
     assert_json_snapshot!(resp.into_json());
 
     let req = Request::get("/api/v1/crates")
         .header(header::USER_AGENT, "")
         .body("")
         .unwrap();
     let resp = anon.run::<()>(req);
-    assert_eq!(resp.status(), StatusCode::OK);
+    assert_eq!(resp.status(), StatusCode::FORBIDDEN);
     assert_json_snapshot!(resp.into_json());
 }
 
@@ -101,6 +101,6 @@ fn block_traffic_via_ip() {
         .empty();
 
     let resp = anon.get::<()>("/api/v1/crates");
-    assert_eq!(resp.status(), StatusCode::OK);
+    assert_eq!(resp.status(), StatusCode::FORBIDDEN);
     assert_json_snapshot!(resp.into_json());
 }

src/tests/util/response.rs

@@ -75,7 +75,7 @@ impl<T> Response<T> {
             detail: String,
         }
 
-        assert_eq!(self.status(), StatusCode::OK);
+        assert_eq!(self.status(), StatusCode::TOO_MANY_REQUESTS);
 
         let expected_message_start = format!("{}. Please try again after ", action.error_message());
         let error: ErrorResponse = json(self.response);

src/tests/util/test_app.rs

@@ -3,6 +3,7 @@ use crate::util::chaosproxy::ChaosProxy;
 use crate::util::github::{MockGitHubClient, MOCK_GITHUB_DATA};
 use anyhow::Context;
 use crates_io::config::{self, BalanceCapacityConfig, Base, DatabasePools, DbPoolConfig};
+use crates_io::middleware::cargo_compat::StatusCodeConfig;
 use crates_io::models::token::{CrateScope, EndpointScope};
 use crates_io::rate_limiter::{LimitedAction, RateLimiterConfig};
 use crates_io::storage::StorageConfig;
@@ -428,6 +429,11 @@ fn simple_config() -> config::Server {
         cdn_user_agent: "Amazon CloudFront".to_string(),
         balance_capacity,
 
+        // The middleware has its own unit tests to verify its functionality.
+        // Here, we can test what would happen if we toggled the status code
+        // enforcement off eventually.
+        cargo_compat_status_code_config: StatusCodeConfig::Disabled,
+
         // The frontend code is not needed for the backend tests.
         serve_dist: false,
         serve_html: false,