fix: settings update and Git secret rotation

src/api/v2/git.ts

@@ -44,13 +44,9 @@ export const migrateGit = async (req: OpenApiRequestExt, res: Response): Promise
       return
     }
   }
-  if (remoteHasContent) {
-    res.json({ message: 'New repository is not empty', statusCode: 400 })
-    return
-  }
 
   // Write config and push to new remote
-  await req.otomi.migrateGitSettings(newGitConfig)
+  await req.otomi.migrateGitSettings(newGitConfig, remoteHasContent)
 
   res.json({})
 }

src/otomi-stack.test.ts

@@ -9,7 +9,7 @@ import {
 } from 'src/otomi-models'
 import OtomiStack from 'src/otomi-stack'
 import { loadSpec } from './app'
-import { NotExistError, ValidationError } from './error'
+import { BadRequestError, NotExistError, ValidationError } from './error'
 import { Git } from './git'
 
 jest.mock('./tty', () => ({
@@ -1362,6 +1362,13 @@ describe('OtomiStack.migrateGitSettings', () => {
     stack = new OtomiStack()
     stack.editor = 'test@example.com'
     ;(stack as any).sessionId = 'test-session-id'
+    ;(stack as any).gitConfig = {
+      repoUrl: 'https://old.example.com/repo.git',
+      branch: 'main',
+      username: 'user',
+      password: 'pass',
+      email: 'old@example.com',
+    }
     jest.spyOn(stack as any, 'getSettings').mockReturnValue({
       otomi: { git: { repoUrl: 'https://old.example.com/repo.git', branch: 'main', email: 'old@example.com' } },
     })
@@ -1377,6 +1384,7 @@ describe('OtomiStack.migrateGitSettings', () => {
       git: { git: { pull: mockRootPull } },
       fileStore: { set: mockRootFileStoreSet },
       refreshGitClient: mockRefreshGitClient,
+      gitConfig: stack.gitConfig,
     })
     jest.spyOn(require('src/middleware/session'), 'cleanSession').mockResolvedValue(undefined)
     ;(stack as any).getApiClient = jest.fn().mockReturnValue({
@@ -1386,19 +1394,75 @@ describe('OtomiStack.migrateGitSettings', () => {
 
   afterEach(() => jest.restoreAllMocks())
 
-  it('calls pushToNewRemote', async () => {
-    await stack.migrateGitSettings({
-      repoUrl: 'https://new.example.com/repo.git',
-      username: 'user',
-      password: 'pass',
-      email: 'new@example.com',
-      branch: 'main',
-    })
+  it('commits and pushes to new remote when repoUrl changes and remote is empty', async () => {
+    await stack.migrateGitSettings(
+      {
+        repoUrl: 'https://new.example.com/repo.git',
+        username: 'user',
+        password: 'pass',
+        email: 'new@example.com',
+        branch: 'main',
+      },
+      false,
+    )
 
     expect(mockCommit).toHaveBeenCalled()
     expect(mockPushToNewRemote).toHaveBeenCalled()
     expect(mockRefreshGitClient).toHaveBeenCalled()
   })
+
+  it('commits and pushes to new remote when branch changes and remote is empty', async () => {
+    await stack.migrateGitSettings(
+      {
+        repoUrl: 'https://old.example.com/repo.git',
+        username: 'user',
+        password: 'pass',
+        email: 'old@example.com',
+        branch: 'new-branch',
+      },
+      false,
+    )
+
+    expect(mockCommit).toHaveBeenCalled()
+    expect(mockPushToNewRemote).toHaveBeenCalled()
+    expect(mockRefreshGitClient).toHaveBeenCalled()
+  })
+
+  it('throws BadRequestError when repoUrl changes and remote already has content', async () => {
+    await expect(
+      stack.migrateGitSettings(
+        {
+          repoUrl: 'https://new.example.com/repo.git',
+          username: 'user',
+          password: 'pass',
+          email: 'new@example.com',
+          branch: 'main',
+        },
+        true,
+      ),
+    ).rejects.toThrow(new BadRequestError('Branch main in repository is not empty'))
+
+    expect(mockCommit).not.toHaveBeenCalled()
+    expect(mockPushToNewRemote).not.toHaveBeenCalled()
+    expect(mockRefreshGitClient).not.toHaveBeenCalled()
+  })
+
+  it('only stores config without migration when only credentials change', async () => {
+    await stack.migrateGitSettings(
+      {
+        repoUrl: 'https://old.example.com/repo.git',
+        username: 'new-user',
+        password: 'new-pass',
+        email: 'new@example.com',
+        branch: 'main',
+      },
+      false,
+    )
+
+    expect(mockCommit).not.toHaveBeenCalled()
+    expect(mockPushToNewRemote).not.toHaveBeenCalled()
+    expect(mockRefreshGitClient).toHaveBeenCalled()
+  })
 })
 
 describe('OtomiStack locked state', () => {

src/otomi-stack.ts

@@ -644,13 +644,11 @@ export default class OtomiStack {
         updatedSettingsData.otomi.nodeSelector = nodeSelectorObject
       }
       const updatedGitSettings = updatedSettingsData.otomi?.git as GitConfig
-      if (updatedGitSettings) {
-        if (!updatedGitSettings.password) {
-          throw new BadRequestError('Git credentials may not be provided without a password')
-        }
+      if (updatedGitSettings?.repoUrl && updatedGitSettings?.password) {
+        // For backwards compatibility, update only if provided data is sufficient, but do not raise an error
         await this.storeGitConfig(updatedGitSettings)
-        unset(updatedSettingsData, 'otomi.git.password')
       }
+      unset(updatedSettingsData, 'otomi.git')
     }
 
     const sealedSecretRecord = await this.extractAndStoreSettingsSecrets(settingId, updatedSettingsData)
@@ -688,8 +686,17 @@ export default class OtomiStack {
     return settings
   }
 
-  async migrateGitSettings(params: GitConfig): Promise<void> {
-    await this.commitAndPushMigration(params)
+  async migrateGitSettings(params: GitConfig, remoteHasContent: boolean): Promise<void> {
+    const rootStack = await getSessionStack()
+    const { repoUrl, branch } = rootStack.gitConfig
+    const isDifferentRepo = repoUrl !== params.repoUrl || branch !== params.branch
+    if (isDifferentRepo) {
+      if (remoteHasContent) {
+        throw new BadRequestError(`Branch ${params.branch} in repository is not empty`)
+      }
+      // Do not migrate only on credential or identity change
+      await this.commitAndPushMigration(params)
+    }
     await this.storeGitConfig(params)
   }