From 50835e9832272379a1d20e9604f0ac5f35ac326d Mon Sep 17 00:00:00 2001 From: Philipp Sonntag Date: Fri, 13 Nov 2020 16:30:12 +0100 Subject: [PATCH 1/5] Added cli tool validation for serve command. --- packages/@ionic/cli/src/lib/errors.ts | 2 ++ packages/@ionic/cli/src/lib/serve.ts | 33 +++++++++++++++++++++------ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/@ionic/cli/src/lib/errors.ts b/packages/@ionic/cli/src/lib/errors.ts index 20adb39388..74838fcbf8 100644 --- a/packages/@ionic/cli/src/lib/errors.ts +++ b/packages/@ionic/cli/src/lib/errors.ts @@ -14,6 +14,8 @@ export class FatalException extends BaseException { export class BuildCLIProgramNotFoundException extends BaseException {} +export class ServeCLIArgumentsException extends BaseException {} + export class ServeCLIProgramNotFoundException extends BaseException {} export class SessionException extends BaseException {} diff --git a/packages/@ionic/cli/src/lib/serve.ts b/packages/@ionic/cli/src/lib/serve.ts index bd7c91e468..679a862e6e 100644 --- a/packages/@ionic/cli/src/lib/serve.ts +++ b/packages/@ionic/cli/src/lib/serve.ts @@ -14,7 +14,7 @@ import * as stream from 'stream'; import { CommandLineInputs, CommandLineOptions, CommandMetadata, CommandMetadataOption, IConfig, ILogger, IProject, IShell, IonicEnvironmentFlags, LabServeDetails, NpmClient, Runner, ServeDetails, ServeOptions } from '../definitions'; import { ancillary, input, strong, weak } from './color'; -import { FatalException, ServeCLIProgramNotFoundException } from './errors'; +import { FatalException, ServeCLIArgumentsException, ServeCLIProgramNotFoundException } from './errors'; import { emit } from './events'; import { Hook } from './hooks'; import { openUrl } from './open'; @@ -496,14 +496,33 @@ export abstract class ServeCLI extends EventEmitter { const p = await this.e.shell.spawn(this.resolvedProgram, args, { stdio: 'pipe', cwd: this.e.project.directory, env: createProcessEnv(env) }); return new Promise((resolve, reject) => { - const errorHandler = (err: NodeJS.ErrnoException) => { - debug('received error for %s: %o', this.resolvedProgram, err); + const errorHandler = async (rootErr: NodeJS.ErrnoException) => { + debug('received error for %s: %o', this.resolvedProgram, rootErr); + + if (this.resolvedProgram === this.program) { + p.removeListener('close', closeHandler); // Do not close the cli until the error is specified. + + // Try to validate the cli tool by executing it. + const validationProcess = await this.e.shell.spawn(this.resolvedProgram, [], { showCommand: false }); + validationProcess.on('error', (validationError: NodeJS.ErrnoException) => { + // Validation also throws an error --> normal error handling with rootErr. + if (validationError.code === 'ENOENT') { + reject(new ServeCLIProgramNotFoundException(`${strong(this.resolvedProgram)} command not found.`)); + } else { + reject(rootErr); + } + }); + validationProcess.on('close', (code: number) => { + if (code == 0) { + // Validation closes successfully with code 0 --> error within arguments. + reject(new ServeCLIArgumentsException(`${strong(this.resolvedProgram)} error within arguments.`)); + } else { + reject(rootErr); + } + }); - if (this.resolvedProgram === this.program && err.code === 'ENOENT') { - p.removeListener('close', closeHandler); // do not exit Ionic CLI, we can gracefully ask to install this CLI - reject(new ServeCLIProgramNotFoundException(`${strong(this.resolvedProgram)} command not found.`)); } else { - reject(err); + reject(rootErr); } }; From 8b4b3a63ccdac5586da367f6ef4a9ac466e94915 Mon Sep 17 00:00:00 2001 From: Philipp Sonntag Date: Fri, 13 Nov 2020 16:47:09 +0100 Subject: [PATCH 2/5] Added new Exception and better handling of cli errors. --- packages/@ionic/cli/src/lib/errors.ts | 2 +- packages/@ionic/cli/src/lib/serve.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/@ionic/cli/src/lib/errors.ts b/packages/@ionic/cli/src/lib/errors.ts index 74838fcbf8..a117017014 100644 --- a/packages/@ionic/cli/src/lib/errors.ts +++ b/packages/@ionic/cli/src/lib/errors.ts @@ -14,7 +14,7 @@ export class FatalException extends BaseException { export class BuildCLIProgramNotFoundException extends BaseException {} -export class ServeCLIArgumentsException extends BaseException {} +export class ServeCLIUndefinedException extends BaseException {} export class ServeCLIProgramNotFoundException extends BaseException {} diff --git a/packages/@ionic/cli/src/lib/serve.ts b/packages/@ionic/cli/src/lib/serve.ts index 679a862e6e..27bf44dfc3 100644 --- a/packages/@ionic/cli/src/lib/serve.ts +++ b/packages/@ionic/cli/src/lib/serve.ts @@ -14,7 +14,7 @@ import * as stream from 'stream'; import { CommandLineInputs, CommandLineOptions, CommandMetadata, CommandMetadataOption, IConfig, ILogger, IProject, IShell, IonicEnvironmentFlags, LabServeDetails, NpmClient, Runner, ServeDetails, ServeOptions } from '../definitions'; import { ancillary, input, strong, weak } from './color'; -import { FatalException, ServeCLIArgumentsException, ServeCLIProgramNotFoundException } from './errors'; +import { FatalException, ServeCLIUndefinedException, ServeCLIProgramNotFoundException } from './errors'; import { emit } from './events'; import { Hook } from './hooks'; import { openUrl } from './open'; @@ -467,12 +467,13 @@ export abstract class ServeCLI extends EventEmitter { if (!(e instanceof ServeCLIProgramNotFoundException)) { throw e; } - + if (this.global) { this.e.log.nl(); throw new FatalException(`${input(this.pkg)} is required for this command to work properly.`); } + this.e.log.nl(); this.e.log.info( `Looks like ${input(this.pkg)} isn't installed in this project.\n` + @@ -515,7 +516,7 @@ export abstract class ServeCLI extends EventEmitter { validationProcess.on('close', (code: number) => { if (code == 0) { // Validation closes successfully with code 0 --> error within arguments. - reject(new ServeCLIArgumentsException(`${strong(this.resolvedProgram)} error within arguments.`)); + reject(new ServeCLIUndefinedException(`${input(this.pkg)} was found. Error while executing command.`)); } else { reject(rootErr); } From 1268f2fbc4189e4617df1596774e76897fedc62c Mon Sep 17 00:00:00 2001 From: Philipp Sonntag Date: Sat, 14 Nov 2020 16:02:08 +0100 Subject: [PATCH 3/5] Revert of exisiting changes after new feedback. --- packages/@ionic/cli/src/lib/errors.ts | 2 -- packages/@ionic/cli/src/lib/serve.ts | 36 ++++++--------------------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/packages/@ionic/cli/src/lib/errors.ts b/packages/@ionic/cli/src/lib/errors.ts index a117017014..20adb39388 100644 --- a/packages/@ionic/cli/src/lib/errors.ts +++ b/packages/@ionic/cli/src/lib/errors.ts @@ -14,8 +14,6 @@ export class FatalException extends BaseException { export class BuildCLIProgramNotFoundException extends BaseException {} -export class ServeCLIUndefinedException extends BaseException {} - export class ServeCLIProgramNotFoundException extends BaseException {} export class SessionException extends BaseException {} diff --git a/packages/@ionic/cli/src/lib/serve.ts b/packages/@ionic/cli/src/lib/serve.ts index 27bf44dfc3..bd7c91e468 100644 --- a/packages/@ionic/cli/src/lib/serve.ts +++ b/packages/@ionic/cli/src/lib/serve.ts @@ -14,7 +14,7 @@ import * as stream from 'stream'; import { CommandLineInputs, CommandLineOptions, CommandMetadata, CommandMetadataOption, IConfig, ILogger, IProject, IShell, IonicEnvironmentFlags, LabServeDetails, NpmClient, Runner, ServeDetails, ServeOptions } from '../definitions'; import { ancillary, input, strong, weak } from './color'; -import { FatalException, ServeCLIUndefinedException, ServeCLIProgramNotFoundException } from './errors'; +import { FatalException, ServeCLIProgramNotFoundException } from './errors'; import { emit } from './events'; import { Hook } from './hooks'; import { openUrl } from './open'; @@ -467,13 +467,12 @@ export abstract class ServeCLI extends EventEmitter { if (!(e instanceof ServeCLIProgramNotFoundException)) { throw e; } - + if (this.global) { this.e.log.nl(); throw new FatalException(`${input(this.pkg)} is required for this command to work properly.`); } - this.e.log.nl(); this.e.log.info( `Looks like ${input(this.pkg)} isn't installed in this project.\n` + @@ -497,33 +496,14 @@ export abstract class ServeCLI extends EventEmitter { const p = await this.e.shell.spawn(this.resolvedProgram, args, { stdio: 'pipe', cwd: this.e.project.directory, env: createProcessEnv(env) }); return new Promise((resolve, reject) => { - const errorHandler = async (rootErr: NodeJS.ErrnoException) => { - debug('received error for %s: %o', this.resolvedProgram, rootErr); - - if (this.resolvedProgram === this.program) { - p.removeListener('close', closeHandler); // Do not close the cli until the error is specified. - - // Try to validate the cli tool by executing it. - const validationProcess = await this.e.shell.spawn(this.resolvedProgram, [], { showCommand: false }); - validationProcess.on('error', (validationError: NodeJS.ErrnoException) => { - // Validation also throws an error --> normal error handling with rootErr. - if (validationError.code === 'ENOENT') { - reject(new ServeCLIProgramNotFoundException(`${strong(this.resolvedProgram)} command not found.`)); - } else { - reject(rootErr); - } - }); - validationProcess.on('close', (code: number) => { - if (code == 0) { - // Validation closes successfully with code 0 --> error within arguments. - reject(new ServeCLIUndefinedException(`${input(this.pkg)} was found. Error while executing command.`)); - } else { - reject(rootErr); - } - }); + const errorHandler = (err: NodeJS.ErrnoException) => { + debug('received error for %s: %o', this.resolvedProgram, err); + if (this.resolvedProgram === this.program && err.code === 'ENOENT') { + p.removeListener('close', closeHandler); // do not exit Ionic CLI, we can gracefully ask to install this CLI + reject(new ServeCLIProgramNotFoundException(`${strong(this.resolvedProgram)} command not found.`)); } else { - reject(rootErr); + reject(err); } }; From 9620abb19e2505201e0bcbe01b471ba879010ae3 Mon Sep 17 00:00:00 2001 From: Philipp Sonntag Date: Sat, 21 Nov 2020 20:21:19 +0100 Subject: [PATCH 4/5] Added exception when directory path is not accessible. --- packages/@ionic/cli/src/lib/errors.ts | 2 ++ packages/@ionic/cli/src/lib/project/index.ts | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/@ionic/cli/src/lib/errors.ts b/packages/@ionic/cli/src/lib/errors.ts index 20adb39388..69f0582f20 100644 --- a/packages/@ionic/cli/src/lib/errors.ts +++ b/packages/@ionic/cli/src/lib/errors.ts @@ -16,6 +16,8 @@ export class BuildCLIProgramNotFoundException extends BaseException {} export class ServeCLIProgramNotFoundException extends BaseException {} +export class DirectoryNotAccessibleException extends FatalException {} + export class SessionException extends BaseException {} export class RunnerException extends BaseException {} diff --git a/packages/@ionic/cli/src/lib/project/index.ts b/packages/@ionic/cli/src/lib/project/index.ts index 522c1b3b8b..96527929ed 100644 --- a/packages/@ionic/cli/src/lib/project/index.ts +++ b/packages/@ionic/cli/src/lib/project/index.ts @@ -2,7 +2,7 @@ import { BaseConfig, BaseConfigOptions, PackageJson, ParsedArgs } from '@ionic/c import { PromptModule } from '@ionic/cli-framework-prompts'; import { resolveValue } from '@ionic/cli-framework/utils/fn'; import { ERROR_INVALID_PACKAGE_JSON, compileNodeModulesPaths, isValidPackageName, readPackageJsonFile } from '@ionic/cli-framework/utils/node'; -import { ensureDir, findBaseDirectory, readFile, writeFile, writeJson } from '@ionic/utils-fs'; +import { ensureDir, findBaseDirectory, pathExistsSync, readFile, writeFile, writeJson } from '@ionic/utils-fs'; import { TTY_WIDTH, prettyPath, wordWrap } from '@ionic/utils-terminal'; import * as Debug from 'debug'; import * as lodash from 'lodash'; @@ -12,7 +12,7 @@ import { PROJECT_FILE, PROJECT_TYPES } from '../../constants'; import { IAilmentRegistry, IClient, IConfig, IIntegration, ILogger, IMultiProjectConfig, IProject, IProjectConfig, ISession, IShell, InfoItem, IntegrationName, IonicContext, IonicEnvironmentFlags, ProjectIntegration, ProjectPersonalizationDetails, ProjectType } from '../../definitions'; import { isMultiProjectConfig, isProjectConfig } from '../../guards'; import { ancillary, failure, input, strong } from '../color'; -import { BaseException, FatalException, IntegrationNotFoundException, RunnerNotFoundException } from '../errors'; +import { BaseException, DirectoryNotAccessibleException, FatalException, IntegrationNotFoundException, RunnerNotFoundException } from '../errors'; import { BaseIntegration } from '../integrations'; import { CAPACITOR_CONFIG_FILE, CapacitorConfig } from '../integrations/capacitor/config'; import { Color } from '../utils/color'; @@ -420,7 +420,10 @@ export abstract class Project implements IProject { readonly details: ProjectDetailsResult, protected readonly e: ProjectDeps ) { - this.rootDirectory = path.dirname(details.configPath); + this.rootDirectory = path.dirname(details.configPath); + if (!pathExistsSync(this.rootDirectory)) { + throw new DirectoryNotAccessibleException(`Path ${input(this.rootDirectory)} is not aschessible.`); + } } get filePath(): string { @@ -434,7 +437,13 @@ export abstract class Project implements IProject { return this.rootDirectory; } - return path.resolve(this.rootDirectory, root); + const resolvedPath = path.resolve(this.rootDirectory, root); + + if (!pathExistsSync(resolvedPath)) { + throw new DirectoryNotAccessibleException(`Path ${input(resolvedPath)} is not accessible.`); + } + + return resolvedPath; } get pathPrefix(): string[] { From e473d8f416bd8d162a82a00f1bc79a80bfbac104 Mon Sep 17 00:00:00 2001 From: Philipp Sonntag Date: Sat, 21 Nov 2020 20:40:08 +0100 Subject: [PATCH 5/5] Added path validations for integrations. --- packages/@ionic/cli/src/lib/project/index.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/@ionic/cli/src/lib/project/index.ts b/packages/@ionic/cli/src/lib/project/index.ts index 96527929ed..bce4cf34c9 100644 --- a/packages/@ionic/cli/src/lib/project/index.ts +++ b/packages/@ionic/cli/src/lib/project/index.ts @@ -440,7 +440,7 @@ export abstract class Project implements IProject { const resolvedPath = path.resolve(this.rootDirectory, root); if (!pathExistsSync(resolvedPath)) { - throw new DirectoryNotAccessibleException(`Path ${input(resolvedPath)} is not accessible.`); + throw new DirectoryNotAccessibleException(`Project root ${input(resolvedPath)} is not accessible.`); } return resolvedPath; @@ -742,9 +742,17 @@ export abstract class Project implements IProject { const integration = this.config.get('integrations')[name]; if (integration) { + let rootDirectory = this.directory; + + if (integration.root) { + rootDirectory = path.resolve(this.rootDirectory, integration.root); + if (!pathExistsSync(rootDirectory)) { + throw new DirectoryNotAccessibleException(`Integration root ${input(rootDirectory)} is not accessible.`); + } + } return { enabled: integration.enabled !== false, - root: integration.root === undefined ? this.directory : path.resolve(this.rootDirectory, integration.root), + root: rootDirectory, }; } }