Skip to content

Commit 55ce8ee

Browse files
dcoricdcoric
committed
fix: improves test coverage
Adds additional tests for better cove
1 parent af724ee commit 55ce8ee

File tree

2 files changed

+295
-5
lines changed

2 files changed

+295
-5
lines changed

‎src/config/ConfigLoader.js‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,17 @@ const envPaths = require('env-paths');
99

1010
// Add path validation helper
1111
function isValidPath(filePath) {
12-
const resolvedPath = path.resolve(filePath);
13-
const cwd = process.cwd();
14-
return resolvedPath.startsWith(cwd);
12+
if (!filePath || typeof filePath !== 'string') return false;
13+
14+
// Check for null bytes and other control characters
15+
if (/[\0]/.test(filePath)) return false;
16+
17+
try {
18+
path.resolve(filePath);
19+
return true;
20+
} catch (error) {
21+
return false;
22+
}
1523
}
1624

1725
// Add URL validation helper
@@ -25,12 +33,18 @@ function isValidGitUrl(url) {
2533

2634
// Add branch name validation helper
2735
function isValidBranchName(branch) {
36+
if (typeof branch !== 'string') return false;
37+
38+
// Check for consecutive dots
39+
if (branch.includes('..')) return false;
40+
41+
// Check other branch name rules
2842
// Branch names can contain alphanumeric, -, _, /, and .
2943
// Cannot start with - or .
3044
// Cannot contain consecutive dots
3145
// Cannot contain control characters or spaces
32-
const validBranchPattern = /^[a-zA-Z0-9][a-zA-Z0-9\-_/.]*$/;
33-
return typeof branch === 'string' && validBranchPattern.test(branch);
46+
const validBranchPattern = /^[a-zA-Z0-9][a-zA-Z0-9_/.-]*$/;
47+
return validBranchPattern.test(branch);
3448
}
3549

3650
class ConfigLoader extends EventEmitter {
@@ -161,6 +175,7 @@ class ConfigLoader extends EventEmitter {
161175
// Use OS-specific cache directory
162176
const paths = envPaths('git-proxy', { suffix: '' });
163177
const tempDir = path.join(paths.cache, 'git-config-cache');
178+
164179
if (!isValidPath(tempDir)) {
165180
throw new Error('Invalid temporary directory path');
166181
}
@@ -227,3 +242,6 @@ function isObject(item) {
227242
}
228243

229244
module.exports = ConfigLoader;
245+
module.exports.isValidGitUrl = isValidGitUrl;
246+
module.exports.isValidPath = isValidPath;
247+
module.exports.isValidBranchName = isValidBranchName;

‎test/ConfigLoader.test.js‎

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const fs = require('fs');
33
const path = require('path');
44
const { expect } = chai;
55
const ConfigLoader = require('../src/config/ConfigLoader');
6+
const { isValidGitUrl, isValidPath, isValidBranchName } = ConfigLoader;
67
const sinon = require('sinon');
78
const axios = require('axios');
89

@@ -139,4 +140,275 @@ describe('ConfigLoader', () => {
139140
expect(spy.calledOnce).to.be.true; // Should only emit once
140141
});
141142
});
143+
144+
describe('initialize', () => {
145+
it('should initialize cache directory using env-paths', async () => {
146+
const configLoader = new ConfigLoader({});
147+
await configLoader.initialize();
148+
149+
// Check that cacheDir is set and is a string
150+
expect(configLoader.cacheDir).to.be.a('string');
151+
152+
// Check that it contains 'git-proxy' in the path
153+
expect(configLoader.cacheDir).to.include('git-proxy');
154+
155+
// On macOS, it should be in the Library/Caches directory
156+
// On Linux, it should be in the ~/.cache directory
157+
// On Windows, it should be in the AppData/Local directory
158+
if (process.platform === 'darwin') {
159+
expect(configLoader.cacheDir).to.include('Library/Caches');
160+
} else if (process.platform === 'linux') {
161+
expect(configLoader.cacheDir).to.include('.cache');
162+
} else if (process.platform === 'win32') {
163+
expect(configLoader.cacheDir).to.include('AppData/Local');
164+
}
165+
});
166+
167+
it('should create cache directory if it does not exist', async () => {
168+
const configLoader = new ConfigLoader({});
169+
await configLoader.initialize();
170+
171+
// Check if directory exists
172+
expect(fs.existsSync(configLoader.cacheDir)).to.be.true;
173+
});
174+
});
175+
176+
describe('loadRemoteConfig', () => {
177+
let configLoader;
178+
beforeEach(async () => {
179+
const configFilePath = path.join(__dirname, '..', 'proxy.config.json');
180+
const config = JSON.parse(fs.readFileSync(configFilePath));
181+
182+
config.configurationSources.enabled = true;
183+
configLoader = new ConfigLoader(config);
184+
await configLoader.initialize();
185+
});
186+
187+
it('should load configuration from git repository', async function () {
188+
// eslint-disable-next-line no-invalid-this
189+
this.timeout(10000);
190+
191+
const source = {
192+
type: 'git',
193+
repository: 'https://github.com/finos/git-proxy.git',
194+
path: 'proxy.config.json',
195+
branch: 'main',
196+
};
197+
198+
const config = await configLoader.loadFromGit(source);
199+
200+
// Verify the loaded config has expected structure
201+
expect(config).to.be.an('object');
202+
expect(config).to.have.property('proxyUrl');
203+
expect(config).to.have.property('cookieSecret');
204+
});
205+
206+
it('should throw error for invalid configuration file path', async function () {
207+
const source = {
208+
type: 'git',
209+
repository: 'https://github.com/finos/git-proxy.git',
210+
path: '\0', // Invalid path
211+
branch: 'main',
212+
};
213+
214+
try {
215+
await configLoader.loadFromGit(source);
216+
throw new Error('Expected error was not thrown');
217+
} catch (error) {
218+
expect(error.message).to.equal('Invalid configuration file path in repository');
219+
}
220+
});
221+
222+
it('should load configuration from http', async function () {
223+
// eslint-disable-next-line no-invalid-this
224+
this.timeout(10000);
225+
226+
const source = {
227+
type: 'http',
228+
url: 'https://raw.githubusercontent.com/finos/git-proxy/refs/heads/main/proxy.config.json',
229+
};
230+
231+
const config = await configLoader.loadFromHttp(source);
232+
233+
// Verify the loaded config has expected structure
234+
expect(config).to.be.an('object');
235+
expect(config).to.have.property('proxyUrl');
236+
expect(config).to.have.property('cookieSecret');
237+
});
238+
});
239+
240+
describe('deepMerge', () => {
241+
let configLoader;
242+
243+
beforeEach(() => {
244+
configLoader = new ConfigLoader({});
245+
});
246+
247+
it('should merge simple objects', () => {
248+
const target = { a: 1, b: 2 };
249+
const source = { b: 3, c: 4 };
250+
251+
const result = configLoader.deepMerge(target, source);
252+
253+
expect(result).to.deep.equal({ a: 1, b: 3, c: 4 });
254+
});
255+
256+
it('should merge nested objects', () => {
257+
const target = {
258+
a: 1,
259+
b: { x: 1, y: 2 },
260+
c: { z: 3 },
261+
};
262+
const source = {
263+
b: { y: 4, w: 5 },
264+
c: { z: 6 },
265+
};
266+
267+
const result = configLoader.deepMerge(target, source);
268+
269+
expect(result).to.deep.equal({
270+
a: 1,
271+
b: { x: 1, y: 4, w: 5 },
272+
c: { z: 6 },
273+
});
274+
});
275+
276+
it('should handle arrays by replacing them', () => {
277+
const target = {
278+
a: [1, 2, 3],
279+
b: { items: [4, 5] },
280+
};
281+
const source = {
282+
a: [7, 8],
283+
b: { items: [9] },
284+
};
285+
286+
const result = configLoader.deepMerge(target, source);
287+
288+
expect(result).to.deep.equal({
289+
a: [7, 8],
290+
b: { items: [9] },
291+
});
292+
});
293+
294+
it('should handle null and undefined values', () => {
295+
const target = {
296+
a: 1,
297+
b: null,
298+
c: undefined,
299+
};
300+
const source = {
301+
a: null,
302+
b: 2,
303+
c: 3,
304+
};
305+
306+
const result = configLoader.deepMerge(target, source);
307+
308+
expect(result).to.deep.equal({
309+
a: null,
310+
b: 2,
311+
c: 3,
312+
});
313+
});
314+
315+
it('should handle empty objects', () => {
316+
const target = {};
317+
const source = { a: 1, b: { c: 2 } };
318+
319+
const result = configLoader.deepMerge(target, source);
320+
321+
expect(result).to.deep.equal({ a: 1, b: { c: 2 } });
322+
});
323+
324+
it('should not modify the original objects', () => {
325+
const target = { a: 1, b: { c: 2 } };
326+
const source = { b: { c: 3 } };
327+
const originalTarget = { ...target };
328+
const originalSource = { ...source };
329+
330+
configLoader.deepMerge(target, source);
331+
332+
expect(target).to.deep.equal(originalTarget);
333+
expect(source).to.deep.equal(originalSource);
334+
});
335+
});
336+
});
337+
338+
describe('Validation Helpers', () => {
339+
describe('isValidGitUrl', () => {
340+
it('should validate git URLs correctly', () => {
341+
// Valid URLs
342+
expect(isValidGitUrl('git://github.com/user/repo.git')).to.be.true;
343+
expect(isValidGitUrl('https://github.com/user/repo.git')).to.be.true;
344+
expect(isValidGitUrl('ssh://git@github.com/user/repo.git')).to.be.true;
345+
expect(isValidGitUrl('user@github.com:user/repo.git')).to.be.true;
346+
347+
// Invalid URLs
348+
expect(isValidGitUrl('not-a-git-url')).to.be.false;
349+
expect(isValidGitUrl('http://github.com/user/repo')).to.be.false;
350+
expect(isValidGitUrl('')).to.be.false;
351+
expect(isValidGitUrl(null)).to.be.false;
352+
expect(isValidGitUrl(undefined)).to.be.false;
353+
expect(isValidGitUrl(123)).to.be.false;
354+
});
355+
});
356+
357+
describe('isValidPath', () => {
358+
it('should validate file paths correctly', () => {
359+
const cwd = process.cwd();
360+
361+
// Valid paths
362+
expect(isValidPath(path.join(cwd, 'config.json'))).to.be.true;
363+
expect(isValidPath(path.join(cwd, 'subfolder/config.json'))).to.be.true;
364+
expect(isValidPath('/etc/passwd')).to.be.true;
365+
expect(isValidPath('../config.json')).to.be.true;
366+
367+
// Invalid paths
368+
expect(isValidPath('')).to.be.false;
369+
expect(isValidPath(null)).to.be.false;
370+
expect(isValidPath(undefined)).to.be.false;
371+
372+
// Additional edge cases
373+
expect(isValidPath({})).to.be.false;
374+
expect(isValidPath([])).to.be.false;
375+
expect(isValidPath(123)).to.be.false;
376+
expect(isValidPath(true)).to.be.false;
377+
expect(isValidPath('\0invalid')).to.be.false;
378+
expect(isValidPath('\u0000')).to.be.false;
379+
});
380+
381+
it('should handle path resolution errors', () => {
382+
// Mock path.resolve to throw an error
383+
const originalResolve = path.resolve;
384+
path.resolve = () => {
385+
throw new Error('Mock path resolution error');
386+
};
387+
388+
expect(isValidPath('some/path')).to.be.false;
389+
390+
// Restore original path.resolve
391+
path.resolve = originalResolve;
392+
});
393+
});
394+
395+
describe('isValidBranchName', () => {
396+
it('should validate git branch names correctly', () => {
397+
// Valid branch names
398+
expect(isValidBranchName('main')).to.be.true;
399+
expect(isValidBranchName('feature/new-feature')).to.be.true;
400+
expect(isValidBranchName('release-1.0')).to.be.true;
401+
expect(isValidBranchName('fix_123')).to.be.true;
402+
expect(isValidBranchName('user/feature/branch')).to.be.true;
403+
404+
// // Invalid branch names
405+
expect(isValidBranchName('.invalid')).to.be.false;
406+
expect(isValidBranchName('-invalid')).to.be.false;
407+
expect(isValidBranchName('branch with spaces')).to.be.false;
408+
expect(isValidBranchName('')).to.be.false;
409+
expect(isValidBranchName(null)).to.be.false;
410+
expect(isValidBranchName(undefined)).to.be.false;
411+
expect(isValidBranchName('branch..name')).to.be.false;
412+
});
413+
});
142414
});

0 commit comments

Comments
 (0)