BUG: contributor checks does not validate number of companies per contributor

[laurentsimon]

a contributor can forge their company association on GH. (tracked in another issue).
In addition, the number of companies is not verified by our code. That makes it easier for a single user to commit 5 PRs and add 3 companies to their profile, hence getting a top score.

We should only take a single company per user.

[github-actions]

Stale issue message

[justaugustus]

@singhsaurabh offered to take this up (ref: https://openssf.slack.com/archives/C0235AR8N2C/p1646951039408139)

[laurentsimon]

Awesome, thanks @singhsaurabh

[singhsaurabh]

Thank you @justaugustus @laurentsimon
Regards,

[singhsaurabh]

Hi @justaugustus I would like to know more about this issue.
Kindly let me know if we can huddle for sometime to understand the background of the issue.
Thank You

[laurentsimon]

There are 2 sides:

1. At the time we created the issue, we fetched the company's name from a field that users can update arbitrarily. So for example, users could set the field to "Google" even though they were not part of the Google org. I'm not 100% sure whether the current codebase has the same problem or not. You nee to check and see if you can forge these result... which is fun!

2. We don't enforce a number of companies per user. So as a user, I can add 10 companies and it would pass the check with high score. I think we should enforce 1-2 companies per user max. Or maybe even just 1 company.

Please let me know if this help or not.

[singhsaurabh]

Hi @laurentsimon, I see some settings in checks/contributors.go, is that somewhere I need to make the changes?

const (
minContributionsPerUser = 5
numberCompaniesForTopScore = 3
// CheckContributors is the registered name for Contributors.
CheckContributors = "Contributors"
)

Kindly guide me. Thank You

[singhsaurabh]

https://github.com/ossf/scorecard/blob/main/docs/checks.md#contributors

do I need to add companies in github profile and test it ?

[laurentsimon]

Hi @laurentsimon, I see some settings in checks/contributors.go, is that somewhere I need to make the changes?

const ( minContributionsPerUser = 5 numberCompaniesForTopScore = 3 // CheckContributors is the registered name for Contributors. CheckContributors = "Contributors" )

Kindly guide me. Thank You

I think this requires no changes

[laurentsimon]

https://github.com/ossf/scorecard/blob/main/docs/checks.md#contributors

do I need to add companies in github profile and test it ?

yes it would be great to see if this is forgeable or not.

[singhsaurabh]

Thank You @laurentsimon

[leec94]

Hi! I'm looking into this issue. Is this issue still a concern/relevant?

Did some test runs and looks like Scorecard looks at both the Company field and Organizations that a user is a part of.
For a repository that was just mine, Scorecard reported 2 companies because I'm part of the IBM org and I added "IBM" to the company field.
Sorry to single @naveensrinivasan out, but his profile was a good example for a test too. Because he's a part of 6 organizations, Scorecard returned 6 companies found on his repo https://github.com/naveensrinivasan/naveensrinivasan.

My question here is what should Scorecard be looking at for company? Company field can be easily forged, but looking at Org may cause many results. We could look at only the first Org that's selected, but it may not be the company the contributor is actually working for.

[LappleApple]

Reviewed in community backlog refinement: Not clear whether we need to generate the unique set or pick one org per user and create a set from that. Needs more discussion.

[klbynum]

Hey @spencerschrock, my team (@SilasVM & @Jordin221) and I have been looking into this issue. We noticed this bug is related to issues #3996 and #4175. We also noticed you closed PR #3673 and mentioned it is not a high priority bug. We were wondering if we should work on this or look into another good first issue.

Thank you!

[spencerschrock]

Hey @spencerschrock, my team (@SilasVM & @Jordin221) and I have been looking into this issue. We noticed this bug is related to issues #3996 and #4175. We also noticed you closed PR #3673 and mentioned it is not a high priority bug. We were wondering if we should work on this or look into another good first issue.

Thank you!

So the Contributors check in general is very much on the heuristic end of analysis, and is game-able from a few different angles. I think the problem is trickier than it seems if we want to avoid edge cases, as it's an optimization problem. There will likely be more complex edge cases when the companies overlap more between contributors.

I think there are some more straightforward issues to tackle if you wanted a first issue, but up to you.

#4029, #4030, #3915, #3832, #3701, #2174