From 844deabfd2e76028bddb6735e72e6ba376f95d4e Mon Sep 17 00:00:00 2001 From: Matthias Erll Date: Mon, 23 Jun 2025 16:32:53 +0200 Subject: [PATCH 1/2] fix: add type checking to message sanitation --- src/git.ts | 5 +++-- src/utils.test.ts | 27 ++++++++++++++++++++++++++- src/utils.ts | 11 +++++++++-- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/git.ts b/src/git.ts index d08833842..b31ec7100 100644 --- a/src/git.ts +++ b/src/git.ts @@ -25,7 +25,7 @@ import { BASEURL } from './constants' import { GitPullError, HttpError, ValidationError } from './error' import { Core } from './otomi-models' import { FileMap, getFilePath, getResourceName, renderManifest, renderManifestForSecrets } from './repo' -import { getSanitizedErrorMessage, removeBlankAttributes } from './utils' +import { getSanitizedErrorMessage, removeBlankAttributes, sanitizeString } from './utils' const debug = Debug('otomi:repo') @@ -473,7 +473,8 @@ export class Git { } } catch (e) { const sanitizedMessage = getSanitizedErrorMessage(e) - debug(`${sanitizedMessage} for command ${JSON.stringify(e.task?.commands).replace(env.GIT_PASSWORD, '****')}`) + const sanitizedCommands = sanitizeString(JSON.stringify(e.task?.commands)) + debug(`${sanitizedMessage} for command ${sanitizedCommands}`) debug('Git save error') throw new GitPullError() } diff --git a/src/utils.test.ts b/src/utils.test.ts index 7faf7f033..c0cc37553 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,5 +1,6 @@ import { Cluster } from 'src/otomi-models' -import { getServiceUrl } from 'src/utils' +import { getSanitizedErrorMessage, getServiceUrl, sanitizeString } from 'src/utils' +import { cleanEnv, GIT_PASSWORD } from './validators' describe('Utils', () => { const cluster: Cluster = { @@ -57,4 +58,28 @@ describe('Utils', () => { expect(service.subdomain).toEqual('aa') expect(service.domain).toEqual('bb.cc.dd.ee') }) + + describe('should remove git credentials', () => { + const env = cleanEnv({ + GIT_PASSWORD, + }) + test('from strings', () => { + expect(sanitizeString('test string')).toBe('test string') + expect(sanitizeString(`test string ${env.GIT_PASSWORD}`)).toBe('test string ****') + }) + test('from objects', () => { + expect(sanitizeString(JSON.stringify({ test: 'some string' }))).toEqual('{"test":"some string"}') + expect(sanitizeString(JSON.stringify({ test: `some string ${env.GIT_PASSWORD}` }))).toEqual( + '{"test":"some string ****"}', + ) + }) + test('return empty string on empty or undefined input', () => { + expect(sanitizeString('')).toEqual('') + expect(sanitizeString(undefined)).toEqual('') + }) + test('extract message from exception', () => { + expect(getSanitizedErrorMessage(new Error('test error'))).toEqual('test error') + expect(getSanitizedErrorMessage(new Error(`test error ${env.GIT_PASSWORD}`))).toEqual('test error ****') + }) + }) }) diff --git a/src/utils.ts b/src/utils.ts index 4d1853e54..b6f7f84d1 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -201,7 +201,14 @@ export const objectToYaml = (obj: Record, indent = 4, lineWidth = 2 return isEmpty(obj) ? '' : stringify(obj, { indent, lineWidth }) } +export function sanitizeString(str?: string) { + return str ? str.replace(env.GIT_PASSWORD, '****') : '' +} + export function getSanitizedErrorMessage(error) { - const errorMessage = typeof error?.message === 'string' ? error.message.replace(env.GIT_PASSWORD, '****') : '' - return errorMessage + const message = error?.message + if (!message) { + return '' + } + return typeof message === 'string' ? sanitizeString(message) : `[unprocessable message type ${typeof message}]` } From 32bfb900decb0da391f4d4f155c9597634cfaf67 Mon Sep 17 00:00:00 2001 From: Matthias Erll Date: Wed, 25 Jun 2025 09:45:59 +0200 Subject: [PATCH 2/2] fix: replace all occurences of credentials --- src/git.ts | 4 ++-- src/utils.test.ts | 16 ++++++++-------- src/utils.ts | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/git.ts b/src/git.ts index b31ec7100..a8dd2fc7a 100644 --- a/src/git.ts +++ b/src/git.ts @@ -25,7 +25,7 @@ import { BASEURL } from './constants' import { GitPullError, HttpError, ValidationError } from './error' import { Core } from './otomi-models' import { FileMap, getFilePath, getResourceName, renderManifest, renderManifestForSecrets } from './repo' -import { getSanitizedErrorMessage, removeBlankAttributes, sanitizeString } from './utils' +import { getSanitizedErrorMessage, removeBlankAttributes, sanitizeGitPassword } from './utils' const debug = Debug('otomi:repo') @@ -473,7 +473,7 @@ export class Git { } } catch (e) { const sanitizedMessage = getSanitizedErrorMessage(e) - const sanitizedCommands = sanitizeString(JSON.stringify(e.task?.commands)) + const sanitizedCommands = sanitizeGitPassword(JSON.stringify(e.task?.commands)) debug(`${sanitizedMessage} for command ${sanitizedCommands}`) debug('Git save error') throw new GitPullError() diff --git a/src/utils.test.ts b/src/utils.test.ts index c0cc37553..fe95d9f10 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,5 +1,5 @@ import { Cluster } from 'src/otomi-models' -import { getSanitizedErrorMessage, getServiceUrl, sanitizeString } from 'src/utils' +import { getSanitizedErrorMessage, getServiceUrl, sanitizeGitPassword } from 'src/utils' import { cleanEnv, GIT_PASSWORD } from './validators' describe('Utils', () => { @@ -59,23 +59,23 @@ describe('Utils', () => { expect(service.domain).toEqual('bb.cc.dd.ee') }) - describe('should remove git credentials', () => { + describe('sanitizeGitPassword should remove git credentials', () => { const env = cleanEnv({ GIT_PASSWORD, }) test('from strings', () => { - expect(sanitizeString('test string')).toBe('test string') - expect(sanitizeString(`test string ${env.GIT_PASSWORD}`)).toBe('test string ****') + expect(sanitizeGitPassword('test string')).toBe('test string') + expect(sanitizeGitPassword(`${env.GIT_PASSWORD} test string ${env.GIT_PASSWORD}`)).toBe('**** test string ****') }) test('from objects', () => { - expect(sanitizeString(JSON.stringify({ test: 'some string' }))).toEqual('{"test":"some string"}') - expect(sanitizeString(JSON.stringify({ test: `some string ${env.GIT_PASSWORD}` }))).toEqual( + expect(sanitizeGitPassword(JSON.stringify({ test: 'some string' }))).toEqual('{"test":"some string"}') + expect(sanitizeGitPassword(JSON.stringify({ test: `some string ${env.GIT_PASSWORD}` }))).toEqual( '{"test":"some string ****"}', ) }) test('return empty string on empty or undefined input', () => { - expect(sanitizeString('')).toEqual('') - expect(sanitizeString(undefined)).toEqual('') + expect(sanitizeGitPassword('')).toEqual('') + expect(sanitizeGitPassword(undefined)).toEqual('') }) test('extract message from exception', () => { expect(getSanitizedErrorMessage(new Error('test error'))).toEqual('test error') diff --git a/src/utils.ts b/src/utils.ts index b6f7f84d1..0113108aa 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -201,8 +201,8 @@ export const objectToYaml = (obj: Record, indent = 4, lineWidth = 2 return isEmpty(obj) ? '' : stringify(obj, { indent, lineWidth }) } -export function sanitizeString(str?: string) { - return str ? str.replace(env.GIT_PASSWORD, '****') : '' +export function sanitizeGitPassword(str?: string) { + return str ? str.replaceAll(env.GIT_PASSWORD, '****') : '' } export function getSanitizedErrorMessage(error) { @@ -210,5 +210,5 @@ export function getSanitizedErrorMessage(error) { if (!message) { return '' } - return typeof message === 'string' ? sanitizeString(message) : `[unprocessable message type ${typeof message}]` + return typeof message === 'string' ? sanitizeGitPassword(message) : `[unprocessable message type ${typeof message}]` }