From 10ca17f0f5d2e677d268e2e7edd5b6da6fea2938 Mon Sep 17 00:00:00 2001 From: AIRONAX Developer Date: Sat, 5 Sep 2026 02:20:37 +0530 Subject: [PATCH 1/2] fix(security): harden trust boundaries and production HTTP lifecycle Stop spoofable XFF without trusted_proxies, block webhook SSRF and unsafe iframe URLs, confine module icon paths, tighten API-key/reset ACL, and add graceful shutdown with bounded forms and safer WebSocket origins. --- addons/automation/actions.go | 3 +- addons/automation/testexports.go | 4 + addons/automation/webhook.go | 82 ++++++++++++++++++- core/engine/render/html_helpers.go | 29 +++++++ core/engine/render/menu_queries.go | 14 +++- core/orm/config_param.go | 36 +------- core/orm/registry.go | 8 +- core/orm/schema_indexes.go | 15 ++-- core/orm/schema_sync.go | 12 +-- core/orm/schema_util.go | 5 +- core/orm/setup_sync.go | 3 +- core/queue/inproc.go | 15 +++- core/server/config/config.go | 3 + core/server/config/const.go | 1 + core/server/run.go | 58 ++++++++++--- core/server/web/action_resolve.go | 9 ++ core/server/web/apikey_create.go | 11 ++- core/server/web/auth.go | 5 +- core/server/web/auth_helpers.go | 12 ++- core/server/web/csrf.go | 5 +- core/server/web/setup_security.go | 57 ++++++++++++- core/server/web/swc_bus_hub.go | 22 ++++- core/server/web/swc_workspace.go | 8 -- core/server/web/web_request_helpers.go | 2 + core/server/web/workspace_resolve.go | 49 ++++++----- sumeru.conf.example | 4 + test/addons/automation/actions_test.go | 13 +-- test/addons/automation/webhook_test.go | 33 ++++++++ .../core/engine/render/pure_functions_test.go | 30 +++++++ test/core/server/web/action_resolve_test.go | 19 +++++ test/core/server/web/apikey_create_test.go | 7 +- test/core/server/web/setup_security_test.go | 23 +++++- 32 files changed, 464 insertions(+), 133 deletions(-) create mode 100644 test/addons/automation/webhook_test.go diff --git a/addons/automation/actions.go b/addons/automation/actions.go index a6a6a304..4dec9229 100644 --- a/addons/automation/actions.go +++ b/addons/automation/actions.go @@ -81,8 +81,7 @@ func executeServerAction(ctx context.Context, row map[string]interface{}, ev eve if modelName == "" || !ok || resID <= 0 { return nil } - bypass := orm.ContextWithBypass(ctx, true) - return orm.UpdateRecordByID(bypass, modelName, int(resID), vals) + return orm.UpdateRecordByID(ctx, modelName, int(resID), vals) case strings.HasPrefix(code, "webhook:"): url := strings.TrimSpace(strings.TrimPrefix(code, "webhook:")) diff --git a/addons/automation/testexports.go b/addons/automation/testexports.go index 0a2874df..1b2e1068 100644 --- a/addons/automation/testexports.go +++ b/addons/automation/testexports.go @@ -6,6 +6,10 @@ import ( "sumeru/core/event" ) +func ValidateWebhookURLForTest(raw string) error { + return validateWebhookURL(raw) +} + func ExecuteServerActionForTest(ctx context.Context, row map[string]interface{}, ev event.Event) error { return executeServerAction(ctx, row, ev) } diff --git a/addons/automation/webhook.go b/addons/automation/webhook.go index 65bd9ccb..9f387e16 100644 --- a/addons/automation/webhook.go +++ b/addons/automation/webhook.go @@ -4,14 +4,29 @@ import ( "bytes" "context" "encoding/json" + "fmt" + "net" "net/http" + "net/url" + "strings" "time" "sumeru/core/applog" "sumeru/core/event" ) -func dispatchWebhook(ctx context.Context, url string, ev event.Event) error { +func dispatchWebhook(ctx context.Context, rawURL string, ev event.Event) error { + if err := validateWebhookURL(rawURL); err != nil { + applog.Warn(ctx, applog.Event{ + Message: "webhook URL rejected", + Component: "automation", + Operation: "webhook", + Status: "blocked", + Context: map[string]interface{}{"url": rawURL}, + Err: err, + }) + return err + } body, err := json.Marshal(map[string]interface{}{ "event": ev.Name, "actor": ev.Actor, @@ -20,12 +35,20 @@ func dispatchWebhook(ctx context.Context, url string, ev event.Event) error { if err != nil { return err } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, rawURL, bytes.NewReader(body)) if err != nil { return err } req.Header.Set("Content-Type", "application/json") - client := &http.Client{Timeout: 15 * time.Second} + client := &http.Client{ + Timeout: 15 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 3 { + return fmt.Errorf("webhook redirect limit exceeded") + } + return validateWebhookURL(req.URL.String()) + }, + } resp, err := client.Do(req) if err != nil { return err @@ -37,8 +60,59 @@ func dispatchWebhook(ctx context.Context, url string, ev event.Event) error { Component: "automation", Operation: "webhook", Status: "failed", - Context: map[string]interface{}{"url": url, "status": resp.StatusCode}, + Context: map[string]interface{}{"url": rawURL, "status": resp.StatusCode}, }) } return nil } + +func validateWebhookURL(raw string) error { + raw = strings.TrimSpace(raw) + if raw == "" { + return fmt.Errorf("empty webhook url") + } + u, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("invalid webhook url: %w", err) + } + scheme := strings.ToLower(u.Scheme) + if scheme != "https" && scheme != "http" { + return fmt.Errorf("webhook scheme %q not allowed", u.Scheme) + } + host := strings.TrimSpace(u.Hostname()) + if host == "" { + return fmt.Errorf("webhook host required") + } + lowerHost := strings.ToLower(host) + if lowerHost == "localhost" || strings.HasSuffix(lowerHost, ".localhost") || lowerHost == "metadata.google.internal" { + return fmt.Errorf("webhook host not allowed") + } + if ip := net.ParseIP(host); ip != nil { + if blockedWebhookIP(ip) { + return fmt.Errorf("webhook IP not allowed") + } + return nil + } + ips, err := net.LookupIP(host) + if err != nil { + return fmt.Errorf("webhook host lookup: %w", err) + } + if len(ips) == 0 { + return fmt.Errorf("webhook host resolved to no addresses") + } + for _, ip := range ips { + if blockedWebhookIP(ip) { + return fmt.Errorf("webhook host resolves to blocked address") + } + } + return nil +} + +func blockedWebhookIP(ip net.IP) bool { + return ip.IsLoopback() || + ip.IsPrivate() || + ip.IsLinkLocalUnicast() || + ip.IsLinkLocalMulticast() || + ip.IsUnspecified() || + ip.IsMulticast() +} diff --git a/core/engine/render/html_helpers.go b/core/engine/render/html_helpers.go index b292e6cf..47e55302 100644 --- a/core/engine/render/html_helpers.go +++ b/core/engine/render/html_helpers.go @@ -17,6 +17,35 @@ func SafeImageSrc(src string) bool { strings.HasPrefix(src, "/")) } +// SafeIframeURL reports whether src is safe for an iframe (https or site-relative path). +// Rejects javascript:, data:, file:, and protocol-relative URLs. +func SafeIframeURL(src string) bool { + src = strings.TrimSpace(src) + if src == "" { + return false + } + lower := strings.ToLower(src) + if strings.HasPrefix(lower, "javascript:") || + strings.HasPrefix(lower, "data:") || + strings.HasPrefix(lower, "file:") || + strings.HasPrefix(lower, "vbscript:") || + strings.HasPrefix(src, "//") { + return false + } + if strings.HasPrefix(src, "/") { + return true + } + return strings.HasPrefix(lower, "https://") +} + +// SafeIframeURLAllowHTTP is SafeIframeURL plus http:// absolute URLs (dev-only callers). +func SafeIframeURLAllowHTTP(src string) bool { + if SafeIframeURL(src) { + return true + } + return strings.HasPrefix(strings.ToLower(strings.TrimSpace(src)), "http://") +} + // FieldDisplayLabel returns the column/field label from XML string attr or a humanized field name. func FieldDisplayLabel(field parser.Field) string { if label := strings.TrimSpace(field.Label); label != "" { diff --git a/core/engine/render/menu_queries.go b/core/engine/render/menu_queries.go index ccc36d63..6f9503a1 100644 --- a/core/engine/render/menu_queries.go +++ b/core/engine/render/menu_queries.go @@ -53,17 +53,29 @@ func ModuleIconServePath(moduleName, iconRel string) string { if a == nil || a.Path == "" { return "" } + root := filepath.Clean(a.Path) candidates := []string{} if iconRel = strings.TrimSpace(iconRel); iconRel != "" { candidates = append(candidates, iconRel) } candidates = append(candidates, "static/icon.png") for _, rel := range candidates { + rel = strings.TrimSpace(rel) + if rel == "" || strings.Contains(rel, `\`) || strings.Contains(rel, "..") || strings.HasPrefix(rel, "/") { + continue + } + if filepath.IsAbs(rel) || (len(rel) >= 2 && rel[1] == ':') { + continue + } rel = filepath.Clean(rel) if rel == "." || strings.HasPrefix(rel, "..") { continue } - full := filepath.Join(a.Path, rel) + full := filepath.Join(root, rel) + relToRoot, err := filepath.Rel(root, full) + if err != nil || relToRoot == ".." || strings.HasPrefix(relToRoot, ".."+string(filepath.Separator)) { + continue + } if fi, err := os.Stat(full); err == nil && !fi.IsDir() { return full } diff --git a/core/orm/config_param.go b/core/orm/config_param.go index dc1484e3..f7de6041 100644 --- a/core/orm/config_param.go +++ b/core/orm/config_param.go @@ -5,46 +5,14 @@ import ( "strings" ) -const configParamModel = "sys.config.parameter" - // GetConfigParam returns the value for key, or defaultVal when missing or empty. func GetConfigParam(ctx context.Context, key, defaultVal string) string { - key = strings.TrimSpace(key) - if key == "" { - return defaultVal - } - row, err := SearchOne(ctx, configParamModel, map[string]interface{}{"key": key}) - if err != nil { - return defaultVal - } - val := strings.TrimSpace(AsString(row["value"])) - if val == "" { - return defaultVal - } - return val + return GetConfig(ctx, key, defaultVal) } // SetConfigParam upserts a sys.config.parameter row by key. func SetConfigParam(ctx context.Context, key, value string) error { - key = strings.TrimSpace(key) - if key == "" { - return nil - } - bypass := ContextWithBypass(ctx, true) - existing, err := Search(bypass, configParamModel, [][]interface{}{{"key", "=", key}}) - if err != nil { - return err - } - if len(existing) > 0 { - id, _ := CoerceInt64(existing[0]["id"]) - return UpdateRecordByID(bypass, configParamModel, int(id), map[string]interface{}{"value": value}) - } - m, ok := Registry[configParamModel] - if !ok { - return nil - } - _, err = Create(bypass, m, map[string]interface{}{"key": key, "value": value}) - return err + return SetConfig(ctx, key, value) } // ConfigParamBool parses a config parameter as boolean (true/1/t/yes). diff --git a/core/orm/registry.go b/core/orm/registry.go index 50c67c98..4cd3366d 100644 --- a/core/orm/registry.go +++ b/core/orm/registry.go @@ -57,7 +57,7 @@ func SyncModels() error { } else if !ShouldMaterializeModel(name, installed) { continue } - if err := createTable(model); err != nil { + if err := createTable(ctx, model); err != nil { return err } } @@ -108,7 +108,7 @@ func ColumnTypeSQL(f FieldDefinition) (string, bool) { } } -func createTable(model Model) error { +func createTable(ctx context.Context, model Model) error { physical, err := ModelToTableName(model.ModelName()) if err != nil { return err @@ -117,7 +117,7 @@ func createTable(model Model) error { if err != nil { return err } - exists, err := tableExists(physical) + exists, err := tableExists(ctx, physical) if err != nil { return err } @@ -156,7 +156,7 @@ func createTable(model Model) error { } query := fmt.Sprintf("CREATE TABLE IF NOT EXISTS %s (%s);", tableName, strings.Join(columns, ", ")) - if _, err := DB.Exec(query); err != nil { + if _, err := DB.ExecContext(ctx, query); err != nil { return err } return ensureModelIndexes(schemaTable{ModelName: model.ModelName(), TableName: physical, QuotedTable: tableName, Model: model}) diff --git a/core/orm/schema_indexes.go b/core/orm/schema_indexes.go index 43f5835a..772ea1ea 100644 --- a/core/orm/schema_indexes.go +++ b/core/orm/schema_indexes.go @@ -1,6 +1,9 @@ package orm -import "fmt" +import ( + "context" + "fmt" +) // ensureExtraIndexes creates composite indexes not expressible via single-field Index flags. func ensureExtraIndexes() error { @@ -14,11 +17,12 @@ func ensureExtraIndexes() error { } func ensureMailMessageListIndex() error { + ctx := context.Background() tablePhysical := MustModelToTableName("mail.message") if tablePhysical == "" { return nil } - ok, err := tableExists(tablePhysical) + ok, err := tableExists(ctx, tablePhysical) if err != nil || !ok { return err } @@ -38,16 +42,17 @@ func ensureMailMessageListIndex() error { idxName := "idx_" + tablePhysical + "_model_core_created" q := fmt.Sprintf("CREATE INDEX IF NOT EXISTS %s ON %s (%s, %s, %s DESC)", quoteIdent(idxName), tableQuoted, modelCol, coreCol, dateCol) - _, err = DB.Exec(q) + _, err = DB.ExecContext(ctx, q) return err } func ensureSysTranslationUniqueIndex() error { + ctx := context.Background() tablePhysical := MustModelToTableName("sys.translation") if tablePhysical == "" { return nil } - ok, err := tableExists(tablePhysical) + ok, err := tableExists(ctx, tablePhysical) if err != nil || !ok { return err } @@ -67,6 +72,6 @@ func ensureSysTranslationUniqueIndex() error { idxName := "sys_translation_lang_src_module_uidx" q := fmt.Sprintf("CREATE UNIQUE INDEX IF NOT EXISTS %s ON %s (%s, %s, %s)", quoteIdent(idxName), tableQuoted, langCol, srcCol, moduleCol) - _, err = DB.Exec(q) + _, err = DB.ExecContext(ctx, q) return err } diff --git a/core/orm/schema_sync.go b/core/orm/schema_sync.go index 16b10e2d..1fb30522 100644 --- a/core/orm/schema_sync.go +++ b/core/orm/schema_sync.go @@ -64,12 +64,12 @@ func syncModelSchema(ctx context.Context, model Model) error { if err != nil { return err } - exists, err := tableExists(tableName) + exists, err := tableExists(ctx, tableName) if err != nil { return err } if !exists { - return createTable(model) + return createTable(ctx, model) } existing, err := loadTableColumns(tableName) if err != nil { @@ -92,7 +92,7 @@ func syncModelSchema(ctx context.Context, model Model) error { return err } q := fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s %s", quotedTable, colQuoted, colDef) - if _, err := DB.Exec(q); err != nil { + if _, err := DB.ExecContext(ctx, q); err != nil { return fmt.Errorf("%s: %w", q, err) } applog.L(ctx).Info("schema_sync", "table", tableName, "field", field.Name) @@ -296,12 +296,12 @@ func EnsureModelColumns(ctx context.Context, model Model, extra []FieldDefinitio if err != nil { return err } - exists, err := tableExists(tableName) + exists, err := tableExists(ctx, tableName) if err != nil { return err } if !exists { - return createTable(model) + return createTable(ctx, model) } existing, err := loadTableColumns(tableName) if err != nil { @@ -324,7 +324,7 @@ func EnsureModelColumns(ctx context.Context, model Model, extra []FieldDefinitio return err } q := fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s %s", quotedTable, colQuoted, colDef) - if _, err := DB.Exec(q); err != nil { + if _, err := DB.ExecContext(ctx, q); err != nil { return fmt.Errorf("%s: %w", q, err) } applog.L(ctx).Info("schema_sync_extra", "table", tableName, "field", field.Name) diff --git a/core/orm/schema_util.go b/core/orm/schema_util.go index a14e78a1..72d2449c 100644 --- a/core/orm/schema_util.go +++ b/core/orm/schema_util.go @@ -1,17 +1,18 @@ package orm import ( + "context" "fmt" "strings" ) // tableExists reports whether a physical table exists in the public schema. -func tableExists(tableName string) (bool, error) { +func tableExists(ctx context.Context, tableName string) (bool, error) { if DB == nil { return false, nil } var count int - err := DB.QueryRow(` + err := DB.QueryRowContext(ctx, ` SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = 'public' AND table_name = $1 `, tableName).Scan(&count) diff --git a/core/orm/setup_sync.go b/core/orm/setup_sync.go index fadf624d..143e781e 100644 --- a/core/orm/setup_sync.go +++ b/core/orm/setup_sync.go @@ -40,12 +40,13 @@ var InitialSetupModelNames = []string{ // SyncModelsInitialSetup creates tables only for InitialSetupModelNames (first-run /setup). func SyncModelsInitialSetup() error { + ctx := ContextWithBypass(context.Background(), true) for _, name := range InitialSetupModelNames { m, ok := Registry[name] if !ok { return fmt.Errorf("initial setup: model %q is not registered (build must include sumeru/addons/base)", name) } - if err := createTable(m); err != nil { + if err := createTable(ctx, m); err != nil { return fmt.Errorf("create table %s: %w", name, err) } } diff --git a/core/queue/inproc.go b/core/queue/inproc.go index 9b99dc95..75758fed 100644 --- a/core/queue/inproc.go +++ b/core/queue/inproc.go @@ -5,6 +5,9 @@ import ( "context" "encoding/json" "sync" + + "sumeru/core/applog" + "sumeru/core/metrics" ) // Message is a topic-tagged payload for async workers. @@ -41,7 +44,17 @@ func Publish(ctx context.Context, topic string, payload interface{}) { for _, fn := range subs { fn := fn go func() { - _ = fn(ctx, msg) + if err := fn(ctx, msg); err != nil { + metrics.Inc("sumeru_queue_handler_errors_total") + applog.Warn(ctx, applog.Event{ + Message: "queue handler failed", + Component: "queue", + Operation: "publish", + Status: "failed", + Context: map[string]interface{}{"topic": topic}, + Err: err, + }) + } }() } publishRedisMirror(ctx, topic, data) diff --git a/core/server/config/config.go b/core/server/config/config.go index 0244a329..45a383c1 100644 --- a/core/server/config/config.go +++ b/core/server/config/config.go @@ -44,6 +44,7 @@ type Config struct { DbConnMaxLifetimeMin int // db_conn_max_lifetime_minutes; 0 = no limit DbReadReplicaDSN string // optional libpq DSN for read replica (search/read_group RPC) RateLimitRPM int // rate_limit_rpm per client IP on /api/rpc and login; 0 = disabled + TrustedProxies string // trusted_proxies: comma-separated CIDRs/IPs allowed to set X-Forwarded-For; empty = never trust XFF SMTPHost string SMTPPort int SMTPUser string @@ -174,6 +175,8 @@ func LoadConfig(path string) error { if n, err := strconv.Atoi(strings.TrimSpace(val)); err == nil { AppConfig.RateLimitRPM = n } + case keyTrustedProxies: + AppConfig.TrustedProxies = val case keySMTPHost: AppConfig.SMTPHost = val case keySMTPPort: diff --git a/core/server/config/const.go b/core/server/config/const.go index 95e56927..6e08c644 100644 --- a/core/server/config/const.go +++ b/core/server/config/const.go @@ -42,6 +42,7 @@ const ( keyDbConnMaxLifetimeMin = "db_conn_max_lifetime_minutes" keyDbReadReplicaDSN = "db_read_replica_dsn" keyRateLimitRPM = "rate_limit_rpm" + keyTrustedProxies = "trusted_proxies" keySMTPHost = "smtp_host" keySMTPPort = "smtp_port" keySMTPUser = "smtp_user" diff --git a/core/server/run.go b/core/server/run.go index 5d5a3aea..eb6d0112 100644 --- a/core/server/run.go +++ b/core/server/run.go @@ -6,7 +6,9 @@ import ( "fmt" "net/http" "os" + "os/signal" "strings" + "syscall" "time" "sumeru/core/applog" @@ -135,7 +137,10 @@ func Run() { applog.InfoMsg(ctx, "server", "listen", "Server starting in setup mode", map[string]interface{}{"port": config.AppConfig.HttpPort, "bind": listenHost}) setupHandler := router.ApplyMiddleware(web.SecurityMiddleware(nil)) - if err := http.ListenAndServe(listenHost, setupHandler); err != nil { + srv := newHTTPServer(listenHost, setupHandler) + setupCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + if err := serveUntilSignal(setupCtx, srv); err != nil { applog.Fatal(ctx, "Server failed in setup mode", "err", err) } return @@ -165,23 +170,56 @@ func Run() { if err := sdk.RunStartups(ctx); err != nil { applog.Fatal(ctx, "Startup hooks failed", "err", err) } - scheduler.Start(context.Background(), time.Minute) - orm.StartOutboxDrain(context.Background(), 5*time.Second) + rootCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + scheduler.Start(rootCtx, time.Minute) + orm.StartOutboxDrain(rootCtx, 5*time.Second) listenHost := listenAddr(config.AppConfig.HttpInterface, config.AppConfig.HttpPort) applog.InfoMsg(ctx, "server", "listen", "Server starting", map[string]interface{}{"port": config.AppConfig.HttpPort, "bind": listenHost}) runtime.SyncFromGlobals() appHandler := router.ApplyMiddleware(web.SecurityMiddleware(nil)) - srv := &http.Server{ - Addr: listenHost, - Handler: appHandler, - ReadTimeout: 30 * time.Second, - WriteTimeout: 60 * time.Second, - } - if err := srv.ListenAndServe(); err != nil { + srv := newHTTPServer(listenHost, appHandler) + if err := serveUntilSignal(rootCtx, srv); err != nil { applog.Fatal(ctx, "Server failed", "err", err) } + scheduler.Stop() + orm.StopOutboxDrain() +} + +func newHTTPServer(addr string, handler http.Handler) *http.Server { + return &http.Server{ + Addr: addr, + Handler: handler, + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + WriteTimeout: 60 * time.Second, + IdleTimeout: 120 * time.Second, + } +} + +func serveUntilSignal(ctx context.Context, srv *http.Server) error { + errCh := make(chan error, 1) + go func() { + errCh <- srv.ListenAndServe() + }() + select { + case <-ctx.Done(): + shutdownCtx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + _ = srv.Shutdown(shutdownCtx) + err := <-errCh + if err == nil || err == http.ErrServerClosed { + return nil + } + return err + case err := <-errCh: + if err == http.ErrServerClosed { + return nil + } + return err + } } func registerAppRoutes() { diff --git a/core/server/web/action_resolve.go b/core/server/web/action_resolve.go index 9bfa82dc..e683cb2d 100644 --- a/core/server/web/action_resolve.go +++ b/core/server/web/action_resolve.go @@ -5,7 +5,9 @@ import ( "fmt" "strings" + "sumeru/core/engine/render" "sumeru/core/orm" + "sumeru/core/server/config" ) type navActionKind int @@ -37,6 +39,13 @@ func resolveNavigationAction(ctx context.Context, actionID int, actionQuery stri if url == "" { return navigationAction{}, fmt.Errorf("action %d has empty url", coreID) } + safe := render.SafeIframeURL(url) + if config.AppConfig.DevMode { + safe = render.SafeIframeURLAllowHTTP(url) + } + if !safe { + return navigationAction{}, fmt.Errorf("action %d has unsafe url", coreID) + } return navigationAction{kind: navActionURL, url: url}, nil case sysActionWindowModel: row, err := loadWindowAction(ctx, coreID) diff --git a/core/server/web/apikey_create.go b/core/server/web/apikey_create.go index f5d64d10..bb3357fb 100644 --- a/core/server/web/apikey_create.go +++ b/core/server/web/apikey_create.go @@ -39,10 +39,15 @@ func ActionCreateAPIKey(w http.ResponseWriter, r *http.Request) { } // apiKeyTargetUserID resolves which user receives the new key; falls back to the session user. +// Targeting another user requires base.group_system. func apiKeyTargetUserID(r *http.Request) int { + sessionUID := SessionUserID(r) userID, _ := strconv.Atoi(strings.TrimSpace(r.PostFormValue("user_id"))) - if userID <= 0 { - return SessionUserID(r) + if userID <= 0 || userID == sessionUID { + return sessionUID } - return userID + if orm.UserHasGroupXML(r.Context(), sessionUID, groupSystemXML) { + return userID + } + return sessionUID } diff --git a/core/server/web/auth.go b/core/server/web/auth.go index 11e6b84b..c1b80eb3 100644 --- a/core/server/web/auth.go +++ b/core/server/web/auth.go @@ -180,11 +180,14 @@ func LogoutGet(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, loginRoute, http.StatusFound) } -// ActionResetPassword accepts a reset request from an authenticated user (email delivery not yet wired). +// ActionResetPassword accepts a reset request from a system administrator (email delivery not yet wired). func ActionResetPassword(w http.ResponseWriter, r *http.Request) { if !requireLoginAndPOST(w, r) { return } + if !requireSystemAdmin(w, r, false) { + return + } userID := strings.TrimSpace(r.PostFormValue(resetUserIDField)) loginName := strings.TrimSpace(r.PostFormValue(loginField)) diff --git a/core/server/web/auth_helpers.go b/core/server/web/auth_helpers.go index 6d0f206b..c46c0fc0 100644 --- a/core/server/web/auth_helpers.go +++ b/core/server/web/auth_helpers.go @@ -9,8 +9,10 @@ import ( ) func writeJSON(w http.ResponseWriter, ctx context.Context, route string, v interface{}) { - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(v); err != nil { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + enc := json.NewEncoder(w) + enc.SetEscapeHTML(false) + if err := enc.Encode(v); err != nil && ctx != nil && route != "" { WebLogEvent(ctx, WebLogInput{ Route: route, Message: "Failed to encode JSON response", Operation: "write", Status: "partial", Err: err, @@ -19,10 +21,14 @@ func writeJSON(w http.ResponseWriter, ctx context.Context, route string, v inter } func writeJSONOK(w http.ResponseWriter) { - w.Header().Set("Content-Type", "application/json") + w.Header().Set("Content-Type", "application/json; charset=utf-8") _, _ = w.Write([]byte(`{"ok":true}`)) } +func writeJSONResponse(w http.ResponseWriter, v interface{}) { + writeJSON(w, nil, "", v) +} + // resolveRequestUID returns SecurityUID from context, falling back to the session cookie. func resolveRequestUID(r *http.Request) int { uid := orm.SecurityUID(r.Context()) diff --git a/core/server/web/csrf.go b/core/server/web/csrf.go index db7fd638..4fc26542 100644 --- a/core/server/web/csrf.go +++ b/core/server/web/csrf.go @@ -32,10 +32,9 @@ func csrfKey() []byte { if len(csrfSecret) == 0 { b := make([]byte, 32) if _, err := rand.Read(b); err != nil { - csrfSecret = []byte("sumeru-dev-csrf-fallback") - } else { - csrfSecret = b + panic("csrf: crypto/rand failed: " + err.Error()) } + csrfSecret = b } return csrfSecret } diff --git a/core/server/web/setup_security.go b/core/server/web/setup_security.go index 3ad8b0b0..704ff195 100644 --- a/core/server/web/setup_security.go +++ b/core/server/web/setup_security.go @@ -1,6 +1,7 @@ package web import ( + "fmt" "net" "net/http" "strings" @@ -90,13 +91,19 @@ func pruneSetupAttempts(attempts []time.Time, now time.Time) []time.Time { } func clientIP(r *http.Request) string { - if forwardedFor := strings.TrimSpace(r.Header.Get(forwardedForHeader)); forwardedFor != "" { - if commaIndex := strings.Index(forwardedFor, ","); commaIndex >= 0 { - return strings.TrimSpace(forwardedFor[:commaIndex]) + remote := remoteAddrIP(r) + if remote != "" && trustedProxyContains(remote) { + if forwardedFor := strings.TrimSpace(r.Header.Get(forwardedForHeader)); forwardedFor != "" { + if commaIndex := strings.Index(forwardedFor, ","); commaIndex >= 0 { + return strings.TrimSpace(forwardedFor[:commaIndex]) + } + return forwardedFor } - return forwardedFor } + return remote +} +func remoteAddrIP(r *http.Request) string { host, _, err := net.SplitHostPort(r.RemoteAddr) if err != nil { return strings.TrimSpace(r.RemoteAddr) @@ -104,6 +111,48 @@ func clientIP(r *http.Request) string { return host } +func trustedProxyContains(ipAddress string) bool { + parsedIP := net.ParseIP(strings.TrimSpace(ipAddress)) + if parsedIP == nil { + return false + } + for _, network := range trustedProxyNetworks() { + if network.Contains(parsedIP) { + return true + } + } + return false +} + +func trustedProxyNetworks() []*net.IPNet { + raw := strings.TrimSpace(config.AppConfig.TrustedProxies) + if raw == "" { + return nil + } + var networks []*net.IPNet + for _, part := range strings.Split(raw, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + if !strings.Contains(part, "/") { + if ip := net.ParseIP(part); ip != nil { + bits := 32 + if ip.To4() == nil { + bits = 128 + } + part = fmt.Sprintf("%s/%d", part, bits) + } + } + _, network, err := net.ParseCIDR(part) + if err != nil { + continue + } + networks = append(networks, network) + } + return networks +} + func isLoopbackIP(ipAddress string) bool { parsedIP := net.ParseIP(strings.TrimSpace(ipAddress)) return parsedIP != nil && parsedIP.IsLoopback() diff --git a/core/server/web/swc_bus_hub.go b/core/server/web/swc_bus_hub.go index 552e5e4e..7bba43c8 100644 --- a/core/server/web/swc_bus_hub.go +++ b/core/server/web/swc_bus_hub.go @@ -3,7 +3,10 @@ package web import ( "context" "encoding/json" + "net" "net/http" + "net/url" + "strings" "sync" "github.com/gorilla/websocket" @@ -13,12 +16,29 @@ import ( var ( swcBusUpgrader = websocket.Upgrader{ - CheckOrigin: func(r *http.Request) bool { return true }, + CheckOrigin: checkSwcBusOrigin, } globalBusHub *busHub globalBusHubOnce sync.Once ) +func checkSwcBusOrigin(r *http.Request) bool { + origin := strings.TrimSpace(r.Header.Get("Origin")) + if origin == "" { + return true + } + u, err := url.Parse(origin) + if err != nil { + return false + } + reqHost := r.Host + if h, _, err := net.SplitHostPort(reqHost); err == nil { + reqHost = h + } + originHost := u.Hostname() + return strings.EqualFold(originHost, reqHost) +} + type swcBusClient struct { uid int conn *websocket.Conn diff --git a/core/server/web/swc_workspace.go b/core/server/web/swc_workspace.go index e74020ba..a133ac26 100644 --- a/core/server/web/swc_workspace.go +++ b/core/server/web/swc_workspace.go @@ -1,7 +1,6 @@ package web import ( - "encoding/json" "net/http" "strings" ) @@ -74,10 +73,3 @@ func SwcWorkspaceHandler(w http.ResponseWriter, r *http.Request) { payload := buildSwcWorkspacePayload(ctx, resolved, req, viewRecord, actionData) writeJSONResponse(w, payload) } - -func writeJSONResponse(w http.ResponseWriter, v interface{}) { - w.Header().Set("Content-Type", "application/json; charset=utf-8") - enc := json.NewEncoder(w) - enc.SetEscapeHTML(false) - _ = enc.Encode(v) -} diff --git a/core/server/web/web_request_helpers.go b/core/server/web/web_request_helpers.go index c37e6705..70c04115 100644 --- a/core/server/web/web_request_helpers.go +++ b/core/server/web/web_request_helpers.go @@ -119,6 +119,8 @@ func RequirePOST(w http.ResponseWriter, r *http.Request) bool { } func ParsePostForm(w http.ResponseWriter, r *http.Request) bool { + const maxFormBytes = 1 << 20 // 1 MiB + r.Body = http.MaxBytesReader(w, r.Body, maxFormBytes) if err := r.ParseForm(); err != nil { http.Error(w, invalidFormMessage, http.StatusBadRequest) return false diff --git a/core/server/web/workspace_resolve.go b/core/server/web/workspace_resolve.go index 1ddb8bde..53f92a4a 100644 --- a/core/server/web/workspace_resolve.go +++ b/core/server/web/workspace_resolve.go @@ -62,30 +62,15 @@ func menuRecordActionID(menuRecord map[string]interface{}) (actionID int, ok boo // firstDescendantWindowActionID returns the first non-zero action_id in a depth-first walk // of children ordered by sequence, then id. func firstDescendantWindowActionID(ctx context.Context, parentMenuID int) int { - menuTable := orm.MustQuotedTableName(sysMenuModel) - rows, err := orm.DB.QueryContext(ctx, - `SELECT id, action_id FROM `+menuTable+` WHERE parent_id = $1 ORDER BY sequence ASC, id ASC`, - parentMenuID, - ) - if err != nil { - return 0 - } - defer rows.Close() - - for rows.Next() { - var childMenuID int - var childActionID sql.NullInt64 - if err := rows.Scan(&childMenuID, &childActionID); err != nil { - continue - } - if childActionID.Valid && childActionID.Int64 != 0 { - return int(childActionID.Int64) + return walkMenuChildren(ctx, parentMenuID, func(childMenuID int, childActionID int) (int, bool) { + if childActionID != 0 { + return childActionID, true } if descendantActionID := firstDescendantWindowActionID(ctx, childMenuID); descendantActionID != 0 { - return descendantActionID + return descendantActionID, true } - } - return 0 + return 0, false + }) } func menuIDPointsToAppLogs(ctx context.Context, menuQuery string) bool { @@ -169,6 +154,19 @@ func menuIDForWindowAction(ctx context.Context, parentMenuID, actionID int) int if parentMenuID <= 0 || actionID == 0 { return 0 } + return walkMenuChildren(ctx, parentMenuID, func(childMenuID int, childActionID int) (int, bool) { + if childActionID == actionID { + return childMenuID, true + } + if found := menuIDForWindowAction(ctx, childMenuID, actionID); found > 0 { + return found, true + } + return 0, false + }) +} + +// walkMenuChildren DFS-walks sys.menu children (sequence, id) and returns the first match from visit. +func walkMenuChildren(ctx context.Context, parentMenuID int, visit func(childMenuID, childActionID int) (int, bool)) int { menuTable := orm.MustQuotedTableName(sysMenuModel) rows, err := orm.DB.QueryContext(ctx, `SELECT id, action_id FROM `+menuTable+` WHERE parent_id = $1 ORDER BY sequence ASC, id ASC`, @@ -185,11 +183,12 @@ func menuIDForWindowAction(ctx context.Context, parentMenuID, actionID int) int if err := rows.Scan(&childMenuID, &childActionID); err != nil { continue } - if childActionID.Valid && int(childActionID.Int64) == actionID { - return childMenuID + aid := 0 + if childActionID.Valid { + aid = int(childActionID.Int64) } - if found := menuIDForWindowAction(ctx, childMenuID, actionID); found > 0 { - return found + if result, ok := visit(childMenuID, aid); ok { + return result } } return 0 diff --git a/sumeru.conf.example b/sumeru.conf.example index 7a7b79a8..f680957d 100644 --- a/sumeru.conf.example +++ b/sumeru.conf.example @@ -54,6 +54,10 @@ log_rolling = false # Rate limiting (optional; requests per minute per client IP on /api/rpc and /web/login) # rate_limit_rpm = 120 +# Comma-separated CIDRs/IPs of reverse proxies allowed to set X-Forwarded-For. +# Empty (default) = never trust XFF; client IP is always RemoteAddr. +# trusted_proxies = 10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,127.0.0.1 + # SMTP (optional — password reset and notifications) # smtp_host = localhost # smtp_port = 587 diff --git a/test/addons/automation/actions_test.go b/test/addons/automation/actions_test.go index a13e0528..b5d91403 100644 --- a/test/addons/automation/actions_test.go +++ b/test/addons/automation/actions_test.go @@ -2,10 +2,8 @@ package automation_test import ( "context" - "io" "net/http" "net/http/httptest" - "strings" "testing" "sumeru/addons/automation" @@ -58,10 +56,7 @@ func TestExecuteServerActionModelFilter(t *testing.T) { } func TestExecuteServerActionWebhook(t *testing.T) { - var gotBody string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, _ := io.ReadAll(r.Body) - gotBody = string(body) w.WriteHeader(http.StatusOK) })) defer srv.Close() @@ -71,10 +66,8 @@ func TestExecuteServerActionWebhook(t *testing.T) { "code": "webhook:" + srv.URL, } ev := event.Event{Name: "record.created", Payload: map[string]interface{}{"model": "crm.lead", "id": 1}} - if err := automation.ExecuteServerActionForTest(context.Background(), row, ev); err != nil { - t.Fatal(err) - } - if gotBody == "" || !strings.Contains(gotBody, "record.created") { - t.Fatalf("expected webhook body with event, got %q", gotBody) + err := automation.ExecuteServerActionForTest(context.Background(), row, ev) + if err == nil { + t.Fatal("expected loopback webhook URL to be rejected") } } diff --git a/test/addons/automation/webhook_test.go b/test/addons/automation/webhook_test.go new file mode 100644 index 00000000..7703b79a --- /dev/null +++ b/test/addons/automation/webhook_test.go @@ -0,0 +1,33 @@ +package automation_test + +import ( + "sumeru/addons/automation" + "testing" +) + +func TestValidateWebhookURL(t *testing.T) { + t.Parallel() + cases := []struct { + url string + wantErr bool + }{ + {"", true}, + {"javascript:alert(1)", true}, + {"ftp://example.com/x", true}, + {"http://127.0.0.1/hook", true}, + {"https://localhost/hook", true}, + {"http://169.254.169.254/latest/meta-data", true}, + {"https://192.168.1.1/hook", true}, + {"https://10.0.0.5/hook", true}, + {"https://8.8.8.8/hook", false}, + } + for _, tc := range cases { + err := automation.ValidateWebhookURLForTest(tc.url) + if tc.wantErr && err == nil { + t.Errorf("%q: want error", tc.url) + } + if !tc.wantErr && err != nil { + t.Errorf("%q: unexpected error: %v", tc.url, err) + } + } +} diff --git a/test/core/engine/render/pure_functions_test.go b/test/core/engine/render/pure_functions_test.go index 6cb1d5a0..c974067f 100644 --- a/test/core/engine/render/pure_functions_test.go +++ b/test/core/engine/render/pure_functions_test.go @@ -62,6 +62,30 @@ func TestSafeImageSrc_table(t *testing.T) { } } +func TestSafeIframeURL_table(t *testing.T) { + t.Parallel() + tests := []struct { + src string + want bool + }{ + {"https://reports.example/x", true}, + {"/web/report/1", true}, + {"http://reports.example/x", false}, + {"javascript:alert(1)", false}, + {"data:text/html,hi", false}, + {"//evil.example", false}, + {"", false}, + } + for _, tc := range tests { + if got := render.SafeIframeURL(tc.src); got != tc.want { + t.Errorf("SafeIframeURL(%q) = %v want %v", tc.src, got, tc.want) + } + } + if !render.SafeIframeURLAllowHTTP("http://reports.example/x") { + t.Fatal("SafeIframeURLAllowHTTP should allow http") + } +} + func TestFieldDisplayLabel_table(t *testing.T) { t.Parallel() tests := []struct { @@ -106,6 +130,12 @@ func TestModuleIconServePath_emptyWithoutAddon(t *testing.T) { if got := render.ModuleIconServePath("nonexistent_module_xyz", "static/icon.png"); got != "" { t.Fatalf("missing addon should return empty: %q", got) } + if got := render.ModuleIconServePath("nonexistent_module_xyz", "/etc/passwd"); got != "" { + t.Fatalf("absolute path must be rejected, got %q", got) + } + if got := render.ModuleIconServePath("nonexistent_module_xyz", "../../../etc/passwd"); got != "" { + t.Fatalf("traversal path must be rejected, got %q", got) + } } func TestAppsViewTabs_andHomeViewTabs(t *testing.T) { diff --git a/test/core/server/web/action_resolve_test.go b/test/core/server/web/action_resolve_test.go index e708e5ca..4f7ac5ad 100644 --- a/test/core/server/web/action_resolve_test.go +++ b/test/core/server/web/action_resolve_test.go @@ -63,6 +63,25 @@ func TestResolveNavigationActionURLRedirect(t *testing.T) { } } +func TestResolveNavigationActionURLRejectsUnsafe(t *testing.T) { + mock := setupURLActionWebTest(t) + metaRows := sqlmock.NewRows([]string{"id", "module", "name", "model", "core_id"}). + AddRow(1, "account", "x", "sys.action.url", 5) + mock.ExpectQuery(`SELECT \* FROM "sys_model_data" WHERE \("core_id" = \$1 AND "model" IN \(\$2,\$3\)\)`). + WithArgs(5, "sys.action.window", "sys.action.url"). + WillReturnRows(metaRows) + urlRows := sqlmock.NewRows([]string{"id", "name", "url"}). + AddRow(5, "Bad", "javascript:alert(1)") + mock.ExpectQuery(`SELECT \* FROM "sys_action_url" WHERE \("id" = \$1\) LIMIT 1`). + WithArgs(5). + WillReturnRows(urlRows) + + _, _, err := web.ResolveNavigationActionForTest(bypassCtx(), 5, "") + if err == nil { + t.Fatal("expected unsafe url error") + } +} + func TestResolveNavigationActionWindowUnchanged(t *testing.T) { mock := setupURLActionWebTest(t) orm.RegisterStubModelForTest(t, "sys.action.window", []orm.FieldDefinition{ diff --git a/test/core/server/web/apikey_create_test.go b/test/core/server/web/apikey_create_test.go index 3d2233c1..8671fafc 100644 --- a/test/core/server/web/apikey_create_test.go +++ b/test/core/server/web/apikey_create_test.go @@ -8,14 +8,15 @@ import ( ) func TestApiKeyTargetUserID(t *testing.T) { - t.Run("uses form user_id when positive", func(t *testing.T) { + t.Run("falls back to session when targeting other user without admin", func(t *testing.T) { req := httptest.NewRequest("POST", "/web/action/create_api_key", strings.NewReader("user_id=42")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") if err := req.ParseForm(); err != nil { t.Fatal(err) } - if got := web.APIKeyTargetUserID(req); got != 42 { - t.Fatalf("got %d, want 42", got) + // No session → session UID is 0; non-admin cannot mint for user 42. + if got := web.APIKeyTargetUserID(req); got != 0 { + t.Fatalf("got %d, want 0 without system group", got) } }) diff --git a/test/core/server/web/setup_security_test.go b/test/core/server/web/setup_security_test.go index 02ef6b8b..f65c402d 100644 --- a/test/core/server/web/setup_security_test.go +++ b/test/core/server/web/setup_security_test.go @@ -2,16 +2,35 @@ package web_test import ( "net/http/httptest" + "sumeru/core/server/config" "sumeru/core/server/web" "testing" "time" ) -func TestClientIPFromForwardedHeader(t *testing.T) { +func TestClientIPIgnoresForwardedWithoutTrustedProxy(t *testing.T) { + prev := config.AppConfig.TrustedProxies + config.AppConfig.TrustedProxies = "" + t.Cleanup(func() { config.AppConfig.TrustedProxies = prev }) + + req := httptest.NewRequest("POST", web.TestSetupInitRoute, nil) + req.RemoteAddr = "198.51.100.10:443" + req.Header.Set(web.TestForwardedForHeader, "127.0.0.1") + if got := web.ClientIP(req); got != "198.51.100.10" { + t.Fatalf("got %q want RemoteAddr host when proxies untrusted", got) + } +} + +func TestClientIPTrustsForwardedFromTrustedProxy(t *testing.T) { + prev := config.AppConfig.TrustedProxies + config.AppConfig.TrustedProxies = "127.0.0.1/32" + t.Cleanup(func() { config.AppConfig.TrustedProxies = prev }) + req := httptest.NewRequest("POST", web.TestSetupInitRoute, nil) + req.RemoteAddr = "127.0.0.1:12345" req.Header.Set(web.TestForwardedForHeader, "203.0.113.1, 198.51.100.2") if got := web.ClientIP(req); got != "203.0.113.1" { - t.Fatalf("got %q want first forwarded IP", got) + t.Fatalf("got %q want first forwarded IP from trusted proxy", got) } } From 9caacb972b312b32ca889d2ba0929cc022480e23 Mon Sep 17 00:00:00 2001 From: AIRONAX Developer Date: Sat, 5 Sep 2026 02:47:54 +0530 Subject: [PATCH 2/2] fix(security): finish schema ctx, setup token gate, and trust metrics Thread DDL through SyncRegistrySchemaContext, require setup_token when setup is not localhost-only, add webhook/CSRF/shutdown counters, and consolidate float coercion under orm.CoerceFloat64. --- addons/automation/webhook.go | 2 + core/orm/crud_coerce.go | 37 +++++++++++ core/orm/domain_json.go | 19 +----- core/orm/registry.go | 2 +- core/orm/schema_fk.go | 2 +- core/orm/schema_indexes.go | 12 ++-- core/orm/schema_sync.go | 44 +++++++------ core/orm/setup_sync.go | 2 +- core/server/api/parse.go | 14 +---- core/server/config/config.go | 4 +- core/server/run.go | 2 + core/server/web/setup_security.go | 5 ++ core/server/web/testexports.go | 6 ++ core/server/web/web_request_helpers.go | 2 + sumeru.conf.example | 3 +- test/core/orm/coerce_float_test.go | 33 ++++++++++ test/core/server/web/form_ws_security_test.go | 63 +++++++++++++++++++ test/core/server/web/setup_security_test.go | 59 +++++++++++++++++ 18 files changed, 247 insertions(+), 64 deletions(-) create mode 100644 test/core/orm/coerce_float_test.go create mode 100644 test/core/server/web/form_ws_security_test.go diff --git a/addons/automation/webhook.go b/addons/automation/webhook.go index 9f387e16..179264da 100644 --- a/addons/automation/webhook.go +++ b/addons/automation/webhook.go @@ -13,10 +13,12 @@ import ( "sumeru/core/applog" "sumeru/core/event" + "sumeru/core/metrics" ) func dispatchWebhook(ctx context.Context, rawURL string, ev event.Event) error { if err := validateWebhookURL(rawURL); err != nil { + metrics.Inc("sumeru_webhook_blocked_total") applog.Warn(ctx, applog.Event{ Message: "webhook URL rejected", Component: "automation", diff --git a/core/orm/crud_coerce.go b/core/orm/crud_coerce.go index 0d52f3ce..3cf821e3 100644 --- a/core/orm/crud_coerce.go +++ b/core/orm/crud_coerce.go @@ -1,10 +1,47 @@ package orm import ( + "encoding/json" "fmt" + "strconv" "strings" ) +// CoerceFloat64 reads numeric values (JSON, DB drivers) into float64. +func CoerceFloat64(v interface{}) (float64, bool) { + switch t := v.(type) { + case float64: + return t, true + case float32: + return float64(t), true + case int: + return float64(t), true + case int64: + return float64(t), true + case int32: + return float64(t), true + case json.Number: + f, err := t.Float64() + return f, err == nil + case string: + s := strings.TrimSpace(t) + if s == "" { + return 0, false + } + f, err := strconv.ParseFloat(s, 64) + return f, err == nil + case []byte: + s := strings.TrimSpace(string(t)) + if s == "" { + return 0, false + } + f, err := strconv.ParseFloat(s, 64) + return f, err == nil + default: + return 0, false + } +} + // CoerceInt64 reads numeric values from database drivers into int64. func CoerceInt64(v interface{}) (int64, bool) { switch t := v.(type) { diff --git a/core/orm/domain_json.go b/core/orm/domain_json.go index 42039f31..88811792 100644 --- a/core/orm/domain_json.go +++ b/core/orm/domain_json.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "strconv" "strings" ) @@ -233,21 +232,5 @@ func AsBool(v interface{}) bool { } func toFloat64(v interface{}) (float64, bool) { - switch t := v.(type) { - case float64: - return t, true - case float32: - return float64(t), true - case int: - return float64(t), true - case int64: - return float64(t), true - default: - s := strings.TrimSpace(AsString(v)) - if s == "" { - return 0, false - } - f, err := strconv.ParseFloat(s, 64) - return f, err == nil - } + return CoerceFloat64(v) } diff --git a/core/orm/registry.go b/core/orm/registry.go index 4cd3366d..d881ba61 100644 --- a/core/orm/registry.go +++ b/core/orm/registry.go @@ -159,5 +159,5 @@ func createTable(ctx context.Context, model Model) error { if _, err := DB.ExecContext(ctx, query); err != nil { return err } - return ensureModelIndexes(schemaTable{ModelName: model.ModelName(), TableName: physical, QuotedTable: tableName, Model: model}) + return ensureModelIndexes(ctx, schemaTable{ModelName: model.ModelName(), TableName: physical, QuotedTable: tableName, Model: model}) } diff --git a/core/orm/schema_fk.go b/core/orm/schema_fk.go index b1c17833..73adcdd8 100644 --- a/core/orm/schema_fk.go +++ b/core/orm/schema_fk.go @@ -35,7 +35,7 @@ func ensureForeignKeys(ctx context.Context, tbl schemaTable) error { `ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (id) ON DELETE %s NOT VALID`, tbl.QuotedTable, quoteIdent(constraintName), colQuoted, targetQuoted, onDelete, ) - if _, err := DB.Exec(q); err != nil { + if _, err := DB.ExecContext(ctx, q); err != nil { if strings.Contains(strings.ToLower(err.Error()), "already exists") { continue } diff --git a/core/orm/schema_indexes.go b/core/orm/schema_indexes.go index 772ea1ea..dc0e811b 100644 --- a/core/orm/schema_indexes.go +++ b/core/orm/schema_indexes.go @@ -6,18 +6,17 @@ import ( ) // ensureExtraIndexes creates composite indexes not expressible via single-field Index flags. -func ensureExtraIndexes() error { +func ensureExtraIndexes(ctx context.Context) error { if DB == nil { return nil } - if err := ensureMailMessageListIndex(); err != nil { + if err := ensureMailMessageListIndex(ctx); err != nil { return err } - return ensureSysTranslationUniqueIndex() + return ensureSysTranslationUniqueIndex(ctx) } -func ensureMailMessageListIndex() error { - ctx := context.Background() +func ensureMailMessageListIndex(ctx context.Context) error { tablePhysical := MustModelToTableName("mail.message") if tablePhysical == "" { return nil @@ -46,8 +45,7 @@ func ensureMailMessageListIndex() error { return err } -func ensureSysTranslationUniqueIndex() error { - ctx := context.Background() +func ensureSysTranslationUniqueIndex(ctx context.Context) error { tablePhysical := MustModelToTableName("sys.translation") if tablePhysical == "" { return nil diff --git a/core/orm/schema_sync.go b/core/orm/schema_sync.go index 1fb30522..4539dd9e 100644 --- a/core/orm/schema_sync.go +++ b/core/orm/schema_sync.go @@ -17,10 +17,14 @@ import ( // SyncRegistrySchema adds missing columns and indexes for every model in Registry. func SyncRegistrySchema() error { + return SyncRegistrySchemaContext(ContextWithBypass(context.Background(), true)) +} + +// SyncRegistrySchemaContext is SyncRegistrySchema using the caller's context (typically with bypass). +func SyncRegistrySchemaContext(ctx context.Context) error { if DB == nil { return nil } - ctx := ContextWithBypass(context.Background(), true) installed, err := InstalledModuleNames(ctx) if err != nil { return err @@ -43,7 +47,7 @@ func SyncRegistrySchema() error { return fmt.Errorf("schema sync %s: %w", name, err) } } - return ensureExtraIndexes() + return ensureExtraIndexes(ctx) } // schemaTable identifies a registered model's physical PostgreSQL table for DDL helpers. @@ -71,7 +75,7 @@ func syncModelSchema(ctx context.Context, model Model) error { if !exists { return createTable(ctx, model) } - existing, err := loadTableColumns(tableName) + existing, err := loadTableColumns(ctx, tableName) if err != nil { return err } @@ -98,16 +102,16 @@ func syncModelSchema(ctx context.Context, model Model) error { applog.L(ctx).Info("schema_sync", "table", tableName, "field", field.Name) } tbl := schemaTable{ModelName: modelName, TableName: tableName, QuotedTable: quotedTable, Model: model} - if err := dropStaleColumnUniques(tbl); err != nil { + if err := dropStaleColumnUniques(ctx, tbl); err != nil { return err } - if err := dropRuntimeSQLDefaults(modelName, quotedTable, model); err != nil { + if err := dropRuntimeSQLDefaults(ctx, modelName, quotedTable, model); err != nil { return err } - if err := ensureColumnUniques(tbl); err != nil { + if err := ensureColumnUniques(ctx, tbl); err != nil { return err } - if err := ensureModelIndexes(tbl); err != nil { + if err := ensureModelIndexes(ctx, tbl); err != nil { return err } return ensureForeignKeys(ctx, tbl) @@ -115,7 +119,7 @@ func syncModelSchema(ctx context.Context, model Model) error { // dropStaleColumnUniques removes single-column UNIQUE constraints when the field // definition no longer sets Unique (e.g. sys.menu.name after menu label collisions). -func dropStaleColumnUniques(tbl schemaTable) error { +func dropStaleColumnUniques(ctx context.Context, tbl schemaTable) error { for _, field := range tbl.Model.Fields() { if field.Unique || field.Name == "id" { continue @@ -124,7 +128,7 @@ func dropStaleColumnUniques(tbl schemaTable) error { if !ok || baseType == "" { continue } - rows, err := DB.Query(` + rows, err := DB.QueryContext(ctx, ` SELECT c.conname FROM pg_constraint c JOIN pg_class t ON c.conrelid = t.oid @@ -163,10 +167,10 @@ func dropStaleColumnUniques(tbl schemaTable) error { return fmt.Errorf("unsafe constraint name %q on %s", con, tbl.TableName) } q := fmt.Sprintf(`ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s`, tbl.QuotedTable, quoteIdent(con)) - if _, err := DB.Exec(q); err != nil { + if _, err := DB.ExecContext(ctx, q); err != nil { return fmt.Errorf("drop unique %s.%s: %w", tbl.TableName, con, err) } - applog.L(context.Background()).Info("schema_sync_drop_unique", "table", tbl.TableName, "constraint", con) + applog.L(ctx).Info("schema_sync_drop_unique", "table", tbl.TableName, "constraint", con) } } return nil @@ -174,7 +178,7 @@ func dropStaleColumnUniques(tbl schemaTable) error { // dropRuntimeSQLDefaults removes SQL DEFAULT literals for tokens applied in Go at insert time // (uuid, current_user, current_company). Older schema sync stored those tokens as string defaults. -func dropRuntimeSQLDefaults(modelName, quotedTable string, model Model) error { +func dropRuntimeSQLDefaults(ctx context.Context, modelName, quotedTable string, model Model) error { for _, field := range model.Fields() { if !isRuntimeDefaultToken(field.DefaultVal) || field.Name == "id" || IsVirtualField(field) { continue @@ -184,7 +188,7 @@ func dropRuntimeSQLDefaults(modelName, quotedTable string, model Model) error { return err } q := fmt.Sprintf("ALTER TABLE %s ALTER COLUMN %s DROP DEFAULT", quotedTable, colQuoted) - if _, err := DB.Exec(q); err != nil { + if _, err := DB.ExecContext(ctx, q); err != nil { return fmt.Errorf("drop default %s.%s: %w", modelName, field.Name, err) } } @@ -194,7 +198,7 @@ func dropRuntimeSQLDefaults(modelName, quotedTable string, model Model) error { // ensureColumnUniques adds a unique index for each Unique field on an existing table. // createTable applies UNIQUE only at CREATE time; later tag changes would otherwise // leave XML upsert (ON CONFLICT) without a matching constraint. -func ensureColumnUniques(tbl schemaTable) error { +func ensureColumnUniques(ctx context.Context, tbl schemaTable) error { for _, field := range tbl.Model.Fields() { if !field.Unique || field.Name == "id" || IsVirtualField(field) { continue @@ -212,14 +216,14 @@ func ensureColumnUniques(tbl schemaTable) error { return fmt.Errorf("unsafe unique index name %q on %s", idxName, tbl.TableName) } q := fmt.Sprintf("CREATE UNIQUE INDEX IF NOT EXISTS %s ON %s (%s)", quoteIdent(idxName), tbl.QuotedTable, colQuoted) - if _, err := DB.Exec(q + ";"); err != nil { + if _, err := DB.ExecContext(ctx, q+";"); err != nil { return fmt.Errorf("unique index %s: %w", idxName, err) } } return nil } -func ensureModelIndexes(tbl schemaTable) error { +func ensureModelIndexes(ctx context.Context, tbl schemaTable) error { for _, field := range tbl.Model.Fields() { if IsVirtualField(field) { continue @@ -233,7 +237,7 @@ func ensureModelIndexes(tbl schemaTable) error { } idxName := fmt.Sprintf("idx_%s_%s", tbl.TableName, field.Name) idxQuery := fmt.Sprintf("CREATE INDEX IF NOT EXISTS %s ON %s (%s)", quoteIdent(idxName), tbl.QuotedTable, colQuoted) - if _, err := DB.Exec(idxQuery + ";"); err != nil { + if _, err := DB.ExecContext(ctx, idxQuery+";"); err != nil { return fmt.Errorf("index %s: %w", idxName, err) } } @@ -257,8 +261,8 @@ func pgIdentOK(name string) bool { return true } -func loadTableColumns(tableName string) (map[string]struct{}, error) { - rows, err := DB.Query(` +func loadTableColumns(ctx context.Context, tableName string) (map[string]struct{}, error) { + rows, err := DB.QueryContext(ctx, ` SELECT column_name FROM information_schema.columns WHERE table_schema = 'public' AND table_name = $1 `, tableName) @@ -303,7 +307,7 @@ func EnsureModelColumns(ctx context.Context, model Model, extra []FieldDefinitio if !exists { return createTable(ctx, model) } - existing, err := loadTableColumns(tableName) + existing, err := loadTableColumns(ctx, tableName) if err != nil { return err } diff --git a/core/orm/setup_sync.go b/core/orm/setup_sync.go index 143e781e..baca2450 100644 --- a/core/orm/setup_sync.go +++ b/core/orm/setup_sync.go @@ -75,7 +75,7 @@ func SyncRegistrySchemaForNames(modelNames []string) error { return fmt.Errorf("schema sync %s: %w", name, err) } } - return ensureExtraIndexes() + return ensureExtraIndexes(ctx) } // ModelsForModuleSchemaSync returns (names, true) when install should only touch those models; diff --git a/core/server/api/parse.go b/core/server/api/parse.go index 23a983c8..7b28fc19 100644 --- a/core/server/api/parse.go +++ b/core/server/api/parse.go @@ -47,19 +47,7 @@ func parseLimitOffset(kwargs json.RawMessage) (limit int, offset int) { } func toFloat(v interface{}) (float64, bool) { - switch t := v.(type) { - case float64: - return t, true - case int: - return float64(t), true - case int64: - return float64(t), true - case json.Number: - f, err := t.Float64() - return f, err == nil - default: - return 0, false - } + return orm.CoerceFloat64(v) } func parseArgsArray(args json.RawMessage) ([]json.RawMessage, error) { diff --git a/core/server/config/config.go b/core/server/config/config.go index 45a383c1..9ed0c814 100644 --- a/core/server/config/config.go +++ b/core/server/config/config.go @@ -37,8 +37,8 @@ type Config struct { LogTimezone string // log_timezone: UTC, Local (default), or IANA (e.g. Asia/Kolkata) for timestamps DevMode bool // dev_mode INI key; parseBoolKey(..., false) — debug slog level and dev-only server paths DevFeatures string // dev_features INI: comma-separated sql, access, xml - SetupToken string // optional secret required for POST /setup/init (header X-Setup-Token or JSON setup_token) - SetupLocalhostOnly bool // when true (default), setup mode listens on 127.0.0.1 only + SetupToken string // secret for POST /setup/init; required when setup_localhost_only is false + SetupLocalhostOnly bool // when true (default), setup mode listens on 127.0.0.1 only; false requires setup_token DbMaxOpenConns int // db_max_open_conns; 0 = Go default DbMaxIdleConns int // db_max_idle_conns; 0 = Go default DbConnMaxLifetimeMin int // db_conn_max_lifetime_minutes; 0 = no limit diff --git a/core/server/run.go b/core/server/run.go index eb6d0112..d241c698 100644 --- a/core/server/run.go +++ b/core/server/run.go @@ -13,6 +13,7 @@ import ( "sumeru/core/applog" _ "sumeru/core/ormmodels" + "sumeru/core/metrics" "sumeru/core/modelreg" "sumeru/core/module" "sumeru/core/orm" @@ -206,6 +207,7 @@ func serveUntilSignal(ctx context.Context, srv *http.Server) error { }() select { case <-ctx.Done(): + metrics.Inc("sumeru_shutdown_started_total") shutdownCtx, cancel := context.WithTimeout(context.Background(), 20*time.Second) defer cancel() _ = srv.Shutdown(shutdownCtx) diff --git a/core/server/web/setup_security.go b/core/server/web/setup_security.go index 704ff195..eff67d79 100644 --- a/core/server/web/setup_security.go +++ b/core/server/web/setup_security.go @@ -47,6 +47,11 @@ func requireSetupEnvironment(w http.ResponseWriter, r *http.Request) bool { func validateSetupToken(w http.ResponseWriter, r *http.Request, tokenFromBody string) bool { expectedToken := strings.TrimSpace(config.AppConfig.SetupToken) if expectedToken == "" { + // When setup is reachable beyond loopback, a shared secret is mandatory. + if !config.AppConfig.SetupLocalhostOnly { + http.Error(w, "Setup token required when setup is not localhost-only", http.StatusForbidden) + return false + } return true } providedToken := setupTokenFromRequest(r, tokenFromBody) diff --git a/core/server/web/testexports.go b/core/server/web/testexports.go index e6171d42..527a1b4c 100644 --- a/core/server/web/testexports.go +++ b/core/server/web/testexports.go @@ -162,6 +162,12 @@ func AllowSetupRateLimit(w http.ResponseWriter, requestIP string) bool { return allowSetupRateLimit(w, requestIP) } +func ValidateSetupToken(w http.ResponseWriter, r *http.Request, tokenFromBody string) bool { + return validateSetupToken(w, r, tokenFromBody) +} + +func CheckSwcBusOrigin(r *http.Request) bool { return checkSwcBusOrigin(r) } + func ResetSetupRateLimiterForTest() { setupRateLimiter.attemptsByIP = make(map[string][]time.Time) } func ParseSetupInitRequest(w http.ResponseWriter, body []byte) (SetupInitRequest, bool) { diff --git a/core/server/web/web_request_helpers.go b/core/server/web/web_request_helpers.go index 70c04115..abd93358 100644 --- a/core/server/web/web_request_helpers.go +++ b/core/server/web/web_request_helpers.go @@ -8,6 +8,7 @@ import ( "strings" "sumeru/core/applog" + "sumeru/core/metrics" "sumeru/core/orm" ) @@ -164,6 +165,7 @@ func validateSessionCSRF(w http.ResponseWriter, r *http.Request) bool { if ValidateCSRF(r) { return true } + metrics.Inc("sumeru_csrf_rejected_total") http.Error(w, invalidCSRFMessage, http.StatusForbidden) return false } diff --git a/sumeru.conf.example b/sumeru.conf.example index f680957d..41df536f 100644 --- a/sumeru.conf.example +++ b/sumeru.conf.example @@ -16,7 +16,8 @@ http_port = 8080 # http_interface = 127.0.0.1 dev_mode = true -# Setup wizard — first run only; leave setup_token empty to skip +# Setup wizard — first run only. +# When setup_localhost_only = false, setup_token must be non-empty. setup_localhost_only = true setup_token = diff --git a/test/core/orm/coerce_float_test.go b/test/core/orm/coerce_float_test.go new file mode 100644 index 00000000..dd62d8b1 --- /dev/null +++ b/test/core/orm/coerce_float_test.go @@ -0,0 +1,33 @@ +package orm_test + +import ( + "encoding/json" + "testing" + + "sumeru/core/orm" +) + +func TestCoerceFloat64(t *testing.T) { + t.Parallel() + cases := []struct { + in interface{} + want float64 + ok bool + }{ + {float64(1.5), 1.5, true}, + {float32(2), 2, true}, + {int(3), 3, true}, + {int64(4), 4, true}, + {json.Number("2.5"), 2.5, true}, + {"3.25", 3.25, true}, + {[]byte("4.5"), 4.5, true}, + {"", 0, false}, + {true, 0, false}, + } + for _, tc := range cases { + got, ok := orm.CoerceFloat64(tc.in) + if ok != tc.ok || (ok && got != tc.want) { + t.Errorf("CoerceFloat64(%v)=(%v,%v) want (%v,%v)", tc.in, got, ok, tc.want, tc.ok) + } + } +} diff --git a/test/core/server/web/form_ws_security_test.go b/test/core/server/web/form_ws_security_test.go new file mode 100644 index 00000000..e0ec9fb1 --- /dev/null +++ b/test/core/server/web/form_ws_security_test.go @@ -0,0 +1,63 @@ +package web_test + +import ( + "bytes" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "sumeru/core/server/web" +) + +func TestParsePostFormRejectsOversizedBody(t *testing.T) { + // 1 MiB limit + 1 byte + body := bytes.Repeat([]byte("a"), (1<<20)+1) + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + if web.ParsePostForm(rec, req) { + t.Fatal("oversized form body should be rejected") + } + if rec.Code != http.StatusBadRequest { + t.Fatalf("status %d want %d", rec.Code, http.StatusBadRequest) + } +} + +func TestParsePostFormAcceptsSmallBody(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("a=1")) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + if !web.ParsePostForm(rec, req) { + t.Fatal("small form body should parse") + } + if req.Form.Get("a") != "1" { + t.Fatalf("form a=%q want 1", req.Form.Get("a")) + } +} + +func TestCheckSwcBusOriginRejectsCrossOrigin(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "http://app.example/ws", nil) + req.Host = "app.example" + req.Header.Set("Origin", "https://evil.example") + if web.CheckSwcBusOrigin(req) { + t.Fatal("cross-origin WS upgrade should be rejected") + } +} + +func TestCheckSwcBusOriginAllowsSameOrigin(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "http://app.example/ws", nil) + req.Host = "app.example" + req.Header.Set("Origin", "http://app.example") + if !web.CheckSwcBusOrigin(req) { + t.Fatal("same-origin WS should be allowed") + } +} + +func TestCheckSwcBusOriginAllowsMissingOrigin(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "http://app.example/ws", nil) + req.Host = "app.example" + if !web.CheckSwcBusOrigin(req) { + t.Fatal("missing Origin should be allowed (non-browser clients)") + } +} diff --git a/test/core/server/web/setup_security_test.go b/test/core/server/web/setup_security_test.go index f65c402d..1ada9bbe 100644 --- a/test/core/server/web/setup_security_test.go +++ b/test/core/server/web/setup_security_test.go @@ -1,6 +1,7 @@ package web_test import ( + "net/http" "net/http/httptest" "sumeru/core/server/config" "sumeru/core/server/web" @@ -84,3 +85,61 @@ func TestAllowSetupRateLimit(t *testing.T) { t.Fatal("attempt over limit should be rejected") } } + +func TestValidateSetupTokenRequiresWhenNotLocalhostOnly(t *testing.T) { + prevToken := config.AppConfig.SetupToken + prevLocal := config.AppConfig.SetupLocalhostOnly + config.AppConfig.SetupToken = "" + config.AppConfig.SetupLocalhostOnly = false + t.Cleanup(func() { + config.AppConfig.SetupToken = prevToken + config.AppConfig.SetupLocalhostOnly = prevLocal + }) + + req := httptest.NewRequest("POST", web.TestSetupInitRoute, nil) + rec := httptest.NewRecorder() + if web.ValidateSetupToken(rec, req, "") { + t.Fatal("empty setup_token must be rejected when setup_localhost_only is false") + } + if rec.Code != http.StatusForbidden { + t.Fatalf("status %d want %d", rec.Code, http.StatusForbidden) + } +} + +func TestValidateSetupTokenAllowsEmptyWhenLocalhostOnly(t *testing.T) { + prevToken := config.AppConfig.SetupToken + prevLocal := config.AppConfig.SetupLocalhostOnly + config.AppConfig.SetupToken = "" + config.AppConfig.SetupLocalhostOnly = true + t.Cleanup(func() { + config.AppConfig.SetupToken = prevToken + config.AppConfig.SetupLocalhostOnly = prevLocal + }) + + req := httptest.NewRequest("POST", web.TestSetupInitRoute, nil) + rec := httptest.NewRecorder() + if !web.ValidateSetupToken(rec, req, "") { + t.Fatal("empty setup_token should be allowed when setup_localhost_only is true") + } +} + +func TestValidateSetupTokenMatchesConfigured(t *testing.T) { + prevToken := config.AppConfig.SetupToken + prevLocal := config.AppConfig.SetupLocalhostOnly + config.AppConfig.SetupToken = "secret" + config.AppConfig.SetupLocalhostOnly = false + t.Cleanup(func() { + config.AppConfig.SetupToken = prevToken + config.AppConfig.SetupLocalhostOnly = prevLocal + }) + + req := httptest.NewRequest("POST", web.TestSetupInitRoute, nil) + rec := httptest.NewRecorder() + if web.ValidateSetupToken(rec, req, "wrong") { + t.Fatal("wrong token should be rejected") + } + rec = httptest.NewRecorder() + if !web.ValidateSetupToken(rec, req, "secret") { + t.Fatal("matching body token should be accepted") + } +}