I bet the post-mortem for this is going to be fun. How this wasn't covered by multiple unit tests, a code review and a roll out strategy is.... impressive.
That this was missed is pretty surprising, given that it's Google and the stakes involved in the encryption/key management code in a secure platform device.
I wonder if we'll even get a postmortem, as this simply cannot happen unless several someones all Seriously Fucked Up simultaneously.
- code error (expected, humans are fallible)
- review error (less expected, this is the kind of thing code review exists for)
- static analysis / linting check failure
- integration test failure (interactive login no longer works on this build)
This is a massive, overlapping fuckup. Heads should probably roll here, especially given that when you opt in to ChromeOS as a user, Google now has "one job" (DFIU).
Google has a blameless post-mortem process (see https://sre.google/sre-book/postmortem-culture/) which essentially boils down to:
0) We want to fix the situation. By the time the post-mortem happens, that should already be done.
1) We want to make sure this never happens again. In order to do that we need to understand as much as we can about what happened.
2) We want everyone to share everything they know about the event. If the participants worry about fallout, they will be compelled to cover up.
3) Most failures happen at a number of points, so you need to dig in deeply if you want to get full value out of the "event".
4) Good post-mortems lead to process fixes to reduce (or eliminate?) the chance of similar events, or to make fixes throughout the code base.
The other thing is that after the post-mortem, all of the folks involved have learned from it. Usually they're better engineers because of that expensive lesson you've just paid for.
Disclaimer: Googler, participant in several post-mortems, opinions my own.
One notes that even years later Google still haven't figured out why the Pixel C occasionally just forgot it's own passcode and encryption key and locked users out. Thousands of reports, and publicly acknowledged as an issue. Never addressed. Pixel devices have had multiple bootloader issues and are currently experiencing a widespread Widevine downgrade for months and not fixed.
// accidentally mutates all names to Sam and persists to the DB
myList.filter { myDomainObj.name = 'Sam' }
In C "if (foo = bar)" and "if (foo == bar)" are both legal, because pretty much any type can be automatically coerced into a boolean. In many modern languages that is not the case. (Of course if you have assignments as expressions you are still screwed if foo and bar are both boolean to begin with, so some languages got away with that, too. And for fairness I should say that C at the very least warns if you don't explicitly put parentheses around the assignment, nowadays.)
That's not true at all.
Whenever you're intending to compare equality, they're usually going to be the same type already unless you're being really slopping in a scripting language like JavaScript or PHP -- but even then they're usually two numbers, two strings, or two booleans.
The whole problem here is where foo and bar are both integers, or booleans, or whatever you want, and instead of typing "if (foo == bar)" you type "if (foo = bar)".
Strong typing does nothing whatsoever to help in this common situation.
Assignment shouldn't have a return type. Compound assignment operators? Likewise. You never need this return value, and sometimes you may use it by mistake. So, abolish it. In C++ not only do operators that shouldn't have a return value have one anyway, the type of that value might be anything. You can even overload both ++ operators to return different unrelated types so that
mytype aThing { blah };
auto a = aThing++; // returns a String "Fuck tha police"
auto b = ++aThing; // returns a double, 8.63
"Your scientists were so preoccupied with whether or not they could, that they didn't stop to think if they should"
Guido van Rossum wondered the same, which is why it isn't possible in Python. Same for "&&" vs. "&", since in Python it's "and" vs. "&".
The gradient of professionalism in our industry is very wide, and the slope is quite shallow.
To your point, however, many languages (such as Go, developed and used by Google, the organization under discussion) have designed their syntax now to completely avoid this type of error even being possible.
I mean its still possible to forget the ":" character and its still possible to mentally scan a PR and see "=" and miss that it should have been a "==".
x == 1 // oops, meant =
// x == 1 evaluated but not used
if x = 1 { ... } // oops, meant ==
// syntax error: assignment x = 1 used as valueGoogle has a codebase that numbers multiple billions of lines. Using := means typing multiple billions of extra characters. That adds up. And it wouldn't even have prevented this bug!
All checks have costs. As Emerson said, "A stitch in time saves nine. So we make 1000 stitches, that by doing so we may save nine."
I personally prefer spaces so that under any circumstances, the code reads the same as the author intended (whether you’re in an editor or viewing a file with a CLI tool). Are we really counting bytes in this day and age?
Alternatively, author chose eight spaces per indent and proceeded to nest things? My tiled windows only have ~100 columns, thanks so much for breaking my workflow with your poor choices.
That being said, I 100% wholeheartedly approve of enforcing code formatting (among other things) on commit. Pre-commit hooks and linters are both awesome.
If you want custom formatting, I don't think that expecting your editor to reformat on the fly is a big ask. At the end of the day that exactly what tab proponents expect.
Which is often unnecessarily difficult for whatever reason, hence my initial comment. Given that tabs already exist there's literally no reason not to use them other than to intentionally spite any future readers who don't agree with your preference for indentation width.
> that exactly what tab proponents expect
Because that capability is built into even the most primitive of text editors since forever. Per-language formatting, on the other hand, is most certainly not.
[1] I'm hyperbolic, sorry if you actually do that although I pity you.
Every line appears to come from the reformat commit. So it's hard to see when the codes function was changed.
I write all my code in notepad.exe with the font Wingdings. I hope that if you ever read any of my code you'll respect my intent with how it's displayed.
I think the ideal situation is a language with an official or at least commonly accepted formatter, so that existing and new projects will essentially always follow the same style and you never have to think about it. Like "go fmt" always using tabs. I don't like tabs, but I like that kind of enforced consistency way more than I dislike tabs.
That's also why I like using opinionated third-party formatters like prettier and black. Combined with pre-commit hooks, you almost never have to think about style and can just focus on the code. Plus diffs are cleaner.
Admittedly this is something of an edge case - how many languages other than Lisps involve alternating layers of indentation and alignment? The more common C like languages don't suffer from this at all.
At this point I'm largely convinced that more or less all of our tools, languages, and conventions are poorly thought out, brittle, and inelegant. /rant
(setq zwei:*indent-with-tabs* nil)I disagree. It's obviously a major balls up by multiple people, but at the end of the day this is an organizational failure. You don't fire people for this, you learn from it and fix the organizational holes that caused it.
By that logic, almost no individual failure is a firing offense. Many people in the organization are responsible for ensuring that the processes themselves overlap in ways that exclude the possibility of failures like this.
I'm talking about the meta-failure in management's oversight of the processes. There is belt person, and there is suspenders person, and there is a third person who makes sure there are both belt person and suspenders person on staff and that they don't go on vacation at the same time.
Somewhere above the team lead and below the CEO there is someone who didn't do their job.
I'd probably agree with that. Firing should be for extreme negligence, or sustained underperformance.
If this doesn't qualify, what does?
Then there’s obviously the automated testing strategy, manual testing strategy and sanitizer strategy that missed this.
The net result is extreme negligence but to me this kind of failure speaks to several layers going wrong at once rather than one person fucking up badly. Organizationally you try to defend against these kinds of failures with multiple layers of defenses.
Now if this was a malicious internal attack that should be investigated but I’d hate to have to seriously reprimand anyone who typo’ed & instead of &&.
Where you do want to reprimand is if the technical team was repeatedly raising concerns related to failures like this & their concerns weren't being addressed OR someone was being deceitful & hiding issues. That's about the only cases where you want to fire/replace anyone in my book.
Programming safety is like safety in auto racing or rock climbing: you build the safest thing you can, but the activity you're doing is inherently unsafe, and it will generate failures despite the presence safety measures, so you do your best to study those failures to mitigate them the best you can.
The only time adverse action like a reprimand is necessary is if the manager or tech lead don't take any action to prevent future issues. The only time a firing is necessary is if the security issue was created with intent, rather than accidentally.
Linting catches this.
And before anyone comes along and says "well, every possible path should be tested then": That would be absolutely impossible even if the halting problem wasn't a thing (which it very much is). Even if you somehow have utopian "100% code coverage", that does not mean you have tested every possible input resulting in every possible state combination.
I bet every one of us has a story about an old bug that only manifested years[1] later because of the confluence of many unfortunate things, one that takes half an hour to tell.
[1] Or even decades, in some cases that then made it to the HN front page.
The article indicates that every person who updated to the latest version could not login. This is a bug that clearly presents itself, every time, in essentially every use case, and is perfectly reproducible. It affects critical functionality that any layperson can recognize is one of the most critical components of a general purpose computing system. The greatest degree of scrutiny should be applied to this system, yet what we see is the functional equivalent of a car factory forgetting to put a wheel on every car it produces.
Sure, there could be a freak accident that makes the 4th wheel robot malfunction, but other stages of the process should catch such an egregious visible failure. Any process that would allow such basic errors to occur without checking or fixing them elsewhere is terrible on its face.
If that is too abstract, imagine you hired a construction company to build a house and they forgot to put in an entire wall. Maybe you just got unlucky and you were the 1 in 1 billion person who gets such a stupid error to happen to them, but the much more likely theory is that they are grossly incompetent, or at the very least far less reliable than you were led to believe.
I mean that even for basic user flows, there is an astronomically high number of possible states and external inputs such that, taken together, something that worked fine during testing may stop working at some point thereafter, or in the hands of the actual users. Latent bugs are called latent for a reason.
For something like this to have happened without those non-stable channel users experiencing it, it seems like there must have been a code change that was pushed direct to the stable channel - which negates the purpose of the other channels in the first place.
At the very least it seems like a severe process failure. I don't think the dev who changed the code is necessarily at fault - it's more a question of how it was allowed to get into the stable channel without going through the normal process.
How many breaking bugs have _NOT_ shipped due to the current processes. Your approach suggests that there is some perfect process manager out there who will let no bugs like this ship (whilst not dropping any other business priorities mind you) that they just haven't hired yet into this position.
This isn't as big of a bug as people on HN want it to be. A few months from few people will remember, and even fewer will care.
That's pretty close to how it should be. That's the core premise of the no-blame post-mortem culture that's great about some companies.
As long as you go through the port-mortem process, which includes doing your best to prevent this from happening again (which may include changing organizational processes, testing strategies, etc), then everyone can move on with their lives.
Firing happens when you have a history of performing below your level. Never for a single engineering failure.
Firing happens when you have a history of performing below your level. Never for a single process failure.
Correct.
It's possible that this bug made it into the wild because the organizational structure made remedies impossible. In that case you probably don't fire anyone, though maybe you reassign some folks.
It's also possible that the organizational structure indicates that a person or a group of people is responsible for this and they failed to do that job. If that's the case then it might very well make sense to fire folks. It also might not.
I think it's important for poor performance to have consequences[1] and I also think that people leave jobs all the time and it's fine to decide that someone should leave their place in the org with a positive reference and look elsewhere for work (perhaps in another part of goog).
[1] for moral and company culture reasons if nothing else.
This is exactly how any and every bug enters a mature software product: it slips past multiple layers.
> This is a massive, overlapping fuckup. Heads should probably roll here
Some process should change, to fix the problem. But firing people is not necessarily the best solution.