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 c0b0757b..a9d6cbdb 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -299,6 +299,20 @@ export class Debuglet extends EventEmitter { return; } + 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 = '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; + } + // TODO: Verify that it is fine for `id` to be undefined. let id: string|undefined; if (process.env.GAE_MINOR_VERSION) { @@ -732,7 +746,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..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/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 3b5d9386..17e199de 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'; @@ -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,6 +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 = '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)); + done(); + }); + + debuglet.once('started', () => { + assert.fail( + 'Should not start if workingDirectory is a root directory'); + }); + + 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}, 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']},