From c7dfe4e460af280bce26f5cd8c9c98c4a045fcb2 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 2 Feb 2026 16:00:11 +0200 Subject: [PATCH] Add validation package with http and group subpackages Graduate the validation package from toolhive to toolhive-core with a structured subpackage layout for better organization and extensibility. validation/http: - ValidateHeaderName: RFC 7230 HTTP header name validation - ValidateHeaderValue: RFC 7230 HTTP header value validation - ValidateResourceURI: RFC 8707 canonical URI validation for OAuth validation/group: - ValidateName: Group name validation (lowercase alphanumeric, dashes, underscores, spaces with strict whitespace rules) Both subpackages include comprehensive documentation with usage examples and thorough test coverage for security-sensitive validation logic. Co-Authored-By: Claude Opus 4.5 --- go.mod | 8 +- go.sum | 4 + validation/group/doc.go | 42 ++++++++ validation/group/group.go | 49 +++++++++ validation/group/group_test.go | 63 +++++++++++ validation/http/doc.go | 41 ++++++++ validation/http/http.go | 88 ++++++++++++++++ validation/http/http_test.go | 187 +++++++++++++++++++++++++++++++++ 8 files changed, 480 insertions(+), 2 deletions(-) create mode 100644 validation/group/doc.go create mode 100644 validation/group/group.go create mode 100644 validation/group/group_test.go create mode 100644 validation/http/doc.go create mode 100644 validation/http/http.go create mode 100644 validation/http/http_test.go diff --git a/go.mod b/go.mod index f8cde36..5fc4302 100644 --- a/go.mod +++ b/go.mod @@ -2,11 +2,15 @@ module github.com/stacklok/toolhive-core go 1.25.6 -require go.uber.org/mock v0.6.0 +require ( + github.com/stretchr/testify v1.11.1 + go.uber.org/mock v0.6.0 + golang.org/x/net v0.49.0 +) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/stretchr/testify v1.11.1 + golang.org/x/text v0.33.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index fd98840..34d649b 100644 --- a/go.sum +++ b/go.sum @@ -6,6 +6,10 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y= go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU= +golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= +golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= +golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= +golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/validation/group/doc.go b/validation/group/doc.go new file mode 100644 index 0000000..cff008f --- /dev/null +++ b/validation/group/doc.go @@ -0,0 +1,42 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/* +Package group provides validation functions for group names. + +Group names are used to organize and categorize resources. This package ensures +group names follow consistent naming conventions for compatibility across systems. + +# Name Validation + +Validate group names against naming rules: + + if err := group.ValidateName("my-team"); err != nil { + // Handle invalid group name + } + +Valid group names must: + - Be non-empty (not just whitespace) + - Contain only lowercase alphanumeric characters, underscores, dashes, and spaces + - Not contain null bytes + - Not have leading or trailing whitespace + - Not contain consecutive spaces + +# Examples + +Valid names: + + "teamalpha" + "team-alpha" + "team_alpha_123" + "team alpha" + +Invalid names: + + "" // empty + "TeamAlpha" // uppercase + "team@alpha" // special characters + " teamalpha" // leading space + "team alpha" // consecutive spaces +*/ +package group diff --git a/validation/group/group.go b/validation/group/group.go new file mode 100644 index 0000000..fe4eb41 --- /dev/null +++ b/validation/group/group.go @@ -0,0 +1,49 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package group provides validation functions for group names. +package group + +import ( + "fmt" + "regexp" + "strings" +) + +var validNameRegex = regexp.MustCompile(`^[a-z0-9_\-\s]+$`) + +// ValidateName validates that a group name only contains allowed characters: +// lowercase alphanumeric, underscore, dash, and space. +// It also enforces no leading/trailing/consecutive spaces and disallows null bytes. +func ValidateName(name string) error { + if name == "" || strings.TrimSpace(name) == "" { + return fmt.Errorf("group name cannot be empty or consist only of whitespace") + } + + // Check for null bytes explicitly + if strings.Contains(name, "\x00") { + return fmt.Errorf("group name cannot contain null bytes") + } + + // Enforce lowercase-only group names + if name != strings.ToLower(name) { + return fmt.Errorf("group name must be lowercase") + } + + // Validate characters + if !validNameRegex.MatchString(name) { + return fmt.Errorf("group name can only contain lowercase alphanumeric characters, underscores, dashes, and spaces: %q", name) + } + + // Check for leading/trailing whitespace + if strings.TrimSpace(name) != name { + return fmt.Errorf("group name cannot have leading or trailing whitespace: %q", name) + } + + // Check for consecutive spaces + if strings.Contains(name, " ") { + return fmt.Errorf("group name cannot contain consecutive spaces: %q", name) + } + + return nil +} diff --git a/validation/group/group_test.go b/validation/group/group_test.go new file mode 100644 index 0000000..d372adc --- /dev/null +++ b/validation/group/group_test.go @@ -0,0 +1,63 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package group + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateName(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + expectErr bool + }{ + // Valid cases + {"valid_simple_name", "teamalpha", false}, + {"valid_with_spaces", "team alpha", false}, + {"valid_with_dash_and_underscore", "team-alpha_123", false}, + + // Empty or whitespace-only + {"empty_string", "", true}, + {"only_spaces", " ", true}, + + // Invalid characters + {"invalid_special_characters", "team@alpha!", true}, + {"invalid_unicode", "团队🚀", true}, + + // Null byte + {"null_byte", "team\x00alpha", true}, + + // Leading/trailing whitespace + {"leading_space", " teamalpha", true}, + {"trailing_space", "teamalpha ", true}, + + // Consecutive spaces + {"consecutive_spaces_middle", "team alpha", true}, + {"consecutive_spaces_start", " teamalpha", true}, + {"consecutive_spaces_end", "teamalpha ", true}, + + // Uppercase letters + {"uppercase_letters", "TeamAlpha", true}, + + // Borderline valid + {"single_char", "t", false}, + {"max_typical", "alpha team 2025 - squad_01", false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + err := ValidateName(tc.input) + if tc.expectErr { + assert.Error(t, err, "Expected error for input: %q", tc.input) + } else { + assert.NoError(t, err, "Did not expect error for input: %q", tc.input) + } + }) + } +} diff --git a/validation/http/doc.go b/validation/http/doc.go new file mode 100644 index 0000000..18f5214 --- /dev/null +++ b/validation/http/doc.go @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/* +Package http provides security-focused validation functions for HTTP headers and URIs. + +This package helps prevent common security vulnerabilities such as HTTP header injection +(CRLF injection) and malformed URI attacks by validating input against RFC specifications. + +# Header Validation + +Validate HTTP header names and values per RFC 7230: + + if err := http.ValidateHeaderName("X-Custom-Header"); err != nil { + // Handle invalid header name + } + + if err := http.ValidateHeaderValue("Bearer token123"); err != nil { + // Handle invalid header value + } + +The validators check for: + - CRLF injection attempts (\r\n sequences) + - Control characters + - RFC 7230 token compliance for header names + - Length limits to prevent DoS (256 bytes for names, 8192 for values) + +# Resource URI Validation + +Validate URIs for use as OAuth 2.0 resource indicators per RFC 8707: + + if err := http.ValidateResourceURI("https://api.example.com/v1"); err != nil { + // Handle invalid URI + } + +Resource URIs must: + - Include a scheme (typically http or https) + - Include a host + - Not contain fragment identifiers (#) +*/ +package http diff --git a/validation/http/http.go b/validation/http/http.go new file mode 100644 index 0000000..6a21328 --- /dev/null +++ b/validation/http/http.go @@ -0,0 +1,88 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package http provides validation functions for HTTP headers and URIs. +package http + +import ( + "fmt" + "net/url" + + "golang.org/x/net/http/httpguts" +) + +// ValidateHeaderName validates that a string is a valid HTTP header name per RFC 7230. +// It checks for CRLF injection, control characters, and ensures RFC token compliance. +func ValidateHeaderName(name string) error { + if name == "" { + return fmt.Errorf("header name cannot be empty") + } + + // Length limit to prevent DoS + if len(name) > 256 { + return fmt.Errorf("header name exceeds maximum length of 256 bytes") + } + + // Use httpguts validation (same as Go's HTTP/2 implementation) + if !httpguts.ValidHeaderFieldName(name) { + return fmt.Errorf("invalid HTTP header name: contains invalid characters") + } + + return nil +} + +// ValidateHeaderValue validates that a string is a valid HTTP header value per RFC 7230. +// It checks for CRLF injection and control characters. +func ValidateHeaderValue(value string) error { + if value == "" { + return fmt.Errorf("header value cannot be empty") + } + + // Length limit to prevent DoS (common HTTP server limit) + if len(value) > 8192 { + return fmt.Errorf("header value exceeds maximum length of 8192 bytes") + } + + // Use httpguts validation + if !httpguts.ValidHeaderFieldValue(value) { + return fmt.Errorf("invalid HTTP header value: contains control characters") + } + + return nil +} + +// ValidateResourceURI validates that a resource URI conforms to RFC 8707 requirements +// for canonical URIs used in OAuth 2.0 resource indicators. +// +// A valid canonical URI must: +// - Include a scheme (http/https) +// - Include a host +// - Not contain fragments +func ValidateResourceURI(resourceURI string) error { + if resourceURI == "" { + return fmt.Errorf("resource URI cannot be empty") + } + + // Parse the URI + parsed, err := url.Parse(resourceURI) + if err != nil { + return fmt.Errorf("invalid resource URI: %w", err) + } + + // Must have a scheme + if parsed.Scheme == "" { + return fmt.Errorf("resource URI must include a scheme (e.g., https://): %s", resourceURI) + } + + // Must have a host + if parsed.Host == "" { + return fmt.Errorf("resource URI must include a host: %s", resourceURI) + } + + // Must not contain fragments + if parsed.Fragment != "" { + return fmt.Errorf("resource URI must not contain fragments (#): %s", resourceURI) + } + + return nil +} diff --git a/validation/http/http_test.go b/validation/http/http_test.go new file mode 100644 index 0000000..7981357 --- /dev/null +++ b/validation/http/http_test.go @@ -0,0 +1,187 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package http + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateHeaderName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + expectErr bool + }{ + // Valid cases + {"valid simple", "X-API-Key", false}, + {"valid authorization", "Authorization", false}, + {"valid with numbers", "X-API-Key-123", false}, + {"valid with dots", "X.Custom.Header", false}, + + // CRLF injection attacks + {"crlf injection", "X-API-Key\r\nX-Injected: malicious", true}, + {"newline injection", "X-API-Key\nInjected", true}, + {"carriage return", "X-API-Key\r", true}, + + // Other invalid characters + {"null byte", "X-API-Key\x00", true}, + {"contains space", "X API Key", true}, + {"empty string", "", true}, + + // Length limits + {"too long", strings.Repeat("A", 300), true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateHeaderName(tt.input) + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateHeaderValue(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + expectErr bool + }{ + // Valid cases + {"valid simple", "my-api-key-12345", false}, + {"valid with spaces", "Bearer token123", false}, + {"valid special chars", "key!@#$%^&*()", false}, + + // CRLF injection attacks + {"crlf injection", "key\r\nX-Injected: malicious", true}, + {"newline injection", "key\ninjected", true}, + {"carriage return", "key\r", true}, + + // Control characters + {"null byte", "key\x00value", true}, + {"control char", "key\x01value", true}, + {"delete char", "key\x7Fvalue", true}, + {"tab allowed", "key\tvalue", false}, // Tab is allowed in values + + // Length limits + {"too long", strings.Repeat("A", 10000), true}, + {"empty string", "", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateHeaderValue(tt.input) + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateResourceURI(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + expectError bool + errorContains string + }{ + // Valid cases + { + name: "valid https URL with path", + input: "https://mcp.example.com/mcp", + expectError: false, + }, + { + name: "valid https URL without path", + input: "https://mcp.example.com", + expectError: false, + }, + { + name: "valid https URL with port", + input: "https://mcp.example.com:8443", + expectError: false, + }, + { + name: "valid https URL with port and path", + input: "https://mcp.example.com:8443/api/mcp", + expectError: false, + }, + { + name: "valid http URL", + input: "http://localhost:3000", + expectError: false, + }, + { + name: "root path slash is valid", + input: "https://mcp.example.com/", + expectError: false, + }, + // Invalid cases + { + name: "empty string", + input: "", + expectError: true, + errorContains: "cannot be empty", + }, + { + name: "missing scheme", + input: "mcp.example.com", + expectError: true, + errorContains: "must include a scheme", + }, + { + name: "missing host", + input: "https://", + expectError: true, + errorContains: "must include a host", + }, + { + name: "contains fragment", + input: "https://mcp.example.com/mcp#section", + expectError: true, + errorContains: "must not contain fragments", + }, + { + name: "invalid URL format", + input: "ht!tp://invalid", + expectError: true, + errorContains: "invalid resource URI", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := ValidateResourceURI(tt.input) + + if tt.expectError { + require.Error(t, err, "Expected an error but got none") + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains, + "Error message should contain expected text") + } + } else { + require.NoError(t, err, "Expected no error but got: %v", err) + } + }) + } +}