DEV Community

Enjoy Kumawat
Enjoy Kumawat

Posted on

My MCP Server's GitHub Helper Function Could POST and DELETE. Every Tool That Called It Only Ever Used GET.

I run a small FastMCP server (server.py in my my-git-manager repo) that exposes my GitHub profile and DEV.to articles as tools for an agent. It has three GitHub tools: get_github_profile, list_repos, get_repo_stats. All three are read-only by intent — none of them are supposed to touch anything.

I went looking for a fresh angle after a trending post ("If Your AI Agent Has Write Access to Public Repos, Audit It Now") made the rounds, and my first reaction was "not my problem, I don't have any write tools." Then I actually read the helper function all three of those tools go through, instead of just the tools themselves.

def _gh(path, method="GET", data=None):
    req = urllib.request.Request(f"https://api.github.com{path}", method=method)
    req.add_header("Authorization", f"token {os.environ['GITHUB_TOKEN']}")
    req.add_header("Accept", "application/vnd.github.v3+json")
    if data:
        req.add_header("Content-Type", "application/json")
        req.data = json.dumps(data).encode()
    with urllib.request.urlopen(req, timeout=30) as r:
        return json.loads(r.read())
Enter fullscreen mode Exit fullscreen mode

method defaults to "GET", but it's a parameter. data gets serialized and attached to the request body if you pass it. There is nothing in this function that says "this repo only does reads." It's a fully general HTTP client for the GitHub REST API, wearing a read-only helper's name.

I checked every call site:

u = _gh(f"/users/{GITHUB_USERNAME}")
repos = _gh(f"/users/{GITHUB_USERNAME}/repos?sort={sort}&per_page={min(limit, 100)}")
r = _gh(f"/repos/{GITHUB_USERNAME}/{repo}")
Enter fullscreen mode Exit fullscreen mode

Three calls, all default GET, none pass data. As of right now, the "no write tools" story is entirely true — but it's true by convention, not by anything the code enforces. Nothing stops a future tool, a bad edit, or a bug from calling _gh(path, method="DELETE") or _gh(path, method="POST", data=payload) and having it just work.

That distinction matters more here than in a lot of repos, because of what's actually backing the request. docs/project_notes/key_facts.md documents the real token's scope:

Token scopes needed: repo, user
Enter fullscreen mode Exit fullscreen mode

repo is not "read some public repo metadata." It's full read/write access to every repository the account can reach, public and private — create, push, delete, the works. That's the credential sitting in os.environ['GITHUB_TOKEN'] for every single call _gh makes, whether the call is get_repo_stats or something that doesn't exist yet. The three tools I've built happen to only ask for reads. The token doesn't know that, and until this run, neither did the function.

This is a different shape of problem from the one I wrote up a couple of days ago, where GITHUB_TOKEN and DEV_TO_API both sit in the same process's environment. That post was about two separate credentials sharing a blast radius because nothing separates the processes that use them. This one is about a single credential having far more reach than anything that currently uses it needs, with the code offering no resistance if that gap ever gets exploited — by a bug, a copy-pasted call, or (the reason I actually went looking) a compromised or over-eager agent deciding a "helpful" write is in scope.

The fix is small on purpose. I don't need _gh to support writes — I need it to refuse to, until I deliberately decide otherwise:

def _gh(path, method="GET", data=None):
    # No GitHub tool in this server writes anything — GITHUB_TOKEN is scoped
    # `repo, user` (full write access, see key_facts.md), so a stray
    # method="POST"/"DELETE" here would be a real write, not a hypothetical
    # one. Enforced, not just true by convention.
    if method != "GET":
        raise ValueError(f"_gh is read-only — got method={method!r}")
    req = urllib.request.Request(f"https://api.github.com{path}", method=method)
    ...
Enter fullscreen mode Exit fullscreen mode

One if, before anything touches the network. If I ever add a real write tool, I have to come back here and consciously widen this, which is exactly the friction I want — a deliberate decision, not a default.

I verified it two ways before calling it done. First, that the three existing read tools still work unchanged — I stubbed urllib.request.urlopen so the test doesn't depend on live network access, called get_github_profile(), and confirmed it still made exactly one GET request and returned the same shape of data as before:

calls = []
def fake_urlopen(req, timeout=30):
    calls.append((req.get_method(), req.full_url))
    return FakeResp(json.dumps({"login": "enjoykumawat", ...}).encode())

server.urllib.request.urlopen = fake_urlopen
prof = server.get_github_profile()
# -> GET path works via _gh: enjoykumawat | method used: GET
Enter fullscreen mode Exit fullscreen mode

Second, that a stray write attempt now fails loudly and before any request goes out, not after:

try:
    server._gh("/repos/enjoykumawat/my-git-manager", method="DELETE")
except ValueError as e:
    print(e)
# -> _gh is read-only — got method='DELETE'
Enter fullscreen mode Exit fullscreen mode

total network calls made: 1 — the DELETE attempt never reached urlopen at all. That's the part I actually cared about: not that the response would come back denied, but that the request would never be constructed in the first place.

The pattern I keep tripping over across this repo is variations on the same thing: a helper function's actual capability is broader than what its current callers need, and nothing marks that gap until you go looking for it. I did the same thing a few weeks back with a substring-based attribution filter that was blocking more than it should; this time it's a helper that could do more than it should. Same root shape, opposite direction — under-restriction instead of over-restriction — but both come from writing a general-purpose function first and only checking "what does this actually get used for" much later, if ever.

If you've got an MCP server (or any tool-calling agent setup) built around a small number of thin wrapper functions over one shared HTTP client, it's worth grepping for every call site of that client the same way I just did here — not to check what your tools do, but to check what the function underneath them could do, and whether the credential behind it agrees with the story your tool list tells.

Top comments (1)

Collapse
 
mads_hansen_27b33ebfee4c9 profile image
Mads Hansen

This is a strong example of capability being defined by the narrowest enforceable layer, not by today’s call sites. The helper-level GET allowlist is useful, but the credential scope is still the decisive boundary: if the process is compromised, the token can write regardless of what _gh permits. I’d pair the code guard with a fine-grained, read-only GitHub token and add two regression tests: every non-GET method must fail before network I/O, and request bodies must be rejected. That turns “read-only” into something you can verify at both the code and identity layers.