Skip to content

Commit b5eb2a7

Browse files
committed
Harden project view mutations
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000
1 parent 82ff55b commit b5eb2a7

4 files changed

Lines changed: 287 additions & 61 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1140,7 +1140,7 @@ The following sets of tools are available:
11401140
- `target_date`: The target date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method. (string, optional)
11411141
- `title`: The project title. Required for 'create_project' method. (string, optional)
11421142
- `updated_field`: The field/value to apply, using {"id": 123, "value": ...} or {"name": "Status", "value": ...}; null clears the field. Required for 'update_project_item' and 'update_project_items', where one top-level field/value applies to every item in a batch. For 'update_project_item' SINGLE_SELECT fields, the name form accepts option names; the ID form expects an option ID. (object, optional)
1143-
- `view_id`: Project view node ID for update or delete. (string, optional)
1143+
- `view_id`: Project view node ID for update or delete; must belong to owner/project_number. (string, optional)
11441144
- `visible_fields`: Field database IDs for table or board creation; unsupported for roadmap. (string[], optional)
11451145

11461146
</details>

pkg/github/__toolsnaps__/projects_write.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@
246246
"type": "object"
247247
},
248248
"view_id": {
249-
"description": "Project view node ID for update or delete.",
249+
"description": "Project view node ID for update or delete; must belong to owner/project_number.",
250250
"type": "string"
251251
},
252252
"visible_fields": {

pkg/github/projects.go

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,17 @@ type projectViewNodeQuery struct {
161161
} `graphql:"node(id: $id)"`
162162
}
163163

164+
type projectViewParentQuery struct {
165+
Node struct {
166+
ProjectView struct {
167+
ID githubv4.ID
168+
Project struct {
169+
ID githubv4.ID
170+
}
171+
} `graphql:"... on ProjectV2View"`
172+
} `graphql:"node(id: $id)"`
173+
}
174+
164175
// CreateProjectV2ViewRequest is the REST request for creating a project view.
165176
type CreateProjectV2ViewRequest struct {
166177
Name string `json:"name"`
@@ -729,7 +740,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
729740
},
730741
"view_id": {
731742
Type: "string",
732-
Description: "Project view node ID for update or delete.",
743+
Description: "Project view node ID for update or delete; must belong to owner/project_number.",
733744
},
734745
"name": {
735746
Type: "string",
@@ -972,9 +983,9 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
972983
case projectsMethodCreateProjectView:
973984
return createProjectView(ctx, client, args, owner, ownerType, projectNumber)
974985
case projectsMethodUpdateProjectView:
975-
return updateProjectView(ctx, gqlClient, args)
986+
return updateProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber)
976987
case projectsMethodDeleteProjectView:
977-
return deleteProjectView(ctx, gqlClient, args)
988+
return deleteProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber)
978989
default:
979990
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
980991
}
@@ -1922,15 +1933,7 @@ func createProjectView(ctx context.Context, client *github.Client, args map[stri
19221933
case "org":
19231934
endpoint = fmt.Sprintf("orgs/%s/projectsV2/%d/views", owner, projectNumber)
19241935
case "user":
1925-
user, resp, err := client.Users.Get(ctx, owner)
1926-
if err != nil {
1927-
return ghErrors.NewGitHubAPIErrorResponse(ctx, ProjectViewCreateFailedError, resp, err), nil, nil
1928-
}
1929-
userID := user.GetID()
1930-
if userID == 0 {
1931-
return utils.NewToolResultError(fmt.Sprintf("%s: user response did not include an ID", ProjectViewCreateFailedError)), nil, nil
1932-
}
1933-
endpoint = fmt.Sprintf("users/%d/projectsV2/%d/views", userID, projectNumber)
1936+
endpoint = fmt.Sprintf("users/%s/projectsV2/%d/views", owner, projectNumber)
19341937
default:
19351938
return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil
19361939
}
@@ -1963,7 +1966,29 @@ func createProjectView(ctx context.Context, client *github.Client, args map[stri
19631966
return MarshalledTextResult(view), nil, nil
19641967
}
19651968

1966-
func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any) (*mcp.CallToolResult, any, error) {
1969+
func verifyProjectViewParent(ctx context.Context, gqlClient *githubv4.Client, viewID, owner, ownerType string, projectNumber int) error {
1970+
expectedProjectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber)
1971+
if err != nil {
1972+
return fmt.Errorf("failed to resolve requested project: %w", err)
1973+
}
1974+
if expectedProjectID == nil || expectedProjectID == "" {
1975+
return fmt.Errorf("requested project was not found")
1976+
}
1977+
1978+
var query projectViewParentQuery
1979+
if err := gqlClient.Query(ctx, &query, map[string]any{"id": githubv4.ID(viewID)}); err != nil {
1980+
return fmt.Errorf("failed to resolve project view: %w", err)
1981+
}
1982+
if query.Node.ProjectView.ID == nil || query.Node.ProjectView.ID == "" {
1983+
return fmt.Errorf("node is not a ProjectV2View or was not found")
1984+
}
1985+
if query.Node.ProjectView.Project.ID != expectedProjectID {
1986+
return fmt.Errorf("project view does not belong to the requested project")
1987+
}
1988+
return nil
1989+
}
1990+
1991+
func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) {
19671992
viewID, err := RequiredParam[string](args, "view_id")
19681993
if err != nil {
19691994
return utils.NewToolResultError(err.Error()), nil, nil
@@ -2003,10 +2028,13 @@ func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map
20032028
value := githubv4.String(filter)
20042029
input.Filter = &value
20052030
}
2031+
if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil {
2032+
return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewUpdateFailedError, err)), nil, nil
2033+
}
20062034

20072035
var mutation struct {
20082036
UpdateProjectV2View struct {
2009-
ProjectV2View projectViewNode
2037+
ProjectV2View projectViewNode `graphql:"projectV2View"`
20102038
} `graphql:"updateProjectV2View(input: $input)"`
20112039
}
20122040
if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil {
@@ -2018,17 +2046,20 @@ func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map
20182046
return MarshalledTextResult(convertToMinimalProjectView(mutation.UpdateProjectV2View.ProjectV2View)), nil, nil
20192047
}
20202048

2021-
func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any) (*mcp.CallToolResult, any, error) {
2049+
func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) {
20222050
viewID, err := RequiredParam[string](args, "view_id")
20232051
if err != nil {
20242052
return utils.NewToolResultError(err.Error()), nil, nil
20252053
}
2054+
if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil {
2055+
return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil
2056+
}
20262057
input := DeleteProjectV2ViewInput{ViewID: githubv4.ID(viewID)}
20272058
var mutation struct {
20282059
DeleteProjectV2View struct {
20292060
ProjectV2View struct {
20302061
ID githubv4.ID
2031-
}
2062+
} `graphql:"projectV2View"`
20322063
} `graphql:"deleteProjectV2View(input: $input)"`
20332064
}
20342065
if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil {

0 commit comments

Comments
 (0)