fix(cli): keep permissioned_as on single-item push, as sync push does (#11000)

* fix(cli): keep permissioned_as on single-item push, as sync push does

* fix(cli): resolve syncBehavior from the target workspace, not the branch alone

* refactor(cli): share the workspace-name resolution between sync and single-item push

* test(cli): import the moved workspace-name helper from its new home
This commit is contained in:
hugocasa
2026-09-07 16:46:35 +02:00
committed by GitHub
parent e2b63d177a
commit 5f3f99ba69
10 changed files with 316 additions and 89 deletions
+18 -2
View File
@@ -12,7 +12,11 @@ import * as wmill from "../../../gen/services.gen.ts";
import { ListableApp, Policy } from "../../../gen/types.gen.ts";
import { GlobalOptions, isSuperset } from "../../types.ts";
import { getWmillYamlPath, mergeConfigWithConfigFile } from "../../core/conf.ts";
import {
getWmillYamlPath,
mergeConfigWithConfigFile,
readEffectiveSyncBehavior,
} from "../../core/conf.ts";
import { readInlinePathSync } from "../../utils/utils.ts";
import devCommand from "./dev.ts";
import lintCommand from "./lint.ts";
@@ -21,6 +25,7 @@ import newCommand from "./new.ts";
import generateAgentsCommand from "./generate_agents.ts";
import { isVersionsGeq1585 } from "../sync/global.ts";
import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
import { buildPermissionedAsContext } from "../../core/permissioned_as.ts";
import { applyExtraPermsDiff } from "../../core/extra_perms.ts";
export interface AppFile {
@@ -420,6 +425,8 @@ async function push(
if (isRawAppByName || hasRawAppYaml) {
const { pushRawApp } = await import("./raw_apps.ts");
const merged = await mergeConfigWithConfigFile(opts);
// Raw-app ownership preservation is not implemented on either push
// path: sync push hands pushRawApp no context either.
await pushRawApp(
workspace.workspaceId,
remotePath,
@@ -429,7 +436,16 @@ async function push(
);
log.info(colors.bold.underline.green("Raw app pushed"));
} else {
await pushApp(workspace.workspaceId, remotePath, absoluteFilePath);
await pushApp(
workspace.workspaceId,
remotePath,
absoluteFilePath,
undefined,
await buildPermissionedAsContext(
workspace.workspaceId,
await readEffectiveSyncBehavior(opts, workspace),
),
);
log.info(colors.bold.underline.green("App pushed"));
}
}
+18 -3
View File
@@ -4,7 +4,7 @@ import { Command } from "@cliffy/command";
import { Confirm } from "@cliffy/prompt/confirm";
import { Table } from "@cliffy/table";
import * as log from "../../core/log.ts";
import { dirname, sep as SEP } from "node:path";
import { dirname, sep as SEP, resolve as pathResolve } from "node:path";
import { stringify as yamlStringify } from "yaml";
import { yamlParseFile } from "../../utils/yaml.ts";
import { readTextFile, validateRequiredArgs } from "../../utils/utils.ts";
@@ -21,11 +21,16 @@ import {
} 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";
import {
SyncOptions,
mergeConfigWithConfigFile,
readEffectiveSyncBehavior,
} from "../../core/conf.ts";
import { FSFSElement, elementsToMap, ignoreF } from "../sync/sync.ts";
import { Flow } from "../../../gen/types.gen.ts";
import { applyExtraPermsDiff } from "../../core/extra_perms.ts";
import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
import { buildPermissionedAsContext } from "../../core/permissioned_as.ts";
import {
collectPathScriptPaths,
replaceInlineScripts,
@@ -327,10 +332,20 @@ async function push(opts: Options & { message?: string }, filePath: string, remo
if (!validatePath(remotePath)) {
return;
}
// Reading the config moves the cwd to the wmill.yaml root when it sits in a
// parent directory, so pin the file against the invocation cwd first.
filePath = pathResolve(filePath);
const workspace = await resolveWorkspace(opts);
await requireLogin(opts);
const syncBehavior = await readEffectiveSyncBehavior(opts, workspace);
await pushFlow(workspace.workspaceId, remotePath, filePath, opts.message);
await pushFlow(
workspace.workspaceId,
remotePath,
filePath,
opts.message,
await buildPermissionedAsContext(workspace.workspaceId, syncBehavior)
);
log.info(colors.bold.underline.green("Flow pushed"));
}
+15 -4
View File
@@ -6,13 +6,19 @@ import { Command } from "@cliffy/command";
import { Table } from "@cliffy/table";
import { colors } from "@cliffy/ansi/colors";
import * as log from "../../core/log.ts";
import { sep as SEP } from "node:path";
import { sep as SEP, resolve as pathResolve } from "node:path";
import { requireLogin } from "../../core/auth.ts";
import { resolveWorkspace, validatePath } from "../../core/context.ts";
import { mergeConfigWithConfigFile } from "../../core/conf.ts";
import {
mergeConfigWithConfigFile,
readEffectiveSyncBehavior,
} from "../../core/conf.ts";
import * as wmill from "../../../gen/services.gen.ts";
import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
import { lookupUsernameByEmail } from "../../core/permissioned_as.ts";
import {
buildPermissionedAsContext,
lookupUsernameByEmail,
} from "../../core/permissioned_as.ts";
import {
GlobalOptions,
@@ -299,8 +305,12 @@ async function disable(opts: GlobalOptions, path: string) {
}
async function push(opts: GlobalOptions, filePath: string, remotePath: string) {
// Reading the config moves the cwd to the wmill.yaml root when it sits in a
// parent directory, so pin the file against the invocation cwd first.
filePath = pathResolve(filePath);
const workspace = await resolveWorkspace(opts);
await requireLogin(opts);
const syncBehavior = await readEffectiveSyncBehavior(opts, workspace);
if (!validatePath(remotePath)) {
return;
@@ -317,7 +327,8 @@ async function push(opts: GlobalOptions, filePath: string, remotePath: string) {
workspace.workspaceId,
remotePath,
undefined,
parseFromFile(filePath)
parseFromFile(filePath),
await buildPermissionedAsContext(workspace.workspaceId, syncBehavior)
);
console.log(colors.bold.underline.green("Schedule pushed"));
}
+7 -1
View File
@@ -7,6 +7,7 @@ import {
validatePath,
} from "../../core/context.ts";
import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
import { buildPermissionedAsContext } from "../../core/permissioned_as.ts";
import { applyExtraPermsDiff } from "../../core/extra_perms.ts";
import { writeFile, stat, mkdir } from "node:fs/promises";
import { Buffer } from "node:buffer";
@@ -58,6 +59,7 @@ import {
SyncOptions,
mergeConfigWithConfigFile,
readConfigFile,
readEffectiveSyncBehavior,
} from "../../core/conf.ts";
import { SyncCodebase, listSyncCodebases } from "../../utils/codebase.ts";
import { pollJobWithQueueLogging } from "../../utils/job_polling.ts";
@@ -231,7 +233,11 @@ async function push(opts: PushOptions, filePath: string) {
opts.message,
opts,
await getRawWorkspaceDependencies(true),
codebases
codebases,
await buildPermissionedAsContext(
workspace.workspaceId,
await readEffectiveSyncBehavior(opts, workspace)
)
);
log.info(colors.bold.underline.green(`Script ${filePath} pushed`));
}
+13 -75
View File
@@ -76,7 +76,8 @@ import {
} from "../../utils/utils.ts";
import {
getEffectiveSettings,
getWorkspaceNames,
inferWsNameFromProfile,
resolveWsNameForConfigFromFlags,
mergeConfigWithConfigFile,
parseSyncBehavior,
SyncOptions,
@@ -85,7 +86,10 @@ import {
WorkspaceEntryConfig,
} from "../../core/conf.ts";
import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
import { preCheckPermissionedAs } from "../../core/permissioned_as.ts";
import {
buildPermissionedAsContext,
preCheckPermissionedAs,
} from "../../core/permissioned_as.ts";
import {
fromWorkspaceSpecificPath,
toWorkspaceSpecificPath,
@@ -429,37 +433,6 @@ export function computeWsSpecificFlagOnlyPushes(
return out;
}
// Resolve workspace name from a --branch override (git branch → workspace name).
// Falls back to using the branch value as-is (backward compat: old key = branch name).
function resolveWsNameFromBranch(
opts: SyncOptions,
branchName: string,
): string {
const match = findWorkspaceByGitBranch(opts.workspaces, branchName);
return match ? match[0] : branchName;
}
// Resolve wsNameForConfig from CLI flags. Prefers --branch → matching config key,
// then --workspace → matching config key (incl. when --base-url is set). Returns
// undefined when no flag-based resolution applies; callers then fall back to
// inferWsNameFromProfile on the resolved workspace profile.
export function resolveWsNameForConfigFromFlags(
opts: SyncOptions & { branch?: string; workspace?: string },
): string | undefined {
if (opts.branch) {
return resolveWsNameFromBranch(opts, opts.branch);
}
if (opts.workspace) {
// Use getWorkspaceNames so reserved keys (e.g. commonSpecificItems) are filtered out,
// matching the behavior of findWorkspaceByGitBranch / inferWsNameFromProfile.
const validKeys = getWorkspaceNames(opts.workspaces);
if (validKeys.includes(opts.workspace)) {
return opts.workspace;
}
}
return undefined;
}
// Warn if --workspace overrides auto-detected branch or if workspace not in config.
function warnWorkspaceOverride(
opts: SyncOptions,
@@ -507,33 +480,6 @@ function resolveWsNameForFiles(_opts: SyncOptions, wsName: string): string {
return wsName;
}
// After resolveWorkspace, infer the workspace config name from the resolved profile
// by matching baseUrl + workspaceId against the workspaces config entries.
function inferWsNameFromProfile(
opts: SyncOptions,
profile: { remote: string; workspaceId: string },
): string | undefined {
if (!opts.workspaces) return undefined;
const wsNames = Object.keys(opts.workspaces).filter(
(k) => k !== "commonSpecificItems",
);
for (const name of wsNames) {
const entry = (opts.workspaces as any)[name] as WorkspaceEntryConfig;
if (!entry?.baseUrl) continue;
try {
const entryUrl = new URL(entry.baseUrl).toString();
const profileUrl = new URL(profile.remote).toString();
const entryWsId = entry.workspaceId ?? name;
if (entryUrl === profileUrl && entryWsId === profile.workspaceId) {
return name;
}
} catch {
continue;
}
}
return undefined;
}
// Merge CLI options with effective settings, preserving CLI flags as overrides
function mergeCliWithEffectiveOptions<
T extends GlobalOptions & SyncOptions & { repository?: string },
@@ -5540,27 +5486,19 @@ export async function push(
return;
}
let permissionedAsContext: PermissionedAsContext | undefined = undefined;
if (parseSyncBehavior(opts.syncBehavior) >= 1) {
const user = await wmill.whoami({ workspace: workspace.workspaceId });
const userIsAdminOrDeployer =
user.is_admin || (user.groups ?? []).includes("wm_deployers");
log.debug(
`permissioned_as: user=${user.email}, is_admin=${user.is_admin}, groups=${JSON.stringify(user.groups)}, isAdminOrDeployer=${userIsAdminOrDeployer}`,
const permissionedAsContext: PermissionedAsContext | undefined =
await buildPermissionedAsContext(
workspace.workspaceId,
opts.syncBehavior,
);
permissionedAsContext = {
userCache: new Map(),
userIsAdminOrDeployer,
userEmail: user.email,
};
if (permissionedAsContext) {
// ws_specific_flag changes have no content payload, so they don't
// affect permissioned_as resolution — filter them out before the
// pre-check (which expects only added/edited/deleted).
await preCheckPermissionedAs(
changes.filter((c) => c.name !== "ws_specific_flag"),
user.email,
userIsAdminOrDeployer,
permissionedAsContext.userEmail,
permissionedAsContext.userIsAdminOrDeployer,
opts.acceptOverridingPermissionedAsWithSelf ?? false,
!!process.stdin.isTTY,
);
+9 -2
View File
@@ -23,7 +23,7 @@ import { Command } from "@cliffy/command";
import { Table } from "@cliffy/table";
import { colors } from "@cliffy/ansi/colors";
import * as log from "../../core/log.ts";
import { sep as SEP } from "node:path";
import { sep as SEP, resolve as pathResolve } from "node:path";
import {
GlobalOptions,
isSuperset,
@@ -41,6 +41,8 @@ import { getCurrentGitBranch } from "../../utils/git.ts";
import { requireLogin } from "../../core/auth.ts";
import { validatePath, resolveWorkspace } from "../../core/context.ts";
import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
import { buildPermissionedAsContext } from "../../core/permissioned_as.ts";
import { readEffectiveSyncBehavior } from "../../core/conf.ts";
type Trigger = {
http: HttpTrigger;
@@ -620,8 +622,12 @@ async function extractTriggerKindFromPath(filePath: string): Promise<string | un
}
async function push(opts: GlobalOptions, filePath: string, remotePath: string) {
// Reading the config moves the cwd to the wmill.yaml root when it sits in a
// parent directory, so pin the file against the invocation cwd first.
filePath = pathResolve(filePath);
const workspace = await resolveWorkspace(opts);
await requireLogin(opts);
const syncBehavior = await readEffectiveSyncBehavior(opts, workspace);
if (!validatePath(remotePath)) {
return;
@@ -643,7 +649,8 @@ async function push(opts: GlobalOptions, filePath: string, remotePath: string) {
workspace.workspaceId,
remotePath,
undefined,
parseFromFile(filePath)
parseFromFile(filePath),
await buildPermissionedAsContext(workspace.workspaceId, syncBehavior)
);
console.log(colors.bold.underline.green("Trigger pushed"));
}
+77
View File
@@ -670,6 +670,83 @@ export async function getEffectiveSettings(
return effective;
}
// Resolve workspace name from a --branch override (git branch → workspace name).
// Falls back to using the branch value as-is (backward compat: old key = branch name).
function resolveWsNameFromBranch(opts: SyncOptions, branchName: string): string {
const match = findWorkspaceByGitBranch(opts.workspaces, branchName);
return match ? match[0] : branchName;
}
// Resolve wsNameForConfig from CLI flags. Prefers --branch → matching config key,
// then --workspace → matching config key (incl. when --base-url is set). Returns
// undefined when no flag-based resolution applies; callers then fall back to
// inferWsNameFromProfile on the resolved workspace profile.
export function resolveWsNameForConfigFromFlags(
opts: SyncOptions & { branch?: string; workspace?: string }
): string | undefined {
if (opts.branch) {
return resolveWsNameFromBranch(opts, opts.branch);
}
if (opts.workspace) {
// Use getWorkspaceNames so reserved keys (e.g. commonSpecificItems) are filtered out,
// matching the behavior of findWorkspaceByGitBranch / inferWsNameFromProfile.
const validKeys = getWorkspaceNames(opts.workspaces);
if (validKeys.includes(opts.workspace)) {
return opts.workspace;
}
}
return undefined;
}
/**
* Match a workspace config entry to a resolved workspace profile by remote +
* workspace id. The fallback for when no flag names the entry outright.
*/
export function inferWsNameFromProfile(
opts: SyncOptions,
profile: { remote: string; workspaceId: string }
): string | undefined {
if (!opts.workspaces) return undefined;
for (const name of getWorkspaceNames(opts.workspaces)) {
const entry = (opts.workspaces as any)[name] as WorkspaceEntryConfig;
if (!entry?.baseUrl) continue;
try {
const entryUrl = new URL(entry.baseUrl).toString();
const profileUrl = new URL(profile.remote).toString();
const entryWsId = entry.workspaceId ?? name;
if (entryUrl === profileUrl && entryWsId === profile.workspaceId) {
return name;
}
} catch {
continue;
}
}
return undefined;
}
/**
* `syncBehavior` as the workspace being pushed to sees it. The top level alone
* misses a `workspaces.<name>.overrides.syncBehavior`, which is where a repo
* that varies settings per workspace puts it, and the entry to read is the one
* `--workspace` names — falling back to the profile, then to the git branch —
* the same order `sync push` resolves it in.
*/
export async function readEffectiveSyncBehavior(
opts: { workspace?: string },
profile?: { remote: string; workspaceId: string }
): Promise<string | undefined> {
const config = await readConfigFile({ warnIfMissing: false });
const named = resolveWsNameForConfigFromFlags({ ...config, ...opts });
const effective = await getEffectiveSettings(
config,
undefined,
false,
true,
named ?? (profile ? inferWsNameFromProfile(config, profile) : undefined)
);
return effective.syncBehavior;
}
const RESERVED_WORKSPACE_KEYS = new Set(["commonSpecificItems"]);
/**
+30
View File
@@ -3,6 +3,7 @@ import * as log from "./log.ts";
import { colors } from "@cliffy/ansi/colors";
import { Confirm } from "@cliffy/prompt/confirm";
import { getTypeStrFromPath } from "../types.ts";
import { parseSyncBehavior } from "./conf.ts";
export interface PermissionedAsContext {
userCache: Map<string, { username: string; email: string }>;
@@ -10,6 +11,35 @@ export interface PermissionedAsContext {
userEmail: string;
}
/**
* The whole-tree `sync push` and the single-item `push` commands must resolve
* ownership the same way, so both build the context here: a push that leaves it
* undefined reassigns `permissioned_as` / `on_behalf_of` to whoever ran it.
* Undefined below syncBehavior v1, where that reassignment is the contract, and
* for a caller who is neither admin nor in `wm_deployers` the backend enforces
* it anyway — the flag on the context is what keeps the CLI from claiming
* otherwise.
*/
export async function buildPermissionedAsContext(
workspace: string,
syncBehavior: string | number | undefined
): Promise<PermissionedAsContext | undefined> {
if (parseSyncBehavior(syncBehavior) < 1) {
return undefined;
}
const user = await wmill.whoami({ workspace });
const userIsAdminOrDeployer =
user.is_admin || (user.groups ?? []).includes("wm_deployers");
log.debug(
`permissioned_as: user=${user.email}, is_admin=${user.is_admin}, groups=${JSON.stringify(user.groups)}, isAdminOrDeployer=${userIsAdminOrDeployer}`
);
return {
userCache: new Map(),
userIsAdminOrDeployer,
userEmail: user.email,
};
}
async function ensureUserCache(
workspace: string,
cache: Map<string, { username: string; email: string }>
@@ -0,0 +1,125 @@
/**
* Regression guard: the standalone `wmill schedule push` must resolve ownership
* the same way `wmill sync push` does. It only preserves the remote's
* `permissioned_as` when the command hands `pushSchedule` a context, so a push
* that builds none silently reassigns the schedule to whoever ran it.
*/
import { expect, test, describe, beforeEach, mock } from "bun:test";
import { mkdtemp, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
let updateScheduleCalls: any[] = [];
let remotePermissionedAs: string | undefined = "u/svc";
const REMOTE_SCHEDULE = () => ({
path: "u/admin/sched",
schedule: "0 0 */6 * * *",
timezone: "Etc/UTC",
script_path: "u/admin/script",
is_flow: false,
args: {},
enabled: false,
summary: "before",
permissioned_as: remotePermissionedAs,
});
mock.module("../gen/services.gen.ts", () => ({
getSchedule: async () => REMOTE_SCHEDULE(),
updateSchedule: async (a: unknown) => {
updateScheduleCalls.push(a);
},
whoami: async () => ({
email: "deployer@windmill.dev",
username: "deployer",
is_admin: true,
groups: [],
}),
}));
const realContext = await import("../src/core/context.ts");
mock.module("../src/core/context.ts", () => ({
...realContext,
resolveWorkspace: async () => ({
workspaceId: "w",
name: "w",
remote: "http://localhost/",
token: "t",
}),
}));
const realAuth = await import("../src/core/auth.ts");
mock.module("../src/core/auth.ts", () => ({
...realAuth,
requireLogin: async () => ({}),
}));
const scheduleCommand = (await import("../src/commands/schedule/schedule.ts"))
.default;
async function pushIn(wmillYamlTail: string): Promise<void> {
const dir = await mkdtemp(join(tmpdir(), "windmill_sched_push_"));
await writeFile(
join(dir, "wmill.yaml"),
`defaultTs: bun\nincludeSchedules: true\n${wmillYamlTail}`,
"utf-8"
);
await writeFile(
join(dir, "sched.schedule.yaml"),
`schedule: "0 0 */6 * * *"\ntimezone: Etc/UTC\nscript_path: u/admin/script\nis_flow: false\nargs: {}\nenabled: false\nsummary: after\n`,
"utf-8"
);
const cwd = process.cwd();
process.chdir(dir);
try {
await scheduleCommand.parse([
"push",
"sched.schedule.yaml",
"u/admin/sched",
]);
} finally {
process.chdir(cwd);
}
}
describe("wmill schedule push ownership", () => {
beforeEach(() => {
updateScheduleCalls = [];
remotePermissionedAs = "u/svc";
});
test("keeps the remote's permissioned_as under syncBehavior v1", async () => {
await pushIn("syncBehavior: v1\n");
expect(updateScheduleCalls).toHaveLength(1);
const body = updateScheduleCalls[0].requestBody;
expect(body.summary).toBe("after");
expect(body.permissioned_as).toBe("u/svc");
expect(body.preserve_permissioned_as).toBe(true);
});
// The entry to read is the one matching the workspace being pushed to, not
// the top level: a repo that varies settings per workspace puts syncBehavior
// under `overrides` and nowhere else.
test("reads syncBehavior from the target workspace's overrides", async () => {
await pushIn(
`workspaces:\n other:\n baseUrl: http://localhost/\n workspaceId: w\n overrides:\n syncBehavior: v1\n`
);
expect(updateScheduleCalls).toHaveLength(1);
const body = updateScheduleCalls[0].requestBody;
expect(body.permissioned_as).toBe("u/svc");
expect(body.preserve_permissioned_as).toBe(true);
});
test("leaves ownership to the backend below syncBehavior v1", async () => {
await pushIn("");
expect(updateScheduleCalls).toHaveLength(1);
const body = updateScheduleCalls[0].requestBody;
expect(body.permissioned_as).toBeUndefined();
expect(body.preserve_permissioned_as).toBeUndefined();
});
});
@@ -7,8 +7,10 @@ import { stringify as yamlStringify } from "yaml";
import { resolveWsNameForGitBranch } from "../src/core/specific_items.ts";
import { findResourceFile } from "../src/commands/script/script.ts";
import { resolveWsNameForConfigFromFlags } from "../src/commands/sync/sync.ts";
import type { SyncOptions } from "../src/core/conf.ts";
import {
resolveWsNameForConfigFromFlags,
type SyncOptions,
} from "../src/core/conf.ts";
// Integration tests covering the bug where workspace-specific filenames used
// the raw git branch name instead of the wmill.yaml workspace config key.