feat(sanitize): add configurable NoSQL injection detector#35
Conversation
Introduce NoSQLInjectionDetector with a default operator set (e.g. , ) and a 4096-byte default input length cap to detect common NoSQL injection patterns. - Options: WithNoSQLDetectMaxLength, WithNoSQLDetectOperators - Validation: ensure maxLength > 0 and operators are non-empty - Errors: ErrInvalidNoSQLConfig, ErrNoSQLInputTooLong, ErrNoSQLInjectionDetected - Docs: update README, security checklist, and usage with examples - Tests: cover default operators (e.g. , ) and custom operator sets - Chore: add “NoSQL/nosql” to cspell dictionary Rationale: extend sanitization beyond SQL to cover NoSQL operator-based attacks commonly found in JSON request bodies.
There was a problem hiding this comment.
Pull request overview
This PR introduces a NoSQL injection detector to extend the sanitization package beyond SQL to cover operator-based NoSQL attacks commonly found in JSON request bodies (e.g., MongoDB query operators like $ne, $where, $regex).
Changes:
- Adds
NoSQLInjectionDetectorwith configurable operator list and input length limits (default 4096 bytes) - Introduces three new errors:
ErrInvalidNoSQLConfig,ErrNoSQLInputTooLong,ErrNoSQLInjectionDetected - Updates documentation (README, usage guide, security checklist) with NoSQL detection examples and guidelines
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/sanitize/nosql_detect.go | Core implementation with detector logic, operator normalization, boundary detection, and default MongoDB operator list |
| pkg/sanitize/nosql_detect_test.go | Basic test coverage for default operators and custom operator configuration |
| pkg/sanitize/errors.go | Adds three new error types for NoSQL detector validation and detection |
| docs/usage.md | Documents NoSQL injection detection API and behavior |
| docs/security-checklist.md | Adds guidance to use NoSQL detector for untrusted JSON-like input |
| README.md | Adds NoSQL detector to feature list and usage example |
| cspell.json | Adds "nosql", "NoSQL", and "elemmatch" to dictionary |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestNoSQLInjectionDetectorCustomOperators(t *testing.T) { | ||
| detector, err := NewNoSQLInjectionDetector(WithNoSQLDetectOperators("custom")) | ||
| if err != nil { | ||
| t.Fatalf("expected detector, got %v", err) | ||
| } | ||
|
|
||
| err = detector.Detect(`{"$custom":true}`) | ||
| if err != ErrNoSQLInjectionDetected { | ||
| t.Fatalf("expected ErrNoSQLInjectionDetected, got %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing test coverage for maximum input length validation. The code includes ErrNoSQLInputTooLong error handling at line 82 of nosql_detect.go, but there's no test verifying this behavior. Consider adding a test similar to other sanitizers in this package that validates inputs exceeding the configured max length are properly rejected.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| func TestNoSQLInjectionDetectorDefault(t *testing.T) { | ||
| detector, err := NewNoSQLInjectionDetector() | ||
| if err != nil { | ||
| t.Fatalf("expected detector, got %v", err) | ||
| } | ||
|
|
||
| err = detector.Detect(`{"username":{"$ne":null}}`) | ||
| if err != ErrNoSQLInjectionDetected { | ||
| t.Fatalf("expected ErrNoSQLInjectionDetected, got %v", err) | ||
| } | ||
|
|
||
| err = detector.Detect(`{"$where":"sleep(1)"}`) | ||
| if err != ErrNoSQLInjectionDetected { | ||
| t.Fatalf("expected ErrNoSQLInjectionDetected, got %v", err) | ||
| } | ||
|
|
||
| err = detector.Detect("price$usd") | ||
| if err != nil { | ||
| t.Fatalf("expected no error, got %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing test coverage for edge cases in operator detection. Consider adding tests for boundary cases such as: dollar sign at end of string (e.g., "price$"), dollar sign followed by non-alphabetic characters (e.g., "$123"), dollar sign followed by valid characters but not matching any operator (e.g., "$unknown"), and operators in various contexts (at start of string, after different delimiters from line 149 of nosql_detect.go like brackets, colons, etc.).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Introduce NoSQLInjectionDetector with a default operator set (e.g. , ) and a 4096-byte default input length cap to detect common NoSQL injection patterns.
Rationale: extend sanitization beyond SQL to cover NoSQL operator-based attacks commonly found in JSON request bodies.