mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-27 08:02:39 +00:00
fix: change an app's access mode under the deploy row lock (#11965)
* fix: change an app's access mode under the deploy row lock Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: defer a mode change to deploy on an app run as another user Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: hold the access toggle while its change is in flight Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * revert: leave the deploy panel's access toggle to the settings drawer PR Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: keep the caller's draft on a mode-only app update Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: refuse a builder's mode-only change on an unsandboxed app Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
28d2556c94
commit
725b15f546
@@ -0,0 +1,311 @@
|
||||
//! `EditApp.execution_mode`: changing who may open an app without sending its policy.
|
||||
//!
|
||||
//! The change is applied to the deployed policy under the row lock a deploy takes, so it keeps
|
||||
//! every other field a concurrent deploy wrote. It is held to the rules a policy write is held
|
||||
//! to, and it is refused to a caller who could not keep the app's run-as user, since a policy
|
||||
//! write from them would rebind it.
|
||||
//!
|
||||
//! One test: the protection rules and the workspace roles are cached per workspace id across
|
||||
//! the process, and every test here uses `test-workspace`.
|
||||
//!
|
||||
//! Users from the `base` fixture:
|
||||
//! test-user (admin, token SECRET_TOKEN)
|
||||
//! test-user-2 (non-admin, token SECRET_TOKEN_2)
|
||||
//! test-user-3 (made a compose-only operator below, token SECRET_TOKEN_3)
|
||||
|
||||
use reqwest::multipart;
|
||||
use serde_json::{json, Value};
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_common::workspaces::invalidate_protection_rules_cache;
|
||||
use windmill_test_utils::*;
|
||||
|
||||
const ADMIN: &str = "SECRET_TOKEN";
|
||||
const USER: &str = "SECRET_TOKEN_2";
|
||||
const BUILDER: &str = "SECRET_TOKEN_3";
|
||||
|
||||
fn client() -> reqwest::Client {
|
||||
reqwest::Client::new()
|
||||
}
|
||||
|
||||
async fn update(ws: &str, path: &str, token: &str, body: Value) -> anyhow::Result<(u16, String)> {
|
||||
let resp = client()
|
||||
.post(format!("{ws}/apps/update/{path}"))
|
||||
.bearer_auth(token)
|
||||
.json(&body)
|
||||
.send()
|
||||
.await?;
|
||||
Ok((resp.status().as_u16(), resp.text().await?))
|
||||
}
|
||||
|
||||
async fn create(ws: &str, token: &str, path: &str, policy: Value) -> anyhow::Result<()> {
|
||||
let resp = client()
|
||||
.post(format!("{ws}/apps/create"))
|
||||
.bearer_auth(token)
|
||||
.json(&json!({ "path": path, "summary": "", "value": {}, "policy": policy }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201, "create {path}: {}", resp.text().await?);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// An anonymous, sandboxed raw app with no runnables, as a builder may deploy one.
|
||||
async fn create_raw(ws: &str, token: &str, path: &str) -> anyhow::Result<()> {
|
||||
let app = json!({
|
||||
"path": path,
|
||||
"summary": "",
|
||||
"value": { "files": {}, "runnables": {} },
|
||||
"policy": { "execution_mode": "anonymous", "sandbox": true, "triggerables_v2": {} },
|
||||
});
|
||||
let form = multipart::Form::new()
|
||||
.part(
|
||||
"app",
|
||||
multipart::Part::text(app.to_string()).mime_str("application/json")?,
|
||||
)
|
||||
.part("js", multipart::Part::text("console.log('app')"));
|
||||
let resp = client()
|
||||
.post(format!("{ws}/apps/create_raw"))
|
||||
.bearer_auth(token)
|
||||
.multipart(form)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
201,
|
||||
"create_raw {path}: {}",
|
||||
resp.text().await?
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn policy(db: &Pool<Postgres>, path: &str) -> anyhow::Result<Value> {
|
||||
Ok(sqlx::query_scalar(
|
||||
"SELECT policy FROM app WHERE path = $1 AND workspace_id = 'test-workspace'",
|
||||
)
|
||||
.bind(path)
|
||||
.fetch_one(db)
|
||||
.await?)
|
||||
}
|
||||
|
||||
fn triggerable(key: &str) -> Value {
|
||||
json!({ key: { "static_inputs": {}, "one_of_inputs": {} } })
|
||||
}
|
||||
|
||||
#[sqlx::test(fixtures("base"))]
|
||||
async fn test_access_mode_only_update(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
invalidate_protection_rules_cache("test-workspace");
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let ws = format!(
|
||||
"http://localhost:{}/api/w/test-workspace",
|
||||
server.addr.port()
|
||||
);
|
||||
|
||||
// ========================================
|
||||
// 1. The mode alone changes; every other field of the deployed policy stays.
|
||||
// ========================================
|
||||
const SHARED: &str = "u/test-user-2/shared";
|
||||
create(
|
||||
&ws,
|
||||
ADMIN,
|
||||
SHARED,
|
||||
json!({
|
||||
"execution_mode": "publisher",
|
||||
"triggerables_v2": triggerable("c:script/u/test-user/noop"),
|
||||
"sandbox": true,
|
||||
"allowed_s3_keys": [{ "s3_path": "a/b.txt" }],
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
let before = policy(&db, SHARED).await?;
|
||||
assert_eq!(before["on_behalf_of"], "u/test-user");
|
||||
|
||||
let (status, body) =
|
||||
update(&ws, SHARED, ADMIN, json!({ "execution_mode": "anonymous" })).await?;
|
||||
assert_eq!(status, 200, "{body}");
|
||||
let mut after = policy(&db, SHARED).await?;
|
||||
assert_eq!(after["execution_mode"], "anonymous");
|
||||
after["execution_mode"] = before["execution_mode"].clone();
|
||||
assert_eq!(
|
||||
after, before,
|
||||
"a mode-only change must keep every other policy field"
|
||||
);
|
||||
|
||||
// Sending a policy too is ambiguous about which mode wins.
|
||||
let (status, _) = update(
|
||||
&ws,
|
||||
SHARED,
|
||||
ADMIN,
|
||||
json!({ "execution_mode": "publisher", "policy": { "execution_mode": "anonymous" } }),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(status, 400);
|
||||
|
||||
// ========================================
|
||||
// 2. Someone who cannot keep the run-as user cannot change the mode of an app that runs
|
||||
// as someone else, even with write access to it.
|
||||
// ========================================
|
||||
let (status, body) =
|
||||
update(&ws, SHARED, USER, json!({ "execution_mode": "publisher" })).await?;
|
||||
assert_eq!(status, 403, "{body}");
|
||||
assert!(body.contains("wm_deployers"), "{body}");
|
||||
assert_eq!(policy(&db, SHARED).await?["execution_mode"], "anonymous");
|
||||
|
||||
// ========================================
|
||||
// 3. A policy without a mode keeps the deployed one.
|
||||
// ========================================
|
||||
const OWN: &str = "u/test-user-2/own";
|
||||
create(&ws, USER, OWN, json!({ "execution_mode": "publisher" })).await?;
|
||||
sqlx::query(
|
||||
"INSERT INTO draft (workspace_id, path, typ, value, email)
|
||||
VALUES ('test-workspace', $1, 'app', '{}', 'test2@windmill.dev')",
|
||||
)
|
||||
.bind(OWN)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
let (status, body) = update(&ws, OWN, USER, json!({ "execution_mode": "anonymous" })).await?;
|
||||
assert_eq!(status, 200, "{body}");
|
||||
let drafts: i64 = sqlx::query_scalar(
|
||||
"SELECT count(*) FROM draft WHERE workspace_id = 'test-workspace' AND path = $1",
|
||||
)
|
||||
.bind(OWN)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(drafts, 1, "a mode-only change must keep the caller's draft");
|
||||
let (status, body) = update(
|
||||
&ws,
|
||||
OWN,
|
||||
USER,
|
||||
json!({ "value": { "v": 2 }, "policy": { "triggerables_v2": triggerable("c:script/f/x/new") } }),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(status, 200, "{body}");
|
||||
let p = policy(&db, OWN).await?;
|
||||
assert_eq!(p["execution_mode"], "anonymous");
|
||||
assert!(
|
||||
p["triggerables_v2"].get("c:script/f/x/new").is_some(),
|
||||
"{p}"
|
||||
);
|
||||
|
||||
// ========================================
|
||||
// 4. A compose-only operator: an omitted mode inherits the deployed one unless it is Viewer.
|
||||
// ========================================
|
||||
sqlx::query("UPDATE usr SET operator = true WHERE workspace_id = 'test-workspace' AND username = 'test-user-3'")
|
||||
.execute(&db)
|
||||
.await?;
|
||||
sqlx::query(
|
||||
r#"UPDATE workspace_settings SET role_overrides = '{"operator": {"capabilities": {"apps": "compose"}}}'
|
||||
WHERE workspace_id = 'test-workspace'"#,
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
// In wm_deployers, so the refusal in the last step below holds for a builder who may keep
|
||||
// another user's identity on deploy.
|
||||
sqlx::query(
|
||||
"INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES ('test-workspace', 'wm_deployers', '', '{}')",
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
sqlx::query(
|
||||
"INSERT INTO usr_to_group (workspace_id, group_, usr) VALUES ('test-workspace', 'wm_deployers', 'test-user-3')",
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
windmill_common::roles::invalidate_workspace_roles_cache("test-workspace");
|
||||
windmill_common::roles::invalidate_member_role_cache("test-workspace", "test-user-3");
|
||||
|
||||
const BUILT: &str = "u/test-user-3/built";
|
||||
create_raw(&ws, BUILDER, BUILT).await?;
|
||||
|
||||
let settings_only = json!({ "policy": { "sandbox": true, "triggerables_v2": {} } });
|
||||
let (status, body) = update(&ws, BUILT, BUILDER, settings_only.clone()).await?;
|
||||
assert_eq!(status, 200, "{body}");
|
||||
assert_eq!(
|
||||
policy(&db, BUILT).await?["execution_mode"],
|
||||
"anonymous",
|
||||
"a builder's policy without a mode must keep a non-Viewer deployed mode"
|
||||
);
|
||||
|
||||
// Viewer stays refused to a builder, alone as in a policy.
|
||||
let (status, body) = update(&ws, BUILT, BUILDER, json!({ "execution_mode": "viewer" })).await?;
|
||||
assert_eq!(status, 403, "{body}");
|
||||
|
||||
// An admin may put the builder's app in Viewer; the builder's next policy without a mode
|
||||
// then cannot inherit it.
|
||||
let (status, body) = update(&ws, BUILT, ADMIN, json!({ "execution_mode": "viewer" })).await?;
|
||||
assert_eq!(status, 200, "{body}");
|
||||
let (status, body) = update(&ws, BUILT, BUILDER, settings_only).await?;
|
||||
assert_eq!(status, 200, "{body}");
|
||||
assert_eq!(policy(&db, BUILT).await?["execution_mode"], "publisher");
|
||||
|
||||
let (status, body) = update(
|
||||
&ws,
|
||||
BUILT,
|
||||
BUILDER,
|
||||
json!({ "execution_mode": "anonymous" }),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
status, 200,
|
||||
"a builder may change the mode of its own app: {body}"
|
||||
);
|
||||
assert_eq!(policy(&db, BUILT).await?["execution_mode"], "anonymous");
|
||||
|
||||
// A builder's deploy would sandbox an app whose policy leaves it unset, which a mode-only
|
||||
// change must not do.
|
||||
sqlx::query(
|
||||
"UPDATE app SET policy = policy - 'sandbox' WHERE path = $1 AND workspace_id = 'test-workspace'",
|
||||
)
|
||||
.bind(BUILT)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
let (status, body) = update(
|
||||
&ws,
|
||||
BUILT,
|
||||
BUILDER,
|
||||
json!({ "execution_mode": "publisher" }),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(status, 403, "{body}");
|
||||
let p = policy(&db, BUILT).await?;
|
||||
assert!(p.get("sandbox").is_none(), "{p}");
|
||||
assert_eq!(p["execution_mode"], "anonymous");
|
||||
|
||||
// Changing the mode alone skips the check of the triggerables against the builder's RLS, so
|
||||
// it stays refused on an app that runs as someone else; deploying it is the way.
|
||||
const ADMIN_RUN: &str = "u/test-user-3/admin_run";
|
||||
create_raw(&ws, ADMIN, ADMIN_RUN).await?;
|
||||
let (status, body) = update(
|
||||
&ws,
|
||||
ADMIN_RUN,
|
||||
BUILDER,
|
||||
json!({ "execution_mode": "publisher" }),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(status, 403, "{body}");
|
||||
assert!(body.contains("by deploying it"), "{body}");
|
||||
|
||||
// ========================================
|
||||
// 5. Widening is gated by the protection rule; narrowing is not.
|
||||
// ========================================
|
||||
let resp = client()
|
||||
.post(format!("{ws}/workspaces/protection_rules"))
|
||||
.bearer_auth(ADMIN)
|
||||
.json(&json!({
|
||||
"name": "no-public-apps",
|
||||
"rules": ["RestrictAnonymousAppDeployment"],
|
||||
"bypass_users": [],
|
||||
"bypass_groups": []
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
|
||||
|
||||
let (status, body) = update(&ws, OWN, USER, json!({ "execution_mode": "publisher" })).await?;
|
||||
assert_eq!(status, 200, "narrowing stays open: {body}");
|
||||
let (status, body) = update(&ws, OWN, USER, json!({ "execution_mode": "anonymous" })).await?;
|
||||
assert_eq!(status, 403, "{body}");
|
||||
assert!(body.contains("no-public-apps"), "{body}");
|
||||
assert_eq!(policy(&db, OWN).await?["execution_mode"], "publisher");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -511,6 +511,7 @@ def main():
|
||||
)
|
||||
.unwrap(),
|
||||
policy: None,
|
||||
execution_mode: None,
|
||||
deployment_message: None,
|
||||
custom_path: None,
|
||||
preserve_on_behalf_of: None,
|
||||
|
||||
@@ -15741,6 +15741,10 @@ paths:
|
||||
value: {}
|
||||
policy:
|
||||
$ref: "#/components/schemas/Policy"
|
||||
execution_mode:
|
||||
type: string
|
||||
enum: [viewer, publisher, guest, anonymous]
|
||||
description: "Changes who may open the app and leaves the rest of the deployed policy, its run-as user included, as it is. Applied under the same lock as a deploy, so neither overwrites the other. Cannot be sent with `policy`, which states its own mode. When the app runs as another user, refused to a caller who is neither an admin nor in 'wm_deployers', and to an operator with builder rights even in 'wm_deployers'; they change the mode by deploying."
|
||||
deployment_message:
|
||||
type: string
|
||||
custom_path:
|
||||
|
||||
@@ -602,6 +602,11 @@ pub struct EditApp {
|
||||
pub summary: Option<String>,
|
||||
pub value: Option<sqlx::types::Json<Box<RawValue>>>,
|
||||
pub policy: Option<Policy>,
|
||||
/// Changes only who may open the app: applied to the deployed policy under the row lock a
|
||||
/// deploy takes, so it neither reverts a concurrent deploy's policy nor is reverted by it.
|
||||
/// Exclusive with `policy`, which states its own mode.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub execution_mode: Option<ExecutionMode>,
|
||||
pub deployment_message: Option<String>,
|
||||
pub custom_path: Option<String>,
|
||||
pub preserve_on_behalf_of: Option<bool>,
|
||||
@@ -2366,18 +2371,12 @@ fn check_operator_composed_app(
|
||||
|
||||
// Viewer mode would make the triggerables collected above non-exhaustive (see the docs), and
|
||||
// the deploy panel shows it as members-only, so an admin deploying a draft would not see it.
|
||||
// Pin an omitted mode on deploy: `update_app_internal` resolves it to the deployed app's, so a
|
||||
// redeploy over an admin's viewer-mode app would otherwise inherit `Viewer`.
|
||||
match policy.stated_execution_mode() {
|
||||
Some(ExecutionMode::Viewer) => {
|
||||
return Err(Error::PermissionDenied(format!(
|
||||
"Operators with builder rights cannot {verb} an app that runs as its viewer"
|
||||
)))
|
||||
}
|
||||
None if write == BuilderAppWrite::Deploy => {
|
||||
policy.set_execution_mode(ExecutionMode::Publisher)
|
||||
}
|
||||
_ => {}
|
||||
// An omitted mode is resolved by the deploy: create's default, or on update the deployed mode,
|
||||
// which `update_app_internal` turns into `Publisher` for a builder when it is `Viewer`.
|
||||
if policy.stated_execution_mode() == Some(ExecutionMode::Viewer) {
|
||||
return Err(Error::PermissionDenied(format!(
|
||||
"Operators with builder rights cannot {verb} an app that runs as its viewer"
|
||||
)));
|
||||
}
|
||||
|
||||
referenced.sort();
|
||||
@@ -2473,15 +2472,7 @@ pub(crate) async fn validate_operator_composed_app(
|
||||
// docs/operator-builder-rights.md); a draft keeps whatever it names, which an admin's
|
||||
// one-click draft deploy preserves without showing it.
|
||||
if let (BuilderAppWrite::Draft, Some(policy)) = (write, policy.as_deref()) {
|
||||
let names_another = policy
|
||||
.on_behalf_of
|
||||
.as_deref()
|
||||
.is_some_and(|p| p != username_to_permissioned_as(&authed.username))
|
||||
|| policy
|
||||
.on_behalf_of_email
|
||||
.as_deref()
|
||||
.is_some_and(|e| e != authed.email);
|
||||
if names_another {
|
||||
if policy_runs_as_another(policy, authed) {
|
||||
return Err(Error::PermissionDenied(
|
||||
"Operators with builder rights cannot save an app that runs as someone else"
|
||||
.to_string(),
|
||||
@@ -2492,6 +2483,17 @@ pub(crate) async fn validate_operator_composed_app(
|
||||
require_runnables_readable(authed, db, user_db, w_id, referenced).await
|
||||
}
|
||||
|
||||
fn policy_runs_as_another(policy: &Policy, authed: &ApiAuthed) -> bool {
|
||||
policy
|
||||
.on_behalf_of
|
||||
.as_deref()
|
||||
.is_some_and(|p| p != username_to_permissioned_as(&authed.username))
|
||||
|| policy
|
||||
.on_behalf_of_email
|
||||
.as_deref()
|
||||
.is_some_and(|e| e != authed.email)
|
||||
}
|
||||
|
||||
async fn require_runnables_readable(
|
||||
authed: &ApiAuthed,
|
||||
db: &DB,
|
||||
@@ -3809,10 +3811,22 @@ async fn update_app_internal<'a>(
|
||||
}
|
||||
}
|
||||
|
||||
if caller_capabilities(&db, w_id, &authed).await?.apps == AuthorLevel::Compose {
|
||||
if ns.policy.is_some() && ns.execution_mode.is_some() {
|
||||
return Err(Error::BadRequest(
|
||||
"Send execution_mode either inside the policy or alone, not both".to_string(),
|
||||
));
|
||||
}
|
||||
// The policy a mode-only change writes is only known under the row lock, so a builder's
|
||||
// check on it runs there.
|
||||
let mode_only = ns.policy.is_none() && ns.value.is_none() && ns.execution_mode.is_some();
|
||||
|
||||
let composes_only = caller_capabilities(&db, w_id, &authed).await?.apps == AuthorLevel::Compose;
|
||||
if composes_only {
|
||||
// Restoring a full-code version always sends allow_kind_change. A builder converts
|
||||
// nothing, so drop it and let the locked kind check below refuse an actual conversion.
|
||||
ns.allow_kind_change = None;
|
||||
}
|
||||
if composes_only && !mode_only {
|
||||
validate_operator_composed_app(
|
||||
&authed,
|
||||
&db,
|
||||
@@ -3890,6 +3904,7 @@ async fn update_app_internal<'a>(
|
||||
|| ns.summary.is_some()
|
||||
|| ns.custom_path.is_some()
|
||||
|| ns.labels.is_some()
|
||||
|| ns.execution_mode.is_some()
|
||||
// A source deploy may send nothing but the value, and its runnables are
|
||||
// what the derived grants describe: skipping the UPDATE here would keep
|
||||
// the grants keyed to the sources this deploy just replaced.
|
||||
@@ -3907,7 +3922,12 @@ async fn update_app_internal<'a>(
|
||||
// that carries no policy keeps the deployed mode, read under the row
|
||||
// lock so a policy update landing alongside cannot slip a guest app
|
||||
// onto a path it cannot be scoped to.
|
||||
let mode = match ns.policy.as_ref().and_then(|p| p.stated_execution_mode()) {
|
||||
let stated = ns
|
||||
.policy
|
||||
.as_ref()
|
||||
.and_then(|p| p.stated_execution_mode())
|
||||
.or(ns.execution_mode);
|
||||
let mode = match stated {
|
||||
Some(mode) => mode,
|
||||
None => sqlx::query_scalar::<_, Option<String>>(
|
||||
"SELECT policy->>'execution_mode' FROM app
|
||||
@@ -3987,12 +4007,13 @@ async fn update_app_internal<'a>(
|
||||
// no policy at all, in which case the rest of the deployed one carries
|
||||
// over. Either way a policy is about to be written wholesale.
|
||||
let caller_sent_policy = ns.policy.is_some();
|
||||
if caller_sent_policy || derived_triggerables.is_some() {
|
||||
if caller_sent_policy || derived_triggerables.is_some() || ns.execution_mode.is_some() {
|
||||
// One locked read serving everything below: the mode an omitted one
|
||||
// inherits, the already-anonymous check, and the policy a source
|
||||
// deploy carries over. FOR UPDATE holds the row until this
|
||||
// transaction's UPDATE commits, so a policy edit landing between the
|
||||
// read and the write cannot be clobbered by this stale snapshot.
|
||||
// deploy or a mode-only change carries over. FOR UPDATE holds the row
|
||||
// until this transaction's UPDATE commits, so a policy edit landing
|
||||
// between the read and the write cannot be clobbered by this stale
|
||||
// snapshot.
|
||||
let deployed = sqlx::query_scalar!(
|
||||
"SELECT policy FROM app WHERE path = $1 AND workspace_id = $2 FOR UPDATE",
|
||||
path,
|
||||
@@ -4006,30 +4027,81 @@ async fn update_app_internal<'a>(
|
||||
|
||||
let mut npolicy = match ns.policy.take() {
|
||||
Some(npolicy) => npolicy,
|
||||
None if deployed.is_none() => {
|
||||
return Err(Error::NotFound(format!("App {path} not found")))
|
||||
}
|
||||
// Carrying the deployed policy forward is only safe if all of it
|
||||
// survived the round trip; a partial one would silently drop
|
||||
// `on_behalf_of`, sandboxing or S3 rules.
|
||||
None => deployed_policy.clone().ok_or_else(|| {
|
||||
Error::internal_err(format!(
|
||||
"app {path} has no readable policy to deploy its new sources under"
|
||||
))
|
||||
Error::internal_err(format!("app {path} has no readable policy to update"))
|
||||
})?,
|
||||
};
|
||||
if let Some(triggerables) = derived_triggerables {
|
||||
npolicy.triggerables = None;
|
||||
npolicy.triggerables_v2 = Some(triggerables);
|
||||
}
|
||||
if let Some(mode) = ns.execution_mode {
|
||||
// A policy write from someone who cannot keep the run-as user rebinds it
|
||||
// to them, so the mode alone must not let them open an app running as
|
||||
// someone else to a new audience. A builder who may keep it is refused too:
|
||||
// its deploy checks the triggerables against its own RLS, which needs a
|
||||
// second pool connection and so cannot run under this lock.
|
||||
if policy_runs_as_another(&npolicy, &authed)
|
||||
&& (composes_only || !windmill_common::can_preserve_on_behalf_of(&authed))
|
||||
{
|
||||
let runs_as = npolicy
|
||||
.on_behalf_of_email
|
||||
.as_deref()
|
||||
.or(npolicy.on_behalf_of.as_deref())
|
||||
.unwrap_or("another user");
|
||||
return Err(Error::PermissionDenied(if composes_only {
|
||||
format!(
|
||||
"app {path} runs as {runs_as}: change who can open it by deploying it"
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"app {path} runs as {runs_as}: only admins and members of \
|
||||
wm_deployers can change who can open it without deploying it, and a \
|
||||
deploy from you makes it run as you"
|
||||
)
|
||||
}));
|
||||
}
|
||||
npolicy.set_execution_mode(mode);
|
||||
if composes_only {
|
||||
// The check below sandboxes an app whose policy leaves `sandbox` unset; a
|
||||
// mode-only change keeps every other field, so that is left to a deploy.
|
||||
if npolicy.sandbox != Some(true) {
|
||||
return Err(Error::PermissionDenied(format!(
|
||||
"app {path} is not sandboxed: change who can open it by deploying it"
|
||||
)));
|
||||
}
|
||||
check_operator_composed_app(
|
||||
BuilderAppWrite::Deploy,
|
||||
raw_app,
|
||||
None,
|
||||
Some(&mut npolicy),
|
||||
)?;
|
||||
}
|
||||
}
|
||||
validate_frontend_sdk_scopes(&npolicy)?;
|
||||
// The policy is written wholesale, so one that states no mode would
|
||||
// otherwise re-permission the app to the create-time default: a
|
||||
// `viewer` app would start running on the publisher's identity. Keep
|
||||
// the deployed mode instead, and make the stored policy state it.
|
||||
// the deployed mode instead, and make the stored policy state it. A
|
||||
// builder may not deploy `Viewer` (see `check_operator_composed_app`),
|
||||
// so a builder's redeploy over a viewer-mode app makes it `Publisher`.
|
||||
if npolicy.stated_execution_mode().is_none() {
|
||||
let deployed_mode = deployed_policy
|
||||
.as_ref()
|
||||
.map(|d| d.execution_mode())
|
||||
.unwrap_or_default();
|
||||
npolicy.set_execution_mode(
|
||||
deployed_policy
|
||||
.as_ref()
|
||||
.map(|d| d.execution_mode())
|
||||
.unwrap_or_default(),
|
||||
if composes_only && deployed_mode == ExecutionMode::Viewer {
|
||||
ExecutionMode::Publisher
|
||||
} else {
|
||||
deployed_mode
|
||||
},
|
||||
);
|
||||
}
|
||||
refuse_unscopable_guest_app(
|
||||
@@ -4155,7 +4227,8 @@ async fn update_app_internal<'a>(
|
||||
// CLI / git-sync deploys ask us to preserve any existing user draft at this
|
||||
// path instead of wiping it as part of the deploy. Only wipe the deployer's
|
||||
// own draft (plus the legacy NULL-email row) — see create_app_internal.
|
||||
if !ns.skip_draft_deletion.unwrap_or(false) {
|
||||
// A mode-only change publishes nothing from the draft, so it keeps it.
|
||||
if !ns.skip_draft_deletion.unwrap_or(false) && !mode_only {
|
||||
sqlx::query!(
|
||||
"DELETE FROM draft WHERE path = $1 AND workspace_id = $2 AND typ IN ('app', 'raw_app') \
|
||||
AND (email = $3 OR email IS NULL)",
|
||||
|
||||
@@ -104,12 +104,19 @@ ignored into `inlineScript`.
|
||||
|
||||
`update_app` takes a builder too, but settings only: the deploy panel saves a deployed full-code
|
||||
app's access and frontend scopes through it with no value. It reaches the same check, so a builder
|
||||
can no more switch the sandbox off there than on deploy.
|
||||
can no more switch the sandbox off there than on deploy. A change of `execution_mode` alone runs
|
||||
`check_operator_composed_app` on the deployed policy under the row lock, but not the readability
|
||||
check, which resolves script versions on a second pool connection. It is therefore refused when
|
||||
the app runs as anyone but the builder, `wm_deployers` or not: what an app running as its builder
|
||||
invokes runs as that builder, who can already run it as themselves by opening the app. It is also
|
||||
refused on an app whose stored policy is not sandboxed: the check would fill in the sandbox, and a
|
||||
change of the mode alone keeps every other field of the policy.
|
||||
|
||||
A builder's **draft** goes through the same `check_operator_composed_app` as its deploy, with
|
||||
`BuilderAppWrite::Draft`: an admin may open it in the editor or deploy it as it stands, so any
|
||||
rule added for deploys must hold for drafts too. Where the two differ, `write` decides: a deploy
|
||||
fills in an omitted sandbox or mode, since it stores the policy it checked, while a draft skips
|
||||
fills in an omitted sandbox, since it stores the policy it checked, and an omitted mode inherits the
|
||||
deployed one, except that `Viewer` becomes `Publisher`, while a draft skips
|
||||
`triggerables_v2` and must not name another user's run identity. Every frontend writer of a
|
||||
builder draft must conform the policy first with `conformBuilderAppPolicy`, since a policy loaded
|
||||
from someone else's app breaks these rules: the raw-app editor component does, which covers both
|
||||
|
||||
Reference in New Issue
Block a user