Skip to content

Commit 3c53198

Browse files
theinfosecguySamMorrowDrums
authored andcommitted
Clarify symlink behavior for repository file writes
1 parent 8ec6249 commit 3c53198

7 files changed

Lines changed: 102 additions & 9 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1288,7 +1288,7 @@ The following sets of tools are available:
12881288
- `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)
12891289
- `message`: Commit message (string, required)
12901290
- `owner`: Repository owner (username or organization) (string, required)
1291-
- `path`: Path where to create/update the file (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)
12921292
- `repo`: Repository name (string, required)
12931293
- `sha`: The blob SHA of the file being replaced. Required if the file already exists. (string, optional)
12941294

pkg/github/__toolsnaps__/create_or_update_file.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"type": "string"
2525
},
2626
"path": {
27-
"description": "Path where to create/update the file",
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.",
2828
"type": "string"
2929
},
3030
"repo": {

pkg/github/__toolsnaps__/push_files.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"type": "string"
2222
},
2323
"path": {
24-
"description": "path to the file",
24+
"description": "Exact Git path to write. Writing to a symbolic link path replaces the link with a regular file; use the linked file's path to update its contents.",
2525
"type": "string"
2626
}
2727
},

pkg/github/repositories.go

Lines changed: 2 additions & 2 deletions
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: "Path where to create/update the file",
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.",
437437
},
438438
"content": {
439439
Type: "string",
@@ -1575,7 +1575,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
15751575
Properties: map[string]*jsonschema.Schema{
15761576
"path": {
15771577
Type: "string",
1578-
Description: "path to the file",
1578+
Description: "Exact Git path to write. Writing to a symbolic link path replaces the link with a regular file; use the linked file's path to update its contents.",
15791579
},
15801580
"content": {
15811581
Type: "string",

pkg/github/tools.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ var (
3838
InstructionsFunc: generateContextToolsetInstructions,
3939
}
4040
ToolsetMetadataRepos = inventory.ToolsetMetadata{
41-
ID: "repos",
42-
Description: "GitHub Repository related tools",
43-
Default: true,
44-
Icon: "repo",
41+
ID: "repos",
42+
Description: "GitHub Repository related tools",
43+
Default: true,
44+
Icon: "repo",
45+
InstructionsFunc: generateReposToolsetInstructions,
4546
}
4647
ToolsetMetadataGit = inventory.ToolsetMetadata{
4748
ID: "git",

pkg/github/toolset_instructions.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ func generateContextToolsetInstructions(_ *inventory.Inventory) string {
99
return "Always call 'get_me' first to understand current user permissions and context."
1010
}
1111

12+
func generateReposToolsetInstructions(_ *inventory.Inventory) string {
13+
return `## Repository file writes
14+
15+
'get_file_contents' may return the target contents when a path is a symbolic link, but repository file writes use exact Git paths and do not follow symbolic links. To edit content that a symlink points to, write to the target path.`
16+
}
17+
1218
func generateIssuesToolsetInstructions(_ *inventory.Inventory) string {
1319
return `## Issues
1420
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package github
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/github/github-mcp-server/pkg/inventory"
8+
"github.com/github/github-mcp-server/pkg/translations"
9+
"github.com/google/jsonschema-go/jsonschema"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestRepositoryInstructionsExplainSymlinkWriteSemantics(t *testing.T) {
15+
t.Setenv("DISABLE_INSTRUCTIONS", "false")
16+
17+
reposInventory, err := inventory.NewBuilder().
18+
SetTools([]inventory.ServerTool{{Toolset: ToolsetMetadataRepos}}).
19+
WithToolsets([]string{"repos"}).
20+
WithServerInstructions().
21+
Build()
22+
require.NoError(t, err)
23+
24+
instructions := strings.ToLower(reposInventory.Instructions())
25+
assert.Contains(t, instructions, "## repository file writes")
26+
assert.Contains(t, instructions, "may return the target contents")
27+
assert.Contains(t, instructions, "do not follow symbolic links")
28+
29+
defaultInventory, err := inventory.NewBuilder().
30+
SetTools([]inventory.ServerTool{
31+
{Toolset: ToolsetMetadataContext},
32+
{Toolset: ToolsetMetadataRepos},
33+
}).
34+
WithToolsets([]string{"default"}).
35+
WithServerInstructions().
36+
Build()
37+
require.NoError(t, err)
38+
assert.Contains(t, strings.ToLower(defaultInventory.Instructions()), "## repository file writes")
39+
40+
contextInventory, err := inventory.NewBuilder().
41+
SetTools([]inventory.ServerTool{{Toolset: ToolsetMetadataContext}}).
42+
WithToolsets([]string{"context"}).
43+
WithServerInstructions().
44+
Build()
45+
require.NoError(t, err)
46+
assert.NotContains(t, strings.ToLower(contextInventory.Instructions()), "## repository file writes")
47+
}
48+
49+
func TestFileWritePathsExplainSymlinkWriteSemantics(t *testing.T) {
50+
createSchema, ok := CreateOrUpdateFile(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema)
51+
require.True(t, ok)
52+
pushSchema, ok := PushFiles(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema)
53+
require.True(t, ok)
54+
createPath := createSchema.Properties["path"]
55+
require.NotNil(t, createPath)
56+
pushFiles := pushSchema.Properties["files"]
57+
require.NotNil(t, pushFiles)
58+
require.NotNil(t, pushFiles.Items)
59+
pushPath := pushFiles.Items.Properties["path"]
60+
require.NotNil(t, pushPath)
61+
62+
tools := []struct {
63+
name string
64+
description string
65+
expectedBehavior string
66+
}{
67+
{
68+
name: "create_or_update_file",
69+
description: createPath.Description,
70+
expectedBehavior: "rewrites the symbolic link's target path",
71+
},
72+
{
73+
name: "push_files",
74+
description: pushPath.Description,
75+
expectedBehavior: "replaces the link with a regular file",
76+
},
77+
}
78+
79+
for _, tool := range tools {
80+
t.Run(tool.name, func(t *testing.T) {
81+
description := strings.ToLower(tool.description)
82+
assert.Contains(t, description, "exact git path")
83+
assert.Contains(t, description, tool.expectedBehavior)
84+
})
85+
}
86+
}

0 commit comments

Comments
 (0)