Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

fix: refuse to start if working dir is root dir #381

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/agent/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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,

Expand Down
15 changes: 14 additions & 1 deletion src/agent/debuglet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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]) {
Expand Down
10 changes: 5 additions & 5 deletions src/agent/io/sourcemapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}
}
Expand Down
1 change: 0 additions & 1 deletion src/agent/state/inspector-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ class StateResolver {
}
}
this.evaluatedExpressions[index2] = evaluated;

});
}
// The frames are resolved after the evaluated expressions so that
Expand Down
1 change: 0 additions & 1 deletion system-test/test-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

});
1 change: 0 additions & 1 deletion system-test/test-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 0 additions & 6 deletions test/test-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -85,11 +84,9 @@ describe('Controller API', () => {
done();
});
});

});

describe('listBreakpoints', () => {

// register before each test
before((done) => {
nock(url).post(api + '/debuggees/register').reply(200, {
Expand Down Expand Up @@ -268,7 +265,4 @@ describe('Controller API', () => {
});
});
});



});
2 changes: 0 additions & 2 deletions test/test-debuggee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -62,5 +61,4 @@ describe('Debuggee', () => {
});
});
});

});
120 changes: 118 additions & 2 deletions test/test-debuglet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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<FindFilesResult> => {
assert.strictEqual(baseDir, root);
return Promise.resolve({
jsStats: {},
mapFiles: [],
errors: new Map<string, Error>()
});
};

// 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},
Expand Down
1 change: 0 additions & 1 deletion test/test-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,4 @@ describe('Debug module', () => {
module.start();
});
});

});
16 changes: 7 additions & 9 deletions test/test-scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) => {
Expand Down
9 changes: 0 additions & 9 deletions test/test-v8debugapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ describe('v8debugapi', () => {
assert.ok(bp.status!.isError);
done();
});

});
});

Expand Down Expand Up @@ -349,7 +348,6 @@ describe('v8debugapi', () => {
done();
});
});

});

function conditionTests(
Expand Down Expand Up @@ -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
Expand All @@ -605,7 +602,6 @@ describe('v8debugapi', () => {
code.foo(1);
});
});

});

it('should resolve actual line number hit rather than originally set',
Expand Down Expand Up @@ -687,7 +683,6 @@ describe('v8debugapi', () => {
code.foo(1);
});
});

});

it('should capture state', (done) => {
Expand Down Expand Up @@ -1312,7 +1307,6 @@ describe('v8debugapi', () => {
code.foo(3);
});
});

});

it('should be possible to set conditional breakpoints', (done) => {
Expand Down Expand Up @@ -1342,7 +1336,6 @@ describe('v8debugapi', () => {
code.foo(5);
});
});

});

it('should be possible to set conditional breakpoints in coffeescript',
Expand Down Expand Up @@ -1637,7 +1630,6 @@ describe('v8debugapi', () => {
assert.ifError(err3);
throw new Error(message);
});

});
process.nextTick(() => {
code.foo(1);
Expand Down Expand Up @@ -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']},
Expand Down