fix: low-severity security hardening across all modules#6
Merged
Conversation
UUID v7 embeds a creation timestamp and provides only 74 bits of entropy. Replace with 32 bytes from crypto/rand encoded as base64url, yielding 256 bits of pure entropy with no timestamp leakage. This also removes the github.com/google/uuid dependency.
Add withDefaults() method on MiddlewareOptions that sets secure defaults when fields are zero-valued: Secure=true, SameSite=Lax, Path="/", Name=DefaultCookie, Key=contract.SessionKey, TTL=DefaultTTL, ExpirationDelta=DefaultExpirationDelta. Called at the start of MiddlewareWith() so callers get safe defaults for unset fields while still being able to override specific ones.
… to Raw/Stream Set Content-Type to text/html; charset=utf-8 for HTML and HTMLTemplate to prevent XSS via charset sniffing in older browsers. Add application/octet-stream as default Content-Type for Raw() and Stream() when no Content-Type has been explicitly set on the response writer.
Add ParamInt, ParamIntOr, QueryInt, and QueryIntOr helpers that parse and validate numeric parameters using only stdlib strconv. This prevents injection via malformed numeric params by returning clear errors for non-integer values.
… matching
Add WARNING doc comments to Group() and Method() explaining that
path.Join normalizes .. segments in route patterns. Document that
catch-all routes (e.g., /files/{path...}) also match their base path
and handlers should account for empty catch-all values.
- Raise default bcrypt cost from 10 to 12 per OWASP recommendation - Enforce minimum cost of bcrypt.MinCost (4) in NewBcryptWith() - Zero password bytes from memory after Hash/Check in both Bcrypt and Argon2 via defer zeroBytes() to limit exposure window - Update tests to use separate byte slices for Hash and Check since the password slice is now zeroed after each call
…eration Return the sentinel error directly without appending the key value. This prevents potential cache key enumeration attacks through error messages while preserving the ability to match errors with errors.Is.
- Add optional AdditionalData field to AES and ChaCha20 structs for binding ciphertext to a context (user ID, resource path) via AAD - Pass AdditionalData to Seal/Open calls in both Encrypt and Decrypt - Add Close() method to both AES and ChaCha20 that zeros key material from memory; callers should defer Close after construction - Store raw key in ChaCha20 struct so Close can zero it
The deliverToHandler method had an empty recover block that silently swallowed panics. Now logs recovered values via slog.Error so panicking event handlers are visible for debugging.
Add Configure method on *SQL that accepts a func(*sql.DB) callback, allowing callers to set pool parameters like SetMaxOpenConns, SetMaxIdleConns, and SetConnMaxLifetime without importing database/sql.
Wrap each AfterResponse callback invocation with panic recovery and slog.Error logging. This prevents a single panicking hook from crashing the server and ensures remaining hooks still execute.
Add security note explaining that slog structured logging mitigates classic log injection attacks by emitting values as discrete key-value pairs. Warn that callers forwarding logs to plain-text renderers should remain aware that attacker-controlled URLs appear in log entries.
…erabilities # Conflicts: # contract/request/param.go # contract/request/query.go # contract/response/static.go # framework/event/memory.go # framework/session/middleware.go # framework/session/session.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
Addresses all 20 LOW-severity findings (#33-#52) from the security audit.
Security Hardening
crypto/rand+ base64url (256-bit entropy, no timestamp leakage) — removesgoogle/uuiddependencyMiddlewareWith()zero-value options now default toSecure=true,SameSite=Lax, sensible TTL/cookie settings viawithDefaults()defer zeroBytes(value)in all Hash/Check methods (Argon2 + Bcrypt)AdditionalDatafield on AES and ChaCha20 for context bindingClose()method on AES and ChaCha20 to zero key material from memorycharset=utf-8to HTML Content-Type to prevent charset-sniffing XSSRaw()andStream()now setapplication/octet-streamwhen unsetr.URL.Pathonly (strips query string)Resilience
slogwg.Done()preventing double-decrement paniccontext.WithoutCancel(ctx)to avoid expired-context failuresNew APIs
ParamInt(),ParamIntOr(),QueryInt(),QueryIntOr()— typed integer parsing helpers incontract/request/sql.Configure(func(*sql.DB))— exposes connection pool tuningAES.Close(),ChaCha20.Close()— key material zeroingDocumentation
path.Joinnormalization in routerGroup()Testing
All 4 modules pass
go vetandgo test:contract,router,problem,frameworkFiles Changed
25 files across all 4 modules (577 additions, 161 deletions)