From d323cac9b0a9c7ce3e86b3b20b877f54e9702204 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 5 Dec 2025 11:41:04 +0900 Subject: [PATCH 01/31] refactor: add missing types for attestation, remove gitAccount from Attestation definition --- src/db/file/pushes.ts | 11 +++++++++-- src/db/index.ts | 5 +++-- src/db/mongo/pushes.ts | 6 +++++- src/proxy/actions/autoActions.ts | 14 ++++++++++++-- src/proxy/processors/types.ts | 3 ++- src/service/routes/push.ts | 4 ++-- src/ui/views/PushDetails/PushDetails.tsx | 11 +++-------- .../PushDetails/components/AttestationView.tsx | 10 +++++----- 8 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index 2875b87f1..40a318245 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -4,6 +4,7 @@ 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'; const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day @@ -98,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`); @@ -112,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/index.ts b/src/db/index.ts index d44b79f3c..d58e046b3 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'; let sink: Sink; if (config.getDatabase().type === 'mongo') { @@ -146,10 +147,10 @@ export const getPushes = (query: Partial): Promise => sink. export const writeAudit = (action: Action): Promise => sink.writeAudit(action); export const getPush = (id: string): Promise => sink.getPush(id); export const deletePush = (id: string): Promise => sink.deletePush(id); -export const authorise = (id: string, attestation: any): Promise<{ message: string }> => +export const authorise = (id: string, attestation?: Attestation): Promise<{ message: string }> => sink.authorise(id, attestation); export const cancel = (id: string): Promise<{ message: string }> => sink.cancel(id); -export const reject = (id: string, attestation: any): Promise<{ message: string }> => +export const reject = (id: string, attestation?: Attestation): Promise<{ message: string }> => sink.reject(id, attestation); export const getRepos = (query?: Partial): Promise => sink.getRepos(query); export const getRepo = (name: string): Promise => sink.getRepo(name); diff --git a/src/db/mongo/pushes.ts b/src/db/mongo/pushes.ts index 968b2858a..2335fce47 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'; @@ -77,7 +78,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/proxy/actions/autoActions.ts b/src/proxy/actions/autoActions.ts index 450c97d80..4b8624ac0 100644 --- a/src/proxy/actions/autoActions.ts +++ b/src/proxy/actions/autoActions.ts @@ -5,7 +5,12 @@ 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.'); @@ -21,7 +26,12 @@ 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.'); diff --git a/src/proxy/processors/types.ts b/src/proxy/processors/types.ts index c4c447b5d..5c1e15a9b 100644 --- a/src/proxy/processors/types.ts +++ b/src/proxy/processors/types.ts @@ -13,10 +13,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/service/routes/push.ts b/src/service/routes/push.ts index d1c2fae2c..f328eb13f 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 { @@ -143,7 +143,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/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{' '} Date: Fri, 5 Dec 2025 21:51:20 +0900 Subject: [PATCH 02/31] chore: add missing Express.Request types in action files --- src/plugin.ts | 8 ++++---- src/proxy/chain.ts | 12 +++++++----- src/proxy/processors/pre-processor/parseAction.ts | 8 +++----- src/proxy/processors/push-action/audit.ts | 4 +++- src/proxy/processors/push-action/blockForAuth.ts | 4 +++- .../processors/push-action/checkAuthorEmails.ts | 6 ++++-- .../processors/push-action/checkCommitMessages.ts | 4 +++- src/proxy/processors/push-action/checkEmptyBranch.ts | 8 +++++--- .../processors/push-action/checkHiddenCommits.ts | 6 ++++-- .../processors/push-action/checkIfWaitingAuth.ts | 4 +++- .../push-action/checkRepoInAuthorisedList.ts | 4 +++- .../push-action/checkUserPushPermission.ts | 4 +++- src/proxy/processors/push-action/clearBareClone.ts | 6 ++++-- src/proxy/processors/push-action/getDiff.ts | 5 +++-- src/proxy/processors/push-action/gitleaks.ts | 9 +++++---- src/proxy/processors/push-action/parsePush.ts | 10 ++++++---- src/proxy/processors/push-action/preReceive.ts | 8 +++++--- src/proxy/processors/push-action/pullRemote.ts | 11 +++++++++-- src/proxy/processors/push-action/scanDiff.ts | 8 +++++--- src/proxy/processors/push-action/writePack.ts | 8 +++++--- src/proxy/processors/types.ts | 4 +++- 21 files changed, 90 insertions(+), 51 deletions(-) diff --git a/src/plugin.ts b/src/plugin.ts index 92fb9a99c..b44854c26 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -177,7 +177,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 +193,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 +205,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 +221,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/chain.ts b/src/proxy/chain.ts index 5aeac2d96..c73b3bc66 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -1,9 +1,11 @@ +import { Request, Response } from 'express'; + import { PluginLoader } from '../plugin'; import { Action } from './actions'; import * as proc from './processors'; import { attemptAutoApproval, attemptAutoRejection } from './actions/autoActions'; -const pushActionChain: ((req: any, action: Action) => Promise)[] = [ +const pushActionChain: ((req: Request, action: Action) => Promise)[] = [ proc.push.parsePush, proc.push.checkEmptyBranch, proc.push.checkRepoInAuthorisedList, @@ -23,17 +25,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; try { @@ -70,7 +72,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/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/audit.ts b/src/proxy/processors/push-action/audit.ts index 32e556fb7..47d07fc8e 100644 --- a/src/proxy/processors/push-action/audit.ts +++ b/src/proxy/processors/push-action/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/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..e08e4ea43 100644 --- a/src/proxy/processors/push-action/checkCommitMessages.ts +++ b/src/proxy/processors/push-action/checkCommitMessages.ts @@ -1,3 +1,5 @@ +import { Request } from 'express'; + import { Action, Step } from '../../actions'; import { getCommitConfig } from '../../../config'; @@ -48,7 +50,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..7b0fd7778 100644 --- a/src/proxy/processors/push-action/checkEmptyBranch.ts +++ b/src/proxy/processors/push-action/checkEmptyBranch.ts @@ -1,8 +1,10 @@ -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'; -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}`); @@ -17,7 +19,7 @@ const isEmptyBranch = async (action: Action) => { 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..062ba6ab9 100644 --- a/src/proxy/processors/push-action/checkHiddenCommits.ts +++ b/src/proxy/processors/push-action/checkHiddenCommits.ts @@ -1,8 +1,10 @@ +import { spawnSync } from 'child_process'; +import { Request } from 'express'; import path from 'path'; + import { Action, Step } from '../../actions'; -import { spawnSync } from 'child_process'; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkHiddenCommits'); try { diff --git a/src/proxy/processors/push-action/checkIfWaitingAuth.ts b/src/proxy/processors/push-action/checkIfWaitingAuth.ts index baedb0df3..9b9030d73 100644 --- a/src/proxy/processors/push-action/checkIfWaitingAuth.ts +++ b/src/proxy/processors/push-action/checkIfWaitingAuth.ts @@ -1,8 +1,10 @@ +import { Request } from 'express'; + import { Action, Step } from '../../actions'; import { getPush } from '../../../db'; // 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); 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/clearBareClone.ts b/src/proxy/processors/push-action/clearBareClone.ts index 91f7f5b22..c4dbd1699 100644 --- a/src/proxy/processors/push-action/clearBareClone.ts +++ b/src/proxy/processors/push-action/clearBareClone.ts @@ -1,7 +1,9 @@ -import { Action, Step } from '../../actions'; +import { Request } from 'express'; import fs from 'node:fs'; -const exec = async (req: any, action: Action): Promise => { +import { Action, Step } from '../../actions'; + +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('clearBareClone'); // Recursively remove the contents of ./.remote and ignore exceptions diff --git a/src/proxy/processors/push-action/getDiff.ts b/src/proxy/processors/push-action/getDiff.ts index dbdc4e4e9..f6144d658 100644 --- a/src/proxy/processors/push-action/getDiff.ts +++ b/src/proxy/processors/push-action/getDiff.ts @@ -1,9 +1,10 @@ -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'; -const exec = async (req: any, action: Action): Promise => { +const exec = async (_req: Request, action: Action): Promise => { const step = new Step('diff'); try { diff --git a/src/proxy/processors/push-action/gitleaks.ts b/src/proxy/processors/push-action/gitleaks.ts index 1cf5b2236..aa0e27860 100644 --- a/src/proxy/processors/push-action/gitleaks.ts +++ b/src/proxy/processors/push-action/gitleaks.ts @@ -1,9 +1,10 @@ -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'; const EXIT_CODE = 99; function runCommand( @@ -109,7 +110,7 @@ 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; diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index ababdb751..0baeda245 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, @@ -27,11 +29,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) { @@ -81,7 +83,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.'); diff --git a/src/proxy/processors/push-action/preReceive.ts b/src/proxy/processors/push-action/preReceive.ts index 1c3ad36b9..10390af4e 100644 --- a/src/proxy/processors/push-action/preReceive.ts +++ b/src/proxy/processors/push-action/preReceive.ts @@ -1,14 +1,16 @@ +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'; -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 => { diff --git a/src/proxy/processors/push-action/pullRemote.ts b/src/proxy/processors/push-action/pullRemote.ts index 73b8981ec..3811e1bf9 100644 --- a/src/proxy/processors/push-action/pullRemote.ts +++ b/src/proxy/processors/push-action/pullRemote.ts @@ -1,11 +1,13 @@ -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'; + const dir = './.remote'; -const exec = async (req: any, action: Action): Promise => { +const exec = async (req: Request, action: Action): Promise => { const step = new Step('pullRemote'); try { @@ -24,6 +26,11 @@ 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(':'); 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 c41181483..caad58348 100644 --- a/src/proxy/processors/push-action/writePack.ts +++ b/src/proxy/processors/push-action/writePack.ts @@ -1,9 +1,11 @@ -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'; -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) { diff --git a/src/proxy/processors/types.ts b/src/proxy/processors/types.ts index 5c1e15a9b..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; } From aff314e1d817c18da807f78055be9cda3fb90d1a Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 5 Dec 2025 21:52:11 +0900 Subject: [PATCH 03/31] chore: add missing db types --- src/db/mongo/pushes.ts | 7 +++++-- src/db/mongo/repo.ts | 4 ++-- src/db/mongo/users.ts | 4 ++-- src/db/types.ts | 5 +++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/db/mongo/pushes.ts b/src/db/mongo/pushes.ts index 2335fce47..36c467661 100644 --- a/src/db/mongo/pushes.ts +++ b/src/db/mongo/pushes.ts @@ -44,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; }; @@ -64,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`); 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; From 8c50807e0b71cd9d8d05f5366fe29f755f5a005b Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 5 Dec 2025 21:53:56 +0900 Subject: [PATCH 04/31] refactor: extend Express.Request to include bodyRaw and customPipe fields, remove any in src/proxy/routes/index --- src/proxy/routes/index.ts | 14 +++++++------- src/types/express.d.ts | 8 ++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) create mode 100644 src/types/express.d.ts diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index a7d39cc6b..bf43ab9b3 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -46,10 +46,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); @@ -156,13 +156,13 @@ const extractRawBody = async (req: Request, res: Response, next: NextFunction) = highWaterMark: 4 * 1024 * 1024, }); - req.pipe(proxyStream); - req.pipe(pluginStream); + req.customPipe?.(proxyStream); + req.customPipe?.(pluginStream); 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.customPipe = (dest, opts) => proxyStream.pipe(dest, opts); next(); } catch (e) { console.error(e); diff --git a/src/types/express.d.ts b/src/types/express.d.ts new file mode 100644 index 000000000..135dc4fc2 --- /dev/null +++ b/src/types/express.d.ts @@ -0,0 +1,8 @@ +import { Readable } from 'stream'; + +declare module 'express-serve-static-core' { + interface Request { + bodyRaw?: Buffer; + customPipe?(dest: any, opts?: any): Readable; + } +} From e928368a5a88fd7da27139206cc1a72b43c10fb8 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 6 Dec 2025 11:34:54 +0900 Subject: [PATCH 05/31] fix: proxy hanging on push due to improper req.pipe override --- src/plugin.ts | 2 ++ src/proxy/routes/index.ts | 6 +++--- src/types/express.d.ts | 1 - 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plugin.ts b/src/plugin.ts index b44854c26..25be81046 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -1,3 +1,5 @@ +import { Request } from 'express'; + import { Action } from './proxy/actions'; const lpModule = import('load-plugin'); diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index bf43ab9b3..f181bf068 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -156,13 +156,13 @@ const extractRawBody = async (req: Request, res: Response, next: NextFunction) = highWaterMark: 4 * 1024 * 1024, }); - req.customPipe?.(proxyStream); - req.customPipe?.(pluginStream); + req.pipe(proxyStream); + req.pipe(pluginStream); try { const buf = await getRawBody(pluginStream, { limit: '1gb' }); req.bodyRaw = buf; - req.customPipe = (dest, opts) => proxyStream.pipe(dest, opts); + req.pipe = (dest, opts) => proxyStream.pipe(dest, opts); next(); } catch (e) { console.error(e); diff --git a/src/types/express.d.ts b/src/types/express.d.ts index 135dc4fc2..891c7e22c 100644 --- a/src/types/express.d.ts +++ b/src/types/express.d.ts @@ -3,6 +3,5 @@ import { Readable } from 'stream'; declare module 'express-serve-static-core' { interface Request { bodyRaw?: Buffer; - customPipe?(dest: any, opts?: any): Readable; } } From 528d2141f24544dc9cb19028ea60199c7613e683 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 7 Dec 2025 13:22:50 +0900 Subject: [PATCH 06/31] refactor: improve UI component and service typing --- src/ui/services/auth.ts | 2 +- src/ui/services/git-push.ts | 6 ++++- src/ui/services/repo.ts | 4 ++-- src/ui/services/user.ts | 11 +++++---- src/ui/types.ts | 4 ++++ src/ui/views/Login/Login.tsx | 2 +- .../OpenPushRequests/OpenPushRequests.tsx | 24 ++++--------------- .../components/PushesTable.tsx | 6 ++++- 8 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/ui/services/auth.ts b/src/ui/services/auth.ts index 81acd399e..bcea836e0 100644 --- a/src/ui/services/auth.ts +++ b/src/ui/services/auth.ts @@ -44,7 +44,7 @@ export const getAxiosConfig = (): AxiosConfig => { /** * Processes authentication errors and returns a user-friendly error message */ -export const processAuthError = (error: AxiosError, jwtAuthEnabled = false): string => { +export const processAuthError = (error: AxiosError, jwtAuthEnabled = false): string => { let errorMessage = `Failed to authorize user: ${error.response?.data?.trim() ?? ''}. `; if (jwtAuthEnabled && !localStorage.getItem('ui_jwt_token')) { errorMessage += diff --git a/src/ui/services/git-push.ts b/src/ui/services/git-push.ts index 3de0dac4d..239371e7e 100644 --- a/src/ui/services/git-push.ts +++ b/src/ui/services/git-push.ts @@ -43,7 +43,11 @@ const getPushes = async ( }, ): Promise => { const url = new URL(`${API_V1_BASE}/push`); - url.search = new URLSearchParams(query as any).toString(); + + const stringifiedQuery = Object.fromEntries( + Object.entries(query).map(([key, value]) => [key, value.toString()]), + ); + url.search = new URLSearchParams(stringifiedQuery).toString(); setIsLoading(true); diff --git a/src/ui/services/repo.ts b/src/ui/services/repo.ts index 59c68342d..300cabea8 100644 --- a/src/ui/services/repo.ts +++ b/src/ui/services/repo.ts @@ -36,10 +36,10 @@ const getRepos = async ( setAuth: (auth: boolean) => void, setIsError: (isError: boolean) => void, setErrorMessage: (errorMessage: string) => void, - query: Record = {}, + query: Record = {}, ): Promise => { const url = new URL(`${API_V1_BASE}/repo`); - url.search = new URLSearchParams(query as any).toString(); + url.search = new URLSearchParams(query).toString(); setIsLoading(true); await axios(url.toString(), getAxiosConfig()) .then((response) => { diff --git a/src/ui/services/user.ts b/src/ui/services/user.ts index 98e97883e..caf0dd981 100644 --- a/src/ui/services/user.ts +++ b/src/ui/services/user.ts @@ -55,7 +55,7 @@ const getUsers = async ( setAuth(false); setErrorMessage(processAuthError(error)); } else { - const msg = (error.response?.data as any)?.message ?? error.message; + const msg = error.response?.data?.message ?? error.message; setErrorMessage(`Error fetching users: ${msg}`); } } else { @@ -70,10 +70,11 @@ const updateUser = async (user: PublicUser): Promise => { console.log(user); try { await axios.post(`${API_BASE}/api/auth/gitAccount`, user, getAxiosConfig()); - } catch (error) { - const axiosError = error as AxiosError; - if (axiosError.response) { - console.log((axiosError.response.data as any).message); + } catch (error: unknown) { + if (axios.isAxiosError(error)) { + console.log(error.response?.data?.message); + } else { + console.log(`Error updating user: ${error}`); } throw error; } diff --git a/src/ui/types.ts b/src/ui/types.ts index 342208d56..eba9c6ec1 100644 --- a/src/ui/types.ts +++ b/src/ui/types.ts @@ -1,3 +1,5 @@ +import { CSSProperties } from '@material-ui/core/styles/withStyles'; + import { Action } from '../proxy/actions'; import { Step } from '../proxy/actions/Step'; import { Repo } from '../db/types'; @@ -89,3 +91,5 @@ export interface SCMRepositoryMetadata { profileUrl?: string; avatarUrl?: string; } + +export type CSSProperty = React.CSSProperties | CSSProperties; diff --git a/src/ui/views/Login/Login.tsx b/src/ui/views/Login/Login.tsx index 7a4ecabfb..d837c6591 100644 --- a/src/ui/views/Login/Login.tsx +++ b/src/ui/views/Login/Login.tsx @@ -74,7 +74,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/OpenPushRequests/OpenPushRequests.tsx b/src/ui/views/OpenPushRequests/OpenPushRequests.tsx index 41c2672a8..7d4022464 100644 --- a/src/ui/views/OpenPushRequests/OpenPushRequests.tsx +++ b/src/ui/views/OpenPushRequests/OpenPushRequests.tsx @@ -18,38 +18,22 @@ const Dashboard: React.FC = () => { { tabName: 'Pending', tabIcon: Visibility, - tabContent: ( - - ), + tabContent: , }, { tabName: 'Approved', tabIcon: CheckCircle, - tabContent: , + tabContent: , }, { tabName: 'Canceled', tabIcon: Cancel, - tabContent: ( - - ), + tabContent: , }, { tabName: 'Rejected', tabIcon: Block, - tabContent: ( - - ), + tabContent: , }, ]; diff --git a/src/ui/views/OpenPushRequests/components/PushesTable.tsx b/src/ui/views/OpenPushRequests/components/PushesTable.tsx index 83cc90be9..f37cfbce5 100644 --- a/src/ui/views/OpenPushRequests/components/PushesTable.tsx +++ b/src/ui/views/OpenPushRequests/components/PushesTable.tsx @@ -20,7 +20,11 @@ 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); From 292c8462081b2984e1107e92c6af1493a5c82d82 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 14:52:52 +0900 Subject: [PATCH 07/31] refactor: add BASE_COMMIT_CONTENT object to remove any types in testParsePush --- test/testParsePush.test.ts | 90 +++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 41 deletions(-) diff --git a/test/testParsePush.test.ts b/test/testParsePush.test.ts index 25740048d..41247bf9b 100644 --- a/test/testParsePush.test.ts +++ b/test/testParsePush.test.ts @@ -11,8 +11,8 @@ 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'; /** * Creates a simplified sample PACK buffer for testing. @@ -137,6 +137,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. @@ -299,9 +309,9 @@ describe('parsePackFile', () => { branch: null, commitFrom: null, commitTo: null, - commitData: [] as any[], + commitData: [], user: null, - steps: [] as any[], + steps: [], addStep: vi.fn(function (this: any, step: any) { this.steps.push(step); }), @@ -456,7 +466,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 +519,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 +562,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 +617,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 +656,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 +686,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 +813,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 +851,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 +879,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 +907,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 +975,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 From 6ff457e48a18c16d5d1182d0527e6cb5fe342cb0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 14:54:21 +0900 Subject: [PATCH 08/31] chore: remove any types in ConfigLoader test (private field access) --- test/ConfigLoader.test.ts | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/test/ConfigLoader.test.ts b/test/ConfigLoader.test.ts index 6764b9f68..787bf65be 100644 --- a/test/ConfigLoader.test.ts +++ b/test/ConfigLoader.test.ts @@ -266,9 +266,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 () => { @@ -314,10 +314,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); }); }); @@ -416,15 +416,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/, ); }); @@ -597,9 +597,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); }); }); @@ -615,14 +612,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); }); @@ -642,8 +631,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); }); }); From 1edc320646eb5c2b17c2504b68e85a7f77110a32 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 15:10:24 +0900 Subject: [PATCH 09/31] refactor: improve typings in various test files --- src/proxy/routes/helper.ts | 3 ++- test/1.test.ts | 5 +++-- test/processors/writePack.test.ts | 6 ++---- test/services/routes/users.test.ts | 9 +++++++-- test/testParseAction.test.ts | 10 +++++----- test/testRepoApi.test.ts | 5 +++-- 6 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/proxy/routes/helper.ts b/src/proxy/routes/helper.ts index 46f73a2c7..b39899845 100644 --- a/src/proxy/routes/helper.ts +++ b/src/proxy/routes/helper.ts @@ -1,4 +1,5 @@ import * as db from '../../db'; +import { IncomingHttpHeaders } from 'http'; /** Regex used to analyze un-proxied Git URLs */ const GIT_URL_REGEX = /(.+:\/\/)([^/]+)(\/.+\.git)(\/.+)*/; @@ -152,7 +153,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/test/1.test.ts b/test/1.test.ts index 3a25b17a8..ef9e18ac9 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 () { @@ -72,7 +73,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/processors/writePack.test.ts b/test/processors/writePack.test.ts index 85d948243..d4acc61b5 100644 --- a/test/processors/writePack.test.ts +++ b/test/processors/writePack.test.ts @@ -7,7 +7,7 @@ vi.mock('child_process'); vi.mock('fs'); describe('writePack', () => { - let exec: any; + let exec: typeof import('../../src/proxy/processors/push-action/writePack').exec; let readdirSyncMock: any; let spawnSyncMock: any; let stepLogSpy: any; @@ -19,9 +19,7 @@ describe('writePack', () => { 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'); stepSetContentSpy = vi.spyOn(Step.prototype, 'setContent'); 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/testParseAction.test.ts b/test/testParseAction.test.ts index a1e424430..dcb9d9b91 100644 --- a/test/testParseAction.test.ts +++ b/test/testParseAction.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import * as preprocessor from '../src/proxy/processors/pre-processor/parseAction'; import * as db from '../src/db'; -let testRepo: any = null; +let testRepo: db.Repo | null = null; const TEST_REPO = { url: 'https://github.com/finos/git-proxy.git', @@ -27,7 +27,7 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a pull request into an action', async () => { - const req = { + const req: any = { originalUrl: '/github.com/finos/git-proxy.git/git-upload-pack', method: 'GET', headers: { 'content-type': 'application/x-git-upload-pack-request' }, @@ -41,7 +41,7 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a pull request with a legacy path into an action', async () => { - const req = { + const req: any = { originalUrl: '/finos/git-proxy.git/git-upload-pack', method: 'GET', headers: { 'content-type': 'application/x-git-upload-pack-request' }, @@ -55,7 +55,7 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a push request into an action', async () => { - const req = { + const req: any = { originalUrl: '/github.com/finos/git-proxy.git/git-receive-pack', method: 'POST', headers: { 'content-type': 'application/x-git-receive-pack-request' }, @@ -69,7 +69,7 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a push request with a legacy path into an action', async () => { - const req = { + const req: any = { originalUrl: '/finos/git-proxy.git/git-receive-pack', method: 'POST', headers: { 'content-type': 'application/x-git-receive-pack-request' }, diff --git a/test/testRepoApi.test.ts b/test/testRepoApi.test.ts index 83d12f71c..0995e0121 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[] = []; From 4535c5aa9cc9dbb0e27e515cef81f6f1650fe06d Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 15:49:58 +0900 Subject: [PATCH 10/31] refactor: add SAMPLE_COMMIT and remove any types in getDiff --- src/proxy/processors/constants.ts | 13 ++++++++ test/processors/getDiff.test.ts | 35 ++++++++++++---------- test/processors/scanDiff.emptyDiff.test.ts | 12 ++++---- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/proxy/processors/constants.ts b/src/proxy/processors/constants.ts index 3ad5784b4..97718312f 100644 --- a/src/proxy/processors/constants.ts +++ b/src/proxy/processors/constants.ts @@ -1,6 +1,19 @@ +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', +}; diff --git a/test/processors/getDiff.test.ts b/test/processors/getDiff.test.ts index ed5a48594..f7bb72449 100644 --- a/test/processors/getDiff.test.ts +++ b/test/processors/getDiff.test.ts @@ -1,11 +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 { Commit } from '../../src/proxy/actions/Action'; +import { EMPTY_COMMIT_HASH, SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; describe('getDiff', () => { let tempDir: string; @@ -40,9 +42,9 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; action.commitTo = 'HEAD'; - action.commitData = [{ parent: '0000000000000000000000000000000000000000' } as Commit]; + 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'); @@ -55,9 +57,9 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = 'HEAD~1'; action.commitTo = 'HEAD'; - action.commitData = [{ parent: '0000000000000000000000000000000000000000' } as Commit]; + 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'); @@ -71,7 +73,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', @@ -84,9 +86,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', @@ -106,11 +108,11 @@ describe('getDiff', () => { action.proxyGitPath = path.dirname(tempDir); action.repoName = path.basename(tempDir); - action.commitFrom = '0000000000000000000000000000000000000000'; + action.commitFrom = EMPTY_COMMIT_HASH; action.commitTo = headCommit; - action.commitData = [{ parent: parentCommit } as Commit]; + 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(); @@ -132,9 +134,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'); @@ -156,9 +161,9 @@ describe('getDiff', () => { action.repoName = 'temp-test-repo'; action.commitFrom = from; action.commitTo = to; - action.commitData = [{ parent: '0000000000000000000000000000000000000000' } as Commit]; + 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/scanDiff.emptyDiff.test.ts b/test/processors/scanDiff.emptyDiff.test.ts index f5a362238..7f0f26f0e 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(); @@ -80,7 +82,7 @@ index 1234567..abcdefg 100644 const diffStep = generateDiffStep(12345 as any); 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'); From 1b4c2ab72324ac973ed8f9ab4b7a6c074a46c72c Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 15:50:35 +0900 Subject: [PATCH 11/31] chore: remove any types in gitLeaks tests --- test/processors/gitLeaks.test.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/processors/gitLeaks.test.ts b/test/processors/gitLeaks.test.ts index 3e9d9234a..666872073 100644 --- a/test/processors/gitLeaks.test.ts +++ b/test/processors/gitLeaks.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; 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 +10,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 +25,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 +34,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 getAPIs: typeof import('../../src/config').getAPIs; + let fsModule: typeof import('node:fs/promises'); let spawn: any; beforeEach(async () => { @@ -129,7 +129,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 +137,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 +171,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 +179,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 +212,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 +220,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); @@ -265,7 +265,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 +305,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 +313,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); From 73c9c0b32840277223b7b35ea8dbb7cb3653c1ee Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 16:11:57 +0900 Subject: [PATCH 12/31] chore: use SAMPLE_COMMIT for commit data and remove any types in checkCommitMessages --- test/processors/checkCommitMessages.test.ts | 200 ++++++++++---------- 1 file changed, 105 insertions(+), 95 deletions(-) diff --git a/test/processors/checkCommitMessages.test.ts b/test/processors/checkCommitMessages.test.ts index 0a85b5691..f5945bd66 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 { Commit } from '../../src/proxy/actions/Action'; +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(() => ({})), @@ -41,9 +42,9 @@ describe('checkCommitMessages', () => { 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: '' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith('No commit message included...'); @@ -51,27 +52,27 @@ describe('checkCommitMessages', () => { it('should block null commit messages', async () => { const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: null as any } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: null as any }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: undefined as any }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 123 as any }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -81,18 +82,18 @@ 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: { text: 'fix: bug' } as any }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: ['fix: bug'] as any }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -101,9 +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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password to config' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -114,30 +115,30 @@ 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 Commit, - { message: 'Store Secret key' } as Commit, - { message: 'Update TOKEN value' } as Commit, + { ...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({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Update mypassword123' }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password and secret token' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -146,27 +147,31 @@ 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'See http://example.com for details' }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: 'Update docs at https://docs.example.com' }, + ]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: 'See http://example.com and https://other.com' }, + ]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -176,9 +181,9 @@ describe('checkCommitMessages', () => { vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'SSN: 123-45-6789' } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'SSN: 123-45-6789' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -188,9 +193,9 @@ describe('checkCommitMessages', () => { vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'This is private information' } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'This is private information' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -199,9 +204,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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'password at http://example.com' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -211,9 +216,9 @@ describe('checkCommitMessages', () => { vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Add secret key' } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add secret key' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -223,9 +228,9 @@ describe('checkCommitMessages', () => { vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Visit http://example.com' } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Visit http://example.com' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -234,9 +239,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 Commit]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: 'fix: resolve bug in user authentication' }, + ]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -247,12 +254,12 @@ 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 Commit, - { message: 'chore: update dependencies' } as Commit, - { message: 'docs: improve documentation' } as Commit, + { ...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({} as Request, action); expect(result.steps[0].error).toBe(false); }); @@ -263,9 +270,9 @@ describe('checkCommitMessages', () => { vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Any message should pass' } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Any message should pass' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); }); @@ -275,12 +282,12 @@ describe('checkCommitMessages', () => { it('should handle multiple valid commits', async () => { const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'feat: add feature A' } as Commit, - { message: 'fix: resolve issue B' } as Commit, - { message: 'chore: update config C' } as Commit, + { ...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({} as Request, action); expect(result.steps[0].error).toBe(false); }); @@ -288,12 +295,12 @@ describe('checkCommitMessages', () => { 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 Commit, - { message: 'fix: add password to config' } as Commit, - { message: 'chore: update config C' } as Commit, + { ...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({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -301,21 +308,24 @@ describe('checkCommitMessages', () => { it('should block when multiple commits are invalid', async () => { const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [ - { message: 'Add password' } as Commit, - { message: 'Store secret' } as Commit, - { message: 'feat: valid message' } as Commit, + { ...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({} as Request, 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 Commit, { message: 'fix: bug' } as Commit]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: 'fix: bug' }, + { ...SAMPLE_COMMIT, message: 'fix: bug' }, + ]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); }); @@ -323,12 +333,12 @@ describe('checkCommitMessages', () => { 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 Commit, - { message: 'Add password' } as Commit, - { message: 'fix: bug' } as Commit, + { ...SAMPLE_COMMIT, message: 'fix: bug' }, + { ...SAMPLE_COMMIT, message: 'Add password' }, + { ...SAMPLE_COMMIT, message: 'fix: bug' }, ]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(true); }); @@ -337,18 +347,18 @@ 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password' }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add password' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); const step = result.steps[0]; // first log is the "push blocked" message @@ -359,9 +369,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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add secret' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); const step = result.steps[0]; expect(step.errorMessage).toContain('Your push has been blocked'); @@ -371,11 +381,11 @@ 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 Commit, - { message: 'Store token' } as Commit, + { ...SAMPLE_COMMIT, message: 'Add password' }, + { ...SAMPLE_COMMIT, message: 'Store token' }, ]; - const result = await exec({}, action); + const result = await exec({} as Request, action); const step = result.steps[0]; expect(step.errorMessage).toContain('Add password'); @@ -388,7 +398,7 @@ describe('checkCommitMessages', () => { const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = undefined; - const result = await exec({}, action); + const result = await exec({} as Request, action); // should handle gracefully expect(result.steps).toHaveLength(1); @@ -398,16 +408,16 @@ describe('checkCommitMessages', () => { const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = []; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: ' ' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); }); @@ -415,9 +425,9 @@ describe('checkCommitMessages', () => { 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: longMessage }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); }); @@ -427,18 +437,18 @@ describe('checkCommitMessages', () => { vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Contains $pecial characters' } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Contains $pecial characters' }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'feat: 添加新功能 🎉' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result.steps[0].error).toBe(false); }); @@ -448,10 +458,10 @@ describe('checkCommitMessages', () => { vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ message: 'Any message' } as Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'Any message' }]; // test that it doesn't crash - expect(() => exec({}, action)).not.toThrow(); + expect(() => exec({} as Request, action)).not.toThrow(); }); }); @@ -464,28 +474,28 @@ 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; const initialStepCount = action.steps.length; - const result = await exec({}, action); + const result = await exec({} as Request, 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 Commit]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; - const result = await exec({}, action); + const result = await exec({} as Request, action); expect(result).toBe(action); }); @@ -494,10 +504,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 Commit]; + 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); }); From 30b86e3fbb8b562dd34b1cbf15d6a25ac16d9805 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 16:13:38 +0900 Subject: [PATCH 13/31] chore: add various missing test stub types Sets stub types to (`ReturnType`) to remove the any type --- test/processors/checkEmptyBranch.test.ts | 10 ++++---- test/processors/gitLeaks.test.ts | 8 +++---- test/processors/scanDiff.test.ts | 29 ++++++++++++------------ test/processors/writePack.test.ts | 10 ++++---- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/test/processors/checkEmptyBranch.test.ts b/test/processors/checkEmptyBranch.test.ts index bb13250ef..20cbf3583 100644 --- a/test/processors/checkEmptyBranch.test.ts +++ b/test/processors/checkEmptyBranch.test.ts @@ -1,12 +1,14 @@ +import { Request } from 'express'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { Action } from '../../src/proxy/actions'; +import { 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 () => { @@ -24,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), @@ -61,7 +63,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/gitLeaks.test.ts b/test/processors/gitLeaks.test.ts index 666872073..54477f343 100644 --- a/test/processors/gitLeaks.test.ts +++ b/test/processors/gitLeaks.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi, MockInstance } from 'vitest'; import { Action, Step } from '../../src/proxy/actions'; vi.mock('../../src/config', async (importOriginal) => { @@ -37,9 +37,9 @@ describe('gitleaks', () => { 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 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; diff --git a/test/processors/scanDiff.test.ts b/test/processors/scanDiff.test.ts index 13c4d54c3..2d009431a 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); @@ -259,7 +260,7 @@ describe('Scan commit diff', () => { const action = new Action('1', 'type', 'method', 1, 'test/repo.git'); action.steps = [generateDiffStep(1337 as any)]; - 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 d4acc61b5..e63c87c6c 100644 --- a/test/processors/writePack.test.ts +++ b/test/processors/writePack.test.ts @@ -8,11 +8,11 @@ vi.mock('fs'); describe('writePack', () => { let exec: typeof import('../../src/proxy/processors/push-action/writePack').exec; - let readdirSyncMock: any; - let spawnSyncMock: any; - let stepLogSpy: any; - let stepSetContentSpy: any; - let stepSetErrorSpy: any; + let readdirSyncMock: ReturnType; + let spawnSyncMock: ReturnType; + let stepLogSpy: ReturnType; + let stepSetContentSpy: ReturnType; + let stepSetErrorSpy: ReturnType; beforeEach(async () => { vi.clearAllMocks(); From d5e21db95c848e0c1954b64869f28a14f6efbe77 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 11 Dec 2025 20:25:26 +0900 Subject: [PATCH 14/31] chore: use SAMPLE_COMMIT for commit data in checkAuthorEmails tests --- src/service/index.ts | 2 +- test/processors/blockForAuth.test.ts | 11 +- test/processors/checkAuthorEmails.test.ts | 133 +++++++++++----------- 3 files changed, 74 insertions(+), 72 deletions(-) diff --git a/src/service/index.ts b/src/service/index.ts index c13e79314..8f4c8cdb8 100644 --- a/src/service/index.ts +++ b/src/service/index.ts @@ -74,7 +74,7 @@ async function createApp(proxy: Proxy): Promise { app.use(express.urlencoded({ extended: true })); app.use('/', routes(proxy)); app.use('/', express.static(absBuildPath)); - app.get('/*', (req, res) => { + app.get('/*path', (req, res) => { res.sendFile(path.join(`${absBuildPath}/index.html`)); }); 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 3319468d1..b78eeacd7 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 { Commit } from '../../src/proxy/actions/Action'; +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 Commit, - { authorEmail: 'jane.smith@company.org' } as Commit, + { ...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 Commit]; + 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 Commit]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: null as any }]; 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 Commit, - { authorEmail: 'missing@domain' } as Commit, - { authorEmail: '@nodomain.com' } as Commit, + { ...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 Commit, - { authorEmail: 'admin@company.org' } as Commit, + { ...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 Commit, - { authorEmail: 'admin@different.org' } as Commit, + { ...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 Commit, - { authorEmail: 'user@example.com.fake.org' } as Commit, + { ...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 Commit, - { authorEmail: 'admin@otherdomain.org' } as Commit, + { ...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 Commit, - { authorEmail: 'donotreply@company.org' } as Commit, + { ...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 Commit, - { authorEmail: 'valid.user@company.org' } as Commit, + { ...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 Commit, - { authorEmail: 'temporary@example.com' } as Commit, - { authorEmail: 'fakeuser@example.com' } as Commit, + { ...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 Commit, - { authorEmail: 'anything@example.com' } as Commit, + { ...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 Commit, // valid - { authorEmail: 'noreply@example.com' } as Commit, // invalid: blocked local - { authorEmail: 'valid@otherdomain.com' } as Commit, // 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 Commit]; + 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 Commit, - { authorEmail: 'user2@example.com' } as Commit, - { authorEmail: 'user1@example.com' } as Commit, // Duplicate - { authorEmail: 'user3@example.com' } as Commit, - { authorEmail: 'user2@example.com' } as Commit, // 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 Commit]; + 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 Commit, - { authorEmail: 'user2@example.com' } as Commit, + { ...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 Commit]; + 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 Commit]; + 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 Commit]; + 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 Commit, - { authorEmail: 'invalid' } as Commit, - { authorEmail: 'also.valid@example.com' } as Commit, + { ...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 Commit]; + 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 Commit]; + 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 Commit]; + 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 Commit, - { authorEmail: 'user.name@example.com' } as Commit, - { authorEmail: 'user_name@example.com' } as Commit, + { ...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 Commit, - { authorEmail: 'user@Example.Com' } as Commit, + { ...SAMPLE_COMMIT, authorEmail: 'user@EXAMPLE.COM' }, + { ...SAMPLE_COMMIT, authorEmail: 'user@Example.Com' }, ]; const result = await exec(mockReq, mockAction); From 82bd72cf8a0f47257073186cc173ffb786e207c0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 12 Dec 2025 15:46:28 +0900 Subject: [PATCH 15/31] chore: remove any types in UI --- .../layouts/dashboardStyle.ts | 2 +- src/ui/components/RouteGuard/RouteGuard.tsx | 2 +- src/ui/layouts/Dashboard.tsx | 6 +++--- src/ui/types.ts | 4 ++-- .../views/OpenPushRequests/components/PushesTable.tsx | 7 +------ src/ui/views/RepoList/Components/RepoOverview.tsx | 11 +++++++---- src/ui/views/UserList/Components/UserList.tsx | 6 +----- 7 files changed, 16 insertions(+), 22 deletions(-) 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/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/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 ( - +

; - icon?: string | React.ComponentType; + component: React.ComponentType; + icon?: string | React.ComponentType; visible?: boolean; } diff --git a/src/ui/views/OpenPushRequests/components/PushesTable.tsx b/src/ui/views/OpenPushRequests/components/PushesTable.tsx index f37cfbce5..9c85f4848 100644 --- a/src/ui/views/OpenPushRequests/components/PushesTable.tsx +++ b/src/ui/views/OpenPushRequests/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'; @@ -27,10 +25,7 @@ interface PushesTableProps { 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); @@ -87,7 +82,7 @@ const PushesTable: React.FC = (props) => {
- +
Timestamp diff --git a/src/ui/views/RepoList/Components/RepoOverview.tsx b/src/ui/views/RepoList/Components/RepoOverview.tsx index 4c647fb8a..9cc20ab72 100644 --- a/src/ui/views/RepoList/Components/RepoOverview.tsx +++ b/src/ui/views/RepoList/Components/RepoOverview.tsx @@ -30,10 +30,13 @@ const Repositories: React.FC = (props) => { setRemoteRepoData( await fetchRemoteRepositoryData(props.repo.project, props.repo.name, remoteUrl), ); - } catch (error: any) { - console.warn( - `Unable to fetch repository data for ${props.repo.project}/${props.repo.name} from '${remoteUrl}' - this may occur if the project is private or from an SCM vendor that is not supported.`, - ); + } catch (error: unknown) { + const errorMessage = `Unable to fetch repository data for ${props.repo.project}/${props.repo.name} from '${remoteUrl}' - this may occur if the project is private or from an SCM vendor that is not supported.`; + if (error instanceof Error) { + console.warn(errorMessage, error.message); + } else { + console.warn(errorMessage); + } } }; diff --git a/src/ui/views/UserList/Components/UserList.tsx b/src/ui/views/UserList/Components/UserList.tsx index 94b8fecb2..035930b12 100644 --- a/src/ui/views/UserList/Components/UserList.tsx +++ b/src/ui/views/UserList/Components/UserList.tsx @@ -11,7 +11,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 { getUsers } from '../../../services/user'; import Pagination from '../../../components/Pagination/Pagination'; import { CloseRounded, Check, KeyboardArrowRight } from '@material-ui/icons'; @@ -19,10 +18,7 @@ import Search from '../../../components/Search/Search'; import Danger from '../../../components/Typography/Danger'; import { PublicUser } from '../../../../db/types'; -const useStyles = makeStyles(styles as any); - const UserList: React.FC = () => { - const classes = useStyles(); const [users, setUsers] = useState([]); const [, setAuth] = useState(true); const [isLoading, setIsLoading] = useState(false); @@ -66,7 +62,7 @@ const UserList: React.FC = () => { -
+
Name From 3320f3fd34f9351b8e6d6ac8f8d1d255344992fa Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 12 Dec 2025 16:04:58 +0900 Subject: [PATCH 16/31] refactor: repo and push UI error handling, remove any error types --- src/service/routes/repo.ts | 13 ++++--- src/ui/context.ts | 2 +- src/ui/services/auth.ts | 10 ++++-- src/ui/services/git-push.ts | 67 +++++++++++++++++++----------------- src/ui/services/repo.ts | 43 +++++++++++++---------- src/ui/types.ts | 4 +++ src/ui/views/Login/Login.tsx | 3 +- 7 files changed, 79 insertions(+), 63 deletions(-) diff --git a/src/service/routes/repo.ts b/src/service/routes/repo.ts index 659767b23..7001933f7 100644 --- a/src/service/routes/repo.ts +++ b/src/service/routes/repo.ts @@ -187,16 +187,15 @@ const repo = (proxy: any) => { await theProxy.stop(); await theProxy.start(); } - } catch (e: any) { - console.error('Repository creation failed due to error: ', e.message ? e.message : e); - console.error(e.stack); + } catch (e: unknown) { + if (e instanceof Error) { + console.error('Repository creation failed due to error: ', e.message); + console.error(e.stack); + res.status(500).send({ message: 'Failed to create repository due to error' }); + } res.status(500).send({ message: 'Failed to create repository due to error' }); } } - } else { - res.status(401).send({ - message: 'You are not authorised to perform this action...', - }); } }); 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/services/auth.ts b/src/ui/services/auth.ts index bcea836e0..08e674efa 100644 --- a/src/ui/services/auth.ts +++ b/src/ui/services/auth.ts @@ -1,7 +1,8 @@ +import { AxiosError } from 'axios'; import { getCookie } from '../utils'; import { PublicUser } from '../../db/types'; import { API_BASE } from '../apiBase'; -import { AxiosError } from 'axios'; +import { BackendResponse } from '../types'; interface AxiosConfig { withCredentials: boolean; @@ -44,8 +45,11 @@ export const getAxiosConfig = (): AxiosConfig => { /** * Processes authentication errors and returns a user-friendly error message */ -export const processAuthError = (error: AxiosError, jwtAuthEnabled = false): string => { - let errorMessage = `Failed to authorize user: ${error.response?.data?.trim() ?? ''}. `; +export const processAuthError = ( + error: AxiosError, + jwtAuthEnabled = false, +): string => { + let errorMessage = `Failed to authorize user: ${error.response?.data?.message?.trim() ?? ''}. `; if (jwtAuthEnabled && !localStorage.getItem('ui_jwt_token')) { errorMessage += 'Set your JWT token in the settings page or disable JWT auth in your app configuration.'; diff --git a/src/ui/services/git-push.ts b/src/ui/services/git-push.ts index 239371e7e..4c8c5fb57 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 { API_BASE } from '../apiBase'; import { Action, Step } from '../../proxy/actions'; -import { PushActionView } from '../types'; +import { BackendResponse, PushActionView } from '../types'; const API_V1_BASE = `${API_BASE}/api/v1`; @@ -16,17 +16,19 @@ const getPush = async ( const url = `${API_V1_BASE}/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 ( @@ -51,22 +53,23 @@ const getPushes = async ( setIsLoading(true); - try { - const response = await axios(url.toString(), getAxiosConfig()); - setPushes(response.data as PushActionView[]); - } catch (error: any) { - 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); - } + 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 ( @@ -88,7 +91,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; @@ -106,7 +109,7 @@ const rejectPush = async ( const url = `${API_V1_BASE}/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; @@ -122,7 +125,7 @@ const cancelPush = async ( setIsError: (isError: boolean) => void, ): Promise => { const url = `${API_BASE}/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 300cabea8..8950c7b27 100644 --- a/src/ui/services/repo.ts +++ b/src/ui/services/repo.ts @@ -1,8 +1,8 @@ -import axios from 'axios'; +import axios, { AxiosError } from 'axios'; import { getAxiosConfig, processAuthError } from './auth.js'; import { API_BASE } from '../apiBase'; import { Repo } from '../../db/types'; -import { RepoView } from '../types'; +import { BackendResponse, RepoView } from '../types'; const API_V1_BASE = `${API_BASE}/api/v1`; @@ -18,8 +18,12 @@ const canAddUser = (repoId: string, user: string, action: string) => { return !repo.users.canPush.includes(user); } }) - .catch((error: any) => { - throw error; + .catch((error: unknown) => { + if (error instanceof Error) { + throw error; + } else { + throw new Error('Unknown error'); + } }); }; @@ -48,13 +52,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(() => { @@ -76,7 +80,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 { @@ -99,12 +103,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; + } } }; @@ -113,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 { @@ -126,8 +133,7 @@ const addUser = async (repoId: string, user: string, action: string): Promise => { const url = new URL(`${API_V1_BASE}/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; }); }; @@ -135,8 +141,7 @@ const deleteUser = async (user: string, repoId: string, action: string): Promise const deleteRepo = async (repoId: string): Promise => { const url = new URL(`${API_V1_BASE}/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/types.ts b/src/ui/types.ts index 64d26b8c5..ebf2b64d9 100644 --- a/src/ui/types.ts +++ b/src/ui/types.ts @@ -6,6 +6,10 @@ 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; } diff --git a/src/ui/views/Login/Login.tsx b/src/ui/views/Login/Login.tsx index d837c6591..010f0562b 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 { API_BASE } from '../../apiBase'; import { getAxiosConfig, processAuthError } from '../../services/auth'; +import { BackendResponse } from '../../types'; interface LoginResponse { username: string; @@ -74,7 +75,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); From ac9cf2b478ba674e8c6cfdfc505d1c431e789acb Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 12 Dec 2025 17:49:11 +0900 Subject: [PATCH 17/31] refactor: test file any removal (proxy, testDb, repo, forcePush, preReceive) --- test/db/file/repo.test.ts | 16 +++++----- .../integration/forcePush.integration.test.ts | 27 ++++++++++------- test/preReceive/preReceive.test.ts | 29 +++++++++---------- test/proxy.test.ts | 3 +- test/testDb.test.ts | 5 +--- 5 files changed, 42 insertions(+), 38 deletions(-) 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/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 bc8f3a416..4be44db5e 100644 --- a/test/preReceive/preReceive.test.ts +++ b/test/preReceive/preReceive.test.ts @@ -1,30 +1,27 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import path from 'path'; import * as fs from 'fs'; +import { Request } from 'express'; import { exec } from '../../src/proxy/processors/push-action/preReceive'; +import { Action, Step } from '../../src/proxy/actions'; // TODO: Replace with memfs to prevent test pollution issues vi.mock('fs', { spy: true }); describe('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/proxy.test.ts b/test/proxy.test.ts index 12950cb20..fbbcb9875 100644 --- a/test/proxy.test.ts +++ b/test/proxy.test.ts @@ -1,6 +1,7 @@ import https from 'https'; import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; import fs from 'fs'; +import { GitProxyConfig } from '../src/config/generated/config'; /* jescalada: these tests are currently causing the following error @@ -71,7 +72,7 @@ describe.skip('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, diff --git a/test/testDb.test.ts b/test/testDb.test.ts index 20e478f97..9f7d6a508 100644 --- a/test/testDb.test.ts +++ b/test/testDb.test.ts @@ -83,7 +83,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; @@ -91,7 +90,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; @@ -391,14 +389,13 @@ describe('Database clients', () => { 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); }); it('should be able to delete a user', async () => { await db.deleteUser(TEST_USER.username); const users = await db.getUsers(); - const cleanUsers = cleanResponseData(TEST_USER, users as any); + const cleanUsers = cleanResponseData(TEST_USER, users); expect(cleanUsers).not.toContainEqual(TEST_USER); }); From f29ddbbbd00048bdf48622aa156d1b876c560337 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 12 Dec 2025 17:52:26 +0900 Subject: [PATCH 18/31] chore: add unknown typing to error catch and process message (plugin, ConfigLoader) --- src/config/ConfigLoader.ts | 59 +++++++++++++++++++++----------------- src/config/index.ts | 5 ++-- src/plugin.ts | 5 ++-- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/config/ConfigLoader.ts b/src/config/ConfigLoader.ts index 22dd6abfd..6c16ebd8b 100644 --- a/src/config/ConfigLoader.ts +++ b/src/config/ConfigLoader.ts @@ -20,7 +20,7 @@ function isValidPath(filePath: string): boolean { try { path.resolve(filePath); return true; - } catch (error) { + } catch (error: unknown) { return false; } } @@ -79,8 +79,9 @@ 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 (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + console.error('Failed to create cache directory:', msg); return false; } } @@ -153,8 +154,9 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error loading from ${source.type} source:`, msg); return null; } }), @@ -189,8 +191,9 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Error reloading configuration:', msg); this.emit('configurationError', error); } finally { this.isReloading = false; @@ -223,10 +226,9 @@ export class ConfigLoader extends EventEmitter { // Use QuickType to validate and parse the configuration try { return Convert.toGitProxyConfig(content); - } catch (error) { - throw new Error( - `Invalid configuration file format: ${error instanceof Error ? error.message : 'Unknown error'}`, - ); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + throw new Error(`Invalid configuration file format: ${msg}`); } } @@ -244,10 +246,9 @@ export class ConfigLoader extends EventEmitter { const configJson = typeof response.data === 'string' ? response.data : JSON.stringify(response.data); return Convert.toGitProxyConfig(configJson); - } catch (error) { - throw new Error( - `Invalid configuration format from HTTP source: ${error instanceof Error ? error.message : 'Unknown error'}`, - ); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + throw new Error(`Invalid configuration format from HTTP source: ${msg}`); } } @@ -306,18 +307,20 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Failed to clone repository:', msg); + throw new Error(`Failed to clone repository: ${msg}`); } } 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Failed to pull repository:', msg); + throw new Error(`Failed to pull repository: ${msg}`); } } @@ -327,9 +330,10 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Failed to checkout branch ${source.branch}:`, msg); + throw new Error(`Failed to checkout branch ${source.branch}: ${msg}`); } } @@ -351,9 +355,10 @@ export class ConfigLoader extends EventEmitter { const config = Convert.toGitProxyConfig(content); console.log('Configuration loaded successfully from Git'); return config; - } catch (error: any) { - console.error('Failed to read or parse configuration file:', error.message); - throw new Error(`Failed to read or parse configuration file: ${error.message}`); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Failed to read or parse configuration file:', msg); + throw new Error(`Failed to read or parse configuration file: ${msg}`); } } diff --git a/src/config/index.ts b/src/config/index.ts index 177998764..b58901a27 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -61,8 +61,9 @@ 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); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error loading user config from ${userConfigFile}:`, msg); throw error; } } diff --git a/src/plugin.ts b/src/plugin.ts index 25be81046..01540d5c8 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -103,8 +103,9 @@ class PluginLoader { combinedPlugins.forEach((plugin) => { console.log(`Loaded plugin: ${plugin.constructor.name}`); }); - } catch (error) { - console.error(`Error loading plugins: ${error}`); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error loading plugins: ${msg}`); } } From 06dc08a28f2dd68e99668791b20816cd21af4e98 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 12 Dec 2025 18:12:22 +0900 Subject: [PATCH 19/31] refactor: add SAMPLE_REPO, replace in tests to remove any, add types for req and stubs --- test/db/db.test.ts | 19 +++- .../integration/forcePush.integration.test.ts | 25 +++-- test/preReceive/preReceive.test.ts | 14 +-- test/processors/checkEmptyBranch.test.ts | 4 +- test/processors/checkIfWaitingAuth.test.ts | 5 +- .../checkUserPushPermission.test.ts | 5 +- test/processors/gitLeaks.test.ts | 8 +- test/processors/writePack.test.ts | 5 +- test/services/routes/auth.test.ts | 9 +- test/testPush.test.ts | 94 +++++++++---------- 10 files changed, 105 insertions(+), 83 deletions(-) diff --git a/test/db/db.test.ts b/test/db/db.test.ts index bea72d574..986fd790e 100644 --- a/test/db/db.test.ts +++ b/test/db/db.test.ts @@ -15,6 +15,16 @@ vi.mock('../../src/config', () => ({ import * as db from '../../src/db'; import * as mongo from '../../src/db/mongo'; +const SAMPLE_REPO = { + project: 'myrepo', + name: 'myrepo', + url: 'https://github.com/myrepo.git', + users: { + canPush: ['alice'], + canAuthorise: ['bob'], + }, +}; + describe('db', () => { beforeEach(() => { vi.clearAllMocks(); @@ -27,11 +37,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 +50,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 +63,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/integration/forcePush.integration.test.ts b/test/integration/forcePush.integration.test.ts index 1cbc2ade3..8d1dd522c 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'); @@ -74,6 +77,7 @@ describe( action.commitTo = rebasedCommitSHA; action.commitData = [ { + ...SAMPLE_COMMIT, parent: parentSHA, message: 'Add feature (rebased)', author: 'Test User', @@ -84,10 +88,10 @@ describe( }, ]; - const afterGetDiff = await getDiff({}, action); + const afterGetDiff = await getDiff({} as Request, 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 +100,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({} as Request, afterGetDiff); + const scanStep = afterScanDiff.steps.find((s: Step) => s.stepName === 'scanDiff'); expect(scanStep).toBeDefined(); expect(scanStep?.error).toBe(false); @@ -118,6 +122,7 @@ describe( action.commitTo = rebasedCommitSHA; action.commitData = [ { + ...SAMPLE_COMMIT, parent: 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef', message: 'Add feature (rebased)', author: 'Test User', @@ -128,10 +133,10 @@ describe( }, ]; - const afterGetDiff = await getDiff({}, action); + const afterGetDiff = await getDiff({} as Request, 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 +149,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({} as Request, afterGetDiff); + const scanStep = afterScanDiff.steps.find((s: Step) => s.stepName === 'scanDiff'); expect(scanStep).toBeDefined(); expect(scanStep?.error).toBe(false); @@ -160,7 +165,7 @@ describe( 'test/repo.git', ); - const result = await scanDiff({}, action); + const result = await scanDiff({} as Request, 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 bc8f3a416..6b7dc9a8e 100644 --- a/test/preReceive/preReceive.test.ts +++ b/test/preReceive/preReceive.test.ts @@ -1,30 +1,32 @@ +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, Step } from '../../src/proxy/actions'; // TODO: Replace with memfs to prevent test pollution issues vi.mock('fs', { spy: true }); describe('Pre-Receive Hook Execution', () => { - let action: any; - let req: any; + let action: Action; + let req: Request; beforeEach(() => { - req = {}; + req = {} as Request; action = { - steps: [] as any[], + steps: [] as Step[], commitFrom: 'oldCommitHash', commitTo: 'newCommitHash', branch: 'feature-branch', proxyGitPath: 'test/preReceive/mock/repo', repoName: 'test-repo', - addStep(step: any) { + addStep(step: Step) { this.steps.push(step); }, setAutoApproval: vi.fn(), setAutoRejection: vi.fn(), - }; + } as unknown as Action; }); afterEach(() => { diff --git a/test/processors/checkEmptyBranch.test.ts b/test/processors/checkEmptyBranch.test.ts index 20cbf3583..7293cfde2 100644 --- a/test/processors/checkEmptyBranch.test.ts +++ b/test/processors/checkEmptyBranch.test.ts @@ -50,10 +50,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'; 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/gitLeaks.test.ts b/test/processors/gitLeaks.test.ts index 54477f343..55940f812 100644 --- a/test/processors/gitLeaks.test.ts +++ b/test/processors/gitLeaks.test.ts @@ -1,4 +1,6 @@ -import { describe, it, expect, beforeEach, afterEach, vi, MockInstance } from 'vitest'; +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) => { @@ -36,7 +38,7 @@ describe('gitleaks', () => { describe('exec', () => { let exec: typeof import('../../src/proxy/processors/push-action/gitleaks').exec; let action: Action; - let req: any; + let req: Request; let stepSpy: ReturnType; let logStub: ReturnType; let errorStub: ReturnType; @@ -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'; diff --git a/test/processors/writePack.test.ts b/test/processors/writePack.test.ts index e63c87c6c..3dadf2915 100644 --- a/test/processors/writePack.test.ts +++ b/test/processors/writePack.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; 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'); @@ -35,12 +36,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/services/routes/auth.test.ts b/test/services/routes/auth.test.ts index 65152f576..b120e50c0 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/testPush.test.ts b/test/testPush.test.ts index 8e605ac60..a7fa51fe2 100644 --- a/test/testPush.test.ts +++ b/test/testPush.test.ts @@ -4,6 +4,7 @@ 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'; // dummy repo const TEST_ORG = 'finos'; @@ -21,38 +22,25 @@ 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: '0000000000000000000000000000000000000000__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/0000000000000000000000000000000000000000__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'] ?? []; @@ -101,18 +89,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`) @@ -186,8 +176,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) as Action; + 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`) @@ -213,10 +205,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) as Action; 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`) @@ -240,7 +232,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`) @@ -264,7 +256,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`) @@ -274,10 +266,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) as Action; 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`) @@ -286,7 +278,7 @@ describe('Push API', () => { }); it('should NOT allow a non-authorizer to reject 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}/reject`) @@ -295,20 +287,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`) @@ -316,14 +310,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`) @@ -331,7 +325,7 @@ describe('Push API', () => { expect(res.status).toBe(401); 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); From a301eaad368cf976ae9383a0d08a31d088667be5 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 14 Dec 2025 11:27:38 +0900 Subject: [PATCH 20/31] refactor: cli test any/as removal, extract SAMPLE_REPO to constant --- packages/git-proxy-cli/index.ts | 51 ++++++++++-------- packages/git-proxy-cli/test/testCli.test.ts | 22 ++++---- packages/git-proxy-cli/test/testCliUtils.ts | 59 +++++++++++++-------- src/proxy/processors/constants.ts | 11 ++++ test/db/db.test.ts | 11 +--- 5 files changed, 89 insertions(+), 65 deletions(-) diff --git a/packages/git-proxy-cli/index.ts b/packages/git-proxy-cli/index.ts index 31ebc8a4c..0cc60b6d5 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'; @@ -47,13 +47,16 @@ 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 { + } else if (error instanceof Error) { console.error(`Error: Login '${username}': '${error.message}'`); process.exitCode = 2; + } else { + console.error(`Error: Login '${username}': '${error}'`); + process.exitCode = 2; } } } @@ -165,8 +168,9 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error: List: '${msg}'`); process.exitCode = 2; } } @@ -207,12 +211,12 @@ async function authoriseGitPush(id: string) { ); console.log(`Authorise: ID: '${id}': OK`); - } catch (error: any) { + } catch (error: unknown) { // default error - let errorMessage = `Error: Authorise: '${error.message}'`; + let errorMessage = `Error: Authorise: '${error instanceof Error ? error.message : String(error)}'`; process.exitCode = 2; - if (error.response) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: errorMessage = 'Error: Authorise: Authentication required'; @@ -254,12 +258,12 @@ async function rejectGitPush(id: string) { ); console.log(`Reject: ID: '${id}': OK`); - } catch (error: any) { + } catch (error: unknown) { // default error - let errorMessage = `Error: Reject: '${error.message}'`; + let errorMessage = `Error: Reject: '${error instanceof Error ? error.message : String(error)}'`; process.exitCode = 2; - if (error.response) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: errorMessage = 'Error: Reject: Authentication required'; @@ -301,12 +305,12 @@ async function cancelGitPush(id: string) { ); console.log(`Cancel: ID: '${id}': OK`); - } catch (error: any) { + } catch (error: unknown) { // default error - let errorMessage = `Error: Cancel: '${error.message}'`; + let errorMessage = `Error: Cancel: '${error instanceof Error ? error.message : String(error)}'`; process.exitCode = 2; - if (error.response) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: errorMessage = 'Error: Cancel: Authentication required'; @@ -338,8 +342,9 @@ async function logout() { headers: { Cookie: cookies }, }, ); - } catch (error: any) { - console.log(`Warning: Logout: '${error.message}'`); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.log(`Warning: Logout: '${msg}'`); } } @@ -362,10 +367,10 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error: Reload config: '${msg}'`); process.exitCode = 2; - console.error(errorMessage); } } @@ -408,11 +413,11 @@ async function createUser( ); console.log(`User '${username}' created successfully`); - } catch (error: any) { - let errorMessage = `Error: Create User: '${error.message}'`; + } catch (error: unknown) { + let errorMessage = `Error: Create User: '${error instanceof Error ? error.message : String(error)}'`; process.exitCode = 2; - if (error.response) { + if (isAxiosError(error) && error.response) { switch (error.response.status) { case 401: errorMessage = 'Error: Create User: Authentication required'; diff --git a/packages/git-proxy-cli/test/testCli.test.ts b/packages/git-proxy-cli/test/testCli.test.ts index 3e5545d1f..4c73db39d 100644 --- a/packages/git-proxy-cli/test/testCli.test.ts +++ b/packages/git-proxy-cli/test/testCli.test.ts @@ -3,8 +3,7 @@ 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'; setConfigFile(path.join(process.cwd(), 'test', 'testCli.proxy.config.json')); @@ -14,6 +13,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 +220,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 +297,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 +420,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 +582,9 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error cleaning up user: ${msg}`); } } }); @@ -612,8 +613,9 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error cleaning up user: ${msg}`); } } }); @@ -625,7 +627,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/proxy/processors/constants.ts b/src/proxy/processors/constants.ts index 97718312f..f48ef6563 100644 --- a/src/proxy/processors/constants.ts +++ b/src/proxy/processors/constants.ts @@ -1,3 +1,4 @@ +import { Repo } from '../../db/types'; import { CommitData } from './types'; export const BRANCH_PREFIX = 'refs/heads/'; @@ -17,3 +18,13 @@ export const SAMPLE_COMMIT: CommitData = { 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/test/db/db.test.ts b/test/db/db.test.ts index 986fd790e..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(), @@ -15,16 +16,6 @@ vi.mock('../../src/config', () => ({ import * as db from '../../src/db'; import * as mongo from '../../src/db/mongo'; -const SAMPLE_REPO = { - project: 'myrepo', - name: 'myrepo', - url: 'https://github.com/myrepo.git', - users: { - canPush: ['alice'], - canAuthorise: ['bob'], - }, -}; - describe('db', () => { beforeEach(() => { vi.clearAllMocks(); From d6664f1a6fb980b1c6732c55750af6637ce9ef9a Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 14 Dec 2025 12:19:28 +0900 Subject: [PATCH 21/31] chore: add missing types for auth files and plugin.ts --- src/plugin.ts | 9 ++--- src/service/passport/activeDirectory.ts | 27 +++++++++----- src/service/passport/jwtUtils.ts | 5 +-- src/service/passport/local.ts | 14 ++++---- src/service/passport/oidc.ts | 46 +++++++++++++----------- src/service/routes/auth.ts | 47 ++++++++++++++----------- src/service/routes/index.ts | 3 +- 7 files changed, 87 insertions(+), 64 deletions(-) diff --git a/src/plugin.ts b/src/plugin.ts index 01540d5c8..abc304316 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -1,6 +1,7 @@ import { Request } from 'express'; import { Action } from './proxy/actions'; +import Module from 'node:module'; const lpModule = import('load-plugin'); /* eslint-disable @typescript-eslint/no-unused-expressions */ @@ -69,7 +70,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); @@ -89,7 +90,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); @@ -112,9 +113,9 @@ class PluginLoader { /** * 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 { + private async _loadPluginModule(target: string): Promise { const lp = await lpModule; const resolvedModuleFile = await lp.resolvePlugin(target); return await lp.loadPlugin(resolvedModuleFile); diff --git a/src/service/passport/activeDirectory.ts b/src/service/passport/activeDirectory.ts index 6814bcacc..9941e0268 100644 --- a/src/service/passport/activeDirectory.ts +++ b/src/service/passport/activeDirectory.ts @@ -43,7 +43,7 @@ export const configure = async (passport: PassportStatic): Promise void, + done: (err: unknown, user: unknown) => void, ) { try { profile.username = profile._json.sAMAccountName?.toLowerCase(); @@ -63,8 +63,9 @@ 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 5fc3a1901..9705a861f 100644 --- a/src/service/passport/jwtUtils.ts +++ b/src/service/passport/jwtUtils.ts @@ -70,8 +70,9 @@ export async function validateJwt( } return { verifiedPayload, error: null }; - } catch (error: any) { - const errorMessage = `JWT validation failed: ${error.message}\n`; + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + const errorMessage = `JWT validation failed: ${msg}\n`; console.error(errorMessage); return { error: errorMessage, verifiedPayload: null }; } diff --git a/src/service/passport/local.ts b/src/service/passport/local.ts index 10324f772..d87baa8de 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'; @@ -11,28 +11,28 @@ export const configure = async (passport: PassportStatic): Promise void, + done: (err: unknown, user?: Partial, info?: IVerifyOptions) => void, ) => { try { const user = await db.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) { + } catch (err: unknown) { return done(err); } }, ), ); - passport.serializeUser((user: any, done) => { + passport.serializeUser((user: Partial, done) => { done(null, user.username); }); @@ -40,7 +40,7 @@ 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,16 @@ 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 +101,26 @@ export const handleUserAuthentication = async ( } return done(null, user); - } catch (err) { - return done(err); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + 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 f6347eb4f..888f8a9c6 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -104,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(401).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(401).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(401).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'); @@ -175,11 +178,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 (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); res .status(500) .send({ - message: `Error updating git account: ${e.message}`, + message: `Error updating git account: ${msg}`, }) .end(); } @@ -224,10 +228,11 @@ router.post('/create-user', async (req: Request, res: Response) => { message: 'User created successfully', username, }); - } catch (error: any) { - console.error('Error creating user:', error); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error creating user: ${msg}`); res.status(400).send({ - message: error.message || 'Failed to create user', + message: msg || 'Failed to create user', }); } }); diff --git a/src/service/routes/index.ts b/src/service/routes/index.ts index 23b63b02a..c3316f4ec 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); From 3cd816faabd312da7b7b2726227e8e111d670b29 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 14 Dec 2025 12:48:46 +0900 Subject: [PATCH 22/31] refactor: remove any types from remaining catch(error) --- src/proxy/actions/autoActions.ts | 10 ++++++---- .../processors/push-action/checkHiddenCommits.ts | 14 +++++++------- .../processors/push-action/checkIfWaitingAuth.ts | 5 +++-- src/proxy/processors/push-action/getDiff.ts | 5 +++-- src/proxy/processors/push-action/parsePush.ts | 11 +++++------ src/proxy/processors/push-action/preReceive.ts | 5 +++-- src/proxy/processors/push-action/pullRemote.ts | 5 +++-- src/proxy/processors/push-action/writePack.ts | 5 +++-- src/service/routes/repo.ts | 13 +++++++------ 9 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/proxy/actions/autoActions.ts b/src/proxy/actions/autoActions.ts index 4b8624ac0..245fee5aa 100644 --- a/src/proxy/actions/autoActions.ts +++ b/src/proxy/actions/autoActions.ts @@ -16,8 +16,9 @@ const attemptAutoApproval = async (action: Action) => { console.log('Push automatically approved by system.'); return true; - } catch (error: any) { - console.error('Error during auto-approval:', error.message); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Error during auto-approval:', msg); return false; } }; @@ -37,8 +38,9 @@ const attemptAutoRejection = async (action: Action) => { console.log('Push automatically rejected by system.'); return true; - } catch (error: any) { - console.error('Error during auto-rejection:', error.message); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Error during auto-rejection:', msg); return false; } }; diff --git a/src/proxy/processors/push-action/checkHiddenCommits.ts b/src/proxy/processors/push-action/checkHiddenCommits.ts index 062ba6ab9..320944bf0 100644 --- a/src/proxy/processors/push-action/checkHiddenCommits.ts +++ b/src/proxy/processors/push-action/checkHiddenCommits.ts @@ -1,8 +1,8 @@ -import { spawnSync } from 'child_process'; -import { Request } from 'express'; import path from 'path'; - import { Action, Step } from '../../actions'; +import { spawnSync } from 'child_process'; +import { EMPTY_COMMIT_HASH } from '../constants'; +import { Request } from 'express'; const exec = async (_req: Request, action: Action): Promise => { const step = new Step('checkHiddenCommits'); @@ -18,8 +18,7 @@ const exec = async (_req: Request, 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') @@ -70,8 +69,9 @@ const exec = async (_req: Request, 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); + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + step.setError(msg); throw e; } finally { action.addStep(step); diff --git a/src/proxy/processors/push-action/checkIfWaitingAuth.ts b/src/proxy/processors/push-action/checkIfWaitingAuth.ts index 9b9030d73..ca43cd42c 100644 --- a/src/proxy/processors/push-action/checkIfWaitingAuth.ts +++ b/src/proxy/processors/push-action/checkIfWaitingAuth.ts @@ -16,8 +16,9 @@ const exec = async (_req: Request, action: Action): Promise => { } } } - } catch (e: any) { - step.setError(e.toString('utf-8')); + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + step.setError(msg); throw e; } finally { action.addStep(step); diff --git a/src/proxy/processors/push-action/getDiff.ts b/src/proxy/processors/push-action/getDiff.ts index f6144d658..53e4739c9 100644 --- a/src/proxy/processors/push-action/getDiff.ts +++ b/src/proxy/processors/push-action/getDiff.ts @@ -34,8 +34,9 @@ const exec = async (_req: Request, 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 (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + step.setError(msg); } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 0baeda245..726c1b8d1 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -101,10 +101,9 @@ async function exec(req: Request, 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 (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + step.setError(`Unable to parse push. Please contact an administrator for support: ${msg}`); } finally { action.addStep(step); } @@ -478,8 +477,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(); diff --git a/src/proxy/processors/push-action/preReceive.ts b/src/proxy/processors/push-action/preReceive.ts index 10390af4e..c34e60c68 100644 --- a/src/proxy/processors/push-action/preReceive.ts +++ b/src/proxy/processors/push-action/preReceive.ts @@ -64,10 +64,11 @@ const exec = async ( action.addStep(step); } return action; - } catch (error: any) { + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(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 3811e1bf9..1776be007 100644 --- a/src/proxy/processors/push-action/pullRemote.ts +++ b/src/proxy/processors/push-action/pullRemote.ts @@ -47,8 +47,9 @@ const exec = async (req: Request, action: Action): Promise => { step.log(`Completed ${cmd}`); step.setContent(`Completed ${cmd}`); - } catch (e: any) { - step.setError(e.toString('utf-8')); + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + step.setError(msg); throw e; } finally { action.addStep(step); diff --git a/src/proxy/processors/push-action/writePack.ts b/src/proxy/processors/push-action/writePack.ts index caad58348..989b296d0 100644 --- a/src/proxy/processors/push-action/writePack.ts +++ b/src/proxy/processors/push-action/writePack.ts @@ -31,8 +31,9 @@ const exec = async (req: Request, action: Action) => { step.log(`new idx files: ${newIdxFiles}`); step.setContent(content); - } catch (e: any) { - step.setError(e.toString('utf-8')); + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + step.setError(msg); throw e; } finally { action.addStep(step); diff --git a/src/service/routes/repo.ts b/src/service/routes/repo.ts index 7001933f7..d7ae59854 100644 --- a/src/service/routes/repo.ts +++ b/src/service/routes/repo.ts @@ -5,11 +5,12 @@ import { getProxyURL } from '../urls'; import { getAllProxiedHosts } from '../../proxy/routes/helper'; import { RepoQuery } from '../../db/types'; import { isAdminUser } from './utils'; +import Proxy from '../../proxy'; // 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 +132,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' }); @@ -184,8 +185,8 @@ 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(); } } catch (e: unknown) { if (e instanceof Error) { From 3e56231595fe2223c650aef1f98d3c1fa236c1d5 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 14 Dec 2025 18:11:09 +0900 Subject: [PATCH 23/31] chore: remove general any/as in tests (req casting, etc.) --- test/checkHiddenCommit.test.ts | 20 ++++++++++++-------- test/testCheckUserPushPermission.test.ts | 14 ++++++++------ test/testConfig.test.ts | 2 +- test/testParseAction.test.ts | 17 +++++++++-------- test/testParsePush.test.ts | 21 ++++++++++++++------- test/testProxyRoute.test.ts | 2 +- test/testPush.test.ts | 12 ++++++------ 7 files changed, 51 insertions(+), 37 deletions(-) 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/testCheckUserPushPermission.test.ts b/test/testCheckUserPushPermission.test.ts index ca9a82c3c..832aaa986 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,7 +14,8 @@ const TEST_EMAIL_2 = 'push-perms-test-2@test.com'; const TEST_EMAIL_3 = 'push-perms-test-3@test.com'; describe('CheckUserPushPermissions...', () => { - let testRepo: Repo | null = null; + let testRepo: db.Repo | null = null; + const req = {} as Request; beforeAll(async () => { testRepo = await db.createRepo({ @@ -23,12 +25,12 @@ describe('CheckUserPushPermissions...', () => { }); await db.createUser(TEST_USERNAME_1, 'abc', TEST_EMAIL_1, TEST_USERNAME_1, false); - await db.addUserCanPush(testRepo._id, TEST_USERNAME_1); + await db.addUserCanPush(testRepo._id!, TEST_USERNAME_1); await db.createUser(TEST_USERNAME_2, 'abc', TEST_EMAIL_2, TEST_USERNAME_2, false); }); 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 a8ae2bbd5..203c81cc5 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/testParseAction.test.ts b/test/testParseAction.test.ts index dcb9d9b91..2631d3b9b 100644 --- a/test/testParseAction.test.ts +++ b/test/testParseAction.test.ts @@ -1,6 +1,7 @@ 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: db.Repo | null = null; @@ -27,11 +28,11 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a pull request into an action', async () => { - const req: any = { + const req = { 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); @@ -41,11 +42,11 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a pull request with a legacy path into an action', async () => { - const req: any = { + const req = { 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); @@ -55,11 +56,11 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a push request into an action', async () => { - const req: any = { + const req = { 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); @@ -69,11 +70,11 @@ describe('Pre-processor: parseAction', () => { }); it('should be able to parse a push request with a legacy path into an action', async () => { - const req: any = { + const req = { 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 41247bf9b..c5d55b012 100644 --- a/test/testParsePush.test.ts +++ b/test/testParsePush.test.ts @@ -13,6 +13,9 @@ import { } 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. @@ -160,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'); @@ -225,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; @@ -301,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 @@ -312,10 +319,10 @@ describe('parsePackFile', () => { commitData: [], user: null, steps: [], - addStep: vi.fn(function (this: any, step: any) { + 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; }), @@ -323,7 +330,7 @@ describe('parsePackFile', () => { req = { body: null, - }; + } as Request; }); afterEach(() => { diff --git a/test/testProxyRoute.test.ts b/test/testProxyRoute.test.ts index 144fd4982..65278ea99 100644 --- a/test/testProxyRoute.test.ts +++ b/test/testProxyRoute.test.ts @@ -360,7 +360,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 a7fa51fe2..2bdfdb33a 100644 --- a/test/testPush.test.ts +++ b/test/testPush.test.ts @@ -1,4 +1,4 @@ -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'; @@ -42,8 +42,8 @@ describe('Push API', () => { let cookie: string | null = null; 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]; @@ -176,7 +176,7 @@ describe('Push API', () => { it('should NOT allow an authorizer to approve if committer is unknown', async () => { // make the approver also the committer - const testPush = Object.assign({}, TEST_PUSH) as Action; + const testPush = Object.assign({}, TEST_PUSH); testPush.user = TEST_USERNAME_3; testPush.userEmail = TEST_EMAIL_3; await db.writeAudit(testPush); @@ -205,7 +205,7 @@ describe('Push API', () => { it('should NOT allow an authorizer to approve their own push', async () => { // make the approver also the committer - const testPush = Object.assign({}, TEST_PUSH) as Action; + const testPush = Object.assign({}, TEST_PUSH); testPush.user = TEST_USERNAME_1; testPush.userEmail = TEST_EMAIL_1; await db.writeAudit(testPush); @@ -266,7 +266,7 @@ describe('Push API', () => { it('should NOT allow an authorizer to reject their own push', async () => { // make the approver also the committer - const testPush = Object.assign({}, TEST_PUSH) as Action; + const testPush = Object.assign({}, TEST_PUSH); testPush.user = TEST_USERNAME_1; testPush.userEmail = TEST_EMAIL_1; await db.writeAudit(testPush); From 1863ff892e0aa4dfc3c5c68f4065b18934b40b30 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 14 Dec 2025 18:12:01 +0900 Subject: [PATCH 24/31] chore: add missing types for auth-related tests (oidc, activeDirectory, jwtAuthHandler) --- test/testActiveDirectoryAuth.test.ts | 30 ++++++++++--------- test/testJwtAuthHandler.test.ts | 44 +++++++++++----------------- test/testOidc.test.ts | 21 ++++++------- 3 files changed, 42 insertions(+), 53 deletions(-) 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/testJwtAuthHandler.test.ts b/test/testJwtAuthHandler.test.ts index 61b625b72..74c43f1c2 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 { describe, it, expect, vi, beforeEach, afterEach, Mock, MockInstance } from 'vitest'; import axios from 'axios'; -import jwt from 'jsonwebtoken'; +import jwt, { JwtPayload } from 'jsonwebtoken'; import * as jwkToBufferModule from 'jwk-to-pem'; import { assignRoles, getJwks, validateJwt } from '../src/service/passport/jwtUtils'; import { jwtAuthHandler } from '../src/service/passport/jwtAuthHandler'; +import { JwtConfig } from '../src/config/generated/config'; +import { NextFunction } from 'express'; describe('getJwks', () => { afterEach(() => vi.restoreAllMocks()); @@ -27,16 +29,17 @@ describe('getJwks', () => { }); describe('validateJwt', () => { - let decodeStub: ReturnType; - let verifyStub: ReturnType; - let pemStub: ReturnType; + let decodeStub: MockInstance; + let verifyStub: MockInstance; let getJwksStub: ReturnType; beforeEach(() => { const jwksResponse = { keys: [{ kid: 'test-key', kty: 'RSA', n: 'abc', e: 'AQAB' }] }; - vi.mock('jwk-to-pem', () => { + vi.mock('jwk-to-pem', async (importOriginal) => { + const actual = await importOriginal(); return { + ...actual, default: vi.fn().mockReturnValue('fake-public-key'), }; }); @@ -46,22 +49,17 @@ describe('validateJwt', () => { .mockResolvedValueOnce({ data: jwksResponse }); getJwksStub = vi.fn().mockResolvedValue(jwksResponse.keys); - decodeStub = vi.spyOn(jwt, 'decode') as any; + decodeStub = vi.spyOn(jwt, 'decode'); verifyStub = vi.spyOn(jwt, 'verify') as any; - pemStub = vi.fn().mockReturnValue('fake-public-key'); - - (jwkToBufferModule.default as Mock).mockImplementation(pemStub); }); afterEach(() => vi.restoreAllMocks()); it('should validate a correct JWT', async () => { const mockJwk = { kid: '123', kty: 'RSA', n: 'abc', e: 'AQAB' }; - const mockPem = 'fake-public-key'; decodeStub.mockReturnValue({ header: { kid: '123' } }); getJwksStub.mockResolvedValue([mockJwk]); - pemStub.mockReturnValue(mockPem); verifyStub.mockReturnValue({ azp: 'client-id', sub: 'user123' }); const { verifiedPayload } = await validateJwt( @@ -124,7 +122,7 @@ describe('assignRoles', () => { const user = { username: 'no-role-user', admin: undefined }; const payload = { admin: 'admin' }; - assignRoles(null as any, payload, user); + assignRoles(undefined, payload, user); expect(user.admin).toBeUndefined(); }); }); @@ -132,9 +130,8 @@ describe('assignRoles', () => { describe('jwtAuthHandler', () => { let req: any; let res: any; - let next: any; - let jwtConfig: any; - let validVerifyResponse: any; + let next: NextFunction; + let jwtConfig: JwtConfig; beforeEach(() => { req = { header: vi.fn(), isAuthenticated: vi.fn(), user: {} }; @@ -147,13 +144,6 @@ describe('jwtAuthHandler', () => { expectedAudience: 'expected-audience', roleMapping: { admin: { admin: 'admin' } }, }; - - validVerifyResponse = { - header: { kid: '123' }, - azp: 'client-id', - sub: 'user123', - admin: 'admin', - }; }); afterEach(() => vi.restoreAllMocks()); @@ -174,8 +164,8 @@ describe('jwtAuthHandler', () => { it('should return 500 if authorityURL not configured', async () => { req.header.mockReturnValue('Bearer fake-token'); - jwtConfig.authorityURL = null; - vi.spyOn(jwt, 'verify').mockReturnValue(validVerifyResponse); + jwtConfig.authorityURL = ''; + vi.spyOn(jwt, 'verify').mockReturnValue(); await jwtAuthHandler(jwtConfig)(req, res, next); @@ -185,8 +175,8 @@ describe('jwtAuthHandler', () => { it('should return 500 if clientID not configured', async () => { req.header.mockReturnValue('Bearer fake-token'); - jwtConfig.clientID = null; - vi.spyOn(jwt, 'verify').mockReturnValue(validVerifyResponse); + jwtConfig.clientID = ''; + vi.spyOn(jwt, 'verify').mockReturnValue(); await jwtAuthHandler(jwtConfig)(req, res, next); 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(); }); From ce58dcc9aead8cda85213bc13af6361879b1a6a7 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 14 Dec 2025 18:14:54 +0900 Subject: [PATCH 25/31] refactor: db & checkCommitMessages tests to use Action instead of push objects --- test/extractRawBody.test.ts | 17 +-- test/processors/checkCommitMessages.test.ts | 138 ++++++++------------ test/testDb.test.ts | 104 +++++++-------- 3 files changed, 107 insertions(+), 152 deletions(-) diff --git a/test/extractRawBody.test.ts b/test/extractRawBody.test.ts index 7c1cf134a..c7b92c94e 100644 --- a/test/extractRawBody.test.ts +++ b/test/extractRawBody.test.ts @@ -1,5 +1,7 @@ -import { describe, it, beforeEach, expect, vi, Mock, afterAll } from 'vitest'; +import { Request } from 'express'; +import rawBody from 'raw-body'; import { PassThrough } from 'stream'; +import { describe, it, beforeEach, expect, vi, Mock, afterAll } 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', () => { @@ -63,20 +64,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/processors/checkCommitMessages.test.ts b/test/processors/checkCommitMessages.test.ts index f5945bd66..c5f5f673f 100644 --- a/test/processors/checkCommitMessages.test.ts +++ b/test/processors/checkCommitMessages.test.ts @@ -14,6 +14,8 @@ vi.mock('../../src/config', async (importOriginal) => { }); describe('checkCommitMessages', () => { + let action: Action; + let req: Request; let consoleLogSpy: ReturnType; let mockCommitConfig: any; @@ -32,6 +34,9 @@ describe('checkCommitMessages', () => { }; vi.mocked(configModule.getCommitConfig).mockReturnValue(mockCommitConfig); + + action = new Action('test', 'test', 'test', 1, 'test'); + req = {} as Request; }); afterEach(() => { @@ -41,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 = [{ ...SAMPLE_COMMIT, message: '' }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: null as any }]; + action.commitData = [{ ...SAMPLE_COMMIT, message: null as unknown as string }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: undefined as any }]; + action.commitData = [{ ...SAMPLE_COMMIT, message: undefined as unknown as string }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: 123 as any }]; + action.commitData = [{ ...SAMPLE_COMMIT, message: 123 as unknown as string }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -81,19 +82,19 @@ describe('checkCommitMessages', () => { }); it('should block object commit messages', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); - action.commitData = [{ ...SAMPLE_COMMIT, message: { text: 'fix: bug' } as any }]; + action.commitData = [ + { ...SAMPLE_COMMIT, message: { text: 'fix: bug' } as unknown as string }, + ]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: ['fix: bug'] as any }]; + action.commitData = [{ ...SAMPLE_COMMIT, message: ['fix: bug'] as unknown as string }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -101,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 = [{ ...SAMPLE_COMMIT, message: 'Add password to config' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -113,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 = [ { ...SAMPLE_COMMIT, message: 'Add PASSWORD to config' }, { ...SAMPLE_COMMIT, message: 'Store Secret key' }, { ...SAMPLE_COMMIT, message: 'Update TOKEN value' }, ]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: 'Update mypassword123' }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: 'Add password and secret token' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -146,32 +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 = [{ ...SAMPLE_COMMIT, message: 'See http://example.com for details' }]; - const result = await exec({} as Request, 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 = [ { ...SAMPLE_COMMIT, message: 'Update docs at https://docs.example.com' }, ]; - const result = await exec({} as Request, 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 = [ { ...SAMPLE_COMMIT, message: 'See http://example.com and https://other.com' }, ]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -180,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 = [{ ...SAMPLE_COMMIT, message: 'SSN: 123-45-6789' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -192,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 = [{ ...SAMPLE_COMMIT, message: 'This is private information' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -203,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 = [{ ...SAMPLE_COMMIT, message: 'password at http://example.com' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -215,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 = [{ ...SAMPLE_COMMIT, message: 'Add secret key' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -227,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 = [{ ...SAMPLE_COMMIT, message: 'Visit http://example.com' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -238,12 +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 = [ { ...SAMPLE_COMMIT, message: 'fix: resolve bug in user authentication' }, ]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); expect(consoleLogSpy).toHaveBeenCalledWith( @@ -252,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 = [ { ...SAMPLE_COMMIT, message: 'feat: add new feature' }, { ...SAMPLE_COMMIT, message: 'chore: update dependencies' }, { ...SAMPLE_COMMIT, message: 'docs: improve documentation' }, ]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -269,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 = [{ ...SAMPLE_COMMIT, message: 'Any message should pass' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -280,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 = [ { ...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({} as Request, 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 = [ { ...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({} as Request, 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 = [ { ...SAMPLE_COMMIT, message: 'Add password' }, { ...SAMPLE_COMMIT, message: 'Store secret' }, { ...SAMPLE_COMMIT, message: 'feat: valid message' }, ]; - const result = await exec({} as Request, 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 = [ { ...SAMPLE_COMMIT, message: 'fix: bug' }, { ...SAMPLE_COMMIT, message: 'fix: bug' }, ]; - const result = await exec({} as Request, 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 = [ { ...SAMPLE_COMMIT, message: 'fix: bug' }, { ...SAMPLE_COMMIT, message: 'Add password' }, { ...SAMPLE_COMMIT, message: 'fix: bug' }, ]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(true); }); @@ -346,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 = [{ ...SAMPLE_COMMIT, message: 'Add password' }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: 'Add password' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); const step = result.steps[0]; // first log is the "push blocked" message @@ -368,10 +347,9 @@ describe('checkCommitMessages', () => { }); it('should set detailed error message', async () => { - const action = new Action('test', 'test', 'test', 1, 'test'); action.commitData = [{ ...SAMPLE_COMMIT, message: 'Add secret' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); const step = result.steps[0]; expect(step.errorMessage).toContain('Your push has been blocked'); @@ -379,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 = [ { ...SAMPLE_COMMIT, message: 'Add password' }, { ...SAMPLE_COMMIT, message: 'Store token' }, ]; - const result = await exec({} as Request, action); + const result = await exec(req, action); const step = result.steps[0]; expect(step.errorMessage).toContain('Add password'); @@ -395,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({} as Request, 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({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: ' ' }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: longMessage }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -436,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 = [{ ...SAMPLE_COMMIT, message: 'Contains $pecial characters' }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: 'feat: 添加新功能 🎉' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result.steps[0].error).toBe(false); }); @@ -457,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 = [{ ...SAMPLE_COMMIT, message: 'Any message' }]; // test that it doesn't crash - expect(() => exec({} as Request, action)).not.toThrow(); + expect(() => exec(req, action)).not.toThrow(); }); }); @@ -473,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 = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; const initialStepCount = action.steps.length; - const result = await exec({} as Request, 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 = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; - const result = await exec({} as Request, action); + const result = await exec(req, action); expect(result).toBe(action); }); @@ -503,7 +470,6 @@ 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 = [{ ...SAMPLE_COMMIT, message: 'fix: bug' }]; const mockRequest = { headers: {}, body: {} }; diff --git a/test/testDb.test.ts b/test/testDb.test.ts index 9f7d6a508..486bda8d3 100644 --- a/test/testDb.test.ts +++ b/test/testDb.test.ts @@ -4,6 +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 { SAMPLE_REPO } from '../src/proxy/processors/constants'; const TEST_REPO = { project: 'finos', @@ -26,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: '0000000000000000000000000000000000000000__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/0000000000000000000000000000000000000000__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', @@ -62,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, @@ -203,7 +189,7 @@ describe('Database clients', () => { it('should be able to create a repo', async () => { 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); }); @@ -211,12 +197,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(); @@ -261,16 +245,21 @@ describe('Database clients', () => { }); it('should be able to create a repo with a blank project', async () => { - const variations = [ - { project: null, name: TEST_REPO.name, url: TEST_REPO.url }, // null value - { project: '', name: TEST_REPO.name, url: TEST_REPO.url }, // empty string - { name: TEST_REPO.name, url: TEST_REPO.url }, // project undefined + const variations: AuthorisedRepo[] = [ + { + ...SAMPLE_REPO, + project: null as unknown as string, + name: TEST_REPO.name, + url: TEST_REPO.url, + }, // null value + { ...SAMPLE_REPO, project: '', name: TEST_REPO.name, url: TEST_REPO.url }, // empty string + { ...SAMPLE_REPO, name: TEST_REPO.name, url: TEST_REPO.url }, // project undefined ]; for (const testRepo of variations) { let threwError = false; try { - const repo = await db.createRepo(testRepo as AuthorisedRepo); + const repo = await db.createRepo(testRepo); await db.deleteRepo(repo._id); } catch { threwError = true; @@ -298,7 +287,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, @@ -316,7 +305,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, ), @@ -384,7 +373,6 @@ 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() }); @@ -395,7 +383,7 @@ describe('Database clients', () => { it('should be able to delete a user', async () => { await db.deleteUser(TEST_USER.username); const users = await db.getUsers(); - const cleanUsers = cleanResponseData(TEST_USER, users); + const cleanUsers = cleanResponseData(TEST_USER, users as any); expect(cleanUsers).not.toContainEqual(TEST_USER); }); @@ -523,43 +511,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 () => { 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'); await db.deletePush(TEST_PUSH.id); }); 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'); await db.deletePush(TEST_PUSH.id); }); 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'); await db.deletePush(TEST_PUSH.id); @@ -580,7 +568,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); @@ -603,7 +591,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); @@ -634,7 +622,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); From 3824d0968a36fdc0bf9e88805242001f08e359d0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 14 Dec 2025 18:15:36 +0900 Subject: [PATCH 26/31] refactor: remaining any/as removal in tests, explicit unknown casting for edge case tests --- test/processors/checkAuthorEmails.test.ts | 2 +- test/processors/scanDiff.emptyDiff.test.ts | 2 +- test/processors/scanDiff.test.ts | 2 +- test/proxy.test.ts | 7 ++++--- test/testProxy.test.ts | 8 ++++---- test/ui/apiBase.test.ts | 2 +- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/test/processors/checkAuthorEmails.test.ts b/test/processors/checkAuthorEmails.test.ts index b78eeacd7..e00ea8619 100644 --- a/test/processors/checkAuthorEmails.test.ts +++ b/test/processors/checkAuthorEmails.test.ts @@ -89,7 +89,7 @@ describe('checkAuthorEmails', () => { it('should reject null/undefined email', async () => { vi.mocked(validator.isEmail).mockReturnValue(false); - mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: null as any }]; + mockAction.commitData = [{ ...SAMPLE_COMMIT, authorEmail: null as unknown as string }]; const result = await exec(mockReq, mockAction); diff --git a/test/processors/scanDiff.emptyDiff.test.ts b/test/processors/scanDiff.emptyDiff.test.ts index 7f0f26f0e..39ca456c4 100644 --- a/test/processors/scanDiff.emptyDiff.test.ts +++ b/test/processors/scanDiff.emptyDiff.test.ts @@ -79,7 +79,7 @@ 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({} as Request, action); diff --git a/test/processors/scanDiff.test.ts b/test/processors/scanDiff.test.ts index 2d009431a..2e475abc8 100644 --- a/test/processors/scanDiff.test.ts +++ b/test/processors/scanDiff.test.ts @@ -258,7 +258,7 @@ 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({} as Request, action); diff --git a/test/proxy.test.ts b/test/proxy.test.ts index fbbcb9875..928fe69e4 100644 --- a/test/proxy.test.ts +++ b/test/proxy.test.ts @@ -1,7 +1,8 @@ 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'; /* jescalada: these tests are currently causing the following error @@ -17,7 +18,7 @@ import { GitProxyConfig } from '../src/config/generated/config'; https://github.com/finos/git-proxy/issues/1294 */ describe.skip('Proxy Module TLS Certificate Loading', () => { - let proxyModule: any; + let proxyModule: Proxy; let mockConfig: any; let mockHttpServer: any; let mockHttpsServer: any; @@ -91,7 +92,7 @@ describe.skip('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, diff --git a/test/testProxy.test.ts b/test/testProxy.test.ts index 05a29a0b2..c4eefb667 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/ui/apiBase.test.ts b/test/ui/apiBase.test.ts index da34dbc30..0cc3e0a22 100644 --- a/test/ui/apiBase.test.ts +++ b/test/ui/apiBase.test.ts @@ -11,7 +11,7 @@ describe('apiBase', () => { const originalLocation = globalThis.location; beforeAll(() => { - globalThis.location = { origin: 'https://lovely-git-proxy.com' } as any; + globalThis.location = { origin: 'https://lovely-git-proxy.com' } as Location; }); afterAll(() => { From a1455ca154f1c6f82bf5114242eb1a1986bda3b4 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 15 Dec 2025 19:27:01 +0900 Subject: [PATCH 27/31] chore: add unknown type to error catch clauses, improve error visibility --- src/config/ConfigLoader.ts | 4 ++-- src/config/index.ts | 15 +++++++++------ src/db/file/pushes.ts | 5 +++-- src/db/file/repo.ts | 5 +++-- src/db/file/users.ts | 10 ++++++---- src/proxy/chain.ts | 5 +++-- .../push-action/checkCommitMessages.ts | 5 +++-- .../processors/push-action/checkEmptyBranch.ts | 5 +++-- .../processors/push-action/checkHiddenCommits.ts | 6 +++--- .../processors/push-action/checkIfWaitingAuth.ts | 6 +++--- src/proxy/processors/push-action/getDiff.ts | 5 +++-- src/proxy/processors/push-action/gitleaks.ts | 12 +++++++----- src/proxy/processors/push-action/parsePush.ts | 11 ++++++----- src/proxy/processors/push-action/pullRemote.ts | 6 +++--- src/proxy/processors/push-action/writePack.ts | 6 +++--- src/proxy/routes/index.ts | 16 +++++++++++----- src/service/passport/activeDirectory.ts | 16 ++++++++-------- src/service/passport/jwtUtils.ts | 5 +++-- src/service/passport/local.ts | 8 ++++---- src/service/passport/oidc.ts | 4 ++-- src/service/routes/auth.ts | 9 +++++---- src/service/routes/repo.ts | 9 ++++----- src/ui/auth/AuthProvider.tsx | 4 +++- .../components/Navbars/DashboardNavbarLinks.tsx | 5 +++-- src/ui/services/auth.ts | 5 +++-- src/ui/services/user.ts | 12 ++++++++---- src/ui/utils.tsx | 5 +++-- src/ui/views/RepoDetails/Components/AddUser.tsx | 9 +++------ .../views/RepoList/Components/RepoOverview.tsx | 7 ++----- test/proxy.test.ts | 5 +++-- 30 files changed, 125 insertions(+), 100 deletions(-) diff --git a/src/config/ConfigLoader.ts b/src/config/ConfigLoader.ts index 6c16ebd8b..bd39d2747 100644 --- a/src/config/ConfigLoader.ts +++ b/src/config/ConfigLoader.ts @@ -79,8 +79,8 @@ export class ConfigLoader extends EventEmitter { fs.mkdirSync(this.cacheDir, { recursive: true }); console.log(`Created cache directory at ${this.cacheDir}`); return true; - } catch (err: unknown) { - const msg = err instanceof Error ? err.message : String(err); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); console.error('Failed to create cache directory:', msg); return false; } diff --git a/src/config/index.ts b/src/config/index.ts index b58901a27..97af243b6 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -312,14 +312,16 @@ 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Failed to apply new configuration:', msg); // 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) { + const msg = startError instanceof Error ? startError.message : String(startError); + console.error('Failed to restart services:', msg); } } }; @@ -356,7 +358,8 @@ try { loadFullConfiguration(); initializeConfigLoader(); console.log('Configuration loaded successfully'); -} catch (error) { - console.error('Failed to load configuration:', error); +} catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Failed to load configuration:', msg); throw error; } diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts index 40a318245..e30a0faea 100644 --- a/src/db/file/pushes.ts +++ b/src/db/file/pushes.ts @@ -17,10 +17,11 @@ if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db'); const db = new Datastore({ filename: './.data/db/pushes.db', autoload: true }); try { db.ensureIndex({ fieldName: 'id', unique: true }); -} catch (e) { +} catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); console.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, + msg, ); } db.setAutocompactionInterval(COMPACTION_INTERVAL); diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts index 79027c490..f7dee8b74 100644 --- a/src/db/file/repo.ts +++ b/src/db/file/repo.ts @@ -18,10 +18,11 @@ export const db = new Datastore({ filename: './.data/db/repos.db', autoload: tru try { db.ensureIndex({ fieldName: 'url', unique: true }); -} catch (e) { +} catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); console.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, + msg, ); } diff --git a/src/db/file/users.ts b/src/db/file/users.ts index 7bab7c1b1..cc882ed1b 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -16,18 +16,20 @@ const db = new Datastore({ filename: './.data/db/users.db', autoload: true }); // Using a unique constraint with the index try { db.ensureIndex({ fieldName: 'username', unique: true }); -} catch (e) { +} catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); console.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, + msg, ); } try { db.ensureIndex({ fieldName: 'email', unique: true }); -} catch (e) { +} catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); console.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, + msg, ); } db.setAutocompactionInterval(COMPACTION_INTERVAL); diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index c73b3bc66..1ba0e6a80 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -48,9 +48,10 @@ export const executeChain = async (req: Request, _res: Response): Promise { console.log('Commit message is blocked via configured literals/patterns...'); return false; } - } catch (error) { - console.log('Invalid regex pattern...'); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.log(`Invalid regex pattern... ${msg}`); return false; } diff --git a/src/proxy/processors/push-action/checkEmptyBranch.ts b/src/proxy/processors/push-action/checkEmptyBranch.ts index 7b0fd7778..c92c4a4fc 100644 --- a/src/proxy/processors/push-action/checkEmptyBranch.ts +++ b/src/proxy/processors/push-action/checkEmptyBranch.ts @@ -11,8 +11,9 @@ const isEmptyBranch = async (action: Action): Promise => { 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) { + const msg = error instanceof Error ? error.message : String(error); + console.log(`Commit ${action.commitTo} not found: ${msg}`); } } diff --git a/src/proxy/processors/push-action/checkHiddenCommits.ts b/src/proxy/processors/push-action/checkHiddenCommits.ts index 320944bf0..5a9c58a74 100644 --- a/src/proxy/processors/push-action/checkHiddenCommits.ts +++ b/src/proxy/processors/push-action/checkHiddenCommits.ts @@ -69,10 +69,10 @@ const exec = async (_req: Request, 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: unknown) { - const msg = e instanceof Error ? e.message : String(e); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); step.setError(msg); - throw e; + 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 ca43cd42c..8b6b17509 100644 --- a/src/proxy/processors/push-action/checkIfWaitingAuth.ts +++ b/src/proxy/processors/push-action/checkIfWaitingAuth.ts @@ -16,10 +16,10 @@ const exec = async (_req: Request, action: Action): Promise => { } } } - } catch (e: unknown) { - const msg = e instanceof Error ? e.message : String(e); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); step.setError(msg); - throw e; + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/getDiff.ts b/src/proxy/processors/push-action/getDiff.ts index 53e4739c9..5308fd807 100644 --- a/src/proxy/processors/push-action/getDiff.ts +++ b/src/proxy/processors/push-action/getDiff.ts @@ -34,9 +34,10 @@ const exec = async (_req: Request, action: Action): Promise => { const diff = await git.diff([revisionRange]); step.log(diff); step.setContent(diff); - } catch (e: unknown) { - const msg = e instanceof Error ? e.message : String(e); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); step.setError(msg); + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/gitleaks.ts b/src/proxy/processors/push-action/gitleaks.ts index aa0e27860..1c6f67425 100644 --- a/src/proxy/processors/push-action/gitleaks.ts +++ b/src/proxy/processors/push-action/gitleaks.ts @@ -67,7 +67,7 @@ async function fileIsReadable(path: PathLike): Promise { } await fs.access(path, fs.constants.R_OK); return true; - } catch (e) { + } catch (error: unknown) { return false; } } @@ -116,8 +116,9 @@ const exec = async (_req: Request, action: Action): Promise => { 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) { + const msg = error instanceof Error ? error.message : String(error); + console.error('failed to get gitleaks config, please fix the error:', msg); action.error = true; step.setError('failed setup gitleaks, please contact an administrator\n'); action.addStep(step); @@ -175,9 +176,10 @@ const exec = async (_req: Request, action: Action): Promise => { console.log('succeeded'); console.log(gitleaks.stderr); } - } catch (e) { + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(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 726c1b8d1..a97b90606 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -101,8 +101,8 @@ async function exec(req: Request, action: Action): Promise { step.content = { meta: meta, }; - } catch (e: unknown) { - const msg = e instanceof Error ? e.message : String(e); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); step.setError(`Unable to parse push. Please contact an administrator for support: ${msg}`); } finally { action.addStep(step); @@ -504,9 +504,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 instanceof Error ? error.message : String(error); + console.warn(`Error during decompression: ${msg}`); + throw new Error(`Error during decompression: ${msg}`); } } const result = { diff --git a/src/proxy/processors/push-action/pullRemote.ts b/src/proxy/processors/push-action/pullRemote.ts index 1776be007..92ac1964b 100644 --- a/src/proxy/processors/push-action/pullRemote.ts +++ b/src/proxy/processors/push-action/pullRemote.ts @@ -47,10 +47,10 @@ const exec = async (req: Request, action: Action): Promise => { step.log(`Completed ${cmd}`); step.setContent(`Completed ${cmd}`); - } catch (e: unknown) { - const msg = e instanceof Error ? e.message : String(e); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); step.setError(msg); - throw e; + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/processors/push-action/writePack.ts b/src/proxy/processors/push-action/writePack.ts index 989b296d0..35c82ebdf 100644 --- a/src/proxy/processors/push-action/writePack.ts +++ b/src/proxy/processors/push-action/writePack.ts @@ -31,10 +31,10 @@ const exec = async (req: Request, action: Action) => { step.log(`new idx files: ${newIdxFiles}`); step.setContent(content); - } catch (e: unknown) { - const msg = e instanceof Error ? e.message : String(e); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); step.setError(msg); - throw e; + throw error; } finally { action.addStep(step); } diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index f181bf068..595996cc3 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -67,8 +67,9 @@ 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 msg = error instanceof Error ? error.message : String(error); + const message = `Error occurred in proxy filter function ${msg}`; logAction(req.url, req.headers.host, req.headers['user-agent'], ActionType.ERROR, message); sendErrorResponse(req, res, message); @@ -164,9 +165,14 @@ const extractRawBody = async (req: Request, res: Response, next: NextFunction) = 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 9941e0268..30a814ea0 100644 --- a/src/service/passport/activeDirectory.ts +++ b/src/service/passport/activeDirectory.ts @@ -63,8 +63,8 @@ export const configure = async (passport: PassportStatic): Promise { const { data: jwks }: { data: JwksResponse } = await axios.get(jwksUri); return jwks.keys; - } catch (error) { - console.error('Error fetching JWKS:', error); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Error fetching JWKS:', msg); throw new Error('Failed to fetch JWKS'); } } diff --git a/src/service/passport/local.ts b/src/service/passport/local.ts index d87baa8de..00dd63984 100644 --- a/src/service/passport/local.ts +++ b/src/service/passport/local.ts @@ -25,8 +25,8 @@ export const configure = async (passport: PassportStatic): Promise async (req: Request, res: Response) => { message: 'success', user: currentUser, }); - } catch (e) { - console.log(`service.routes.auth.login: Error logging user in ${JSON.stringify(e)}`); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + console.log(`service.routes.auth.login: Error logging user in ${msg}`); res.status(500).send('Failed to login').end(); } }; @@ -178,8 +179,8 @@ router.post('/gitAccount', async (req: Request, res: Response) => { user.gitAccount = req.body.gitAccount; db.updateUser(user); res.status(200).end(); - } catch (e: unknown) { - const msg = e instanceof Error ? e.message : String(e); + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); res .status(500) .send({ diff --git a/src/service/routes/repo.ts b/src/service/routes/repo.ts index d7ae59854..63132a22d 100644 --- a/src/service/routes/repo.ts +++ b/src/service/routes/repo.ts @@ -188,11 +188,10 @@ const repo = (proxy: Proxy) => { await theProxy?.stop(); await theProxy?.start(); } - } catch (e: unknown) { - if (e instanceof Error) { - console.error('Repository creation failed due to error: ', e.message); - console.error(e.stack); - res.status(500).send({ message: 'Failed to create repository due to error' }); + } catch (error: unknown) { + if (error instanceof Error) { + console.error('Repository creation failed due to error: ', error.message); + console.error(error.stack); } res.status(500).send({ message: 'Failed to create repository due to error' }); } 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 2ed5c3d8f..313f754aa 100644 --- a/src/ui/components/Navbars/DashboardNavbarLinks.tsx +++ b/src/ui/components/Navbars/DashboardNavbarLinks.tsx @@ -59,8 +59,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/services/auth.ts b/src/ui/services/auth.ts index 08e674efa..46eb7e4f6 100644 --- a/src/ui/services/auth.ts +++ b/src/ui/services/auth.ts @@ -22,8 +22,9 @@ export const getUserInfo = async (): Promise => { }); 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; } }; diff --git a/src/ui/services/user.ts b/src/ui/services/user.ts index caf0dd981..692e5fd67 100644 --- a/src/ui/services/user.ts +++ b/src/ui/services/user.ts @@ -24,11 +24,13 @@ const getUser = async ( setUser?.(user); setIsLoading?.(false); - } catch (error) { + } catch (error: unknown) { const axiosError = error as AxiosError; if (axiosError.response?.status === 401) { setAuth?.(false); } else { + const msg = error instanceof Error ? error.message : String(error); + console.error(`Error fetching user: ${msg}`); setIsError?.(true); } setIsLoading?.(false); @@ -49,7 +51,7 @@ const getUsers = async ( getAxiosConfig(), ); setUsers(response.data); - } catch (error) { + } catch (error: unknown) { if (axios.isAxiosError(error)) { if (error.response?.status === 401) { setAuth(false); @@ -59,7 +61,8 @@ const getUsers = async ( setErrorMessage(`Error fetching users: ${msg}`); } } else { - setErrorMessage(`Error fetching users: ${(error as Error).message ?? 'Unknown error'}`); + const msg = error instanceof Error ? error.message : String(error); + setErrorMessage(`Error fetching users: ${msg}`); } } finally { setIsLoading(false); @@ -74,7 +77,8 @@ const updateUser = async (user: PublicUser): Promise => { if (axios.isAxiosError(error)) { console.log(error.response?.data?.message); } else { - console.log(`Error updating user: ${error}`); + const msg = error instanceof Error ? error.message : String(error); + console.log(`Error updating user: ${msg}`); } throw error; } diff --git a/src/ui/utils.tsx b/src/ui/utils.tsx index 6a8abfc17..38ce0418a 100644 --- a/src/ui/utils.tsx +++ b/src/ui/utils.tsx @@ -210,8 +210,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 = error instanceof Error ? error.message : String(error); + console.warn('Could not fetch language data:', msg); } return { 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/RepoList/Components/RepoOverview.tsx b/src/ui/views/RepoList/Components/RepoOverview.tsx index 9cc20ab72..688c04943 100644 --- a/src/ui/views/RepoList/Components/RepoOverview.tsx +++ b/src/ui/views/RepoList/Components/RepoOverview.tsx @@ -31,12 +31,9 @@ const Repositories: React.FC = (props) => { await fetchRemoteRepositoryData(props.repo.project, props.repo.name, remoteUrl), ); } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); const errorMessage = `Unable to fetch repository data for ${props.repo.project}/${props.repo.name} from '${remoteUrl}' - this may occur if the project is private or from an SCM vendor that is not supported.`; - if (error instanceof Error) { - console.warn(errorMessage, error.message); - } else { - console.warn(errorMessage); - } + console.warn(errorMessage, msg); } }; diff --git a/test/proxy.test.ts b/test/proxy.test.ts index 928fe69e4..313182b6a 100644 --- a/test/proxy.test.ts +++ b/test/proxy.test.ts @@ -114,8 +114,9 @@ describe.skip('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) { + const msg = error instanceof Error ? error.message : String(error); + console.error('Error occurred when stopping the proxy: ', msg); } vi.restoreAllMocks(); }); From 62a7d84e792f8867db7b1958688a36ba01fe60a0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 17 Dec 2025 10:53:26 +0900 Subject: [PATCH 28/31] chore: fix failing test after error refactors --- src/proxy/index.ts | 2 +- test/1.test.ts | 2 +- test/processors/gitLeaks.test.ts | 4 ++-- test/testLogin.test.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/proxy/index.ts b/src/proxy/index.ts index a50f7531f..b3e81d3df 100644 --- a/src/proxy/index.ts +++ b/src/proxy/index.ts @@ -110,7 +110,7 @@ export class Proxy { } resolve(); - } catch (error) { + } catch (error: unknown) { reject(error); } }); diff --git a/test/1.test.ts b/test/1.test.ts index 501ff956a..255f6f41d 100644 --- a/test/1.test.ts +++ b/test/1.test.ts @@ -12,7 +12,7 @@ import { describe, it, beforeAll, afterAll, beforeEach, afterEach, expect, vi } import request from 'supertest'; import { Service } from '../src/service'; import * as db from '../src/db'; -import Proxy from '../src/proxy'; +import { Proxy } from '../src/proxy'; import { Express } from 'express'; // Create constants for values used in multiple tests diff --git a/test/processors/gitLeaks.test.ts b/test/processors/gitLeaks.test.ts index 55940f812..7e3e184fa 100644 --- a/test/processors/gitLeaks.test.ts +++ b/test/processors/gitLeaks.test.ts @@ -93,7 +93,7 @@ describe('gitleaks', () => { ); expect(errorStub).toHaveBeenCalledWith( 'failed to get gitleaks config, please fix the error:', - expect.any(Error), + 'Config error', ); }); @@ -246,7 +246,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', ); }); diff --git a/test/testLogin.test.ts b/test/testLogin.test.ts index e56f48add..2cbb0ba46 100644 --- a/test/testLogin.test.ts +++ b/test/testLogin.test.ts @@ -232,7 +232,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'); }); }); From 4d203e6c50c74a4191182197ff3abb9f0e9f4e31 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 17 Dec 2025 11:26:48 +0900 Subject: [PATCH 29/31] chore: fix failing tests (revision range error) --- src/proxy/processors/push-action/getDiff.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/proxy/processors/push-action/getDiff.ts b/src/proxy/processors/push-action/getDiff.ts index 5308fd807..c7c9791a6 100644 --- a/src/proxy/processors/push-action/getDiff.ts +++ b/src/proxy/processors/push-action/getDiff.ts @@ -37,7 +37,6 @@ const exec = async (_req: Request, action: Action): Promise => { } catch (error: unknown) { const msg = error instanceof Error ? error.message : String(error); step.setError(msg); - throw error; } finally { action.addStep(step); } From edcfe3cfa5c95be5c3f2a539e04d34f6fd7e1968 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 17 Dec 2025 11:56:17 +0900 Subject: [PATCH 30/31] chore: fix Proxy type errors --- src/service/routes/index.ts | 2 +- src/service/routes/repo.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/service/routes/index.ts b/src/service/routes/index.ts index c3316f4ec..f2d76a4d0 100644 --- a/src/service/routes/index.ts +++ b/src/service/routes/index.ts @@ -7,7 +7,7 @@ import users from './users'; import healthcheck from './healthcheck'; import config from './config'; import { jwtAuthHandler } from '../passport/jwtAuthHandler'; -import Proxy from '../../proxy'; +import { Proxy } from '../../proxy'; const routes = (proxy: Proxy) => { const router = express.Router(); diff --git a/src/service/routes/repo.ts b/src/service/routes/repo.ts index e5fff0489..06b6690b8 100644 --- a/src/service/routes/repo.ts +++ b/src/service/routes/repo.ts @@ -5,7 +5,7 @@ import { getProxyURL } from '../urls'; import { getAllProxiedHosts } from '../../db'; import { RepoQuery } from '../../db/types'; import { isAdminUser } from './utils'; -import Proxy from '../../proxy'; +import { Proxy } from '../../proxy'; // 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 From b7b803c7871c8358409c5322123989c9c8207c1a Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 17 Dec 2025 12:58:10 +0900 Subject: [PATCH 31/31] fix: user.admin errors and failing e2e tests --- src/ui/components/Navbars/DashboardNavbarLinks.tsx | 3 +-- src/ui/views/RepoDetails/RepoDetails.tsx | 14 +++++++------- src/ui/views/RepoList/Components/Repositories.tsx | 2 +- src/ui/views/User/UserProfile.tsx | 2 +- src/ui/views/UserList/Components/UserList.tsx | 2 +- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/ui/components/Navbars/DashboardNavbarLinks.tsx b/src/ui/components/Navbars/DashboardNavbarLinks.tsx index 350f73d81..ae1eda7ce 100644 --- a/src/ui/components/Navbars/DashboardNavbarLinks.tsx +++ b/src/ui/components/Navbars/DashboardNavbarLinks.tsx @@ -28,7 +28,7 @@ const DashboardNavbarLinks: React.FC = () => { const [openProfile, setOpenProfile] = useState(null); const [, setAuth] = useState(true); const [, setIsLoading] = useState(true); - const [errorMessage, setErrorMessage] = useState(''); + const [, setErrorMessage] = useState(''); const [user, setUser] = useState(null); useEffect(() => { @@ -67,7 +67,6 @@ const DashboardNavbarLinks: React.FC = () => { return (
- {errorMessage &&
{errorMessage}
}
; if (isError) return {errorMessage}; - const addrepoButton = user.admin ? ( + const addrepoButton = user?.admin ? ( diff --git a/src/ui/views/User/UserProfile.tsx b/src/ui/views/User/UserProfile.tsx index f904850b6..fb3830783 100644 --- a/src/ui/views/User/UserProfile.tsx +++ b/src/ui/views/User/UserProfile.tsx @@ -139,7 +139,7 @@ export default function UserProfile(): React.ReactElement { )} Administrator - {user.admin ? ( + {user?.admin ? ( diff --git a/src/ui/views/UserList/Components/UserList.tsx b/src/ui/views/UserList/Components/UserList.tsx index 035930b12..41cb3c0f0 100644 --- a/src/ui/views/UserList/Components/UserList.tsx +++ b/src/ui/views/UserList/Components/UserList.tsx @@ -91,7 +91,7 @@ const UserList: React.FC = () => { - {user.admin ? ( + {user?.admin ? ( ) : (