Skip to content
Merged
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
19 changes: 11 additions & 8 deletions packages/code/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,17 @@ verification are implemented; use macOS, Linux, or WSL2. This also applies to Gi
private keys.

Git receives authentication through process-scoped `GIT_CONFIG_*` variables.
The same isolated config supplies the standard Git LFS filters; hosts using LFS
must install `git-lfs`, and checkout fails instead of silently leaving pointer
files when it is unavailable.
SRT replaces only the bearer-token portion with a sentinel inside the sandbox
and substitutes the real value in its host proxy only for `github.com` HTTPS
traffic. TLS termination is enabled for that substitution. The worker restores
the parent environment immediately after constructing the sandbox command; it
never writes credentials into the repository, a remote URL, or Git config.
When the GitHub CLI is installed, `gh api`, pull-request, issue, and workflow
commands receive the same installation scope through `GH_TOKEN` (or
`GH_ENTERPRISE_TOKEN` for GHES). The same isolated Git config supplies the
standard Git LFS filters; hosts using LFS must install `git-lfs`, and checkout
fails instead of silently leaving pointer files when it is unavailable.
SRT replaces each real credential with a sentinel inside the sandbox and
substitutes the real value in its host proxy only for the corresponding Git or
GitHub API host. TLS termination is enabled for that substitution. The worker
restores the parent environment immediately after constructing the sandbox
command; it never writes credentials into the repository, a remote URL, Git
config, or the GitHub CLI credential store.
GitHub's required domains are added to the command egress allowlist only when
authentication is configured. The worker identity, GitHub App key path, token
source variables, and mutation-quarantine record remain denied to sandboxed
Expand Down
15 changes: 5 additions & 10 deletions packages/code/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ import type { NativeProcessSandboxOptions } from './native-process.js';
import type { LocalWorkspaceConfig } from './workspace.js';
import {
GITHUB_ALLOWED_DOMAINS,
GITHUB_CREDENTIAL_ENV_NAME,
GitHubAppCredentialProvider,
gitHubCommandCredentialEnvironment,
gitHubMaskedCredentialVariables,
StaticGitHubCredentialProvider,
gitHubAuthenticationPolicyIdentity,
gitHubCredentialEnvironment,
normalizeGitHubHost,
wrapGitHubCredentialCommand,
} from './github.js';
Expand Down Expand Up @@ -783,16 +783,11 @@ async function run(
...(github.provider
? {
maskedEnvironment: {
variables: [
{
name: GITHUB_CREDENTIAL_ENV_NAME,
extract: '^(.+)$',
injectHosts: [github.host],
},
],
variables: gitHubMaskedCredentialVariables(github.host),
async resolve(signal?: AbortSignal) {
return gitHubCredentialEnvironment(
return gitHubCommandCredentialEnvironment(
await github.provider!.getCredential(signal),
github.host,
);
},
wrapCommand(command: string, platform: NodeJS.Platform) {
Expand Down
82 changes: 80 additions & 2 deletions packages/code/src/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
GitHubAppCredentialProvider,
StaticGitHubCredentialProvider,
gitHubAuthenticationPolicyIdentity,
gitHubCliTokenEnvironmentName,
gitHubCommandCredentialEnvironment,
gitHubMaskedCredentialVariables,
GITHUB_CREDENTIAL_ENV_NAME,
gitHubCredentialEnvironment,
normalizeGitHubHost,
Expand Down Expand Up @@ -137,12 +140,76 @@ test('builds process-scoped Git HTTPS authorization without embedding credential
const provider = new StaticGitHubCredentialProvider(
'github_pat_abcdefghijklmnopqrstuvwxyz',
);
const encodedCredential = Buffer.from(
'x-access-token:github_pat_abcdefghijklmnopqrstuvwxyz',
'utf8',
).toString('base64');
assert.deepEqual(
gitHubCredentialEnvironment(await provider.getCredential()),
{
[GITHUB_CREDENTIAL_ENV_NAME]: 'github_pat_abcdefghijklmnopqrstuvwxyz',
[GITHUB_CREDENTIAL_ENV_NAME]: encodedCredential,
},
);
assert.ok(!encodedCredential.includes('github_pat_'));
});

test('adds a GitHub CLI token only to the command-sandbox credential bundle', async () => {
const provider = new StaticGitHubCredentialProvider(
'github_pat_abcdefghijklmnopqrstuvwxyz',
);
const credential = await provider.getCredential();
assert.deepEqual(gitHubCommandCredentialEnvironment(credential), {
[GITHUB_CREDENTIAL_ENV_NAME]: Buffer.from(
'x-access-token:github_pat_abcdefghijklmnopqrstuvwxyz',
'utf8',
).toString('base64'),
GH_TOKEN: 'github_pat_abcdefghijklmnopqrstuvwxyz',
});
assert.deepEqual(
gitHubCommandCredentialEnvironment(credential, 'github.example.test'),
{
[GITHUB_CREDENTIAL_ENV_NAME]: Buffer.from(
'x-access-token:github_pat_abcdefghijklmnopqrstuvwxyz',
'utf8',
).toString('base64'),
GH_ENTERPRISE_TOKEN: 'github_pat_abcdefghijklmnopqrstuvwxyz',
},
);
});

test('selects the GitHub CLI token variable for public and enterprise hosts', () => {
assert.equal(gitHubCliTokenEnvironmentName('github.com'), 'GH_TOKEN');
assert.equal(
gitHubCliTokenEnvironmentName('github.example.test'),
'GH_ENTERPRISE_TOKEN',
);
});

test('restricts Git and GitHub CLI credential substitution to their respective hosts', () => {
assert.deepEqual(gitHubMaskedCredentialVariables('github.com'), [
{
name: GITHUB_CREDENTIAL_ENV_NAME,
extract: '^(.+)$',
injectHosts: ['github.com'],
},
{
name: 'GH_TOKEN',
extract: '^(.+)$',
injectHosts: ['api.github.com'],
},
]);
assert.deepEqual(gitHubMaskedCredentialVariables('github.example.test'), [
{
name: GITHUB_CREDENTIAL_ENV_NAME,
extract: '^(.+)$',
injectHosts: ['github.example.test'],
},
{
name: 'GH_ENTERPRISE_TOKEN',
extract: '^(.+)$',
injectHosts: ['github.example.test'],
},
]);
});

test('composes the masked credential with SRT Git configuration inside the sandbox', () => {
Expand All @@ -155,10 +222,21 @@ test('composes the masked credential with SRT Git configuration inside the sandb
assert.match(wrapped, /http\.https:\/\/github\.com\/\.extraheader/);
assert.match(wrapped, /\$\{LIBRECHAT_CODE_GITHUB_AUTHORIZATION\}/);
assert.match(wrapped, /unset LIBRECHAT_CODE_GITHUB_AUTHORIZATION/);
assert.equal(wrapped.match(/Authorization: Bearer/g)?.length, 1);
assert.doesNotMatch(wrapped, /unset GH_TOKEN/);
assert.equal(wrapped.match(/Authorization: Basic/g)?.length, 1);
assert.ok(!wrapped.includes('github_pat_'));
});

test('targets GitHub CLI at an enterprise host without exposing its token', () => {
const wrapped = wrapGitHubCredentialCommand(
'gh pr create',
'github.example.test',
'linux',
);
assert.match(wrapped, /GH_HOST=github\.example\.test/);
assert.doesNotMatch(wrapped, /GH_ENTERPRISE_TOKEN=/);
});

test('rejects an insecure GitHub App API endpoint before reading the private key', () => {
assert.throws(
() =>
Expand Down
49 changes: 46 additions & 3 deletions packages/code/src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,50 @@ export function gitHubCredentialEnvironment(
credential: GitHubCredential,
): Record<string, string> {
return {
[GITHUB_CREDENTIAL_ENV_NAME]: credential.value,
[GITHUB_CREDENTIAL_ENV_NAME]: Buffer.from(
`x-access-token:${credential.value}`,
'utf8',
).toString('base64'),
};
}

export function gitHubCommandCredentialEnvironment(
credential: GitHubCredential,
host = 'github.com',
): Record<string, string> {
return {
...gitHubCredentialEnvironment(credential),
[gitHubCliTokenEnvironmentName(host)]: credential.value,
};
}

export function gitHubCliTokenEnvironmentName(host: string): string {
return host === 'github.com' ? 'GH_TOKEN' : 'GH_ENTERPRISE_TOKEN';
}

export function gitHubApiHost(host: string): string {
return host === 'github.com' ? 'api.github.com' : host;
}

export function gitHubMaskedCredentialVariables(host: string): Array<{
name: string;
injectHosts: string[];
extract: string;
}> {
return [
{
name: GITHUB_CREDENTIAL_ENV_NAME,
extract: '^(.+)$',
injectHosts: [host],
},
{
name: gitHubCliTokenEnvironmentName(host),
extract: '^(.+)$',
injectHosts: [gitHubApiHost(host)],
},
];
}

export function gitHubAuthenticationPolicyIdentity(options: {
mode?: 'app' | 'token';
host: string;
Expand Down Expand Up @@ -246,18 +286,21 @@ export function wrapGitHubCredentialCommand(
platform: NodeJS.Platform = process.platform,
): string {
const key = `http.https://${host}/.extraheader`;
const cliHost = host === 'github.com' ? undefined : host;
if (platform === 'win32') {
return [
'set "GIT_CONFIG_GLOBAL=NUL"',
'set "GIT_CONFIG_NOSYSTEM=1"',
`set "GIT_CONFIG_PARAMETERS='http.proxyAuthMethod=basic' '${key}=Authorization: Bearer %${GITHUB_CREDENTIAL_ENV_NAME}%'"`,
...(cliHost ? [`set "GH_HOST=${cliHost}"`] : []),
`set "GIT_CONFIG_PARAMETERS='http.proxyAuthMethod=basic' '${key}=Authorization: Basic %${GITHUB_CREDENTIAL_ENV_NAME}%'"`,
`set "${GITHUB_CREDENTIAL_ENV_NAME}="`,
command,
].join(' && ');
}
return [
'export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1',
`export GIT_CONFIG_PARAMETERS="'http.proxyAuthMethod=basic' '${key}=Authorization: Bearer \${${GITHUB_CREDENTIAL_ENV_NAME}}'"`,
...(cliHost ? [`export GH_HOST=${cliHost}`] : []),
`export GIT_CONFIG_PARAMETERS="'http.proxyAuthMethod=basic' '${key}=Authorization: Basic \${${GITHUB_CREDENTIAL_ENV_NAME}}'"`,
`unset ${GITHUB_CREDENTIAL_ENV_NAME}`,
command,
].join(';\n');
Expand Down
6 changes: 6 additions & 0 deletions service/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,12 @@ export const env = {
// Files List Rate Limits
FETCH_LIMIT_WINDOW: Number(process.env.FETCH_LIMIT_WINDOW) || 60 * 1000, // 1 minute
FETCH_MAX_REQUESTS: Number(process.env.FETCH_MAX_REQUESTS) || 120, // 120 requests per minute
// File Delete Rate Limits. Fall back to the fetch settings so existing
// deployments keep their current limits while using an independent bucket.
DELETE_LIMIT_WINDOW:
Number(process.env.DELETE_LIMIT_WINDOW) || Number(process.env.FETCH_LIMIT_WINDOW) || 60 * 1000,
DELETE_MAX_REQUESTS:
Number(process.env.DELETE_MAX_REQUESTS) || Number(process.env.FETCH_MAX_REQUESTS) || 120,
// Redis Key Cache Config
SESSION_CACHE_TTL: Number(process.env.SESSION_CACHE_TTL) || 86400,
/** TTL for the durable `session-owner:<session_id>` record that backs
Expand Down
56 changes: 56 additions & 0 deletions service/src/middleware/limits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,40 @@ async function startRateLimitedApp(max: number, windowMs: number): Promise<strin
return `http://127.0.0.1:${address.port}`;
}

async function startIndependentFileLimiterApp(max: number, windowMs: number): Promise<string> {
const redis = new TestRedisRateLimitStore();
setRateLimitRedisForTests(redis);

const app = express();
app.use((req, _res, next) => {
applyPrincipal(req as AuthenticatedRequest, {
userId: 'user-a',
tenantId: 'tenant-a',
principalSource: 'librechat_jwt',
});
next();
});
app.get(
'/v1/files/session-a',
createRateLimiter('test-fetch', windowMs, max, { message: 'Too many file list requests.' }),
(_req, res) => res.status(200).json({ ok: true }),
);
app.delete(
'/v1/files/session-a/file-a',
createRateLimiter('test-delete', windowMs, max, {
message: 'Too many file deletion requests.',
structuredBody: true,
}),
(_req, res) => res.status(200).json({ ok: true }),
);

const server = app.listen(0, '127.0.0.1');
servers.push(server);
await once(server, 'listening');
const address = server.address() as AddressInfo;
return `http://127.0.0.1:${address.port}`;
}

function postExec(url: string, headers: Record<string, string> = {}): Promise<Response> {
return fetch(`${url}/v1/exec`, {
method: 'POST',
Expand Down Expand Up @@ -228,3 +262,25 @@ describe('execution rate limiting', () => {
expect((await postExec(url)).status).toBe(200);
});
});

describe('file operation rate limiting', () => {
test('keeps deletion traffic out of the file-list bucket and returns structured retry guidance', async () => {
const url = await startIndependentFileLimiterApp(1, 30_000);

expect((await fetch(`${url}/v1/files/session-a`)).status).toBe(200);
expect((await fetch(`${url}/v1/files/session-a/file-a`, { method: 'DELETE' })).status).toBe(200);

const rejectedDelete = await fetch(`${url}/v1/files/session-a/file-a`, { method: 'DELETE' });
const body = await rejectedDelete.json() as ReturnType<typeof rateLimitResponseBody>;
expect(rejectedDelete.status).toBe(429);
expect(rejectedDelete.headers.get('retry-after')).not.toBeNull();
expect(body.error).toBe('rate_limited');
expect(body.message).toContain('Too many file deletion requests.');

const rejectedList = await fetch(`${url}/v1/files/session-a`);
expect(rejectedList.status).toBe(429);
expect(await rejectedList.json()).toEqual({
error: expect.stringContaining('Too many file list requests.'),
});
});
});
11 changes: 11 additions & 0 deletions service/src/middleware/limits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,14 @@ export const fetchLimiter = createRateLimiter(
env.FETCH_MAX_REQUESTS,
{ message: 'Too many file list requests.' }
);

export const deleteLimiter = createRateLimiter(
'delete',
env.DELETE_LIMIT_WINDOW,
env.DELETE_MAX_REQUESTS,
{
message: 'Too many file deletion requests.',
structuredBody: true,
logRejections: true,
}
);
2 changes: 1 addition & 1 deletion service/src/service/exec-timeout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('/exec validates timeout before enqueue and forwards its cap to both langua
mock.module('./src/middleware/auth', () => ({ sessionAuth: passthrough }));
mock.module('./src/middleware/limits', () => ({
executionLimiter: passthrough, uploadLimiter: passthrough,
downloadLimiter: passthrough, fetchLimiter: passthrough,
downloadLimiter: passthrough, fetchLimiter: passthrough, deleteLimiter: passthrough,
}));
mock.module('./src/lifecycle', () => ({
checkServiceStartUp: () => false, checkServiceShutDown: () => false,
Expand Down
12 changes: 9 additions & 3 deletions service/src/service/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import { Readable } from 'stream';
import type * as t from '../types';
import { checkServiceStartUp, checkServiceShutDown } from '../lifecycle';
import { sessionAuth } from '../middleware/auth';
import { executionLimiter, uploadLimiter, downloadLimiter, fetchLimiter } from '../middleware/limits';
import {
executionLimiter,
uploadLimiter,
downloadLimiter,
fetchLimiter,
deleteLimiter,
} from '../middleware/limits';
import { internalServiceHeaders } from '../internal-service-auth';
import { resolveSessionKey, resolveOutputBucketSessionKey, SessionKeyResolutionError, parseUploadSessionKeyInput, type SessionKeyInput } from '../session-key';
import { pyQueue, otherQueue, pyQueueEvents, otherQueueEvents, queueNames, connection } from '../queue';
Expand Down Expand Up @@ -997,7 +1003,7 @@ const deleteSessionObject = async (req: t.AuthenticatedRequest, res: Response) =
}
};

router.delete('/files/:session_id/:fileId', fetchLimiter, sessionAuth, deleteSessionObject);
router.delete('/files/:session_id/:fileId', deleteLimiter, sessionAuth, deleteSessionObject);

/**
* Alias of the route above, on the path LibreChat's `deleteCodeEnvFile`
Expand All @@ -1013,6 +1019,6 @@ router.delete('/files/:session_id/:fileId', fetchLimiter, sessionAuth, deleteSes
*
* GET on this same path is the metadata proxy above.
*/
router.delete('/sessions/:session_id/objects/:fileId', fetchLimiter, sessionAuth, deleteSessionObject);
router.delete('/sessions/:session_id/objects/:fileId', deleteLimiter, sessionAuth, deleteSessionObject);

export default router;