Skip to content

Commit 11ccce0

Browse files
authored
fix: bound co-owner invites per request (#9300)
Bound the number of co-owner invites that can be sent in a single API request, effectively bounding the work done per request. This limits the number of iterations the N^2 membership test loop has to perform, limiting the amount of CPU time and memory needed for both it and the subsequent database queries per invitee.
1 parent 504d528 commit 11ccce0

File tree

3 files changed

+31
-1
lines changed

3 files changed

+31
-1
lines changed

src/controllers/krate/owners.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,14 @@ async fn modify_owners(
108108
) -> AppResult<Json<Value>> {
109109
let logins = body.owners;
110110

111+
// Bound the number of invites processed per request to limit the cost of
112+
// processing them all.
113+
if logins.len() > 10 {
114+
return Err(bad_request(
115+
"too many invites for this request - maximum 10",
116+
));
117+
}
118+
111119
let conn = app.db_write().await?;
112120
spawn_blocking(move || {
113121
let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into();

src/tests/routes/crates/owners/add.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,3 +353,22 @@ async fn test_unknown_team() {
353353
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
354354
assert_snapshot!(response.text(), @r###"{"errors":[{"detail":"could not find the github team unknown/unknown. Make sure that you have the right permissions in GitHub. See https://doc.rust-lang.org/cargo/reference/publishing.html#github-permissions"}]}"###);
355355
}
356+
357+
#[tokio::test(flavor = "multi_thread")]
358+
async fn max_invites_per_request() {
359+
let (app, _, _, owner) = TestApp::init().with_token();
360+
app.db(|conn| CrateBuilder::new("crate_name", owner.as_model().user_id).expect_build(conn));
361+
362+
let usernames = (0..11)
363+
.map(|i| format!("user_{i}"))
364+
.collect::<Vec<String>>();
365+
366+
// Populate enough users in the database to submit 11 invites at once.
367+
for user in &usernames {
368+
app.db_new_user(user);
369+
}
370+
371+
let response = owner.add_named_owners("crate_name", &usernames).await;
372+
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
373+
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"too many invites for this request - maximum 10"}]}"#);
374+
}

src/tests/util.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,10 @@ impl MockTokenUser {
332332
}
333333

334334
/// Add to the specified crate the specified owners.
335-
pub async fn add_named_owners(&self, krate_name: &str, owners: &[&str]) -> Response<OkBool> {
335+
pub async fn add_named_owners<T>(&self, krate_name: &str, owners: &[T]) -> Response<OkBool>
336+
where
337+
T: serde::Serialize,
338+
{
336339
let url = format!("/api/v1/crates/{krate_name}/owners");
337340
let body = json!({ "owners": owners }).to_string();
338341
self.put(&url, body).await

0 commit comments

Comments
 (0)