From 640da55702b1992e90b26c657d256e7589cb1b48 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 13 Sep 2026 19:19:17 +0200 Subject: [PATCH] refactor: derive an app's address beside its policy, not inside it Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EYWxvwmteVyzcWenJfbnzs --- backend/tests/preserve_on_behalf_of.rs | 15 +++-- backend/windmill-api/openapi.yaml | 8 +-- backend/windmill-api/src/apps.rs | 56 +++++++++---------- cli/src/commands/app/app.ts | 5 +- cli/src/commands/workspace/merge.ts | 6 +- cli/windmill-utils-internal/src/deploy.ts | 4 +- .../lib/components/CompareWorkspaces.svelte | 2 +- .../components/apps/editor/AppEditor.svelte | 12 +++- .../apps/editor/AppEditorHeader.svelte | 13 +++-- .../apps/editor/AppEditorHeaderDeploy.svelte | 9 +++ .../components/apps/editor/AppPreview.svelte | 8 ++- .../components/apps/editor/GridEditor.svelte | 7 ++- .../components/apps/editor/PublicApp.svelte | 1 + .../lib/components/apps/editor/appPolicy.ts | 12 ---- frontend/src/lib/components/apps/types.ts | 3 + .../raw_apps/RawAppEditorHeader.svelte | 7 +-- frontend/src/lib/rawAppDeploy.ts | 7 +-- frontend/src/lib/utils_workspace_deploy.ts | 6 +- .../(logged)/apps/edit/[...path]/+page.svelte | 1 + .../apps_raw/edit/[...path]/+page.svelte | 8 +-- 20 files changed, 103 insertions(+), 87 deletions(-) diff --git a/backend/tests/preserve_on_behalf_of.rs b/backend/tests/preserve_on_behalf_of.rs index 09c1a2696f..caae211b52 100644 --- a/backend/tests/preserve_on_behalf_of.rs +++ b/backend/tests/preserve_on_behalf_of.rs @@ -447,9 +447,13 @@ async fn test_preserve_on_behalf_of(db: Pool) -> anyhow::Result<()> { .await?; let returned: serde_json::Value = resp.json().await?; assert_eq!( - returned["policy"]["on_behalf_of_email"].as_str(), + returned["on_behalf_of_email"].as_str(), Some("original@windmill.dev"), - "the response derives the address from the principal" + "the response derives the address from the principal, beside the policy" + ); + assert!( + returned["policy"]["on_behalf_of_email"].is_null(), + "and never inside it, where a client echoing the policy back would resend it" ); // ======================================== @@ -2951,9 +2955,9 @@ async fn test_app_draft_read_does_not_derive_an_address(db: Pool) -> a let body: serde_json::Value = resp.json().await?; assert_eq!( - body["policy"]["on_behalf_of_email"].as_str(), + body["on_behalf_of_email"].as_str(), Some("original@windmill.dev"), - "the deployed policy carries the address derived from its principal" + "the deployed app carries the address derived from its principal" ); // Asserted before the absence below, so that a response carrying no draft at all cannot // pass this test by having nothing to derive from. @@ -2963,7 +2967,8 @@ async fn test_app_draft_read_does_not_derive_an_address(db: Pool) -> a "the draft is returned, principal and all: {body}" ); assert!( - body["draft"]["policy"]["on_behalf_of_email"].is_null(), + body["draft"]["on_behalf_of_email"].is_null() + && body["draft"]["policy"]["on_behalf_of_email"].is_null(), "the draft's principal must not be resolved: {}", body["draft"] ); diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index d48b99cec3..a318c1a3cd 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -33947,10 +33947,6 @@ components: on_behalf_of: type: string description: The user or group the app runs as in anonymous or publisher mode (e.g. 'u/admin' or 'g/mygroup'). The authority for the app's identity. - on_behalf_of_email: - type: string - readOnly: true - description: Address of `on_behalf_of`, derived from it on every read and never stored. Absent when the policy names no identity. Read-only - sending it beside `on_behalf_of` is ignored, and sending it alone with `preserve_on_behalf_of` is rejected, so a deploy that means to keep an identity has to name it in `on_behalf_of`. sandbox: type: boolean description: > @@ -34161,6 +34157,10 @@ components: type: boolean bundle_secret: type: string + on_behalf_of_email: + type: string + readOnly: true + description: Address of `policy.on_behalf_of`, derived from it on every read and never stored. Absent when the policy names no identity. Beside the policy rather than inside it so that a client echoing a policy back cannot send a value only the server may author. labels: type: array items: diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 35397f3c7e..81795b0e72 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -255,6 +255,16 @@ pub struct AppWithLastVersion { #[sqlx(skip)] #[serde(skip_serializing_if = "Option::is_none")] pub bundle_secret: Option, + /// The address `policy.on_behalf_of` resolves to, derived per read and never stored — see + /// [`attach_on_behalf_of_email`], which is also the only thing that sets it. + /// + /// Beside the policy rather than inside it on purpose. The policy is read, edited and written + /// back by a dozen callers; a field only the server may author, living in there, makes every + /// one of them responsible for stripping it before it goes out again, and makes the same + /// field serve both what the editor displays and what it submits. + #[sqlx(skip)] + #[serde(skip_serializing_if = "Option::is_none")] + pub on_behalf_of_email: Option, #[serde(skip_serializing_if = "Option::is_none")] pub labels: Option>, } @@ -491,14 +501,11 @@ pub struct S3Key { #[derive(Serialize, Deserialize, Debug, Clone, Default)] pub struct Policy { pub on_behalf_of: Option, - /// The address `on_behalf_of` resolves to. Response-only: never stored and never read from - /// the request. [`attach_on_behalf_of_email`] puts it here, and states which reads may. - #[serde(skip_deserializing, skip_serializing_if = "Option::is_none")] - pub on_behalf_of_email: Option, - /// The same key on the way in, taken only so that a request naming the identity by address - /// alone can be refused. Silently ignoring it would redeploy the app as whoever pushed it, - /// which is how a client written before the principal existed names an identity. An echo of - /// a read carries the principal too, so this is never the whole of what a caller sent. + /// An address a caller sent, taken only so that a request naming the identity by address + /// alone can be refused. The policy has no address of its own: it is derived per read onto + /// `AppWithLastVersion::on_behalf_of_email`, so nothing here is ever stored or served. + /// Silently ignoring it would redeploy the app as whoever pushed it, which is how a client + /// written before the principal existed names an identity. #[serde(default, rename = "on_behalf_of_email", skip_serializing)] pub submitted_on_behalf_of_email: Option, //paths: @@ -1169,7 +1176,7 @@ async fn get_app( // The deployed policy only. A draft overlaid below keeps whatever its editor last saved. if let Some(app) = app_o.as_mut() { - attach_on_behalf_of_email(&mut app.app.policy, &w_id, &db).await?; + attach_on_behalf_of_email(&mut app.app, &w_id, &db).await?; } // No deployed row + `get_draft`: fall back to the draft table; see scripts.rs. @@ -1221,7 +1228,7 @@ async fn get_app_lite( tx.commit().await?; let mut app = not_found_if_none(app_o, "App", path)?; - attach_on_behalf_of_email(&mut app.policy, &w_id, &db).await?; + attach_on_behalf_of_email(&mut app, &w_id, &db).await?; Ok(Json(app)) } @@ -1360,7 +1367,7 @@ async fn get_app_by_id( check_scopes(&authed, || format!("apps:read:{}", &app.path))?; - attach_on_behalf_of_email(&mut app.policy, &w_id, &db).await?; + attach_on_behalf_of_email(&mut app, &w_id, &db).await?; Ok(Json(app)) } @@ -1414,7 +1421,7 @@ async fn get_public_app_by_secret( app.bundle_secret = Some(compute_bundle_secret(&db, &w_id, &app.versions).await?); } - attach_on_behalf_of_email(&mut app.policy, &w_id, &db).await?; + attach_on_behalf_of_email(&mut app, &w_id, &db).await?; Ok(Json(app)) } @@ -4583,7 +4590,6 @@ async fn upload_s3_file_from_app( triggerables: None, triggerables_v2: None, on_behalf_of: None, - on_behalf_of_email: None, submitted_on_behalf_of_email: None, s3_inputs: Some(vec![S3Input { file_key_regex: file_key_regex, @@ -5009,7 +5015,6 @@ async fn get_on_behalf_authed_from_app( triggerables: None, triggerables_v2: None, on_behalf_of: None, - on_behalf_of_email: None, submitted_on_behalf_of_email: None, s3_inputs: None, allowed_s3_keys: Some(force_allowed_s3_keys), @@ -5034,7 +5039,6 @@ async fn get_on_behalf_authed_from_app( triggerables: None, triggerables_v2: None, on_behalf_of: None, - on_behalf_of_email: None, submitted_on_behalf_of_email: None, s3_inputs: None, allowed_s3_keys: None, @@ -5577,7 +5581,7 @@ fn refuse_address_only_identity(policy: &Policy, preserve: Option) -> Resu Ok(()) } -/// Attach to a policy on its way out the address its principal resolves to. +/// Derive onto an app, on its way out, the address its policy's principal resolves to. /// /// Derived here rather than left to the caller because the app bundle reads it as `ctx.author` /// in the browser, where an anonymous viewer has no session to resolve `u/alice` with. Cached, @@ -5595,27 +5599,23 @@ fn refuse_address_only_identity(policy: &Policy, preserve: Option) -> Resu /// see this app. Every route that calls it does so after its own read check — the RLS /// transaction that produced the row, `check_scopes`, or `authorize_app_viewer`. pub async fn attach_on_behalf_of_email( - policy: &mut sqlx::types::Json>, + app: &mut AppWithLastVersion, w_id: &str, db: &DB, ) -> Result<()> { - // Field-level, not a `Policy` round-trip: a legacy policy carries keys this struct has - // never had, and returning it shorn of them is not this function's job. - let mut value = serde_json::from_str::(policy.0.get()).map_err(to_anyhow)?; - let Some(object) = value.as_object_mut() else { - return Ok(()); - }; - let Some(permissioned_as) = object + // Field-level, not a `Policy` parse: a legacy policy carries keys that struct has never had, + // and this reads one key rather than deciding what the policy is. + let Some(permissioned_as) = serde_json::from_str::(app.policy.0.get()) + .map_err(to_anyhow)? .get("on_behalf_of") .and_then(|v| v.as_str()) .map(str::to_string) else { return Ok(()); }; - let email = - windmill_common::users::get_email_from_permissioned_as(&permissioned_as, w_id, db).await?; - object.insert("on_behalf_of_email".to_string(), json!(email)); - *policy = sqlx::types::Json(serde_json::value::to_raw_value(&value).map_err(to_anyhow)?); + app.on_behalf_of_email = Some( + windmill_common::users::get_email_from_permissioned_as(&permissioned_as, w_id, db).await?, + ); Ok(()) } diff --git a/cli/src/commands/app/app.ts b/cli/src/commands/app/app.ts index 9cba4e8409..7735011dd0 100644 --- a/cli/src/commands/app/app.ts +++ b/cli/src/commands/app/app.ts @@ -317,7 +317,7 @@ export function preserveOnBehalfOfFields( return {}; } log.info( - `Preserving ${deployedPolicy?.on_behalf_of_email ?? onBehalfOf} as permissioned_as for app ${remotePath}` + `Preserving ${onBehalfOf} as permissioned_as for app ${remotePath}` ); return { preserve_on_behalf_of: true }; } @@ -592,10 +592,7 @@ const command = new Command() // Only spread remote.policy — spreading the full remote object would include // `value` and trigger a new app version on every call. EditApp has all-Option // fields, so a minimal body only updates the policy column. - // The read carries a derived `on_behalf_of_email`; a server old enough to still read it - // would see the previous holder's address beside the new principal and reject the pair. const policy = { ...(remote.policy as any), on_behalf_of: `u/${username}` }; - delete policy.on_behalf_of_email; await wmill.updateApp({ workspace: workspace.workspaceId, path: appPath, diff --git a/cli/src/commands/workspace/merge.ts b/cli/src/commands/workspace/merge.ts index 10fe77da9b..91e421987b 100644 --- a/cli/src/commands/workspace/merge.ts +++ b/cli/src/commands/workspace/merge.ts @@ -200,9 +200,9 @@ function triggerService(kind: TriggerDeployKind) { * over that fallback, so carrying the name into a workspace that has a member of its own by that * name would hand the app to them. * - * The member's address comes from `usr` via `listUsers`, never from the app read's derived - * `on_behalf_of_email`: that one is served from a cache that may be a notify poll stale, and this - * answer is about to be persisted as the target's identity. + * The member's address comes from `usr` via `listUsers`, never from the address the app read + * derives: that one is served from a cache that may be a notify poll stale, and this answer is + * about to be persisted as the target's identity. * * Throws rather than guessing whenever the two workspaces disagree about who a name belongs to. * Deploying as the pusher instead would silently widen what the app runs as. diff --git a/cli/windmill-utils-internal/src/deploy.ts b/cli/windmill-utils-internal/src/deploy.ts index 7a2e8ca10a..d4f6d240a5 100644 --- a/cli/windmill-utils-internal/src/deploy.ts +++ b/cli/windmill-utils-internal/src/deploy.ts @@ -516,14 +516,12 @@ export async function deployItem( // principal is always replaced by `onBehalfOf`, which **the caller must have made valid // in the target**: `u/` names are per-workspace, so a caller reading the source has to map // it (`preservedIdentity` in the CLI's merge does), and one reading the target can send it - // as is. The address the read derived goes too: it is the source's, and a server old - // enough to still read it would take the pair as naming two accounts. + // as is. const app = { ...rawApp, policy: { ...rawApp.policy, on_behalf_of: onBehalfOf, - on_behalf_of_email: undefined, }, }; if (alreadyExists) { diff --git a/frontend/src/lib/components/CompareWorkspaces.svelte b/frontend/src/lib/components/CompareWorkspaces.svelte index c93a9fc777..64032d1abe 100644 --- a/frontend/src/lib/components/CompareWorkspaces.svelte +++ b/frontend/src/lib/components/CompareWorkspaces.svelte @@ -1005,7 +1005,7 @@ ) ) - // Fetch summaries and on_behalf_of_email when comparison data loads + // Fetch summaries and the on-behalf-of identity when comparison data loads $effect(() => { if (comparison?.diffs) { fetchSummaries(comparison.diffs) diff --git a/frontend/src/lib/components/apps/editor/AppEditor.svelte b/frontend/src/lib/components/apps/editor/AppEditor.svelte index 565ac5d5d3..8a13fdb204 100644 --- a/frontend/src/lib/components/apps/editor/AppEditor.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditor.svelte @@ -68,6 +68,7 @@ app, path, policy, + onBehalfOfEmail = undefined, summary, labels, deployedBaseline = undefined, @@ -90,6 +91,11 @@ migrateApp(untrack(() => app)) + // What the deploy drawer's identity picker last chose, preferred over the address the read + // derived so the editor names who the app *will* run as. Undefined until something is picked. + let pickedOnBehalfOfEmail = $state(undefined) + const shownOnBehalfOfEmail = $derived(pickedOnBehalfOfEmail ?? onBehalfOfEmail) + // Migrated clone of the deployed baseline for the autosave `discardIf`. The // live `stateApp` is `migrateApp`'d on mount, so the baseline must be too or // an unedited draft would never compare equal. Captured once per mount (the @@ -189,7 +195,7 @@ workspace: $workspaceStore, mode: 'editor', summary: $summaryStore, - author: untrack(() => policy).on_behalf_of_email + author: untrack(() => shownOnBehalfOfEmail) }) const darkMode: Writable = writable(document.documentElement.classList.contains('dark')) @@ -884,6 +890,7 @@ {#if !$userStore?.operator} {#if $appStore} - + {/if} {#if !$appStore?.mobileViewOnSmallerScreens && $breakpoint === 'sm'} diff --git a/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte b/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte index 4a32bfd7ae..962ab5f992 100644 --- a/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte @@ -64,7 +64,7 @@ import type { DiffDrawerI } from '$lib/components/diff_drawer' import AppEditorHeaderDeploy from './AppEditorHeaderDeploy.svelte' import { computeSecretUrl } from './appDeploy.svelte' - import { policyForDeploy, updatePolicy } from './appPolicy' + import { updatePolicy } from './appPolicy' import { editInForkAllowed, editInForkLabel, openEditInFork } from '$lib/utils/editInFork' import { isCloudHosted } from '$lib/cloud' @@ -110,10 +110,14 @@ // (not `on:restore` forwarding): forwarding a `createEventDispatcher` // event up through these runes-mode components silently drops it. onRestore?: (restoredApp: any) => void + /** Address of the identity selected in the deploy drawer; a display value the editor + * shows, never part of what a deploy sends. */ + onBehalfOfEmail?: string | undefined } let { policy = $bindable(), + onBehalfOfEmail = $bindable(undefined), fromHub = false, diffDrawer = undefined, savedApp = $bindable(undefined), @@ -253,7 +257,7 @@ value: $app, path, summary: $summary, - policy: policyForDeploy(policy), + policy, deployment_message: deploymentMsg, custom_path: customPath, preserve_on_behalf_of: preserveOnBehalfOf || undefined, @@ -357,7 +361,7 @@ requestBody: { value: $app!, summary: $summary, - policy: policyForDeploy(policy), + policy, path: npath, deployment_message: deploymentMsg, // custom_path requires admin so to accept update without it, we need to send as undefined when non-admin (when undefined, it will be ignored) @@ -410,7 +414,7 @@ await AppService.updateApp({ workspace: $workspaceStore!, path: $appPath, - requestBody: { policy: policyForDeploy(policy) } + requestBody: { policy } }) if (message) { sendUserToast(message) @@ -812,6 +816,7 @@ {/snippet} void appPath: string customPath: string | undefined @@ -145,6 +150,7 @@ } let canPreserve = $derived(!!$userStore?.is_admin || !!$userStore?.is_super_admin || isDeployer) let savedOnBehalfOf = $derived(savedApp?.policy?.on_behalf_of) + let savedOnBehalfOfEmail = $derived(savedApp?.on_behalf_of_email) let onBehalfOfChoice: OnBehalfOfChoice = $state(undefined) let customOnBehalfOfEmail: string = $state('') let dirtyCustomPath = $state(false) @@ -320,14 +326,17 @@ onBehalfOfChoice = choice if (choice === 'me') { policy.on_behalf_of = `u/${$userStore?.username}` + onBehalfOfEmail = $userStore?.email customOnBehalfOfEmail = '' preserveOnBehalfOf = false } else if (choice === 'target') { policy.on_behalf_of = savedOnBehalfOf + onBehalfOfEmail = savedOnBehalfOfEmail customOnBehalfOfEmail = '' preserveOnBehalfOf = true } else if (choice === 'custom' && details) { policy.on_behalf_of = details.permissionedAs + onBehalfOfEmail = details.email customOnBehalfOfEmail = details.email preserveOnBehalfOf = true } diff --git a/frontend/src/lib/components/apps/editor/AppPreview.svelte b/frontend/src/lib/components/apps/editor/AppPreview.svelte index a33e26504d..494c6af479 100644 --- a/frontend/src/lib/components/apps/editor/AppPreview.svelte +++ b/frontend/src/lib/components/apps/editor/AppPreview.svelte @@ -30,6 +30,9 @@ appPath?: string breakpoint?: Writable policy?: Policy + /** Address `policy.on_behalf_of` resolves to, derived by the app read. Beside the policy, + * not in it, so a client echoing the policy back cannot send a server-authored value. */ + onBehalfOfEmail?: string summary?: string workspace?: string isEditor?: boolean @@ -47,6 +50,7 @@ appPath = '', breakpoint = writable('lg'), policy = {}, + onBehalfOfEmail = undefined, summary = '', workspace = $workspaceStore!, isEditor = false, @@ -78,7 +82,7 @@ workspace: untrack(() => workspace), mode: 'viewer', summary: untrack(() => summary), - author: untrack(() => policy).on_behalf_of_email, + author: untrack(() => onBehalfOfEmail), app_path: untrack(() => appPath) } @@ -301,7 +305,7 @@

{summary}

- {policy.on_behalf_of ? `on behalf of ${policy.on_behalf_of_email}` : ''} + {policy.on_behalf_of ? `on behalf of ${onBehalfOfEmail ?? policy.on_behalf_of}` : ''}
diff --git a/frontend/src/lib/components/apps/editor/GridEditor.svelte b/frontend/src/lib/components/apps/editor/GridEditor.svelte index d787564c0c..24766051e3 100644 --- a/frontend/src/lib/components/apps/editor/GridEditor.svelte +++ b/frontend/src/lib/components/apps/editor/GridEditor.svelte @@ -30,9 +30,12 @@ interface Props { policy: Policy + /** Address `policy.on_behalf_of` resolves to, derived by the app read. Beside the policy, + * not in it, so a client echoing the policy back cannot send a server-authored value. */ + onBehalfOfEmail?: string } - let { policy }: Props = $props() + let { policy, onBehalfOfEmail = undefined }: Props = $props() const { selectedComponent, @@ -174,7 +177,7 @@ />
- {policy.on_behalf_of ? `Author ${policy.on_behalf_of_email}` : ''} + {policy.on_behalf_of ? `Author ${onBehalfOfEmail ?? policy.on_behalf_of}` : ''} The scripts will be run on behalf of the author and a tight policy ensure security about the possible inputs of the runnables. diff --git a/frontend/src/lib/components/apps/editor/PublicApp.svelte b/frontend/src/lib/components/apps/editor/PublicApp.svelte index 6de8f9df0c..f1727dc370 100644 --- a/frontend/src/lib/components/apps/editor/PublicApp.svelte +++ b/frontend/src/lib/components/apps/editor/PublicApp.svelte @@ -196,6 +196,7 @@ appPath={app.path} {breakpoint} policy={app.policy} + onBehalfOfEmail={app.on_behalf_of_email} isEditor={false} replaceStateFn={(path) => goto(path)} gotoFn={(path, opt) => (embedNav ? embedNav.navigateTop(path) : goto(path, opt))} diff --git a/frontend/src/lib/components/apps/editor/appPolicy.ts b/frontend/src/lib/components/apps/editor/appPolicy.ts index 2984e54254..b6e35bd65e 100644 --- a/frontend/src/lib/components/apps/editor/appPolicy.ts +++ b/frontend/src/lib/components/apps/editor/appPolicy.ts @@ -189,18 +189,6 @@ export async function updatePolicy(app: App, currentPolicy: Policy | undefined): return next } -/** - * The policy as a deploy should send it. - * - * A read derives `on_behalf_of_email` from the principal, and the editor keeps it to show who the - * app runs as — but sending it back beside a principal the user has just changed reads, to a peer - * that still stores the address, as a pair naming two accounts, and the deploy is refused. Built - * as a copy so the object the editor renders from keeps the address it displays. - */ -export function policyForDeploy>(policy: T): T { - return { ...policy, on_behalf_of_email: undefined } -} - export async function processRunnable( id: string, runnable: Runnable, diff --git a/frontend/src/lib/components/apps/types.ts b/frontend/src/lib/components/apps/types.ts index 960e273ac3..d4bde89b03 100644 --- a/frontend/src/lib/components/apps/types.ts +++ b/frontend/src/lib/components/apps/types.ts @@ -143,6 +143,9 @@ export interface AppEditorProps { app: App path: string policy: Policy + /** Address `policy.on_behalf_of` resolves to, derived by the app read. Beside the policy, + * not in it, so a client echoing the policy back cannot send a server-authored value. */ + onBehalfOfEmail?: string summary: string /** Initial labels for the app, threaded from the loaded app data. */ labels?: string[] diff --git a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte index 6aa8847c11..58329fac6c 100644 --- a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte @@ -74,7 +74,6 @@ import type { RawAppData } from './dataTableRefUtils' import { editInForkAllowed, editInForkLabel, openEditInFork } from '$lib/utils/editInFork' import { isCloudHosted } from '$lib/cloud' - import { policyForDeploy } from '$lib/components/apps/editor/appPolicy' // async function hash(message) { // try { @@ -315,7 +314,7 @@ value: app, path, summary: summary, - policy: policyForDeploy(policy), + policy, deployment_message: deploymentMsg, custom_path: customPath, preserve_on_behalf_of: preserveOnBehalfOf || undefined, @@ -436,7 +435,7 @@ app: { value: app!, summary: summary, - policy: policyForDeploy(policy), + policy, path: npath, deployment_message: deploymentMsg, preserve_on_behalf_of: preserveOnBehalfOf || undefined, @@ -486,7 +485,7 @@ await AppService.updateApp({ workspace: opWorkspace!, path: appPath, - requestBody: { policy: policyForDeploy(policy) } + requestBody: { policy } }) if (message) { sendUserToast(message) diff --git a/frontend/src/lib/rawAppDeploy.ts b/frontend/src/lib/rawAppDeploy.ts index 65277e2628..2a9bea2973 100644 --- a/frontend/src/lib/rawAppDeploy.ts +++ b/frontend/src/lib/rawAppDeploy.ts @@ -23,7 +23,6 @@ import { normalizeRawAppData } from '$lib/components/raw_apps/rawAppDraftValue' import { stateSnapshot } from '$lib/svelte5Utils.svelte' -import { policyForDeploy } from '$lib/components/apps/editor/appPolicy' /** * Deploy an explicit raw-app value — one the user edited as JSON, or one @@ -74,7 +73,7 @@ export async function deployRawAppValue({ // version can still carry the pre-`data` datatable shapes. value: { files, runnables, data: normalizeRawAppData(plainValue) }, summary: summary ?? '', - policy: policyForDeploy(policy), + policy, deployment_message: deploymentMessage, // custom_path changes require admin (see deployRawAppDraft). custom_path: isAdmin ? (customPath ?? '') : undefined, @@ -141,7 +140,7 @@ export async function deployRawAppDraft( path: targetPath, value: rawAppValue, summary, - policy: policyForDeploy(policy), + policy, deployment_message: deploymentMessage, custom_path: isAdmin ? (value.custom_path ?? '') : undefined, // Preserve the policy's on_behalf_of: this draft-deploy path has no @@ -161,7 +160,7 @@ export async function deployRawAppDraft( path: targetPath, value: rawAppValue, summary, - policy: policyForDeploy(policy), + policy, deployment_message: deploymentMessage, custom_path: value.custom_path, // Preserve the policy's on_behalf_of (see update branch above). diff --git a/frontend/src/lib/utils_workspace_deploy.ts b/frontend/src/lib/utils_workspace_deploy.ts index 67258e876e..a9fd353692 100644 --- a/frontend/src/lib/utils_workspace_deploy.ts +++ b/frontend/src/lib/utils_workspace_deploy.ts @@ -206,7 +206,8 @@ export type DeployConflict = { hit: boolean } * An app carries its identity inside its `policy` instead, and as a principal rather than an * address — `deployItemIdentityIsPrincipal` says which kinds do. So `onBehalfOf` is already * the `u/`/`g/` form for one, and it is stamped into the policy here because the published - * package leaves that untouched. + * package leaves that untouched. Nothing has to be stripped on the way out: the address the + * read derives arrives beside the policy, not in it. */ function makeProvider( onBehalfOfPrincipal?: string, @@ -230,8 +231,7 @@ function makeProvider( ...app, policy: { ...app.policy, - on_behalf_of: onBehalfOf, - on_behalf_of_email: undefined + on_behalf_of: onBehalfOf } } : app diff --git a/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte index 4490f70f39..c6b31f09ff 100644 --- a/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte @@ -474,6 +474,7 @@ newPath={app.value?.draft_path ?? app.path} path={page.params.path ?? ''} policy={app.policy} + onBehalfOfEmail={app.on_behalf_of_email} bind:savedApp {diffDrawer} version={app.versions ? app.versions[app.versions.length - 1] : undefined} diff --git a/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte index 7b69a1f07b..2a6a354e5a 100644 --- a/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte @@ -5,7 +5,6 @@ import { AppService } from '$lib/gen' import { userStore, workspaceStore } from '$lib/stores' import { readFieldsRecursively } from '$lib/utils' - import { policyForDeploy } from '$lib/components/apps/editor/appPolicy' import { goto } from '$lib/navigation' import { sendUserToast } from '$lib/toast' import DiffDrawer from '$lib/components/DiffDrawer.svelte' @@ -121,10 +120,7 @@ runnables, data, summary, - // Shaped on the way into the row, not just on the way to a deploy: the address is - // derived per read, and a copy persisted here is the one thing that outlives the - // sweeps that exist to keep it out of a draft. - policy: policyForDeploy(policy), + policy, custom_path: savedApp?.custom_path, // Persist the typed path as `draft_path` only when it actually differs // from the current path — a `draft_path` equal to the baseline is a @@ -410,7 +406,7 @@ runnables, data, summary, - policy: policyForDeploy(policy), + policy, custom_path: savedApp?.custom_path, ...(pendingDraftPath ? { draft_path: pendingDraftPath } : {}) } as RawAppDraft,