Persist creds to a separate file (#2286)

__test__/git-auth-helper.test.ts

@@ -86,16 +86,29 @@ describe('git-auth-helper tests', () => {
     // Act
     await authHelper.configureAuth()
 
-    // Assert config
-    const configContent = (
+    // Assert config - check that .git/config contains includeIf entries
+    const localConfigContent = (
       await fs.promises.readFile(localGitConfigPath)
     ).toString()
+    expect(
+      localConfigContent.indexOf('includeIf.gitdir:')
+    ).toBeGreaterThanOrEqual(0)
+
+    // Assert credentials config file contains the actual credentials
+    const credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+      f => f.startsWith('git-credentials-') && f.endsWith('.config')
+    )
+    expect(credentialsFiles.length).toBe(1)
+    const credentialsConfigPath = path.join(runnerTemp, credentialsFiles[0])
+    const credentialsContent = (
+      await fs.promises.readFile(credentialsConfigPath)
+    ).toString()
     const basicCredential = Buffer.from(
       `x-access-token:${settings.authToken}`,
       'utf8'
     ).toString('base64')
     expect(
-      configContent.indexOf(
+      credentialsContent.indexOf(
         `http.${expectedServerUrl}/.extraheader AUTHORIZATION: basic ${basicCredential}`
       )
     ).toBeGreaterThanOrEqual(0)
@@ -120,7 +133,7 @@ describe('git-auth-helper tests', () => {
     'inject https://github.com as github server url'
   it(configureAuth_AcceptsGitHubServerUrlSetToGHEC, async () => {
     await testAuthHeader(
-      configureAuth_AcceptsGitHubServerUrl,
+      configureAuth_AcceptsGitHubServerUrlSetToGHEC,
       'https://github.com'
     )
   })
@@ -141,12 +154,17 @@ describe('git-auth-helper tests', () => {
       // Act
       await authHelper.configureAuth()
 
-      // Assert config
-      const configContent = (
-        await fs.promises.readFile(localGitConfigPath)
+      // Assert config - check credentials config file (not local .git/config)
+      const credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+        f => f.startsWith('git-credentials-') && f.endsWith('.config')
+      )
+      expect(credentialsFiles.length).toBe(1)
+      const credentialsConfigPath = path.join(runnerTemp, credentialsFiles[0])
+      const credentialsContent = (
+        await fs.promises.readFile(credentialsConfigPath)
       ).toString()
       expect(
-        configContent.indexOf(
+        credentialsContent.indexOf(
           `http.https://github.com/.extraheader AUTHORIZATION`
         )
       ).toBeGreaterThanOrEqual(0)
@@ -251,13 +269,16 @@ describe('git-auth-helper tests', () => {
       expectedSshCommand
     )
 
-    // Asserty git config
+    // Assert git config
     const gitConfigLines = (await fs.promises.readFile(localGitConfigPath))
       .toString()
       .split('\n')
       .filter(x => x)
-    expect(gitConfigLines).toHaveLength(1)
-    expect(gitConfigLines[0]).toMatch(/^http\./)
+    // Should have includeIf entries pointing to credentials file
+    expect(gitConfigLines.length).toBeGreaterThan(0)
+    expect(
+      gitConfigLines.some(line => line.indexOf('includeIf.gitdir:') >= 0)
+    ).toBeTruthy()
   })
 
   const configureAuth_setsSshCommandWhenPersistCredentialsTrue =
@@ -419,8 +440,20 @@ describe('git-auth-helper tests', () => {
     expect(
       configContent.indexOf('value-from-global-config')
     ).toBeGreaterThanOrEqual(0)
+    // Global config should have include.path pointing to credentials file
+    expect(configContent.indexOf('include.path')).toBeGreaterThanOrEqual(0)
+
+    // Check credentials in the separate config file
+    const credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+      f => f.startsWith('git-credentials-') && f.endsWith('.config')
+    )
+    expect(credentialsFiles.length).toBeGreaterThan(0)
+    const credentialsConfigPath = path.join(runnerTemp, credentialsFiles[0])
+    const credentialsContent = (
+      await fs.promises.readFile(credentialsConfigPath)
+    ).toString()
     expect(
-      configContent.indexOf(
+      credentialsContent.indexOf(
         `http.https://github.com/.extraheader AUTHORIZATION: basic ${basicCredential}`
       )
     ).toBeGreaterThanOrEqual(0)
@@ -463,8 +496,20 @@ describe('git-auth-helper tests', () => {
       const configContent = (
         await fs.promises.readFile(path.join(git.env['HOME'], '.gitconfig'))
       ).toString()
+      // Global config should have include.path pointing to credentials file
+      expect(configContent.indexOf('include.path')).toBeGreaterThanOrEqual(0)
+
+      // Check credentials in the separate config file
+      const credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+        f => f.startsWith('git-credentials-') && f.endsWith('.config')
+      )
+      expect(credentialsFiles.length).toBeGreaterThan(0)
+      const credentialsConfigPath = path.join(runnerTemp, credentialsFiles[0])
+      const credentialsContent = (
+        await fs.promises.readFile(credentialsConfigPath)
+      ).toString()
       expect(
-        configContent.indexOf(
+        credentialsContent.indexOf(
           `http.https://github.com/.extraheader AUTHORIZATION: basic ${basicCredential}`
         )
       ).toBeGreaterThanOrEqual(0)
@@ -550,15 +595,15 @@ describe('git-auth-helper tests', () => {
       await authHelper.configureSubmoduleAuth()
 
       // Assert
-      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(4)
+      // Should configure insteadOf (2 calls for two values)
+      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(3)
       expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
         /unset-all.*insteadOf/
       )
-      expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/http.*extraheader/)
-      expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(
+      expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(
         /url.*insteadOf.*git@github.com:/
       )
-      expect(mockSubmoduleForeach.mock.calls[3][0]).toMatch(
+      expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(
         /url.*insteadOf.*org-123456@github.com:/
       )
     }
@@ -589,12 +634,12 @@ describe('git-auth-helper tests', () => {
       await authHelper.configureSubmoduleAuth()
 
       // Assert
-      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(3)
+      // Should configure sshCommand (1 call)
+      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(2)
       expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
         /unset-all.*insteadOf/
       )
-      expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/http.*extraheader/)
-      expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(/core\.sshCommand/)
+      expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/core\.sshCommand/)
     }
   )
 
@@ -660,19 +705,201 @@ describe('git-auth-helper tests', () => {
     await setup(removeAuth_removesToken)
     const authHelper = gitAuthHelper.createAuthHelper(git, settings)
     await authHelper.configureAuth()
-    let gitConfigContent = (
+
+    // Verify includeIf entries exist in local config
+    let localConfigContent = (
       await fs.promises.readFile(localGitConfigPath)
     ).toString()
-    expect(gitConfigContent.indexOf('http.')).toBeGreaterThanOrEqual(0) // sanity check
+    expect(
+      localConfigContent.indexOf('includeIf.gitdir:')
+    ).toBeGreaterThanOrEqual(0)
+
+    // Verify both host and container includeIf entries are present
+    const hostGitDir = path.join(workspace, '.git').replace(/\\/g, '/')
+    expect(
+      localConfigContent.indexOf(`includeIf.gitdir:${hostGitDir}.path`)
+    ).toBeGreaterThanOrEqual(0)
+    expect(
+      localConfigContent.indexOf('includeIf.gitdir:/github/workspace/.git.path')
+    ).toBeGreaterThanOrEqual(0)
+
+    // Verify credentials file exists
+    let credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+      f => f.startsWith('git-credentials-') && f.endsWith('.config')
+    )
+    expect(credentialsFiles.length).toBe(1)
+    const credentialsFilePath = path.join(runnerTemp, credentialsFiles[0])
+
+    // Verify credentials file contains the auth token
+    let credentialsContent = (
+      await fs.promises.readFile(credentialsFilePath)
+    ).toString()
+    const basicCredential = Buffer.from(
+      `x-access-token:${settings.authToken}`,
+      'utf8'
+    ).toString('base64')
+    expect(
+      credentialsContent.indexOf(
+        `http.https://github.com/.extraheader AUTHORIZATION: basic ${basicCredential}`
+      )
+    ).toBeGreaterThanOrEqual(0)
+
+    // Verify the includeIf entries point to the credentials file
+    const containerCredentialsPath = path.posix.join(
+      '/github/runner_temp',
+      path.basename(credentialsFilePath)
+    )
+    expect(
+      localConfigContent.indexOf(credentialsFilePath)
+    ).toBeGreaterThanOrEqual(0)
+    expect(
+      localConfigContent.indexOf(containerCredentialsPath)
+    ).toBeGreaterThanOrEqual(0)
 
     // Act
     await authHelper.removeAuth()
 
-    // Assert git config
-    gitConfigContent = (
+    // Assert all includeIf entries removed from local git config
+    localConfigContent = (
       await fs.promises.readFile(localGitConfigPath)
     ).toString()
-    expect(gitConfigContent.indexOf('http.')).toBeLessThan(0)
+    expect(localConfigContent.indexOf('includeIf.gitdir:')).toBeLessThan(0)
+    expect(
+      localConfigContent.indexOf(`includeIf.gitdir:${hostGitDir}.path`)
+    ).toBeLessThan(0)
+    expect(
+      localConfigContent.indexOf('includeIf.gitdir:/github/workspace/.git.path')
+    ).toBeLessThan(0)
+    expect(localConfigContent.indexOf(credentialsFilePath)).toBeLessThan(0)
+    expect(localConfigContent.indexOf(containerCredentialsPath)).toBeLessThan(0)
+
+    // Assert credentials config file deleted
+    credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+      f => f.startsWith('git-credentials-') && f.endsWith('.config')
+    )
+    expect(credentialsFiles.length).toBe(0)
+
+    // Verify credentials file no longer exists on disk
+    try {
+      await fs.promises.stat(credentialsFilePath)
+      throw new Error('Credentials file should have been deleted')
+    } catch (err) {
+      if ((err as any)?.code !== 'ENOENT') {
+        throw err
+      }
+    }
+  })
+
+  const removeAuth_removesTokenFromSubmodules =
+    'removeAuth removes token from submodules'
+  it(removeAuth_removesTokenFromSubmodules, async () => {
+    // Arrange
+    await setup(removeAuth_removesTokenFromSubmodules)
+
+    // Create fake submodule config paths
+    const submodule1Dir = path.join(workspace, '.git', 'modules', 'submodule-1')
+    const submodule2Dir = path.join(workspace, '.git', 'modules', 'submodule-2')
+    const submodule1ConfigPath = path.join(submodule1Dir, 'config')
+    const submodule2ConfigPath = path.join(submodule2Dir, 'config')
+
+    await fs.promises.mkdir(submodule1Dir, {recursive: true})
+    await fs.promises.mkdir(submodule2Dir, {recursive: true})
+    await fs.promises.writeFile(submodule1ConfigPath, '')
+    await fs.promises.writeFile(submodule2ConfigPath, '')
+
+    // Mock getSubmoduleConfigPaths to return our fake submodules (for both configure and remove)
+    const mockGetSubmoduleConfigPaths =
+      git.getSubmoduleConfigPaths as jest.Mock<any, any>
+    mockGetSubmoduleConfigPaths.mockResolvedValue([
+      submodule1ConfigPath,
+      submodule2ConfigPath
+    ])
+
+    const authHelper = gitAuthHelper.createAuthHelper(git, settings)
+    await authHelper.configureAuth()
+    await authHelper.configureSubmoduleAuth()
+
+    // Verify credentials file exists
+    let credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+      f => f.startsWith('git-credentials-') && f.endsWith('.config')
+    )
+    expect(credentialsFiles.length).toBe(1)
+    const credentialsFilePath = path.join(runnerTemp, credentialsFiles[0])
+
+    // Verify submodule 1 config has includeIf entries
+    let submodule1Content = (
+      await fs.promises.readFile(submodule1ConfigPath)
+    ).toString()
+    const submodule1GitDir = submodule1Dir.replace(/\\/g, '/')
+    expect(
+      submodule1Content.indexOf(`includeIf.gitdir:${submodule1GitDir}.path`)
+    ).toBeGreaterThanOrEqual(0)
+    expect(
+      submodule1Content.indexOf(credentialsFilePath)
+    ).toBeGreaterThanOrEqual(0)
+
+    // Verify submodule 2 config has includeIf entries
+    let submodule2Content = (
+      await fs.promises.readFile(submodule2ConfigPath)
+    ).toString()
+    const submodule2GitDir = submodule2Dir.replace(/\\/g, '/')
+    expect(
+      submodule2Content.indexOf(`includeIf.gitdir:${submodule2GitDir}.path`)
+    ).toBeGreaterThanOrEqual(0)
+    expect(
+      submodule2Content.indexOf(credentialsFilePath)
+    ).toBeGreaterThanOrEqual(0)
+
+    // Verify both host and container paths are in each submodule config
+    const containerCredentialsPath = path.posix.join(
+      '/github/runner_temp',
+      path.basename(credentialsFilePath)
+    )
+    expect(
+      submodule1Content.indexOf(containerCredentialsPath)
+    ).toBeGreaterThanOrEqual(0)
+    expect(
+      submodule2Content.indexOf(containerCredentialsPath)
+    ).toBeGreaterThanOrEqual(0)
+
+    // Act - ensure mock persists for removeAuth
+    mockGetSubmoduleConfigPaths.mockResolvedValue([
+      submodule1ConfigPath,
+      submodule2ConfigPath
+    ])
+    await authHelper.removeAuth()
+
+    // Assert submodule 1 includeIf entries removed
+    submodule1Content = (
+      await fs.promises.readFile(submodule1ConfigPath)
+    ).toString()
+    expect(submodule1Content.indexOf('includeIf.gitdir:')).toBeLessThan(0)
+    expect(submodule1Content.indexOf(credentialsFilePath)).toBeLessThan(0)
+    expect(submodule1Content.indexOf(containerCredentialsPath)).toBeLessThan(0)
+
+    // Assert submodule 2 includeIf entries removed
+    submodule2Content = (
+      await fs.promises.readFile(submodule2ConfigPath)
+    ).toString()
+    expect(submodule2Content.indexOf('includeIf.gitdir:')).toBeLessThan(0)
+    expect(submodule2Content.indexOf(credentialsFilePath)).toBeLessThan(0)
+    expect(submodule2Content.indexOf(containerCredentialsPath)).toBeLessThan(0)
+
+    // Assert credentials config file deleted
+    credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
+      f => f.startsWith('git-credentials-') && f.endsWith('.config')
+    )
+    expect(credentialsFiles.length).toBe(0)
+
+    // Verify credentials file no longer exists on disk
+    try {
+      await fs.promises.stat(credentialsFilePath)
+      throw new Error('Credentials file should have been deleted')
+    } catch (err) {
+      if ((err as any)?.code !== 'ENOENT') {
+        throw err
+      }
+    }
   })
 
   const removeGlobalConfig_removesOverride =
@@ -701,6 +928,52 @@ describe('git-auth-helper tests', () => {
       }
     }
   })
+
+  const testCredentialsConfigPath_matchesCredentialsConfigPaths =
+    'testCredentialsConfigPath matches credentials config paths'
+  it(testCredentialsConfigPath_matchesCredentialsConfigPaths, async () => {
+    // Arrange
+    await setup(testCredentialsConfigPath_matchesCredentialsConfigPaths)
+    const authHelper = gitAuthHelper.createAuthHelper(git, settings)
+
+    // Get a real credentials config path
+    const credentialsConfigPath = await (
+      authHelper as any
+    ).getCredentialsConfigPath()
+
+    // Act & Assert
+    expect(
+      (authHelper as any).testCredentialsConfigPath(credentialsConfigPath)
+    ).toBe(true)
+    expect(
+      (authHelper as any).testCredentialsConfigPath(
+        '/some/path/git-credentials-12345678-abcd-1234-5678-123456789012.config'
+      )
+    ).toBe(true)
+    expect(
+      (authHelper as any).testCredentialsConfigPath(
+        '/some/path/git-credentials-abcdef12-3456-7890-abcd-ef1234567890.config'
+      )
+    ).toBe(true)
+
+    // Test invalid paths
+    expect(
+      (authHelper as any).testCredentialsConfigPath(
+        '/some/path/other-config.config'
+      )
+    ).toBe(false)
+    expect(
+      (authHelper as any).testCredentialsConfigPath(
+        '/some/path/git-credentials-invalid.config'
+      )
+    ).toBe(false)
+    expect(
+      (authHelper as any).testCredentialsConfigPath(
+        '/some/path/git-credentials-.config'
+      )
+    ).toBe(false)
+    expect((authHelper as any).testCredentialsConfigPath('')).toBe(false)
+  })
 })
 
 async function setup(testName: string): Promise<void> {
@@ -715,6 +988,7 @@ async function setup(testName: string): Promise<void> {
   await fs.promises.mkdir(tempHomedir, {recursive: true})
   process.env['RUNNER_TEMP'] = runnerTemp
   process.env['HOME'] = tempHomedir
+  process.env['GITHUB_WORKSPACE'] = workspace
 
   // Create git config
   globalGitConfigPath = path.join(tempHomedir, '.gitconfig')
@@ -733,10 +1007,20 @@ async function setup(testName: string): Promise<void> {
     checkout: jest.fn(),
     checkoutDetach: jest.fn(),
     config: jest.fn(
-      async (key: string, value: string, globalConfig?: boolean) => {
-        const configPath = globalConfig
-          ? path.join(git.env['HOME'] || tempHomedir, '.gitconfig')
-          : localGitConfigPath
+      async (
+        key: string,
+        value: string,
+        globalConfig?: boolean,
+        add?: boolean,
+        configFile?: string
+      ) => {
+        const configPath =
+          configFile ||
+          (globalConfig
+            ? path.join(git.env['HOME'] || tempHomedir, '.gitconfig')
+            : localGitConfigPath)
+        // Ensure directory exists
+        await fs.promises.mkdir(path.dirname(configPath), {recursive: true})
         await fs.promises.appendFile(configPath, `\n${key} ${value}`)
       }
     ),
@@ -756,6 +1040,7 @@ async function setup(testName: string): Promise<void> {
     env: {},
     fetch: jest.fn(),
     getDefaultBranch: jest.fn(),
+    getSubmoduleConfigPaths: jest.fn(async () => []),
     getWorkingDirectory: jest.fn(() => workspace),
     init: jest.fn(),
     isDetached: jest.fn(),
@@ -794,8 +1079,72 @@ async function setup(testName: string): Promise<void> {
         return true
       }
     ),
+    tryConfigUnsetValue: jest.fn(
+      async (
+        key: string,
+        value: string,
+        globalConfig?: boolean,
+        configPath?: string
+      ): Promise<boolean> => {
+        const targetConfigPath =
+          configPath ||
+          (globalConfig
+            ? path.join(git.env['HOME'] || tempHomedir, '.gitconfig')
+            : localGitConfigPath)
+        let content = await fs.promises.readFile(targetConfigPath)
+        let lines = content
+          .toString()
+          .split('\n')
+          .filter(x => x)
+          .filter(x => !(x.startsWith(key) && x.includes(value)))
+        await fs.promises.writeFile(targetConfigPath, lines.join('\n'))
+        return true
+      }
+    ),
     tryDisableAutomaticGarbageCollection: jest.fn(),
     tryGetFetchUrl: jest.fn(),
+    tryGetConfigValues: jest.fn(
+      async (
+        key: string,
+        globalConfig?: boolean,
+        configPath?: string
+      ): Promise<string[]> => {
+        const targetConfigPath =
+          configPath ||
+          (globalConfig
+            ? path.join(git.env['HOME'] || tempHomedir, '.gitconfig')
+            : localGitConfigPath)
+        const content = await fs.promises.readFile(targetConfigPath)
+        const lines = content
+          .toString()
+          .split('\n')
+          .filter(x => x && x.startsWith(key))
+          .map(x => x.substring(key.length).trim())
+        return lines
+      }
+    ),
+    tryGetConfigKeys: jest.fn(
+      async (
+        pattern: string,
+        globalConfig?: boolean,
+        configPath?: string
+      ): Promise<string[]> => {
+        const targetConfigPath =
+          configPath ||
+          (globalConfig
+            ? path.join(git.env['HOME'] || tempHomedir, '.gitconfig')
+            : localGitConfigPath)
+        const content = await fs.promises.readFile(targetConfigPath)
+        const lines = content
+          .toString()
+          .split('\n')
+          .filter(x => x)
+        const keys = lines
+          .filter(x => new RegExp(pattern).test(x.split(' ')[0]))
+          .map(x => x.split(' ')[0])
+        return [...new Set(keys)] // Remove duplicates
+      }
+    ),
     tryReset: jest.fn(),
     version: jest.fn()
   }
@@ -830,6 +1179,7 @@ async function setup(testName: string): Promise<void> {
 
 async function getActualSshKeyPath(): Promise<string> {
   let actualTempFiles = (await fs.promises.readdir(runnerTemp))
+    .filter(x => !x.startsWith('git-credentials-')) // Exclude credentials config file
     .sort()
     .map(x => path.join(runnerTemp, x))
   if (actualTempFiles.length === 0) {
@@ -843,6 +1193,7 @@ async function getActualSshKeyPath(): Promise<string> {
 
 async function getActualSshKnownHostsPath(): Promise<string> {
   let actualTempFiles = (await fs.promises.readdir(runnerTemp))
+    .filter(x => !x.startsWith('git-credentials-')) // Exclude credentials config file
     .sort()
     .map(x => path.join(runnerTemp, x))
   if (actualTempFiles.length === 0) {

__test__/git-directory-helper.test.ts

@@ -471,6 +471,7 @@ async function setup(testName: string): Promise<void> {
     configExists: jest.fn(),
     fetch: jest.fn(),
     getDefaultBranch: jest.fn(),
+    getSubmoduleConfigPaths: jest.fn(async () => []),
     getWorkingDirectory: jest.fn(() => repositoryPath),
     init: jest.fn(),
     isDetached: jest.fn(),
@@ -493,12 +494,15 @@ async function setup(testName: string): Promise<void> {
       return true
     }),
     tryConfigUnset: jest.fn(),
+    tryConfigUnsetValue: jest.fn(),
     tryDisableAutomaticGarbageCollection: jest.fn(),
     tryGetFetchUrl: jest.fn(async () => {
       // Sanity check - this function shouldn't be called when the .git directory doesn't exist
       await fs.promises.stat(path.join(repositoryPath, '.git'))
       return repositoryUrl
     }),
+    tryGetConfigValues: jest.fn(),
+    tryGetConfigKeys: jest.fn(),
     tryReset: jest.fn(async () => {
       return true
     }),

__test__/verify-submodules-recursive.sh

@@ -17,7 +17,7 @@ fi
 
 echo "Testing persisted credential"
 pushd ./submodules-recursive/submodule-level-1/submodule-level-2
-git config --local --name-only --get-regexp http.+extraheader && git fetch
+git config --local --includes --name-only --get-regexp http.+extraheader && git fetch
 if [ "$?" != "0" ]; then
     echo "Failed to validate persisted credential"
     popd

__test__/verify-submodules-true.sh

@@ -17,7 +17,7 @@ fi
 
 echo "Testing persisted credential"
 pushd ./submodules-true/submodule-level-1
-git config --local --name-only --get-regexp http.+extraheader && git fetch
+git config --local --includes --name-only --get-regexp http.+extraheader && git fetch
 if [ "$?" != "0" ]; then
     echo "Failed to validate persisted credential"
     popd