Fix low-severity audit findings across all modules#13
Merged
Conversation
- Rename abbreviated params: k->name, d->fallback in ParamInt/QueryInt variants - Standardize doc comments from bulleted parameter lists to concise prose - Add t.Parallel() to all test functions across all contract test files - Add NeedsRehash doc comment on Rehashable interface - Fix WithTransaction doc wording about nested operations
- Rewrite all doc comments to start with symbol name, end with period - Fix typos: recursivity->recursion, an handler->a handler, responses->requests and responses - Remove redundant zero-value initializers in New() - Rename Record parameter r->request for full-word convention - Add panic message assertions in dot-dot and empty-method tests - Remove stale RecordHandler reference
…uilder - Remove unused embed import (no //go:embed directive) - Fix all doc comments: Problem type interfaces, With/Without copy-on-write, Error/Errors/Unwrap descriptions, MarshalJSON/UnmarshalJSON behavior, Defaulted field enumeration, strack-trace typo - Add magic number comment for +5 RFC 9457 fields - Fix internal/accept.go doc comments: Accept type, find grammar, Order ordering - Replace make([]error, 0) with var for nil-error fast path - Use strings.Builder instead of += concatenation in textHandler - Use httptest.NewRequest in accept_test.go
…tant, hook fixes - Add doc comment on handleError explaining error inspection logic - Extract StatusClientClosedRequest = 499 constant replacing magic number - Initialize afterResponseHooks in NewHooks for consistency - Add mutex doc comment in Hooks struct - Fix ServerOptions doc to cover all zero-valued fields - Use t.Context() in handler_test.go for proper test cancellation
…t consistency - Separate imports into three groups (stdlib/cosmos/external) in all test files - Rename abbreviated receiver rs->readerStringer in recover_test.go - Fix RecoverWith doc to suggest interface pattern instead of unexported type - Remove redundant 1* in rate limit CleanupInterval default - Remove compile-time interface check on test-only testTextMarshaler - Improve Provide doc comment with parameter guidance and retrieval example
….Prefix - Remove redundant sync.Mutex zero-value initialization - Fix informal comment and Regenerate/Delete doc inaccuracies - Replace string literal "GET" with http.MethodGet in all tests - Export CacheDriverOptions.Prefix field for external configurability - Fix import grouping in all test files
…l atomicity - Fix import grouping to three groups in memory.go, redis.go, memory_test.go - Align redis doc comments with memory counterparts (Delete, Has, Forever, Increment, Decrement) - Remove named return values from redis Pull method - Document Pull's non-atomic nature in memory.go
…uct field order - Rename TestItCan* tests to TestAES*/TestChaCha20* naming convention - Rename abbreviated variable e->encrypter and cypher->ciphertext - Align ChaCha20 struct field order with AES (key, aead, AdditionalData) - Add ChaCha20 concurrency safety doc note matching AES - Fix import grouping in test files
…r zero checks - Rename TestItCan* tests to TestArgon2*/TestBcrypt* naming convention - Rename abbreviated variables h->hasher, r->hashed - Replace manual all-zero byte loops with require.Equal assertions - Use parenthesized import in argon2.go for consistency - Fix import grouping in test files
- Separate imports into three groups (stdlib/cosmos/external)
- Replace &sql.TxOptions{} with nil for idiomatic driver defaults
- Fix import grouping to three groups in all source and test files - Replace fmt.Sprintf with strconv.FormatUint for subscription IDs - Use var declarations instead of empty composite literals for zero-value types - Remove unnecessary var() grouping for single ErrBrokerClosed - Fix deliverToHandler doc comment accuracy - Add explicit _ = for discarded Close errors in AMQP broker
…t-findings # Conflicts: # contract/hash.go # contract/request/body_test.go # framework/cache/memory.go # framework/cache/redis.go # framework/crypto/chacha20.go # framework/event/memory.go # framework/event/redis.go # framework/hash/bcrypt_test.go # framework/middleware/recover.go # router/router.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
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
Fourth and final tier of fixes from the comprehensive security and quality audit. This PR addresses low-severity findings across all modules — code style, doc comments, naming conventions, test consistency, import grouping, and minor ergonomic improvements.
Contract
k→name,d→fallback) inParamInt/QueryIntvariantst.Parallel()to all test functions across 13 test filesNeedsRehashdoc comment onRehashableinterfaceWithTransactiondoc wording about nested operationsRouter
pattern: "",parent: nil) inNew()Recordparameterr→requestfor full-word conventionRecordHandlercross-referenceProblem
embedimport (no//go:embeddirective)Problemtype interfaces,With/Withoutcopy-on-write,Error/Errors/Unwrapdescriptions,MarshalJSON/UnmarshalJSONbehavior,Defaultedfield enumeration, typo fixesmake([]error, 0)withvarfor nil-error fast pathstrings.Builderinstead of+=concatenation intextHandlerhttptest.NewRequestinaccept_test.goFramework Core
handleErrorexplaining error inspection logicStatusClientClosedRequest = 499constant replacing magic numberafterResponseHooksinNewHooksfor consistency with other hook slicesmutexdoc comment inHooksstructServerOptionsdoc to cover all zero-valued fieldst.Context()inhandler_test.gofor proper test cancellationMiddleware
rs→readerStringerinrecover_test.goRecoverWithdoc to suggest interface pattern instead of referencing unexported type1 *in rate limitCleanupIntervaldefaulttestTextMarshalerProvidedoc with parameter guidance and retrieval exampleSession
sync.Mutex{}zero-value initializationRegenerate/Deletedoc inaccuracies"GET"withhttp.MethodGetin all testsCacheDriverOptions.Prefixfield for external configurabilityCache
Delete,Has,Forever,Increment,Decrement)PullmethodPull's non-atomic nature in memory implementationCrypto
TestItCan*tests toTestAES*/TestChaCha20*naming conventione→encrypter,cypher→ciphertextChaCha20struct field order withAES(key,aead,AdditionalData)ChaCha20concurrency safety doc note matchingAESHash
TestItCan*tests toTestArgon2*/TestBcrypt*naming conventionh→hasher,r→hashedrequire.Equalassertionsargon2.gofor consistencyDatabase
&sql.TxOptions{}withnilfor idiomatic driver defaultsEvent
fmt.Sprintfwithstrconv.FormatUintfor subscription IDsvardeclarations instead of empty composite literals for zero-value typesvar()grouping for singleErrBrokerCloseddeliverToHandlerdoc comment accuracy_ =for discardedCloseerrors in AMQP brokerBreaking Changes
CacheDriverOptions.Prefixis now exportedWhat changed:
prefixfield renamed toPrefix.Migration: If constructing
CacheDriverOptionswith struct literal, renameprefix→Prefix.Why: The struct is exported but the field was unexported, making it unconfigurable by external callers.
StatusClientClosedRequestconstant addedWhat changed: New exported constant
StatusClientClosedRequest = 499.Migration: No migration needed — this is a new export. Callers can optionally use it instead of magic number 499.