Skip to content

chore: add tests for validation#112

Merged
hwbrzzl merged 2 commits into
masterfrom
bowen/use-new-validation
Mar 22, 2026
Merged

chore: add tests for validation#112
hwbrzzl merged 2 commits into
masterfrom
bowen/use-new-validation

Conversation

@hwbrzzl
Copy link
Copy Markdown
Contributor

@hwbrzzl hwbrzzl commented Mar 22, 2026

📑 Description

Relate goravel/framework#1417

Main Features

This PR updates the validation test suite and controller logic to support enhanced validation rules and more complex data structures. The key changes include:

  • Extended validation schema - Added support for nested items (with array validation), metadata fields, and age integer field in validation requests
  • Type generalization - Changed validation rule maps from map[string]string to map[string]any throughout the validation framework
  • Custom validation rule - Renamed the Exists rule signature from "exists" to "custom_exists" and removed the NotExists rule entirely
  • Test refactoring - Replaced feature-level HTTP validation tests with focused unit tests covering JSON, form, and request validation with nested structures and comprehensive error assertions
  • Dependency updates - Upgraded Go dependencies including Gin to v1.12.0, OpenTelemetry modules to v1.42.0, and gRPC to v1.79.2

Technical Changes

Controller Updates (validation_controller.go):

  • Extended User struct with Age int, Items []ValidationItem, and Meta map[string]any fields
  • Updated validation rules to use map[string]any and added validation for nested items.*.name and optional meta.name
  • Modified response construction to include the newly bound fields

Request Schema Changes (validation_create.go):

  • Introduced new ValidationItem type with Name field
  • Added Items, Meta, and Age fields to ValidationCreate struct
  • Updated Rules() and Filters() return types from map[string]string to map[string]any

Validation Rule Changes:

  • Renamed Exists rule signature to "custom_exists" for custom validation logic
  • Removed NotExists rule implementation and its bootstrap registration

Test Suite Refactoring (validation_test.go):

  • Replaced feature-level HTTP tests (TestValidationForm, TestValidationJson, TestValidationRequest) with granular unit tests
  • Added new test methods: TestValidateJson, TestValidateForm, TestValidateRequest, and TestValidateMake
  • Enhanced test coverage with nested JSON/form payloads and structured error assertions
  • Updated all validation rule references from map[string]string to map[string]any

✅ Checks

  • Added test cases for my code

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 22, 2026

📝 Walkthrough

Walkthrough

The pull request extends validation schema and controller functionality by adding new fields (Age, Items, Meta) to validation requests, changes validation rule type signatures from map[string]string to map[string]any, renames the "exists" rule to "custom_exists", removes the "not_exists" rule entirely, and updates corresponding test cases and Go module dependencies to reflect these structural changes.

Changes

Cohort / File(s) Summary
Validation Controller & Request Schema
app/http/controllers/validation_controller.go, app/http/requests/validation_create.go, app/http/requests/user_create.go
Extended ValidationCreate and User structs with new fields (Age int, Items []ValidationItem, Meta map[string]any). Changed Rules() and Filters() method return types from map[string]string to map[string]any. Updated controllers to validate and include new fields in response payloads.
Validation Rules
app/rules/exists.go, app/rules/not_exists.go, bootstrap/rules.go
Renamed "exists" validation rule to "custom_exists" via Signature(). Removed NotExists rule implementation entirely and deregistered it from bootstrap.
Unit & Integration Tests
app/http/controllers/validation_controller_test.go, tests/feature/validation_test.go, tests/feature/http_test.go
Updated test mocks to expect map[string]any validation rules. Removed three HTTP integration test cases (TestValidationForm, TestValidationJson, TestValidationRequest). Refactored validation test cases to use new rule naming and validate new nested fields (items, meta, age).
Dependency Management
go.mod
Updated direct dependencies: gin to v1.12.0, framework, fiber, and gin packages to newer pseudo-versions. Upgraded OpenTelemetry modules from v1.40.0 to v1.42.0, grpc to v1.79.2, and numerous indirect transitive dependencies. Updated replace directive for framework.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • chore: add validation tests #111: Directly related—modifies the same validation controller and validation test behavior, sharing code-level changes in validation_controller.go and validation test fixtures.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'chore: add tests for validation' does not match the primary changes, which include substantial structural modifications to validation logic, request handling, rule signatures, and dependency updates beyond just adding tests. Revise the PR title to better reflect the main scope of changes, such as 'refactor: extend validation with new rules, types, and map-based validation' or break into multiple PRs separating testing from structural changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bowen/use-new-validation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@hwbrzzl hwbrzzl changed the title optimize chore: add test for validation Mar 22, 2026
@hwbrzzl hwbrzzl changed the title chore: add test for validation chore: add tests for validation Mar 22, 2026
@hwbrzzl hwbrzzl marked this pull request as ready for review March 22, 2026 07:21
@hwbrzzl hwbrzzl requested a review from a team as a code owner March 22, 2026 07:21
@hwbrzzl
Copy link
Copy Markdown
Contributor Author

hwbrzzl commented Mar 22, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
app/http/requests/validation_create.go (1)

52-66: ⚠️ Potential issue | 🟡 Minor

Unsafe type assertion in PrepareForValidation.

At line 60, c.(string) and ctx.Value("ctx").(string) are direct type assertions that could panic if the values are nil or not strings.

🛡️ Proposed defensive fix
 func (r *ValidationCreate) PrepareForValidation(ctx http.Context, data validation.Data) error {
 	if scores, exist := data.Get("scores"); exist {
 		if err := data.Set("scores", cast.ToIntSlice(scores)); err != nil {
 			return err
 		}
 	}
 	if c, exist := data.Get("context"); exist {
-		// Test getting value from context: ValidationController.Request
-		if err := data.Set("context", c.(string)+"_"+ctx.Value("ctx").(string)); err != nil {
-			return err
+		cStr, ok := c.(string)
+		if !ok {
+			return nil
+		}
+		ctxVal := ctx.Value("ctx")
+		ctxStr, ok := ctxVal.(string)
+		if !ok {
+			return nil
+		}
+		if err := data.Set("context", cStr+"_"+ctxStr); err != nil {
+			return err
 		}
 	}

 	return nil
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/http/requests/validation_create.go` around lines 52 - 66, In
PrepareForValidation (type ValidationCreate) avoid unsafe type assertions for
c.(string) and ctx.Value("ctx").(string) which can panic; instead, check types
using the comma-ok idiom (e.g. v, ok := c.(string) and ctxVal, ok :=
ctx.Value("ctx").(string)), handle missing or wrong-type cases by returning an
error (or a safe fallback) before calling data.Set("context", ...), and keep the
existing scores cast logic unchanged; update the code paths in
PrepareForValidation to validate types and return a clear error when either
value is absent or not a string.
app/rules/exists.go (2)

29-33: ⚠️ Potential issue | 🟠 Major

Unsafe type assertions and missing bounds checks may cause runtime panics.

The code directly accesses options[0], options[1], and type-asserts val without any safety checks. If the rule is invoked with fewer than 2 options or with non-string values, this will panic at runtime.

🛡️ Proposed fix to add safety checks
 func (receiver *Exists) Passes(ctx context.Context, _ validation.Data, val any, options ...any) bool {
+	if len(options) < 2 {
+		return false
+	}
+	tableName, ok := options[0].(string)
+	if !ok {
+		return false
+	}
+	fieldName, ok := options[1].(string)
+	if !ok {
+		return false
+	}
+	requestValue, ok := val.(string)
+	if !ok {
+		return false
+	}
-	tableName := options[0].(string)
-	fieldName := options[1].(string)
-	requestValue := val.(string)

 	if len(requestValue) == 0 {
 		return false
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/rules/exists.go` around lines 29 - 33, In Exists.Passes ensure you guard
against panics by validating inputs before type assertions: check len(options)
>= 2, use safe type assertions for options[0] and options[1] into strings (e.g.,
ok idiom) to populate tableName and fieldName, and safely assert val is a string
into requestValue (or handle non-string values); if any check fails, return
false (or an appropriate error/false result) instead of letting the function
panic. This targets the Exists.Pass(es) function where options, tableName,
fieldName and requestValue are accessed.

41-45: ⚠️ Potential issue | 🟡 Minor

Unsafe type assertion in loop for additional field options.

The loop at lines 42-44 also performs an unsafe type assertion options[i].(string) which could panic if a non-string value is passed.

🛡️ Proposed fix
 	if len(options) > 2 {
 		for i := 2; i < len(options); i++ {
-			query = query.OrWhere(options[i].(string), requestValue)
+			fieldOpt, ok := options[i].(string)
+			if !ok {
+				continue
+			}
+			query = query.OrWhere(fieldOpt, requestValue)
 		}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/rules/exists.go` around lines 41 - 45, The loop in app/rules/exists.go
uses an unsafe type assertion options[i].(string) which can panic; update the
loop that builds query.OrWhere to validate each options[i] before using it
(e.g., perform a type assertion with the two-value form or a type switch), and
handle non-string values safely by skipping them or returning an error instead
of panicking; reference the existing query.OrWhere call, the options slice and
requestValue when implementing this check so only valid strings are passed into
OrWhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/http/controllers/validation_controller.go`:
- Around line 93-102: The code builds a response using
user.Date.ToDateTimeString() without guarding against a nil pointer; update the
response construction in validation controller (where response, user and
ctx.Response().Success().Json are used) to nil-check user.Date before calling
ToDateTimeString() — e.g. determine a safe date value (empty string or nil) by
checking if user.Date != nil and only calling ToDateTimeString() when non-nil,
then include that safe value in the "date" field of the response.

In `@go.mod`:
- Line 34: Update the vulnerable module versions in go.mod: bump
google.golang.org/grpc to v1.79.3 or later to address the :path pseudo-header
auth bypass, and ensure the indirect dependency github.com/gofiber/utils is
upgraded to v2.0.0-rc.3 or later to eliminate the predictable-UUID fallback;
after editing the module versions run dependency resolution (go mod tidy / go
get equivalent) to update go.sum and vendor, then run the test suite and your
dependency vulnerability scanner to verify the fixes.

---

Outside diff comments:
In `@app/http/requests/validation_create.go`:
- Around line 52-66: In PrepareForValidation (type ValidationCreate) avoid
unsafe type assertions for c.(string) and ctx.Value("ctx").(string) which can
panic; instead, check types using the comma-ok idiom (e.g. v, ok := c.(string)
and ctxVal, ok := ctx.Value("ctx").(string)), handle missing or wrong-type cases
by returning an error (or a safe fallback) before calling data.Set("context",
...), and keep the existing scores cast logic unchanged; update the code paths
in PrepareForValidation to validate types and return a clear error when either
value is absent or not a string.

In `@app/rules/exists.go`:
- Around line 29-33: In Exists.Passes ensure you guard against panics by
validating inputs before type assertions: check len(options) >= 2, use safe type
assertions for options[0] and options[1] into strings (e.g., ok idiom) to
populate tableName and fieldName, and safely assert val is a string into
requestValue (or handle non-string values); if any check fails, return false (or
an appropriate error/false result) instead of letting the function panic. This
targets the Exists.Pass(es) function where options, tableName, fieldName and
requestValue are accessed.
- Around line 41-45: The loop in app/rules/exists.go uses an unsafe type
assertion options[i].(string) which can panic; update the loop that builds
query.OrWhere to validate each options[i] before using it (e.g., perform a type
assertion with the two-value form or a type switch), and handle non-string
values safely by skipping them or returning an error instead of panicking;
reference the existing query.OrWhere call, the options slice and requestValue
when implementing this check so only valid strings are passed into OrWhere.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 794c50d2-3ea3-4a6e-a070-6cb6e4d5665c

📥 Commits

Reviewing files that changed from the base of the PR and between 49c2b2a and ce84007.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (10)
  • app/http/controllers/validation_controller.go
  • app/http/controllers/validation_controller_test.go
  • app/http/requests/user_create.go
  • app/http/requests/validation_create.go
  • app/rules/exists.go
  • app/rules/not_exists.go
  • bootstrap/rules.go
  • go.mod
  • tests/feature/http_test.go
  • tests/feature/validation_test.go
💤 Files with no reviewable changes (3)
  • bootstrap/rules.go
  • tests/feature/http_test.go
  • app/rules/not_exists.go

Comment on lines +93 to +102
response := http.Json{
"context": user.Context,
"name": user.Name,
"date": user.Date.ToDateTimeString(),
})
"age": user.Age,
"items": user.Items,
"meta": user.Meta,
}

return ctx.Response().Success().Json(response)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential nil pointer dereference on user.Date.

At line 96, user.Date.ToDateTimeString() is called without checking if Date is nil. Since Date is a pointer type (*carbon.DateTime), this could cause a panic if the validation passes but the date field is somehow not populated during binding.

🛡️ Proposed defensive check
 	response := http.Json{
 		"context": user.Context,
 		"name":    user.Name,
-		"date":    user.Date.ToDateTimeString(),
+		"date":    "",
 		"age":     user.Age,
 		"items":   user.Items,
 		"meta":    user.Meta,
 	}
+	if user.Date != nil {
+		response["date"] = user.Date.ToDateTimeString()
+	}

 	return ctx.Response().Success().Json(response)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
response := http.Json{
"context": user.Context,
"name": user.Name,
"date": user.Date.ToDateTimeString(),
})
"age": user.Age,
"items": user.Items,
"meta": user.Meta,
}
return ctx.Response().Success().Json(response)
response := http.Json{
"context": user.Context,
"name": user.Name,
"date": "",
"age": user.Age,
"items": user.Items,
"meta": user.Meta,
}
if user.Date != nil {
response["date"] = user.Date.ToDateTimeString()
}
return ctx.Response().Success().Json(response)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/http/controllers/validation_controller.go` around lines 93 - 102, The
code builds a response using user.Date.ToDateTimeString() without guarding
against a nil pointer; update the response construction in validation controller
(where response, user and ctx.Response().Success().Json are used) to nil-check
user.Date before calling ToDateTimeString() — e.g. determine a safe date value
(empty string or nil) by checking if user.Date != nil and only calling
ToDateTimeString() when non-nil, then include that safe value in the "date"
field of the response.

Comment thread go.mod
google.golang.org/grpc v1.78.0
go.opentelemetry.io/otel v1.42.0
go.opentelemetry.io/otel/metric v1.42.0
google.golang.org/grpc v1.79.2
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for latest versions and security advisories

echo "=== Checking grpc security advisories ==="
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: GO, package: "google.golang.org/grpc") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

echo ""
echo "=== Checking gofiber/utils security advisories ==="
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/gofiber/utils") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Repository: goravel/example

Length of output: 456


🏁 Script executed:

cat go.mod | head -150

Repository: goravel/example

Length of output: 7429


🌐 Web query:

GHSA-p77j-4mvh-x3m3 google.golang.org/grpc vulnerability

💡 Result:

GHSA-p77j-4mvh-x3m3 is a critical authorization bypass vulnerability (CVSS 9.1) in google.golang.org/grpc, also known as CVE-2026-33186. It affects versions before 1.79.3. The gRPC-Go server accepts HTTP/2 requests with malformed :path pseudo-headers missing the leading slash (e.g., "Service/Method" instead of "/Service/Method"). The server routes these to the correct handler, but authorization interceptors like grpc/authz evaluate the raw path. Deny rules for canonical paths (with "/") fail to match, allowing bypass if a fallback allow rule exists. Exploitation requires sending raw HTTP/2 frames, network access, path-based auth interceptors with deny rules + fallback allow. No known public exploits. Fix: Upgrade to v1.79.3 or later, where non-canonical paths are rejected with Unimplemented error. Published March 17, 2026.

Citations:


🌐 Web query:

GHSA-m98w-cqp3-qcqr github.com/gofiber/utils vulnerability

💡 Result:

GHSA-m98w-cqp3-qcqr is a critical vulnerability (CVSS 9.2) in github.com/gofiber/utils affecting the UUIDv4 and UUID functions. When crypto/rand fails (on Go <1.24), they silently return predictable values like the zero UUID "00000000-0000-0000-0000-000000000000", compromising security in Fiber apps using these for session IDs, CSRF tokens, etc. CVE-2025-66565. Published Dec 2025. Affected: github.com/gofiber/utils <=1.1.0; github.com/gofiber/utils/v2 <=2.0.0-rc.3 (last known vulnerable: <2.0.0-rc.3.0.20251205210924-6c6cf047032b). Patched in: gofiber/utils@6c6cf04 (Dec 2025, post-advisory). Update to >=2.0.0-rc.3.0.20251205210924-6c6cf047032b (v2 branch). Go >=1.24 is unaffected as crypto/rand panics on failure. Workaround: Replace with github.com/google/uuid.New. CWE-252,331,338.

Citations:


🌐 Web query:

GO-2025-4208 golang vulnerability

💡 Result:

GO-2025-4208 is a vulnerability in the github.com/gofiber/utils and github.com/gofiber/utils/v2 Go packages. It affects UUID and UUIDv4 functions, which silently fall back to predictable values (including the zero UUID "00000000-0000-0000-0000-000000000000") when crypto/rand.Read fails. This compromises security for applications using these for critical operations. Affected versions: - github.com/gofiber/utils >=1.2.0 (no fixed version listed explicitly, but related to pre-fix commits) - github.com/gofiber/utils/v2 from 2.0.0-rc.4 (earlier rc versions and below) Aliases: CVE-2025-66565, GHSA-m98w-cqp3-qcqr Published: 2025-12-15 Fixed: In github.com/gofiber/utils commit 6c6cf047032b9c8dff43d29f990b4b10e9b02d47 (v2.0.0-rc.4 for v2). Note: Go 1.24+ panics on crypto/rand failures, mitigating the silent fallback. The report is unreviewed (auto-generated). Official Go security page: https://pkg.go.dev/vuln/GO-2025-4208 CVSS v3.1: 9.8 (Critical), allows high impact if rand fails (e.g., resource exhaustion or misconfiguration). Mitigation: Update to fixed versions, avoid using these UUID functions for security-sensitive purposes, ensure crypto/rand availability.

Citations:


Critical security vulnerabilities require immediate updates.

Two critical vulnerabilities affect the current dependencies:

  1. google.golang.org/grpc v1.79.2: Authorization bypass via missing leading slash in :path pseudo-headers (GHSA-p77j-4mvh-x3m3, CVSS 9.1). Upgrade to v1.79.3 or later.

  2. github.com/gofiber/utils v1.1.0 (indirect): UUIDv4 and UUID functions silently fall back to predictable values when crypto/rand fails (GHSA-m98w-cqp3-qcqr / GO-2025-4208, CVSS 9.2). Upgrade to v2.0.0-rc.3 or later.

🧰 Tools
🪛 OSV Scanner (2.3.3)

[CRITICAL] 34-34: google.golang.org/grpc 1.79.2: gRPC-Go has an authorization bypass via missing leading slash in :path

(GHSA-p77j-4mvh-x3m3)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@go.mod` at line 34, Update the vulnerable module versions in go.mod: bump
google.golang.org/grpc to v1.79.3 or later to address the :path pseudo-header
auth bypass, and ensure the indirect dependency github.com/gofiber/utils is
upgraded to v2.0.0-rc.3 or later to eliminate the predictable-UUID fallback;
after editing the module versions run dependency resolution (go mod tidy / go
get equivalent) to update go.sum and vendor, then run the test suite and your
dependency vulnerability scanner to verify the fixes.

@hwbrzzl hwbrzzl merged commit 128ca12 into master Mar 22, 2026
5 checks passed
@hwbrzzl hwbrzzl deleted the bowen/use-new-validation branch March 22, 2026 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant