Merge pull request #338 from meienberger/fix/empty-storage-path-error

fix: empty storage path error

scripts/app.sh

@@ -16,14 +16,6 @@ ROOT_FOLDER_HOST=$(grep -v '^#' "${ENV_FILE}" | xargs -n 1 | grep ROOT_FOLDER_HO
 REPO_ID=$(grep -v '^#' "${ENV_FILE}" | xargs -n 1 | grep APPS_REPO_ID | cut -d '=' -f2)
 STORAGE_PATH=$(grep -v '^#' "${ENV_FILE}" | xargs -n 1 | grep STORAGE_PATH | cut -d '=' -f2)
 
-# Override vars with values from settings.json
-if [[ -f "${STATE_FOLDER}/settings.json" ]]; then
-  # If storagePath is set in settings.json, use it
-  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" storagePath)" != "null" ]]; then
-    STORAGE_PATH="$(get_json_field "${STATE_FOLDER}/settings.json" storagePath)"
-  fi
-fi
-
 write_log "Running app script: ROOT_FOLDER=${ROOT_FOLDER}, ROOT_FOLDER_HOST=${ROOT_FOLDER_HOST}, REPO_ID=${REPO_ID}, STORAGE_PATH=${STORAGE_PATH}"
 
 if [ -z ${1+x} ]; then

scripts/start-dev.sh

@@ -31,6 +31,7 @@ POSTGRES_HOST=tipi-db
 REDIS_HOST=tipi-redis
 TIPI_VERSION=$(get_json_field "${ROOT_FOLDER}/package.json" version)
 INTERNAL_IP=localhost
+DEMO_MODE=false
 storage_path="${ROOT_FOLDER}"
 STORAGE_PATH_ESCAPED="$(echo "${storage_path}" | sed 's/\//\\\//g')"
 if [[ "$ARCHITECTURE" == "aarch64" ]]; then
@@ -93,6 +94,47 @@ if [[ "$OS" == "Darwin" ]]; then
     sed_args=(-i '')
 fi
 
+if [[ -f "${STATE_FOLDER}/settings.json" ]]; then
+  # If dnsIp is set in settings.json, use it
+  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" dnsIp)" != "null" ]]; then
+    DNS_IP=$(get_json_field "${STATE_FOLDER}/settings.json" dnsIp)
+  fi
+
+  # If domain is set in settings.json, use it
+  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" domain)" != "null" ]]; then
+    DOMAIN=$(get_json_field "${STATE_FOLDER}/settings.json" domain)
+  fi
+
+  # If appsRepoUrl is set in settings.json, use it
+  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" appsRepoUrl)" != "null" ]]; then
+    apps_repository=$(get_json_field "${STATE_FOLDER}/settings.json" appsRepoUrl)
+    APPS_REPOSITORY_ESCAPED="$(echo "${apps_repository}" | sed 's/\//\\\//g')"
+    REPO_ID="$("${ROOT_FOLDER}"/scripts/git.sh get_hash "${apps_repository}")"
+  fi
+
+  # If port is set in settings.json, use it
+  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" port)" != "null" ]]; then
+    NGINX_PORT=$(get_json_field "${STATE_FOLDER}/settings.json" port)
+  fi
+
+  # If sslPort is set in settings.json, use it
+  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" sslPort)" != "null" ]]; then
+    NGINX_PORT_SSL=$(get_json_field "${STATE_FOLDER}/settings.json" sslPort)
+  fi
+
+  # If listenIp is set in settings.json, use it
+  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" listenIp)" != "null" ]]; then
+    INTERNAL_IP=$(get_json_field "${STATE_FOLDER}/settings.json" listenIp)
+  fi
+
+  # If storagePath is set in settings.json, use it
+  storage_path_settings=$(get_json_field "${STATE_FOLDER}/settings.json" storagePath)
+  if [[ "${storage_path_settings}" != "null" && "${storage_path_settings}" != "" ]]; then
+    storage_path="${storage_path_settings}"
+    STORAGE_PATH_ESCAPED="$(echo "${storage_path}" | sed 's/\//\\\//g')"
+  fi
+fi
+
 # Function below is modified from Umbrel
 # Required Notice: Copyright
 # Umbrel (https://umbrel.com)
@@ -116,6 +158,7 @@ for template in ${ENV_FILE}; do
     sed "${sed_args[@]}" "s/<postgres_port>/${POSTGRES_PORT}/g" "${template}"
     sed "${sed_args[@]}" "s/<postgres_host>/${POSTGRES_HOST}/g" "${template}"
     sed "${sed_args[@]}" "s/<redis_host>/${REDIS_HOST}/g" "${template}"
+    sed "${sed_args[@]}" "s/<demo_mode>/${DEMO_MODE}/g" "${template}"
 done
 
 mv -f "$ENV_FILE" "$ROOT_FOLDER/.env.dev"

scripts/start.sh

@@ -57,6 +57,7 @@ TIPI_VERSION=$(get_json_field "${ROOT_FOLDER}/package.json" version)
 storage_path="${ROOT_FOLDER}"
 STORAGE_PATH_ESCAPED="$(echo "${storage_path}" | sed 's/\//\\\//g')"
 REDIS_HOST=tipi-redis
+DEMO_MODE=false
 INTERNAL_IP=
 
 if [[ "$ARCHITECTURE" == "aarch64" ]] || [[ "$ARCHITECTURE" == "armv8"* ]]; then
@@ -78,6 +79,7 @@ while [ -n "${1-}" ]; do
   case "$1" in
   --rc) rc="true" ;;
   --ci) ci="true" ;;
+  --demo) DEMO_MODE=true ;;
   --port)
     port="${2-}"
 
@@ -228,8 +230,9 @@ if [[ -f "${STATE_FOLDER}/settings.json" ]]; then
   fi
 
   # If storagePath is set in settings.json, use it
-  if [[ "$(get_json_field "${STATE_FOLDER}/settings.json" storagePath)" != "null" ]]; then
-    storage_path="$(get_json_field "${STATE_FOLDER}/settings.json" storagePath)"
+  storage_path_settings=$(get_json_field "${STATE_FOLDER}/settings.json" storagePath)
+  if [[ "${storage_path_settings}" != "null" && "${storage_path_settings}" != "" ]]; then
+    storage_path="${storage_path_settings}"
     STORAGE_PATH_ESCAPED="$(echo "${storage_path}" | sed 's/\//\\\//g')"
   fi
 fi
@@ -268,6 +271,7 @@ for template in ${ENV_FILE}; do
   sed -i "s/<domain>/${DOMAIN}/g" "${template}"
   sed -i "s/<storage_path>/${STORAGE_PATH_ESCAPED}/g" "${template}"
   sed -i "s/<redis_host>/${REDIS_HOST}/g" "${template}"
+  sed -i "s/<demo_mode>/${DEMO_MODE}/g" "${template}"
 done
 
 mv -f "$ENV_FILE" "$ROOT_FOLDER/.env"

src/client/modules/Settings/components/SettingsForm/SettingsForm.tsx

@@ -97,7 +97,7 @@ export const SettingsForm = (props: IProps) => {
       </div>
       <div className="mb-3">
         <Input {...register('internalIp')} label="Internal IP" error={errors.internalIp?.message} placeholder="192.168.1.100" />
-        <span className="text-muted">IP address your server is listening on. Keep localhost for default</span>
+        <span className="text-muted">IP address your server is listening on.</span>
       </div>
       <div className="mb-3">
         <Input {...register('appsRepoUrl')} label="Apps repo URL" error={errors.appsRepoUrl?.message} placeholder="https://github.com/meienberger/runtipi-appstore" />
@@ -105,7 +105,7 @@ export const SettingsForm = (props: IProps) => {
       </div>
       <div className="mb-3">
         <Input {...register('storagePath')} label="Storage path" error={errors.storagePath?.message} placeholder="Storage path" />
-        <span className="text-muted">Path to the storage directory. Keep empty for default</span>
+        <span className="text-muted">Path to the storage directory. Keep empty for default (runtipi/app-data). Make sure it is an absolute path and that it exists</span>
       </div>
       <Button loading={loading} type="submit" className="btn-success">
         Save

src/server/core/TipiConfig/TipiConfig.test.ts

@@ -1,57 +1,87 @@
 import { faker } from '@faker-js/faker';
 import fs from 'fs-extra';
-import { getConfig, setConfig, TipiConfig } from '.';
+import { getConfig, setConfig, getSettings, setSettings, TipiConfig } from '.';
 import { readJsonFile } from '../../common/fs.helpers';
 
 beforeEach(async () => {
-  jest.resetModules();
-  jest.resetAllMocks();
   // @ts-expect-error - We are mocking fs
   fs.__resetAllMocks();
   jest.mock('fs-extra');
 });
 
+jest.mock('next/config', () =>
+  jest.fn(() => ({
+    serverRuntimeConfig: {
+      DNS_IP: '1.1.1.1',
+    },
+  })),
+);
+
+// eslint-disable-next-line
+import nextConfig from 'next/config';
+
+describe('Test: process.env', () => {
+  it('should return config from .env', () => {
+    const config = new TipiConfig().getConfig();
+
+    expect(config).toBeDefined();
+    expect(config.dnsIp).toBe('1.1.1.1');
+  });
+
+  it('should throw an error if there are invalid values', () => {
+    // @ts-expect-error - We are mocking next/config
+    nextConfig.mockImplementationOnce(() => ({
+      serverRuntimeConfig: {
+        DNS_IP: 'invalid',
+      },
+    }));
+
+    expect(() => new TipiConfig().getConfig()).toThrow();
+  });
+});
+
 describe('Test: getConfig', () => {
   it('It should return config from .env', () => {
+    // arrange
     const config = getConfig();
 
+    // assert
     expect(config).toBeDefined();
     expect(config.NODE_ENV).toBe('test');
-    expect(config.dnsIp).toBe('9.9.9.9');
     expect(config.rootFolder).toBe('/runtipi');
     expect(config.internalIp).toBe('localhost');
   });
 
   it('It should overrides config from settings.json file', () => {
+    // arrange
     const settingsJson = {
       appsRepoUrl: faker.internet.url(),
       appsRepoId: faker.random.word(),
       domain: faker.random.word(),
     };
-
     const MockFiles = {
       '/runtipi/state/settings.json': JSON.stringify(settingsJson),
     };
-
     // @ts-expect-error - We are mocking fs
     fs.__createMockFiles(MockFiles);
 
+    // act
     const config = new TipiConfig().getConfig();
 
+    // assert
     expect(config).toBeDefined();
-
     expect(config.appsRepoUrl).toBe(settingsJson.appsRepoUrl);
     expect(config.appsRepoId).toBe(settingsJson.appsRepoId);
     expect(config.domain).toBe(settingsJson.domain);
   });
 
   it('Should not be able to apply an invalid value from json config', () => {
+    // arrange
     const settingsJson = {
       appsRepoUrl: faker.random.word(),
       appsRepoId: faker.random.word(),
       domain: 10,
     };
-
     const MockFiles = {
       '/runtipi/state/settings.json': JSON.stringify(settingsJson),
     };
@@ -59,16 +89,21 @@ describe('Test: getConfig', () => {
     // @ts-expect-error - We are mocking fs
     fs.__createMockFiles(MockFiles);
 
+    // act & assert
     expect(() => new TipiConfig().getConfig()).toThrow();
   });
 });
 
 describe('Test: setConfig', () => {
   it('It should be able set config', () => {
+    // arrange
     const randomWord = faker.internet.url();
+
+    // act
     setConfig('appsRepoUrl', randomWord);
     const config = getConfig();
 
+    // assert
     expect(config).toBeDefined();
     expect(config.appsRepoUrl).toBe(randomWord);
   });
@@ -115,7 +150,7 @@ describe('Test: getSettings', () => {
     fs.__createMockFiles(MockFiles);
 
     // act
-    const settings = new TipiConfig().getSettings();
+    const settings = getSettings();
 
     // assert
     expect(settings).toBeDefined();
@@ -147,11 +182,10 @@ describe('Test: setSettings', () => {
     };
 
     // act
-    new TipiConfig().setSettings(fakeSettings);
-
-    // assert
+    setSettings(fakeSettings);
     const settingsJson = readJsonFile('/runtipi/state/settings.json') as { [key: string]: string };
 
+    // assert
     expect(settingsJson).toBeDefined();
     expect(settingsJson.appsRepoUrl).toBe(fakeSettings.appsRepoUrl);
   });
@@ -161,12 +195,77 @@ describe('Test: setSettings', () => {
     const fakeSettings = { appsRepoUrl: 10 };
 
     // act
-    new TipiConfig().setSettings(fakeSettings as object);
-
-    // assert
+    setSettings(fakeSettings as object);
     const settingsJson = (readJsonFile('/runtipi/state/settings.json') || {}) as { [key: string]: string };
 
+    // assert
     expect(settingsJson).toBeDefined();
     expect(settingsJson.appsRepoUrl).not.toBe(fakeSettings.appsRepoUrl);
   });
+
+  it('should throw and error if demo mode is enabled', async () => {
+    // arrange
+    let error;
+    const fakeSettings = { appsRepoUrl: faker.internet.url() };
+    const tipiConf = new TipiConfig();
+    tipiConf.setConfig('demoMode', true);
+
+    // act
+    try {
+      await tipiConf.setSettings(fakeSettings);
+    } catch (e) {
+      error = e;
+    }
+
+    // assert
+    expect(error).toBeDefined();
+  });
+
+  it('should replace empty string with undefined if storagePath is empty', async () => {
+    // arrange
+    const fakeSettings = { storagePath: '' };
+    const tipiConf = new TipiConfig();
+
+    // act
+    await tipiConf.setSettings(fakeSettings);
+
+    // assert
+    expect(tipiConf.getConfig().storagePath).toBeUndefined();
+  });
+
+  it('should trim storagePath if it is not empty', async () => {
+    // arrange
+    const fakeSettings = { storagePath: ' /tmp ' };
+    const tipiConf = new TipiConfig();
+
+    // act
+    await tipiConf.setSettings(fakeSettings);
+
+    // assert
+    expect(tipiConf.getConfig().storagePath).toBe('/tmp');
+  });
+
+  it('should trim storagePath and return undefined if it is empty', async () => {
+    // arrange
+    const fakeSettings = { storagePath: '   ' };
+    const tipiConf = new TipiConfig();
+
+    // act
+    await tipiConf.setSettings(fakeSettings);
+
+    // assert
+    expect(tipiConf.getConfig().storagePath).toBeUndefined();
+  });
+
+  it('should remove all whitespaces from storagePath', async () => {
+    // arrange
+    const fakeSettings = { storagePath: ' /tmp /test ' };
+    const tipiConf = new TipiConfig();
+
+    // act
+    await tipiConf.setSettings(fakeSettings);
+
+    // assert
+    expect(tipiConf.getConfig().storagePath).toBe('/tmp/test');
+  });
 });

src/server/core/TipiConfig/TipiConfig.ts

@@ -11,50 +11,47 @@ export const ARCHITECTURES = {
 } as const;
 export type Architecture = (typeof ARCHITECTURES)[keyof typeof ARCHITECTURES];
 
-const conf = { ...process.env, ...nextConfig()?.serverRuntimeConfig };
-const {
-  NODE_ENV,
-  JWT_SECRET,
-  INTERNAL_IP,
-  TIPI_VERSION,
-  APPS_REPO_ID,
-  APPS_REPO_URL,
-  DOMAIN,
-  REDIS_HOST,
-  STORAGE_PATH,
-  ARCHITECTURE = 'amd64',
-  POSTGRES_HOST,
-  POSTGRES_DBNAME,
-  POSTGRES_USERNAME,
-  POSTGRES_PASSWORD,
-  POSTGRES_PORT = 5432,
-} = conf;
-
-export const configSchema = z.object({
+const configSchema = z.object({
   NODE_ENV: z.union([z.literal('development'), z.literal('production'), z.literal('test')]),
   REDIS_HOST: z.string(),
   status: z.union([z.literal('RUNNING'), z.literal('UPDATING'), z.literal('RESTARTING')]),
   architecture: z.nativeEnum(ARCHITECTURES),
-  dnsIp: z.string().ip(),
+  dnsIp: z.string().ip().trim(),
   rootFolder: z.string(),
   internalIp: z.string(),
   version: z.string(),
   jwtSecret: z.string(),
   appsRepoId: z.string(),
-  appsRepoUrl: z.string().url(),
-  domain: z.string(),
-  storagePath: z.string().optional(),
+  appsRepoUrl: z.string().url().trim(),
+  domain: z.string().trim(),
+  storagePath: z
+    .string()
+    .trim()
+    .optional()
+    .transform((value) => {
+      if (!value) return undefined;
+      return value?.replace(/\s/g, '');
+    }),
   postgresHost: z.string(),
   postgresDatabase: z.string(),
   postgresUsername: z.string(),
   postgresPassword: z.string(),
   postgresPort: z.number(),
+  demoMode: z
+    .string()
+    .or(z.boolean())
+    .optional()
+    .transform((value) => {
+      if (typeof value === 'boolean') return value;
+      return value === 'true';
+    }),
 });
 
 export const settingsSchema = configSchema.partial().pick({ dnsIp: true, internalIp: true, appsRepoUrl: true, domain: true, storagePath: true });
+
 export type TipiSettingsType = z.infer<typeof settingsSchema>;
 
-export const formatErrors = (errors: { fieldErrors: Record<string, string[]> }) =>
+const formatErrors = (errors: { fieldErrors: Record<string, string[]> }) =>
   Object.entries(errors.fieldErrors)
     .map(([name, value]) => `${name}: ${value[0]}`)
     .filter(Boolean)
@@ -66,25 +63,27 @@ export class TipiConfig {
   private config: z.infer<typeof configSchema>;
 
   constructor() {
+    const conf = { ...process.env, ...nextConfig()?.serverRuntimeConfig };
     const envConfig: z.infer<typeof configSchema> = {
-      postgresHost: POSTGRES_HOST,
-      postgresDatabase: POSTGRES_DBNAME,
-      postgresUsername: POSTGRES_USERNAME,
-      postgresPassword: POSTGRES_PASSWORD,
-      postgresPort: Number(POSTGRES_PORT),
-      REDIS_HOST,
-      NODE_ENV,
-      architecture: ARCHITECTURE as z.infer<typeof configSchema>['architecture'],
+      postgresHost: conf.POSTGRES_HOST,
+      postgresDatabase: conf.POSTGRES_DBNAME,
+      postgresUsername: conf.POSTGRES_USERNAME,
+      postgresPassword: conf.POSTGRES_PASSWORD,
+      postgresPort: Number(conf.POSTGRES_PORT || 5432),
+      REDIS_HOST: conf.REDIS_HOST,
+      NODE_ENV: conf.NODE_ENV,
+      architecture: conf.ARCHITECTURE || 'amd64',
       rootFolder: '/runtipi',
-      internalIp: INTERNAL_IP,
-      version: TIPI_VERSION,
-      jwtSecret: JWT_SECRET,
-      appsRepoId: APPS_REPO_ID,
-      appsRepoUrl: APPS_REPO_URL,
-      domain: DOMAIN,
-      dnsIp: '9.9.9.9',
+      internalIp: conf.INTERNAL_IP,
+      version: conf.TIPI_VERSION,
+      jwtSecret: conf.JWT_SECRET,
+      appsRepoId: conf.APPS_REPO_ID,
+      appsRepoUrl: conf.APPS_REPO_URL,
+      domain: conf.DOMAIN,
+      dnsIp: conf.DNS_IP || '9.9.9.9',
       status: 'RUNNING',
-      storagePath: STORAGE_PATH,
+      storagePath: conf.STORAGE_PATH,
+      demoMode: conf.DEMO_MODE,
     };
 
     const fileConfig = readJsonFile('/runtipi/state/settings.json') || {};
@@ -149,6 +148,10 @@ export class TipiConfig {
   }
 
   public async setSettings(settings: TipiSettingsType) {
+    if (this.config.demoMode) {
+      throw new Error('Cannot update settings in demo mode');
+    }
+
     const newConf: z.infer<typeof configSchema> = { ...this.getConfig() };
     const parsed = settingsSchema.safeParse(settings);
 

src/server/services/system/system.service.test.ts

@@ -122,6 +122,14 @@ describe('Test: restart', () => {
     // Assert
     expect(restart).toBeTruthy();
   });
+
+  it('should throw an error in demo mode', async () => {
+    // Arrange
+    await setConfig('demoMode', true);
+
+    // Act & Assert
+    await expect(SystemService.restart()).rejects.toThrow('Cannot restart in demo mode');
+  });
 });
 
 describe('Test: update', () => {

src/server/services/system/system.service.ts

@@ -105,6 +105,10 @@ export class SystemServiceClass {
       throw new Error('Cannot restart in development mode');
     }
 
+    if (TipiConfig.getConfig().demoMode) {
+      throw new Error('Cannot restart in demo mode');
+    }
+
     TipiConfig.setConfig('status', 'RESTARTING');
     this.dispatcher.dispatchEventAsync('restart');
 

templates/env-sample

@@ -20,3 +20,4 @@ POSTGRES_USERNAME=<postgres_username>
 POSTGRES_PASSWORD=<postgres_password>
 POSTGRES_PORT=<postgres_port>
 REDIS_HOST=<redis_host>
+DEMO_MODE=<demo_mode>