diff --git a/pkg/datasource/datasource.go b/pkg/datasource/datasource.go index 29723f0..6e5437d 100644 --- a/pkg/datasource/datasource.go +++ b/pkg/datasource/datasource.go @@ -119,6 +119,9 @@ func (ds *ZabbixDatasource) QueryData(ctx context.Context, req *backend.QueryDat ds.logger.Debug("DS query", "query", q) if err != nil { res = backend.ErrorResponseWithErrorSource(err) + } else if err := ValidateTimeRange(query.TimeRange); err != nil { + // Validate time range before processing any query + res = backend.ErrorResponseWithErrorSource(err) } else if query.QueryType == MODE_METRICS { frames, err := zabbixDS.queryNumericItems(ctx, &query) if err != nil { diff --git a/pkg/datasource/guardrails.go b/pkg/datasource/guardrails.go new file mode 100644 index 0000000..8bc7b96 --- /dev/null +++ b/pkg/datasource/guardrails.go @@ -0,0 +1,134 @@ +package datasource + +import ( + "errors" + "regexp" + "strings" + "time" + + "github.com/grafana/grafana-plugin-sdk-go/backend" +) + +// Guardrail errors +var ( + ErrEmptyItemIDs = errors.New("itemids cannot be empty for item ID query mode") + ErrInvalidItemID = errors.New("itemid must be a valid numeric value") + ErrInvalidTimeRange = errors.New("invalid time range: 'from' must be before 'to'") + ErrTimeRangeTooLarge = errors.New("time range exceeds maximum allowed duration") + ErrTooManyItems = errors.New("query would return too many items") + ErrAPIMethodNotAllowed = errors.New("API method is not allowed") +) + +// Default guardrail limits +// These limits help prevent performance issues and excessive resource consumption. +// Based on Zabbix best practices: +// - Zabbix "Max period for time selector" defaults to 2 years (range: 1-10 years) +// See: https://www.zabbix.com/documentation/current/en/manual/web_interface/frontend_sections/administration/general +// - Zabbix search_limit parameter defaults to 1000 items +// See: https://www.zabbix.com/documentation/current/en/manual/api/reference/settings/object +// +// Our limits are conservative to ensure good performance: +const ( + DefaultMaxTimeRangeDays = 365 // 1 year + DefaultMaxItems = 500 +) + +// ValidateItemIDs validates that itemids string is not empty and contains valid numeric IDs +func ValidateItemIDs(itemids string) error { + trimmed := strings.TrimSpace(itemids) + if trimmed == "" { + return backend.DownstreamError(ErrEmptyItemIDs) + } + + // Split and validate each ID + ids := strings.Split(trimmed, ",") + numericPattern := regexp.MustCompile(`^\d+$`) + hasValidID := false + + for _, id := range ids { + id = strings.TrimSpace(id) + if id == "" { + continue // Skip empty entries from trailing commas + } + if !numericPattern.MatchString(id) { + return backend.DownstreamError(ErrInvalidItemID) + } + hasValidID = true + } + + if !hasValidID { + return backend.DownstreamError(ErrEmptyItemIDs) + } + + return nil +} + +// ValidateTimeRange validates that the time range is valid (from < to) +func ValidateTimeRange(timeRange backend.TimeRange) error { + if !timeRange.From.Before(timeRange.To) { + return backend.DownstreamError(ErrInvalidTimeRange) + } + return nil +} + +// ValidateTimeRangeDuration validates that the time range doesn't exceed the maximum allowed duration +func ValidateTimeRangeDuration(timeRange backend.TimeRange, maxDays int) error { + if maxDays <= 0 { + maxDays = DefaultMaxTimeRangeDays + } + + duration := timeRange.To.Sub(timeRange.From) + maxDuration := time.Duration(maxDays) * 24 * time.Hour + + if duration > maxDuration { + return backend.DownstreamError(ErrTimeRangeTooLarge) + } + return nil +} + +// ValidateItemCount validates that the number of items doesn't exceed the maximum allowed +func ValidateItemCount(count int, maxItems int) error { + if maxItems <= 0 { + maxItems = DefaultMaxItems + } + + if count > maxItems { + return backend.DownstreamError(ErrTooManyItems) + } + return nil +} + +// AllowedAPIMethods defines the Zabbix API methods that are allowed to be called. +// This list should be kept in sync with src/datasource/zabbix/types.ts (zabbixMethodName type) +var AllowedAPIMethods = map[string]bool{ + "alert.get": true, + "apiinfo.version": true, + "application.get": true, + "event.acknowledge": true, + "event.get": true, + "history.get": true, + "host.get": true, + "hostgroup.get": true, + "item.get": true, + "problem.get": true, + "proxy.get": true, + "script.execute": true, + "script.get": true, + "service.get": true, + "service.getsla": true, + "sla.get": true, + "sla.getsli": true, + "trend.get": true, + "trigger.get": true, + "user.get": true, + "usermacro.get": true, + "valuemap.get": true, +} + +// ValidateAPIMethod checks if the API method is in the allowed list +func ValidateAPIMethod(method string) error { + if _, allowed := AllowedAPIMethods[method]; !allowed { + return backend.DownstreamError(ErrAPIMethodNotAllowed) + } + return nil +} diff --git a/pkg/datasource/guardrails_test.go b/pkg/datasource/guardrails_test.go new file mode 100644 index 0000000..338b3ef --- /dev/null +++ b/pkg/datasource/guardrails_test.go @@ -0,0 +1,307 @@ +package datasource + +import ( + "testing" + "time" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/stretchr/testify/assert" +) + +func TestValidateItemIDs(t *testing.T) { + tests := []struct { + name string + itemids string + wantErr bool + errType error + }{ + { + name: "valid single itemid", + itemids: "12345", + wantErr: false, + }, + { + name: "valid multiple itemids", + itemids: "12345,67890,11111", + wantErr: false, + }, + { + name: "valid itemids with spaces", + itemids: "12345, 67890, 11111", + wantErr: false, + }, + { + name: "valid itemids with trailing comma", + itemids: "12345,67890,", + wantErr: false, + }, + { + name: "empty string", + itemids: "", + wantErr: true, + errType: ErrEmptyItemIDs, + }, + { + name: "only whitespace", + itemids: " ", + wantErr: true, + errType: ErrEmptyItemIDs, + }, + { + name: "non-numeric itemid", + itemids: "abc123", + wantErr: true, + errType: ErrInvalidItemID, + }, + { + name: "mixed valid and invalid itemids", + itemids: "12345,abc,67890", + wantErr: true, + errType: ErrInvalidItemID, + }, + { + name: "itemid with special characters", + itemids: "123-45", + wantErr: true, + errType: ErrInvalidItemID, + }, + { + name: "only commas", + itemids: ",,,", + wantErr: true, + errType: ErrEmptyItemIDs, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateItemIDs(tt.itemids) + if tt.wantErr { + assert.Error(t, err) + // Check that the underlying error matches + if tt.errType != nil { + assert.ErrorIs(t, err, tt.errType) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateTimeRange(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + timeRange backend.TimeRange + wantErr bool + }{ + { + name: "valid time range", + timeRange: backend.TimeRange{ + From: now.Add(-1 * time.Hour), + To: now, + }, + wantErr: false, + }, + { + name: "from equals to", + timeRange: backend.TimeRange{ + From: now, + To: now, + }, + wantErr: true, + }, + { + name: "from after to", + timeRange: backend.TimeRange{ + From: now, + To: now.Add(-1 * time.Hour), + }, + wantErr: true, + }, + { + name: "large valid range", + timeRange: backend.TimeRange{ + From: now.Add(-365 * 24 * time.Hour), + To: now, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTimeRange(tt.timeRange) + if tt.wantErr { + assert.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidTimeRange) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateTimeRangeDuration(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + timeRange backend.TimeRange + maxDays int + wantErr bool + }{ + { + name: "within limit", + timeRange: backend.TimeRange{ + From: now.Add(-7 * 24 * time.Hour), + To: now, + }, + maxDays: 30, + wantErr: false, + }, + { + name: "exactly at limit", + timeRange: backend.TimeRange{ + From: now.Add(-30 * 24 * time.Hour), + To: now, + }, + maxDays: 30, + wantErr: false, + }, + { + name: "exceeds limit", + timeRange: backend.TimeRange{ + From: now.Add(-31 * 24 * time.Hour), + To: now, + }, + maxDays: 30, + wantErr: true, + }, + { + name: "uses default limit when maxDays is 0", + timeRange: backend.TimeRange{ + From: now.Add(-30 * 24 * time.Hour), + To: now, + }, + maxDays: 0, // Should use DefaultMaxTimeRangeDays (365) + wantErr: false, + }, + { + name: "exceeds default limit", + timeRange: backend.TimeRange{ + From: now.Add(-400 * 24 * time.Hour), + To: now, + }, + maxDays: 0, // Should use DefaultMaxTimeRangeDays (365) + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTimeRangeDuration(tt.timeRange, tt.maxDays) + if tt.wantErr { + assert.Error(t, err) + assert.ErrorIs(t, err, ErrTimeRangeTooLarge) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateItemCount(t *testing.T) { + tests := []struct { + name string + count int + maxItems int + wantErr bool + }{ + { + name: "within limit", + count: 100, + maxItems: 500, + wantErr: false, + }, + { + name: "exactly at limit", + count: 500, + maxItems: 500, + wantErr: false, + }, + { + name: "exceeds limit", + count: 501, + maxItems: 500, + wantErr: true, + }, + { + name: "uses default limit when maxItems is 0", + count: 100, + maxItems: 0, // Should use DefaultMaxItems (500) + wantErr: false, + }, + { + name: "exceeds default limit", + count: 501, + maxItems: 0, // Should use DefaultMaxItems (500) + wantErr: true, + }, + { + name: "zero items", + count: 0, + maxItems: 500, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateItemCount(tt.count, tt.maxItems) + if tt.wantErr { + assert.Error(t, err) + assert.ErrorIs(t, err, ErrTooManyItems) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateAPIMethod(t *testing.T) { + // Test a few allowed methods (matching src/datasource/zabbix/types.ts) + allowedMethods := []string{ + "host.get", + "history.get", + "item.get", + "event.acknowledge", + "script.execute", + } + for _, method := range allowedMethods { + t.Run("allowed_"+method, func(t *testing.T) { + err := ValidateAPIMethod(method) + assert.NoError(t, err) + }) + } + + // Test blocked methods (not in the allowlist) + blockedMethods := []string{ + "host.create", + "host.delete", + "host.update", + "user.create", + "item.delete", + "unknown.method", + "", + } + for _, method := range blockedMethods { + t.Run("blocked_"+method, func(t *testing.T) { + err := ValidateAPIMethod(method) + assert.Error(t, err) + assert.ErrorIs(t, err, ErrAPIMethodNotAllowed) + }) + } +} diff --git a/pkg/datasource/resource_handler.go b/pkg/datasource/resource_handler.go index 9f2019e..e08a5b3 100644 --- a/pkg/datasource/resource_handler.go +++ b/pkg/datasource/resource_handler.go @@ -51,6 +51,13 @@ func (ds *ZabbixDatasource) ZabbixAPIHandler(rw http.ResponseWriter, req *http.R return } + // Validate API method is allowed (guardrail) + if err := ValidateAPIMethod(reqData.Method); err != nil { + ds.logger.Warn("Blocked API method", "method", reqData.Method) + writeError(rw, http.StatusForbidden, err) + return + } + ctx := req.Context() pluginCxt := backend.PluginConfigFromContext(ctx) dsInstance, err := ds.getDSInstance(ctx, pluginCxt) diff --git a/pkg/datasource/zabbix.go b/pkg/datasource/zabbix.go index e990d8c..dccb38c 100644 --- a/pkg/datasource/zabbix.go +++ b/pkg/datasource/zabbix.go @@ -78,6 +78,11 @@ func (ds *ZabbixDatasourceInstance) queryNumericItems(ctx context.Context, query } func (ds *ZabbixDatasourceInstance) queryItemIdData(ctx context.Context, query *QueryModel) ([]*data.Frame, error) { + // Validate itemids before processing + if err := ValidateItemIDs(query.ItemIDs); err != nil { + return nil, err + } + itemids := strings.Split(query.ItemIDs, ",") for i, id := range itemids { itemids[i] = strings.Trim(id, " ")