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

Send the scanned repository's origin url as an extra header #747

Merged
merged 4 commits into from
Oct 30, 2023

Conversation

Paul-GitGuardian
Copy link
Contributor

@Paul-GitGuardian Paul-GitGuardian commented Sep 21, 2023

Compute the repository remote url. Pick one at random if several are found. This will allow a better connection between ggshield and GIM's database.
This will appear in the request headers: 'GGShield-Repository-URL': 'https://github.com/bridgecrewio/terragoat.git'

All verticals will send this header. For IAC/SCA, the repository found is based on the provided path in the command.
For Secrets:

  • No repository are sent for subcommands archive, docker, docker-archive, docset, pypi.
  • Current working directory is sent for subcommands ci, pre-commit, pre-push, pre-receive, range. This is the directory that is checked by check_git_dir.
  • For path subcommand, if the paths argument has exactly one element, it is sent. Otherwise cwd is sent.
  • For repo subcommand, the repository argument or the local clone path is sent, depending on the type of the provided argument.

@Paul-GitGuardian
Copy link
Contributor Author

@sylvain-baud-gg IMO sending the entire URL is better, as GIM can then extract the required information from it (repo name, type of repo...)

Copy link
Contributor

@sylvain-baud-gg sylvain-baud-gg left a comment

Choose a reason for hiding this comment

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

I only see a change on the scan diff . I think the iac_scan_all should also fetch the remote url and send it.

Some edge case to keep in mind :

  • Local repo only --> We want to send None
  • Case of repo containing sub repos. --> We want to send the repo linked to the path provided.
  • Launching the command inside a repoA to scan a repoB --> We want to send repoB

@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch from 29ee9e3 to 71f8c8a Compare September 21, 2023 14:53
@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch from 71f8c8a to d133243 Compare October 23, 2023 12:23
@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2023

Codecov Report

Merging #747 (f2ad1be) into main (20d0b01) will increase coverage by 0.18%.
Report is 2 commits behind head on main.
The diff coverage is 93.75%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #747      +/-   ##
==========================================
+ Coverage   91.68%   91.86%   +0.18%     
==========================================
  Files         154      154              
  Lines        6348     6380      +32     
==========================================
+ Hits         5820     5861      +41     
+ Misses        528      519       -9     
Flag Coverage Δ
unittests 91.86% <93.75%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
ggshield/cmd/hmsl/hmsl_common_options.py 100.00% <ø> (ø)
ggshield/cmd/iac/scan/all.py 100.00% <ø> (ø)
ggshield/cmd/iac/scan/diff.py 80.76% <ø> (+2.56%) ⬆️
ggshield/cmd/sca/scan/sca_scan_utils.py 87.23% <ø> (+8.51%) ⬆️
ggshield/cmd/secret/scan/ci.py 96.42% <100.00%> (+0.13%) ⬆️
ggshield/cmd/secret/scan/path.py 100.00% <ø> (ø)
ggshield/cmd/secret/scan/precommit.py 100.00% <100.00%> (ø)
ggshield/cmd/secret/scan/prepush.py 100.00% <100.00%> (ø)
ggshield/cmd/secret/scan/prereceive.py 93.54% <100.00%> (+0.10%) ⬆️
ggshield/cmd/secret/scan/range.py 100.00% <100.00%> (ø)
... and 3 more

... and 1 file with indirect coverage changes

@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch 4 times, most recently from a51d836 to 006853a Compare October 24, 2023 07:48
@Paul-GitGuardian
Copy link
Contributor Author

I suggest you review commits one by one

@Paul-GitGuardian Paul-GitGuardian requested review from sylvain-baud-gg and removed request for sylvain-baud-gg October 24, 2023 07:49
@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch 3 times, most recently from c681d44 to 8ae0002 Compare October 24, 2023 08:50
@Paul-GitGuardian Paul-GitGuardian marked this pull request as ready for review October 24, 2023 09:52
@Paul-GitGuardian
Copy link
Contributor Author

@agateau-gg thank you for your review on my other PR, this one became our priority for the next days. Can you check it please? As said above, it is easier commit by commit imo

@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch 2 times, most recently from b911e99 to a536937 Compare October 24, 2023 14:27
ggshield/core/scan/scan_context.py Show resolved Hide resolved
ggshield/utils/git_shell.py Outdated Show resolved Hide resolved
ggshield/utils/git_shell.py Outdated Show resolved Hide resolved
ggshield/utils/git_shell.py Show resolved Hide resolved
ggshield/utils/git_shell.py Outdated Show resolved Hide resolved
ggshield/utils/git_shell.py Outdated Show resolved Hide resolved
tests/unit/core/scan/test_scan_context.py Outdated Show resolved Hide resolved
tests/unit/core/scan/test_scan_context.py Outdated Show resolved Hide resolved
@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch 2 times, most recently from 1d23ca0 to cc0e04c Compare October 25, 2023 12:19
@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch 4 times, most recently from 5c07b61 to e96bb79 Compare October 25, 2023 14:29
@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch 3 times, most recently from 2dc67a6 to 785275b Compare October 26, 2023 08:44
ggshield/utils/git_shell.py Outdated Show resolved Hide resolved
@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch 2 times, most recently from 144a86f to c99790a Compare October 26, 2023 10:14
@Paul-GitGuardian Paul-GitGuardian force-pushed the pbes/IAC-350/send-repo-url-in-scan-request branch from c99790a to f2ad1be Compare October 26, 2023 11:10
@Paul-GitGuardian Paul-GitGuardian merged commit 38c916e into main Oct 30, 2023
22 checks passed
@Paul-GitGuardian Paul-GitGuardian deleted the pbes/IAC-350/send-repo-url-in-scan-request branch October 30, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants