From fd17e4b1749caadd3709ec99a46554bee0b07106 Mon Sep 17 00:00:00 2001 From: Kohei Date: Thu, 26 Feb 2026 18:07:02 +0900 Subject: [PATCH] fix: improve gateway security and clean up helpers Use constant-time comparison for auth token validation to prevent timing attacks. Consolidate writeJSONError into writeJSON, add error logging for JSON encoding failures, remove unused isSecretKey helper, and handle syscall.Exec error on restart. Co-Authored-By: Claude Opus 4.6 --- cmd/clawdroid/main.go | 5 ++++- pkg/gateway/auth.go | 8 +++----- pkg/gateway/handlers.go | 16 ++++------------ pkg/gateway/schema.go | 4 ++-- pkg/gateway/server_test.go | 24 ++++++++++-------------- 5 files changed, 23 insertions(+), 34 deletions(-) diff --git a/cmd/clawdroid/main.go b/cmd/clawdroid/main.go index a45e77f1c..46a3c2d45 100644 --- a/cmd/clawdroid/main.go +++ b/cmd/clawdroid/main.go @@ -590,7 +590,10 @@ func gatewayCmd() { fmt.Printf("Error finding executable: %v\n", err) os.Exit(1) } - syscall.Exec(exe, os.Args, os.Environ()) + if err := syscall.Exec(exe, os.Args, os.Environ()); err != nil { + fmt.Printf("Error restarting: %v\n", err) + os.Exit(1) + } } } diff --git a/pkg/gateway/auth.go b/pkg/gateway/auth.go index 1765f4f41..0899bab71 100644 --- a/pkg/gateway/auth.go +++ b/pkg/gateway/auth.go @@ -1,7 +1,7 @@ package gateway import ( - "encoding/json" + "crypto/subtle" "net/http" "strings" ) @@ -29,7 +29,7 @@ func (s *Server) authMiddleware(next http.HandlerFunc) http.HandlerFunc { } token := authHeader[len(prefix):] - if token != apiKey { + if subtle.ConstantTimeCompare([]byte(token), []byte(apiKey)) != 1 { writeJSONError(w, http.StatusForbidden, "invalid token") return } @@ -39,7 +39,5 @@ func (s *Server) authMiddleware(next http.HandlerFunc) http.HandlerFunc { } func writeJSONError(w http.ResponseWriter, code int, message string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(code) - json.NewEncoder(w).Encode(map[string]string{"error": message}) + writeJSON(w, code, map[string]string{"error": message}) } diff --git a/pkg/gateway/handlers.go b/pkg/gateway/handlers.go index 9a406b85f..f75ee7c00 100644 --- a/pkg/gateway/handlers.go +++ b/pkg/gateway/handlers.go @@ -3,10 +3,10 @@ package gateway import ( "encoding/json" "net/http" - "strings" "time" "github.com/KarakuriAgent/clawdroid/pkg/config" + "github.com/KarakuriAgent/clawdroid/pkg/logger" ) // handleGetSchema returns the configuration schema. @@ -134,15 +134,7 @@ func restoreSecrets(incoming, current map[string]interface{}) { func writeJSON(w http.ResponseWriter, code int, v interface{}) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(code) - json.NewEncoder(w).Encode(v) -} - -// isSecretKey returns true if the given JSON key should be treated as a secret. -// This is used by the handlers to check whether to mask a value, and is also -// exposed for testing purposes. -func isSecretKey(key string) bool { - // Check the leaf key (last segment after dots) - parts := strings.SplitN(key, ".", -1) - leaf := parts[len(parts)-1] - return secretKeys[leaf] + if err := json.NewEncoder(w).Encode(v); err != nil { + logger.ErrorCF("gateway", "failed to encode JSON response", map[string]interface{}{"error": err.Error()}) + } } diff --git a/pkg/gateway/schema.go b/pkg/gateway/schema.go index 58273d345..e9ea9aba1 100644 --- a/pkg/gateway/schema.go +++ b/pkg/gateway/schema.go @@ -48,8 +48,8 @@ var secretKeys = map[string]bool{ func BuildSchema(defaultCfg *config.Config) SchemaResponse { var sections []SchemaSection - cfgType := reflect.TypeOf(*defaultCfg) - cfgVal := reflect.ValueOf(*defaultCfg) + cfgType := reflect.TypeOf(defaultCfg).Elem() + cfgVal := reflect.ValueOf(defaultCfg).Elem() for i := 0; i < cfgType.NumField(); i++ { field := cfgType.Field(i) diff --git a/pkg/gateway/server_test.go b/pkg/gateway/server_test.go index 47858bdb3..81c350ef8 100644 --- a/pkg/gateway/server_test.go +++ b/pkg/gateway/server_test.go @@ -962,22 +962,18 @@ func TestAuthMiddleware_CorrectToken_200(t *testing.T) { } } -func TestIsSecretKey(t *testing.T) { - tests := []struct { - key string - want bool - }{ - {"api_key", true}, - {"gateway.api_key", true}, - {"channels.telegram.token", true}, - {"model", false}, - {"gateway.port", false}, +func TestSecretKeys(t *testing.T) { + wantSecret := []string{"api_key", "token", "bot_token", "app_token", "channel_secret", "channel_access_token"} + for _, k := range wantSecret { + if !secretKeys[k] { + t.Errorf("secretKeys[%q] = false, want true", k) + } } - for _, tc := range tests { - got := isSecretKey(tc.key) - if got != tc.want { - t.Errorf("isSecretKey(%q) = %v, want %v", tc.key, got, tc.want) + notSecret := []string{"model", "host", "port", "enabled"} + for _, k := range notSecret { + if secretKeys[k] { + t.Errorf("secretKeys[%q] = true, want false", k) } } }