scaffolder: add validation on action input

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>

packages/backend/src/plugins/permission.ts

@@ -18,6 +18,7 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
 import { createRouter } from '@backstage/plugin-permission-backend';
 import {
   AuthorizeResult,
+  isPermission,
   PolicyDecision,
 } from '@backstage/plugin-permission-common';
 import {
@@ -28,8 +29,19 @@ import {
   DefaultPlaylistPermissionPolicy,
   isPlaylistPermission,
 } from '@backstage/plugin-playlist-backend';
+import {
+  createScaffolderActionConditionalDecision,
+  scaffolderActionConditions,
+} from '@backstage/plugin-scaffolder-backend';
+import { actionExecutePermission } from '@backstage/plugin-scaffolder-common';
 import { Router } from 'express';
 import { PluginEnvironment } from '../types';
+import { z } from 'zod';
+import zodToJsonSchema from 'zod-to-json-schema';
+
+const inputSchema = zodToJsonSchema(
+  z.object({ message: z.enum(['hello']) }).strict(),
+);
 
 class ExamplePermissionPolicy implements PermissionPolicy {
   private playlistPermissionPolicy = new DefaultPlaylistPermissionPolicy();
@@ -42,6 +54,16 @@ class ExamplePermissionPolicy implements PermissionPolicy {
       return this.playlistPermissionPolicy.handle(request, user);
     }
 
+    if (isPermission(request.permission, actionExecutePermission)) {
+      return createScaffolderActionConditionalDecision(
+        request.permission,
+        scaffolderActionConditions.matchesInput({
+          action: 'debug:log',
+          schema: inputSchema,
+        }),
+      );
+    }
+
     return {
       result: AuthorizeResult.ALLOW,
     };

plugins/scaffolder-backend/package.json

@@ -68,6 +68,7 @@
     "@octokit/webhooks": "^10.0.0",
     "@types/express": "^4.17.6",
     "@types/luxon": "^3.0.0",
+    "ajv": "^8.12.0",
     "azure-devops-node-api": "^11.0.1",
     "command-exists": "^1.2.9",
     "compression": "^1.7.4",

plugins/scaffolder-backend/src/service/conditionExports.ts

@@ -14,9 +14,12 @@
  * limitations under the License.
  */
 
-import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha';
+import {
+  RESOURCE_TYPE_SCAFFOLDER_TEMPLATE,
+  RESOURCE_TYPE_SCAFFOLDER_ACTION,
+} from '@backstage/plugin-scaffolder-common/alpha';
 import { createConditionExports } from '@backstage/plugin-permission-node';
-import { scaffolderTemplateRules } from './rules';
+import { scaffolderTemplateRules, scaffolderActionRules } from './rules';
 
 const { conditions, createConditionalDecision } = createConditionExports({
   pluginId: 'scaffolder',
@@ -24,6 +27,15 @@ const { conditions, createConditionalDecision } = createConditionExports({
   rules: scaffolderTemplateRules,
 });
 
+const {
+  conditions: scaffolderActionConditions,
+  createConditionalDecision: createScaffolderActionConditionalDecision,
+} = createConditionExports({
+  pluginId: 'scaffolder',
+  resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+  rules: scaffolderActionRules,
+});
+
 /**
  * These conditions are used when creating conditional decisions for scaffolder
  * templates that are returned by authorization policies.
@@ -65,3 +77,7 @@ export const scaffolderConditions = conditions;
  * @alpha
  */
 export const createScaffolderConditionalDecision = createConditionalDecision;
+export {
+  createScaffolderActionConditionalDecision,
+  scaffolderActionConditions,
+};

plugins/scaffolder-backend/src/service/router.ts

@@ -40,6 +40,7 @@ import {
   scaffolderPermissions,
   templateParameterReadPermission,
   templateStepReadPermission,
+  actionExecutePermission,
 } from '@backstage/plugin-scaffolder-common/alpha';
 import express from 'express';
 import Router from 'express-promise-router';
@@ -71,7 +72,7 @@ import {
   createPermissionIntegrationRouter,
   PermissionRule,
 } from '@backstage/plugin-permission-node';
-import { scaffolderTemplateRules } from './rules';
+import { scaffolderTemplateRules, scaffolderActionRules } from './rules';
 
 /**
  *
@@ -86,6 +87,10 @@ export type TemplatePermissionRuleInput<
   TParams
 >;
 
+const isActionAuthorized = createConditionAuthorizer(
+  Object.values(scaffolderActionRules),
+);
+
 /**
  * RouterOptions
  *
@@ -376,9 +381,16 @@ export async function createRouter(
         }
       }
 
-      const baseUrl = getEntityBaseUrl(template);
-
-      const taskSpec = await authorizeTaskSpec(template);
+      const taskSpec = await authorizeActions(
+        {
+          parameters: values,
+          template,
+          templateRef: { kind, name, namespace },
+          user: userEntity as UserEntity,
+          userEntityRef,
+        },
+        { token },
+      );
 
       const result = await taskBroker.dispatch({
         spec: taskSpec,
@@ -627,7 +639,24 @@ export async function createRouter(
     return template;
   }
 
-  async function authorizeTaskSpec(template: TemplateEntityV1beta3): TaskSpec {
+  async function authorizeActions(
+    {
+      parameters,
+      template,
+      user,
+      userEntityRef,
+      templateRef,
+    }: {
+      template: TemplateEntityV1beta3;
+      parameters: JsonObject;
+      user: UserEntity;
+      userEntityRef: string | undefined;
+      templateRef: CompoundEntityRef;
+    },
+    { token }: { token: string | undefined },
+  ): Promise<TaskSpec> {
+    const baseUrl = getEntityBaseUrl(template);
+
     const taskSpec: TaskSpec = {
       apiVersion: template.apiVersion,
       steps: template.spec.steps.map((step, index) => ({
@@ -636,17 +665,13 @@ export async function createRouter(
         name: step.name ?? step.action,
       })),
       output: template.spec.output ?? {},
-      parameters: values,
+      parameters,
       user: {
-        entity: userEntity as UserEntity,
+        entity: user,
         ref: userEntityRef,
       },
       templateInfo: {
-        entityRef: stringifyEntityRef({
-          kind,
-          namespace,
-          name: template.metadata?.name,
-        }),
+        entityRef: stringifyEntityRef(templateRef),
         baseUrl,
         entity: {
           metadata: template.metadata,
@@ -654,6 +679,20 @@ export async function createRouter(
       },
     };
 
+    if (permissionApi) {
+      const [decision] = await permissionApi.authorizeConditional(
+        [{ permission: actionExecutePermission }],
+        { token },
+      );
+
+      for (const step of taskSpec.steps) {
+        const { action, input } = step;
+        if (!isActionAuthorized(decision, { action, input })) {
+          throw new InputError(`Unauthorized action: ${action}, ${input}`);
+        }
+      }
+    }
+
     return taskSpec;
   }
 

plugins/scaffolder-backend/src/service/rules.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import Ajv from 'ajv';
 import { makeCreatePermissionRule } from '@backstage/plugin-permission-node';
 import {
   RESOURCE_TYPE_SCAFFOLDER_TEMPLATE,
@@ -34,11 +35,11 @@ export const createTemplatePermissionRule = makeCreatePermissionRule<
   typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE
 >();
 
+const ajv = new Ajv({ allErrors: true });
 export const createActionPermissionRule = makeCreatePermissionRule<
   {
-    actionId: string;
-    input: JsonObject;
-    template: TemplateEntityStepV1beta3 | TemplateParametersV1beta3;
+    action: string;
+    input: JsonObject | undefined;
   },
   {},
   typeof RESOURCE_TYPE_SCAFFOLDER_ACTION
@@ -52,7 +53,7 @@ export const hasActionId = createActionPermissionRule({
     actionId: z.string().describe('Name of the actionId to match on'),
   }),
   apply: (resource, { actionId }) => {
-    return resource.actionId === actionId;
+    return resource.action === actionId;
   },
   toQuery: () => ({}),
 });
@@ -62,23 +63,19 @@ export const matchesInput = createActionPermissionRule({
   resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
   description: `Matches actionId and the input given`,
   paramsSchema: z.object({
-    actionId: z.string().describe('Name of the actionId to match on'),
-
+    action: z.string().describe('Name of the actionId to match on'),
     // Pass in a json schema to validate the input against
-    input: z.jsonSchema({}).describe('Input to match on'),
+    schema: z.record(z.any()),
   }),
-  apply: (resource, { actionId, input }) => {
-    if (resource.actionId !== actionId) {
-      return false;
+  apply: (resource, { action, schema }) => {
+    if (resource.action !== action) {
+      return true;
+    }
+    if (!schema) {
+      return true;
     }
 
-    for (const [key, value] of Object.entries(input)) {
-      if (resource.input[key] !== value) {
-        return false;
-      }
-    }
-
-    return true;
+    return ajv.validate(schema, resource.input);
   },
   toQuery: () => ({}),
 });
@@ -97,3 +94,4 @@ export const hasTag = createTemplatePermissionRule({
 });
 
 export const scaffolderTemplateRules = { hasTag };
+export const scaffolderActionRules = { matchesInput };

template.yaml

@@ -0,0 +1,30 @@
+apiVersion: scaffolder.backstage.io/v1beta3
+kind: Template
+metadata:
+  name: my-custom-template
+  parameters:
+    - title: Provide some simple information
+      metadata:
+        tags:
+          - bang
+      properties:
+        description:
+          title: Description
+          type: string
+            metadata:
+              tags:
+                - bar
+  steps:
+    - id: step-one
+      name: First log
+      action: debug:log
+      input:
+        message: hello
+      metadata:
+        tags:
+          - foo
+    - id: step-two
+      name: Second log
+      action: debug:log
+      input:
+        message: world

yarn.lock

@@ -8020,6 +8020,7 @@ __metadata:
     "@types/nunjucks": ^3.1.4
     "@types/supertest": ^2.0.8
     "@types/zen-observable": ^0.8.0
+    ajv: ^8.12.0
     azure-devops-node-api: ^11.0.1
     command-exists: ^1.2.9
     compression: ^1.7.4