Skip to content

Commit 693a884

Browse files
committed
fix(addNewMask): fail by default
1 parent 166c210 commit 693a884

File tree

2 files changed

+185
-62
lines changed

2 files changed

+185
-62
lines changed

lib/addNewMask.js

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,25 @@
11
const { getServerAddress } = require('./helpers');
22

3+
const exitCodes = {
4+
success: 0,
5+
error: 1,
6+
missingArguments: 2,
7+
unexpectedSuccess: 3,
8+
};
9+
10+
/**
11+
* Unexpected exit with code 0 can lead to the leakage of secrets in the build logs.
12+
* The exit should never be successful unless the secret was successfully masked.
13+
*/
14+
let exitWithError = true;
15+
const exitHandler = (exitCode) => {
16+
if ((exitCode === 0 || process.exitCode === 0) && exitWithError) {
17+
console.warn(`Unexpected exit with code 0. Exiting with ${exitCodes.unexpectedSuccess} instead`);
18+
process.exitCode = exitCodes.unexpectedSuccess;
19+
}
20+
};
21+
process.on('exit', exitHandler);
22+
323
async function updateMasks(secret) {
424
try {
525
const serverAddress = await getServerAddress();
@@ -10,29 +30,32 @@ async function updateMasks(secret) {
1030
const { default: httpClient } = await import('got');
1131
const response = await httpClient.post(url, {
1232
json: secret,
33+
throwHttpErrors: false,
1334
});
1435

15-
if (response.statusCode !== 201) {
16-
console.error(`could not create mask for secret: ${secret.key}, because server responded with: ${response.statusCode}\n\n${JSON.stringify(response.body)}`);
17-
process.exit(1);
36+
if (response.statusCode === 201) {
37+
console.log(`successfully updated masks with secret: ${secret.key}`);
38+
exitWithError = false;
39+
process.exit(exitCodes.success);
40+
} else {
41+
console.error(`could not create mask for secret: ${secret.key}. Server responded with: ${response.statusCode}\n\n${response.body}`);
42+
process.exit(exitCodes.error);
1843
}
19-
console.log(`successfully updated masks with secret: ${secret.key}`);
20-
process.exit(0);
2144
} catch (error) {
2245
console.error(`could not create mask for secret: ${secret.key}. Error: ${error}`);
23-
process.exit(1);
46+
process.exit(exitCodes.error);
2447
}
2548
}
2649

2750
if (require.main === module) {
2851
// first argument is the secret key second argument is the secret value
2952
if (process.argv.length < 4) {
3053
console.log('not enough arguments, need secret key and secret value');
31-
process.exit(2);
54+
process.exit(exitCodes.missingArguments);
3255
}
3356
const key = process.argv[2];
3457
const value = process.argv[3];
3558
updateMasks({ key, value });
3659
} else {
37-
module.exports = updateMasks;
60+
module.exports = { updateMasks, exitHandler };
3861
}

test/addNewMask.unit.spec.js

Lines changed: 154 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,81 +1,181 @@
1-
/* jshint ignore:start */
2-
3-
const timers = require('node:timers/promises');
4-
const chai = require('chai');
5-
const expect = chai.expect;
6-
const sinon = require('sinon');
7-
const sinonChai = require('sinon-chai');
8-
const { getPromiseWithResolvers } = require('../lib/helpers');
1+
const chai = require('chai');
2+
const sinon = require('sinon');
3+
const sinonChai = require('sinon-chai');
94
const proxyquire = require('proxyquire').noCallThru();
5+
6+
const expect = chai.expect;
107
chai.use(sinonChai);
118

12-
const originalProcessExit = process.exit;
139

1410
describe('addNewMask', () => {
15-
before(() => {
16-
process.exit = sinon.spy();
11+
const originalProcessExit = process.exit;
12+
const originalConsole = console;
13+
14+
const stubGetServerAddress = sinon.stub();
15+
const stubProcessExit = sinon.stub();
16+
const stubConsole = {
17+
debug: sinon.stub(),
18+
error: sinon.stub(),
19+
warn: sinon.stub(),
20+
log: sinon.stub(),
21+
};
22+
const stubGot = {
23+
post: sinon.stub().resolves({ statusCode: 201 }),
24+
};
25+
26+
const secret = {
27+
key: '123',
28+
value: 'ABC',
29+
};
30+
31+
before(async () => {
32+
process.exit = stubProcessExit;
33+
console = stubConsole;
34+
const { default: httpClient } = await import('got');
35+
sinon.stub(httpClient, 'post').callsFake(stubGot.post);
1736
});
1837

1938
beforeEach(() => {
20-
process.exit.resetHistory();
39+
stubProcessExit.resetHistory();
40+
stubGetServerAddress.resetHistory();
41+
for (const stub in stubConsole) {
42+
stubConsole[stub].resetHistory();
43+
}
44+
for (const stub in stubGot) {
45+
stubGot[stub].resetHistory();
46+
}
2147
});
2248

2349
after(() => {
2450
process.exit = originalProcessExit;
51+
console = originalConsole;
2552
});
2653

2754
describe('positive', () => {
2855
it('should send a request to add a secret', async () => {
29-
const rpSpy = sinon.spy(async () => ({ statusCode: 201 }));
30-
const deferredAddress = getPromiseWithResolvers();
31-
const addNewMask = proxyquire('../lib/addNewMask', {
32-
'request-promise': rpSpy,
33-
'./logger': {
34-
secretsServerAddress: deferredAddress.promise,
35-
},
56+
const serverAddress = 'https://xkcd.com/605/'
57+
stubGetServerAddress.resolves(serverAddress);
58+
stubGot.post.resolves({ statusCode: 201 });
59+
60+
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
61+
'./helpers': { getServerAddress: stubGetServerAddress },
3662
});
37-
38-
const secret = {
39-
key: '123',
40-
value: 'ABC',
41-
};
42-
43-
deferredAddress.resolve('http://127.0.0.1:1337');
44-
addNewMask(secret);
45-
46-
await timers.setTimeout(10);
47-
expect(rpSpy).to.have.been.calledOnceWith({
48-
uri: `http://127.0.0.1:1337/secrets`,
49-
method: 'POST',
50-
json: true,
51-
body: secret,
52-
resolveWithFullResponse: true,
63+
process.listeners('exit').forEach((listener) => {
64+
if (listener === exitHandler) {
65+
process.removeListener('exit', listener);
66+
}
5367
});
54-
await timers.setTimeout(10);
55-
expect(process.exit).to.have.been.calledOnceWith(0);
68+
await updateMasks(secret);
69+
expect(stubGot.post).to.have.been.calledOnceWith(new URL('secrets', serverAddress), {
70+
json: secret,
71+
throwHttpErrors: false,
72+
});
73+
expect(stubProcessExit).to.have.been.calledOnceWith(0);
5674
});
5775
});
5876

5977
describe('negative', () => {
60-
it('should send a request to add a secret', async () => {
61-
const rpSpy = sinon.spy(async () => { throw 'could not send request';});
62-
deferredAddress = getPromiseWithResolvers();
63-
const addNewMask = proxyquire('../lib/addNewMask', {
64-
'request-promise': rpSpy,
65-
'./logger': {
66-
secretsServerAddress: deferredAddress.promise,
78+
it('should fail if the server address is not available', async () => {
79+
stubGetServerAddress.rejects('could not get server address');
80+
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
81+
'./helpers': {
82+
getServerAddress: stubGetServerAddress,
6783
},
6884
});
69-
70-
const secret = {
71-
key: '123',
72-
value: 'ABC',
73-
};
74-
75-
deferredAddress.resolve('http://127.0.0.1:1337');
76-
addNewMask(secret);
77-
await timers.setTimeout(10);
78-
expect(process.exit).to.have.been.calledOnceWith(1);
85+
process.listeners('exit').forEach((listener) => {
86+
if (listener === exitHandler) {
87+
process.removeListener('exit', listener);
88+
}
89+
});
90+
await updateMasks(secret);
91+
expect(stubConsole.error).to.have.been.calledOnceWith('could not create mask for secret: 123. Error: could not get server address');
92+
expect(stubProcessExit).to.have.been.calledOnceWith(1);
93+
});
94+
95+
it('should fail if the server address is not valid URL', async () => {
96+
stubGetServerAddress.resolves('foo');
97+
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
98+
'./helpers': {
99+
getServerAddress: stubGetServerAddress,
100+
},
101+
});
102+
process.listeners('exit').forEach((listener) => {
103+
if (listener === exitHandler) {
104+
process.removeListener('exit', listener);
105+
}
106+
});
107+
await updateMasks(secret);
108+
expect(stubConsole.error).to.have.been.calledOnceWith('could not create mask for secret: 123. Error: TypeError: Invalid URL');
109+
expect(stubProcessExit).to.have.been.calledOnceWith(1);
110+
});
111+
112+
it('should fail if server responded not with 201', async () => {
113+
const serverAddress = 'https://g.codefresh.io'
114+
stubGetServerAddress.resolves(serverAddress);
115+
stubGot.post.resolves({
116+
statusCode: 500,
117+
body: 'Internal Server Error',
118+
});
119+
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
120+
'./helpers': { getServerAddress: stubGetServerAddress },
121+
});
122+
process.listeners('exit').forEach((listener) => {
123+
if (listener === exitHandler) {
124+
process.removeListener('exit', listener);
125+
}
126+
});
127+
await updateMasks(secret);
128+
expect(stubConsole.error).to.have.been.calledOnceWith('could not create mask for secret: 123. Server responded with: 500\n\nInternal Server Error');
129+
expect(stubProcessExit).to.have.been.calledOnceWith(1);
130+
});
131+
});
132+
133+
describe('exitHandler', () => {
134+
it('should set exit code to 3 if the original exit code is 0 and variable was not masked', () => {
135+
const { exitHandler } = proxyquire('../lib/addNewMask', {});
136+
process.listeners('exit').forEach((listener) => {
137+
if (listener === exitHandler) {
138+
process.removeListener('exit', listener);
139+
}
140+
});
141+
exitHandler(0);
142+
expect(process.exitCode).to.be.equal(3);
143+
expect(stubConsole.warn).to.have.been.calledOnceWith('Unexpected exit with code 0. Exiting with 3 instead');
144+
process.exitCode = undefined;
145+
});
146+
147+
it('should set exit code to 3 if the original exit code is 0 and variable was not masked', () => {
148+
const { exitHandler } = proxyquire('../lib/addNewMask', {});
149+
process.listeners('exit').forEach((listener) => {
150+
if (listener === exitHandler) {
151+
process.removeListener('exit', listener);
152+
}
153+
});
154+
if (process.exitCode !== undefined) {
155+
throw new Error('process.exitCode should be undefined to run this test');
156+
}
157+
process.exitCode = 0;
158+
exitHandler();
159+
expect(process.exitCode).to.be.equal(3);
160+
expect(stubConsole.warn).to.have.been.calledOnceWith('Unexpected exit with code 0. Exiting with 3 instead');
161+
process.exitCode = 0;
162+
});
163+
164+
it('should not change exit code if the variable was masked successfully', async () => {
165+
const serverAddress = 'https://xkcd.com/605/'
166+
stubGetServerAddress.resolves(serverAddress);
167+
stubGot.post.resolves({ statusCode: 201 });
168+
const { updateMasks, exitHandler } = proxyquire('../lib/addNewMask', {
169+
'./helpers': { getServerAddress: stubGetServerAddress },
170+
});
171+
process.listeners('exit').forEach((listener) => {
172+
if (listener === exitHandler) {
173+
process.removeListener('exit', listener);
174+
}
175+
});
176+
await updateMasks(secret);
177+
expect(process.exitCode).not.to.be.equal(3);
178+
expect(stubConsole.warn).not.to.have.been.calledOnceWith('Unexpected exit with code 0. Exiting with 3 instead');
79179
});
80180
});
81181
});

0 commit comments

Comments
 (0)