Files
windmill/cli/test/deploy_on_behalf_of_unit.test.ts
T
Ruben FiszelandClaude Opus 5 c90d1d95c2 refactor: make the app policy's principal the authority for its identity (#10440)
* refactor: make the app policy's principal the authority for its identity

* fix: align the app backfill with the sibling migration and audit the uncached address

* chore: refresh the sqlx cache after rebasing onto the merged base

* fix: resolve the app execution address uncached, it decides the job's authorization

* chore: cache the EE queries at the ref this branch pins

* chore: cache the EE queries at the ref this branch pins

* fix: derive the app draft's on-behalf-of address on read

* chore: cache the query the draft derivation test added

* fix: derive the app identity on the draft-table and version reads too

* docs: state the draft resolver's authorization contract

* fix: resolve a draft's principal against workspace membership only

* chore: cache the membership lookup the draft resolver added

* fix: drop an unresolvable draft's address instead of leaving it stale

* perf: evict the address cache on change so app dispatch can read it

* fix: evict on superadmin role changes, not only address changes

* refactor: make the app policy's address optional instead of derived on read

* fix: follow an external superadmin's rename into the apps that name them

* docs: state the removal gate once, and correctly

* refactor: drop the app-policy version constant that gated nothing

* docs: drop the last reference to the removed constant

* perf: read the address cache everywhere now that eviction reaches every replica

* fix: keep persisted addresses off the cache the poller evicts asynchronously

* docs: state where the cached address is accepted and where it is not

* docs: keep the cache rule in one place and drop the stale premise

* docs: sort the two lookups by how long a wrong answer lives

* fix: resolve the schedule address uncached where it is written to the row

* docs: name the release this actually ships in

* perf: evict a superadmin's key per workspace instead of the whole cache

* fix: evict every alias a superadmin principal can be spelled as

* docs: describe the trigger as it is

* docs: cover the round-tripped read in the cache rule

* docs: record why a stale dispatch address cannot escalate

* fix: validate a dispatch address against the principal's live binding

* fix: carry the validated address through to the job row and token

* fix: record the validated address on the job row, not the one handed in

* test: run the substep tag check as the non-superadmin it means to test

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

* fix: rewrite a stored app address that disagrees with its principal

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

* docs: record the accepted staleness window of the cached dispatch address

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

* fix: record the validated address on the job's audit row

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

* docs: record the accepted rename race of pre-transaction identity resolution

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

* docs: separate the app's stored address from the derived one in the resolver doc

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

* docs: describe the job identity fast path the push comments skipped

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

* fix: backfill a legacy group-prefixed username as the group it names

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

* fix: resolve a schedule edit's identity before opening its transaction

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

* fix: never resolve a disabled member to a same-named superadmin

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

* docs: state what the email-change notify buys, and rewrap two comment lines

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

* fix: keep a group's runnables when offboarding a legacy group-prefixed member

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

* fix: read the app author from the stored address, as execution does

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

* docs: record the rename race's full consequence as a known, accepted limitation

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

* docs: record the keep-target group address case as a known, accepted limitation

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-12 09:10:15 +02:00

108 lines
3.4 KiB
TypeScript

import { expect, test } from "bun:test";
import { deployItem } from "../windmill-utils-internal/src/deploy.ts";
// `deployItem` spreads the source item into the request body, and the principal it carries
// (`on_behalf_of`, at the top level for a script or flow and inside the policy for an app)
// names a username that only exists in the source workspace. Sending it to the target pairs
// one workspace's principal with the other's email, which the backend rejects. Deleting the
// spread is an easy regression, so pin that the principal never reaches the wire while the
// caller's chosen address does.
function recordingProvider(captured: [string, any][], flowExists: boolean) {
const source = {
on_behalf_of_email: "alice@corp",
on_behalf_of: "u/alice",
};
return {
existsFlowByPath: async () => flowExists,
existsScriptByPath: async () => true,
getFlowByPath: async () => ({
path: "f/x/f",
summary: "",
value: { modules: [] },
...source,
}),
createFlow: async (p: any) => void captured.push(["createFlow", p.requestBody]),
updateFlow: async (p: any) => void captured.push(["updateFlow", p.requestBody]),
getScriptByPath: async () => ({
path: "f/x/s",
summary: "",
content: "x",
language: "bun",
hash: "abc",
...source,
}),
createScript: async (p: any) =>
void captured.push(["createScript", p.requestBody]),
existsApp: async () => false,
getAppByPath: async () => ({
path: "f/x/a",
summary: "",
value: {},
raw_app: false,
policy: {
execution_mode: "publisher",
on_behalf_of: "u/alice",
on_behalf_of_email: "alice@corp",
},
}),
createApp: async (p: any) => void captured.push(["createApp", p.requestBody]),
} as any;
}
test("deployItem: never sends the source workspace's on_behalf_of", async () => {
const captured: [string, any][] = [];
// The clear is written out once per branch, so exercise all three: a flow that
// does not exist in the target (create), one that does (update — the branch
// `wmill workspace merge` takes for anything already deployed), and a script.
await deployItem(
recordingProvider(captured, false),
"flow" as any,
"f/x/f",
"src",
"dst",
"alice@corp",
);
await deployItem(
recordingProvider(captured, true),
"flow" as any,
"f/x/f",
"src",
"dst",
"alice@corp",
);
await deployItem(
recordingProvider(captured, false),
"script" as any,
"f/x/s",
"src",
"dst",
"alice@corp",
);
await deployItem(
recordingProvider(captured, false),
"app" as any,
"f/x/a",
"src",
"dst",
"alice@corp",
);
expect(captured.map(([fn]) => fn)).toEqual([
"createFlow",
"updateFlow",
"createScript",
"createApp",
]);
for (const [name, body] of captured) {
expect(body.preserve_on_behalf_of).toBe(true);
// Both surfaces spell it `on_behalf_of`; only its nesting differs — an app carries the
// identity inside its policy, the others at the top level.
const identity = name === "createApp" ? body.policy : body;
// The email is still overridden with the caller's choice...
expect(identity.on_behalf_of_email).toBe("alice@corp");
// ...while the principal is dropped, so the backend derives the target's own.
expect("on_behalf_of" in JSON.parse(JSON.stringify(identity))).toBe(false);
}
});