mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-30 08:16:49 +00:00
fix: key build artifact caches on a runnable's inline modules (#10819)
* fix: key build artifact caches on a runnable's inline modules Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: seal the cache-key base and skip prebundling multi-file bun scripts Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: tighten cache-key invariant comments and name the retained-artifact residual Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: version the build artifact keyspace so pre-fix artifacts are abandoned Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: namespace the artifact cache by keyspace version instead of the hash preimage Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: namespace module-bearing artifacts instead of versioning the whole keyspace Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: pin the cache-name base seal and name the retained-artifact residual Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: bump ee ref for agent-worker module resolution fix Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: align agent-worker module resolution with the worker for previews by hash Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: drop calculate_hash imports left unused by artifact_cache_name Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: update ee-repo-ref to 2d6c66b32f20d9605c6a677727473ab66fcc8a87 This commit updates the EE repository reference after PR #743 was merged in windmill-ee-private. Previous ee-repo-ref: efce983cae3d53175bbb286a10205a2a360c2a9e New ee-repo-ref: 2d6c66b32f20d9605c6a677727473ab66fcc8a87 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com> Co-authored-by: Ruben Fiszel <ruben@windmill.dev>
This commit is contained in:
co-authored by
Claude Opus 5
windmill-internal-app[bot]
Ruben Fiszel
parent
0bbd559ac8
commit
b72ccc3593
@@ -1 +1 @@
|
||||
c3b6f62ea579a3583d4b474e9885c77104cfc87e
|
||||
2d6c66b32f20d9605c6a677727473ab66fcc8a87
|
||||
|
||||
@@ -380,6 +380,7 @@ async fn cache_hub_scripts(file_path: Option<String>) -> anyhow::Result<()> {
|
||||
"",
|
||||
&mut None,
|
||||
&None,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -1256,6 +1256,7 @@ pub async fn prebundle_bun_script(
|
||||
token: &str,
|
||||
occupancy_metrics: &mut Option<&mut OccupancyMetrics>,
|
||||
temp_script_refs: &Option<HashMap<String, String>>,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> Result<()> {
|
||||
let (local_path, remote_path) = compute_bundle_local_and_remote_path(
|
||||
inner_content,
|
||||
@@ -1264,6 +1265,7 @@ pub async fn prebundle_bun_script(
|
||||
db,
|
||||
w_id,
|
||||
temp_script_refs,
|
||||
modules,
|
||||
)
|
||||
.await;
|
||||
if exists_in_cache(&local_path, &remote_path).await {
|
||||
@@ -1442,6 +1444,7 @@ pub async fn compute_bundle_local_and_remote_path(
|
||||
db: Option<&DB>,
|
||||
w_id: &str,
|
||||
temp_script_refs: &Option<HashMap<String, String>>,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> (String, String) {
|
||||
let mut input_src = format!("{inner_content}{lock}",);
|
||||
|
||||
@@ -1470,7 +1473,10 @@ pub async fn compute_bundle_local_and_remote_path(
|
||||
|
||||
let ws_suffix = crate::workspace_registry_cache_suffix(w_id).await;
|
||||
input_src.push_str(&ws_suffix);
|
||||
let hash = windmill_common::utils::calculate_hash(&input_src);
|
||||
|
||||
// The loader resolves relative imports against the module files in the job dir, so
|
||||
// their content is inlined into the bundle this name covers.
|
||||
let hash = crate::worker::artifact_cache_name(input_src, modules);
|
||||
let local_path = format!("{}/{hash}", *BUN_BUNDLE_CACHE_DIR);
|
||||
|
||||
#[cfg(windows)]
|
||||
@@ -1569,6 +1575,7 @@ pub async fn handle_bun_job(
|
||||
Some(db),
|
||||
&job.workspace_id,
|
||||
&temp_script_refs,
|
||||
modules.as_ref(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -2962,9 +2969,8 @@ pub async fn handle_wac_v2_output(
|
||||
version: flow_info.version,
|
||||
labels: flow_info.labels.clone(),
|
||||
};
|
||||
let on_behalf_of = flow_info
|
||||
.on_behalf_of(&job.workspace_id, db)
|
||||
.await?;
|
||||
let on_behalf_of =
|
||||
flow_info.on_behalf_of(&job.workspace_id, db).await?;
|
||||
let step_args: HashMap<String, Box<RawValue>> = step
|
||||
.args
|
||||
.iter()
|
||||
@@ -4382,4 +4388,49 @@ export function main(x: number) { return x; }"#;
|
||||
assert!(wrapper.contains(r#"line.startsWith("exec_preprocess:")"#));
|
||||
assert!(wrapper.contains(r#"line.startsWith("exec:")"#));
|
||||
}
|
||||
|
||||
/// The bundle cache is global and content-keyed, so a key that ignores the inline
|
||||
/// modules hands one workspace's bundle — attacker helper code and all — to the next
|
||||
/// job whose main content and lockfile happen to match.
|
||||
#[tokio::test]
|
||||
async fn bundle_cache_key_separates_inline_module_content() {
|
||||
use windmill_common::scripts::ScriptModule;
|
||||
|
||||
async fn key_for(modules: Option<&HashMap<String, ScriptModule>>) -> String {
|
||||
compute_bundle_local_and_remote_path(
|
||||
"import { h } from './helper.ts';\nexport async function main() { return h(); }",
|
||||
"{}\n//bun.lock\n<empty>",
|
||||
"u/alice/script",
|
||||
None,
|
||||
"w1",
|
||||
&None,
|
||||
modules,
|
||||
)
|
||||
.await
|
||||
.1
|
||||
}
|
||||
fn modules(content: &str) -> HashMap<String, ScriptModule> {
|
||||
HashMap::from([(
|
||||
"helper.ts".to_string(),
|
||||
ScriptModule {
|
||||
content: content.to_string(),
|
||||
language: ScriptLang::Bun,
|
||||
lock: None,
|
||||
},
|
||||
)])
|
||||
}
|
||||
|
||||
let attacker = key_for(Some(&modules("export const h = () => 'attacker'"))).await;
|
||||
let victim = key_for(Some(&modules("export const h = () => 'victim'"))).await;
|
||||
assert_ne!(attacker, victim);
|
||||
assert_eq!(
|
||||
attacker,
|
||||
key_for(Some(&modules("export const h = () => 'attacker'"))).await,
|
||||
"same modules must still share a cache slot"
|
||||
);
|
||||
|
||||
// An absent map and an empty one are the same script, so they share a slot.
|
||||
assert_eq!(key_for(None).await, key_for(Some(&HashMap::new())).await);
|
||||
assert_ne!(key_for(None).await, attacker);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,7 +13,7 @@ use itertools::Itertools;
|
||||
#[cfg(feature = "csharp")]
|
||||
use tokio::{fs::File, io::AsyncReadExt, process::Command};
|
||||
#[cfg(feature = "csharp")]
|
||||
use windmill_common::{utils::calculate_hash, worker::write_file};
|
||||
use windmill_common::worker::write_file;
|
||||
|
||||
#[cfg(feature = "csharp")]
|
||||
use crate::global_cache::save_cache;
|
||||
@@ -72,13 +72,21 @@ const CSHARP_OBJECT_STORE_PREFIX: &str =
|
||||
/// Cache key of a C# build. The run path and the deploy-time prebuild must derive it the
|
||||
/// same way or the prebuilt binary is never found and gets rebuilt on first run.
|
||||
#[cfg(feature = "csharp")]
|
||||
async fn csharp_cache_key(code: &str, requirements_o: Option<&str>, w_id: &str) -> String {
|
||||
let mut hash = calculate_hash(&format!(
|
||||
async fn csharp_cache_key(
|
||||
code: &str,
|
||||
requirements_o: Option<&str>,
|
||||
w_id: &str,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> String {
|
||||
// The SDK project globs every `.cs` under the job dir, so companion modules are
|
||||
// compiled into the binary this key names and have to be part of it.
|
||||
let base = format!(
|
||||
"{}{}{}",
|
||||
code,
|
||||
requirements_o.unwrap_or(""),
|
||||
DOTNET_TARGET_FRAMEWORK.as_str()
|
||||
));
|
||||
);
|
||||
let mut hash = crate::worker::artifact_cache_name(base, modules);
|
||||
hash.push_str(&crate::workspace_registry_cache_suffix(w_id).await);
|
||||
hash
|
||||
}
|
||||
@@ -487,10 +495,11 @@ pub async fn prebuild_csharp_binary(
|
||||
worker_name: &str,
|
||||
base_internal_url: &str,
|
||||
occupancy_metrics: &mut OccupancyMetrics,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> error::Result<Option<String>> {
|
||||
check_executor_binary_exists("dotnet", DOTNET_PATH.as_str(), "C#")?;
|
||||
|
||||
let hash = csharp_cache_key(code, Some(lock), &job.workspace_id).await;
|
||||
let hash = csharp_cache_key(code, Some(lock), &job.workspace_id, modules).await;
|
||||
let remote_path = format!("{CSHARP_OBJECT_STORE_PREFIX}{hash}");
|
||||
if crate::global_cache::exists_in_object_store(&remote_path).await {
|
||||
return Ok(None);
|
||||
@@ -549,6 +558,7 @@ pub async fn handle_csharp_job(
|
||||
_worker_name: &str,
|
||||
_envs: HashMap<String, String>,
|
||||
_occupancy_metrics: &mut OccupancyMetrics,
|
||||
_modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> Result<Box<RawValue>, Error> {
|
||||
Err(anyhow!("C# is not available because the feature is not enabled").into())
|
||||
}
|
||||
@@ -568,6 +578,7 @@ pub async fn handle_csharp_job(
|
||||
worker_name: &str,
|
||||
envs: HashMap<String, String>,
|
||||
occupancy_metrics: &mut OccupancyMetrics,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> Result<Box<RawValue>, Error> {
|
||||
check_executor_binary_exists("dotnet", DOTNET_PATH.as_str(), "C#")?;
|
||||
|
||||
@@ -575,6 +586,7 @@ pub async fn handle_csharp_job(
|
||||
inner_content,
|
||||
requirements_o.map(|x| x.as_str()),
|
||||
&job.workspace_id,
|
||||
modules,
|
||||
)
|
||||
.await;
|
||||
let bin_path = format!("{}/{hash}", *CSHARP_CACHE_DIR);
|
||||
|
||||
@@ -230,8 +230,15 @@ fn go_runtime_int32(v: &str) -> Option<i32> {
|
||||
|
||||
/// Cache key of a Go build. The run path and the deploy-time prebuild must derive it the
|
||||
/// same way or the prebuilt binary is never found and gets rebuilt on first run.
|
||||
fn go_cache_key(code: &str, maybe_lock: &MaybeLock) -> String {
|
||||
calculate_hash(&format!("{}{:?}v2", code, maybe_lock))
|
||||
fn go_cache_key(
|
||||
code: &str,
|
||||
maybe_lock: &MaybeLock,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> String {
|
||||
// A module whose path starts with `go/` lands inside the module dir this builds, so
|
||||
// its content ends up in the binary the key names.
|
||||
let base = format!("{}{:?}v2", code, maybe_lock);
|
||||
crate::worker::artifact_cache_name(base, modules)
|
||||
}
|
||||
|
||||
/// Install the deps, generate the entrypoint wrapper, `go build`, and push the binary to
|
||||
@@ -459,9 +466,10 @@ pub async fn prebuild_go_binary(
|
||||
worker_name: &str,
|
||||
base_internal_url: &str,
|
||||
occupancy_metrics: &mut OccupancyMetrics,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> Result<Option<String>, Error> {
|
||||
let maybe_lock = MaybeLock::Resolved { lock: lock.to_string() };
|
||||
let hash = go_cache_key(code, &maybe_lock);
|
||||
let hash = go_cache_key(code, &maybe_lock, modules);
|
||||
let remote_path = format!("{GO_OBJECT_STORE_PREFIX}{hash}");
|
||||
if crate::global_cache::exists_in_object_store(&remote_path).await {
|
||||
return Ok(None);
|
||||
@@ -512,6 +520,7 @@ pub async fn handle_go_job(
|
||||
envs: HashMap<String, String>,
|
||||
occupation_metrics: &mut OccupancyMetrics,
|
||||
maybe_lock: MaybeLock,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> Result<Box<RawValue>, Error> {
|
||||
//go does not like executing modules at temp root
|
||||
let job_dir = &format!("{job_dir}/go");
|
||||
@@ -520,7 +529,7 @@ pub async fn handle_go_job(
|
||||
.create(&job_dir)
|
||||
.expect("could not create go job dir");
|
||||
|
||||
let hash = go_cache_key(inner_content, &maybe_lock);
|
||||
let hash = go_cache_key(inner_content, &maybe_lock, modules);
|
||||
let bin_path = format!("{}/{hash}", *GO_BIN_CACHE_DIR);
|
||||
let remote_path = format!("{GO_OBJECT_STORE_PREFIX}{hash}");
|
||||
let (cache, cache_logs) = crate::global_cache::load_cache(&bin_path, &remote_path, false).await;
|
||||
|
||||
@@ -66,6 +66,7 @@ pub(crate) struct JobHandlerInput<'a> {
|
||||
pub requirements_o: Option<&'a String>,
|
||||
pub shared_mount: &'a str,
|
||||
pub worker_name: &'a str,
|
||||
pub modules: Option<&'a HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
}
|
||||
|
||||
pub async fn handle_java_job<'a>(mut args: JobHandlerInput<'a>) -> Result<Box<RawValue>, Error> {
|
||||
@@ -612,25 +613,33 @@ async fn compile<'a>(
|
||||
inner_content,
|
||||
requirements_o,
|
||||
parent_runnable_path,
|
||||
modules,
|
||||
..
|
||||
}: &mut JobHandlerInput<'a>,
|
||||
classpath: &'a str,
|
||||
// plugins: Vec<&'a str>,
|
||||
) -> Result<(), Error> {
|
||||
fn compute_hash(code: &str, requirements_o: Option<&String>) -> String {
|
||||
calculate_hash(&format!(
|
||||
// The cached artifact is the whole `target/` dir, and companion modules are written
|
||||
// into the job dir before this runs, so their content can land in it.
|
||||
fn compute_hash(
|
||||
code: &str,
|
||||
requirements_o: Option<&String>,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> String {
|
||||
let base = format!(
|
||||
"{}{}",
|
||||
code,
|
||||
requirements_o
|
||||
.as_ref()
|
||||
.map(|x| x.to_string())
|
||||
.unwrap_or_default()
|
||||
))
|
||||
);
|
||||
crate::worker::artifact_cache_name(base, modules)
|
||||
}
|
||||
let reserved_variables =
|
||||
get_reserved_variables(job, &client.token, conn, parent_runnable_path.clone()).await?;
|
||||
let ws_suffix = crate::workspace_registry_cache_suffix(&job.workspace_id).await;
|
||||
let mut hash = compute_hash(inner_content, *requirements_o);
|
||||
let mut hash = compute_hash(inner_content, *requirements_o, *modules);
|
||||
hash.push_str(&ws_suffix);
|
||||
let bin_path = format!("{}/{hash}", *JAVA_CACHE_DIR);
|
||||
let remote_path = format!("java_jar/{hash}");
|
||||
|
||||
@@ -14,7 +14,6 @@ use tokio::{
|
||||
};
|
||||
use windmill_common::{
|
||||
error::{self, Error},
|
||||
utils::calculate_hash,
|
||||
worker::{write_file, Connection},
|
||||
};
|
||||
use windmill_queue::MiniPulledJob;
|
||||
@@ -602,8 +601,13 @@ pub async fn build_rust_crate(
|
||||
|
||||
/// Cache key of a Rust build. The run path and the deploy-time prebuild must derive it
|
||||
/// the same way or the prebuilt binary is never found and gets rebuilt on first run.
|
||||
async fn rust_cache_key(code: &str, requirements_o: Option<&String>, w_id: &str) -> String {
|
||||
let mut hash = compute_rust_hash(code, requirements_o);
|
||||
async fn rust_cache_key(
|
||||
code: &str,
|
||||
requirements_o: Option<&String>,
|
||||
w_id: &str,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> String {
|
||||
let mut hash = compute_rust_hash(code, requirements_o, modules);
|
||||
hash.push_str(&crate::workspace_registry_cache_suffix(w_id).await);
|
||||
hash
|
||||
}
|
||||
@@ -621,11 +625,12 @@ pub async fn prebuild_rust_binary(
|
||||
worker_name: &str,
|
||||
base_internal_url: &str,
|
||||
occupancy_metrics: &mut OccupancyMetrics,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> error::Result<Option<String>> {
|
||||
ensure_rust_runtime_dirs();
|
||||
check_executor_binary_exists("cargo", CARGO_PATH.as_str(), "rust")?;
|
||||
|
||||
let hash = rust_cache_key(code, Some(&lock.to_string()), &job.workspace_id).await;
|
||||
let hash = rust_cache_key(code, Some(&lock.to_string()), &job.workspace_id, modules).await;
|
||||
let remote_path = format!("{RUST_OBJECT_STORE_PREFIX}{hash}");
|
||||
if crate::global_cache::exists_in_object_store(&remote_path).await {
|
||||
return Ok(None);
|
||||
@@ -652,15 +657,22 @@ pub async fn prebuild_rust_binary(
|
||||
Ok(Some(logs))
|
||||
}
|
||||
|
||||
pub fn compute_rust_hash(code: &str, requirements_o: Option<&String>) -> String {
|
||||
calculate_hash(&format!(
|
||||
pub fn compute_rust_hash(
|
||||
code: &str,
|
||||
requirements_o: Option<&String>,
|
||||
// Companion modules are written into the crate dir and compiled into the binary this
|
||||
// key names, so leaving them out shares one script's binary with another.
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> String {
|
||||
let base = format!(
|
||||
"{}{}",
|
||||
code,
|
||||
requirements_o
|
||||
.as_ref()
|
||||
.map(|x| x.to_string())
|
||||
.unwrap_or_default()
|
||||
))
|
||||
);
|
||||
crate::worker::artifact_cache_name(base, modules)
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "trace", skip_all)]
|
||||
@@ -679,11 +691,12 @@ pub async fn handle_rust_job(
|
||||
worker_name: &str,
|
||||
envs: HashMap<String, String>,
|
||||
occupancy_metrics: &mut OccupancyMetrics,
|
||||
modules: Option<&HashMap<String, windmill_common::scripts::ScriptModule>>,
|
||||
) -> Result<Box<RawValue>, Error> {
|
||||
ensure_rust_runtime_dirs();
|
||||
check_executor_binary_exists("cargo", CARGO_PATH.as_str(), "rust")?;
|
||||
|
||||
let hash = rust_cache_key(inner_content, requirements_o, &job.workspace_id).await;
|
||||
let hash = rust_cache_key(inner_content, requirements_o, &job.workspace_id, modules).await;
|
||||
let bin_path = format!("{}/{hash}", *RUST_CACHE_DIR);
|
||||
let remote_path = format!("{RUST_OBJECT_STORE_PREFIX}{hash}");
|
||||
|
||||
|
||||
@@ -950,6 +950,51 @@ pub async fn workspace_registry_cache_suffix(w_id: &str) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// The name a build artifact is cached under, derived from `base` — the runnable's own
|
||||
/// cache-key input — and its inline modules.
|
||||
///
|
||||
/// `write_module_files` puts module content in the job dir where the build inlines it into
|
||||
/// the artifact, so a name without it serves one runnable's modules to another whose main
|
||||
/// content and lockfile match — across workspaces, the cache being global.
|
||||
///
|
||||
/// Only the path and content may name the artifact, because they are all the build reads.
|
||||
/// `ScriptModule::lock` especially must stay out: deploy regenerates it *after* the parent
|
||||
/// has prebuilt, so naming it would strand every prebuilt artifact.
|
||||
pub(crate) fn artifact_cache_name(
|
||||
base: String,
|
||||
modules: Option<&std::collections::HashMap<String, ScriptModule>>,
|
||||
) -> String {
|
||||
let Some(modules) = modules.filter(|m| !m.is_empty()) else {
|
||||
// Byte-identical to the name a module-free runnable had before modules entered this,
|
||||
// so its cached artifacts stay reachable. A pre-fix multi-file runnable also stored
|
||||
// here, so one of those stays reachable too — accepted over invalidating every cache,
|
||||
// and once this ships nothing can be stored here with module content in it again.
|
||||
return windmill_common::utils::calculate_hash(&base);
|
||||
};
|
||||
let mut entries: Vec<(&String, &ScriptModule)> = modules.iter().collect();
|
||||
entries.sort_by(|a, b| a.0.cmp(b.0));
|
||||
// `base` ends in caller-supplied bytes (a preview brings its own lockfile), so it is
|
||||
// sealed to a fixed width before the module block is appended — raw, a crafted lockfile
|
||||
// could spell out another runnable's block and reach its slot.
|
||||
let mut keyed = format!(
|
||||
"{}:modules:{}",
|
||||
windmill_common::utils::calculate_hash(&base),
|
||||
entries.len()
|
||||
);
|
||||
for (path, module) in entries {
|
||||
// Both length-prefixed, else `{"a": "bc"}` and `{"ab": "c"}` encode alike.
|
||||
keyed.push_str(&format!(
|
||||
":{}:{path}:{}:{}",
|
||||
path.len(),
|
||||
module.content.len(),
|
||||
module.content,
|
||||
));
|
||||
}
|
||||
// Its own namespace: `calculate_hash` emits hex, so however a module-free runnable
|
||||
// crafts its content and lockfile it can never land on a module-bearing name.
|
||||
format!("mod-{}", windmill_common::utils::calculate_hash(&keyed))
|
||||
}
|
||||
|
||||
pub fn is_sandboxing_enabled() -> bool {
|
||||
if !*DISABLE_NSJAIL {
|
||||
return true;
|
||||
@@ -5459,7 +5504,9 @@ async fn handle_code_execution_job(
|
||||
None => job,
|
||||
};
|
||||
|
||||
// For preview jobs, extract modules from args._MODULES if not already set
|
||||
// Any job kind, not just previews: whatever is here is what gets written to the job dir
|
||||
// and built in, so the agent-worker server precomputing a cache name has to resolve
|
||||
// modules the same way (`windmill-api-agent-workers`, `get_code_and_lock`).
|
||||
let modules = modules_from_data.clone().or_else(|| {
|
||||
job.args.as_ref().and_then(|args| {
|
||||
args.get("_MODULES").and_then(|raw| {
|
||||
@@ -5652,11 +5699,106 @@ mod write_module_files_tests {
|
||||
use super::*;
|
||||
use std::collections::HashMap;
|
||||
use windmill_common::scripts::ScriptLang;
|
||||
use windmill_common::utils::calculate_hash;
|
||||
|
||||
fn module(content: &str) -> ScriptModule {
|
||||
ScriptModule { content: content.to_string(), language: ScriptLang::Python3, lock: None }
|
||||
}
|
||||
|
||||
/// Every language's artifact cache name funnels module content through this, so an
|
||||
/// ambiguous encoding puts two different runnables back on one name.
|
||||
#[test]
|
||||
fn artifact_name_cannot_be_re_cut_into_another_module_map() {
|
||||
fn name(entries: &[(&str, &str)]) -> String {
|
||||
let map: HashMap<String, ScriptModule> = entries
|
||||
.iter()
|
||||
.map(|(p, c)| (p.to_string(), module(c)))
|
||||
.collect();
|
||||
artifact_cache_name("base".to_string(), Some(&map))
|
||||
}
|
||||
|
||||
// Naive `path + content` concatenation renders both of these as "abc".
|
||||
assert_ne!(name(&[("a", "bc")]), name(&[("ab", "c")]));
|
||||
// Splitting one module into two must not read back as the joined one.
|
||||
assert_ne!(name(&[("a", "b"), ("c", "d")]), name(&[("ac", "bd")]));
|
||||
// Iteration order of the map must not move the name.
|
||||
assert_eq!(
|
||||
name(&[("a", "1"), ("b", "2")]),
|
||||
name(&[("b", "2"), ("a", "1")])
|
||||
);
|
||||
}
|
||||
|
||||
/// A module-free runnable must keep the exact name it had before modules entered the
|
||||
/// derivation, or upgrading strands every artifact already in the cache.
|
||||
#[test]
|
||||
fn artifact_name_is_unchanged_without_modules() {
|
||||
assert_eq!(
|
||||
artifact_cache_name("code+lock".to_string(), None),
|
||||
calculate_hash("code+lock")
|
||||
);
|
||||
assert_eq!(
|
||||
artifact_cache_name("code+lock".to_string(), Some(&HashMap::new())),
|
||||
calculate_hash("code+lock")
|
||||
);
|
||||
}
|
||||
|
||||
/// A preview brings its own source and lockfile, so a module-free runnable picks its
|
||||
/// whole `base`. Module-bearing names live in their own namespace precisely so that no
|
||||
/// crafted `base` can be made to land on one.
|
||||
#[test]
|
||||
fn a_module_free_runnable_cannot_forge_a_module_bearing_name() {
|
||||
let modules = HashMap::from([("h.ts".to_string(), module("evil"))]);
|
||||
let victim = artifact_cache_name("code+lock".to_string(), Some(&modules));
|
||||
|
||||
// `calculate_hash` emits hex, so the namespace is unreachable however `base` is
|
||||
// chosen — including by feeding it the victim's own name.
|
||||
assert!(victim.starts_with("mod-"));
|
||||
assert_ne!(artifact_cache_name(victim.clone(), None), victim);
|
||||
assert!(!artifact_cache_name("anything".to_string(), None).starts_with("mod-"));
|
||||
}
|
||||
|
||||
/// The `mod-` namespace separates module-free from module-bearing, and nothing separates
|
||||
/// two module-bearing runnables — only the seal does. Unsealed, `base` is variable-width,
|
||||
/// so the split between it and the module block is ambiguous and a preview (which brings
|
||||
/// its own source *and* lockfile) can absorb part of another runnable's block.
|
||||
#[test]
|
||||
fn a_module_bearing_runnable_cannot_absorb_another_ones_block() {
|
||||
let victim = artifact_cache_name(
|
||||
"V".to_string(),
|
||||
Some(&HashMap::from([(
|
||||
"h.ts".to_string(),
|
||||
module(":modules:1:1:a:1:b"),
|
||||
)])),
|
||||
);
|
||||
// Byte-identical to the victim's without the seal: the forger's `base` spells out the
|
||||
// victim's leading block, leaving its own single module to supply the tail.
|
||||
let forged = artifact_cache_name(
|
||||
"V:modules:1:4:h.ts:18:".to_string(),
|
||||
Some(&HashMap::from([("a".to_string(), module("b"))])),
|
||||
);
|
||||
|
||||
assert_ne!(victim, forged);
|
||||
}
|
||||
|
||||
/// Deploy fills a module's lock in after the parent has prebuilt, so a name that moved
|
||||
/// with it would leave every prebuilt artifact unreachable by the runs it was built for.
|
||||
#[test]
|
||||
fn artifact_name_ignores_the_lock_deploy_fills_in_later() {
|
||||
let prebuild = artifact_cache_name(
|
||||
"base".to_string(),
|
||||
Some(&HashMap::from([("h.ts".to_string(), module("x"))])),
|
||||
);
|
||||
|
||||
let mut locked = module("x");
|
||||
locked.lock = Some("{}\n//bun.lock\n<empty>".to_string());
|
||||
let after_deploy = artifact_cache_name(
|
||||
"base".to_string(),
|
||||
Some(&HashMap::from([("h.ts".to_string(), locked)])),
|
||||
);
|
||||
|
||||
assert_eq!(prebuild, after_deploy);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn contained_relative_path_rejects_traversal_and_absolute() {
|
||||
assert!(is_contained_relative_path("u/admin/pkg"));
|
||||
@@ -6386,6 +6528,7 @@ mount {{
|
||||
envs,
|
||||
occupancy_metrics,
|
||||
maybe_lock,
|
||||
modules.as_ref(),
|
||||
))
|
||||
.await
|
||||
}
|
||||
@@ -6515,6 +6658,7 @@ mount {{
|
||||
worker_name,
|
||||
envs,
|
||||
occupancy_metrics,
|
||||
modules.as_ref(),
|
||||
))
|
||||
.await
|
||||
}
|
||||
@@ -6573,6 +6717,7 @@ mount {{
|
||||
worker_name,
|
||||
envs,
|
||||
occupancy_metrics,
|
||||
modules.as_ref(),
|
||||
))
|
||||
.await
|
||||
}
|
||||
@@ -6637,6 +6782,7 @@ mount {{
|
||||
worker_name,
|
||||
envs,
|
||||
occupancy_metrics,
|
||||
modules: modules.as_ref(),
|
||||
}))
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -199,6 +199,7 @@ async fn handle_build_binary_job(
|
||||
worker_name,
|
||||
base_internal_url,
|
||||
occupancy_metrics,
|
||||
script_data.modules.as_ref(),
|
||||
)
|
||||
.await?
|
||||
}
|
||||
@@ -214,6 +215,7 @@ async fn handle_build_binary_job(
|
||||
worker_name,
|
||||
base_internal_url,
|
||||
occupancy_metrics,
|
||||
script_data.modules.as_ref(),
|
||||
)
|
||||
.await?
|
||||
}
|
||||
@@ -235,6 +237,7 @@ async fn handle_build_binary_job(
|
||||
worker_name,
|
||||
base_internal_url,
|
||||
occupancy_metrics,
|
||||
script_data.modules.as_ref(),
|
||||
)
|
||||
.await?
|
||||
}
|
||||
@@ -3155,7 +3158,11 @@ async fn capture_dependency_job(
|
||||
)
|
||||
.await?
|
||||
{
|
||||
if !wd_exist {
|
||||
// Nothing here writes the module files, so a bundle built now resolves a
|
||||
// multi-file script's relative imports remotely rather than from its
|
||||
// modules; caching that under a key naming them would serve the wrong
|
||||
// code to every run. Leave it to the first run instead.
|
||||
if !wd_exist && modules.map_or(true, |m| m.is_empty()) {
|
||||
crate::bun_executor::prebundle_bun_script(
|
||||
job_raw_code,
|
||||
&lock,
|
||||
@@ -3169,6 +3176,7 @@ async fn capture_dependency_job(
|
||||
&token,
|
||||
&mut Some(occupancy_metrics),
|
||||
temp_script_refs,
|
||||
modules,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user