fix: Implement path validation, rate limiting and Input safety across core modules

package.json

@@ -37,7 +37,8 @@
     "validator": "13.15.15",
     "yaml": "2.8.1",
     "yargs": "^17.7.2",
-    "zx": "8.8.3"
+    "zx": "8.8.3",
+    "express-rate-limit": "^8.1.0"
   },
   "description": "Otomi Core is an opinionated stack of Kubernetes apps and configurations. Part of Otomi Container Platform.",
   "devDependencies": {

src/common/crypt.ts

@@ -1,7 +1,8 @@
 import { EventEmitter } from 'events'
-import { existsSync } from 'fs'
+import { existsSync, realpathSync } from 'fs'
 import { readFile, stat, utimes, writeFile } from 'fs/promises'
 import { chunk } from 'lodash'
+import * as pathModule from 'path'
 import { $, cd, ProcessOutput } from 'zx'
 import { terminal } from './debug'
 import { cleanEnv, cliEnvSpec, env, isCli } from './envalid'
@@ -94,10 +95,32 @@ const processFileChunk = async (crypt: CR, files: string[]): Promise<(ProcessOut
 const runOnSecretFiles = async (path: string, crypt: CR, filesArgs: string[] = []): Promise<void> => {
   const d = terminal(`common:crypt:runOnSecretFiles`)
   let files: string[] = filesArgs
+  const MAX_FILES = 1000
+
+  // Defensive: ensure files is a real array and safely sized
+  if (!Array.isArray(files)) {
+    d.error('Expected files to be an Array')
+    throw new Error("Files parameter must be an Array")
+  }
+  // Bound the files length
+  if (typeof files.length !== 'number' || !Number.isInteger(files.length) || files.length < 0 || files.length > MAX_FILES) {
+    d.warn(`Too many files supplied (${files.length}), truncating to ${MAX_FILES}`)
+    files = files.slice(0, MAX_FILES)
+  }
 
   if (files.length === 0) {
     files = await getAllSecretFiles(path)
+    // Defensive: ensure safe array and length for auto-discovered files as well
+    if (!Array.isArray(files)) {
+      d.error('Expected files from getAllSecretFiles to be an Array')
+      throw new Error("getAllSecretFiles must return an Array")
+    }
+    if (typeof files.length !== 'number' || !Number.isInteger(files.length) || files.length < 0 || files.length > MAX_FILES) {
+      d.warn(`Too many files discovered (${files.length}), truncating to ${MAX_FILES}`)
+      files = files.slice(0, MAX_FILES)
+    }
   }
+
   files = files.filter(async (f) => {
     const suffix = crypt.cmd === CryptType.ENCRYPT ? '.dec' : ''
     let file = `${f}${suffix}`
@@ -146,17 +169,30 @@ const matchTimestamps = async (path, file: string) => {
 
 export const decrypt = async (path = env.ENV_DIR, ...files: string[]): Promise<void> => {
   const d = terminal(`common:crypt:decrypt`)
-  if (!existsSync(`${path}/.sops.yaml`)) {
+  // Validate that `path` is within rootDir
+  const envRoot = rootDir
+  let resolvedPath: string
+  try {
+    resolvedPath = realpathSync(pathModule.resolve(envRoot, pathModule.relative(envRoot, path)))
+  } catch (err) {
+    d.warn(`Failed to resolve environment path '${path}': ${err}`)
+    return
+  }
+  if (!resolvedPath.startsWith(envRoot)) {
+    d.warn(`Denied access to environment path: '${path}' (resolved: '${resolvedPath}')`)
+    return
+  }
+  if (!existsSync(`${resolvedPath}/.sops.yaml`)) {
     d.info('Skipping decryption')
     return
   }
   d.info('Starting decryption')
 
   await runOnSecretFiles(
-    path,
+    resolvedPath,
     {
       cmd: CryptType.DECRYPT,
-      post: async (f) => matchTimestamps(path, f),
+      post: async (f) => matchTimestamps(resolvedPath, f),
     },
     files,
   )
@@ -166,13 +202,26 @@ export const decrypt = async (path = env.ENV_DIR, ...files: string[]): Promise<v
 
 export const encrypt = async (path = env.ENV_DIR, ...files: string[]): Promise<void> => {
   const d = terminal(`common:crypt:encrypt`)
-  if (!existsSync(`${path}/.sops.yaml`)) {
+  // Validate that `path` is within rootDir
+  const envRoot = rootDir
+  let resolvedPath: string
+  try {
+    resolvedPath = realpathSync(pathModule.resolve(envRoot, pathModule.relative(envRoot, path)))
+  } catch (err) {
+    d.warn(`Failed to resolve environment path '${path}': ${err}`)
+    return
+  }
+  if (!resolvedPath.startsWith(envRoot)) {
+    d.warn(`Denied access to environment path: '${path}' (resolved: '${resolvedPath}')`)
+    return
+  }
+  if (!existsSync(`${resolvedPath}/.sops.yaml`)) {
     d.info('Skipping encryption')
     return
   }
   d.info('Starting encryption')
   await runOnSecretFiles(
-    path,
+    resolvedPath,
     {
       condition: async (file: string): Promise<boolean> => {
         if (!existsSync(file)) {
@@ -206,7 +255,7 @@ export const encrypt = async (path = env.ENV_DIR, ...files: string[]): Promise<v
         return false
       },
       cmd: CryptType.ENCRYPT,
-      post: async (f: string) => matchTimestamps(path, f),
+      post: async (f: string) => matchTimestamps(resolvedPath, f),
     },
     files,
   )

src/common/repo.ts

@@ -7,6 +7,8 @@ import path from 'path'
 import { getDirNames, loadYaml } from './utils'
 import { objectToYaml, writeValuesToFile } from './values'
 
+// Define the base directory allowed for all envDir input (adjust as appropriate for the project)
+const ROOT = path.resolve('/var/data')  // Change /var/data to desired allowed directory
 export async function getTeamNames(envDir: string): Promise<Array<string>> {
   const teamsDir = path.join(envDir, 'env', 'teams')
   return await getDirNames(teamsDir, { skipHidden: true })
@@ -515,21 +517,27 @@ export async function setValuesFile(
   envDir: string,
   deps = { pathExists: existsSync, loadValues, writeFile },
 ): Promise<string> {
-  const valuesPath = path.join(envDir, 'values-repo.yaml')
+  const safeEnvDir = path.resolve(ROOT, envDir)
+  if (!safeEnvDir.startsWith(ROOT)) throw new Error('Invalid envDir: outside permitted directory')
+  const valuesPath = path.join(safeEnvDir, 'values-repo.yaml')
   // if (await deps.pathExists(valuesPath)) return valuesPath
-  const allValues = await deps.loadValues(envDir)
+  const allValues = await deps.loadValues(safeEnvDir)
   await deps.writeFile(valuesPath, objectToYaml(allValues))
   return valuesPath
 }
 
 export async function unsetValuesFile(envDir: string): Promise<string> {
-  const valuesPath = path.join(envDir, 'values-repo.yaml')
+  const safeEnvDir = path.resolve(ROOT, envDir)
+  if (!safeEnvDir.startsWith(ROOT)) throw new Error('Invalid envDir: outside permitted directory')
+  const valuesPath = path.join(safeEnvDir, 'values-repo.yaml')
   await rm(valuesPath, { force: true })
   return valuesPath
 }
 
 export function unsetValuesFileSync(envDir: string): string {
-  const valuesPath = path.join(envDir, 'values-repo.yaml')
+  const safeEnvDir = path.resolve(ROOT, envDir)
+  if (!safeEnvDir.startsWith(ROOT)) throw new Error('Invalid envDir: outside permitted directory')
+  const valuesPath = path.join(safeEnvDir, 'values-repo.yaml')
   rmSync(valuesPath, { force: true })
   return valuesPath
 }

src/common/utils.ts

@@ -17,6 +17,13 @@ const packagePath = process.cwd()
 // we keep the rootDir for zx, but have to fix it for drone, which starts in /home/app/stack/env (to accommodate write perms):
 export const rootDir = process.cwd() === '/home/app/stack/env' ? '/home/app/stack' : process.cwd()
 export const pkg = JSON.parse(readFileSync(`${rootDir}/package.json`, 'utf8'))
+// Ensure a resolved path is within rootDir
+const isSubPath = (target: string, base: string): boolean => {
+  // Ensure trailing slash for correct startsWith
+  const baseNorm = resolve(base) + '/'
+  const targetNorm = resolve(target)
+  return targetNorm === resolve(base) || targetNorm.startsWith(baseNorm)
+}
 export const getFilename = (path: string): string => path.split('/').pop()?.split('.')[0] as string
 
 export const asArray = (args: string | string[]): string[] => {
@@ -35,6 +42,11 @@ export const removeBlankAttributes = (obj: Record<string, any>): Record<string,
 }
 
 export const readdirRecurse = async (dir: string, opts?: { skipHidden: boolean }): Promise<string[]> => {
+  // Validate that dir is within rootDir
+  if (!isSubPath(dir, rootDir)) {
+    terminal('common:utils').warn(`Refusing to recurse outside rootDir: ${dir}`)
+    return []
+  }
   const dirs = await readdir(dir, { withFileTypes: true })
   const files = await Promise.all(
     dirs.map(async (dirOrFile) => {

src/server.ts

@@ -1,19 +1,28 @@
 import $RefParser, { JSONSchema } from '@apidevtools/json-schema-ref-parser'
 import express, { Request, Response } from 'express'
+import rateLimit from 'express-rate-limit'
+import { copyFile } from 'fs/promises'
 import { Server } from 'http'
+import * as path from 'path'
 import { bootstrapSops } from 'src/cmd/bootstrap'
 import { decrypt, encrypt } from 'src/common/crypt'
 import { terminal } from 'src/common/debug'
 import { hfValues } from './common/hf'
 import { setValuesFile, unsetValuesFile } from './common/repo'
 import { loadYaml, rootDir } from './common/utils'
 import { objectToYaml } from './common/values'
-import { copyFile } from 'fs/promises'
 
 const d = terminal('server')
 const app = express()
 let server: Server
 
+// Rate limiter for expensive routes
+const prepareLimiter = rateLimit({
+  windowMs: 1 * 60 * 1000, // 1 minute
+  max: 10, // Limit each IP to 10 requests per minute
+  message: 'Too many requests, please try again later.',
+})
+
 export const stopServer = (): void => {
   server?.close()
 }
@@ -39,18 +48,28 @@ app.get('/init', async (req: Request, res: Response): Promise<void> => {
   }
 })
 
-app.get('/prepare', async (req: Request, res: Response): Promise<void> => {
+app.get('/prepare', prepareLimiter, async (req: Request, res: Response): Promise<void> => {
   const { envDir, files } = req.query as QueryParams
+  // Define the allowed environment root directory
+  const allowedEnvsRoot = path.resolve(rootDir, '.envs')
+  let resolvedEnvDir = path.resolve(allowedEnvsRoot, envDir || '')
   try {
-    d.log('Request to prepare values repo on', envDir)
+    // Check if the resolved environment directory is inside the allowed root
+    if (!resolvedEnvDir.startsWith(allowedEnvsRoot)) {
+      res.status(403).send('Forbidden: Invalid envDir path.')
+      return
+    }
+    d.log('Request to prepare values repo on', resolvedEnvDir)
     const file = '.editorconfig'
-    await copyFile(`${rootDir}/.values/${file}`, `${envDir}/${file}`)
-    await bootstrapSops(envDir)
-    await setValuesFile(envDir)
+    const srcFile = path.resolve(rootDir, '.values', file)
+    const destFile = path.resolve(resolvedEnvDir, file)
+    await copyFile(srcFile, destFile)
+    await bootstrapSops(resolvedEnvDir)
+    await setValuesFile(resolvedEnvDir)
     // Encrypt ensures that a brand new secret file is encrypted in place
-    await encrypt(envDir, ...(files ?? []))
+    await encrypt(resolvedEnvDir, ...(files ?? []))
     // Decrypt ensures that a brand new encrypted secret file is decrypted to the .dec file
-    await decrypt(envDir, ...(files ?? []))
+    await decrypt(resolvedEnvDir, ...(files ?? []))
     res.status(200).send('ok')
   } catch (error) {
     const err = `${error}`
@@ -61,7 +80,7 @@ app.get('/prepare', async (req: Request, res: Response): Promise<void> => {
     }
     res.status(status).send(err)
   } finally {
-    await unsetValuesFile(envDir)
+    await unsetValuesFile(resolvedEnvDir)
   }
 })