diff --git a/script/command-executor.ts b/script/command-executor.ts index 04ade02..a050034 100644 --- a/script/command-executor.ts +++ b/script/command-executor.ts @@ -80,6 +80,12 @@ let connectionInfo: ILoginConnectionInfo; export const confirm = (message: string = "Are you sure?"): Promise => { message += " (y/N):"; return Promise((resolve, reject, notify): void => { + if (!process.stdin.isTTY) { + log(chalk.cyan(message) + " no (not a terminal, so nothing was asked). Nothing was changed."); + resolve(false); + return; + } + prompt.message = ""; prompt.delimiter = ""; @@ -94,6 +100,10 @@ export const confirm = (message: string = "Are you sure?"): Promise => }, }, (err: any, result: any): void => { + if (err || !result) { + resolve(false); + return; + } const accepted = result.response && result.response.toLowerCase() === "y"; const rejected = !result.response || result.response.toLowerCase() === "n"; @@ -289,9 +299,10 @@ function deleteConnectionInfoCache(printMessage: boolean = true): void { } catch (ex) {} } -function deleteFolder(folderPath: string): Promise { +/** rimraf 4+ needs `glob` to expand a pattern. Off by default so a literal path like "build[1]" is safe. */ +export function deleteFolder(folderPath: string, glob: boolean = false): Promise { return Q.Promise((resolve, reject) => { - rimraf(folderPath).then(() => resolve(null)).catch(reject); + rimraf(folderPath, { glob }).then(() => resolve(null)).catch(reject); }); } @@ -1361,7 +1372,7 @@ export const releaseReact = (command: cli.IReleaseReactCommand): Promise = }) // This is needed to clear the react native bundler cache: // https://github.com/facebook/react-native/issues/4289 - .then(() => deleteFolder(`${os.tmpdir()}/react-*`)) + .then(() => deleteFolder(`${os.tmpdir()}/react-*`, /*glob*/ true)) .then(() => runReactNativeBundleCommand( bundleName, diff --git a/script/command-parser.ts b/script/command-parser.ts index 029a06a..2101af3 100644 --- a/script/command-parser.ts +++ b/script/command-parser.ts @@ -357,6 +357,8 @@ yargs addCommonConfiguration(yargs); }) .command("transfer", "Transfer the ownership of an app to another account", (yargs: yargs.Argv) => { + // Required, or the category check below rejects every invocation. + isValidCommand = true; yargs .usage(USAGE_PREFIX + " app transfer ") .demand(/*count*/ 2, /*max*/ 2) // Require exactly two non-option arguments diff --git a/script/management-sdk.ts b/script/management-sdk.ts index 5f84c97..375e71d 100644 --- a/script/management-sdk.ts +++ b/script/management-sdk.ts @@ -298,13 +298,13 @@ class AccountManager { public getAutoRollbackConfig(appName: string, deploymentName: string): Promise { return this.get(urlEncode([`/apps/${appName}/deployments/${deploymentName}/auto-rollback`])).then( - (res: JsonResponse) => res.body.autoRollback + (res: JsonResponse) => res.body.autoRollbackConfig ); } public setAutoRollbackConfig(appName: string, deploymentName: string, config: { enabled: boolean; errorRateThreshold: number; minDevices: number }): Promise { return this.put(urlEncode([`/apps/${appName}/deployments/${deploymentName}/auto-rollback`]), JSON.stringify(config)).then( - (res: JsonResponse) => res.body.autoRollback + (res: JsonResponse) => res.body.autoRollbackConfig ); } diff --git a/test/command-parser.ts b/test/command-parser.ts index 0457e94..c46cf8a 100644 --- a/test/command-parser.ts +++ b/test/command-parser.ts @@ -125,3 +125,68 @@ describe("command line", function () { // Two dependencies of the destructive-command path, kept honest here because both broke silently once. // --------------------------------------------------------------------------- + +describe("confirm on a non-terminal", () => { + const cmdexec = require("../script/command-executor"); + + it("declines instead of hanging when nobody can answer", function (done: Mocha.Done) { + // Forced, not inherited: stdin is piped under CI but a terminal under `npm test`. + this.timeout(5000); + const descriptor = Object.getOwnPropertyDescriptor(process.stdin, "isTTY"); + Object.defineProperty(process.stdin, "isTTY", { value: false, configurable: true }); + const restore = (): void => { + if (descriptor) Object.defineProperty(process.stdin, "isTTY", descriptor); + else delete (process.stdin as any).isTTY; + }; + + let settled = false; + const timer = setTimeout(() => { + if (!settled) { + restore(); + done(new Error("confirm never settled: it is hanging again")); + } + }, 3000); + + cmdexec.confirm("Delete it?").then( + (answer: boolean) => { + settled = true; + clearTimeout(timer); + restore(); + assert.strictEqual(answer, false, "an unanswered destructive question is a no"); + done(); + }, + (error: any) => { + restore(); + done(error); + } + ); + }); +}); + +describe("deleteFolder", () => { + const cmdexec = require("../script/command-executor"); + + it("only expands a pattern when asked to", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "dpctl-glob-test-")); + const pattern = path.join(os.tmpdir(), "dpctl-glob-test-*"); + + await cmdexec.deleteFolder(pattern); + assert.strictEqual(fs.existsSync(dir), true, "without glob a pattern is a literal path, so nothing matches"); + + await cmdexec.deleteFolder(pattern, true); + assert.strictEqual(fs.existsSync(dir), false, "the glob flag is what actually deletes"); + }); + + it("leaves glob off by default, so a literal path with metacharacters is safe", async () => { + // An --outputDir like "build[1]" is a real directory name, not a pattern. + const parent = fs.mkdtempSync(path.join(os.tmpdir(), "dpctl-literal-")); + const literal = path.join(parent, "build[1]"); + fs.mkdirSync(literal); + fs.writeFileSync(path.join(literal, "keep.txt"), "x"); + + await cmdexec.deleteFolder(literal); + assert.strictEqual(fs.existsSync(literal), false, "the literal directory itself is removed"); + + fs.rmSync(parent, { recursive: true, force: true }); + }); +}); diff --git a/test/management-sdk.ts b/test/management-sdk.ts index 71bff05..452fe0a 100644 --- a/test/management-sdk.ts +++ b/test/management-sdk.ts @@ -403,6 +403,16 @@ describe("Management SDK", () => { (error: Error) => done() ); }); + + it("getAutoRollbackConfig reads autoRollbackConfig, the key the API actually sends", (done: Mocha.Done) => { + mockReturn(JSON.stringify({ autoRollbackConfig: { enabled: true, threshold: 25 } }), 200); + manager.getAutoRollbackConfig("appName", "Staging").done((config: any) => { + assert.ok(config, "expected the config, not undefined"); + assert.strictEqual(config.enabled, true); + assert.strictEqual(config.threshold, 25); + done(); + }, rejectHandler); + }); }); // Helper method that is used everywhere that an assert.fail() is needed in a promise handler