Skip to content

Commit c8f6977

Browse files
fix(repos): guard writes to symbolic links
Detect existing symlinks through the Git tree and require an explicit opt-in before changing their targets. Return the resolved repository target so callers can safely update the linked file instead. Refs #2997 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 3c53198 commit c8f6977

8 files changed

Lines changed: 308 additions & 5 deletions

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,11 +1284,12 @@ The following sets of tools are available:
12841284

12851285
- **create_or_update_file** - Create or update file
12861286
- **Required OAuth Scopes**: `repo`
1287+
- `allow_symlink_write`: Set to true only to intentionally change a symbolic link's target. The content must be the new link target path. By default, writes to existing symbolic links are rejected to prevent replacing the link target with file contents returned by get_file_contents. (boolean, optional)
12871288
- `branch`: Branch to create/update the file in (string, required)
12881289
- `content`: Content of the file, exactly as it should appear once written. Do not base64-encode it; this server does that before calling the REST API. (string, required)
12891290
- `message`: Commit message (string, required)
12901291
- `owner`: Repository owner (username or organization) (string, required)
1291-
- `path`: Exact Git path to write. Writing to a symbolic link path rewrites the symbolic link's target path; use the linked file's path to update its contents. (string, required)
1292+
- `path`: Exact Git path to write. Writing to a symbolic link path changes the link target to the supplied content; it does not update the linked file. (string, required)
12921293
- `repo`: Repository name (string, required)
12931294
- `sha`: The blob SHA of the file being replaced. Required if the file already exists. (string, optional)
12941295

pkg/github/__toolsnaps__/create_or_update_file.snap

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
"description": "Create or update a single file in a GitHub repository. \nIf updating, you should provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations.\n\nIn order to obtain the SHA of original file version before updating, use the following git command:\ngit rev-parse \u003cbranch\u003e:\u003cpath to file\u003e\n\nSHA MUST be provided for existing file updates.\n",
88
"inputSchema": {
99
"properties": {
10+
"allow_symlink_write": {
11+
"default": false,
12+
"description": "Set to true only to intentionally change a symbolic link's target. The content must be the new link target path. By default, writes to existing symbolic links are rejected to prevent replacing the link target with file contents returned by get_file_contents.",
13+
"type": "boolean"
14+
},
1015
"branch": {
1116
"description": "Branch to create/update the file in",
1217
"type": "string"
@@ -24,7 +29,7 @@
2429
"type": "string"
2530
},
2631
"path": {
27-
"description": "Exact Git path to write. Writing to a symbolic link path rewrites the symbolic link's target path; use the linked file's path to update its contents.",
32+
"description": "Exact Git path to write. Writing to a symbolic link path changes the link target to the supplied content; it does not update the linked file.",
2833
"type": "string"
2934
},
3035
"repo": {

pkg/github/helper_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ const (
4545
ListCollaborators = "GET /repos/{owner}/{repo}/collaborators"
4646

4747
// Git endpoints
48+
GetReposGitBlobsByOwnerByRepoByFileSHA = "GET /repos/{owner}/{repo}/git/blobs/{file_sha}"
4849
GetReposGitTreesByOwnerByRepoByTree = "GET /repos/{owner}/{repo}/git/trees/{tree}"
4950
GetReposGitRefByOwnerByRepoByRef = "GET /repos/{owner}/{repo}/git/ref/{ref:.*}"
5051
PostReposGitRefsByOwnerByRepo = "POST /repos/{owner}/{repo}/git/refs"

pkg/github/repositories.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ SHA MUST be provided for existing file updates.
433433
},
434434
"path": {
435435
Type: "string",
436-
Description: "Exact Git path to write. Writing to a symbolic link path rewrites the symbolic link's target path; use the linked file's path to update its contents.",
436+
Description: "Exact Git path to write. Writing to a symbolic link path changes the link target to the supplied content; it does not update the linked file.",
437437
},
438438
"content": {
439439
Type: "string",
@@ -451,6 +451,11 @@ SHA MUST be provided for existing file updates.
451451
Type: "string",
452452
Description: "The blob SHA of the file being replaced. Required if the file already exists.",
453453
},
454+
"allow_symlink_write": {
455+
Type: "boolean",
456+
Description: "Set to true only to intentionally change a symbolic link's target. The content must be the new link target path. By default, writes to existing symbolic links are rejected to prevent replacing the link target with file contents returned by get_file_contents.",
457+
Default: json.RawMessage("false"),
458+
},
454459
},
455460
Required: []string{"owner", "repo", "path", "content", "message", "branch"},
456461
},
@@ -501,6 +506,11 @@ SHA MUST be provided for existing file updates.
501506
opts.SHA = github.Ptr(sha)
502507
}
503508

509+
allowSymlinkWrite, err := OptionalParam[bool](args, "allow_symlink_write")
510+
if err != nil {
511+
return utils.NewToolResultError(err.Error()), nil, nil
512+
}
513+
504514
// Create or update the file
505515
client, err := deps.GetClient(ctx)
506516
if err != nil {
@@ -541,6 +551,19 @@ SHA MUST be provided for existing file updates.
541551
"Pull the latest changes and use git rev-parse %s:%s to get the current SHA.",
542552
sha, currentSHA, branch, path)), nil, nil
543553
}
554+
if !allowSymlinkWrite {
555+
symlinkTarget, isSymlink, respTree, err := symlinkTargetAtPath(ctx, client, owner, repo, branch, path)
556+
if err != nil {
557+
return ghErrors.NewGitHubAPIErrorResponse(ctx,
558+
"failed to verify whether file path is a symbolic link",
559+
respTree,
560+
err,
561+
), nil, nil
562+
}
563+
if isSymlink {
564+
return newSymlinkWriteBlockedResult(path, symlinkTarget), nil, nil
565+
}
566+
}
544567
}
545568
} else {
546569
// No SHA provided - check if file already exists

pkg/github/repositories_helper.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"net/http"
8+
pathpkg "path"
89
"strings"
910

1011
ghErrors "github.com/github/github-mcp-server/pkg/errors"
@@ -90,6 +91,113 @@ func createReferenceFromDefaultBranch(ctx context.Context, client *github.Client
9091
return createdRef, nil
9192
}
9293

94+
const gitSymlinkMode = "120000"
95+
96+
type symlinkWriteBlockedError struct {
97+
Error string `json:"error"`
98+
Path string `json:"path"`
99+
SymlinkTarget string `json:"symlink_target"`
100+
ResolvedTargetPath string `json:"resolved_target_path,omitempty"`
101+
Message string `json:"message"`
102+
}
103+
104+
func newSymlinkWriteBlockedResult(path, target string) *mcp.CallToolResult {
105+
resolvedTargetPath := resolveRepositorySymlinkTarget(path, target)
106+
message := "The exact Git path is a symbolic link. get_file_contents may have returned the linked file's content and SHA, " +
107+
"but create_or_update_file would write that content into the symlink itself. "
108+
if resolvedTargetPath != "" {
109+
message += fmt.Sprintf("Write to %q instead, or set allow_symlink_write to true only to intentionally change the link target.", resolvedTargetPath)
110+
} else {
111+
message += "The link target resolves outside this repository. Set allow_symlink_write to true only to intentionally change the link target."
112+
}
113+
114+
payload, _ := json.Marshal(symlinkWriteBlockedError{
115+
Error: "symlink_write_requires_explicit_opt_in",
116+
Path: path,
117+
SymlinkTarget: target,
118+
ResolvedTargetPath: resolvedTargetPath,
119+
Message: message,
120+
})
121+
return utils.NewToolResultError(string(payload))
122+
}
123+
124+
func symlinkTargetAtPath(ctx context.Context, client *github.Client, owner, repo, treeish, path string) (string, bool, *github.Response, error) {
125+
ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+treeish)
126+
if err != nil {
127+
return "", false, resp, err
128+
}
129+
if resp != nil && resp.Body != nil {
130+
_ = resp.Body.Close()
131+
}
132+
headSHA := ref.GetObject().GetSHA()
133+
if headSHA == "" {
134+
return "", false, nil, fmt.Errorf("branch %q has no commit SHA", treeish)
135+
}
136+
137+
entry, resp, err := getTreeEntry(ctx, client, owner, repo, headSHA, path)
138+
if err != nil {
139+
return "", false, resp, err
140+
}
141+
if entry == nil {
142+
return "", false, nil, fmt.Errorf("path %q exists according to the Contents API but was not found in the Git tree", path)
143+
}
144+
if entry.GetMode() != gitSymlinkMode {
145+
return "", false, nil, nil
146+
}
147+
148+
target, resp, err := client.Git.GetBlobRaw(ctx, owner, repo, entry.GetSHA())
149+
if err != nil {
150+
return "", false, resp, err
151+
}
152+
if resp != nil && resp.Body != nil {
153+
_ = resp.Body.Close()
154+
}
155+
return string(target), true, nil, nil
156+
}
157+
158+
func getTreeEntry(ctx context.Context, client *github.Client, owner, repo, treeish, path string) (*github.TreeEntry, *github.Response, error) {
159+
segments := strings.Split(pathpkg.Clean(strings.TrimPrefix(path, "/")), "/")
160+
for i, segment := range segments {
161+
tree, resp, err := client.Git.GetTree(ctx, owner, repo, treeish, false)
162+
if err != nil {
163+
return nil, resp, err
164+
}
165+
if resp != nil && resp.Body != nil {
166+
_ = resp.Body.Close()
167+
}
168+
169+
var matched *github.TreeEntry
170+
for _, entry := range tree.Entries {
171+
if entry.GetPath() == segment {
172+
matched = entry
173+
break
174+
}
175+
}
176+
if matched == nil {
177+
return nil, nil, nil
178+
}
179+
if i == len(segments)-1 {
180+
return matched, nil, nil
181+
}
182+
if matched.GetType() != "tree" {
183+
return nil, nil, nil
184+
}
185+
treeish = matched.GetSHA()
186+
}
187+
return nil, nil, nil
188+
}
189+
190+
func resolveRepositorySymlinkTarget(linkPath, target string) string {
191+
if pathpkg.IsAbs(target) {
192+
return ""
193+
}
194+
resolved := pathpkg.Clean(pathpkg.Join(pathpkg.Dir(linkPath), target))
195+
if resolved == ".." || strings.HasPrefix(resolved, "../") {
196+
return ""
197+
}
198+
return resolved
199+
}
200+
93201
// matchFiles searches for files in the Git tree that match the given path.
94202
// It's used when GetContents fails or returns unexpected results.
95203
func matchFiles(ctx context.Context, client *github.Client, owner, repo, ref, path string, rawOpts *raw.ContentOpts, rawAPIResponseCode int) (*mcp.CallToolResult, any, error) {

0 commit comments

Comments
 (0)