Create Private Repo creates public repository about visualstudio

[gaui]

👍

[shiftkey]

cc @thecarlo

[NickCraver]

I have just signed up for private repros and I can reproduce this, specifically from the Team Explorer -> Sync route. The other route of creating a repo correctly sets it as private.

[shiftkey]

@NickCraver thanks for the repro steps. I'm looking at this now to see if I can organize a fix.

[shiftkey]

I've also been able to recreate this. Talking with @haacked about how to solve this, but will open something soon with details.

[ghuntley]

Reddit discussion over at https://www.reddit.com/r/programming/comments/3j4ydl/how_a_bug_in_visual_studio_2015_exposed_my_source/

[shiftkey]

The updated build has landed on the gallery - https://visualstudiogallery.msdn.microsoft.com/75be44fb-0794-4391-8865-c3279527e97d.

[haacked]

First of all, I'm very sorry for this bug. It's inexcusable and I feel horrible.

As @shiftkey points out, we updated the extension with a fix on the Visual Studio Gallery and at https://visualstudio.github.com/.

The problem only affected publishing repositories, not in creating repositories which used pretty much the same code, but in a slightly different context.

You can see the fix in this PR: #64

Based on our preliminary investigation, checkboxes in a Team Explorer pane appear to behave differently than checkboxes in a standard WPF control. We're not sure why that is and we'll continue to investigate.

Once we have a root cause down, we'll post a post-mortem somewhere appropriate.

[niik]

We've deployed a change to the GitHub.com API that prevents new public repositories from being created by versions of the Visual Studio extension prior to 1.0.14.

Old versions of the extension will show an error message requesting users to update their extension along with an explanation on how to do so.

[shana]

This is the latest version of the message.

[NickCraver]

Kudos on getting multiple fixes out immediately after this came to light. The active blocking on older versions is a nice touch and absolutely the right call, good job all around.

I really do look forward to seeing why this doesn't behave correctly on the bindings as soon as it's figured out. I had it up in a debugger last night and couldn't figure out why the binding was failing only here either - good luck tracking it down.

[phillip-haydon]

That's a nice solution, to block the API for the old version. Well done!

[thecarlo]

Kudos on getting multiple fixes out immediately after this came to light. The active blocking on older versions is a nice touch and absolutely the right call, good job all around.

Yes well done guys for jumping on this so quick.

[shana]

Huge apologies to everyone affected by this, this is a crazy situation which should never happen. We're improving our processes to make sure this never happens again.

@NickCraver I've tracked it down, and it's a doozy - a combination of a bunch of circumstances plus a seemingly innocuous style change, all leading up to Bad Things Happening™

[NickCraver]

@shana Please do a writeup here or elsewhere as time allows. I know I and others are curious as to the causes here and would very much appreciate a technical explanation of what happened. If nothing else, it helps someone down the road from hitting the same thing.

[garvincasimir]

@niik Does your team have any plans for mitigating this type of thing in the future? Please permit me to make a few suggestions. I don't know how feasible they are but I think it is important to acknowledge that people make mistakes and do what we can to limit the impact of those mistakes.

• Add an option either per repo or account wide which allows an account admin to explicitly block any commits which contain privileged information (AWS Keys, Azure Storage Keys, SSL Private Keys).
• Allow for custom commit filters
• Send a courtesy warning email/text notifying a committer/account owner that one of their commits may contain private information.
• Implement some sort of delayed publish for commits suspected of containing privileged information. It could be as simple as an email saying "Hey we noticed your commit contains an AWS key so we held it back just in case you mistakenly committed it. If this was truly your intention please click here to finalize this commit."

I understand some people might see some of these options more as intrusions than solutions but I would say that an occasional intrusion that mitigates silly mistakes is a lot better than a $7,000 AWS bill.

[NickCraver]

@garvincasimir While I understand the sentiment, I don't think such actions are reasonable or effectual. Though to your second item, there may be something tractable with custom commit filters that's more usable & approachable to more users than commit hooks and such are now.

The other 3 hinge on determining "privileged information". What is privileged information? Just auth keys? Credit Cards? Personal phone numbers? How about bank accounts? Routing information? SNMP strings? What about username and passwords (common in test cases)? Social security numbers? Salaries? Emails? It's a very long list of candidates.

You can see how even choosing the list is an issue and reliably matching the list is hard to the point of being impossible without huge amounts of false positives. And all of that is to say nothing of things like test data which may include dummy info, connecting to public test databases or APIs, etc.

Given the above, even figuring out something bad was pushed is often a non-starter. It feels intrusive, which is what matters, whether it is or not. The most comparable case I can think of would be your gmail and how they scan for ads - that being both free and silent is a huge portion of why it's "acceptable". Compared to GitHub where you can pay for your private hosting, it's far less okay. The expectation there (IMO) is that GitHub is only hosting them, nothing more intrusive than that.

Now, with all of that said - that wasn't the problem here. There were 2 problems in play:

1. A private repo creation wasn't private (this issue).
2. Someone put sensitive information where they shouldn't have.

I think number 2 is a distinctly separate issue and not one that can be readily solved programmatically. This is a really bad practice in general, and it happens all the time. Can we better educate people about why it's a bad practice, and can GitHub possibly help there in the getting started documentation? Yeah, absolutely. I don't think most people would be opposed to better educating everyone around this.

Can the plugin do something to help here, by knowing the project type and config areas to look in? Probably, but again lots of false positives and lots of misses. For example a secret in the config vs. a constant somewhere. Furthermore, you wouldn't want to rely on a system that's fundamentally unreliable. Ultimately, this one is on the programmer. We have to help the programmer, not the code they commit.

[garvincasimir]

@NickCraver thanks for your reply. In line with my hope that we could give more time to a solution rather than bashing.

I appreciate that defining privileged info could potentially become a rabbit hole. And we also have the issue of false positives. That is why I suggested we limit the scope of things we check and make it easy for an account owner to turn it off altogether.

Ultimately I believe there is more that can be done in terms of mitigation without being too intrusive. In this case the dev had good reason to believe that his tools worked the way he expected. We run millions of lines of other people's code every day. It is impossible for even the most security conscious of us to validate all of it.

[haacked]

Thanks for the feedback everyone! I'm going to close this issue now that the fix has been released. In the meanwhile, we're working on a proper post-mortem. I'll give a few preliminary notes here.

If you have the GitHub Extension for Visual Studio installed, there are two ways to upgrade it.

You can download the updated VSIX from the Visual Studio Extension Gallery and double click on the VSIX to install it.

Or you can upgrade it directly from Visual Studio 2015 by clicking on the Tools > Extensions and Updates menu item to launch the Visual Studio Extension manager. From there click on the Updates node in the left column and select Visual Studio Gallery. From there, "GitHub Extension for Visual Studio" will be listed if you're not running the latest. You can select that and click the "Update" button.

The bug was introduced in version 1.0.9 released on June 23, 2015. It was fixed on August 31, 2015 in version 1.0.14. In addition to fixing the bug, we updated the API to prevent public repository creation for all versions prior to 1.0.14 to be safe. Users using older versions will receive a message suggesting they upgrade to the latest version.

The commit that introduced the bug was a XAML styling change that caused the problem in a very subtle manner. We'll write more details on this later as it'll take some explaining.

Our initial fix was a simple effective workaround to the bug. We're currently working on a proper fix to the root cause.

As for preventing this in the future, we are trying to take a comprehensive look at the conditions and systems that allowed this happen in the first place and how we can improve those systems to mitigate such issues in the future.

For example, we have good unit test coverage, but a unit test would not have caught this. Nor would an integration test. So we need to investigate the cost/benefit of attempting UI automation of Visual Studio (which can take a lot of time and effort) vs having a more comprehensive manual test plan for every release that we complete without fail for every release no matter how trivial seeming the change is.

We're still working through these issues and I'll follow up when we have something to share more formally.

[aferencz]

I don't know if it is a regression, new issue, or some obscure configuration issue, but with the latest build downloaded from https://visualstudio.github.com I cannot create a new private repository using the plugin and it also ignores my git ignore settings.

[shiftkey]

@aferencz please open a new issue with as much information as possible about what you're seeing. I don't believe this is related to the original issue, but would like to confirm this first.