From de4553f25a09c3d82113eb3d476566d7bbf3c59b Mon Sep 17 00:00:00 2001
From: Nikola Jokic <97525037+nikola-jokic@users.noreply.github.com>
Date: Wed, 15 Jun 2022 14:54:50 +0200
Subject: [PATCH] added permission check for secrets (#14)

* added permission check for secrets

* typo in subresource

* moved auth check to the command receiver
---
 packages/k8s/src/hooks/cleanup-job.ts |  5 ++-
 packages/k8s/src/hooks/prepare-job.ts | 10 ------
 packages/k8s/src/index.ts             | 10 +++++-
 packages/k8s/src/k8s/index.ts         | 51 +++++++--------------------
 4 files changed, 23 insertions(+), 53 deletions(-)

diff --git a/packages/k8s/src/hooks/cleanup-job.ts b/packages/k8s/src/hooks/cleanup-job.ts
index b36dd57..acec106 100644
--- a/packages/k8s/src/hooks/cleanup-job.ts
+++ b/packages/k8s/src/hooks/cleanup-job.ts
@@ -1,6 +1,5 @@
-import { pruneSecrets, prunePods } from '../k8s'
+import { prunePods, pruneSecrets } from '../k8s'
 
 export async function cleanupJob(): Promise<void> {
-  await prunePods()
-  await pruneSecrets()
+  await Promise.all([prunePods(), pruneSecrets()])
 }
diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts
index 9f9beba..96c3bc5 100644
--- a/packages/k8s/src/hooks/prepare-job.ts
+++ b/packages/k8s/src/hooks/prepare-job.ts
@@ -6,11 +6,8 @@ import path from 'path'
 import {
   containerPorts,
   createPod,
-  isAuthPermissionsOK,
   isPodContainerAlpine,
-  namespace,
   prunePods,
-  requiredPermissions,
   waitForPodPhases
 } from '../k8s'
 import {
@@ -30,13 +27,6 @@ export async function prepareJob(
   }
 
   await prunePods()
-  if (!(await isAuthPermissionsOK())) {
-    throw new Error(
-      `The Service account needs the following permissions ${JSON.stringify(
-        requiredPermissions
-      )} on the pod resource in the '${namespace}' namespace. Please contact your self hosted runner administrator.`
-    )
-  }
   await copyExternalsToRoot()
   let container: k8s.V1Container | undefined = undefined
   if (args.container?.image) {
diff --git a/packages/k8s/src/index.ts b/packages/k8s/src/index.ts
index 047bfe2..e4642e5 100644
--- a/packages/k8s/src/index.ts
+++ b/packages/k8s/src/index.ts
@@ -1,11 +1,12 @@
-import { Command, getInputFromStdin, prepareJobArgs } from 'hooklib'
 import * as core from '@actions/core'
+import { Command, getInputFromStdin, prepareJobArgs } from 'hooklib'
 import {
   cleanupJob,
   prepareJob,
   runContainerStep,
   runScriptStep
 } from './hooks'
+import { isAuthPermissionsOK, namespace, requiredPermissions } from './k8s'
 
 async function run(): Promise<void> {
   const input = await getInputFromStdin()
@@ -17,6 +18,13 @@ async function run(): Promise<void> {
 
   let exitCode = 0
   try {
+    if (!(await isAuthPermissionsOK())) {
+      throw new Error(
+        `The Service account needs the following permissions ${JSON.stringify(
+          requiredPermissions
+        )} on the pod resource in the '${namespace}' namespace. Please contact your self hosted runner administrator.`
+      )
+    }
     switch (command) {
       case Command.PrepareJob:
         await prepareJob(args as prepareJobArgs, responseFile)
diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts
index 445c2c8..f3c16cc 100644
--- a/packages/k8s/src/k8s/index.ts
+++ b/packages/k8s/src/k8s/index.ts
@@ -45,16 +45,15 @@ export const requiredPermissions = [
     verbs: ['get', 'list', 'create', 'delete'],
     resource: 'jobs',
     subresource: ''
+  },
+  {
+    group: '',
+    verbs: ['create', 'delete', 'get', 'list'],
+    resource: 'secrets',
+    subresource: ''
   }
 ]
 
-const secretPermission = {
-  group: '',
-  verbs: ['get', 'list', 'create', 'delete'],
-  resource: 'secrets',
-  subresource: ''
-}
-
 export async function createPod(
   jobContainer?: k8s.V1Container,
   services?: k8s.V1Container[],
@@ -94,19 +93,13 @@ export async function createPod(
   ]
 
   if (registry) {
-    if (await isSecretsAuthOK()) {
-      const secret = await createDockerSecret(registry)
-      if (!secret?.metadata?.name) {
-        throw new Error(`created secret does not have secret.metadata.name`)
-      }
-      const secretReference = new k8s.V1LocalObjectReference()
-      secretReference.name = secret.metadata.name
-      appPod.spec.imagePullSecrets = [secretReference]
-    } else {
-      throw new Error(
-        `Pulls from private registry is not allowed. Please contact your self hosted runner administrator. Service account needs permissions for ${secretPermission.verbs} in resource ${secretPermission.resource}`
-      )
+    const secret = await createDockerSecret(registry)
+    if (!secret?.metadata?.name) {
+      throw new Error(`created secret does not have secret.metadata.name`)
     }
+    const secretReference = new k8s.V1LocalObjectReference()
+    secretReference.name = secret.metadata.name
+    appPod.spec.imagePullSecrets = [secretReference]
   }
 
   const { body } = await k8sApi.createNamespacedPod(namespace(), appPod)
@@ -440,26 +433,6 @@ export async function isAuthPermissionsOK(): Promise<boolean> {
   return responses.every(resp => resp.body.status?.allowed)
 }
 
-export async function isSecretsAuthOK(): Promise<boolean> {
-  const sar = new k8s.V1SelfSubjectAccessReview()
-  const asyncs: Promise<{
-    response: unknown
-    body: k8s.V1SelfSubjectAccessReview
-  }>[] = []
-  for (const verb of secretPermission.verbs) {
-    sar.spec = new k8s.V1SelfSubjectAccessReviewSpec()
-    sar.spec.resourceAttributes = new k8s.V1ResourceAttributes()
-    sar.spec.resourceAttributes.verb = verb
-    sar.spec.resourceAttributes.namespace = namespace()
-    sar.spec.resourceAttributes.group = secretPermission.group
-    sar.spec.resourceAttributes.resource = secretPermission.resource
-    sar.spec.resourceAttributes.subresource = secretPermission.subresource
-    asyncs.push(k8sAuthorizationV1Api.createSelfSubjectAccessReview(sar))
-  }
-  const responses = await Promise.all(asyncs)
-  return responses.every(resp => resp.body.status?.allowed)
-}
-
 export async function isPodContainerAlpine(
   podName: string,
   containerName: string