feat(dashboard): provide ns when querying kv

[jog1t]

No description provided.

[jog1t]

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

• feat(dashboard): provide ns when querying kv #3831 : 2 dependent PRs (#3561 , #3803 ) 👈 (View in Graphite)
• chore: update sdks #3830
• refactor: performance improvements for inspector #3452
• fix(dashboard): use proper timestamp for actor status #3809
• fix(engine): require namespace for actor id-specific routes #3828
• main

---

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]

Pull Request Review: feat(dashboard): provide ns when querying kv

Summary

This PR makes several improvements related to KV querying and security:

• Adds namespace parameter when querying KV for inspector tokens
• Refactors timing-safe secret comparison into a reusable utility
• Cleans up frontend code (removes console.log, unused imports)
• Fixes React useEffect dependencies with useRef pattern

Code Quality & Best Practices

✅ Strengths

1. Security Improvement: The new timingSafeEqual function in rivetkit-typescript/packages/rivetkit/src/utils/crypto.ts is well-implemented with proper timing-attack mitigation. The padding to max length (line 14) prevents length-based timing leaks.

2. Good Refactoring: Moving compareSecrets → timingSafeEqual and relocating it to utils/crypto.ts improves code organization and reusability.

3. Proper React Patterns: The useRef pattern in frontend/src/components/actors/worker/actor-worker-context.tsx (lines 81-85) correctly fixes the stale closure issue with engineToken without causing unnecessary effect re-runs.

4. Code Cleanup: Removing console.log statements and unused imports improves code hygiene.

5. Better Export Structure: Moving KV_KEYS to the main actor export makes it more discoverable and properly encapsulates the inspector implementation detail.

⚠️ Issues & Concerns

1. Critical: Incorrect KV Key Usage (frontend/src/app/data-providers/engine-data-provider.tsx:386-388)

KV_KEYS
    // @ts-expect-error
    .toBase64()

Problem: KV_KEYS is an object with properties like INSPECTOR_TOKEN, not an object with a toBase64() method. This should be:

KV_KEYS.INSPECTOR_TOKEN
    // @ts-expect-error
    .toBase64()

The code appears incomplete - KV_KEYS needs to be followed by .INSPECTOR_TOKEN to access the actual key. The @ts-expect-error suppresses the TypeScript error, but this will fail at runtime because objects don't have a toBase64() method.

Impact: This is likely a breaking bug that would cause the inspector token query to fail.

2. Missing Test Coverage

The new timingSafeEqual function has no test coverage. Security-critical functions should have comprehensive tests including:

• Equal strings/buffers of same length
• Different strings/buffers of same length
• Different lengths
• Empty inputs
• Unicode/special characters
• Edge cases (very long inputs)

3. Incomplete Security Implementation

While the timingSafeEqual implementation is good, there's a subtle issue:

// Pad to max length to avoid leaking length information
const maxLength = Math.max(bufferA.byteLength, bufferB.byteLength);
let result = bufferA.byteLength ^ bufferB.byteLength;

The length check at line 15 (result = bufferA.byteLength ^ bufferB.byteLength) still reveals length inequality through early termination. While you do pad and compare all bytes, a sophisticated attacker could potentially measure timing differences. Consider:

// More robust: always use a fixed-length comparison or hash
let result = 0;
for (let i = 0; i < maxLength; i++) {
    const byteA = i < bufferA.byteLength ? bufferA[i] : 0;
    const byteB = i < bufferB.byteLength ? bufferB[i] : 0;
    result |= byteA ^ byteB;
}
// Add final length check without early return
result |= bufferA.byteLength ^ bufferB.byteLength;

However, your current implementation should be secure in practice for typical use cases.

4. Browser Compatibility Consideration

The implementation uses TextEncoder which is well-supported, but the Node.js crypto module's timingSafeEqual is not available in browsers. Your implementation correctly provides a cross-platform solution, but consider documenting this in a comment.

5. Unused Variable in actor-console.tsx (line 22)

const combinedStatus =
    isRpcsError || actorInspector.connectionStatus === "error"
        ? "error"
        : actorInspector.connectionStatus === "connecting" || isRpcsLoading
            ? "pending"
            : "ready";

The combinedStatus variable is computed but appears to be unused in the component. If it's not needed, remove it. If it should be used, this might be a bug.

6. Type Safety Bypass

Multiple @ts-expect-error comments suggest type mismatches that should be properly addressed:

• frontend/src/app/data-providers/engine-data-provider.tsx:387

These suppressions hide potential bugs and make refactoring harder.

Performance Considerations

✅ Good:

• The useRef pattern prevents unnecessary effect re-runs
• Timing-safe comparison has O(n) complexity which is acceptable
• No obvious performance regressions

Security Concerns

⚠️ Medium Priority:

1. The timingSafeEqual implementation is mostly solid but could be slightly more robust (see issue [SVC-2504] Fix 5 GB upload limit for local development from Cloudflare #3 above)
2. Missing test coverage for security-critical code
3. The function should be audited by security experts for production use with sensitive tokens

✅ Good:

• Timing-attack mitigation is properly implemented
• Secret comparison moved to dedicated utility
• No hardcoded secrets or credentials

Recommendations

Must Fix (Blocking):

1. Fix the KV_KEYS usage in engine-data-provider.tsx - add .INSPECTOR_TOKEN after KV_KEYS
2. Verify runtime behavior - test that inspector token querying actually works

Should Fix (High Priority):

3. Add comprehensive tests for timingSafeEqual function
4. Remove or use the combinedStatus variable in actor-console.tsx
5. Fix type errors instead of suppressing them with @ts-expect-error

Nice to Have:

6. Add JSDoc comments to timingSafeEqual explaining when to use it vs Node's native implementation
7. Consider security audit of timing-safe comparison for production critical paths
8. Add unit tests for the namespace parameter in KV queries

Test Coverage Assessment

❌ Missing Tests:

• No tests for new timingSafeEqual utility
• No tests verifying namespace parameter is passed correctly
• No tests for the React ref pattern fix

Recommendation: Add at least basic unit tests for the new crypto utility before merging.

Conclusion

This PR has good intentions and makes several valuable improvements, but has one critical bug that needs immediate attention (the incomplete KV_KEYS usage). The security refactoring is well-done, but needs test coverage. The React patterns are correct.

Recommendation: Request changes to fix the KV_KEYS bug and add test coverage before merging.

---

Files Changed: 13 files (+62/-60 lines)
Risk Level: Medium (one critical bug, but otherwise solid changes)

[pkg-pr-new]

More templates

• actor-actions
• example-ai-agent
• example-ai-and-user-generated-actors-freestyle
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-cloudflare-workers-inline-client
• cross-actor-actions
• example-cursors
• example-cursors-raw-websocket
• custom-serverless
• example-sqlite
• example-elysia
• example-experimental-durable-streams-ai-agent
• hello-world
• example-hono
• hono-react
• example-kitchen-sink
• multi-region
• native-websockets
• next-js
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• scheduling
• state
• example-stream
• example-trpc

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3831

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 9c5ca61

[graphite-app]

Graphite Automations

"Test" took an action on this PR • (01/11/26)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

[graphite-app]

Merge activity

• Jan 12, 9:55 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Jan 12, 9:56 PM UTC: CI is running for this pull request on a draft pull request (#3840) due to your merge queue CI optimization settings.
• Jan 12, 9:56 PM UTC: Merged by the Graphite merge queue via draft PR: #3840.