ci(ci): remove git credentials after checkout

[Fdawgs]

This PR removes Git credentials after checkout as a security precaution by setting persist-credentials to false. They are not used after the initial checkout, and this stops them from accidentally leaking through a script; see related GitHub security post and related actions/checkout issue.

[github-actions]

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

[simoneb]

I'm not sure about this one. I assume that by default the action is checking out code via HTTPs, and if this comment is valid, this input does not have effect on HTTP auth, as GITHUB_TOKEN is used instead.

[Uzlopak]

@simoneb

If you have persist credentials set to true (default is true), then git is configured in the shell.

[simoneb]

@simoneb

If you have persist credentials set to true (default is true), then git is configured in the shell.

Link to the source that does this?

[Uzlopak]

https://github.com/actions/checkout?tab=readme-ov-file#checkout-v4

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.

When Git 2.18 or higher is not in your PATH, falls back to the REST API to download the files.

[simoneb]

I don't personally see a valid point to do this:

1. the token lives only for as long as the workflow is executing
2. the token allows to do only the operations that it's configured for, as set in the permissions configuration of the workflow. The current permission (which is typical for this type of workflow) only allows reading the repository contents, which doesn't carry any security risk

Considering that this change implies changing the defaults of an action provided by github which in my opinion already has sensible defaults, I don't see a strong reason to introduce this change.

It would be a very different story if the access was happening via SSH or if the token's permissions were wider than they are.

[Uzlopak]

We are currently taking care of potential supply chain attacks. Yes, the GITHUB_TOKEN lives only as long as the workflow runs. But this means, that still somebody could theoretically e.g. force push an empty branch as default branch to a remove repository by only doing few git command in bash.

So of course the GITHUB_TOKEN should be properly defined. But this PR basically makes it a little bit harder for an attacker.

[simoneb]

They can't because the token has readonly access to the repository contents.

[Copilot]

Pull Request Overview

This PR enhances CI security by disabling credential persistence after Git checkout to prevent potential credential leakage through scripts.

• Adds persist-credentials: false to the checkout action configuration

---

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

[Fdawgs]

@Uzlopak has pretty much said all there is to say about this.

For a real world example of why removing these is a good idea: ArtiPACKED: Hacking Giants Through a Race Condition in GitHub Actions Artifacts

[simoneb]

I tend to disagree because all the attacks that have been mentioned require that the token has extended privileges, which by default they don't. Hence, we'd be putting in place a mitigation for a problem that can't inadvertently leak into the repository unless somebody approves and merges a PR that increases the permissions allowed to the token. Hence, the mitigation is useless.

[Fdawgs]

@simoneb I know it's a redundant mitigation in our repos but it costs nothing to add it. As part of our involvement in the GitHub Secure Open Source Fund we have committed to adopting defence-in-depth changes like this even when the immediate risk is null. We're future proofing against the off chance someone does manage to sneak a commit in to up token permissions.

[simoneb]

Sure thing, I'm by no means going to block this, but besides future proofing you're also adding maintenance burden, so it's a compromise really.