Merge pull request #5 from StudioLambda/fix/medium-security-vulnerabilities

fix: address all MEDIUM tier security vulnerabilities (#13-#32)

Author: ConsoleTVs

.github/workflows/framework.yml

@@ -5,11 +5,13 @@ on:
     branches: ["main"]
     paths:
       - .github/workflows/framework.yml
+      - contract/**
       - framework/**
   pull_request:
     branches: ["main"]
     paths:
       - .github/workflows/framework.yml
+      - contract/**
       - framework/**
 
 jobs:
@@ -28,6 +30,9 @@ jobs:
           go-version-file: "framework/go.mod"
           cache-dependency-path: "framework/go.sum"
 
+      - name: Link local modules
+        run: go mod edit -replace github.com/studiolambda/cosmos/contract=../contract -replace github.com/studiolambda/cosmos/problem=../problem -replace github.com/studiolambda/cosmos/router=../router
+
       - name: Download Dependencies
         run: go mod download
 

AGENTS.md

@@ -31,17 +31,9 @@ cd contract && go generate ./...
 
 Always run tests from workspace root for proper module resolution. Each module has its own go.mod. Use full import paths: `github.com/studiolambda/cosmos/contract`
 
-## Code Style
+## Framework Patterns
 
-- Standard Go formatting with gofmt
-- Descriptive names, early returns, single-purpose functions
-- Doc comments starting with name being documented
-- Always check errors with errors.Is() and errors.As()
-- Table-driven tests with testify assertions
-
-### Framework Patterns
-
-Error-returning handlers:
+Error-returning handlers (unlike stdlib, Cosmos handlers return errors):
 ```go
 func handler(w http.ResponseWriter, r *http.Request) error {
     return response.JSON(w, http.StatusOK, data)

contract/mock/session.go

@@ -66,9 +66,12 @@ func LimitedString(r *http.Request, maxSize int64) (string, error) {
 	return string(body), nil
 }
 
-// JSON decodes JSON data from the request body into a value of type T.
-// It uses a streaming decoder for memory efficiency. The type parameter
-// T should match the expected JSON structure.
+// JSON decodes JSON data from the request body into a value of
+// type T. It uses a streaming decoder for memory efficiency. The
+// type parameter T should match the expected JSON structure.
+//
+// Unknown fields in the JSON input are silently ignored. Use
+// [StrictJSON] if unknown fields should cause an error.
 //
 // WARNING: This function decodes without any body size limit.
 // Prefer [LimitedJSON] or apply [http.MaxBytesReader] in a
@@ -81,6 +84,27 @@ func JSON[T any](r *http.Request) (value T, err error) {
 	return value, nil
 }
 
+// StrictJSON decodes JSON data from the request body into a value
+// of type T, rejecting any fields not present in T's definition.
+// This is useful for APIs that require exact schema compliance
+// and want to surface typos or unsupported fields to callers.
+//
+// For a lenient variant that ignores unknown fields, use [JSON].
+//
+// WARNING: This function decodes without any body size limit.
+// Prefer [StrictLimitedJSON] or apply [http.MaxBytesReader] in a
+// middleware to prevent memory exhaustion from oversized requests.
+func StrictJSON[T any](r *http.Request) (value T, err error) {
+	decoder := json.NewDecoder(r.Body)
+	decoder.DisallowUnknownFields()
+
+	if err := decoder.Decode(&value); err != nil {
+		return value, err
+	}
+
+	return value, nil
+}
+
 // LimitedJSON decodes JSON data from the request body into a value
 // of type T, reading at most maxSize bytes. This prevents
 // denial-of-service attacks via oversized JSON payloads. Pass -1
@@ -99,6 +123,25 @@ func LimitedJSON[T any](r *http.Request, maxSize int64) (value T, err error) {
 	return value, nil
 }
 
+// StrictLimitedJSON decodes JSON data from the request body into
+// a value of type T, reading at most maxSize bytes and rejecting
+// unknown fields. Pass -1 to use [DefaultMaxBodySize].
+func StrictLimitedJSON[T any](r *http.Request, maxSize int64) (value T, err error) {
+	if maxSize < 0 {
+		maxSize = DefaultMaxBodySize
+	}
+
+	limited := io.LimitReader(r.Body, maxSize+1)
+	decoder := json.NewDecoder(limited)
+	decoder.DisallowUnknownFields()
+
+	if err := decoder.Decode(&value); err != nil {
+		return value, err
+	}
+
+	return value, nil
+}
+
 // XML decodes XML data from the request body into a value of type T.
 // It uses a streaming decoder for memory efficiency. The type parameter
 // T should have appropriate xml struct tags or implement [xml.Unmarshaler].

contract/request/body.go

@@ -11,62 +11,87 @@ type sessionKey struct{}
 // SessionKey is the context key used to store and retrieve the session from a context.Context.
 var SessionKey = sessionKey{}
 
-// Session represents a user session with data storage and lifecycle management capabilities.
-// It provides methods to store, retrieve, and manage session data, as well as control
-// session expiration and regeneration for security purposes.
+// Session represents a user session with data storage and lifecycle
+// management capabilities. It provides methods to store, retrieve,
+// and manage session data, as well as control session expiration
+// and regeneration for security purposes.
 type Session interface {
-	// SessionID returns the current session identifier. This may differ from the original
-	// session ID if the session has been regenerated.
+	// SessionID returns the current session identifier. This may
+	// differ from the original session ID if the session has been
+	// regenerated.
 	SessionID() string
 
-	// OriginalSessionID returns the session ID that was originally assigned to this session.
-	// This remains constant even if the session is regenerated.
+	// OriginalSessionID returns the session ID that was originally
+	// assigned to this session. This remains constant even if the
+	// session is regenerated.
 	OriginalSessionID() string
 
-	// Get retrieves a value from the session by key. It returns the value and a boolean
-	// indicating whether the key exists in the session. The value can be of any type.
+	// Get retrieves a value from the session by key. It returns
+	// the value and a boolean indicating whether the key exists
+	// in the session. The value can be of any type.
 	Get(key string) (any, bool)
 
-	// Put stores a value in the session associated with the given key. If the key already
-	// exists, its value is overwritten.
+	// Put stores a value in the session associated with the given
+	// key. If the key already exists, its value is overwritten.
+	//
+	// WARNING: When storing authentication-related state, callers
+	// MUST call Regenerate immediately after to prevent session
+	// fixation attacks.
 	Put(key string, value any)
 
-	// Delete removes the value associated with the given key from the session.
-	// If the key does not exist, this operation is a no-op.
+	// Delete removes the value associated with the given key from
+	// the session. If the key does not exist, this is a no-op.
 	Delete(key string)
 
-	// Extend updates the session's expiration time to the specified time. This is useful
-	// for extending a session's lifetime during active use.
+	// Extend updates the session's expiration time to the specified
+	// time. This is useful for extending a session's lifetime
+	// during active use.
 	Extend(expiresAt time.Time)
 
-	// Regenerate creates a new session ID and associates it with this session.
-	// This is commonly used after authentication to prevent session
-	// fixation attacks. It returns an error if the regeneration process fails.
+	// Regenerate creates a new session ID and associates it with
+	// this session. This is commonly used after authentication to
+	// prevent session fixation attacks. It returns an error if
+	// the regeneration process fails.
+	//
+	// WARNING: This method MUST be called after any authentication
+	// state change (login, logout, privilege escalation).
 	Regenerate() error
 
-	// Clear removes all data from the session while maintaining the session itself.
+	// Clear removes all data from the session while maintaining
+	// the session itself.
 	Clear()
 
+	// CreatedAt returns the absolute time the session was first
+	// created. This value never changes, even if the session is
+	// regenerated or extended.
+	CreatedAt() time.Time
+
 	// ExpiresAt returns the time at which the session will expire.
 	ExpiresAt() time.Time
 
-	// HasExpired returns true if the current time is past the session's expiration time.
+	// HasExpired returns true if the current time is past the
+	// session's expiration time.
 	HasExpired() bool
 
-	// ExpiresSoon returns true if the session will expire within the specified duration
-	// from the current time. This is useful for triggering session renewal prompts.
+	// ExpiresSoon returns true if the session will expire within
+	// the specified duration from the current time. This is useful
+	// for triggering session renewal prompts.
 	ExpiresSoon(delta time.Duration) bool
 
-	// HasChanged returns true if the session data has been modified since it was loaded.
-	// This is useful for determining whether the session needs to be persisted.
+	// HasChanged returns true if the session data has been modified
+	// since it was loaded. This is useful for determining whether
+	// the session needs to be persisted.
 	HasChanged() bool
 
-	// HasRegenerated returns true if the session has been regenerated (i.e., the session ID
-	// has changed). This is useful for sending updated session identifiers to the client.
+	// HasRegenerated returns true if the session has been
+	// regenerated (i.e., the session ID has changed). This is
+	// useful for sending updated session identifiers to the
+	// client.
 	HasRegenerated() bool
 
-	// MarkAsUnchanged sets the session as if nothing has changed, therefore avoiding saving
-	// the session when the request finishes.
+	// MarkAsUnchanged sets the session as if nothing has changed,
+	// therefore avoiding saving the session when the request
+	// finishes.
 	MarkAsUnchanged()
 }
 

contract/session.go

@@ -118,9 +118,16 @@ func (memory *Memory) Decrement(ctx context.Context, key string, by int64) (int6
 	return memory.store.DecrementInt64(key, by)
 }
 
-// Remember retrieves the cached value for the given key, or computes
-// and stores it if the key is not found. The compute function is only
-// called on a cache miss.
+// Remember retrieves the cached value for the given key, or
+// computes and stores it if the key is not found. The compute
+// function is only called on a cache miss.
+//
+// WARNING: This method is not protected against thundering herd
+// (cache stampede). Under high concurrency, multiple goroutines
+// may observe a cache miss simultaneously and all invoke the
+// compute function. For expensive computations, callers should
+// use golang.org/x/sync/singleflight to deduplicate concurrent
+// calls for the same key.
 func (memory *Memory) Remember(ctx context.Context, key string, ttl time.Duration, compute func() (any, error)) (any, error) {
 	val, err := memory.Get(ctx, key)
 

framework/cache/memory.go

@@ -105,9 +105,17 @@ func (client *RedisClient) Decrement(ctx context.Context, key string, by int64)
 	return (*redis.Client)(client).DecrBy(ctx, key, by).Result()
 }
 
-// Remember retrieves the cached value for key, or computes and stores
-// it with the given TTL on a cache miss. Non-"key not found" errors
-// from Get are returned immediately without calling compute.
+// Remember retrieves the cached value for key, or computes and
+// stores it with the given TTL on a cache miss. Non-"key not
+// found" errors from Get are returned immediately without calling
+// compute.
+//
+// WARNING: This method is not protected against thundering herd
+// (cache stampede). Under high concurrency, multiple goroutines
+// may observe a cache miss simultaneously and all invoke the
+// compute function. For expensive computations, callers should
+// use golang.org/x/sync/singleflight to deduplicate concurrent
+// calls for the same key.
 func (client *RedisClient) Remember(ctx context.Context, key string, ttl time.Duration, compute func() (any, error)) (any, error) {
 	val, err := client.Get(ctx, key)
 

framework/cache/redis.go

@@ -25,9 +25,16 @@ type SQL struct {
 	raw *sqlx.DB // needed for transactions
 }
 
-// NewSQL connects to the database using the given driver name and DSN,
-// returning a ready-to-use SQL instance or an error if the connection
-// cannot be established.
+// NewSQL connects to the database using the given driver name and
+// DSN, returning a ready-to-use SQL instance or an error if the
+// connection cannot be established.
+//
+// WARNING: No default query timeout is applied. Long-running or
+// runaway queries will block indefinitely unless the caller
+// passes a context.Context with a deadline or timeout. For
+// production use, configure statement timeouts at the database
+// driver level (e.g., statement_timeout for PostgreSQL) or wrap
+// all query contexts with context.WithTimeout.
 func NewSQL(driver string, dsn string) (*SQL, error) {
 	db, err := sqlx.Connect(driver, dsn)
 
@@ -41,6 +48,9 @@ func NewSQL(driver string, dsn string) (*SQL, error) {
 // NewSQLFrom wraps an existing sqlx.DB connection in a SQL instance.
 // This is useful when you need to configure the connection pool or
 // driver options before handing it to the framework.
+//
+// WARNING: No default query timeout is applied. See [NewSQL] for
+// recommendations on configuring query timeouts.
 func NewSQLFrom(db *sqlx.DB) *SQL {
 	return &SQL{db: db, raw: db}
 }

framework/database/sql.go

@@ -227,8 +227,6 @@ func (broker *AMQPBroker) Subscribe(
 	wg := sync.WaitGroup{}
 
 	wg.Go(func() {
-		defer wg.Done()
-
 		for delivery := range deliveries {
 			handler(func(dest any) error {
 				return json.Unmarshal(delivery.Body, dest)

framework/event/amqp.go

@@ -351,9 +351,12 @@ func (broker *MQTTBroker) Subscribe(
 		broker.mu.Unlock()
 
 		if shouldUnsubscribe {
-			_, err := broker.client.Unsubscribe(ctx, &paho.Unsubscribe{
-				Topics: []string{topic},
-			})
+			_, err := broker.client.Unsubscribe(
+				context.WithoutCancel(ctx),
+				&paho.Unsubscribe{
+					Topics: []string{topic},
+				},
+			)
 
 			return err
 		}