Retarget routines via PATCH and enforce target authorization (CL-7353, CL-7354)

apps/hub/src/index.ts

@@ -2901,6 +2901,11 @@ export async function createHub(config: HubConfig) {
         grantStore: routineGrantStore,
         conditionRegistry: chatConditionRegistry,
       }),
+      // CL-7354: a create or retarget's resolved target must also
+      // authorize for the acting principal — same `grantStore` the grant
+      // check above already uses.
+      grantStore: routineGrantStore,
+      conditionRegistry: chatConditionRegistry,
       // A run-now or a scheduled fire's result is a message into the
       // routine's delivery workbench root timeline — never a pre-opened
       // thread; see `@corbits/routines`' `RoutineLauncher` doc comment
@@ -2955,6 +2960,10 @@ export async function createHub(config: HubConfig) {
           },
           query,
         ),
+      // CL-7354: same authorization the tenant-session surface enforces
+      // above, for Myra's own create/retarget path.
+      grantStore: routineGrantStore,
+      conditionRegistry: chatConditionRegistry,
       webhookTriggerInTenant,
       deliveryWorkbenchRequired: routineDeliveryWorkbenchRequired,
       // A routine created from inside a workbench delivers into that

packages/routines/package.json

@@ -17,6 +17,7 @@
   },
   "dependencies": {
     "@corbits/chat": "workspace:*",
+    "@corbits/error-sink": "workspace:*",
     "@corbits/folded-run-one-shot": "workspace:*",
     "@corbits/migration-runner": "workspace:*",
     "@corbits/slug": "workspace:*",

packages/routines/src/routes.ts

@@ -15,6 +15,9 @@ import type { RequireGrant } from "@intx/hub-api";
 import { idResource } from "@intx/hub-api";
 import { getLogger } from "@intx/log";
 import { generateId } from "@intx/hub-common";
+import { authorize } from "@intx/authz";
+import type { ConditionRegistry, GrantStore } from "@intx/types/authz";
+import { reportError } from "@corbits/error-sink";
 import {
   FoldedRunFailedError,
   FoldedRunTimedOutError,
@@ -33,6 +36,7 @@ import {
   routineTargetRejection,
   type LaunchableDefinitionResolver,
 } from "./target";
+import { validateRetarget } from "./routine-operations";
 import { makeErrorEnvelope } from "@workbench/hub-client";
 import {
   MyraRoutineDraftingUnavailableError,
@@ -127,6 +131,17 @@ export type CreateRoutineRoutesDeps = {
    * `definitionId: null`.
    */
   resolveTarget?: LaunchableDefinitionResolver;
+  /**
+   * When wired alongside `resolveTarget`, a create/retarget's resolved
+   * definition is also authorized (`workflow-definition:<definitionId>`
+   * / `read`, the same verb `listRoutineTargets` checks in
+   * `./targets.ts`) for the acting principal before the target is
+   * accepted — a denial is the typed 403 `rejectUnlaunchableTarget`
+   * returns. Both must be wired together; either omitted (the prior
+   * default) skips authorization, unchanged behavior.
+   */
+  grantStore?: GrantStore;
+  conditionRegistry?: ConditionRegistry;
   /**
    * When provided, a `{kind: "webhook"}` trigger is rejected with 404
    * unless the referenced `@corbits/webhook-triggers` row exists in the
@@ -232,6 +247,9 @@ const UpdateRoutineBody = type({
   "input?": "Record<string, unknown>",
   "enabled?": "boolean",
   "deliveryWorkbenchId?": "string",
+  // Retargets the routine (CL-7353): a workflow asset id, same rule as
+  // `CreateRoutineBody`'s own field — an explicit target, never inferred.
+  "definitionAssetId?": "string > 0",
 });
 
 const RunNowBody = type({
@@ -303,19 +321,48 @@ export async function resolvedRoutineView(
 }
 
 /**
- * Refuses a create/retarget whose target does not resolve — the typed
- * envelope UI and Myra branch on. `undefined` means the target is
- * launchable (or no resolver is wired). Exported for
- * `./workflow-routine-routes.ts`.
+ * Refuses a create/retarget whose target does not resolve, or that the
+ * acting principal is not authorized to reference — the typed envelope
+ * UI and Myra branch on. `undefined` means the target is launchable (or
+ * no resolver is wired). Exported for `./workflow-routine-routes.ts`.
+ *
+ * Authorization checks the same `workflow-definition:<definitionId>` /
+ * `read` verb `listRoutineTargets` (`./targets.ts`) checks per row, so a
+ * routine can never be pointed at a definition its own target listing
+ * would never have offered — but only when `grantStore` and
+ * `conditionRegistry` are both wired; either omitted skips it (CL-7351's
+ * prior default), matching every caller's existing tests.
  */
 export async function rejectUnlaunchableTarget(
-  deps: Pick<CreateRoutineRoutesDeps, "resolveTarget">,
+  deps: Pick<
+    CreateRoutineRoutesDeps,
+    "resolveTarget" | "grantStore" | "conditionRegistry"
+  >,
   tenantId: string,
+  principalId: string,
   definitionAssetId: string,
 ): Promise<ReturnType<typeof routineTargetRejection> | undefined> {
   if (deps.resolveTarget === undefined) return undefined;
   const target = await deps.resolveTarget(tenantId, definitionAssetId);
-  return target.ok ? undefined : routineTargetRejection(target.reason);
+  if (!target.ok) return routineTargetRejection(target.reason);
+  if (deps.grantStore === undefined || deps.conditionRegistry === undefined) {
+    return undefined;
+  }
+  const decision = await authorize(
+    deps.grantStore,
+    principalId,
+    tenantId,
+    `workflow-definition:${target.definitionId}`,
+    "read",
+    deps.conditionRegistry,
+  );
+  if (decision.effect === "allow") return undefined;
+  // A denial is reported identically to "not found" — naming a
+  // deployed-but-ungranted definition must not let a caller distinguish
+  // "exists, no access" from "doesn't exist" by probing different ids,
+  // same rule `target.ts`'s `routineTargetRejection` states for a
+  // cross-tenant asset.
+  return routineTargetRejection("not_found");
 }
 
 async function runView(
@@ -342,6 +389,14 @@ async function runView(
  * root timeline — see `RoutineLauncher`'s own doc comment for the
  * multi-message contract.
  *
+ * A retarget (CL-7353) is safe against an in-flight fire because the
+ * launcher itself re-resolves `definitionAssetId` through
+ * `resolveLaunchableDefinition` exactly once, at the moment this call
+ * launches (see `RoutineLauncher`'s doc comment and the hub's own
+ * `createHubRoutineLauncher`) — a run always launches against whatever
+ * one definition that single read named, never a definition read before
+ * the retarget landed spliced with one read after.
+ *
  * Exported: `./workflow-routine-routes.ts`'s "run now" reuses this exact
  * launch-then-correlate call, never a second launch path for Myra's own
  * tenant-scoped routine surface either.
@@ -519,14 +574,11 @@ export async function postRoutineEnabledNotice(
       text,
     });
   } catch (err) {
-    log.error(
-      "Failed to post routine-{verb} notice into workbench {workbenchId}",
-      {
-        verb: input.verb,
-        workbenchId: input.workbenchId,
-        err,
-      },
-    );
+    reportError(err, {
+      operation: "routines.postRoutineEnabledNotice",
+      tenantId: input.tenantId,
+      extra: { verb: input.verb, workbenchId: input.workbenchId },
+    });
   }
 }
 
@@ -569,6 +621,7 @@ export function createRoutineRoutes(
       const rejection = await rejectUnlaunchableTarget(
         deps,
         tenant.id,
+        principal.id,
         body.definitionAssetId,
       );
       if (rejection !== undefined) {
@@ -787,13 +840,54 @@ export function createRoutineRoutes(
         );
       }
 
+      const isRetarget =
+        body.definitionAssetId !== undefined &&
+        body.definitionAssetId !== existing.definitionAssetId;
+
+      if (isRetarget && body.definitionAssetId !== undefined) {
+        const rejection = await rejectUnlaunchableTarget(
+          deps,
+          tenant.id,
+          principal.id,
+          body.definitionAssetId,
+        );
+        if (rejection !== undefined) {
+          return c.json(
+            makeErrorEnvelope({
+              code: rejection.code,
+              userMessage: rejection.userMessage,
+            }),
+            rejection.status,
+          );
+        }
+
+        const retargetRejection = await validateRetarget(
+          deps,
+          tenant.id,
+          body.definitionAssetId,
+          existing,
+        );
+        if (retargetRejection !== undefined) {
+          return c.json(
+            makeErrorEnvelope({
+              code: retargetRejection.code,
+              userMessage: retargetRejection.userMessage,
+            }),
+            400,
+          );
+        }
+      }
+
+      const effectiveDefinitionAssetId =
+        body.definitionAssetId ?? existing.definitionAssetId;
+
       if (
         body.trigger !== undefined &&
         !(await webhookTriggerValid(
           deps,
           tenant.id,
           body.trigger,
-          existing.definitionAssetId,
+          effectiveDefinitionAssetId,
         ))
       ) {
         return c.json(
@@ -817,6 +911,9 @@ export function createRoutineRoutes(
       if (body.deliveryWorkbenchId !== undefined) {
         patch = { ...patch, deliveryWorkbenchId: body.deliveryWorkbenchId };
       }
+      if (body.definitionAssetId !== undefined) {
+        patch = { ...patch, definitionAssetId: body.definitionAssetId };
+      }
 
       const row = await deps.store.updateRoutine(tenant.id, routineId, patch);
 
@@ -1153,6 +1250,7 @@ export function createRoutineRoutes(
       const rejection = await rejectUnlaunchableTarget(
         deps,
         tenant.id,
+        principal.id,
         definitionAssetId,
       );
       if (rejection !== undefined) {

packages/routines/src/routine-operations.ts

@@ -0,0 +1,82 @@
+// The one place a routine retarget's non-authz validation lives, shared
+// by both route surfaces that can retarget a routine: `./routes.ts`'s
+// tenant-session `PATCH /routines/:id` and `./workflow-routine-routes.ts`'s
+// run-authenticated mirror. `rejectUnlaunchableTarget` (./routes.ts)
+// already covers whether the new target resolves and is authorized for
+// the acting principal; this covers the two checks `POST /routines`
+// (create) also runs against a fresh target — an unsuited delivery
+// workbench and an existing `input` that no longer satisfies the new
+// definition's input schema — so a retarget can't silently produce a
+// routine that fails at next launch the way create never could. Greybeard
+// review (CL-7353): PR 555 had to patch authz into both route factories
+// independently; this module is the seam that stops the next fix from
+// needing the same double patch for retarget validation specifically.
+// No import from `./routes` here on purpose: `./routes.ts` imports
+// `validateRetarget` from this module, so this module importing back from
+// `./routes.ts` would be a cycle. `deliveryWorkbenchRequired`'s
+// "omitted means required" default is duplicated from
+// `routes.ts`'s `isDeliveryWorkbenchRequired` (one line, unlikely to
+// drift; both are covered by the same tests via each route's PATCH).
+export type RetargetValidationDeps = {
+  readonly deliveryWorkbenchRequired?: (
+    tenantId: string,
+    definitionAssetId: string,
+  ) => Promise<boolean>;
+  readonly validateRoutineInput?: (
+    tenantId: string,
+    definitionAssetId: string,
+    input: Record<string, unknown>,
+  ) => Promise<
+    { readonly ok: true } | { readonly ok: false; readonly message: string }
+  >;
+};
+
+export type RetargetValidationRejection = {
+  readonly code: "bad_request";
+  readonly userMessage: string;
+};
+
+/**
+ * Re-runs create's delivery-workbench-required and input-schema checks
+ * against a routine being retargeted at `effectiveDefinitionAssetId`,
+ * using the routine's own existing `deliveryWorkbenchId`/`input` (a
+ * retarget-only PATCH never asks the caller to resupply fields it isn't
+ * changing). `undefined` means the retarget's non-target fields are still
+ * satisfied by the new target.
+ */
+export async function validateRetarget(
+  deps: RetargetValidationDeps,
+  tenantId: string,
+  effectiveDefinitionAssetId: string,
+  existing: {
+    readonly deliveryWorkbenchId: string | null;
+    readonly input: Record<string, unknown>;
+  },
+): Promise<RetargetValidationRejection | undefined> {
+  const deliveryRequired =
+    deps.deliveryWorkbenchRequired === undefined
+      ? true
+      : await deps.deliveryWorkbenchRequired(
+          tenantId,
+          effectiveDefinitionAssetId,
+        );
+  if (deliveryRequired && existing.deliveryWorkbenchId === null) {
+    return {
+      code: "bad_request",
+      userMessage: "deliveryWorkbenchId is required for this workflow",
+    };
+  }
+
+  if (deps.validateRoutineInput !== undefined) {
+    const validated = await deps.validateRoutineInput(
+      tenantId,
+      effectiveDefinitionAssetId,
+      existing.input,
+    );
+    if (!validated.ok) {
+      return { code: "bad_request", userMessage: validated.message };
+    }
+  }
+
+  return undefined;
+}

packages/routines/src/store.ts

@@ -95,8 +95,15 @@ export type CreateRoutineIfAbsentResult =
 
 export interface UpdateRoutineInput {
   readonly name?: string;
-  /** Retargets the routine at a different workflow asset (CL-7359) —
-   * see `CreateRoutineInput.definitionAssetId`'s own doc comment. */
+  /**
+   * Retargets the routine to a different workflow asset (CL-7359) — the
+   * same single UPDATE as every other field here, so a launch that reads
+   * the row once (`resolveLaunchableDefinition` at fire time,
+   * `./target.ts`) never sees a half-applied retarget. The route
+   * validates this through `resolveLaunchableDefinition` before it ever
+   * reaches the store (see `./routes.ts`'s `rejectUnlaunchableTarget`) —
+   * see `CreateRoutineInput.definitionAssetId`'s own doc comment.
+   */
   readonly definitionAssetId?: string;
   readonly trigger?: RoutineTriggerT;
   readonly input?: Record<string, unknown>;