shallow cloning + GitHub Action

[joeleonjr]

Description:

This proposed GitHub Action updates the existing action by implementing a few new features:

1. The action defaults to using git shallow cloning. In most CI/CD use cases, users only want to scan the code diff in a push or PR and not the entire repo. By performing a shallow clone, we only clone down the relevant commits (and files). From there we invoke TruffleHog to scan only code diffs, thus creating a more performant action.
2. The action includes git cloning (using the default actions/checkout@v3 action). Why? Two reasons: (A) user experience: users can now invoke this action in their GH workflow yaml files without any additional steps

jobs:
  test:
    runs-on: ubuntu-latest
    steps:
   - name: TruffleHog OSS
        id: trufflehog
        uses: trufflesecurity/trufflehog@v2
        with:
           extra_args: --only-verified

(B) Git shallow cloning requires a few computations (ex: count of commits in the push or PR). This is simpler for the user; otherwise, they'd have to follow a slightly convoluted set-up process.
3. The current action does not provide a sufficiently robust process to scan pushes to the default branch. In some workflows, engineers might push multiple commits inside one push to main. Given the current action's tutorial, I couldn't easily scan a push to main containing multiple commits. (Maybe just my user error though):

...
with:
    path: ./
    base: ${{ github.event.repository.default_branch }}
    head: HEAD

It appears that in order to scan a push with multiple commits, the user would need to provide the commit hash prior to the new commits into the base argument. There doesn't seem to be a straightforward way to do that in the current action.

The updated action will do this by default.

4. The action allows users to scan their entire repo by passing in the full_repo flag (conducting a full git clone). This is for users that want to scan more than just the code diff in their push or PR.
5. The action removes references to path, base and head, which seem to confuse some users.

Note: To prevent backwards compatibility issues, I propose we tag the original action v1 (it's currently main). Then we tag the new action v2. Users will still be able to use v1 with an always-updated version of TH OSS, since we invoke the latest docker image.

Comments and discussion are welcomed!! Also, I did my best to test this out, but it would be helpful for others to run this action inside larger repositories (alongside the existing action).

Checklist:

• Tests passing (make test-community)?
• Lint passing (make lint this requires golangci-lint)?

[rosecodym]

(1) and (3) seem like great improvements. I find myself skeptical of (2) and (5) - personally, I prefer CI actions that are explicit, granular, and as un-magical as possible. Batteries-included actions are great when they work, but as soon as they don't, I'm left with a paucity of options to debug.

But! I claim no authority for this opinion, and you interact with other developers who use this more than I do, so I'll defer to the judgment of the crowd.

[joeleonjr]

(1) and (3) seem like great improvements. I find myself skeptical of (2) and (5) - personally, I prefer CI actions that are explicit, granular, and as un-magical as possible. Batteries-included actions are great when they work, but as soon as they don't, I'm left with a paucity of options to debug.

But! I claim no authority for this opinion, and you interact with other developers who use this more than I do, so I'll defer to the judgment of the crowd.

I think that's a totally fair response. The two main set-up difficulties that I'm trying to abstract for users is: (1) conducting a git shallow clone, and (2) counting commits in pushes, so that the entire push can be easily scanned.

If the better option is to write up two new documentation sections and add a new flag (or two) to the existing GH Action structure, I'm happy to do that.

[codevbus]

2. The action includes git cloning (using the default actions/checkout@v3 action). Why? Two reasons: (A) user experience: users can now invoke this action in their GH workflow yaml files without any additional steps

This is the only one I have issue with. To echo @rosecodym; this is abstracting away the modularity/granularity of a composable CI/CD pipeline. Since we can generally assume most users will consume this as part of a larger pipeline, they're going to end up checking out their code twice.

That being said, it's also clear that's what's needed to abstract away the cloning options needed to make this work. I'm not sure there's an alternative and this may just need to be a compromise.

[zricethezav]

Thought: the term FULL REPO has the potential to confuse some folks. This is something I saw at GitLab and w/ gitleaks-action; people would be confused about whether they were scanning the filesystem or the full git history. I think it would be useful to either change the name from full_repo to full_git_history or document that full_repo is scanning the entire history.

[zricethezav]

Note: To prevent backwards compatibility issues, I propose we tag the original action v1 (it's currently main). Then we tag the new action v2. Users will still be able to use v1 with an always-updated version of TH OSS, since we invoke the latest docker image.

I think this is as good plan

[joeleonjr]

I searched on GitHub for instances of repos using TH's GH Action and found some interesting results:

1. There are many, many cases of workflows running TH on pushes, but they aren't actually scanning anything.

This is one common setup:

name: Security

on:
  push:
    branches:
      - master
  pull_request:

jobs:
  TruffleHog:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v3
        with:
          fetch-depth: 0

      - name: TruffleHog Scan
        uses: trufflesecurity/trufflehog@main
        with:
          path: ./
          base: ${{ github.event.repository.default_branch }}
          head: HEAD
          extra_args: --only-verified

Here's another one that I saw:

uses: trufflesecurity/trufflehog@main
        with:
          path: ./
          base: ${{ github.ref }}

In my testing, these do not actually scan anything. TH looks like it runs (which it technically does), but it scans 0 chunks of data. This seems like something that deserves immediate attention since users believe they're invoking TH, but they are not.

2. This repo scans pushes, but to do so, they are scanning the entire branch's history.

- name: scan-push
        uses: trufflesecurity/trufflehog@main
        if: ${{ github.event_name == 'push' }}
        with:
          path: ./
          base: ""
          head: ${{ github.ref_name }}
          extra_args: --debug --only-verified

Surprisingly, this same repo also calls out shallow cloning, saying in some cases TH will fail without an error if a shallow clone is used. I'm not sure why or how that would be the case.

[rosecodym]

@joeleonjr is this problem

There are many, many cases of workflows running TH on pushes, but they aren't actually scanning anything.

resolved by this pr?

[joeleonjr]

@joeleonjr is this problem

There are many, many cases of workflows running TH on pushes, but they aren't actually scanning anything.

resolved by this pr?

Yes and no. This PR addresses that issue; however, it uses a new set of flags. There's a whole set of folks using the current GH Action (and different flags), that would need to upgrade their action. Since most folks won't proactively do that, we probably should issue a patch for the current action that addresses this. One idea suggested was to check if the values passed into --since-commit and --branch are equal. If so, then scan the entire branch or repo, just to be safe.

I'll also note that @zricethezav shared the gitleaks implementation and this PR needs to address the workflow_dispatch and schedule event types in addition to push and pull_request.

Finally, I know folks were split on including the git cloning logic inside the action. Depending on where that ends up, this PR might need some small updates.

[joeleonjr]

Just pushed a major update to the PR. A few changes:

1. This PR no longer embeds git cloning and instead provides users with instructions for shallow cloning in the README.
2. It addresses the issue of users not scanning during pushes. If values inputted by a user for the base and head fields are equal, it will throw an error. The error message links to our README and explains that TruffleHog wouldn't have scanned anything since the git diff was null.
3. This PR is fully backward compatible, so no versioning needed. All prior flags still exist and work as expected.
4. Users can now implement the action with no flags. The default is to scan all code changes any time it runs (in main, a feature-branch, or any PR). If the action is invoked via workflow_dispatch or schedule, it will default to scanning the entire git repository history.

[rosecodym]

Just pushed a major update to the PR. A few changes:

1. This PR no longer embeds git cloning and instead provides users with instructions for shallow cloning in the README.
2. It addresses the issue of users not scanning during pushes. If values inputted by a user for the base and head fields are equal, it will throw an error. The error message links to our README and explains that TruffleHog wouldn't have scanned anything since the git diff was null.
3. This PR is fully backward compatible, so no versioning needed. All prior flags still exist and work as expected.
4. Users can now implement the action with no flags. The default is to scan all code changes any time it runs (in main, a feature-branch, or any PR). If the action is invoked via workflow_dispatch or schedule, it will default to scanning the entire git repository history.

Since (as you reported) many users were mistakenly scanning nothing, does that mean that their pipelines will suddenly stop completing successfully once this goes out? I'm not saying that's necessarily bad, I just want to be sure.

[joeleonjr]

Just pushed a major update to the PR. A few changes:

1. This PR no longer embeds git cloning and instead provides users with instructions for shallow cloning in the README.
2. It addresses the issue of users not scanning during pushes. If values inputted by a user for the base and head fields are equal, it will throw an error. The error message links to our README and explains that TruffleHog wouldn't have scanned anything since the git diff was null.
3. This PR is fully backward compatible, so no versioning needed. All prior flags still exist and work as expected.
4. Users can now implement the action with no flags. The default is to scan all code changes any time it runs (in main, a feature-branch, or any PR). If the action is invoked via workflow_dispatch or schedule, it will default to scanning the entire git repository history.

Since (as you reported) many users were mistakenly scanning nothing, does that mean that their pipelines will suddenly stop completing successfully once this goes out? I'm not saying that's necessarily bad, I just want to be sure.

Yes. It would look like this:

[zricethezav]

thought (non-blocking): I could see how users might want to scan a single commit. As a user I don't want to determine the range, I just want to provide a single commit. We could change the base==head behavior to scan base/head against closets parent commit.

Just a thought, this isn't blocking and we could revisit this down the line.

[joeleonjr]

Definitely a valid use case. I'm open to adjusting this. I think we should ship this asap since some folks aren't scanning commits atm and think they are. But I'm down to think through what the behavior should look like.

[joeleonjr]

Separately, I'm reviewing this code before merging in and I'm looking at this line if [ $base_commit == $head_commit ] ; then. Do you think there are folks that leave base equal to '' and head equal to '', so that they can trigger a full git repository history scan? @zricethezav

[zricethezav]

Just left a non-blocking comment for discussion. I think this PR is good to go. Huge huge huge improvement, thanks Joe!

[aldy505]

Hi @joeleonjr, it turns out this commit broke my workflow. I'm using self-hosted GitHub Actions runner for my organization, and I'm guarantee CI environment isolation by running the CI via Docker. This PR removes the Docker image and it fails on everyone who uses Docker container base on their GitHub Actions.

Here's my GitHub Actions output:

I'm not saying we should revert this PR, as from what I read this is a good direction going forward. What we can do is providing a documentation on the README that points out that the current quickstart for GitHub Actions won't work if you're using a container base.

A small reproduction would be this (you can run this on both self-hosted runner and SaaS -- or GitHub-hosted runners):

name: PR

on:
  pull_request:

jobs:
  ci:
    name: CI
    container: golang:bookworm # This is the part that defines the container base image
    runs-on: "ubuntu-latest"
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - name: TruffleHog OSS
        uses: trufflesecurity/trufflehog@main
        with:
          path: ./
          base: ${{ github.event.repository.default_branch }}
          head: HEAD
          extra_args: --debug --only-verified

[joeleonjr]

Hi @joeleonjr, it turns out this commit broke my workflow. I'm using self-hosted GitHub Actions runner for my organization, and I'm guarantee CI environment isolation by running the CI via Docker. This PR removes the Docker image and it fails on everyone who uses Docker container base on their GitHub Actions.

Here's my GitHub Actions output:

I'm not saying we should revert this PR, as from what I read this is a good direction going forward. What we can do is providing a documentation on the README that points out that the current quickstart for GitHub Actions won't work if you're using a container base.

A small reproduction would be this (you can run this on both self-hosted runner and SaaS -- or GitHub-hosted runners):

name: PR

on:
  pull_request:

jobs:
  ci:
    name: CI
    container: golang:bookworm # This is the part that defines the container base image
    runs-on: "ubuntu-latest"
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - name: TruffleHog OSS
        uses: trufflesecurity/trufflehog@main
        with:
          path: ./
          base: ${{ github.event.repository.default_branch }}
          head: HEAD
          extra_args: --debug --only-verified

Thanks for bringing this to our attention. I think I have a solution that would require an update to the TruffleHog action.yml file. I'm proposing a check to determine whether the action is already containerized. If so, then it will install golang, clone/build TruffleHog and then run TruffleHog. If not, it will just use the dockerized version.

Do you mind @aldy505 letting me know if this works for your use case?

jobs:
  ci1:
    name: CI
    container: golang:bookworm # This is the part that defines the container base image
    runs-on: "ubuntu-latest"
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - name: Install Golang (Running in Container)
        if: ${{ job.container != '' }}
        uses: actions/setup-go@v4
        with:
          go-version: '1.21'

      - name: Install/Compile TruffleHog (Running in Container)
        if: ${{ job.container != '' }}
        run: |
          git clone https://github.com/trufflesecurity/trufflehog.git
          cd trufflehog; go install; cd -
      - name: Run TruffleHog (Running in Container)
        if: ${{ job.container != '' }}
        run: |
          trufflehog git file://./ \
          --since-commit ${{ github.event.repository.default_branch }} \
          --branch 'HEAD' \
          --fail \
          --no-update \
          --github-actions \
          --debug --trace \

      - name: Run TruffleHog (with Docker)
        if: ${{ job.container == '' }}
        run: |
            docker run --rm -v "/tmp":/tmp -w /tmp \
            ghcr.io/trufflesecurity/trufflehog:latest \
            git file:///tmp/ \
            --since-commit \
            ${{ github.event.repository.default_branch }} \
            --branch \
            'HEAD' \
            --fail \
            --no-update \
            --github-actions \
            --debug --trace \
  

Note: If this works, then you'd be able to use the TruffleHog action just like you showed in the "small reproduction" you provided above.