Recursively clean git credentials from all checkouts in workspace and /tmp/#17086
Recursively clean git credentials from all checkouts in workspace and /tmp/#17086
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… checkouts in workspace and /tmp/ Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧪 Smoke Project is now testing project operations... |
|
🧪 Smoke Temporary ID is now testing temporary ID functionality... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke macOS ARM64 is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 DEVELOPING STORY: Smoke macOS ARM64 reports failed. Our correspondents are investigating the incident... |
|
@copilot add integration test |
|
✅ Smoke Project completed successfully. All project operations validated. |
|
🤖 Smoke test results for
Overall:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR improves security by recursively cleaning git credentials from all checkouts. The refactoring into clean_git_config() is clean. Minor suggestion: document the maxdepth 10 rationale.
📰 BREAKING: Report filed by Smoke Copilot for issue #17086
| WORKSPACE="${GITHUB_WORKSPACE:-.}" | ||
| GIT_CONFIG_PATH="${WORKSPACE}/.git/config" | ||
| # clean_git_config removes credentials from a single .git/config file | ||
| clean_git_config() { |
There was a problem hiding this comment.
Nice refactor extracting this into a reusable clean_git_config() function — makes the recursive logic much cleaner to follow.
| echo "ERROR: Git config file is corrupted after cleaning" | ||
| exit 1 | ||
| if [ "${CLEANED}" -eq 0 ]; then | ||
| echo "No .git/config files found, nothing to clean" |
There was a problem hiding this comment.
-maxdepth 10 is a reasonable guard against deep nesting, but consider documenting why 10 was chosen — or using a named constant like MAX_DEPTH=10 for clarity.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Smoke test (Codex) results
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added
All 13 assertions pass. |
Smoke Test Results — Run §22211157844Core tests (#1–#10): ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ — all passed PR review tests (#11–#17): ✅ ✅ ✅
Overall: PARTIAL — all non-skipped tests passed.
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal! The recursive credential cleaning approach looks solid. Good improvements to the clean_git_credentials.sh script.
💥 [THE END] — Illustrated by Smoke Claude for issue #17086
There was a problem hiding this comment.
Pull request overview
Updates the git-credential cleanup helper to recursively sanitize credentials across all git checkouts under the workspace and /tmp, and adds shell-based integration tests for the new behavior.
Changes:
- Refactors
clean_git_credentials.shto discover and clean multiple.git/configfiles (workspace +/tmp) via a per-config helper. - Adds
clean_git_credentials_test.shwith integration-style test cases for section removal, header removal, URL sanitization, and recursive discovery. - Includes unrelated additions: a workflow allowlist domain tweak and a smoke-test run marker file, plus a changeset entry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/sh/clean_git_credentials.sh | Recursively finds .git/config files in workspace and /tmp and sanitizes each one. |
| actions/setup/sh/clean_git_credentials_test.sh | Adds integration tests for credential removal and recursive discovery behavior. |
| .github/workflows/smoke-test-tools.lock.yml | Adds downloads.gradle-dn.com to the awf allow-domains list. |
| .smoke-test-22211157844 | Adds a smoke-test run marker file at repo root. |
| .changeset/patch-clean-git-credentials-recursive.md | Declares a patch changeset for the recursive credential cleaning update. |
Comments suppressed due to low confidence (1)
actions/setup/sh/clean_git_credentials_test.sh:54
- Test 1 assumes there are no
.git/configfiles anywhere, butclean_git_credentials.shnow scans/tmpas well as the workspace. On many environments/tmpmay already contain git checkouts, which will make the script print cleaning output (and not the "No .git/config" message), causing this test to be flaky. Consider adding an env var toclean_git_credentials.shto override/disable/tmpscanning for tests (e.g., configurable scan roots), or relax this assertion to only validate workspace behavior.
# ── Test 1: No .git/config anywhere (no-op, exit 0) ─────────────────────────
echo "Test 1: No .git/config files → exit 0 with informational message"
EMPTY_WS=$(mktemp -d)
OUTPUT=$(GITHUB_WORKSPACE="${EMPTY_WS}" bash "${CLEAN_SCRIPT}" 2>&1)
EXIT_CODE=$?
rmdir "${EMPTY_WS}"
assert "exits 0 when no .git/config found" "[ ${EXIT_CODE} -eq 0 ]"
assert "prints informational message" "echo '${OUTPUT}' | grep -q 'No .git/config'"
echo ""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # ── Test 6: Finds git repo in /tmp ────────────────────────────────────────── | ||
| echo "Test 6: Cleans git repo located in /tmp" | ||
| TMP_REPO=$(mktemp -d) | ||
| make_git_config "${TMP_REPO}" "[core] | ||
| repositoryformatversion = 0 | ||
| [credential] | ||
| helper = store" | ||
| # Use a workspace that does NOT contain the repo so it is found only via /tmp | ||
| GITHUB_WORKSPACE="${TEST_WORKSPACE}/workspace6" | ||
| mkdir -p "${GITHUB_WORKSPACE}" | ||
| GITHUB_WORKSPACE="${GITHUB_WORKSPACE}" bash "${CLEAN_SCRIPT}" >/dev/null 2>&1 | ||
| assert "tmp repo credential section cleaned" "! grep -q '\[credential\]' '${TMP_REPO}/.git/config'" | ||
| rm -rf "${TMP_REPO}" |
There was a problem hiding this comment.
TMP_REPO=$(mktemp -d) is not guaranteed to create the directory under /tmp (e.g., on macOS it often uses $TMPDIR). Since the script explicitly scans /tmp, this test can fail depending on the platform. Create the temp repo explicitly under /tmp (portable pattern: mktemp -d /tmp/gh-aw-clean-creds.XXXXXX).
This issue also appears on line 46 of the same file.
| # Collect all .git/config files to clean from workspace and /tmp/ | ||
| CLEANED=0 | ||
| while IFS= read -r git_config; do | ||
| clean_git_config "${git_config}" | ||
| CLEANED=$((CLEANED + 1)) | ||
| done < <(find "${WORKSPACE}" /tmp -maxdepth 10 -name "config" -path "*/.git/config" 2>/dev/null | sort -u) |
There was a problem hiding this comment.
If $GITHUB_WORKSPACE is under /tmp, find "${WORKSPACE}" /tmp ... will traverse the workspace twice (once via ${WORKSPACE}, once as a subtree of /tmp). sort -u dedupes results but doesn’t avoid the extra traversal. Consider conditionally pruning ${WORKSPACE} from the /tmp walk (or only scanning /tmp when it’s not a parent of ${WORKSPACE}), and also add -type f so only regular files are passed to git config.
See below for a potential fix:
WORKSPACE_ABS="$(cd "${WORKSPACE}" && pwd -P)"
# Collect all .git/config files to clean from workspace and /tmp/
CLEANED=0
while IFS= read -r git_config; do
clean_git_config "${git_config}"
CLEANED=$((CLEANED + 1))
done < <(find "${WORKSPACE_ABS}" /tmp -maxdepth 10 \( -path "${WORKSPACE_ABS}" -prune -o -type f -name "config" -path "*/.git/config" -print \) 2>/dev/null | sort -u)
| @@ -0,0 +1 @@ | |||
| Smoke test run 22211157844 | |||
There was a problem hiding this comment.
This file appears to be an ephemeral artifact from a specific smoke test run and isn’t referenced elsewhere in the repo. It will accumulate over time and add noise to the repository history; please remove it from the PR (or add an explicit convention + .gitignore entry if these files are intended to be tracked).
| set -o pipefail | ||
| sudo -E awf --env-all --container-workdir "${GITHUB_WORKSPACE}" --allow-domains '*.githubusercontent.com,*.jsr.io,*.pythonhosted.org,adoptium.net,anaconda.org,api.adoptium.net,api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.foojay.io,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,api.npms.io,api.nuget.org,api.snapcraft.io,archive.apache.org,archive.ubuntu.com,azure.archive.ubuntu.com,azuresearch-usnc.nuget.org,azuresearch-ussc.nuget.org,binstar.org,bootstrap.pypa.io,builds.dotnet.microsoft.com,bun.sh,cdn.azul.com,cdn.jsdelivr.net,central.sonatype.com,ci.dot.net,codeload.github.com,conda.anaconda.org,conda.binstar.org,crates.io,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,dc.services.visualstudio.com,deb.nodesource.com,deno.land,dist.nuget.org,dl.google.com,dlcdn.apache.org,dot.net,dotnet.microsoft.com,dotnetcli.blob.core.windows.net,download.eclipse.org,download.java.net,download.oracle.com,esm.sh,files.pythonhosted.org,get.pnpm.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,github.githubassets.com,go.dev,golang.org,googleapis.deno.dev,googlechromelabs.github.io,goproxy.io,gradle.org,host.docker.internal,index.crates.io,jcenter.bintray.com,jdk.java.net,json-schema.org,json.schemastore.org,jsr.io,keyserver.ubuntu.com,lfs.github.com,maven.apache.org,maven.google.com,maven.oracle.com,maven.pkg.github.com,nodejs.org,npm.pkg.github.com,npmjs.com,npmjs.org,nuget.org,nuget.pkg.github.com,nugetregistryv2prod.blob.core.windows.net,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,oneocsp.microsoft.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,pip.pypa.io,pkg.go.dev,pkgs.dev.azure.com,plugins-artifacts.gradle.org,plugins.gradle.org,ppa.launchpad.net,proxy.golang.org,pypi.org,pypi.python.org,raw.githubusercontent.com,registry.bower.io,registry.npmjs.com,registry.npmjs.org,registry.yarnpkg.com,repo.anaconda.com,repo.continuum.io,repo.gradle.org,repo.grails.org,repo.maven.apache.org,repo.spring.io,repo.yarnpkg.com,repo1.maven.org,s.symcb.com,s.symcd.com,security.ubuntu.com,services.gradle.org,skimdb.npmjs.com,static.crates.io,storage.googleapis.com,sum.golang.org,telemetry.enterprise.githubcopilot.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.java.com,www.microsoft.com,www.npmjs.com,www.npmjs.org,yarnpkg.com' --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --enable-host-access --image-tag 0.20.2 --skip-pull --enable-api-proxy \ | ||
| sudo -E awf --env-all --container-workdir "${GITHUB_WORKSPACE}" --allow-domains '*.githubusercontent.com,*.jsr.io,*.pythonhosted.org,adoptium.net,anaconda.org,api.adoptium.net,api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.foojay.io,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,api.npms.io,api.nuget.org,api.snapcraft.io,archive.apache.org,archive.ubuntu.com,azure.archive.ubuntu.com,azuresearch-usnc.nuget.org,azuresearch-ussc.nuget.org,binstar.org,bootstrap.pypa.io,builds.dotnet.microsoft.com,bun.sh,cdn.azul.com,cdn.jsdelivr.net,central.sonatype.com,ci.dot.net,codeload.github.com,conda.anaconda.org,conda.binstar.org,crates.io,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,dc.services.visualstudio.com,deb.nodesource.com,deno.land,dist.nuget.org,dl.google.com,dlcdn.apache.org,dot.net,dotnet.microsoft.com,dotnetcli.blob.core.windows.net,download.eclipse.org,download.java.net,download.oracle.com,downloads.gradle-dn.com,esm.sh,files.pythonhosted.org,get.pnpm.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,github.githubassets.com,go.dev,golang.org,googleapis.deno.dev,googlechromelabs.github.io,goproxy.io,gradle.org,host.docker.internal,index.crates.io,jcenter.bintray.com,jdk.java.net,json-schema.org,json.schemastore.org,jsr.io,keyserver.ubuntu.com,lfs.github.com,maven.apache.org,maven.google.com,maven.oracle.com,maven.pkg.github.com,nodejs.org,npm.pkg.github.com,npmjs.com,npmjs.org,nuget.org,nuget.pkg.github.com,nugetregistryv2prod.blob.core.windows.net,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,oneocsp.microsoft.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,pip.pypa.io,pkg.go.dev,pkgs.dev.azure.com,plugins-artifacts.gradle.org,plugins.gradle.org,ppa.launchpad.net,proxy.golang.org,pypi.org,pypi.python.org,raw.githubusercontent.com,registry.bower.io,registry.npmjs.com,registry.npmjs.org,registry.yarnpkg.com,repo.anaconda.com,repo.continuum.io,repo.gradle.org,repo.grails.org,repo.maven.apache.org,repo.spring.io,repo.yarnpkg.com,repo1.maven.org,s.symcb.com,s.symcd.com,security.ubuntu.com,services.gradle.org,skimdb.npmjs.com,static.crates.io,storage.googleapis.com,sum.golang.org,telemetry.enterprise.githubcopilot.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.java.com,www.microsoft.com,www.npmjs.com,www.npmjs.org,yarnpkg.com' --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --enable-host-access --image-tag 0.20.2 --skip-pull --enable-api-proxy \ | ||
| -- /bin/bash -c '/usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --add-dir "${GITHUB_WORKSPACE}" --disable-builtin-mcps --allow-all-tools --allow-all-paths --share /tmp/gh-aw/sandbox/agent/logs/conversation.md --prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"${GH_AW_MODEL_AGENT_COPILOT:+ --model "$GH_AW_MODEL_AGENT_COPILOT"}' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log |
There was a problem hiding this comment.
This PR is scoped to recursively cleaning git credentials, but this workflow change adds downloads.gradle-dn.com to the firewall allowlist. If this domain addition is required, please call it out in the PR description (or split it into a separate PR) so the security-impacting change is reviewed in the right context.
clean_git_credentials.shonly cleaned the single.git/configat the workspace root, leaving credentials intact in any additional git checkouts (submodules, tool-created clones, etc.) under$GITHUB_WORKSPACEor/tmp/.Changes
find "${WORKSPACE}" /tmp -maxdepth 10 -path "*/.git/config"instead of targeting a single hardcoded pathsort -uto avoid double-processing when workspace lives inside/tmp/clean_git_config()and called once per discovered configclean_git_credentials_test.shwith 7 test cases covering credential removal, URL cleaning, recursive discovery, and/tmp/scanningOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Changeset
clean_git_credentials.shdiscovers every.git/configunder the workspace and/tmp/, deduplicates the list, and reuses the helper while cleaning each file.✨ PR Review Safe Output Test - Run 22211157844