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

fix(apps): run-mode inline app component uses only pinned content (#11135)

Browse Source 
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-15 14:36:16 +00:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 129c045595
commit 781b5a57e8
2 changed files with 292 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/app_run_mode_lock_strip.rs
+240
View File
@@ -0,0 +1,240 @@
//! Regression: run mode of `execute_component`'s no-id inline-`raw_code` arm runs
//! *only* the `rawscript/<sha>`-pinned `content`, dropping the caller `hash`,
//! `lock`, `modules` and `dedicated_worker` and deriving `path` server-side —
//! all of which would otherwise run or install unpinned code as the app identity.
//! Preview mode keeps honoring the caller's fields.
use serde_json::json;
use sha2::{Digest, Sha256};
use sqlx::{Pool, Postgres};
use windmill_test_utils::*;
fn client() -> reqwest::Client {
reqwest::Client::new()
}
fn authed(b: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuilder {
b.header("Authorization", format!("Bearer {}", token))
}
const CONTENT: &str = "print('benign')\n";
// A caller lock whose presence is the whole point: if it reaches the job, the
// worker installs it. The value only needs to be recognizable in `v2_job`.
const CALLER_LOCK: &str = "evilpkg @ file:///tmp/attacker-controlled-sdist";
// A non-codebase-sentinel hash: if it reaches the job as `runnable_id`, the
// worker fetches (and runs) a deployed script by hash instead of the pinned
// content. It need not resolve to a real row — the guard is that it never
// becomes `runnable_id`.
const CALLER_HASH: i64 = 123456789;
// A caller path in someone else's namespace: if it reaches the job as
// `runnable_path` it redirects where the pinned content's relative imports
// resolve. Run mode must instead derive the path from `<app_path>/<component>`.
const CALLER_PATH: &str = "u/attacker/evil/comp";
/// The pin key `execute_component` computes for a no-id inline script:
/// `rawscript/<sha256(content)>`.
fn rawscript_pin(content: &str) -> String {
let mut h = Sha256::new();
h.update(content);
format!("rawscript/{:x}", h.finalize())
}
fn inline_raw_code(hash: Option<i64>, dedicated: bool) -> serde_json::Value {
let mut rc = json!({
"language": "python3",
"content": CONTENT,
"path": CALLER_PATH,
"lock": CALLER_LOCK,
"modules": {
"m.py": { "content": "print('x')\n", "language": "python3", "lock": CALLER_LOCK }
}
});
if let Some(h) = hash {
rc["hash"] = json!(h);
}
if dedicated {
rc["dedicated_worker"] = json!(true);
}
rc
}
/// Fetch `(raw_lock, args-has-_MODULES, runnable_id, tag, runnable_path)` for an
/// enqueued job.
async fn job_fields(
db: &Pool<Postgres>,
uuid: uuid::Uuid,
) -> anyhow::Result<(Option<String>, bool, Option<i64>, String, Option<String>)> {
Ok(sqlx::query_as(
"SELECT raw_lock, (args ? '_MODULES'), runnable_id, tag, runnable_path \
FROM v2_job WHERE id = $1",
)
.bind(uuid)
.fetch_one(db)
.await?)
}
#[sqlx::test(fixtures("base"))]
async fn test_run_mode_strips_caller_lock_and_modules(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let ws = format!("http://localhost:{port}/api/w/test-workspace");
let app_path = "u/test-user/lockstrip";
let pin = format!("comp:{}", rawscript_pin(CONTENT));
// Deployed Viewer-mode app whose only runnable is an inline script pinned by
// content hash and with no `app_script` row — the legacy `rawscript/<sha>`
// case that reaches the no-id run-mode arm this fix touches.
let resp = authed(client().post(format!("{ws}/apps/create")), "SECRET_TOKEN")
.json(&json!({
"path": app_path,
"summary": "",
"value": {},
"policy": {
"execution_mode": "viewer",
"triggerables_v2": { pin: { "static_inputs": {}, "one_of_inputs": {} } }
}
}))
.send()
.await?;
assert_eq!(resp.status(), 201, "create app: {}", resp.text().await?);
// Run mode (no `force_viewer_static_fields`): the pin authorizes the run, but
// every caller field that selects, installs, or routes code — hash, lock,
// modules, dedicated_worker — must be dropped.
let resp = authed(
client().post(format!("{ws}/apps_u/execute_component/{app_path}")),
"SECRET_TOKEN_2",
)
.json(&json!({
// The args map is the other injection channel: an inline run is a
// `JobKind::Preview` job, so the worker/executors read `_MODULES` and
// `_TEMP_SCRIPT_REFS` back out of the job args. Both must be stripped.
"args": {
"_MODULES": { "m.py": { "content": "print('evil')\n", "language": "python3" } },
"_TEMP_SCRIPT_REFS": { "../evil": "deadbeef" }
},
"component": "comp",
"raw_code": inline_raw_code(Some(CALLER_HASH), true)
}))
.send()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(
status, 200,
"run-mode pinned inline run must be accepted: {body}"
);
let uuid = uuid::Uuid::parse_str(body.trim())?;
let (raw_lock, has_modules, runnable_id, tag, runnable_path) = job_fields(&db, uuid).await?;
assert_eq!(
raw_lock, None,
"run mode must strip the caller-supplied lock"
);
assert!(
!has_modules,
"run mode must strip caller modules (both `raw_code.modules` and an `_MODULES` arg)"
);
let has_temp_refs: bool =
sqlx::query_scalar("SELECT (args ? '_TEMP_SCRIPT_REFS') FROM v2_job WHERE id = $1")
.bind(uuid)
.fetch_one(&db)
.await?;
assert!(
!has_temp_refs,
"run mode must strip a caller `_TEMP_SCRIPT_REFS` arg (relative-import redirect)"
);
assert_eq!(
runnable_id, None,
"run mode must strip the caller-supplied hash (no substituting a deployed script by hash)"
);
assert!(
!tag.starts_with("dedi:"),
"run mode must strip caller `dedicated_worker` (no routing to a path-keyed dedicated worker), got tag {tag:?}"
);
assert_eq!(
runnable_path.as_deref(),
Some(format!("{app_path}/comp").as_str()),
"run mode must derive the path server-side, not trust the caller's (relative-import base)"
);
// Preview mode (editor): the caller runs their own code as themselves, so the
// lock and modules are honored — the `/jobs/run/preview`-equivalent path.
let resp = authed(
client().post(format!("{ws}/apps_u/execute_component/{app_path}")),
"SECRET_TOKEN_2",
)
.json(&json!({
"args": {},
"component": "comp",
"raw_code": inline_raw_code(None, false),
"force_viewer_static_fields": {}
}))
.send()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(status, 200, "preview must be accepted: {body}");
let uuid = uuid::Uuid::parse_str(body.trim())?;
let (raw_lock, has_modules, _, _, _) = job_fields(&db, uuid).await?;
assert_eq!(
raw_lock.as_deref(),
Some(CALLER_LOCK),
"preview must keep the caller-supplied lock"
);
assert!(has_modules, "preview must keep the caller-supplied modules");
Ok(())
}
/// A bare `rawscript/<sha>` policy key (no `<component>:` prefix, as `empty_triggerables`
/// migrates v1 policies) matches for any `component`, so run mode must not let a
/// path-traversing `component` steer the server-derived `runnable_path`.
#[sqlx::test(fixtures("base"))]
async fn test_run_mode_rejects_traversal_component(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let ws = format!("http://localhost:{port}/api/w/test-workspace");
let app_path = "u/test-user/lockstrip_bare";
// Bare key: no `comp:` prefix, so the pin matches regardless of `component`.
let resp = authed(client().post(format!("{ws}/apps/create")), "SECRET_TOKEN")
.json(&json!({
"path": app_path,
"summary": "",
"value": {},
"policy": {
"execution_mode": "viewer",
"triggerables_v2": { rawscript_pin(CONTENT): { "static_inputs": {}, "one_of_inputs": {} } }
}
}))
.send()
.await?;
assert_eq!(resp.status(), 201, "create app: {}", resp.text().await?);
// A component that isn't a single plain segment steers the derived path's
// base: separators and `..` traverse, and an empty one shifts it up a level.
for bad in ["../../u/attacker/evil", "..", "a/b", ""] {
let resp = authed(
client().post(format!("{ws}/apps_u/execute_component/{app_path}")),
"SECRET_TOKEN_2",
)
.json(&json!({
"args": {},
"component": bad,
"raw_code": inline_raw_code(None, false)
}))
.send()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(
status, 400,
"run mode must reject component {bad:?}: got {status}: {body}"
);
}
Ok(())
}
backend/windmill-api/src/apps.rs
+52
View File
@@ -3853,6 +3853,26 @@ fn digest(code: &str) -> String {
format!("rawscript/{:x}", result)
}
/// Canonical `runnable_path` for a run-mode no-id inline app component:
/// `<app_path>/<component>` — byte-for-byte what the runtime frontend sends. It is
/// the relative-import base, so the caller must not steer it: reject a `component`
/// that isn't a single non-empty path segment (empty, a separator, or `.`/`..`
/// would walk the base out of the app, and a bare `rawscript/<sha>` policy key
/// does not pin the component).
fn inline_run_path(app_path: &str, component: &str) -> Result<String> {
if component.is_empty()
|| component.contains('/')
|| component.contains('\\')
|| component == "."
|| component == ".."
{
return Err(Error::BadRequest(
"component id must be a single non-empty path segment".to_string(),
));
}
Ok(format!("{app_path}/{component}"))
}
async fn get_on_behalf_details_from_policy_and_authed(
policy: &Policy,
opt_authed: &Option<ApiAuthed>,
@@ -4257,6 +4277,14 @@ async fn execute_component(
let resolved_delete_secs =
resolve_delete_after_secs(None, policy_triggerables.delete_after_secs);
// `_MODULES` and `_TEMP_SCRIPT_REFS` are server-injected control keys (into
// `extra`) that the worker reads back for a `Preview` job — which an inline run
// is. A caller supplying them in `args` would inject module content/locks or
// redirect relative-import resolution, unpinned, as the app identity. Drop them;
// legitimate values ride in `extra`, never the request `args`.
payload.args.remove("_MODULES");
payload.args.remove("_TEMP_SCRIPT_REFS");
let (mut args, job_id) = build_args(
policy,
policy_triggerables,
@@ -4294,6 +4322,7 @@ async fn execute_component(
}
.filter(|t| !t.is_empty())
};
let component = payload.component.clone();
let (job_payload, tag, _runnable_on_behalf_of) =
match (payload.path, payload.raw_code, payload.id) {
// flow or script:
@@ -4304,6 +4333,29 @@ async fn execute_component(
// `app_script` table (legacy `rawscript/<sha>`-keyed triggerables).
(None, Some(raw_code), None) => {
let tag = resolved_inline_tag(raw_code.tag.clone());
let raw_code = if is_preview {
// Preview (editor / `wmill app dev`): the caller runs their own
// code, like `/jobs/run/preview` — honored verbatim.
raw_code
} else {
// Run mode. Legacy back-compat only: current deploys assign an
// `app_script` id (reduce_app) and take the `Some(id)` arm;
// drop this branch once id-less deployed apps are gone.
//
// Only `content` is pinned (`rawscript/<sha>`), so keep just
// that plus `language`/`cache_ttl`, derive `path` server-side
// (`inline_run_path`), and default the rest: a caller `hash`/
// `lock`/`modules`/`path`/`dedicated_worker` would otherwise run
// or install unpinned code as the app identity. Reconstructing
// (vs nulling) keeps a new field defaulting safe.
RawCode {
content: raw_code.content,
language: raw_code.language,
path: Some(inline_run_path(path, &component)?),
cache_ttl: raw_code.cache_ttl,
..Default::default()
}
};
(JobPayload::Code(raw_code), tag, None)
}
// inline script: run mode (deployed app) with an entry in `app_script`.