Four Bugs in One Feature

On my screen, the agent types a command to remove one account, and a different one disappears.

Nothing warns. Nothing asks. The tool prints Account 'a.c' removed. Then it hands back the prompt, as if it had done exactly what it was asked.

The name is a.c, and the one that vanishes is abc. They’re test names in a throwaway reproduction the agent built, so nothing real is lost here. On somebody’s own machine, abc would be an account they still needed.

The tool was small — a command-line helper for anyone juggling several accounts on a hosted database service. It kept each account’s access token in the Mac’s keychain and logged you into whichever one you named. A couple hundred lines of shell, and nobody had ever filed an issue against it.

I’d pointed the agent at it to read every line and answer one question. Is anything in here going to hurt the person who runs it?

Several things were.

Characters with secret meanings

Four of them lived in one feature — the account’s name. You choose it when you add an account and type it again to rename or remove one. The most innocuous thing imaginable. A name is just a string. Except this string was doing two jobs it was never safe to do. It was stored in a single list joined by commas. And it was handed, as a pattern rather than as plain text, to grep and sed, two old shell tools that read certain characters as instructions.

Except this string was doing two jobs it was never safe to do.

The first bug is the comma. Names went into that list joined by commas and came back out split on them. Add an account called x,y next to one called abc, and the list shows three accounts: abc, x and y. Two of them don’t exist.

The second is the dot. To find a name in the list, the tool asked grep. And grep, left to its defaults, reads a name as a regular expression, where a dot means any character at all. So a.c is a pattern — and abc fits it. Ask to remove a.c and the tool drops abc from its list instead. The token for abc stays behind in the keychain — still usable if you remember its exact name, but gone from every list the tool shows you. The confirmation on the screen never mentions abc.

The third is the slash, and it lived in the rename command, which was only about six weeks old. Renaming worked by dropping the old and new names into a sed substitution — a tiny text-replacement script whose parts are divided by slashes. Put a slash in a name and the script’s own syntax comes apart, and sed gives up with a cryptic complaint about a bad flag.

What happened next was worse. The tool never checked whether sed succeeded, and by then it had already moved the token to the new name. It wrote sed’s empty output back as the list and announced that the rename worked. Rerun the old script against a stand-in keychain and every account drops off the list, including one that had nothing to do with the rename.

The fourth is the subtlest, and the one I like best, because you would not guess it, or think to test for it. In that same replacement, an ampersand doesn’t mean ampersand. To sed, an ampersand in the replacement stands for the entire thing you just matched. So rename an account called alpha to x&y, and the list does not say x&y. It says x, then the whole old name spliced in, then y — xalphay. Ask to switch to xalphay and you get a flat not found, while the real token sits under x&y, where the list can’t see it.

Four bugs. One feature. Every one of them a place where a human being’s ordinary choice of name collided with some program’s invisible convictions about what certain characters secretly mean.

Someone else’s tool

I did not find these four by squinting at the source and feeling clever. I asked. Then I had the agent reproduce each one on the script as it stood, and check it again against the patch, before I let myself believe any of it. Eleven assertions, all passing.

The fix was small — twenty-four lines in, four out, one file. Refuse new names that contain a comma or whitespace. Tell grep to match the name as plain text. Do the rename by comparing each stored name to the old one instead of building a script out of whatever the user typed. Four sharp edges, filed down.

There was one judgment call I couldn’t make on my own, so I didn’t. A comma truly can’t live in a comma-joined list. But the fix refuses any whitespace too. A name a user could type yesterday, they cannot type today. For a comma, that turns a silent mess into an honest error. I thought it was the right trade. But it isn’t my tool, and the person who owns it gets the last word.

So the change told its owner, under a heading of its own. It named what would now be refused and flagged the trade, in case you’d rather sanitise than reject. It also said those names had never stored correctly. For a space in the middle, that was wrong. Rerun today on a stand-in keychain, the old script keeps a name with a space in the middle and lists it fine. The fix traded a name that worked for a rule simple enough to trust, and my note never said so.

The owner’s decision, not mine. The pull request merged eleven minutes after I opened it — just before ten on a Thursday morning — with a short thank-you four seconds before. Three of my changes went in that morning, in a little over two minutes.

The reason crossed over

The next morning the owner opened an issue about taking the tool beyond the Mac. Someone else had built a Linux copy, and the issue asked for Windows as well. Less than three hours later the owner had merged a rewrite in another language — and deleted the shell script the fixes had lived in. It would be easy to file that under pointless. Merged Thursday morning, deleted Friday afternoon.

The dot, the slash and the ampersand have nowhere left to live, because the new code checks whether two names are exactly equal, with no pattern for a character to break.

The comma was different. The new code kept the comma-joined list as it was, so existing users would keep their accounts after upgrading. And it carried my rule across. Above the check sits a comment saying it mirrors the original: no commas, no whitespace, because the index is a comma-separated list. The original it mirrors had no such rule until that Thursday. The error a user sees is the same sentence the patch introduced, word for word. The merge had already answered the question in my note. The rewrite answered it again: reject, not sanitise, with a test to hold it there.

The patch had given that same reason above the bash rule. The script was thrown away. The reason crossed into a new language, its mistake about spaces intact, still sitting next to the rule it explains.

A name is never just a string.

Somewhere under every harmless feature there is a character that means something you didn’t intend, waiting for the one person who types it.