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

Only check owned repositories when running in owner mode #1585

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

Doridian
Copy link
Contributor

@Doridian Doridian commented Sep 6, 2022

Q A
πŸ› Bug fix? yes
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues N/A
❓ Documentation no

Description

Before this change, using the GitHub provider on your own user account (and not on an organization) would list repositories owned by other users that the current user had commit access to, such as being marked a collaborator (personal repositories, that is). This then caused driftctl to usually crash trying to access things it shouldn't (it would prefix the owner's username to someone else's repository name).

This fix ensures that only repositories owned by the actual user queried get taken into account.

IMO, there is no situation where the previous behavior would be desired, even if the "prefix the owner's name before other person's repository" bug was fixed.

@Doridian Doridian requested a review from a team as a code owner September 6, 2022 04:17
@Doridian Doridian requested review from wbeuil and removed request for a team September 6, 2022 04:17
@CLAassistant
Copy link

CLAassistant commented Sep 6, 2022

CLA assistant check
All committers have signed the CLA.

@Doridian Doridian changed the title Actually make sure to only list owned repos Only check owned repositories when running in owner mode Sep 6, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1585 (6372718) into main (b664f62) will decrease coverage by 3.95%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1585      +/-   ##
==========================================
- Coverage   82.59%   78.63%   -3.96%     
==========================================
  Files         226      219       -7     
  Lines        7113     6952     -161     
==========================================
- Hits         5875     5467     -408     
- Misses       1055     1288     +233     
- Partials      183      197      +14     
Impacted Files Coverage Ξ”
pkg/resource/azurerm/azurerm_image.go 12.50% <0.00%> (-87.50%) ⬇️
pkg/resource/azurerm/azurerm_resource_group.go 12.50% <0.00%> (-87.50%) ⬇️
pkg/resource/azurerm/azurerm_container_registry.go 12.50% <0.00%> (-87.50%) ⬇️
pkg/resource/google/google_compute_address.go 14.28% <0.00%> (-85.72%) ⬇️
...g/resource/google/google_compute_global_address.go 14.28% <0.00%> (-85.72%) ⬇️
...ce/google/google_compute_instance_group_manager.go 14.28% <0.00%> (-85.72%) ⬇️
pkg/resource/google/google_compute_disk.go 16.66% <0.00%> (-83.34%) ⬇️
pkg/resource/google/google_compute_image.go 16.66% <0.00%> (-83.34%) ⬇️
pkg/resource/google/google_bigquery_table.go 16.66% <0.00%> (-83.34%) ⬇️
pkg/resource/google/google_bigquery_dataset.go 16.66% <0.00%> (-83.34%) ⬇️
... and 39 more

Copy link
Contributor

@eliecharra eliecharra left a comment

Choose a reason for hiding this comment

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

Thanks a lot @Doridian that's a great catch πŸ™πŸ»

@eliecharra eliecharra merged commit b8b14bb into snyk:main Sep 6, 2022
@eliecharra
Copy link
Contributor

@all-contributors please add @Doridian for bug

@allcontributors
Copy link
Contributor

@eliecharra

I've put up a pull request to add @Doridian! πŸŽ‰

@Doridian Doridian deleted the fix/owner branch September 6, 2022 10:52
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.

4 participants