Merge f2a823c53a into f548e57e54

README.md

@@ -32,6 +32,8 @@ Only a single commit is fetched by default, for the ref/SHA that triggered the w
 
 The auth token is persisted in the local git config. This enables your scripts to run authenticated git commands. The token is removed during post-job cleanup. Set `persist-credentials: false` to opt-out.
 
+For token-authenticated submodules, checkout keeps its SSH-to-HTTPS URL rewrites in a temporary config alongside the submodule credentials. Both are removed during credential cleanup, without changing pre-existing URL rewrite rules. Unmarked local rewrites left by older checkout versions are not removed automatically because they cannot be distinguished from user configuration.
+
 When Git 2.18 or higher is not in your PATH, falls back to the REST API to download the files.
 
 ### Note

__test__/git-auth-helper.test.ts

@@ -30,6 +30,8 @@ jest.unstable_mockModule('@actions/core', () => ({
 jest.unstable_mockModule('../src/state-helper.js', () => ({
   setSshKeyPath: jest.fn(),
   setSshKnownHostsPath: jest.fn(),
+  setCredentialsConfigPaths: jest.fn(),
+  CredentialsConfigPaths: [],
   IsPost: false,
   RepositoryPath: ''
 }))
@@ -37,6 +39,7 @@ jest.unstable_mockModule('../src/state-helper.js', () => ({
 // Dynamic imports after mocking
 const core = await import('@actions/core')
 const gitAuthHelper = await import('../src/git-auth-helper.js')
+const stateHelper = await import('../src/state-helper.js')
 type IGitCommandManager =
   import('../src/git-command-manager.js').IGitCommandManager
 type IGitSourceSettings =
@@ -67,6 +70,7 @@ describe('git-auth-helper tests', () => {
 
   beforeEach(() => {
     jest.clearAllMocks()
+    stateHelper.CredentialsConfigPaths.length = 0
   })
 
   afterEach(() => {
@@ -557,10 +561,7 @@ describe('git-auth-helper tests', () => {
       await authHelper.configureSubmoduleAuth()
 
       // Assert
-      expect(mockSubmoduleForeach).toBeCalledTimes(1)
+      expect(mockSubmoduleForeach).not.toHaveBeenCalled()
-      expect(mockSubmoduleForeach.mock.calls[0][0] as string).toMatch(
-        /unset-all.*insteadOf/
-      )
     }
   )
 
@@ -590,10 +591,7 @@ describe('git-auth-helper tests', () => {
       await authHelper.configureSubmoduleAuth()
 
       // Assert
-      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(1)
+      expect(mockSubmoduleForeach).not.toHaveBeenCalled()
-      expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
-        /unset-all.*insteadOf/
-      )
     }
   )
 
@@ -616,16 +614,21 @@ describe('git-auth-helper tests', () => {
       await authHelper.configureSubmoduleAuth()
 
       // Assert
-      // Should configure insteadOf (2 calls for two values)
+      expect(mockSubmoduleForeach).not.toHaveBeenCalled()
-      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(3)
+      const credentialsFiles = await fs.promises.readdir(runnerTemp)
-      expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
+      const contents = await Promise.all(
-        /unset-all.*insteadOf/
+        credentialsFiles.map(file =>
+          fs.promises.readFile(path.join(runnerTemp, file), 'utf8')
+        )
       )
-      expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(
+      const submoduleConfig = contents.find(content =>
-        /url.*insteadOf.*git@github.com:/
+        content.includes('url.https://github.com/.insteadOf')
       )
-      expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(
+      expect(submoduleConfig).toContain(
-        /url.*insteadOf.*org-123456@github.com:/
+        'url.https://github.com/.insteadOf git@github.com:'
+      )
+      expect(submoduleConfig).toContain(
+        'url.https://github.com/.insteadOf org-123456@github.com:'
       )
     }
   )
@@ -656,11 +659,8 @@ describe('git-auth-helper tests', () => {
 
       // Assert
       // Should configure sshCommand (1 call)
-      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(2)
+      expect(mockSubmoduleForeach).toHaveBeenCalledTimes(1)
-      expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(
+      expect(mockSubmoduleForeach.mock.calls[0][0]).toMatch(/core\.sshCommand/)
-        /unset-all.*insteadOf/
-      )
-      expect(mockSubmoduleForeach.mock.calls[1][0]).toMatch(/core\.sshCommand/)
     }
   )
 
@@ -812,10 +812,11 @@ describe('git-auth-helper tests', () => {
   })
 
   const removeAuth_removesTokenFromSubmodules =
-    'removeAuth removes token from submodules'
+    'post removeAuth removes token and rewrites from submodules'
   it(removeAuth_removesTokenFromSubmodules, async () => {
     // Arrange
     await setup(removeAuth_removesTokenFromSubmodules)
+    settings.sshKey = ''
 
     // Create fake submodule config paths
     const submodule1Dir = path.join(workspace, '.git', 'modules', 'submodule-1')
@@ -844,14 +845,19 @@ describe('git-auth-helper tests', () => {
     let credentialsFiles = (await fs.promises.readdir(runnerTemp)).filter(
       f => f.startsWith('git-credentials-') && f.endsWith('.config')
     )
-    expect(credentialsFiles.length).toBe(1)
+    expect(credentialsFiles.length).toBe(2)
-    const credentialsFilePath = path.join(runnerTemp, credentialsFiles[0])
+    const submodule1GitDir = submodule1Dir.replace(/\\/g, '/')
+    const [credentialsFilePath] = await git.tryGetConfigValues(
+      `includeIf.gitdir:${submodule1GitDir}.path`,
+      false,
+      submodule1ConfigPath
+    )
+    expect(credentialsFilePath).toBeTruthy()
 
     // 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)
@@ -883,12 +889,16 @@ describe('git-auth-helper tests', () => {
       submodule2Content.indexOf(containerCredentialsPath)
     ).toBeGreaterThanOrEqual(0)
 
-    // Act - ensure mock persists for removeAuth
+    // Restore saved state in a fresh helper, as the post action does.
-    mockGetSubmoduleConfigPaths.mockResolvedValue([
+    const savedPaths = jest.mocked(stateHelper.setCredentialsConfigPaths).mock
-      submodule1ConfigPath,
+      .calls[1][0]
-      submodule2ConfigPath
+    expect(savedPaths.map(file => path.basename(file)).sort()).toEqual(
-    ])
+      credentialsFiles.sort()
-    await authHelper.removeAuth()
+    )
+    stateHelper.CredentialsConfigPaths.push(
+      ...JSON.parse(JSON.stringify(savedPaths))
+    )
+    await gitAuthHelper.createAuthHelper(git).removeAuth()
 
     // Assert submodule 1 includeIf entries removed
     submodule1Content = (
@@ -923,6 +933,42 @@ describe('git-auth-helper tests', () => {
     }
   })
 
+  it.each(['copy', 'rewrite'])(
+    'post removeAuth cleans up saved paths after submodule %s failure',
+    async stage => {
+      await setup(`post cleanup after ${stage} failure`)
+      settings.sshKey = ''
+      const authHelper = gitAuthHelper.createAuthHelper(git, settings)
+      await authHelper.configureAuth()
+      const [mainPath] = jest.mocked(stateHelper.setCredentialsConfigPaths).mock
+        .calls[0][0]
+      const error = new Error('Submodule setup failed')
+      if (stage === 'copy') {
+        jest.spyOn(fs.promises, 'copyFile').mockRejectedValueOnce(error)
+      } else {
+        jest.mocked(git.config).mockRejectedValueOnce(error)
+      }
+
+      await expect(authHelper.configureSubmoduleAuth()).rejects.toThrow(error)
+      const savedPaths = jest.mocked(stateHelper.setCredentialsConfigPaths).mock
+        .calls[1][0]
+      expect(savedPaths).toHaveLength(2)
+      expect(savedPaths[0]).toBe(mainPath)
+      const existingPaths = stage === 'copy' ? [mainPath] : savedPaths
+      expect((await fs.promises.readdir(runnerTemp)).sort()).toEqual(
+        existingPaths.map(file => path.basename(file)).sort()
+      )
+
+      // No includes are discoverable; cleanup must use only the saved state.
+      await fs.promises.writeFile(localGitConfigPath, '')
+      stateHelper.CredentialsConfigPaths.push(
+        ...JSON.parse(JSON.stringify(savedPaths))
+      )
+      await gitAuthHelper.createAuthHelper(git).removeAuth()
+      expect(await fs.promises.readdir(runnerTemp)).toEqual([])
+    }
+  )
+
   const removeGlobalConfig_removesOverride =
     'removeGlobalConfig removes override'
   it(removeGlobalConfig_removesOverride, async () => {

dist/index.js

@@ -34926,6 +34926,13 @@ const SshKeyPath = getState('sshKeyPath');
  * The SSH known hosts path for the POST action. The value is empty during the MAIN action.
  */
 const SshKnownHostsPath = getState('sshKnownHostsPath');
+/**
+ * Credentials config files owned by this action, including incomplete setup.
+ */
+const CredentialsConfigPaths = JSON.parse(getState('credentialsConfigPaths') || '[]');
+function setCredentialsConfigPaths(credentialsConfigPaths) {
+    saveState('credentialsConfigPaths', credentialsConfigPaths);
+}
 /**
  * Save the repository path so the POST action can retrieve the value.
  */
@@ -35051,6 +35058,7 @@ class GitAuthHelper {
     sshKnownHostsPath = '';
     temporaryHomePath = '';
     credentialsConfigPath = ''; // Path to separate credentials config file in RUNNER_TEMP
+    submoduleCredentialsConfigPath = '';
     constructor(gitCommandManager, gitSourceSettings) {
         this.git = gitCommandManager;
         this.settings = gitSourceSettings || {};
@@ -35135,11 +35143,15 @@ class GitAuthHelper {
         }
     }
     async configureSubmoduleAuth() {
-        // Remove possible previous HTTPS instead of SSH
-        await this.removeSubmoduleGitConfig(this.insteadOfKey);
         if (this.settings.persistCredentials) {
-            // Get the credentials config file path in RUNNER_TEMP
+            const credentialsConfigPath = this.getCredentialsConfigPath(!this.settings.sshKey);
-            const credentialsConfigPath = this.getCredentialsConfigPath();
+            if (!this.settings.sshKey) {
+                await external_fs_namespaceObject.promises.copyFile(this.getCredentialsConfigPath(), credentialsConfigPath);
+                // Keep rewrites with their credentials, without changing user config or main-repository URLs.
+                for (const insteadOfValue of this.insteadOfValues) {
+                    await this.git.config(this.insteadOfKey, insteadOfValue, false, true, credentialsConfigPath);
+                }
+            }
             // Container credentials config path
             const containerCredentialsPath = external_path_namespaceObject.posix.join('/github/runner_temp', external_path_namespaceObject.basename(credentialsConfigPath));
             // Get submodule config file paths.
@@ -35151,9 +35163,7 @@ class GitAuthHelper {
                 let submoduleGitDir = external_path_namespaceObject.dirname(configPath); // The config file is at .git/modules/submodule-name/config
                 submoduleGitDir = submoduleGitDir.replace(/\\/g, '/'); // Use forward slashes, even on Windows
                 // Configure host includeIf
-                await this.git.config(`includeIf.gitdir:${submoduleGitDir}.path`, credentialsConfigPath, false, // globalConfig?
+                await this.configureSubmoduleIncludeIf(`includeIf.gitdir:${submoduleGitDir}.path`, credentialsConfigPath, configPath);
-                false, // add?
-                configPath);
                 // Container submodule git directory
                 const githubWorkspace = process.env['GITHUB_WORKSPACE'];
                 external_assert_.ok(githubWorkspace, 'GITHUB_WORKSPACE is not defined');
@@ -35161,20 +35171,12 @@ class GitAuthHelper {
                 relativeSubmoduleGitDir = relativeSubmoduleGitDir.replace(/\\/g, '/'); // Use forward slashes, even on Windows
                 const containerSubmoduleGitDir = external_path_namespaceObject.posix.join('/github/workspace', relativeSubmoduleGitDir);
                 // Configure container includeIf
-                await this.git.config(`includeIf.gitdir:${containerSubmoduleGitDir}.path`, containerCredentialsPath, false, // globalConfig?
+                await this.configureSubmoduleIncludeIf(`includeIf.gitdir:${containerSubmoduleGitDir}.path`, containerCredentialsPath, configPath);
-                false, // add?
-                configPath);
             }
             if (this.settings.sshKey) {
                 // Configure core.sshCommand
                 await this.git.submoduleForeach(`git config --local '${SSH_COMMAND_KEY}' '${this.sshCommand}'`, this.settings.nestedSubmodules);
             }
-            else {
-                // Configure HTTPS instead of SSH
-                for (const insteadOfValue of this.insteadOfValues) {
-                    await this.git.submoduleForeach(`git config --local --add '${this.insteadOfKey}' '${insteadOfValue}'`, this.settings.nestedSubmodules);
-                }
-            }
         }
     }
     async removeAuth() {
@@ -35188,6 +35190,12 @@ class GitAuthHelper {
             await rmRF(this.temporaryHomePath);
         }
     }
+    async configureSubmoduleIncludeIf(key, value, configPath) {
+        const values = await this.git.tryGetConfigValues(key, false, configPath);
+        if (!values.includes(value)) {
+            await this.git.config(key, value, false, true, configPath);
+        }
+    }
     /**
      * Configures SSH authentication by writing the SSH key and known hosts,
      * and setting up the GIT_SSH_COMMAND environment variable.
@@ -35305,20 +35313,31 @@ class GitAuthHelper {
         }
     }
     /**
-     * Gets or creates the path to the credentials config file in RUNNER_TEMP.
+     * Gets or creates the path to a credentials config file in RUNNER_TEMP.
      * @returns The absolute path to the credentials config file
      */
-    getCredentialsConfigPath() {
+    getCredentialsConfigPath(forSubmodules = false) {
-        if (this.credentialsConfigPath) {
+        const existingPath = forSubmodules
-            return this.credentialsConfigPath;
+            ? this.submoduleCredentialsConfigPath
+            : this.credentialsConfigPath;
+        if (existingPath) {
+            return existingPath;
         }
         const runnerTemp = process.env['RUNNER_TEMP'] || '';
         external_assert_.ok(runnerTemp, 'RUNNER_TEMP is not defined');
         // Create a unique filename for this checkout instance
         const configFileName = `git-credentials-${(0,external_crypto_namespaceObject.randomUUID)()}.config`;
-        this.credentialsConfigPath = external_path_namespaceObject.join(runnerTemp, configFileName);
+        const configPath = external_path_namespaceObject.join(runnerTemp, configFileName);
-        core_debug(`Credentials config path: ${this.credentialsConfigPath}`);
+        if (forSubmodules) {
-        return this.credentialsConfigPath;
+            this.submoduleCredentialsConfigPath = configPath;
+        }
+        else {
+            this.credentialsConfigPath = configPath;
+        }
+        // Save before writing so post cleanup also handles incomplete setup or removed submodules.
+        setCredentialsConfigPaths([this.credentialsConfigPath, this.submoduleCredentialsConfigPath].filter(Boolean));
+        core_debug(`Credentials config path: ${configPath}`);
+        return configPath;
     }
     /**
      * Removes SSH authentication configuration by cleaning up SSH keys,
@@ -35364,7 +35383,11 @@ class GitAuthHelper {
         await this.removeGitConfig(this.tokenConfigKey);
         await this.removeSubmoduleGitConfig(this.tokenConfigKey);
         // Collect credentials config paths that need to be removed
-        const credentialsPaths = new Set();
+        const credentialsPaths = new Set([
+            this.credentialsConfigPath,
+            this.submoduleCredentialsConfigPath,
+            ...CredentialsConfigPaths
+        ].filter(Boolean));
         // Remove includeIf entries that point to git-credentials-*.config files
         info('Removing includeIf entries pointing to credentials config files');
         const mainCredentialsPaths = await this.removeIncludeIfCredentials();

src/git-auth-helper.ts

@@ -44,6 +44,7 @@ class GitAuthHelper {
   private sshKnownHostsPath = ''
   private temporaryHomePath = ''
   private credentialsConfigPath = '' // Path to separate credentials config file in RUNNER_TEMP
+  private submoduleCredentialsConfigPath = ''
 
   constructor(
     gitCommandManager: IGitCommandManager,
@@ -155,12 +156,27 @@ class GitAuthHelper {
   }
 
   async configureSubmoduleAuth(): Promise<void> {
-    // Remove possible previous HTTPS instead of SSH
-    await this.removeSubmoduleGitConfig(this.insteadOfKey)
-
     if (this.settings.persistCredentials) {
-      // Get the credentials config file path in RUNNER_TEMP
+      const credentialsConfigPath = this.getCredentialsConfigPath(
-      const credentialsConfigPath = this.getCredentialsConfigPath()
+        !this.settings.sshKey
+      )
+      if (!this.settings.sshKey) {
+        await fs.promises.copyFile(
+          this.getCredentialsConfigPath(),
+          credentialsConfigPath
+        )
+
+        // Keep rewrites with their credentials, without changing user config or main-repository URLs.
+        for (const insteadOfValue of this.insteadOfValues) {
+          await this.git.config(
+            this.insteadOfKey,
+            insteadOfValue,
+            false,
+            true,
+            credentialsConfigPath
+          )
+        }
+      }
 
       // Container credentials config path
       const containerCredentialsPath = path.posix.join(
@@ -181,11 +197,9 @@ class GitAuthHelper {
         submoduleGitDir = submoduleGitDir.replace(/\\/g, '/') // Use forward slashes, even on Windows
 
         // Configure host includeIf
-        await this.git.config(
+        await this.configureSubmoduleIncludeIf(
           `includeIf.gitdir:${submoduleGitDir}.path`,
           credentialsConfigPath,
-          false, // globalConfig?
-          false, // add?
           configPath
         )
 
@@ -203,11 +217,9 @@ class GitAuthHelper {
         )
 
         // Configure container includeIf
-        await this.git.config(
+        await this.configureSubmoduleIncludeIf(
           `includeIf.gitdir:${containerSubmoduleGitDir}.path`,
           containerCredentialsPath,
-          false, // globalConfig?
-          false, // add?
           configPath
         )
       }
@@ -218,14 +230,6 @@ class GitAuthHelper {
           `git config --local '${SSH_COMMAND_KEY}' '${this.sshCommand}'`,
           this.settings.nestedSubmodules
         )
-      } else {
-        // Configure HTTPS instead of SSH
-        for (const insteadOfValue of this.insteadOfValues) {
-          await this.git.submoduleForeach(
-            `git config --local --add '${this.insteadOfKey}' '${insteadOfValue}'`,
-            this.settings.nestedSubmodules
-          )
-        }
       }
     }
   }
@@ -243,6 +247,17 @@ class GitAuthHelper {
     }
   }
 
+  private async configureSubmoduleIncludeIf(
+    key: string,
+    value: string,
+    configPath: string
+  ): Promise<void> {
+    const values = await this.git.tryGetConfigValues(key, false, configPath)
+    if (!values.includes(value)) {
+      await this.git.config(key, value, false, true, configPath)
+    }
+  }
+
   /**
    * Configures SSH authentication by writing the SSH key and known hosts,
    * and setting up the GIT_SSH_COMMAND environment variable.
@@ -410,12 +425,15 @@ class GitAuthHelper {
   }
 
   /**
-   * Gets or creates the path to the credentials config file in RUNNER_TEMP.
+   * Gets or creates the path to a credentials config file in RUNNER_TEMP.
    * @returns The absolute path to the credentials config file
    */
-  private getCredentialsConfigPath(): string {
+  private getCredentialsConfigPath(forSubmodules = false): string {
-    if (this.credentialsConfigPath) {
+    const existingPath = forSubmodules
-      return this.credentialsConfigPath
+      ? this.submoduleCredentialsConfigPath
+      : this.credentialsConfigPath
+    if (existingPath) {
+      return existingPath
     }
 
     const runnerTemp = process.env['RUNNER_TEMP'] || ''
@@ -423,10 +441,21 @@ class GitAuthHelper {
 
     // Create a unique filename for this checkout instance
     const configFileName = `git-credentials-${randomUUID()}.config`
-    this.credentialsConfigPath = path.join(runnerTemp, configFileName)
+    const configPath = path.join(runnerTemp, configFileName)
+    if (forSubmodules) {
+      this.submoduleCredentialsConfigPath = configPath
+    } else {
+      this.credentialsConfigPath = configPath
+    }
+    // Save before writing so post cleanup also handles incomplete setup or removed submodules.
+    stateHelper.setCredentialsConfigPaths(
+      [this.credentialsConfigPath, this.submoduleCredentialsConfigPath].filter(
+        Boolean
+      )
+    )
 
-    core.debug(`Credentials config path: ${this.credentialsConfigPath}`)
+    core.debug(`Credentials config path: ${configPath}`)
-    return this.credentialsConfigPath
+    return configPath
   }
 
   /**
@@ -476,7 +505,13 @@ class GitAuthHelper {
     await this.removeSubmoduleGitConfig(this.tokenConfigKey)
 
     // Collect credentials config paths that need to be removed
-    const credentialsPaths = new Set<string>()
+    const credentialsPaths = new Set(
+      [
+        this.credentialsConfigPath,
+        this.submoduleCredentialsConfigPath,
+        ...stateHelper.CredentialsConfigPaths
+      ].filter(Boolean)
+    )
 
     // Remove includeIf entries that point to git-credentials-*.config files
     core.info('Removing includeIf entries pointing to credentials config files')

src/state-helper.ts

@@ -25,6 +25,17 @@ export const SshKeyPath = core.getState('sshKeyPath')
  */
 export const SshKnownHostsPath = core.getState('sshKnownHostsPath')
 
+/**
+ * Credentials config files owned by this action, including incomplete setup.
+ */
+export const CredentialsConfigPaths: string[] = JSON.parse(
+  core.getState('credentialsConfigPaths') || '[]'
+)
+
+export function setCredentialsConfigPaths(credentialsConfigPaths: string[]) {
+  core.saveState('credentialsConfigPaths', credentialsConfigPaths)
+}
+
 /**
  * Save the repository path so the POST action can retrieve the value.
  */