Persist creds to a separate file #2286

ericsciple · 2025-10-14T15:20:41Z

No description provided.

ericsciple · 2025-10-17T17:41:13Z

src/git-auth-helper.ts

  async configureGlobalAuth(): Promise<void> {
    // 'configureTempGlobalConfig' noops if already set, just returns the path
-    const newGitConfigPath = await this.configureTempGlobalConfig()
+    await this.configureTempGlobalConfig()


The path to the global config was previously needed to replace the placeholder-token. Now the token is written to a separate file, not the global config file directly.

Note, the placeholder-token is related to avoiding the actual token on the command line, when calling the git CLI.

ericsciple · 2025-10-17T17:45:04Z

src/git-auth-helper.ts

  async configureSubmoduleAuth(): Promise<void> {
    // Remove possible previous HTTPS instead of SSH
-    await this.removeGitConfig(this.insteadOfKey, true)
+    await this.removeSubmoduleGitConfig(this.insteadOfKey)


For readability purposes I split the existing signature into separate functions.

Old:

private async removeGitConfig(configKey: string, submoduleOnly: boolean = false): Promise<void>

New:

private async removeGitConfig(configKey: string): Promise<void> private async removeSubmoduleGitConfig(configKey: string): Promise<void>

ericsciple · 2025-10-17T18:19:06Z

src/git-auth-helper.ts

+    await this.removeSubmoduleGitConfig(this.insteadOfKey)

    if (this.settings.persistCredentials) {
-      // Configure a placeholder value. This approach avoids the credential being captured


The code to get the submodule git-config file paths moved into this.git.getSubmoduleConfigPaths(...)

ericsciple · 2025-10-17T18:22:11Z

src/git-auth-helper.ts

+      // Configure both host and container paths to support Docker container actions.
      for (const configPath of configPaths) {
-        core.debug(`Replacing token placeholder in '${configPath}'`)
-        await this.replaceTokenPlaceholder(configPath)


Previously the token was written directly to the submodule git config file (refer submoduleForeach command above).

Now the token is written to a separate config file under RUNNER_TEMP, which gets "included"

ericsciple · 2025-10-17T18:28:45Z

src/git-auth-helper.ts

      if (!this.settings.sshKey) {
        for (const insteadOfValue of this.insteadOfValues) {
-          await this.git.config(this.insteadOfKey, insteadOfValue, true, true)
+          await this.git.config(


No change here, I just added comments for readability

ericsciple · 2025-10-17T18:34:34Z

src/git-auth-helper.ts

-    await this.replaceTokenPlaceholder(configPath || '')
-  }
-
-  private async replaceTokenPlaceholder(configPath: string): Promise<void> {


Since this is only called from one place and is a relatively tight block of code, I inclined it into the configureToken function

ericsciple · 2025-10-17T18:39:50Z

src/git-auth-helper.ts

+    // Add include or includeIf to reference the credentials config
+    if (globalConfig) {
+      // Global config file is temporary
+      await this.git.config(


The global config file is temporary to clone the repo and submodules. So in this case include is used.

Whereas includeIf is used when persisting the credentials to the local git config. One includeIf for the host path and one includeIf to support container-actions.

ericsciple · 2025-10-17T18:43:25Z

src/git-auth-helper.ts

    const keyPath = this.sshKeyPath || stateHelper.SshKeyPath
    if (keyPath) {
      try {
+        core.info(`Removing SSH key '${keyPath}'`)


Better logging

ericsciple · 2025-10-17T18:44:53Z

src/git-auth-helper.ts

+    core.info('Removing HTTP extra header')
    await this.removeGitConfig(this.tokenConfigKey)
-  }
+    await this.removeSubmoduleGitConfig(this.tokenConfigKey)


For readability purposes I split the existing signature into separate functions.

Old:

private async removeGitConfig(configKey: string, submoduleOnly: boolean = false): Promise<void>

New:

private async removeGitConfig(configKey: string): Promise<void> private async removeSubmoduleGitConfig(configKey: string): Promise<void>

ericsciple · 2025-10-17T18:45:50Z

src/git-auth-helper.ts

  private async removeToken(): Promise<void> {
-    // HTTP extra header
+    // Remove HTTP extra header
+    core.info('Removing HTTP extra header')


Still removing the extraheader in case it was left behind by previous actions/checkout versions. For example if a machine was abruptly rebooted and cleanup never ran on the previous job.

ericsciple · 2025-10-17T18:47:37Z

src/git-auth-helper.ts

-        core.warning(`Failed to remove '${configKey}' from the git config`)
+    // Remove credentials config files
+    for (const credentialsPath of credentialsPaths) {
+      // Only remove credentials config files if they are under RUNNER_TEMP


Only the host file path needs to be attempted, not the container file path

Better to cleanup the file, even though the runner should clean up RUNNER_TEMP

ericsciple · 2025-10-17T18:49:21Z

src/git-auth-helper.ts

+   * Removes a git config key from the local repository config.
+   * @param configKey The git config key to remove
+   */
+  private async removeGitConfig(configKey: string): Promise<void> {


This isn't new, but it did change slightly. Refer line 353 on the left side.

Copilot

Pull Request Overview

This PR refactors Git credential management to persist credentials in a separate config file instead of directly in the repository's .git/config. The change improves security by isolating credentials from the main Git configuration and uses Git's includeIf mechanism to conditionally include the credentials when needed.

Key changes:

Moves credential storage from .git/config to a separate file in RUNNER_TEMP
Uses Git's includeIf.gitdir feature to conditionally include credentials based on Git directory path
Updates both main repository and submodule authentication to use the new approach

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/git-command-manager.ts	Adds new methods for config file operations and `configFile` parameter support
src/git-auth-helper.ts	Refactors credential management to use separate config files with `includeIf` entries
test/verify-submodules-true.sh	Updates test to use `--includes` flag when checking for credentials
test/verify-submodules-recursive.sh	Updates test to use `--includes` flag when checking for credentials
test/git-directory-helper.test.ts	Adds mock implementations for new git command manager methods
test/git-auth-helper.test.ts	Updates tests to verify new credential file structure and behavior
.github/workflows/test.yml	Adjusts workflow paths to avoid conflicts with new checkout behavior

Comments suppressed due to low confidence (1)

src/git-auth-helper.ts:1

The test name configureAuth_AcceptsGitHubServerUrlSetToGHEC doesn't match the test description 'inject https://github.com as github server url'. Consider using a more descriptive name like configureAuth_AcceptsGitHubServerUrl to match the actual test purpose.

import * as assert from 'assert'

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/git-auth-helper.ts

__test__/git-auth-helper.test.ts

TingluoHuang · 2025-10-17T19:35:58Z

.github/workflows/test.yml

          fi
-
-      # needed to make checkout post cleanup succeed
-      - name: Fix Checkout


we no longer needs this?

We used to stomp over the cloned repo. I fixed that with separate path inputs

ericsciple · 2025-10-20T15:20:12Z

.github/workflows/test.yml

  test-output:
    runs-on: ubuntu-latest
    steps:
      # Clone this repo


In addition to the PR checks and unit tests, here is the workflow I used for testing

on: workflow_dispatch: push: paths: - .github/workflows/test-checkout.yml jobs: basic: if: false strategy: matrix: runs-on: - ubuntu-latest - macos-latest - windows-latest runs-on: ${{ matrix.runs-on }} defaults: run: shell: bash steps: - uses: &checkout actions/checkout@users/ericsciple/25-10-cred - run: | cat .git/config - run: | git config list - run: | set -eo pipefail git ls-remote origin | head cleanup: runs-on: self-hosted # container: bitnami/git:latest steps: - uses: *checkout with: ref: submodule-test submodules: recursive - name: Find all files run: | find . -name .git -prune -o -print - name: Verify submodule files exist run: | test -f ./submodule-test.txt test -f ./submodule-test-nested/submodule-test-nested.txt test -f ./submodule-test-nested/submodule-test-nested-nested/submodule-test-nested-nested.txt echo "All submodule files verified successfully" - name: Root - cat .git/config run: | cat .git/config - name: Root - git config list run: | git config list - name: submodule-test-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/config - name: submodule-test-nested - git config list run: | cd submodule-test-nested git config list - name: submodule-test-nested-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/modules/submodule-test-nested-nested/config - name: submodule-test-nested-nested - git config list run: | cd submodule-test-nested/submodule-test-nested-nested git config list - name: Root - git ls-remote origin run: | git fetch - name: submodule-test-nested - git ls-remote origin run: | cd submodule-test-nested git config --global --add safe.directory /workspaces/runner/_layout/_work/ericsciple-test/ericsciple-test/submodule-test-nested git fetch - name: submodule-test-nested-nested - git ls-remote origin run: | cd submodule-test-nested/submodule-test-nested-nested git config --global --add safe.directory /workspaces/runner/_layout/_work/ericsciple-test/ericsciple-test/submodule-test-nested/submodule-test-nested-nested git fetch container: if: false runs-on: ubuntu-latest container: bitnami/git:latest steps: - uses: *checkout - run: | cat .git/config - run: | git config list - run: | git config --global --add safe.directory /__w/ericsciple-test/ericsciple-test - run: | git fetch container-step: if: false runs-on: ubuntu-latest steps: - uses: *checkout - name: Print working directory uses: ./my-container-action with: command: pwd - name: Printenv uses: ./my-container-action with: command: printenv - name: List files uses: ./my-container-action with: command: ls -la - name: Cat .git/config uses: ./my-container-action with: command: cat .git/config - name: Git config list uses: ./my-container-action with: command: git config --list - name: List files in /github uses: ./my-container-action with: command: ls -la /github - name: List files in /github/runner_temp uses: ./my-container-action with: command: ls -la /github/runner_temp - name: List files in RUNNER_TEMP uses: ./my-container-action with: command: ls -la $RUNNER_TEMP - name: Git fetch uses: ./my-container-action with: command: git config --global --add safe.directory /github/workspace && git fetch not-persisted: if: false strategy: matrix: runs-on: - ubuntu-latest - macos-latest - windows-latest runs-on: ${{ matrix.runs-on }} defaults: run: shell: bash steps: - uses: *checkout with: persist-credentials: false - run: | cat .git/config - run: | git config list - run: | set +e set -o pipefail git ls-remote origin | head if [ $? -eq 0 ]; then echo 'Expected command to fail' exit 1 fi echo 'Command correctly failed' exit 0 submodules-true: if: false strategy: matrix: runs-on: - ubuntu-latest - macos-latest - windows-latest runs-on: ${{ matrix.runs-on }} defaults: run: shell: bash steps: - uses: *checkout with: ref: submodule-test submodules: true - name: Find all files run: | find . -name .git -prune -o -print - name: Verify submodule files exist run: | test -f ./submodule-test.txt test -f ./submodule-test-nested/submodule-test-nested.txt echo "All submodule files verified successfully" - name: Verify nested-nested file does not exist run: | if [ -f ./submodule-test-nested/submodule-test-nested-nested/submodule-test-nested-nested.txt ]; then echo "Error: nested-nested file should not exist with submodules: true" exit 1 fi echo "Verified nested-nested file does not exist" - name: Root - cat .git/config run: | cat .git/config - name: Root - git config list run: | git config list - name: submodule-test-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/config - name: submodule-test-nested - git config list run: | cd submodule-test-nested git config list - name: Root - git ls-remote origin run: | set -eo pipefail git ls-remote origin | head - name: submodule-test-nested - git ls-remote origin run: | set -eo pipefail cd submodule-test-nested git ls-remote origin | head submodules-recursive: if: false strategy: matrix: runs-on: - ubuntu-latest - macos-latest - windows-latest runs-on: ${{ matrix.runs-on }} defaults: run: shell: bash steps: - uses: *checkout with: ref: submodule-test submodules: recursive - name: Find all files run: | find . -name .git -prune -o -print - name: Verify submodule files exist run: | test -f ./submodule-test.txt test -f ./submodule-test-nested/submodule-test-nested.txt test -f ./submodule-test-nested/submodule-test-nested-nested/submodule-test-nested-nested.txt echo "All submodule files verified successfully" - name: Root - cat .git/config run: | cat .git/config - name: Root - git config list run: | git config list - name: submodule-test-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/config - name: submodule-test-nested - git config list run: | cd submodule-test-nested git config list - name: submodule-test-nested-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/modules/submodule-test-nested-nested/config - name: submodule-test-nested-nested - git config list run: | cd submodule-test-nested/submodule-test-nested-nested git config list - name: Root - git ls-remote origin run: | set -eo pipefail git ls-remote origin | head - name: submodule-test-nested - git ls-remote origin run: | set -eo pipefail cd submodule-test-nested git ls-remote origin | head - name: submodule-test-nested-nested - git ls-remote origin run: | set -eo pipefail cd submodule-test-nested/submodule-test-nested-nested git ls-remote origin | head not-persisted-submodules-recursive: if: false strategy: matrix: runs-on: - ubuntu-latest - macos-latest - windows-latest runs-on: ${{ matrix.runs-on }} defaults: run: shell: bash steps: - uses: *checkout with: ref: submodule-test submodules: recursive persist-credentials: false - name: Find all files run: | find . -name .git -prune -o -print - name: Verify submodule files exist run: | test -f ./submodule-test.txt test -f ./submodule-test-nested/submodule-test-nested.txt test -f ./submodule-test-nested/submodule-test-nested-nested/submodule-test-nested-nested.txt echo "All submodule files verified successfully" - name: Root - cat .git/config run: | cat .git/config - name: Root - git config list run: | git config list - name: submodule-test-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/config - name: submodule-test-nested - git config list run: | cd submodule-test-nested git config list - name: submodule-test-nested-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/modules/submodule-test-nested-nested/config - name: submodule-test-nested-nested - git config list run: | cd submodule-test-nested/submodule-test-nested-nested git config list - name: Root - git ls-remote origin run: | set +e set -o pipefail git ls-remote origin | head if [ $? -eq 0 ]; then echo 'Expected command to fail' exit 1 fi echo 'Command correctly failed' - name: submodule-test-nested - git ls-remote origin run: | set +e set -o pipefail cd submodule-test-nested git ls-remote origin | head if [ $? -eq 0 ]; then echo 'Expected command to fail' exit 1 fi echo 'Command correctly failed' - name: submodule-test-nested-nested - git ls-remote origin run: | set +e set -o pipefail cd submodule-test-nested/submodule-test-nested-nested git ls-remote origin | head if [ $? -eq 0 ]; then echo 'Expected command to fail' exit 1 fi echo 'Command correctly failed' container-submodules-recursive: if: false runs-on: ubuntu-latest container: bitnami/git:latest steps: - uses: *checkout with: ref: submodule-test submodules: recursive - name: Find all files run: | find . -name .git -prune -o -print - name: Verify submodule files exist run: | test -f ./submodule-test.txt test -f ./submodule-test-nested/submodule-test-nested.txt test -f ./submodule-test-nested/submodule-test-nested-nested/submodule-test-nested-nested.txt echo "All submodule files verified successfully" - name: Root - cat .git/config run: | cat .git/config - name: Root - git config list run: | git config list - name: submodule-test-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/config - name: submodule-test-nested - git config list run: | cd submodule-test-nested git config list - name: submodule-test-nested-nested - cat .git/config run: | cat .git/modules/submodule-test-nested/modules/submodule-test-nested-nested/config - name: submodule-test-nested-nested - git config list run: | cd submodule-test-nested/submodule-test-nested-nested git config list - name: Add safe directories run: | git config --global --add safe.directory /__w/ericsciple-test/ericsciple-test git config --global --add safe.directory /__w/ericsciple-test/ericsciple-test/submodule-test-nested git config --global --add safe.directory /__w/ericsciple-test/ericsciple-test/submodule-test-nested/submodule-test-nested-nested - name: Root - git fetch run: | git fetch - name: submodule-test-nested - git fetch run: | cd submodule-test-nested git fetch - name: submodule-test-nested-nested - git fetch run: | cd submodule-test-nested/submodule-test-nested-nested git fetch container-step-submodules-recursive: if: false runs-on: ubuntu-latest steps: - uses: *checkout with: path: actions-checkout - uses: *checkout with: ref: submodule-test submodules: recursive path: test - name: Find all files uses: ./actions-checkout/my-container-action with: command: find test -name .git -prune -o -print - name: Verify submodule files exist run: | test -f test/submodule-test.txt test -f test/submodule-test-nested/submodule-test-nested.txt test -f test/submodule-test-nested/submodule-test-nested-nested/submodule-test-nested-nested.txt echo "All submodule files verified successfully" - name: Root - cat .git/config uses: ./actions-checkout/my-container-action with: command: cat test/.git/config - name: Root - git config list uses: ./actions-checkout/my-container-action with: command: cd test && git config --list - name: submodule-test-nested - cat .git/config uses: ./actions-checkout/my-container-action with: command: cat test/.git/modules/submodule-test-nested/config - name: submodule-test-nested - git config list uses: ./actions-checkout/my-container-action with: command: cd test/submodule-test-nested && git config --list - name: submodule-test-nested-nested - cat .git/config uses: ./actions-checkout/my-container-action with: command: cat test/.git/modules/submodule-test-nested/modules/submodule-test-nested-nested/config - name: submodule-test-nested-nested - git config list uses: ./actions-checkout/my-container-action with: command: cd test/submodule-test-nested/submodule-test-nested-nested && git config --list - name: Root - git fetch uses: ./actions-checkout/my-container-action with: command: cd test && git config --global --add safe.directory /github/workspace/test && git fetch - name: submodule-test-nested - git fetch uses: ./actions-checkout/my-container-action with: command: cd test/submodule-test-nested && git config --global --add safe.directory /github/workspace/test/submodule-test-nested && git fetch - name: submodule-test-nested-nested - git fetch uses: ./actions-checkout/my-container-action with: command: cd test/submodule-test-nested/submodule-test-nested-nested && git config --global --add safe.directory /github/workspace/test/submodule-test-nested/submodule-test-nested-nested && git fetch

A couple more jobs testing ssh

ssh: if: false strategy: matrix: runs-on: - ubuntu-latest - macos-latest - windows-latest runs-on: ${{ matrix.runs-on }} defaults: run: shell: bash steps: - uses: actions/checkout@users/ericsciple/25-10-cred with: ssh-key: ${{ secrets.DEPLOY_KEY_OTHER_REPO }} repository: my-org/other-repo - run: | cat README.md - run: | git ls-remote origin | head ssh-not-persisted: strategy: matrix: runs-on: - ubuntu-latest - macos-latest - windows-latest runs-on: ${{ matrix.runs-on }} defaults: run: shell: bash steps: - uses: actions/checkout@users/ericsciple/25-10-cred with: ssh-key: ${{ secrets.DEPLOY_KEY_OTHER_REPO }} repository: my-org/other-repo persist-credentials: false - run: | cat README.md - run: | set +e git fetch if [ $? -eq 0 ]; then echo 'Expected command to fail' exit 1 fi echo 'Command correctly failed' exit 0

ericsciple commented Oct 17, 2025

View reviewed changes

ericsciple force-pushed the users/ericsciple/25-10-cred branch from a370417 to eddff11 Compare October 17, 2025 19:00

ericsciple marked this pull request as ready for review October 17, 2025 19:00

Copilot AI review requested due to automatic review settings October 17, 2025 19:00

Copilot AI reviewed Oct 17, 2025

View reviewed changes

src/git-auth-helper.ts Show resolved Hide resolved

__test__/git-auth-helper.test.ts Show resolved Hide resolved

TingluoHuang reviewed Oct 17, 2025

View reviewed changes

ericsciple commented Oct 20, 2025

View reviewed changes

Persist creds to a separate file

cb17bfb

ericsciple force-pushed the users/ericsciple/25-10-cred branch from eddff11 to cb17bfb Compare October 20, 2025 18:56

Persist creds to a separate file #2286

Are you sure you want to change the base?

Persist creds to a separate file #2286

Uh oh!

Conversation

ericsciple commented Oct 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericsciple Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericsciple Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericsciple Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ericsciple Oct 17, 2025 •

edited

Loading

ericsciple Oct 17, 2025 •

edited

Loading

ericsciple Oct 20, 2025 •

edited

Loading