Merge pull request #4 from StudioLambda/fix/high-security-vulnerabilities

Fix high-severity security vulnerabilities

Author: ConsoleTVs

contract/request/body.go

@@ -44,26 +44,26 @@ func LimitedBytes(r *http.Request, maxSize int64) ([]byte, error) {
 // Prefer [LimitedString] or apply [http.MaxBytesReader] in a
 // middleware to prevent memory exhaustion from oversized requests.
 func String(r *http.Request) (string, error) {
-	b, err := Bytes(r)
+	body, err := Bytes(r)
 
 	if err != nil {
 		return "", err
 	}
 
-	return string(b), nil
+	return string(body), nil
 }
 
 // LimitedString reads the request body up to maxSize bytes and
 // returns it as a string. If the body exceeds maxSize, the result
 // is truncated. Pass -1 to use [DefaultMaxBodySize].
 func LimitedString(r *http.Request, maxSize int64) (string, error) {
-	b, err := LimitedBytes(r, maxSize)
+	body, err := LimitedBytes(r, maxSize)
 
 	if err != nil {
 		return "", err
 	}
 
-	return string(b), nil
+	return string(body), nil
 }
 
 // JSON decodes JSON data from the request body into a value of type T.

contract/request/cookie.go

@@ -8,11 +8,11 @@ import "net/http"
 //
 // Parameters:
 //   - r: The HTTP request to search for the cookie
-//   - k: The name of the cookie to retrieve
+//   - name: The name of the cookie to retrieve
 //
 // Returns the cookie object or nil if not found.
-func Cookie(r *http.Request, k string) *http.Cookie {
-	if cookie, err := r.Cookie(k); err == nil {
+func Cookie(r *http.Request, name string) *http.Cookie {
+	if cookie, err := r.Cookie(name); err == nil {
 		return cookie
 	}
 
@@ -25,12 +25,12 @@ func Cookie(r *http.Request, k string) *http.Cookie {
 //
 // Parameters:
 //   - r: The HTTP request to search for the cookie
-//   - k: The name of the cookie whose value to retrieve
+//   - name: The name of the cookie whose value to retrieve
 //
 // Returns the cookie value as a string, or empty string if not found.
-func CookieValue(r *http.Request, k string) string {
-	if c := Cookie(r, k); c != nil {
-		return c.Value
+func CookieValue(r *http.Request, name string) string {
+	if cookie := Cookie(r, name); cookie != nil {
+		return cookie.Value
 	}
 
 	return ""
@@ -42,14 +42,14 @@ func CookieValue(r *http.Request, k string) string {
 //
 // Parameters:
 //   - r: The HTTP request to search for the cookie
-//   - k: The name of the cookie whose value to retrieve
-//   - d: The default value to return if the cookie is not found or empty
+//   - name: The name of the cookie whose value to retrieve
+//   - fallback: The default value to return if the cookie is not found or empty
 //
 // Returns the cookie value if found and non-empty, otherwise the default value.
-func CookieValueOr(r *http.Request, k string, d string) string {
-	if v := CookieValue(r, k); v != "" {
-		return v
+func CookieValueOr(r *http.Request, name string, fallback string) string {
+	if value := CookieValue(r, name); value != "" {
+		return value
 	}
 
-	return d
+	return fallback
 }

contract/request/header.go

@@ -17,12 +17,12 @@ func Header(r *http.Request, key string) string {
 
 // HeaderOr returns the first value for the given header key, falling
 // back to the provided default value if the header is missing or empty.
-func HeaderOr(r *http.Request, key string, def string) string {
-	if h := Header(r, key); h != "" {
-		return h
+func HeaderOr(r *http.Request, key string, fallback string) string {
+	if value := Header(r, key); value != "" {
+		return value
 	}
 
-	return def
+	return fallback
 }
 
 // HeaderValues returns all values associated with the given header

contract/request/hooks.go

@@ -18,10 +18,24 @@ var ErrNoHooksMiddleware = problem.Problem{
 // Hooks returns the hooks instance used to attach callbacks
 // to lifecycle events. It panics if the hooks middleware has
 // not been applied to the request's context.
+//
+// WARNING: This function panics when hooks are missing. Use
+// [TryHooks] for a non-panicking alternative, or ensure the
+// [framework.Recover] middleware is in place.
 func Hooks(r *http.Request) contract.Hooks {
 	if hooks, ok := r.Context().Value(contract.HooksKey).(contract.Hooks); ok {
 		return hooks
 	}
 
 	panic(ErrNoHooksMiddleware)
 }
+
+// TryHooks retrieves the hooks instance from the request
+// context without panicking. The boolean return value indicates
+// whether hooks were found. This is the safe alternative to
+// [Hooks] for use outside the framework handler chain.
+func TryHooks(r *http.Request) (contract.Hooks, bool) {
+	hooks, ok := r.Context().Value(contract.HooksKey).(contract.Hooks)
+
+	return hooks, ok
+}

contract/request/param.go

@@ -8,11 +8,11 @@ import "net/http"
 //
 // Parameters:
 //   - r: The HTTP request containing the path parameters
-//   - k: The name of the path parameter to retrieve
+//   - name: The name of the path parameter to retrieve
 //
 // Returns the parameter value as a string, or empty string if not found.
-func Param(r *http.Request, k string) string {
-	return r.PathValue(k)
+func Param(r *http.Request, name string) string {
+	return r.PathValue(name)
 }
 
 // ParamOr retrieves a path parameter value by name, returning a default
@@ -22,14 +22,14 @@ func Param(r *http.Request, k string) string {
 //
 // Parameters:
 //   - r: The HTTP request containing the path parameters
-//   - k: The name of the path parameter to retrieve
-//   - d: The default value to return if the parameter is not found or empty
+//   - name: The name of the path parameter to retrieve
+//   - fallback: The default value to return if the parameter is not found or empty
 //
 // Returns the parameter value if found and non-empty, otherwise the default value.
-func ParamOr(r *http.Request, k string, d string) string {
-	if p := Param(r, k); p != "" {
-		return p
+func ParamOr(r *http.Request, name string, fallback string) string {
+	if value := Param(r, name); value != "" {
+		return value
 	}
 
-	return d
+	return fallback
 }

contract/request/query.go

@@ -8,11 +8,11 @@ import "net/http"
 //
 // Parameters:
 //   - r: The HTTP request containing the URL with query parameters
-//   - k: The name of the query parameter to retrieve
+//   - name: The name of the query parameter to retrieve
 //
 // Returns the first value associated with the key, or empty string if not found.
-func Query(r *http.Request, k string) string {
-	return r.URL.Query().Get(k)
+func Query(r *http.Request, name string) string {
+	return r.URL.Query().Get(name)
 }
 
 // HasQuery checks if a query parameter exists in the HTTP request URL,
@@ -21,11 +21,11 @@ func Query(r *http.Request, k string) string {
 //
 // Parameters:
 //   - r: The HTTP request containing the URL with query parameters
-//   - k: The name of the query parameter to check for
+//   - name: The name of the query parameter to check for
 //
 // Returns true if the parameter exists in the query string, false otherwise.
-func HasQuery(r *http.Request, k string) bool {
-	return r.URL.Query().Has(k)
+func HasQuery(r *http.Request, name string) bool {
+	return r.URL.Query().Has(name)
 }
 
 // QueryOr retrieves a query parameter value by name, returning a default
@@ -35,14 +35,14 @@ func HasQuery(r *http.Request, k string) bool {
 //
 // Parameters:
 //   - r: The HTTP request containing the URL with query parameters
-//   - k: The name of the query parameter to retrieve
-//   - d: The default value to return if the parameter doesn't exist
+//   - name: The name of the query parameter to retrieve
+//   - fallback: The default value to return if the parameter doesn't exist
 //
 // Returns the parameter value if it exists, otherwise the default value.
-func QueryOr(r *http.Request, k string, d string) string {
-	if HasQuery(r, k) {
-		return Query(r, k)
+func QueryOr(r *http.Request, name string, fallback string) string {
+	if HasQuery(r, name) {
+		return Query(r, name)
 	}
 
-	return d
+	return fallback
 }

contract/request/session.go

@@ -34,6 +34,10 @@ func SessionKeyed(r *http.Request, key any) (contract.Session, bool) {
 // MustSessionKeyed retrieves the session from the request context
 // using the provided key. It panics with [ErrSessionNotFound] if
 // no session is found.
+//
+// WARNING: This function panics when the session is missing. Use
+// [SessionKeyed] for a non-panicking alternative, or ensure the
+// [framework.Recover] middleware is in place.
 func MustSessionKeyed(r *http.Request, key any) contract.Session {
 	if s, ok := SessionKeyed(r, key); ok {
 		return s
@@ -45,6 +49,10 @@ func MustSessionKeyed(r *http.Request, key any) contract.Session {
 // MustSession retrieves the session from the request context using
 // the default [contract.SessionKey]. It panics with [ErrSessionNotFound]
 // if no session is found.
+//
+// WARNING: This function panics when the session is missing. Use
+// [Session] for a non-panicking alternative, or ensure the
+// [framework.Recover] middleware is in place.
 func MustSession(r *http.Request) contract.Session {
 	return MustSessionKeyed(r, contract.SessionKey)
 }

contract/response/static.go

@@ -3,8 +3,11 @@ package response
 import (
 	"encoding/json"
 	"encoding/xml"
+	"errors"
 	htmltemplate "html/template"
 	"net/http"
+	"net/url"
+	"strings"
 	"text/template"
 )
 
@@ -159,6 +162,9 @@ func XML(w http.ResponseWriter, status int, data any) error {
 // This is a generic redirect function that allows you to specify any redirect status code.
 // The Location header is set to the provided URL and the appropriate status code is returned.
 //
+// WARNING: This function does not validate the redirect URL. If the URL comes
+// from user input, use [SafeRedirect] instead to prevent open redirect attacks.
+//
 // Common redirect status codes:
 //   - 301: Moved Permanently
 //   - 302: Found (temporary redirect)
@@ -175,3 +181,52 @@ func Redirect(w http.ResponseWriter, status int, url string) error {
 
 	return Status(w, status)
 }
+
+// ErrUnsafeRedirect is returned by [SafeRedirect] when the target
+// URL is not a safe relative path. This prevents open redirect
+// attacks where an attacker tricks users into visiting a malicious
+// external site via your application's redirect endpoint.
+var ErrUnsafeRedirect = errors.New("unsafe redirect URL: must be a relative path")
+
+// SafeRedirect sends an HTTP redirect response only if the target
+// URL is a safe relative path (starts with "/" and does not contain
+// a scheme, host, or protocol-relative prefix "//"). This prevents
+// open redirect vulnerabilities when the redirect target comes from
+// user input such as query parameters or form fields.
+//
+// Returns [ErrUnsafeRedirect] if the URL fails validation.
+//
+// Parameters:
+//   - w: The HTTP response writer
+//   - status: The HTTP redirect status code to set
+//   - rawURL: The URL to redirect the user to (must be a relative path)
+func SafeRedirect(w http.ResponseWriter, status int, rawURL string) error {
+	if !isRelativePath(rawURL) {
+		return ErrUnsafeRedirect
+	}
+
+	return Redirect(w, status, rawURL)
+}
+
+// isRelativePath validates that a URL is a safe relative path
+// and not an absolute URL, protocol-relative URL, javascript:
+// URI, or data: URI that could be used in an open redirect attack.
+func isRelativePath(rawURL string) bool {
+	if !strings.HasPrefix(rawURL, "/") {
+		return false
+	}
+
+	// Reject protocol-relative URLs like "//evil.com"
+	if strings.HasPrefix(rawURL, "//") {
+		return false
+	}
+
+	parsed, err := url.Parse(rawURL)
+
+	if err != nil {
+		return false
+	}
+
+	// Reject if a scheme or host is present.
+	return parsed.Scheme == "" && parsed.Host == ""
+}

framework/cache/redis.go

@@ -35,7 +35,7 @@ func NewRedisFrom(client *redis.Client) *RedisClient {
 // Get retrieves a value by key. Returns contract.ErrCacheKeyNotFound
 // wrapped with the key name when the key does not exist.
 func (client *RedisClient) Get(ctx context.Context, key string) (any, error) {
-	v, err := (*redis.Client)(client).Get(ctx, key).Result()
+	value, err := (*redis.Client)(client).Get(ctx, key).Result()
 
 	if errors.Is(err, redis.Nil) {
 		return nil, fmt.Errorf("%w: %s", contract.ErrCacheKeyNotFound, key)
@@ -45,7 +45,7 @@ func (client *RedisClient) Get(ctx context.Context, key string) (any, error) {
 		return nil, err
 	}
 
-	return v, nil
+	return value, nil
 }
 
 // Put stores a value with the given TTL. A zero TTL means the key
@@ -61,18 +61,18 @@ func (client *RedisClient) Delete(ctx context.Context, key string) error {
 
 // Has reports whether the key exists in Redis.
 func (client *RedisClient) Has(ctx context.Context, key string) (bool, error) {
-	n, err := (*redis.Client)(client).Exists(ctx, key).Result()
+	count, err := (*redis.Client)(client).Exists(ctx, key).Result()
 
 	if err != nil {
 		return false, err
 	}
 
-	return n > 0, nil
+	return count > 0, nil
 }
 
 // Pull atomically retrieves and deletes a key using Redis GETDEL.
 // The stored value is JSON-decoded into the return value.
-func (client *RedisClient) Pull(ctx context.Context, key string) (v any, e error) {
+func (client *RedisClient) Pull(ctx context.Context, key string) (value any, err error) {
 	encoded, err := (*redis.Client)(client).GetDel(ctx, key).Result()
 
 	if errors.Is(err, redis.Nil) {
@@ -83,11 +83,11 @@ func (client *RedisClient) Pull(ctx context.Context, key string) (v any, e error
 		return nil, err
 	}
 
-	if err := json.Unmarshal([]byte(encoded), &v); err != nil {
+	if err := json.Unmarshal([]byte(encoded), &value); err != nil {
 		return nil, err
 	}
 
-	return v, nil
+	return value, nil
 }
 
 // Forever stores a value with no expiration.