mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-24 08:01:38 +00:00
fix: scope capture deletion to the workspace in the request path (#10795)
* fix: scope capture deletion to the workspace in the request path Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: layer the capture fixture on base instead of duplicating it Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
-14
@@ -1,14 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n DELETE FROM \n capture\n WHERE \n id = $1\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "7d5a161d916cb8d1485f8d72e6b4044f505f49edcd2ebeee4a77eae737f48795"
|
||||
}
|
||||
+15
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n DELETE FROM\n capture\n WHERE\n id = $1\n AND workspace_id = $2\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "f1a6018b184967f0065847ce5e41f786a5451847ca5c95b93203b75b6d56ae34"
|
||||
}
|
||||
@@ -0,0 +1,55 @@
|
||||
//! `DELETE /w/{workspace}/capture/{id}` must stay inside the workspace in the URL.
|
||||
//!
|
||||
//! Capture ids come from one instance-wide sequence and the capture RLS policies
|
||||
//! key on the path segment only, never on `workspace_id` — so an id alone is not
|
||||
//! an authorization boundary. A member of one workspace can name any id and, if
|
||||
//! the row's path happens to sit inside their grants (`u/<their username>/…`,
|
||||
//! `g/<their group>/…`, a same-named folder), reach a row belonging to a
|
||||
//! workspace they are not a member of.
|
||||
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_test_utils::*;
|
||||
|
||||
async fn capture_workspace(db: &Pool<Postgres>, id: i64) -> Option<String> {
|
||||
sqlx::query_scalar::<_, String>("SELECT workspace_id FROM capture WHERE id = $1")
|
||||
.bind(id)
|
||||
.fetch_optional(db)
|
||||
.await
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[sqlx::test(fixtures("base", "capture_cross_workspace"))]
|
||||
async fn delete_capture_is_confined_to_the_url_workspace(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let base = format!(
|
||||
"http://localhost:{}/api/w/test-workspace",
|
||||
server.addr.port()
|
||||
);
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
// Capture 1 lives in test-workspace-2, which this member has no access to.
|
||||
let resp = client
|
||||
.delete(format!("{base}/capture/1"))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN_2")
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status();
|
||||
assert_eq!(
|
||||
capture_workspace(&db, 1).await.as_deref(),
|
||||
Some("test-workspace-2"),
|
||||
"capture of another workspace was deleted (status {status})"
|
||||
);
|
||||
|
||||
// The member's own capture in test-workspace still deletes.
|
||||
let resp = client
|
||||
.delete(format!("{base}/capture/2"))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN_2")
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
|
||||
assert_eq!(capture_workspace(&db, 2).await, None);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
-- Layers on `base`, which already provides test-workspace and `test-user-2`, a
|
||||
-- plain non-admin member of it (token SECRET_TOKEN_2).
|
||||
--
|
||||
-- Adds a second workspace holding a capture under `u/test-user-2/…`: the capture
|
||||
-- policies key on the path segment alone, so that path is inside the member's
|
||||
-- grants in *every* workspace.
|
||||
|
||||
INSERT INTO workspace (id, name, owner) VALUES
|
||||
('test-workspace-2', 'test-workspace-2', 'test-user');
|
||||
|
||||
INSERT INTO workspace_key(workspace_id, kind, key) VALUES
|
||||
('test-workspace-2', 'cloud', 'test-key-2');
|
||||
|
||||
INSERT INTO workspace_settings (workspace_id) VALUES
|
||||
('test-workspace-2');
|
||||
|
||||
INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
|
||||
('test-workspace-2', 'all', 'All users', '{}');
|
||||
|
||||
INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES
|
||||
('test-workspace-2', 'test@windmill.dev', 'test-user', true, 'Admin');
|
||||
|
||||
INSERT INTO capture (id, workspace_id, path, created_by, main_args, is_flow, trigger_kind) VALUES
|
||||
(1, 'test-workspace-2', 'u/test-user-2/victim', 'test-user', '{"secret": "other workspace payload"}'::jsonb, false, 'webhook'),
|
||||
(2, 'test-workspace', 'u/test-user-2/own', 'test-user-2', '{}'::jsonb, false, 'webhook');
|
||||
@@ -742,17 +742,21 @@ async fn get_capture(
|
||||
async fn delete_capture(
|
||||
authed: ApiAuthed,
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
Path((_, id)): Path<(String, i64)>,
|
||||
Path((w_id, id)): Path<(String, i64)>,
|
||||
) -> Result<()> {
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
// capture RLS only keys on the path segment, so without workspace_id an id from
|
||||
// another workspace whose path collides with the caller's grants would be deleted.
|
||||
sqlx::query!(
|
||||
r#"
|
||||
DELETE FROM
|
||||
DELETE FROM
|
||||
capture
|
||||
WHERE
|
||||
WHERE
|
||||
id = $1
|
||||
AND workspace_id = $2
|
||||
"#,
|
||||
id
|
||||
id,
|
||||
&w_id,
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
Reference in New Issue
Block a user