From e0bede054774287cc28ba514915ad8bd497a43a4 Mon Sep 17 00:00:00 2001
From: Mathias <mthbqv@gmail.com>
Date: Fri, 3 Jul 2026 22:47:05 +0200
Subject: [PATCH] fix(tools): reject empty repo/name identifier at tool layer
 (#37)

An empty required repo identifier built a trailing-empty path segment
(`/api/v1/repos/{owner}/`) and leaked gitea's bare 404. parseArgs now
validates, via reflection, that `repo`/`name` string args are non-empty and
returns a typed ErrValidation naming the field. Optional identifiers (e.g.
code_search's owner-wide fan-out `repo`) opt out with `,omitempty`. `owner` is
already enforced by the allowlist check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 internal/tools/code_search.go   |  2 +-
 internal/tools/tool.go          | 44 +++++++++++++++++++++++++++++++--
 internal/tools/validate_test.go | 32 ++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 3 deletions(-)
 create mode 100644 internal/tools/validate_test.go

diff --git a/internal/tools/code_search.go b/internal/tools/code_search.go
index 2706111..f23a82b 100644
--- a/internal/tools/code_search.go
+++ b/internal/tools/code_search.go
@@ -49,7 +49,7 @@ func (t *CodeSearch) Descriptor() registry.ToolDescriptor {
 type codeSearchArgs struct {
 	Q     string `json:"q"`
 	Owner string `json:"owner"`
-	Repo  string `json:"repo"`
+	Repo  string `json:"repo,omitempty"` // optional: empty => owner-wide fan-out (#37 opt-out)
 	Page  int    `json:"page"`
 	Limit int    `json:"limit"`
 }
diff --git a/internal/tools/tool.go b/internal/tools/tool.go
index ec4c287..65b75b7 100644
--- a/internal/tools/tool.go
+++ b/internal/tools/tool.go
@@ -2,8 +2,11 @@ package tools
 
 import (
 	"encoding/json"
+	"fmt"
+	"reflect"
 	"strings"
 
+	"gitea.d-ma.be/mathias/gitea-mcp/internal/gitea"
 	"gitea.d-ma.be/mathias/gitea-mcp/internal/registry"
 )
 
@@ -16,9 +19,46 @@ func textOK(v any) (json.RawMessage, error) {
 
 func parseArgs(raw json.RawMessage, dst any) error {
 	if len(raw) == 0 {
-		return json.Unmarshal([]byte("{}"), dst)
+		if err := json.Unmarshal([]byte("{}"), dst); err != nil {
+			return err
+		}
+	} else if err := json.Unmarshal(normalizeAliases(raw), dst); err != nil {
+		return err
 	}
-	return json.Unmarshal(normalizeAliases(raw), dst)
+	return validateRequiredIdentifiers(dst)
+}
+
+// validateRequiredIdentifiers rejects an empty repo identifier at the tool layer
+// before it reaches the client, where it would build a trailing-empty path
+// segment (`/api/v1/repos/{owner}/`) and leak gitea's bare 404 (#37). `repo` and
+// `name` are always required where present — the per-repo identifier tools and
+// the two create tools respectively; `owner` is already enforced by the
+// allowlist check. Returns a typed ErrValidation naming the missing field.
+func validateRequiredIdentifiers(dst any) error {
+	v := reflect.ValueOf(dst)
+	if v.Kind() != reflect.Ptr || v.IsNil() {
+		return nil
+	}
+	v = v.Elem()
+	if v.Kind() != reflect.Struct {
+		return nil
+	}
+	t := v.Type()
+	for i := 0; i < t.NumField(); i++ {
+		base, opts, _ := strings.Cut(t.Field(i).Tag.Get("json"), ",")
+		if base != "repo" && base != "name" {
+			continue
+		}
+		// An optional identifier (e.g. code_search's `repo`, empty => owner-wide
+		// fan-out) opts out by carrying `,omitempty`.
+		if strings.Contains(opts, "omitempty") {
+			continue
+		}
+		if v.Field(i).Kind() == reflect.String && v.Field(i).String() == "" {
+			return fmt.Errorf("%q is required: %w", base, gitea.ErrValidation)
+		}
+	}
+	return nil
 }
 
 // normalizeAliases reconciles the two spellings of the repo identifier so a tool
diff --git a/internal/tools/validate_test.go b/internal/tools/validate_test.go
new file mode 100644
index 0000000..ab2b954
--- /dev/null
+++ b/internal/tools/validate_test.go
@@ -0,0 +1,32 @@
+package tools_test
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"gitea.d-ma.be/mathias/gitea-mcp/internal/allowlist"
+	"gitea.d-ma.be/mathias/gitea-mcp/internal/gitea"
+	"gitea.d-ma.be/mathias/gitea-mcp/internal/tools"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// #37: an empty required repo identifier must fail fast at the tool layer with a
+// typed ErrValidation naming the field, rather than building a trailing-empty
+// path segment (`/api/v1/repos/{owner}/`) that leaks gitea's bare 404.
+func TestEmptyRepoRejectedAsValidation(t *testing.T) {
+	c := gitea.NewClient("http://unused", "")
+	a := allowlist.New([]string{"mathias"})
+
+	_, err := tools.NewRepoGet(c, a).Call(context.Background(), json.RawMessage(`{"owner":"mathias","repo":""}`))
+	require.Error(t, err)
+	assert.ErrorIs(t, err, gitea.ErrValidation)
+	assert.Contains(t, err.Error(), "repo")
+
+	// same for an empty `name` on a create tool (name = required new-repo name)
+	_, err = tools.NewRepoCreate(c, a).Call(context.Background(), json.RawMessage(`{"owner":"mathias","name":""}`))
+	require.Error(t, err)
+	assert.ErrorIs(t, err, gitea.ErrValidation)
+	assert.Contains(t, err.Error(), "name")
+}