fix: address all MEDIUM tier security vulnerabilities (#13-#32)#5
Merged
Conversation
…ware ordering Remove TRACE and CONNECT from the allMethods slice used by Any() to prevent cross-site tracing (XST) attacks and inappropriate CONNECT usage in non-proxy applications. Individual Trace() and Connect() methods remain available for explicit opt-in. Add warning documentation on Use() explaining that middleware execution order matters and security middleware should be registered first. Fixes #14, #23
…unded reads Limit io.Reader panic values to 1 MB via io.LimitReader to prevent unbounded memory consumption during panic recovery. Wrap all recovered panic values with ErrRecoverUnexpected via errors.Join so that internal error details are never exposed to HTTP clients. The original panic value remains accessible via errors.Unwrap for server-side logging and debugging. Fixes #19, #20
…veHTTPDev Use r.URL.Path instead of r.URL.String() in Defaulted() to avoid reflecting user-supplied query parameters in the Problem Instance field. Query parameters may contain sensitive data such as tokens, credentials, or PII. Add a WARNING doc comment on ServeHTTPDev making it explicit that the method must never be used in production environments. Fixes #21, #32
… docs Add MaxLifetime field to MiddlewareOptions (default: 24h) that enforces an absolute session age limit. Sessions exceeding this age from their createdAt timestamp are force-regenerated, preventing indefinite session extension through continued use. Add CreatedAt field to Session and the contract.Session interface to track the absolute creation time. Add ErrorHandler callback to MiddlewareOptions for surfacing session driver errors (save, delete, regenerate) that were previously silently discarded. Default remains silent for backward compatibility. Add WARNING doc comments on Put() and Regenerate() explaining that Regenerate MUST be called after authentication state changes to prevent session fixation attacks. Add WARNING doc comment on CacheDriver explaining that session data is stored unencrypted. Fixes #15, #16, #17, #18
…middleware Add WARNING doc comment explaining that logged request URLs and error values may contain sensitive data such as tokens, PII, or internal details. Recommends securing log storage and considering sanitisation before exporting to external systems. Fixes #22
…rrors Check WriteHeaderCalled() before calling handleError in ServeHTTP. If the handler already started writing the response, attempting to write an error response would corrupt the output. Instead, log the error for server-side visibility using slog. Fixes #24
Add WARNING doc comment on NATSBrokerOptions.TLSConfig explaining that InsecureSkipVerify disables certificate verification and must not be used in production. Fixes #25
Add WARNING doc comments on Memory.Remember and RedisClient.Remember explaining the cache stampede risk under concurrent access and recommending singleflight for expensive computations. Fixes #26
Add WARNING doc comments on NewSQL and NewSQLFrom explaining that no default query timeout is applied and recommending driver-level statement timeouts or context.WithTimeout for production use. Fixes #27
Add WARNING doc comments on Argon2 and Bcrypt types explaining that hash operations are intentionally CPU/memory intensive and endpoints triggering hashing must implement rate limiting to prevent denial-of-service attacks. Fixes #28
Remove the redundant defer wg.Done() inside the function passed to sync.WaitGroup.Go(). The Go method already calls Add(1) before launching and Done() after the function returns, so the explicit defer wg.Done() would double-decrement the counter and panic. Fixes #29
The unsubscribe closure captured the original subscribe context, which may have been cancelled by the time unsubscribe is called. Use context.WithoutCancel to derive a context that inherits values but not the cancellation signal, ensuring unsubscribe succeeds even after the subscribe context expires. Fixes #30
Add StrictJSON[T] to contract/request that enables DisallowUnknownFields on the JSON decoder, causing requests with unexpected fields to be rejected with an error. This allows APIs that require exact schema compliance to surface typos or unsupported fields to callers. The existing JSON[T] function is unchanged and continues to silently ignore unknown fields. Documentation cross-references both variants. Fixes #31
Add a CORS middleware that supports configurable allowed origins, methods, headers, exposed headers, credentials, and max-age. Provides sensible defaults via DefaultCORSOptions (wildcard origin, GET/POST/HEAD, 5-minute preflight cache, no credentials). Handles preflight OPTIONS requests by responding with 204 No Content after setting appropriate Access-Control-* headers. Non-matching origins are passed through without CORS headers. Includes comprehensive tests covering preflight, disallowed origins, wildcard, no-origin passthrough, and exposed headers. Fixes #13
CI workflows now run from the workspace root using go.work so that local replace directives resolve correctly. This fixes the framework test failure where contract v0.8.0 from the module proxy was missing the new CreatedAt interface method.
ConsoleTVs
force-pushed
the
fix/medium-security-vulnerabilities
branch
from
af253a5 to
2ee963f
Compare
…ulnerabilities # Conflicts: # contract/request/body.go # framework/handler.go # framework/middleware/recover.go # framework/session/middleware.go # framework/session/session.go # problem/problem.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses all 20 MEDIUM tier security vulnerabilities identified in the security audit (findings #13 through #32). Changes span all four modules:
contract,router,problem, andframework.Code Changes
Router
TRACEandCONNECTfromallMethodsslice used byAny(). TRACE enables cross-site tracing (XST) attacks; CONNECT is for proxies only. IndividualTrace()/Connect()methods remain for explicit opt-in.Use()explaining middleware ordering importance for security.Problem
r.URL.Pathinstead ofr.URL.String()inDefaulted()to strip query parameters from theInstancefield, preventing reflection of sensitive data.ServeHTTPDevmaking it explicit this must never be used in production.Framework - Recover Middleware
io.LimitReader(1 MB cap) forio.Readerpanic values to prevent unbounded memory consumption.ErrRecoverUnexpectedviaerrors.Joinso internal details are never exposed to HTTP clients. Original values remain accessible viaerrors.Unwrapfor logging.Framework - Session
Put()andRegenerate()explaining session fixation risks.MaxLifetimefield (default: 24h) andcreatedAttracking to enforce absolute session age limits, preventing indefinite session extension.CacheDriverabout unencrypted session storage.ErrorHandlercallback toMiddlewareOptionsfor surfacing session driver errors that were silently discarded.Framework - Handler
WriteHeaderCalled()beforehandleErrorinServeHTTP. If the handler already started writing, log the error instead of corrupting the response.Framework - Logger
Framework - Events
TLSConfigaboutInsecureSkipVerifyrisks.defer wg.Done()insidesync.WaitGroup.Go()in AMQP subscriber, preventing double-decrement panic.context.WithoutCancel(ctx)for MQTT unsubscribe to prevent failure when the subscribe context has expired.Framework - Cache
Remember()methods documenting thundering herd risk and recommendingsingleflight.Framework - Database
NewSQL/NewSQLFromabout missing default query timeouts.Framework - Hash
Argon2andBcrypttypes about rate limiting requirements.Contract
StrictJSON[T]variant withDisallowUnknownFields()for strict schema validation. ExistingJSON[T]unchanged.New Feature
CORSmiddleware with configurable origins, methods, headers, credentials, and max-age. IncludesDefaultCORSOptionsand comprehensive tests.Testing
All four modules pass
go vetandgo test:contract: vet + test cleanrouter: vet + test cleanproblem: vet + test cleanframework: vet + test clean (including new CORS middleware tests)