chore(rivetkit): add required zod validation for json encoding #3431

NathanFlurry · 2025-11-10T03:56:32Z

No description provided.

vercel · 2025-11-10T03:56:39Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Nov 13, 2025 2:49am

3 Skipped Deployments

Project	Deployment	Preview	Updated (UTC)
rivet-cloud	Ignored		Nov 13, 2025 2:49am
rivet-inspector	Ignored	Preview	Nov 13, 2025 2:49am
rivet-site	Ignored	Preview	Nov 13, 2025 2:49am

NathanFlurry · 2025-11-10T03:57:04Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

claude · 2025-11-10T03:58:10Z

Code Review: Add Required Zod Validation for JSON Encoding

Summary

This PR adds Zod schema validation to the JSON encoding path in RivetKit's serialization/deserialization flow. The changes ensure type safety when using JSON encoding for client-server communication, while maintaining backward compatibility with CBOR and bare encodings.

Positive Aspects

✅ Strong Type Safety: The new Zod schemas provide runtime validation for JSON-encoded messages, catching malformed data early
✅ Comprehensive Coverage: All protocol message types are covered (ToClient, ToServer, HTTP actions, errors, etc.)
✅ Consistent Pattern: The validation is applied uniformly across all serialization/deserialization call sites
✅ Backward Compatible: CBOR and bare encodings remain unchanged, only JSON gets validation

Issues & Concerns

🔴 Critical: ArrayBuffer Schema Validation Issue

Location: rivetkit-typescript/packages/rivetkit/src/actor/client-protocol-schema-json/mod.ts:4

const ArrayBufferSchema = z.instanceof(ArrayBuffer);

Problem: This schema will fail validation for JSON-encoded data because:

When data is JSON-encoded, ArrayBuffers are converted to base64 strings via jsonStringifyCompat (see src/actor/protocol/serde.ts:154-176)
When JSON-decoded, these base64 strings are converted back to ArrayBuffers via jsonParseCompat (see src/actor/protocol/serde.ts:178-210)
However, Zod validation happens after JSON.parse but before the custom reviver in jsonParseCompat processes the special types

Expected flow:

Serialize: ArrayBuffer -> ["", base64] -> JSON string
Deserialize: JSON string -> parse -> validate with Zod -> revive special types

But validation happens on the raw parsed JSON (where ArrayBuffers are still ["", base64] arrays), not on the final revived objects.

Impact: All JSON-encoded messages with ArrayBuffer fields will fail validation, breaking JSON encoding completely.

Recommended Fix: The schemas should match the intermediate JSON representation:

const ArrayBufferSchema = z.tuple([z.literal(""), z.string()]);
const UintSchema = z.tuple([z.literal(""), z.string()]);

Or alternatively, validate after custom parsing, not before.

⚠️ Major: Type Safety Escape Hatch

Location: rivetkit-typescript/packages/rivetkit/src/remote-manager-driver/api-utils.ts:55-56

requestZodSchema: z.any() as z.ZodType<TInput>,
responseZodSchema: z.any() as z.ZodType<TOutput>,

Problem: Using z.any() defeats the entire purpose of this PR. The apiCall function bypasses all validation.

Impact: API calls won't benefit from the validation layer, creating inconsistent behavior between different code paths.

Recommended Fix: Either:

Require callers to pass explicit schemas
Create a registry of schemas for known API endpoints
At minimum, use z.unknown() instead of z.any() to preserve some type checking

⚠️ Medium: Validation Order Issue

Location: rivetkit-typescript/packages/rivetkit/src/serde.ts:62-64 and 85-94

// Serialize
const validated = zodSchema.parse(value);
return jsonStringifyCompat(validated);

// Deserialize  
parsed = jsonParseCompat(jsonString);
return zodSchema.parse(parsed);

Problem: For deserialization, you're validating the data structure after JSON.parse but before jsonParseCompat applies the custom reviver that converts special types. This means:

BigInts are validated as ["", string] arrays, not as bigint
ArrayBuffers are validated as ["", string] arrays, not as ArrayBuffer

Current flow:

JSON.parse -> Zod validation -> jsonParseCompat reviver

Expected flow (likely):

JSON.parse + jsonParseCompat reviver -> Zod validation

Impact: Schema definitions don't match the actual runtime types they're meant to validate.

ℹ️ Minor: Type Inconsistency

Location: rivetkit-typescript/packages/rivetkit/src/actor/protocol/serde.ts:25-26

export class CachedSerializer<T, TJson = T> {
	#data: T | TJson;

Observation: The #data field accepts either T or TJson, but there's no runtime tracking of which type it actually contains. This could lead to incorrect serialization if a TJson value is passed with bare encoding (line 73 casts to T unconditionally).

Recommendation: Consider narrowing the type or adding runtime guards.

Testing Recommendations

Given the critical issue with ArrayBuffer validation, I strongly recommend:

Add tests that verify JSON encoding/decoding with:
- Messages containing ArrayBuffer fields
- Messages containing BigInt fields
- Nested structures with these types
Test the validation actually rejects malformed data (wrong types, missing fields, etc.)
Test all three encodings (json, cbor, bare) to ensure no regressions
Add integration tests covering the full serialization pipeline

Performance Considerations

The Zod validation adds overhead to every JSON-encoded message. Consider:

Benchmarking the performance impact on high-throughput scenarios
Whether validation should be optional in production (with a config flag)
Caching parsed schemas if not already done by Zod

Conclusion

While this PR adds valuable type safety, the ArrayBuffer schema validation issue is a blocker that will break JSON encoding. This needs to be fixed before merging. Once the validation logic is corrected to match the actual serialization format, this will be a solid improvement to the codebase.

Recommendation: ❌ Request changes - fix critical validation issue before merge

pkg-pr-new · 2025-11-10T03:58:50Z

More templates

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/actor@3431

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3431

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/core@3431

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3431

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3431

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3431

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3431

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3431

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3431

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3431

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3431

commit: be3dc7d

claude · 2025-11-12T17:23:00Z

Code Review: Add Required Zod Validation for JSON Encoding

Overview

This PR adds Zod schema validation for JSON encoding in the RivetKit client protocol. The implementation creates comprehensive schemas and integrates validation into the serialization/deserialization pipeline.

✅ Strengths

Type Safety Enhancement: The addition of Zod schemas provides runtime validation that catches malformed data early in the JSON encoding path.
Comprehensive Schema Coverage: The new client-protocol-schema-json/mod.ts file covers all protocol messages (ToClient, ToServer, HTTP actions, errors, etc.) with proper discriminated unions.
Minimal API Surface Changes: The changes are largely additive - existing code continues to work while gaining validation benefits for JSON encoding.
Consistent Pattern: The validation is applied consistently across all serialization/deserialization call sites.

⚠️ Issues & Concerns

1. CBOR and BARE Encodings Bypass Validation (Critical)

The validation only runs for JSON encoding in serde.ts:56-77:

export function serializeWithEncoding<T, TJson = T>(
  encoding: Encoding,
  value: T | TJson,
  versionedDataHandler: VersionedDataHandler<T> | undefined,
  zodSchema: z.ZodType<TJson>,
): Uint8Array | string {
  if (encoding === "json") {
    const validated = zodSchema.parse(value);  // ✅ Validated
    return jsonStringifyCompat(validated);
  } else if (encoding === "cbor") {
    return cbor.encode(value);  // ❌ NOT validated
  } else if (encoding === "bare") {
    return versionedDataHandler.serializeWithEmbeddedVersion(value as T);  // ❌ NOT validated
  }
}

Impact: Malformed data can still pass through if using CBOR or BARE encoding. Consider either:

Validating for all encoding types (with appropriate type casting)
Documenting that validation is JSON-only
Renaming schemas to indicate JSON-specificity (e.g., ToClientJsonSchema)

2. ArrayBuffer Schema May Be Too Permissive

In client-protocol-schema-json/mod.ts:4:

const ArrayBufferSchema = z.instanceof(ArrayBuffer);

Issue: This accepts any ArrayBuffer without validating structure or contents. For protocol messages, you may want to:

Add length constraints where applicable
Consider validating that buffers contain valid CBOR-encoded data for nested structures
Document expected buffer formats

3. Use of `z.any()` in API Utils (Security Risk)

In remote-manager-driver/api-utils.ts:55-56:

requestZodSchema: z.any() as z.ZodType<TInput>,
responseZodSchema: z.any() as z.ZodType<TOutput>,

Problem: This completely bypasses validation for all API calls, defeating the purpose of adding validation.

Recommendation:

Create specific schemas for each API endpoint
Use z.unknown() instead of z.any() to maintain type safety while requiring explicit parsing
Add a comment explaining why validation is skipped if intentional

4. Performance Impact Not Assessed

Concern: Zod validation runs on every JSON serialization/deserialization. For high-throughput scenarios:

Validation overhead could be significant
No benchmarks or performance testing mentioned
Consider adding a development-only mode or caching validated schemas

Recommendation: Profile the performance impact, especially for:

WebSocket message handling in actor-conn.ts
Event broadcasting in event-manager.ts
High-frequency action requests/responses

5. Inconsistent Generic Type Handling

The new generic parameter TJson is added but the relationship between T and TJson is unclear:

export function serializeWithEncoding<T, TJson = T>(
  encoding: Encoding,
  value: T | TJson,  // Why union type here?
  // ...
): Uint8Array | string {

Questions:

When would T differ from TJson?
Why accept T | TJson as input?
Should there be a constraint like TJson extends T?

This adds complexity without clear documentation of the intent.

🧪 Test Coverage

Missing:

No tests for the new Zod schemas
No tests verifying validation failures are caught
No tests for edge cases (null metadata, optional fields, etc.)
No integration tests ensuring CBOR/BARE still work

Recommendation: Add tests in a new file like actor/protocol/serde.test.ts covering:

// Valid cases
✓ JSON serialization with valid data
✓ CBOR serialization (validation bypass)
✓ BARE serialization (validation bypass)

// Invalid cases  
✓ JSON serialization with invalid schema throws
✓ Missing required fields caught
✓ Wrong type fields caught

// Edge cases
✓ Null optional fields handled
✓ ArrayBuffer handling
✓ BigInt handling

🎯 Recommendations

High Priority:

Replace z.any() in api-utils.ts with proper schemas or z.unknown()
Add comprehensive tests for validation behavior
Document validation scope (JSON-only) in code comments
Clarify T vs TJson generic relationship

Medium Priority:
5. Profile performance impact and consider optimization strategies
6. Validate all encodings or document why only JSON is validated
7. Add length/format constraints to ArrayBuffer schemas where applicable

Low Priority:
8. Consider extracting shared schema patterns (e.g., base64 encoded buffers)
9. Add JSDoc comments to exported schemas explaining their use

🔐 Security Assessment

Positive: Runtime validation prevents some classes of injection/corruption attacks via malformed JSON.

Concerns:

Bypassing validation for CBOR/BARE creates an inconsistent security posture
z.any() usage is a validation escape hatch that should be avoided

📊 Overall Assessment

This is a solid improvement that adds important runtime validation for JSON encoding. However, the incomplete coverage (CBOR/BARE bypass, z.any() usage) and lack of testing are significant gaps that should be addressed before merge.

Recommendation: Request changes to address the high-priority items, particularly:

Proper schema usage in api-utils.ts
Test coverage for validation paths
Documentation of validation scope

Great work on the schema definitions themselves - they're comprehensive and well-structured! 🎉

claude · 2025-11-13T01:37:03Z

PR Review: Add Required Zod Validation for JSON Encoding

Overview

This PR adds Zod schema validation for JSON encoding in RivetKit's client-server protocol, ensuring type safety and runtime validation when using JSON as the encoding format. The implementation is generally solid but there are a few concerns to address.

Strengths

Type Safety Enhancement: Adding Zod validation for JSON encoding significantly improves runtime type safety and catches serialization/deserialization errors early.
Comprehensive Schema Coverage: The new client-protocol-schema-json/mod.ts file covers all protocol messages with proper use of discriminated unions for tagged message types.
Consistent Application: The schemas are consistently applied across all serialization/deserialization call sites (WebSocket connections, server-side message processing, HTTP action handling, event manager).
Backward Compatibility: The changes maintain backward compatibility by only validating JSON encoding (not CBOR or bare) and keeping existing VersionedDataHandler for non-JSON formats.
Clean Architecture: The generic type parameters allow for flexible typing where JSON schemas can differ from internal representations.

Issues and Concerns

1. Security: Use of z.any() in API Utils (Priority: High)

Location: src/remote-manager-driver/api-utils.ts:55-56

Using z.any() completely bypasses validation, defeating the purpose of this PR. This creates a security and data integrity risk.

Recommendation: Replace with z.unknown() at minimum, or better yet, define proper Zod schemas for the API endpoints that use this utility.

2. ArrayBuffer Schema Validation (Priority: Medium)

Location: src/actor/client-protocol-schema-json/mod.ts:4

z.instanceof(ArrayBuffer) may not work correctly across different execution contexts or after JSON serialization where ArrayBuffers are converted to base64 strings. Consider using a custom Zod schema that validates the array format that jsonStringifyCompat produces.

3. Missing Test Coverage (Priority: Medium)

No new tests were added for the Zod validation logic. Tests should verify that validation catches malformed JSON messages, invalid discriminated union tags, missing required fields, and wrong types for bigint/ArrayBuffer.

4. Error Handling (Priority: Low)

Using .parse() throws errors synchronously. Consider using .safeParse() and handling validation errors more gracefully with better context about what was being validated.

5. Type Complexity (Priority: Low)

The dual generic types add complexity. Add JSDoc comments explaining when T !== TJson and why, documenting that TJson represents the JSON-serializable form while T represents the internal representation.

Additional Observations

Positive: The use of discriminated unions is excellent for type narrowing and validation
Positive: Proper handling of nullable types with OptionalArrayBufferSchema and OptionalUintSchema
Positive: The schemas align well with the existing jsonStringifyCompat/jsonParseCompat logic

Checklist for Author

Replace z.any() usage in api-utils.ts with proper schemas or at minimum z.unknown()
Add validation tests for the new Zod schemas
Consider improving ArrayBuffer schema validation to handle JSON representation
Use .safeParse() instead of .parse() for better error handling
Add JSDoc comments explaining the T vs TJson generic types

Verdict

Approve with changes requested. The core implementation is solid and adds valuable type safety. However, the use of z.any() in API utils undermines the security benefits and should be addressed before merging. The missing tests are also concerning for such a critical change to the serialization layer.

Once the z.any() issue is resolved and basic tests are added, this will be a strong improvement to the codebase.

claude · 2025-11-13T03:04:25Z

Code Review - PR #3431

This PR adds Zod schema validation for JSON-encoded protocol messages in RivetKit.

Positive Aspects

Security & Data Integrity: Adding Zod validation provides runtime type checking for JSON encoding
Good Type Safety: Proper use of TypeScript generics (TJson parameter)
Comprehensive Coverage: All protocol message types covered with discriminated unions
Backward Compatible: Only affects JSON encoding

Critical Issues

Ineffective Validation for CBOR/Bare - Validation only applies to JSON encoding (src/serde.ts:56-77). CBOR and bare encodings bypass validation entirely.
z.any() in api-utils.ts:55-56 - Using z.any() defeats the purpose of validation for remote manager API calls.
Unsafe Type Cast at src/serde.ts:73 - The 'as T' cast is unsafe when value might be TJson type.
Missing Test Coverage - No tests found for the new validation logic.
Performance Concern - CachedSerializer validates on every serialize() call, not cached.
Schema Mismatch Risk - Zod schemas manually maintained separately from protocol definitions.

Recommendations Before Merge

Critical:

Address z.any() usage or add explanatory comment
Add comprehensive test coverage
Document JSON-only validation behavior

Important:

Consider validating all encodings or document why excluded
Add schema maintenance documentation

See full details in src/serde.ts, src/actor/protocol/serde.ts, and src/remote-manager-driver/api-utils.ts

NathanFlurry mentioned this pull request Nov 10, 2025

chore(rivetkit): close websockets on out of sequence messages #3404

Open

This was referenced Nov 10, 2025

chore(rivetkit): add listing actors by only name to manager api #3430

Open

feat(rivetkit): add json protocol support #3432

Open

vercel bot deployed to Preview – rivetkit-serverless November 10, 2025 03:58 View deployment

This was referenced Nov 10, 2025

feat(rivetkit): add asyncapi spec #3442

Open

chore(rivetkit): add support for conn params on onRequest and onWebSocket #3445

Open

feat(site): add typedoc docs #3446

Open

chore(rivetkit): remove deprecated packages #3447

Open

NathanFlurry requested a review from MasterPtato November 11, 2025 00:10

jog1t force-pushed the 11-09-chore_rivetkit_add_required_zod_validation_for_json_encoding branch from cefe601 to 7205295 Compare November 12, 2025 17:06

vercel bot deployed to Preview – rivetkit-serverless November 12, 2025 17:07 View deployment

jog1t mentioned this pull request Nov 12, 2025

fix(fe): use publishable token for integrating next.js with other services #3451

Draft

jog1t mentioned this pull request Nov 12, 2025

refactor: performance improvements for inspector #3452

Draft

NathanFlurry force-pushed the 11-09-chore_rivetkit_add_required_zod_validation_for_json_encoding branch from 7205295 to d27a9f0 Compare November 13, 2025 01:31

vercel bot deployed to Preview – rivetkit-serverless November 13, 2025 01:32 View deployment

chore(rivetkit): add required zod validation for json encoding

be3dc7d

NathanFlurry force-pushed the 11-09-chore_rivetkit_add_required_zod_validation_for_json_encoding branch from d27a9f0 to be3dc7d Compare November 13, 2025 02:47

vercel bot deployed to Preview – rivetkit-serverless November 13, 2025 02:49 View deployment

This was referenced Nov 13, 2025

chore(rivetkit-typescript): remove dependency on node modules #3460

Open

chore(rivetkit): switch dynamic node imports to use require #3461

Open

chore(rivetkit): fix type checks #3462

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(rivetkit): add required zod validation for json encoding #3431

chore(rivetkit): add required zod validation for json encoding #3431

NathanFlurry commented Nov 10, 2025

Uh oh!

vercel bot commented Nov 10, 2025 •

edited

Loading

Uh oh!

NathanFlurry commented Nov 10, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 10, 2025

Uh oh!

pkg-pr-new bot commented Nov 10, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 12, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chore(rivetkit): add required zod validation for json encoding #3431

Are you sure you want to change the base?

chore(rivetkit): add required zod validation for json encoding #3431

Conversation

NathanFlurry commented Nov 10, 2025

Uh oh!

vercel bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NathanFlurry commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

claude bot commented Nov 10, 2025

Code Review: Add Required Zod Validation for JSON Encoding

Summary

Positive Aspects

Issues & Concerns

🔴 Critical: ArrayBuffer Schema Validation Issue

⚠️ Major: Type Safety Escape Hatch

⚠️ Medium: Validation Order Issue

ℹ️ Minor: Type Inconsistency

Testing Recommendations

Performance Considerations

Conclusion

Uh oh!

pkg-pr-new bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Nov 12, 2025

Code Review: Add Required Zod Validation for JSON Encoding

Overview

✅ Strengths

⚠️ Issues & Concerns

1. CBOR and BARE Encodings Bypass Validation (Critical)

2. ArrayBuffer Schema May Be Too Permissive

3. Use of z.any() in API Utils (Security Risk)

4. Performance Impact Not Assessed

5. Inconsistent Generic Type Handling

🧪 Test Coverage

🎯 Recommendations

🔐 Security Assessment

📊 Overall Assessment

Uh oh!

claude bot commented Nov 13, 2025

PR Review: Add Required Zod Validation for JSON Encoding

Overview

Strengths

Issues and Concerns

1. Security: Use of z.any() in API Utils (Priority: High)

2. ArrayBuffer Schema Validation (Priority: Medium)

3. Missing Test Coverage (Priority: Medium)

4. Error Handling (Priority: Low)

5. Type Complexity (Priority: Low)

Additional Observations

Checklist for Author

Verdict

Uh oh!

claude bot commented Nov 13, 2025

Code Review - PR #3431

Positive Aspects

Critical Issues

Recommendations Before Merge

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Nov 10, 2025 •

edited

Loading

NathanFlurry commented Nov 10, 2025 •

edited

Loading

pkg-pr-new bot commented Nov 10, 2025 •

edited

Loading

3. Use of `z.any()` in API Utils (Security Risk)