mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
3cda447621
* fix(flows): reject corrupt step paths at deploy + atomic cache writes (#9751) A flow step could execute an unrelated (and in the reported case, destructive) script at runtime even though every stored definition looked correct. A forensic dump traced it to two issues: - Deploy accepted absolute/local step paths. `wmill sync push` from a feature- branch checkout under /tmp baked an absolute path (`/tmp/.../ops/scripts/clean_device/...`) into a step's `value.path`. Persisted verbatim, it mis-resolved to an unrelated script at runtime. - The on-disk cache write was neither truncating nor atomic. `FsBackedCache::put` used `write+create`, so a shorter overwrite left stale trailing bytes and concurrent writers could interleave into a torn file — a corrupt cached blob that a worker then scheduled from. Fixes: - Reject non-workspace flow step paths (must be u/, f/, g/ or hub/) in `validate_flow_value` (covers create_flow + update_flow, recursively through loops/branches/AI-agent tools) and early in the CLI `pushFlow`. - Make `FsBackedCache::put` write a unique temp file (truncate + fsync) then atomically rename it over the target, cleaning up on error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(flows): validate failure/preprocessor module paths + sub-flow paths in CLI Address PR review (cubic + claude): - Backend `validate_flow_value` is the authoritative guard but only walked `modules`; extend it to also validate `failure_module` and `preprocessor_module` (which can themselves be sub-flows/loops/branches), so an absolute path there can't be persisted. - CLI preflight only collected `type: "script"` paths; now collects sub-flow (`type: "flow"`) step paths too (recursively, incl. failure/preprocessor), so the comment's claim matches the behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): include AI-agent tool step paths in flow path preflight Address Codex review: collectStepPaths skipped aiagent tools, so a bad path in a tool fell through to the API error instead of the local fail-fast. The backend already validates these (traverse_modules walks AIAgent tools); this aligns the CLI early-error with it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(flows): make failure/preprocessor path test key explicit The test used `slot:` as a json! key. json! does interpolate an ident key to its variable's value (json!({slot:1}) with slot="failure_module" => {"failure_module":1}), so the test was correct and exercised the validation — but the behavior is subtle, so build the key explicitly via serde_json::Map to remove ambiguity (review nit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cache): use a UUID temp name for atomic put (shared-volume safe) Address Codex (P1): pid+counter temp names collide across container PID namespaces on a shared cache volume (same pid, PUT_SEQ resets to 0 per process), so two workers could truncate/clobber the same temp file before rename. Use a random UUID suffix (matching worker.rs's atomic-write helpers) — globally unique, so the cross-process temp-file hazard is closed. Also trims the comment to the AGENTS.md <=4-line limit (Pi nit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>