Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion service/src/agent-run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import { CodeApiJwtAuthError, verifyLibreChatJwt } from './auth/librechat-jwt';
import { applyPrincipal } from './auth/principal';
import {
OWNER_BINDING_HEADER,
OWNER_BINDING_PATTERN,
sessionOwnerBindingValue,
agentRunSessionKey,
ownerBindingFromHeader,
ownerBindingValue,
Expand Down Expand Up @@ -136,7 +138,7 @@ function uploadedRef(result: { body: unknown }): { sid: string; fid: string } {
/** The new file server's owner-checked delete, applied to the stub's objects. */
const newFileServerOwnerDelete = (req: { headers: Record<string, string | string[] | undefined> }, key: string) => {
const expected = req.headers['x-codeapi-owner-expect'];
if (typeof expected !== 'string' || !/^agent_run\.[0-9a-f]{64}$/.test(expected)) return { status: 400, body: { error: 'Expected owner is required' } };
if (typeof expected !== 'string' || !OWNER_BINDING_PATTERN.test(expected)) return { status: 400, body: { error: 'Expected owner is required' } };
if (!harness.objects.has(key)) return { status: 200, body: { outcome: 'absent' } };
const stored = bindings.get(key);
if (!stored || stored !== expected) return { status: 403, body: { error: 'Owner binding does not match' } };
Expand All @@ -163,6 +165,10 @@ beforeAll(async () => {
const owner = ownerBindingFromHeader(headers[OWNER_BINDING_HEADER.toLowerCase()], sid, fid);
if (!owner.ok) return { status: 400, body: { error: owner.error } };
if (owner.binding) bindings.set(`${sid}/${fid}`, owner.binding);
else {
const sessionKey = harness.redis.entries.get(`session:${sid}`)?.value;
if (sessionKey?.includes(':user:')) bindings.set(`${sid}/${fid}`, sessionOwnerBindingValue(sessionKey));
}
return undefined;
});
harness.setOwnerDeleteHandler(newFileServerOwnerDelete);
Expand All @@ -177,6 +183,34 @@ beforeEach(() => {
delete process.env.CODEAPI_TENANT_ISOLATION_STRICT;
});

describe('durable user deletion', () => {
test('expired user session keeps bytes private and lets only the owner delete', async () => {
const { sid, fid } = uploadedRef(await agentUpload({ kind: 'user', id: USER }, userToken()));
expect(harness.objects.get(`${sid}/${fid}`)?.bytes.toString()).toBe('agent input');
await harness.redis.del(`session:${sid}`);
const path = `/v1/files/${sid}/${fid}?kind=user&id=${USER}`;
for (const token of [userToken({ sub: 'other-user' }), userToken({ tenant_id: OTHER_TENANT })]) {
expect((await call(harness.baseUrl, token, 'DELETE', path)).status).toBe(403);
expect(harness.objects.has(`${sid}/${fid}`)).toBe(true);
}
expect((await call(harness.baseUrl, userToken(), 'GET', `/v1/download/${sid}/${fid}?kind=user&id=${USER}`)).status).toBe(403);
expect((await call(harness.baseUrl, userToken(), 'DELETE', path)).status).toBe(200);
expect(harness.objects.has(`${sid}/${fid}`)).toBe(false);
expect((await call(harness.baseUrl, userToken(), 'DELETE', path)).status).toBe(404);
});

test('unbound old objects and conflicting live sessions cannot use the fallback', async () => {
const { sid, fid } = uploadedRef(await agentUpload({ kind: 'user', id: USER }, userToken()));
const path = `/v1/files/${sid}/${fid}?kind=user&id=${USER}`;
await harness.redis.set(`session:${sid}`, `${OTHER_TENANT}:user:${USER}`);
expect((await call(harness.baseUrl, userToken(), 'DELETE', path)).status).toBe(403);
await harness.redis.del(`session:${sid}`);
bindings.delete(`${sid}/${fid}`);
expect((await call(harness.baseUrl, userToken(), 'DELETE', path)).status).toBe(403);
expect(harness.objects.has(`${sid}/${fid}`)).toBe(true);
});
});

describe('agent_run claim grammar', () => {
test('a well-formed agent_run token verifies to an Agent principal with no user', () => {
const principal = verifyLibreChatJwt(agentToken());
Expand Down
7 changes: 6 additions & 1 deletion service/src/agent-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const OWNER_BINDING_HEADER = 'X-CodeAPI-Owner-Binding';
/** Internal api → file_server header: delete only if the stored binding equals this. */
export const OWNER_EXPECT_HEADER = 'X-CodeAPI-Owner-Expect';
/** Shape of a stored binding (see `ownerBindingValue`). */
export const OWNER_BINDING_PATTERN = /^agent_run\.[0-9a-f]{64}$/;
export const OWNER_BINDING_PATTERN = /^(?:agent_run|session)\.[0-9a-f]{64}$/;
/**
* Last path segment of the file server's owner-checked delete operation,
* `POST /sessions/:sid/objects/:fid/owner-delete`. File servers that predate
Expand All @@ -88,6 +88,11 @@ export function ownerBindingValue(tenantId: string, subject: AgentRunSubject): s
return `agent_run.${digest}`;
}

/** Bind bytes to the server-derived session namespace, never a caller-supplied id. */
export function sessionOwnerBindingValue(sessionKey: string): string {
return `session.${createHash('sha256').update(sessionKey).digest('hex')}`;
}

function ownerBindingKey(): Buffer | undefined {
const token = (process.env[INTERNAL_SERVICE_TOKEN_ENV] ?? '').trim();
if (!token) return undefined;
Expand Down
18 changes: 9 additions & 9 deletions service/src/file-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
OWNER_BINDING_HEADER,
ownerBindingFromHeader,
sessionKeyForLog,
sessionOwnerBindingValue,
} from './agent-run';

const { INSTANCE_ID } = env;
Expand Down Expand Up @@ -263,14 +264,13 @@ async function uploadFile(
if (readOnly) {
metaData['X-Amz-Meta-Read-Only'] = 'true';
}
/* Durable agent-run owner binding (C4). Only a binding an internal caller
* signed for this exact object reaches here; it is stored with the bytes
* so it outlives the session cache and disappears with the object. */
if (ownerBinding) {
metaData[OWNER_METADATA] = ownerBinding;
}

/* The signed run binding takes precedence. User objects retain the
* server-owned session namespace with their bytes, including sandbox outputs. */
const sessionKey = await redisClient.get(`session:${session_id}`);
const durableOwner = ownerBinding ?? (sessionKey?.includes(':user:') ? sessionOwnerBindingValue(sessionKey) : undefined);
if (durableOwner) {
metaData[OWNER_METADATA] = durableOwner;
}
const peeked = await peekStreamForEmpty(fileStream);
let receivedBytes = 0;
if (peeked.empty) {
Expand Down Expand Up @@ -752,8 +752,8 @@ app.delete('/sessions/:session_id/objects/:fileId', async (req, res) => {
});

/**
* Owner-checked deletion for agent_run objects whose session cache has
* expired. A separate operation (not the plain DELETE) so a file server that
* Owner-checked deletion for user and agent_run objects whose session cache
* has expired. A separate operation (not the plain DELETE) so a file server that
* predates owner bindings answers 404 instead of deleting with the internal
* credential. It never deletes without an expected owner, and never deletes an
* object that has no stored binding or a different one. Outcomes:
Expand Down
17 changes: 9 additions & 8 deletions service/src/middleware/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { resolveSessionKey, parseUploadSessionKeyInput, SessionKeyResolutionErro
import { LibreChatJwtAuthProvider, CodeApiJwtAuthError } from '../auth/librechat-jwt';
import { applyPrincipal, type AgentRunPrincipal, type CodeApiPrincipal } from '../auth/principal';
import { applyLocalPrincipal } from '../auth/local';
import { agentRunLogFields, ownerBindingValue, sessionKeyForLog } from '../agent-run';
import { agentRunLogFields, ownerBindingValue, sessionOwnerBindingValue, sessionKeyForLog } from '../agent-run';
import { internalServiceAuthEnabled } from '../internal-service-auth';
import { AuthProviderConfigError, getAuthProviderMode } from '../auth/provider';
import {
Expand Down Expand Up @@ -311,14 +311,15 @@ export const sessionAuth = async (req: AuthenticatedRequest, res: Response, next
}
const cachedSessionKey = await connection.get(`session:${session_id}`);
if (cachedSessionKey !== sessionKey) {
/* The session cache expires after SESSION_CACHE_TTL. A deletion-only
* agent_run token may then still delete, but only against the durable
* owner binding stored with the bytes: the file server removes the object
* only if that binding names this tenant, Agent and run. A present but
* different cache entry is a refusal. */
if (cachedSessionKey === null && agentRunPrincipal?.agentRun.fileDelete && req.method === 'DELETE') {
/* An expired cache can be replaced only by the object's durable binding.
* A present but different cache entry still refuses the request. Shared
* resource identities never authorize this user-private fallback. */
const userDelete = !agentRunPrincipal && sessionKeyInput.kind === 'user' && internalServiceAuthEnabled();
if (cachedSessionKey === null && req.method === 'DELETE' && (agentRunPrincipal?.agentRun.fileDelete || userDelete)) {
req.sessionKey = sessionKey;
req.ownerBindingExpectation = ownerBindingValue(agentRunPrincipal.tenantId, agentRunPrincipal.agentRun);
req.ownerBindingExpectation = agentRunPrincipal
? ownerBindingValue(agentRunPrincipal.tenantId, agentRunPrincipal.agentRun)
: sessionOwnerBindingValue(sessionKey);
logger.info('Session cache absent; deleting against the durable owner binding', authLogMeta(req));
next();
return;
Expand Down
5 changes: 2 additions & 3 deletions service/src/types/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,8 @@ export interface AuthenticatedRequest extends Request {
planId?: string;
executionIdentity?: ExecutionIdentity;
codeApiAuthContext?: CodeApiAuthContext;
/** Set by sessionAuth when a deletion-only agent_run token authorizes a
* DELETE against the durable owner binding because the session cache entry
* has expired. The file server deletes only if the stored binding matches. */
/** Set by sessionAuth for a user or deletion-only agent_run DELETE after
* session expiry. The file server deletes only if the durable binding matches. */
ownerBindingExpectation?: string;
codeApiPrincipal?: CodeApiPrincipal;
}
Expand Down
Loading