-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Description
Summary
create_or_update_file has multiple bugs in its SHA validation logic that cause it to fail on legitimate update operations. The tool should be split into separate create_file and update_file tools, or the validation logic needs significant fixes.
Bug Details
1. ETag vs Blob SHA Mismatch (Critical)
The SHA validation performs a HEAD request and compares the user-provided blob SHA against the response's ETag header:
req.Header.Set("If-None-Match", fmt.Sprintf(`"%s"`, sha))
// ...
case http.StatusOK:
currentSHA := strings.Trim(resp.Header.Get("ETag"), `"`)
return utils.NewToolResultError(fmt.Sprintf(
"SHA mismatch: provided SHA %s is stale...", sha, currentSHA))The problem: GitHub's Contents API returns a weak ETag (W/"...") that is NOT the blob SHA. It's an opaque server-generated value. Comparing a blob SHA against an ETag is comparing two different things, causing the validation to reject perfectly valid updates where the SHA is actually current.
2. url.PathEscape Breaks Multi-Segment Paths
contentURL := fmt.Sprintf("repos/%s/%s/contents/%s", owner, repo, url.PathEscape(path))For paths like sassymcp/modules/vision.py, PathEscape encodes / as %2F, producing sassymcp%2Fmodules%2Fvision.py. The Contents API expects literal path separators. This causes the HEAD validation request to 404, which then falls through to the "new file" code path. The subsequent CreateFile call then fails with 409 Conflict because the file already exists but no SHA was provided.
Should use url.PathEscape per-segment or just pass the raw path (the go-github client handles encoding).
3. Blind Update ETag-as-SHA Injection
When no SHA is provided and the file exists, the code extracts the ETag and uses it as the SHA for the update:
previousSHA = strings.Trim(resp.Header.Get("ETag"), `"`)
// ...
if previousSHA != "" {
opts.SHA = github.Ptr(previousSHA)
}Since the ETag is not a blob SHA, this causes client.Repositories.CreateFile to fail with a 409 because GitHub's API rejects the invalid SHA value.
4. Multiple Deferred Body Closes
The validation HEAD requests and the final CreateFile call all stack defer resp.Body.Close(). If response bodies aren't fully drained before the next request on the same HTTP/2 connection, this can cause connection reuse issues.
Recommended Fix
Option A: Split into two tools
create_file— creates new files only, no SHA parameter neededupdate_file— updates existing files, requires SHA parameter, validates usingget_file_contentsblob SHA (not ETag)
Option B: Fix the validation
- Replace the HEAD+ETag approach with a GET to
/repos/{owner}/{repo}/contents/{path}?ref={branch}and compare against the response's.shafield (the actual blob SHA) - Fix
url.PathEscapeto not encode path separators - Don't inject ETag values as SHA parameters
Workaround
Use push_files instead of create_or_update_file for all update operations. push_files uses the Git Data API (create tree → create commit → update ref) which completely bypasses the Contents API and its SHA validation issues. It works reliably for both creates and updates, single or multi-file.
Environment
- GitHub MCP Server (hosted by Anthropic as Claude.ai connector)
- Reproduces on:
sassyconsultingllc/SassyMCPrepo, updating existing files onmainbranch - Failure rate: ~30-40% of update attempts via
create_or_update_file