### Overview As part of the [MVC work to implement pre-receive secret detection](https://gitlab.com/groups/gitlab-org/-/epics/11587), this epic focus on [creating a push check to run secrets detection on pushed code](https://gitlab.com/groups/gitlab-org/-/epics/11587#two-create-a-push-check-to-run-secrets-detection-scans-on-pushed-code). The aim here is to have the check running on every `git` push, for which the gem built in &11612 is invoked to scan blobs fetched from `gitaly` and the results being displayed to the user whether the push was succesful or failed. ### Proposal :point_right_tone2: _This is copied from https://gitlab.com/groups/gitlab-org/-/epics/11587#two-create-a-push-check-to-run-secrets-detection-scans-on-pushed-code._ :point_left_tone2: To detect secrets before git pushes, we [considered](https://gitlab.com/gitlab-org/gitlab/-/issues/422574#other-discussed-solutions) a number of approaches, namely: * Customer-built server-side hooks. * Using client-side pre-commit hooks. * Using [push checks](https://docs.gitlab.com/ee/development/internal_api/internal_api_allowed.html#push-checks) (invoked within the `/internal/allowed` API endpoint from a `pre-receive` git hook in `gitaly`). And we have agreed to move forward with the push checks approach. Please refer to the [rationale section](https://gitlab.com/gitlab-org/gitlab/-/issues/422574#rationale) to learn more. With that in mind, the plan is to create a new push check based on the work done in our earlier POCs: * https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131414+ * https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131583+ Such that: * We introduce a new push rule (e.g. `EE::GitLab::Checks::PushRules::SecretsCheck`). * We update `EE::Gitlab::Checks::PushRuleCheck` to run `SecretsCheck` along with other checks. * We enumerate through new blobs fetched from `gitaly` in the new push check. * We pass the blobs to `shush` gem for regex matching and: * If a secret is found, we raise a `::Gitlab::GitAccess::ForbiddenError` and display an error message with the failure. * If no secrets are matched, check passes successfully, and the push continues. There are a number of considerations to keep in mind though while developing the push check: **Enumerating through blobs** To enumerate through the blobs of a certain git push, we have two approaches: 1. `git-rev-list(1)-base` approach (via `Gitlab::Git::Repository#new_blobs` method) 1. `quarantine-directory-based` approach (via `Gitlab::GitalyClient::BlobService#list_all_blobs` method) The latter is the most efficient way to fetch blobs when a quarantine directory is available. A quarantine directory is where `git` stages new objects before applying the reference updates. The presence of the `GIT_OBJECT_DIRECTORY_RELATIVE` environment variable indicates that a quarantine directory is used. However, the former comes in handy when a quarantine directory isn't available, as `list_all_blobs` would then fetch all blobs of the repository instead of the new blobs. In such case, we need to fall back to using `new_blobs` (or its underlying method `list_blobs`) while passing the new revisions to the method to retrieve their corresponding blobs. Please read more on this topic in [this thread](https://gitlab.com/gitlab-org/gitlab/-/issues/423834#note_1556350434) and [this comment](https://gitlab.com/gitlab-org/gitlab/-/issues/422574#note_1526604961). Also, check `Gitlab::Checks::FileSizeCheck::HookEnvironmentAwareAnyOversizedBlobs` to see an example for how it works. **Limit scans to certain file sizes and types** As part of the [requirements](https://gitlab.com/gitlab-org/gitlab/-/issues/422574#secret-scanner-likely-based-on-httpsgitlabcomgitlab-orggitlab-issues413274), and to ensure secret detection isn't blocking `git` pushes, we have to consider limiting our scanning to certain files sizes and types. Since secrets are often included in configuration files, that tend to be small in size, it would be advisable to limit to scanning to file size below a certain threshold, this can be done by utilising either one of: * `Gitlab::Checks::FileSizeCheck::AnyOversizedBlobs` * `Gitlab::Checks::FileSizeCheck::HookEnvironmentAwareAnyOversizedBlobs` So that we dimiss large blobs from being sent over to `shush` for scanning to begin with. Another thing to consider is skipping binary files from scanning, which include lots of high entropy strings anyway, and might lead to false positives. **Handling false positives** To ensure customers could skip pre-receive secret detection, for example in cases where a developer need to commit a dummy secret that is used in tests, we decided to use a special commit message flag [similar to ](https://docs.gitlab.com/ee/ci/pipelines/#skip-a-pipeline)`[ci skip]`. This will happen as part of the push check, in which we will check for the presence of this flag (e.g. `[skip secret detection]`) in any of the commit messages for a certain `git` push. If the flag is present, the push check returns early, and no blobs will be sent to `shush` for scanning. Please check [this thread](https://gitlab.com/gitlab-org/gitlab/-/issues/423834#note_1553609088) and [related POC](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131583) for an example of how this might work. **Releasing the feature behind a feature flag** To ensure the gradual roll-out of this feature, we have agreed to put it behind a feature flag. Checking the flag will have to happen inside the push check, so that we only invoke `shush` gem if the flag is enabled for a [certain actor](https://docs.gitlab.com/ee/development/feature_flags/index.html#feature-actors) (e.g. a `project` or `group`). Please also check the process for [developing with feature flags](https://docs.gitlab.com/ee/development/feature_flags/index.html#feature-flags-in-gitlab-development) and [rolling them out](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/issue_templates/Feature%20Flag%20Roll%20Out.md).