Skip to content

Commit 9c7f396

Browse files
committed
controllers/krate/publish: prevent deleted crate version reuse
If a crate has been deleted before, `deleted_crates.min_version` is enforced when validating a crate version that is being published. This also refactors the deleted crate check in general into a new module to make it easier to unit test and reason about. Mostly the latter, honestly.
1 parent 1891ef9 commit 9c7f396

9 files changed

+368
-18
lines changed

src/controllers/krate/publish.rs

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use crate::worker::jobs::{
99
use axum::body::Bytes;
1010
use axum::Json;
1111
use cargo_manifest::{Dependency, DepsSet, TargetDepsSet};
12-
use chrono::{DateTime, SecondsFormat, Utc};
1312
use crates_io_tarball::{process_tarball, TarballError};
1413
use crates_io_worker::BackgroundJob;
1514
use diesel::connection::DefaultLoadingMode;
@@ -42,6 +41,8 @@ use crate::views::{
4241
EncodableCrate, EncodableCrateDependency, GoodCrate, PublishMetadata, PublishWarnings,
4342
};
4443

44+
mod deleted;
45+
4546
const MISSING_RIGHTS_ERROR_MESSAGE: &str = "this crate exists but you don't seem to be an owner. \
4647
If you believe this is a mistake, perhaps you need \
4748
to accept an invitation to be an owner before \
@@ -87,21 +88,8 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
8788
let (existing_crate, auth) = {
8889
use diesel_async::RunQueryDsl;
8990

90-
let deleted_crate: Option<(String, DateTime<Utc>)> = deleted_crates::table
91-
.filter(canon_crate_name(deleted_crates::name).eq(canon_crate_name(&metadata.name)))
92-
.filter(deleted_crates::available_at.gt(Utc::now()))
93-
.select((deleted_crates::name, deleted_crates::available_at))
94-
.first(&mut conn)
95-
.await
96-
.optional()?;
97-
98-
if let Some(deleted_crate) = deleted_crate {
99-
return Err(bad_request(format!(
100-
"A crate with the name `{}` was recently deleted. Reuse of this name will be available after {}.",
101-
deleted_crate.0,
102-
deleted_crate.1.to_rfc3339_opts(SecondsFormat::Secs, true)
103-
)));
104-
}
91+
// Check that this publish doesn't conflict with any deleted crates.
92+
deleted::validate(&mut conn, &metadata.name, &semver).await?;
10593

10694
// this query should only be used for the endpoint scope calculation
10795
// since a race condition there would only cause `publish-new` instead of
Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
use chrono::{DateTime, SecondsFormat, Utc};
2+
use diesel_async::AsyncPgConnection;
3+
use http::StatusCode;
4+
use semver::Version;
5+
6+
use crate::{
7+
schema::deleted_crates,
8+
sql::canon_crate_name,
9+
util::{
10+
diesel::prelude::*,
11+
errors::{custom, AppResult},
12+
},
13+
};
14+
15+
/// Checks the given crate name and version against the deleted crates table,
16+
/// ensuring that the crate version is allowed to be published.
17+
///
18+
/// If the crate version cannot be published, a
19+
/// [`crate::util::errors::BoxedAppError`] will be returned with a user-oriented
20+
/// message suitable for being returned directly by a controller.
21+
pub async fn validate(
22+
conn: &mut AsyncPgConnection,
23+
name: &str,
24+
version: &Version,
25+
) -> AppResult<()> {
26+
use diesel_async::RunQueryDsl;
27+
28+
// Since the same crate may have been deleted multiple times, we need to
29+
// calculate the most restrictive set of conditions that the crate version
30+
// being published must adhere to; specifically: the latest available_at
31+
// time, and the highest min_version.
32+
let mut state = State::new();
33+
34+
// To do this, we need to iterate over all the relevant deleted crates.
35+
for (available_at, min_version) in deleted_crates::table
36+
.filter(canon_crate_name(deleted_crates::name).eq(canon_crate_name(name)))
37+
.select((deleted_crates::available_at, deleted_crates::min_version))
38+
.load::<(DateTime<Utc>, Option<String>)>(conn)
39+
.await?
40+
{
41+
state.observe_available_at(available_at);
42+
43+
// We shouldn't really end up with an invalid semver in the
44+
// `min_version` field, so we're going to silently swallow any errors
45+
// for now.
46+
if let Some(Ok(min_version)) = min_version.map(|v| Version::parse(&v)) {
47+
state.observe_min_version(min_version);
48+
}
49+
}
50+
51+
// Finally, we can check the given name and version against the built up
52+
// state and see if it passes.
53+
state.into_result(name, version, Utc::now())
54+
}
55+
56+
#[cfg_attr(test, derive(Clone))]
57+
struct State {
58+
available_at: Option<DateTime<Utc>>,
59+
min_version: Option<Version>,
60+
}
61+
62+
impl State {
63+
fn new() -> Self {
64+
Self {
65+
available_at: None,
66+
min_version: None,
67+
}
68+
}
69+
70+
fn observe_available_at(&mut self, available_at: DateTime<Utc>) {
71+
if let Some(current) = self.available_at {
72+
self.available_at = Some(std::cmp::max(current, available_at));
73+
} else {
74+
self.available_at = Some(available_at);
75+
}
76+
}
77+
78+
fn observe_min_version(&mut self, min_version: Version) {
79+
if let Some(current) = self.min_version.take() {
80+
self.min_version = Some(std::cmp::max(current, min_version));
81+
} else {
82+
self.min_version = Some(min_version);
83+
}
84+
}
85+
86+
fn into_result(self, name: &str, version: &Version, now: DateTime<Utc>) -> AppResult<()> {
87+
let mut messages = Vec::new();
88+
89+
if let Some(available_at) = self.available_at {
90+
if now < available_at {
91+
messages.push(format!(
92+
"Reuse of this name will be available after {}.",
93+
available_at.to_rfc3339_opts(SecondsFormat::Secs, true)
94+
));
95+
}
96+
}
97+
98+
if let Some(min_version) = self.min_version {
99+
if version < &min_version {
100+
messages.push(format!("To avoid conflicts with previously published versions of this crate, the minimum version that can be published is {min_version}."));
101+
}
102+
}
103+
104+
if messages.is_empty() {
105+
Ok(())
106+
} else {
107+
Err(custom(
108+
StatusCode::UNPROCESSABLE_ENTITY,
109+
format!(
110+
"A crate with the name `{name}` was previously deleted.\n\n* {}",
111+
messages.join("\n* "),
112+
),
113+
))
114+
}
115+
}
116+
}
117+
118+
#[cfg(test)]
119+
mod tests {
120+
use chrono::TimeDelta;
121+
use insta::assert_snapshot;
122+
123+
use super::*;
124+
125+
macro_rules! assert_result_status {
126+
($result:expr) => {{
127+
let response = $result.unwrap_err().response();
128+
assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY);
129+
130+
String::from_utf8(
131+
axum::body::to_bytes(response.into_body(), usize::MAX)
132+
.await
133+
.unwrap()
134+
.into(),
135+
)
136+
.unwrap()
137+
}};
138+
}
139+
140+
macro_rules! assert_result_failed {
141+
($result:expr) => {{
142+
let text = assert_result_status!($result);
143+
assert_snapshot!(text);
144+
}};
145+
($result:expr, $name:tt) => {{
146+
let text = assert_result_status!($result);
147+
assert_snapshot!($name, text);
148+
}};
149+
}
150+
151+
#[test]
152+
fn empty_state() {
153+
let state = State::new();
154+
155+
// Any combination of values should result in Ok, since there are no
156+
// deleted crates.
157+
for (name, version, now) in [
158+
("foo", "0.0.0", "2024-11-20T01:00:00Z"),
159+
("bar", "1.0.0", "1970-01-01T00:00:00Z"),
160+
] {
161+
assert_ok!(state.clone().into_result(
162+
name,
163+
&Version::parse(version).unwrap(),
164+
now.parse().unwrap()
165+
));
166+
}
167+
}
168+
169+
#[tokio::test]
170+
async fn available_at_only() {
171+
let available_at = "2024-11-20T00:00:00Z".parse().unwrap();
172+
let version = Version::parse("0.0.0").unwrap();
173+
174+
let mut state = State::new();
175+
state.observe_available_at(available_at);
176+
177+
// There should be no error for a crate after available_at.
178+
assert_ok!(state.clone().into_result(
179+
"foo",
180+
&version,
181+
available_at + TimeDelta::seconds(60)
182+
));
183+
184+
// Similarly, a crate actually _at_ available_at should be fine.
185+
assert_ok!(state.clone().into_result("foo", &version, available_at));
186+
187+
// But a crate one second earlier should error.
188+
assert_result_failed!(state.into_result(
189+
"foo",
190+
&version,
191+
available_at - TimeDelta::seconds(1)
192+
));
193+
}
194+
195+
#[tokio::test]
196+
async fn min_version_only() {
197+
let available_at = "2024-11-20T00:00:00Z".parse().unwrap();
198+
199+
let mut state = State::new();
200+
state.observe_available_at(available_at);
201+
202+
// Test the versions that we expect to pass.
203+
for (min_version, publish_version) in [
204+
("0.1.0", "0.1.0"),
205+
("0.1.0", "0.1.1"),
206+
("0.1.0", "0.2.0"),
207+
("0.1.0", "1.0.0"),
208+
("1.0.0", "1.0.0"),
209+
("1.0.0", "1.0.1"),
210+
("1.0.0", "2.0.0"),
211+
] {
212+
let mut state = state.clone();
213+
state.observe_min_version(Version::parse(min_version).unwrap());
214+
215+
assert_ok!(state.into_result(
216+
"foo",
217+
&Version::parse(publish_version).unwrap(),
218+
available_at
219+
));
220+
}
221+
222+
// Now test the versions that we expect to fail.
223+
for (min_version, publish_version) in [("0.1.0", "0.0.0"), ("1.0.0", "0.1.0")] {
224+
let mut state = state.clone();
225+
state.observe_min_version(Version::parse(min_version).unwrap());
226+
227+
assert_result_failed!(
228+
state.into_result(
229+
"foo",
230+
&Version::parse(publish_version).unwrap(),
231+
available_at,
232+
),
233+
publish_version
234+
);
235+
}
236+
}
237+
238+
#[tokio::test]
239+
async fn multiple_deleted() {
240+
// We won't repeat everything from the above here, but let's make sure
241+
// the most restrictive available_at and min_version are used when
242+
// multiple deleted crates are observed.
243+
let mut state = State::new();
244+
245+
let earlier_available_at = "2024-11-20T00:00:00Z".parse().unwrap();
246+
let later_available_at = "2024-11-21T12:00:00Z".parse().unwrap();
247+
state.observe_available_at(earlier_available_at);
248+
state.observe_available_at(later_available_at);
249+
state.observe_available_at(earlier_available_at);
250+
251+
let first_version = Version::parse("0.1.0").unwrap();
252+
let second_version = Version::parse("1.0.0").unwrap();
253+
state.observe_min_version(first_version.clone());
254+
state.observe_min_version(second_version.clone());
255+
state.observe_min_version(first_version.clone());
256+
257+
assert_ok!(state
258+
.clone()
259+
.into_result("foo", &second_version, later_available_at));
260+
261+
// Now the bad cases.
262+
for (name, version, now) in [
263+
("min_version", &first_version, later_available_at),
264+
("available_at", &second_version, earlier_available_at),
265+
("both", &first_version, earlier_available_at),
266+
] {
267+
assert_result_failed!(state.clone().into_result("foo", version, now), name);
268+
}
269+
}
270+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/controllers/krate/publish/deleted.rs
3+
expression: text
4+
---
5+
{"errors":[{"detail":"A crate with the name `foo` was previously deleted.\n\n* To avoid conflicts with previously published versions of this crate, the minimum version that can be published is 0.1.0."}]}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/controllers/krate/publish/deleted.rs
3+
expression: text
4+
---
5+
{"errors":[{"detail":"A crate with the name `foo` was previously deleted.\n\n* To avoid conflicts with previously published versions of this crate, the minimum version that can be published is 1.0.0."}]}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/controllers/krate/publish/deleted.rs
3+
expression: text
4+
---
5+
{"errors":[{"detail":"A crate with the name `foo` was previously deleted.\n\n* Reuse of this name will be available after 2024-11-21T12:00:00Z."}]}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/controllers/krate/publish/deleted.rs
3+
expression: text
4+
---
5+
{"errors":[{"detail":"A crate with the name `foo` was previously deleted.\n\n* Reuse of this name will be available after 2024-11-20T00:00:00Z."}]}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/controllers/krate/publish/deleted.rs
3+
expression: text
4+
---
5+
{"errors":[{"detail":"A crate with the name `foo` was previously deleted.\n\n* Reuse of this name will be available after 2024-11-21T12:00:00Z.\n* To avoid conflicts with previously published versions of this crate, the minimum version that can be published is 1.0.0."}]}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/controllers/krate/publish/deleted.rs
3+
expression: text
4+
---
5+
{"errors":[{"detail":"A crate with the name `foo` was previously deleted.\n\n* To avoid conflicts with previously published versions of this crate, the minimum version that can be published is 1.0.0."}]}

0 commit comments

Comments
 (0)