mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
feat(secure-cli): check-binary endpoint, agent select dropdown, fix ambiguous column
Backend: - Add POST /v1/cli-credentials/check-binary to resolve binary via exec.LookPath - Security: safeBinaryNameRe regex prevents filesystem probing - Fix ambiguous column in LookupByBinary LEFT JOIN (secureCLISelectColsAliased) - Add diagnostic logging to lookupCredentialedBinary Frontend: - Replace agent ID text input with Select dropdown (reuse useAgents hook) - Add Check Binary button next to binary name (auto-fills resolved path) - Add binary path hint explaining auto-detection from PATH - Update i18n keys (en/vi/zh) for new UI elements
This commit is contained in:
1 parent
0370cabdc9
commit
10cd911dc7
7 files changed
+150
-30
No files matched your search
@@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"os/exec"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strings"
|
||||
@@ -19,6 +20,10 @@ import (
|
||||
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
|
||||
)
|
||||
|
||||
// safeBinaryNameRe allows only simple binary names: alphanumeric, hyphens, underscores, dots.
|
||||
// No path separators or shell metacharacters — prevents filesystem probing via LookPath.
|
||||
var safeBinaryNameRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$`)
|
||||
|
||||
// SecureCLIHandler handles secure CLI binary credential CRUD endpoints.
|
||||
type SecureCLIHandler struct {
|
||||
store store.SecureCLIStore
|
||||
@@ -35,6 +40,7 @@ func (h *SecureCLIHandler) RegisterRoutes(mux *http.ServeMux) {
|
||||
mux.HandleFunc("GET /v1/cli-credentials", h.auth(h.handleList))
|
||||
mux.HandleFunc("POST /v1/cli-credentials", h.auth(h.handleCreate))
|
||||
mux.HandleFunc("GET /v1/cli-credentials/presets", h.auth(h.handlePresets))
|
||||
mux.HandleFunc("POST /v1/cli-credentials/check-binary", h.auth(h.handleCheckBinary))
|
||||
mux.HandleFunc("GET /v1/cli-credentials/{id}", h.auth(h.handleGet))
|
||||
mux.HandleFunc("PUT /v1/cli-credentials/{id}", h.auth(h.handleUpdate))
|
||||
mux.HandleFunc("DELETE /v1/cli-credentials/{id}", h.auth(h.handleDelete))
|
||||
@@ -343,6 +349,34 @@ func (h *SecureCLIHandler) handlePresets(w http.ResponseWriter, _ *http.Request)
|
||||
writeJSON(w, http.StatusOK, map[string]any{"presets": tools.CLIPresets})
|
||||
}
|
||||
|
||||
// handleCheckBinary resolves a binary name to its absolute path via exec.LookPath.
|
||||
func (h *SecureCLIHandler) handleCheckBinary(w http.ResponseWriter, r *http.Request) {
|
||||
locale := store.LocaleFromContext(r.Context())
|
||||
var req struct {
|
||||
BinaryName string `json:"binary_name"`
|
||||
}
|
||||
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<20)).Decode(&req); err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidJSON)})
|
||||
return
|
||||
}
|
||||
if req.BinaryName == "" {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgRequired, "binary_name")})
|
||||
return
|
||||
}
|
||||
// Security: only allow simple binary names (no path separators, no shell metacharacters).
|
||||
// This prevents probing arbitrary filesystem paths via LookPath.
|
||||
if !safeBinaryNameRe.MatchString(req.BinaryName) {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid binary name"})
|
||||
return
|
||||
}
|
||||
absPath, err := exec.LookPath(req.BinaryName)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusOK, map[string]any{"found": false, "error": fmt.Sprintf("binary %q not found in PATH", req.BinaryName)})
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]any{"found": true, "path": absPath})
|
||||
}
|
||||
|
||||
// dryRunRequest tests commands against deny patterns.
|
||||
type dryRunRequest struct {
|
||||
TestCommands []string `json:"test_commands"`
|
||||
|
||||
@@ -26,6 +26,11 @@ func NewPGSecureCLIStore(db *sql.DB, encryptionKey string) *PGSecureCLIStore {
|
||||
const secureCLISelectCols = `id, binary_name, binary_path, description, encrypted_env,
|
||||
deny_args, deny_verbose, timeout_seconds, tips, agent_id, enabled, created_by, created_at, updated_at`
|
||||
|
||||
// secureCLISelectColsAliased is the same as secureCLISelectCols but prefixed with table alias "b."
|
||||
// Required for LookupByBinary which uses LEFT JOIN (ambiguous column names without prefix).
|
||||
const secureCLISelectColsAliased = `b.id, b.binary_name, b.binary_path, b.description, b.encrypted_env,
|
||||
b.deny_args, b.deny_verbose, b.timeout_seconds, b.tips, b.agent_id, b.enabled, b.created_by, b.created_at, b.updated_at`
|
||||
|
||||
func (s *PGSecureCLIStore) Create(ctx context.Context, b *store.SecureCLIBinary) error {
|
||||
if err := store.ValidateUserID(b.CreatedBy); err != nil {
|
||||
return err
|
||||
@@ -271,7 +276,8 @@ func (s *PGSecureCLIStore) LookupByBinary(ctx context.Context, binaryName string
|
||||
}
|
||||
|
||||
// Build query with optional LEFT JOIN for per-user credentials.
|
||||
selectCols := secureCLISelectCols
|
||||
// Use aliased columns (b.) to avoid ambiguous column reference with JOIN.
|
||||
selectCols := secureCLISelectColsAliased
|
||||
joinClause := ""
|
||||
if userID != "" {
|
||||
selectCols += ", uc.encrypted_env AS user_env"
|
||||
|
||||
@@ -281,6 +281,7 @@ func formatCredentialedResult(binary string, args []string,
|
||||
// Returns the credential config and parsed args, or nil if not credentialed.
|
||||
func (t *ExecTool) lookupCredentialedBinary(ctx context.Context, command string) (*store.SecureCLIBinary, string, []string) {
|
||||
if t.secureCLIStore == nil {
|
||||
slog.Warn("secure_cli.lookup: store is nil, skipping credentialed exec", "command", command)
|
||||
return nil, "", nil
|
||||
}
|
||||
binary, args, err := parseCommandBinary(command)
|
||||
@@ -296,9 +297,15 @@ func (t *ExecTool) lookupCredentialedBinary(ctx context.Context, command string)
|
||||
// Pass userID for per-user credential resolution (LEFT JOIN, zero extra queries).
|
||||
userID := store.UserIDFromContext(ctx)
|
||||
cred, err := t.secureCLIStore.LookupByBinary(ctx, binary, agentIDPtr, userID)
|
||||
if err != nil || cred == nil {
|
||||
if err != nil {
|
||||
slog.Warn("secure_cli.lookup: query failed", "binary", binary, "agent_id", agentID, "error", err)
|
||||
return nil, "", nil
|
||||
}
|
||||
if cred == nil {
|
||||
slog.Warn("secure_cli.lookup: no credential found", "binary", binary, "agent_id", agentID)
|
||||
return nil, "", nil
|
||||
}
|
||||
slog.Info("secure_cli.lookup: found credential", "binary", binary, "cred_id", cred.ID, "env_size", len(cred.EncryptedEnv))
|
||||
return cred, binary, args
|
||||
}
|
||||
|
||||
|
||||
@@ -29,7 +29,7 @@
|
||||
"denyVerbose": "Deny Verbose Args",
|
||||
"timeout": "Timeout (s)",
|
||||
"tips": "Tips",
|
||||
"agentId": "Agent ID",
|
||||
"agentId": "Agent",
|
||||
"agentIdHint": "optional — leave blank for global",
|
||||
"commaSeparated": "comma-separated",
|
||||
"binaryNameRequired": "Binary name is required.",
|
||||
@@ -38,16 +38,21 @@
|
||||
"noEnvVarsHint": "Click \"Add Variable\" to define environment variables for this CLI tool.",
|
||||
"envKeyPlaceholder": "ENV_VAR_NAME",
|
||||
"envValuePlaceholder": "value",
|
||||
"invalidEnvKey": "\"{{key}}\" is not a valid env variable name (use A-Z, 0-9, underscore)."
|
||||
"invalidEnvKey": "\"{{key}}\" is not a valid env variable name (use A-Z, 0-9, underscore).",
|
||||
"binaryPathHint": "Leave blank to auto-detect from PATH",
|
||||
"checkBinary": "Check",
|
||||
"binaryFound": "Found at {{path}}",
|
||||
"binaryNotFound": "Binary not found in server PATH",
|
||||
"checking": "Checking..."
|
||||
},
|
||||
"placeholders": {
|
||||
"binaryName": "e.g. gh",
|
||||
"binaryPath": "/usr/local/bin/gh",
|
||||
"binaryPath": "/usr/local/bin/...",
|
||||
"description": "What this CLI tool does",
|
||||
"denyArgs": "--admin, --force",
|
||||
"denyVerbose": "-v, --verbose",
|
||||
"tips": "Usage tips for the agent",
|
||||
"agentId": "agent-id"
|
||||
"agentId": "Global (all agents)"
|
||||
},
|
||||
"toast": {
|
||||
"created": "CLI credential created",
|
||||
|
||||
@@ -29,7 +29,7 @@
|
||||
"denyVerbose": "Verbose Args bị chặn",
|
||||
"timeout": "Thời gian chờ (s)",
|
||||
"tips": "Mẹo sử dụng",
|
||||
"agentId": "ID Agent",
|
||||
"agentId": "Agent",
|
||||
"agentIdHint": "tùy chọn — để trống cho toàn cục",
|
||||
"commaSeparated": "phân cách bằng dấu phẩy",
|
||||
"binaryNameRequired": "Tên binary là bắt buộc.",
|
||||
@@ -38,16 +38,21 @@
|
||||
"noEnvVarsHint": "Nhấn \"Thêm biến\" để khai báo biến môi trường cho công cụ CLI này.",
|
||||
"envKeyPlaceholder": "TÊN_BIẾN",
|
||||
"envValuePlaceholder": "giá trị",
|
||||
"invalidEnvKey": "\"{{key}}\" không phải tên biến môi trường hợp lệ (dùng A-Z, 0-9, gạch dưới)."
|
||||
"invalidEnvKey": "\"{{key}}\" không phải tên biến môi trường hợp lệ (dùng A-Z, 0-9, gạch dưới).",
|
||||
"binaryPathHint": "Để trống để tự động tìm từ PATH",
|
||||
"checkBinary": "Kiểm tra",
|
||||
"binaryFound": "Tìm thấy tại {{path}}",
|
||||
"binaryNotFound": "Không tìm thấy binary trên server",
|
||||
"checking": "Đang kiểm tra..."
|
||||
},
|
||||
"placeholders": {
|
||||
"binaryName": "vd: gh",
|
||||
"binaryPath": "/usr/local/bin/gh",
|
||||
"binaryPath": "/usr/local/bin/...",
|
||||
"description": "CLI tool này dùng để làm gì",
|
||||
"denyArgs": "--admin, --force",
|
||||
"denyVerbose": "-v, --verbose",
|
||||
"tips": "Mẹo sử dụng cho agent",
|
||||
"agentId": "agent-id"
|
||||
"agentId": "Toàn cục (tất cả agent)"
|
||||
},
|
||||
"toast": {
|
||||
"created": "Đã tạo thông tin CLI",
|
||||
|
||||
@@ -29,7 +29,7 @@
|
||||
"denyVerbose": "禁止详细参数",
|
||||
"timeout": "超时 (秒)",
|
||||
"tips": "提示",
|
||||
"agentId": "代理 ID",
|
||||
"agentId": "代理",
|
||||
"agentIdHint": "可选 — 留空表示全局",
|
||||
"commaSeparated": "逗号分隔",
|
||||
"binaryNameRequired": "二进制名称为必填项。",
|
||||
@@ -38,16 +38,21 @@
|
||||
"noEnvVarsHint": "点击\"添加变量\"为此 CLI 工具定义环境变量。",
|
||||
"envKeyPlaceholder": "变量名",
|
||||
"envValuePlaceholder": "值",
|
||||
"invalidEnvKey": "\"{{key}}\" 不是有效的环境变量名(使用 A-Z、0-9、下划线)。"
|
||||
"invalidEnvKey": "\"{{key}}\" 不是有效的环境变量名(使用 A-Z、0-9、下划线)。",
|
||||
"binaryPathHint": "留空自动从 PATH 检测",
|
||||
"checkBinary": "检查",
|
||||
"binaryFound": "已找到:{{path}}",
|
||||
"binaryNotFound": "在服务器 PATH 中未找到",
|
||||
"checking": "检查中..."
|
||||
},
|
||||
"placeholders": {
|
||||
"binaryName": "例如 gh",
|
||||
"binaryPath": "/usr/local/bin/gh",
|
||||
"binaryPath": "/usr/local/bin/...",
|
||||
"description": "此 CLI 工具的用途",
|
||||
"denyArgs": "--admin, --force",
|
||||
"denyVerbose": "-v, --verbose",
|
||||
"tips": "给代理的使用提示",
|
||||
"agentId": "agent-id"
|
||||
"agentId": "全局(所有代理)"
|
||||
},
|
||||
"toast": {
|
||||
"created": "CLI 凭证已创建",
|
||||
|
||||
@@ -11,8 +11,9 @@ import { Switch } from "@/components/ui/switch";
|
||||
import {
|
||||
Select, SelectContent, SelectItem, SelectTrigger, SelectValue,
|
||||
} from "@/components/ui/select";
|
||||
import { Plus, X } from "lucide-react";
|
||||
import { Plus, X, Search, Check, AlertCircle } from "lucide-react";
|
||||
import { useHttp } from "@/hooks/use-ws";
|
||||
import { useAgents } from "@/pages/agents/hooks/use-agents";
|
||||
import type { SecureCLIBinary, CLICredentialInput, CLIPreset } from "./hooks/use-cli-credentials";
|
||||
|
||||
interface ManualEnvEntry {
|
||||
@@ -29,11 +30,13 @@ interface Props {
|
||||
}
|
||||
|
||||
const NONE_PRESET = "__none__";
|
||||
const GLOBAL_AGENT = "__global__";
|
||||
|
||||
export function CliCredentialFormDialog({ open, onOpenChange, credential, presets, onSubmit }: Props) {
|
||||
const { t } = useTranslation("cli-credentials");
|
||||
const { t: tc } = useTranslation("common");
|
||||
const http = useHttp();
|
||||
const { agents } = useAgents();
|
||||
|
||||
const [selectedPreset, setSelectedPreset] = useState(NONE_PRESET);
|
||||
const [binaryName, setBinaryName] = useState("");
|
||||
@@ -52,6 +55,10 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [error, setError] = useState("");
|
||||
|
||||
// Check binary state
|
||||
const [checking, setChecking] = useState(false);
|
||||
const [checkResult, setCheckResult] = useState<{ found: boolean; path?: string; error?: string } | null>(null);
|
||||
|
||||
const isEdit = !!credential;
|
||||
const presetEntries: Array<[string, CLIPreset]> = Object.entries(presets).filter(
|
||||
(e): e is [string, CLIPreset] => e[1] !== undefined,
|
||||
@@ -76,6 +83,7 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
|
||||
setEnabled(credential?.enabled ?? true);
|
||||
setEnvValues({});
|
||||
setError("");
|
||||
setCheckResult(null);
|
||||
|
||||
if (!credential) {
|
||||
setInitialEnvKeys([]);
|
||||
@@ -123,6 +131,27 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
|
||||
setManualEnvEntries([]);
|
||||
};
|
||||
|
||||
const handleCheckBinary = async () => {
|
||||
const name = binaryName.trim();
|
||||
if (!name) return;
|
||||
setChecking(true);
|
||||
setCheckResult(null);
|
||||
try {
|
||||
const res = await http.post<{ found: boolean; path?: string; error?: string }>(
|
||||
"/v1/cli-credentials/check-binary",
|
||||
{ binary_name: name },
|
||||
);
|
||||
setCheckResult(res);
|
||||
if (res.found && res.path) {
|
||||
setBinaryPath(res.path);
|
||||
}
|
||||
} catch {
|
||||
setCheckResult({ found: false, error: t("form.binaryNotFound") });
|
||||
} finally {
|
||||
setChecking(false);
|
||||
}
|
||||
};
|
||||
|
||||
const addManualEnvEntry = useCallback(() => {
|
||||
setManualEnvEntries((prev) => [...prev, { key: "", value: "" }]);
|
||||
}, []);
|
||||
@@ -304,16 +333,37 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Binary name + check button */}
|
||||
<div className="grid grid-cols-1 gap-4 sm:grid-cols-2">
|
||||
<div className="grid gap-1.5">
|
||||
<Label htmlFor="cc-name">{t("form.binaryName")}</Label>
|
||||
<Input
|
||||
id="cc-name"
|
||||
value={binaryName}
|
||||
onChange={(e) => setBinaryName(e.target.value)}
|
||||
placeholder={t("placeholders.binaryName")}
|
||||
className="text-base md:text-sm"
|
||||
/>
|
||||
<div className="flex gap-1.5">
|
||||
<Input
|
||||
id="cc-name"
|
||||
value={binaryName}
|
||||
onChange={(e) => { setBinaryName(e.target.value); setCheckResult(null); }}
|
||||
placeholder={t("placeholders.binaryName")}
|
||||
className="text-base md:text-sm"
|
||||
/>
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
size="icon"
|
||||
className="shrink-0"
|
||||
disabled={!binaryName.trim() || checking}
|
||||
onClick={handleCheckBinary}
|
||||
title={t("form.checkBinary")}
|
||||
>
|
||||
<Search className="h-4 w-4" />
|
||||
</Button>
|
||||
</div>
|
||||
{checkResult && (
|
||||
<p className={`text-xs flex items-center gap-1 ${checkResult.found ? "text-green-600 dark:text-green-400" : "text-destructive"}`}>
|
||||
{checkResult.found ? <Check className="h-3 w-3" /> : <AlertCircle className="h-3 w-3" />}
|
||||
{checkResult.found ? t("form.binaryFound", { path: checkResult.path }) : (checkResult.error || t("form.binaryNotFound"))}
|
||||
</p>
|
||||
)}
|
||||
{checking && <p className="text-xs text-muted-foreground">{t("form.checking")}</p>}
|
||||
</div>
|
||||
<div className="grid gap-1.5">
|
||||
<Label htmlFor="cc-path">{t("form.binaryPath")} <span className="text-xs text-muted-foreground">({tc("optional")})</span></Label>
|
||||
@@ -324,6 +374,7 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
|
||||
placeholder={t("placeholders.binaryPath")}
|
||||
className="text-base md:text-sm"
|
||||
/>
|
||||
<p className="text-xs text-muted-foreground">{t("form.binaryPathHint")}</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -386,15 +437,22 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
|
||||
/>
|
||||
</div>
|
||||
|
||||
{/* Agent selector */}
|
||||
<div className="grid gap-1.5">
|
||||
<Label htmlFor="cc-agent">{t("form.agentId")} <span className="text-xs text-muted-foreground">({t("form.agentIdHint")})</span></Label>
|
||||
<Input
|
||||
id="cc-agent"
|
||||
value={agentId}
|
||||
onChange={(e) => setAgentId(e.target.value)}
|
||||
placeholder={t("placeholders.agentId")}
|
||||
className="text-base md:text-sm"
|
||||
/>
|
||||
<Label>{t("form.agentId")} <span className="text-xs text-muted-foreground">({t("form.agentIdHint")})</span></Label>
|
||||
<Select value={agentId || GLOBAL_AGENT} onValueChange={(v) => setAgentId(v === GLOBAL_AGENT ? "" : v)}>
|
||||
<SelectTrigger className="text-base md:text-sm">
|
||||
<SelectValue placeholder={t("placeholders.agentId")} />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
<SelectItem value={GLOBAL_AGENT}>{t("placeholders.agentId")}</SelectItem>
|
||||
{agents.map((a) => (
|
||||
<SelectItem key={a.id} value={a.id}>
|
||||
{a.display_name || a.agent_key || a.id}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
</div>
|
||||
|
||||
<div className="flex items-center gap-2">
|
||||
|
||||
Reference in new issue
Block a user