From 5536c27dbb9b8e3947ec47011015e481ef13addd Mon Sep 17 00:00:00 2001
From: Axel Hecht <axel@pike.org>
Date: Mon, 16 Aug 2021 10:48:39 +0200
Subject: [PATCH] address review comments and test input validation

Signed-off-by: Axel Hecht <axel@pike.org>
---
 .../actions/builtin/fetch/template.test.ts    | 43 +++++++++++++++++++
 .../actions/builtin/fetch/template.ts         | 17 +++++---
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts
index 659fe1692a..61b77ad679 100644
--- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts
+++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts
@@ -105,6 +105,49 @@ describe('fetch:template', () => {
       ).rejects.toThrowError(/copyWithoutRender must be an array/i);
     });
 
+    it('throws if copyWithoutRender is used with extension', async () => {
+      await expect(() =>
+        action.handler(
+          mockContext({
+            copyWithoutRender: ['abc'],
+            extension: true,
+          }),
+        ),
+      ).rejects.toThrowError(
+        /input extension incompatible with copyWithoutRender and cookiecutterCompat/,
+      );
+    });
+
+    it('throws if cookiecutterCompat is used with extension', async () => {
+      await expect(() =>
+        action.handler(
+          mockContext({
+            cookiecutterCompat: true,
+            extension: true,
+          }),
+        ),
+      ).rejects.toThrowError(
+        /input extension incompatible with copyWithoutRender and cookiecutterCompat/,
+      );
+    });
+
+    it('throws if extension string lacks a leading dot', async () => {
+      await expect(() =>
+        action.handler(
+          mockContext({
+            extension: 'njk',
+          }),
+        ),
+      ).rejects.toThrowError(/extension needs to start with a `.`/);
+      await expect(() =>
+        action.handler(
+          mockContext({
+            extension: '.',
+          }),
+        ),
+      ).rejects.toThrowError(/extension needs to start with a `.`/);
+    });
+
     describe('with valid input', () => {
       let context: ActionContext<FetchTemplateInput>;
 
diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts
index f6afa5e8c6..6a4fc3865b 100644
--- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts
+++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { resolve as resolvePath } from 'path';
+import { resolve as resolvePath, extname } from 'path';
 import { resolveSafeChildPath, UrlReader } from '@backstage/backend-common';
 import { InputError } from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
@@ -138,10 +138,15 @@ export function createFetchTemplateAction(options: {
 
       let extension: string | false = false;
       if (ctx.input.extension) {
-        extension =
-          typeof ctx.input.extension === 'boolean'
-            ? '.njk'
-            : ctx.input.extension;
+        extension = ctx.input.extension === true ? '.njk' : ctx.input.extension;
+      }
+      if (
+        extension !== false &&
+        (extension.length < 2 || !extension.startsWith('.'))
+      ) {
+        throw new InputError(
+          'Fetch action input extension needs to start with a `.`',
+        );
       }
 
       await fetchContents({
@@ -224,7 +229,7 @@ export function createFetchTemplateAction(options: {
 
         let localOutputPath = location;
         if (extension) {
-          if (localOutputPath.endsWith(extension)) {
+          if (extname(localOutputPath) === extension) {
             localOutputPath = localOutputPath.slice(0, -extension.length);
           } else {
             shouldCopyWithoutRender = true;