mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: a no-op routed discard names the destination only to someone who could write there
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5b4d745b55
commit
1cd0ee680b
@@ -396,29 +396,43 @@ async fn test_a_retried_discard_still_names_the_destination(
|
||||
|
||||
rename(port, HEAD_HASH, "u/test-user/follow_b").await?;
|
||||
|
||||
let discard = || async {
|
||||
client
|
||||
.post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/drafts/update/script/u/test-user/follow_a"
|
||||
))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN")
|
||||
.json(&json!({ "value": null }))
|
||||
.send()
|
||||
.await?
|
||||
.json::<Value>()
|
||||
.await
|
||||
let discard = |token: &'static str| {
|
||||
let client = client.clone();
|
||||
async move {
|
||||
client
|
||||
.post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/drafts/update/script/u/test-user/follow_a"
|
||||
))
|
||||
.header("Authorization", format!("Bearer {token}"))
|
||||
.json(&json!({ "value": null }))
|
||||
.send()
|
||||
.await?
|
||||
.json::<Value>()
|
||||
.await
|
||||
}
|
||||
};
|
||||
|
||||
let first = discard().await?;
|
||||
let first = discard("SECRET_TOKEN").await?;
|
||||
assert_eq!(first["status"], "saved", "{first}");
|
||||
assert_eq!(first["path"], "u/test-user/follow_b", "{first}");
|
||||
|
||||
let retry = discard().await?;
|
||||
let retry = discard("SECRET_TOKEN").await?;
|
||||
assert_eq!(
|
||||
retry["status"], "saved",
|
||||
"the retry was not idempotent: {retry}"
|
||||
);
|
||||
assert_eq!(retry["path"], "u/test-user/follow_b", "{retry}");
|
||||
|
||||
// A third user has no draft on this item, so their discard deletes nothing and the
|
||||
// destination is a move record and nothing else: it is answered only to a caller who
|
||||
// could write there.
|
||||
let other = discard("SECRET_TOKEN_3").await?;
|
||||
assert_eq!(other["status"], "saved", "{other}");
|
||||
assert_eq!(
|
||||
other["path"],
|
||||
Value::Null,
|
||||
"a user who cannot write the destination was told where the item went: {other}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -579,13 +579,27 @@ async fn update_draft(
|
||||
let now = sqlx::query_scalar!(r#"SELECT now() as "now!""#)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
// A retry of a routed discard whose answer was lost lands here: the row is
|
||||
// gone but the editor is still on the path the item left, so it needs the
|
||||
// destination as much as the first attempt did. Unlike the arm above there is
|
||||
// no deleted row proving the caller ever held that draft, and an own discard
|
||||
// skips the write check, so name the destination only to someone who could
|
||||
// write there. Otherwise any member could read `draft_move` by discarding at
|
||||
// a guessed path.
|
||||
let disclosed = match moved_to {
|
||||
Some(dest) => {
|
||||
match require_can_write_path(&authed, &db, &user_db, &w_id, kind, &dest).await {
|
||||
Ok(()) => Some(dest),
|
||||
Err(Error::NotAuthorized(_)) | Err(Error::BadRequest(_)) => None,
|
||||
Err(e) => return Err(e),
|
||||
}
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
Ok(Json(SaveDraftResponse {
|
||||
status: SaveDraftStatus::Saved,
|
||||
current_timestamp: now,
|
||||
// A retry of a routed discard whose answer was lost lands here: the row
|
||||
// is gone but the editor is still on the path the item left, so it needs
|
||||
// the destination as much as the first attempt did.
|
||||
path: moved_to,
|
||||
path: disclosed,
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user