github / github/gh-gei

[gei] Secret scanning alert matching logic might need to be reversed for locations

Open
#1,238 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C#
Stars
478
Forks
146
Avg merge
7d 21h
Merged PRs (30d)
2

Description

🚨 Please Read Before Posting 🚨

You are currently in a PUBLIC REPOSITORY.

No Internal Information: Do not share links to internal Zendesk tickets, repositories, team names, or any internal
details.

Confidentiality Matters: Remember that this repository is visible to the public. Avoid posting anything that should
remain confidential or private.

## Description

For each secret scanning alert in a source repo, the code currently loops through all locations in that source alert, and expect to find a corresponding location in an alert in the target repo.
However, repo/org migration does not migrate everything, and therefore, the number of alerts and number of locations in each alert will always be a subset of what the source contains.
Therefore, the logic might need to be reversed and loop through all target locations instead of source locations, and consider a match if all target locations have a match in the source locations.

Below is a example:
- secret introduced in 5 different places in the code and sent for review
- secret scanning creates an alert with 5 locations
- reviewers asked author to rework the code to have only that value in 1 place instead of 5, amending or doing a commit on top
- when merging with a "squash and merge" strategy, the alert will still mention 5 locations from commits that were seen, even if not reachable any longer
- when migrating the repo, the target alert will only contain 1 location as these non-existing commits do not exist any more and are not migrated (same thing as if we were cloning as a mirror and trying to find the commit sha)

Here is another example scenario:
- a secret is committed to the source code in one file and is also mentioned in an issue.
- the source alert will have 2 locations.
- the user now edits the issue and remove the secret from the issue.
- the source alert will still have 2 locations.
- now doing the repo migration, as it does not migrate edit history of issues, only 1 location will be found in a corresponding target alert for that same secret.
- the current code will consider it is not a match and not migrate the resolution.

## Reproduction Steps

Copy pasting an extract of a test case added to a pull request that will be submitted for review/comments.
The following test case should pass:
```csharp

// Source should always have the same or more locations than the target
// assuming secret migration is done straight after the repo/org migration
var sourceLocations = new[] {
new GithubSecretScanningAlertLocation() {
Path = "my-file.txt",
StartLine = 17,
EndLine = 18,
StartColumn = 22,
EndColumn = 29,
BlobSha = "abc123"
},
new GithubSecretScanningAlertLocation() {
Path = "another-file.txt",
StartLine = 99,
EndLine = 103,
StartColumn = 22,
EndColumn = 29,
BlobSha = "def456"
}
};

// target location having on less location than source
var targetSecretLocation = new GithubSecretScanningAlertLocation()
{
Path = "my-file.txt",
StartLine = 17,
EndLine = 18,
StartColumn = 22,
EndColumn = 29,
BlobSha = "abc123"
};

// Add a check showing that this is a match and that the alert resolution will be migrated
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.