mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 00:16:38 +00:00
fix(cli-credentials): stabilize git preset creation
Squash merge PR #96 after PR #92. PR checks were green; dev beta CI will be monitored after merge.
This commit is contained in:
1 parent
1d75e38ac2
commit
c4e0bd2be5
8 files changed
+216
-19
No files matched your search
@@ -4,6 +4,30 @@ Significant changes, features, and fixes in reverse chronological order.
|
||||
|
||||
---
|
||||
|
||||
## 2026-05-29
|
||||
|
||||
### CLI Credentials git preset null-env crash (issue #93)
|
||||
|
||||
**Fixes**
|
||||
|
||||
- Stopped `/v1/cli-credentials/presets` from returning nullable preset arrays
|
||||
for adapter-managed CLIs such as `git`; frontend now also normalizes older
|
||||
`null` payloads defensively.
|
||||
- Allowed the `git` preset to create a SecureCLI binary without legacy env
|
||||
vars and persisted its `adapter_name=git`, so the PAT/SSH user credential
|
||||
flow activates after creation.
|
||||
- Guarded the preset env-var renderer against nullish `env_vars` to keep the
|
||||
Runtime & Packages → CLI Credentials tab renderable.
|
||||
|
||||
**Tests**
|
||||
|
||||
- Added backend regression coverage for stable preset arrays and git preset
|
||||
creation without legacy env.
|
||||
- Added frontend normalization coverage for nullable adapter-managed preset
|
||||
arrays.
|
||||
|
||||
---
|
||||
|
||||
## 2026-05-28
|
||||
|
||||
### CLI credential adapter framework + git adapter (issue #82)
|
||||
|
||||
+63
-12
@@ -116,9 +116,10 @@ func (h *SecureCLIHandler) handleCreate(w http.ResponseWriter, r *http.Request)
|
||||
return
|
||||
}
|
||||
|
||||
var preset *tools.CLIPreset
|
||||
// Apply preset defaults if specified
|
||||
if req.Preset != "" {
|
||||
preset := tools.GetPreset(req.Preset)
|
||||
preset = tools.GetPreset(req.Preset)
|
||||
if preset == nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "unknown preset: " + req.Preset})
|
||||
return
|
||||
@@ -147,22 +148,31 @@ func (h *SecureCLIHandler) handleCreate(w http.ResponseWriter, r *http.Request)
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgRequired, "binary_name")})
|
||||
return
|
||||
}
|
||||
if len(req.Env) == 0 {
|
||||
adapterManagedPreset := preset != nil && strings.TrimSpace(preset.AdapterName) != "" && len(preset.EnvVars) == 0
|
||||
if len(req.Env) == 0 && !adapterManagedPreset {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgRequired, "env")})
|
||||
return
|
||||
}
|
||||
|
||||
envEntries, err := store.ParseSecureCLIEnv(req.Env)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, err.Error())})
|
||||
return
|
||||
}
|
||||
envJSON, err := store.SerializeSecureCLIEnv(envEntries)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, err.Error())})
|
||||
return
|
||||
envJSON := []byte("{}")
|
||||
if len(req.Env) > 0 {
|
||||
envEntries, err := store.ParseSecureCLIEnv(req.Env)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, err.Error())})
|
||||
return
|
||||
}
|
||||
envJSON, err = store.SerializeSecureCLIEnv(envEntries)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, err.Error())})
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
var adapterName *string
|
||||
if preset != nil && strings.TrimSpace(preset.AdapterName) != "" {
|
||||
name := strings.TrimSpace(preset.AdapterName)
|
||||
adapterName = &name
|
||||
}
|
||||
b := &store.SecureCLIBinary{
|
||||
BinaryName: req.BinaryName,
|
||||
BinaryPath: req.BinaryPath,
|
||||
@@ -175,6 +185,7 @@ func (h *SecureCLIHandler) handleCreate(w http.ResponseWriter, r *http.Request)
|
||||
IsGlobal: req.IsGlobal == nil || *req.IsGlobal, // default true
|
||||
Enabled: req.Enabled,
|
||||
CreatedBy: store.UserIDFromContext(r.Context()),
|
||||
AdapterName: adapterName,
|
||||
}
|
||||
if b.TimeoutSeconds <= 0 {
|
||||
b.TimeoutSeconds = 30
|
||||
@@ -290,8 +301,48 @@ func (h *SecureCLIHandler) handleDelete(w http.ResponseWriter, r *http.Request)
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "deleted"})
|
||||
}
|
||||
|
||||
type cliPresetResponse struct {
|
||||
BinaryName string `json:"binary_name"`
|
||||
Description string `json:"description"`
|
||||
EnvVars []tools.EnvVarDef `json:"env_vars"`
|
||||
DenyArgs []string `json:"deny_args"`
|
||||
DenyVerbose []string `json:"deny_verbose"`
|
||||
Timeout int `json:"timeout"`
|
||||
Tips string `json:"tips"`
|
||||
AdapterName string `json:"adapter_name,omitempty"`
|
||||
}
|
||||
|
||||
func normalizedCLIPresets() map[string]cliPresetResponse {
|
||||
out := make(map[string]cliPresetResponse, len(tools.CLIPresets))
|
||||
for key, preset := range tools.CLIPresets {
|
||||
envVars := preset.EnvVars
|
||||
if envVars == nil {
|
||||
envVars = []tools.EnvVarDef{}
|
||||
}
|
||||
denyArgs := preset.DenyArgs
|
||||
if denyArgs == nil {
|
||||
denyArgs = []string{}
|
||||
}
|
||||
denyVerbose := preset.DenyVerbose
|
||||
if denyVerbose == nil {
|
||||
denyVerbose = []string{}
|
||||
}
|
||||
out[key] = cliPresetResponse{
|
||||
BinaryName: preset.BinaryName,
|
||||
Description: preset.Description,
|
||||
EnvVars: envVars,
|
||||
DenyArgs: denyArgs,
|
||||
DenyVerbose: denyVerbose,
|
||||
Timeout: preset.Timeout,
|
||||
Tips: preset.Tips,
|
||||
AdapterName: preset.AdapterName,
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func (h *SecureCLIHandler) handlePresets(w http.ResponseWriter, _ *http.Request) {
|
||||
writeJSON(w, http.StatusOK, map[string]any{"presets": tools.CLIPresets})
|
||||
writeJSON(w, http.StatusOK, map[string]any{"presets": normalizedCLIPresets()})
|
||||
}
|
||||
|
||||
// handleCheckBinary resolves a binary name to its absolute path via exec.LookPath.
|
||||
|
||||
@@ -110,6 +110,61 @@ func TestSecureCLIPresetsIncludesGoogleWorkspace(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSecureCLIPresetsReturnStableArraysForGit(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodGet, "/v1/cli-credentials/presets", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
NewSecureCLIHandler(nil, nil).handlePresets(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var got struct {
|
||||
Presets map[string]struct {
|
||||
EnvVars []struct{} `json:"env_vars"`
|
||||
DenyVerbose []string `json:"deny_verbose"`
|
||||
AdapterName string `json:"adapter_name"`
|
||||
} `json:"presets"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
git := got.Presets["git"]
|
||||
if git.EnvVars == nil {
|
||||
t.Fatalf("git env_vars = nil; response must use [] for frontend compatibility")
|
||||
}
|
||||
if git.DenyVerbose == nil {
|
||||
t.Fatalf("git deny_verbose = nil; response must use [] for frontend compatibility")
|
||||
}
|
||||
if git.AdapterName != "git" {
|
||||
t.Fatalf("git adapter_name = %q, want git", git.AdapterName)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSecureCLICreateAllowsGitPresetWithoutLegacyEnv(t *testing.T) {
|
||||
st := &fakeSecureCLIStore{}
|
||||
req := httptest.NewRequest(http.MethodPost, "/v1/cli-credentials", strings.NewReader(`{"preset":"git"}`))
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
NewSecureCLIHandler(st, nil).handleCreate(rec, req)
|
||||
|
||||
if rec.Code != http.StatusCreated {
|
||||
t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if st.created == nil {
|
||||
t.Fatal("expected store.Create to be called")
|
||||
}
|
||||
if st.created.BinaryName != "git" {
|
||||
t.Fatalf("binary_name = %q, want git", st.created.BinaryName)
|
||||
}
|
||||
if st.created.AdapterName == nil || *st.created.AdapterName != "git" {
|
||||
t.Fatalf("adapter_name = %#v, want git", st.created.AdapterName)
|
||||
}
|
||||
if string(st.created.EncryptedEnv) != "{}" {
|
||||
t.Fatalf("encrypted env = %q, want {}", st.created.EncryptedEnv)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSecureCLICheckBinaryFindsNpmPackageCliAlias(t *testing.T) {
|
||||
runtimeDir := t.TempDir()
|
||||
t.Setenv("RUNTIME_DIR", runtimeDir)
|
||||
|
||||
@@ -14,11 +14,16 @@ import (
|
||||
)
|
||||
|
||||
type fakeSecureCLIStore struct {
|
||||
binary *store.SecureCLIBinary
|
||||
user *store.SecureCLIUserCredential
|
||||
binary *store.SecureCLIBinary
|
||||
user *store.SecureCLIUserCredential
|
||||
created *store.SecureCLIBinary
|
||||
}
|
||||
|
||||
func (s *fakeSecureCLIStore) Create(context.Context, *store.SecureCLIBinary) error { return nil }
|
||||
func (s *fakeSecureCLIStore) Create(_ context.Context, b *store.SecureCLIBinary) error {
|
||||
cp := *b
|
||||
s.created = &cp
|
||||
return nil
|
||||
}
|
||||
func (s *fakeSecureCLIStore) Get(context.Context, uuid.UUID) (*store.SecureCLIBinary, error) {
|
||||
cp := *s.binary
|
||||
return &cp, nil
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { normalizeCliPreset, normalizeCliPresets } from "../hooks/use-cli-credentials";
|
||||
|
||||
describe("cli credential preset normalization", () => {
|
||||
it("normalizes nullable arrays from adapter-managed presets", () => {
|
||||
const preset = normalizeCliPreset({
|
||||
binary_name: "git",
|
||||
description: "Git with credential adapter",
|
||||
env_vars: null,
|
||||
deny_args: ["(?i)credential-helper"],
|
||||
deny_verbose: null,
|
||||
timeout: 300,
|
||||
tips: "Adapter handles auth automatically",
|
||||
adapter_name: "git",
|
||||
});
|
||||
|
||||
expect(preset).toEqual({
|
||||
binary_name: "git",
|
||||
description: "Git with credential adapter",
|
||||
env_vars: [],
|
||||
deny_args: ["(?i)credential-helper"],
|
||||
deny_verbose: [],
|
||||
timeout: 300,
|
||||
tips: "Adapter handles auth automatically",
|
||||
adapter_name: "git",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns an empty preset map for missing API payloads", () => {
|
||||
expect(normalizeCliPresets(null)).toEqual({});
|
||||
expect(normalizeCliPresets(undefined)).toEqual({});
|
||||
});
|
||||
});
|
||||
@@ -53,12 +53,14 @@ export function CliCredentialEnvVarsSection({
|
||||
);
|
||||
}, [setManualEnvEntries]);
|
||||
|
||||
const presetEnvVars = activePreset?.env_vars ?? [];
|
||||
|
||||
// Preset-driven env var inputs
|
||||
if (!isManualMode && activePreset && activePreset.env_vars.length > 0) {
|
||||
if (!isManualMode && presetEnvVars.length > 0) {
|
||||
return (
|
||||
<div className="grid gap-3 rounded-md border p-3">
|
||||
<p className="text-sm font-medium">{t("form.envVars")}</p>
|
||||
{activePreset.env_vars.map((ev) => (
|
||||
{presetEnvVars.map((ev) => (
|
||||
<div key={ev.name} className="grid gap-1.5">
|
||||
<Label htmlFor={`env-${ev.name}`}>
|
||||
{ev.name}
|
||||
|
||||
@@ -10,6 +10,32 @@ export type { SecureCLIBinary, CLICredentialInput, CLIPreset, CLIAgentGrant, CLI
|
||||
const QUERY_KEY = ["cliCredentials"] as const;
|
||||
const PRESETS_KEY = ["cliCredentials", "presets"] as const;
|
||||
|
||||
type RawCLIPreset = Partial<Omit<CLIPreset, "env_vars" | "deny_args" | "deny_verbose">> & {
|
||||
env_vars?: CLIPreset["env_vars"] | null;
|
||||
deny_args?: string[] | null;
|
||||
deny_verbose?: string[] | null;
|
||||
};
|
||||
|
||||
export function normalizeCliPreset(raw: RawCLIPreset | null | undefined): CLIPreset {
|
||||
return {
|
||||
binary_name: raw?.binary_name ?? "",
|
||||
description: raw?.description ?? "",
|
||||
env_vars: Array.isArray(raw?.env_vars) ? raw.env_vars : [],
|
||||
deny_args: Array.isArray(raw?.deny_args) ? raw.deny_args : [],
|
||||
deny_verbose: Array.isArray(raw?.deny_verbose) ? raw.deny_verbose : [],
|
||||
timeout: typeof raw?.timeout === "number" ? raw.timeout : 30,
|
||||
tips: raw?.tips ?? "",
|
||||
...(raw?.adapter_name ? { adapter_name: raw.adapter_name } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
export function normalizeCliPresets(raw: Record<string, RawCLIPreset | null> | null | undefined): Record<string, CLIPreset> {
|
||||
if (!raw) return {};
|
||||
return Object.fromEntries(
|
||||
Object.entries(raw).map(([key, preset]) => [key, normalizeCliPreset(preset)]),
|
||||
);
|
||||
}
|
||||
|
||||
export function useCliCredentials() {
|
||||
const http = useHttp();
|
||||
const queryClient = useQueryClient();
|
||||
@@ -85,8 +111,8 @@ export function useCliCredentialPresets() {
|
||||
const { data, isLoading } = useQuery({
|
||||
queryKey: PRESETS_KEY,
|
||||
queryFn: async () => {
|
||||
const res = await http.get<{ presets: Record<string, CLIPreset> }>("/v1/cli-credentials/presets");
|
||||
return res.presets ?? {};
|
||||
const res = await http.get<{ presets: Record<string, RawCLIPreset | null> }>("/v1/cli-credentials/presets");
|
||||
return normalizeCliPresets(res.presets);
|
||||
},
|
||||
staleTime: 5 * 60 * 1000,
|
||||
});
|
||||
|
||||
@@ -58,6 +58,7 @@ export interface CLIPreset {
|
||||
deny_verbose: string[];
|
||||
timeout: number;
|
||||
tips: string;
|
||||
adapter_name?: string;
|
||||
}
|
||||
|
||||
export interface CLICredentialInput {
|
||||
|
||||
Reference in new issue
Block a user