Gate BMF payload versions per project

Each project now declares the highest BMF payload version it accepts. The field is
`bmf_version` on the project, it defaults to 0 everywhere, and it is visible to
everyone that can see the project. Only a server admin can move it: a PATCH that
carries the field from anyone else is forbidden, and a PATCH that does not carry it
is the patch it has always been. The gate is a plain setting, so an admin can lower
it again just as easily.

The gate is a maximum, not an exact match. A project at version 1 still ingests a
payload that declares version 0 and a payload that declares nothing at all, so
raising the gate refuses nothing that ingested before it moved.

Ingest checks the gate twice, once for each way a payload states its version, and
both refusals are the same class and name both versions, the payload's and the
project's. The `bmf_version` key a payload declares is checked before anything is
created for the report, because later layers hang payload shapes off the declared
version and none of them should reach a project that does not accept them. The
version the results actually parsed as is checked after parsing, because a payload
can reach a v1 leaf without declaring anything: the `json_v1` adapter names the leaf
outright, and the `magic` and `json` nodes fall back to it. Without the second check
the gate would be decorative.

That second check is a deliberate behavior change for those two undeclared paths on
a project still at version 0. They are not a documented way to send v1 results, and
a gate a payload can walk around is not a gate.

Report ingest, `/v0/run`, and a job based run share one check rather than three
copies of it. A job declares nothing, since its results are the runner's own output
rather than a submitted payload, but the project gate still applies to what those
results parse as.

Author: epompeii

lib/api_projects/src/projects.rs

@@ -3,7 +3,7 @@ use bencher_endpoint::{
 };
 use bencher_json::{
     JsonDirection, JsonPagination, JsonProject, JsonProjects, ProjectResourceId, ResourceName,
-    Search,
+    Sanitize as _, Search,
     project::{JsonUpdateProject, Visibility},
 };
 use bencher_rbac::project::Permission;
@@ -277,6 +277,7 @@ pub async fn get_one_inner(
 ///
 /// Update a project.
 /// The user must have `edit` permissions for the project.
+/// Setting the `bmf_version` field requires a server admin.
 #[endpoint {
     method = PATCH,
     path =  "/v0/projects/{project}",
@@ -321,6 +322,16 @@ async fn patch_inner(
         Permission::Edit,
     )?;
 
+    // Only a server admin can move the project's BMF payload version gate.
+    // Every other field of the patch is unaffected by this check.
+    if json_project.bmf_version().is_some() && !auth_user.is_admin(&context.rbac) {
+        let mut auth_user = auth_user.clone();
+        auth_user.sanitize();
+        return Err(forbidden_error(format!(
+            "Only admins can update the `bmf_version` field for a project. User is not an admin: {auth_user:?}",
+        )));
+    }
+
     // Check project visibility
     if let Some(visibility) = json_project.visibility() {
         #[cfg(not(feature = "plus"))]

lib/api_projects/tests/bmf_version.rs

@@ -14,6 +14,7 @@
 //! before is refused now and every payload lands on the same leaf either way.
 
 use bencher_api_tests::{TestServer, TestUser};
+use bencher_json::{BmfVersion, ProjectSlug};
 use http::StatusCode;
 
 /// A BMF v0 payload: a benchmark maps straight to its measures.
@@ -42,8 +43,11 @@ fn v1_results() -> String {
 }
 
 /// A signed up user with an organization and a project to report into.
+///
+/// The user is the server's first signup, which makes them its admin, so this
+/// fixture is also what moves the project's BMF version gate.
 struct Fixture {
-    project_slug: String,
+    project_slug: ProjectSlug,
     user: TestUser,
 }
 
@@ -56,11 +60,22 @@ async fn fixture(server: &TestServer, label: &str) -> Fixture {
         .create_project(&user, &org, &format!("Bmf Project {label}"))
         .await;
     Fixture {
-        project_slug: project.slug.to_string(),
+        project_slug: project.slug,
         user,
     }
 }
 
+/// The same fixture with the project's gate raised to BMF version 1.
+///
+/// Every payload that states version 1, by declaring the key or by parsing as v1,
+/// needs this: a project accepts version 0 and nothing above it until an admin
+/// says otherwise.
+async fn gated_fixture(server: &TestServer, label: &str) -> Fixture {
+    let fixture = fixture(server, label).await;
+    server.set_bmf_version(&fixture.project_slug, BmfVersion::V1);
+    fixture
+}
+
 /// Post one report and return its status and body, whatever they are.
 ///
 /// Every report a comparison uses carries the same `hash`, so they all resolve to
@@ -196,7 +211,7 @@ async fn report_absent_bmf_version_is_version_0() {
 #[tokio::test]
 async fn report_bmf_version_1_ingests_a_v1_payload_identically_to_version_0() {
     let server = TestServer::new().await;
-    let fixture = fixture(&server, "v1").await;
+    let fixture = gated_fixture(&server, "v1").await;
 
     let one = report(
         &server,
@@ -236,7 +251,7 @@ async fn report_bmf_version_1_ingests_a_v1_payload_identically_to_version_0() {
 #[tokio::test]
 async fn report_bmf_version_1_still_ingests_a_v0_payload() {
     let server = TestServer::new().await;
-    let fixture = fixture(&server, "fallback").await;
+    let fixture = gated_fixture(&server, "fallback").await;
 
     let one = report(
         &server,
@@ -305,7 +320,7 @@ async fn report_unknown_bmf_version_is_rejected() {
 #[tokio::test]
 async fn report_explicit_leaf_wins_over_bmf_version() {
     let server = TestServer::new().await;
-    let fixture = fixture(&server, "leaf").await;
+    let fixture = gated_fixture(&server, "leaf").await;
 
     let (status, body) = try_report(
         &server,
@@ -341,3 +356,277 @@ async fn report_explicit_leaf_wins_over_bmf_version() {
 
     server.close().await;
 }
+
+// --- The project gate ---
+//
+// A project declares the highest BMF payload version it accepts. It defaults to 0
+// everywhere, only a server admin can raise it, and it is visible to everyone.
+
+/// GET one project as `user`.
+async fn get_project(
+    server: &TestServer,
+    user: &TestUser,
+    project: &ProjectSlug,
+) -> serde_json::Value {
+    let resp = server
+        .client
+        .get(server.api_url(&format!("/v0/projects/{project}")))
+        .header(
+            bencher_json::AUTHORIZATION,
+            bencher_json::bearer_header(&user.token),
+        )
+        .send()
+        .await
+        .expect("Request failed");
+    let status = resp.status();
+    assert_eq!(status, StatusCode::OK, "GET project: {status}");
+    resp.json().await.expect("Failed to parse the project")
+}
+
+/// PATCH one project with whatever body, and return its status and body.
+async fn try_patch(
+    server: &TestServer,
+    user: &TestUser,
+    project: &ProjectSlug,
+    body: &serde_json::Value,
+) -> (StatusCode, String) {
+    let resp = server
+        .client
+        .patch(server.api_url(&format!("/v0/projects/{project}")))
+        .header(
+            bencher_json::AUTHORIZATION,
+            bencher_json::bearer_header(&user.token),
+        )
+        .json(body)
+        .send()
+        .await
+        .expect("Request failed");
+    let status = resp.status();
+    let body = resp.text().await.expect("Failed to read the response");
+    (status, body)
+}
+
+/// What an error response said, without the request id the server mints for it.
+fn message(body: &str) -> String {
+    let error: serde_json::Value = serde_json::from_str(body).expect("Failed to parse the error");
+    error["message"]
+        .as_str()
+        .expect("the error carries a message")
+        .to_owned()
+}
+
+/// The refusal has to name the payload's version and the project's, so the message
+/// says both what was sent and what the project would take.
+fn assert_gate_refusal(status: StatusCode, body: &str, payload: u8, project: u8) {
+    assert_eq!(status, StatusCode::LOCKED, "{body}");
+    assert!(
+        body.contains(&payload.to_string()) && body.contains(&project.to_string()),
+        "expected the refusal to name BMF version {payload} and {project}: {body}"
+    );
+}
+
+/// A new project accepts BMF version 0 and says so.
+#[tokio::test]
+async fn project_bmf_version_defaults_to_0() {
+    let server = TestServer::new().await;
+    let fixture = fixture(&server, "default").await;
+
+    let project = get_project(&server, &fixture.user, &fixture.project_slug).await;
+    assert_eq!(project["bmf_version"], serde_json::json!(0));
+
+    server.close().await;
+}
+
+/// A payload that declares a version above the gate is refused, and the same
+/// project still takes a version 0 payload.
+#[tokio::test]
+async fn report_declared_version_above_the_gate_is_refused() {
+    let server = TestServer::new().await;
+    let fixture = fixture(&server, "declared").await;
+
+    let (status, body) = try_report(
+        &server,
+        &fixture,
+        &v0_results(),
+        Some(serde_json::json!(1)),
+        None,
+    )
+    .await;
+    assert_gate_refusal(status, &body, 1, 0);
+
+    // The gate is about the version, not about the project: version 0 still ingests.
+    report(
+        &server,
+        &fixture,
+        &v0_results(),
+        Some(serde_json::json!(0)),
+        None,
+    )
+    .await;
+
+    server.close().await;
+}
+
+/// Results that parse as v1 are refused even when no version was declared.
+///
+/// These are the two side doors the declared key does not cover: the `json_v1`
+/// leaf named outright, and the default `magic` adapter falling back to that leaf.
+/// Both are refused, and refused the same way, because both are the same statement
+/// about the payload.
+#[tokio::test]
+async fn report_parsed_v1_above_the_gate_is_refused() {
+    let server = TestServer::new().await;
+    let fixture = fixture(&server, "parsed").await;
+
+    let (magic_status, magic_body) = try_report(&server, &fixture, &v1_results(), None, None).await;
+    assert_gate_refusal(magic_status, &magic_body, 1, 0);
+
+    let (leaf_status, leaf_body) =
+        try_report(&server, &fixture, &v1_results(), None, Some("json_v1")).await;
+    assert_gate_refusal(leaf_status, &leaf_body, 1, 0);
+
+    // The same refusal, not just the same class. Only the request id differs,
+    // since the server mints a fresh one per request.
+    assert_eq!(message(&magic_body), message(&leaf_body));
+
+    server.close().await;
+}
+
+/// An admin raises the gate, and every payload the gate refused ingests.
+#[tokio::test]
+async fn admin_raising_the_gate_admits_the_refused_payloads() {
+    let server = TestServer::new().await;
+    let fixture = fixture(&server, "raise").await;
+
+    for (results, version, adapter) in [
+        (v0_results(), Some(serde_json::json!(1)), None),
+        (v1_results(), None, None),
+        (v1_results(), None, Some("json_v1")),
+    ] {
+        let (status, body) =
+            try_report(&server, &fixture, &results, version.clone(), adapter).await;
+        assert_gate_refusal(status, &body, 1, 0);
+    }
+
+    let (status, body) = try_patch(
+        &server,
+        &fixture.user,
+        &fixture.project_slug,
+        &serde_json::json!({ "bmf_version": 1 }),
+    )
+    .await;
+    assert_eq!(status, StatusCode::OK, "{body}");
+    let project = get_project(&server, &fixture.user, &fixture.project_slug).await;
+    assert_eq!(project["bmf_version"], serde_json::json!(1));
+
+    for (results, version, adapter) in [
+        (v0_results(), Some(serde_json::json!(1)), None),
+        (v1_results(), None, None),
+        (v1_results(), None, Some("json_v1")),
+    ] {
+        report(&server, &fixture, &results, version.clone(), adapter).await;
+    }
+
+    server.close().await;
+}
+
+/// The gate is a maximum, so a raised project takes every lower version unchanged.
+#[tokio::test]
+async fn a_raised_gate_still_accepts_version_0_payloads() {
+    let server = TestServer::new().await;
+    let fixture = gated_fixture(&server, "maximum").await;
+
+    let absent = report(&server, &fixture, &v0_results(), None, None).await;
+    let zero = report(
+        &server,
+        &fixture,
+        &v0_results(),
+        Some(serde_json::json!(0)),
+        None,
+    )
+    .await;
+    assert_eq!(absent, zero);
+
+    server.close().await;
+}
+
+/// Nothing ratchets: the gate is a plain setting an admin can put back.
+#[tokio::test]
+async fn admin_can_lower_the_gate() {
+    let server = TestServer::new().await;
+    let fixture = gated_fixture(&server, "lower").await;
+
+    report(
+        &server,
+        &fixture,
+        &v1_results(),
+        Some(serde_json::json!(1)),
+        None,
+    )
+    .await;
+
+    let (status, body) = try_patch(
+        &server,
+        &fixture.user,
+        &fixture.project_slug,
+        &serde_json::json!({ "bmf_version": 0 }),
+    )
+    .await;
+    assert_eq!(status, StatusCode::OK, "{body}");
+    let project = get_project(&server, &fixture.user, &fixture.project_slug).await;
+    assert_eq!(project["bmf_version"], serde_json::json!(0));
+
+    let (status, body) = try_report(
+        &server,
+        &fixture,
+        &v1_results(),
+        Some(serde_json::json!(1)),
+        None,
+    )
+    .await;
+    assert_gate_refusal(status, &body, 1, 0);
+
+    server.close().await;
+}
+
+/// Only a server admin can move the gate, and the rest of the patch is unaffected.
+///
+/// The second user owns their own organization and project, so they are allowed to
+/// edit it. The only thing standing between them and the field is the admin check.
+#[tokio::test]
+async fn non_admin_cannot_move_the_gate() {
+    let server = TestServer::new().await;
+    // The first signup is the server admin, so the second one is not.
+    let _admin = fixture(&server, "owner").await;
+    let user = server.signup("Other User", "bmfother@example.com").await;
+    let org = server.create_org(&user, "Bmf Other Org").await;
+    let project = server
+        .create_project(&user, &org, "Bmf Other Project")
+        .await;
+
+    let (status, body) = try_patch(
+        &server,
+        &user,
+        &project.slug,
+        &serde_json::json!({ "bmf_version": 1 }),
+    )
+    .await;
+    assert_eq!(status, StatusCode::FORBIDDEN, "{body}");
+    assert!(body.contains("bmf_version"), "{body}");
+
+    // The same patch without the field is the patch it has always been.
+    let (status, body) = try_patch(
+        &server,
+        &user,
+        &project.slug,
+        &serde_json::json!({ "name": "Bmf Renamed Project" }),
+    )
+    .await;
+    assert_eq!(status, StatusCode::OK, "{body}");
+
+    let project = get_project(&server, &user, &project.slug).await;
+    assert_eq!(project["name"], serde_json::json!("Bmf Renamed Project"));
+    assert_eq!(project["bmf_version"], serde_json::json!(0));
+
+    server.close().await;
+}