From d7758a91c4ae40f9f21957ad64139686219e1fad Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 28 Jul 2026 21:46:59 +0100 Subject: [PATCH] fix: gosec enabled, fixed slowloris on the http.Server, no-timeout serve, webui path traversal, overly permissive perms on themes dir --- service/.golangci.yml | 69 +++++++++++++++++++++ service/internal/httpservers/frontend.go | 9 ++- service/internal/httpservers/prometheus.go | 16 ++++- service/internal/httpservers/webuiServer.go | 14 ++--- 4 files changed, 96 insertions(+), 12 deletions(-) diff --git a/service/.golangci.yml b/service/.golangci.yml index fc14403..6719989 100644 --- a/service/.golangci.yml +++ b/service/.golangci.yml @@ -10,6 +10,7 @@ linters: - errcheck - gocritic - gocyclo + - gosec - ineffassign - misspell - staticcheck @@ -18,6 +19,74 @@ linters: settings: gocyclo: min-complexity: 5 + gosec: + # Full gosec rule set (G101–G6xx), including Slowloris checks G112/G114. + enable-all-rules: true exclusions: paths: - gen + rules: + # Noise / fixtures in tests and local tooling. + - path: _test\.go + linters: + - gosec + - path: scripts/ + linters: + - gosec + - path: cmd/ + linters: + - gosec + + # OliveTin's purpose is controlled command execution from config. + - path: internal/executor/ + text: "G204:" + linters: + - gosec + + # Operator-configured filesystem paths (entity files, touch/write helpers, persisted logs). + - path: internal/entities/ + text: "G304:" + linters: + - gosec + - path: internal/filehelper/ + text: "G304:" + linters: + - gosec + - path: internal/configcheck/ + text: "G304:" + linters: + - gosec + - path: internal/executor/ + text: "G304:" + linters: + - gosec + - path: internal/auth/otjwt/ + text: "G304:" + linters: + - gosec + - path: internal/httpservers/ + text: "G304:" + linters: + - gosec + + # Legacy GitHub webhook HMAC-SHA1 is still a supported authType. + - path: internal/webhooks/auth\.go + text: "G505:" + linters: + - gosec + + # InsecureSkipVerify is an explicit OAuth2 provider config option. + - path: internal/auth/otoauth2/ + text: "G402:" + linters: + - gosec + + # Secure is set dynamically from TLS / ForceSecureCookies; gosec wants a literal true. + - text: "G124:" + linters: + - gosec + + # Protobuf / process exit codes mapped into int32 fields. + - text: "G115:" + linters: + - gosec diff --git a/service/internal/httpservers/frontend.go b/service/internal/httpservers/frontend.go index e317b18..2d0da21 100644 --- a/service/internal/httpservers/frontend.go +++ b/service/internal/httpservers/frontend.go @@ -14,6 +14,7 @@ import ( "net/url" "path" "strings" + "time" "github.com/OliveTin/OliveTin/internal/api" "github.com/OliveTin/OliveTin/internal/auth" @@ -153,8 +154,12 @@ func StartFrontendMux(cfg *config.Config, ex *executor.Executor) { } srv := &http.Server{ - Addr: cfg.ListenAddressSingleHTTPFrontend, - Handler: securityHeadersMiddleware(cfg, mux), + Addr: cfg.ListenAddressSingleHTTPFrontend, + Handler: securityHeadersMiddleware(cfg, mux), + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, + // WriteTimeout intentionally unset: EventStream and StartActionAndWait need long-lived writes. } log.Fatal(srv.ListenAndServe()) diff --git a/service/internal/httpservers/prometheus.go b/service/internal/httpservers/prometheus.go index 868e113..75c7107 100644 --- a/service/internal/httpservers/prometheus.go +++ b/service/internal/httpservers/prometheus.go @@ -2,6 +2,7 @@ package httpservers import ( "net/http" + "time" config "github.com/OliveTin/OliveTin/internal/config" "github.com/prometheus/client_golang/prometheus" @@ -19,8 +20,19 @@ func StartPrometheus(cfg *config.Config) { prometheus.Unregister(collectors.NewGoCollector()) } - http.Handle("/", promhttp.Handler()) - err := http.ListenAndServe(cfg.ListenAddressPrometheus, nil) + mux := http.NewServeMux() + mux.Handle("/", promhttp.Handler()) + + srv := &http.Server{ + Addr: cfg.ListenAddressPrometheus, + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, + } + + err := srv.ListenAndServe() if err != nil { log.WithFields(log.Fields{ diff --git a/service/internal/httpservers/webuiServer.go b/service/internal/httpservers/webuiServer.go index 5b76c06..b43dc35 100644 --- a/service/internal/httpservers/webuiServer.go +++ b/service/internal/httpservers/webuiServer.go @@ -35,18 +35,16 @@ func NewWebUIServer(cfg *config.Config) *webUIServer { } func (s *webUIServer) handleWebui(w http.ResponseWriter, r *http.Request) { - // dirName := path.Dir(r.URL.Path) - // Mangle requests for any path like /logs or /config to load the webui index.html if path.Ext(r.URL.Path) == "" && r.URL.Path != "/" { log.Debugf("Mangling request for %s to /index.html", r.URL.Path) - http.ServeFile(w, r, path.Join(s.webuiDir, "index.html")) - } else { - log.Tracef("Serving webui from %s for %s", s.webuiDir, r.URL.Path) - http.ServeFile(w, r, path.Join(s.webuiDir, r.URL.Path)) - // http.StripPrefix(dirName, http.FileServer(http.Dir(s.webuiDir))).ServeHTTP(w, r) + return } + + log.Tracef("Serving webui from %s for %s", s.webuiDir, r.URL.Path) + // http.Dir rejects path traversal; do not Join raw URL paths into ServeFile. + http.FileServer(http.Dir(s.webuiDir)).ServeHTTP(w, r) } func (s *webUIServer) findWebuiDir() string { @@ -84,7 +82,7 @@ func (s *webUIServer) findCustomWebuiDir() string { func (s *webUIServer) setupCustomWebuiDir() { dir := s.findCustomWebuiDir() - err := os.MkdirAll(path.Join(dir, "themes/"), 0775) + err := os.MkdirAll(path.Join(dir, "themes/"), 0o750) if err != nil { log.Warnf("Could not create themes directory: %v", err)