DEV Community

Enjoy Kumawat
Enjoy Kumawat

Posted on

"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']}")
    ...
Enter fullscreen mode Exit fullscreen mode
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"])
    ...
Enter fullscreen mode Exit fullscreen mode

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"]
    ...
Enter fullscreen mode Exit fullscreen mode

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'
Enter fullscreen mode Exit fullscreen mode

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")
Enter fullscreen mode Exit fullscreen mode
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}")
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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.

Top comments (2)

Collapse
 
mads_hansen_27b33ebfee4c9 profile image
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.

Collapse
 
talha_ramzan_3878156fea8c profile image
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.