Shell
-
I Wrote 993 Lines of Tests for a Shell Script, Then Deleted the Script
My MCP server has a
just mcp-installrecipe that prints aclaude mcp add ...command you paste into a terminal. It reads your.envand emits one--env NAME=valuepair per line. Above that block sits a banner:Secret values are redacted below — substitute them by hand.The thing doing the redacting was a single
sed:sed -E 's/^(EMBED_API_KEY|TEI_API_KEY|NEO4J_PASSWORD)=.*/\1=<redacted>/'Three names on an allowlist. Everything else prints in full, under a banner that promises otherwise.
It took me about 19 hours to go from noticing that to deleting the entire replacement I’d built. Here’s the trip.
An allowlist fails open, and mine had already failed twice
The problem with an allowlist for secrets isn’t theoretical. It’s that the default is print the value, so every new credential leaks until somebody remembers to extend the list.
Mine had already broken twice, and I only worked that out while writing the fix. First, I renamed the
TEI_*env vars toEMBED_*and left the allowlist matchingTEI_API_KEY, a name that no longer existed. Second, the grep feeding the sed was^[A-Z_]+=with no digits in the character class, which silently dropped everyNEO4J_*row before the sed ever saw it. So theNEO4J_PASSWORDarm of that allowlist was unreachable for its entire existence. It never redacted anything. It just sat there looking reassuring.Switching to a pattern match takes ten seconds:
sed -E 's/^([A-Z0-9_]*(KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)[A-Z0-9_]*)=.*/\1=<redacted>/'I checked it against a synthetic
.envfull of sentinel values. 7 of 7 secrets redacted, includingOAUTH2_TOKEN,S3_SECRET_KEYandEMBED_API_KEY2, all three of which printed in cleartext before. All 10 non-secret variables kept their values, which matters, because the emitted command is useless withoutQDRANT_HOSTandEMBED_MODELintact.Over-redacting a non-secret costs the user a hand-edit. Under-redacting a real one costs them a rotated key. Pick the direction you fail in.
Then I gave it tests, and the tests got mean
A security-relevant expression inlined in a
justrecipe has no test coverage by construction. So I pulled it intoscripts/render-mcp-env.sh(73 lines) and wrotescripts/render_mcp_env_test.go(352 lines) to drive the script throughos/exec.The tests immediately found things the sed one-liner couldn’t have handled.
Credentials hiding in values, not names. The rule only ever inspected the variable name. A value like
bolt://neo4j:hunter2@hostmatches no keyword, so it printed in full.NEO4J_URLandDATABASE_URLare the obvious cases, and they’re exactly the vars people paste into chat.PATis a trap. Adding it for personal access tokens also swallowsPATH,GOPATH,CONFIG_PATH,LOG_PATTERNandCOMPATIBILITY_MODE. A*_PATHvalue is one of the things the command needs to keep. It now matches only as a whole underscore-delimited segment.And my redaction broke the output it was redacting. This one’s my favorite. The emitted line was unquoted, so:
--env FOO=<redacted>parses in a shell as the word
FOO=plus a redirection from a file namedredacted. The block failed to paste correctly whenever a secret was present, which is to say for every actual user. The feature that existed to protect people was the feature that broke the thing for them. ShellCheck flags exactly this and I’d have caught it a week earlier if the logic had been in a file a linter could see, instead of hidden in a recipe body.Fixing the quoting meant leaving sed behind. Per-character shell quoting isn’t expressible in portable sed, so the engine became
grep | awk. I kept the awk POSIX-only and checked it byte-for-byte under mawk (what CI’s Ubuntu runner ships), gawk, andgawk --posix.The script was now 238 lines. The test file was 993.
The tests were right and the script was wrong
Here’s what those 993 lines were actually telling me, once I stopped admiring them.
The awk had to parse
.envto find names and values. The Go binary parses the same file with godotenv v1.5.1. Two parsers, one file, and nobody checking they agreed.They didn’t. Six shapes, each one verified rather than assumed:
$VARand${VAR}expansion- Whitespace trimming
NAME=with an empty value, emitted by one and dropped by the other- Names containing a
. - Indented assignments
#inline comments
Every one of those is a case where the command I told you to paste differs from what the server actually reads. That’s a worse bug than the leak, because it’s silent and it looks like it worked.
So I deleted it. The 238-line script and its 993-line test both went in a single commit, replaced by
internal/envblockat 134 lines with an 868-line test, called from the Justfile as:go run ./cmd/mem0-mcp --print-env-block .envThe binary already links godotenv. Now the pasted block can’t drift from the server’s own parsing, because there’s one parser.
The classifier also got simpler in a way that matters. It’s
strings.Containsover the uppercased name and nothing else:func isSecretName(name string) bool { upper := strings.ToUpper(name) for _, keyword := range secretKeywords { if strings.Contains(upper, keyword) { return true } } // PAT only as a whole underscore-delimited segment, or it would swallow // PATH, LOG_PATTERN, COMPATIBILITY_MODE and friends. return strings.Contains("_"+upper+"_", "_PAT_") }No regexp is reachable from it, on purpose. An anchored or whole-name match classifies
MY.API_KEYas non-secret and prints it in the clear, which was a real bug in the awk version. You can’t write that mistake in this shape.The old test suite had a guard that scanned the script’s source for forbidden patterns. The replacement is a behavioural test, and it’s stronger: it caught a deliberately mutated classifier with a hidden regexp by finding a sentinel value leaking into fixture output. Same evasion class, caught by observing output instead of reading code.
The part that made me laugh
I added a ShellCheck CI job, with a
lint-shell.shthat hard-fails if it discovers fewer than 4 shell scripts, so nobody can quietly delete one past the linter.I deleted one of the scripts it was guarding, and had to lower the floor from 4 to 3. There are three
.shfiles inscripts/now, and the default sits atmin_scripts="${MEM0_MIN_SHELL_SCRIPTS-3}".Five hours from “protect these scripts” to “one fewer script to protect.”
I don’t think the shell version was wasted. I couldn’t have argued for the Go rewrite on day one, because “awk might diverge from godotenv” is a hunch. It only became an argument once I’d written enough tests to enumerate six specific divergences and point at them. The tests didn’t make the script correct. They made the case for its deletion, which was the more useful outcome.
If you’ve got security-relevant logic inlined in a Makefile, a Justfile, or a CI step, that’s the same shape my bug was. Nothing lints it, nothing tests it, and it fails open. Pull it into a file first. You might end up deleting the file, and that’s a fine place to land.
Sources
- godotenv — the Go dotenv parser the server links, v1.5.1
- ShellCheck SC2086 — unquoted expansion, word splitting and redirection
- ShellCheck — the linter itself
- POSIX awk specification — the subset I held the script to for mawk/gawk parity
- POSIX shell command language — quoting and redirection rules behind the
<redacted>bug
I’d appreciate a follow. You can subscribe with your email below. The emails go out once a week, or you can find me on Mastodon at @[email protected].