tools: 为 spark_submit 的未 mint 绝对路径增加可观测警告

LLM 有时会把自己本地的绝对路径直接传给 spark_submit;这个路径在 MCP
服务器上不存在,spark-submit 会失败,但当前 translateArgs 是静默透传,
问题难以定位。添加 logger.Warn("spark_submit.unminted_path"),对
服务器未 mint 的绝对路径/路径型参数发出警告,同时保持透传不变。

生产环境中这条警告是诚实的可观测信号,而不是回归指标:集群本地合法路径
(如 /opt/spark/examples/pi.py)也会触发警告,这是设计上的,因为我们
无法区分“合法集群路径”和“LLM 本地路径”。

测试锁定:
- minted_path_no_warn:上传文件返回的 path 不触发警告
- cluster_local_path_warns:未 mint 的绝对路径触发一次警告且 path 正确
- non_path_args_never_warn:--class、Main、--master 等普通参数不触发警告

translateArgs 对 logger 做 nil-safe 处理:测试中 deps.Logger 未设置时不
会 panic,也不会产生日志,保持现有测试的简洁。

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
tao.chen
2026-07-13 19:18:35 +08:00
co-authored by Claude
parent f374ebfdb4
commit 2570713fbf
2 changed files with 134 additions and 7 deletions
+40 -7
View File
@@ -3,6 +3,9 @@ package tools
import ( import (
"context" "context"
"fmt" "fmt"
"log/slog"
"path/filepath"
"strings"
"github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/mcp"
@@ -15,7 +18,7 @@ const SparkSubmitName = "spark_submit"
// NewSparkSubmitTool returns the schema for the spark_submit MCP Tool. // NewSparkSubmitTool returns the schema for the spark_submit MCP Tool.
func NewSparkSubmitTool() mcp.Tool { func NewSparkSubmitTool() mcp.Tool {
return mcp.NewTool(SparkSubmitName, return mcp.NewTool(SparkSubmitName,
mcp.WithDescription("Submit a Spark application to a configured cluster using the cluster's local spark-submit binary. The `args` array is appended to the cluster's default_submit_args; pass one argv element per CLI flag. If the script to run was just uploaded via upload_file, put the `path` field from that response as one of the args — do NOT construct your own path. Cluster-local paths (e.g. /opt/spark/examples/pi.py) are also accepted. Returns {app_id, exit_code, stdout_tail, stderr_tail, duration_ms}. Binary is invoked as a child process — no shell, no command injection."), mcp.WithDescription("Submit a Spark application to a configured cluster using the cluster's local spark-submit binary. The `args` array is appended to the cluster's default_submit_args; pass one argv element per CLI flag. If the script to run was just uploaded via upload_file, put the `path` field from that response as one of the args — do NOT construct your own path. Cluster-local paths (e.g. /opt/spark/examples/pi.py) are accepted but will be logged as a warning. To avoid the warning, always use the `path` field returned by upload_file. Returns {app_id, exit_code, stdout_tail, stderr_tail, duration_ms}. Binary is invoked as a child process — no shell, no command injection."),
mcp.WithString("cluster_id", mcp.WithString("cluster_id",
mcp.Required(), mcp.Required(),
mcp.Description("ID of the configured cluster (from list_clusters)"), mcp.Description("ID of the configured cluster (from list_clusters)"),
@@ -64,7 +67,7 @@ func (d *Deps) SparkSubmitHandler(ctx context.Context, req mcp.CallToolRequest)
} }
fullArgs := append([]string{}, cl.DefaultSubmitArgs...) fullArgs := append([]string{}, cl.DefaultSubmitArgs...)
fullArgs = append(fullArgs, translateArgs(userArgs, d.UploadStore)...) fullArgs = append(fullArgs, translateArgs(userArgs, d.UploadStore, d.Logger)...)
result, err := executor.Run(ctx, executor.SparkSubmitOpts{ result, err := executor.Run(ctx, executor.SparkSubmitOpts{
Binary: cl.SparkSubmitExecuteBin, Binary: cl.SparkSubmitExecuteBin,
@@ -103,19 +106,49 @@ func (d *Deps) SparkSubmitHandler(ctx context.Context, req mcp.CallToolRequest)
// translateArgs canonicalizes server-minted upload paths and leaves // translateArgs canonicalizes server-minted upload paths and leaves
// cluster-local paths untouched. It is a code-level guardrail, not a // cluster-local paths untouched. It is a code-level guardrail, not a
// validator: non-minted absolute paths are intentionally accepted. // validator: non-minted absolute paths are intentionally accepted.
func translateArgs(userArgs []string, store *uploads.Store) []string { func translateArgs(userArgs []string, store *uploads.Store, logger *slog.Logger) []string {
if store == nil { if store == nil {
return userArgs return userArgs
} }
out := make([]string, len(userArgs)) out := make([]string, len(userArgs))
for i, arg := range userArgs { for i, arg := range userArgs {
if fileID, err := store.Validate(arg); err == nil { pathPart, isPath := pathLikeArg(arg)
if canonical, err := store.AbsPath(fileID); err == nil { if isPath {
out[i] = canonical if fileID, err := store.Validate(pathPart); err == nil {
continue if canonical, err := store.AbsPath(fileID); err == nil {
out[i] = canonical
continue
}
} else if logger != nil && filepath.IsAbs(pathPart) {
// Absolute path that this server did not mint. Still allowed,
// but warn so operators can spot LLM-invented remote paths.
logger.Warn("spark_submit.unminted_path",
"path", pathPart,
"note", "not produced by upload_file on this server; cluster-local paths are valid, but if this came from a remote agent the path is unreachable")
} }
} }
out[i] = arg out[i] = arg
} }
return out return out
} }
// pathLikeArg extracts an absolute path from a spark-submit argv element.
// It handles plain absolute paths ("/opt/spark/pi.py") and flag-style values
// ("--conf=/x/y"). Non-path args like "--class" or "Main" return ok=false.
func pathLikeArg(arg string) (string, bool) {
if arg == "" {
return "", false
}
if strings.HasPrefix(arg, "/") {
return arg, true
}
if strings.HasPrefix(arg, "-") {
if eq := strings.Index(arg, "="); eq > 0 && eq < len(arg)-1 {
val := arg[eq+1:]
if strings.HasPrefix(val, "/") {
return val, true
}
}
}
return "", false
}
@@ -1,7 +1,10 @@
package tools package tools
import ( import (
"bytes"
"context" "context"
"encoding/json"
"log/slog"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@@ -70,6 +73,97 @@ func TestSparkSubmit_PathPassthrough(t *testing.T) {
} }
} }
func TestTranslateArgs_Logging(t *testing.T) {
deps, _ := testDepsWithDataDir(t)
t.Run("minted_path_no_warn", func(t *testing.T) {
store := deps.UploadStore
fileID, _, _, _, absPath, err := store.Save([]byte("print('hi')\n"), "hello.py")
if err != nil {
t.Fatalf("save upload: %v", err)
}
logger, buf := capturingLogger(t)
out := translateArgs([]string{absPath, "--class", "Main"}, deps.UploadStore, logger)
if out[0] != absPath {
t.Errorf("out[0]=%q, want %q", out[0], absPath)
}
for _, rec := range parseLogRecords(t, buf) {
if rec["msg"] == "spark_submit.unminted_path" {
t.Errorf("unexpected warn for minted path: %v", rec)
}
}
if !strings.HasSuffix(absPath, "/"+fileID) {
t.Errorf("absPath=%q does not end with fileID %q", absPath, fileID)
}
})
t.Run("cluster_local_path_warns", func(t *testing.T) {
logger, buf := capturingLogger(t)
out := translateArgs([]string{"/opt/spark/examples/pi.py"}, deps.UploadStore, logger)
if len(out) != 1 || out[0] != "/opt/spark/examples/pi.py" {
t.Errorf("out=%v, want [/opt/spark/examples/pi.py]", out)
}
var warns int
for _, rec := range parseLogRecords(t, buf) {
if rec["msg"] == "spark_submit.unminted_path" {
warns++
if rec["path"] != "/opt/spark/examples/pi.py" {
t.Errorf("warn path=%v, want /opt/spark/examples/pi.py", rec["path"])
}
}
}
if warns != 1 {
t.Errorf("warns=%d, want 1; log=%s", warns, buf.String())
}
})
t.Run("non_path_args_never_warn", func(t *testing.T) {
logger, buf := capturingLogger(t)
out := translateArgs([]string{"--class", "Main", "--master", "yarn"}, deps.UploadStore, logger)
want := []string{"--class", "Main", "--master", "yarn"}
if len(out) != len(want) {
t.Fatalf("out=%v, want %v", out, want)
}
for i := range out {
if out[i] != want[i] {
t.Errorf("out[%d]=%q, want %q", i, out[i], want[i])
}
}
for _, rec := range parseLogRecords(t, buf) {
if rec["msg"] == "spark_submit.unminted_path" {
t.Errorf("unexpected warn for non-path args: %v", rec)
}
}
})
}
func capturingLogger(t *testing.T) (*slog.Logger, *bytes.Buffer) {
t.Helper()
buf := &bytes.Buffer{}
return slog.New(slog.NewJSONHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})), buf
}
func parseLogRecords(t *testing.T, buf *bytes.Buffer) []map[string]any {
t.Helper()
var recs []map[string]any
for _, line := range strings.Split(strings.TrimSpace(buf.String()), "\n") {
if line == "" {
continue
}
var rec map[string]any
if err := json.Unmarshal([]byte(line), &rec); err != nil {
t.Fatalf("invalid log JSON: %q (err=%v)", line, err)
}
recs = append(recs, rec)
}
return recs
}
func TestSparkSubmit_ClusterLocalPathPassthrough(t *testing.T) { func TestSparkSubmit_ClusterLocalPathPassthrough(t *testing.T) {
deps, repo := testDepsWithDataDir(t) deps, repo := testDepsWithDataDir(t)
deps.SparkSubmitTimeout = 5 * time.Second deps.SparkSubmitTimeout = 5 * time.Second