Skip to content

Commit f999cf6

Browse files
committed
fix(session-ingest): enforce current organization access
Scope session-ingest operations to the user's current organization and revoke access when org membership changes. Adds resolveAccessibleKiloSession and a putValidated cache entry carrying organizationId, used across the session delete/ingest/share/unshare routes. Moves the org member removal rowCount check ahead of the audit log so failures bail early. Addresses review findings on access revocation.
1 parent a10b9b7 commit f999cf6

27 files changed

Lines changed: 1408 additions & 302 deletions

apps/web/src/lib/cloud-agent/session-ownership.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import {
88
type User,
99
} from '@kilocode/db/schema';
1010
import { and, eq, inArray } from 'drizzle-orm';
11-
import { queryAccessibleCloudAgentSession } from '@kilocode/worker-utils/cloud-agent-session-access';
11+
import {
12+
queryAccessibleCloudAgentSession,
13+
queryAccessibleKiloSession,
14+
} from '@kilocode/worker-utils/cloud-agent-session-access';
1215
import {
1316
verifyOrgOwnsSessionV2ByCloudAgentId,
1417
verifyUserOwnsSessionV2ByCloudAgentId,
@@ -71,6 +74,10 @@ describe('Cloud Agent session ownership', () => {
7174
});
7275

7376
beforeEach(async () => {
77+
await db
78+
.update(organizations)
79+
.set({ deleted_at: null })
80+
.where(eq(organizations.id, organization.id));
7481
await db.insert(cli_sessions_v2).values([
7582
{
7683
session_id: PERSONAL_SESSION_ID,
@@ -178,6 +185,12 @@ describe('Cloud Agent session ownership', () => {
178185
cloudAgentSessionId: ORGANIZATION_CLOUD_AGENT_SESSION_ID,
179186
})
180187
).resolves.toBeNull();
188+
await expect(
189+
queryAccessibleKiloSession(db, {
190+
kiloUserId: owner.id,
191+
kiloSessionId: ORGANIZATION_SESSION_ID,
192+
})
193+
).resolves.toBeNull();
181194

182195
await db.insert(organization_memberships).values({
183196
organization_id: organization.id,
@@ -202,5 +215,34 @@ describe('Cloud Agent session ownership', () => {
202215
kiloSessionId: ORGANIZATION_SESSION_ID,
203216
organizationId: organization.id,
204217
});
218+
await expect(
219+
queryAccessibleKiloSession(db, {
220+
kiloUserId: owner.id,
221+
kiloSessionId: ORGANIZATION_SESSION_ID,
222+
})
223+
).resolves.toEqual({
224+
kiloSessionId: ORGANIZATION_SESSION_ID,
225+
organizationId: organization.id,
226+
});
227+
});
228+
229+
it('denies access after the organization is soft-deleted', async () => {
230+
await db
231+
.update(organizations)
232+
.set({ deleted_at: new Date().toISOString() })
233+
.where(eq(organizations.id, organization.id));
234+
235+
await expect(
236+
queryAccessibleCloudAgentSession(db, {
237+
kiloUserId: owner.id,
238+
cloudAgentSessionId: ORGANIZATION_CLOUD_AGENT_SESSION_ID,
239+
})
240+
).resolves.toBeNull();
241+
await expect(
242+
queryAccessibleKiloSession(db, {
243+
kiloUserId: owner.id,
244+
kiloSessionId: ORGANIZATION_SESSION_ID,
245+
})
246+
).resolves.toBeNull();
205247
});
206248
});

apps/web/src/lib/organizations/organizations.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, test, expect, afterEach } from '@jest/globals';
1+
import { describe, test, expect, afterEach, beforeEach } from '@jest/globals';
22
import { db, sql } from '@/lib/drizzle';
33
import {
44
organizations,
@@ -25,6 +25,11 @@ import {
2525
} from './organizations';
2626
import { fromMicrodollars } from '@/lib/utils';
2727
import { DEFAULT_MEMBER_DAILY_LIMIT_USD } from '@/lib/organizations/constants';
28+
import { invalidateOrganizationSessionAccess } from '@/lib/session-ingest-client';
29+
30+
jest.mock('@/lib/session-ingest-client', () => ({
31+
invalidateOrganizationSessionAccess: jest.fn().mockResolvedValue(undefined),
32+
}));
2833

2934
describe('Organizations', () => {
3035
afterEach(async () => {
@@ -400,6 +405,10 @@ describe('Organizations', () => {
400405
});
401406

402407
describe('removeUserFromOrganization', () => {
408+
beforeEach(() => {
409+
jest.mocked(invalidateOrganizationSessionAccess).mockClear();
410+
});
411+
403412
test('should remove user from organization', async () => {
404413
const owner = await insertTestUser();
405414
const member = await insertTestUser();
@@ -418,6 +427,7 @@ describe('Organizations', () => {
418427
// Verify user was removed
419428
memberOrgs = await getUserOrganizationsWithSeats(member.id);
420429
expect(memberOrgs).toHaveLength(0);
430+
expect(invalidateOrganizationSessionAccess).toHaveBeenCalledWith(member.id, organization.id);
421431
});
422432

423433
test('should handle removing non-existent membership gracefully', async () => {
@@ -430,6 +440,23 @@ describe('Organizations', () => {
430440

431441
expect(result).toBeDefined();
432442
// Should not throw error, just return empty result
443+
expect(invalidateOrganizationSessionAccess).not.toHaveBeenCalled();
444+
});
445+
446+
test('should complete removal when session access invalidation fails', async () => {
447+
const owner = await insertTestUser();
448+
const member = await insertTestUser();
449+
const organization = await createOrganization('Test Org', owner.id);
450+
await addUserToOrganization(organization.id, member.id, 'member');
451+
jest
452+
.mocked(invalidateOrganizationSessionAccess)
453+
.mockRejectedValueOnce(new Error('invalidation unavailable'));
454+
455+
await expect(removeUserFromOrganization(organization.id, member.id)).resolves.toBeDefined();
456+
457+
const memberOrgs = await getUserOrganizationsWithSeats(member.id);
458+
expect(memberOrgs).toHaveLength(0);
459+
expect(invalidateOrganizationSessionAccess).toHaveBeenCalledWith(member.id, organization.id);
433460
});
434461

435462
test('should remove specific user without affecting others', async () => {

apps/web/src/lib/organizations/organizations.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ import { randomUUID } from 'crypto';
3030
import { fromMicrodollars, getLowerDomainFromEmail, normalizeEmail } from '@/lib/utils';
3131
import { resolveEffectiveOrganizationSsoPolicy } from './organization-sso-policy';
3232
import { classifyOrganizationEntitlement } from './trial-utils';
33-
import { logExceptInTest } from '@/lib/utils.server';
33+
import { errorExceptInTest, logExceptInTest } from '@/lib/utils.server';
34+
import { invalidateOrganizationSessionAccess } from '@/lib/session-ingest-client';
3435
import { APP_URL } from '@/lib/constants';
3536
import { createAuditLog } from '@/lib/organizations/organization-audit-logs';
3637
import { failureResult, successResult } from '@/lib/maybe-result';
@@ -362,7 +363,7 @@ export async function removeUserFromOrganization(
362363
userId: User['id'],
363364
removedBy?: User['id']
364365
): Promise<{ rowCount: number | null }> {
365-
return await db.transaction(async tx => {
366+
const result = await db.transaction(async tx => {
366367
await lockOrganizationMembershipMutation(tx, organizationId, userId);
367368
// Look up the user's current role before deleting
368369
const [membership] = await tx
@@ -422,6 +423,34 @@ export async function removeUserFromOrganization(
422423

423424
return result;
424425
});
426+
427+
if ((result.rowCount ?? 0) > 0) {
428+
await invalidateRemovedMemberSessionAccess(organizationId, userId);
429+
}
430+
431+
return result;
432+
}
433+
434+
/**
435+
* Best-effort: a removed member loses cached Session Ingest access within the
436+
* cache TTL even when this call fails, so removal never fails on it.
437+
*/
438+
async function invalidateRemovedMemberSessionAccess(
439+
organizationId: Organization['id'],
440+
userId: User['id']
441+
): Promise<void> {
442+
try {
443+
await invalidateOrganizationSessionAccess(userId, organizationId);
444+
} catch (error) {
445+
errorExceptInTest(
446+
'Failed to invalidate cached session access for removed organization member',
447+
{
448+
organizationId,
449+
userId,
450+
error: error instanceof Error ? error.message : String(error),
451+
}
452+
);
453+
}
425454
}
426455

427456
export async function updateUserRoleInOrganization(

apps/web/src/lib/session-ingest-client.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
fetchSessionMessages,
77
deleteSession,
88
shareSession,
9+
invalidateOrganizationSessionAccess,
910
} from './session-ingest-client';
1011

1112
// ---------------------------------------------------------------------------
@@ -18,6 +19,7 @@ jest.mock('@sentry/nextjs', () => ({
1819

1920
jest.mock('@/lib/config.server', () => ({
2021
SESSION_INGEST_WORKER_URL: 'https://ingest.test.example.com',
22+
INTERNAL_API_SECRET: 'internal-secret',
2123
}));
2224

2325
jest.mock('@/lib/tokens', () => ({
@@ -410,6 +412,82 @@ describe('shareSession', () => {
410412
// fetchSessionMessages (thin wrapper)
411413
// ---------------------------------------------------------------------------
412414

415+
describe('invalidateOrganizationSessionAccess', () => {
416+
beforeEach(() => {
417+
mockFetch.mockReset();
418+
mockCaptureException.mockReset();
419+
});
420+
421+
it('reports and throws invalidation failures', async () => {
422+
mockFetch.mockResolvedValue({
423+
ok: false,
424+
status: 503,
425+
statusText: 'Service Unavailable',
426+
text: () => Promise.resolve('cache unavailable'),
427+
});
428+
429+
await expect(
430+
invalidateOrganizationSessionAccess('usr_removed', '11111111-1111-4111-8111-111111111111')
431+
).rejects.toThrow(
432+
'Session access invalidation failed: 503 Service Unavailable - cache unavailable'
433+
);
434+
expect(mockCaptureException).toHaveBeenCalledWith(
435+
expect.any(Error),
436+
expect.objectContaining({
437+
tags: { source: 'session-ingest-client', endpoint: 'invalidate-session-access' },
438+
extra: {
439+
kiloUserId: 'usr_removed',
440+
organizationId: '11111111-1111-4111-8111-111111111111',
441+
status: 503,
442+
},
443+
})
444+
);
445+
});
446+
447+
it('calls the secret-protected organization invalidation endpoint', async () => {
448+
mockFetch.mockResolvedValue({ ok: true, status: 204 });
449+
450+
await invalidateOrganizationSessionAccess(
451+
'usr_removed',
452+
'11111111-1111-4111-8111-111111111111'
453+
);
454+
455+
expect(mockFetch).toHaveBeenCalledWith(
456+
'https://ingest.test.example.com/internal/session-access/invalidate',
457+
{
458+
method: 'POST',
459+
headers: {
460+
'content-type': 'application/json',
461+
'X-Internal-Secret': 'internal-secret',
462+
},
463+
body: JSON.stringify({
464+
kiloUserId: 'usr_removed',
465+
organizationId: '11111111-1111-4111-8111-111111111111',
466+
}),
467+
signal: expect.any(AbortSignal),
468+
}
469+
);
470+
});
471+
472+
it('sets a 30-second invalidation deadline', async () => {
473+
const signal = new AbortController().signal;
474+
const timeout = jest.spyOn(AbortSignal, 'timeout').mockReturnValue(signal);
475+
mockFetch.mockResolvedValue({ ok: true, status: 204 });
476+
477+
await invalidateOrganizationSessionAccess(
478+
'usr_removed',
479+
'11111111-1111-4111-8111-111111111111'
480+
);
481+
482+
expect(timeout).toHaveBeenCalledWith(30_000);
483+
expect(mockFetch).toHaveBeenCalledWith(
484+
'https://ingest.test.example.com/internal/session-access/invalidate',
485+
expect.objectContaining({ signal })
486+
);
487+
timeout.mockRestore();
488+
});
489+
});
490+
413491
describe('fetchSessionMessages', () => {
414492
beforeEach(() => {
415493
mockFetch.mockReset();

apps/web/src/lib/session-ingest-client.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import 'server-only';
22

33
import { captureException } from '@sentry/nextjs';
44
import { z } from 'zod';
5-
import { SESSION_INGEST_WORKER_URL } from '@/lib/config.server';
5+
import { INTERNAL_API_SECRET, SESSION_INGEST_WORKER_URL } from '@/lib/config.server';
66
import { generateInternalServiceToken } from '@/lib/tokens';
77
import type { User } from '@kilocode/db/schema';
88

@@ -162,6 +162,44 @@ export async function shareSession(
162162
return { public_id: body.public_id };
163163
}
164164

165+
// ---------------------------------------------------------------------------
166+
// Authorization cache invalidation
167+
// ---------------------------------------------------------------------------
168+
169+
export async function invalidateOrganizationSessionAccess(
170+
kiloUserId: string,
171+
organizationId: string
172+
): Promise<void> {
173+
if (!SESSION_INGEST_WORKER_URL) {
174+
throw new Error('SESSION_INGEST_WORKER_URL is not configured');
175+
}
176+
if (!INTERNAL_API_SECRET) {
177+
throw new Error('INTERNAL_API_SECRET is not configured');
178+
}
179+
180+
const response = await fetch(`${SESSION_INGEST_WORKER_URL}/internal/session-access/invalidate`, {
181+
method: 'POST',
182+
headers: {
183+
'content-type': 'application/json',
184+
'X-Internal-Secret': INTERNAL_API_SECRET,
185+
},
186+
body: JSON.stringify({ kiloUserId, organizationId }),
187+
signal: AbortSignal.timeout(30_000),
188+
});
189+
190+
if (!response.ok) {
191+
const errorText = await response.text().catch(() => '');
192+
const error = new Error(
193+
`Session access invalidation failed: ${response.status} ${response.statusText}${errorText ? ` - ${errorText}` : ''}`
194+
);
195+
captureException(error, {
196+
tags: { source: 'session-ingest-client', endpoint: 'invalidate-session-access' },
197+
extra: { kiloUserId, organizationId, status: response.status },
198+
});
199+
throw error;
200+
}
201+
}
202+
165203
// ---------------------------------------------------------------------------
166204
// Delete
167205
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)