From cac39337104c50076a0ddc3dd075e07ec9bd4479 Mon Sep 17 00:00:00 2001
From: rinaldofesta <festarinaldo@gmail.com>
Date: Fri, 5 Jun 2026 17:11:05 +0200
Subject: [PATCH] fix(validators): reject valueHint on named arguments

valueHint identifies a positional slot for transport URL variable
substitution, so it is only valid on positional arguments. The model
carries ValueHint flat on Argument and validateArgument never checked it
for named arguments, and the JSON schema does not catch it (the argument
union branches do not set additionalProperties: false), so a named
argument with a valueHint validated. Enforce it in the semantic validator
with ErrValueHintOnNamedArgument (code valuehint-on-named-argument), with
tests both directions.

Closes #662.
---
 internal/validators/constants.go       |  1 +
 internal/validators/validators.go      | 14 +++++++++++++
 internal/validators/validators_test.go | 28 ++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/internal/validators/constants.go b/internal/validators/constants.go
index 90dec391..41c1743c 100644
--- a/internal/validators/constants.go
+++ b/internal/validators/constants.go
@@ -26,6 +26,7 @@ var (
 	ErrInvalidNamedArgumentName      = errors.New("invalid named argument name format")
 	ErrArgumentValueStartsWithName   = errors.New("argument value cannot start with the argument name")
 	ErrArgumentDefaultStartsWithName = errors.New("argument default cannot start with the argument name")
+	ErrValueHintOnNamedArgument      = errors.New("valueHint is only valid for positional arguments, not named arguments")
 
 	// Server name validation errors
 	ErrMultipleSlashesInServerName = errors.New("server name cannot contain multiple slashes")
diff --git a/internal/validators/validators.go b/internal/validators/validators.go
index 114ed3c7..5e7007e0 100644
--- a/internal/validators/validators.go
+++ b/internal/validators/validators.go
@@ -404,6 +404,20 @@ func validateArgument(ctx *ValidationContext, obj *model.Argument) *ValidationRe
 		// Validate value and default don't start with the name
 		valueResult := validateArgumentValueFields(ctx, obj.Name, obj.Value, obj.Default)
 		result.Merge(valueResult)
+
+		// valueHint names a positional slot for transport URL variable
+		// substitution, so it only makes sense on positional arguments. The
+		// schema allows it on named arguments (the union branches don't set
+		// additionalProperties: false), so reject it here instead.
+		if obj.ValueHint != "" {
+			issue := NewValidationIssueFromError(
+				ValidationIssueTypeSemantic,
+				ctx.Field("valueHint").String(),
+				ErrValueHintOnNamedArgument,
+				"valuehint-on-named-argument",
+			)
+			result.AddIssue(issue)
+		}
 	}
 	return result
 }
diff --git a/internal/validators/validators_test.go b/internal/validators/validators_test.go
index c3e5585b..35a0dbf0 100644
--- a/internal/validators/validators_test.go
+++ b/internal/validators/validators_test.go
@@ -1286,6 +1286,34 @@ func TestValidateArgument_InvalidNamedArgumentNames(t *testing.T) {
 	}
 }
 
+func TestValidateArgument_ValueHintOnNamedArgument(t *testing.T) {
+	// valueHint identifies a positional slot, so a named argument must not carry
+	// one. The schema misses this (the argument union branches allow extra
+	// properties), so the semantic validator is the enforcement point.
+	t.Run("named_with_valueHint_rejected", func(t *testing.T) {
+		arg := model.Argument{
+			Type:      model.ArgumentTypeNamed,
+			Name:      "--port",
+			ValueHint: "port_number",
+		}
+		server := createValidServerWithArgument(arg)
+		result := validators.ValidateServerJSON(&server, validators.ValidationSchemaVersionAndSemantic)
+		assert.Error(t, result.FirstError(), "named argument carrying valueHint should be rejected")
+	})
+
+	// A positional argument with a valueHint stays valid (no regression).
+	t.Run("positional_with_valueHint_valid", func(t *testing.T) {
+		arg := model.Argument{
+			InputWithVariables: model.InputWithVariables{Input: model.Input{Value: "/etc/config.json"}},
+			Type:               model.ArgumentTypePositional,
+			ValueHint:          "config_path",
+		}
+		server := createValidServerWithArgument(arg)
+		result := validators.ValidateServerJSON(&server, validators.ValidationSchemaVersionAndSemantic)
+		assert.NoError(t, result.FirstError(), "positional argument with valueHint should remain valid")
+	})
+}
+
 func TestValidateArgument_InvalidValueFields(t *testing.T) {
 	invalidValueCases := []struct {
 		name string