starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-25 16:02:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: lock the app row for the kind guard and route MCP away from raw apps

Browse Source
This commit is contained in:
Ruben Fiszel
2026-08-04 07:25:59 +00:00
parent a88cf31b5b
commit 781e5d1d4b
11 changed files with 103 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/.sqlx/query-221ebaace16df131a8a75de233869bcce087fc5630dc74c62831847959f2f345.json
Generated
-23
View File
@@ -1,23 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "SELECT app_version.raw_app FROM app\n JOIN app_version ON app_version.id = app.versions[array_upper(app.versions, 1)]\n WHERE app.path = $1 AND app.workspace_id = $2",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "raw_app",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
false
]
},
"hash": "221ebaace16df131a8a75de233869bcce087fc5630dc74c62831847959f2f345"
}
backend/.sqlx/query-925359a7b0fe8f4ed17f072f7299c7d503eac045a6d5d13ac3c953a3fbdac0ff.json
Generated
+23
View File
@@ -0,0 +1,23 @@
{
"db_name": "PostgreSQL",
"query": "SELECT versions[array_upper(versions, 1)] FROM app\n WHERE path = $1 AND workspace_id = $2 FOR UPDATE",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "versions",
"type_info": "Int8"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
null
]
},
"hash": "925359a7b0fe8f4ed17f072f7299c7d503eac045a6d5d13ac3c953a3fbdac0ff"
}
backend/tests/dependency_map.rs
+1
View File
@@ -515,6 +515,7 @@ def main():
preserve_on_behalf_of: None,
labels: None,
skip_draft_deletion: None,
allow_kind_change: None,
})
.send()
.await
backend/windmill-api-integration-tests/tests/apps.rs
+9
View File
@@ -379,6 +379,13 @@ async fn test_raw_app_kind_is_not_flipped_by_update(db: Pool<Postgres>) -> anyho
.unwrap();
assert_eq!(resp.status(), 201, "create: {}", resp.text().await?);
let versions_of = |path: &'static str| async move {
let resp = authed_get(port, "get/p", path).await;
let body = resp.json::<serde_json::Value>().await.unwrap();
body["versions"].as_array().unwrap().len()
};
let raw_versions = versions_of(raw_path).await;
// Neither endpoint may deploy a value onto an app of the other kind.
let resp = authed(client().post(format!("{base}/update/{raw_path}")))
.json(&json!({ "value": { "grid": [] } }))
@@ -390,6 +397,8 @@ async fn test_raw_app_kind_is_not_flipped_by_update(db: Pool<Postgres>) -> anyho
resp.text().await?.contains("is a raw app"),
"expected the low-code update of a raw app to be refused"
);
// The refusal has to land before the version insert, not roll one back.
assert_eq!(versions_of(raw_path).await, raw_versions);
let resp = authed(client().post(format!("{base}/update_raw/{low_code_path}")))
.multipart(raw_app_form(low_code_path))
backend/windmill-api/openapi.yaml
+1
View File
@@ -12259,6 +12259,7 @@ paths:
summary: update app
operationId: updateApp
x-mcp-tool: true
x-mcp-instructions: "Low-code apps only. An app whose `raw_app` field (from getAppByPath) is true is a raw (full-code) app: its value holds source files that must be compiled to a js/css bundle, which this tool cannot upload, so updating one here is refused. Edit a raw app in its editor at /apps_raw/edit/<path> instead."
x-mcp-tool-include-fields:
- path
- value
backend/windmill-api/src/apps.rs
+29 -19
View File
@@ -366,9 +366,8 @@ pub struct EditApp {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub skip_draft_deletion: Option<bool>,
/// Caller-intent flag: when true this deploy may switch the app between
/// low-code and raw. Only a deliberate conversion sets it (restoring a
/// version from the app's other-kind history); every other write is
/// refused rather than converted. Transient — never persisted.
/// low-code and raw (see `update_app_internal`). Transient — never
/// persisted.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub allow_kind_change: Option<bool>,
}
@@ -2425,32 +2424,43 @@ async fn update_app_internal<'a>(
let mut tx = user_db.clone().begin(&authed).await?;
// `app_version.raw_app` is whatever the endpoint that wrote the version says
// it is, so deploying a value through the wrong one converts the app in
// place: a raw app updated via /apps/update becomes a low-code app whose
// value no editor can render, and its js/css bundle (keyed on the previous
// version id) is orphaned. Refuse instead of letting the write land — the
// caller reached for the wrong endpoint, unless it says otherwise.
// `app_version.raw_app` is set by whichever endpoint writes the version, so a
// value deployed through the wrong one converts the app and strands its bundle.
// `FOR UPDATE` holds the app row until this transaction appends its own version,
// so a concurrent deploy of the other kind can't land between check and append.
if ns.value.is_some() && !ns.allow_kind_change.unwrap_or(false) {
let deployed_raw_app = sqlx::query_scalar!(
"SELECT app_version.raw_app FROM app
JOIN app_version ON app_version.id = app.versions[array_upper(app.versions, 1)]
WHERE app.path = $1 AND app.workspace_id = $2",
let deployed_version = sqlx::query_scalar!(
"SELECT versions[array_upper(versions, 1)] FROM app
WHERE path = $1 AND workspace_id = $2 FOR UPDATE",
path,
w_id
)
.fetch_optional(&mut *tx)
.await?;
.await?
.flatten();
let deployed_raw_app = match deployed_version {
// A separate statement: it needs the head the lock above pinned, not
// the snapshot the locking statement started from.
Some(version) => {
sqlx::query_scalar!("SELECT raw_app FROM app_version WHERE id = $1", version)
.fetch_optional(&mut *tx)
.await?
}
None => None,
};
if deployed_raw_app.is_some_and(|deployed| deployed != raw_app) {
let (kind, endpoint) = if raw_app {
("a low-code app", "/apps/update")
// Name the folder suffix too: a sync push picks the endpoint from the
// repo layout, so its operator has no endpoint to swap, only a folder.
let (kind, endpoint, folder) = if raw_app {
("a low-code app", "/apps/update", ".app")
} else {
("a raw app", "/apps/update_raw")
("a raw app", "/apps/update_raw", ".raw_app")
};
return Err(Error::BadRequest(format!(
"App {path} is {kind}: deploying a value to it through the other kind's endpoint \
would convert it and strand its bundle. Use {endpoint} instead, or set \
allow_kind_change to convert it on purpose."
would convert it and strand its bundle. Deploy it through {endpoint} instead \
(from a synced repo, from a `{folder}` folder), or set allow_kind_change to \
convert it on purpose."
)));
}
}
backend/windmill-api/src/mcp/auto_generated_endpoints.rs
+5 -1
View File
@@ -1113,7 +1113,7 @@ Creates a new version of an existing script when called with the same path and t
EndpointTool {
name: Cow::Borrowed("updateApp"),
description: Cow::Borrowed("update app"),
instructions: Cow::Borrowed(""),
instructions: Cow::Borrowed("Low-code apps only. An app whose `raw_app` field (from getAppByPath) is true is a raw (full-code) app: its value holds source files that must be compiled to a js/css bundle, which this tool cannot upload, so updating one here is refused. Edit a raw app in its editor at /apps_raw/edit/<path> instead."),
path: Cow::Borrowed("/w/{workspace}/apps/update/{path}"),
method: Cow::Borrowed("POST"),
path_params_schema: Some(serde_json::json!({
@@ -1521,6 +1521,10 @@ Creates a new version of an existing script when called with the same path and t
"type": "boolean",
"description": "is the job skipped"
},
"resolved": {
"type": "boolean",
"description": "filter on whether a failure has been marked as handled. true keeps only resolved failures, false hides them"
},
"is_flow_step": {
"type": "boolean",
"description": "is the job a flow step"
frontend/src/lib/components/apps/editor/AppDeploymentHistory.svelte
+6 -7
View File
@@ -17,13 +17,13 @@
historyBrowserDrawerOpen = true
}
// Picking a version is an explicit choice of what the app should become, so a
// version from before the app changed kind is allowed to convert it back —
// that is the only supported way to undo an accidental conversion.
async function updateApp(app: any) {
if (app.raw_app) {
// Restoring a raw app version means re-bundling its sources: `updateApp`
// would write a low-code version and leave the bundle behind.
// A raw version restores through the raw endpoint, which rebuilds the
// bundle from its sources. `allowKindChange` so picking the last version
// from before an app was converted to low-code restores it as raw — the
// supported way to undo such a conversion. The low-code direction stays
// refused: it has no bundle to write, so it would only break the app.
await deployRawAppValue({
workspace: $workspaceStore!,
path: app.path,
@@ -38,8 +38,7 @@
workspace: $workspaceStore!,
path: app.path,
requestBody: {
...app,
allow_kind_change: true
...app
}
})
}
frontend/src/lib/components/apps/editor/AppJsonEditor.svelte
+11 -3
View File
@@ -51,7 +51,17 @@
loading = false
}
// The button drops a rejection, so failures — invalid JSON, a raw app whose
// sources don't bundle — have to surface here or Deploy looks like a no-op.
export async function saveApp() {
try {
await deploy()
} catch (err: any) {
sendUserToast(`Could not save app: ${err.body ?? err.message}`, true)
}
}
async function deploy() {
const parsed = JSON.parse(code)
if (isDraftOnly) {
// No deployed row — `updateApp` would 404. Route through the syncer
@@ -67,9 +77,7 @@
dispatch('change')
sendUserToast('Draft saved')
} else if (isRawApp) {
// A raw app's value holds its sources, not a renderable grid: it has to be
// re-bundled and written through the raw endpoint. `updateApp` would flag
// the new version low-code and leave the bundle on the old one.
// A raw app's value holds its sources, so deploying it means re-bundling.
await deployRawAppValue({
workspace: $workspaceStore!,
path,
frontend/src/lib/mcpEndpointTools.ts
+5 -1
View File
@@ -1121,7 +1121,7 @@ export const mcpEndpointTools: EndpointTool[] = [
{
name: "updateApp",
description: "update app",
instructions: "",
instructions: "Low-code apps only. An app whose `raw_app` field (from getAppByPath) is true is a raw (full-code) app: its value holds source files that must be compiled to a js/css bundle, which this tool cannot upload, so updating one here is refused. Edit a raw app in its editor at /apps_raw/edit/<path> instead.",
path: "/w/{workspace}/apps/update/{path}",
method: "POST",
pathParamsSchema: {
@@ -1529,6 +1529,10 @@ export const mcpEndpointTools: EndpointTool[] = [
"type": "boolean",
"description": "is the job skipped"
},
"resolved": {
"type": "boolean",
"description": "filter on whether a failure has been marked as handled. true keeps only resolved failures, false hides them"
},
"is_flow_step": {
"type": "boolean",
"description": "is the job a flow step"
frontend/src/lib/rawAppDeploy.ts
+13 -11
View File
@@ -18,15 +18,15 @@ import { bundleRawAppDraft } from '$lib/components/copilot/chat/global/rawAppBun
import type { AppDraftValue } from '$lib/components/copilot/chat/global/workspaceItems'
import { updateRawAppPolicy } from '$lib/components/raw_apps/rawAppPolicy'
import { DEFAULT_DATA as DEFAULT_RAW_APP_DATA } from '$lib/components/raw_apps/dataTableRefUtils'
import { appSourceToDraftValue } from '$lib/components/raw_apps/rawAppDraftValue'
import {
appSourceToDraftValue,
normalizeRawAppData
} from '$lib/components/raw_apps/rawAppDraftValue'
import { stateSnapshot } from '$lib/svelte5Utils.svelte'
/**
* Deploy an explicit raw-app value — one the user edited as JSON, or one
* restored from a previous version — onto an already deployed raw app. Going
* through AppService.updateApp instead would write a version flagged low-code
* and leave the bundle behind on the old version, so the app stops rendering;
* the backend rejects that mismatch.
* restored from a previous version — onto a deployed app.
*/
export async function deployRawAppValue({
workspace,
@@ -45,15 +45,15 @@ export async function deployRawAppValue({
policy?: Policy
customPath?: string | null
deploymentMessage?: string
/** Let this deploy turn a low-code app into a raw one (restoring a version
* from before an accidental conversion). Off for every ordinary deploy. */
/** Let this deploy turn a low-code app into a raw one. Off for every ordinary
* deploy — see the backend's `allow_kind_change`. */
allowKindChange?: boolean
}): Promise<void> {
// The value often comes straight out of a `$state` field, and the bundler
// runs in an iframe: postMessage refuses to clone a state proxy.
const plainValue = stateSnapshot(value) as any
const files = (plainValue?.files ?? {}) as Record<string, string>
const runnables = plainValue?.runnables ?? {}
const plainValue = (stateSnapshot(value) ?? {}) as Record<string, any>
const files = (plainValue.files ?? {}) as Record<string, string>
const runnables = plainValue.runnables ?? {}
// The value carries the runnables, so the policy's triggerables have to be
// recomputed from it or the deployed app can't call what it now contains.
const policy = (await updateRawAppPolicy(runnables, currentPolicy)) as Policy
@@ -69,7 +69,9 @@ export async function deployRawAppValue({
path,
formData: {
app: {
value: { files, runnables, data: plainValue?.data ?? { ...DEFAULT_RAW_APP_DATA } },
// Through `normalizeRawAppData`, like every other deploy path: an old
// version can still carry the pre-`data` datatable shapes.
value: { files, runnables, data: normalizeRawAppData(plainValue) },
summary: summary ?? '',
policy,
deployment_message: deploymentMessage,