diff --git a/src/proxy/processors/push-action/checkAuthorEmails.ts b/src/proxy/processors/push-action/checkAuthorEmails.ts index e8d51f09d..27240e9da 100644 --- a/src/proxy/processors/push-action/checkAuthorEmails.ts +++ b/src/proxy/processors/push-action/checkAuthorEmails.ts @@ -39,8 +39,6 @@ const exec = async (req: any, action: Action): Promise => { const illegalEmails = uniqueAuthorEmails.filter((email) => !isEmailAllowed(email)); if (illegalEmails.length > 0) { - console.log(`The following commit author e-mails are illegal: ${illegalEmails}`); - step.error = true; step.log(`The following commit author e-mails are illegal: ${illegalEmails}`); step.setError( @@ -51,7 +49,7 @@ const exec = async (req: any, action: Action): Promise => { return action; } - console.log(`The following commit author e-mails are legal: ${uniqueAuthorEmails}`); + step.log(`The following commit author e-mails are legal: ${uniqueAuthorEmails}`); action.addStep(step); return action; }; diff --git a/src/proxy/processors/push-action/checkCommitMessages.ts b/src/proxy/processors/push-action/checkCommitMessages.ts index 7eb9f6cad..d131a818d 100644 --- a/src/proxy/processors/push-action/checkCommitMessages.ts +++ b/src/proxy/processors/push-action/checkCommitMessages.ts @@ -1,20 +1,18 @@ import { Action, Step } from '../../actions'; import { getCommitConfig } from '../../../config'; -const isMessageAllowed = (commitMessage: string): boolean => { +const isMessageAllowed = (commitMessage: any): string | null => { try { const commitConfig = getCommitConfig(); // Commit message is empty, i.e. '', null or undefined if (!commitMessage) { - console.log('No commit message included...'); - return false; + return 'No commit message included...'; } // Validation for configured block pattern(s) check... if (typeof commitMessage !== 'string') { - console.log('A non-string value has been captured for the commit message...'); - return false; + return 'A non-string value has been captured for the commit message...'; } // Configured blocked literals and patterns @@ -36,15 +34,13 @@ const isMessageAllowed = (commitMessage: string): boolean => { // Commit message matches configured block pattern(s) if (literalMatches.length || patternMatches.length) { - console.log('Commit message is blocked via configured literals/patterns...'); - return false; + return 'Commit message is blocked via configured literals/patterns...'; } } catch (error) { - console.log('Invalid regex pattern...'); - return false; + return 'Invalid regex pattern...'; } - return true; + return null; }; // Execute if the repo is approved @@ -53,13 +49,18 @@ const exec = async (req: any, action: Action): Promise => { const uniqueCommitMessages = [...new Set(action.commitData?.map((commit) => commit.message))]; - const illegalMessages = uniqueCommitMessages.filter((message) => !isMessageAllowed(message)); + const illegalMessages = uniqueCommitMessages.filter( + (message) => isMessageAllowed(message) !== null, + ); if (illegalMessages.length > 0) { - console.log(`The following commit messages are illegal: ${illegalMessages}`); + illegalMessages.forEach((message) => { + const error = isMessageAllowed(message); + step.log( + `Illegal commit message detected: "${message}" - Reason: ${error ?? 'Unknown reason'}`, + ); + }); - step.error = true; - step.log(`The following commit messages are illegal: ${illegalMessages}`); step.setError( `\n\n\nYour push has been blocked.\nPlease ensure your commit message(s) does not contain sensitive information or URLs.\n\nThe following commit messages are illegal: ${JSON.stringify(illegalMessages)}\n\n`, ); @@ -68,7 +69,7 @@ const exec = async (req: any, action: Action): Promise => { return action; } - console.log(`The following commit messages are legal: ${uniqueCommitMessages}`); + step.log(`The following commit messages are legal: ${JSON.stringify(uniqueCommitMessages)}`); action.addStep(step); return action; }; diff --git a/src/proxy/processors/push-action/checkEmptyBranch.ts b/src/proxy/processors/push-action/checkEmptyBranch.ts index 86f6b5138..a6dac8419 100644 --- a/src/proxy/processors/push-action/checkEmptyBranch.ts +++ b/src/proxy/processors/push-action/checkEmptyBranch.ts @@ -2,7 +2,7 @@ import { Action, Step } from '../../actions'; import simpleGit from 'simple-git'; import { EMPTY_COMMIT_HASH } from '../constants'; -const isEmptyBranch = async (action: Action) => { +const isEmptyBranch = async (action: Action, step: Step) => { if (action.commitFrom === EMPTY_COMMIT_HASH) { try { const git = simpleGit(`${action.proxyGitPath}/${action.repoName}`); @@ -10,7 +10,7 @@ const isEmptyBranch = async (action: Action) => { const type = await git.raw(['cat-file', '-t', action.commitTo || '']); return type.trim() === 'commit'; } catch (err) { - console.log(`Commit ${action.commitTo} not found: ${err}`); + step.log(`Commit ${action.commitTo} not found: ${err}`); } } @@ -24,7 +24,7 @@ const exec = async (req: any, action: Action): Promise => { return action; } - if (await isEmptyBranch(action)) { + if (await isEmptyBranch(action, step)) { step.setError('Push blocked: Empty branch. Please make a commit before pushing a new branch.'); action.addStep(step); step.error = true; diff --git a/src/proxy/processors/push-action/checkUserPushPermission.ts b/src/proxy/processors/push-action/checkUserPushPermission.ts index 83f16c968..7f8b3f910 100644 --- a/src/proxy/processors/push-action/checkUserPushPermission.ts +++ b/src/proxy/processors/push-action/checkUserPushPermission.ts @@ -31,8 +31,8 @@ const validateUser = async (userEmail: string, action: Action, step: Step): Prom const list = await getUsers({ email: userEmail }); if (list.length > 1) { - console.error(`Multiple users found with email address ${userEmail}, ending`); step.error = true; + step.log(`Multiple users found with email address ${userEmail}, ending`); step.log( `Multiple Users have email <${userEmail}> so we cannot uniquely identify the user, ending`, ); @@ -43,15 +43,15 @@ const validateUser = async (userEmail: string, action: Action, step: Step): Prom action.addStep(step); return action; } else if (list.length == 0) { - console.error(`No user with email address ${userEmail} found`); + step.log(`No user with email address ${userEmail} found`); } else { isUserAllowed = await isUserPushAllowed(action.url, list[0].username); } - console.log(`User ${userEmail} permission on Repo ${action.url} : ${isUserAllowed}`); + step.log(`User ${userEmail} permission on Repo ${action.url} : ${isUserAllowed}`); if (!isUserAllowed) { - console.log('User not allowed to Push'); + step.log('User not allowed to Push'); step.error = true; step.log(`User ${userEmail} is not allowed to push on repo ${action.url}, ending`); step.setError( diff --git a/src/proxy/processors/push-action/clearBareClone.ts b/src/proxy/processors/push-action/clearBareClone.ts index 91f7f5b22..f4df63c94 100644 --- a/src/proxy/processors/push-action/clearBareClone.ts +++ b/src/proxy/processors/push-action/clearBareClone.ts @@ -9,7 +9,7 @@ const exec = async (req: any, action: Action): Promise => { if (err) { throw err; } - console.log(`.remote is deleted!`); + step.log(`.remote is deleted!`); }); action.addStep(step); diff --git a/src/proxy/processors/push-action/gitleaks.ts b/src/proxy/processors/push-action/gitleaks.ts index 1cf5b2236..117c261a4 100644 --- a/src/proxy/processors/push-action/gitleaks.ts +++ b/src/proxy/processors/push-action/gitleaks.ts @@ -116,7 +116,7 @@ const exec = async (req: any, action: Action): Promise => { try { config = await getPluginConfig(); } catch (e) { - console.error('failed to get gitleaks config, please fix the error:', e); + step.log(`failed to get gitleaks config, please fix the error: ${String(e)}`); action.error = true; step.setError('failed setup gitleaks, please contact an administrator\n'); action.addStep(step); @@ -124,14 +124,14 @@ const exec = async (req: any, action: Action): Promise => { } if (!config.enabled) { - console.log('gitleaks is disabled, skipping'); + step.log('gitleaks is disabled, skipping'); action.addStep(step); return action; } const { commitFrom, commitTo } = action; const workingDir = `${action.proxyGitPath}/${action.repoName}`; - console.log(`Scanning range with gitleaks: ${commitFrom}:${commitTo}`, workingDir); + step.log(`Scanning range with gitleaks: ${commitFrom}:${commitTo}, ${workingDir}`); try { const gitRootCommit = await runCommand(workingDir, 'git', [ @@ -171,8 +171,8 @@ const exec = async (req: any, action: Action): Promise => { step.setError('\n' + gitleaks.stdout + gitleaks.stderr); } } else { - console.log('succeeded'); - console.log(gitleaks.stderr); + step.log('succeeded'); + step.log(gitleaks.stderr); } } catch (e) { action.error = true; diff --git a/test/processors/checkAuthorEmails.test.ts b/test/processors/checkAuthorEmails.test.ts index 6e928005e..ecdf626db 100644 --- a/test/processors/checkAuthorEmails.test.ts +++ b/test/processors/checkAuthorEmails.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { exec } from '../../src/proxy/processors/push-action/checkAuthorEmails'; -import { Action } from '../../src/proxy/actions'; +import { Action, Step } from '../../src/proxy/actions'; import * as configModule from '../../src/config'; import * as validator from 'validator'; import { CommitData } from '../../src/proxy/processors/types'; @@ -47,7 +47,7 @@ describe('checkAuthorEmails', () => { }); // mock console.log to suppress output and verify calls - consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + consoleLogSpy = vi.spyOn(Step.prototype, 'log').mockImplementation(() => {}); // setup mock action mockAction = { diff --git a/test/processors/checkCommitMessages.test.ts b/test/processors/checkCommitMessages.test.ts index c1fff3c02..f26bbe2cd 100644 --- a/test/processors/checkCommitMessages.test.ts +++ b/test/processors/checkCommitMessages.test.ts @@ -46,7 +46,7 @@ describe('checkCommitMessages', () => { const result = await exec({}, action); expect(result.steps[0].error).toBe(true); - expect(consoleLogSpy).toHaveBeenCalledWith('No commit message included...'); + expect(result.steps[0].logs.join('\n')).toContain('No commit message included...'); }); it('should block null commit messages', async () => { @@ -74,7 +74,7 @@ describe('checkCommitMessages', () => { const result = await exec({}, action); expect(result.steps[0].error).toBe(true); - expect(consoleLogSpy).toHaveBeenCalledWith( + expect(JSON.stringify(result.steps[0])).toContain( 'A non-string value has been captured for the commit message...', ); }); @@ -106,7 +106,7 @@ describe('checkCommitMessages', () => { const result = await exec({}, action); expect(result.steps[0].error).toBe(true); - expect(consoleLogSpy).toHaveBeenCalledWith( + expect(JSON.stringify(result.steps[0])).toContain( 'Commit message is blocked via configured literals/patterns...', ); }); @@ -241,8 +241,8 @@ describe('checkCommitMessages', () => { const result = await exec({}, action); expect(result.steps[0].error).toBe(false); - expect(consoleLogSpy).toHaveBeenCalledWith( - expect.stringContaining('The following commit messages are legal:'), + expect(JSON.stringify(result.steps[0])).toContain( + 'The following commit messages are legal:', ); }); diff --git a/test/processors/checkUserPushPermission.test.ts b/test/processors/checkUserPushPermission.test.ts index 6e029a321..222f31d5d 100644 --- a/test/processors/checkUserPushPermission.test.ts +++ b/test/processors/checkUserPushPermission.test.ts @@ -15,14 +15,10 @@ import { exec } from '../../src/proxy/processors/push-action/checkUserPushPermis describe('checkUserPushPermission', () => { let getUsersMock: Mock; let isUserPushAllowedMock: Mock; - let consoleLogSpy: ReturnType; - let consoleErrorSpy: ReturnType; beforeEach(() => { getUsersMock = vi.mocked(getUsers); isUserPushAllowedMock = vi.mocked(isUserPushAllowed); - consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); }); afterEach(() => { @@ -62,7 +58,7 @@ describe('checkUserPushPermission', () => { expect(stepLogSpy).toHaveBeenLastCalledWith( 'User db-user@test.com is allowed to push on repo https://github.com/finos/git-proxy.git', ); - expect(consoleLogSpy).toHaveBeenLastCalledWith( + expect(stepLogSpy).toHaveBeenCalledWith( 'User db-user@test.com permission on Repo https://github.com/finos/git-proxy.git : true', ); }); @@ -81,7 +77,7 @@ describe('checkUserPushPermission', () => { `Your push has been blocked (db-user@test.com is not allowed to push on repo https://github.com/finos/git-proxy.git)`, ); expect(result.steps[0].errorMessage).toContain('Your push has been blocked'); - expect(consoleLogSpy).toHaveBeenLastCalledWith('User not allowed to Push'); + expect(stepLogSpy).toHaveBeenCalledWith('User not allowed to Push'); }); it('should reject push when no user found for git account', async () => { @@ -110,7 +106,7 @@ describe('checkUserPushPermission', () => { expect(stepLogSpy).toHaveBeenLastCalledWith( 'Your push has been blocked (there are multiple users with email db-user@test.com)', ); - expect(consoleErrorSpy).toHaveBeenLastCalledWith( + expect(stepLogSpy).toHaveBeenCalledWith( 'Multiple users found with email address db-user@test.com, ending', ); }); diff --git a/test/processors/gitLeaks.test.ts b/test/processors/gitLeaks.test.ts index 3e9d9234a..3f481ee21 100644 --- a/test/processors/gitLeaks.test.ts +++ b/test/processors/gitLeaks.test.ts @@ -43,6 +43,7 @@ describe('gitleaks', () => { let getAPIs: any; let fsModule: any; let spawn: any; + const stepLogSpy = vi.spyOn(Step.prototype, 'log'); beforeEach(async () => { vi.clearAllMocks(); @@ -89,9 +90,8 @@ describe('gitleaks', () => { expect(stepSpy).toHaveBeenCalledWith( 'failed setup gitleaks, please contact an administrator\n', ); - expect(errorStub).toHaveBeenCalledWith( - 'failed to get gitleaks config, please fix the error:', - expect.any(Error), + expect(stepLogSpy).toHaveBeenCalledWith( + expect.stringContaining('failed to get gitleaks config, please fix the error:'), ); }); @@ -103,7 +103,7 @@ describe('gitleaks', () => { expect(result.error).toBe(false); expect(result.steps).toHaveLength(1); expect(result.steps[0].error).toBe(false); - expect(logStub).toHaveBeenCalledWith('gitleaks is disabled, skipping'); + expect(JSON.stringify(result.steps[0])).toContain('gitleaks is disabled, skipping'); }); it('should handle successful scan with no findings', async () => { @@ -140,12 +140,13 @@ describe('gitleaks', () => { } as any); const result = await exec(req, action); + const stepRes = JSON.stringify(result.steps[0]); expect(result.error).toBe(false); expect(result.steps).toHaveLength(1); expect(result.steps[0].error).toBe(false); - expect(logStub).toHaveBeenCalledWith('succeeded'); - expect(logStub).toHaveBeenCalledWith('No leaks found'); + expect(stepRes).toContain('succeeded'); + expect(stepRes).toContain('No leaks found'); }); it('should handle scan with findings', async () => {