🏢 feat: Look Up Pull Requests on a GitHub Enterprise Host

librechat.example.yaml

@@ -940,6 +940,8 @@ endpoints:
   #   # whose branch has moved on since its last command does.
   #   pullRequests:
   #     enabled: true # set false to turn the feature off
+  #     # GitHub Enterprise Server or a *.ghe.com tenant; github.com when omitted.
+  #     # host: "github.example.com"
   #     # Environment variable reference only, never the token itself. Optional, see above.
   #     token: "${GITHUB_PULL_REQUEST_TOKEN}"
   #     # Repositories the token may be used for, as owner/name or owner/*. A worker reports its

packages/api/src/pulls/github.spec.ts

@@ -1,4 +1,4 @@
-import { createGitHubPullRequestSource, summarizeChecks } from './github';
+import { createGitHubPullRequestSource, summarizeChecks, gitHubApiBase } from './github';
 import { PullRequestSourceError } from './types';
 
 const sha = 'a'.repeat(40);
@@ -187,13 +187,49 @@ describe('createGitHubPullRequestSource', () => {
     await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' });
   });
 
+  it('rejects a github.com page when the configured host is an Enterprise one', async () => {
+    const { source } = sourceFor(routes());
+    await expect(
+      source.find({ repo: 'o/r', branch: 'feat/x', token: 't', host: 'ghe.example.com' }),
+    ).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' });
+  });
+
   it('bounds the title', async () => {
     const { source } = sourceFor(routes(pull({ title: 'x'.repeat(1000) })));
     const result = await find(source);
     expect(result?.title).toHaveLength(256);
   });
 });
 
+describe('GitHub host', () => {
+  it.each([
+    ['github.com', 'https://api.github.com'],
+    ['acme.ghe.com', 'https://api.acme.ghe.com'],
+    ['ghe.example.com', 'https://ghe.example.com/api/v3'],
+    ['ghe.local:8443', 'https://ghe.local:8443/api/v3'],
+  ])('serves the API of %s from %s', (host, base) => {
+    expect(gitHubApiBase(host)).toBe(base);
+  });
+
+  it('sends every request of an Enterprise lookup to that host and accepts its pages', async () => {
+    const host = 'ghe.example.com';
+    const fetchFn = jest.fn(async (input: string) => {
+      const url = String(input);
+      if (url.includes('/pulls?state=open')) return json(listed());
+      if (url.includes('/pulls/7')) return json(pull({ html_url: `https://${host}/o/r/pull/7` }));
+      if (url.includes('/check-runs')) return json({ check_runs: [] });
+      throw new Error(`unexpected ${url}`);
+    });
+    const source = createGitHubPullRequestSource({ fetchFn });
+    const result = await source.find({ repo: 'o/r', branch: 'feat/x', token: 't', host });
+    expect(result).toMatchObject({ number: 7, url: `https://${host}/o/r/pull/7` });
+    expect(fetchFn.mock.calls.length).toBeGreaterThan(0);
+    for (const [url] of fetchFn.mock.calls) {
+      expect(String(url).startsWith(`https://${host}/api/v3/`)).toBe(true);
+    }
+  });
+});
+
 describe('check runs across pages', () => {
   const run = (conclusion: string | null, status = 'completed') => ({ status, conclusion });
   const passing = (count: number) => Array.from({ length: count }, () => run('success'));

packages/api/src/pulls/github.ts

@@ -8,7 +8,7 @@ import type { PullRequestSource } from './types';
 import { isAllowedRepository } from './repository';
 import { PullRequestSourceError } from './types';
 
-const GITHUB_API_BASE = 'https://api.github.com';
+export const DEFAULT_GITHUB_HOST = 'github.com';
 /** Defaults for the operator bounds in `endpoints.agents.pullRequests`. */
 const DEFAULT_REQUEST_TIMEOUT_MS = 10_000;
 const DEFAULT_LOOKUP_TIMEOUT_MS = 30_000;
@@ -64,23 +64,33 @@ const isRecord = (value: unknown): value is Record<string, unknown> =>
 const isCount = (value: unknown): value is number =>
   typeof value === 'number' && Number.isSafeInteger(value) && value >= 0;
 
-/** The URL ends up in an href, so only a plain https github.com page is accepted. */
-function isGitHubPageUrl(value: unknown): value is string {
+/**
+ * The REST API of a GitHub host: `api.github.com` for github.com, `api.<host>` for a data residency
+ * tenant on `ghe.com`, and `/api/v3` on the host itself for GitHub Enterprise Server.
+ */
+export function gitHubApiBase(host: string): string {
+  if (host === DEFAULT_GITHUB_HOST) return 'https://api.github.com';
+  if (host.endsWith('.ghe.com')) return `https://api.${host}`;
+  return `https://${host}/api/v3`;
+}
+
+/** The URL ends up in an href, so only a plain https page on the configured host is accepted. */
+function isGitHubPageUrl(value: unknown, host: string): value is string {
   if (typeof value !== 'string') return false;
   try {
     const url = new URL(value);
-    return url.protocol === 'https:' && url.hostname === 'github.com';
+    return url.protocol === 'https:' && url.host === host;
   } catch {
     return false;
   }
 }
 
-function parsePull(value: unknown): GitHubPull {
+function parsePull(value: unknown, host: string): GitHubPull {
   if (
     !isRecord(value) ||
     !isCount(value.number) ||
     typeof value.title !== 'string' ||
-    !isGitHubPageUrl(value.html_url) ||
+    !isGitHubPageUrl(value.html_url, host) ||
     (value.state !== 'open' && value.state !== 'closed') ||
     typeof value.merged !== 'boolean' ||
     typeof value.draft !== 'boolean' ||
@@ -242,15 +252,15 @@ const isPathSegment = (value: string): boolean => value !== '.' && value !== '..
 
 export function createGitHubPullRequestSource({
   fetchFn,
-  apiBase = GITHUB_API_BASE,
 }: {
   /** The deployment's HTTP client, so proxying and instrumentation are decided by the caller. */
   fetchFn: PullRequestFetch;
-  apiBase?: string;
 }): PullRequestSource {
   /** One lookup's credential and deadlines, shared by every request it makes. */
   type Lookup = {
     token: string;
+    /** The host's REST API root; every request of the lookup goes there and nowhere else. */
+    apiBase: string;
     requestTimeoutMs: number;
     /** Aborts when the whole lookup runs out of time. */
     deadline: AbortSignal;
@@ -270,7 +280,7 @@ export function createGitHubPullRequestSource({
   ): Promise<unknown | null> {
     let response: Response;
     try {
-      response = await fetchFn(`${apiBase}${pathname}`, {
+      response = await fetchFn(`${lookup.apiBase}${pathname}`, {
         headers: {
           Accept: 'application/vnd.github+json',
           Authorization: `Bearer ${lookup.token}`,
@@ -338,6 +348,7 @@ export function createGitHubPullRequestSource({
       branch,
       head: recordedHead,
       pullRequest,
+      host = DEFAULT_GITHUB_HOST,
       token,
       limits,
       allowedRepositories,
@@ -350,6 +361,7 @@ export function createGitHubPullRequestSource({
       if (recordedHead === null) return null;
       const lookup: Lookup = {
         token,
+        apiBase: gitHubApiBase(host),
         requestTimeoutMs: limits?.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS,
         deadline: AbortSignal.timeout(limits?.lookupTimeoutMs ?? DEFAULT_LOOKUP_TIMEOUT_MS),
         maxCheckRunPages: limits?.maxCheckRunPages ?? DEFAULT_MAX_CHECK_RUN_PAGES,
@@ -450,7 +462,7 @@ export function createGitHubPullRequestSource({
       if (pullRequest != null && Number.isSafeInteger(pullRequest) && pullRequest > 0) {
         const linked = await getJson(`${base}/pulls/${pullRequest}`, lookup);
         if (linked != null) {
-          const pull = parsePull(linked);
+          const pull = parsePull(linked, host);
           const sameRepository = pull.headRepo?.toLowerCase() === repo.toLowerCase();
           if (
             pull.headRef === branch &&
@@ -501,7 +513,7 @@ export function createGitHubPullRequestSource({
        */
       const detail = await getJson(`${base}/pulls/${chosen.number}`, lookup);
       if (detail == null) return null;
-      const pull = parsePull(detail);
+      const pull = parsePull(detail, host);
       /** A push between the list and the detail can move the head off the recorded commit. */
       if (pull.headSha !== chosen.sha && !(await contains(pull.headSha))) return null;
       return report(pull);

packages/api/src/pulls/handler.spec.ts

@@ -90,6 +90,7 @@ describe('createConversationPullRequestHandler', () => {
       branch: 'feat/x',
       token: 'ghp_secret',
       head: null,
+      host: 'github.com',
       ttlMs: 30_000,
       cacheMaxEntries: 500,
       cacheMaxCredentials: 256,
@@ -107,6 +108,51 @@ describe('createConversationPullRequestHandler', () => {
     expect(res.json).toHaveBeenCalledWith({ pullRequest: pr });
   });
 
+  it('looks up on the configured GitHub host, normalized', async () => {
+    const { run, lookup } = setup({ settings: { ...enabled, host: ' GHE.Example.com ' } });
+    await run();
+    expect(lookup).toHaveBeenCalledWith(expect.objectContaining({ host: 'ghe.example.com' }));
+  });
+
+  /** The operator's YAML holds `base`; a role, group or user override in the database holds `override`. */
+  async function hostFor(base: Record<string, unknown>, override: Record<string, unknown>) {
+    const lookup = jest.fn().mockResolvedValue({ ok: true, value: pr });
+    const handler = createConversationPullRequestHandler({
+      getConvoLaneGit: jest.fn().mockResolvedValue({ branch: 'feat/x', head: null, repo: 'o/r' }),
+      getAppConfig: jest
+        .fn()
+        .mockImplementation(async (options: { baseOnly?: boolean }) =>
+          configWith(options.baseOnly ? base : override),
+        ),
+      lookup,
+      env: { GH_TOKEN: 'ghp_secret' },
+    });
+    const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };
+    await handler(
+      { user: { id: 'u1' }, params: { conversationId: 'c1' } } as unknown as ServerRequest,
+      res as unknown as Response,
+    );
+    return lookup.mock.calls[0]?.[0]?.host;
+  }
+
+  it('takes the host from the operator config, never from a database override', async () => {
+    const override = { ...enabled, host: 'attacker.example' };
+    expect(await hostFor({ ...enabled, host: 'ghe.example.com' }, override)).toBe(
+      'ghe.example.com',
+    );
+    expect(await hostFor(enabled, override)).toBe('github.com');
+  });
+
+  it('never sends the token anywhere when the host is not a host name', async () => {
+    const { run, res, lookup } = setup({
+      settings: { ...enabled, host: 'https://ghe.example.com' },
+    });
+    await run();
+    expect(lookup).not.toHaveBeenCalled();
+    expect(res.status).toHaveBeenCalledWith(200);
+    expect(res.json).toHaveBeenCalledWith({ pullRequest: null });
+  });
+
   it('passes the pull request the conversation linked to', async () => {
     const { run, lookup } = setup({
       laneGit: { branch: 'feat/x', head: null, repo: 'o/r', pullRequest: 7 },
@@ -280,12 +326,13 @@ describe('createConversationPullRequestHandler', () => {
 
   it('starts the config and the owner-scoped lane reads together, not one after the other', async () => {
     const order: string[] = [];
-    let releaseConfig: (config: unknown) => void = () => undefined;
+    const pending: Array<(config: unknown) => void> = [];
+    const releaseConfig = (config: unknown) => pending.forEach((resolve) => resolve(config));
     const getAppConfig = jest.fn(
       () =>
         new Promise((resolve) => {
           order.push('config:start');
-          releaseConfig = resolve;
+          pending.push(resolve);
         }),
     );
     const getConvoLaneGit = jest.fn(async () => {
@@ -304,7 +351,8 @@ describe('createConversationPullRequestHandler', () => {
       res as unknown as Response,
     );
     await new Promise((resolve) => setImmediate(resolve));
-    expect(order).toEqual(['config:start', 'lane:start']);
+    /** The resolved config and the operator's base config, then the lane, all before any settles. */
+    expect(order).toEqual(['config:start', 'config:start', 'lane:start']);
     releaseConfig(configWith(enabled));
     await done;
     expect(res.json).toHaveBeenCalledWith({ pullRequest: pr });
@@ -417,6 +465,26 @@ describe('createConversationPullRequestsHandler', () => {
     return { run, res, lookup, getConvosLaneGit };
   }
 
+  it('takes the host from the operator config for the batch too', async () => {
+    const lookup = jest.fn().mockResolvedValue({ ok: true, value: pr });
+    const handler = createConversationPullRequestsHandler({
+      getConvosLaneGit: jest.fn().mockResolvedValue([lane('a')]),
+      getAppConfig: jest
+        .fn()
+        .mockImplementation(async (options: { baseOnly?: boolean }) =>
+          configWith(options.baseOnly ? enabled : { ...enabled, host: 'attacker.example' }),
+        ),
+      lookup,
+      env: { GH_TOKEN: 'ghp_secret' },
+    });
+    const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };
+    await handler(
+      { user: { id: 'u1' }, body: { conversationIds: ['a'] } } as unknown as ServerRequest,
+      res as unknown as Response,
+    );
+    expect(lookup).toHaveBeenCalledWith(expect.objectContaining({ host: 'github.com' }));
+  });
+
   it('keeps conversations on one lane apart when they linked different pull requests', async () => {
     const linked = (conversationId: string, pullRequest: number) => ({
       conversationId,

packages/api/src/pulls/handler.ts

@@ -14,7 +14,11 @@ import type { GetAppConfigOptions } from '~/app/service';
 import type { PullRequestSettings } from './settings';
 import type { PullRequestLookup } from './types';
 import type { ServerRequest } from '~/types';
-import { isPullRequestFeatureActive, resolvePullRequestToken } from './settings';
+import {
+  resolvePullRequestHost,
+  resolvePullRequestToken,
+  isPullRequestFeatureActive,
+} from './settings';
 import { ALL_REPOSITORIES, isAllowedRepository } from './repository';
 import { getAppConfigOptionsFromUser } from '~/app/service';
 import { getSafeErrorMetadata } from '~/utils';
@@ -177,13 +181,40 @@ function eligibleLane(
   };
 }
 
+type ServerUser = ServerRequest['user'];
+
+const agentsPullRequests = (config: AppConfig): PullRequestSettings | undefined =>
+  (config.endpoints?.[EModelEndpoint.agents] as TAgentsEndpoint | undefined)?.pullRequests;
+
+/**
+ * The user's settings, with `host` taken from the operator's YAML alone. Role, group and user
+ * overrides live in the database and may be edited by delegated administrators; letting one set
+ * the host would let it send the deployment's token to a server of its choosing.
+ */
+async function readSettings(
+  getAppConfig: (options: GetAppConfigOptions) => Promise<AppConfig>,
+  user: ServerUser,
+): Promise<PullRequestSettings | undefined> {
+  const options = { ...getAppConfigOptionsFromUser(user), skipRuntimeAugmentation: true };
+  const [resolved, base] = await Promise.all([
+    getAppConfig({ ...options, failClosed: true }),
+    getAppConfig({ ...options, baseOnly: true }),
+  ]);
+  const settings = agentsPullRequests(resolved);
+  if (settings == null) return undefined;
+  const { host: _override, ...rest } = settings;
+  const host = agentsPullRequests(base)?.host;
+  return host != null ? { ...rest, host } : rest;
+}
+
 /** Everything one lookup needs from the settings, so the single and batch routes cannot differ. */
 function lookupInput(settings: PullRequestSettings, token: string, lane: EligibleLane) {
   return {
     repo: lane.repo,
     branch: lane.branch,
     head: lane.head,
     ...(lane.pullRequest != null ? { pullRequest: lane.pullRequest } : {}),
+    host: resolvePullRequestHost(settings.host) ?? undefined,
     token,
     ttlMs: (settings.cacheTtlSeconds ?? 30) * 1000,
     cacheMaxEntries: settings.cacheMaxEntries ?? 500,
@@ -253,16 +284,10 @@ export function createConversationPullRequestHandler(deps: {
        * used when the resolved config turns the feature on, which keeps a disabled deployment's
        * answer the same as before.
        */
-      const [appConfig, laneGit] = await Promise.all([
-        deps.getAppConfig({
-          ...getAppConfigOptionsFromUser(req.user),
-          skipRuntimeAugmentation: true,
-          failClosed: true,
-        }),
+      const [settings, laneGit] = await Promise.all([
+        readSettings(deps.getAppConfig, req.user),
         deps.getConvoLaneGit(userId, conversationId),
       ]);
-      const settings = (appConfig.endpoints?.[EModelEndpoint.agents] as TAgentsEndpoint | undefined)
-        ?.pullRequests;
       if (!settings || !isPullRequestFeatureActive(settings, deps.env)) {
         res.status(200).json(NONE);
         return;
@@ -334,11 +359,7 @@ export function createConversationPullRequestsHandler(deps: {
        * overlapped.
        */
       const startedAt = Date.now();
-      const configRead = deps.getAppConfig({
-        ...getAppConfigOptionsFromUser(req.user),
-        skipRuntimeAugmentation: true,
-        failClosed: true,
-      });
+      const configRead = readSettings(deps.getAppConfig, req.user);
       const laneRead = deps.getConvosLaneGit(userId, ids);
       /** A lane read nobody waits for (feature off, config timed out) must not fail unhandled. */
       laneRead.catch(() => undefined);
@@ -351,9 +372,7 @@ export function createConversationPullRequestsHandler(deps: {
         unavailable();
         return;
       }
-      const appConfig = loaded;
-      const settings = (appConfig.endpoints?.[EModelEndpoint.agents] as TAgentsEndpoint | undefined)
-        ?.pullRequests;
+      const settings = loaded;
       const deadline =
         startedAt + (settings?.batchTimeoutSeconds ?? DEFAULT_BATCH_TIMEOUT_SECONDS) * 1000;
       const none = (): TConversationPullRequestsResponse => ({