fix: keep an app's deployed policy on wmill push (#11049)

* fix: keep an app's deployed policy on wmill push

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: keep a first push's file-stated app policy

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: deploy a file-stated viewer app as viewer, not publisher

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: never put a repo-stated run identity on the wire

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: send the deployed run identity only when the push may claim it

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: let a file-stated execution mode win in both directions

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* refactor: drop the app-file execution mode helper with no caller

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* feat: warn before a raw-app push takes over the run-as user

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: keep an app's run-as user when a push only deletes one of its files

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* refactor: settle an app's ownership check before any content parsing

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: share one rule for which raw-app files a push sends

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: stop tracking raw-app files no push ever sends

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* test: bundle for real instead of stubbing the module for every suite

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* docs: record why a module mock cannot be undone by afterAll

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* fix: keep tracking a runnable whose file shares a bundle-excluded name

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

* test: pin both halves of the backend runnable rule

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X47gNFqf1SsA67zTm71f8c

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-10 17:19:02 +02:00
committed by GitHub
co-authored by Claude Opus 5
parent 385086ffc2
commit 0af7675588
10 changed files with 623 additions and 78 deletions
+22
View File
@@ -35,6 +35,28 @@ binary and starts a shared backend instance.
Examples: `sync_pull_push`, `dev_server`, `standalone_commands`
## Module mocks
`mock.module` replaces a module for the **whole process**, and it does reach modules that
were already imported — a stub one file installs lands on a consumer an earlier file
loaded.
Handing the module back in `afterAll` is not a reliable undo. Files do run one at a time
(a root-level `afterAll` completes before the next file's body evaluates), so it looks
like it should be — but stubbing `bundle.ts` and restoring it that way still left
`raw_app_svelte_plugin_unit.test.ts` asserting against an empty bundle, green on Linux
and red on Windows, where the `readdir` file order differs. Treat a stub as permanent for
the run.
So the rule is about what you stub, not how you clean up: **stub only a module no other
in-process suite imports.** Check with `grep -rl "<exported fn>" test/` before reaching
for one. A suite that drives the CLI through a spawned process is out of reach of a
module mock and doesn't count.
`raw_app_push_policy_unit.test.ts` is the worked example: it stubs `gen/services.gen.ts`,
which passes the rule because nothing else in `test/` imports the three API functions it
replaces, and deliberately does not stub `bundle.ts`, which failed it.
## AI Benchmark Caveats
The repo-level benchmark CLI lives under `ai_evals/`, but it currently depends on
+106 -29
View File
@@ -119,9 +119,10 @@ 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 of the policy is preserved from the deployed app on
* push (see `generatingPolicy`). */
export function markAccessFromPolicy(app: any) {
if (isExecutionModeAnonymous(app)) {
app.public = true;
@@ -129,14 +130,35 @@ export function markAccessFromPolicy(app: any) {
app.guests = true;
}
}
export function executionModeFromAppFile(app: any): AppExecutionMode {
/** The mode the tracked file states, or `undefined` when it states none — the
* normal case, since a pull writes only the two open-access markers. `viewer`
* and `publisher` have no marker of their own, so a file can only name them
* through a policy block it was hand-written with. */
function statedExecutionMode(app: any): AppExecutionMode | undefined {
if (app?.["public"] ?? isExecutionModeAnonymous(app)) {
return "anonymous";
}
if (app?.["guests"] ?? isExecutionModeGuest(app)) {
return "guest";
}
return "publisher";
const mode = app?.["policy"]?.["execution_mode"];
return mode === "viewer" || mode === "publisher" ? mode : undefined;
}
/** The mode this push deploys under. A file that states one is authoritative, in
* both directions. Otherwise the two open-access markers are all it says, so
* their absence closes a deployed `anonymous`/`guest` app back down to
* `publisher` — while a deployed `viewer` is not a grant those markers revoke,
* so it carries over rather than widening to `publisher`. */
export function executionModeForPush(
localApp: any,
deployedPolicy: Policy | undefined,
): AppExecutionMode {
const stated = statedExecutionMode(localApp);
if (stated) {
return stated;
}
return deployedPolicy?.execution_mode === "viewer" ? "viewer" : "publisher";
}
export async function pushApp(
workspace: string,
@@ -161,12 +183,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;
}
// `app.policy` is cleared a few lines down, so capture it first: it is the
// base the regenerated policy is built on.
const deployedPolicy: Policy | undefined = app?.policy;
markAccessFromPolicy(app);
// console.log(app);
@@ -181,20 +200,18 @@ export async function pushApp(
const localApp = (await yamlParseFile(path)) as AppFile;
replaceInlineScripts(localApp.value, localPath, true);
await generatingPolicy(localApp, remotePath, executionModeFromAppFile(localApp));
const preserveFields: { preserve_on_behalf_of?: boolean } = {};
if (permissionedAsContext?.userIsAdminOrDeployer) {
if (app) {
if (localApp.policy && remoteOnBehalfOf) {
(localApp.policy as any).on_behalf_of = remoteOnBehalfOf;
(localApp.policy as any).on_behalf_of_email = remoteOnBehalfOfEmail;
preserveFields.preserve_on_behalf_of = true;
log.info(`Preserving ${remoteOnBehalfOfEmail ?? remoteOnBehalfOf} as permissioned_as for app ${remotePath}`);
}
}
// On create: backend applies folder defaults
}
// On create the backend applies folder defaults, so there is nothing to preserve.
const preserveFields = preserveOnBehalfOfFields(
remotePath,
deployedPolicy,
permissionedAsContext
);
await generatingPolicy(
localApp,
remotePath,
executionModeForPush(localApp, deployedPolicy),
basePolicy(localApp, deployedPolicy, !!preserveFields.preserve_on_behalf_of)
);
// extra_perms goes through /acls/* — strip from the body so a perms-only
// edit never bumps the app version (see applyExtraPermsDiff for details).
@@ -251,18 +268,76 @@ export async function pushApp(
export async function generatingPolicy(
app: any,
path: string,
executionMode: AppExecutionMode
executionMode: AppExecutionMode,
base: Policy | undefined
) {
log.info(colors.gray(`Generating fresh policy for app ${path}...`));
try {
app.policy = await windmillUtils.updatePolicy(app.value, undefined);
app.policy.execution_mode = executionMode;
app.policy = await windmillUtils.updatePolicy(app.value, base);
finalizeDerivedPolicy(app.policy, executionMode);
} catch (e) {
log.error(colors.red(`Error generating policy for app ${path}: ${e}`));
throw e;
}
}
/** What the regenerated policy starts from: the deployed one, so a push keeps
* settings the tracked file doesn't record; on a first push, whatever the file
* states. The run identity rides along only when `claimsOnBehalfOf` — never
* from the file, never from a pusher who may not preserve one, since `wmill`
* is regularly pointed at servers older than the rewrite that would fix it. */
export function basePolicy(
localApp: any,
deployedPolicy: Policy | undefined,
claimsOnBehalfOf: boolean
): Policy | undefined {
const stated = deployedPolicy ?? (localApp?.policy as Policy | undefined);
if (!stated || claimsOnBehalfOf) {
return stated;
}
const base: Policy = { ...stated };
delete base.on_behalf_of;
delete base.on_behalf_of_email;
return base;
}
/** Claim the run-as identity the regenerated policy carries over from the
* deployed app. Only a deployed identity may be claimed, never one the tracked
* file states — a repo doesn't get to pick who an app runs as. Without the flag
* the backend rewrites `on_behalf_of` to whoever is pushing, and it only honors
* the flag for an admin or a `wm_deployers` member, so a caller who is neither
* doesn't get to claim it here either. */
export function preserveOnBehalfOfFields(
remotePath: string,
deployedPolicy: Policy | undefined,
permissionedAsContext: PermissionedAsContext | undefined
): { preserve_on_behalf_of?: boolean } {
const onBehalfOf = deployedPolicy?.on_behalf_of;
if (!permissionedAsContext?.userIsAdminOrDeployer || !onBehalfOf) {
return {};
}
log.info(
`Preserving ${deployedPolicy?.on_behalf_of_email ?? onBehalfOf} as permissioned_as for app ${remotePath}`
);
return { preserve_on_behalf_of: true };
}
/** The policy is written wholesale by the deploy, so the fields it does not
* derive from the tracked sources have to survive the trip. The policy builder
* has already recomputed what it can — the triggerables on both paths, plus the
* S3 rules on the low-code one, which `updateRawAppPolicy` has no equivalent of
* and so carries over. This sets the two left: the access mode, and the legacy
* `triggerables`, which still grant execution (the backend folds them into
* `triggerables_v2` at run time) and so are dropped rather than carried, or a
* deployed app would keep being able to run runnables this push removed. */
export function finalizeDerivedPolicy(
policy: Policy,
executionMode: AppExecutionMode
) {
policy.triggerables = undefined;
policy.execution_mode = executionMode;
}
async function list(opts: GlobalOptions & { includeDraftOnly?: boolean; json?: boolean }) {
const workspace = await resolveWorkspace(opts);
await requireLogin(opts);
@@ -425,14 +500,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 {
+45 -18
View File
@@ -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";
@@ -17,13 +20,22 @@ import { deepEqual, readTextFile } from "../../utils/utils.ts";
import {
type AppExecutionMode,
executionModeFromAppFile,
basePolicy,
executionModeForPush,
finalizeDerivedPolicy,
markAccessFromPolicy,
preserveOnBehalfOfFields,
replaceInlineScripts,
repopulateFields,
} from "./app.ts";
import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
import { buildPermissionedAsContext } from "../../core/permissioned_as.ts";
import { createBundle, detectFrameworks } from "./bundle.ts";
import { APP_BACKEND_FOLDER, RECORDINGS_FOLDER } from "./app_metadata.ts";
import {
NEVER_DEPLOYED_DIRS,
NEVER_DEPLOYED_FILES,
} from "../../utils/app_files.ts";
import { writeIfChanged } from "../../utils/utils.ts";
import { yamlOptions } from "../sync/sync.ts";
import { applyExtraPermsDiff } from "../../core/extra_perms.ts";
@@ -316,13 +328,11 @@ async function collectAppFiles(
const relativePath = basePath + entry.name;
if (entry.isDirectory()) {
// Skip the runnables, node_modules, and sql_to_apply subfolders
// The backend folder deploys as `value.runnables`, not as a bundled
// file; the rest reach the server through no channel at all.
if (
entry.name === APP_BACKEND_FOLDER ||
entry.name === "node_modules" ||
entry.name === "dist" ||
entry.name === ".claude" ||
entry.name === "sql_to_apply"
NEVER_DEPLOYED_DIRS.has(entry.name)
) {
continue;
}
@@ -334,13 +344,11 @@ async function collectAppFiles(
}
await readDirRecursive(fullPath + SEP, relativePath + "/");
} else if (entry.isFile()) {
// Skip generated/metadata files that shouldn't be part of the app
// `raw_app.yaml` deploys as the request's metadata rather than as a
// bundled file; the rest reach the server through no channel at all.
if (
entry.name === "raw_app.yaml" ||
entry.name === "package-lock.json" ||
entry.name === "DATATABLES.md" ||
entry.name === "AGENTS.md" ||
entry.name === "wmill.d.ts"
NEVER_DEPLOYED_FILES.has(entry.name)
) {
continue;
}
@@ -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,10 @@ export async function pushRawApp(
} catch {
//ignore
}
// `app.policy` is cleared a few lines down, so capture it first. `raw_app.yaml`
// records none of the policy, so anything the deploy drawer set is only here.
const deployedPolicy: Policy | undefined = app?.policy;
markAccessFromPolicy(app);
// console.log(app);
if (app) {
@@ -424,10 +437,21 @@ export async function pushRawApp(
// Create a temporary app object for policy generation
const appForPolicy = { ...localApp, runnables };
// On create the backend applies folder defaults, so there is nothing to preserve.
const preserveFields = preserveOnBehalfOfFields(
remotePath,
deployedPolicy,
permissionedAsContext,
);
await generatingPolicy(
appForPolicy,
remotePath,
executionModeFromAppFile(localApp),
executionModeForPush(localApp, deployedPolicy),
basePolicy(
localApp,
deployedPolicy,
!!preserveFields.preserve_on_behalf_of,
),
);
const files = await collectAppFiles(localPath);
@@ -482,6 +506,7 @@ export async function pushRawApp(
path: remotePath,
summary: localApp.summary,
policy: appForPolicy.policy,
...preserveFields,
deployment_message: message,
// Preserve any user draft at this path (see backend skip_draft_deletion).
skip_draft_deletion: true,
@@ -532,14 +557,12 @@ export async function generatingPolicy(
app: any,
path: string,
executionMode: AppExecutionMode,
base: Policy | undefined,
) {
log.info(colors.gray(`Generating fresh policy for app ${path}...`));
try {
app.policy = await windmillUtils.updateRawAppPolicy(
app.runnables,
app.policy,
);
app.policy.execution_mode = executionMode;
app.policy = await windmillUtils.updateRawAppPolicy(app.runnables, base);
finalizeDerivedPolicy(app.policy, executionMode);
} catch (e) {
log.error(colors.red(`Error generating policy for app ${path}: ${e}`));
throw e;
@@ -564,6 +587,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"));
}
+16 -13
View File
@@ -152,6 +152,7 @@ import {
generateAppLocksInternal,
RECORDINGS_FOLDER,
} from "../app/app_metadata.ts";
import { deploysWithRawApp } from "../../utils/app_files.ts";
import {
isFlowPath,
isAppPath,
@@ -2018,20 +2019,18 @@ export async function elementsToMap(
}
if (isRawAppFile(path)) {
// FSFSElement builds paths with the platform separator, while the checks
// below are written with "/": without normalizing, none of them match on
// Windows and the push collector's own exclusions become perpetual diffs.
// FSFSElement builds paths with the platform separator, while
// `deploysWithRawApp` is written with "/": without normalizing it matches
// nothing on Windows and the push collector's own exclusions become
// perpetual diffs.
const suffix = path
.split(getFolderSuffix("raw_app") + SEP)
.pop()
?.replaceAll(SEP, "/");
if (
suffix?.startsWith("dist/") ||
suffix?.startsWith(RECORDINGS_FOLDER + "/") ||
suffix == "wmill.d.ts" ||
suffix == "package-lock.json" ||
suffix == "DATATABLES.md"
) {
// A file no push sends is not a change to track. Listing it leaves it
// pending forever — nothing ever uploads it — and pushing it redeploys
// the whole app, reassigning its run-as user, to ship nothing.
if (suffix && !deploysWithRawApp(suffix)) {
continue;
}
}
@@ -6142,7 +6141,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 +6186,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 +6232,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,
+67 -11
View File
@@ -3,6 +3,12 @@ import * as log from "./log.ts";
import { colors } from "@cliffy/ansi/colors";
import { Confirm } from "@cliffy/prompt/confirm";
import { getTypeStrFromPath } from "../types.ts";
import {
extractFolderPath,
isAppFolderMetadataFile,
isRawAppFolderMetadataFile,
} from "../utils/resource_folders.ts";
import { deploysWithRawApp } from "../utils/app_files.ts";
import { parseSyncBehavior } from "./conf.ts";
export interface PermissionedAsContext {
@@ -89,6 +95,45 @@ function contentHasOnBehalfOf(content: string, typeStr: string): boolean {
return false;
}
type AppTypeStr = "app" | "raw_app";
function isAppTypeStr(typeStr: string): typeStr is AppTypeStr {
return typeStr === "app" || typeStr === "raw_app";
}
/** The app folder a file belongs to. `isAppFolderMetadataFile` and its raw twin
* match a literal `/`, unlike `extractFolderPath` — so normalize before either,
* or a Windows path takes a different branch from the same file on Linux. */
function appFolderOf(path: string, typeStr: AppTypeStr): string {
return extractFolderPath(path, typeStr) ?? path;
}
function toPosix(path: string): string {
return path.replaceAll("\\", "/");
}
/** App folders whose own metadata file is being added or deleted, which is how a
* whole app arrives or goes rather than being redeployed. Neither takes an owner
* over: a create has none yet, and a delete leaves none behind. */
function appsArrivingOrLeaving(changes: Change[]): Set<string> {
const folders = new Set<string>();
for (const change of changes) {
if (change.name === "edited") continue;
const path = toPosix(change.path);
if (!isAppFolderMetadataFile(path) && !isRawAppFolderMetadataFile(path)) {
continue;
}
let typeStr: string;
try {
typeStr = getTypeStrFromPath(path);
} catch {
continue;
}
if (isAppTypeStr(typeStr)) folders.add(appFolderOf(path, typeStr));
}
return folders;
}
export async function preCheckPermissionedAs(
changes: Change[],
userEmail: string,
@@ -101,6 +146,12 @@ export async function preCheckPermissionedAs(
if (userIsAdminOrDeployer) return;
const wouldChangeItems: { path: string; currentOwner: string }[] = [];
const addItem = (item: { path: string; currentOwner: string }) => {
if (!wouldChangeItems.some((i) => i.path === item.path)) {
wouldChangeItems.push(item);
}
};
const arrivingOrLeaving = appsArrivingOrLeaving(changes);
for (const change of changes) {
let typeStr: string;
@@ -110,6 +161,22 @@ export async function preCheckPermissionedAs(
continue;
}
// An app is redeployed whole by any change to any of the files it actually
// sends — added, edited or deleted alike — so its policy is rewritten
// regardless of what the file holds. Settled here, before the content the
// other kinds parse to find their owner, which an app has none of to parse.
if (isAppTypeStr(typeStr)) {
const path = toPosix(change.path);
const folder = appFolderOf(path, typeStr);
if (
!arrivingOrLeaving.has(folder) &&
(typeStr === "app" || deploysWithRawApp(path.slice(folder.length)))
) {
addItem({ path: folder, currentOwner: "(app policy owner)" });
}
continue;
}
if (change.name === "added") {
const content = change.content;
if (!content) continue;
@@ -130,11 +197,6 @@ export async function preCheckPermissionedAs(
const label =
typeStr === "script" ? "(script owner)" : "(flow owner)";
wouldChangeItems.push({ path: change.path, currentOwner: label });
} else if (typeStr === "app") {
wouldChangeItems.push({
path: change.path,
currentOwner: "(app policy owner)",
});
}
continue;
}
@@ -177,12 +239,6 @@ export async function preCheckPermissionedAs(
}
}
continue;
} else if (typeStr === "app") {
wouldChangeItems.push({
path: change.path,
currentOwner: "(app policy owner)",
});
continue;
} else if (typeStr === "schedule") {
const match = beforeContent.match(
/email:\s*["']?([^\s"']+)["']?/
+1 -1
View File
@@ -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") {
+48
View File
@@ -0,0 +1,48 @@
import {
APP_BACKEND_FOLDER,
RECORDINGS_FOLDER,
} from "../commands/app/app_metadata.ts";
/** Directories under a raw app that no push sends. */
const NEVER_DEPLOYED_DIRS = new Set([
"node_modules",
"dist",
".claude",
"sql_to_apply",
]);
/** Files under a raw app that no push sends. */
const NEVER_DEPLOYED_FILES = new Set([
"package-lock.json",
"DATATABLES.md",
"AGENTS.md",
"wmill.d.ts",
]);
/**
* Whether an app-root-relative path (`/` separators, leading slash optional)
* reaches the server through any of a push's three channels: `raw_app.yaml` as
* metadata, the backend folder as runnables, the rest bundled by
* `collectAppFiles`. A path this rejects deploys nothing, so changing it is not
* a change to the app however much the sync diff lists it. `collectAppFiles`
* must not drift from this — it reads the same two sets.
*/
export function deploysWithRawApp(relativePath: string): boolean {
const segments = relativePath.split("/").filter(Boolean);
if (segments.length === 0) return false;
const name = segments[segments.length - 1];
const dirs = segments.slice(0, -1);
// The sets below describe the bundle, which never walks into the backend
// folder — applying them there would strip a runnable whose file shares a
// name (`backend/wmill.d.ts` is the runnable `wmill.d`). Depth 1 because
// `loadRunnablesFromBackend` reads that folder's top level only.
if (dirs[0] === APP_BACKEND_FOLDER) return dirs.length === 1;
if (NEVER_DEPLOYED_FILES.has(name)) return false;
if (dirs.some((d) => NEVER_DEPLOYED_DIRS.has(d))) return false;
// Session recordings are written at the app root only, so an app with a
// `recordings/` component folder of its own still ships it.
if (dirs[0] === RECORDINGS_FOLDER) return false;
return true;
}
export { NEVER_DEPLOYED_DIRS, NEVER_DEPLOYED_FILES };
+29 -6
View File
@@ -1,6 +1,6 @@
import { expect, test } from "bun:test";
import {
executionModeFromAppFile,
executionModeForPush,
generatingPolicy,
markAccessFromPolicy,
} from "../src/commands/app/app.ts";
@@ -13,16 +13,39 @@ test("the access mode survives the app.yaml round trip", async () => {
guest.policy = undefined;
expect(guest.guests).toBe(true);
expect(guest.public).toBeUndefined();
expect(executionModeFromAppFile(guest)).toBe("guest");
await generatingPolicy(guest, "u/test/app", executionModeFromAppFile(guest));
expect(executionModeForPush(guest, undefined)).toBe("guest");
await generatingPolicy(
guest,
"u/test/app",
executionModeForPush(guest, undefined),
undefined,
);
expect(guest.policy.execution_mode).toBe("guest");
const anonymous: any = { policy: { execution_mode: "anonymous" }, value: {} };
markAccessFromPolicy(anonymous);
anonymous.policy = undefined;
expect(anonymous.public).toBe(true);
expect(executionModeFromAppFile(anonymous)).toBe("anonymous");
expect(executionModeForPush(anonymous, undefined)).toBe("anonymous");
expect(executionModeFromAppFile({ policy: { execution_mode: "publisher" } })).toBe("publisher");
expect(executionModeFromAppFile({})).toBe("publisher");
expect(executionModeForPush({ policy: { execution_mode: "publisher" } }, undefined)).toBe("publisher");
expect(executionModeForPush({}, undefined)).toBe("publisher");
});
// `viewer` is the narrowest mode — each runnable runs as the viewer, not as the
// app's identity — and the only one with no marker in the file, so both ways it
// can reach a push must survive rather than widen to `publisher`.
test("viewer is never widened to publisher by a push", () => {
// Carried over from the deployed app: a pull writes no marker for it.
expect(executionModeForPush({}, { execution_mode: "viewer" })).toBe("viewer");
// Stated by the file, which is all a first push has to go on.
expect(executionModeForPush({ policy: { execution_mode: "viewer" } }, undefined)).toBe("viewer");
// The open-access markers still win, in either direction.
expect(executionModeForPush({ public: true }, { execution_mode: "viewer" })).toBe("anonymous");
expect(executionModeForPush({}, { execution_mode: "anonymous" })).toBe("publisher");
// A stated mode is authoritative both ways: the carry-over is for a file that
// says nothing, so it must not pin a deployed app to `viewer` forever.
expect(
executionModeForPush({ policy: { execution_mode: "publisher" } }, { execution_mode: "viewer" })
).toBe("publisher");
});
@@ -0,0 +1,147 @@
/**
* The pre-check is what stops a push from silently reassigning an item's run-as
* user. Raw apps were missing from it, so the one kind whose file records no
* policy at all was also the one that changed owner without a word.
*/
import { expect, test } from "bun:test";
import { preCheckPermissionedAs } from "../src/core/permissioned_as.ts";
/** Non-interactive and without the override flag, the pre-check exits rather
* than reassigning silently — so a thrown exit is the signal it fired. */
type Shape = "edited" | "added" | "deleted";
function change(path: string, name: Shape = "edited") {
return { name, path, before: "summary: x\n", content: "summary: x\n" };
}
async function precheck(
changes: ReturnType<typeof change>[],
): Promise<string | undefined> {
const exit = process.exit;
let code: number | undefined;
(process as any).exit = (c?: number) => {
code = c;
throw new Error(`exit:${c}`);
};
const logged: string[] = [];
const err = console.error;
console.error = (...a: unknown[]) => void logged.push(a.join(" "));
try {
await preCheckPermissionedAs(changes, "pusher@corp", false, false, false);
} catch (e) {
if (!String(e).startsWith("Error: exit:")) throw e;
} finally {
(process as any).exit = exit;
console.error = err;
}
return code === undefined ? undefined : logged.join("\n");
}
test("a raw-app push warns the non-deployer it will take over the run-as user", async () => {
const message = await precheck([change("f/test/myapp.raw_app/index.tsx")]);
expect(message).toBeDefined();
expect(message).toContain("f/test/myapp.raw_app");
expect(message).toContain("pusher@corp");
});
// Deleting one file re-pushes the whole app rather than deleting it, so the
// takeover happens there too.
test("deleting one of an app's files warns like editing one", async () => {
const message = await precheck([
change("f/test/myapp.raw_app/gone.tsx", "deleted"),
]);
expect(message).toContain("f/test/myapp.raw_app");
});
// The metadata file going with it means the app itself is created or removed —
// neither takes an owner over.
test("an app arriving or leaving whole is not a takeover", async () => {
const created = await precheck([
change("f/test/new.raw_app/raw_app.yaml", "added"),
change("f/test/new.raw_app/index.tsx", "added"),
]);
const removed = await precheck([
change("f/test/old.raw_app/raw_app.yaml", "deleted"),
change("f/test/old.raw_app/index.tsx", "deleted"),
]);
expect(created).toBeUndefined();
expect(removed).toBeUndefined();
});
// An app carries no owner in its files, so nothing about it depends on their
// content — an empty one redeploys it exactly like any other.
test("an empty file still counts as a change to the app", async () => {
const added = await precheck([
{ name: "added", path: "f/test/myapp.raw_app/blank.ts", content: "" },
]);
const edited = await precheck([
{ name: "edited", path: "f/test/myapp.raw_app/blank.ts", before: "" },
]);
expect(added).toContain("f/test/myapp.raw_app");
expect(edited).toContain("f/test/myapp.raw_app");
});
// `extractFolderPath` normalizes separators but the metadata predicates match a
// literal `/`, so a Windows path must not take a different branch.
test("a Windows path classifies the same as its posix twin", async () => {
const created = await precheck([
change("f\\test\\new.raw_app\\raw_app.yaml", "added"),
change("f\\test\\new.raw_app\\index.tsx", "added"),
]);
const edited = await precheck([
change("f\\test\\myapp.raw_app\\index.tsx"),
]);
expect(created).toBeUndefined();
expect(edited).toContain("f/test/myapp.raw_app");
});
// `collectAppFiles` never sends these, and the sync diff never stops listing
// them (nothing uploads them, so they stay "added" forever) — so warning on one
// would gate every push of a scaffolded app on the override flag.
test("a file the push never sends is not a change to the app", async () => {
const artifacts = await precheck([
change("f/test/myapp.raw_app/AGENTS.md", "added"),
change("f/test/myapp.raw_app/sql_to_apply/a.sql", "added"),
change("f/test/myapp.raw_app/node_modules/dep/index.js", "added"),
change("f/test/myapp.raw_app/recordings/r.json", "added"),
change("f/test/myapp.raw_app/package-lock.json"),
change("f/test/myapp.raw_app/wmill.d.ts"),
// Only the backend folder's *top level* is a runnable; nothing reads deeper,
// so the depth limit is what keeps a `backend/node_modules/` from becoming
// the perpetual diff this predicate exists to remove.
change("f/test/myapp.raw_app/backend/node_modules/dep/index.js", "added"),
]);
// The three channels a push does send through: bundled file, metadata, runnable.
const sent = await precheck([change("f/test/myapp.raw_app/index.tsx")]);
const meta = await precheck([change("f/test/myapp.raw_app/raw_app.yaml")]);
const runnable = await precheck([change("f/test/myapp.raw_app/backend/a.ts")]);
// The runnable channel is not the bundle: the bundle's name exclusions don't
// reach into it, so a runnable file sharing one of those names still deploys.
const namesake = await precheck([
change("f/test/myapp.raw_app/backend/wmill.d.ts"),
]);
expect(artifacts).toBeUndefined();
expect(sent).toContain("f/test/myapp.raw_app");
expect(meta).toContain("f/test/myapp.raw_app");
expect(runnable).toContain("f/test/myapp.raw_app");
expect(namesake).toContain("f/test/myapp.raw_app");
});
test("an app is listed once however many of its files changed", async () => {
const message = await precheck([
change("f/test/myapp.raw_app/index.tsx"),
change("f/test/myapp.raw_app/raw_app.yaml"),
change("f/test/myapp.raw_app/backend/a.ts"),
change("f/test/low.app/app.yaml"),
change("f/test/low.app/inline.ts"),
]);
expect(message).toContain("2 item(s)");
});
+142
View File
@@ -0,0 +1,142 @@
/**
* `raw_app.yaml` records none of the policy but the access-mode markers, so a
* push that regenerated the whole policy reset the deploy drawer's settings —
* run-as identity, sandbox isolation — to the pushing user's. Pin that the
* deployed policy is carried over, that a first push still starts from what the
* file states, and that the markers still close a deployed open app back down.
*/
import { afterAll, beforeEach, expect, mock, test } from "bun:test";
import { mkdtemp, symlink, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
let calls: any[] = [];
let deployedPolicy: any;
/** No app deployed at the path: `getAppByPath` 404s and the push creates one. */
let deployed = true;
// Stub only what no other in-process suite imports, and treat a stub as
// permanent for the run (see "Module mocks" in cli/TESTING.md). These three API
// functions qualify — nothing else in `test/` imports them. `bundle.ts` did not:
// stubbing it left `raw_app_svelte_plugin_unit.test.ts` asserting against an
// empty bundle, which an `afterAll` hand-back did not prevent. So the real
// bundler runs instead, on the app each push writes below.
const realServices = await import("../gen/services.gen.ts");
mock.module("../gen/services.gen.ts", () => ({
...realServices,
getAppByPath: async () => {
if (!deployed) throw new Error("not found");
return {
path: "f/test/raw",
summary: "raw",
value: { files: {}, runnables: {} },
policy: deployedPolicy,
};
},
updateAppRaw: async (a: unknown) => {
calls.push(a);
},
createAppRaw: async (a: unknown) => {
calls.push(a);
},
}));
// Belt and braces: nothing else in-process calls these, and a hand-back is not
// what makes that safe.
afterAll(() => {
mock.module("../gen/services.gen.ts", () => realServices);
});
const { pushRawApp } = await import("../src/commands/app/raw_apps.ts");
const ADMIN = {
userCache: new Map(),
userIsAdminOrDeployer: true,
userEmail: "deployer@windmill.dev",
};
async function push(yamlTail: string, admin = true): Promise<any> {
calls = [];
const dir = await mkdtemp(join(tmpdir(), "windmill_raw_push_"));
await writeFile(
join(dir, "raw_app.yaml"),
`summary: raw\nrunnables: {}\n${yamlTail}`,
"utf-8",
);
// Any file the remote doesn't have, so the push isn't short-circuited as
// up to date. It is also the bundler's entry point, so it has to compile.
await writeFile(join(dir, "index.tsx"), "export default 1\n", "utf-8");
await writeFile(
join(dir, "package.json"),
JSON.stringify({ name: "app", private: true }),
"utf-8",
);
// `ensureNodeModules` only checks the directory is there; borrowing the CLI's
// own skips an npm install per push.
await symlink(join(process.cwd(), "node_modules"), join(dir, "node_modules"));
await pushRawApp("w", "f/test/raw", dir, undefined, "bun", admin ? ADMIN : undefined);
expect(calls).toHaveLength(1);
return calls[0].formData.app;
}
beforeEach(() => {
deployed = true;
deployedPolicy = {
on_behalf_of: "u/svc",
on_behalf_of_email: "svc@corp",
sandbox: true,
frontend_sdk_scopes: ["jobs:run"],
execution_mode: "anonymous",
// Legacy v1 grants: the backend folds them into v2 at run time, so keeping
// them would keep granting runnables a push has removed.
triggerables: { "script/f/test/gone": {} },
triggerables_v2: { "a:script/f/test/gone": {} },
};
});
test("a raw-app push keeps the deployed run-as and sandbox settings", async () => {
const body = await push("public: true\n");
expect(body.policy.on_behalf_of).toBe("u/svc");
expect(body.policy.on_behalf_of_email).toBe("svc@corp");
expect(body.preserve_on_behalf_of).toBe(true);
expect(body.policy.sandbox).toBe(true);
expect(body.policy.frontend_sdk_scopes).toEqual(["jobs:run"]);
expect(body.policy.execution_mode).toBe("anonymous");
expect(body.policy.triggerables).toBeUndefined();
expect(body.policy.triggerables_v2).toEqual({});
});
test("a raw-app push without the marker closes an anonymous app back down", async () => {
const body = await push("");
expect(body.policy.execution_mode).toBe("publisher");
});
test("a push that may not claim the deployed identity doesn't send it", async () => {
const body = await push("", false);
expect(body.preserve_on_behalf_of).toBeUndefined();
// Not just the flag: the identity itself stays off the wire, so no server can
// deploy this push under it.
expect(body.policy.on_behalf_of).toBeUndefined();
expect(body.policy.on_behalf_of_email).toBeUndefined();
// Everything the pusher is entitled to carry over still comes along.
expect(body.policy.sandbox).toBe(true);
});
test("a first raw-app push deploys the policy its file states", async () => {
deployed = false;
const body = await push(
"policy:\n sandbox: true\n on_behalf_of: u/impostor\n on_behalf_of_email: impostor@corp\n",
);
expect(body.policy.sandbox).toBe(true);
// A repo doesn't get to pick who an app runs as: the identity never reaches
// the wire, so no server can be talked into deploying under it.
expect(body.policy.on_behalf_of).toBeUndefined();
expect(body.policy.on_behalf_of_email).toBeUndefined();
expect(body.preserve_on_behalf_of).toBeUndefined();
});