improvement(config): explicitly validate sub-paths when applicable

We add a `path` rule, with options to disallow absolute paths or parent paths, i.e. stepping up a directory tree, in order to ensure that some paths are sub-paths of a project or module. This required a refactor where we use a custom Joi instance across our codebase, hence the size of the change. This is done both for general hardening, and in preparation of #853 and other similar configuration options.
garden-io · Jun 24, 2019 · 6343603 · 6343603
1 parent 5067f0c
commit 6343603
Show file tree

Hide file tree

Showing 66 changed files with 717 additions and 603 deletions.
diff --git a/docs/reference/module-types/container.md b/docs/reference/module-types/container.md
@@ -265,7 +265,7 @@ hotReload:
 
 ### `dockerfile`
 
-POSIX-style name of Dockerfile, relative to project root. Defaults to $MODULE_ROOT/Dockerfile.
+POSIX-style name of Dockerfile, relative to module root.
 
 | Type     | Required |
 | -------- | -------- |

diff --git a/docs/reference/module-types/maven-container.md b/docs/reference/module-types/maven-container.md
@@ -270,7 +270,7 @@ hotReload:
 
 ### `dockerfile`
 
-POSIX-style name of Dockerfile, relative to project root. Defaults to $MODULE_ROOT/Dockerfile.
+POSIX-style name of Dockerfile, relative to module root.
 
 | Type     | Required |
 | -------- | -------- |
@@ -890,7 +890,7 @@ Key/value map of environment variables. Keys must be valid POSIX environment var
 
 ### `jarPath`
 
-The path to the packaged JAR artifact, relative to the module directory.
+POSIX-style path to the packaged JAR artifact, relative to the module directory.
 
 | Type     | Required |
 | -------- | -------- |

diff --git a/garden-service/package-lock.json b/garden-service/package-lock.json
diff --git a/garden-service/package.json b/garden-service/package.json
@@ -22,6 +22,7 @@
     "static"
   ],
   "dependencies": {
+    "@hapi/joi": "^15.0.3",
     "@kubernetes/client-node": "0.10.1",
     "JSONStream": "^1.3.5",
     "analytics-node": "3.3.0",
@@ -60,7 +61,6 @@
     "ignore": "^5.1.1",
     "indent-string": "^4.0.0",
     "inquirer": "^6.3.1",
-    "joi": "^14.3.1",
     "js-yaml": "^3.13.1",
     "json-diff": "^0.5.4",
     "json-merge-patch": "^0.2.3",
@@ -120,9 +120,9 @@
     "@types/dockerode": "^2.5.16",
     "@types/execa": "^0.9.0",
     "@types/fs-extra": "^7.0.0",
+    "@types/hapi__joi": "^15.0.2",
     "@types/has-ansi": "^3.0.0",
     "@types/inquirer": "6.0.1",
-    "@types/joi": "^14.3.3",
     "@types/js-yaml": "^3.12.1",
     "@types/json-merge-patch": "0.0.4",
     "@types/json-stringify-safe": "^5.0.0",

diff --git a/garden-service/src/actions.ts b/garden-service/src/actions.ts
@@ -9,12 +9,11 @@
 import Bluebird = require("bluebird")
 
 import chalk from "chalk"
-import * as Joi from "joi"
 import { fromPairs, keyBy, mapValues, omit, pickBy, values } from "lodash"
 
 import { PublishModuleParams, PublishResult } from "./types/plugin/module/publishModule"
 import { SetSecretParams, SetSecretResult } from "./types/plugin/provider/setSecret"
-import { validate } from "./config/common"
+import { validate, joi } from "./config/common"
 import { defaultProvider, Provider } from "./config/provider"
 import { ConfigurationError, ParameterError, PluginError } from "./exceptions"
 import { ActionHandlerMap, Garden, ModuleActionHandlerMap, ModuleActionMap, PluginActionMap } from "./garden"
@@ -261,8 +260,8 @@ export class ActionHelper implements TypeGuard {
       moduleType,
       defaultHandler: async ({ }) => ({
         docs: "",
-        outputsSchema: Joi.object().options({ allowUnknown: true }),
-        schema: Joi.object().options({ allowUnknown: true }),
+        outputsSchema: joi.object().options({ allowUnknown: true }),
+        schema: joi.object().options({ allowUnknown: true }),
       }),
     })
 

diff --git a/garden-service/src/commands/base.ts b/garden-service/src/commands/base.ts
@@ -6,7 +6,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import Joi = require("joi")
+import Joi = require("@hapi/joi")
+import stripAnsi from "strip-ansi"
 import { GardenError, RuntimeError, InternalError, ParameterError } from "../exceptions"
 import { TaskResults } from "../task-graph"
 import { LoggerType } from "../logger/logger"
@@ -15,7 +16,7 @@ import { Garden } from "../garden"
 import { LogEntry } from "../logger/log-entry"
 import { printFooter } from "../logger/util"
 import { GlobalOptions } from "../cli/cli"
-import stripAnsi from "strip-ansi"
+import { joi } from "../config/common"
 
 export interface ParameterConstructor<T> {
   help: string,
@@ -75,7 +76,7 @@ export abstract class Parameter<T> {
 
 export class StringParameter extends Parameter<string> {
   type = "string"
-  schema = Joi.string()
+  schema = joi.string()
 
   parseString(input: string) {
     return input
@@ -86,7 +87,7 @@ export class StringParameter extends Parameter<string> {
 // FIXME: Maybe use a Required<Parameter> type to enforce presence, rather that an option flag?
 export class StringOption extends Parameter<string | undefined> {
   type = "string"
-  schema = Joi.string()
+  schema = joi.string()
 
   parseString(input?: string) {
     return input
@@ -99,7 +100,7 @@ export interface StringsConstructor extends ParameterConstructor<string[]> {
 
 export class StringsParameter extends Parameter<string[] | undefined> {
   type = "array:string"
-  schema = Joi.array().items(Joi.string())
+  schema = joi.array().items(joi.string())
   delimiter: string
 
   constructor(args: StringsConstructor) {
@@ -125,7 +126,7 @@ export class StringsParameter extends Parameter<string[] | undefined> {
 
 export class PathParameter extends Parameter<string> {
   type = "path"
-  schema = Joi.string().uri({ relativeOnly: true })
+  schema = joi.string().posixPath()
 
   parseString(input: string) {
     return input
@@ -134,7 +135,7 @@ export class PathParameter extends Parameter<string> {
 
 export class PathsParameter extends Parameter<string[]> {
   type = "array:path"
-  schema = Joi.array().items(Joi.string().uri({ relativeOnly: true }))
+  schema = joi.array().items(joi.string().posixPath())
 
   parseString(input: string) {
     return input.split(",")
@@ -143,7 +144,7 @@ export class PathsParameter extends Parameter<string[]> {
 
 export class IntegerParameter extends Parameter<number> {
   type = "number"
-  schema = Joi.number().integer()
+  schema = joi.number().integer()
 
   parseString(input: string) {
     try {
@@ -164,13 +165,13 @@ export interface ChoicesConstructor extends ParameterConstructor<string> {
 export class ChoicesParameter extends Parameter<string> {
   type = "choice"
   choices: string[]
-  schema = Joi.string()
+  schema = joi.string()
 
   constructor(args: ChoicesConstructor) {
     super(args)
 
     this.choices = args.choices
-    this.schema = Joi.string().only(args.choices)
+    this.schema = joi.string().only(args.choices)
   }
 
   parseString(input: string) {
@@ -191,7 +192,7 @@ export class ChoicesParameter extends Parameter<string> {
 
 export class BooleanParameter extends Parameter<boolean> {
   type = "boolean"
-  schema = Joi.boolean()
+  schema = joi.boolean()
 
   parseString(input: any) {
     return !!input

diff --git a/garden-service/src/config-store.ts b/garden-service/src/config-store.ts
@@ -6,13 +6,12 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import * as Joi from "joi"
 import * as yaml from "js-yaml"
 import { join } from "path"
 import { ensureFile, readFile } from "fs-extra"
 import { get, isPlainObject, unset } from "lodash"
 
-import { Primitive, validate, joiArray, joiUserIdentifier, joiPrimitive } from "./config/common"
+import { Primitive, validate, joiArray, joiUserIdentifier, joiPrimitive, joi } from "./config/common"
 import { LocalConfigError } from "./exceptions"
 import { dumpYaml } from "./util/util"
 import {
@@ -188,23 +187,23 @@ export interface LocalConfig {
   analytics: AnalyticsLocalConfig
 }
 
-const kubernetesLocalConfigSchema = Joi.object()
+const kubernetesLocalConfigSchema = joi.object()
   .keys({
     "username": joiUserIdentifier().allow("").optional(),
-    "previous-usernames": Joi.array().items(joiUserIdentifier()).optional(),
+    "previous-usernames": joi.array().items(joiUserIdentifier()).optional(),
   })
   .meta({ internal: true })
 
-const linkedSourceSchema = Joi.object()
+const linkedSourceSchema = joi.object()
   .keys({
     name: joiUserIdentifier(),
-    path: Joi.string(),
+    path: joi.string(),
   })
   .meta({ internal: true })
 
-const AnalyticsLocalConfigSchema = Joi.object()
+const AnalyticsLocalConfigSchema = joi.object()
   .keys({
-    projectId: Joi.string(),
+    projectId: joi.string(),
   }).meta({ internal: true })
 
 const localConfigSchemaKeys = {
@@ -219,12 +218,12 @@ export const localConfigKeys = Object.keys(localConfigSchemaKeys).reduce((acc, k
   return acc
 }, {}) as { [K in keyof typeof localConfigSchemaKeys]: K }
 
-const localConfigSchema = Joi.object()
+const localConfigSchema = joi.object()
   .keys(localConfigSchemaKeys)
   .meta({ internal: true })
 
 // TODO: we should not be passing this to provider actions
-export const configStoreSchema = Joi.object()
+export const configStoreSchema = joi.object()
   .description("Helper class for managing local configuration for plugins.")
 
 export class LocalConfigStore extends ConfigStore<LocalConfig> {
@@ -259,11 +258,11 @@ export interface GlobalConfig {
   analytics?: AnalyticsGlobalConfig
 }
 
-const AnalyticsGlobalConfigSchema = Joi.object()
+const AnalyticsGlobalConfigSchema = joi.object()
   .keys({
     userId: joiPrimitive().allow("").optional(),
-    optedIn: Joi.boolean().optional(),
-    firstRun: Joi.boolean().optional(),
+    optedIn: joi.boolean().optional(),
+    firstRun: joi.boolean().optional(),
   }).meta({ internal: true })
 
 const globalConfigSchemaKeys = {
@@ -284,7 +283,7 @@ export const globalConfigKeys = Object.keys(globalConfigSchemaKeys).reduce((acc,
   return acc
 }, {}) as { [K in keyof typeof globalConfigSchemaKeys]: K }
 
-const globalConfigSchema = Joi.object()
+const globalConfigSchema = joi.object()
   .keys(globalConfigSchemaKeys)
   .meta({ internal: true })