mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-09 00:04:10 +00:00
fix: report a NUL-poisoned draft on move instead of 500ing
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem Lemouel
co-authored by
Claude Opus 5
Claude Opus 5
parent
e895cdd4db
commit
c04942e341
+66
@@ -0,0 +1,66 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ = $2 AND email = $4) as \"at_target!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\" ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "at_target!",
|
||||
"type_info": "Bool"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "poisoned!",
|
||||
"type_info": "Bool"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
{
|
||||
"Custom": {
|
||||
"name": "draft_kind",
|
||||
"kind": {
|
||||
"Enum": [
|
||||
"script",
|
||||
"flow",
|
||||
"app",
|
||||
"raw_app",
|
||||
"resource",
|
||||
"variable",
|
||||
"trigger_schedule",
|
||||
"trigger_webhook",
|
||||
"trigger_default_email",
|
||||
"trigger_email",
|
||||
"trigger_http",
|
||||
"trigger_websocket",
|
||||
"trigger_postgres",
|
||||
"trigger_kafka",
|
||||
"trigger_nats",
|
||||
"trigger_mqtt",
|
||||
"trigger_sqs",
|
||||
"trigger_gcp",
|
||||
"trigger_azure",
|
||||
"trigger_poll",
|
||||
"trigger_cli",
|
||||
"trigger_nextcloud",
|
||||
"trigger_google",
|
||||
"trigger_github",
|
||||
"data_pipeline",
|
||||
"trigger_amqp"
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
"Text",
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null,
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "25ce9aff8844fbfc1a9e16ac7c4b9d0c37c27da9f9768f6520ac3c96b5765840"
|
||||
}
|
||||
+62
@@ -0,0 +1,62 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE draft\n SET path = $3,\n value = to_json(\n jsonb_set(\n CASE WHEN $7::text IS NULL THEN to_jsonb(value)\n ELSE jsonb_set(to_jsonb(value), ARRAY['summary'], to_jsonb($7::text))\n END,\n ARRAY[$5::text], to_jsonb($3::text), false\n )\n )\n WHERE workspace_id = $1\n AND path = $2\n AND typ = $4\n AND email = $6\n -- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded\n -- here so the statement can't 500; reported below instead. Unlike the\n -- passive carry, rewriting the value IS this operation, so skipping it\n -- silently would move the row and leave its typed path stale.\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0\n -- Skipped on a summary-only edit, where the \"target\" row is this\n -- row and the guard would refuse the update against itself.\n AND ($2 = $3 OR NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ = $4 AND o.email = $6\n ))\n RETURNING id",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "id",
|
||||
"type_info": "Int8"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text",
|
||||
"Text",
|
||||
{
|
||||
"Custom": {
|
||||
"name": "draft_kind",
|
||||
"kind": {
|
||||
"Enum": [
|
||||
"script",
|
||||
"flow",
|
||||
"app",
|
||||
"raw_app",
|
||||
"resource",
|
||||
"variable",
|
||||
"trigger_schedule",
|
||||
"trigger_webhook",
|
||||
"trigger_default_email",
|
||||
"trigger_email",
|
||||
"trigger_http",
|
||||
"trigger_websocket",
|
||||
"trigger_postgres",
|
||||
"trigger_kafka",
|
||||
"trigger_nats",
|
||||
"trigger_mqtt",
|
||||
"trigger_sqs",
|
||||
"trigger_gcp",
|
||||
"trigger_azure",
|
||||
"trigger_poll",
|
||||
"trigger_cli",
|
||||
"trigger_nextcloud",
|
||||
"trigger_google",
|
||||
"trigger_github",
|
||||
"data_pipeline",
|
||||
"trigger_amqp"
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
"Text",
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "c902cf38cea14284032312f833ed84637926113f1f6aef6b58e24c88930f1432"
|
||||
}
|
||||
@@ -1404,8 +1404,9 @@ async fn update_flow(
|
||||
.await?;
|
||||
if outcome.left_behind > 0 {
|
||||
tracing::warn!(
|
||||
"{} flow draft(s) stranded at {flow_path}: their owner already has a draft at {}",
|
||||
"{} of {} flow draft(s) stranded at {flow_path}: their owner already has a draft at {}",
|
||||
outcome.left_behind,
|
||||
outcome.moved + outcome.left_behind,
|
||||
&nf.path
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2157,8 +2157,9 @@ async fn create_script_internal<'c>(
|
||||
.await?;
|
||||
if outcome.left_behind > 0 {
|
||||
tracing::warn!(
|
||||
"{} script draft(s) stranded at {p_path}: their owner already has a draft at {}",
|
||||
"{} of {} script draft(s) stranded at {p_path}: their owner already has a draft at {}",
|
||||
outcome.left_behind,
|
||||
outcome.moved + outcome.left_behind,
|
||||
&ns.path
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3469,8 +3469,9 @@ async fn update_app_internal<'a>(
|
||||
.await?;
|
||||
if outcome.left_behind > 0 {
|
||||
tracing::warn!(
|
||||
"{} app draft(s) stranded at {path}: their owner already has a draft at {npath}",
|
||||
outcome.left_behind
|
||||
"{} of {} app draft(s) stranded at {path}: their owner already has a draft at {npath}",
|
||||
outcome.left_behind,
|
||||
outcome.moved + outcome.left_behind
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -842,6 +842,11 @@ async fn move_draft(
|
||||
AND path = $2
|
||||
AND typ = $4
|
||||
AND email = $6
|
||||
-- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded
|
||||
-- here so the statement can't 500; reported below instead. Unlike the
|
||||
-- passive carry, rewriting the value IS this operation, so skipping it
|
||||
-- silently would move the row and leave its typed path stale.
|
||||
AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0
|
||||
-- Skipped on a summary-only edit, where the "target" row is this
|
||||
-- row and the guard would refuse the update against itself.
|
||||
AND ($2 = $3 OR NOT EXISTS (
|
||||
@@ -861,17 +866,28 @@ async fn move_draft(
|
||||
.await?;
|
||||
|
||||
if moved.is_none() {
|
||||
let exists_at_target = sqlx::query_scalar!(
|
||||
"SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $2 AND typ = $3 AND email = $4",
|
||||
let row = sqlx::query!(
|
||||
r#"SELECT
|
||||
EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3
|
||||
AND typ = $2 AND email = $4) as "at_target!",
|
||||
EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5
|
||||
AND typ = $2 AND email = $4
|
||||
AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0
|
||||
) as "poisoned!" "#,
|
||||
&w_id,
|
||||
new_path,
|
||||
kind as UserDraftItemKind,
|
||||
new_path,
|
||||
&authed.email,
|
||||
path,
|
||||
)
|
||||
.fetch_optional(&db)
|
||||
.await?
|
||||
.is_some();
|
||||
return Err(Error::BadRequest(if exists_at_target && new_path != path {
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
return Err(Error::BadRequest(if row.poisoned {
|
||||
format!(
|
||||
"'{path}' contains a NUL character and predates the sanitizer, so it cannot be \
|
||||
moved. Reopen it, re-save to rewrite it cleanly, then move it."
|
||||
)
|
||||
} else if row.at_target && new_path != path {
|
||||
format!("You already have a draft at '{new_path}'")
|
||||
} else {
|
||||
format!("You have no draft at '{path}'")
|
||||
|
||||
Reference in New Issue
Block a user