diff --git a/.mocharc.yml b/.mocharc.yml index f69495a0..54eca61d 100644 --- a/.mocharc.yml +++ b/.mocharc.yml @@ -1,3 +1,4 @@ recursive: true timeout: '10000' -reporter: 'spec' \ No newline at end of file +reporter: 'spec' +require: 'test/helpers/isolate-environment.js' diff --git a/test/commands/bulk.test.js b/test/commands/bulk.test.js index fce77a08..28f9173e 100644 --- a/test/commands/bulk.test.js +++ b/test/commands/bulk.test.js @@ -32,11 +32,8 @@ describe('Bulk', function () { '..', 'fixtures/bulk/input.csv' ), - saveFilePath = path.join( - __dirname, - '..', - 'fixtures/bulk/saveTest.txt' - ), + // Inside the temp home, so the test does not overwrite a tracked file + saveFilePath = path.join(os.homedir(), 'saveTest.txt'), csvOutput = getFixture('bulk/post_collaborations_csv.csv'), multipleFlagValuesInputFilePath = path.join( __dirname, @@ -198,6 +195,10 @@ describe('Bulk', function () { .post('/2.0/collaborations', addCollaborationBody3) .reply(200, addCollaborationFixture3) ) + .do(() => { + // Existing file, to check that -y overwrites it without asking + fs.writeFileSync(saveFilePath, 'foo', 'utf8'); + }) .stdout() .stderr() .command([ @@ -728,11 +729,8 @@ describe('Bulk', function () { __dirname, '../fixtures/bulk/input_metadata_update.json' ), - saveFilePath = path.join( - __dirname, - '..', - 'fixtures/bulk/saveTest.txt' - ); + // Inside the temp home, so the test does not overwrite a tracked file + saveFilePath = path.join(os.homedir(), 'saveTest.txt'); let addCollaborationBody1 = { item: { @@ -935,6 +933,10 @@ describe('Bulk', function () { .post('/2.0/collaborations', addCollaborationBody3) .reply(200, addCollaborationFixture3) ) + .do(() => { + // Existing file, to check that -y overwrites it without asking + fs.writeFileSync(saveFilePath, 'foo', 'utf8'); + }) .stdout() .stderr() .command([ diff --git a/test/commands/files.test.js b/test/commands/files.test.js index c3246682..caae86c9 100644 --- a/test/commands/files.test.js +++ b/test/commands/files.test.js @@ -2202,6 +2202,7 @@ describe('Files', function () { }) ) .do(() => { + fs.mkdirSync(DEFAULT_DOWNLOAD_PATH, { recursive: true }); fs.writeFileSync( path.join(DEFAULT_DOWNLOAD_PATH, saveAsFileName), 'foo', @@ -2235,6 +2236,7 @@ describe('Files', function () { api.get(`/2.0/files/${fileId}`).reply(200, getFileFixture) ) .do(() => { + fs.mkdirSync(DEFAULT_DOWNLOAD_PATH, { recursive: true }); fs.writeFileSync( path.join(DEFAULT_DOWNLOAD_PATH, saveAsFileName), 'foo', @@ -2552,6 +2554,7 @@ describe('Files', function () { }) ) .do(() => { + fs.mkdirSync(DEFAULT_DOWNLOAD_PATH, { recursive: true }); fs.writeFileSync( path.join(DEFAULT_DOWNLOAD_PATH, saveAsFileName), 'foo', @@ -2586,6 +2589,7 @@ describe('Files', function () { api.get(`/2.0/files/${fileId}`).reply(200, getFileFixture) ) .do(() => { + fs.mkdirSync(DEFAULT_DOWNLOAD_PATH, { recursive: true }); fs.writeFileSync( path.join(DEFAULT_DOWNLOAD_PATH, saveAsFileName), 'foo', @@ -2767,6 +2771,7 @@ describe('Files', function () { api.get(statusUrl).reply(200, downloadStatusFixture) ) .do(async () => { + fs.mkdirSync(DEFAULT_DOWNLOAD_PATH, { recursive: true }); await fs.writeFileSync( path.join(DEFAULT_DOWNLOAD_PATH, fileName), 'foo' @@ -2798,6 +2803,7 @@ describe('Files', function () { } ); test.do(async () => { + fs.mkdirSync(DEFAULT_DOWNLOAD_PATH, { recursive: true }); await fs.writeFileSync( path.join(DEFAULT_DOWNLOAD_PATH, fileName), 'foo' diff --git a/test/fixtures/bulk/saveTest.txt b/test/fixtures/bulk/saveTest.txt deleted file mode 100644 index 19102815..00000000 --- a/test/fixtures/bulk/saveTest.txt +++ /dev/null @@ -1 +0,0 @@ -foo \ No newline at end of file diff --git a/test/helpers/isolate-environment.js b/test/helpers/isolate-environment.js new file mode 100644 index 00000000..9c1c5b49 --- /dev/null +++ b/test/helpers/isolate-environment.js @@ -0,0 +1,153 @@ +'use strict'; + +/** + * Test setup, not a test. Mocha loads this file once, before any test file + * (see `require` in .mocharc.yml). It makes sure tests never read or change + * the developer's real CLI configuration, keychain entries, or files. + * + * 1. Temporary home directory + * + * HOME (USERPROFILE on Windows) is pointed at a new temp directory. Every + * path the CLI builds from the home directory, such as `~/.box` (settings, + * environments, token caches) and `~/Downloads/Box-Downloads`, now ends up + * there instead of in the real home directory. + * + * This has to run before any file from src/ is loaded, because + * src/box-command.js reads os.homedir() once, when it is loaded. The check + * below throws if that already happened. + * + * Before each test the temp home is emptied and an empty `~/.box` is + * created again (the CLI creates that folder at startup and other code + * expects it). This way a file written by one test is never seen by the + * next one. The whole directory is deleted when the test run ends. + * + * 2. In-memory keychain + * + * src/secure-storage.js stores secrets in the system keychain through one + * of two npm modules: + * - macOS: `keychain` (macOS Keychain) + * - Windows and Linux: `@github/keytar` (Windows Credential Manager, + * or libsecret on Linux) + * Their getPassword/setPassword/deletePassword functions are replaced with + * versions that use a plain Map, cleared before each test. If keytar cannot + * be loaded (for example on Linux without libsecret), nothing is replaced; + * the CLI then stores secrets in files, which are in the temp home anyway. + * + * Why replace these modules and not the methods of secure-storage.js? + * Because then the real secure-storage.js code still runs in tests and + * only the last step, talking to the OS, is fake: + * + * CLI command -> src/secure-storage.js -> keychain/keytar -> OS keychain + * ^ replaced here + * + * test/secure-storage.test.js depends on this and does not break it: + * - It stubs the `keychain` functions with sinon to check that + * secure-storage.js calls them. When sinon restores them after the + * test, it puts back our in-memory versions, not the real ones. + * - It removes src/secure-storage.js from the require cache and loads it + * again. The new copy calls require('keychain') and gets the same, + * already replaced module object, because that module stays cached. + */ + +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const srcDir = path.resolve(__dirname, '..', '..', 'src') + path.sep; +const loadedSrcModule = Object.keys(require.cache).find((file) => + file.startsWith(srcDir) +); +if (loadedSrcModule) { + throw new Error( + `${loadedSrcModule} was loaded before the test home directory was set up` + ); +} + +const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'boxcli-test-home-')); +process.env.HOME = tempHome; +process.env.USERPROFILE = tempHome; +if (os.homedir() !== tempHome) { + throw new Error( + `os.homedir() returned ${os.homedir()} instead of the test home directory ${tempHome}` + ); +} +process.on('exit', () => { + try { + fs.rmSync(tempHome, { recursive: true, force: true, maxRetries: 3 }); + } catch { + // A file may still be open (Windows). Leaving the temp dir behind is + // better than failing a run whose tests all passed. + } +}); + +// Empties the temp home, then recreates an empty ~/.box. The CLI creates that +// folder at startup, and code such as the token cache expects it to exist. +function resetTempHome() { + for (const entry of fs.readdirSync(tempHome)) { + fs.rmSync(path.join(tempHome, entry), { + recursive: true, + force: true, + maxRetries: 3, + }); + } + fs.mkdirSync(path.join(tempHome, '.box')); +} + +function loadOptional(packageName) { + try { + return require(packageName); + } catch { + // Not loadable on this machine. Secure storage is then unavailable and + // the CLI uses files in the temp home. + return null; + } +} + +const store = new Map(); +const keyOf = (service, account) => `${service}\u0000${account}`; + +const keychain = + process.platform === 'darwin' ? loadOptional('keychain') : null; +const keytar = + process.platform === 'darwin' ? null : loadOptional('@github/keytar'); + +// macOS backend: callback API, reports a missing entry as an error. +if (keychain) { + keychain.getPassword = (opts, fn) => { + const key = keyOf(opts.service, opts.account); + if (store.has(key)) { + fn(null, store.get(key)); + } else { + fn(new Error('Could not find password'), null); + } + }; + keychain.setPassword = (opts, fn) => { + store.set(keyOf(opts.service, opts.account), opts.password); + fn(null); + }; + keychain.deletePassword = (opts, fn) => { + if (store.delete(keyOf(opts.service, opts.account))) { + fn(null); + } else { + fn(new Error('Could not find password')); + } + }; +} + +// Windows/Linux backend: promise API. +if (keytar) { + keytar.getPassword = async (service, account) => + store.get(keyOf(service, account)) ?? null; + keytar.setPassword = async (service, account, password) => { + store.set(keyOf(service, account), password); + }; + keytar.deletePassword = async (service, account) => + store.delete(keyOf(service, account)); +} + +exports.mochaHooks = { + beforeEach() { + store.clear(); + resetTempHome(); + }, +}; diff --git a/test/token-cache.test.js b/test/token-cache.test.js index 6da729eb..30b834cb 100644 --- a/test/token-cache.test.js +++ b/test/token-cache.test.js @@ -31,15 +31,6 @@ describe('CLITokenCache', function () { }); afterEach(function () { - // Clean up any test files - if (fs.existsSync(testFilePath)) { - try { - fs.unlinkSync(testFilePath); - } catch { - // Ignore cleanup errors - } - } - // Restore stubs if (keytarStub) { keytarStub.restore();