diff --git a/internal/config/guard_policy_coverage_test.go b/internal/config/guard_policy_coverage_test.go new file mode 100644 index 00000000..2ea80b03 --- /dev/null +++ b/internal/config/guard_policy_coverage_test.go @@ -0,0 +1,115 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestValidateWriteSinkPolicy_EmptyAcceptEntry covers the branch where an accept +// entry is empty (or whitespace-only) after trimming. +// (guard_policy_validation.go lines ~44-46) +func TestValidateWriteSinkPolicy_EmptyAcceptEntry(t *testing.T) { + tests := []struct { + name string + policy *WriteSinkPolicy + }{ + { + name: "empty string entry", + policy: &WriteSinkPolicy{Accept: []string{"valid-server", ""}}, + }, + { + name: "whitespace-only entry", + policy: &WriteSinkPolicy{Accept: []string{"valid-server", " "}}, + }, + { + name: "tab-only entry", + policy: &WriteSinkPolicy{Accept: []string{"\t"}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateWriteSinkPolicy(tt.policy) + require.Error(t, err) + assert.ErrorContains(t, err, "must not be empty") + }) + } +} + +// TestNormalizeGuardPolicy_StringSliceReposError covers the case where Repos is a +// []string and normalizeAndValidateScopeArray returns an error (e.g., invalid scope). +// (guard_policy_validation.go lines ~189-192) +func TestNormalizeGuardPolicy_StringSliceReposError(t *testing.T) { + tests := []struct { + name string + repos []string + errContains string + }{ + { + name: "invalid scope no slash", + repos: []string{"invalid-no-slash"}, + errContains: "invalid", + }, + { + name: "empty scope string", + repos: []string{""}, + errContains: "must not be empty", + }, + { + name: "duplicate scopes", + repos: []string{"owner/repo", "owner/repo"}, + errContains: "duplicates", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + policy := &GuardPolicy{ + AllowOnly: &AllowOnlyPolicy{ + Repos: tt.repos, // []string + MinIntegrity: "none", + }, + } + + _, err := NormalizeGuardPolicy(policy) + + require.Error(t, err) + assert.ErrorContains(t, err, tt.errContains) + }) + } +} + +// TestNormalizeToolCallLimits_EmptyKey covers the branch where a tool-name key +// becomes empty after TrimSpace. +// (guard_policy_validation.go lines ~353-355) +func TestNormalizeToolCallLimits_EmptyKey(t *testing.T) { + tests := []struct { + name string + input map[string]int + }{ + { + name: "empty string key", + input: map[string]int{"": 5}, + }, + { + name: "whitespace-only key", + input: map[string]int{" ": 10}, + }, + { + name: "tab-only key", + input: map[string]int{"\t": 1}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := normalizeToolCallLimits(tt.input) + + assert.Nil(t, result) + require.Error(t, err) + assert.ErrorContains(t, err, "keys must not be empty") + }) + } +} diff --git a/internal/config/schema_error_paths_test.go b/internal/config/schema_error_paths_test.go new file mode 100644 index 00000000..5df85869 --- /dev/null +++ b/internal/config/schema_error_paths_test.go @@ -0,0 +1,248 @@ +package config + +import ( + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// resetSchemaState saves and resets the package-level schema caching variables. +// The caller gets a clean sync.Once so getOrCompileSchema runs its Do() callback +// again. t.Cleanup restores the originals after the test. +func resetSchemaState(t *testing.T) { + t.Helper() + origBytes := embeddedSchemaBytes + origCached := cachedSchema + origErr := schemaErr + origDone := origCached != nil || origErr != nil + t.Cleanup(func() { + embeddedSchemaBytes = origBytes + cachedSchema = origCached + schemaErr = origErr + + // Avoid copying sync.Once (copylocks); restore only whether it had already run. + schemaOnce = sync.Once{} + if origDone { + schemaOnce.Do(func() {}) + } + }) + schemaOnce = sync.Once{} + cachedSchema = nil + schemaErr = nil +} + +// --- fixSchemaBytes branch coverage --- + +// TestFixSchemaBytes_TypeFieldNoPattern covers the branch where customServerConfig +// has a "type" property that is a proper object but has no "pattern" key at all. +// (validation_schema.go lines ~120-122) +func TestFixSchemaBytes_TypeFieldNoPattern(t *testing.T) { + schema := map[string]interface{}{ + "definitions": map[string]interface{}{ + "customServerConfig": map[string]interface{}{ + "properties": map[string]interface{}{ + "type": map[string]interface{}{ + "type": "string", // object but no "pattern" key + }, + }, + }, + }, + } + + result, err := fixSchemaBytes(marshalSchema(t, schema)) + + require.NoError(t, err, "no error expected when type field has no pattern") + assert.NotEmpty(t, result) + // "pattern" was never there — structure should be unchanged + got := unmarshalSchema(t, result) + defs := got["definitions"].(map[string]interface{}) + csConf := defs["customServerConfig"].(map[string]interface{}) + props := csConf["properties"].(map[string]interface{}) + typeField := props["type"].(map[string]interface{}) + _, hasPattern := typeField["pattern"] + assert.False(t, hasPattern, "pattern key should still be absent") +} + +// TestFixSchemaBytes_TypeFieldPatternNotString covers the branch where the "pattern" +// key exists but its value is not a string (e.g., an integer). +// (validation_schema.go lines ~122-124) +func TestFixSchemaBytes_TypeFieldPatternNotString(t *testing.T) { + schema := map[string]interface{}{ + "definitions": map[string]interface{}{ + "customServerConfig": map[string]interface{}{ + "properties": map[string]interface{}{ + "type": map[string]interface{}{ + "pattern": 42, // not a string + }, + }, + }, + }, + } + + result, err := fixSchemaBytes(marshalSchema(t, schema)) + + require.NoError(t, err, "no error expected when type field pattern is not a string") + assert.NotEmpty(t, result) + // non-string pattern should be preserved without modification + got := unmarshalSchema(t, result) + defs := got["definitions"].(map[string]interface{}) + csConf := defs["customServerConfig"].(map[string]interface{}) + props := csConf["properties"].(map[string]interface{}) + typeField := props["type"].(map[string]interface{}) + assert.InDelta(t, float64(42), typeField["pattern"], 0, "non-string pattern value should be preserved") +} + +// TestFixSchemaBytes_TypeFieldPatternNoNegativeLookahead covers the branch where +// the "pattern" is a proper string but does not contain a negative-lookahead "(?!". +// (validation_schema.go lines ~124-126) +func TestFixSchemaBytes_TypeFieldPatternNoNegativeLookahead(t *testing.T) { + schema := map[string]interface{}{ + "definitions": map[string]interface{}{ + "customServerConfig": map[string]interface{}{ + "properties": map[string]interface{}{ + "type": map[string]interface{}{ + "pattern": "^[a-z]+$", // no negative lookahead + }, + }, + }, + }, + } + + result, err := fixSchemaBytes(marshalSchema(t, schema)) + + require.NoError(t, err, "no error expected when pattern has no negative lookahead") + got := unmarshalSchema(t, result) + defs := got["definitions"].(map[string]interface{}) + csConf := defs["customServerConfig"].(map[string]interface{}) + props := csConf["properties"].(map[string]interface{}) + typeField := props["type"].(map[string]interface{}) + // pattern without negative lookahead must be left unchanged + assert.Equal(t, "^[a-z]+$", typeField["pattern"], + "pattern without negative lookahead should be preserved unchanged") + _, hasNot := typeField["not"] + assert.False(t, hasNot, "no not constraint should be added when pattern has no negative lookahead") +} + +// TestFixSchemaBytes_TypeValueNotObject covers the else-branch reached when the +// value of customServerConfig.properties["type"] is not a map[string]interface{}. +// (validation_schema.go lines ~139-141) +func TestFixSchemaBytes_TypeValueNotObject(t *testing.T) { + schema := map[string]interface{}{ + "definitions": map[string]interface{}{ + "customServerConfig": map[string]interface{}{ + "properties": map[string]interface{}{ + "type": "not-an-object", // string, not a map + }, + }, + }, + } + + result, err := fixSchemaBytes(marshalSchema(t, schema)) + + require.NoError(t, err, "no error expected when type value is not an object") + assert.NotEmpty(t, result) + // the string value should be preserved as-is + got := unmarshalSchema(t, result) + defs := got["definitions"].(map[string]interface{}) + csConf := defs["customServerConfig"].(map[string]interface{}) + props := csConf["properties"].(map[string]interface{}) + assert.Equal(t, "not-an-object", props["type"], "non-object type value should be preserved") +} + +// --- getOrCompileSchema error-path coverage --- + +// TestGetOrCompileSchema_FixSchemaError covers the path where fixSchemaBytes returns +// an error (because embeddedSchemaBytes is malformed JSON). +// (validation_schema.go lines ~369-373) +func TestGetOrCompileSchema_FixSchemaError(t *testing.T) { + resetSchemaState(t) + embeddedSchemaBytes = []byte("{{{ not valid json at all") + + schema, err := getOrCompileSchema() + + assert.Nil(t, schema, "schema should be nil when fix fails") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to process embedded schema", + "error message should indicate schema processing failure") +} + +// TestGetOrCompileSchema_SchemaWithoutID covers the branch where the schema JSON +// has no "$id" field, causing the fallback to use embeddedSchemaID. +// (validation_schema.go line ~383-385) +func TestGetOrCompileSchema_SchemaWithoutID(t *testing.T) { + resetSchemaState(t) + // Minimal valid JSON Schema with no "$id" field. + embeddedSchemaBytes = []byte(`{"type":"object"}`) + + schema, err := getOrCompileSchema() + + // The fallback embeddedSchemaID is used; compilation should succeed. + require.NoError(t, err, "compilation should succeed with fallback $id") + assert.NotNil(t, schema, "compiled schema should not be nil") +} + +// TestGetOrCompileSchema_AddResourceError covers the branch where compiler.AddResource +// fails because the schema's $id is an invalid URL (percent-encoded escape is malformed). +// (validation_schema.go lines ~391-394) +func TestGetOrCompileSchema_AddResourceError(t *testing.T) { + resetSchemaState(t) + // "%zz" is an invalid percent-encoding; url.Parse returns an error for it. + embeddedSchemaBytes = []byte(`{"$id":"http://example.com/bad%zzurl"}`) + + schema, err := getOrCompileSchema() + + assert.Nil(t, schema, "schema should be nil when AddResource fails") + require.Error(t, err, "error expected when schema has invalid $id URL") + assert.ErrorContains(t, err, "failed to add schema resource") +} + +// TestGetOrCompileSchema_CompileError covers the branch where AddResource succeeds but +// Compile fails because the schema has duplicate canonical URIs in its definitions. +// (validation_schema.go lines ~397-401) +func TestGetOrCompileSchema_CompileError(t *testing.T) { + resetSchemaState(t) + // Two definitions share the same $id as the root schema, which causes the + // jsonschema compiler to report a "same canonical-uri" error at compile time. + embeddedSchemaBytes = []byte(`{ + "$id": "http://test-compile-error.example.com/schema.json", + "definitions": { + "alpha": {"$id": "http://test-compile-error.example.com/schema.json"}, + "beta": {"$id": "http://test-compile-error.example.com/schema.json"} + } + }`) + + schema, err := getOrCompileSchema() + + assert.Nil(t, schema, "schema should be nil when Compile fails") + require.Error(t, err, "error expected when schema has duplicate canonical URIs") + assert.ErrorContains(t, err, "failed to compile schema") +} + +// --- validateJSONSchema invalid-input coverage --- + +// TestValidateJSONSchema_InvalidJSON covers the json.Unmarshal failure inside +// validateJSONSchema when the supplied config bytes are not valid JSON. +// (validation_schema.go lines ~425-427) +func TestValidateJSONSchema_InvalidJSON(t *testing.T) { + err := validateJSONSchema([]byte("{{{ this is not json at all")) + + require.Error(t, err) + assert.ErrorContains(t, err, "failed to parse configuration JSON", + "error message should indicate JSON parsing failure") +} + +// TestValidateJSONSchema_SchemaError covers the path where getOrCompileSchema +// itself returns an error, which validateJSONSchema propagates. +// (validation_schema.go lines ~417-419) +func TestValidateJSONSchema_SchemaError(t *testing.T) { + resetSchemaState(t) + embeddedSchemaBytes = []byte("{{{ invalid}") + + err := validateJSONSchema([]byte(`{"mcpServers":{}}`)) + + require.Error(t, err) + assert.ErrorContains(t, err, "failed to process embedded schema", + "validateJSONSchema should propagate schema compilation errors") +} diff --git a/internal/config/validate_against_custom_schema_test.go b/internal/config/validate_against_custom_schema_test.go index e65b977f..78cdf73b 100644 --- a/internal/config/validate_against_custom_schema_test.go +++ b/internal/config/validate_against_custom_schema_test.go @@ -324,3 +324,45 @@ func TestValidateCustomServerConfig_UnregisteredType(t *testing.T) { assert.ErrorContains(t, err, "not registered in customSchemas") assert.ErrorContains(t, err, "mytype") } + +// TestValidateAgainstCustomSchema_CacheHitWrongType covers the branch where the +// schema URL is found in the cache but the cached value is not a *jsonschema.Schema. +// In that case the cached value is ignored and the schema is re-fetched. +// (validation.go lines ~231-237) +func TestValidateAgainstCustomSchema_CacheHitWrongType(t *testing.T) { + // We need a unique URL so it doesn't collide with other tests' cache entries. + var requestCount atomic.Int32 + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount.Add(1) + schema := map[string]interface{}{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": map[string]interface{}{ + "type": map[string]interface{}{"type": "string"}, + "container": map[string]interface{}{"type": "string"}, + }, + "required": []string{"type", "container"}, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(schema) + })) + defer mockServer.Close() + + // Use a unique URL (including a path) so it doesn't collide with other tests' cache entries. + schemaURL := mockServer.URL + "/cache-hit-wrong-type" + t.Cleanup(func() { customSchemaCache.Delete(schemaURL) }) + + // Seed the cache with a non-*jsonschema.Schema value at this URL. + customSchemaCache.Store(schemaURL, "unexpected-string-value") + + server := &StdinServerConfig{ + Type: "mytype", + Container: "ghcr.io/example/mytype:latest", + } + + // The wrong-type cache hit should be ignored; the schema should be re-fetched. + err := validateAgainstCustomSchema("test-server", server, schemaURL, "mcpServers.test-server") + + require.NoError(t, err, "validation should succeed after ignoring the wrong-type cache entry") + assert.Equal(t, int32(1), requestCount.Load(), "schema should be fetched once after ignoring the bad cache entry") +}