Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added
- `POST /api/connections/{id}/sync` enqueues a new `ConnectionSyncJob` for a connection in the caller's org; the caller must be an OWNER, cross-org access returns 404, and the response is `{ jobId }` so on-call scripts can force a re-sync without going through the web UI. [#1520](https://github.com/sourcebot-dev/sourcebot/pull/1520)

### Fixed
- Upgraded `seroval` to `^1.5.6`. [#1508](https://github.com/sourcebot-dev/sourcebot/pull/1508)

Expand Down
96 changes: 96 additions & 0 deletions docs/api-reference/sourcebot-public.openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,18 @@
"status"
]
},
"PublicSyncConnectionResponse": {
"type": "object",
"properties": {
"jobId": {
"type": "string",
"description": "The id of the new ConnectionSyncJob row. Poll the connection detail endpoint to see when the job transitions from PENDING/IN_PROGRESS to COMPLETED/FAILED."
}
},
"required": [
"jobId"
]
},
"PublicFileSourceResponse": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -1479,6 +1491,90 @@
}
}
},
"/api/connections/{id}/sync": {
"post": {
"operationId": "triggerConnectionSync",
"tags": [
"System"
],
"summary": "Trigger a connection sync",
"description": "Enqueues a new ConnectionSyncJob row for the connection in the caller's org. The job runs asynchronously; the response returns the new job id. The caller must be an OWNER in the org. Cross-org access returns 404 (not 403) to avoid leaking the existence of connections in other orgs.",
"parameters": [
{
"schema": {
"type": "integer",
"minimum": 0,
"exclusiveMinimum": true
},
"required": true,
"name": "id",
"in": "path"
}
],
"responses": {
"202": {
"description": "Sync job enqueued. Poll `GET /api/connections/{id}` to see the job progress.",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/PublicSyncConnectionResponse"
}
}
}
},
"400": {
"description": "Invalid `id` path parameter.",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/PublicApiServiceError"
}
}
}
},
"401": {
"description": "Not authenticated.",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/PublicApiServiceError"
}
}
}
},
"403": {
"description": "Caller is not an OWNER in the org.",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/PublicApiServiceError"
}
}
}
},
"404": {
"description": "Connection not found in the org.",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/PublicApiServiceError"
}
}
}
},
"500": {
"description": "Worker is unreachable or returned an unexpected response.",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/PublicApiServiceError"
}
}
}
}
}
}
},
"/api/source": {
"get": {
"operationId": "getFileSource",
Expand Down
3 changes: 2 additions & 1 deletion docs/docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@
"icon": "server",
"pages": [
"GET /api/version",
"GET /api/health"
"GET /api/health",
"POST /api/connections/{id}/sync"
]
}
]
Expand Down
193 changes: 193 additions & 0 deletions packages/web/src/app/api/(server)/connections/[id]/sync/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
import { beforeEach, describe, expect, test, vi } from 'vitest';
import { NextRequest } from 'next/server';
import { OrgRole } from '@sourcebot/db';

type AuthContext = {
user: { id: string };
org: { id: number };
role: OrgRole;
prisma: unknown;
};

const mocks = vi.hoisted(() => ({
authContext: undefined as AuthContext | undefined,
findFirst: vi.fn(),
workerResponse: undefined as { ok: boolean; status: number; json: () => Promise<unknown> } | undefined,
}));

vi.mock('server-only', () => ({}));

vi.mock('@sourcebot/db', () => ({
OrgRole: { MEMBER: 'MEMBER', OWNER: 'OWNER' },
}));

vi.mock('@sourcebot/shared', () => ({
env: { WORKER_API_URL: 'http://worker.example.com' },
createLogger: () => ({
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
}),
}));

vi.mock('@/lib/posthog', () => ({
captureEvent: vi.fn(),
}));

vi.mock('@/middleware/withAuth', () => ({
withAuth: vi.fn(async (callback: (ctx: AuthContext) => unknown) => {
if (!mocks.authContext) {
return { statusCode: 401, errorCode: 'NOT_AUTHENTICATED', message: 'Not authenticated' };
}
return callback(mocks.authContext);
}),
}));

vi.mock('@sentry/nextjs', () => ({
captureException: vi.fn(),
captureRequestError: vi.fn(),
}));

vi.mock('@opentelemetry/sdk-trace-base', () => ({
getEnv: () => ({}),
TraceIdRatioBasedSampler: vi.fn(),
ParentBasedSampler: vi.fn(),
AlwaysOnSampler: vi.fn(),
AlwaysOffSampler: vi.fn(),
}));

const setAuth = (opts: {
role?: OrgRole;
connectionExists?: boolean;
}) => {
mocks.findFirst.mockImplementation(async () => opts.connectionExists ? { id: 1 } : null);
mocks.authContext = {
user: { id: 'user_1' },
org: { id: 1 },
role: opts.role ?? OrgRole.OWNER,
prisma: { connection: { findFirst: mocks.findFirst } },
};
};

const makeRequest = (id: string): NextRequest =>
new NextRequest(`http://localhost/api/connections/${id}/sync`, { method: 'POST' });

const { POST } = await import('./route');

// Mock the global fetch that the action uses to call the worker.
const originalFetch = globalThis.fetch;
beforeEach(() => {
vi.restoreAllMocks();
globalThis.fetch = vi.fn(async () => {
if (!mocks.workerResponse) {
throw new Error('workerResponse not set');
}
return new Response(JSON.stringify(await mocks.workerResponse.json()), {
status: mocks.workerResponse.status,
headers: { 'Content-Type': 'application/json' },
});
}) as typeof fetch;
mocks.authContext = undefined;
mocks.workerResponse = undefined;
});

// Restore the original fetch when the suite finishes so other test
// files are not affected.
import { afterAll } from 'vitest';
afterAll(() => {
globalThis.fetch = originalFetch;
});

describe('POST /api/connections/:id/sync', () => {
test('returns 401 when no authenticated user', async () => {
mocks.authContext = undefined;
const response = await POST(makeRequest('1'), { params: Promise.resolve({ id: '1' }) });
expect(response.status).toBe(401);
});

test('returns 403 when the caller is a MEMBER (not OWNER)', async () => {
setAuth({ role: OrgRole.MEMBER, connectionExists: true });
const response = await POST(makeRequest('1'), { params: Promise.resolve({ id: '1' }) });
expect(response.status).toBe(403);
// The worker must not be called when the caller is a MEMBER.
expect(globalThis.fetch).not.toHaveBeenCalled();
});

test('returns 400 when id is not a positive integer', async () => {
setAuth({ role: OrgRole.OWNER, connectionExists: false });
const response = await POST(makeRequest('abc'), { params: Promise.resolve({ id: 'abc' }) });
expect(response.status).toBe(400);
expect(globalThis.fetch).not.toHaveBeenCalled();
});

test('returns 400 when id is zero or negative', async () => {
setAuth({ role: OrgRole.OWNER, connectionExists: false });
const response = await POST(makeRequest('0'), { params: Promise.resolve({ id: '0' }) });
expect(response.status).toBe(400);
expect(globalThis.fetch).not.toHaveBeenCalled();
});

test('returns 404 when the connection does not exist in the org', async () => {
setAuth({ role: OrgRole.OWNER, connectionExists: false });
const response = await POST(makeRequest('999'), { params: Promise.resolve({ id: '999' }) });
expect(response.status).toBe(404);
expect(globalThis.fetch).not.toHaveBeenCalled();
});

test('returns 404 (not 403) for cross-org access to avoid leaking existence', async () => {
// The mock returns null for `connectionExists: false` regardless
// of which id is requested, simulating the case where the
// connection lives in a different org and is therefore invisible
// to the current org. The action scopes by `where: { id, orgId }`
// so cross-org ids collapse to "not found", same as the detail
// endpoint.
setAuth({ role: OrgRole.OWNER, connectionExists: false });
const response = await POST(makeRequest('42'), { params: Promise.resolve({ id: '42' }) });
expect(response.status).toBe(404);
// Assert that the action scoped the query by orgId. A future
// change that drops the orgId filter would still pass the 404
// assertion (mock returns null regardless of args) but would
// expose cross-org data in production.
expect(mocks.findFirst).toHaveBeenCalledWith(
expect.objectContaining({ where: { id: 42, orgId: 1 } }),
);
expect(globalThis.fetch).not.toHaveBeenCalled();
});

test('returns 202 with the new jobId on the happy path', async () => {
setAuth({ role: OrgRole.OWNER, connectionExists: true });
mocks.workerResponse = {
ok: true,
status: 200,
json: async () => ({ jobId: 'job_abc' }),
};

const response = await POST(makeRequest('1'), { params: Promise.resolve({ id: '1' }) });
const body = await response.json();

expect(response.status).toBe(202);
expect(body).toEqual({ jobId: 'job_abc' });

// The action called the worker with the connectionId in the body.
expect(globalThis.fetch).toHaveBeenCalledWith(
'http://worker.example.com/api/sync-connection',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ connectionId: 1 }),
}),
);
});

test('returns 500 when the worker returns a non-ok response', async () => {
setAuth({ role: OrgRole.OWNER, connectionExists: true });
mocks.workerResponse = {
ok: false,
status: 500,
json: async () => ({ error: 'worker exploded' }),
} as unknown as { ok: boolean; status: number; json: () => Promise<unknown> };

const response = await POST(makeRequest('1'), { params: Promise.resolve({ id: '1' }) });
expect(response.status).toBe(500);
});
});
32 changes: 32 additions & 0 deletions packages/web/src/app/api/(server)/connections/[id]/sync/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use server';

import { NextRequest } from 'next/server';
import { z } from 'zod';

import { triggerConnectionSync } from '@/features/workerApi/actions';
import { apiHandler } from '@/lib/apiHandler';
import { queryParamsSchemaValidationError, serviceErrorResponse } from '@/lib/serviceError';
import { isServiceError } from '@/lib/utils';

const connectionIdParamSchema = z.coerce.number().int().positive();

// eslint-disable-next-line authz/require-auth-wrapper -- delegates to triggerConnectionSync() which calls withAuth + withMinimumOrgRole(OWNER)
export const POST = apiHandler(async (
_request: NextRequest,
{ params }: { params: Promise<{ id: string }> },
) => {
const { id: rawId } = await params;
const idParsed = connectionIdParamSchema.safeParse(rawId);
if (!idParsed.success) {
return serviceErrorResponse(queryParamsSchemaValidationError(idParsed.error));
}
Comment on lines +19 to +22

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the queryParamsSchemaValidationError implementation and whether a dedicated
# path-param error helper already exists in the codebase.
rg -n -A8 'export function queryParamsSchemaValidationError|export const queryParamsSchemaValidationError' packages/web/src/lib/serviceError.ts
rg -n 'pathParam|PathParam' packages/web/src/lib/serviceError.ts

Repository: sourcebot-dev/sourcebot

Length of output: 552


🏁 Script executed:

#!/bin/bash
set -e

echo "== file outline =="
ast-grep outline 'packages/web/src/app/api/(server)/connections/[id]/sync/route.ts' --view expanded || true

echo "== relevant route text =="
cat -n 'packages/web/src/app/api/(server)/connections/[id]/sync/route.ts' | sed -n '1,180p'

echo "== error helpers references =="
rg -n 'queryParamsSchemaValidationError|queryParamsSchemaValidation|schemaValidationError|INVALID_QUERY_PARAMS|Invalid |path parameter' packages/web/src/openapi packages/web/src/app/api packages/web/src/lib/serviceError.ts | sed -n '1,220p'

Repository: sourcebot-dev/sourcebot

Length of output: 8789


🏁 Script executed:

#!/bin/bash
set -e

echo "== OpenAPI invalid id schema references =="
rg -n -C 5 'Invalid `id` path parameter|id.*path|INVALID_QUERY_PARAMS|INVALID_PATH|queryParamsSchemaValidationError' packages/web/src/openapi docs/docs.json packages/web/src/app/api packages/web/src/lib/serviceError.ts | sed -n '1,260p'

Repository: sourcebot-dev/sourcebot

Length of output: 23180


Use a path-parameter schema validation helper for this 400.

id is parsed from params.id, but queryParamsSchemaValidationError returns INVALID_QUERY_PARAMS and "Query params validation failed with: ...", while the OpenAPI response documents this 400 as "Invalid \id` path parameter."Add/return a path-parameter validation error instead so the client error code and message match theid` path segment.

🤖 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/web/src/app/api/`(server)/connections/[id]/sync/route.ts around
lines 19 - 22, Replace queryParamsSchemaValidationError in the rawId validation
branch with the existing path-parameter schema validation helper, passing
idParsed.error and preserving serviceErrorResponse. Ensure the returned 400 uses
the path-parameter error code and message documented for the id segment.

const id = idParsed.data;

const result = await triggerConnectionSync(id);

if (isServiceError(result)) {
return serviceErrorResponse(result);
}

return Response.json({ jobId: result.jobId }, { status: 202 });
});
Loading