From 7e78399cd592e3cb8d95c0e1793709f2472eda59 Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Fri, 28 Aug 2026 12:41:21 +0200 Subject: [PATCH 1/2] fix(server-nestjs): reuse Vault AppRole secret-id instead of minting on every sync Refs #2622 Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: Ie3d3b7df1e0539c02d6a215ce7eff82a6a6a6964 --- .../src/modules/argocd/argocd.service.spec.ts | 14 +++--- .../src/modules/argocd/argocd.service.ts | 2 +- .../vault/vault-client.service.spec.ts | 44 +++++++++++++++++++ .../src/modules/vault/vault-client.service.ts | 11 ++++- .../src/modules/vault/vault.utils.ts | 4 ++ 5 files changed, 65 insertions(+), 10 deletions(-) diff --git a/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts b/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts index 5eddeef014..6698c48859 100644 --- a/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts +++ b/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts @@ -249,7 +249,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -450,7 +450,7 @@ describe('argoCDService', () => { ), ]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -541,7 +541,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockResolvedValue(null) @@ -586,7 +586,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -742,7 +742,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -805,7 +805,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -842,7 +842,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) diff --git a/apps/server-nestjs/src/modules/argocd/argocd.service.ts b/apps/server-nestjs/src/modules/argocd/argocd.service.ts index 9a371d9d50..dfa94b2417 100644 --- a/apps/server-nestjs/src/modules/argocd/argocd.service.ts +++ b/apps/server-nestjs/src/modules/argocd/argocd.service.ts @@ -404,7 +404,7 @@ export class ArgoCDService { this.logger.warn(`Couldn't find app role (project=${projectSlug})`) return undefined }) - const secretId = await this.vault.createAuthApproleRoleSecretId(projectSlug).catch(() => { + const secretId = await this.vault.ensureAuthApproleRoleSecretId(projectSlug).catch(() => { this.logger.warn(`Couldn't find secret (project=${projectSlug})`) return undefined }) diff --git a/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts b/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts index 01adf6a96b..bf3a09c4ce 100644 --- a/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts +++ b/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts @@ -144,4 +144,48 @@ describe('vault', () => { expect(capturedPath).toBe('forge/my-project/GITLAB') }) }) + + describe('ensureAuthApproleRoleSecretId', () => { + it('mints and persists a secret-id on first sync', async () => { + let minted = false + let persisted: unknown + server.use( + http.get(`${vaultUrl}/v1/kv/data/*`, () => { + return HttpResponse.json({}, { status: HttpStatus.NOT_FOUND }) + }), + http.post(`${vaultUrl}/v1/auth/approle/role/*/secret-id`, () => { + minted = true + return HttpResponse.json({ data: { secret_id: 'minted-secret' } }) + }), + http.post(`${vaultUrl}/v1/kv/data/*`, async ({ request }) => { + persisted = await request.json() + return HttpResponse.json({}) + }), + ) + + const secretId = await service.ensureAuthApproleRoleSecretId('my-project') + + expect(secretId).toBe('minted-secret') + expect(minted).toBe(true) + expect(persisted).toEqual({ data: { secret_id: 'minted-secret' } }) + }) + + it('reuses a persisted secret-id on subsequent syncs without minting', async () => { + let minted = false + server.use( + http.get(`${vaultUrl}/v1/kv/data/*`, () => { + return HttpResponse.json({ data: { data: { secret_id: 'persisted-secret' }, metadata: { created_time: '2023-01-01T00:00:00.000Z', version: 1 } } }) + }), + http.post(`${vaultUrl}/v1/auth/approle/role/*/secret-id`, () => { + minted = true + return HttpResponse.json({ data: { secret_id: 'unexpected' } }) + }), + ) + + const secretId = await service.ensureAuthApproleRoleSecretId('my-project') + + expect(secretId).toBe('persisted-secret') + expect(minted).toBe(false) + }) + }) }) diff --git a/apps/server-nestjs/src/modules/vault/vault-client.service.ts b/apps/server-nestjs/src/modules/vault/vault-client.service.ts index 18fb021913..7fa9454e4d 100644 --- a/apps/server-nestjs/src/modules/vault/vault-client.service.ts +++ b/apps/server-nestjs/src/modules/vault/vault-client.service.ts @@ -5,7 +5,7 @@ import { baseConfigFactory } from '../../config/base.config' import { vaultConfigFactory } from '../../config/vault.config' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' import { VaultError, VaultHttpClientService } from './vault-http-client.service' -import { generateGitlabMirrorCredPath, generateSecretGroupPath, generateSonarqubeCredPath, generateTechReadOnlyCredPath, isVaultNotFound } from './vault.utils' +import { generateAppRoleSecretIdPath, generateGitlabMirrorCredPath, generateSecretGroupPath, generateSonarqubeCredPath, generateTechReadOnlyCredPath, isVaultNotFound } from './vault.utils' export interface VaultSysPoliciesAclUpsertRequest { policy: string @@ -401,7 +401,13 @@ export class VaultClientService { } @StartActiveSpan() - async createAuthApproleRoleSecretId(roleName: string) { + async ensureAuthApproleRoleSecretId(roleName: string) { + const kvPath = generateAppRoleSecretIdPath(this.baseConfig.projectsRootDir, roleName) + const existing = await this.read<{ secret_id: string }>(kvPath).catch(() => null) + if (existing?.data?.secret_id) { + this.logger.verbose(`Reusing Vault AppRole secret-id for ${roleName}`) + return existing.data.secret_id + } const path = `auth/approle/role/${roleName}/secret-id` this.logger.verbose(`Creating Vault AppRole secret-id for ${roleName}`) const response = await this.http.fetch(path, { method: 'POST' }) @@ -409,6 +415,7 @@ export class VaultClientService { if (!secretId) { throw new VaultError('InvalidResponse', `Vault secret-id not generated for role ${roleName}`, { method: 'POST', path }) } + await this.write({ secret_id: secretId }, kvPath) return secretId } diff --git a/apps/server-nestjs/src/modules/vault/vault.utils.ts b/apps/server-nestjs/src/modules/vault/vault.utils.ts index c6b1782673..72d2700cb6 100644 --- a/apps/server-nestjs/src/modules/vault/vault.utils.ts +++ b/apps/server-nestjs/src/modules/vault/vault.utils.ts @@ -27,3 +27,7 @@ export function isVaultNotFound(error: unknown): error is VaultError { export function isVaultBadRequest(error: unknown): error is VaultError { return error instanceof VaultError && error.kind === 'HttpError' && error.status === 400 } + +export function generateAppRoleSecretIdPath(projectRootDir: string, projectSlug: string) { + return `${generateProjectPath(projectRootDir, projectSlug)}/APPROLE_SECRET_ID` +} From 67f366e34c9d4d16efa30a7634db9a86613a8499 Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Fri, 25 Sep 2026 10:47:01 +0200 Subject: [PATCH 2/2] test(vault): cover secret-id path and malformed persisted entry Co-authored-by: Automata Signed-off-by: William Phetsinorath --- .../vault/vault-client.service.spec.ts | 19 +++++++++++++++++++ .../src/modules/vault/vault.utils.spec.ts | 6 +++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts b/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts index bf3a09c4ce..44a6513c5e 100644 --- a/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts +++ b/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts @@ -187,5 +187,24 @@ describe('vault', () => { expect(secretId).toBe('persisted-secret') expect(minted).toBe(false) }) + + it('mints when the persisted entry lacks secret_id', async () => { + let minted = false + server.use( + http.get(`${vaultUrl}/v1/kv/data/*`, () => { + return HttpResponse.json({ data: { data: {} } }) + }), + http.post(`${vaultUrl}/v1/auth/approle/role/*/secret-id`, () => { + minted = true + return HttpResponse.json({ data: { secret_id: 'fresh-secret' } }) + }), + http.post(`${vaultUrl}/v1/kv/data/*`, () => HttpResponse.json({})), + ) + + const secretId = await service.ensureAuthApproleRoleSecretId('my-project') + + expect(secretId).toBe('fresh-secret') + expect(minted).toBe(true) + }) }) }) diff --git a/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts b/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts index bf10dc54fc..f79d5397ef 100644 --- a/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts +++ b/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts @@ -1,11 +1,15 @@ import { describe, expect, it } from 'vitest' import { VaultError } from './vault-http-client.service' -import { generateSecretGroupPath, isVaultBadRequest, isVaultNotFound } from './vault.utils' +import { generateAppRoleSecretIdPath, generateSecretGroupPath, isVaultBadRequest, isVaultNotFound } from './vault.utils' describe('vault path helpers', () => { it('scopes a group to the project path', () => { expect(generateSecretGroupPath('forge', 'my-project', 'GITLAB')).toBe('forge/my-project/GITLAB') }) + + it('scopes the AppRole secret-id to the project path', () => { + expect(generateAppRoleSecretIdPath('forge', 'my-project')).toBe('forge/my-project/APPROLE_SECRET_ID') + }) }) describe('vault error guards', () => {