diff --git a/cli/src/commands/flow/flow.ts b/cli/src/commands/flow/flow.ts index 97da2f6699..6ace1eaf48 100644 --- a/cli/src/commands/flow/flow.ts +++ b/cli/src/commands/flow/flow.ts @@ -13,7 +13,12 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { buildFolderPath, getMetadataFileName, loadNonDottedPathsSetting } from "../../utils/resource_folders.ts"; import { requireLogin } from "../../core/auth.ts"; -import { resolveWorkspace, validatePath } from "../../core/context.ts"; +import { + assertRemotePath, + resolveWorkspace, + toSyncRootRelativePath, + validatePath, +} from "../../core/context.ts"; import { resolve, track_job, pollForJobResult } from "../script/script.ts"; import { defaultFlowDefinition } from "../../../bootstrap/flow_bootstrap.ts"; import { SyncOptions, mergeConfigWithConfigFile } from "../../core/conf.ts"; @@ -605,6 +610,8 @@ async function preview( log.setSilent(true); } const useLocalPathScripts = !opts.remote; + // Captured before the config read, which chdirs to the wmill.yaml root. + const cwdBeforeConfig = process.cwd(); if (useLocalPathScripts) { opts = await mergeConfigWithConfigFile(opts); } @@ -612,6 +619,9 @@ async function preview( await requireLogin(opts); const codebases = useLocalPathScripts ? listSyncCodebases(opts) : []; + const argPath = flowPath; + flowPath = toSyncRootRelativePath(flowPath, cwdBeforeConfig); + // Normalize path - ensure it's a directory path to a .flow or __flow folder const isFlowDir = flowPath.endsWith(".flow") || flowPath.endsWith(".flow" + SEP) || flowPath.endsWith("__flow") || flowPath.endsWith("__flow" + SEP); @@ -633,6 +643,14 @@ async function preview( flowPath += SEP; } + // The flow's windmill path (e.g. "f/cli_smoke/myrelflow"). It is what the + // preview job runs under, and the anchor for relative-import resolution: + // inline scripts in this flow are treated as living at + // "/", so "./util" resolves to + // "/util" — matching the keys in temp_script_refs. + const flowWmPath = stripFlowSuffix(flowPath).replaceAll(SEP, "/"); + assertRemotePath(flowWmPath, argPath); + // Read and parse the flow definition const localFlow = (await yamlParseFile(flowPath + "flow.yaml")) as FlowFile; @@ -703,12 +721,6 @@ async function preview( // too — PathScript modules have already been rewritten to inline rawscript // when `useLocalPathScripts` is set, and tempScriptRefs covers relative // imports in inline scripts. - // Compute the flow's windmill path (e.g. "f/cli_smoke/myrelflow"). Used as - // the anchor for relative-import resolution: inline scripts in this flow are - // treated as living at "/", so "./util" resolves to - // "/util" — matching the keys in temp_script_refs. - const flowWmPath = stripFlowSuffix(flowPath).replaceAll(SEP, "/"); - if (opts.step) { await previewStep(opts.step, localFlow, flowWmPath, workspace, input, tempScriptRefs, opts.silent, opts.tag); return; diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index 34a2b09949..37000c9ff3 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -1,6 +1,11 @@ import { GlobalOptions } from "../../types.ts"; import { requireLogin } from "../../core/auth.ts"; -import { resolveWorkspace, validatePath } from "../../core/context.ts"; +import { + assertRemotePath, + resolveWorkspace, + toSyncRootRelativePath, + validatePath, +} from "../../core/context.ts"; import type { PermissionedAsContext } from "../../core/permissioned_as.ts"; import { applyExtraPermsDiff } from "../../core/extra_perms.ts"; import { writeFile, stat, mkdir } from "node:fs/promises"; @@ -1808,13 +1813,16 @@ async function preview( if (opts.silent) { log.setSilent(true); } + // Captured before the config read, which chdirs to the wmill.yaml root. + const cwdBeforeConfig = process.cwd(); opts = await mergeConfigWithConfigFile(opts); const workspace = await resolveWorkspace(opts); await requireLogin(opts); - if (!validatePath(filePath)) { - return; - } + const argPath = filePath; + filePath = toSyncRootRelativePath(filePath, cwdBeforeConfig); + const remotePath = scriptPathToRemotePath(filePath); + assertRemotePath(remotePath, argPath); // Same as push: a descriptor-less dbt project's content path is deliberately // absent, and the project beside it is what says the script is real. @@ -1869,11 +1877,7 @@ async function preview( const { extractRelativeImports } = await import( "../../utils/relative_imports.ts" ); - const relImports = await extractRelativeImports( - content, - scriptPathToRemotePath(filePath), - language - ); + const relImports = await extractRelativeImports(content, remotePath, language); if (relImports.length > 0) { const { buildPreviewTempScriptRefs } = await import( "../generate-metadata/generate-metadata.ts" @@ -1976,7 +1980,7 @@ async function preview( const form = new FormData(); const previewPayload = { content: content, // Pass the original content (frontend does this too) - path: filePath.substring(0, filePath.indexOf(".")).replaceAll(SEP, "/"), + path: remotePath, args: input, language: language, tag: opts.tag, @@ -2046,7 +2050,7 @@ async function preview( workspace: workspace.workspaceId, requestBody: { content, - path: filePath.substring(0, filePath.indexOf(".")).replaceAll(SEP, "/"), + path: remotePath, args: input, language: language as any, tag: opts.tag, diff --git a/cli/src/core/context.ts b/cli/src/core/context.ts index 6440213d5c..502157f0a7 100644 --- a/cli/src/core/context.ts +++ b/cli/src/core/context.ts @@ -22,8 +22,11 @@ import { readConfigFile, findWorkspaceByGitBranch, getEffectiveWorkspaceId, + getWmillYamlPath, WorkspaceEntryConfig, } from "./conf.ts"; +import { existsSync, realpathSync } from "node:fs"; +import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; import { getCurrentGitBranch, getOriginalBranchForWorkspaceForks, @@ -743,6 +746,92 @@ export async function tryResolveVersion( } } +/** + * Directory the local tree mirrors the workspace from: the one holding + * wmill.yaml. Not `process.cwd()` — that only lands there once a config read + * has chdir'd into it, which `--remote` and fully-flagged invocations skip. + */ +function syncRoot(): string { + const wmillYaml = getWmillYamlPath(); + return wmillYaml ? dirname(wmillYaml) : process.cwd(); +} + +/** + * Re-express a user-supplied file or folder argument as a path relative to the + * sync root, so the Windmill path derived from it is the same whatever shape + * the argument had (`./f/a/b.ts`, `/abs/repo/f/a/b.ts`, `b.ts` from inside + * `f/a`). + * + * `cwdBeforeConfig` must be the working directory as it was *before* the + * command read wmill.yaml: reading it chdirs into the directory holding it, and + * a relative argument was written against the directory the user was in. + * Arguments are resolved against that directory first and the sync root second, + * so both readings work. + * + * Resolving the sync root leaves the process in it, which is what makes the + * returned path readable by the caller — keep that in step if this ever stops + * going through `getWmillYamlPath`. + */ +export function toSyncRootRelativePath( + arg: string, + cwdBeforeConfig: string +): string { + const root = syncRoot(); + const candidates = isAbsolute(arg) + ? [arg] + : [resolve(cwdBeforeConfig, arg), resolve(root, arg)]; + // A descriptor-less dbt project is named by a file that is deliberately not + // there, so an argument existing under neither reading is still a real one if + // the directory holding it is; only a path whose directory is missing too + // falls through to the sync-root reading for the caller to reject. + const abs = + candidates.find((c) => existsSync(c)) ?? + candidates.find((c) => existsSync(dirname(c))) ?? + candidates.at(-1)!; + const rel = relative(root, abs); + if (rel === "") return "."; + if (!rel.startsWith("..")) return rel; + // `relative` is purely lexical, so a root reached through a symlink (macOS' + // /var -> /private/var, a symlinked checkout) makes an absolute argument look + // like it escapes the tree. Resolve links only then, so a symlinked file + // *inside* the tree keeps the path it is filed under. + try { + const resolved = relative(realpathSync(root), realpathOfNamed(abs)); + return resolved === "" ? "." : resolved; + } catch { + return rel; + } +} + +/** + * `realpathSync` needs its target to exist, and a dbt descriptor deliberately + * does not. The directory naming it does, so resolve that and reattach. + */ +function realpathOfNamed(p: string): string { + return existsSync(p) + ? realpathSync(p) + : join(realpathSync(dirname(p)), basename(p)); +} + +/** Windmill workspace path: `u|f|g` followed by at least a folder and a name. */ +const REMOTE_PATH_RE = /^[ufg](\/[^/]+){2,}$/; + +/** + * Guard the Windmill path a preview run is pushed under. A preview job carries + * no runnable of its own, so this path is the only identity it has: it is what + * `WM_JOB_PATH` reports, what the runs page links to, and what relative imports + * inside the previewed code resolve against. + */ +export function assertRemotePath(remotePath: string, arg: string): void { + if (REMOTE_PATH_RE.test(remotePath)) return; + throw new Error( + `Cannot derive a Windmill path from '${arg}'` + + (remotePath ? ` (it maps to '${remotePath}')` : "") + + `: a preview runs under the path of the file it previews, which must sit inside the ` + + `wmill.yaml root and be of the form //.` + ); +} + export function validatePath(path: string): boolean { if (!(path.startsWith("g") || path.startsWith("u") || path.startsWith("f"))) { log.infoStderr( diff --git a/cli/test/preview.test.ts b/cli/test/preview.test.ts index 4ead0a06c9..ffa900c8f7 100644 --- a/cli/test/preview.test.ts +++ b/cli/test/preview.test.ts @@ -185,6 +185,60 @@ test("script preview: regular script (non-codebase)", async () => { }); }); +test("script preview: job path is the script's Windmill path for every argument shape", async () => { + await withTestBackend(async (backend, tempDir) => { + await createWmillConfig(tempDir, { defaultTs: "bun" }); + await createScript( + tempDir, + "f/test/job_path_script.ts", + `export function main() { + return process.env.WM_JOB_PATH; +}` + ); + + const invocations: Array<[string, string]> = [ + ["f/test/job_path_script.ts", tempDir], + ["./f/test/job_path_script.ts", tempDir], + [`${tempDir}/f/test/job_path_script.ts`, tempDir], + ["job_path_script.ts", `${tempDir}/f/test`], + ]; + + for (const [arg, workingDir] of invocations) { + const result = await backend.runCLICommand( + ["script", "preview", arg, "--silent"], + workingDir + ); + + expect(result.code).toEqual(0); + expect(result.stdout.trim()).toEqual(`"f/test/job_path_script"`); + } + + // Folder layout: the job runs under the script's path, not the entry file's. + await createScript( + tempDir, + "f/test/job_path_module__mod/script.ts", + `export function main() { + return process.env.WM_JOB_PATH; +}` + ); + const moduleResult = await backend.runCLICommand( + ["script", "preview", "f/test/job_path_module__mod/script.ts", "--silent"], + tempDir + ); + expect(moduleResult.code).toEqual(0); + expect(moduleResult.stdout.trim()).toEqual(`"f/test/job_path_module"`); + + // A file outside the workspace tree has no Windmill path to run under, so + // the run is refused rather than pushed with a made-up one. + await writeFile(`${tempDir}/stray.ts`, `export function main() {}`, "utf-8"); + const strayResult = await backend.runCLICommand( + ["script", "preview", "stray.ts", "--silent"], + tempDir + ); + expect(strayResult.code).toEqual(1); + }); +}); + test("script preview: codebase script (CJS)", async () => { await withTestBackend(async (backend, tempDir) => { await createWmillConfig(tempDir, { @@ -529,6 +583,37 @@ test("flow preview: simple flow", async () => { }); }); +test("flow preview: step job path is anchored on the flow's Windmill path", async () => { + await withTestBackend(async (backend, tempDir) => { + await createWmillConfig(tempDir, { defaultTs: "bun" }); + await createFlow(tempDir, "f/test/job_path_flow.flow", { + summary: "Test flow", + scriptContent: `export function main() { return process.env.WM_JOB_PATH; }`, + }); + + // The last one is `--remote` from a subdirectory: that combination reads + // no config of its own, so it is the one shape where nothing but the path + // resolution can put the process in the sync root. + const invocations: Array<[string[], string]> = [ + [["f/test/job_path_flow.flow"], tempDir], + [["./f/test/job_path_flow.flow"], tempDir], + [[`${tempDir}/f/test/job_path_flow.flow`], tempDir], + [["job_path_flow.flow"], `${tempDir}/f/test`], + [["--remote", "job_path_flow.flow"], `${tempDir}/f/test`], + ]; + + for (const [args, workingDir] of invocations) { + const result = await backend.runCLICommand( + ["flow", "preview", ...args, "--silent"], + workingDir + ); + + expect(result.code).toEqual(0); + expect(result.stdout.trim()).toEqual(`"f/test/job_path_flow/a"`); + } + }); +}); + test("flow preview: uses local PathScript by default and remote PathScript with --remote", async () => { await withTestBackend(async (backend, tempDir) => { await createWmillConfig(tempDir, { defaultTs: "bun" }); diff --git a/cli/test/preview_path_unit.test.ts b/cli/test/preview_path_unit.test.ts new file mode 100644 index 0000000000..4bf9e8632c --- /dev/null +++ b/cli/test/preview_path_unit.test.ts @@ -0,0 +1,112 @@ +/** + * A preview job carries no runnable, so the path derived from the file + * argument is the whole of its identity — it has to come out the same + * whatever shape the argument had, and be refused rather than guessed when + * the file has no place in the workspace tree. + */ +import { expect, test, describe, beforeEach, afterEach } from "bun:test"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import * as os from "node:os"; +import { + assertRemotePath, + toSyncRootRelativePath, +} from "../src/core/context.ts"; + +describe("toSyncRootRelativePath", () => { + let root: string; + let previousCwd: string; + let temps: string[]; + + beforeEach(() => { + previousCwd = process.cwd(); + temps = []; + // realpath: macOS' tmpdir is /var -> /private/var, and the assertions + // compare against what the process reports as its own directory. + root = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), "wmill_preview_path_")), + ); + temps.push(root); + fs.writeFileSync(path.join(root, "wmill.yaml"), "defaultTs: bun\n"); + fs.mkdirSync(path.join(root, "f", "test"), { recursive: true }); + fs.writeFileSync(path.join(root, "f", "test", "script.ts"), ""); + process.chdir(root); + }); + + afterEach(() => { + process.chdir(previousCwd); + for (const dir of temps) fs.rmSync(dir, { recursive: true, force: true }); + }); + + const normalized = (p: string) => p.replaceAll("\\", "/"); + + /** A dbt project whose descriptor is deliberately not written. */ + function dbtProject(): string { + const project = path.join(root, "f", "test", "proj__dbt"); + fs.mkdirSync(project, { recursive: true }); + fs.writeFileSync(path.join(project, "dbt_project.yml"), "name: proj\n"); + return project; + } + + test("every spelling of the same file lands on the same path", () => { + const fromRoot = ["f/test/script.ts", "./f/test/script.ts"].map((arg) => + normalized(toSyncRootRelativePath(arg, root)), + ); + const absolute = normalized( + toSyncRootRelativePath(path.join(root, "f", "test", "script.ts"), root), + ); + // As typed from the directory the file is in: the config read has already + // moved the process to the root by the time the argument is resolved. + const fromSubdir = normalized( + toSyncRootRelativePath("script.ts", path.join(root, "f", "test")), + ); + + expect(fromRoot).toEqual(["f/test/script.ts", "f/test/script.ts"]); + expect(absolute).toEqual("f/test/script.ts"); + expect(fromSubdir).toEqual("f/test/script.ts"); + }); + + test("a file that is deliberately absent keeps the directory it was named in", () => { + // A dbt project's descriptor is optional; `wmill script preview + // wm_dbt.yaml` from inside the project must still resolve to the project. + const project = dbtProject(); + + expect(normalized(toSyncRootRelativePath("wm_dbt.yaml", project))).toEqual( + "f/test/proj__dbt/wm_dbt.yaml", + ); + }); + + // Windows only creates symlinks for a privileged process. + test.skipIf(process.platform === "win32")( + "an absent file reached through a symlinked root still lands in the tree", + () => { + dbtProject(); + const aliasDir = fs.mkdtempSync(path.join(os.tmpdir(), "wmill_alias_")); + temps.push(aliasDir); + const alias = path.join(aliasDir, "link"); + fs.symlinkSync(root, alias, "dir"); + + const arg = path.join(alias, "f", "test", "proj__dbt", "wm_dbt.yaml"); + expect(normalized(toSyncRootRelativePath(arg, root))).toEqual( + "f/test/proj__dbt/wm_dbt.yaml", + ); + }, + ); + + test("a file outside the tree stays outside it", () => { + const outside = path.join(root, "..", "elsewhere.ts"); + expect(toSyncRootRelativePath(outside, root).startsWith("..")).toBe(true); + }); +}); + +describe("assertRemotePath", () => { + test("accepts a workspace path and refuses anything else", () => { + expect(() => assertRemotePath("f/test/script", "f/test/script.ts")).not.toThrow(); + expect(() => assertRemotePath("u/admin/script", "script.ts")).not.toThrow(); + for (const bad of ["", "script", "f/script", "../elsewhere"]) { + expect(() => assertRemotePath(bad, "arg.ts")).toThrow( + /Cannot derive a Windmill path/, + ); + } + }); +});