From 10b824fcac9c8e68100d51e765989647782e656a Mon Sep 17 00:00:00 2001 From: Luis Pater Date: Sat, 28 Mar 2026 04:48:23 +0800 Subject: [PATCH] fix(security): validate auth file names to prevent unsafe input --- .../api/handlers/management/auth_files.go | 23 +++++-- .../management/auth_files_download_test.go | 62 +++++++++++++++++++ .../auth_files_download_windows_test.go | 51 +++++++++++++++ 3 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 internal/api/handlers/management/auth_files_download_test.go create mode 100644 internal/api/handlers/management/auth_files_download_windows_test.go diff --git a/internal/api/handlers/management/auth_files.go b/internal/api/handlers/management/auth_files.go index b9bdc983..2e1f02bf 100644 --- a/internal/api/handlers/management/auth_files.go +++ b/internal/api/handlers/management/auth_files.go @@ -541,10 +541,23 @@ func isRuntimeOnlyAuth(auth *coreauth.Auth) bool { return strings.EqualFold(strings.TrimSpace(auth.Attributes["runtime_only"]), "true") } +func isUnsafeAuthFileName(name string) bool { + if strings.TrimSpace(name) == "" { + return true + } + if strings.ContainsAny(name, "/\\") { + return true + } + if filepath.VolumeName(name) != "" { + return true + } + return false +} + // Download single auth file by name func (h *Handler) DownloadAuthFile(c *gin.Context) { - name := c.Query("name") - if name == "" || strings.Contains(name, string(os.PathSeparator)) { + name := strings.TrimSpace(c.Query("name")) + if isUnsafeAuthFileName(name) { c.JSON(400, gin.H{"error": "invalid name"}) return } @@ -626,8 +639,8 @@ func (h *Handler) UploadAuthFile(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "no files uploaded"}) return } - name := c.Query("name") - if name == "" || strings.Contains(name, string(os.PathSeparator)) { + name := strings.TrimSpace(c.Query("name")) + if isUnsafeAuthFileName(name) { c.JSON(400, gin.H{"error": "invalid name"}) return } @@ -860,7 +873,7 @@ func uniqueAuthFileNames(names []string) []string { func (h *Handler) deleteAuthFileByName(ctx context.Context, name string) (string, int, error) { name = strings.TrimSpace(name) - if name == "" || strings.Contains(name, string(os.PathSeparator)) { + if isUnsafeAuthFileName(name) { return "", http.StatusBadRequest, fmt.Errorf("invalid name") } diff --git a/internal/api/handlers/management/auth_files_download_test.go b/internal/api/handlers/management/auth_files_download_test.go new file mode 100644 index 00000000..a2a20d30 --- /dev/null +++ b/internal/api/handlers/management/auth_files_download_test.go @@ -0,0 +1,62 @@ +package management + +import ( + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "testing" + + "github.com/gin-gonic/gin" + "github.com/router-for-me/CLIProxyAPI/v6/internal/config" +) + +func TestDownloadAuthFile_ReturnsFile(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + gin.SetMode(gin.TestMode) + + authDir := t.TempDir() + fileName := "download-user.json" + expected := []byte(`{"type":"codex"}`) + if err := os.WriteFile(filepath.Join(authDir, fileName), expected, 0o600); err != nil { + t.Fatalf("failed to write auth file: %v", err) + } + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: authDir}, nil) + + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + ctx.Request = httptest.NewRequest(http.MethodGet, "/v0/management/auth-files/download?name="+url.QueryEscape(fileName), nil) + h.DownloadAuthFile(ctx) + + if rec.Code != http.StatusOK { + t.Fatalf("expected download status %d, got %d with body %s", http.StatusOK, rec.Code, rec.Body.String()) + } + if got := rec.Body.Bytes(); string(got) != string(expected) { + t.Fatalf("unexpected download content: %q", string(got)) + } +} + +func TestDownloadAuthFile_RejectsPathSeparators(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + gin.SetMode(gin.TestMode) + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: t.TempDir()}, nil) + + for _, name := range []string{ + "../external/secret.json", + `..\\external\\secret.json`, + "nested/secret.json", + `nested\\secret.json`, + } { + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + ctx.Request = httptest.NewRequest(http.MethodGet, "/v0/management/auth-files/download?name="+url.QueryEscape(name), nil) + h.DownloadAuthFile(ctx) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected %d for name %q, got %d with body %s", http.StatusBadRequest, name, rec.Code, rec.Body.String()) + } + } +} diff --git a/internal/api/handlers/management/auth_files_download_windows_test.go b/internal/api/handlers/management/auth_files_download_windows_test.go new file mode 100644 index 00000000..8c174ccf --- /dev/null +++ b/internal/api/handlers/management/auth_files_download_windows_test.go @@ -0,0 +1,51 @@ +//go:build windows + +package management + +import ( + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "testing" + + "github.com/gin-gonic/gin" + "github.com/router-for-me/CLIProxyAPI/v6/internal/config" +) + +func TestDownloadAuthFile_PreventsWindowsSlashTraversal(t *testing.T) { + t.Setenv("MANAGEMENT_PASSWORD", "") + gin.SetMode(gin.TestMode) + + tempDir := t.TempDir() + authDir := filepath.Join(tempDir, "auth") + externalDir := filepath.Join(tempDir, "external") + if err := os.MkdirAll(authDir, 0o700); err != nil { + t.Fatalf("failed to create auth dir: %v", err) + } + if err := os.MkdirAll(externalDir, 0o700); err != nil { + t.Fatalf("failed to create external dir: %v", err) + } + + secretName := "secret.json" + secretPath := filepath.Join(externalDir, secretName) + if err := os.WriteFile(secretPath, []byte(`{"secret":true}`), 0o600); err != nil { + t.Fatalf("failed to write external file: %v", err) + } + + h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: authDir}, nil) + + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + ctx.Request = httptest.NewRequest( + http.MethodGet, + "/v0/management/auth-files/download?name="+url.QueryEscape("../external/"+secretName), + nil, + ) + h.DownloadAuthFile(ctx) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status %d, got %d with body %s", http.StatusBadRequest, rec.Code, rec.Body.String()) + } +}