mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(cli): stop sync push from resetting an app's run-as, sandbox and execution mode
A raw app's policy was regenerated from the local sources alone on every push: raw_app.yaml records only the access mode, so `on_behalf_of` was rewritten to whoever ran the push, and `sandbox`, `frontend_sdk_scopes` and a `viewer` execution mode were dropped. A CI job running `wmill sync push` silently re-permissioned every raw app it touched. Start the regenerated policy from the deployed one, so the fields nothing local states carry over, and send `preserve_on_behalf_of` the way the low-code push already does. Low-code apps went through the same regeneration, so they lost `sandbox` too; both paths now share `deployedPolicyBase`, which drops the legacy `triggerables` the backend folds into `triggerables_v2` rather than carrying stale grants forward. The deletion-driven re-push (a file removed inside a flow/app/raw-app folder) also reached `pushObj` without the permissioned-as context, so it reset ownership even where the normal path preserved it. Fixes #11046 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaJ9KHDeaWrWUdVsHeHQ8L
This commit is contained in:
co-authored by
Claude Opus 5
parent
8aa8b7ee6c
commit
493d19689c
+48
-13
@@ -119,9 +119,14 @@ export function isExecutionModeAnonymous(app: any) {
|
||||
export function isExecutionModeGuest(app: any) {
|
||||
return app?.["policy"]?.["execution_mode"] == "guest";
|
||||
}
|
||||
export type AppExecutionMode = "anonymous" | "guest" | "publisher";
|
||||
export type AppExecutionMode =
|
||||
| "anonymous"
|
||||
| "guest"
|
||||
| "publisher"
|
||||
| "viewer";
|
||||
/** The access mode is the one policy field a tracked app keeps, as `public` (anonymous)
|
||||
* or `guests` (guest); the rest of the policy is regenerated on push. */
|
||||
* or `guests` (guest); the rest is carried over from the deployed policy on push
|
||||
* (see {@link deployedPolicyBase}). */
|
||||
export function markAccessFromPolicy(app: any) {
|
||||
if (isExecutionModeAnonymous(app)) {
|
||||
app.public = true;
|
||||
@@ -129,15 +134,42 @@ export function markAccessFromPolicy(app: any) {
|
||||
app.guests = true;
|
||||
}
|
||||
}
|
||||
export function executionModeFromAppFile(app: any): AppExecutionMode {
|
||||
export function executionModeFromAppFile(
|
||||
app: any,
|
||||
deployedPolicy?: Policy,
|
||||
): AppExecutionMode {
|
||||
if (app?.["public"] ?? isExecutionModeAnonymous(app)) {
|
||||
return "anonymous";
|
||||
}
|
||||
if (app?.["guests"] ?? isExecutionModeGuest(app)) {
|
||||
return "guest";
|
||||
}
|
||||
// `public`/`guests` are the only modes the file records, so "neither" covers
|
||||
// both publisher and viewer: keep the deployed one rather than demoting a
|
||||
// viewer app to publisher on every push.
|
||||
if (deployedPolicy?.execution_mode === "viewer") {
|
||||
return "viewer";
|
||||
}
|
||||
return "publisher";
|
||||
}
|
||||
|
||||
/**
|
||||
* What a push's regenerated policy starts from. The app file states only the
|
||||
* access mode, so regenerating from the local sources alone silently drops every
|
||||
* other policy field the deployed app carries — its run-as identity, sandbox
|
||||
* isolation, SDK scopes. Anything the file does state still wins.
|
||||
*
|
||||
* Legacy `triggerables` are dropped: the backend folds them into
|
||||
* `triggerables_v2` on read, so carrying them over would keep granting runnables
|
||||
* this deploy no longer contains.
|
||||
*/
|
||||
export function deployedPolicyBase(
|
||||
deployedPolicy: Policy | undefined,
|
||||
localPolicy: Policy | undefined,
|
||||
): Policy {
|
||||
const { triggerables: _legacy, ...deployed } = deployedPolicy ?? {};
|
||||
return { ...deployed, ...(localPolicy ?? {}) } as Policy;
|
||||
}
|
||||
export async function pushApp(
|
||||
workspace: string,
|
||||
remotePath: string,
|
||||
@@ -161,12 +193,9 @@ export async function pushApp(
|
||||
//ignore
|
||||
}
|
||||
|
||||
let remoteOnBehalfOf: string | undefined;
|
||||
let remoteOnBehalfOfEmail: string | undefined;
|
||||
if (app?.policy) {
|
||||
remoteOnBehalfOf = app.policy.on_behalf_of;
|
||||
remoteOnBehalfOfEmail = app.policy.on_behalf_of_email;
|
||||
}
|
||||
const remotePolicy = app?.policy as Policy | undefined;
|
||||
const remoteOnBehalfOf = remotePolicy?.on_behalf_of;
|
||||
const remoteOnBehalfOfEmail = remotePolicy?.on_behalf_of_email;
|
||||
|
||||
markAccessFromPolicy(app);
|
||||
// console.log(app);
|
||||
@@ -181,7 +210,11 @@ export async function pushApp(
|
||||
const localApp = (await yamlParseFile(path)) as AppFile;
|
||||
|
||||
replaceInlineScripts(localApp.value, localPath, true);
|
||||
await generatingPolicy(localApp, remotePath, executionModeFromAppFile(localApp));
|
||||
// Read the mode off the file before the deployed policy is merged in, so
|
||||
// dropping `public` from app.yaml still demotes the app.
|
||||
const executionMode = executionModeFromAppFile(localApp, remotePolicy);
|
||||
localApp.policy = deployedPolicyBase(remotePolicy, localApp.policy);
|
||||
await generatingPolicy(localApp, remotePath, executionMode);
|
||||
|
||||
const preserveFields: { preserve_on_behalf_of?: boolean } = {};
|
||||
if (permissionedAsContext?.userIsAdminOrDeployer) {
|
||||
@@ -255,7 +288,7 @@ export async function generatingPolicy(
|
||||
) {
|
||||
log.info(colors.gray(`Generating fresh policy for app ${path}...`));
|
||||
try {
|
||||
app.policy = await windmillUtils.updatePolicy(app.value, undefined);
|
||||
app.policy = await windmillUtils.updatePolicy(app.value, app.policy);
|
||||
app.policy.execution_mode = executionMode;
|
||||
} catch (e) {
|
||||
log.error(colors.red(`Error generating policy for app ${path}: ${e}`));
|
||||
@@ -425,14 +458,16 @@ 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,
|
||||
absoluteFilePath,
|
||||
undefined,
|
||||
merged.defaultTs,
|
||||
await buildPermissionedAsContext(
|
||||
workspace.workspaceId,
|
||||
await readEffectiveSyncBehavior(opts, workspace),
|
||||
),
|
||||
);
|
||||
log.info(colors.bold.underline.green("Raw app pushed"));
|
||||
} else {
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
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 { colors } from "@cliffy/ansi/colors";
|
||||
import * as log from "../../core/log.ts";
|
||||
import { sep as SEP } from "node:path";
|
||||
@@ -13,10 +16,15 @@ import path from "node:path";
|
||||
import { readdir } from "node:fs/promises";
|
||||
|
||||
import { GlobalOptions, isSuperset } from "../../types.ts";
|
||||
import {
|
||||
buildPermissionedAsContext,
|
||||
type PermissionedAsContext,
|
||||
} from "../../core/permissioned_as.ts";
|
||||
import { deepEqual, readTextFile } from "../../utils/utils.ts";
|
||||
|
||||
import {
|
||||
type AppExecutionMode,
|
||||
deployedPolicyBase,
|
||||
executionModeFromAppFile,
|
||||
markAccessFromPolicy,
|
||||
replaceInlineScripts,
|
||||
@@ -360,6 +368,7 @@ export async function pushRawApp(
|
||||
localPath: string,
|
||||
message?: string,
|
||||
defaultTs: "bun" | "deno" = "bun",
|
||||
permissionedAsContext?: PermissionedAsContext,
|
||||
): Promise<void> {
|
||||
if (alreadySynced.includes(localPath)) {
|
||||
return;
|
||||
@@ -376,6 +385,8 @@ export async function pushRawApp(
|
||||
} catch {
|
||||
//ignore
|
||||
}
|
||||
|
||||
const remotePolicy = app?.policy as Policy | undefined;
|
||||
markAccessFromPolicy(app);
|
||||
// console.log(app);
|
||||
if (app) {
|
||||
@@ -423,13 +434,33 @@ export async function pushRawApp(
|
||||
repopulateFields(runnables);
|
||||
|
||||
// Create a temporary app object for policy generation
|
||||
const appForPolicy = { ...localApp, runnables };
|
||||
const appForPolicy = {
|
||||
...localApp,
|
||||
runnables,
|
||||
policy: deployedPolicyBase(remotePolicy, localApp.policy),
|
||||
};
|
||||
await generatingPolicy(
|
||||
appForPolicy,
|
||||
remotePath,
|
||||
executionModeFromAppFile(localApp),
|
||||
executionModeFromAppFile(localApp, remotePolicy),
|
||||
);
|
||||
|
||||
// Submitting a policy is how the backend reads a claim on the app's execution
|
||||
// identity; without the flag it rewrites on_behalf_of to whoever ran the push.
|
||||
const preserveFields: { preserve_on_behalf_of?: boolean } = {};
|
||||
if (
|
||||
permissionedAsContext?.userIsAdminOrDeployer &&
|
||||
appForPolicy.policy.on_behalf_of
|
||||
) {
|
||||
preserveFields.preserve_on_behalf_of = true;
|
||||
log.info(
|
||||
`Preserving ${
|
||||
appForPolicy.policy.on_behalf_of_email ??
|
||||
appForPolicy.policy.on_behalf_of
|
||||
} as permissioned_as for app ${remotePath}`,
|
||||
);
|
||||
}
|
||||
|
||||
const files = await collectAppFiles(localPath);
|
||||
async function createBundleRaw() {
|
||||
log.info(colors.yellow.bold(`Creating raw app ${remotePath} bundle...`));
|
||||
@@ -483,6 +514,7 @@ export async function pushRawApp(
|
||||
summary: localApp.summary,
|
||||
policy: appForPolicy.policy,
|
||||
deployment_message: message,
|
||||
...preserveFields,
|
||||
// Preserve any user draft at this path (see backend skip_draft_deletion).
|
||||
skip_draft_deletion: true,
|
||||
...(localApp.custom_path
|
||||
@@ -505,6 +537,7 @@ export async function pushRawApp(
|
||||
summary: localApp.summary,
|
||||
policy: appForPolicy.policy,
|
||||
deployment_message: message,
|
||||
...preserveFields,
|
||||
// Preserve any user draft at this path (see backend skip_draft_deletion).
|
||||
skip_draft_deletion: true,
|
||||
...(localApp.custom_path
|
||||
@@ -564,6 +597,10 @@ async function pushRawAppCommand(
|
||||
filePath,
|
||||
undefined,
|
||||
merged.defaultTs,
|
||||
await buildPermissionedAsContext(
|
||||
workspace.workspaceId,
|
||||
await readEffectiveSyncBehavior(opts, workspace),
|
||||
),
|
||||
);
|
||||
log.info(colors.bold.underline.green("Raw app pushed"));
|
||||
}
|
||||
|
||||
@@ -6142,7 +6142,7 @@ export async function push(
|
||||
undefined,
|
||||
opts.plainSecrets ?? false,
|
||||
alreadySynced,
|
||||
{ message: opts.message },
|
||||
{ message: opts.message, permissionedAsContext },
|
||||
);
|
||||
} else {
|
||||
// Flow folder doesn't exist locally — delete on server
|
||||
@@ -6187,7 +6187,7 @@ export async function push(
|
||||
undefined,
|
||||
opts.plainSecrets ?? false,
|
||||
alreadySynced,
|
||||
{ message: opts.message },
|
||||
{ message: opts.message, permissionedAsContext },
|
||||
);
|
||||
} else {
|
||||
// App folder doesn't exist locally — delete on server
|
||||
@@ -6233,7 +6233,11 @@ export async function push(
|
||||
undefined,
|
||||
opts.plainSecrets ?? false,
|
||||
alreadySynced,
|
||||
{ message: opts.message, defaultTs: opts.defaultTs },
|
||||
{
|
||||
message: opts.message,
|
||||
defaultTs: opts.defaultTs,
|
||||
permissionedAsContext,
|
||||
},
|
||||
);
|
||||
} else {
|
||||
// The entire raw app folder was deleted locally,
|
||||
|
||||
+1
-1
@@ -234,7 +234,7 @@ export async function pushObj(
|
||||
if (!rawAppName) {
|
||||
throw new Error(`Could not extract raw app name from path: ${p}`);
|
||||
}
|
||||
await pushRawApp(workspace, rawAppName, buildFolderPath(rawAppName, "raw_app"), message, defaultTs);
|
||||
await pushRawApp(workspace, rawAppName, buildFolderPath(rawAppName, "raw_app"), message, defaultTs, permissionedAsContext);
|
||||
} else if (typeEnding === "folder") {
|
||||
await pushFolder(workspace, p, befObj, newObj);
|
||||
} else if (typeEnding === "variable") {
|
||||
|
||||
@@ -746,3 +746,80 @@ excludes: []`, "utf-8");
|
||||
expect(lowercased).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
test("Raw App: push preserves the deployed policy's run-as, sandbox and execution mode", async () => {
|
||||
await withTestBackend(async (backend, tempDir) => {
|
||||
const testWorkspace = {
|
||||
remote: backend.baseUrl,
|
||||
workspaceId: backend.workspace,
|
||||
name: "raw_app_policy_test",
|
||||
token: backend.token
|
||||
};
|
||||
await addWorkspace(testWorkspace, { force: true, configDir: backend.testConfigDir });
|
||||
|
||||
// syncBehavior v1 is what enables ownership preservation on update.
|
||||
await writeFile(`${tempDir}/wmill.yaml`, `defaultTs: bun
|
||||
includes:
|
||||
- "**"
|
||||
excludes: []
|
||||
syncBehavior: v1`, "utf-8");
|
||||
|
||||
const appDir = path.join(tempDir, "f", "test", "policy_app.raw_app");
|
||||
await mkdir(path.join(tempDir, "f", "test"), { recursive: true });
|
||||
await createRawAppOnDisk(appDir);
|
||||
|
||||
const pushResult1 = await backend.runCLICommand(
|
||||
["sync", "push", "--yes"],
|
||||
tempDir, "raw_app_policy_test"
|
||||
);
|
||||
expect(pushResult1.code).toEqual(0);
|
||||
await waitForDeploymentJobs(backend);
|
||||
|
||||
// Stand in for the deploy drawer: give the app a run-as identity that is
|
||||
// not the pushing user, sandbox isolation and a non-default mode. None of
|
||||
// it is recorded in raw_app.yaml, so only the deployed policy carries it.
|
||||
const setPolicy = await backend.apiRequest!(
|
||||
`/api/w/${backend.workspace}/apps/update/f/test/policy_app`,
|
||||
{
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
policy: {
|
||||
on_behalf_of: "u/svc",
|
||||
on_behalf_of_email: "svc@windmill.dev",
|
||||
sandbox: true,
|
||||
execution_mode: "viewer",
|
||||
triggerables_v2: {},
|
||||
},
|
||||
preserve_on_behalf_of: true,
|
||||
}),
|
||||
}
|
||||
);
|
||||
expect(setPolicy.status).toEqual(200);
|
||||
|
||||
// Edit a source file so the app is not up to date and gets redeployed.
|
||||
const appTsxPath = path.join(appDir, "App.tsx");
|
||||
await writeFile(
|
||||
appTsxPath,
|
||||
(await readFileContent(appTsxPath)).replace("hello world", "hello again"),
|
||||
"utf-8"
|
||||
);
|
||||
|
||||
const pushResult2 = await backend.runCLICommand(
|
||||
["sync", "push", "--yes"],
|
||||
tempDir, "raw_app_policy_test"
|
||||
);
|
||||
expect(pushResult2.code).toEqual(0);
|
||||
await waitForDeploymentJobs(backend);
|
||||
|
||||
const getResp = await backend.apiRequest!(
|
||||
`/api/w/${backend.workspace}/apps/get/p/f/test/policy_app`
|
||||
);
|
||||
expect(getResp.status).toEqual(200);
|
||||
const policy = (await getResp.json()).policy;
|
||||
expect(policy.on_behalf_of).toEqual("u/svc");
|
||||
expect(policy.on_behalf_of_email).toEqual("svc@windmill.dev");
|
||||
expect(policy.sandbox).toEqual(true);
|
||||
expect(policy.execution_mode).toEqual("viewer");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user