diff --git a/.custom-gcl.yml b/.custom-gcl.yml index 9036aca5da..1b94d3d6ca 100644 --- a/.custom-gcl.yml +++ b/.custom-gcl.yml @@ -9,3 +9,6 @@ plugins: - module: "github.com/fleetdm/fleet/v4/tools/ci/setboolcheck" import: "github.com/fleetdm/fleet/v4/tools/ci/setboolcheck/cmd/gclplugin" path: "tools/ci/setboolcheck" + - module: "github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck" + import: "github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck/cmd/gclplugin" + path: "tools/ci/apiparamcheck" diff --git a/.golangci-incremental.yml b/.golangci-incremental.yml index a6066bfd05..aa5e12b82f 100644 --- a/.golangci-incremental.yml +++ b/.golangci-incremental.yml @@ -16,6 +16,7 @@ linters: - nilaway - setboolcheck - depguard + - apiparamcheck settings: gosec: # Only enable rules that are too noisy on existing code but valuable for new code. @@ -50,6 +51,9 @@ linters: setboolcheck: type: module description: Flags map[T]bool used as sets; suggests map[T]struct{} instead. + apiparamcheck: + type: module + description: Flags json/url/query struct tags using deprecated "team"/"teams" and "query"/"queries" terms. exclusions: generated: strict rules: diff --git a/cmd/fleetctl/fleetctl/generate_gitops.go b/cmd/fleetctl/fleetctl/generate_gitops.go index 7761e03a64..7579ed2c48 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops.go +++ b/cmd/fleetctl/fleetctl/generate_gitops.go @@ -900,7 +900,7 @@ func (cmd *GenerateGitopsCommand) generateSSOSettings(ssoSettings *fleet.SSOSett type GlobalOrTeamIntegrations struct { GlobalIntegrations *fleet.Integrations `json:"global_integrations,omitempty"` - TeamIntegrations *fleet.TeamIntegrations `json:"team_integrations,omitempty"` + TeamIntegrations *fleet.TeamIntegrations `json:"team_integrations,omitempty"` //nolint:apiparamcheck // internal routing key, not emitted to user } func (cmd *GenerateGitopsCommand) generateIntegrations(filePath string, integrations *GlobalOrTeamIntegrations) (map[string]interface{}, error) { diff --git a/cmd/fleetctl/fleetctl/get.go b/cmd/fleetctl/fleetctl/get.go index 7a59ad50ff..d702309cce 100644 --- a/cmd/fleetctl/fleetctl/get.go +++ b/cmd/fleetctl/fleetctl/get.go @@ -229,13 +229,13 @@ type UserRoles struct { } type TeamRole struct { - Team string `json:"team"` + Team string `json:"team" renameto:"fleet"` // renameto doesn't actually do anything here but adding for visibility Role string `json:"role"` } type UserRole struct { GlobalRole *string `json:"global_role"` - Teams []TeamRole `json:"teams"` + Teams []TeamRole `json:"teams" renameto:"fleets"` // renameto doesn't actually do anything here but adding for visibility } func usersToUserRoles(users []fleet.User) UserRoles { diff --git a/server/datastore/mysql/migrations/tables/20210818151828_AddJSONKeyValueTable.go b/server/datastore/mysql/migrations/tables/20210818151828_AddJSONKeyValueTable.go index 206258e7f9..61a26697a1 100644 --- a/server/datastore/mysql/migrations/tables/20210818151828_AddJSONKeyValueTable.go +++ b/server/datastore/mysql/migrations/tables/20210818151828_AddJSONKeyValueTable.go @@ -48,13 +48,13 @@ type testAppConfig struct { } `json:"host_expiry_settings"` ServerSettings struct { ServerURL string `json:"server_url"` - LiveQueryDisabled bool `json:"live_query_disabled"` + LiveQueryDisabled bool `json:"live_query_disabled"` //nolint:apiparamcheck // frozen migration; preserves historical shape EnableAnalytics bool `json:"enable_analytics"` } `json:"server_settings"` HostSettings struct { EnableHostUsers bool `json:"enable_host_users"` EnableSoftwareInventory bool `json:"enable_software_inventory"` - AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"` + AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"` //nolint:apiparamcheck // frozen migration; preserves historical shape } `json:"host_settings"` VulnerabilitySettings struct { DatabasesPath string `json:"databases_path"` diff --git a/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured.go b/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured.go index 3d7db95b63..b0c96faf87 100644 --- a/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured.go +++ b/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured.go @@ -31,7 +31,7 @@ func Up_20240320145650(tx *sql.Tx) error { type macosSetupAssistantArgs struct { Task string `json:"task"` - TeamID *uint `json:"team_id,omitempty"` + TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // frozen migration; preserves historical shape HostSerialNumbers []string `json:"host_serial_numbers,omitempty"` } argsJSON, err := json.Marshal(macosSetupAssistantArgs{Task: taskName}) diff --git a/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured_test.go b/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured_test.go index 5ec92d352a..7ba4f6a52c 100644 --- a/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured_test.go +++ b/server/datastore/mysql/migrations/tables/20240320145650_UpdateDEPProfilesToAwaitDeviceConfigured_test.go @@ -13,7 +13,7 @@ func TestUp_20240320145650(t *testing.T) { type macosSetupAssistantArgs struct { Task string `json:"task"` - TeamID *uint `json:"team_id,omitempty"` + TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // matches frozen migration payload shape HostSerialNumbers []string `json:"host_serial_numbers,omitempty"` } diff --git a/server/fleet/agent_options.go b/server/fleet/agent_options.go index 1a2ea92d75..ba1cb970ce 100644 --- a/server/fleet/agent_options.go +++ b/server/fleet/agent_options.go @@ -205,7 +205,7 @@ type osqueryAgentOptions struct { FileAccesses []string `json:"file_accesses"` // Documentation for the following 2 fields is "hidden" in osquery's FIM page: // https://osquery.readthedocs.io/en/stable/deployment/file-integrity-monitoring/ - FilePathsQuery map[string][]string `json:"file_paths_query"` + FilePathsQuery map[string][]string `json:"file_paths_query"` //nolint:apiparamcheck // osquery FIM field ExcludePaths map[string][]string `json:"exclude_paths"` YARA struct { diff --git a/server/fleet/app.go b/server/fleet/app.go index ea0576aa7b..4565fad2bf 100644 --- a/server/fleet/app.go +++ b/server/fleet/app.go @@ -189,7 +189,7 @@ type MDM struct { AppleServerURL string `json:"apple_server_url"` // Deprecated: use AppleBussinessManager instead - DeprecatedAppleBMDefaultTeam string `json:"apple_bm_default_team,omitempty"` + DeprecatedAppleBMDefaultTeam string `json:"apple_bm_default_team,omitempty"` //nolint:apiparamcheck // not renaming already-deprecated field // AppleBusinessManager defines the associations between ABM tokens // and the teams used to assign hosts when they're ingested from ABM. @@ -1348,8 +1348,8 @@ type ActivityExpirySettings struct { type Features struct { EnableHostUsers bool `json:"enable_host_users"` EnableSoftwareInventory bool `json:"enable_software_inventory"` - AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"` - DetailQueryOverrides map[string]*string `json:"detail_query_overrides,omitempty"` + AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"` //nolint:apiparamcheck // osquery host-details queries + DetailQueryOverrides map[string]*string `json:"detail_query_overrides,omitempty"` //nolint:apiparamcheck // osquery detail-query overrides HistoricalData HistoricalDataSettings `json:"historical_data"` ///////////////////////////////////////////////////////////////// diff --git a/server/fleet/campaigns.go b/server/fleet/campaigns.go index ebe9b27d91..b370be37c8 100644 --- a/server/fleet/campaigns.go +++ b/server/fleet/campaigns.go @@ -39,7 +39,7 @@ type DistributedQueryCampaignTarget struct { // thus it should be kept as small as possible. type DistributedQueryResult struct { // DistributedQueryCampaignID is the unique ID of the live query campaign. - DistributedQueryCampaignID uint `json:"distributed_query_execution_id"` + DistributedQueryCampaignID uint `json:"distributed_query_execution_id"` //nolint:apiparamcheck // osquery distributed-query concept // Host holds the host's data from where the query result comes from. Host ResultHostData `json:"host"` Rows []map[string]string `json:"rows"` diff --git a/server/fleet/db.go b/server/fleet/db.go index 1df8253a2f..c51f24cdba 100644 --- a/server/fleet/db.go +++ b/server/fleet/db.go @@ -7,10 +7,10 @@ import "github.com/jmoiron/sqlx" type DBLock struct { WaitingTrxID string `db:"waiting_trx_id" json:"waiting_trx_id"` WaitingThread uint64 `db:"waiting_thread" json:"waiting_thread"` - WaitingQuery *string `db:"waiting_query" json:"waiting_query,omitempty"` + WaitingQuery *string `db:"waiting_query" json:"waiting_query,omitempty"` //nolint:apiparamcheck // MySQL InnoDB lock query text BlockingTrxID string `db:"blocking_trx_id" json:"blocking_trx_id"` BlockingThread uint64 `db:"blocking_thread" json:"blocking_thread"` - BlockingQuery *string `db:"blocking_query" json:"blocking_query,omitempty"` + BlockingQuery *string `db:"blocking_query" json:"blocking_query,omitempty"` //nolint:apiparamcheck // MySQL InnoDB lock query text } // DBReader is an interface that defines the methods required for reads. diff --git a/server/fleet/hosts.go b/server/fleet/hosts.go index 6152f737a3..2ad3092294 100644 --- a/server/fleet/hosts.go +++ b/server/fleet/hosts.go @@ -395,7 +395,7 @@ type Host struct { // add a "reason" field with well-known labels so we know what condition(s) // are expected to clear the timestamp. For now there's a single use-case // so we don't need this. - RefetchCriticalQueriesUntil *time.Time `json:"refetch_critical_queries_until" db:"refetch_critical_queries_until" csv:"-"` + RefetchCriticalQueriesUntil *time.Time `json:"refetch_critical_queries_until" db:"refetch_critical_queries_until" csv:"-"` //nolint:apiparamcheck // DEPAssignedToFleet is set to true if the host is assigned to Fleet in Apple Business. // It is a *bool becase we want it to be returned from only a subset of endpoints related to diff --git a/server/fleet/scripts.go b/server/fleet/scripts.go index 4ae230e4aa..2e948ad624 100644 --- a/server/fleet/scripts.go +++ b/server/fleet/scripts.go @@ -600,7 +600,7 @@ type SoftwareInstallerPayload struct { // the path field, this uses "script://filename" to pass the filename; in that // case InstallScript contains the script content directly. URL string `json:"url"` - PreInstallQuery string `json:"pre_install_query"` + PreInstallQuery string `json:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install // InstallScript is the script to run after downloading the installer. For script // packages via "script://" URL, this contains the package content itself. InstallScript string `json:"install_script"` diff --git a/server/fleet/software.go b/server/fleet/software.go index 9157135cfb..7a37701c70 100644 --- a/server/fleet/software.go +++ b/server/fleet/software.go @@ -629,7 +629,7 @@ func (hse *HostSoftwareEntry) UnmarshalJSON(b []byte) error { type PathSignatureInformation struct { InstalledPath string `json:"installed_path"` - TeamIdentifier string `json:"team_identifier"` + TeamIdentifier string `json:"team_identifier"` //nolint:apiparamcheck // Apple developer team identifier (code-signing) // json struct tag difference here is for backwards compatibility. API field was initially "hash_sha256", though it is specifically the CD hash (sha256). CDHashSHA256 *string `json:"hash_sha256"` ExecutableSHA256 *string `json:"executable_sha256"` diff --git a/server/fleet/software_installer.go b/server/fleet/software_installer.go index 9719edeb28..aa739bdddb 100644 --- a/server/fleet/software_installer.go +++ b/server/fleet/software_installer.go @@ -95,7 +95,7 @@ type SoftwareInstaller struct { // UninstallScriptContentID is the ID of the uninstall script content. UninstallScriptContentID uint `json:"-" db:"uninstall_script_content_id"` // PreInstallQuery is the query to run as a condition to installing the software package. - PreInstallQuery string `json:"pre_install_query" db:"pre_install_query"` + PreInstallQuery string `json:"pre_install_query" db:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install // PostInstallScript is the script to run after installing the software package. PostInstallScript string `json:"post_install_script" db:"post_install_script"` // UninstallScript is the script to run to uninstall the software package. @@ -423,7 +423,7 @@ type HostSoftwareInstallerResult struct { // Output is the output of the software installer package on the host. Output *string `json:"output" db:"install_script_output"` // PreInstallQueryOutput is the output of the pre-install query on the host. - PreInstallQueryOutput *string `json:"pre_install_query_output" db:"pre_install_query_output"` + PreInstallQueryOutput *string `json:"pre_install_query_output" db:"pre_install_query_output"` //nolint:apiparamcheck // SQL precondition output // PostInstallScriptOutput is the output of the post-install script on the host. PostInstallScriptOutput *string `json:"post_install_script_output" db:"post_install_script_output"` // CreatedAt is the time the software installer request was triggered. @@ -825,7 +825,7 @@ func (s *SoftwarePackageOrApp) FullyQualifiedName() string { type SoftwarePackageSpec struct { URL string `json:"url"` SelfService bool `json:"self_service"` - PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"` + PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install InstallScript TeamSpecSoftwareAsset `json:"install_script"` PostInstallScript TeamSpecSoftwareAsset `json:"post_install_script"` UninstallScript TeamSpecSoftwareAsset `json:"uninstall_script"` @@ -885,7 +885,7 @@ type MaintainedAppSpec struct { Slug string `json:"slug"` Version string `json:"version"` SelfService bool `json:"self_service"` - PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"` + PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install InstallScript TeamSpecSoftwareAsset `json:"install_script"` PostInstallScript TeamSpecSoftwareAsset `json:"post_install_script"` UninstallScript TeamSpecSoftwareAsset `json:"uninstall_script"` diff --git a/server/fleet/software_title_icons.go b/server/fleet/software_title_icons.go index e1e14fd1bf..7952c9b1b2 100644 --- a/server/fleet/software_title_icons.go +++ b/server/fleet/software_title_icons.go @@ -26,7 +26,7 @@ type SoftwareTitleIcon struct { // can properly check team ownership (rego marshals the struct to JSON to pass it to // the rego policies script). This struct is never marshalled directly to JSON in // API responses at this time so it doesn't affect anything else. - TeamID uint `db:"team_id" json:"team_id"` // TODO -- rename to `fleet_id` when authz code switches to using `fleet_id` instead of `team_id` + TeamID uint `db:"team_id" json:"team_id"` //nolint:apiparamcheck // TODO -- rename when authz code switches to using `fleet_id` instead of `team_id` SoftwareTitleID uint `db:"software_title_id"` StorageID string `db:"storage_id"` Filename string `db:"filename"` diff --git a/server/fleet/statistics.go b/server/fleet/statistics.go index e03efdb0f0..14d7d3265f 100644 --- a/server/fleet/statistics.go +++ b/server/fleet/statistics.go @@ -19,9 +19,9 @@ type StatisticsPayload struct { NumHostSoftwareInstalledPaths int `json:"numHostSoftwareInstalledPaths"` NumSoftwareCPEs int `json:"numSoftwareCPEs"` NumSoftwareCVEs int `json:"numSoftwareCVEs"` - NumTeams int `json:"numTeams"` + NumTeams int `json:"numTeams"` //nolint:apiparamcheck // don't want to break analytics ingestion NumPolicies int `json:"numPolicies"` - NumQueries int `json:"numQueries"` + NumQueries int `json:"numQueries"` //nolint:apiparamcheck // don't want to break analytics ingestion NumLabels int `json:"numLabels"` SoftwareInventoryEnabled bool `json:"softwareInventoryEnabled"` VulnDetectionEnabled bool `json:"vulnDetectionEnabled"` @@ -31,7 +31,7 @@ type StatisticsPayload struct { HostExpiryEnabled bool `json:"hostExpiryEnabled"` MDMWindowsEnabled bool `json:"mdmWindowsEnabled"` MDMRecoveryLockPasswordEnabled bool `json:"mdmRecoveryLockPasswordEnabled"` - LiveQueryDisabled bool `json:"liveQueryDisabled"` + LiveQueryDisabled bool `json:"liveQueryDisabled"` //nolint:apiparamcheck // osquery live-query feature NumWeeklyActiveUsers int `json:"numWeeklyActiveUsers"` // NumWeeklyPolicyViolationDaysActual is an aggregate count of actual policy violation days. One // policy violation day is added for each policy that a host is failing as of the time the count diff --git a/server/platform/endpointer/json_key_duplicator_test.go b/server/platform/endpointer/json_key_duplicator_test.go index a4b3b57ead..f47d1169f8 100644 --- a/server/platform/endpointer/json_key_duplicator_test.go +++ b/server/platform/endpointer/json_key_duplicator_test.go @@ -407,7 +407,7 @@ func TestDuplicateJSONKeysWithEncoder(t *testing.T) { } type response struct { - TeamID int `json:"team_id"` + TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings Name string `json:"name"` } diff --git a/server/platform/endpointer/json_key_rewriter_test.go b/server/platform/endpointer/json_key_rewriter_test.go index 9216aa6938..5ac9ff9698 100644 --- a/server/platform/endpointer/json_key_rewriter_test.go +++ b/server/platform/endpointer/json_key_rewriter_test.go @@ -336,7 +336,7 @@ func TestJSONKeyRewriteReader_WithJSONDecoderOldKey(t *testing.T) { rewriter := NewJSONKeyRewriteReader(strings.NewReader(input), rules) type request struct { - TeamID int `json:"team_id"` + TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings Name string `json:"name"` } var req request @@ -356,7 +356,7 @@ func TestJSONKeyRewriteReader_WithJSONDecoderNewKey(t *testing.T) { rewriter := NewJSONKeyRewriteReader(strings.NewReader(input), rules) type request struct { - TeamID int `json:"team_id"` + TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings Name string `json:"name"` } var req request @@ -374,7 +374,7 @@ func TestJSONKeyRewriteReader_AliasConflictWithJSONDecoder(t *testing.T) { rewriter := NewJSONKeyRewriteReader(strings.NewReader(input), rules) type request struct { - TeamID int `json:"team_id"` + TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings } var req request err := json.NewDecoder(rewriter).Decode(&req) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index ba7d09eefb..ea5431870b 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -3008,7 +3008,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamWebhookConfig() { // Test that we can configure webhooks for "No Team" (team ID 0) // Use a generic response that will work with DefaultTeam var defaultTeamResp struct { - Team *fleet.DefaultTeam `json:"team"` + Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape } // First clear any existing webhook configuration for "No Team" @@ -3042,7 +3042,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamWebhookConfig() { // Get the config again to verify it persisted defaultTeamResp = struct { - Team *fleet.DefaultTeam `json:"team"` + Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape }{} s.DoJSON("GET", "/api/latest/fleet/teams/0", nil, http.StatusOK, &defaultTeamResp) require.Equal(t, uint(0), defaultTeamResp.Team.ID) @@ -3131,7 +3131,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamFailingPolicyWebhookTrigger() // Configure webhook for "No Team" - only include pol1 and pol2 var defaultTeamResp struct { - Team *fleet.DefaultTeam `json:"team"` + Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape } s.DoJSON("PATCH", "/api/latest/fleet/teams/0", fleet.TeamPayload{WebhookSettings: &fleet.TeamWebhookSettings{ FailingPoliciesWebhook: fleet.FailingPoliciesWebhookSettings{ @@ -3169,7 +3169,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamFailingPolicyWebhookTrigger() // Test that we can configure and retrieve the "No Team" webhook settings defaultTeamResp = struct { - Team *fleet.DefaultTeam `json:"team"` + Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape }{} s.DoJSON("GET", "/api/latest/fleet/teams/0", nil, http.StatusOK, &defaultTeamResp) require.Equal(t, uint(0), defaultTeamResp.Team.ID) @@ -28834,7 +28834,7 @@ func (s *integrationEnterpriseTestSuite) TestCreateAPIOnlyUserPremium() { APIOnly bool `json:"api_only"` GlobalRole *string `json:"global_role"` APIEndpoints []apiEndpoint `json:"api_endpoints"` - Teams []teamEntry `json:"teams"` + Teams []teamEntry `json:"teams"` //nolint:apiparamcheck // test helper; matches server response shape } `json:"user"` Token string `json:"token"` Err string `json:"error,omitempty"` @@ -29009,7 +29009,7 @@ func (s *integrationEnterpriseTestSuite) TestModifyAPIOnlyUserPremium() { Teams []struct { ID uint `json:"id"` Role string `json:"role"` - } `json:"teams"` + } `json:"teams"` //nolint:apiparamcheck // test helper; matches server response shape } `json:"user"` Err string `json:"error,omitempty"` } diff --git a/server/service/integration_mdm_dep_test.go b/server/service/integration_mdm_dep_test.go index c25dd38a58..4d77e6dcf0 100644 --- a/server/service/integration_mdm_dep_test.go +++ b/server/service/integration_mdm_dep_test.go @@ -985,7 +985,7 @@ func (s *integrationMDMTestSuite) TestDEPProfileAssignment() { require.NotNil(t, pending[0].Args) var gotArgs struct { Task string `json:"task"` - TeamID *uint `json:"team_id,omitempty"` + TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // matches worker job payload shape (see server/worker/macos_setup_assistant.go) HostSerialNumbers []string `json:"host_serial_numbers,omitempty"` } require.NoError(t, json.Unmarshal(*pending[0].Args, &gotArgs)) diff --git a/server/service/maintained_apps.go b/server/service/maintained_apps.go index 77e7a7b2f1..90d1602a9e 100644 --- a/server/service/maintained_apps.go +++ b/server/service/maintained_apps.go @@ -13,14 +13,14 @@ import ( ) type addFleetMaintainedAppRequest struct { - TeamID *uint `json:"team_id"` + TeamID *uint `json:"team_id"` //nolint:apiparamcheck // alias handled manually via sibling FleetID field (see DecodeRequest below) // Note that we're adding an explicit FleetID field rather than using `renameto`. // The POST /software/fleet_maintained_apps endpoint has a custom decoder // and in this special case it's easier to handle the aliasing manually. FleetID *uint `json:"fleet_id"` AppID uint `json:"fleet_maintained_app_id"` InstallScript string `json:"install_script"` - PreInstallQuery string `json:"pre_install_query"` + PreInstallQuery string `json:"pre_install_query"` //nolint:apiparamcheck PostInstallScript string `json:"post_install_script"` SelfService bool `json:"self_service"` UninstallScript string `json:"uninstall_script"` diff --git a/server/worker/apple_mdm.go b/server/worker/apple_mdm.go index f4923f0ce2..ded6471315 100644 --- a/server/worker/apple_mdm.go +++ b/server/worker/apple_mdm.go @@ -58,7 +58,7 @@ func (a *AppleMDM) Name() string { type appleMDMArgs struct { Task AppleMDMTask `json:"task"` HostUUID string `json:"host_uuid"` - TeamID *uint `json:"team_id,omitempty"` + TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // EnrollReference is the UUID of the MDM IdP account used to enroll the // device. It is used to set the username and full name of the user // associated with the device. diff --git a/server/worker/macos_setup_assistant.go b/server/worker/macos_setup_assistant.go index f14ca69158..7f55e36926 100644 --- a/server/worker/macos_setup_assistant.go +++ b/server/worker/macos_setup_assistant.go @@ -46,7 +46,7 @@ func (m *MacosSetupAssistant) Name() string { // macosSetupAssistantArgs is the payload for the macos setup assistant job. type macosSetupAssistantArgs struct { Task MacosSetupAssistantTask `json:"task"` - TeamID *uint `json:"team_id,omitempty"` + TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // Note that only DEP-enrolled hosts in Fleet MDM should be provided. HostSerialNumbers []string `json:"host_serial_numbers,omitempty"` } diff --git a/server/worker/software_worker.go b/server/worker/software_worker.go index 4d0acd09f9..8bfad9bc46 100644 --- a/server/worker/software_worker.go +++ b/server/worker/software_worker.go @@ -46,13 +46,13 @@ type softwareWorkerArgs struct { EnterpriseName string `json:"enterprise_name,omitempty"` // AppTeamID is *not* a team ID, it is the vpp_apps_teams.id value. This is a bit confusing // as a name, but that is what is expected in this field. - AppTeamID uint `json:"app_team_id,omitempty"` + AppTeamID uint `json:"app_team_id,omitempty"` //nolint:apiparamcheck HostID uint `json:"host_id,omitempty"` // HostEnrollTeamID is the team ID associated with the host at the time // of enrollment, which is the one used to run the setup experience. // A value of 0 is used to represent "no team". - HostEnrollTeamID uint `json:"host_enroll_team_id,omitempty"` + HostEnrollTeamID uint `json:"host_enroll_team_id,omitempty"` //nolint:apiparamcheck // not user-facing // PolicyID is the Android Management API Policy ID associated with the host, *not* // a Fleet policy ID. @@ -281,7 +281,8 @@ func (v *SoftwareWorker) makeAndroidAppsAvailableForHost(ctx context.Context, ho } func (v *SoftwareWorker) runAndroidSetupExperience(ctx context.Context, - hostUUID string, hostEnrollTeamID uint, enterpriseName string) error { + hostUUID string, hostEnrollTeamID uint, enterpriseName string, +) error { host, err := v.Datastore.AndroidHostLiteByHostUUID(ctx, hostUUID) if err != nil { return ctxerr.Wrapf(ctx, err, "getting android host lite by uuid %s", hostUUID) @@ -400,8 +401,8 @@ func (v *SoftwareWorker) bulkMakeAndroidAppsAvailableForHost(ctx context.Context } func buildApplicationPolicyWithConfig(ctx context.Context, appIDs []string, - configsByAppID map[string]json.RawMessage, installType string) ([]*androidmanagement.ApplicationPolicy, error) { - + configsByAppID map[string]json.RawMessage, installType string, +) ([]*androidmanagement.ApplicationPolicy, error) { appPolicies := make([]*androidmanagement.ApplicationPolicy, 0, len(appIDs)) for _, appID := range appIDs { var androidAppConfig struct { @@ -430,8 +431,8 @@ func buildApplicationPolicyWithConfig(ctx context.Context, appIDs []string, } func QueueRunAndroidSetupExperience(ctx context.Context, ds fleet.Datastore, logger *slog.Logger, - hostUUID string, hostEnrollTeamID *uint, enterpriseName string) error { - + hostUUID string, hostEnrollTeamID *uint, enterpriseName string, +) error { var enrollTeamID uint if hostEnrollTeamID != nil { enrollTeamID = *hostEnrollTeamID @@ -496,7 +497,6 @@ func QueueBulkSetAndroidAppsAvailableForHost( applicationIDs []string, enterpriseName string, ) error { - args := &softwareWorkerArgs{ Task: bulkSetAndroidAppsAvailableForHostTask, HostUUID: hostUUID, @@ -560,7 +560,6 @@ func (v *SoftwareWorker) bulkSetAndroidAppsAvailableForHosts(ctx context.Context } err = v.AndroidModule.SetAppsForAndroidPolicy(ctx, enterpriseName, appPolicies, map[string]string{uuid: uuid}) - if err != nil { return ctxerr.WrapWithData(ctx, err, "set apps for android policy", map[string]any{"host_id": hostID}) } @@ -568,7 +567,6 @@ func (v *SoftwareWorker) bulkSetAndroidAppsAvailableForHosts(ctx context.Context } return nil - } func QueueBulkSetAndroidAppsAvailableForHosts( @@ -576,8 +574,8 @@ func QueueBulkSetAndroidAppsAvailableForHosts( ds fleet.Datastore, logger *slog.Logger, uuidsToIDs map[string]uint, - enterpriseName string) error { - + enterpriseName string, +) error { args := &softwareWorkerArgs{ Task: bulkSetAndroidAppsAvailableForHostsTask, UUIDsToIDs: uuidsToIDs, diff --git a/server/worker/worker.go b/server/worker/worker.go index 111620d40d..75524481bb 100644 --- a/server/worker/worker.go +++ b/server/worker/worker.go @@ -44,7 +44,7 @@ type failingPolicyArgs struct { PolicyName string `json:"policy_name"` PolicyCritical bool `json:"policy_critical"` Hosts []fleet.PolicySetHost `json:"hosts"` - TeamID *uint `json:"team_id,omitempty"` + TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // these are written to the db, changing likely requires migration } // vulnArgs are the args common to all integrations that can process diff --git a/tools/ci/apiparamcheck/apiparamcheck.go b/tools/ci/apiparamcheck/apiparamcheck.go new file mode 100644 index 0000000000..f29ce00481 --- /dev/null +++ b/tools/ci/apiparamcheck/apiparamcheck.go @@ -0,0 +1,156 @@ +// Package apiparamcheck defines an analyzer that flags json/url/query struct +// tags whose name contains deprecated Fleet terminology. +// +// Two renames are enforced: +// - "team" / "teams" was renamed to "fleet" / "fleets". Any occurrence of +// "team" or "teams" as a token in a tag name is flagged, including in +// snake_case, camelCase/PascalCase, and kebab-case names. +// - "query" / "queries" was renamed to "report" / "reports" when referring +// to the product concept (the SQL sense of the word is fine). A tag name +// of exactly "query" or "queries" is allowed; any larger name containing +// "query" or "queries" as a token is flagged. +package apiparamcheck + +import ( + "go/ast" + "reflect" + "strconv" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +// Analyzer flags struct tags using deprecated "team"/"teams"/"query"/"queries" +// terminology in json/url/query tag names. +var Analyzer = &analysis.Analyzer{ + Name: "apiparamcheck", + Doc: "flags json/url/query struct tags using deprecated team/teams or query/queries terms", + URL: "https://github.com/fleetdm/fleet", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +var checkedTagKeys = []string{"json", "url", "query"} + +func run(pass *analysis.Pass) (any, error) { + insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + insp.Preorder([]ast.Node{(*ast.StructType)(nil)}, func(n ast.Node) { + st := n.(*ast.StructType) + if st.Fields == nil { + return + } + for _, field := range st.Fields.List { + if field.Tag == nil { + continue + } + unquoted, err := strconv.Unquote(field.Tag.Value) + if err != nil { + continue + } + tag := reflect.StructTag(unquoted) + // A non-empty `renameto:"new_name"` tag opts the field out of this + // check. It declares the new tag name this field will eventually + // be renamed to, letting us grandfather legacy names while still + // flagging anything new. + if renameTo, ok := tag.Lookup("renameto"); ok && renameTo != "" { + continue + } + for _, key := range checkedTagKeys { + v, ok := tag.Lookup(key) + if !ok { + continue + } + name, _, _ := strings.Cut(v, ",") + if msg := violationMessage(name); msg != "" { + pass.Reportf(field.Tag.Pos(), "%s tag %q: %s", key, name, msg) + } + } + } + }) + + return nil, nil +} + +// violationMessage returns a non-empty message describing why the tag name +// is invalid, or "" if the name is allowed. +func violationMessage(name string) string { + tokens := splitTokens(name) + if len(tokens) == 0 { + return "" + } + for _, t := range tokens { + low := strings.ToLower(t) + if low == "team" || low == "teams" { + return `uses deprecated "team"/"teams" — use "fleet"/"fleets" instead` + } + } + // "query"/"queries" are allowed when they are the entire tag name + // (referring to a SQL query), but not as part of a larger name. + if len(tokens) > 1 { + for _, t := range tokens { + low := strings.ToLower(t) + if low == "query" || low == "queries" { + return `uses "query"/"queries" as part of a name — use "report"/"reports" instead (bare "query"/"queries" is ok)` + } + } + } + return "" +} + +// splitTokens splits a tag name into its snake_case and camelCase components. +// Examples: +// +// "team_id" -> ["team", "id"] +// "hostTeamID" -> ["host", "Team", "ID"] +// "query" -> ["query"] +// "osquery_version" -> ["osquery", "version"] +func splitTokens(name string) []string { + if name == "" { + return nil + } + // First split on underscores and hyphens (snake_case / kebab-case). + var out []string + for _, part := range strings.FieldsFunc(name, func(r rune) bool { + return r == '_' || r == '-' + }) { + out = append(out, splitCamel(part)...) + } + return out +} + +// splitCamel splits a camelCase or PascalCase identifier into its components. +// Runs of consecutive uppercase letters are kept together (e.g. "ID"). +func splitCamel(s string) []string { + if s == "" { + return nil + } + runes := []rune(s) + var parts []string + start := 0 + for i := 1; i < len(runes); i++ { + cur, prev := runes[i], runes[i-1] + // Boundary: lowercase/digit followed by uppercase. + if isUpper(cur) && !isUpper(prev) { + parts = append(parts, string(runes[start:i])) + start = i + continue + } + // Boundary: end of an uppercase run before a lowercase + // (e.g. "IDs" -> "I", "Ds" is wrong; "IDs" should split as + // "ID"+"s" — detect the split one rune earlier). + if i+1 < len(runes) && isUpper(prev) && isUpper(cur) && !isUpper(runes[i+1]) { + parts = append(parts, string(runes[start:i])) + start = i + continue + } + } + parts = append(parts, string(runes[start:])) + return parts +} + +func isUpper(r rune) bool { + return r >= 'A' && r <= 'Z' +} diff --git a/tools/ci/apiparamcheck/apiparamcheck_test.go b/tools/ci/apiparamcheck/apiparamcheck_test.go new file mode 100644 index 0000000000..d8e9f1ae6d --- /dev/null +++ b/tools/ci/apiparamcheck/apiparamcheck_test.go @@ -0,0 +1,13 @@ +package apiparamcheck_test + +import ( + "testing" + + "github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck" + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, apiparamcheck.Analyzer, "example") +} diff --git a/tools/ci/apiparamcheck/cmd/gclplugin/plugin.go b/tools/ci/apiparamcheck/cmd/gclplugin/plugin.go new file mode 100644 index 0000000000..5e9dda1748 --- /dev/null +++ b/tools/ci/apiparamcheck/cmd/gclplugin/plugin.go @@ -0,0 +1,27 @@ +// Package gclplugin provides the golangci-lint module plugin entry point. +package gclplugin + +import ( + "github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck" + "github.com/golangci/plugin-module-register/register" + "golang.org/x/tools/go/analysis" +) + +func init() { + register.Plugin("apiparamcheck", New) +} + +// New returns the golangci-lint plugin for the apiparamcheck analyzer. +func New(_ any) (register.LinterPlugin, error) { + return &plugin{}, nil +} + +type plugin struct{} + +func (p *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { + return []*analysis.Analyzer{apiparamcheck.Analyzer}, nil +} + +func (p *plugin) GetLoadMode() string { + return register.LoadModeSyntax +} diff --git a/tools/ci/apiparamcheck/go.mod b/tools/ci/apiparamcheck/go.mod new file mode 100644 index 0000000000..b6afeaa45f --- /dev/null +++ b/tools/ci/apiparamcheck/go.mod @@ -0,0 +1,13 @@ +module github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck + +go 1.26.2 + +require ( + github.com/golangci/plugin-module-register v0.1.2 + golang.org/x/tools v0.42.0 +) + +require ( + golang.org/x/mod v0.33.0 // indirect + golang.org/x/sync v0.19.0 // indirect +) diff --git a/tools/ci/apiparamcheck/go.sum b/tools/ci/apiparamcheck/go.sum new file mode 100644 index 0000000000..3bbf9d7108 --- /dev/null +++ b/tools/ci/apiparamcheck/go.sum @@ -0,0 +1,10 @@ +github.com/golangci/plugin-module-register v0.1.2 h1:e5WM6PO6NIAEcij3B053CohVp3HIYbzSuP53UAYgOpg= +github.com/golangci/plugin-module-register v0.1.2/go.mod h1:1+QGTsKBvAIvPvoY/os+G5eoqxWn70HYDm2uvUyGuVw= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +golang.org/x/mod v0.33.0 h1:tHFzIWbBifEmbwtGz65eaWyGiGZatSrT9prnU8DbVL8= +golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w= +golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= +golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= diff --git a/tools/ci/apiparamcheck/testdata/src/example/example.go b/tools/ci/apiparamcheck/testdata/src/example/example.go new file mode 100644 index 0000000000..500c667757 --- /dev/null +++ b/tools/ci/apiparamcheck/testdata/src/example/example.go @@ -0,0 +1,49 @@ +package example + +// --- team / teams should be flagged --- + +type teamsBad struct { + A string `json:"team"` // want `json tag "team": uses deprecated "team"/"teams"` + B string `json:"teams"` // want `json tag "teams": uses deprecated "team"/"teams"` + C string `json:"team_id"` // want `json tag "team_id": uses deprecated "team"/"teams"` + D string `url:"host_team_id"` // want `url tag "host_team_id": uses deprecated "team"/"teams"` + E string `query:"my_teams"` // want `query tag "my_teams": uses deprecated "team"/"teams"` + F string `json:"numTeams"` // want `json tag "numTeams": uses deprecated "team"/"teams"` + G string `json:"team,omitempty"` // want `json tag "team": uses deprecated "team"/"teams"` +} + +// --- query / queries as part of a larger name should be flagged --- + +type queriesBad struct { + H string `json:"query_id"` // want `json tag "query_id": uses "query"/"queries" as part of a name` + I string `json:"saved_queries"` // want `json tag "saved_queries": uses "query"/"queries" as part of a name` + J string `url:"my_query"` // want `url tag "my_query": uses "query"/"queries" as part of a name` + K string `json:"queryName"` // want `json tag "queryName": uses "query"/"queries" as part of a name` + L string `json:"scheduled_query_id"` // want `json tag "scheduled_query_id": uses "query"/"queries" as part of a name` +} + +// --- renameto escape hatch --- + +type renameToEscape struct { + // Non-empty renameto suppresses the check. + AA string `json:"team_id" renameto:"fleet_id"` + BB string `json:"query_id" renameto:"report_id"` + CC string `url:"teams" renameto:"fleets"` + // Empty renameto does NOT suppress. + DD string `json:"team_id" renameto:""` // want `json tag "team_id": uses deprecated "team"/"teams"` +} + +// --- allowed --- + +type okStruct struct { + M string `json:"fleet_id"` + N string `json:"report_id"` + O string `json:"query"` + P string `json:"queries"` + Q string `json:"osquery_version"` + R string `json:"osquery"` + S string `json:"stream_name"` + T string `json:"query,omitempty"` + U string `db:"team_id"` // db tag is not checked + V string `json:"-"` +}