fix(repo_mirror_push): resolve mirror credential from server env, not the payload (#49)
The mirror credential no longer has to ride the tool-call payload (which is persisted to transcript → claudewatcher → brain → gitea history). Adds remote_password_env: the name of a server-side env var the tool resolves at call time, so the secret stays in the server process. An env name that resolves to empty errors loudly rather than silently sending an empty password. Raw remote_password still works but the schema/description now mark it DISCOURAGED. Tests: password resolved from the env var (never in output); unset env var → ErrValidation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user