From 1123d06b7e370bdc28ec9afd638ab389809d1e20 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 10 Jul 2023 16:18:24 +0100 Subject: [PATCH 1/7] chore: enable exhaustruct linter --- .golangci.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.golangci.yaml b/.golangci.yaml index 1c2b2b2e851ca..0686360be69c7 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -195,6 +195,7 @@ issues: # We use assertions rather than explicitly checking errors in tests - errcheck - forcetypeassert + - exhaustruct # This is unhelpful in tests. fix: true max-issues-per-linter: 0 @@ -219,6 +220,7 @@ linters: - errcheck - errname - errorlint + - exhaustruct - exportloopref - forcetypeassert - gocritic From 9c35bedea478f1f9df757960c2ea898d1275e0b2 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 11 Jul 2023 09:43:03 +0100 Subject: [PATCH 2/7] add exlusion rules --- .golangci.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.golangci.yaml b/.golangci.yaml index 0686360be69c7..9160771b01fbc 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -2,6 +2,14 @@ # Over time we should try tightening some of these. linters-settings: + exhaustruct: + exclude: + - 'clibase\.Cmd' + - 'clibase\.Option' + - 'codersdk\.Response' + - 'codersdk\.\w+Request' + - 'http\.Client' + - "TypescriptType" gocognit: min-complexity: 46 # Min code complexity (def 30). @@ -196,6 +204,9 @@ issues: - errcheck - forcetypeassert - exhaustruct # This is unhelpful in tests. + - path: scripts/* + linters: + - exhaustruct fix: true max-issues-per-linter: 0 From c2c8ff21d19b4d151744c55e6253fdc426fbacad Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 11 Jul 2023 11:06:10 +0100 Subject: [PATCH 3/7] move to allowlist instead --- .golangci.yaml | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 9160771b01fbc..e3f3797d06b81 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -3,13 +3,9 @@ linters-settings: exhaustruct: - exclude: - - 'clibase\.Cmd' - - 'clibase\.Option' - - 'codersdk\.Response' - - 'codersdk\.\w+Request' - - 'http\.Client' - - "TypescriptType" + include: + # Gradually extend to cover more of the codebase. + - 'httpmw\.\w+' gocognit: min-complexity: 46 # Min code complexity (def 30). From bf2ab5e3c8c6055de745e5e8295506b5508580a7 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 11 Jul 2023 11:19:49 +0100 Subject: [PATCH 4/7] exhaustruct httpmw package --- coderd/coderd.go | 3 +++ coderd/httpmw/hsts.go | 2 +- coderd/httpmw/realip.go | 10 ++++++++-- enterprise/coderd/coderd.go | 2 ++ site/site.go | 8 +++++--- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 79d58565b53d9..797be73c34e0b 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -390,6 +390,7 @@ func New(options *Options) *API { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: false, + SessionTokenFunc: nil, // Default behaviour }) // Same as above but it redirects to the login page. apiKeyMiddlewareRedirect := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ @@ -398,6 +399,7 @@ func New(options *Options) *API { RedirectToLogin: true, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: false, + SessionTokenFunc: nil, // Default behaviour }) // Same as the first but it's optional. apiKeyMiddlewareOptional := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ @@ -406,6 +408,7 @@ func New(options *Options) *API { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: true, + SessionTokenFunc: nil, // Default behaviour }) // API rate limit middleware. The counter is local and not shared between diff --git a/coderd/httpmw/hsts.go b/coderd/httpmw/hsts.go index 52cfb79bbc403..fa72c83889d66 100644 --- a/coderd/httpmw/hsts.go +++ b/coderd/httpmw/hsts.go @@ -20,7 +20,7 @@ type HSTSConfig struct { func HSTSConfigOptions(maxAge int, options []string) (HSTSConfig, error) { if maxAge <= 0 { // No header, so no need to build the header string. - return HSTSConfig{}, nil + return HSTSConfig{HeaderValue: ""}, nil } // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security diff --git a/coderd/httpmw/realip.go b/coderd/httpmw/realip.go index ee708680e03cd..032e4c41ff2a1 100644 --- a/coderd/httpmw/realip.go +++ b/coderd/httpmw/realip.go @@ -56,7 +56,10 @@ func ExtractRealIP(config *RealIPConfig) func(next http.Handler) http.Handler { // configuration and headers. It does not mutate the original request. func ExtractRealIPAddress(config *RealIPConfig, req *http.Request) (net.IP, error) { if config == nil { - config = &RealIPConfig{} + config = &RealIPConfig{ + TrustedOrigins: []*net.IPNet{}, + TrustedHeaders: []string{}, + } } cf := isContainedIn(config.TrustedOrigins, getRemoteAddress(req.RemoteAddr)) @@ -208,7 +211,10 @@ func RealIP(ctx context.Context) *RealIPState { // ParseRealIPConfig takes a raw string array of headers and origins // to produce a config. func ParseRealIPConfig(headers, origins []string) (*RealIPConfig, error) { - config := &RealIPConfig{} + config := &RealIPConfig{ + TrustedOrigins: []*net.IPNet{}, + TrustedHeaders: []string{}, + } for _, origin := range origins { _, network, err := net.ParseCIDR(origin) if err != nil { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 75476b508eaad..aa61cb04b7869 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -88,6 +88,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: false, + SessionTokenFunc: nil, // Default behaviour }) apiKeyMiddlewareOptional := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: options.Database, @@ -95,6 +96,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: true, + SessionTokenFunc: nil, // Default behaviour }) deploymentID, err := options.Database.GetDeploymentID(ctx) diff --git a/site/site.go b/site/site.go index 98ec8000fb761..d267b7c21bf02 100644 --- a/site/site.go +++ b/site/site.go @@ -291,12 +291,14 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f // Cookies are sent when requesting HTML, so we can get the user // and pre-populate the state for the frontend to reduce requests. apiKey, actor, _ := httpmw.ExtractAPIKey(rw, r, httpmw.ExtractAPIKeyConfig{ - Optional: true, - DB: h.opts.Database, - OAuth2Configs: h.opts.OAuth2Configs, + DB: h.opts.Database, + OAuth2Configs: h.opts.OAuth2Configs, + RedirectToLogin: false, // Special case for site, we can always disable refresh here because // the frontend will perform API requests if this fails. DisableSessionExpiryRefresh: true, + Optional: true, + SessionTokenFunc: nil, }) if apiKey != nil && actor != nil { ctx := dbauthz.As(r.Context(), actor.Actor) From acc1a6358481fd0ec4d43a26053aab83a3fdc090 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 11 Jul 2023 11:22:53 +0100 Subject: [PATCH 5/7] fixup! exhaustruct httpmw package --- site/site.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/site/site.go b/site/site.go index d267b7c21bf02..6519e539e1458 100644 --- a/site/site.go +++ b/site/site.go @@ -291,13 +291,13 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f // Cookies are sent when requesting HTML, so we can get the user // and pre-populate the state for the frontend to reduce requests. apiKey, actor, _ := httpmw.ExtractAPIKey(rw, r, httpmw.ExtractAPIKeyConfig{ - DB: h.opts.Database, - OAuth2Configs: h.opts.OAuth2Configs, - RedirectToLogin: false, + Optional: true, + DB: h.opts.Database, + OAuth2Configs: h.opts.OAuth2Configs, // Special case for site, we can always disable refresh here because // the frontend will perform API requests if this fails. DisableSessionExpiryRefresh: true, - Optional: true, + RedirectToLogin: false, SessionTokenFunc: nil, }) if apiKey != nil && actor != nil { From 60139866b53ede0108a8513d93798bc357526edf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 11 Jul 2023 12:37:22 +0100 Subject: [PATCH 6/7] make lint --- coderd/coderd.go | 6 +++--- coderd/httpmw/realip.go | 5 ++++- enterprise/coderd/coderd.go | 4 ++-- install.sh | 2 ++ 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 797be73c34e0b..dc14727879f08 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -390,7 +390,7 @@ func New(options *Options) *API { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: false, - SessionTokenFunc: nil, // Default behaviour + SessionTokenFunc: nil, // Default behavior }) // Same as above but it redirects to the login page. apiKeyMiddlewareRedirect := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ @@ -399,7 +399,7 @@ func New(options *Options) *API { RedirectToLogin: true, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: false, - SessionTokenFunc: nil, // Default behaviour + SessionTokenFunc: nil, // Default behavior }) // Same as the first but it's optional. apiKeyMiddlewareOptional := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ @@ -408,7 +408,7 @@ func New(options *Options) *API { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: true, - SessionTokenFunc: nil, // Default behaviour + SessionTokenFunc: nil, // Default behavior }) // API rate limit middleware. The counter is local and not shared between diff --git a/coderd/httpmw/realip.go b/coderd/httpmw/realip.go index 032e4c41ff2a1..ef4f5693d9de1 100644 --- a/coderd/httpmw/realip.go +++ b/coderd/httpmw/realip.go @@ -84,7 +84,10 @@ func ExtractRealIPAddress(config *RealIPConfig, req *http.Request) (net.IP, erro // of each proxy header is set. func FilterUntrustedOriginHeaders(config *RealIPConfig, req *http.Request) { if config == nil { - config = &RealIPConfig{} + config = &RealIPConfig{ + TrustedOrigins: []*net.IPNet{}, + TrustedHeaders: []string{}, + } } cf := isContainedIn(config.TrustedOrigins, getRemoteAddress(req.RemoteAddr)) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index aa61cb04b7869..1576950b97b61 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -88,7 +88,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: false, - SessionTokenFunc: nil, // Default behaviour + SessionTokenFunc: nil, // Default behavior }) apiKeyMiddlewareOptional := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: options.Database, @@ -96,7 +96,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { RedirectToLogin: false, DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(), Optional: true, - SessionTokenFunc: nil, // Default behaviour + SessionTokenFunc: nil, // Default behavior }) deploymentID, err := options.Database.GetDeploymentID(ctx) diff --git a/install.sh b/install.sh index 3c0b7b2781e36..32f8799ddecbf 100755 --- a/install.sh +++ b/install.sh @@ -527,6 +527,7 @@ distro() { if [ -f /etc/os-release ]; then ( + # shellcheck disable=SC1091 . /etc/os-release if [ "${ID_LIKE-}" ]; then for id_like in $ID_LIKE; do @@ -553,6 +554,7 @@ distro_name() { if [ -f /etc/os-release ]; then ( + # shellcheck disable=SC1091 . /etc/os-release echo "$PRETTY_NAME" ) From 5270d5b5b693cbe26219730faa00a5afbc33f680 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 11 Jul 2023 13:20:32 +0100 Subject: [PATCH 7/7] address PR comments --- coderd/httpmw/realip.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/httpmw/realip.go b/coderd/httpmw/realip.go index ef4f5693d9de1..e7cc679ba95c0 100644 --- a/coderd/httpmw/realip.go +++ b/coderd/httpmw/realip.go @@ -57,8 +57,8 @@ func ExtractRealIP(config *RealIPConfig) func(next http.Handler) http.Handler { func ExtractRealIPAddress(config *RealIPConfig, req *http.Request) (net.IP, error) { if config == nil { config = &RealIPConfig{ - TrustedOrigins: []*net.IPNet{}, - TrustedHeaders: []string{}, + TrustedOrigins: nil, + TrustedHeaders: nil, } } @@ -85,8 +85,8 @@ func ExtractRealIPAddress(config *RealIPConfig, req *http.Request) (net.IP, erro func FilterUntrustedOriginHeaders(config *RealIPConfig, req *http.Request) { if config == nil { config = &RealIPConfig{ - TrustedOrigins: []*net.IPNet{}, - TrustedHeaders: []string{}, + TrustedOrigins: nil, + TrustedHeaders: nil, } }