From 4a68901413f50ed939db1cf19d797bf58dea13ec Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 12:05:51 -0800 Subject: [PATCH 1/9] fix: Refuse to start if working dir is root dir --- src/agent/debuglet.ts | 8 ++++++++ test/test-debuglet.ts | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index c0b0757b..f533df43 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -291,6 +291,14 @@ export class Debuglet extends EventEmitter { const that = this; const stat = promisify(fs.stat); + const workingDir = that.config.workingDirectory; + // Don't continue if the working directory is a root directory + if (path.join(workingDir, '..') === workingDir) { + that.logger.error(`Refusing to start with \`workingDirectory\` set to a root directory: '${workingDir}'`); + that.emit('initError', new Error(`Cannot start the agent when the working directory is a root directory`)); + return; + } + try { await stat(path.join(that.config.workingDirectory, 'package.json')); } catch (err) { diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index 3b5d9386..ddf7d269 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -377,6 +377,30 @@ describe('Debuglet', () => { assert.deepEqual(mergedConfig, compareConfig); }); + it('should not start when workingDirectory is the a directory', (done) => { + const debug = new Debug({}, packageInfo); + const config = extend({}, defaultConfig, {workingDirectory: '/'}); + const debuglet = new Debuglet(debug, config); + let text = ''; + debuglet.logger.error = (str: string) => { + text += str; + }; + + debuglet.on('initError', (err: Error) => { + assert.ok(err); + assert.strictEqual(err.message, 'Cannot start the agent when the working directory is a root directory'); + assert(text.includes('Refusing to start with `workingDirectory` set to a root directory: ')); + done(); + }); + + debuglet.once('started', () => { + assert.fail('Should not start if workingDirectory is a root directory'); + }); + + debuglet.start(); + + }); + it('should not start when projectId is not available', (done) => { const savedGetProjectId = Debuglet.getProjectId; Debuglet.getProjectId = () => { From e13f6f7fe855a08c36ab17ec789cb997c963045d Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 12:07:46 -0800 Subject: [PATCH 2/9] Run `gts fix` --- src/agent/debuglet.ts | 10 +++++++--- src/agent/io/sourcemapper.ts | 10 +++++----- src/agent/state/inspector-state.ts | 1 - system-test/test-controller.ts | 1 - system-test/test-e2e.ts | 1 - test/test-controller.ts | 6 ------ test/test-debuggee.ts | 2 -- test/test-debuglet.ts | 8 +++++--- test/test-module.ts | 1 - test/test-scanner.ts | 16 +++++++--------- test/test-v8debugapi.ts | 9 --------- 11 files changed, 24 insertions(+), 41 deletions(-) diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index f533df43..aed0f315 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -294,8 +294,13 @@ export class Debuglet extends EventEmitter { const workingDir = that.config.workingDirectory; // Don't continue if the working directory is a root directory if (path.join(workingDir, '..') === workingDir) { - that.logger.error(`Refusing to start with \`workingDirectory\` set to a root directory: '${workingDir}'`); - that.emit('initError', new Error(`Cannot start the agent when the working directory is a root directory`)); + that.logger.error( + `Refusing to start with \`workingDirectory\` set to a root directory: '${ + workingDir}'`); + that.emit( + 'initError', + new Error( + `Cannot start the agent when the working directory is a root directory`)); return; } @@ -740,7 +745,6 @@ export class Debuglet extends EventEmitter { formatBreakpoints('Server breakpoints: ', updatedBreakpointMap)); } breakpoints.forEach((breakpoint: stackdriver.Breakpoint) => { - // TODO: Address the case when `breakpoint.id` is `undefined`. if (!that.completedBreakpointMap[breakpoint.id as string] && !that.activeBreakpointMap[breakpoint.id as string]) { diff --git a/src/agent/io/sourcemapper.ts b/src/agent/io/sourcemapper.ts index bf17095a..e1011022 100644 --- a/src/agent/io/sourcemapper.ts +++ b/src/agent/io/sourcemapper.ts @@ -236,11 +236,11 @@ export class SourceMapper { // TODO: The `sourceMap.Position` type definition has a `column` // attribute and not a `col` attribute. Determine if the type // definition or this code is correct. - column: (mappedPos as {} as { - col: number - }).col // SourceMapConsumer uses zero-based column - // numbers which is the same as the - // expected output + column: + (mappedPos as {} as {col: number}).col // SourceMapConsumer uses + // zero-based column numbers + // which is the same as the + // expected output }; } } diff --git a/src/agent/state/inspector-state.ts b/src/agent/state/inspector-state.ts index a56221fe..6e2cd499 100644 --- a/src/agent/state/inspector-state.ts +++ b/src/agent/state/inspector-state.ts @@ -174,7 +174,6 @@ class StateResolver { } } this.evaluatedExpressions[index2] = evaluated; - }); } // The frames are resolved after the evaluated expressions so that diff --git a/system-test/test-controller.ts b/system-test/test-controller.ts index 45be27bd..edd3d292 100644 --- a/system-test/test-controller.ts +++ b/system-test/test-controller.ts @@ -121,5 +121,4 @@ describe('Controller', function() { // To be able to write the following test we need a service for adding a // breakpoint (need the debugger API). TODO. it('should update an active breakpoint'); - }); diff --git a/system-test/test-e2e.ts b/system-test/test-e2e.ts index c13afd7d..2e6b9c44 100644 --- a/system-test/test-e2e.ts +++ b/system-test/test-e2e.ts @@ -223,7 +223,6 @@ describe('@google-cloud/debug end-to-end behavior', () => { // TODO: Determine if the results parameter should be used. }) .then((results: stackdriver.Breakpoint[]) => { - // Check the contents of the log, but keep the original breakpoint. // TODO: This is never used. Determine if it should be used. diff --git a/test/test-controller.ts b/test/test-controller.ts index 2a179438..ecba2f41 100644 --- a/test/test-controller.ts +++ b/test/test-controller.ts @@ -39,7 +39,6 @@ const api = '/v2/controller'; nock.disableNetConnect(); describe('Controller API', () => { - describe('register', () => { it('should get a debuggeeId', (done) => { const scope = nock(url).post(api + '/debuggees/register').reply(200, { @@ -85,11 +84,9 @@ describe('Controller API', () => { done(); }); }); - }); describe('listBreakpoints', () => { - // register before each test before((done) => { nock(url).post(api + '/debuggees/register').reply(200, { @@ -268,7 +265,4 @@ describe('Controller API', () => { }); }); }); - - - }); diff --git a/test/test-debuggee.ts b/test/test-debuggee.ts index d60fcaa4..f17ab793 100644 --- a/test/test-debuggee.ts +++ b/test/test-debuggee.ts @@ -20,7 +20,6 @@ import {Debuggee} from '../src/debuggee'; const agentVersion = `SomeName/client/SomeVersion`; describe('Debuggee', () => { - it('should create a Debuggee instance on valid input', () => { const debuggee = new Debuggee({ project: 'project', @@ -62,5 +61,4 @@ describe('Debuggee', () => { }); }); }); - }); diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index ddf7d269..29173e39 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -388,8 +388,11 @@ describe('Debuglet', () => { debuglet.on('initError', (err: Error) => { assert.ok(err); - assert.strictEqual(err.message, 'Cannot start the agent when the working directory is a root directory'); - assert(text.includes('Refusing to start with `workingDirectory` set to a root directory: ')); + assert.strictEqual( + err.message, + 'Cannot start the agent when the working directory is a root directory'); + assert(text.includes( + 'Refusing to start with `workingDirectory` set to a root directory: ')); done(); }); @@ -398,7 +401,6 @@ describe('Debuglet', () => { }); debuglet.start(); - }); it('should not start when projectId is not available', (done) => { diff --git a/test/test-module.ts b/test/test-module.ts index e44d4de7..04176a1e 100644 --- a/test/test-module.ts +++ b/test/test-module.ts @@ -37,5 +37,4 @@ describe('Debug module', () => { module.start(); }); }); - }); diff --git a/test/test-scanner.ts b/test/test-scanner.ts index 2398939a..b25460fd 100644 --- a/test/test-scanner.ts +++ b/test/test-scanner.ts @@ -33,7 +33,6 @@ import * as scanner from '../src/agent/io/scanner'; const COFFEE_FILES_REGEX = /^(.*\.js\.map)|(.*\.js)|(.*\.coffee)$/; describe('scanner', () => { - describe('scan', () => { it('should complain when called without a path', (done) => { // TODO: The second argument must be a string. Fix that. @@ -56,14 +55,13 @@ describe('scanner', () => { return; } - scanner.scan(true, fixture('broken-links'), /.*/) - .then((fileStats) => { - assert.strictEqual( - fileStats.selectFiles(/broken-link\.js/, '').length, 0); - assert.strictEqual( - fileStats.selectFiles(/intended-link\.js/, '').length, 0); - done(); - }); + scanner.scan(true, fixture('broken-links'), /.*/).then((fileStats) => { + assert.strictEqual( + fileStats.selectFiles(/broken-link\.js/, '').length, 0); + assert.strictEqual( + fileStats.selectFiles(/intended-link\.js/, '').length, 0); + done(); + }); }); it('should be able to return all file stats directly', (done) => { diff --git a/test/test-v8debugapi.ts b/test/test-v8debugapi.ts index 90e16f77..03f57251 100644 --- a/test/test-v8debugapi.ts +++ b/test/test-v8debugapi.ts @@ -308,7 +308,6 @@ describe('v8debugapi', () => { assert.ok(bp.status!.isError); done(); }); - }); }); @@ -349,7 +348,6 @@ describe('v8debugapi', () => { done(); }); }); - }); function conditionTests( @@ -584,7 +582,6 @@ describe('v8debugapi', () => { }); describe('set and wait', () => { - it('should be possible to wait on a breakpoint', (done) => { // clone a clean breakpointInFoo // TODO: Have this actually implement Breakpoint @@ -605,7 +602,6 @@ describe('v8debugapi', () => { code.foo(1); }); }); - }); it('should resolve actual line number hit rather than originally set', @@ -687,7 +683,6 @@ describe('v8debugapi', () => { code.foo(1); }); }); - }); it('should capture state', (done) => { @@ -1312,7 +1307,6 @@ describe('v8debugapi', () => { code.foo(3); }); }); - }); it('should be possible to set conditional breakpoints', (done) => { @@ -1342,7 +1336,6 @@ describe('v8debugapi', () => { code.foo(5); }); }); - }); it('should be possible to set conditional breakpoints in coffeescript', @@ -1637,7 +1630,6 @@ describe('v8debugapi', () => { assert.ifError(err3); throw new Error(message); }); - }); process.nextTick(() => { code.foo(1); @@ -1710,7 +1702,6 @@ describe('v8debugapi.findScriptsFuzzy', () => { }); it('should do suffix matches correctly', () => { - const TESTS = [ // Exact match. {scriptPath: 'foo.js', fileList: ['/foo.js'], result: ['/foo.js']}, From 2cf48a90f68b54271fad1c76e9ca4ee0153071d2 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 13:20:48 -0800 Subject: [PATCH 3/9] Make tests work on Windows and Unix --- test/test-debuglet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index 29173e39..6b459486 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -379,7 +379,7 @@ describe('Debuglet', () => { it('should not start when workingDirectory is the a directory', (done) => { const debug = new Debug({}, packageInfo); - const config = extend({}, defaultConfig, {workingDirectory: '/'}); + const config = extend({}, defaultConfig, {workingDirectory: path.sep}); const debuglet = new Debuglet(debug, config); let text = ''; debuglet.logger.error = (str: string) => { From e782e1ee83a45f53b59cdc07c97e60e0ea4bb56f Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 14:00:36 -0800 Subject: [PATCH 4/9] Add comments to the debuglet test --- test/test-debuglet.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index 6b459486..4e65871c 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -377,8 +377,17 @@ describe('Debuglet', () => { assert.deepEqual(mergedConfig, compareConfig); }); - it('should not start when workingDirectory is the a directory', (done) => { + it('should not start when workingDirectory is a root directory', (done) => { const debug = new Debug({}, packageInfo); + // `path.sep` represents a root directory on both Windows and Unix. + // On Windows, `path.sep` resolves to the current drive. + // + // That is, after opening a command prompt in Windows, relative to the + // drive C: and starting the Node REPL, the value of `path.sep` + // represents `C:\\'. + // + // If `D:` is entered in the prompt to switch to the D: drive before starting + // the Node REPL, `path.sep` represents `D:\\`. const config = extend({}, defaultConfig, {workingDirectory: path.sep}); const debuglet = new Debuglet(debug, config); let text = ''; @@ -391,7 +400,7 @@ describe('Debuglet', () => { assert.strictEqual( err.message, 'Cannot start the agent when the working directory is a root directory'); - assert(text.includes( + assert.ok(text.includes( 'Refusing to start with `workingDirectory` set to a root directory: ')); done(); }); From 9bc9f8975ff7a40409dbc79f95c94a373c7ddf60 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 14:22:21 -0800 Subject: [PATCH 5/9] Address GitHub comments --- src/agent/debuglet.ts | 23 +++++------ test/test-debuglet.ts | 93 ++++++++++++++++++++++++++----------------- 2 files changed, 67 insertions(+), 49 deletions(-) diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index aed0f315..46f2c040 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -291,19 +291,6 @@ export class Debuglet extends EventEmitter { const that = this; const stat = promisify(fs.stat); - const workingDir = that.config.workingDirectory; - // Don't continue if the working directory is a root directory - if (path.join(workingDir, '..') === workingDir) { - that.logger.error( - `Refusing to start with \`workingDirectory\` set to a root directory: '${ - workingDir}'`); - that.emit( - 'initError', - new Error( - `Cannot start the agent when the working directory is a root directory`)); - return; - } - try { await stat(path.join(that.config.workingDirectory, 'package.json')); } catch (err) { @@ -312,6 +299,16 @@ export class Debuglet extends EventEmitter { return; } + const workingDir = that.config.workingDirectory; + // Don't continue if the working directory is a root directory + if (path.join(workingDir, '..') === workingDir) { + const message = `Refusing to start the cloud debugger with \`workingDirectory\` set to a root ` + + `directory (${workingDir}) to avoid scanning the entire filesystem for Javascript files.`; + that.logger.error(message); + that.emit('initError',new Error(message)); + return; + } + // TODO: Verify that it is fine for `id` to be undefined. let id: string|undefined; if (process.env.GAE_MINOR_VERSION) { diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index 4e65871c..808e274d 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -15,7 +15,7 @@ */ import * as assert from 'assert'; -import * as rawFs from 'fs'; +import * as fs from 'fs'; import * as _ from 'lodash'; import * as path from 'path'; import * as proxyquire from 'proxyquire'; @@ -377,41 +377,6 @@ describe('Debuglet', () => { assert.deepEqual(mergedConfig, compareConfig); }); - it('should not start when workingDirectory is a root directory', (done) => { - const debug = new Debug({}, packageInfo); - // `path.sep` represents a root directory on both Windows and Unix. - // On Windows, `path.sep` resolves to the current drive. - // - // That is, after opening a command prompt in Windows, relative to the - // drive C: and starting the Node REPL, the value of `path.sep` - // represents `C:\\'. - // - // If `D:` is entered in the prompt to switch to the D: drive before starting - // the Node REPL, `path.sep` represents `D:\\`. - const config = extend({}, defaultConfig, {workingDirectory: path.sep}); - const debuglet = new Debuglet(debug, config); - let text = ''; - debuglet.logger.error = (str: string) => { - text += str; - }; - - debuglet.on('initError', (err: Error) => { - assert.ok(err); - assert.strictEqual( - err.message, - 'Cannot start the agent when the working directory is a root directory'); - assert.ok(text.includes( - 'Refusing to start with `workingDirectory` set to a root directory: ')); - done(); - }); - - debuglet.once('started', () => { - assert.fail('Should not start if workingDirectory is a root directory'); - }); - - debuglet.start(); - }); - it('should not start when projectId is not available', (done) => { const savedGetProjectId = Debuglet.getProjectId; Debuglet.getProjectId = () => { @@ -849,6 +814,62 @@ describe('Debuglet', () => { debuglet.start(); }); + it('should error when workingDirectory is a root directory with a package.json', (done) => { + const debug = new Debug({}, packageInfo); + /* + * `path.sep` represents a root directory on both Windows and Unix. + * On Windows, `path.sep` resolves to the current drive. + * + * That is, after opening a command prompt in Windows, relative to the + * drive C: and starting the Node REPL, the value of `path.sep` + * represents `C:\\'. + * + * If `D:` is entered at the prompt to switch to the D: drive before starting + * the Node REPL, `path.sep` represents `D:\\`. + */ + const root = path.sep; + const mockedDebuglet = proxyquire('../src/agent/debuglet', { + /* + * Mock the 'fs' module to verify that if the root directory is used, + * and the root directory is reported to contain a `package.json` file, + * then the agent still produces an `initError` when the working directory + * is the root directory. + */ + fs: { + stat: (filepath: string|Buffer, cb: (err: Error|null, stats: {}) => void) => { + if (filepath === path.join(root, 'package.json')) { + // The key point is that looking for `package.json` in the root + // directory does not cause an error. + return cb(null, {}); + } + fs.stat(filepath, cb); + } + } + }); + const config = extend({}, defaultConfig, {workingDirectory: root}); + const debuglet = new mockedDebuglet.Debuglet(debug, config); + let text = ''; + debuglet.logger.error = (str: string) => { + text += str; + }; + + debuglet.on('initError', (err: Error) => { + const errorMessage = `Refusing to start the cloud debugger with \`workingDirectory\` ` + + `set to a root directory (${root}) to avoid scanning the entire filesystem ` + + `for Javascript files.`; + assert.ok(err); + assert.strictEqual(err.message, errorMessage); + assert.ok(text.includes(errorMessage)); + done(); + }); + + debuglet.once('started', () => { + assert.fail('Should not start if workingDirectory is a root directory'); + }); + + debuglet.start(); + }); + it('should register successfully otherwise', (done) => { const debug = new Debug( {projectId: 'fake-project', credentials: fakeCredentials}, From 6459734c7e467de88c2d2311b346e20191155d4a Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 15:02:03 -0800 Subject: [PATCH 6/9] Add config option `allowRootAsWorkingDirectory` --- src/agent/config.ts | 3 +++ src/agent/debuglet.ts | 4 ++-- test/test-debuglet.ts | 54 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/agent/config.ts b/src/agent/config.ts index b2c5f642..50468078 100644 --- a/src/agent/config.ts +++ b/src/agent/config.ts @@ -23,6 +23,8 @@ export type DebugAgentConfig = { export interface ResolvedDebugAgentConfig extends common.AuthenticationConfig { workingDirectory: string; + allowRootAsWorkingDirectory: boolean; + /** * A user specified way of identifying the service */ @@ -174,6 +176,7 @@ export const defaultConfig: ResolvedDebugAgentConfig = { // FIXME(ofrobots): presently this is dependent what cwd() is at the time this // file is first required. We should make the default config static. workingDirectory: process.cwd(), + allowRootAsWorkingDirectory: false, description: undefined, allowExpressions: false, diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index 46f2c040..63be9508 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -301,11 +301,11 @@ export class Debuglet extends EventEmitter { const workingDir = that.config.workingDirectory; // Don't continue if the working directory is a root directory - if (path.join(workingDir, '..') === workingDir) { + if (!that.config.allowRootAsWorkingDirectory && path.join(workingDir, '..') === workingDir) { const message = `Refusing to start the cloud debugger with \`workingDirectory\` set to a root ` + `directory (${workingDir}) to avoid scanning the entire filesystem for Javascript files.`; that.logger.error(message); - that.emit('initError',new Error(message)); + that.emit('initError', new Error(message)); return; } diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index 808e274d..c84abbec 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -28,7 +28,7 @@ import * as stackdriver from '../src/types/stackdriver'; DEFAULT_CONFIG.allowExpressions = true; DEFAULT_CONFIG.workingDirectory = path.join(__dirname, '..', '..'); -import {Debuglet, CachedPromise} from '../src/agent/debuglet'; +import {Debuglet, CachedPromise, FindFilesResult} from '../src/agent/debuglet'; import {ScanResults} from '../src/agent/io/scanner'; import * as dns from 'dns'; import * as extend from 'extend'; @@ -814,7 +814,7 @@ describe('Debuglet', () => { debuglet.start(); }); - it('should error when workingDirectory is a root directory with a package.json', (done) => { + it('should by default error when workingDirectory is a root directory with a package.json', (done) => { const debug = new Debug({}, packageInfo); /* * `path.sep` represents a root directory on both Windows and Unix. @@ -870,6 +870,56 @@ describe('Debuglet', () => { debuglet.start(); }); + it('should be able to force the workingDirectory to be a root directory', (done) => { + const root = path.sep; + // Act like the root directory contains a `package.json` file + const mockedDebuglet = proxyquire('../src/agent/debuglet', { + fs: { + stat: (filepath: string|Buffer, cb: (err: Error|null, stats: {}) => void) => { + if (filepath === path.join(root, 'package.json')) { + return cb(null, {}); + } + fs.stat(filepath, cb); + } + } + }); + + // Don't actually scan the entire filesystem. Act like the filesystem is empty. + mockedDebuglet.Debuglet.findFiles = (shouldHash: boolean, baseDir: string): Promise => { + assert.strictEqual(baseDir, root); + return Promise.resolve({ + jsStats: {}, + mapFiles: [], + errors: new Map() + }); + } + + // Act like the debuglet can get a project id + mockedDebuglet.Debuglet.getProjectId = () => 'some-project-id'; + + // No need to restore `findFiles` and `getProjectId` because we are modifying + // a mocked version of `Debuglet` not `Debuglet` itself. + + const config = extend({}, defaultConfig, { + workingDirectory: root, + allowRootAsWorkingDirectory: true + }); + const debug = new Debug({}, packageInfo); + const debuglet = new mockedDebuglet.Debuglet(debug, config); + + debuglet.on('initError', (err: Error) => { + assert.ifError(err); + done(); + }); + + debuglet.once('started', () => { + debuglet.stop(); + done(); + }); + + debuglet.start(); + }); + it('should register successfully otherwise', (done) => { const debug = new Debug( {projectId: 'fake-project', credentials: fakeCredentials}, From 7ad9c6962af2ad5b5a09a0a7f74359b6f53dbcb4 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 15:04:26 -0800 Subject: [PATCH 7/9] Run `gts fix` --- src/agent/debuglet.ts | 9 +- src/agent/io/sourcemapper.ts | 10 +- test/test-debuglet.ts | 205 ++++++++++++++++++----------------- 3 files changed, 119 insertions(+), 105 deletions(-) diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index 63be9508..d0227e4d 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -301,9 +301,12 @@ export class Debuglet extends EventEmitter { const workingDir = that.config.workingDirectory; // Don't continue if the working directory is a root directory - if (!that.config.allowRootAsWorkingDirectory && path.join(workingDir, '..') === workingDir) { - const message = `Refusing to start the cloud debugger with \`workingDirectory\` set to a root ` + - `directory (${workingDir}) to avoid scanning the entire filesystem for Javascript files.`; + if (!that.config.allowRootAsWorkingDirectory && + path.join(workingDir, '..') === workingDir) { + const message = + `Refusing to start the cloud debugger with \`workingDirectory\` set to a root ` + + `directory (${ + workingDir}) to avoid scanning the entire filesystem for Javascript files.`; that.logger.error(message); that.emit('initError', new Error(message)); return; diff --git a/src/agent/io/sourcemapper.ts b/src/agent/io/sourcemapper.ts index e1011022..75243bea 100644 --- a/src/agent/io/sourcemapper.ts +++ b/src/agent/io/sourcemapper.ts @@ -236,11 +236,11 @@ export class SourceMapper { // TODO: The `sourceMap.Position` type definition has a `column` // attribute and not a `col` attribute. Determine if the type // definition or this code is correct. - column: - (mappedPos as {} as {col: number}).col // SourceMapConsumer uses - // zero-based column numbers - // which is the same as the - // expected output + column: (mappedPos as {} as {col: number}).col // SourceMapConsumer uses + // zero-based column + // numbers which is the + // same as the expected + // output }; } } diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index c84abbec..c5cab48e 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -814,111 +814,122 @@ describe('Debuglet', () => { debuglet.start(); }); - it('should by default error when workingDirectory is a root directory with a package.json', (done) => { - const debug = new Debug({}, packageInfo); - /* - * `path.sep` represents a root directory on both Windows and Unix. - * On Windows, `path.sep` resolves to the current drive. - * - * That is, after opening a command prompt in Windows, relative to the - * drive C: and starting the Node REPL, the value of `path.sep` - * represents `C:\\'. - * - * If `D:` is entered at the prompt to switch to the D: drive before starting - * the Node REPL, `path.sep` represents `D:\\`. - */ - const root = path.sep; - const mockedDebuglet = proxyquire('../src/agent/debuglet', { - /* - * Mock the 'fs' module to verify that if the root directory is used, - * and the root directory is reported to contain a `package.json` file, - * then the agent still produces an `initError` when the working directory - * is the root directory. - */ - fs: { - stat: (filepath: string|Buffer, cb: (err: Error|null, stats: {}) => void) => { - if (filepath === path.join(root, 'package.json')) { - // The key point is that looking for `package.json` in the root - // directory does not cause an error. - return cb(null, {}); - } - fs.stat(filepath, cb); - } - } - }); - const config = extend({}, defaultConfig, {workingDirectory: root}); - const debuglet = new mockedDebuglet.Debuglet(debug, config); - let text = ''; - debuglet.logger.error = (str: string) => { - text += str; - }; - - debuglet.on('initError', (err: Error) => { - const errorMessage = `Refusing to start the cloud debugger with \`workingDirectory\` ` + - `set to a root directory (${root}) to avoid scanning the entire filesystem ` + - `for Javascript files.`; - assert.ok(err); - assert.strictEqual(err.message, errorMessage); - assert.ok(text.includes(errorMessage)); - done(); - }); - - debuglet.once('started', () => { - assert.fail('Should not start if workingDirectory is a root directory'); - }); - - debuglet.start(); - }); + it('should by default error when workingDirectory is a root directory with a package.json', + (done) => { + const debug = new Debug({}, packageInfo); + /* + * `path.sep` represents a root directory on both Windows and Unix. + * On Windows, `path.sep` resolves to the current drive. + * + * That is, after opening a command prompt in Windows, relative to the + * drive C: and starting the Node REPL, the value of `path.sep` + * represents `C:\\'. + * + * If `D:` is entered at the prompt to switch to the D: drive before + * starting the Node REPL, `path.sep` represents `D:\\`. + */ + const root = path.sep; + const mockedDebuglet = proxyquire('../src/agent/debuglet', { + /* + * Mock the 'fs' module to verify that if the root directory is used, + * and the root directory is reported to contain a `package.json` + * file, then the agent still produces an `initError` when the + * working directory is the root directory. + */ + fs: { + stat: + (filepath: string|Buffer, + cb: (err: Error|null, stats: {}) => void) => { + if (filepath === path.join(root, 'package.json')) { + // The key point is that looking for `package.json` in the + // root directory does not cause an error. + return cb(null, {}); + } + fs.stat(filepath, cb); + } + } + }); + const config = extend({}, defaultConfig, {workingDirectory: root}); + const debuglet = new mockedDebuglet.Debuglet(debug, config); + let text = ''; + debuglet.logger.error = (str: string) => { + text += str; + }; - it('should be able to force the workingDirectory to be a root directory', (done) => { - const root = path.sep; - // Act like the root directory contains a `package.json` file - const mockedDebuglet = proxyquire('../src/agent/debuglet', { - fs: { - stat: (filepath: string|Buffer, cb: (err: Error|null, stats: {}) => void) => { - if (filepath === path.join(root, 'package.json')) { - return cb(null, {}); - } - fs.stat(filepath, cb); - } - } - }); + debuglet.on('initError', (err: Error) => { + const errorMessage = + `Refusing to start the cloud debugger with \`workingDirectory\` ` + + `set to a root directory (${ + root}) to avoid scanning the entire filesystem ` + + `for Javascript files.`; + assert.ok(err); + assert.strictEqual(err.message, errorMessage); + assert.ok(text.includes(errorMessage)); + done(); + }); - // Don't actually scan the entire filesystem. Act like the filesystem is empty. - mockedDebuglet.Debuglet.findFiles = (shouldHash: boolean, baseDir: string): Promise => { - assert.strictEqual(baseDir, root); - return Promise.resolve({ - jsStats: {}, - mapFiles: [], - errors: new Map() - }); - } + debuglet.once('started', () => { + assert.fail( + 'Should not start if workingDirectory is a root directory'); + }); - // Act like the debuglet can get a project id - mockedDebuglet.Debuglet.getProjectId = () => 'some-project-id'; + debuglet.start(); + }); - // No need to restore `findFiles` and `getProjectId` because we are modifying - // a mocked version of `Debuglet` not `Debuglet` itself. + it('should be able to force the workingDirectory to be a root directory', + (done) => { + const root = path.sep; + // Act like the root directory contains a `package.json` file + const mockedDebuglet = proxyquire('../src/agent/debuglet', { + fs: { + stat: + (filepath: string|Buffer, + cb: (err: Error|null, stats: {}) => void) => { + if (filepath === path.join(root, 'package.json')) { + return cb(null, {}); + } + fs.stat(filepath, cb); + } + } + }); - const config = extend({}, defaultConfig, { - workingDirectory: root, - allowRootAsWorkingDirectory: true - }); - const debug = new Debug({}, packageInfo); - const debuglet = new mockedDebuglet.Debuglet(debug, config); + // Don't actually scan the entire filesystem. Act like the filesystem + // is empty. + mockedDebuglet.Debuglet.findFiles = + (shouldHash: boolean, baseDir: string): + Promise => { + assert.strictEqual(baseDir, root); + return Promise.resolve({ + jsStats: {}, + mapFiles: [], + errors: new Map() + }); + }; + + // Act like the debuglet can get a project id + mockedDebuglet.Debuglet.getProjectId = () => 'some-project-id'; + + // No need to restore `findFiles` and `getProjectId` because we are + // modifying a mocked version of `Debuglet` not `Debuglet` itself. + + const config = extend( + {}, defaultConfig, + {workingDirectory: root, allowRootAsWorkingDirectory: true}); + const debug = new Debug({}, packageInfo); + const debuglet = new mockedDebuglet.Debuglet(debug, config); - debuglet.on('initError', (err: Error) => { - assert.ifError(err); - done(); - }); + debuglet.on('initError', (err: Error) => { + assert.ifError(err); + done(); + }); - debuglet.once('started', () => { - debuglet.stop(); - done(); - }); + debuglet.once('started', () => { + debuglet.stop(); + done(); + }); - debuglet.start(); - }); + debuglet.start(); + }); it('should register successfully otherwise', (done) => { const debug = new Debug( From bb9d1f853a54184291072d8964063b91cce2a61f Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 15:05:36 -0800 Subject: [PATCH 8/9] Add a comment --- src/agent/debuglet.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index d0227e4d..8731a8eb 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -301,6 +301,7 @@ export class Debuglet extends EventEmitter { const workingDir = that.config.workingDirectory; // Don't continue if the working directory is a root directory + // unless the user wants to force using the root directory if (!that.config.allowRootAsWorkingDirectory && path.join(workingDir, '..') === workingDir) { const message = From 501e4299549ba702160eefc342b56b8e872acab3 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 20 Dec 2017 15:34:32 -0800 Subject: [PATCH 9/9] Update the error message --- src/agent/debuglet.ts | 8 ++++---- test/test-debuglet.ts | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index 8731a8eb..a9d6cbdb 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -304,10 +304,10 @@ export class Debuglet extends EventEmitter { // unless the user wants to force using the root directory if (!that.config.allowRootAsWorkingDirectory && path.join(workingDir, '..') === workingDir) { - const message = - `Refusing to start the cloud debugger with \`workingDirectory\` set to a root ` + - `directory (${ - workingDir}) to avoid scanning the entire filesystem for Javascript files.`; + const message = 'The working directory is a root directory. Disabling ' + + 'to avoid a scan of the entire filesystem for JavaScript files. ' + + 'Use config \allowRootAsWorkingDirectory` if you really want to ' + + 'do this.'; that.logger.error(message); that.emit('initError', new Error(message)); return; diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index c5cab48e..17e199de 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -857,11 +857,10 @@ describe('Debuglet', () => { }; debuglet.on('initError', (err: Error) => { - const errorMessage = - `Refusing to start the cloud debugger with \`workingDirectory\` ` + - `set to a root directory (${ - root}) to avoid scanning the entire filesystem ` + - `for Javascript files.`; + const errorMessage = 'The working directory is a root ' + + 'directory. Disabling to avoid a scan of the entire filesystem ' + + 'for JavaScript files. Use config \allowRootAsWorkingDirectory` ' + + 'if you really want to do this.'; assert.ok(err); assert.strictEqual(err.message, errorMessage); assert.ok(text.includes(errorMessage));