diff --git a/packages/git-proxy-cli/index.ts b/packages/git-proxy-cli/index.ts index 31ebc8a4c..f90456122 100644 --- a/packages/git-proxy-cli/index.ts +++ b/packages/git-proxy-cli/index.ts @@ -1,5 +1,5 @@ #!/usr/bin/env node -import axios from 'axios'; +import axios, { isAxiosError } from 'axios'; import yargs from 'yargs/yargs'; import { hideBin } from 'yargs/helpers'; import fs from 'fs'; @@ -7,6 +7,7 @@ import util from 'util'; import { PushQuery } from '@finos/git-proxy/db'; import { Action } from '@finos/git-proxy/proxy/actions'; +import { handleAndLogError } from '@finos/git-proxy/utils/errors'; const GIT_PROXY_COOKIE_FILE = 'git-proxy-cookie'; // GitProxy UI HOST and PORT (configurable via environment variable) @@ -47,12 +48,12 @@ async function login(username: string, password: string) { const user = `"${response.data.username}" <${response.data.email}>`; const isAdmin = response.data.admin ? ' (admin)' : ''; console.log(`Login ${user}${isAdmin}: OK`); - } catch (error: any) { - if (error.response) { + } catch (error: unknown) { + if (isAxiosError(error) && error.response) { console.error(`Error: Login '${username}': '${error.response.status}'`); process.exitCode = 1; } else { - console.error(`Error: Login '${username}': '${error.message}'`); + handleAndLogError(error, `Error: Login '${username}'`); process.exitCode = 2; } } @@ -165,8 +166,8 @@ async function getGitPushes(filters: Partial) { }); console.log(util.inspect(records, false, null, false)); - } catch (error: any) { - console.error(`Error: List: '${error.message}'`); + } catch (error: unknown) { + handleAndLogError(error, 'Error: List'); process.exitCode = 2; } } @@ -207,23 +208,21 @@ async function authoriseGitPush(id: string) { ); console.log(`Authorise: ID: '${id}': OK`); - } catch (error: any) { - // default error - let errorMessage = `Error: Authorise: '${error.message}'`; - process.exitCode = 2; - - if (error.response) { + } catch (error: unknown) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: - errorMessage = 'Error: Authorise: Authentication required'; + console.error('Error: Authorise: Authentication required'); process.exitCode = 3; break; case 404: - errorMessage = `Error: Authorise: ID: '${id}': Not Found`; + console.error(`Error: Authorise: ID: '${id}': Not Found`); process.exitCode = 4; } + } else { + handleAndLogError(error, `Error: Authorise: '${id}'`); + process.exitCode = 2; } - console.error(errorMessage); } } @@ -254,23 +253,21 @@ async function rejectGitPush(id: string) { ); console.log(`Reject: ID: '${id}': OK`); - } catch (error: any) { - // default error - let errorMessage = `Error: Reject: '${error.message}'`; - process.exitCode = 2; - - if (error.response) { + } catch (error: unknown) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: - errorMessage = 'Error: Reject: Authentication required'; + console.error('Error: Reject: Authentication required'); process.exitCode = 3; break; case 404: - errorMessage = `Error: Reject: ID: '${id}': Not Found`; + console.error(`Error: Reject: ID: '${id}': Not Found`); process.exitCode = 4; } + } else { + handleAndLogError(error, `Error: Reject: '${id}'`); + process.exitCode = 2; } - console.error(errorMessage); } } @@ -301,23 +298,21 @@ async function cancelGitPush(id: string) { ); console.log(`Cancel: ID: '${id}': OK`); - } catch (error: any) { - // default error - let errorMessage = `Error: Cancel: '${error.message}'`; - process.exitCode = 2; - - if (error.response) { + } catch (error: unknown) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: - errorMessage = 'Error: Cancel: Authentication required'; + console.error('Error: Cancel: Authentication required'); process.exitCode = 3; break; case 404: - errorMessage = `Error: Cancel: ID: '${id}': Not Found`; + console.error(`Error: Cancel: ID: '${id}': Not Found`); process.exitCode = 4; } + } else { + handleAndLogError(error, `Error: Cancel: '${id}'`); + process.exitCode = 2; } - console.error(errorMessage); } } @@ -338,8 +333,8 @@ async function logout() { headers: { Cookie: cookies }, }, ); - } catch (error: any) { - console.log(`Warning: Logout: '${error.message}'`); + } catch (error: unknown) { + handleAndLogError(error, 'Warning: Logout'); } } @@ -362,10 +357,9 @@ async function reloadConfig() { await axios.post(`${baseUrl}/api/v1/admin/reload-config`, {}, { headers: { Cookie: cookies } }); console.log('Configuration reloaded successfully'); - } catch (error: any) { - const errorMessage = `Error: Reload config: '${error.message}'`; + } catch (error: unknown) { + handleAndLogError(error, 'Error: Reload config'); process.exitCode = 2; - console.error(errorMessage); } } @@ -408,23 +402,22 @@ async function createUser( ); console.log(`User '${username}' created successfully`); - } catch (error: any) { - let errorMessage = `Error: Create User: '${error.message}'`; - process.exitCode = 2; - - if (error.response) { + } catch (error: unknown) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: - errorMessage = 'Error: Create User: Authentication required'; + console.error('Error: Create User: Authentication required'); process.exitCode = 3; break; case 400: - errorMessage = `Error: Create User: ${error.response.data.message}`; + console.error(`Error: Create User: ${error.response.data.message}`); process.exitCode = 4; break; } + } else { + handleAndLogError(error, `Error: Create User: '${username}'`); + process.exitCode = 2; } - console.error(errorMessage); } } diff --git a/packages/git-proxy-cli/test/testCli.test.ts b/packages/git-proxy-cli/test/testCli.test.ts index 3e5545d1f..6480e0f60 100644 --- a/packages/git-proxy-cli/test/testCli.test.ts +++ b/packages/git-proxy-cli/test/testCli.test.ts @@ -3,8 +3,8 @@ import path from 'path'; import { describe, it, beforeAll, afterAll } from 'vitest'; import { setConfigFile } from '../../../src/config/file'; - -import { Repo } from '../../../src/db/types'; +import { SAMPLE_REPO } from '../../../src/proxy/processors/constants'; +import { handleAndLogError } from '../../../src/utils/errors'; setConfigFile(path.join(process.cwd(), 'test', 'testCli.proxy.config.json')); @@ -14,6 +14,7 @@ const GHOST_PUSH_ID = '0000000000000000000000000000000000000000__79b4d8953cbc324bcc1eb53d6412ff89666c241f'; // repo for test cases const TEST_REPO_CONFIG = { + ...SAMPLE_REPO, project: 'finos', name: 'git-proxy-test', url: 'https://github.com/finos/git-proxy-test.git', @@ -220,7 +221,7 @@ describe('test git-proxy-cli', function () { const pushId = `auth000000000000000000000000000000000000__${Date.now()}`; beforeAll(async function () { - await helper.addRepoToDb(TEST_REPO_CONFIG as Repo); + await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); await helper.addGitPushToDb(pushId, TEST_REPO_CONFIG.url, TEST_USER, TEST_EMAIL); }); @@ -297,7 +298,7 @@ describe('test git-proxy-cli', function () { const pushId = `cancel0000000000000000000000000000000000__${Date.now()}`; beforeAll(async function () { - await helper.addRepoToDb(TEST_REPO_CONFIG as Repo); + await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); await helper.addGitPushToDb(pushId, TEST_USER, TEST_EMAIL, TEST_REPO); }); @@ -420,7 +421,7 @@ describe('test git-proxy-cli', function () { const pushId = `reject0000000000000000000000000000000000__${Date.now()}`; beforeAll(async function () { - await helper.addRepoToDb(TEST_REPO_CONFIG as Repo); + await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); await helper.addGitPushToDb(pushId, TEST_REPO_CONFIG.url, TEST_USER, TEST_EMAIL); }); @@ -582,8 +583,8 @@ describe('test git-proxy-cli', function () { // Clean up the created user try { await helper.removeUserFromDb(uniqueUsername); - } catch (error: any) { - // Ignore cleanup errors + } catch (error: unknown) { + handleAndLogError(error, 'Error cleaning up user'); } } }); @@ -612,8 +613,8 @@ describe('test git-proxy-cli', function () { // Clean up the created user try { await helper.removeUserFromDb(uniqueUsername); - } catch (error: any) { - console.error('Error cleaning up user', error); + } catch (error: unknown) { + handleAndLogError(error, 'Error cleaning up user'); } } }); @@ -625,7 +626,7 @@ describe('test git-proxy-cli', function () { const pushId = `0000000000000000000000000000000000000000__${Date.now()}`; beforeAll(async function () { - await helper.addRepoToDb(TEST_REPO_CONFIG as Repo); + await helper.addRepoToDb(TEST_REPO_CONFIG); await helper.addUserToDb(TEST_USER, TEST_PASSWORD, TEST_EMAIL, TEST_GIT_ACCOUNT); await helper.addGitPushToDb(pushId, TEST_REPO_CONFIG.url, TEST_USER, TEST_EMAIL); }); diff --git a/packages/git-proxy-cli/test/testCliUtils.ts b/packages/git-proxy-cli/test/testCliUtils.ts index a0b19ceb0..320642527 100644 --- a/packages/git-proxy-cli/test/testCliUtils.ts +++ b/packages/git-proxy-cli/test/testCliUtils.ts @@ -2,6 +2,7 @@ import fs from 'fs'; import util from 'util'; import { exec } from 'child_process'; import { expect } from 'vitest'; +import { Request } from 'express'; import Proxy from '../../../src/proxy'; import { Action } from '../../../src/proxy/actions/Action'; @@ -10,12 +11,24 @@ import { exec as execProcessor } from '../../../src/proxy/processors/push-action import * as db from '../../../src/db'; import { Repo } from '../../../src/db/types'; import service from '../../../src/service'; +import { CommitData } from '../../../src/proxy/processors/types'; const execAsync = util.promisify(exec); // cookie file name const GIT_PROXY_COOKIE_FILE = 'git-proxy-cookie'; +/** + * Type guard to check if error is from child_process exec + */ +function isExecError(error: unknown): error is Error & { + code: number; + stdout: string; + stderr: string; +} { + return error instanceof Error && 'code' in error && 'stdout' in error && 'stderr' in error; +} + /** * @async * @param {string} cli - The CLI command to be executed. @@ -54,28 +67,29 @@ async function runCli( expect(stderr).toContain(expectedErrorMessage); }); } - } catch (error: any) { - const exitCode = error.code; - if (!exitCode) { - // an AssertionError is thrown from failing some of the expectations - // in the 'try' block: forward it to Mocha to process + } catch (error: unknown) { + if (isExecError(error)) { + const exitCode = error.code; + + if (debug) { + console.log(`error.stdout: ${error.stdout}`); + console.log(`error.stderr: ${error.stderr}`); + } + expect(exitCode).toEqual(expectedExitCode); + if (expectedMessages) { + expectedMessages.forEach((expectedMessage) => { + expect(error.stdout).toContain(expectedMessage); + }); + } + if (expectedErrorMessages) { + expectedErrorMessages.forEach((expectedErrorMessage) => { + expect(error.stderr).toContain(expectedErrorMessage); + }); + } + } else { + // Assertion error, forward to Vitest to process throw error; } - if (debug) { - console.log(`error.stdout: ${error.stdout}`); - console.log(`error.stderr: ${error.stderr}`); - } - expect(exitCode).toEqual(expectedExitCode); - if (expectedMessages) { - expectedMessages.forEach((expectedMessage) => { - expect(error.stdout).toContain(expectedMessage); - }); - } - if (expectedErrorMessages) { - expectedErrorMessages.forEach((expectedErrorMessage) => { - expect(error.stderr).toContain(expectedErrorMessage); - }); - } } finally { if (debug) { console.log(`cli: '${cli}': done`); @@ -214,7 +228,7 @@ async function addGitPushToDb( `\n\n\nGitProxy has received your push:\n\nhttp://localhost:8080/requests/${id}\n\n\n`, // blockedMessage null, // content ); - const commitData = []; + const commitData: CommitData[] = []; commitData.push({ tree: 'tree test', parent: 'parent', @@ -223,10 +237,11 @@ async function addGitPushToDb( message: 'message', authorEmail: 'authorEmail', committerEmail: 'committerEmail', + commitTimestamp: '1234567890', }); action.commitData = commitData; action.addStep(step); - const result = await execProcessor(null, action); + const result = await execProcessor({} as Request, action); if (debug) { console.log(`New git push added to DB: ${util.inspect(result)}`); } diff --git a/src/config/ConfigLoader.ts b/src/config/ConfigLoader.ts index 9c4d70625..6eb50ea2a 100644 --- a/src/config/ConfigLoader.ts +++ b/src/config/ConfigLoader.ts @@ -8,6 +8,7 @@ import envPaths from 'env-paths'; import { GitProxyConfig } from './generated/config'; import { Configuration, ConfigurationSource, FileSource, HttpSource, GitSource } from './types'; import { loadConfig, validateConfig } from './validators'; +import { handleAndLogError, handleAndThrowError } from '../utils/errors'; const execFileAsync = promisify(execFile); @@ -21,7 +22,7 @@ function isValidPath(filePath: string): boolean { try { path.resolve(filePath); return true; - } catch (error) { + } catch (error: unknown) { return false; } } @@ -80,8 +81,8 @@ export class ConfigLoader extends EventEmitter { fs.mkdirSync(this.cacheDir, { recursive: true }); console.log(`Created cache directory at ${this.cacheDir}`); return true; - } catch (err) { - console.error('Failed to create cache directory:', err); + } catch (error: unknown) { + handleAndLogError(error, 'Failed to create cache directory'); return false; } } @@ -154,8 +155,8 @@ export class ConfigLoader extends EventEmitter { try { console.log(`Loading configuration from ${source.type} source`); return await this.loadFromSource(source); - } catch (error: any) { - console.error(`Error loading from ${source.type} source:`, error.message); + } catch (error: unknown) { + handleAndLogError(error, `Error loading from ${source.type} source`); return null; } }), @@ -197,8 +198,8 @@ export class ConfigLoader extends EventEmitter { } else { console.log('Configuration has not changed, no update needed'); } - } catch (error: any) { - console.error('Error reloading configuration:', error); + } catch (error: unknown) { + handleAndLogError(error, 'Error reloading configuration'); this.emit('configurationError', error); } finally { this.isReloading = false; @@ -297,18 +298,16 @@ export class ConfigLoader extends EventEmitter { try { await execFileAsync('git', ['clone', source.repository, repoDir], execOptions); console.log('Repository cloned successfully'); - } catch (error: any) { - console.error('Failed to clone repository:', error.message); - throw new Error(`Failed to clone repository: ${error.message}`); + } catch (error: unknown) { + handleAndThrowError(error, 'Failed to clone repository'); } } else { console.log(`Pulling latest changes from ${source.repository}`); try { await execFileAsync('git', ['pull'], { cwd: repoDir }); console.log('Repository pulled successfully'); - } catch (error: any) { - console.error('Failed to pull repository:', error.message); - throw new Error(`Failed to pull repository: ${error.message}`); + } catch (error: unknown) { + handleAndThrowError(error, 'Failed to pull repository'); } } @@ -318,9 +317,8 @@ export class ConfigLoader extends EventEmitter { try { await execFileAsync('git', ['checkout', source.branch], { cwd: repoDir }); console.log(`Branch ${source.branch} checked out successfully`); - } catch (error: any) { - console.error(`Failed to checkout branch ${source.branch}:`, error.message); - throw new Error(`Failed to checkout branch ${source.branch}: ${error.message}`); + } catch (error: unknown) { + handleAndThrowError(error, `Failed to checkout branch ${source.branch}`); } } diff --git a/src/config/index.ts b/src/config/index.ts index 133750dbb..a3a81b338 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -7,6 +7,7 @@ import { Configuration } from './types'; import { serverConfig } from './env'; import { getConfigFile } from './file'; import { validateConfig } from './validators'; +import { handleAndLogError, handleAndThrowError } from '../utils/errors'; // Cache for current configuration let _currentConfig: GitProxyConfig | null = null; @@ -62,9 +63,8 @@ function loadFullConfiguration(): GitProxyConfig { // Don't use QuickType validation for partial configurations const rawUserConfig = JSON.parse(userConfigContent); userSettings = cleanUndefinedValues(rawUserConfig); - } catch (error) { - console.error(`Error loading user config from ${userConfigFile}:`, error); - throw error; + } catch (error: unknown) { + handleAndThrowError(error, `Error loading user config from ${userConfigFile}`); } } @@ -321,14 +321,14 @@ const handleConfigUpdate = async (newConfig: Configuration) => { await proxy.start(); console.log('Services restarted with new configuration'); - } catch (error) { - console.error('Failed to apply new configuration:', error); + } catch (error: unknown) { + handleAndLogError(error, 'Failed to apply new configuration'); // Attempt to restart with previous config try { const proxy = require('../proxy'); await proxy.start(); - } catch (startError) { - console.error('Failed to restart services:', startError); + } catch (startError: unknown) { + handleAndLogError(startError, 'Failed to restart services'); } } }; @@ -364,7 +364,7 @@ export const reloadConfiguration = async () => { try { initializeConfigLoader(); console.log('Configuration loaded successfully'); -} catch (error) { - console.error('Failed to load configuration:', error); +} catch (error: unknown) { + handleAndThrowError(error, 'Failed to load configuration'); throw error; } diff --git a/src/config/validators.ts b/src/config/validators.ts index 17da0617b..0c3f7425a 100644 --- a/src/config/validators.ts +++ b/src/config/validators.ts @@ -1,3 +1,4 @@ +import { getErrorMessage } from '../utils/errors'; import { Convert, GitProxyConfig } from './generated/config'; const validationChain = [validateCommitConfig]; @@ -103,9 +104,7 @@ export async function loadConfig( function parseGitProxyConfig(raw: string, context: string): GitProxyConfig { try { return Convert.toGitProxyConfig(raw); - } catch (error) { - throw new Error( - `Invalid configuration format in ${context}: ${error instanceof Error ? error.message : 'Unknown error'}`, - ); + } catch (error: unknown) { + throw new Error(`Invalid configuration format in ${context}: ${getErrorMessage}`); } } diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index 416845688..1472d4618 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -3,6 +3,8 @@ import Datastore from '@seald-io/nedb'; import { Action } from '../../proxy/actions/Action'; import { toClass } from '../helper'; import { PushQuery } from '../types'; +import { Attestation } from '../../proxy/processors/types'; +import { handleAndLogError } from '../../utils/errors'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day @@ -15,10 +17,10 @@ if (process.env.NODE_ENV === 'test') { } try { db.ensureIndex({ fieldName: 'id', unique: true }); -} catch (e) { - console.error( +} catch (error: unknown) { + handleAndLogError( + error, 'Failed to build a unique index of push id values. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ', - e, ); } db.setAutocompactionInterval(COMPACTION_INTERVAL); @@ -97,7 +99,10 @@ export const writeAudit = async (action: Action): Promise => { }); }; -export const authorise = async (id: string, attestation: any): Promise<{ message: string }> => { +export const authorise = async ( + id: string, + attestation?: Attestation, +): Promise<{ message: string }> => { const action = await getPush(id); if (!action) { throw new Error(`push ${id} not found`); @@ -111,7 +116,10 @@ export const authorise = async (id: string, attestation: any): Promise<{ message return { message: `authorised ${id}` }; }; -export const reject = async (id: string, attestation: any): Promise<{ message: string }> => { +export const reject = async ( + id: string, + attestation?: Attestation, +): Promise<{ message: string }> => { const action = await getPush(id); if (!action) { throw new Error(`push ${id} not found`); diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index fed991578..77c121622 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -3,6 +3,7 @@ import _ from 'lodash'; import { Repo, RepoQuery } from '../types'; import { toClass } from '../helper'; +import { handleAndLogError } from '../../utils/errors'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day @@ -15,10 +16,10 @@ if (process.env.NODE_ENV === 'test') { } try { db.ensureIndex({ fieldName: 'url', unique: true }); -} catch (e) { - console.error( +} catch (error: unknown) { + handleAndLogError( + error, 'Failed to build a unique index of Repository URLs. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ', - e, ); } diff --git a/src/db/file/users.ts b/src/db/file/users.ts index a39b5b170..bc2bafa03 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -2,6 +2,7 @@ import fs from 'fs'; import Datastore from '@seald-io/nedb'; import { User, UserQuery } from '../types'; +import { handleAndLogError } from '../../utils/errors'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day @@ -22,18 +23,18 @@ if (process.env.NODE_ENV === 'test') { // Using a unique constraint with the index try { db.ensureIndex({ fieldName: 'username', unique: true }); -} catch (e) { - console.error( +} catch (error: unknown) { + handleAndLogError( + error, 'Failed to build a unique index of usernames. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ', - e, ); } try { db.ensureIndex({ fieldName: 'email', unique: true }); -} catch (e) { - console.error( +} catch (error: unknown) { + handleAndLogError( + error, 'Failed to build a unique index of user email addresses. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ', - e, ); } db.setAutocompactionInterval(COMPACTION_INTERVAL); diff --git a/src/db/index.ts b/src/db/index.ts index f71179cf3..5a727d94f 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -6,6 +6,7 @@ import * as mongo from './mongo'; import * as neDb from './file'; import { Action } from '../proxy/actions/Action'; import MongoDBStore from 'connect-mongo'; +import { Attestation } from '../proxy/processors/types'; import { processGitUrl } from '../proxy/routes/helper'; import { initializeFolders } from './file/helper'; @@ -164,10 +165,10 @@ export const getPushes = (query: Partial): Promise => start export const writeAudit = (action: Action): Promise => start().writeAudit(action); export const getPush = (id: string): Promise => start().getPush(id); export const deletePush = (id: string): Promise => start().deletePush(id); -export const authorise = (id: string, attestation: any): Promise<{ message: string }> => +export const authorise = (id: string, attestation?: Attestation): Promise<{ message: string }> => start().authorise(id, attestation); export const cancel = (id: string): Promise<{ message: string }> => start().cancel(id); -export const reject = (id: string, attestation: any): Promise<{ message: string }> => +export const reject = (id: string, attestation?: Attestation): Promise<{ message: string }> => start().reject(id, attestation); export const getRepos = (query?: Partial): Promise => start().getRepos(query); export const getRepo = (name: string): Promise => start().getRepo(name); diff --git a/src/db/mongo/pushes.ts b/src/db/mongo/pushes.ts index 968b2858a..36c467661 100644 --- a/src/db/mongo/pushes.ts +++ b/src/db/mongo/pushes.ts @@ -2,6 +2,7 @@ import { connect, findDocuments, findOneDocument } from './helper'; import { Action } from '../../proxy/actions'; import { toClass } from '../helper'; import { PushQuery } from '../types'; +import { Attestation } from '../../proxy/processors/types'; const collectionName = 'pushes'; @@ -43,7 +44,7 @@ export const getPushes = async ( }; export const getPush = async (id: string): Promise => { - const doc = await findOneDocument(collectionName, { id }); + const doc = await findOneDocument(collectionName, { id }); return doc ? (toClass(doc, Action.prototype) as Action) : null; }; @@ -63,7 +64,10 @@ export const writeAudit = async (action: Action): Promise => { await collection.updateOne({ id: data.id }, { $set: data }, options); }; -export const authorise = async (id: string, attestation: any): Promise<{ message: string }> => { +export const authorise = async ( + id: string, + attestation?: Attestation, +): Promise<{ message: string }> => { const action = await getPush(id); if (!action) { throw new Error(`push ${id} not found`); @@ -77,7 +81,10 @@ export const authorise = async (id: string, attestation: any): Promise<{ message return { message: `authorised ${id}` }; }; -export const reject = async (id: string, attestation: any): Promise<{ message: string }> => { +export const reject = async ( + id: string, + attestation?: Attestation, +): Promise<{ message: string }> => { const action = await getPush(id); if (!action) { throw new Error(`push ${id} not found`); diff --git a/src/db/mongo/repo.ts b/src/db/mongo/repo.ts index 655ef40b1..17be25e2a 100644 --- a/src/db/mongo/repo.ts +++ b/src/db/mongo/repo.ts @@ -1,11 +1,11 @@ import _ from 'lodash'; -import { Repo } from '../types'; +import { Repo, RepoQuery } from '../types'; import { connect } from './helper'; import { toClass } from '../helper'; import { ObjectId, OptionalId, Document } from 'mongodb'; const collectionName = 'repos'; -export const getRepos = async (query: any = {}): Promise => { +export const getRepos = async (query: Partial = {}): Promise => { const collection = await connect(collectionName); const docs = await collection.find(query).toArray(); return _.chain(docs) diff --git a/src/db/mongo/users.ts b/src/db/mongo/users.ts index f4300c39e..c352acf53 100644 --- a/src/db/mongo/users.ts +++ b/src/db/mongo/users.ts @@ -1,6 +1,6 @@ import { OptionalId, Document, ObjectId } from 'mongodb'; import { toClass } from '../helper'; -import { User } from '../types'; +import { User, UserQuery } from '../types'; import { connect } from './helper'; import _ from 'lodash'; const collectionName = 'users'; @@ -23,7 +23,7 @@ export const findUserByOIDC = async function (oidcId: string): Promise { +export const getUsers = async function (query: Partial = {}): Promise { if (query.username) { query.username = query.username.toLowerCase(); } diff --git a/src/db/types.ts b/src/db/types.ts index e4ae2eab5..5f7a7d6ba 100644 --- a/src/db/types.ts +++ b/src/db/types.ts @@ -1,5 +1,6 @@ import { Action } from '../proxy/actions/Action'; import MongoDBStore from 'connect-mongo'; +import { Attestation } from '../proxy/processors/types'; export type PushQuery = { error: boolean; @@ -96,9 +97,9 @@ export interface Sink { writeAudit: (action: Action) => Promise; getPush: (id: string) => Promise; deletePush: (id: string) => Promise; - authorise: (id: string, attestation: any) => Promise<{ message: string }>; + authorise: (id: string, attestation?: Attestation) => Promise<{ message: string }>; cancel: (id: string) => Promise<{ message: string }>; - reject: (id: string, attestation: any) => Promise<{ message: string }>; + reject: (id: string, attestation?: Attestation) => Promise<{ message: string }>; getRepos: (query?: Partial) => Promise; getRepo: (name: string) => Promise; getRepoByUrl: (url: string) => Promise; diff --git a/src/plugin.ts b/src/plugin.ts index 92fb9a99c..b434a2243 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -1,6 +1,10 @@ +import { Request } from 'express'; + import { Action } from './proxy/actions'; +import Module from 'node:module'; -const lpModule = import('load-plugin'); +import { loadPlugin, resolvePlugin } from 'load-plugin'; +import { handleAndLogError } from './utils/errors'; /* eslint-disable @typescript-eslint/no-unused-expressions */ ('use strict'); @@ -67,7 +71,7 @@ class PluginLoader { const moduleResults = await Promise.allSettled(modulePromises); const loadedModules = moduleResults .filter( - (result): result is PromiseFulfilledResult => + (result): result is PromiseFulfilledResult => result.status === 'fulfilled' && result.value !== null, ) .map((result) => result.value); @@ -87,7 +91,7 @@ class PluginLoader { */ const pluginTypeResults = settledPluginTypeResults .filter( - (result): result is PromiseFulfilledResult => + (result): result is PromiseFulfilledResult => result.status === 'fulfilled' && result.value !== null, ) .map((result) => result.value); @@ -101,20 +105,19 @@ class PluginLoader { combinedPlugins.forEach((plugin) => { console.log(`Loaded plugin: ${plugin.constructor.name}`); }); - } catch (error) { - console.error(`Error loading plugins: ${error}`); + } catch (error: unknown) { + handleAndLogError(error, 'Error loading plugins'); } } /** * Resolve & load a Node module from either a given specifier (file path, import specifier or package name) using load-plugin. * @param {string} target The module specifier to load - * @return {Promise} A resolved & loaded Module + * @return {Promise} A resolved & loaded Module */ - private async _loadPluginModule(target: string): Promise { - const lp = await lpModule; - const resolvedModuleFile = await lp.resolvePlugin(target); - return await lp.loadPlugin(resolvedModuleFile); + private async _loadPluginModule(target: string): Promise { + const resolvedModuleFile = await resolvePlugin(target); + return loadPlugin(resolvedModuleFile); } /** @@ -177,7 +180,7 @@ class ProxyPlugin { */ class PushActionPlugin extends ProxyPlugin { isGitProxyPushActionPlugin: boolean; - exec: (req: any, action: Action) => Promise; + exec: (req: Request, action: Action) => Promise; /** * Wrapper class which contains at least one function executed as part of the action chain for git push operations. @@ -193,7 +196,7 @@ class PushActionPlugin extends ProxyPlugin { * - Takes in an Action object as the second parameter (`action`). * - Returns a Promise that resolves to an Action. */ - constructor(exec: (req: any, action: Action) => Promise) { + constructor(exec: (req: Request, action: Action) => Promise) { super(); this.isGitProxyPushActionPlugin = true; this.exec = exec; @@ -205,7 +208,7 @@ class PushActionPlugin extends ProxyPlugin { */ class PullActionPlugin extends ProxyPlugin { isGitProxyPullActionPlugin: boolean; - exec: (req: any, action: Action) => Promise; + exec: (req: Request, action: Action) => Promise; /** * Wrapper class which contains at least one function executed as part of the action chain for git pull operations. @@ -221,7 +224,7 @@ class PullActionPlugin extends ProxyPlugin { * - Takes in an Action object as the second parameter (`action`). * - Returns a Promise that resolves to an Action. */ - constructor(exec: (req: any, action: Action) => Promise) { + constructor(exec: (req: Request, action: Action) => Promise) { super(); this.isGitProxyPullActionPlugin = true; this.exec = exec; diff --git a/src/proxy/actions/autoActions.ts b/src/proxy/actions/autoActions.ts index 450c97d80..5b5907ac0 100644 --- a/src/proxy/actions/autoActions.ts +++ b/src/proxy/actions/autoActions.ts @@ -1,18 +1,24 @@ import { authorise, reject } from '../../db'; +import { handleAndLogError } from '../../utils/errors'; import { Action } from './Action'; const attemptAutoApproval = async (action: Action) => { try { const attestation = { timestamp: new Date(), - autoApproved: true, + automated: true, + questions: [], + reviewer: { + username: 'system', + email: 'system@git-proxy.com', + }, }; await authorise(action.id, attestation); console.log('Push automatically approved by system.'); return true; - } catch (error: any) { - console.error('Error during auto-approval:', error.message); + } catch (error: unknown) { + handleAndLogError(error, 'Error during auto-approval'); return false; } }; @@ -21,14 +27,19 @@ const attemptAutoRejection = async (action: Action) => { try { const attestation = { timestamp: new Date(), - autoApproved: true, + automated: true, + questions: [], + reviewer: { + username: 'system', + email: 'system@git-proxy.com', + }, }; await reject(action.id, attestation); console.log('Push automatically rejected by system.'); return true; - } catch (error: any) { - console.error('Error during auto-rejection:', error.message); + } catch (error: unknown) { + handleAndLogError(error, 'Error during auto-rejection'); return false; } }; diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index b6c1b7609..fd3d54626 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -1,9 +1,12 @@ +import { Request, Response } from 'express'; + import { PluginLoader } from '../plugin'; import { Action } from './actions'; import * as proc from './processors'; import { attemptAutoApproval, attemptAutoRejection } from './actions/autoActions'; +import { getErrorMessage, handleAndLogError } from '../utils/errors'; -const pushActionChain: ((req: any, action: Action) => Promise)[] = [ +const pushActionChain: ((req: Request, action: Action) => Promise)[] = [ proc.push.parsePush, proc.push.checkEmptyBranch, proc.push.checkRepoInAuthorisedList, @@ -21,17 +24,17 @@ const pushActionChain: ((req: any, action: Action) => Promise)[] = [ proc.push.blockForAuth, ]; -const pullActionChain: ((req: any, action: Action) => Promise)[] = [ +const pullActionChain: ((req: Request, action: Action) => Promise)[] = [ proc.push.checkRepoInAuthorisedList, ]; -const defaultActionChain: ((req: any, action: Action) => Promise)[] = [ +const defaultActionChain: ((req: Request, action: Action) => Promise)[] = [ proc.push.checkRepoInAuthorisedList, ]; let pluginsInserted = false; -export const executeChain = async (req: any, res: any): Promise => { +export const executeChain = async (req: Request, _res: Response): Promise => { let action: Action = {} as Action; let checkoutCleanUpRequired = false; @@ -49,10 +52,10 @@ export const executeChain = async (req: any, res: any): Promise => { checkoutCleanUpRequired = true; } } - } catch (e) { + } catch (error: unknown) { + const msg = handleAndLogError(error, 'An error occurred when executing the chain'); action.error = true; - action.errorMessage = `An error occurred when executing the chain: ${e}`; - console.error(action.errorMessage); + action.errorMessage = msg; } finally { //clean up the clone created if (checkoutCleanUpRequired) { @@ -79,7 +82,7 @@ let chainPluginLoader: PluginLoader; export const getChain = async ( action: Action, -): Promise<((req: any, action: Action) => Promise)[]> => { +): Promise<((req: Request, action: Action) => Promise)[]> => { if (chainPluginLoader === undefined) { console.error( 'Plugin loader was not initialized! This is an application error. Please report it to the GitProxy maintainers. Skipping plugins...', diff --git a/src/proxy/processors/constants.ts b/src/proxy/processors/constants.ts index 3ad5784b4..f48ef6563 100644 --- a/src/proxy/processors/constants.ts +++ b/src/proxy/processors/constants.ts @@ -1,6 +1,30 @@ +import { Repo } from '../../db/types'; +import { CommitData } from './types'; + export const BRANCH_PREFIX = 'refs/heads/'; export const EMPTY_COMMIT_HASH = '0000000000000000000000000000000000000000'; export const FLUSH_PACKET = '0000'; export const PACK_SIGNATURE = 'PACK'; export const PACKET_SIZE = 4; export const GIT_OBJECT_TYPE_COMMIT = 1; + +export const SAMPLE_COMMIT: CommitData = { + tree: '1234567890', + parent: '0000000000000000000000000000000000000000', + author: 'test', + committer: 'test', + authorEmail: 'test@test.com', + committerEmail: 'test@test.com', + commitTimestamp: '1234567890', + message: 'test', +}; + +export const SAMPLE_REPO = { + project: 'myrepo', + name: 'myrepo', + url: 'https://github.com/myrepo.git', + users: { + canPush: ['alice'], + canAuthorise: ['bob'], + }, +}; diff --git a/src/proxy/processors/post-processor/audit.ts b/src/proxy/processors/post-processor/audit.ts index 32e556fb7..47d07fc8e 100644 --- a/src/proxy/processors/post-processor/audit.ts +++ b/src/proxy/processors/post-processor/audit.ts @@ -1,7 +1,9 @@ +import { Request } from 'express'; + import { writeAudit } from '../../../db'; import { Action } from '../../actions'; -const exec = async (req: any, action: Action) => { +const exec = async (_req: Request, action: Action) => { if (action.type !== 'pull') { await writeAudit(action); } diff --git a/src/proxy/processors/pre-processor/parseAction.ts b/src/proxy/processors/pre-processor/parseAction.ts index 619deea93..22a1d3a2b 100644 --- a/src/proxy/processors/pre-processor/parseAction.ts +++ b/src/proxy/processors/pre-processor/parseAction.ts @@ -1,12 +1,10 @@ +import { Request } from 'express'; + import { Action } from '../../actions'; import { processUrlPath } from '../../routes/helper'; import * as db from '../../../db'; -const exec = async (req: { - originalUrl: string; - method: string; - headers: Record; -}) => { +const exec = async (req: Request) => { const id = Date.now(); const timestamp = id; let type = 'default'; diff --git a/src/proxy/processors/push-action/blockForAuth.ts b/src/proxy/processors/push-action/blockForAuth.ts index 4fde08e0d..86628995c 100644 --- a/src/proxy/processors/push-action/blockForAuth.ts +++ b/src/proxy/processors/push-action/blockForAuth.ts @@ -1,7 +1,9 @@ +import { Request } from 'express'; + import { Action, Step } from '../../actions'; import { getServiceUIURL } from '../../../service/urls'; -const exec = async (req: any, action: Action) => { +const exec = async (req: Request, action: Action) => { const step = new Step('authBlock'); const url = getServiceUIURL(req); diff --git a/src/proxy/processors/push-action/checkAuthorEmails.ts b/src/proxy/processors/push-action/checkAuthorEmails.ts index e8d51f09d..333ddaae4 100644 --- a/src/proxy/processors/push-action/checkAuthorEmails.ts +++ b/src/proxy/processors/push-action/checkAuthorEmails.ts @@ -1,7 +1,9 @@ +import { Request } from 'express'; +import { isEmail } from 'validator'; + import { Action, Step } from '../../actions'; import { getCommitConfig } from '../../../config'; import { CommitData } from '../types'; -import { isEmail } from 'validator'; const isEmailAllowed = (email: string): boolean => { const commitConfig = getCommitConfig(); @@ -29,7 +31,7 @@ const isEmailAllowed = (email: string): boolean => { return true; }; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkAuthorEmails'); const uniqueAuthorEmails = [ diff --git a/src/proxy/processors/push-action/checkCommitMessages.ts b/src/proxy/processors/push-action/checkCommitMessages.ts index 7eb9f6cad..91fbc3c58 100644 --- a/src/proxy/processors/push-action/checkCommitMessages.ts +++ b/src/proxy/processors/push-action/checkCommitMessages.ts @@ -1,5 +1,8 @@ +import { Request } from 'express'; + import { Action, Step } from '../../actions'; import { getCommitConfig } from '../../../config'; +import { handleAndLogError } from '../../../utils/errors'; const isMessageAllowed = (commitMessage: string): boolean => { try { @@ -39,8 +42,8 @@ const isMessageAllowed = (commitMessage: string): boolean => { console.log('Commit message is blocked via configured literals/patterns...'); return false; } - } catch (error) { - console.log('Invalid regex pattern...'); + } catch (error: unknown) { + handleAndLogError(error, 'Error checking commit messages'); return false; } @@ -48,7 +51,7 @@ const isMessageAllowed = (commitMessage: string): boolean => { }; // Execute if the repo is approved -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkCommitMessages'); const uniqueCommitMessages = [...new Set(action.commitData?.map((commit) => commit.message))]; diff --git a/src/proxy/processors/push-action/checkEmptyBranch.ts b/src/proxy/processors/push-action/checkEmptyBranch.ts index 86f6b5138..b61ecde7d 100644 --- a/src/proxy/processors/push-action/checkEmptyBranch.ts +++ b/src/proxy/processors/push-action/checkEmptyBranch.ts @@ -1,23 +1,26 @@ -import { Action, Step } from '../../actions'; +import { Request } from 'express'; import simpleGit from 'simple-git'; + +import { Action, Step } from '../../actions'; import { EMPTY_COMMIT_HASH } from '../constants'; +import { handleAndLogError } from '../../../utils/errors'; -const isEmptyBranch = async (action: Action) => { +const isEmptyBranch = async (action: Action): Promise => { if (action.commitFrom === EMPTY_COMMIT_HASH) { try { const git = simpleGit(`${action.proxyGitPath}/${action.repoName}`); const type = await git.raw(['cat-file', '-t', action.commitTo || '']); return type.trim() === 'commit'; - } catch (err) { - console.log(`Commit ${action.commitTo} not found: ${err}`); + } catch (error: unknown) { + handleAndLogError(error, `Error checking if branch is empty`); } } return false; }; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkEmptyBranch'); if (action.commitData && action.commitData.length > 0) { diff --git a/src/proxy/processors/push-action/checkHiddenCommits.ts b/src/proxy/processors/push-action/checkHiddenCommits.ts index 852328287..ebdfeaa16 100644 --- a/src/proxy/processors/push-action/checkHiddenCommits.ts +++ b/src/proxy/processors/push-action/checkHiddenCommits.ts @@ -1,8 +1,11 @@ import path from 'path'; import { Action, Step } from '../../actions'; import { spawnSync } from 'child_process'; +import { EMPTY_COMMIT_HASH } from '../constants'; +import { Request } from 'express'; +import { getErrorMessage } from '../../../utils/errors'; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkHiddenCommits'); try { @@ -16,8 +19,7 @@ const exec = async (req: any, action: Action): Promise => { // build introducedCommits set const introducedCommits = new Set(); - const revRange = - oldOid === '0000000000000000000000000000000000000000' ? newOid : `${oldOid}..${newOid}`; + const revRange = oldOid === EMPTY_COMMIT_HASH ? newOid : `${oldOid}..${newOid}`; const revList = spawnSync('git', ['rev-list', revRange], { cwd: repoPath, encoding: 'utf-8' }) .stdout.trim() .split('\n') @@ -68,9 +70,10 @@ const exec = async (req: any, action: Action): Promise => { step.log('All pack commits are referenced in the introduced range.'); step.setContent(`All ${packCommits.size} pack commits are within introduced commits.`); } - } catch (e: any) { - step.setError(e.message); - throw e; + } catch (error: unknown) { + const msg = getErrorMessage(error); + step.setError(msg); + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/checkIfWaitingAuth.ts b/src/proxy/processors/push-action/checkIfWaitingAuth.ts index baedb0df3..dbae0e6de 100644 --- a/src/proxy/processors/push-action/checkIfWaitingAuth.ts +++ b/src/proxy/processors/push-action/checkIfWaitingAuth.ts @@ -1,8 +1,11 @@ +import { Request } from 'express'; + import { Action, Step } from '../../actions'; import { getPush } from '../../../db'; +import { getErrorMessage } from '../../../utils/errors'; // Execute function -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkIfWaitingAuth'); try { const existingAction = await getPush(action.id); @@ -14,9 +17,10 @@ const exec = async (req: any, action: Action): Promise => { } } } - } catch (e: any) { - step.setError(e.toString('utf-8')); - throw e; + } catch (error: unknown) { + const msg = getErrorMessage(error); + step.setError(msg); + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts b/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts index d34e52d48..286953a06 100644 --- a/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts +++ b/src/proxy/processors/push-action/checkRepoInAuthorisedList.ts @@ -1,8 +1,10 @@ +import { Request } from 'express'; + import { Action, Step } from '../../actions'; import { getRepoByUrl } from '../../../db'; // Execute if the repo is approved -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkRepoInAuthorisedList'); const found = (await getRepoByUrl(action.url)) !== null; diff --git a/src/proxy/processors/push-action/checkUserPushPermission.ts b/src/proxy/processors/push-action/checkUserPushPermission.ts index 83f16c968..2064be561 100644 --- a/src/proxy/processors/push-action/checkUserPushPermission.ts +++ b/src/proxy/processors/push-action/checkUserPushPermission.ts @@ -1,8 +1,10 @@ +import { Request } from 'express'; + import { Action, Step } from '../../actions'; import { getUsers, isUserPushAllowed } from '../../../db'; // Execute if the repo is approved -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkUserPushPermission'); const userEmail = action.userEmail; diff --git a/src/proxy/processors/push-action/getDiff.ts b/src/proxy/processors/push-action/getDiff.ts index dbdc4e4e9..e3ed36dcc 100644 --- a/src/proxy/processors/push-action/getDiff.ts +++ b/src/proxy/processors/push-action/getDiff.ts @@ -1,9 +1,11 @@ -import { Action, Step } from '../../actions'; +import { Request } from 'express'; import simpleGit from 'simple-git'; +import { Action, Step } from '../../actions'; import { EMPTY_COMMIT_HASH } from '../constants'; +import { getErrorMessage } from '../../../utils/errors'; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('diff'); try { @@ -33,8 +35,9 @@ const exec = async (req: any, action: Action): Promise => { const diff = await git.diff([revisionRange]); step.log(diff); step.setContent(diff); - } catch (e: any) { - step.setError(e.toString('utf-8')); + } catch (error: unknown) { + const msg = getErrorMessage(error); + step.setError(msg); } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/gitleaks.ts b/src/proxy/processors/push-action/gitleaks.ts index 1cf5b2236..0fcf8d5ba 100644 --- a/src/proxy/processors/push-action/gitleaks.ts +++ b/src/proxy/processors/push-action/gitleaks.ts @@ -1,9 +1,11 @@ -import { Action, Step } from '../../actions'; -import { getAPIs } from '../../../config'; import { spawn } from 'node:child_process'; -import fs from 'node:fs/promises'; import { PathLike } from 'node:fs'; +import fs from 'node:fs/promises'; +import { Request } from 'express'; +import { Action, Step } from '../../actions'; +import { getAPIs } from '../../../config'; +import { getErrorMessage, handleAndLogError } from '../../../utils/errors'; const EXIT_CODE = 99; function runCommand( @@ -66,7 +68,7 @@ async function fileIsReadable(path: PathLike): Promise { } await fs.access(path, fs.constants.R_OK); return true; - } catch (e) { + } catch (error: unknown) { return false; } } @@ -109,14 +111,14 @@ const getPluginConfig = async (): Promise => { }; }; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('gitleaks'); let config: ConfigOptions | undefined = undefined; try { config = await getPluginConfig(); - } catch (e) { - console.error('failed to get gitleaks config, please fix the error:', e); + } catch (error: unknown) { + handleAndLogError(error, 'failed to get gitleaks config, please fix the error'); action.error = true; step.setError('failed setup gitleaks, please contact an administrator\n'); action.addStep(step); @@ -174,9 +176,10 @@ const exec = async (req: any, action: Action): Promise => { console.log('succeeded'); console.log(gitleaks.stderr); } - } catch (e) { + } catch (error: unknown) { + const msg = getErrorMessage(error); action.error = true; - step.setError('failed to spawn gitleaks, please contact an administrator\n'); + step.setError(`failed to spawn gitleaks, please contact an administrator\n: ${msg}`); action.addStep(step); return action; } diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 307fe6286..b94af7367 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -1,7 +1,9 @@ -import { Action, Step } from '../../actions'; +import { Request } from 'express'; import fs from 'fs'; import lod from 'lodash'; import { createInflate } from 'zlib'; + +import { Action, Step } from '../../actions'; import { CommitContent, CommitData, CommitHeader, PackMeta, PersonLine } from '../types'; import { BRANCH_PREFIX, @@ -10,6 +12,7 @@ import { PACKET_SIZE, GIT_OBJECT_TYPE_COMMIT, } from '../constants'; +import { getErrorMessage } from '../../../utils/errors'; const dir = './.tmp/'; @@ -27,11 +30,11 @@ const EIGHTH_BIT_MASK = 0x80; /** * Executes the parsing of a push request. - * @param {*} req - The request object containing the push data. + * @param {Request} req - The Express Request object containing the push data. * @param {Action} action - The action object to be modified. * @return {Promise} The modified action object. */ -async function exec(req: any, action: Action): Promise { +async function exec(req: Request, action: Action): Promise { const step = new Step('parsePackFile'); try { if (!req.body || req.body.length === 0) { @@ -83,7 +86,7 @@ async function exec(req: any, action: Action): Promise { const [meta, contentBuff] = getPackMeta(buf); const contents = await getContents(contentBuff, meta.entries); - action.commitData = getCommitData(contents as any); + action.commitData = getCommitData(contents); if (action.commitData.length === 0) { step.log('No commit data found when parsing push.'); @@ -101,10 +104,9 @@ async function exec(req: any, action: Action): Promise { step.content = { meta: meta, }; - } catch (e: any) { - step.setError( - `Unable to parse push. Please contact an administrator for support: ${e.toString('utf-8')}`, - ); + } catch (error: unknown) { + const msg = getErrorMessage(error); + step.setError(`Unable to parse push. Please contact an administrator for support: ${msg}`); } finally { action.addStep(step); } @@ -478,8 +480,8 @@ const decompressGitObjects = async (buffer: Buffer): Promise => { }; // stop on errors, except maybe buffer errors? - const onError = (e: any) => { - error = e; + const onError = (e: unknown) => { + error = e instanceof Error ? e : new Error(String(e)); console.warn(`Error during inflation: ${JSON.stringify(e)}`); error = new Error('Error during inflation', { cause: e }); inflater.end(); @@ -505,9 +507,10 @@ const decompressGitObjects = async (buffer: Buffer): Promise => { offset++; } }); - } catch (e) { - console.warn(`Error during decompression: ${JSON.stringify(e)}`); - error = new Error('Error during decompression', { cause: e }); + } catch (error: unknown) { + const msg = `Error during decompression: ${error instanceof Error ? error.message : String(error)}`; + console.warn(msg); + throw new Error(msg); } } const result = { diff --git a/src/proxy/processors/push-action/preReceive.ts b/src/proxy/processors/push-action/preReceive.ts index 9c3ad1116..d9f5ede29 100644 --- a/src/proxy/processors/push-action/preReceive.ts +++ b/src/proxy/processors/push-action/preReceive.ts @@ -1,14 +1,17 @@ +import { spawnSync } from 'child_process'; +import { Request } from 'express'; import fs from 'fs'; import path from 'path'; + import { Action, Step } from '../../actions'; -import { spawnSync } from 'child_process'; +import { getErrorMessage } from '../../../utils/errors'; -const sanitizeInput = (_req: any, action: Action): string => { +const sanitizeInput = (_req: Request, action: Action): string => { return `${action.commitFrom} ${action.commitTo} ${action.branch} \n`; }; const exec = async ( - req: any, + req: Request, action: Action, hookFilePath: string = './hooks/pre-receive.sh', ): Promise => { @@ -69,10 +72,11 @@ const exec = async ( action.addStep(step); } return action; - } catch (error: any) { + } catch (error: unknown) { + const msg = getErrorMessage(error); step.error = true; step.log('Push failed, pre-receive hook returned an error.'); - step.setError(`Hook execution error: ${stderrTrimmed || error.message}`); + step.setError(`Hook execution error: ${stderrTrimmed || msg}`); action.addStep(step); return action; } diff --git a/src/proxy/processors/push-action/pullRemote.ts b/src/proxy/processors/push-action/pullRemote.ts index 4583cfdc7..b02cfa602 100644 --- a/src/proxy/processors/push-action/pullRemote.ts +++ b/src/proxy/processors/push-action/pullRemote.ts @@ -1,11 +1,14 @@ -import { Action, Step } from '../../actions'; +import { Request } from 'express'; import fs from 'fs'; import git from 'isomorphic-git'; import gitHttpClient from 'isomorphic-git/http/node'; +import { Action, Step } from '../../actions'; +import { getErrorMessage } from '../../../utils/errors'; + const dir = './.remote'; -const exec = async (req: any, action: Action): Promise => { +const exec = async (req: Request, action: Action): Promise => { const step = new Step('pullRemote'); action.proxyGitPath = `${dir}/${action.id}`; @@ -27,6 +30,9 @@ const exec = async (req: any, action: Action): Promise => { step.log(`Executing ${cmd}`); const authHeader = req.headers?.authorization; + if (!authHeader) { + throw new Error('Authorization header is required'); + } const [username, password] = Buffer.from(authHeader.split(' ')[1], 'base64') .toString() .split(':'); @@ -44,14 +50,14 @@ const exec = async (req: any, action: Action): Promise => { step.log(`Completed ${cmd}`); step.setContent(`Completed ${cmd}`); - } catch (e: any) { - step.setError(e.toString('utf-8')); + } catch (error: unknown) { + step.setError(getErrorMessage(error)); //clean-up the check out folder so it doesn't block subsequent attempts fs.rmSync(action.proxyGitPath, { recursive: true, force: true }); step.log(`.remote is deleted!`); - throw e; + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/scanDiff.ts b/src/proxy/processors/push-action/scanDiff.ts index 56f3ddc11..e7511bc10 100644 --- a/src/proxy/processors/push-action/scanDiff.ts +++ b/src/proxy/processors/push-action/scanDiff.ts @@ -1,7 +1,9 @@ +import escapeStringRegexp from 'escape-string-regexp'; +import { Request } from 'express'; +import parseDiff, { File } from 'parse-diff'; + import { Action, Step } from '../../actions'; import { getCommitConfig, getPrivateOrganizations } from '../../../config'; -import parseDiff, { File } from 'parse-diff'; -import escapeStringRegexp from 'escape-string-regexp'; const commitConfig = getCommitConfig(); const privateOrganizations = getPrivateOrganizations(); @@ -154,7 +156,7 @@ const formatMatches = (matches: Match[]) => { }); }; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('scanDiff'); const { steps, commitFrom, commitTo } = action; diff --git a/src/proxy/processors/push-action/writePack.ts b/src/proxy/processors/push-action/writePack.ts index 2e9f5792d..fbcb7afda 100644 --- a/src/proxy/processors/push-action/writePack.ts +++ b/src/proxy/processors/push-action/writePack.ts @@ -1,9 +1,12 @@ -import path from 'path'; -import { Action, Step } from '../../actions'; import { spawnSync } from 'child_process'; +import { Request } from 'express'; import fs from 'fs'; +import path from 'path'; + +import { Action, Step } from '../../actions'; +import { getErrorMessage } from '../../../utils/errors'; -const exec = async (req: any, action: Action) => { +const exec = async (req: Request, action: Action) => { const step = new Step('writePack'); try { if (!action.proxyGitPath || !action.repoName) { @@ -18,7 +21,7 @@ const exec = async (req: any, action: Action) => { encoding: 'utf-8', }); const before = new Set(fs.readdirSync(packDir).filter((f) => f.endsWith('.idx'))); - const content = spawnSync('git', ['receive-pack', action.repoName], { + spawnSync('git', ['receive-pack', action.repoName], { cwd: action.proxyGitPath, input: req.body, }); @@ -27,9 +30,10 @@ const exec = async (req: any, action: Action) => { ]; action.newIdxFiles = newIdxFiles; step.log(`new idx files: ${newIdxFiles}`); - } catch (e: any) { - step.setError(e.toString('utf-8')); - throw e; + } catch (error: unknown) { + const msg = getErrorMessage(error); + step.setError(msg); + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/processors/types.ts b/src/proxy/processors/types.ts index c4c447b5d..09c352369 100644 --- a/src/proxy/processors/types.ts +++ b/src/proxy/processors/types.ts @@ -1,8 +1,10 @@ +import { Request } from 'express'; + import { Question } from '../../config/generated/config'; import { Action } from '../actions'; export interface Processor { - exec(req: any, action: Action): Promise; + exec(req: Request, action: Action): Promise; metadata: ProcessorMetadata; } @@ -13,10 +15,11 @@ export interface ProcessorMetadata { export type Attestation = { reviewer: { username: string; - gitAccount: string; + email: string; }; timestamp: string | Date; questions: Question[]; + automated?: boolean; }; export type CommitContent = { diff --git a/src/proxy/routes/helper.ts b/src/proxy/routes/helper.ts index 54d72edca..5c4821359 100644 --- a/src/proxy/routes/helper.ts +++ b/src/proxy/routes/helper.ts @@ -1,3 +1,5 @@ +import { IncomingHttpHeaders } from 'http'; + /** Regex used to analyze un-proxied Git URLs */ const GIT_URL_REGEX = /(.+:\/\/)([^/]+)(\/.+\.git)(\/.+)*/; @@ -150,7 +152,7 @@ export const processGitURLForNameAndOrg = (gitUrl: string): GitNameBreakdown | n * @return {boolean} If true, this is a valid and expected git request. * Otherwise, false. */ -export const validGitRequest = (gitPath: string, headers: any): boolean => { +export const validGitRequest = (gitPath: string, headers: IncomingHttpHeaders): boolean => { const { 'user-agent': agent, accept } = headers; if (!agent) { return false; diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index ac53f0d2d..49f8109f2 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -6,6 +6,7 @@ import { executeChain } from '../chain'; import { processUrlPath, validGitRequest } from './helper'; import { getAllProxiedHosts } from '../../db'; import { ProxyOptions } from 'express-http-proxy'; +import { getErrorMessage, handleAndLogError } from '../../utils/errors'; enum ActionType { ALLOWED = 'Allowed', @@ -47,10 +48,10 @@ const proxyFilter: ProxyOptions['filter'] = async (req, res) => { } // For POST pack requests, use the raw body extracted by extractRawBody middleware - if (isPackPost(req) && (req as any).bodyRaw) { - (req as any).body = (req as any).bodyRaw; + if (isPackPost(req) && req.bodyRaw) { + req.body = req.bodyRaw; // Clean up the bodyRaw property before forwarding the request - delete (req as any).bodyRaw; + delete req.bodyRaw; } const action = await executeChain(req, res); @@ -68,8 +69,8 @@ const proxyFilter: ProxyOptions['filter'] = async (req, res) => { // this is the only case where we do not respond directly, instead we return true to proxy the request return true; - } catch (e) { - const message = `Error occurred in proxy filter function ${(e as Error).message ?? e}`; + } catch (error: unknown) { + const message = handleAndLogError(error, 'Error occurred in proxy filter function'); logAction(req.url, req.headers.host, req.headers['user-agent'], ActionType.ERROR, message); sendErrorResponse(req, res, message); @@ -162,12 +163,17 @@ const extractRawBody = async (req: Request, res: Response, next: NextFunction) = try { const buf = await getRawBody(pluginStream, { limit: '1gb' }); - (req as any).bodyRaw = buf; - (req as any).pipe = (dest: any, opts: any) => proxyStream.pipe(dest, opts); + req.bodyRaw = buf; + req.pipe = (dest, opts) => proxyStream.pipe(dest, opts); next(); - } catch (e) { - console.error(e); - proxyStream.destroy(e as Error); + } catch (error: unknown) { + if (error instanceof Error) { + console.error(error.message); + proxyStream.destroy(error); + } else { + console.error(String(error)); + proxyStream.destroy(new Error(String(error))); + } res.status(500).end('Proxy error'); } }; diff --git a/src/service/passport/activeDirectory.ts b/src/service/passport/activeDirectory.ts index 6814bcacc..8519c58f6 100644 --- a/src/service/passport/activeDirectory.ts +++ b/src/service/passport/activeDirectory.ts @@ -7,6 +7,7 @@ import * as ldaphelper from './ldaphelper'; import * as db from '../../db'; import { getAuthMethods } from '../../config'; import { ADProfile } from './types'; +import { handleAndLogError } from '../../utils/errors'; export const type = 'activedirectory'; @@ -43,7 +44,7 @@ export const configure = async (passport: PassportStatic): Promise void, + done: (err: unknown, user: unknown) => void, ) { try { profile.username = profile._json.sAMAccountName?.toLowerCase(); @@ -63,8 +64,11 @@ export const configure = async (passport: PassportStatic): Promise void) { + passport.serializeUser(function ( + user: Partial, + done: (err: unknown, user: Partial) => void, + ) { done(null, user); }); - passport.deserializeUser(function (user: any, done: (err: any, user: any) => void) { + passport.deserializeUser(function ( + user: Partial, + done: (err: unknown, user: Partial) => void, + ) { done(null, user); }); diff --git a/src/service/passport/jwtUtils.ts b/src/service/passport/jwtUtils.ts index eefe262cd..af44b1770 100644 --- a/src/service/passport/jwtUtils.ts +++ b/src/service/passport/jwtUtils.ts @@ -4,6 +4,7 @@ import jwt, { type JwtPayload } from 'jsonwebtoken'; import { JwkKey, JwksResponse, JwtValidationResult } from './types'; import { RoleMapping } from '../../config/generated/config'; +import { handleAndLogError } from '../../utils/errors'; /** * Obtain the JSON Web Key Set (JWKS) from the OIDC authority. @@ -17,8 +18,8 @@ export async function getJwks(authorityUrl: string): Promise { const { data: jwks }: { data: JwksResponse } = await axios.get(jwksUri); return jwks.keys; - } catch (error) { - console.error('Error fetching JWKS:', error); + } catch (error: unknown) { + handleAndLogError(error, 'Error fetching JWKS'); throw new Error('Failed to fetch JWKS'); } } @@ -73,9 +74,8 @@ export async function validateJwt( } return { verifiedPayload, error: null }; - } catch (error: any) { - const errorMessage = `JWT validation failed: ${error.message}\n`; - console.error(errorMessage); + } catch (error: unknown) { + const errorMessage = handleAndLogError(error, 'JWT validation failed'); return { error: errorMessage, verifiedPayload: null }; } } diff --git a/src/service/passport/local.ts b/src/service/passport/local.ts index fe9b2d89d..b79213023 100644 --- a/src/service/passport/local.ts +++ b/src/service/passport/local.ts @@ -1,5 +1,5 @@ import bcrypt from 'bcryptjs'; -import { Strategy as LocalStrategy } from 'passport-local'; +import { IVerifyOptions, Strategy as LocalStrategy } from 'passport-local'; import type { PassportStatic } from 'passport'; import * as db from '../../db'; @@ -15,29 +15,29 @@ export const configure = async (passport: PassportStatic): Promise void, + done: (err: unknown, user?: Partial, info?: IVerifyOptions) => void, ) => { try { const dbModule = await getDb(); const user = await dbModule.findUser(username); if (!user) { - return done(null, false, { message: 'Incorrect username.' }); + return done(null, undefined, { message: 'Incorrect username.' }); } const passwordCorrect = await bcrypt.compare(password, user.password ?? ''); if (!passwordCorrect) { - return done(null, false, { message: 'Incorrect password.' }); + return done(null, undefined, { message: 'Incorrect password.' }); } return done(null, user); - } catch (err) { - return done(err); + } catch (error: unknown) { + return done(error); } }, ), ); - passport.serializeUser((user: any, done) => { + passport.serializeUser((user: Partial, done) => { done(null, user.username); }); @@ -46,8 +46,8 @@ export const configure = async (passport: PassportStatic): Promise void) => { + async (tokenSet: any, done: (err: unknown, user?: Partial) => void) => { const idTokenClaims = tokenSet.claims(); const expectedSub = idTokenClaims.sub; const userInfo = await fetchUserInfo(config, tokenSet.access_token, expectedSub); @@ -41,7 +42,7 @@ export const configure = async (passport: PassportStatic): Promise { + passport.serializeUser((user: Partial, done) => { done(null, user.oidcId || user.username); }); @@ -59,15 +60,15 @@ export const configure = async (passport: PassportStatic): Promise void, + done: (err: unknown, user?: Partial) => void, ): Promise => { - console.log('handleUserAuthentication called'); try { const user = await db.findUserByOIDC(userInfo.sub); @@ -100,21 +100,26 @@ export const handleUserAuthentication = async ( } return done(null, user); - } catch (err) { - return done(err); + } catch (error: unknown) { + const msg = handleAndLogError(error, 'Error during OIDC user authentication'); + return done(msg); } }; /** * Extracts email from OIDC profile. * Different providers use different fields to store the email. - * @param {any} profile - The user profile from the OIDC provider + * @param {UserInfoResponse} profile - The user profile from the OIDC provider * @return {string | null} - The email address from the profile */ -export const safelyExtractEmail = (profile: any): string | null => { - return ( - profile.email || (profile.emails && profile.emails.length > 0 ? profile.emails[0].value : null) - ); +export const safelyExtractEmail = (profile: UserInfoResponse): string | null => { + if (profile.email) { + return profile.email; + } + if (profile.emails && Array.isArray(profile.emails) && profile.emails.length > 0) { + return (profile.emails[0] as { value: string }).value; + } + return null; }; /** diff --git a/src/service/routes/auth.ts b/src/service/routes/auth.ts index 9835af3c8..009f13202 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -10,6 +10,7 @@ import { User } from '../../db/types'; import { AuthenticationElement } from '../../config/generated/config'; import { isAdminUser, toPublicUser } from './utils'; +import { handleAndLogError } from '../../utils/errors'; const router = express.Router(); const passport = getPassport(); @@ -66,9 +67,9 @@ const loginSuccessHandler = () => async (req: Request, res: Response) => { message: 'success', user: currentUser, }); - } catch (e) { - console.log(`service.routes.auth.login: Error logging user in ${JSON.stringify(e)}`); - res.status(500).send('Failed to login').end(); + } catch (error: unknown) { + const msg = handleAndLogError(error, 'Error logging user in'); + res.status(500).send(`Failed to login: ${msg}`).end(); } }; @@ -103,28 +104,31 @@ router.post( router.get('/openidconnect', passport.authenticate(authStrategies['openidconnect'].type)); router.get('/openidconnect/callback', (req: Request, res: Response, next: NextFunction) => { - passport.authenticate(authStrategies['openidconnect'].type, (err: any, user: any, info: any) => { - if (err) { - console.error('Authentication error:', err); - return res.status(500).end(); - } - if (!user) { - console.error('No user found:', info); - return res.status(401).end(); - } - req.logIn(user, (err) => { + passport.authenticate( + authStrategies['openidconnect'].type, + (err: unknown, user: Partial, info: unknown) => { if (err) { - console.error('Login error:', err); + console.error('Authentication error:', err); return res.status(500).end(); } - console.log('Logged in successfully. User:', user); - return res.redirect(`${uiHost}:${uiPort}/dashboard/profile`); - }); - })(req, res, next); + if (!user) { + console.error('No user found:', info); + return res.status(401).end(); + } + req.logIn(user, (err) => { + if (err) { + console.error('Login error:', err); + return res.status(500).end(); + } + console.log('Logged in successfully. User:', user); + return res.redirect(`${uiHost}:${uiPort}/dashboard/profile`); + }); + }, + )(req, res, next); }); router.post('/logout', (req: Request, res: Response, next: NextFunction) => { - req.logout((err: any) => { + req.logout((err: unknown) => { if (err) return next(err); }); res.clearCookie('connect.sid'); @@ -204,11 +208,12 @@ router.post('/gitAccount', async (req: Request, res: Response) => { user.gitAccount = req.body.gitAccount; db.updateUser(user); res.status(200).end(); - } catch (e: any) { + } catch (error: unknown) { + const msg = handleAndLogError(error, 'Failed to update git account'); res .status(500) .send({ - message: `Failed to update git account: ${e.message}`, + message: msg, }) .end(); } @@ -247,11 +252,14 @@ router.post('/create-user', async (req: Request, res: Response) => { username, }) .end(); - } catch (error: any) { - console.error('Error creating user:', error); - res.status(500).send({ - message: error.message || 'Failed to create user', - }); + } catch (error: unknown) { + const msg = handleAndLogError(error, 'Failed to create user'); + res + .status(500) + .send({ + message: msg, + }) + .end(); } }); diff --git a/src/service/routes/index.ts b/src/service/routes/index.ts index 23b63b02a..f2d76a4d0 100644 --- a/src/service/routes/index.ts +++ b/src/service/routes/index.ts @@ -7,8 +7,9 @@ import users from './users'; import healthcheck from './healthcheck'; import config from './config'; import { jwtAuthHandler } from '../passport/jwtAuthHandler'; +import { Proxy } from '../../proxy'; -const routes = (proxy: any) => { +const routes = (proxy: Proxy) => { const router = express.Router(); router.use('/api', home); router.use('/api/auth', auth.router); diff --git a/src/service/routes/push.ts b/src/service/routes/push.ts index fbce5335e..f6baea681 100644 --- a/src/service/routes/push.ts +++ b/src/service/routes/push.ts @@ -71,7 +71,7 @@ router.post('/:id/reject', async (req: Request, res: Response) => { const isAllowed = await db.canUserApproveRejectPush(id, username); if (isAllowed) { - const result = await db.reject(id, null); + const result = await db.reject(id); console.log(`User ${username} rejected push request for ${id}`); res.send(result); } else { @@ -156,7 +156,7 @@ router.post('/:id/authorise', async (req: Request, res: Response) => { timestamp: new Date(), reviewer: { username, - reviewerEmail, + email: reviewerEmail, }, }; const result = await db.authorise(id, attestation); diff --git a/src/service/routes/repo.ts b/src/service/routes/repo.ts index 2c4e1b54b..0e6960a94 100644 --- a/src/service/routes/repo.ts +++ b/src/service/routes/repo.ts @@ -5,11 +5,13 @@ import { getProxyURL } from '../urls'; import { getAllProxiedHosts } from '../../db'; import { RepoQuery } from '../../db/types'; import { isAdminUser } from './utils'; +import { Proxy } from '../../proxy'; +import { handleAndLogError } from '../../utils/errors'; // create a reference to the proxy service as arrow functions will lose track of the `proxy` parameter // used to restart the proxy when a new host is added -let theProxy: any = null; -const repo = (proxy: any) => { +let theProxy: Proxy | null = null; +const repo = (proxy: Proxy) => { theProxy = proxy; const router = express.Router(); @@ -131,8 +133,8 @@ const repo = (proxy: any) => { if (currentHosts.length < previousHosts.length) { // restart the proxy console.log('Restarting the proxy to remove a host'); - await theProxy.stop(); - await theProxy.start(); + await theProxy?.stop(); + await theProxy?.start(); } res.send({ message: 'deleted' }); @@ -181,22 +183,17 @@ const repo = (proxy: any) => { // restart the proxy if we're proxying a new domain if (newOrigin) { console.log('Restarting the proxy to handle an additional host'); - await theProxy.stop(); - await theProxy.start(); + await theProxy?.stop(); + await theProxy?.start(); } // return data on the new repository (including it's _id and the proxyUrl) res.send({ ...repoDetails, proxyURL, message: 'created' }); - } catch (e: any) { - console.error('Repository creation failed due to error: ', e.message ? e.message : e); - console.error(e.stack); - res.status(500).send({ message: 'Failed to create repository due to error' }); + } catch (error: unknown) { + const msg = handleAndLogError(error, 'Repository creation failed'); + res.status(500).send({ message: msg }); } } - } else { - res.status(401).send({ - message: 'You are not authorised to perform this action...', - }); } }); diff --git a/src/types/express.d.ts b/src/types/express.d.ts new file mode 100644 index 000000000..891c7e22c --- /dev/null +++ b/src/types/express.d.ts @@ -0,0 +1,7 @@ +import { Readable } from 'stream'; + +declare module 'express-serve-static-core' { + interface Request { + bodyRaw?: Buffer; + } +} diff --git a/src/ui/assets/jss/material-dashboard-react/layouts/dashboardStyle.ts b/src/ui/assets/jss/material-dashboard-react/layouts/dashboardStyle.ts index 411803438..a9dba3b52 100644 --- a/src/ui/assets/jss/material-dashboard-react/layouts/dashboardStyle.ts +++ b/src/ui/assets/jss/material-dashboard-react/layouts/dashboardStyle.ts @@ -27,7 +27,7 @@ const appStyle = (theme: Theme): AppStyleProps => ({ ...transition, maxHeight: '100%', width: '100%', - WebkitOverflowScrolling: 'touch' as any, + WebkitOverflowScrolling: 'touch', }, content: { marginTop: '70px', diff --git a/src/ui/auth/AuthProvider.tsx b/src/ui/auth/AuthProvider.tsx index 57e6913c0..ab70788dd 100644 --- a/src/ui/auth/AuthProvider.tsx +++ b/src/ui/auth/AuthProvider.tsx @@ -11,7 +11,9 @@ export const AuthProvider: React.FC> = ({ childr try { const data = await getUserInfo(); setUser(data); - } catch (error) { + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error refreshing user: ${msg}`); setUser(null); } finally { setIsLoading(false); diff --git a/src/ui/components/Navbars/DashboardNavbarLinks.tsx b/src/ui/components/Navbars/DashboardNavbarLinks.tsx index e1166c339..04c4fb906 100644 --- a/src/ui/components/Navbars/DashboardNavbarLinks.tsx +++ b/src/ui/components/Navbars/DashboardNavbarLinks.tsx @@ -58,8 +58,9 @@ const DashboardNavbarLinks: React.FC = () => { setAuth(false); navigate(0); } - } catch (error) { - console.error('Logout failed:', error); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Logout failed:', msg); } }; diff --git a/src/ui/components/RouteGuard/RouteGuard.tsx b/src/ui/components/RouteGuard/RouteGuard.tsx index a975ce2fb..a38032b47 100644 --- a/src/ui/components/RouteGuard/RouteGuard.tsx +++ b/src/ui/components/RouteGuard/RouteGuard.tsx @@ -6,7 +6,7 @@ import { UIRouteAuth } from '../../../config/generated/config'; import CircularProgress from '@material-ui/core/CircularProgress'; interface RouteGuardProps { - component: React.ComponentType; + component: React.ComponentType; fullRoutePath: string; } diff --git a/src/ui/context.ts b/src/ui/context.ts index fcf7a7da5..5c57c7cf5 100644 --- a/src/ui/context.ts +++ b/src/ui/context.ts @@ -15,7 +15,7 @@ export interface UserContextType { export interface AuthContextType { user: PublicUser | null; - setUser: React.Dispatch; + setUser: React.Dispatch>; refreshUser: () => Promise; isLoading: boolean; } diff --git a/src/ui/layouts/Dashboard.tsx b/src/ui/layouts/Dashboard.tsx index 3666a2bd1..e6f86a428 100644 --- a/src/ui/layouts/Dashboard.tsx +++ b/src/ui/layouts/Dashboard.tsx @@ -9,7 +9,7 @@ import Sidebar from '../components/Sidebar/Sidebar'; import routes from '../../routes'; import styles from '../assets/jss/material-dashboard-react/layouts/dashboardStyle'; import logo from '../assets/img/git-proxy.png'; -import { UserContext } from '../context'; +import { UserContext, UserContextType } from '../context'; import { getUser } from '../services/user'; import { Route as RouteType } from '../types'; import { PublicUser } from '../../db/types'; @@ -28,7 +28,7 @@ const Dashboard: React.FC = ({ ...rest }) => { const mainPanel = useRef(null); const [color] = useState<'purple' | 'blue' | 'green' | 'orange' | 'red'>('blue'); const [mobileOpen, setMobileOpen] = useState(false); - const [user, setUser] = useState({} as PublicUser); + const [user, setUser] = useState(null); const { id } = useParams<{ id?: string }>(); const handleDrawerToggle = () => setMobileOpen((prev) => !prev); @@ -82,7 +82,7 @@ const Dashboard: React.FC = ({ ...rest }) => { }, [id]); return ( - +
=> { }); if (!response.ok) throw new Error(`Failed to fetch user info: ${response.statusText}`); return await response.json(); - } catch (error) { - console.error('Error fetching user info:', error); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Error fetching user info:', msg); return null; } }; @@ -45,9 +47,12 @@ export const getAxiosConfig = (): AxiosConfig => { /** * Processes authentication errors and returns a user-friendly error message */ -export const processAuthError = (error: AxiosError, jwtAuthEnabled = false): string => { - const errorMessage = (error.response?.data as any)?.message ?? 'Unknown error'; - let msg = `Failed to authorize user: ${errorMessage.trim()}. `; +export const processAuthError = ( + error: AxiosError, + jwtAuthEnabled = false, +): string => { + const errorMessage = error.response?.data?.message?.trim() ?? 'Unknown error'; + let msg = `Failed to authorize user: ${errorMessage}. `; if (jwtAuthEnabled && !localStorage.getItem('ui_jwt_token')) { msg += 'Set your JWT token in the settings page or disable JWT auth in your app configuration.'; } else { diff --git a/src/ui/services/git-push.ts b/src/ui/services/git-push.ts index eafe5c96d..b9b9a52c0 100644 --- a/src/ui/services/git-push.ts +++ b/src/ui/services/git-push.ts @@ -1,8 +1,8 @@ -import axios from 'axios'; +import axios, { AxiosError } from 'axios'; import { getAxiosConfig, processAuthError } from './auth'; import { getBaseUrl, getApiV1BaseUrl } from './apiConfig'; import { Action, Step } from '../../proxy/actions'; -import { PushActionView } from '../types'; +import { BackendResponse, PushActionView } from '../types'; const getPush = async ( id: string, @@ -15,17 +15,19 @@ const getPush = async ( const url = `${apiV1Base}/push/${id}`; setIsLoading(true); - try { - const response = await axios(url, getAxiosConfig()); - const data: Action & { diff?: Step } = response.data; - data.diff = data.steps.find((x: Step) => x.stepName === 'diff'); - setPush(data as PushActionView); - } catch (error: any) { - if (error.response?.status === 401) setAuth(false); - else setIsError(true); - } finally { - setIsLoading(false); - } + await axios(url, getAxiosConfig()) + .then((response) => { + const data: Action & { diff?: Step } = response.data; + data.diff = data.steps.find((x: Step) => x.stepName === 'diff'); + setPush(data as PushActionView); + }) + .catch((error: AxiosError) => { + if (error.response?.status === 401) setAuth(false); + else setIsError(true); + }) + .finally(() => { + setIsLoading(false); + }); }; const getPushes = async ( @@ -43,26 +45,31 @@ const getPushes = async ( ): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/push`); - url.search = new URLSearchParams(query as any).toString(); - setIsLoading(true); + const stringifiedQuery = Object.fromEntries( + Object.entries(query).map(([key, value]) => [key, value.toString()]), + ); + url.search = new URLSearchParams(stringifiedQuery).toString(); - try { - const response = await axios(url.toString(), getAxiosConfig()); - setPushes(response.data as PushActionView[]); - } catch (error: any) { - setIsError(true); + setIsLoading(true); - if (error.response?.status === 401) { - setAuth(false); - setErrorMessage(processAuthError(error)); - } else { - const message = error.response?.data?.message || error.message; - setErrorMessage(`Error fetching pushes: ${message}`); - } - } finally { - setIsLoading(false); - } + await axios(url.toString(), getAxiosConfig()) + .then((response) => { + setPushes(response.data as PushActionView[]); + }) + .catch((error: AxiosError) => { + setIsError(true); + if (error.response?.status === 401) { + setAuth(false); + setErrorMessage(processAuthError(error)); + } else { + const message = error.response?.data?.message ?? error.message; + setErrorMessage(`Error fetching pushes: ${message}`); + } + }) + .finally(() => { + setIsLoading(false); + }); }; const authorisePush = async ( @@ -85,7 +92,7 @@ const authorisePush = async ( }, getAxiosConfig(), ) - .catch((error: any) => { + .catch((error: AxiosError) => { if (error.response && error.response.status === 401) { errorMsg = 'You are not authorised to approve...'; isUserAllowedToApprove = false; @@ -104,7 +111,7 @@ const rejectPush = async ( const url = `${apiV1Base}/push/${id}/reject`; let errorMsg = ''; let isUserAllowedToReject = true; - await axios.post(url, {}, getAxiosConfig()).catch((error: any) => { + await axios.post(url, {}, getAxiosConfig()).catch((error: AxiosError) => { if (error.response && error.response.status === 401) { errorMsg = 'You are not authorised to reject...'; isUserAllowedToReject = false; @@ -121,7 +128,7 @@ const cancelPush = async ( ): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}/cancel`; - await axios.post(url, {}, getAxiosConfig()).catch((error: any) => { + await axios.post(url, {}, getAxiosConfig()).catch((error: AxiosError) => { if (error.response && error.response.status === 401) { setAuth(false); } else { diff --git a/src/ui/services/repo.ts b/src/ui/services/repo.ts index 8aa883d39..b2cc9d29d 100644 --- a/src/ui/services/repo.ts +++ b/src/ui/services/repo.ts @@ -1,7 +1,7 @@ -import axios from 'axios'; +import axios, { AxiosError } from 'axios'; import { getAxiosConfig, processAuthError } from './auth.js'; import { Repo } from '../../db/types'; -import { RepoView } from '../types'; +import { BackendResponse, RepoView } from '../types'; import { getApiV1BaseUrl } from './apiConfig'; const canAddUser = async (repoId: string, user: string, action: string) => { @@ -17,7 +17,7 @@ const canAddUser = async (repoId: string, user: string, action: string) => { return !repo.users.canPush.includes(user); } }) - .catch((error: any) => { + .catch((error: AxiosError) => { throw error; }); }; @@ -35,11 +35,12 @@ const getRepos = async ( setAuth: (auth: boolean) => void, setIsError: (isError: boolean) => void, setErrorMessage: (errorMessage: string) => void, - query: Record = {}, + query: Record = {}, ): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/repo`); - url.search = new URLSearchParams(query as any).toString(); + url.search = new URLSearchParams(query).toString(); + setIsLoading(true); await axios(url.toString(), getAxiosConfig()) .then((response) => { @@ -48,13 +49,13 @@ const getRepos = async ( ); setRepos(sortedRepos); }) - .catch((error: any) => { + .catch((error: AxiosError) => { setIsError(true); if (error.response && error.response.status === 401) { setAuth(false); setErrorMessage(processAuthError(error)); } else { - setErrorMessage(`Error fetching repos: ${error.response.data.message}`); + setErrorMessage(`Error fetching repos: ${error.response?.data?.message ?? error.message}`); } }) .finally(() => { @@ -77,7 +78,7 @@ const getRepo = async ( const repo = response.data; setRepo(repo); }) - .catch((error: any) => { + .catch((error: AxiosError) => { if (error.response && error.response.status === 401) { setAuth(false); } else { @@ -101,12 +102,16 @@ const addRepo = async ( success: true, repo: response.data, }; - } catch (error: any) { - return { - success: false, - message: error.response?.data?.message || error.message, - repo: null, - }; + } catch (error: unknown) { + if (axios.isAxiosError(error)) { + return { + success: false, + message: error.response?.data?.message ?? error.message, + repo: null, + }; + } else { + throw error; + } } }; @@ -116,8 +121,7 @@ const addUser = async (repoId: string, user: string, action: string): Promise { - console.log(error.response.data.message); + await axios.patch(url.toString(), data, getAxiosConfig()).catch((error: AxiosError) => { throw error; }); } else { @@ -130,8 +134,7 @@ const deleteUser = async (user: string, repoId: string, action: string): Promise const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/repo/${repoId}/user/${action}/${user}`); - await axios.delete(url.toString(), getAxiosConfig()).catch((error: any) => { - console.log(error.response.data.message); + await axios.delete(url.toString(), getAxiosConfig()).catch((error: AxiosError) => { throw error; }); }; @@ -140,8 +143,7 @@ const deleteRepo = async (repoId: string): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/repo/${repoId}/delete`); - await axios.delete(url.toString(), getAxiosConfig()).catch((error: any) => { - console.log(error.response.data.message); + await axios.delete(url.toString(), getAxiosConfig()).catch((error: AxiosError) => { throw error; }); }; diff --git a/src/ui/services/user.ts b/src/ui/services/user.ts index 40c0394b5..52086c236 100644 --- a/src/ui/services/user.ts +++ b/src/ui/services/user.ts @@ -1,6 +1,7 @@ import axios, { AxiosError, AxiosResponse } from 'axios'; import { getAxiosConfig, processAuthError } from './auth'; import { PublicUser } from '../../db/types'; +import { BackendResponse } from '../types'; import { getBaseUrl, getApiV1BaseUrl } from './apiConfig'; type SetStateCallback = (value: T | ((prevValue: T) => T)) => void; @@ -26,15 +27,15 @@ const getUser = async ( setUser?.(user); setIsLoading?.(false); - } catch (error) { - const axiosError = error as AxiosError; + } catch (error: unknown) { + const axiosError = error as AxiosError; const status = axiosError.response?.status; if (status === 401) { setAuth?.(false); setErrorMessage?.(processAuthError(axiosError)); } else { - const msg = (axiosError.response?.data as any)?.message ?? 'Unknown error'; - setErrorMessage?.(`Error fetching user: ${status} ${msg}`); + const msg = error instanceof Error ? error.message : String(error); + setErrorMessage?.(`Error fetching user: ${msg}`); } setIsLoading?.(false); } @@ -55,15 +56,19 @@ const getUsers = async ( getAxiosConfig(), ); setUsers(response.data); - } catch (error) { - const axiosError = error as AxiosError; - const status = axiosError.response?.status; - if (status === 401) { - setAuth(false); - setErrorMessage(processAuthError(axiosError)); + } catch (error: unknown) { + if (axios.isAxiosError(error)) { + const status = error.response?.status; + if (status === 401) { + setAuth(false); + setErrorMessage(processAuthError(error)); + } else { + const msg = error.response?.data?.message ?? error.message; + setErrorMessage(`Error fetching users: ${status} ${msg}`); + } } else { - const msg = (axiosError.response?.data as any)?.message ?? 'Unknown error'; - setErrorMessage(`Error fetching users: ${status} ${msg}`); + const msg = error instanceof Error ? error.message : String(error); + setErrorMessage(`Error fetching users: ${msg}`); } } finally { setIsLoading(false); @@ -78,11 +83,15 @@ const updateUser = async ( try { const baseUrl = await getBaseUrl(); await axios.post(`${baseUrl}/api/auth/gitAccount`, user, getAxiosConfig()); - } catch (error) { - const axiosError = error as AxiosError; - const status = axiosError.response?.status; - const msg = (axiosError.response?.data as any)?.message ?? 'Unknown error'; - setErrorMessage(`Error updating user: ${status} ${msg}`); + } catch (error: unknown) { + if (axios.isAxiosError(error)) { + const status = error.response?.status; + const msg = error.response?.data?.message; + setErrorMessage(`Error updating user: ${status} ${msg}`); + } else { + const msg = error instanceof Error ? error.message : String(error); + setErrorMessage(`Error updating user: ${msg}`); + } setIsLoading(false); } }; diff --git a/src/ui/types.ts b/src/ui/types.ts index 342208d56..ebf2b64d9 100644 --- a/src/ui/types.ts +++ b/src/ui/types.ts @@ -1,9 +1,15 @@ +import { CSSProperties } from '@material-ui/core/styles/withStyles'; + import { Action } from '../proxy/actions'; import { Step } from '../proxy/actions/Step'; import { Repo } from '../db/types'; import { Attestation } from '../proxy/processors/types'; import { Question } from '../config/generated/config'; +export interface BackendResponse { + message: string; +} + export interface PushActionView extends Action { diff: Step; } @@ -27,8 +33,8 @@ export interface Route { layout: string; name: string; rtlName?: string; - component: React.ComponentType; - icon?: string | React.ComponentType; + component: React.ComponentType; + icon?: string | React.ComponentType; visible?: boolean; } @@ -89,3 +95,5 @@ export interface SCMRepositoryMetadata { profileUrl?: string; avatarUrl?: string; } + +export type CSSProperty = React.CSSProperties | CSSProperties; diff --git a/src/ui/utils.tsx b/src/ui/utils.tsx index 6a8abfc17..5b8a78d35 100644 --- a/src/ui/utils.tsx +++ b/src/ui/utils.tsx @@ -3,6 +3,7 @@ import React from 'react'; import { GitHubRepositoryMetadata, GitLabRepositoryMetadata, SCMRepositoryMetadata } from './types'; import { CommitData } from '../proxy/processors/types'; import moment from 'moment'; +import { getErrorMessage } from '../utils/errors'; /** * Retrieve a decoded cookie value from `document.cookie` with given `name`. @@ -210,8 +211,9 @@ export const fetchRemoteRepositoryData = async ( const languages = languagesResponse.data; // Get the first key (primary language) from the ordered hash primaryLanguage = Object.keys(languages)[0]; - } catch (languageError) { - console.warn('Could not fetch language data:', languageError); + } catch (error: unknown) { + const msg = getErrorMessage(error); + console.warn('Could not fetch language data:', msg); } return { diff --git a/src/ui/views/Login/Login.tsx b/src/ui/views/Login/Login.tsx index 72962a5f8..8f871b5c8 100644 --- a/src/ui/views/Login/Login.tsx +++ b/src/ui/views/Login/Login.tsx @@ -16,6 +16,7 @@ import { Badge, CircularProgress, FormLabel, Snackbar } from '@material-ui/core' import { useAuth } from '../../auth/AuthProvider'; import { getBaseUrl } from '../../services/apiConfig'; import { getAxiosConfig, processAuthError } from '../../services/auth'; +import { BackendResponse } from '../../types'; interface LoginResponse { username: string; @@ -78,7 +79,7 @@ const Login: React.FC = () => { setSuccess(true); authContext.refreshUser().then(() => navigate(0)); }) - .catch((error: AxiosError) => { + .catch((error: AxiosError) => { if (error.response?.status === 307) { window.sessionStorage.setItem('git.proxy.login', 'success'); setGitAccountError(true); diff --git a/src/ui/views/PushDetails/PushDetails.tsx b/src/ui/views/PushDetails/PushDetails.tsx index aec01fa20..1963bcb18 100644 --- a/src/ui/views/PushDetails/PushDetails.tsx +++ b/src/ui/views/PushDetails/PushDetails.tsx @@ -200,19 +200,14 @@ const Dashboard: React.FC = () => { )}

- {isGitHub && ( - - {push.attestation.reviewer.gitAccount} - - )} - {!isGitHub && }{' '} - approved this contribution + approved this + contribution

diff --git a/src/ui/views/PushDetails/components/AttestationView.tsx b/src/ui/views/PushDetails/components/AttestationView.tsx index c322573f9..9d9802b4d 100644 --- a/src/ui/views/PushDetails/components/AttestationView.tsx +++ b/src/ui/views/PushDetails/components/AttestationView.tsx @@ -75,9 +75,9 @@ const AttestationView: React.FC = ({ attestation, setAttes

- Prior to making this code contribution publicly accessible via GitHub, this code - contribution was reviewed and approved by{' '} - {data.reviewer.gitAccount}. As a + Prior to making this code contribution publicly accessible, this code contribution was + reviewed and approved by{' '} + {data.reviewer.username}. As a reviewer, it was their responsibility to confirm that open sourcing this contribution followed the requirements of the company open source contribution policy.

@@ -85,8 +85,8 @@ const AttestationView: React.FC = ({ attestation, setAttes

- {data.reviewer.gitAccount}{' '} - approved this contribution{' '} + {data.reviewer.email} approved + this contribution{' '} { { tabName: 'Pending', tabIcon: Visibility, - tabContent: ( - - ), + tabContent: , }, { tabName: 'Approved', tabIcon: CheckCircle, - tabContent: , + tabContent: , }, { tabName: 'Canceled', tabIcon: Cancel, - tabContent: ( - - ), + tabContent: , }, { tabName: 'Rejected', tabIcon: Block, - tabContent: ( - - ), + tabContent: , }, { tabName: 'Error', diff --git a/src/ui/views/PushRequests/components/PushesTable.tsx b/src/ui/views/PushRequests/components/PushesTable.tsx index 88052c300..ed9b7565b 100644 --- a/src/ui/views/PushRequests/components/PushesTable.tsx +++ b/src/ui/views/PushRequests/components/PushesTable.tsx @@ -1,5 +1,4 @@ import React, { useState, useEffect } from 'react'; -import { makeStyles } from '@material-ui/core/styles'; import moment from 'moment'; import { useNavigate } from 'react-router-dom'; import Button from '@material-ui/core/Button'; @@ -10,7 +9,6 @@ import TableContainer from '@material-ui/core/TableContainer'; import TableHead from '@material-ui/core/TableHead'; import TableRow from '@material-ui/core/TableRow'; import Paper from '@material-ui/core/Paper'; -import styles from '../../../assets/jss/material-dashboard-react/views/dashboardStyle'; import { getPushes } from '../../../services/git-push'; import { KeyboardArrowRight } from '@material-ui/icons'; import Search from '../../../components/Search/Search'; @@ -20,13 +18,14 @@ import { trimPrefixRefsHeads, trimTrailingDotGit } from '../../../../db/helper'; import { generateAuthorLinks, generateEmailLink } from '../../../utils'; interface PushesTableProps { - [key: string]: any; + blocked?: boolean; + canceled?: boolean; + authorised?: boolean; + rejected?: boolean; + handleError: (error: string) => void; } -const useStyles = makeStyles(styles as any); - const PushesTable: React.FC = (props) => { - const classes = useStyles(); const [pushes, setPushes] = useState([]); const [filteredData, setFilteredData] = useState([]); const [isLoading, setIsLoading] = useState(false); @@ -86,7 +85,7 @@ const PushesTable: React.FC = (props) => {

- +
Timestamp diff --git a/src/ui/views/RepoDetails/Components/AddUser.tsx b/src/ui/views/RepoDetails/Components/AddUser.tsx index bc2d5743f..d79f171f4 100644 --- a/src/ui/views/RepoDetails/Components/AddUser.tsx +++ b/src/ui/views/RepoDetails/Components/AddUser.tsx @@ -62,13 +62,10 @@ const AddUserDialog: React.FC = ({ await addUser(repoId, username, type); handleSuccess(); handleClose(); - } catch (e) { + } catch (error: unknown) { setIsLoading(false); - if (e instanceof Error) { - setError(e.message); - } else { - setError('An unknown error occurred'); - } + const msg = error instanceof Error ? error.message : String(error); + setError(`Error adding user: ${msg}`); } }; diff --git a/src/ui/views/RepoDetails/RepoDetails.tsx b/src/ui/views/RepoDetails/RepoDetails.tsx index c5c1c2ccb..f95fc8f47 100644 --- a/src/ui/views/RepoDetails/RepoDetails.tsx +++ b/src/ui/views/RepoDetails/RepoDetails.tsx @@ -101,7 +101,7 @@ const RepoDetails: React.FC = () => { justifyContent='flex-end' alignItems='center' > - {user.admin && ( + {user?.admin && (
+
Name @@ -95,7 +91,7 @@ const UserList: React.FC = () => { - {user.admin ? ( + {user?.admin ? ( ) : ( diff --git a/src/utils/errors.ts b/src/utils/errors.ts new file mode 100644 index 000000000..9e416d748 --- /dev/null +++ b/src/utils/errors.ts @@ -0,0 +1,15 @@ +export const getErrorMessage = (error: unknown) => { + return error instanceof Error ? error.message : String(error); +}; + +export const handleAndLogError = (error: unknown, message: string): string => { + const msg = `${message}: ${getErrorMessage(error)}`; + console.error(msg); + return msg; +}; + +export const handleAndThrowError = (error: unknown, message: string) => { + const msg = getErrorMessage(error); + console.error(message); + throw new Error(`${message}: ${msg}`); +}; diff --git a/test/1.test.ts b/test/1.test.ts index 28f9d12f3..48d47eb5f 100644 --- a/test/1.test.ts +++ b/test/1.test.ts @@ -13,6 +13,7 @@ import request from 'supertest'; import { Service } from '../src/service'; import * as db from '../src/db'; import { Proxy } from '../src/proxy'; +import { Express } from 'express'; // Create constants for values used in multiple tests const TEST_REPO = { @@ -23,7 +24,7 @@ const TEST_REPO = { }; describe('init', () => { - let app: any; + let app: Express; // Runs before all tests beforeAll(async function () { @@ -74,7 +75,7 @@ describe('init', () => { // fs must be mocked BEFORE importing the config module // We also mock existsSync to ensure the file "exists" vi.doMock('fs', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, readFileSync: vi.fn().mockReturnValue( diff --git a/test/ConfigLoader.test.ts b/test/ConfigLoader.test.ts index 6fea3b27c..c3b8aec23 100644 --- a/test/ConfigLoader.test.ts +++ b/test/ConfigLoader.test.ts @@ -374,9 +374,9 @@ describe('ConfigLoader', () => { configLoader = new ConfigLoader(mockConfig); // private property overridden for testing - (configLoader as any).reloadTimer = setInterval(() => {}, 1000); + configLoader['reloadTimer'] = setInterval(() => {}, 1000); await configLoader.start(); - expect((configLoader as any).reloadTimer).toBe(null); + expect(configLoader['reloadTimer']).toBe(null); }); it('should run reloadConfiguration multiple times on short reload interval', async () => { @@ -422,10 +422,10 @@ describe('ConfigLoader', () => { configLoader = new ConfigLoader(mockConfig); // private property overridden for testing - (configLoader as any).reloadTimer = setInterval(() => {}, 1000); - expect((configLoader as any).reloadTimer).not.toBe(null); + configLoader['reloadTimer'] = setInterval(() => {}, 1000); + expect(configLoader['reloadTimer']).not.toBe(null); await configLoader.stop(); - expect((configLoader as any).reloadTimer).toBe(null); + expect(configLoader['reloadTimer']).toBe(null); }); }); @@ -524,15 +524,15 @@ describe('ConfigLoader', () => { }); it('should throw error if configuration source is invalid', async () => { - const source: ConfigurationSource = { - type: 'invalid' as any, // invalid type + const source = { + type: 'invalid', // invalid type repository: 'https://github.com/finos/git-proxy.git', path: 'proxy.config.json', branch: 'main', enabled: true, }; - await expect(configLoader.loadFromSource(source)).rejects.toThrow( + await expect(configLoader.loadFromSource(source as ConfigurationSource)).rejects.toThrow( /Unsupported configuration source type/, ); }); @@ -732,9 +732,6 @@ describe('Validation Helpers', () => { expect(isValidGitUrl('not-a-git-url')).toBe(false); expect(isValidGitUrl('http://github.com/user/repo')).toBe(false); expect(isValidGitUrl('')).toBe(false); - expect(isValidGitUrl(null as any)).toBe(false); - expect(isValidGitUrl(undefined as any)).toBe(false); - expect(isValidGitUrl(123 as any)).toBe(false); }); }); @@ -750,14 +747,6 @@ describe('Validation Helpers', () => { // Invalid paths expect(isValidPath('')).toBe(false); - expect(isValidPath(null as any)).toBe(false); - expect(isValidPath(undefined as any)).toBe(false); - - // Additional edge cases - expect(isValidPath({} as any)).toBe(false); - expect(isValidPath([] as any)).toBe(false); - expect(isValidPath(123 as any)).toBe(false); - expect(isValidPath(true as any)).toBe(false); expect(isValidPath('\0invalid')).toBe(false); expect(isValidPath('\u0000')).toBe(false); }); @@ -777,8 +766,6 @@ describe('Validation Helpers', () => { expect(isValidBranchName('-invalid')).toBe(false); expect(isValidBranchName('branch with spaces')).toBe(false); expect(isValidBranchName('')).toBe(false); - expect(isValidBranchName(null as any)).toBe(false); - expect(isValidBranchName(undefined as any)).toBe(false); expect(isValidBranchName('branch..name')).toBe(false); }); }); diff --git a/test/chain.test.ts b/test/chain.test.ts index 8ef00c6f5..e38d05216 100644 --- a/test/chain.test.ts +++ b/test/chain.test.ts @@ -396,7 +396,7 @@ describe('proxy chain', function () { await chain.executeChain(req); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error during auto-approval:', error.message); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error during auto-approval: Database error'); }); it('executeChain should handle exceptions in attemptAutoRejection', async () => { @@ -426,6 +426,6 @@ describe('proxy chain', function () { await chain.executeChain(req); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error during auto-rejection:', error.message); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error during auto-rejection: Database error'); }); }); diff --git a/test/checkHiddenCommit.test.ts b/test/checkHiddenCommit.test.ts index 3d07946f4..922b8586f 100644 --- a/test/checkHiddenCommit.test.ts +++ b/test/checkHiddenCommit.test.ts @@ -1,13 +1,15 @@ import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; import { exec as checkHidden } from '../src/proxy/processors/push-action/checkHiddenCommits'; import { Action } from '../src/proxy/actions'; +import { EMPTY_COMMIT_HASH } from '../src/proxy/processors/constants'; +import { Request } from 'express'; // must hoist these before mocking the modules const mockSpawnSync = vi.hoisted(() => vi.fn()); const mockReaddirSync = vi.hoisted(() => vi.fn()); vi.mock('child_process', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, spawnSync: mockSpawnSync, @@ -15,7 +17,7 @@ vi.mock('child_process', async (importOriginal) => { }); vi.mock('fs', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, readdirSync: mockReaddirSync, @@ -24,6 +26,7 @@ vi.mock('fs', async (importOriginal) => { describe('checkHiddenCommits.exec', () => { let action: Action; + let req: Request; beforeEach(() => { // reset all mocks before each test @@ -32,9 +35,10 @@ describe('checkHiddenCommits.exec', () => { // prepare a fresh Action action = new Action('some-id', 'push', 'POST', Date.now(), 'repo.git'); action.proxyGitPath = '/fake'; - action.commitFrom = '0000000000000000000000000000000000000000'; + action.commitFrom = EMPTY_COMMIT_HASH; action.commitTo = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; action.newIdxFiles = ['pack-test.idx']; + req = { body: '' } as Request; }); afterEach(() => { @@ -53,7 +57,7 @@ describe('checkHiddenCommits.exec', () => { mockReaddirSync.mockReturnValue(['pack-test.idx']); - await checkHidden({ body: '' }, action); + await checkHidden(req, action); const step = action.steps.find((s) => s.stepName === 'checkHiddenCommits'); expect(step?.logs).toContain(`checkHiddenCommits - Referenced commits: 0`); @@ -78,7 +82,7 @@ describe('checkHiddenCommits.exec', () => { mockReaddirSync.mockReturnValue(['pack-test.idx']); - await checkHidden({ body: '' }, action); + await checkHidden(req, action); const step = action.steps.find((s) => s.stepName === 'checkHiddenCommits'); expect(step?.logs).toContain('checkHiddenCommits - Referenced commits: 1'); @@ -100,7 +104,7 @@ describe('checkHiddenCommits.exec', () => { mockReaddirSync.mockReturnValue(['pack-test.idx']); - await checkHidden({ body: '' }, action); + await checkHidden(req, action); const step = action.steps.find((s) => s.stepName === 'checkHiddenCommits'); expect(step?.logs).toContain('checkHiddenCommits - Total introduced commits: 2'); @@ -112,9 +116,9 @@ describe('checkHiddenCommits.exec', () => { }); it('throws if commitFrom or commitTo is missing', async () => { - delete (action as any).commitFrom; + delete action.commitFrom; - await expect(checkHidden({ body: '' }, action)).rejects.toThrow( + await expect(checkHidden(req, action)).rejects.toThrow( /Both action.commitFrom and action.commitTo must be defined/, ); }); diff --git a/test/db/db.test.ts b/test/db/db.test.ts index bea72d574..47c9401bc 100644 --- a/test/db/db.test.ts +++ b/test/db/db.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, afterEach, vi, beforeEach } from 'vitest'; +import { SAMPLE_REPO } from '../../src/proxy/processors/constants'; vi.mock('../../src/db/mongo', () => ({ getRepoByUrl: vi.fn(), @@ -27,11 +28,12 @@ describe('db', () => { describe('isUserPushAllowed', () => { it('returns true if user is in canPush', async () => { vi.mocked(mongo.getRepoByUrl).mockResolvedValue({ + ...SAMPLE_REPO, users: { canPush: ['alice'], canAuthorise: [], }, - } as any); + }); const result = await db.isUserPushAllowed('myrepo', 'alice'); expect(result).toBe(true); @@ -39,11 +41,12 @@ describe('db', () => { it('returns true if user is in canAuthorise', async () => { vi.mocked(mongo.getRepoByUrl).mockResolvedValue({ + ...SAMPLE_REPO, users: { canPush: [], canAuthorise: ['bob'], }, - } as any); + }); const result = await db.isUserPushAllowed('myrepo', 'bob'); expect(result).toBe(true); @@ -51,11 +54,12 @@ describe('db', () => { it('returns false if user is in neither', async () => { vi.mocked(mongo.getRepoByUrl).mockResolvedValue({ + ...SAMPLE_REPO, users: { canPush: [], canAuthorise: [], }, - } as any); + }); const result = await db.isUserPushAllowed('myrepo', 'charlie'); expect(result).toBe(false); diff --git a/test/db/file/repo.test.ts b/test/db/file/repo.test.ts index 1a583bc5a..5effe6041 100644 --- a/test/db/file/repo.test.ts +++ b/test/db/file/repo.test.ts @@ -19,8 +19,8 @@ describe('File DB', () => { url: 'http://example.com/sample-repo.git', }; - vi.spyOn(repoModule.db, 'findOne').mockImplementation((query: any, cb: any) => - cb(null, repoData), + vi.spyOn(repoModule.db, 'findOne').mockImplementation( + (_: unknown, cb: (err: Error | null, doc: any) => void) => cb(null, repoData), ); const result = await repoModule.getRepo('Sample'); @@ -36,8 +36,8 @@ describe('File DB', () => { url: 'https://github.com/finos/git-proxy.git', }; - vi.spyOn(repoModule.db, 'findOne').mockImplementation((query: any, cb: any) => - cb(null, repoData), + vi.spyOn(repoModule.db, 'findOne').mockImplementation( + (_: unknown, cb: (err: Error | null, doc: any) => void) => cb(null, repoData), ); const result = await repoModule.getRepoByUrl('https://github.com/finos/git-proxy.git'); @@ -47,7 +47,9 @@ describe('File DB', () => { it('should return null if the repo is not found', async () => { const spy = vi .spyOn(repoModule.db, 'findOne') - .mockImplementation((query: any, cb: any) => cb(null, null)); + .mockImplementation((_: unknown, cb: (err: Error | null, doc: any) => void) => + cb(null, null), + ); const result = await repoModule.getRepoByUrl('https://github.com/finos/missing-repo.git'); @@ -59,8 +61,8 @@ describe('File DB', () => { }); it('should reject if the database returns an error', async () => { - vi.spyOn(repoModule.db, 'findOne').mockImplementation((query: any, cb: any) => - cb(new Error('DB error')), + vi.spyOn(repoModule.db, 'findOne').mockImplementation( + (_: unknown, cb: (err: Error | null, doc: any) => void) => cb(new Error('DB error'), null), ); await expect( diff --git a/test/extractRawBody.test.ts b/test/extractRawBody.test.ts index 1430cf7a9..d6a3851bf 100644 --- a/test/extractRawBody.test.ts +++ b/test/extractRawBody.test.ts @@ -1,5 +1,7 @@ -import { describe, it, beforeEach, expect, vi, Mock } from 'vitest'; +import { Request } from 'express'; +import rawBody from 'raw-body'; import { PassThrough } from 'stream'; +import { describe, it, beforeEach, expect, vi, Mock } from 'vitest'; // Tell Vitest to mock dependencies vi.mock('raw-body', () => ({ @@ -12,7 +14,6 @@ vi.mock('../src/proxy/chain', () => ({ // Now import the module-under-test, which will receive the mocked deps import { extractRawBody, isPackPost } from '../src/proxy/routes'; -import rawBody from 'raw-body'; import * as chain from '../src/proxy/chain'; describe('extractRawBody middleware', () => { @@ -61,20 +62,20 @@ describe('extractRawBody middleware', () => { describe('isPackPost()', () => { it('returns true for git-upload-pack POST', () => { - expect(isPackPost({ method: 'POST', url: '/a/b.git/git-upload-pack' } as any)).toBe(true); + expect(isPackPost({ method: 'POST', url: '/a/b.git/git-upload-pack' } as Request)).toBe(true); }); it('returns true for git-upload-pack POST, with a gitlab style multi-level org', () => { - expect(isPackPost({ method: 'POST', url: '/a/bee/sea/dee.git/git-upload-pack' } as any)).toBe( - true, - ); + expect( + isPackPost({ method: 'POST', url: '/a/bee/sea/dee.git/git-upload-pack' } as Request), + ).toBe(true); }); it('returns true for git-upload-pack POST, with a bare (no org) repo URL', () => { - expect(isPackPost({ method: 'POST', url: '/a.git/git-upload-pack' } as any)).toBe(true); + expect(isPackPost({ method: 'POST', url: '/a.git/git-upload-pack' } as Request)).toBe(true); }); it('returns false for other URLs', () => { - expect(isPackPost({ method: 'POST', url: '/info/refs' } as any)).toBe(false); + expect(isPackPost({ method: 'POST', url: '/info/refs' } as Request)).toBe(false); }); }); diff --git a/test/integration/forcePush.integration.test.ts b/test/integration/forcePush.integration.test.ts index 1cbc2ade3..4844722b1 100644 --- a/test/integration/forcePush.integration.test.ts +++ b/test/integration/forcePush.integration.test.ts @@ -2,10 +2,12 @@ import path from 'path'; import simpleGit, { SimpleGit } from 'simple-git'; import fs from 'fs/promises'; import { describe, it, beforeAll, afterAll, expect } from 'vitest'; +import { Request } from 'express'; -import { Action } from '../../src/proxy/actions'; +import { Action, Step } from '../../src/proxy/actions'; import { exec as getDiff } from '../../src/proxy/processors/push-action/getDiff'; import { exec as scanDiff } from '../../src/proxy/processors/push-action/scanDiff'; +import { SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; describe( 'Force Push Integration Test', @@ -14,6 +16,7 @@ describe( let git: SimpleGit; let initialCommitSHA: string; let rebasedCommitSHA: string; + let req: Request; beforeAll(async () => { tempDir = path.join(__dirname, '../temp-integration-repo'); @@ -45,6 +48,8 @@ describe( console.log(`Initial SHA: ${initialCommitSHA}`); console.log(`Rebased SHA: ${rebasedCommitSHA}`); + + req = {} as Request; }, 10000); afterAll(async () => { @@ -74,6 +79,7 @@ describe( action.commitTo = rebasedCommitSHA; action.commitData = [ { + ...SAMPLE_COMMIT, parent: parentSHA, message: 'Add feature (rebased)', author: 'Test User', @@ -84,10 +90,10 @@ describe( }, ]; - const afterGetDiff = await getDiff({}, action); + const afterGetDiff = await getDiff(req, action); expect(afterGetDiff.steps.length).toBeGreaterThan(0); - const diffStep = afterGetDiff.steps.find((s: any) => s.stepName === 'diff'); + const diffStep = afterGetDiff.steps.find((s: Step) => s.stepName === 'diff'); if (!diffStep) { throw new Error('Diff step not found'); } @@ -96,8 +102,8 @@ describe( expect(typeof diffStep.content).toBe('string'); expect(diffStep.content.length).toBeGreaterThan(0); - const afterScanDiff = await scanDiff({}, afterGetDiff); - const scanStep = afterScanDiff.steps.find((s: any) => s.stepName === 'scanDiff'); + const afterScanDiff = await scanDiff(req, afterGetDiff); + const scanStep = afterScanDiff.steps.find((s: Step) => s.stepName === 'scanDiff'); expect(scanStep).toBeDefined(); expect(scanStep?.error).toBe(false); @@ -118,6 +124,7 @@ describe( action.commitTo = rebasedCommitSHA; action.commitData = [ { + ...SAMPLE_COMMIT, parent: 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef', message: 'Add feature (rebased)', author: 'Test User', @@ -128,10 +135,10 @@ describe( }, ]; - const afterGetDiff = await getDiff({}, action); + const afterGetDiff = await getDiff(req, action); expect(afterGetDiff.steps.length).toBeGreaterThan(0); - const diffStep = afterGetDiff.steps.find((s: any) => s.stepName === 'diff'); + const diffStep = afterGetDiff.steps.find((s: Step) => s.stepName === 'diff'); if (!diffStep) { throw new Error('Diff step not found'); } @@ -144,8 +151,8 @@ describe( ); // scanDiff should not block on missing diff due to error - const afterScanDiff = await scanDiff({}, afterGetDiff); - const scanStep = afterScanDiff.steps.find((s: any) => s.stepName === 'scanDiff'); + const afterScanDiff = await scanDiff(req, afterGetDiff); + const scanStep = afterScanDiff.steps.find((s: Step) => s.stepName === 'scanDiff'); expect(scanStep).toBeDefined(); expect(scanStep?.error).toBe(false); @@ -160,7 +167,7 @@ describe( 'test/repo.git', ); - const result = await scanDiff({}, action); + const result = await scanDiff(req, action); expect(result.steps.length).toBe(1); expect(result.steps[0].stepName).toBe('scanDiff'); diff --git a/test/preReceive/preReceive.test.ts b/test/preReceive/preReceive.test.ts index 7286ce100..2329025b4 100644 --- a/test/preReceive/preReceive.test.ts +++ b/test/preReceive/preReceive.test.ts @@ -1,31 +1,28 @@ +import { Request } from 'express'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import path from 'path'; import * as fs from 'fs'; import { exec } from '../../src/proxy/processors/push-action/preReceive'; +import { Action } from '../../src/proxy/actions'; // TODO: Replace with memfs to prevent test pollution issues vi.mock('fs', { spy: true }); // Pre-receive hooks execute Unix shell scripts, which is not supported on Windows describe.skipIf(process.platform === 'win32')('Pre-Receive Hook Execution', () => { - let action: any; - let req: any; + let action: Action; + let req: Request; beforeEach(() => { - req = {}; - action = { - steps: [] as any[], - commitFrom: 'oldCommitHash', - commitTo: 'newCommitHash', - branch: 'feature-branch', - proxyGitPath: 'test/preReceive/mock/repo', - repoName: 'test-repo', - addStep(step: any) { - this.steps.push(step); - }, - setAutoApproval: vi.fn(), - setAutoRejection: vi.fn(), - }; + req = {} as Request; + action = new Action('123', 'push', 'POST', 1234567890, 'test/repo.git'); + action.commitFrom = 'oldCommitHash'; + action.commitTo = 'newCommitHash'; + action.branch = 'feature-branch'; + action.proxyGitPath = 'test/preReceive/mock/repo'; + action.repoName = 'test-repo'; + action.setAutoApproval = vi.fn(); + action.setAutoRejection = vi.fn(); }); afterEach(() => { diff --git a/test/processors/blockForAuth.test.ts b/test/processors/blockForAuth.test.ts index dc97d0059..2330f84b3 100644 --- a/test/processors/blockForAuth.test.ts +++ b/test/processors/blockForAuth.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import fc from 'fast-check'; +import { Request } from 'express'; import { exec } from '../../src/proxy/processors/push-action/blockForAuth'; import { Step, Action } from '../../src/proxy/actions'; @@ -7,7 +8,7 @@ import * as urls from '../../src/service/urls'; describe('blockForAuth.exec', () => { let mockAction: Action; - let mockReq: any; + let mockReq: Request; beforeEach(() => { // create a fake Action with spies @@ -16,7 +17,7 @@ describe('blockForAuth.exec', () => { addStep: vi.fn(), } as unknown as Action; - mockReq = { some: 'req' }; + mockReq = { some: 'req' } as unknown as Request; // mock getServiceUIURL vi.spyOn(urls, 'getServiceUIURL').mockReturnValue('http://mocked-service-ui'); @@ -32,7 +33,7 @@ describe('blockForAuth.exec', () => { expect(urls.getServiceUIURL).toHaveBeenCalledWith(mockReq); expect(mockAction.addStep).toHaveBeenCalledTimes(1); - const stepArg = (mockAction.addStep as any).mock.calls[0][0]; + const stepArg = vi.mocked(mockAction.addStep).mock.calls[0][0]; expect(stepArg).toBeInstanceOf(Step); expect(stepArg.stepName).toBe('authBlock'); @@ -42,7 +43,7 @@ describe('blockForAuth.exec', () => { it('should set the async block message with the correct format', async () => { await exec(mockReq, mockAction); - const stepArg = (mockAction.addStep as any).mock.calls[0][0]; + const stepArg = vi.mocked(mockAction.addStep).mock.calls[0][0]; const blockMessage = (stepArg as Step).blockedMessage; expect(blockMessage).toContain('GitProxy has received your push ✅'); @@ -62,7 +63,7 @@ describe('blockForAuth.exec', () => { it('should not crash on random req', () => { fc.assert( fc.property(fc.anything(), (req) => { - exec(req, mockAction); + exec(req as Request, mockAction); }), { numRuns: 1000 }, ); diff --git a/test/processors/checkAuthorEmails.test.ts b/test/processors/checkAuthorEmails.test.ts index 6e928005e..e00ea8619 100644 --- a/test/processors/checkAuthorEmails.test.ts +++ b/test/processors/checkAuthorEmails.test.ts @@ -1,20 +1,21 @@ +import { Request } from 'express'; 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 * as configModule from '../../src/config'; import * as validator from 'validator'; -import { CommitData } from '../../src/proxy/processors/types'; +import { SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; // mock dependencies vi.mock('../../src/config', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, getCommitConfig: vi.fn(() => ({})), }; }); vi.mock('validator', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, isEmail: vi.fn(), @@ -23,8 +24,8 @@ vi.mock('validator', async (importOriginal) => { describe('checkAuthorEmails', () => { let mockAction: Action; - let mockReq: any; - let consoleLogSpy: any; + let mockReq: Request; + let consoleLogSpy: ReturnType; beforeEach(async () => { // setup default mocks @@ -55,7 +56,7 @@ describe('checkAuthorEmails', () => { addStep: vi.fn(), } as unknown as Action; - mockReq = {}; + mockReq = {} as Request; }); afterEach(() => { @@ -66,8 +67,8 @@ describe('checkAuthorEmails', () => { describe('basic email validation', () => { it('should allow valid email addresses', async () => { mockAction.commitData = [ - { authorEmail: 'john.doe@example.com' } as CommitData, - { authorEmail: 'jane.smith@company.org' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'john.doe@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'jane.smith@company.org' }, ]; const result = await exec(mockReq, mockAction); @@ -78,7 +79,7 @@ describe('checkAuthorEmails', () => { }); it('should reject empty email', async () => { - mockAction.commitData = [{ authorEmail: '' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: '' }]; const result = await exec(mockReq, mockAction); @@ -88,7 +89,7 @@ describe('checkAuthorEmails', () => { it('should reject null/undefined email', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); - mockAction.commitData = [{ authorEmail: null as any } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: null as unknown as string }]; const result = await exec(mockReq, mockAction); @@ -99,9 +100,9 @@ describe('checkAuthorEmails', () => { it('should reject invalid email format', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); mockAction.commitData = [ - { authorEmail: 'not-an-email' } as CommitData, - { authorEmail: 'missing@domain' } as CommitData, - { authorEmail: '@nodomain.com' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'not-an-email' }, + { ...SAMPLE_COMMIT, authorEmail: 'missing@domain' }, + { ...SAMPLE_COMMIT, authorEmail: '@nodomain.com' }, ]; const result = await exec(mockReq, mockAction); @@ -124,11 +125,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'user@example.com' } as CommitData, - { authorEmail: 'admin@company.org' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'user@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'admin@company.org' }, ]; const result = await exec(mockReq, mockAction); @@ -149,11 +150,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'user@notallowed.com' } as CommitData, - { authorEmail: 'admin@different.org' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'user@notallowed.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'admin@different.org' }, ]; const result = await exec(mockReq, mockAction); @@ -174,11 +175,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'user@subdomain.example.com' } as CommitData, - { authorEmail: 'user@example.com.fake.org' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'user@subdomain.example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'user@example.com.fake.org' }, ]; const result = await exec(mockReq, mockAction); @@ -200,11 +201,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'user@anydomain.com' } as CommitData, - { authorEmail: 'admin@otherdomain.org' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'user@anydomain.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'admin@otherdomain.org' }, ]; const result = await exec(mockReq, mockAction); @@ -227,11 +228,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'noreply@example.com' } as CommitData, - { authorEmail: 'donotreply@company.org' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'noreply@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'donotreply@company.org' }, ]; const result = await exec(mockReq, mockAction); @@ -252,11 +253,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'john.doe@example.com' } as CommitData, - { authorEmail: 'valid.user@company.org' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'john.doe@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'valid.user@company.org' }, ]; const result = await exec(mockReq, mockAction); @@ -277,12 +278,12 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'test@example.com' } as CommitData, - { authorEmail: 'temporary@example.com' } as CommitData, - { authorEmail: 'fakeuser@example.com' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'test@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'temporary@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'fakeuser@example.com' }, ]; const result = await exec(mockReq, mockAction); @@ -303,11 +304,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'noreply@example.com' } as CommitData, - { authorEmail: 'anything@example.com' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'noreply@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'anything@example.com' }, ]; const result = await exec(mockReq, mockAction); @@ -330,12 +331,12 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'valid@example.com' } as CommitData, // valid - { authorEmail: 'noreply@example.com' } as CommitData, // invalid: blocked local - { authorEmail: 'valid@otherdomain.com' } as CommitData, // invalid: wrong domain + { ...SAMPLE_COMMIT, authorEmail: 'valid@example.com' }, // valid + { ...SAMPLE_COMMIT, authorEmail: 'noreply@example.com' }, // invalid: blocked local + { ...SAMPLE_COMMIT, authorEmail: 'valid@otherdomain.com' }, // invalid: wrong domain ]; const result = await exec(mockReq, mockAction); @@ -348,7 +349,7 @@ describe('checkAuthorEmails', () => { describe('exec function behavior', () => { it('should create a step with name "checkAuthorEmails"', async () => { - mockAction.commitData = [{ authorEmail: 'user@example.com' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: 'user@example.com' }]; await exec(mockReq, mockAction); @@ -361,11 +362,11 @@ describe('checkAuthorEmails', () => { it('should handle unique author emails correctly', async () => { mockAction.commitData = [ - { authorEmail: 'user1@example.com' } as CommitData, - { authorEmail: 'user2@example.com' } as CommitData, - { authorEmail: 'user1@example.com' } as CommitData, // Duplicate - { authorEmail: 'user3@example.com' } as CommitData, - { authorEmail: 'user2@example.com' } as CommitData, // Duplicate + { ...SAMPLE_COMMIT, authorEmail: 'user1@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'user2@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'user1@example.com' }, // Duplicate + { ...SAMPLE_COMMIT, authorEmail: 'user3@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'user2@example.com' }, // Duplicate ]; await exec(mockReq, mockAction); @@ -395,15 +396,15 @@ describe('checkAuthorEmails', () => { it('should log error message when illegal emails found', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); - mockAction.commitData = [{ authorEmail: 'invalid-email' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: 'invalid-email' }]; await exec(mockReq, mockAction); }); it('should log success message when all emails are legal', async () => { mockAction.commitData = [ - { authorEmail: 'user1@example.com' } as CommitData, - { authorEmail: 'user2@example.com' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'user1@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'user2@example.com' }, ]; await exec(mockReq, mockAction); @@ -415,7 +416,7 @@ describe('checkAuthorEmails', () => { it('should set error on step when illegal emails found', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); - mockAction.commitData = [{ authorEmail: 'bad@email' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: 'bad@email' }]; await exec(mockReq, mockAction); @@ -425,7 +426,7 @@ describe('checkAuthorEmails', () => { it('should call step.setError with user-friendly message', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); - mockAction.commitData = [{ authorEmail: 'bad' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: 'bad' }]; await exec(mockReq, mockAction); @@ -437,7 +438,7 @@ describe('checkAuthorEmails', () => { }); it('should return the action object', async () => { - mockAction.commitData = [{ authorEmail: 'user@example.com' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: 'user@example.com' }]; const result = await exec(mockReq, mockAction); @@ -446,9 +447,9 @@ describe('checkAuthorEmails', () => { it('should handle mixed valid and invalid emails', async () => { mockAction.commitData = [ - { authorEmail: 'valid@example.com' } as CommitData, - { authorEmail: 'invalid' } as CommitData, - { authorEmail: 'also.valid@example.com' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'valid@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'invalid' }, + { ...SAMPLE_COMMIT, authorEmail: 'also.valid@example.com' }, ]; vi.mocked(validator.isEmail).mockImplementation((email: string) => { @@ -471,7 +472,7 @@ describe('checkAuthorEmails', () => { describe('edge cases', () => { it('should handle email with multiple @ symbols', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); - mockAction.commitData = [{ authorEmail: 'user@@example.com' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: 'user@@example.com' }]; const result = await exec(mockReq, mockAction); @@ -481,7 +482,7 @@ describe('checkAuthorEmails', () => { it('should handle email without domain', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); - mockAction.commitData = [{ authorEmail: 'user@' } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: 'user@' }]; const result = await exec(mockReq, mockAction); @@ -492,7 +493,7 @@ describe('checkAuthorEmails', () => { it('should handle very long email addresses', async () => { const longLocal = 'a'.repeat(64); const longEmail = `${longLocal}@example.com`; - mockAction.commitData = [{ authorEmail: longEmail } as CommitData]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: longEmail }]; const result = await exec(mockReq, mockAction); @@ -501,9 +502,9 @@ describe('checkAuthorEmails', () => { it('should handle special characters in local part', async () => { mockAction.commitData = [ - { authorEmail: 'user+tag@example.com' } as CommitData, - { authorEmail: 'user.name@example.com' } as CommitData, - { authorEmail: 'user_name@example.com' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'user+tag@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'user.name@example.com' }, + { ...SAMPLE_COMMIT, authorEmail: 'user_name@example.com' }, ]; const result = await exec(mockReq, mockAction); @@ -524,11 +525,11 @@ describe('checkAuthorEmails', () => { }, }, }, - } as any); + }); mockAction.commitData = [ - { authorEmail: 'user@EXAMPLE.COM' } as CommitData, - { authorEmail: 'user@Example.Com' } as CommitData, + { ...SAMPLE_COMMIT, authorEmail: 'user@EXAMPLE.COM' }, + { ...SAMPLE_COMMIT, authorEmail: 'user@Example.Com' }, ]; const result = await exec(mockReq, mockAction); diff --git a/test/processors/checkCommitMessages.test.ts b/test/processors/checkCommitMessages.test.ts index c1fff3c02..c5f5f673f 100644 --- a/test/processors/checkCommitMessages.test.ts +++ b/test/processors/checkCommitMessages.test.ts @@ -1,11 +1,12 @@ +import { Request } from 'express'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { exec } from '../../src/proxy/processors/push-action/checkCommitMessages'; import { Action } from '../../src/proxy/actions'; import * as configModule from '../../src/config'; -import { CommitData } from '../../src/proxy/processors/types'; +import { SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; vi.mock('../../src/config', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, getCommitConfig: vi.fn(() => ({})), @@ -13,6 +14,8 @@ vi.mock('../../src/config', async (importOriginal) => { }); describe('checkCommitMessages', () => { + let action: Action; + let req: Request; let consoleLogSpy: ReturnType; let mockCommitConfig: any; @@ -31,6 +34,9 @@ describe('checkCommitMessages', () => { }; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); + + action = new Action('test', 'test', 'test', 1, 'test'); + req = {} as Request; }); afterEach(() => { @@ -40,38 +46,34 @@ describe('checkCommitMessages', () => { describe('isMessageAllowed', () => { describe('Empty or invalid messages', () => { it('should block empty string commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: '' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: '' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith('No commit message included...'); }); it('should block null commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: null as any } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: null as unknown as string }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block undefined commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: undefined as any } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: undefined as unknown as string }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block non-string commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 123 as any } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 123 as unknown as string }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -80,19 +82,19 @@ describe('checkCommitMessages', () => { }); it('should block object commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: { text: 'fix: bug' } as any } as CommitData]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: { text: 'fix: bug' } as unknown as string }, + ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block array commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: ['fix: bug'] as any } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: ['fix: bug'] as unknown as string }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -100,10 +102,9 @@ describe('checkCommitMessages', () => { describe('Blocked literals', () => { it('should block messages containing blocked literals (exact case)', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Add password to config' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password to config' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -112,32 +113,29 @@ describe('checkCommitMessages', () => { }); it('should block messages containing blocked literals (case insensitive)', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'Add PASSWORD to config' } as CommitData, - { message: 'Store Secret key' } as CommitData, - { message: 'Update TOKEN value' } as CommitData, + { ...SAMPLE_COMMIT, message: 'Add PASSWORD to config' }, + { ...SAMPLE_COMMIT, message: 'Store Secret key' }, + { ...SAMPLE_COMMIT, message: 'Update TOKEN value' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block messages with literals in the middle of words', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Update mypassword123' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Update mypassword123' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block when multiple literals are present', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Add password and secret token' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password and secret token' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -145,30 +143,29 @@ describe('checkCommitMessages', () => { describe('Blocked patterns', () => { it('should block messages containing http URLs', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'See http://example.com for details' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'See http://example.com for details' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block messages containing https URLs', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Update docs at https://docs.example.com' } as CommitData]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: 'Update docs at https://docs.example.com' }, + ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block messages with multiple URLs', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'See http://example.com and https://other.com' } as CommitData, + { ...SAMPLE_COMMIT, message: 'See http://example.com and https://other.com' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -177,10 +174,9 @@ describe('checkCommitMessages', () => { mockCommitConfig.message.block.patterns = ['\\d{3}-\\d{2}-\\d{4}']; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'SSN: 123-45-6789' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'SSN: 123-45-6789' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -189,10 +185,9 @@ describe('checkCommitMessages', () => { mockCommitConfig.message.block.patterns = ['PRIVATE']; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'This is private information' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'This is private information' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -200,10 +195,9 @@ describe('checkCommitMessages', () => { describe('Combined blocking (literals and patterns)', () => { it('should block when both literals and patterns match', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'password at http://example.com' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'password at http://example.com' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -212,10 +206,9 @@ describe('checkCommitMessages', () => { mockCommitConfig.message.block.patterns = []; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Add secret key' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add secret key' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -224,10 +217,9 @@ describe('checkCommitMessages', () => { mockCommitConfig.message.block.literals = []; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Visit http://example.com' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Visit http://example.com' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -235,10 +227,11 @@ describe('checkCommitMessages', () => { describe('Allowed messages', () => { it('should allow valid commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'fix: resolve bug in user authentication' } as CommitData]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: 'fix: resolve bug in user authentication' }, + ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -247,14 +240,13 @@ describe('checkCommitMessages', () => { }); it('should allow messages with no blocked content', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'feat: add new feature' } as CommitData, - { message: 'chore: update dependencies' } as CommitData, - { message: 'docs: improve documentation' } as CommitData, + { ...SAMPLE_COMMIT, message: 'feat: add new feature' }, + { ...SAMPLE_COMMIT, message: 'chore: update dependencies' }, + { ...SAMPLE_COMMIT, message: 'docs: improve documentation' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -264,10 +256,9 @@ describe('checkCommitMessages', () => { mockCommitConfig.message.block.patterns = []; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Any message should pass' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Any message should pass' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -275,65 +266,60 @@ describe('checkCommitMessages', () => { describe('Multiple commits', () => { it('should handle multiple valid commits', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'feat: add feature A' } as CommitData, - { message: 'fix: resolve issue B' } as CommitData, - { message: 'chore: update config C' } as CommitData, + { ...SAMPLE_COMMIT, message: 'feat: add feature A' }, + { ...SAMPLE_COMMIT, message: 'fix: resolve issue B' }, + { ...SAMPLE_COMMIT, message: 'chore: update config C' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); it('should block when any commit is invalid', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'feat: add feature A' } as CommitData, - { message: 'fix: add password to config' } as CommitData, - { message: 'chore: update config C' } as CommitData, + { ...SAMPLE_COMMIT, message: 'feat: add feature A' }, + { ...SAMPLE_COMMIT, message: 'fix: add password to config' }, + { ...SAMPLE_COMMIT, message: 'chore: update config C' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should block when multiple commits are invalid', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'Add password' } as CommitData, - { message: 'Store secret' } as CommitData, - { message: 'feat: valid message' } as CommitData, + { ...SAMPLE_COMMIT, message: 'Add password' }, + { ...SAMPLE_COMMIT, message: 'Store secret' }, + { ...SAMPLE_COMMIT, message: 'feat: valid message' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should deduplicate commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'fix: bug' } as CommitData, - { message: 'fix: bug' } as CommitData, + { ...SAMPLE_COMMIT, message: 'fix: bug' }, + { ...SAMPLE_COMMIT, message: 'fix: bug' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); it('should handle mix of duplicate valid and invalid messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'fix: bug' } as CommitData, - { message: 'Add password' } as CommitData, - { message: 'fix: bug' } as CommitData, + { ...SAMPLE_COMMIT, message: 'fix: bug' }, + { ...SAMPLE_COMMIT, message: 'Add password' }, + { ...SAMPLE_COMMIT, message: 'fix: bug' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -341,19 +327,17 @@ describe('checkCommitMessages', () => { describe('Error handling and logging', () => { it('should set error flag on step when messages are illegal', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Add password' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should log error message to step', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Add password' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password' }]; - const result = await exec({}, action); + const result = await exec(req, action); const step = result.steps[0]; // first log is the "push blocked" message @@ -363,10 +347,9 @@ describe('checkCommitMessages', () => { }); it('should set detailed error message', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Add secret' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add secret' }]; - const result = await exec({}, action); + const result = await exec(req, action); const step = result.steps[0]; expect(step.errorMessage).toContain('Your push has been blocked'); @@ -374,13 +357,12 @@ describe('checkCommitMessages', () => { }); it('should include all illegal messages in error', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'Add password' } as CommitData, - { message: 'Store token' } as CommitData, + { ...SAMPLE_COMMIT, message: 'Add password' }, + { ...SAMPLE_COMMIT, message: 'Store token' }, ]; - const result = await exec({}, action); + const result = await exec(req, action); const step = result.steps[0]; expect(step.errorMessage).toContain('Add password'); @@ -390,39 +372,35 @@ describe('checkCommitMessages', () => { describe('Edge cases', () => { it('should handle action with no commitData', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = undefined; - const result = await exec({}, action); + const result = await exec(req, action); // should handle gracefully expect(result.steps).toHaveLength(1); }); it('should handle action with empty commitData array', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = []; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); it('should handle whitespace-only messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: ' ' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: ' ' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); it('should handle very long commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); const longMessage = 'fix: ' + 'a'.repeat(10000); - action.commitData = [{ message: longMessage } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: longMessage }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -431,19 +409,17 @@ describe('checkCommitMessages', () => { mockCommitConfig.message.block.literals = ['$pecial', 'char*']; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Contains $pecial characters' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Contains $pecial characters' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); it('should handle unicode characters in messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'feat: 添加新功能 🎉' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'feat: 添加新功能 🎉' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -452,11 +428,10 @@ describe('checkCommitMessages', () => { mockCommitConfig.message.block.patterns = ['[invalid']; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Any message' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Any message' }]; // test that it doesn't crash - expect(() => exec({}, action)).not.toThrow(); + expect(() => exec(req, action)).not.toThrow(); }); }); @@ -468,29 +443,26 @@ describe('checkCommitMessages', () => { describe('Step management', () => { it('should create a step named "checkCommitMessages"', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'fix: bug' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps[0].stepName).toBe('checkCommitMessages'); }); it('should add step to action', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'fix: bug' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; const initialStepCount = action.steps.length; - const result = await exec({}, action); + const result = await exec(req, action); expect(result.steps.length).toBe(initialStepCount + 1); }); it('should return the same action object', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'fix: bug' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; - const result = await exec({}, action); + const result = await exec(req, action); expect(result).toBe(action); }); @@ -498,11 +470,10 @@ describe('checkCommitMessages', () => { describe('Request parameter', () => { it('should accept request parameter without using it', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'fix: bug' } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; const mockRequest = { headers: {}, body: {} }; - const result = await exec(mockRequest, action); + const result = await exec(mockRequest as Request, action); expect(result.steps[0].error).toBe(false); }); diff --git a/test/processors/checkEmptyBranch.test.ts b/test/processors/checkEmptyBranch.test.ts index 1b5e182c4..f150b9c87 100644 --- a/test/processors/checkEmptyBranch.test.ts +++ b/test/processors/checkEmptyBranch.test.ts @@ -1,13 +1,14 @@ +import { Request } from 'express'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { Action } from '../../src/proxy/actions'; -import { EMPTY_COMMIT_HASH } from '../../src/proxy/processors/constants'; +import { EMPTY_COMMIT_HASH, SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; vi.mock('simple-git'); vi.mock('fs'); describe('checkEmptyBranch', () => { - let exec: (req: any, action: Action) => Promise; - let simpleGitMock: any; + let exec: (req: Request, action: Action) => Promise; + let simpleGitMock: ReturnType; let gitRawMock: ReturnType; beforeEach(async () => { @@ -25,7 +26,7 @@ describe('checkEmptyBranch', () => { // mocking fs to prevent simple-git from validating directories vi.doMock('fs', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, existsSync: vi.fn().mockReturnValue(true), @@ -48,10 +49,10 @@ describe('checkEmptyBranch', () => { describe('exec', () => { let action: Action; - let req: any; + let req: Request; beforeEach(() => { - req = {}; + req = {} as Request; action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo'); action.proxyGitPath = '/tmp/gitproxy'; action.repoName = 'test-repo'; @@ -61,7 +62,7 @@ describe('checkEmptyBranch', () => { }); it('should pass through if commitData is already populated', async () => { - action.commitData = [{ message: 'Existing commit' }] as any; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Existing commit' }]; const result = await exec(req, action); diff --git a/test/processors/checkIfWaitingAuth.test.ts b/test/processors/checkIfWaitingAuth.test.ts index fe68bab4a..9645d522a 100644 --- a/test/processors/checkIfWaitingAuth.test.ts +++ b/test/processors/checkIfWaitingAuth.test.ts @@ -1,3 +1,4 @@ +import { Request } from 'express'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { Action } from '../../src/proxy/actions'; import * as checkIfWaitingAuthModule from '../../src/proxy/processors/push-action/checkIfWaitingAuth'; @@ -20,10 +21,10 @@ describe('checkIfWaitingAuth', () => { describe('exec', () => { let action: Action; - let req: any; + let req: Request; beforeEach(() => { - req = {}; + req = {} as Request; action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo.git'); }); diff --git a/test/processors/checkUserPushPermission.test.ts b/test/processors/checkUserPushPermission.test.ts index 6e029a321..e4627711f 100644 --- a/test/processors/checkUserPushPermission.test.ts +++ b/test/processors/checkUserPushPermission.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import fc from 'fast-check'; import { Action, Step } from '../../src/proxy/actions'; import type { Mock } from 'vitest'; +import { Request } from 'express'; vi.mock('../../src/db', () => ({ getUsers: vi.fn(), @@ -32,11 +33,11 @@ describe('checkUserPushPermission', () => { describe('exec', () => { let action: Action; - let req: any; + let req: Request; let stepLogSpy: ReturnType; beforeEach(() => { - req = {}; + req = {} as Request; action = new Action( '1234567890', 'push', diff --git a/test/processors/getDiff.test.ts b/test/processors/getDiff.test.ts index 67233642a..103b69b54 100644 --- a/test/processors/getDiff.test.ts +++ b/test/processors/getDiff.test.ts @@ -1,12 +1,13 @@ +import { Request } from 'express'; import path from 'path'; import simpleGit, { SimpleGit } from 'simple-git'; import fs from 'fs/promises'; import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import fc from 'fast-check'; + import { Action } from '../../src/proxy/actions'; import { exec } from '../../src/proxy/processors/push-action/getDiff'; -import { CommitData } from '../../src/proxy/processors/types'; -import { EMPTY_COMMIT_HASH } from '../../src/proxy/processors/constants'; +import { EMPTY_COMMIT_HASH, SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; describe('getDiff', () => { let tempDir: string; @@ -51,9 +52,9 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; action.commitTo = 'HEAD'; - action.commitData = [{ parent: EMPTY_COMMIT_HASH } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, parent: EMPTY_COMMIT_HASH }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); expect(result.steps[0].content).toContain('modified content'); @@ -75,9 +76,9 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; action.commitTo = 'HEAD'; - action.commitData = [{ parent: EMPTY_COMMIT_HASH } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, parent: EMPTY_COMMIT_HASH }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); expect(result.steps[0].content).toContain('initial content'); @@ -91,7 +92,7 @@ describe('getDiff', () => { action.commitTo = 'HEAD'; action.commitData = []; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); expect(result.steps[0].errorMessage).toContain( 'Your push has been blocked because no commit data was found', @@ -104,9 +105,9 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; action.commitTo = 'HEAD'; - action.commitData = undefined as any; + action.commitData = undefined; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); expect(result.steps[0].errorMessage).toContain( 'Your push has been blocked because no commit data was found', @@ -128,9 +129,9 @@ describe('getDiff', () => { action.repoName = path.basename(tempDir); action.commitFrom = EMPTY_COMMIT_HASH; action.commitTo = headCommit; - action.commitData = [{ parent: parentCommit } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, parent: parentCommit }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); expect(result.steps[0].content).not.toBeNull(); @@ -152,9 +153,12 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = from; action.commitTo = to; - action.commitData = commitData as any; + action.commitData = commitData.map((commit) => ({ + ...SAMPLE_COMMIT, + parent: commit.parent, + })); - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result).toHaveProperty('steps'); expect(result.steps[0]).toHaveProperty('error'); @@ -176,9 +180,9 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = from; action.commitTo = to; - action.commitData = [{ parent: EMPTY_COMMIT_HASH } as CommitData]; + action.commitData = [{ ...SAMPLE_COMMIT, parent: EMPTY_COMMIT_HASH }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); expect(result.steps[0].errorMessage).toContain('Invalid revision range'); diff --git a/test/processors/gitLeaks.test.ts b/test/processors/gitLeaks.test.ts index 3e9d9234a..c8c0d6310 100644 --- a/test/processors/gitLeaks.test.ts +++ b/test/processors/gitLeaks.test.ts @@ -1,8 +1,10 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { Request } from 'express'; + import { Action, Step } from '../../src/proxy/actions'; vi.mock('../../src/config', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, getAPIs: vi.fn(), @@ -10,7 +12,7 @@ vi.mock('../../src/config', async (importOriginal) => { }); vi.mock('node:fs/promises', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, default: { @@ -25,7 +27,7 @@ vi.mock('node:fs/promises', async (importOriginal) => { }); vi.mock('node:child_process', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, spawn: vi.fn(), @@ -34,14 +36,14 @@ vi.mock('node:child_process', async (importOriginal) => { describe('gitleaks', () => { describe('exec', () => { - let exec: any; + let exec: typeof import('../../src/proxy/processors/push-action/gitleaks').exec; let action: Action; - let req: any; - let stepSpy: any; - let logStub: any; - let errorStub: any; - let getAPIs: any; - let fsModule: any; + let req: Request; + let stepSpy: ReturnType; + let logStub: ReturnType; + let errorStub: ReturnType; + let getAPIs: typeof import('../../src/config').getAPIs; + let fsModule: typeof import('node:fs/promises'); let spawn: any; beforeEach(async () => { @@ -62,7 +64,7 @@ describe('gitleaks', () => { const gitleaksModule = await import('../../src/proxy/processors/push-action/gitleaks'); exec = gitleaksModule.exec; - req = {}; + req = {} as Request; action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo.git'); action.proxyGitPath = '/tmp'; action.repoName = 'test-repo'; @@ -90,8 +92,7 @@ describe('gitleaks', () => { 'failed setup gitleaks, please contact an administrator\n', ); expect(errorStub).toHaveBeenCalledWith( - 'failed to get gitleaks config, please fix the error:', - expect.any(Error), + 'failed to get gitleaks config, please fix the error: Config error', ); }); @@ -129,7 +130,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitRootCommitMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitRootCommitMock.stderr) }, - } as any) + }) .mockReturnValueOnce({ on: (event: string, cb: (exitCode: number) => void) => { if (event === 'close') cb(gitleaksMock.exitCode); @@ -137,7 +138,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitleaksMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitleaksMock.stderr) }, - } as any); + }); const result = await exec(req, action); @@ -171,7 +172,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitRootCommitMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitRootCommitMock.stderr) }, - } as any) + }) .mockReturnValueOnce({ on: (event: string, cb: (exitCode: number) => void) => { if (event === 'close') cb(gitleaksMock.exitCode); @@ -179,7 +180,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitleaksMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitleaksMock.stderr) }, - } as any); + }); const result = await exec(req, action); @@ -212,7 +213,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitRootCommitMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitRootCommitMock.stderr) }, - } as any) + }) .mockReturnValueOnce({ on: (event: string, cb: (exitCode: number) => void) => { if (event === 'close') cb(gitleaksMock.exitCode); @@ -220,7 +221,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitleaksMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitleaksMock.stderr) }, - } as any); + }); const result = await exec(req, action); @@ -244,7 +245,7 @@ describe('gitleaks', () => { expect(result.steps).toHaveLength(1); expect(result.steps[0].error).toBe(true); expect(stepSpy).toHaveBeenCalledWith( - 'failed to spawn gitleaks, please contact an administrator\n', + 'failed to spawn gitleaks, please contact an administrator\n: Spawn error', ); }); @@ -265,7 +266,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb('') }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb('') }, - } as any); + }); const result = await exec(req, action); @@ -305,7 +306,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitRootCommitMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitRootCommitMock.stderr) }, - } as any) + }) .mockReturnValueOnce({ on: (event: string, cb: (exitCode: number) => void) => { if (event === 'close') cb(gitleaksMock.exitCode); @@ -313,7 +314,7 @@ describe('gitleaks', () => { }, stdout: { on: (_: string, cb: (stdout: string) => void) => cb(gitleaksMock.stdout) }, stderr: { on: (_: string, cb: (stderr: string) => void) => cb(gitleaksMock.stderr) }, - } as any); + }); const result = await exec(req, action); diff --git a/test/processors/scanDiff.emptyDiff.test.ts b/test/processors/scanDiff.emptyDiff.test.ts index f5a362238..39ca456c4 100644 --- a/test/processors/scanDiff.emptyDiff.test.ts +++ b/test/processors/scanDiff.emptyDiff.test.ts @@ -1,4 +1,6 @@ import { describe, it, expect } from 'vitest'; +import { Request } from 'express'; + import { Action, Step } from '../../src/proxy/actions'; import { exec } from '../../src/proxy/processors/push-action/scanDiff'; import { generateDiffStep } from './scanDiff.test'; @@ -12,7 +14,7 @@ describe('scanDiff - Empty Diff Handling', () => { const diffStep = generateDiffStep(''); action.steps = [diffStep as Step]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps.length).toBe(2); // diff step + scanDiff step expect(result.steps[1].error).toBe(false); @@ -26,7 +28,7 @@ describe('scanDiff - Empty Diff Handling', () => { const diffStep = generateDiffStep(null); action.steps = [diffStep as Step]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps.length).toBe(2); expect(result.steps[1].error).toBe(false); @@ -40,7 +42,7 @@ describe('scanDiff - Empty Diff Handling', () => { const diffStep = generateDiffStep(undefined); action.steps = [diffStep as Step]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps.length).toBe(2); expect(result.steps[1].error).toBe(false); @@ -67,7 +69,7 @@ index 1234567..abcdefg 100644 const diffStep = generateDiffStep(normalDiff); action.steps = [diffStep as Step]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[1].error).toBe(false); expect(result.steps[1].errorMessage).toBeNull(); @@ -77,10 +79,10 @@ index 1234567..abcdefg 100644 describe('Error conditions', () => { it('should handle non-string diff content', async () => { const action = new Action('non-string-test', 'push', 'POST', Date.now(), 'test/repo.git'); - const diffStep = generateDiffStep(12345 as any); + const diffStep = generateDiffStep(12345 as unknown as string); action.steps = [diffStep as Step]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[1].error).toBe(true); expect(result.steps[1].errorMessage).toContain('non-string value'); diff --git a/test/processors/scanDiff.test.ts b/test/processors/scanDiff.test.ts index 13c4d54c3..2e475abc8 100644 --- a/test/processors/scanDiff.test.ts +++ b/test/processors/scanDiff.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import crypto from 'crypto'; +import { Request } from 'express'; import * as processor from '../../src/proxy/processors/push-action/scanDiff'; import { Action, Step } from '../../src/proxy/actions'; import * as config from '../../src/config'; @@ -77,7 +78,7 @@ const TEST_REPO = { project: 'private-org-test', name: 'repo.git', url: 'https://github.com/private-org-test/repo.git', - _id: undefined as any, + _id: '', }; describe('Scan commit diff', () => { @@ -117,7 +118,7 @@ describe('Scan commit diff', () => { action.setBranch('b'); action.setMessage('Message'); - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -130,7 +131,7 @@ describe('Scan commit diff', () => { action.steps = [diffStep]; action.setCommit('8b97e49', 'de18d43'); - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -145,7 +146,7 @@ describe('Scan commit diff', () => { action.steps = [diffStep]; action.setCommit('8b97e49', 'de18d43'); - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -164,7 +165,7 @@ describe('Scan commit diff', () => { action.commitFrom = '38cdc3e'; action.commitTo = '8a9c321'; - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -177,7 +178,7 @@ describe('Scan commit diff', () => { ); action.steps = [diffStep]; - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -192,7 +193,7 @@ describe('Scan commit diff', () => { action.commitFrom = '38cdc3e'; action.commitTo = '8a9c321'; - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -207,7 +208,7 @@ describe('Scan commit diff', () => { action.commitFrom = '38cdc3e'; action.commitTo = '8a9c321'; - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -224,7 +225,7 @@ describe('Scan commit diff', () => { action.commitFrom = '38cdc3e'; action.commitTo = '8a9c321'; - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -238,7 +239,7 @@ describe('Scan commit diff', () => { action.commitFrom = '38cdc3e'; action.commitTo = '8a9c321'; - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -249,7 +250,7 @@ describe('Scan commit diff', () => { const action = new Action('1', 'type', 'method', 1, 'test/repo.git'); action.steps = [generateDiffStep(null)]; - const result = await processor.exec(null, action); + const result = await processor.exec({} as Request, action); const scanDiffStep = result.steps.find((s) => s.stepName === 'scanDiff'); expect(scanDiffStep?.error).toBe(false); @@ -257,9 +258,9 @@ describe('Scan commit diff', () => { it('should block push when diff is not a string', async () => { const action = new Action('1', 'type', 'method', 1, 'test/repo.git'); - action.steps = [generateDiffStep(1337 as any)]; + action.steps = [generateDiffStep(1337 as unknown as string)]; - const { error, errorMessage } = await processor.exec(null, action); + const { error, errorMessage } = await processor.exec({} as Request, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); @@ -271,7 +272,7 @@ describe('Scan commit diff', () => { action.commitFrom = '38cdc3e'; action.commitTo = '8a9c321'; - const { error } = await processor.exec(null, action); + const { error } = await processor.exec({} as Request, action); expect(error).toBe(false); }); @@ -287,7 +288,7 @@ describe('Scan commit diff', () => { const diffStep = generateDiffStep(generateDiff('AKIAIOSFODNN7EXAMPLE')); action.steps = [diffStep]; - const { error } = await processor.exec(null, action); + const { error } = await processor.exec({} as Request, action); expect(error).toBe(false); }); diff --git a/test/processors/writePack.test.ts b/test/processors/writePack.test.ts index 494ef504d..b0a0cf061 100644 --- a/test/processors/writePack.test.ts +++ b/test/processors/writePack.test.ts @@ -3,25 +3,24 @@ import path from 'path'; import { Action, Step } from '../../src/proxy/actions'; import * as childProcess from 'child_process'; import * as fs from 'fs'; +import { Request } from 'express'; vi.mock('child_process'); vi.mock('fs'); describe('writePack', () => { - let exec: any; - let readdirSyncMock: any; - let spawnSyncMock: any; - let stepLogSpy: any; - let stepSetErrorSpy: any; + let exec: typeof import('../../src/proxy/processors/push-action/writePack').exec; + let readdirSyncMock: ReturnType; + let spawnSyncMock: ReturnType; + let stepLogSpy: ReturnType; + let stepSetErrorSpy: ReturnType; beforeEach(async () => { vi.clearAllMocks(); spawnSyncMock = vi.mocked(childProcess.spawnSync); readdirSyncMock = vi.mocked(fs.readdirSync); - readdirSyncMock - .mockReturnValueOnce(['old1.idx'] as any) - .mockReturnValueOnce(['old1.idx', 'new1.idx'] as any); + readdirSyncMock.mockReturnValueOnce(['old1.idx']).mockReturnValueOnce(['old1.idx', 'new1.idx']); stepLogSpy = vi.spyOn(Step.prototype, 'log'); stepSetErrorSpy = vi.spyOn(Step.prototype, 'setError'); @@ -36,12 +35,12 @@ describe('writePack', () => { describe('exec', () => { let action: Action; - let req: any; + let req: Request; beforeEach(() => { req = { body: 'pack data', - }; + } as Request; action = new Action( '1234567890', diff --git a/test/proxy.test.ts b/test/proxy.test.ts index d839981ca..700109a59 100644 --- a/test/proxy.test.ts +++ b/test/proxy.test.ts @@ -1,23 +1,13 @@ import http from 'http'; import https from 'https'; -import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; +import { describe, it, beforeEach, afterEach, expect, vi, Mock } from 'vitest'; import fs from 'fs'; +import { GitProxyConfig } from '../src/config/generated/config'; +import { Proxy } from '../src/proxy'; +import { handleAndLogError } from '../src/utils/errors'; -/* - jescalada: these tests are currently causing the following error - when running tests in the CI or for the first time locally: - Error: listen EADDRINUSE: address already in use :::8000 - - This is likely due to improper test isolation or cleanup in another test file - especially related to proxy.start() and proxy.stop() calls - - Related: skipped tests in testProxyRoute.test.ts - these have a race condition - where either these or those tests fail depending on execution order - TODO: Find root cause of this error and fix it - https://github.com/finos/git-proxy/issues/1294 -*/ describe('Proxy Module TLS Certificate Loading', () => { - let proxyModule: any; + let proxyModule: Proxy; let mockConfig: any; let mockHttpServer: any; let mockHttpsServer: any; @@ -74,7 +64,7 @@ describe('Proxy Module TLS Certificate Loading', () => { }); vi.doMock('../src/config', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, getTLSEnabled: mockConfig.getTLSEnabled, @@ -98,7 +88,7 @@ describe('Proxy Module TLS Certificate Loading', () => { }); vi.doMock('../src/proxy/chain', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, chainPluginLoader: null, @@ -116,8 +106,8 @@ describe('Proxy Module TLS Certificate Loading', () => { afterEach(async () => { try { await proxyModule.stop(); - } catch (err) { - console.error('Error occurred when stopping the proxy: ', err); + } catch (error: unknown) { + handleAndLogError(error, 'Error occurred when stopping the proxy'); } vi.restoreAllMocks(); }); diff --git a/test/services/routes/auth.test.ts b/test/services/routes/auth.test.ts index 2307e09c3..b7c0695d2 100644 --- a/test/services/routes/auth.test.ts +++ b/test/services/routes/auth.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; import request from 'supertest'; -import express, { Express } from 'express'; +import express, { Express, Request, Response } from 'express'; import authRoutes from '../../../src/service/routes/auth'; import * as db from '../../../src/db'; @@ -200,9 +200,12 @@ describe('Auth API', () => { const sendSpy = vi.fn(); const res = { send: sendSpy, - } as any; + }; - await authRoutes.loginSuccessHandler()({ user } as any, res); + await authRoutes.loginSuccessHandler()( + { user } as unknown as Request, + res as unknown as Response, + ); expect(sendSpy).toHaveBeenCalledOnce(); expect(sendSpy).toHaveBeenCalledWith({ diff --git a/test/services/routes/users.test.ts b/test/services/routes/users.test.ts index 2dc401ad9..3df5a3f98 100644 --- a/test/services/routes/users.test.ts +++ b/test/services/routes/users.test.ts @@ -18,14 +18,19 @@ describe('Users API', () => { password: 'secret-hashed-password', email: 'alice@example.com', displayName: 'Alice Walker', + gitAccount: '', + admin: false, }, - ] as any); + ]); vi.spyOn(db, 'findUser').mockResolvedValue({ username: 'bob', password: 'hidden', email: 'bob@example.com', - } as any); + displayName: '', + gitAccount: '', + admin: false, + }); }); afterEach(() => { diff --git a/test/testActiveDirectoryAuth.test.ts b/test/testActiveDirectoryAuth.test.ts index b48d4c34a..f27bf668e 100644 --- a/test/testActiveDirectoryAuth.test.ts +++ b/test/testActiveDirectoryAuth.test.ts @@ -1,4 +1,6 @@ import { describe, it, beforeEach, expect, vi, type Mock, afterEach } from 'vitest'; +import { ADProfile } from '../src/service/passport/types.js'; +import ActiveDirectory from 'activedirectory2'; let ldapStub: { isUserInAdGroup: Mock }; let dbStub: { updateUser: Mock }; @@ -8,10 +10,10 @@ let passportStub: { deserializeUser: Mock; }; let strategyCallback: ( - req: any, - profile: any, - ad: any, - done: (err: any, user: any) => void, + req: Request, + profile: ADProfile, + ad: ActiveDirectory, + done: (err: unknown, user: unknown) => void, ) => void; const newConfig = JSON.stringify({ @@ -32,6 +34,9 @@ const newConfig = JSON.stringify({ }); describe('ActiveDirectory auth method', () => { + const mockReq = {} as Request; + const mockAd = {} as ActiveDirectory; + beforeEach(async () => { ldapStub = { isUserInAdGroup: vi.fn(), @@ -62,7 +67,7 @@ describe('ActiveDirectory auth method', () => { vi.doMock('../src/db', () => dbStub); vi.doMock('passport-activedirectory', () => ({ - default: function (options: any, callback: (err: any, user: any) => void) { + default: function (_: unknown, callback: (err: unknown, user: unknown) => void) { strategyCallback = callback; return { name: 'ActiveDirectory', @@ -87,7 +92,6 @@ describe('ActiveDirectory auth method', () => { }); it('should authenticate a valid user and mark them as admin', async () => { - const mockReq = {}; const mockProfile = { _json: { sAMAccountName: 'test-user', @@ -98,13 +102,13 @@ describe('ActiveDirectory auth method', () => { displayName: 'Test User', }; - (ldapStub.isUserInAdGroup as Mock) + ldapStub.isUserInAdGroup .mockResolvedValueOnce(true) // adminGroup check .mockResolvedValueOnce(true); // userGroup check const done = vi.fn(); - await strategyCallback(mockReq, mockProfile, {}, done); + await strategyCallback(mockReq, mockProfile, mockAd, done); expect(done).toHaveBeenCalledOnce(); const [err, user] = done.mock.calls[0]; @@ -121,7 +125,6 @@ describe('ActiveDirectory auth method', () => { }); it('should fail if user is not in user group', async () => { - const mockReq = {}; const mockProfile = { _json: { sAMAccountName: 'bad-user', @@ -132,11 +135,11 @@ describe('ActiveDirectory auth method', () => { displayName: 'Bad User', }; - (ldapStub.isUserInAdGroup as Mock).mockResolvedValueOnce(false); + ldapStub.isUserInAdGroup.mockResolvedValueOnce(false); const done = vi.fn(); - await strategyCallback(mockReq, mockProfile, {}, done); + await strategyCallback(mockReq, mockProfile, mockAd, done); expect(done).toHaveBeenCalledOnce(); const [err, user] = done.mock.calls[0]; @@ -147,7 +150,6 @@ describe('ActiveDirectory auth method', () => { }); it('should handle LDAP errors gracefully', async () => { - const mockReq = {}; const mockProfile = { _json: { sAMAccountName: 'error-user', @@ -158,11 +160,11 @@ describe('ActiveDirectory auth method', () => { displayName: 'Error User', }; - (ldapStub.isUserInAdGroup as Mock).mockRejectedValueOnce(new Error('LDAP error')); + ldapStub.isUserInAdGroup.mockRejectedValueOnce(new Error('LDAP error')); const done = vi.fn(); - await strategyCallback(mockReq, mockProfile, {}, done); + await strategyCallback(mockReq, mockProfile, mockAd, done); expect(done).toHaveBeenCalledOnce(); const [err, user] = done.mock.calls[0]; diff --git a/test/testCheckUserPushPermission.test.ts b/test/testCheckUserPushPermission.test.ts index 435e7c4d8..6bd04c647 100644 --- a/test/testCheckUserPushPermission.test.ts +++ b/test/testCheckUserPushPermission.test.ts @@ -1,4 +1,5 @@ import { describe, it, beforeAll, afterAll, expect } from 'vitest'; +import { Request } from 'express'; import * as processor from '../src/proxy/processors/push-action/checkUserPushPermission'; import { Action } from '../src/proxy/actions/Action'; import * as db from '../src/db'; @@ -13,6 +14,7 @@ const TEST_EMAIL_2 = 'push-perms-test-2@test.com'; const TEST_EMAIL_3 = 'push-perms-test-3@test.com'; describe('CheckUserPushPermissions...', () => { + const req = {} as Request; let testRepo: Required | null = null; beforeAll(async () => { @@ -28,7 +30,7 @@ describe('CheckUserPushPermissions...', () => { }); afterAll(async () => { - await db.deleteRepo(testRepo!._id); + if (testRepo) await db.deleteRepo(testRepo._id); await db.deleteUser(TEST_USERNAME_1); await db.deleteUser(TEST_USERNAME_2); }); @@ -36,14 +38,14 @@ describe('CheckUserPushPermissions...', () => { it('A committer that is approved should be allowed to push...', async () => { const action = new Action('1', 'type', 'method', 1, TEST_URL); action.userEmail = TEST_EMAIL_1; - const { error } = await processor.exec(null as any, action); + const { error } = await processor.exec(req, action); expect(error).toBe(false); }); it('A committer that is NOT approved should NOT be allowed to push...', async () => { const action = new Action('1', 'type', 'method', 1, TEST_URL); action.userEmail = TEST_EMAIL_2; - const { error, errorMessage } = await processor.exec(null as any, action); + const { error, errorMessage } = await processor.exec(req, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); }); @@ -51,7 +53,7 @@ describe('CheckUserPushPermissions...', () => { it('An unknown committer should NOT be allowed to push...', async () => { const action = new Action('1', 'type', 'method', 1, TEST_URL); action.userEmail = TEST_EMAIL_3; - const { error, errorMessage } = await processor.exec(null as any, action); + const { error, errorMessage } = await processor.exec(req, action); expect(error).toBe(true); expect(errorMessage).toContain('Your push has been blocked'); }); diff --git a/test/testConfig.test.ts b/test/testConfig.test.ts index 972f5e8cb..d7ecdf0e6 100644 --- a/test/testConfig.test.ts +++ b/test/testConfig.test.ts @@ -97,7 +97,7 @@ describe('user configuration', () => { const config = await import('../src/config'); config.invalidateCache(); const authMethods = config.getAuthMethods(); - const oidcAuth = authMethods.find((method: any) => method.type === 'openidconnect'); + const oidcAuth = authMethods.find((method) => method.type === 'openidconnect'); expect(oidcAuth).toBeDefined(); expect(oidcAuth?.enabled).toBe(true); diff --git a/test/testDb.test.ts b/test/testDb.test.ts index 48e926bc7..4effc1267 100644 --- a/test/testDb.test.ts +++ b/test/testDb.test.ts @@ -4,7 +4,7 @@ import { Repo, User } from '../src/db/types'; import { Action } from '../src/proxy/actions/Action'; import { Step } from '../src/proxy/actions/Step'; import { AuthorisedRepo } from '../src/config/generated/config'; -import { EMPTY_COMMIT_HASH } from '../src/proxy/processors/constants'; +import { SAMPLE_REPO } from '../src/proxy/processors/constants'; const TEST_REPO = { project: 'finos', @@ -27,33 +27,15 @@ const TEST_USER = { admin: true, }; -const TEST_PUSH = { - steps: [], - error: false, - blocked: true, - allowPush: false, - authorised: false, - canceled: true, - rejected: false, - autoApproved: false, - autoRejected: false, - commitData: [], - id: `${EMPTY_COMMIT_HASH}__1744380874110`, - type: 'push', - method: 'get', - timestamp: 1744380903338, - project: 'finos', - repoName: 'db-test-repo.git', - url: TEST_REPO.url, - repo: 'finos/db-test-repo.git', - user: 'db-test-user', - userEmail: 'db-test@test.com', - lastStep: null, - blockedMessage: - '\n\n\nGitProxy has received your push:\n\nhttp://localhost:8080/requests/${EMPTY_COMMIT_HASH}__1744380874110\n\n\n', - _id: 'GIMEz8tU2KScZiTz', - attestation: null, -}; +const TEST_PUSH = new Action( + '0000000000000000000000000000000000000000__1744380874110', + 'push', + 'get', + 1744380903338, + TEST_REPO.url, +); +TEST_PUSH.user = TEST_USER.username; +TEST_PUSH.userEmail = TEST_USER.email; const TEST_REPO_DOT_GIT = { project: 'finos', @@ -63,12 +45,15 @@ const TEST_REPO_DOT_GIT = { // the same as TEST_PUSH but with .git somewhere valid within the name // to ensure a global replace isn't done when trimming, just to the end -const TEST_PUSH_DOT_GIT = { - ...TEST_PUSH, - repoName: 'db.git-test-repo.git', - url: 'https://github.com/finos/db.git-test-repo.git', - repo: 'finos/db.git-test-repo.git', -}; +const TEST_PUSH_DOT_GIT = new Action( + '0000000000000000000000000000000000000000__1744380874110', + 'push', + 'get', + 1744380903338, + 'https://github.com/finos/db.git-test-repo.git', +); +TEST_PUSH_DOT_GIT.project = 'finos'; +TEST_PUSH_DOT_GIT.repoName = 'db.git-test-repo.git'; /** * Clean up response data from the DB by removing an extraneous properties, @@ -84,7 +69,6 @@ const cleanResponseData = (example: T, responses: T[] | T): T[ return responses.map((response) => { const cleanResponse: Partial = {}; columns.forEach((col) => { - // @ts-expect-error dynamic indexing cleanResponse[col] = response[col]; }); return cleanResponse as T; @@ -92,7 +76,6 @@ const cleanResponseData = (example: T, responses: T[] | T): T[ } else if (typeof responses === 'object') { const cleanResponse: Partial = {}; columns.forEach((col) => { - // @ts-expect-error dynamic indexing cleanResponse[col] = responses[col]; }); return cleanResponse as T; @@ -244,7 +227,7 @@ describe('Database clients', () => { await db.createRepo(TEST_REPO); const repos = await db.getRepos(); - const cleanRepos = cleanResponseData(TEST_REPO, repos) as (typeof TEST_REPO)[]; + const cleanRepos = cleanResponseData(TEST_REPO, repos); expect(cleanRepos).toContainEqual(TEST_REPO); }); @@ -254,12 +237,10 @@ describe('Database clients', () => { // uppercase the filter value to confirm db client is lowercasing inputs const repos = await db.getRepos({ name: TEST_REPO.name.toUpperCase() }); const cleanRepos = cleanResponseData(TEST_REPO, repos); - // @ts-expect-error dynamic indexing expect(cleanRepos[0]).toEqual(TEST_REPO); const repos2 = await db.getRepos({ url: TEST_REPO.url }); const cleanRepos2 = cleanResponseData(TEST_REPO, repos2); - // @ts-expect-error dynamic indexing expect(cleanRepos2[0]).toEqual(TEST_REPO); const repos3 = await db.getRepos(); @@ -334,7 +315,7 @@ describe('Database clients', () => { // null username await expect( db.createUser( - null as any, + null as unknown as string, TEST_USER.password, TEST_USER.email, TEST_USER.gitAccount, @@ -352,7 +333,7 @@ describe('Database clients', () => { db.createUser( TEST_USER.username, TEST_USER.password, - null as any, + null as unknown as string, TEST_USER.gitAccount, TEST_USER.admin, ), @@ -431,12 +412,10 @@ describe('Database clients', () => { const users = await db.getUsers({ username: TEST_USER.username.toUpperCase() }); const { password: _, ...TEST_USER_CLEAN } = TEST_USER; const cleanUsers = cleanResponseData(TEST_USER_CLEAN, users); - // @ts-expect-error dynamic indexing expect(cleanUsers[0]).toEqual(TEST_USER_CLEAN); const users2 = await db.getUsers({ email: TEST_USER.email.toUpperCase() }); const cleanUsers2 = cleanResponseData(TEST_USER_CLEAN, users2); - // @ts-expect-error dynamic indexing expect(cleanUsers2[0]).toEqual(TEST_USER_CLEAN); }); @@ -587,43 +566,43 @@ describe('Database clients', () => { }); it('should be able to create a push', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); const pushes = await db.getPushes({}); - const cleanPushes = cleanResponseData(TEST_PUSH, pushes as any); + const cleanPushes = cleanResponseData(TEST_PUSH, pushes); expect(cleanPushes).toContainEqual(TEST_PUSH); }, 20000); it('should be able to delete a push', async () => { // Create push - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); await db.deletePush(TEST_PUSH.id); const pushes = await db.getPushes({}); - const cleanPushes = cleanResponseData(TEST_PUSH, pushes as any); + const cleanPushes = cleanResponseData(TEST_PUSH, pushes); expect(cleanPushes).not.toContainEqual(TEST_PUSH); }); it('should be able to authorise a push', async () => { - await db.writeAudit(TEST_PUSH as any); - const msg = await db.authorise(TEST_PUSH.id, null); + await db.writeAudit(TEST_PUSH); + const msg = await db.authorise(TEST_PUSH.id, undefined); expect(msg).toHaveProperty('message'); }); it('should throw an error when authorising a non-existent a push', async () => { - await expect(db.authorise(TEST_PUSH.id, null)).rejects.toThrow(); + await expect(db.authorise(TEST_PUSH.id, undefined)).rejects.toThrow(); }); it('should be able to reject a push', async () => { - await db.writeAudit(TEST_PUSH as any); - const msg = await db.reject(TEST_PUSH.id, null); + await db.writeAudit(TEST_PUSH); + const msg = await db.reject(TEST_PUSH.id, undefined); expect(msg).toHaveProperty('message'); }); it('should throw an error when rejecting a non-existent a push', async () => { - await expect(db.reject(TEST_PUSH.id, null)).rejects.toThrow(); + await expect(db.reject(TEST_PUSH.id, undefined)).rejects.toThrow(); }); it('should be able to cancel a push', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); const msg = await db.cancel(TEST_PUSH.id); expect(msg).toHaveProperty('message'); }); @@ -644,7 +623,7 @@ describe('Database clients', () => { expect(allowed).toBe(false); // create the push - user should already exist and not authorised to push - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); allowed = await db.canUserCancelPush(TEST_PUSH.id, TEST_USER.username); expect(allowed).toBe(false); @@ -667,7 +646,7 @@ describe('Database clients', () => { expect(allowed).toBe(false); // push does not exist yet, should return false - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); allowed = await db.canUserApproveRejectPush(TEST_PUSH.id, TEST_USER.username); expect(allowed).toBe(false); @@ -696,7 +675,7 @@ describe('Database clients', () => { expect(allowed).toBe(false); // create the push - user should already exist and not authorised to push - await db.writeAudit(TEST_PUSH_DOT_GIT as any); + await db.writeAudit(TEST_PUSH_DOT_GIT); allowed = await db.canUserApproveRejectPush(TEST_PUSH_DOT_GIT.id, TEST_USER.username); expect(allowed).toBe(false); diff --git a/test/testJwtAuthHandler.test.ts b/test/testJwtAuthHandler.test.ts index e9dd38c6a..1b954f91d 100644 --- a/test/testJwtAuthHandler.test.ts +++ b/test/testJwtAuthHandler.test.ts @@ -1,10 +1,12 @@ -import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest'; import axios from 'axios'; -import jwt from 'jsonwebtoken'; import crypto from 'crypto'; +import { NextFunction } from 'express'; +import jwt, { JwtPayload } from 'jsonwebtoken'; +import { describe, it, expect, vi, beforeEach, afterEach, MockInstance } from 'vitest'; import { assignRoles, getJwks, validateJwt } from '../src/service/passport/jwtUtils'; import { jwtAuthHandler } from '../src/service/passport/jwtAuthHandler'; +import { JwtConfig, RoleMapping } from '../src/config/generated/config'; function generateRsaKeyPair() { return crypto.generateKeyPairSync('rsa', { @@ -42,9 +44,9 @@ describe('JWT', () => { }); describe('validateJwt', () => { - let decodeStub: any; - let verifyStub: any; - let getJwksStub: any; + let decodeStub: MockInstance; + let verifyStub: MockInstance; + let getJwksStub: ReturnType; beforeEach(() => { const jwksResponse = { keys: [{ kid: 'test-key', kty: 'RSA', n: 'abc', e: 'AQAB' }] }; @@ -166,7 +168,7 @@ describe('JWT', () => { const user = { username: 'no-role-user', admin: undefined }; const payload = { admin: 'admin' }; - assignRoles(null as any, payload, user); + assignRoles({} as RoleMapping, payload, user); expect(user.admin).toBeUndefined(); }); }); @@ -174,9 +176,9 @@ describe('JWT', () => { describe('jwtAuthHandler', () => { let req: any; let res: any; - let next: any; - let jwtConfig: any; - let validVerifyResponse: any; + let next: NextFunction; + let jwtConfig: JwtConfig; + let validVerifyResponse: JwtPayload; beforeEach(() => { req = { header: vi.fn(), isAuthenticated: vi.fn(), user: {} }; @@ -245,7 +247,7 @@ describe('JWT', () => { await jwtAuthHandler(jwtConfig)(req, res, next); expect(res.status).toHaveBeenCalledWith(401); - expect(res.send).toHaveBeenCalledWith(expect.stringMatching(/JWT validation failed:/)); + expect(res.send).toHaveBeenCalledWith(expect.stringMatching(/Invalid JWT:/)); }); }); }); diff --git a/test/testLogin.test.ts b/test/testLogin.test.ts index 2dcd8f783..08986f894 100644 --- a/test/testLogin.test.ts +++ b/test/testLogin.test.ts @@ -244,7 +244,7 @@ describe('login', () => { }); expect(failCreateRes.status).toBe(500); - expect(failCreateRes.body.message).toBe('user newuser already exists'); + expect(failCreateRes.body.message).toBe('Failed to create user: user newuser already exists'); }); }); diff --git a/test/testOidc.test.ts b/test/testOidc.test.ts index 5561b7be8..038b059e7 100644 --- a/test/testOidc.test.ts +++ b/test/testOidc.test.ts @@ -1,3 +1,4 @@ +import { PassportStatic } from 'passport'; import { describe, it, beforeEach, afterEach, expect, vi, type Mock } from 'vitest'; import { @@ -9,7 +10,7 @@ import { describe('OIDC auth method', () => { let dbStub: any; let passportStub: any; - let configure: any; + let configure: (passport: PassportStatic) => Promise; let discoveryStub: Mock; let fetchUserInfoStub: Mock; @@ -44,7 +45,7 @@ describe('OIDC auth method', () => { discoveryStub = vi.fn().mockResolvedValue({ some: 'config' }); fetchUserInfoStub = vi.fn(); - const strategyCtorStub = function (_options: any, verifyFn: any) { + const strategyCtorStub = function (_options: unknown, _verifyFn: unknown) { return { name: 'openidconnect', currentUrl: vi.fn().mockReturnValue({}), @@ -54,18 +55,14 @@ describe('OIDC auth method', () => { // First mock the dependencies vi.resetModules(); vi.doMock('../src/config', async () => { - const actual = await vi.importActual('../src/config'); + const actual = await vi.importActual('../src/config'); return { ...actual, - default: { - ...actual.default, - initUserConfig: vi.fn(), - }, initUserConfig: vi.fn(), }; }); vi.doMock('fs', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, existsSync: vi.fn().mockReturnValue(true), @@ -74,7 +71,7 @@ describe('OIDC auth method', () => { }); vi.doMock('../../db', () => dbStub); vi.doMock('../../config', async () => { - const actual = await vi.importActual('../src/config'); + const actual = await vi.importActual('../src/config'); return actual; }); vi.doMock('openid-client', () => ({ @@ -130,19 +127,19 @@ describe('OIDC auth method', () => { describe('safelyExtractEmail', () => { it('should extract email from profile', () => { - const profile = { email: 'test@test.com' }; + const profile = { sub: 'sub-test', email: 'test@test.com' }; const email = safelyExtractEmail(profile); expect(email).toBe('test@test.com'); }); it('should extract email from profile with emails array', () => { - const profile = { emails: [{ value: 'test@test.com' }] }; + const profile = { sub: 'sub-test', emails: [{ value: 'test@test.com' }] }; const email = safelyExtractEmail(profile); expect(email).toBe('test@test.com'); }); it('should return null if no email in profile', () => { - const profile = { name: 'test' }; + const profile = { sub: 'sub-test', name: 'test' }; const email = safelyExtractEmail(profile); expect(email).toBeNull(); }); diff --git a/test/testParseAction.test.ts b/test/testParseAction.test.ts index a1e424430..2631d3b9b 100644 --- a/test/testParseAction.test.ts +++ b/test/testParseAction.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import * as preprocessor from '../src/proxy/processors/pre-processor/parseAction'; import * as db from '../src/db'; +import { Request } from 'express'; -let testRepo: any = null; +let testRepo: db.Repo | null = null; const TEST_REPO = { url: 'https://github.com/finos/git-proxy.git', @@ -31,7 +32,7 @@ describe('Pre-processor: parseAction', () => { originalUrl: '/github.com/finos/git-proxy.git/git-upload-pack', method: 'GET', headers: { 'content-type': 'application/x-git-upload-pack-request' }, - }; + } as Request; const action = await preprocessor.exec(req); expect(action.timestamp).toBeGreaterThan(0); @@ -45,7 +46,7 @@ describe('Pre-processor: parseAction', () => { originalUrl: '/finos/git-proxy.git/git-upload-pack', method: 'GET', headers: { 'content-type': 'application/x-git-upload-pack-request' }, - }; + } as Request; const action = await preprocessor.exec(req); expect(action.timestamp).toBeGreaterThan(0); @@ -59,7 +60,7 @@ describe('Pre-processor: parseAction', () => { originalUrl: '/github.com/finos/git-proxy.git/git-receive-pack', method: 'POST', headers: { 'content-type': 'application/x-git-receive-pack-request' }, - }; + } as Request; const action = await preprocessor.exec(req); expect(action.timestamp).toBeGreaterThan(0); @@ -73,7 +74,7 @@ describe('Pre-processor: parseAction', () => { originalUrl: '/finos/git-proxy.git/git-receive-pack', method: 'POST', headers: { 'content-type': 'application/x-git-receive-pack-request' }, - }; + } as Request; const action = await preprocessor.exec(req); expect(action.timestamp).toBeGreaterThan(0); diff --git a/test/testParsePush.test.ts b/test/testParsePush.test.ts index 25740048d..c5d55b012 100644 --- a/test/testParsePush.test.ts +++ b/test/testParsePush.test.ts @@ -11,8 +11,11 @@ import { getPackMeta, parsePacketLines, } from '../src/proxy/processors/push-action/parsePush'; - import { EMPTY_COMMIT_HASH, FLUSH_PACKET, PACK_SIGNATURE } from '../src/proxy/processors/constants'; +import { CommitContent } from '../src/proxy/processors/types'; +import { Action } from '../src/proxy/actions/Action'; +import { Request } from 'express'; +import { Step } from '../src/proxy/actions/Step'; /** * Creates a simplified sample PACK buffer for testing. @@ -137,6 +140,16 @@ const TEST_MULTI_OBJ_COMMIT_CONTENT = [ }, ]; +const BASE_COMMIT_CONTENT: CommitContent = { + item: 0, + type: 1, + typeName: 'commit', + size: 0, + baseSha: null, + baseOffset: null, + content: 'tree 123\nparent 456\nauthor A 123 +0000\ncommitter C 456 +0000\n\nmessage', +}; + /** Creates a multi-object sample PACK buffer for testing PACK file decompression. * Creates a relatively large example as decompression steps involve variable length * headers depending on content and size. @@ -150,7 +163,7 @@ function createMultiObjectSamplePackBuffer() { header.writeUInt32BE(2, 4); // Version header.writeUInt32BE(numEntries, 8); // Number of entries - const packContents = []; + const packContents: Buffer[] = []; for (let i = 0; i < numEntries; i++) { const commitContent = TEST_MULTI_OBJ_COMMIT_CONTENT[i]; const originalContent = Buffer.from(commitContent.content, 'utf8'); @@ -215,8 +228,12 @@ const encodeOfsDeltaOffset = (distance: number) => { * @param {Buffer} [options.baseSha] - SHA-1 hash for ref_delta (20 bytes). * @return {Buffer} - Encoded header buffer. */ -function encodeGitObjectHeader(type: number, size: number, options: any = {}) { - const headerBytes = []; +function encodeGitObjectHeader( + type: number, + size: number, + options: { baseOffset?: number; baseSha?: Buffer } = {}, +) { + const headerBytes: number[] = []; // First byte: type (3 bits), size (lower 4 bits), continuation bit const firstSizeBits = size & 0x0f; @@ -291,7 +308,7 @@ function createEmptyPackBuffer() { describe('parsePackFile', () => { let action: any; - let req: any; + let req: Request; beforeEach(() => { // Mock Action and Step and spy on methods @@ -299,13 +316,13 @@ describe('parsePackFile', () => { branch: null, commitFrom: null, commitTo: null, - commitData: [] as any[], + commitData: [], user: null, - steps: [] as any[], - addStep: vi.fn(function (this: any, step: any) { + steps: [], + addStep: vi.fn(function (this: Action, step: Step) { this.steps.push(step); }), - setCommit: vi.fn(function (this: any, from: string, to: string) { + setCommit: vi.fn(function (this: Action, from: string, to: string) { this.commitFrom = from; this.commitTo = to; }), @@ -313,7 +330,7 @@ describe('parsePackFile', () => { req = { body: null, - }; + } as Request; }); afterEach(() => { @@ -456,7 +473,7 @@ describe('parsePackFile', () => { expect(result).toBe(action); // Check step and action properties - const step = action.steps.find((s: any) => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).toBeDefined(); expect(step.error).toBe(false); expect(step.errorMessage).toBeNull(); @@ -509,7 +526,7 @@ describe('parsePackFile', () => { expect(result).toBe(action); // Check step and action properties - const step = action.steps.find((s: any) => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).toBeDefined(); expect(step.error).toBe(false); expect(step.errorMessage).toBeNull(); @@ -552,7 +569,7 @@ describe('parsePackFile', () => { expect(result).toBe(action); // Check step and action properties - const step = action.steps.find((s: any) => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).toBeDefined(); expect(step.error).toBe(false); expect(step.errorMessage).toBeNull(); @@ -607,7 +624,7 @@ describe('parsePackFile', () => { const result = await exec(req, action); expect(result).toBe(action); - const step = action.steps.find((s: any) => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).toBeDefined(); expect(step.error).toBe(false); @@ -646,7 +663,7 @@ describe('parsePackFile', () => { expect(result).toBe(action); // Check step and action properties - const step = action.steps.find((s: any) => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).toBeDefined(); expect(step.error).toBe(false); @@ -676,7 +693,7 @@ describe('parsePackFile', () => { const result = await exec(req, action); expect(result).toBe(action); - const step = action.steps.find((s: any) => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).toBeDefined(); expect(step.error).toBe(true); expect(step.errorMessage).toContain('Invalid commit data: Missing tree'); @@ -803,7 +820,7 @@ describe('parsePackFile', () => { const result = await exec(req, action); expect(result).toBe(action); - const step = action.steps.find((s: any) => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).toBeTruthy(); expect(step.error).toBe(false); @@ -841,17 +858,17 @@ describe('parsePackFile', () => { }); describe('getCommitData', () => { it('should return empty array if no type 1 contents', () => { - const contents = [ - { type: 2, content: 'blob' }, - { type: 3, content: 'tree' }, + const contents: CommitContent[] = [ + { ...BASE_COMMIT_CONTENT, type: 2, content: 'blob' }, + { ...BASE_COMMIT_CONTENT, type: 3, content: 'tree' }, ]; - expect(getCommitData(contents as any)).toEqual([]); + expect(getCommitData(contents)).toEqual([]); }); it('should parse a single valid commit object', () => { const commitContent = `tree 123\nparent 456\nauthor Au Thor 111 +0000\ncommitter Com Itter 222 +0100\n\nCommit message here`; - const contents = [{ type: 1, content: commitContent }]; - const result = getCommitData(contents as any); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + const result = getCommitData(contents); expect(result).toHaveLength(1); expect(result[0]).toEqual({ @@ -869,13 +886,13 @@ describe('parsePackFile', () => { it('should parse multiple valid commit objects', () => { const commit1 = `tree 111\nparent 000\nauthor A1 1678880001 +0000\ncommitter C1 1678880002 +0000\n\nMsg1`; const commit2 = `tree 222\nparent 111\nauthor A2 1678880003 +0100\ncommitter C2 1678880004 +0100\n\nMsg2`; - const contents = [ - { type: 1, content: commit1 }, - { type: 3, content: 'tree data' }, // non-commit types must be ignored - { type: 1, content: commit2 }, + const contents: CommitContent[] = [ + { ...BASE_COMMIT_CONTENT, content: commit1 }, + { ...BASE_COMMIT_CONTENT, type: 3, content: 'tree data' }, // non-commit types must be ignored + { ...BASE_COMMIT_CONTENT, content: commit2 }, ]; - const result = getCommitData(contents as any); + const result = getCommitData(contents); expect(result).toHaveLength(2); // Check first commit data @@ -897,49 +914,47 @@ describe('parsePackFile', () => { it('should default parent to zero hash if not present', () => { const commitContent = `tree 123\nauthor Au Thor 111 +0000\ncommitter Com Itter 222 +0100\n\nCommit message here`; - const contents = [{ type: 1, content: commitContent }]; - const result = getCommitData(contents as any); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + const result = getCommitData(contents); expect(result[0].parent).toBe('0'.repeat(40)); }); it('should handle commit messages with multiple lines', () => { const commitContent = `tree 123\nparent 456\nauthor A 111 +0000\ncommitter C 222 +0100\n\nLine one\nLine two\n\nLine four`; - const contents = [{ type: 1, content: commitContent }]; - const result = getCommitData(contents as any); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + const result = getCommitData(contents); expect(result[0].message).toBe('Line one\nLine two\n\nLine four'); }); it('should handle commits without a message body', () => { const commitContent = `tree 123\nparent 456\nauthor A 111 +0000\ncommitter C 222 +0100\n`; - const contents = [{ type: 1, content: commitContent }]; - const result = getCommitData(contents as any); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + const result = getCommitData(contents); expect(result[0].message).toBe(''); }); it('should throw error for invalid commit data (missing tree)', () => { const commitContent = `parent 456\nauthor A 1234567890 +0000\ncommitter C 1234567890 +0000\n\nMsg`; - const contents = [{ type: 1, content: commitContent }]; - expect(() => getCommitData(contents as any)).toThrow('Invalid commit data: Missing tree'); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + expect(() => getCommitData(contents)).toThrow('Invalid commit data: Missing tree'); }); it('should throw error for invalid commit data (missing author)', () => { const commitContent = `tree 123\nparent 456\ncommitter C 1234567890 +0000\n\nMsg`; - const contents = [{ type: 1, content: commitContent }]; - expect(() => getCommitData(contents as any)).toThrow('Invalid commit data: Missing author'); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + expect(() => getCommitData(contents)).toThrow('Invalid commit data: Missing author'); }); it('should throw error for invalid commit data (missing committer)', () => { const commitContent = `tree 123\nparent 456\nauthor A 1234567890 +0000\n\nMsg`; - const contents = [{ type: 1, content: commitContent }]; - expect(() => getCommitData(contents as any)).toThrow( - 'Invalid commit data: Missing committer', - ); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + expect(() => getCommitData(contents)).toThrow('Invalid commit data: Missing committer'); }); it('should throw error for invalid author line (missing timezone offset)', () => { const commitContent = `tree 123\nparent 456\nauthor A 1234567890\ncommitter C 1234567890 +0000\n\nMsg`; - const contents = [{ type: 1, content: commitContent }]; - expect(() => getCommitData(contents as any)).toThrow('Failed to parse person line'); + const contents: CommitContent[] = [{ ...BASE_COMMIT_CONTENT, content: commitContent }]; + expect(() => getCommitData(contents)).toThrow('Failed to parse person line'); }); it('should correctly parse a commit with a GPG signature header', () => { @@ -967,15 +982,15 @@ describe('parsePackFile', () => { 'It can span multiple lines.\n\n' + 'And include blank lines internally.'; - const contents = [ - { type: 1, content: gpgSignedCommit }, + const contents: CommitContent[] = [ + { ...BASE_COMMIT_CONTENT, content: gpgSignedCommit }, { - type: 1, + ...BASE_COMMIT_CONTENT, content: `tree 111\nparent 000\nauthor A1 1744814600 +0200\ncommitter C1 1744814610 +0200\n\nMsg1`, }, ]; - const result = getCommitData(contents as any); + const result = getCommitData(contents); expect(result).toHaveLength(2); // Check the GPG signed commit data diff --git a/test/testProxy.test.ts b/test/testProxy.test.ts index e8c48a57e..6e6f42fb4 100644 --- a/test/testProxy.test.ts +++ b/test/testProxy.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, beforeEach, afterEach, vi, afterAll } from 'vitest'; vi.mock('http', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, createServer: vi.fn(() => ({ @@ -15,7 +15,7 @@ vi.mock('http', async (importOriginal) => { }); vi.mock('https', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, createServer: vi.fn(() => ({ @@ -63,7 +63,7 @@ vi.mock('../src/config/env', () => ({ })); vi.mock('fs', async (importOriginal) => { - const actual: any = await importOriginal(); + const actual = await importOriginal(); return { ...actual, readFileSync: vi.fn(), @@ -213,7 +213,7 @@ describe('Proxy', () => { const app = proxy.getExpressApp(); expect(app).not.toBeNull(); expect(app).toBeTypeOf('function'); - expect((app as any).use).toBeTypeOf('function'); + expect(app?.use).toBeTypeOf('function'); await proxy.stop(); }); diff --git a/test/testProxyRoute.test.ts b/test/testProxyRoute.test.ts index 29d50a20f..69ec67caf 100644 --- a/test/testProxyRoute.test.ts +++ b/test/testProxyRoute.test.ts @@ -373,7 +373,7 @@ describe('proxyFilter', () => { await proxyFilter?.(mockReq as Request, mockRes as Response); - expect((mockReq as any).body).toEqual(Buffer.from('test data')); + expect(mockReq.body).toEqual(Buffer.from('test data')); expect((mockReq as any).bodyRaw).toBeUndefined(); }); }); diff --git a/test/testPush.test.ts b/test/testPush.test.ts index 731ed69e5..ea8c9744f 100644 --- a/test/testPush.test.ts +++ b/test/testPush.test.ts @@ -1,9 +1,10 @@ -import request from 'supertest'; +import request, { Response } from 'supertest'; import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest'; import * as db from '../src/db'; import { Service } from '../src/service'; import { Proxy } from '../src/proxy'; import { Express } from 'express'; +import { Action } from '../src/proxy/actions/Action'; import { EMPTY_COMMIT_HASH } from '../src/proxy/processors/constants'; // dummy repo @@ -22,41 +23,28 @@ const TEST_PASSWORD_2 = 'test5678'; const TEST_USERNAME_3 = 'push-test-3'; const TEST_EMAIL_3 = 'push-test-3@test.com'; -const TEST_PUSH = { - steps: [], - error: false, - blocked: false, - allowPush: false, - authorised: false, - canceled: false, - rejected: false, - autoApproved: false, - autoRejected: false, - commitData: [], - id: `${EMPTY_COMMIT_HASH}__1744380874110`, - type: 'push', - method: 'get', - timestamp: 1744380903338, - project: TEST_ORG, - repoName: TEST_REPO + '.git', - url: TEST_URL, - repo: TEST_ORG + '/' + TEST_REPO + '.git', - user: TEST_USERNAME_2, - userEmail: TEST_EMAIL_2, - lastStep: null, - blockedMessage: - '\n\n\nGitProxy has received your push:\n\nhttp://localhost:8080/requests/${EMPTY_COMMIT_HASH}__1744380874110\n\n\n', - _id: 'GIMEz8tU2KScZiTz', - attestation: null, -}; +const TEST_PUSH: Action = new Action( + '0000000000000000000000000000000000000000__1744380874110', + 'push', + 'get', + 1744380903338, + TEST_URL, +); +TEST_PUSH.project = TEST_ORG; +TEST_PUSH.repoName = TEST_REPO + '.git'; +TEST_PUSH.repo = TEST_ORG + '/' + TEST_REPO + '.git'; +TEST_PUSH.user = TEST_USERNAME_2; +TEST_PUSH.userEmail = TEST_EMAIL_2; +TEST_PUSH.blockedMessage = + '\n\n\nGitProxy has received your push:\n\nhttp://localhost:8080/requests/0000000000000000000000000000000000000000__1744380874110\n\n\n'; describe('Push API', () => { let app: Express; let cookie: string | null = null; - let testRepo: any; + let testRepo: db.Repo; - const setCookie = (res: any) => { - const cookies: string[] = res.headers['set-cookie'] ?? []; + const setCookie = (res: Response) => { + const cookies = res.headers['set-cookie'] ?? []; for (const x of cookies) { if (x.startsWith('connect')) { cookie = x.split(';')[0]; @@ -102,18 +90,18 @@ describe('Push API', () => { // Create a new user for the approver await db.createUser(TEST_USERNAME_1, TEST_PASSWORD_1, TEST_EMAIL_1, TEST_USERNAME_1, false); - await db.addUserCanAuthorise(testRepo._id, TEST_USERNAME_1); + await db.addUserCanAuthorise(testRepo._id!, TEST_USERNAME_1); // create a new user for the committer await db.createUser(TEST_USERNAME_2, TEST_PASSWORD_2, TEST_EMAIL_2, TEST_USERNAME_2, false); - await db.addUserCanPush(testRepo._id, TEST_USERNAME_2); + await db.addUserCanPush(testRepo._id!, TEST_USERNAME_2); // logout of admin account await logout(); }); afterAll(async () => { - await db.deleteRepo(testRepo._id); + await db.deleteRepo(testRepo._id!); await db.deleteUser(TEST_USERNAME_1); await db.deleteUser(TEST_USERNAME_2); @@ -135,7 +123,7 @@ describe('Push API', () => { }); it('should allow an authorizer to approve a push', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); await loginAsApprover(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) @@ -160,8 +148,10 @@ describe('Push API', () => { it('should NOT allow an authorizer to approve if attestation is incomplete', async () => { // make the approver also the committer - const testPush = { ...TEST_PUSH, user: TEST_USERNAME_1, userEmail: TEST_EMAIL_1 }; - await db.writeAudit(testPush as any); + const testPush = Object.assign({}, TEST_PUSH); + testPush.user = TEST_USERNAME_1; + testPush.userEmail = TEST_EMAIL_1; + await db.writeAudit(testPush); await loginAsApprover(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) @@ -187,8 +177,10 @@ describe('Push API', () => { it('should NOT allow an authorizer to approve if committer is unknown', async () => { // make the approver also the committer - const testPush = { ...TEST_PUSH, user: TEST_USERNAME_3, userEmail: TEST_EMAIL_3 }; - await db.writeAudit(testPush as any); + const testPush = Object.assign({}, TEST_PUSH); + testPush.user = TEST_USERNAME_3; + testPush.userEmail = TEST_EMAIL_3; + await db.writeAudit(testPush); await loginAsApprover(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) @@ -217,10 +209,10 @@ describe('Push API', () => { it('should NOT allow an authorizer to approve their own push', async () => { // make the approver also the committer - const testPush = { ...TEST_PUSH }; + const testPush = Object.assign({}, TEST_PUSH); testPush.user = TEST_USERNAME_1; testPush.userEmail = TEST_EMAIL_1; - await db.writeAudit(testPush as any); + await db.writeAudit(testPush); await loginAsApprover(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) @@ -245,7 +237,7 @@ describe('Push API', () => { }); it('should NOT allow a non-authorizer to approve a push', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); await loginAsCommitter(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/authorise`) @@ -270,7 +262,7 @@ describe('Push API', () => { }); it('should allow an authorizer to reject a push', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); await loginAsApprover(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/reject`) @@ -280,10 +272,10 @@ describe('Push API', () => { it('should NOT allow an authorizer to reject their own push', async () => { // make the approver also the committer - const testPush = { ...TEST_PUSH }; + const testPush = Object.assign({}, TEST_PUSH); testPush.user = TEST_USERNAME_1; testPush.userEmail = TEST_EMAIL_1; - await db.writeAudit(testPush as any); + await db.writeAudit(testPush); await loginAsApprover(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/reject`) @@ -293,11 +285,11 @@ describe('Push API', () => { }); it('should NOT allow a non-authorizer to reject a push', async () => { - const pushWithOtherUser = { ...TEST_PUSH }; + const pushWithOtherUser = Object.assign({}, TEST_PUSH); pushWithOtherUser.user = TEST_USERNAME_1; pushWithOtherUser.userEmail = TEST_EMAIL_1; - await db.writeAudit(pushWithOtherUser as any); + await db.writeAudit(pushWithOtherUser); await loginAsCommitter(); const res = await request(app) .post(`/api/v1/push/${pushWithOtherUser.id}/reject`) @@ -309,20 +301,22 @@ describe('Push API', () => { }); it('should fetch all pushes', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); await loginAsApprover(); const res = await request(app).get('/api/v1/push').set('Cookie', `${cookie}`); expect(res.status).toBe(200); expect(Array.isArray(res.body)).toBe(true); - const push = res.body.find((p: any) => p.id === TEST_PUSH.id); + const push = res.body.find((p: Action) => p.id === TEST_PUSH.id); expect(push).toBeDefined(); - expect(push).toEqual(TEST_PUSH); + + // Check that all values in push are in TEST_PUSH, except for _id + expect(push).toMatchObject(TEST_PUSH); expect(push.canceled).toBe(false); }); it('should allow a committer to cancel a push', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); await loginAsCommitter(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/cancel`) @@ -330,14 +324,14 @@ describe('Push API', () => { expect(res.status).toBe(200); const pushes = await request(app).get('/api/v1/push').set('Cookie', `${cookie}`); - const push = pushes.body.find((p: any) => p.id === TEST_PUSH.id); + const push = pushes.body.find((p: Action) => p.id === TEST_PUSH.id); expect(push).toBeDefined(); expect(push.canceled).toBe(true); }); it('should not allow a non-committer to cancel a push (even if admin)', async () => { - await db.writeAudit(TEST_PUSH as any); + await db.writeAudit(TEST_PUSH); await loginAsAdmin(); const res = await request(app) .post(`/api/v1/push/${TEST_PUSH.id}/cancel`) @@ -348,7 +342,7 @@ describe('Push API', () => { ); const pushes = await request(app).get('/api/v1/push').set('Cookie', `${cookie}`); - const push = pushes.body.find((p: any) => p.id === TEST_PUSH.id); + const push = pushes.body.find((p: Action) => p.id === TEST_PUSH.id); expect(push).toBeDefined(); expect(push.canceled).toBe(false); diff --git a/test/testRepoApi.test.ts b/test/testRepoApi.test.ts index 73886c51a..633293aa1 100644 --- a/test/testRepoApi.test.ts +++ b/test/testRepoApi.test.ts @@ -1,3 +1,4 @@ +import { Express } from 'express'; import request from 'supertest'; import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import * as db from '../src/db'; @@ -43,8 +44,8 @@ const fetchRepoOrThrow = async (url: string) => { }; describe('add new repo', () => { - let app: any; - let proxy: any; + let app: Express; + let proxy: Proxy; let cookie: string; const repoIds: string[] = [];