From d4cd79e28c163d272d0930db529243ff214d0caf Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 12 May 2026 17:33:57 +0200 Subject: [PATCH] Use int64 for GitHub database IDs and add lint check Change all struct fields representing GitHub database IDs from int to int64 to match the API spec and prevent potential overflow on 32-bit architectures. Add a custom go/analysis linter (idtype-checker) that flags struct fields with ID-like names or JSON tags using int instead of int64, integrated into make lint. Closes #9247 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Makefile | 1 + cmd/idtype-checker/main.go | 16 +++ internal/codespaces/api/api.go | 12 +- pkg/cmd/agent-task/capi/job.go | 4 +- pkg/cmd/cache/delete/delete.go | 2 +- pkg/cmd/cache/shared/shared.go | 2 +- pkg/cmd/codespace/common.go | 6 +- pkg/cmd/codespace/create.go | 2 +- pkg/cmd/codespace/create_test.go | 14 +-- pkg/cmd/codespace/mock_api.go | 42 +++---- pkg/cmd/gpg-key/delete/delete.go | 2 +- pkg/cmd/gpg-key/delete/http.go | 2 +- pkg/cmd/repo/autolink/shared/autolink.go | 2 +- pkg/cmd/repo/deploy-key/list/http.go | 2 +- pkg/cmd/repo/deploy-key/list/list.go | 2 +- pkg/cmd/ruleset/list/list.go | 2 +- pkg/cmd/ruleset/shared/shared.go | 8 +- pkg/cmd/ruleset/view/view.go | 6 +- pkg/cmd/skills/publish/publish.go | 2 +- pkg/cmd/ssh-key/list/list.go | 2 +- pkg/cmd/ssh-key/shared/user_keys.go | 2 +- pkg/linter/idtype/analyzer.go | 119 ++++++++++++++++++ pkg/linter/idtype/analyzer_test.go | 12 ++ .../idtype/testdata/src/example/example.go | 66 ++++++++++ 24 files changed, 272 insertions(+), 58 deletions(-) create mode 100644 cmd/idtype-checker/main.go create mode 100644 pkg/linter/idtype/analyzer.go create mode 100644 pkg/linter/idtype/analyzer_test.go create mode 100644 pkg/linter/idtype/testdata/src/example/example.go diff --git a/Makefile b/Makefile index c3b18f313..f930711a9 100644 --- a/Makefile +++ b/Makefile @@ -45,6 +45,7 @@ completions: bin/gh$(EXE) .PHONY: lint lint: golangci-lint run ./... + go run ./cmd/idtype-checker ./... # just convenience tasks around `go test` .PHONY: test diff --git a/cmd/idtype-checker/main.go b/cmd/idtype-checker/main.go new file mode 100644 index 000000000..b7cbf6f9e --- /dev/null +++ b/cmd/idtype-checker/main.go @@ -0,0 +1,16 @@ +// Command idtype-checker runs the idtype analyzer to flag struct fields +// representing GitHub database IDs that use int instead of int64. +// +// Usage: +// +// go run ./cmd/idtype-checker ./... +package main + +import ( + "github.com/cli/cli/v2/pkg/linter/idtype" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { + singlechecker.Main(idtype.Analyzer) +} diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 0d1eaf5b3..d85b6dc61 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -150,7 +150,7 @@ type RepositoryOwner struct { // Repository represents a GitHub repository. type Repository struct { - ID int `json:"id"` + ID int64 `json:"id"` FullName string `json:"full_name"` DefaultBranch string `json:"default_branch"` Owner RepositoryOwner `json:"owner"` @@ -602,7 +602,7 @@ type Machine struct { } // GetCodespacesMachines returns the codespaces machines for the given repo, branch and location. -func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*Machine, error) { +func (a *API) GetCodespacesMachines(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*Machine, error) { reqURL := fmt.Sprintf("%s/repositories/%d/codespaces/machines", a.githubAPI, repoID) req, err := http.NewRequest(http.MethodGet, reqURL, nil) if err != nil { @@ -642,7 +642,7 @@ func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, loc } // GetCodespacesPermissionsCheck returns a bool indicating whether the user has accepted permissions for the given repo and devcontainer path. -func (a *API) GetCodespacesPermissionsCheck(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { +func (a *API) GetCodespacesPermissionsCheck(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { reqURL := fmt.Sprintf("%s/repositories/%d/codespaces/permissions_check", a.githubAPI, repoID) req, err := http.NewRequest(http.MethodGet, reqURL, nil) if err != nil { @@ -804,7 +804,7 @@ func (a *API) GetCodespaceBillableOwner(ctx context.Context, nwo string) (*User, // CreateCodespaceParams are the required parameters for provisioning a Codespace. type CreateCodespaceParams struct { - RepositoryID int + RepositoryID int64 IdleTimeoutMinutes int RetentionPeriodMinutes *int Branch string @@ -855,7 +855,7 @@ func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams } type startCreateRequest struct { - RepositoryID int `json:"repository_id"` + RepositoryID int64 `json:"repository_id"` IdleTimeoutMinutes int `json:"idle_timeout_minutes,omitempty"` RetentionPeriodMinutes *int `json:"retention_period_minutes,omitempty"` Ref string `json:"ref"` @@ -1009,7 +1009,7 @@ type DevContainerEntry struct { // ListDevContainers returns a list of valid devcontainer.json files for the repo. Pass a negative limit to request all pages from // the API until all devcontainer.json files have been fetched. -func (a *API) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []DevContainerEntry, err error) { +func (a *API) ListDevContainers(ctx context.Context, repoID int64, branch string, limit int) (devcontainers []DevContainerEntry, err error) { perPage := 100 if limit > 0 && limit < 100 { perPage = limit diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index a09338695..bfc887ce3 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -35,12 +35,12 @@ type Job struct { } type JobActor struct { - ID int `json:"id"` + ID int64 `json:"id"` Login string `json:"login"` } type JobPullRequest struct { - ID int `json:"id"` + ID int64 `json:"id"` Number int `json:"number"` BaseRef string `json:"base_ref,omitempty"` } diff --git a/pkg/cmd/cache/delete/delete.go b/pkg/cmd/cache/delete/delete.go index 9cb188008..d8b620f7b 100644 --- a/pkg/cmd/cache/delete/delete.go +++ b/pkg/cmd/cache/delete/delete.go @@ -147,7 +147,7 @@ func deleteRun(opts *DeleteOptions) error { } } for _, cache := range caches.ActionsCaches { - toDelete = append(toDelete, strconv.Itoa(cache.Id)) + toDelete = append(toDelete, strconv.FormatInt(cache.Id, 10)) } } else { toDelete = append(toDelete, opts.Identifier) diff --git a/pkg/cmd/cache/shared/shared.go b/pkg/cmd/cache/shared/shared.go index 3e8096396..a853b143f 100644 --- a/pkg/cmd/cache/shared/shared.go +++ b/pkg/cmd/cache/shared/shared.go @@ -22,7 +22,7 @@ var CacheFields = []string{ type Cache struct { CreatedAt time.Time `json:"created_at"` - Id int `json:"id"` + Id int64 `json:"id"` Key string `json:"key"` LastAccessedAt time.Time `json:"last_accessed_at"` Ref string `json:"ref"` diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index e56e6c0b8..a410b1a7d 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -76,10 +76,10 @@ type apiClient interface { CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) EditCodespace(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) - GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*api.Machine, error) - GetCodespacesPermissionsCheck(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) + GetCodespacesMachines(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*api.Machine, error) + GetCodespacesPermissionsCheck(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) - ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []api.DevContainerEntry, err error) + ListDevContainers(ctx context.Context, repoID int64, branch string, limit int) (devcontainers []api.DevContainerEntry, err error) GetCodespaceRepoSuggestions(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) GetCodespaceBillableOwner(ctx context.Context, nwo string) (*api.User, error) HTTPClient() (*http.Client, error) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index bd2eb4623..734509a30 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -511,7 +511,7 @@ func (a *App) showStatus(ctx context.Context, codespace *api.Codespace) error { } // getMachineName prompts the user to select the machine type, or validates the machine if non-empty. -func getMachineName(ctx context.Context, apiClient apiClient, prompter SurveyPrompter, repoID int, machine, branch, location string, devcontainerPath string) (string, error) { +func getMachineName(ctx context.Context, apiClient apiClient, prompter SurveyPrompter, repoID int64, machine, branch, location string, devcontainerPath string) (string, error) { machines, err := apiClient.GetCodespacesMachines(ctx, repoID, branch, location, devcontainerPath) if err != nil { return "", fmt.Errorf("error requesting machine instance types: %w", err) diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index c5f11bd2f..9a959f5da 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -164,7 +164,7 @@ func TestApp_Create(t *testing.T) { name: "create codespace with nonexistent machine results in error", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*api.Machine, error) { + GetCodespacesMachinesFunc: func(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*api.Machine, error) { return []*api.Machine{ { Name: "GIGA", @@ -208,7 +208,7 @@ func TestApp_Create(t *testing.T) { name: "create codespace with devcontainer path results in selecting the correct machine type", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*api.Machine, error) { + GetCodespacesMachinesFunc: func(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*api.Machine, error) { if devcontainerPath == "" { return []*api.Machine{ { @@ -270,7 +270,7 @@ func TestApp_Create(t *testing.T) { name: "create codespace with default branch with default devcontainer if no path provided and no devcontainer files exist in the repo", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + ListDevContainersFunc: func(ctx context.Context, repoID int64, branch string, limit int) ([]api.DevContainerEntry, error) { return []api.DevContainerEntry{}, nil }, CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { @@ -305,7 +305,7 @@ func TestApp_Create(t *testing.T) { name: "returns error when getting devcontainer paths fails", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + ListDevContainersFunc: func(ctx context.Context, repoID int64, branch string, limit int) ([]api.DevContainerEntry, error) { return nil, fmt.Errorf("some error") }, }), @@ -793,7 +793,7 @@ func TestHandleAdditionalPermissions(t *testing.T) { CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { return nil, tt.createCodespaceErr }, - GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { + GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { if tt.pollForPermissionsErr != nil { return false, tt.pollForPermissionsErr } @@ -844,12 +844,12 @@ func apiCreateDefaults(c *apiClientMock) *apiClientMock { } } if c.ListDevContainersFunc == nil { - c.ListDevContainersFunc = func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + c.ListDevContainersFunc = func(ctx context.Context, repoID int64, branch string, limit int) ([]api.DevContainerEntry, error) { return []api.DevContainerEntry{{Path: ".devcontainer/devcontainer.json"}}, nil } } if c.GetCodespacesMachinesFunc == nil { - c.GetCodespacesMachinesFunc = func(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*api.Machine, error) { + c.GetCodespacesMachinesFunc = func(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*api.Machine, error) { return []*api.Machine{ { Name: "GIGA", diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index e4de0ae7a..c96bcf118 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -38,10 +38,10 @@ import ( // GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespace *codespacesAPI.Codespace, path string) ([]byte, error) { // panic("mock out the GetCodespaceRepositoryContents method") // }, -// GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { +// GetCodespacesMachinesFunc: func(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { // panic("mock out the GetCodespacesMachines method") // }, -// GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { +// GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { // panic("mock out the GetCodespacesPermissionsCheck method") // }, // GetOrgMemberCodespaceFunc: func(ctx context.Context, orgName string, userName string, codespaceName string) (*codespacesAPI.Codespace, error) { @@ -59,7 +59,7 @@ import ( // ListCodespacesFunc: func(ctx context.Context, opts codespacesAPI.ListCodespacesOptions) ([]*codespacesAPI.Codespace, error) { // panic("mock out the ListCodespaces method") // }, -// ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { +// ListDevContainersFunc: func(ctx context.Context, repoID int64, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { // panic("mock out the ListDevContainers method") // }, // ServerURLFunc: func() string { @@ -100,10 +100,10 @@ type apiClientMock struct { GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespace *codespacesAPI.Codespace, path string) ([]byte, error) // GetCodespacesMachinesFunc mocks the GetCodespacesMachines method. - GetCodespacesMachinesFunc func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) + GetCodespacesMachinesFunc func(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) // GetCodespacesPermissionsCheckFunc mocks the GetCodespacesPermissionsCheck method. - GetCodespacesPermissionsCheckFunc func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) + GetCodespacesPermissionsCheckFunc func(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) // GetOrgMemberCodespaceFunc mocks the GetOrgMemberCodespace method. GetOrgMemberCodespaceFunc func(ctx context.Context, orgName string, userName string, codespaceName string) (*codespacesAPI.Codespace, error) @@ -121,7 +121,7 @@ type apiClientMock struct { ListCodespacesFunc func(ctx context.Context, opts codespacesAPI.ListCodespacesOptions) ([]*codespacesAPI.Codespace, error) // ListDevContainersFunc mocks the ListDevContainers method. - ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) + ListDevContainersFunc func(ctx context.Context, repoID int64, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) // ServerURLFunc mocks the ServerURL method. ServerURLFunc func() string @@ -200,7 +200,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // RepoID is the repoID argument value. - RepoID int + RepoID int64 // Branch is the branch argument value. Branch string // Location is the location argument value. @@ -213,7 +213,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // RepoID is the repoID argument value. - RepoID int + RepoID int64 // Branch is the branch argument value. Branch string // DevcontainerPath is the devcontainerPath argument value. @@ -257,7 +257,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // RepoID is the repoID argument value. - RepoID int + RepoID int64 // Branch is the branch argument value. Branch string // Limit is the limit argument value. @@ -582,13 +582,13 @@ func (mock *apiClientMock) GetCodespaceRepositoryContentsCalls() []struct { } // GetCodespacesMachines calls GetCodespacesMachinesFunc. -func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { +func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { if mock.GetCodespacesMachinesFunc == nil { panic("apiClientMock.GetCodespacesMachinesFunc: method is nil but apiClient.GetCodespacesMachines was just called") } callInfo := struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Location string DevcontainerPath string @@ -611,14 +611,14 @@ func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int // len(mockedapiClient.GetCodespacesMachinesCalls()) func (mock *apiClientMock) GetCodespacesMachinesCalls() []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Location string DevcontainerPath string } { var calls []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Location string DevcontainerPath string @@ -630,13 +630,13 @@ func (mock *apiClientMock) GetCodespacesMachinesCalls() []struct { } // GetCodespacesPermissionsCheck calls GetCodespacesPermissionsCheckFunc. -func (mock *apiClientMock) GetCodespacesPermissionsCheck(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { +func (mock *apiClientMock) GetCodespacesPermissionsCheck(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { if mock.GetCodespacesPermissionsCheckFunc == nil { panic("apiClientMock.GetCodespacesPermissionsCheckFunc: method is nil but apiClient.GetCodespacesPermissionsCheck was just called") } callInfo := struct { Ctx context.Context - RepoID int + RepoID int64 Branch string DevcontainerPath string }{ @@ -657,13 +657,13 @@ func (mock *apiClientMock) GetCodespacesPermissionsCheck(ctx context.Context, re // len(mockedapiClient.GetCodespacesPermissionsCheckCalls()) func (mock *apiClientMock) GetCodespacesPermissionsCheckCalls() []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string DevcontainerPath string } { var calls []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string DevcontainerPath string } @@ -849,13 +849,13 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { } // ListDevContainers calls ListDevContainersFunc. -func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { +func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int64, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { if mock.ListDevContainersFunc == nil { panic("apiClientMock.ListDevContainersFunc: method is nil but apiClient.ListDevContainers was just called") } callInfo := struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Limit int }{ @@ -876,13 +876,13 @@ func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, br // len(mockedapiClient.ListDevContainersCalls()) func (mock *apiClientMock) ListDevContainersCalls() []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Limit int } { var calls []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Limit int } diff --git a/pkg/cmd/gpg-key/delete/delete.go b/pkg/cmd/gpg-key/delete/delete.go index d0fb5c0d6..7ca17de81 100644 --- a/pkg/cmd/gpg-key/delete/delete.go +++ b/pkg/cmd/gpg-key/delete/delete.go @@ -74,7 +74,7 @@ func deleteRun(opts *DeleteOptions) error { id := "" for _, gpgKey := range gpgKeys { if gpgKey.KeyID == opts.KeyID { - id = strconv.Itoa(gpgKey.ID) + id = strconv.FormatInt(gpgKey.ID, 10) break } } diff --git a/pkg/cmd/gpg-key/delete/http.go b/pkg/cmd/gpg-key/delete/http.go index cfb11b9c3..22b43133b 100644 --- a/pkg/cmd/gpg-key/delete/http.go +++ b/pkg/cmd/gpg-key/delete/http.go @@ -11,7 +11,7 @@ import ( ) type gpgKey struct { - ID int + ID int64 KeyID string `json:"key_id"` } diff --git a/pkg/cmd/repo/autolink/shared/autolink.go b/pkg/cmd/repo/autolink/shared/autolink.go index 28e975e7c..66db44e3d 100644 --- a/pkg/cmd/repo/autolink/shared/autolink.go +++ b/pkg/cmd/repo/autolink/shared/autolink.go @@ -3,7 +3,7 @@ package shared import "github.com/cli/cli/v2/pkg/cmdutil" type Autolink struct { - ID int `json:"id"` + ID int64 `json:"id"` IsAlphanumeric bool `json:"is_alphanumeric"` KeyPrefix string `json:"key_prefix"` URLTemplate string `json:"url_template"` diff --git a/pkg/cmd/repo/deploy-key/list/http.go b/pkg/cmd/repo/deploy-key/list/http.go index 4470cec04..134b128dc 100644 --- a/pkg/cmd/repo/deploy-key/list/http.go +++ b/pkg/cmd/repo/deploy-key/list/http.go @@ -13,7 +13,7 @@ import ( ) type deployKey struct { - ID int `json:"id"` + ID int64 `json:"id"` Key string `json:"key"` Title string `json:"title"` CreatedAt time.Time `json:"created_at"` diff --git a/pkg/cmd/repo/deploy-key/list/list.go b/pkg/cmd/repo/deploy-key/list/list.go index 79d00c912..b0475ddf2 100644 --- a/pkg/cmd/repo/deploy-key/list/list.go +++ b/pkg/cmd/repo/deploy-key/list/list.go @@ -81,7 +81,7 @@ func listRun(opts *ListOptions) error { now := time.Now() for _, deployKey := range deployKeys { - sshID := strconv.Itoa(deployKey.ID) + sshID := strconv.FormatInt(deployKey.ID, 10) t.AddField(sshID) t.AddField(deployKey.Title) sshType := "read-only" diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 8c79b5d96..1f3aabb4b 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -164,7 +164,7 @@ func listRun(opts *ListOptions) error { tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "NAME", "SOURCE", "STATUS", "RULES")) for _, rs := range result.Rulesets { - tp.AddField(strconv.Itoa(rs.DatabaseId), tableprinter.WithColor(cs.Cyan)) + tp.AddField(strconv.FormatInt(rs.DatabaseId, 10), tableprinter.WithColor(cs.Cyan)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) tp.AddField(shared.RulesetSource(rs)) tp.AddField(strings.ToLower(rs.Enforcement)) diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 536e8a5a9..e74c66221 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -10,7 +10,7 @@ import ( ) type RulesetGraphQL struct { - DatabaseId int + DatabaseId int64 Name string Target string Enforcement string @@ -24,13 +24,13 @@ type RulesetGraphQL struct { } type RulesetREST struct { - Id int + Id int64 Name string Target string Enforcement string CurrentUserCanBypass string `json:"current_user_can_bypass"` BypassActors []struct { - ActorId int `json:"actor_id"` + ActorId int64 `json:"actor_id"` ActorType string `json:"actor_type"` BypassMode string `json:"bypass_mode"` } `json:"bypass_actors"` @@ -50,7 +50,7 @@ type RulesetRule struct { Parameters map[string]interface{} RulesetSourceType string `json:"ruleset_source_type"` RulesetSource string `json:"ruleset_source"` - RulesetId int `json:"ruleset_id"` + RulesetId int64 `json:"ruleset_id"` } // Returns the source of the ruleset in the format "owner/name (repo)" or "owner (org)" diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index b535a1033..8cc43927c 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -150,7 +150,7 @@ func viewRun(opts *ViewOptions) error { } if rs != nil { - opts.ID = strconv.Itoa(rs.DatabaseId) + opts.ID = strconv.FormatInt(rs.DatabaseId, 10) // can't get a ruleset lower in the chain than what was queried, so no need to handle repos here if rs.Source.TypeName == "Organization" { @@ -185,7 +185,7 @@ func viewRun(opts *ViewOptions) error { } fmt.Fprintf(w, "\n%s\n", cs.Bold(rs.Name)) - fmt.Fprintf(w, "ID: %s\n", cs.Cyan(strconv.Itoa(rs.Id))) + fmt.Fprintf(w, "ID: %s\n", cs.Cyan(strconv.FormatInt(rs.Id, 10))) fmt.Fprintf(w, "Source: %s (%s)\n", rs.Source, rs.SourceType) fmt.Fprint(w, "Enforcement: ") @@ -262,7 +262,7 @@ func selectRulesetID(rsList *shared.RulesetList, p prompter.Prompter, cs *iostre for i, rs := range rsList.Rulesets { s := fmt.Sprintf( "%s: %s | %s | contains %s | configured in %s", - cs.Cyan(strconv.Itoa(rs.DatabaseId)), + cs.Cyan(strconv.FormatInt(rs.DatabaseId, 10)), rs.Name, strings.ToLower(rs.Enforcement), text.Pluralize(rs.Rules.TotalCount, "rule"), diff --git a/pkg/cmd/skills/publish/publish.go b/pkg/cmd/skills/publish/publish.go index c53ea0b6b..f5c3f990b 100644 --- a/pkg/cmd/skills/publish/publish.go +++ b/pkg/cmd/skills/publish/publish.go @@ -64,7 +64,7 @@ type tagEntry struct { // rulesetsResponse is a single ruleset from the rulesets API. type rulesetsResponse struct { - ID int `json:"id"` + ID int64 `json:"id"` Name string `json:"name"` Target string `json:"target"` Enforcement string `json:"enforcement"` diff --git a/pkg/cmd/ssh-key/list/list.go b/pkg/cmd/ssh-key/list/list.go index f69e57ea3..b92f4274c 100644 --- a/pkg/cmd/ssh-key/list/list.go +++ b/pkg/cmd/ssh-key/list/list.go @@ -83,7 +83,7 @@ func listRun(opts *ListOptions) error { now := time.Now() for _, sshKey := range sshKeys { - id := strconv.Itoa(sshKey.ID) + id := strconv.FormatInt(sshKey.ID, 10) if t.IsTTY() { t.AddField(sshKey.Title) t.AddField(id) diff --git a/pkg/cmd/ssh-key/shared/user_keys.go b/pkg/cmd/ssh-key/shared/user_keys.go index 035d00244..6a3d286ab 100644 --- a/pkg/cmd/ssh-key/shared/user_keys.go +++ b/pkg/cmd/ssh-key/shared/user_keys.go @@ -17,7 +17,7 @@ const ( ) type sshKey struct { - ID int + ID int64 Key string Title string Type string diff --git a/pkg/linter/idtype/analyzer.go b/pkg/linter/idtype/analyzer.go new file mode 100644 index 000000000..fcf659daa --- /dev/null +++ b/pkg/linter/idtype/analyzer.go @@ -0,0 +1,119 @@ +// Package idtype provides a go/analysis analyzer that flags struct fields +// representing GitHub database IDs that use int instead of int64. +// +// GitHub database IDs are internally 64-bit, and the REST API OpenAPI spec +// declares many of them with format: int64. Using Go's int (which is +// platform-dependent) risks overflow on 32-bit architectures. +package idtype + +import ( + "go/ast" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +var Analyzer = &analysis.Analyzer{ + Name: "idtype", + Doc: "checks that struct fields representing GitHub database IDs use int64 instead of int", + Run: run, + Requires: []*analysis.Analyzer{inspect.Analyzer}, +} + +func run(pass *analysis.Pass) (interface{}, error) { + insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.StructType)(nil), + } + + insp.Preorder(nodeFilter, func(n ast.Node) { + st := n.(*ast.StructType) + for _, field := range st.Fields.List { + if !isIntType(field.Type) { + continue + } + for _, name := range field.Names { + if isIDField(name.Name, field.Tag) { + pass.Reportf(field.Pos(), "struct field %s looks like a GitHub database ID but uses int; use int64 instead", name.Name) + } + } + } + }) + + return nil, nil +} + +// isIntType checks if the type expression is the bare "int" type. +func isIntType(expr ast.Expr) bool { + ident, ok := expr.(*ast.Ident) + return ok && ident.Name == "int" +} + +// isIDField returns true if the field name or JSON tag suggests this is a +// database ID. It checks: +// - Field name is exactly "ID" or "Id" +// - Field name ends with "ID" or "Id" (e.g. DatabaseId, RepositoryID, ActorId) +// - JSON tag is exactly "id" or ends with "_id" +func isIDField(fieldName string, tag *ast.BasicLit) bool { + if matchesIDName(fieldName) { + return true + } + if tag != nil && tag.Kind == token.STRING { + jsonTag := extractJSONTag(tag.Value) + if matchesIDTag(jsonTag) { + return true + } + } + return false +} + +// matchesIDName checks if a Go field name looks like a database ID field. +func matchesIDName(name string) bool { + if name == "ID" || name == "Id" { + return true + } + if strings.HasSuffix(name, "ID") || strings.HasSuffix(name, "Id") { + return true + } + return false +} + +// matchesIDTag checks if a JSON tag value looks like a database ID field. +func matchesIDTag(tag string) bool { + if tag == "" { + return false + } + if tag == "id" { + return true + } + if strings.HasSuffix(tag, "_id") { + return true + } + return false +} + +// extractJSONTag extracts the JSON field name from a struct tag literal. +// For example, given `json:"repository_id,omitempty"`, it returns "repository_id". +func extractJSONTag(rawTag string) string { + // Remove surrounding backticks or quotes + tag := strings.Trim(rawTag, "`\"") + + const prefix = `json:"` + idx := strings.Index(tag, prefix) + if idx < 0 { + return "" + } + tag = tag[idx+len(prefix):] + if end := strings.Index(tag, `"`); end >= 0 { + tag = tag[:end] + } + // Strip options like omitempty + if comma := strings.Index(tag, ","); comma >= 0 { + tag = tag[:comma] + } + return tag +} diff --git a/pkg/linter/idtype/analyzer_test.go b/pkg/linter/idtype/analyzer_test.go new file mode 100644 index 000000000..122406855 --- /dev/null +++ b/pkg/linter/idtype/analyzer_test.go @@ -0,0 +1,12 @@ +package idtype + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "example") +} diff --git a/pkg/linter/idtype/testdata/src/example/example.go b/pkg/linter/idtype/testdata/src/example/example.go new file mode 100644 index 000000000..de62dee5f --- /dev/null +++ b/pkg/linter/idtype/testdata/src/example/example.go @@ -0,0 +1,66 @@ +package example + +// Should be flagged - int fields with ID-like names or tags. + +type Repository struct { + ID int `json:"id"` // want `struct field ID looks like a GitHub database ID but uses int; use int64 instead` + Name string `json:"name"` +} + +type Cache struct { + Id int `json:"id"` // want `struct field Id looks like a GitHub database ID but uses int; use int64 instead` + Key string `json:"key"` +} + +type RulesetRule struct { + Type string + RulesetId int `json:"ruleset_id"` // want `struct field RulesetId looks like a GitHub database ID but uses int; use int64 instead` +} + +type Autolink struct { + ID int `json:"id"` // want `struct field ID looks like a GitHub database ID but uses int; use int64 instead` +} + +type DeployKey struct { + ID int // want `struct field ID looks like a GitHub database ID but uses int; use int64 instead` +} + +type Actor struct { + ActorId int `json:"actor_id"` // want `struct field ActorId looks like a GitHub database ID but uses int; use int64 instead` +} + +type WithDatabaseID struct { + DatabaseId int // want `struct field DatabaseId looks like a GitHub database ID but uses int; use int64 instead` +} + +type RepositoryIDField struct { + RepositoryID int `json:"repository_id"` // want `struct field RepositoryID looks like a GitHub database ID but uses int; use int64 instead` +} + +// Should NOT be flagged - correct types or non-ID fields. + +type CorrectRepository struct { + ID int64 `json:"id"` + Name string `json:"name"` +} + +type CorrectRule struct { + RulesetId int64 `json:"ruleset_id"` +} + +type NotAnID struct { + Count int `json:"count"` + Timeout int `json:"idle_timeout_minutes"` + Name string `json:"name"` + Description string +} + +type StringID struct { + ID string `json:"id"` +} + +type NumberField struct { + TotalCount int + Number int `json:"number"` + Port int `json:"port"` +}