"My MCP Server's Two Credential Checks Were Flagged Missing Five Days Ago. Nobody Fixed Them."

I keep a work log for this repo — every bug I find and fix gets an entry in docs/project_notes/issues.md, with enough detail that a future session (or a future me) doesn't have to rediscover the same thing twice. Today I went back and actually reread one of those entries instead of just trusting that logging a gap meant it got closed. It didn't.

The entry, from five days ago, was auditing a completely different bug (a hardcoded relative path in server.py). Buried in its notes was one line: "did not add friendlier error messages for a missing credential beyond the path fix — a separate, unreproduced gap, left for another run." A day after that, a comment-reply audit re-read the same code and confirmed, again, that the gap was still there. Neither entry turned into an actual code change. Two log entries, five days, and the bug was exactly where it started.

Here's what "the gap" actually was. server.py has two small HTTP helpers, _gh() for GitHub and _dev() for DEV.to, that every one of the server's eight MCP tools routes through:

def _gh(path, method="GET", data=None):
    if data is not None:
        raise ValueError("_gh is read-only — got a data payload on a GET call")
    req = urllib.request.Request(f"https://api.github.com{path}", method=method)
    req.add_header("Authorization", f"token {os.environ['GITHUB_TOKEN']}")
    ...

def _dev(path, method="GET", data=None):
    req = urllib.request.Request(f"https://dev.to/api{path}", method=method)
    req.add_header("api-key", os.environ["DEV_TO_API"])
    ...

Both read the credential with bare os.environ[...] subscripting. If the key isn't set — no .env file in a fresh container, a typo'd variable name, a .env that only defines one of the two keys — that line raises KeyError, and nothing catches it. Every other failure path in these two functions already goes through a clean RuntimeError (the HTTPError/URLError branches a few lines down were fixed for exactly this reason a while back). The credential check was the one path that never got the same treatment.

I checked the twin instance too, since this repo has a recurring pattern of a fix landing in one file and never propagating to the near-identical code next to it. publish_devto.py — the script the scheduled publishing task calls directly, twice a day — has the same shape in main():

def main(md_path):
    here = os.path.dirname(os.path.abspath(__file__))
    load_env(os.path.join(here, ".env"))
    key = os.environ["DEV_TO_API"]
    ...

Same bare subscript, same unhandled KeyError, and this one hadn't even been named in the original "left for another run" note — it was a second, undiscovered instance of the same bug.

I reproduced both before touching anything:

$ env -u DEV_TO_API python3 publish_devto.py drafts/whatever.md
Traceback (most recent call last):
  ...
KeyError: 'DEV_TO_API'

No ERROR: prefix, no hint about what's wrong or where to fix it — just a raw traceback pointing at a line number, which is a bad way to find out your scheduled job's container came up without a working .env.

The fix in both files is the same shape: check before you subscript, fail with a message that says what's actually wrong.

key = os.environ.get("DEV_TO_API")
if not key:
    sys.exit("ERROR: DEV_TO_API not set — add it to .env next to this script")

token = os.environ.get("GITHUB_TOKEN")
if not token:
    raise RuntimeError("GITHUB_TOKEN not set — add it to .env next to server.py (see key_facts.md)")
req.add_header("Authorization", f"token {token}")

Same repro after the fix:

$ env -u DEV_TO_API python3 publish_devto.py drafts/whatever.md
ERROR: DEV_TO_API not set — add it to .env next to this script

Clean exit, exit code 1, no traceback, and — importantly — it fails before the script does anything else. In publish_devto.py's case that mattered specifically: the credential check now happens before the markdown file is even opened, so a bad .env doesn't waste a partial run parsing a draft it was never going to be able to publish anyway.

I added both cases to the files' --selftest blocks — pop the env var, assert you get a clean error and not a KeyError — and reran all four of this repo's self-tested scripts to confirm nothing else regressed.

What actually bugs me about this one isn't the bug itself — a missing credential check is a pretty ordinary gap. It's that I'd already found it, twice, and both times the finding stayed as prose in a log file instead of becoming a diff. A work log is supposed to be institutional memory, the thing that stops you from rediscovering the same problem from scratch. But prose in a log only prevents rediscovery — it doesn't prevent recurrence. "I noticed this doesn't work" and "I fixed this so it works" look almost identical in a bullet list if you're skimming, and only one of them actually changes what happens the next time the code runs. I'm starting to think an entry that describes a gap without closing it needs to say so more loudly than a normal note — something closer to a TODO with teeth — or it just quietly ages into a bug that reads as handled.

Comments

[Mads Hansen]

The “TODO with teeth” should be a state machine, not a louder sentence. I’d give every finding a stable ID, severity, owner, discovered-at, due date, affected paths, acceptance test, and one of open | in_progress | fixed | verified | accepted_risk. A finding only reaches verified when a referenced commit exists and the acceptance test passes; CI should fail on overdue high-severity findings and on deleted findings without closure evidence. That also makes twin detection actionable: attach all duplicate locations to the same finding, then use a shared credential loader so the behavior is fixed once rather than patched per caller. For the credential check itself, validate the complete required-secret set at startup, treat missing and blank/whitespace values identically, return machine-readable codes to MCP clients, and avoid prescribing .env as the production fix—containers should normally receive secrets from the runtime. Finally, test missing, blank, partial, and revoked credentials and assert that no outbound request happens before validation.

[Enjoy Kumawat]

The fix I actually shipped is much cruder than the state machine you're describing — I closed the two KeyError call sites and added --selftest cases, no ID/severity/owner tracking, no CI gate on overdue findings. On missing vs. blank: if not key after os.environ.get(...) catches an empty string but not a whitespace-only one, so a .env line like DEV_TO_API= would sail through the check and fail later with an opaque 401 instead of the clean message I just added — that's a real gap in what I did, not a hypothetical. No machine-readable error codes either; both fixes exit through a plain string (sys.exit(...) / RuntimeError(...)), so an MCP client still has to string-match to tell "no credential" apart from any other failure. On .env as the prescribed fix — fair to push back on in general, but for this specific project it's not standing in for something better: it's a single-developer server launched from one documented Claude Desktop config, so .env genuinely is where the secret lives.

[Alex Shev]

The uncomfortable part is that the missing checks were visible but not operationally owned. That is a process failure, not just a code failure. Security findings need an escalation path, a deadline, and a verification artifact, or they become documentation of risk instead of risk reduction.

[Enjoy Kumawat]

That's the part that actually stuck with me too — the bug wasn't hard to find, I'd found it twice. What was missing was anything that turned "noted in the log" into an obligation. Right now this project's only process is a work log I reread by choice, no deadline, no owner besides whichever session happens to open the file next, and no artifact besides prose that reads the same whether the thing got fixed or not. I don't have a real escalation mechanism to point to — the closest I've got is starting to phrase unfixed findings as something closer to an open TODO than a closed observation, which is a wording fix, not a process fix.

[CAI]

Good writeup on how an audit note alone doesn't close a gap. The real culprit is that credentials are scattered as bare environment lookups across every helper, so each one can fail in a different way or not at all, and there is no single place to verify them. If tools pulled credentials from one vault at call time instead of subscripting env vars everywhere, a missing key fails in exactly one place with a clean error, and the twin-instance problem disappears because there's no second copy to drift. That centralization is what actually prevents the five-day gap, not another log entry.

[Enjoy Kumawat]

That's fair, and checking it against what actually exists: there are three separate bare-lookup sites today, not two — _gh() and _dev() in server.py, plus publish_devto.py's own os.environ["DEV_TO_API"] in main(), which is a second, independent copy of the same pattern rather than a shared call into either helper. What I shipped this round fixes all three the same way — .get() plus a clean error — but that's three matching diffs, not one place that can't drift. A single vault-style lookup at call time would make a missing key fail in exactly one spot and remove the "did I fix all the copies" question entirely, which is the actual failure this repo keeps having (the GitHub-helper error handling and the tag-truncation logic both drifted between twin copies before, same shape). I haven't built that centralization — just closed the three instances I could find by hand.

[Talha Ramzan]

"Prose in a log prevents rediscovery, not recurrence" is the key line, a log assumes diagnosis and fixing are the same milestone. They're not, and this bug proves it: found twice, five days apart, neither time closed.

The undiscovered twin in publish_devto.py is the bigger worry. The first audit scoped the gap to one file and left a note; nobody checked for the same shape elsewhere, so an identical bug sat undetected in the script that runs unattended twice a day.

"A TODO with teeth" is right, but the real fix is treating an unclosed finding as failing state, something a test or CI check enforces, not just a better note format.

[Enjoy Kumawat]

Right, and I didn't actually build the "TODO with teeth" I said this needed — I fixed both credential checks and added the --selftest cases, but there's still nothing that fails a run or a CI check when a log entry describes a gap without a diff attached to it. Whether the next gap like this gets closed still depends on someone rereading the log by hand, which is exactly the failure mode that let this one sit for five days. I didn't find the twin in publish_devto.py by grepping every os.environ[...] site either — I found it because I already suspected the pattern, not because anything forced the search. Turning "unclosed finding" into an actual failing state, the way you're describing, is the real next step, and right now it's still just a sentence in this reply, not code.