Skip to content

Commit ba62245

Browse files
arjankowskiarjankowski
authored
feat: Mask client secret in configure environments command (#654)
1 parent 307d181 commit ba62245

5 files changed

Lines changed: 194 additions & 3 deletions

File tree

‎src/commands/configure/environments/get.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const BoxCommand = require('../../../box-command');
44
const { Flags } = require('@oclif/core');
55
const _ = require('lodash');
6+
const utilities = require('../../../util');
67

78
class EnvironmentsGetCommand extends BoxCommand {
89
async run() {
@@ -22,7 +23,7 @@ class EnvironmentsGetCommand extends BoxCommand {
2223
if (_.isEmpty(environment)) {
2324
this.error('No environment(s) exists');
2425
} else {
25-
await this.output(environment);
26+
await this.output(utilities.maskObjectValuesByKey(environment));
2627
}
2728
}
2829
}
@@ -31,7 +32,8 @@ class EnvironmentsGetCommand extends BoxCommand {
3132
EnvironmentsGetCommand.noClient = true;
3233
EnvironmentsGetCommand.aliases = ['configure:environments:list'];
3334

34-
EnvironmentsGetCommand.description = 'Get a Box environment';
35+
EnvironmentsGetCommand.description =
36+
'Get a Box environment or list all configured Box environments.\nclientSecret values are masked in CLI output. To view full secrets, access secure storage directly (for example, macOS Keychain, Windows Credential Manager, or a supported Linux equivalent).';
3537

3638
EnvironmentsGetCommand.flags = {
3739
...BoxCommand.minFlags,

‎src/commands/configure/environments/update.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class EnvironmentsUpdateCommand extends BoxCommand {
6767
}
6868

6969
await this.updateEnvironments(environmentsObject);
70-
await this.output(environment);
70+
await this.output(utilities.maskObjectValuesByKey(environment));
7171
}
7272
}
7373

‎src/util.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ const PATH_ESCAPES = Object.freeze({
3030
'/': '~1',
3131
'~': '~0',
3232
});
33+
const DEFAULT_SECRET_KEY = 'clientSecret';
34+
const DEFAULT_VISIBLE_SECRET_CHARS = 3;
3335

3436
/**
3537
* Unescape a string that no longer needs to be escaped with slashes,
@@ -299,6 +301,43 @@ async function unlinkAsync(path) {
299301
});
300302
}
301303

304+
function maskSecret(secret, visibleChars = DEFAULT_VISIBLE_SECRET_CHARS) {
305+
const normalizedVisibleChars =
306+
_.isInteger(visibleChars) && visibleChars >= 0
307+
? visibleChars
308+
: DEFAULT_VISIBLE_SECRET_CHARS;
309+
310+
if (!_.isString(secret) || secret.length <= normalizedVisibleChars) {
311+
return '*'.repeat(normalizedVisibleChars);
312+
}
313+
314+
return `${'*'.repeat(secret.length - normalizedVisibleChars)}${secret.slice(-normalizedVisibleChars)}`;
315+
}
316+
317+
function maskObjectValuesByKey(
318+
value,
319+
keyToMask = DEFAULT_SECRET_KEY,
320+
visibleChars = DEFAULT_VISIBLE_SECRET_CHARS
321+
) {
322+
if (_.isArray(value)) {
323+
return value.map((item) =>
324+
maskObjectValuesByKey(item, keyToMask, visibleChars)
325+
);
326+
}
327+
328+
if (_.isPlainObject(value)) {
329+
return _.mapValues(value, (objectValue, key) => {
330+
if (key === keyToMask && !_.isNil(objectValue)) {
331+
return maskSecret(objectValue, visibleChars);
332+
}
333+
334+
return maskObjectValuesByKey(objectValue, keyToMask, visibleChars);
335+
});
336+
}
337+
338+
return value;
339+
}
340+
302341
module.exports = {
303342
/**
304343
* Validates the a configuration object has all required properties
@@ -388,6 +427,8 @@ module.exports = {
388427
parseMetadataOp(value) {
389428
return parseMetadataString(value);
390429
},
430+
maskSecret,
431+
maskObjectValuesByKey,
391432
parseStringToObject,
392433
checkDir,
393434
readFileAsync,

‎test/commands/configure-environments.test.js‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
'use strict';
2+
3+
const { test } = require('@oclif/test');
4+
const { assert } = require('chai');
5+
const sinon = require('sinon');
6+
const BoxCommand = require('../../src/box-command');
7+
8+
describe('Configure environments masking', function () {
9+
let sandbox;
10+
let getEnvironmentsStub;
11+
let updateEnvironmentsStub;
12+
13+
const RAW_SECRET = 'abcdefghijklmnopqrstuvwxyz123456';
14+
const MASKED_SECRET = `${'*'.repeat(RAW_SECRET.length - 3)}${RAW_SECRET.slice(-3)}`;
15+
16+
beforeEach(function () {
17+
sandbox = sinon.createSandbox();
18+
getEnvironmentsStub = sandbox.stub(BoxCommand.prototype, 'getEnvironments');
19+
updateEnvironmentsStub = sandbox.stub(
20+
BoxCommand.prototype,
21+
'updateEnvironments'
22+
);
23+
updateEnvironmentsStub.resolves();
24+
});
25+
26+
afterEach(function () {
27+
sandbox.restore();
28+
});
29+
30+
it('configure:environments:get should not print raw client secrets', function () {
31+
getEnvironmentsStub.resolves({
32+
default: 'dev',
33+
environments: {
34+
dev: {
35+
name: 'dev',
36+
clientSecret: RAW_SECRET,
37+
},
38+
},
39+
});
40+
41+
return test
42+
.stub(BoxCommand.prototype, 'getEnvironments', getEnvironmentsStub)
43+
.stdout()
44+
.command(['configure:environments:get'])
45+
.it('masks clientSecret in output', (ctx) => {
46+
assert.notInclude(ctx.stdout, RAW_SECRET);
47+
assert.include(ctx.stdout, MASKED_SECRET);
48+
});
49+
});
50+
51+
it(
52+
'configure:environments:update should not print raw client secrets',
53+
function () {
54+
getEnvironmentsStub.resolves({
55+
default: 'dev',
56+
environments: {
57+
dev: {
58+
name: 'dev',
59+
clientSecret: RAW_SECRET,
60+
},
61+
},
62+
});
63+
64+
return test
65+
.stub(BoxCommand.prototype, 'getEnvironments', getEnvironmentsStub)
66+
.stub(
67+
BoxCommand.prototype,
68+
'updateEnvironments',
69+
updateEnvironmentsStub
70+
)
71+
.stdout()
72+
.command(['configure:environments:update'])
73+
.it('masks clientSecret in output', (ctx) => {
74+
assert.notInclude(ctx.stdout, RAW_SECRET);
75+
assert.include(ctx.stdout, MASKED_SECRET);
76+
});
77+
}
78+
);
79+
});

‎test/util.test.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,75 @@ describe('Utilities', function () {
584584
);
585585
});
586586

587+
describe('secret masking helpers', function () {
588+
it('maskSecret() should preserve length and reveal only last 3 chars by default', function () {
589+
const secret = 'abcdefghijklmnopqrstuvwxyz123456';
590+
const masked = cliUtilities.maskSecret(secret);
591+
592+
assert.strictEqual(masked.length, secret.length);
593+
assert.strictEqual(masked.slice(-3), '456');
594+
assert.strictEqual(
595+
masked,
596+
`${'*'.repeat(secret.length - 3)}${secret.slice(-3)}`
597+
);
598+
});
599+
600+
it('maskSecret() should fallback to default visible chars for invalid values', function () {
601+
const secret = 'abcdef';
602+
const masked = cliUtilities.maskSecret(secret, -1);
603+
604+
assert.strictEqual(masked, '***def');
605+
});
606+
607+
it('maskObjectValuesByKey() should mask clientSecret recursively by default', function () {
608+
const input = {
609+
name: 'dev',
610+
clientSecret: '1234567890',
611+
nested: {
612+
clientSecret: 'abcdefghij',
613+
},
614+
items: [
615+
{
616+
clientSecret: 'qwertyuiop',
617+
},
618+
{
619+
other: 'value',
620+
},
621+
],
622+
};
623+
624+
const masked = cliUtilities.maskObjectValuesByKey(input);
625+
626+
assert.strictEqual(input.clientSecret, '1234567890');
627+
assert.strictEqual(masked.clientSecret, '*******890');
628+
assert.strictEqual(input.nested.clientSecret, 'abcdefghij');
629+
assert.strictEqual(masked.nested.clientSecret, '*******hij');
630+
assert.strictEqual(input.items[0].clientSecret, 'qwertyuiop');
631+
assert.strictEqual(masked.items[0].clientSecret, '*******iop');
632+
assert.strictEqual(masked.items[1].other, 'value');
633+
634+
});
635+
636+
it('maskObjectValuesByKey() should support custom key and visible chars', function () {
637+
const input = {
638+
token: 'abcd1234',
639+
nested: {
640+
token: 'wxyz9876',
641+
},
642+
clientSecret: 'dont-mask-default-key-if-custom-is-used',
643+
};
644+
645+
const masked = cliUtilities.maskObjectValuesByKey(input, 'token', 2);
646+
647+
assert.strictEqual(input.token, 'abcd1234');
648+
assert.strictEqual(masked.token, '******34');
649+
assert.strictEqual(input.nested.token, 'wxyz9876');
650+
assert.strictEqual(masked.nested.token, '******76');
651+
assert.strictEqual(input.clientSecret, 'dont-mask-default-key-if-custom-is-used');
652+
assert.strictEqual(masked.clientSecret,'dont-mask-default-key-if-custom-is-used');
653+
});
654+
});
655+
587656
describe('checkDir()', function () {
588657
it('should create directory if create flag is true', async function () {
589658
const destination = `${process.cwd()}/temp`;

0 commit comments

Comments
 (0)