Skip to content

Fix medium-severity audit findings across all modules#12

Merged
ConsoleTVs merged 13 commits into
mainfrom
fix/medium-severity-audit-findings
Apr 12, 2026
Merged

Fix medium-severity audit findings across all modules#12
ConsoleTVs merged 13 commits into
mainfrom
fix/medium-severity-audit-findings

Conversation

@ConsoleTVs

Copy link
Copy Markdown
Member

Summary

Third and final tier of fixes from the comprehensive security and quality audit of all Cosmos modules. This PR addresses all 42 medium-severity findings (M1–M42) across every module and subpackage.

Contract (M1–M3, M36)

  • QueryOr: parse query string once instead of per-call to reduce allocations
  • HTMLTemplate: buffer template execution to prevent partial response writes on error
  • Add Rehashable interface for hash algorithm migration support

Router (M4–M6)

  • Reject patterns containing .. segments to prevent path traversal
  • Panic on empty method in Handle() to catch misuse early
  • Add concurrency safety documentation for router registration

Problem (M7–M9)

  • Ensure deterministic JSON field ordering (type, status, title, detail, instance, extensions)
  • Support */* wildcard in Accept header content negotiation
  • Fix media type matching to respect slash boundaries

Framework Core (M10–M12)

  • Pass request context (not background) to hooks in handleError
  • Change DefaultServerOptions to function to prevent global state mutation
  • Add panic recovery in Before* hook callbacks on ResponseWriter

Middleware (M13–M18)

  • CORS: add Vary headers for cacheability, handle preflight consistently
  • Logger: log URL path only to prevent token leakage in query strings
  • Rate limiter: respect context cancellation for graceful shutdown
  • Recover: capture stack traces for debugging
  • Secure headers: remove deprecated X-XSS-Protection header

Session (M19–M22)

  • Check MaxLifetime during load to reject expired sessions
  • Set sensible MaxLifetime default (24h) when zero value
  • Fix Secure cookie flag to use r.TLS instead of request scheme
  • Use context.WithoutCancel for session persistence after response

Cache (M23–M27)

  • Remove unnecessary inner mutex from entry struct (outer map lock suffices)
  • Add documentation warning about reference poisoning with pointer values
  • Discriminate cache miss vs computation error in Remember
  • Add Redis increment overflow documentation

Crypto (M28–M32)

  • Add ErrEncrypterClosed sentinel error for use-after-close detection
  • Make Close() idempotent by checking for nil AEAD
  • Use clear() builtin for key zeroing instead of manual loop
  • Add runtime.KeepAlive to prevent premature garbage collection
  • Document nonce size limits and AAD usage

Hash (M33–M37)

  • Export BcryptOptions.Cost field for external configuration
  • Document bcrypt 72-byte password truncation behavior
  • Use clear() builtin for byte slice zeroing with KeepAlive
  • Add NeedsRehash to Argon2 for algorithm migration support
  • Document argon2 input mutation of password slice

Database (M38–M40)

  • Use db.QueryContext/ExecContext directly instead of Prepare+Execute
  • Document connection pool configuration recommendations
  • Make sqlTx.Close() a no-op instead of closing underlying connection
  • Add comprehensive test suite with 14 tests using sqlite3

Event (M41–M42)

  • Extract validateEvent helper to deduplicate validation across all brokers
  • Document wildcard event matching semantics for each broker

Breaking Changes

DefaultServerOptions is now a function

What changed: DefaultServerOptions changed from var ServerOptions to func DefaultServerOptions() ServerOptions

Migration:

// Before
opts := framework.DefaultServerOptions
opts.ReadTimeout = 10 * time.Second

// After
opts := framework.DefaultServerOptions()
opts.ReadTimeout = 10 * time.Second

Why: Prevents global state mutation where one caller modifying the variable affects all subsequent callers.

CORS panics on AllowCredentials: true with wildcard origins

What changed: (from high-severity PR) CORS middleware now panics at startup if AllowCredentials: true with wildcard * origin.

BcryptOptions.Cost is now exported

What changed: cost field renamed to Cost for external configuration.

Migration: If constructing BcryptOptions with struct literal, rename costCost.

Why: Allows users to configure bcrypt cost without using option functions.

…erface

- QueryOr: parse query string once instead of per-call
- HTMLTemplate: buffer template execution to prevent partial writes on error
- Add Rehashable interface for hash algorithm migration support
- Reject patterns containing '..' segments to prevent path traversal
- Panic on empty method in Handle() to catch misuse early
- Add concurrency safety documentation for router registration
…boundary matching

- Ensure deterministic JSON field order (type, status, title, detail, instance, extensions)
- Support */* wildcard in Accept header content negotiation
- Fix media type matching to respect slash boundaries
- Pass request context (not background) to hooks in handleError
- Change DefaultServerOptions to function to prevent global mutation
- Add panic recovery in Before* hook callbacks on ResponseWriter
…r, and secure headers

- CORS: add Vary headers for cacheability, handle preflight consistently
- Logger: log URL path only to prevent token leakage in query strings
- Rate limiter: respect context cancellation for graceful shutdown
- Recover: capture stack traces for debugging
- Secure headers: remove deprecated X-XSS-Protection header
…sistence

- Check MaxLifetime during load to reject expired sessions
- Set sensible MaxLifetime default (24h) when zero
- Fix Secure cookie flag to use r.TLS instead of request scheme
- Use context.WithoutCancel for session persistence after response
…d add docs

- Remove unnecessary mutex from entry struct (outer map lock suffices)
- Add docs warning about reference poisoning with pointer values
- Discriminate cache miss vs computation error in Remember
- Add Redis increment overflow documentation
…y zeroing

- Add ErrEncrypterClosed sentinel error for use-after-close detection
- Make Close() idempotent by checking for nil AEAD
- Use clear() builtin for key zeroing instead of manual loop
- Add runtime.KeepAlive to prevent premature garbage collection
- Document nonce size limits and AAD usage
… zeroing

- Export BcryptOptions.Cost field for external configuration
- Document bcrypt 72-byte password truncation behavior
- Use clear() builtin for byte slice zeroing with KeepAlive
- Add NeedsRehash to Argon2 for algorithm migration support
- Document argon2 input mutation of password slice
…x sqlTx Close

- Use db.QueryContext/ExecContext directly instead of Prepare+Execute
- Document connection pool configuration recommendations
- Make sqlTx.Close() a no-op instead of closing underlying connection
- Add comprehensive test suite with 14 tests using sqlite3
…rd semantics

- Extract validateEvent helper to deduplicate validation across all brokers
- Document wildcard event matching semantics for each broker
- Ensure consistent empty/whitespace event name rejection
Add mattn/go-sqlite3 for database integration tests.
@ConsoleTVs ConsoleTVs marked this pull request as ready for review April 12, 2026 15:11
…udit-findings

# Conflicts:
#	framework/database/sql.go
#	framework/database/sql_test.go
#	framework/hooks_writer_test.go
#	framework/middleware/cors_test.go
#	framework/middleware/rate_limit.go
#	framework/middleware/rate_limit_test.go
#	framework/session/middleware.go
#	router/router_test.go
@ConsoleTVs ConsoleTVs merged commit fb6e474 into main Apr 12, 2026
3 checks passed
@ConsoleTVs ConsoleTVs deleted the fix/medium-severity-audit-findings branch April 12, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant