diff --git a/internal/tools/repo_mirror_push.go b/internal/tools/repo_mirror_push.go index 62ef329..2ffadc5 100644 --- a/internal/tools/repo_mirror_push.go +++ b/internal/tools/repo_mirror_push.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "os" "git.d-ma.be/mathias/gitea-mcp/internal/allowlist" "git.d-ma.be/mathias/gitea-mcp/internal/gitea" @@ -22,7 +23,7 @@ func NewRepoMirrorPush(c *gitea.Client, a *allowlist.Allowlist) *RepoMirrorPush func (t *RepoMirrorPush) Descriptor() registry.ToolDescriptor { return registry.ToolDescriptor{ Name: "repo_mirror_push", - Description: "Manage push mirrors for a repository: add, list, or delete.", + Description: "Manage push mirrors for a repository: add, list, or delete. For the mirror credential, PREFER remote_password_env (the name of an env var the server reads) so the secret never rides the tool-call payload/transcript; remote_password (raw) is discouraged and will be persisted in logs.", InputSchema: json.RawMessage(`{ "type":"object", "properties":{ @@ -31,7 +32,8 @@ func (t *RepoMirrorPush) Descriptor() registry.ToolDescriptor { "action":{"type":"string","enum":["add","list","delete"]}, "remote_address":{"type":"string","description":"Mirror target URL (required for add)."}, "remote_username":{"type":"string"}, - "remote_password":{"type":"string","description":"Never logged or returned."}, + "remote_password_env":{"type":"string","description":"PREFERRED: name of a server-side env var holding the mirror credential; the server resolves it, so the secret is never in this call. Errors if the var is unset."}, + "remote_password":{"type":"string","description":"DISCOURAGED: raw credential — lands in the tool-call transcript/logs. Use remote_password_env instead."}, "interval":{"type":"string","description":"Sync interval, e.g. '8h0m0s'."}, "sync_on_commit":{"type":"boolean"}, "mirror_name":{"type":"string","description":"Remote name to delete (required for delete)."} @@ -42,15 +44,16 @@ func (t *RepoMirrorPush) Descriptor() registry.ToolDescriptor { } type repoMirrorPushArgs struct { - Owner string `json:"owner"` - Repo string `json:"repo"` - Action string `json:"action"` - RemoteAddress string `json:"remote_address"` - RemoteUsername string `json:"remote_username"` - RemotePassword string `json:"remote_password"` - Interval string `json:"interval"` - SyncOnCommit bool `json:"sync_on_commit"` - MirrorName string `json:"mirror_name"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Action string `json:"action"` + RemoteAddress string `json:"remote_address"` + RemoteUsername string `json:"remote_username"` + RemotePassword string `json:"remote_password"` + RemotePasswordEnv string `json:"remote_password_env"` + Interval string `json:"interval"` + SyncOnCommit bool `json:"sync_on_commit"` + MirrorName string `json:"mirror_name"` } // safeMirror omits remote_password so it is never returned to the caller. @@ -72,6 +75,22 @@ func toSafeMirror(m *gitea.PushMirror) safeMirror { } } +// resolveMirrorPassword prefers remote_password_env — the name of a server-side +// env var — so the credential never appears in the tool-call payload (#49). It +// falls back to the raw (discouraged) remote_password. An env name that resolves +// to empty is a loud error, not a silent empty password. +func resolveMirrorPassword(args repoMirrorPushArgs) (string, error) { + if args.RemotePasswordEnv != "" { + pw := os.Getenv(args.RemotePasswordEnv) + if pw == "" { + return "", fmt.Errorf("remote_password_env %q is unset or empty in the server environment: %w", + args.RemotePasswordEnv, gitea.ErrValidation) + } + return pw, nil + } + return args.RemotePassword, nil +} + func (t *RepoMirrorPush) Call(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) { var args repoMirrorPushArgs if err := parseArgs(raw, &args); err != nil { @@ -82,10 +101,14 @@ func (t *RepoMirrorPush) Call(ctx context.Context, raw json.RawMessage) (json.Ra } switch args.Action { case "add": + password, err := resolveMirrorPassword(args) + if err != nil { + return nil, err + } m, err := t.c.AddPushMirror(ctx, args.Owner, args.Repo, gitea.AddPushMirrorArgs{ RemoteAddress: args.RemoteAddress, RemoteUsername: args.RemoteUsername, - RemotePassword: args.RemotePassword, + RemotePassword: password, Interval: args.Interval, SyncOnCommit: args.SyncOnCommit, }) diff --git a/internal/tools/repo_mirror_push_test.go b/internal/tools/repo_mirror_push_test.go index 77f5437..dac18e8 100644 --- a/internal/tools/repo_mirror_push_test.go +++ b/internal/tools/repo_mirror_push_test.go @@ -3,6 +3,7 @@ package tools_test import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "testing" @@ -14,6 +15,45 @@ import ( "github.com/stretchr/testify/require" ) +// #49: remote_password_env names a server-side env var; the secret is resolved +// from the server environment and never rides the tool-call payload. +func TestRepoMirrorPushTool_PasswordFromEnv(t *testing.T) { + t.Setenv("TEST_MIRROR_PW", "env-secret") + var gotPw string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + var m map[string]any + _ = json.Unmarshal(body, &m) + gotPw, _ = m["remote_password"].(string) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":1,"remote_name":"m","remote_address":"a"}`)) + })) + defer srv.Close() + + tool := tools.NewRepoMirrorPush(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"mathias"})) + out, err := tool.Call(context.Background(), json.RawMessage(`{ + "owner":"mathias","name":"infra","action":"add", + "remote_address":"https://github.com/mathias/infra.git", + "remote_username":"mathias","remote_password_env":"TEST_MIRROR_PW" + }`)) + require.NoError(t, err) + assert.Equal(t, "env-secret", gotPw, "password must be resolved from the server env var") + assert.NotContains(t, string(out), "env-secret") +} + +// remote_password_env pointing at an unset var must fail loudly, not silently +// send an empty password. +func TestRepoMirrorPushTool_EnvUnsetErrors(t *testing.T) { + tool := tools.NewRepoMirrorPush(gitea.NewClient("http://unused", ""), allowlist.New([]string{"mathias"})) + _, err := tool.Call(context.Background(), json.RawMessage(`{ + "owner":"mathias","name":"infra","action":"add", + "remote_address":"https://github.com/x/y.git","remote_username":"u", + "remote_password_env":"DEFINITELY_UNSET_MIRROR_VAR_XYZ" + }`)) + require.Error(t, err) + assert.ErrorIs(t, err, gitea.ErrValidation) +} + func TestRepoMirrorPushTool_Add(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodPost, r.Method)