Arcade: Support running CredScan during PR validation

[dagood]

• Is this issue blocking (yes/no)
• Is this issue causing unreasonable pain (yes/no)

Right now, a PR can get merged into GitHub, then the mirror into AzDO fails because CredScan detects credentials. Adding CredScan to PR validation would at least prevent blocking the build due to mirror failures.

(This should ideally be detected by the push to GitHub and reject the developer's push, so we're actually protected if it's a real secret, but it's useless to track that here.)

I see https://github.com/dotnet/core-eng/issues/5747 on core-eng, but it's unclear what it's tracking when I read the thread, so I'm filing this new issue to track it specifically and visibly.

We recently ran into this issue in dotnet/runtime with a false positive that blocked official builds. https://github.com/dotnet/runtime/pull/1993

[riarenas]

@sunandabalu There was a reason why we didn't enable this for PRs as there were some challenges with these tools running in the open. Could you refresh my memory why that was?

[sunandabalu]

@sunandabalu There was a reason why we didn't enable this for PRs as there were some challenges with these tools running in the open. Could you refresh my memory why that was?

As per the guidance from the security team, the SDL tools should not be run in open. That is the reason we run SDL in the internal builds only.

[dagood]

Can you elaborate on the connection between CredScan and SDL? Are you certain the guidance applies to running CredScan in particular without the overall SDL tooling?

[sunandabalu]

Can you elaborate on the connection between CredScan and SDL? Are you certain the guidance applies to running CredScan in particular without the overall SDL tooling?

The guidance was for all the SDL static analysis tools, not the infrastructure behind running them. CredScan is a type of SDL static analysis tool :)

[dagood]

Got it, thanks. That's very unfortunate.

[markwilkie]

Is there more to do here @dagood or should this issue be closed now?

[jonfortescue]

I'm going to close this since it's not possible atm. I've edited dotnet/core-eng#5747 to reflect what the issue actually is. That issue is triaged for the SDL Automation V2 epic, so it will be tracked there.

[JohnTortugo]

IMHO It's quite unfortunate that we don't have a validation to prevent pushing secrets/* to the open.

@sunandabalu - Isn't it possible to configure at least the CredScan tool to run in the open? What I'm thinking is: isn't there a flag or parameter to pass to the tool so that it could be used in public builds?

[riarenas]

I don't think it's a configuration issue, the way I interpret the response is that it's a policy issue that these tools should not run in public machines against public code?

@sunandabalu do you know perhaps how other orgs with code in GitHub are running these tools? do they all only run them against internal closed repos?

[dagood]

IMHO It's quite unfortunate that we don't have a validation to prevent pushing secrets/* to the open.

This wouldn't solve that, we'd need GitHub to support this natively and reject pushes like AzDO does. Running PR validation is far too late to prevent leaking secrets--it's already exposed at that point and would need to be cycled.

[dagood]

To be clear though, it would help. 🙂 The earlier the secret is detected and invalidated, the better.

[JohnTortugo]

@dagood - yeah, fully agree with you. Currently we kind of rely on someone spotting the secret in the PR, if no one see it..

[sunandabalu]

I don't think it's a configuration issue, the way I interpret the response is that it's a policy issue that these tools should not run in public machines against public code?

@sunandabalu do you know perhaps how other orgs with code in GitHub are running these tools? do they all only run them against internal closed repos?

Atleast within Microsoft, security related stuff are run against private repos. And yes, its not shortcoming of the tool, its the policy issue.

[dagood]

Could we have a system that downloads the tarball of the PR submitter's code (to be a little more secure maybe than Git clone) and marks the PR "reject" with a bot if it contains CredScan flags, while not providing any links to the security run and not executing any submitted code to avoid exposure?

(Checks API also seems perfectly acceptable for this.)

[jonfortescue]

@riarenas is correct; it's not a configuration issue but a policy one. To my knowledge, all the other repos are doing the same thing as us right now.

[sunandabalu]

Could we have a system that downloads the tarball of the PR submitter's code (to be a little more secure maybe than Git clone) and marks the PR "reject" with a bot if it contains CredScan flags, while not providing any links to the security run and not executing any submitted code to avoid exposure?

(Checks API also seems perfectly acceptable for this.)

We could possible achieve this with the Checks API, dotnet/core-eng#5747 was filed to find possible solutions to this problem.