fix(cli-credentials): show existing env keys on edit and support merge/remove (#629)

fix(cli-credentials): show existing env keys on edit and support merge/remove

- Expose env_keys on secure CLI list/get and per-user credential list (values stay hidden)
- PUT merges env with stored secrets: empty value keeps existing, removed keys dropped
- Generic error messages to prevent env key enumeration
- Audit logging for user credential set/delete
- DRY: reuse envKeysFromDecryptedJSON helper
- Client-side env key name validation (en/vi/zh i18n)

internal/http/secure_cli.go

@@ -2,9 +2,11 @@ package http
 
 import (
 	"encoding/json"
+	"fmt"
 	"log/slog"
 	"net/http"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/google/uuid"
@@ -59,6 +61,72 @@ func (h *SecureCLIHandler) emitCacheInvalidate(key string) {
 	})
 }
 
+// envKeysFromDecryptedJSON returns sorted env variable names from plaintext env JSON (decrypted blob).
+func envKeysFromDecryptedJSON(env []byte) []string {
+	empty := []string{}
+	if len(env) == 0 {
+		return empty
+	}
+	var m map[string]any
+	if err := json.Unmarshal(env, &m); err != nil {
+		return empty
+	}
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
+// mergeSecureCLIEnv merges incoming env from the UI with existing stored env.
+// Incoming defines the full set of keys shown in the form: keys omitted were removed.
+// Empty string means "keep existing value" for that key when it already exists.
+func mergeSecureCLIEnv(existingJSON []byte, incoming map[string]any) (map[string]string, error) {
+	existing := map[string]string{}
+	if len(existingJSON) > 0 {
+		if err := json.Unmarshal(existingJSON, &existing); err != nil {
+			return nil, fmt.Errorf("parse existing env: %w", err)
+		}
+	}
+	out := make(map[string]string)
+	for k, v := range incoming {
+		if k == "" {
+			continue
+		}
+		sv, err := envValueAsString(v)
+		if err != nil {
+			return nil, fmt.Errorf("invalid environment variable value")
+		}
+		if sv != "" {
+			out[k] = sv
+			continue
+		}
+		if ev, ok := existing[k]; ok {
+			out[k] = ev
+		}
+	}
+	return out, nil
+}
+
+func envValueAsString(v any) (string, error) {
+	switch t := v.(type) {
+	case string:
+		return t, nil
+	case float64:
+		return fmt.Sprint(t), nil
+	case bool:
+		if t {
+			return "true", nil
+		}
+		return "false", nil
+	case nil:
+		return "", nil
+	default:
+		return "", fmt.Errorf("value must be a string")
+	}
+}
+
 func (h *SecureCLIHandler) handleList(w http.ResponseWriter, r *http.Request) {
 	locale := store.LocaleFromContext(r.Context())
 	result, err := h.store.List(r.Context())
@@ -67,8 +135,9 @@ func (h *SecureCLIHandler) handleList(w http.ResponseWriter, r *http.Request) {
 		writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgFailedToList, "CLI credentials")})
 		return
 	}
-	// Mask encrypted env — never send raw credentials to UI
+	// Never send env values; only variable names for editing.
 	for i := range result {
+		result[i].EnvKeys = envKeysFromDecryptedJSON(result[i].EncryptedEnv)
 		result[i].EncryptedEnv = nil
 	}
 	writeJSON(w, http.StatusOK, map[string]any{"items": result})
@@ -184,7 +253,8 @@ func (h *SecureCLIHandler) handleGet(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	b.EncryptedEnv = nil // don't expose credentials
+	b.EnvKeys = envKeysFromDecryptedJSON(b.EncryptedEnv)
+	b.EncryptedEnv = nil // don't expose credential values
 	writeJSON(w, http.StatusOK, b)
 }
 
@@ -214,10 +284,24 @@ func (h *SecureCLIHandler) handleUpdate(w http.ResponseWriter, r *http.Request)
 		}
 	}
 
-	// If env is updated, serialize as JSON string for the store encrypt path
+	// If env is updated, merge with stored env so empty values mean "keep existing secret".
 	if envVal, ok := updates["env"]; ok {
 		if envMap, isMap := envVal.(map[string]any); isMap {
-			envJSON, _ := json.Marshal(envMap)
+			cur, err := h.store.Get(r.Context(), id)
+			if err != nil {
+				writeJSON(w, http.StatusNotFound, map[string]string{"error": i18n.T(locale, i18n.MsgNotFound, "credential", id.String())})
+				return
+			}
+			merged, err := mergeSecureCLIEnv(cur.EncryptedEnv, envMap)
+			if err != nil {
+				writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()})
+				return
+			}
+			envJSON, err := json.Marshal(merged)
+			if err != nil {
+				writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid env"})
+				return
+			}
 			updates["encrypted_env"] = string(envJSON)
 			delete(updates, "env")
 		}

internal/http/secure_cli_user_credentials.go

@@ -23,22 +23,25 @@ func (h *SecureCLIHandler) handleListUserCredentials(w http.ResponseWriter, r *h
 		writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, err.Error())})
 		return
 	}
-	// Return without encrypted env for listing (only user_id + timestamps)
+	// Return without env values for listing (names only + timestamps)
 	type entry struct {
 		ID        uuid.UUID `json:"id"`
 		BinaryID  uuid.UUID `json:"binary_id"`
 		UserID    string    `json:"user_id"`
 		HasEnv    bool      `json:"has_env"`
+		EnvKeys   []string  `json:"env_keys,omitempty"`
 		CreatedAt string    `json:"created_at"`
 		UpdatedAt string    `json:"updated_at"`
 	}
 	entries := make([]entry, 0, len(creds))
 	for _, c := range creds {
+		envKeys := envKeysFromDecryptedJSON(c.EncryptedEnv)
 		entries = append(entries, entry{
 			ID:        c.ID,
 			BinaryID:  c.BinaryID,
 			UserID:    c.UserID,
 			HasEnv:    len(c.EncryptedEnv) > 0,
+			EnvKeys:   envKeys,
 			CreatedAt: c.CreatedAt,
 			UpdatedAt: c.UpdatedAt,
 		})
@@ -119,6 +122,7 @@ func (h *SecureCLIHandler) handleSetUserCredentials(w http.ResponseWriter, r *ht
 		return
 	}
 
+	emitAudit(h.msgBus, r, "secure_cli.user_credentials.updated", "secure_cli_user_credentials", binaryID.String()+"/"+userID)
 	h.emitCacheInvalidate("")
 	writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
 }
@@ -142,6 +146,7 @@ func (h *SecureCLIHandler) handleDeleteUserCredentials(w http.ResponseWriter, r
 		return
 	}
 
+	emitAudit(h.msgBus, r, "secure_cli.user_credentials.deleted", "secure_cli_user_credentials", binaryID.String()+"/"+userID)
 	h.emitCacheInvalidate("")
 	writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
 }

internal/store/secure_cli_store.go

@@ -23,6 +23,8 @@ type SecureCLIBinary struct {
 	Enabled        bool            `json:"enabled"`
 	CreatedBy      string          `json:"created_by"`
 	UserEnv        []byte          `json:"-"` // per-user encrypted env (populated by LookupByBinary LEFT JOIN)
+	// EnvKeys is set by HTTP handlers only (names from decrypted env, no values); not a DB column.
+	EnvKeys []string `json:"env_keys,omitempty"`
 }
 
 // SecureCLIUserCredential holds per-user encrypted env overrides for a binary.

ui/web/src/i18n/locales/en/cli-credentials.json

@@ -21,7 +21,7 @@
     "presetPlaceholder": "Select a preset (optional)",
     "noPreset": "No preset (manual)",
     "presetHint": "Selecting a preset auto-fills fields and shows required env vars.",
-    "encryptedHint": "Credentials are encrypted and cannot be displayed. Leave environment variables empty to keep existing values, or re-enter to update.",
+    "encryptedHint": "Values are encrypted and never shown. Variable names are listed below; leave a value empty to keep the stored secret, or type a new value to replace it.",
     "envVars": "Environment Variables",
     "binaryName": "Binary Name",
     "binaryPath": "Binary Path",
@@ -37,7 +37,8 @@
     "addEnvVar": "Add Variable",
     "noEnvVarsHint": "Click \"Add Variable\" to define environment variables for this CLI tool.",
     "envKeyPlaceholder": "ENV_VAR_NAME",
-    "envValuePlaceholder": "value"
+    "envValuePlaceholder": "value",
+    "invalidEnvKey": "\"{{key}}\" is not a valid env variable name (use A-Z, 0-9, underscore)."
   },
   "placeholders": {
     "binaryName": "e.g. gh",

ui/web/src/i18n/locales/vi/cli-credentials.json

@@ -21,7 +21,7 @@
     "presetPlaceholder": "Chọn mẫu (tùy chọn)",
     "noPreset": "Không dùng mẫu (nhập tay)",
     "presetHint": "Chọn mẫu sẽ tự động điền các trường và hiển thị biến môi trường cần thiết.",
-    "encryptedHint": "Thông tin xác thực đã được mã hóa và không thể hiển thị. Để trống biến môi trường để giữ giá trị hiện tại, hoặc nhập lại để cập nhật.",
+    "encryptedHint": "Giá trị được mã hóa và không hiển thị. Tên biến được liệt kê bên dưới; để trống giá trị để giữ bí mật đã lưu, hoặc nhập giá trị mới để thay thế.",
     "envVars": "Biến môi trường",
     "binaryName": "Tên Binary",
     "binaryPath": "Đường dẫn Binary",
@@ -37,7 +37,8 @@
     "addEnvVar": "Thêm biến",
     "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ị"
+    "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)."
   },
   "placeholders": {
     "binaryName": "vd: gh",

ui/web/src/i18n/locales/zh/cli-credentials.json

@@ -21,7 +21,7 @@
     "presetPlaceholder": "选择预设（可选）",
     "noPreset": "不使用预设（手动）",
     "presetHint": "选择预设会自动填充字段并显示所需的环境变量。",
-    "encryptedHint": "凭证已加密，无法显示。留空环境变量以保留现有值，或重新输入以更新。",
+    "encryptedHint": "值已加密，不会显示。下方列出变量名；留空值以保留已保存的密钥，或输入新值以替换。",
     "envVars": "环境变量",
     "binaryName": "二进制名称",
     "binaryPath": "二进制路径",
@@ -37,7 +37,8 @@
     "addEnvVar": "添加变量",
     "noEnvVarsHint": "点击\"添加变量\"为此 CLI 工具定义环境变量。",
     "envKeyPlaceholder": "变量名",
-    "envValuePlaceholder": "值"
+    "envValuePlaceholder": "值",
+    "invalidEnvKey": "\"{{key}}\" 不是有效的环境变量名（使用 A-Z、0-9、下划线）。"
   },
   "placeholders": {
     "binaryName": "例如 gh",

ui/web/src/pages/cli-credentials/cli-credential-form-dialog.tsx

@@ -12,6 +12,7 @@ import {
   Select, SelectContent, SelectItem, SelectTrigger, SelectValue,
 } from "@/components/ui/select";
 import { Plus, X } from "lucide-react";
+import { useHttp } from "@/hooks/use-ws";
 import type { SecureCLIBinary, CLICredentialInput, CLIPreset } from "./hooks/use-cli-credentials";
 
 interface ManualEnvEntry {
@@ -32,6 +33,7 @@ const NONE_PRESET = "__none__";
 export function CliCredentialFormDialog({ open, onOpenChange, credential, presets, onSubmit }: Props) {
   const { t } = useTranslation("cli-credentials");
   const { t: tc } = useTranslation("common");
+  const http = useHttp();
 
   const [selectedPreset, setSelectedPreset] = useState(NONE_PRESET);
   const [binaryName, setBinaryName] = useState("");
@@ -45,6 +47,8 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
   const [enabled, setEnabled] = useState(true);
   const [envValues, setEnvValues] = useState<Record<string, string>>({});
   const [manualEnvEntries, setManualEnvEntries] = useState<ManualEnvEntry[]>([]);
+  /** Snapshot when dialog opens (edit) — detect clearing all env rows */
+  const [initialEnvKeys, setInitialEnvKeys] = useState<string[]>([]);
   const [loading, setLoading] = useState(false);
   const [error, setError] = useState("");
 
@@ -71,9 +75,38 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
     setAgentId(credential?.agent_id ?? "");
     setEnabled(credential?.enabled ?? true);
     setEnvValues({});
-    setManualEnvEntries([]);
     setError("");
-  }, [open, credential]);
+
+    if (!credential) {
+      setInitialEnvKeys([]);
+      setManualEnvEntries([]);
+      return;
+    }
+
+    const applyEnvKeys = (keys: string[]) => {
+      setInitialEnvKeys(keys);
+      setManualEnvEntries(keys.length > 0 ? keys.map((k) => ({ key: k, value: "" })) : []);
+    };
+
+    if (credential.env_keys !== undefined) {
+      applyEnvKeys(credential.env_keys ?? []);
+      return;
+    }
+
+    let cancelled = false;
+    void (async () => {
+      try {
+        const full = await http.get<SecureCLIBinary>(`/v1/cli-credentials/${credential.id}`);
+        if (cancelled) return;
+        applyEnvKeys(full.env_keys ?? []);
+      } catch {
+        if (!cancelled) applyEnvKeys([]);
+      }
+    })();
+    return () => {
+      cancelled = true;
+    };
+  }, [open, credential, http]);
 
   const applyPreset = (key: string) => {
     setSelectedPreset(key);
@@ -107,12 +140,18 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
   const splitCommaList = (v: string): string[] =>
     v.split(",").map((s) => s.trim()).filter(Boolean);
 
-  const buildEnvPayload = (): Record<string, string> => {
+  const ENV_KEY_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/;
+
+  const buildEnvPayload = (): Record<string, string> | null => {
     if (!isManualMode) return envValues;
     const env: Record<string, string> = {};
     for (const entry of manualEnvEntries) {
       const k = entry.key.trim();
-      if (k && entry.value) env[k] = entry.value;
+      if (k && !ENV_KEY_PATTERN.test(k)) {
+        setError(t("form.invalidEnvKey", { key: k }));
+        return null;
+      }
+      if (k) env[k] = entry.value;
     }
     return env;
   };
@@ -138,7 +177,12 @@ export function CliCredentialFormDialog({ open, onOpenChange, credential, preset
       };
       if (selectedPreset !== NONE_PRESET) payload.preset = selectedPreset;
       const env = buildEnvPayload();
-      if (Object.keys(env).length > 0) payload.env = env;
+      if (!env) return;
+      if (Object.keys(env).length > 0) {
+        payload.env = env;
+      } else if (isEdit && isManualMode && initialEnvKeys.length > 0) {
+        payload.env = {};
+      }
       await onSubmit(payload);
       onOpenChange(false);
     } catch (err) {

ui/web/src/pages/cli-credentials/cli-user-credentials-dialog.tsx

@@ -24,6 +24,8 @@ interface UserCredEntry {
   binary_id: string;
   user_id: string;
   has_env: boolean;
+  /** Env variable names (no values) for display */
+  env_keys?: string[];
   created_at: string;
   updated_at: string;
 }
@@ -103,7 +105,8 @@ export function CLIUserCredentialsDialog({ open, onOpenChange, binary }: CLIUser
   const handleSave = async () => {
     const uid = userId.trim();
     if (!uid) return;
-    if (Object.keys(env).length === 0) {
+    // New entry needs at least one variable; edits may clear all keys (empty object).
+    if (!editEntry && Object.keys(env).length === 0) {
       toast.error(i18next.t("cli-credentials:userCredentials.envRequired"));
       return;
     }
@@ -169,12 +172,19 @@ export function CLIUserCredentialsDialog({ open, onOpenChange, binary }: CLIUser
                     key={entry.id}
                     className="flex items-center justify-between rounded-md border px-3 py-2"
                   >
-                    <div className="flex items-center gap-2 min-w-0">
-                      <span className="font-mono text-sm truncate">{entry.user_id}</span>
-                      {entry.has_env && (
-                        <Badge variant="secondary" className="shrink-0 text-xs">
-                          env
-                        </Badge>
+                    <div className="flex flex-col gap-1 min-w-0">
+                      <div className="flex items-center gap-2 min-w-0">
+                        <span className="font-mono text-sm truncate">{entry.user_id}</span>
+                        {entry.has_env && (
+                          <Badge variant="secondary" className="shrink-0 text-xs">
+                            env
+                          </Badge>
+                        )}
+                      </div>
+                      {entry.env_keys && entry.env_keys.length > 0 && (
+                        <p className="text-xs text-muted-foreground font-mono truncate" title={entry.env_keys.join(", ")}>
+                          {entry.env_keys.join(", ")}
+                        </p>
                       )}
                     </div>
                     <div className="flex items-center gap-1 shrink-0">

ui/web/src/types/cli-credential.ts

@@ -12,6 +12,8 @@ export interface SecureCLIBinary {
   created_by: string;
   created_at: string;
   updated_at: string;
+  /** Env variable names only (no values); from API for edit form */
+  env_keys?: string[];
 }
 
 export interface CLIPresetEnvVar {