[St0rmz1]

Summary

Loosen sandbox ownership checks in kilo-chat to accept the instance keyed ki_<uuid hex> sandboxId form when the database row still stores the legacy userId derived value. Restores chat for users whose kiloclaw Durable Object has moved to the new sandboxId format while their kiloclaw_instances.sandbox_id row has not. The same tolerance is applied to the companion owner lookup so bot status pushes and bot conversation creation also work for the same population.

services/kilo-chat/src/services/sandbox-ownership.ts:

• New helper candidateInstanceIdFromSandboxId(sandboxId) returns the instance UUID a ki_ form references, or null. Revalidates with isValidInstanceId before returning so non hex content cannot reach the uuid column and produce a Postgres cast error (which would turn a 4xx into a 500 on caller controlled input).
• queryOwnsSandbox matches WHERE user_id = $userId AND destroyed_at IS NULL AND (sandbox_id = $sandboxId OR id = derivedInstanceId). The user_id and destroyed_at filters are unchanged.
• querySandboxOwner (behind lookupSandboxOwnerUserId) uses the same OR shape against destroyed_at IS NULL.

services/kilo-chat/src/__tests__/sandbox-ownership.test.ts:

• Replaced the where swallowing mock with a predicate driven one. The new mock imports drizzle helpers via vi.importActual and overrides eq, and, or, isNull with predicate factories, so the WHERE expression is actually evaluated against dbState.rows.
• Added 12 cases covering exact match, ki_ form via id, ki_ with non hex content, ki_ referencing another user, ki_ matching a destroyed row, legacy sandbox_id belonging to another user, and the existing destroyed_at cache invalidation paths.

Verification

• Post deploy: a user whose kiloclaw_instances.sandbox_id is in legacy form and whose DO reports a ki_<hex> value can load /claw/chat and create a conversation without a 403.
• Post deploy: bot status pushes and bot conversation creation flows continue to work for the same population.

Visual Changes

N/A

Reviewer Notes

Ownership scope is intentionally unchanged. The relaxation only adds a second accept form for the same row, derived deterministically from the row's own UUID. queryOwnsSandbox still filters by user_id from the verified bearer, so a caller cannot use the ki_ form to reach another user's row. The derived UUID is validated before reaching the uuid column, so length 35 strings with non hex content cannot produce a cast error.

The legacy form derived from userId is intentionally not accepted as a third form. That would still be ownership scoped, but would let downstream code that uses the caller provided sandboxId as a bot identifier (bot:kiloclaw:${sandboxId}) split conversations across two sandbox buckets if some clients send legacy and others send ki_.

The predicate driven test mock evaluates drizzle's WHERE expression in JavaScript so the new OR branch and the isValidInstanceId guard are actually exercised rather than skipped by the previous mock.

[kilo-code-bot]

WARNING: The secondary isValidInstanceId re-validation is correct and necessary, but the comment above it says isInstanceKeyedSandboxId "only checks prefix + total length" — which is accurate. However, worth noting: instanceIdFromSandboxId will format any 32-character string into UUID shape regardless of whether those characters are valid hex. isValidInstanceId then re-checks with a strict hex UUID regex. This defense-in-depth is good.

One edge case to be aware of: if sandboxId is exactly ki_ followed by 32 non-hex chars (e.g. ki_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz), instanceIdFromSandboxId returns a UUID-shaped string with non-hex chars (e.g. zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz), and isValidInstanceId correctly returns false, so null is returned. The guard works — just confirming the reasoning is sound.

[St0rmz1]

Nothing to fix here.

[kilo-code-bot]

WARNING: The test mock in sandbox-ownership.test.ts swallows the .where() clause entirely (the mock returns rows regardless of the predicate). This means the new candidateInstanceId OR-branch and the security guard (isValidInstanceId re-validation) have zero test coverage of their actual database query logic.

Consider adding tests that cover:

1. A ki_<valid-uuid-hex> sandboxId that should match via kiloclaw_instances.id (migration-tolerance path)
2. A ki_<32-non-hex-chars> sandboxId (attacker-controlled input) — should not match even when the DB has a row
3. A legacy non-ki_ sandboxId — should only use the sandbox_id match branch

The current test structure mocks at too high a level to observe which WHERE predicate was actually constructed.

[St0rmz1]

fixed in b98318f

[kilo-code-bot]

Code Review Summary

Status: 1 Issue Remaining | Recommendation: Address before merge

Executive Summary

The test mock coverage gap (previous WARNING #2) has been resolved by the new commit — drizzle-orm operators are now properly stubbed as predicate functions and all ki_ migration-tolerance paths are covered. The remaining WARNING on sandbox-ownership.ts:27 is a pre-existing observation about the correctness of the re-validation guard (the logic is sound and unchanged).

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
services/kilo-chat/src/services/sandbox-ownership.ts	27	Re-validation with isValidInstanceId is correct and necessary — the comment in the inline thread documents the reasoning. No code change needed.

Resolved Issues

File	Issue
services/kilo-chat/src/__tests__/sandbox-ownership.test.ts	FIXED: Test mock now properly evaluates WHERE predicates by stubbing drizzle-orm eq/isNull/and/or as predicate factories. Tests now cover the ki_ OR-branch, the non-hex attacker path, destroyed-row rejection, and cross-user rejection.

Files Reviewed (2 files)

• services/kilo-chat/src/services/sandbox-ownership.ts — 1 pre-existing observation (logic correct, no change needed)
• services/kilo-chat/src/__tests__/sandbox-ownership.test.ts — coverage gap resolved

Fix these issues in Kilo Cloud

---

_{Reviewed by claude-4.6-sonnet-20260217 · 676,526 tokens}

_{Review guidance: REVIEW.md from base branch main}