Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix scans of shallow clones such as those made by GitHub Actions #270

Merged
merged 8 commits into from Nov 12, 2021

Conversation

tarkatronic
Copy link
Contributor

@tarkatronic tarkatronic commented Nov 5, 2021

To help us get this pull request reviewed and merged quickly, please be sure to include the following items:

  • Tests (if applicable)
  • Documentation (if applicable)
  • Changelog entry
  • A full explanation here in the PR description of the work done

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Backward Compatibility

Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the API in any way?

  • Yes (backward compatible)
  • No (breaking changes)

Issue Linking

Fixes #209

What's new?

This slightly changes the way we gather branches and commits so that we can more easily detect when we are in a scenario where there is no history or local branches. When that scenario is detected, we examine the entire current contents of the repository (the HEAD) as though it were a single commit.

In most cases, this code will never be encountered nor run. The primary thing this enables is for tartufo to run natively in an environment created by the actions/checkout GitHub Action. Specifically, an environment with a max-depth=1 meaning no cloned history, and also no local refs or branches, with a detached HEAD.

At this point in time, a few tests still need to be updated, but I wanted to open this PR to show my work so far and get more eyes on it to make sure I'm not missing any obviously glaring points.

@tarkatronic tarkatronic marked this pull request as ready for review November 9, 2021 22:21
@tarkatronic tarkatronic requested a review from a team as a code owner November 9, 2021 22:21
@tarkatronic tarkatronic requested review from jmink-godaddy, rscottbailey and rfox-godaddy and removed request for a team November 9, 2021 22:21
@tarkatronic tarkatronic changed the title WIP: Fix scans of shallow clones such as those made by GitHub Actions Fix scans of shallow clones such as those made by GitHub Actions Nov 9, 2021
Copy link
Contributor

@rbailey-godaddy rbailey-godaddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Out of morbid curiosity, what is the effective difference between this (presumably a scan-local-repo) and scan-folder on the working directory? Probably just that we've instantiated the wrong scanner class by the time you realize this and can't change tracks. ;-)

@tarkatronic
Copy link
Contributor Author

Looks good to me.

Out of morbid curiosity, what is the effective difference between this (presumably a scan-local-repo) and scan-folder on the working directory? Probably just that we've instantiated the wrong scanner class by the time you realize this and can't change tracks. ;-)

Yeah, I hadn't thought about that, but realistically they're about the same thing. Although the scan-folder would also scan the .git folder and anything listed in the .gitignore, whereas this would not scan either.

Good callout!

Copy link
Contributor

@sushantmimani sushantmimani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🦅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When no refs/branches are present, tartufo doesn't actually scan anything
4 participants