From a3f08d30fd71ea65b08b5ffe554f1fe868336159 Mon Sep 17 00:00:00 2001 From: CarmenDou <15951653662@163.com> Date: Thu, 30 Jul 2026 16:22:05 -0700 Subject: [PATCH 1/3] feat: connect Apify with a token on self-hosted backends Adds `insforge webscraper apify connect --token ` so agents without dashboard access can wire up Apify on a self-hosted InsForge backend, using the PUT /api/webscraper/apify/config endpoint instead of OAuth. Also fixes the ossFetch 404 message under /api/webscraper, which used to claim the feature is cloud-only and unsupported when self-hosted; a 404 now just means the backend predates the feature. --- src/commands/webscraper/apify/connect.test.ts | 251 ++++++++++++++++++ src/commands/webscraper/apify/connect.ts | 25 ++ src/lib/api/apify-config.test.ts | 64 +++++ src/lib/api/apify-config.ts | 33 +++ src/lib/api/oss.test.ts | 25 ++ src/lib/api/oss.ts | 2 +- 6 files changed, 399 insertions(+), 1 deletion(-) create mode 100644 src/commands/webscraper/apify/connect.test.ts create mode 100644 src/lib/api/apify-config.test.ts create mode 100644 src/lib/api/apify-config.ts diff --git a/src/commands/webscraper/apify/connect.test.ts b/src/commands/webscraper/apify/connect.test.ts new file mode 100644 index 00000000..bd7dd085 --- /dev/null +++ b/src/commands/webscraper/apify/connect.test.ts @@ -0,0 +1,251 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { Command } from 'commander'; + +const apiMock = vi.hoisted(() => ({ + startApifyCliFlow: vi.fn(), + pollApifyConnection: vi.fn(), + fetchApifyConnection: vi.fn(), +})); +vi.mock('../../../lib/api/apify.js', () => apiMock); + +const apifyConfigMock = vi.hoisted(() => ({ + storeApifyToken: vi.fn(), +})); +vi.mock('../../../lib/api/apify-config.js', () => apifyConfigMock); + +const configMock = vi.hoisted(() => ({ + getProjectConfig: vi.fn(() => ({ project_id: 'p1', project_name: 'Test Project' })), + getAccessToken: vi.fn((): string | null => 'tok'), +})); +vi.mock('../../../lib/config.js', () => configMock); + +const analyticsMock = vi.hoisted(() => ({ + trackGroupCommand: vi.fn(), + shutdownAnalytics: vi.fn(async () => {}), +})); +vi.mock('../../../lib/analytics.js', () => analyticsMock); + +const bridgeMock = vi.hoisted(() => ({ + runApifyAuthBridge: vi.fn(async () => ({ skillsInstalled: true })), +})); +vi.mock('../../../lib/apify-bridge.js', () => bridgeMock); + +vi.mock('../../../lib/prompts.js', () => ({ isInteractive: false })); + +// `open` is loaded dynamically inside runConnectFlow (OAuth path); mock so the +// real browser launch doesn't fire during tests. +vi.mock('open', () => ({ default: vi.fn() })); + +// Silence interactive UI noise from clack — tests assert on mocks, not stdout. +vi.mock('@clack/prompts', async (orig) => { + const actual = (await orig()) as Record; + return { + ...actual, + intro: vi.fn(), + outro: vi.fn(), + log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn() }, + spinner: vi.fn(() => ({ start: vi.fn(), stop: vi.fn(), message: vi.fn() })), + }; +}); + +const outputMock = vi.hoisted(() => ({ + outputJson: vi.fn(), + outputSuccess: vi.fn(), +})); +vi.mock('../../../lib/output.js', () => outputMock); + +// Imports must come AFTER the vi.mock calls because Vitest hoists the mocks +// but ESM module evaluation order still matters. +import { registerApifyConnectCommand } from './connect.js'; +import { CLIError } from '../../../lib/errors.js'; + +interface RunResult { + exitCode?: number; +} + +// Override process.exit so handleError doesn't kill the test process; capture +// the first exit code. Mirrors src/commands/posthog/setup.test.ts. +async function runConnect(argv: string[]): Promise { + const program = new Command(); + program.option('--json').option('--api-url ').option('-y, --yes'); + const webscraper = program.command('webscraper'); + const apify = webscraper.command('apify'); + registerApifyConnectCommand(apify); + + const origExit = process.exit; + const result: RunResult = {}; + (process.exit as unknown) = (code?: number) => { + if (result.exitCode === undefined) result.exitCode = code; + throw new Error('__exit__'); + }; + try { + await program + .parseAsync(['node', 'test', 'webscraper', 'apify', 'connect', ...argv]) + .catch((err) => { + if (err instanceof Error && err.message === '__exit__') return; + throw err; + }); + } finally { + process.exit = origExit; + } + return result; +} + +beforeEach(() => { + apiMock.startApifyCliFlow.mockReset(); + apiMock.pollApifyConnection.mockReset(); + apiMock.fetchApifyConnection.mockReset(); + apifyConfigMock.storeApifyToken.mockReset(); + bridgeMock.runApifyAuthBridge.mockReset().mockResolvedValue({ skillsInstalled: true }); + outputMock.outputJson.mockReset(); + outputMock.outputSuccess.mockReset(); + analyticsMock.trackGroupCommand.mockReset(); + analyticsMock.shutdownAnalytics.mockClear(); + configMock.getProjectConfig.mockReturnValue({ project_id: 'p1', project_name: 'Test Project' }); + configMock.getAccessToken.mockReturnValue('tok'); +}); + +describe('apify connect', () => { + describe('--token path (self-hosted)', () => { + it('stores the token and skips OAuth entirely', async () => { + apifyConfigMock.storeApifyToken.mockResolvedValue({ + configured: true, + maskedKey: 'apify_ap••••••••mnop', + }); + + await runConnect(['--token', 'apify_api_tok1234567890']); + + expect(apifyConfigMock.storeApifyToken).toHaveBeenCalledWith('apify_api_tok1234567890'); + expect(apiMock.startApifyCliFlow).not.toHaveBeenCalled(); + expect(apiMock.pollApifyConnection).not.toHaveBeenCalled(); + expect(apiMock.fetchApifyConnection).not.toHaveBeenCalled(); + expect(bridgeMock.runApifyAuthBridge).not.toHaveBeenCalled(); + }); + + it('does not require a login token', async () => { + configMock.getAccessToken.mockReturnValue(null); + apifyConfigMock.storeApifyToken.mockResolvedValue({ + configured: true, + maskedKey: 'apify_ap••••••••mnop', + }); + + const r = await runConnect(['--token', 'apify_api_tok1234567890']); + + expect(r.exitCode).toBeUndefined(); + expect(apifyConfigMock.storeApifyToken).toHaveBeenCalledOnce(); + }); + + it('prints the masked key, never the raw token, in non-json mode', async () => { + apifyConfigMock.storeApifyToken.mockResolvedValue({ + configured: true, + maskedKey: 'apify_ap••••••••mnop', + }); + + await runConnect(['--token', 'apify_api_tok1234567890']); + + expect(outputMock.outputSuccess).toHaveBeenCalledWith( + expect.stringContaining('apify_ap••••••••mnop'), + ); + const allCalls = [...outputMock.outputSuccess.mock.calls, ...outputMock.outputJson.mock.calls]; + const serialized = JSON.stringify(allCalls); + expect(serialized).not.toContain('apify_api_tok1234567890'); + }); + + it('emits the masked token status in --json mode', async () => { + apifyConfigMock.storeApifyToken.mockResolvedValue({ + configured: true, + maskedKey: 'apify_ap••••••••mnop', + }); + + await runConnect(['--json', '--token', 'apify_api_tok1234567890']); + + expect(outputMock.outputJson).toHaveBeenCalledOnce(); + const payload = outputMock.outputJson.mock.calls[0][0] as { + success: boolean; + connectionState: string; + token: { configured: boolean; maskedKey: string | null }; + }; + expect(payload.success).toBe(true); + expect(payload.connectionState).toBe('newly-connected'); + expect(payload.token).toEqual({ configured: true, maskedKey: 'apify_ap••••••••mnop' }); + }); + + it('propagates a rejected token as a CLI error and exits non-zero', async () => { + apifyConfigMock.storeApifyToken.mockRejectedValue( + new CLIError('Apify rejected this API token.', 1, 'INVALID_INPUT', 400), + ); + + const r = await runConnect(['--token', 'bogus']); + + expect(r.exitCode).toBe(1); + }); + }); + + describe('OAuth path (--token absent) is unchanged', () => { + it('fast path: cli-start says connected → verifies via /connection, skips polling, runs the auth bridge', async () => { + apiMock.startApifyCliFlow.mockResolvedValue({ type: 'connected' }); + apiMock.fetchApifyConnection.mockResolvedValue({ + kind: 'connected', + connection: { apifyUsername: 'someone', plan: 'free', status: 'active' }, + }); + + await runConnect(['--skip-browser']); + + expect(apiMock.startApifyCliFlow).toHaveBeenCalledOnce(); + expect(apiMock.fetchApifyConnection).toHaveBeenCalledOnce(); + expect(apiMock.pollApifyConnection).not.toHaveBeenCalled(); + expect(bridgeMock.runApifyAuthBridge).toHaveBeenCalledOnce(); + expect(apifyConfigMock.storeApifyToken).not.toHaveBeenCalled(); + }); + + it('slow path: cli-start returns authorizeUrl → polls until connected', async () => { + apiMock.startApifyCliFlow.mockResolvedValue({ + type: 'authorize', + authorizeUrl: 'https://example.com/auth', + }); + apiMock.pollApifyConnection.mockResolvedValue({ + apifyUsername: 'someone', + plan: 'free', + status: 'active', + }); + + await runConnect(['--skip-browser']); + + expect(apiMock.pollApifyConnection).toHaveBeenCalledOnce(); + expect(apiMock.fetchApifyConnection).not.toHaveBeenCalled(); + expect(apifyConfigMock.storeApifyToken).not.toHaveBeenCalled(); + }); + + it('still requires a login token, exactly as before', async () => { + configMock.getAccessToken.mockReturnValue(null); + apiMock.startApifyCliFlow.mockResolvedValue({ type: 'connected' }); + + const r = await runConnect(['--skip-browser']); + + expect(r.exitCode).toBeGreaterThan(0); + expect(apiMock.startApifyCliFlow).not.toHaveBeenCalled(); + }); + + it('emits JSON with connectionState and connection, no token field', async () => { + apiMock.startApifyCliFlow.mockResolvedValue({ type: 'connected' }); + apiMock.fetchApifyConnection.mockResolvedValue({ + kind: 'connected', + connection: { apifyUsername: 'someone', plan: 'free', status: 'active' }, + }); + + await runConnect(['--json', '--skip-browser']); + + expect(outputMock.outputJson).toHaveBeenCalledOnce(); + const payload = outputMock.outputJson.mock.calls[0][0] as { + success: boolean; + connectionState: string; + connection: unknown; + token?: unknown; + }; + expect(payload.success).toBe(true); + expect(payload.connectionState).toBe('already-connected'); + expect(payload.connection).toMatchObject({ apifyUsername: 'someone', plan: 'free' }); + expect(payload.token).toBeUndefined(); + }); + }); +}); diff --git a/src/commands/webscraper/apify/connect.ts b/src/commands/webscraper/apify/connect.ts index 18de2e3a..c7b95559 100644 --- a/src/commands/webscraper/apify/connect.ts +++ b/src/commands/webscraper/apify/connect.ts @@ -16,6 +16,7 @@ import { startApifyCliFlow, type ApifyConnectionResponse, } from '../../../lib/api/apify.js'; +import { storeApifyToken, type ApifyTokenStatus } from '../../../lib/api/apify-config.js'; import { outputJson, outputSuccess } from '../../../lib/output.js'; import { trackGroupCommand, shutdownAnalytics } from '../../../lib/analytics.js'; import { runApifyAuthBridge } from '../../../lib/apify-bridge.js'; @@ -33,6 +34,8 @@ interface ConnectResult { plan?: string | null; status?: string; }; + /** Present only on the self-hosted `--token` path. */ + token?: ApifyTokenStatus; } export function registerApifyConnectCommand(program: Command): void { @@ -40,6 +43,7 @@ export function registerApifyConnectCommand(program: Command): void { .command('connect') .description('Connect your Apify account to your InsForge project') .option('--skip-browser', 'Do not auto-open the browser for OAuth; only print the URL') + .option('--token ', 'Apify API token (self-hosted; skips the OAuth flow)') .action(async (opts, cmd) => { const { json, apiUrl } = getRootOpts(cmd); try { @@ -47,6 +51,7 @@ export function registerApifyConnectCommand(program: Command): void { json, apiUrl, skipBrowser: Boolean(opts.skipBrowser), + token: opts.token, }); if (json) { outputJson({ success: true, ...result }); @@ -69,6 +74,8 @@ interface RunConnectOpts { json: boolean; apiUrl?: string; skipBrowser: boolean; + /** Self-hosted path: an Apify API token to store directly, bypassing OAuth. */ + token?: string; } // Ensures the InsForge project has an Apify connection (cli-start / OAuth). @@ -82,6 +89,24 @@ async function runConnect(opts: RunConnectOpts): Promise { throw new ProjectNotLinkedError(); } + // Self-hosted token path: the token is validated and stored server-side via + // the existing admin-authenticated ossFetch, so it needs neither the login + // token below nor any of the OAuth / auth-bridge flow further down. Short- + // circuits before any OAuth work; everything below is unreachable and thus + // unchanged when --token is absent. + if (opts.token) { + trackGroupCommand('apify', 'connect', proj); + const status = await storeApifyToken(opts.token); + if (!opts.json) { + outputSuccess(`Apify connected with token ${status.maskedKey ?? '(hidden)'}`); + } + return { + connectionState: 'newly-connected', + connection: { apifyUsername: null, plan: null, status: 'active' }, + token: status, + }; + } + // 2. Login token const token = getAccessToken(); if (!token) { diff --git a/src/lib/api/apify-config.test.ts b/src/lib/api/apify-config.test.ts new file mode 100644 index 00000000..1e5a4647 --- /dev/null +++ b/src/lib/api/apify-config.test.ts @@ -0,0 +1,64 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import * as oss from './oss.js'; +import { CLIError } from '../errors.js'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('storeApifyToken', () => { + it('PUTs the token to the OSS host and returns the masked status', async () => { + const spy = vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response( + JSON.stringify({ token: { configured: true, maskedKey: 'apify_ap••••••••mnop' } }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ), + ); + + const { storeApifyToken } = await import('./apify-config.js'); + + await expect(storeApifyToken('apify_api_tok1234567890')).resolves.toEqual({ + configured: true, + maskedKey: 'apify_ap••••••••mnop', + }); + expect(spy).toHaveBeenCalledWith('/api/webscraper/apify/config', { + method: 'PUT', + body: JSON.stringify({ apiToken: 'apify_api_tok1234567890' }), + }); + }); + + it('propagates the backend message when Apify rejects the token', async () => { + vi.spyOn(oss, 'ossFetch').mockRejectedValue( + new CLIError('Apify rejected this API token.', 1, 'INVALID_INPUT', 400), + ); + + const { storeApifyToken } = await import('./apify-config.js'); + + await expect(storeApifyToken('bogus')).rejects.toThrow(/Apify rejected this API token/); + }); + + it('propagates the cloud-managed message on a cloud project', async () => { + vi.spyOn(oss, 'ossFetch').mockRejectedValue( + new CLIError('The Apify connection is managed by InsForge Cloud.', 1, 'INVALID_INPUT', 400), + ); + + const { storeApifyToken } = await import('./apify-config.js'); + + await expect(storeApifyToken('apify_api_tok1234567890')).rejects.toThrow(/InsForge Cloud/); + }); + + it('fails loudly when the response carries no token status', async () => { + vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response(JSON.stringify({}), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + const { storeApifyToken } = await import('./apify-config.js'); + + await expect(storeApifyToken('apify_api_tok1234567890')).rejects.toThrow( + /no token status/i, + ); + }); +}); diff --git a/src/lib/api/apify-config.ts b/src/lib/api/apify-config.ts new file mode 100644 index 00000000..00fda3f8 --- /dev/null +++ b/src/lib/api/apify-config.ts @@ -0,0 +1,33 @@ +import { ossFetch } from './oss.js'; +import { CLIError } from '../errors.js'; + +export interface ApifyTokenStatus { + configured: boolean; + maskedKey: string | null; +} + +/** + * Store a developer-supplied Apify API token on a self-hosted InsForge backend. + * + * Calls PUT /api/webscraper/apify/config on the project's OSS host. The backend + * validates the token against Apify before saving, so a 400 here means the token + * is bad — or that this is a cloud project, where the connection is made by OAuth + * instead. Both arrive as a CLIError from ossFetch carrying the backend's message, + * so they are left to propagate unchanged. + */ +export async function storeApifyToken(apiToken: string): Promise { + const res = await ossFetch('/api/webscraper/apify/config', { + method: 'PUT', + body: JSON.stringify({ apiToken }), + }); + + const data = (await res.json()) as { token?: ApifyTokenStatus }; + if (!data.token) { + throw new CLIError( + 'Apify config endpoint returned no token status; try again.', + 1, + 'APIFY_CONFIG_MALFORMED', + ); + } + return data.token; +} diff --git a/src/lib/api/oss.test.ts b/src/lib/api/oss.test.ts index 6893316a..9563136b 100644 --- a/src/lib/api/oss.test.ts +++ b/src/lib/api/oss.test.ts @@ -76,4 +76,29 @@ describe('ossFetch', () => { /Upgrade your InsForge project to a version with Model Gateway support/, ); }); + + it('shows an upgrade+token message for a route-level 404 under /api/webscraper (not a cloud-only warning)', async () => { + // A 404 here means the backend predates the web scraper feature, not that + // self-hosting is unsupported — self-hosted backends do serve this route + // once they're upgraded. Guards against regressing to the old wording. + vi.spyOn(config, 'getProjectConfig').mockReturnValue({ + project_id: 'p1', + project_name: 'demo', + org_id: 'o1', + appkey: 'app', + region: 'us-east', + api_key: 'ik_test', + oss_host: 'https://app.us-east.insforge.app', + } satisfies ProjectConfig); + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + new Response(JSON.stringify({ error: 'NOT_FOUND' }), { + status: 404, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect(ossFetch('/api/webscraper/apify/config')).rejects.toThrow( + /Upgrade your InsForge instance.*insforge webscraper apify connect --token/s, + ); + }); }); diff --git a/src/lib/api/oss.ts b/src/lib/api/oss.ts index 09231427..f786e2a0 100644 --- a/src/lib/api/oss.ts +++ b/src/lib/api/oss.ts @@ -210,7 +210,7 @@ export async function ossFetch( } if (res.status === 404 && isRouteLevel404 && path.startsWith('/api/webscraper')) { - message = 'The web scraper is not available on this backend.\nThe Apify web scraper is cloud-only. Self-hosted: this feature is not supported. Cloud: contact your InsForge admin to enable it.'; + message = 'The web scraper is not available on this backend.\nUpgrade your InsForge instance to a version with web scraper support, then run `insforge webscraper apify connect --token ` to connect your Apify account.'; } // Safe to treat any 404 on /api/advisor/* as a route-level miss: the OSS From 0da73ebdb93de74bd3d1dcfa6879a0a4d5e04b77 Mon Sep 17 00:00:00 2001 From: CarmenDou <15951653662@163.com> Date: Thu, 30 Jul 2026 17:30:50 -0700 Subject: [PATCH 2/3] refactor: name the CLI api module by domain and fold in the token call --- src/commands/webscraper/apify/connect.test.ts | 4 +- src/commands/webscraper/apify/connect.ts | 5 +- src/lib/api/apify-config.test.ts | 64 ------------------- src/lib/api/apify-config.ts | 33 ---------- .../api/{apify.test.ts => webscraper.test.ts} | 60 ++++++++++++++++- src/lib/api/{apify.ts => webscraper.ts} | 32 ++++++++++ 6 files changed, 95 insertions(+), 103 deletions(-) delete mode 100644 src/lib/api/apify-config.test.ts delete mode 100644 src/lib/api/apify-config.ts rename src/lib/api/{apify.test.ts => webscraper.test.ts} (72%) rename src/lib/api/{apify.ts => webscraper.ts} (89%) diff --git a/src/commands/webscraper/apify/connect.test.ts b/src/commands/webscraper/apify/connect.test.ts index bd7dd085..e70deb51 100644 --- a/src/commands/webscraper/apify/connect.test.ts +++ b/src/commands/webscraper/apify/connect.test.ts @@ -6,12 +6,10 @@ const apiMock = vi.hoisted(() => ({ pollApifyConnection: vi.fn(), fetchApifyConnection: vi.fn(), })); -vi.mock('../../../lib/api/apify.js', () => apiMock); - const apifyConfigMock = vi.hoisted(() => ({ storeApifyToken: vi.fn(), })); -vi.mock('../../../lib/api/apify-config.js', () => apifyConfigMock); +vi.mock('../../../lib/api/webscraper.js', () => ({ ...apiMock, ...apifyConfigMock })); const configMock = vi.hoisted(() => ({ getProjectConfig: vi.fn(() => ({ project_id: 'p1', project_name: 'Test Project' })), diff --git a/src/commands/webscraper/apify/connect.ts b/src/commands/webscraper/apify/connect.ts index c7b95559..794f32c6 100644 --- a/src/commands/webscraper/apify/connect.ts +++ b/src/commands/webscraper/apify/connect.ts @@ -14,9 +14,10 @@ import { fetchApifyConnection, pollApifyConnection, startApifyCliFlow, + storeApifyToken, type ApifyConnectionResponse, -} from '../../../lib/api/apify.js'; -import { storeApifyToken, type ApifyTokenStatus } from '../../../lib/api/apify-config.js'; + type ApifyTokenStatus, +} from '../../../lib/api/webscraper.js'; import { outputJson, outputSuccess } from '../../../lib/output.js'; import { trackGroupCommand, shutdownAnalytics } from '../../../lib/analytics.js'; import { runApifyAuthBridge } from '../../../lib/apify-bridge.js'; diff --git a/src/lib/api/apify-config.test.ts b/src/lib/api/apify-config.test.ts deleted file mode 100644 index 1e5a4647..00000000 --- a/src/lib/api/apify-config.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import * as oss from './oss.js'; -import { CLIError } from '../errors.js'; - -afterEach(() => { - vi.restoreAllMocks(); -}); - -describe('storeApifyToken', () => { - it('PUTs the token to the OSS host and returns the masked status', async () => { - const spy = vi.spyOn(oss, 'ossFetch').mockResolvedValue( - new Response( - JSON.stringify({ token: { configured: true, maskedKey: 'apify_ap••••••••mnop' } }), - { status: 200, headers: { 'Content-Type': 'application/json' } }, - ), - ); - - const { storeApifyToken } = await import('./apify-config.js'); - - await expect(storeApifyToken('apify_api_tok1234567890')).resolves.toEqual({ - configured: true, - maskedKey: 'apify_ap••••••••mnop', - }); - expect(spy).toHaveBeenCalledWith('/api/webscraper/apify/config', { - method: 'PUT', - body: JSON.stringify({ apiToken: 'apify_api_tok1234567890' }), - }); - }); - - it('propagates the backend message when Apify rejects the token', async () => { - vi.spyOn(oss, 'ossFetch').mockRejectedValue( - new CLIError('Apify rejected this API token.', 1, 'INVALID_INPUT', 400), - ); - - const { storeApifyToken } = await import('./apify-config.js'); - - await expect(storeApifyToken('bogus')).rejects.toThrow(/Apify rejected this API token/); - }); - - it('propagates the cloud-managed message on a cloud project', async () => { - vi.spyOn(oss, 'ossFetch').mockRejectedValue( - new CLIError('The Apify connection is managed by InsForge Cloud.', 1, 'INVALID_INPUT', 400), - ); - - const { storeApifyToken } = await import('./apify-config.js'); - - await expect(storeApifyToken('apify_api_tok1234567890')).rejects.toThrow(/InsForge Cloud/); - }); - - it('fails loudly when the response carries no token status', async () => { - vi.spyOn(oss, 'ossFetch').mockResolvedValue( - new Response(JSON.stringify({}), { - status: 200, - headers: { 'Content-Type': 'application/json' }, - }), - ); - - const { storeApifyToken } = await import('./apify-config.js'); - - await expect(storeApifyToken('apify_api_tok1234567890')).rejects.toThrow( - /no token status/i, - ); - }); -}); diff --git a/src/lib/api/apify-config.ts b/src/lib/api/apify-config.ts deleted file mode 100644 index 00fda3f8..00000000 --- a/src/lib/api/apify-config.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { ossFetch } from './oss.js'; -import { CLIError } from '../errors.js'; - -export interface ApifyTokenStatus { - configured: boolean; - maskedKey: string | null; -} - -/** - * Store a developer-supplied Apify API token on a self-hosted InsForge backend. - * - * Calls PUT /api/webscraper/apify/config on the project's OSS host. The backend - * validates the token against Apify before saving, so a 400 here means the token - * is bad — or that this is a cloud project, where the connection is made by OAuth - * instead. Both arrive as a CLIError from ossFetch carrying the backend's message, - * so they are left to propagate unchanged. - */ -export async function storeApifyToken(apiToken: string): Promise { - const res = await ossFetch('/api/webscraper/apify/config', { - method: 'PUT', - body: JSON.stringify({ apiToken }), - }); - - const data = (await res.json()) as { token?: ApifyTokenStatus }; - if (!data.token) { - throw new CLIError( - 'Apify config endpoint returned no token status; try again.', - 1, - 'APIFY_CONFIG_MALFORMED', - ); - } - return data.token; -} diff --git a/src/lib/api/apify.test.ts b/src/lib/api/webscraper.test.ts similarity index 72% rename from src/lib/api/apify.test.ts rename to src/lib/api/webscraper.test.ts index 5c7d0421..4ae9d191 100644 --- a/src/lib/api/apify.test.ts +++ b/src/lib/api/webscraper.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; +import * as oss from './oss.js'; import { CLIError } from '../errors.js'; -import { fetchApifyConnection, pollApifyConnection } from './apify.js'; +import { fetchApifyConnection, pollApifyConnection } from './webscraper.js'; const API = 'https://platform.test'; @@ -153,3 +154,60 @@ describe('pollApifyConnection', () => { ).rejects.toThrow(/cancelled/i); }); }); + +describe('storeApifyToken', () => { + it('PUTs the token to the OSS host and returns the masked status', async () => { + const spy = vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response( + JSON.stringify({ token: { configured: true, maskedKey: 'apify_ap••••••••mnop' } }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + await expect(storeApifyToken('apify_api_tok1234567890')).resolves.toEqual({ + configured: true, + maskedKey: 'apify_ap••••••••mnop', + }); + expect(spy).toHaveBeenCalledWith('/api/webscraper/apify/config', { + method: 'PUT', + body: JSON.stringify({ apiToken: 'apify_api_tok1234567890' }), + }); + }); + + it('propagates the backend message when Apify rejects the token', async () => { + vi.spyOn(oss, 'ossFetch').mockRejectedValue( + new CLIError('Apify rejected this API token.', 1, 'INVALID_INPUT', 400), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + await expect(storeApifyToken('bogus')).rejects.toThrow(/Apify rejected this API token/); + }); + + it('propagates the cloud-managed message on a cloud project', async () => { + vi.spyOn(oss, 'ossFetch').mockRejectedValue( + new CLIError('The Apify connection is managed by InsForge Cloud.', 1, 'INVALID_INPUT', 400), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + await expect(storeApifyToken('apify_api_tok1234567890')).rejects.toThrow(/InsForge Cloud/); + }); + + it('fails loudly when the response carries no token status', async () => { + vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response(JSON.stringify({}), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + await expect(storeApifyToken('apify_api_tok1234567890')).rejects.toThrow( + /no token status/i, + ); + }); +}); diff --git a/src/lib/api/apify.ts b/src/lib/api/webscraper.ts similarity index 89% rename from src/lib/api/apify.ts rename to src/lib/api/webscraper.ts index 67b20bcd..0fee6de2 100644 --- a/src/lib/api/apify.ts +++ b/src/lib/api/webscraper.ts @@ -1,5 +1,6 @@ import { getPlatformApiUrl } from '../config.js'; import { CLIError, formatFetchError } from '../errors.js'; +import { ossFetch } from './oss.js'; const REQUEST_TIMEOUT_MS = 30_000; @@ -136,6 +137,37 @@ export async function fetchApifyConnection( return { kind: 'connected', connection: conn }; } +export interface ApifyTokenStatus { + configured: boolean; + maskedKey: string | null; +} + +/** + * Store a developer-supplied Apify API token on a self-hosted InsForge backend. + * + * Calls PUT /api/webscraper/apify/config on the project's OSS host. The backend + * validates the token against Apify before saving, so a 400 here means the token + * is bad — or that this is a cloud project, where the connection is made by OAuth + * instead. Both arrive as a CLIError from ossFetch carrying the backend's message, + * so they are left to propagate unchanged. + */ +export async function storeApifyToken(apiToken: string): Promise { + const res = await ossFetch('/api/webscraper/apify/config', { + method: 'PUT', + body: JSON.stringify({ apiToken }), + }); + + const data = (await res.json()) as { token?: ApifyTokenStatus }; + if (!data.token) { + throw new CLIError( + 'Apify config endpoint returned no token status; try again.', + 1, + 'APIFY_CONFIG_MALFORMED', + ); + } + return data.token; +} + export interface PollOptions { /** Total deadline before giving up. */ timeoutMs: number; From dd56f9739690a7fb87441e42a13e52bd5d56b3eb Mon Sep 17 00:00:00 2001 From: CarmenDou <15951653662@163.com> Date: Thu, 30 Jul 2026 21:16:00 -0700 Subject: [PATCH 3/3] fix: address review findings on the self-hosted token connect path --- src/commands/webscraper/apify/connect.test.ts | 20 ++++++ src/commands/webscraper/apify/connect.ts | 11 +++- src/lib/api/webscraper.test.ts | 61 +++++++++++++++++++ src/lib/api/webscraper.ts | 38 +++++++++++- 4 files changed, 126 insertions(+), 4 deletions(-) diff --git a/src/commands/webscraper/apify/connect.test.ts b/src/commands/webscraper/apify/connect.test.ts index e70deb51..fcd956fa 100644 --- a/src/commands/webscraper/apify/connect.test.ts +++ b/src/commands/webscraper/apify/connect.test.ts @@ -177,6 +177,26 @@ describe('apify connect', () => { expect(r.exitCode).toBe(1); }); + + it('rejects an empty --token locally instead of falling through to OAuth', async () => { + const r = await runConnect(['--token', '']); + + expect(r.exitCode).toBe(1); + expect(apifyConfigMock.storeApifyToken).not.toHaveBeenCalled(); + expect(apiMock.startApifyCliFlow).not.toHaveBeenCalled(); + expect(apiMock.pollApifyConnection).not.toHaveBeenCalled(); + expect(apiMock.fetchApifyConnection).not.toHaveBeenCalled(); + expect(bridgeMock.runApifyAuthBridge).not.toHaveBeenCalled(); + }); + + it('rejects a whitespace-only --token locally instead of falling through to OAuth', async () => { + const r = await runConnect(['--token', ' ']); + + expect(r.exitCode).toBe(1); + expect(apifyConfigMock.storeApifyToken).not.toHaveBeenCalled(); + expect(apiMock.startApifyCliFlow).not.toHaveBeenCalled(); + expect(bridgeMock.runApifyAuthBridge).not.toHaveBeenCalled(); + }); }); describe('OAuth path (--token absent) is unchanged', () => { diff --git a/src/commands/webscraper/apify/connect.ts b/src/commands/webscraper/apify/connect.ts index 794f32c6..be933535 100644 --- a/src/commands/webscraper/apify/connect.ts +++ b/src/commands/webscraper/apify/connect.ts @@ -95,7 +95,16 @@ async function runConnect(opts: RunConnectOpts): Promise { // token below nor any of the OAuth / auth-bridge flow further down. Short- // circuits before any OAuth work; everything below is unreachable and thus // unchanged when --token is absent. - if (opts.token) { + // + // Branch on the option being *supplied*, not on its truthiness: `--token ""` + // must not silently fall through to the OAuth flow below (which would print + // a confusing "not logged in" error in CI when an env var expands to empty). + // Reject it locally instead — the backend's zod message for this case + // ("String must contain at least 1 character(s)") is far less clear. + if (opts.token !== undefined) { + if (opts.token.trim().length === 0) { + throw new CLIError('--token requires a non-empty Apify API token.'); + } trackGroupCommand('apify', 'connect', proj); const status = await storeApifyToken(opts.token); if (!opts.json) { diff --git a/src/lib/api/webscraper.test.ts b/src/lib/api/webscraper.test.ts index 4ae9d191..7f92a290 100644 --- a/src/lib/api/webscraper.test.ts +++ b/src/lib/api/webscraper.test.ts @@ -210,4 +210,65 @@ describe('storeApifyToken', () => { /no token status/i, ); }); + + it('throws rather than resolving when the backend reports configured: false', async () => { + vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response(JSON.stringify({ token: { configured: false, maskedKey: null } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + const err = await storeApifyToken('apify_api_tok1234567890').catch((e) => e as CLIError); + expect(err).toBeInstanceOf(CLIError); + expect((err as CLIError).message).toMatch(/did not report the token as stored/i); + expect((err as CLIError).code).toBe('APIFY_TOKEN_NOT_STORED'); + }); + + it('raises APIFY_CONFIG_MALFORMED instead of a raw parser error on a non-JSON 2xx body', async () => { + vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response('not json', { + status: 200, + headers: { 'Content-Type': 'text/plain' }, + }), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + const err = await storeApifyToken('apify_api_tok1234567890').catch((e) => e as CLIError); + expect(err).toBeInstanceOf(CLIError); + expect((err as CLIError).code).toBe('APIFY_CONFIG_MALFORMED'); + }); + + it('raises APIFY_CONFIG_MALFORMED on an empty 2xx body', async () => { + vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response('', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + const err = await storeApifyToken('apify_api_tok1234567890').catch((e) => e as CLIError); + expect(err).toBeInstanceOf(CLIError); + expect((err as CLIError).code).toBe('APIFY_CONFIG_MALFORMED'); + }); + + it('raises APIFY_CONFIG_MALFORMED when the 2xx body parses to null', async () => { + vi.spyOn(oss, 'ossFetch').mockResolvedValue( + new Response('null', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + const { storeApifyToken } = await import('./webscraper.js'); + + const err = await storeApifyToken('apify_api_tok1234567890').catch((e) => e as CLIError); + expect(err).toBeInstanceOf(CLIError); + expect((err as CLIError).code).toBe('APIFY_CONFIG_MALFORMED'); + }); }); diff --git a/src/lib/api/webscraper.ts b/src/lib/api/webscraper.ts index 0fee6de2..1d34373d 100644 --- a/src/lib/api/webscraper.ts +++ b/src/lib/api/webscraper.ts @@ -150,6 +150,14 @@ export interface ApifyTokenStatus { * is bad — or that this is a cloud project, where the connection is made by OAuth * instead. Both arrive as a CLIError from ossFetch carrying the backend's message, * so they are left to propagate unchanged. + * + * A 2xx response is not itself proof the token was stored: an empty/non-JSON + * body is treated the same as a missing `token` object (APIFY_CONFIG_MALFORMED + * — the endpoint's response shape can't be trusted), while a well-formed body + * with `configured: false` is a distinct condition — the write may have + * succeeded but the read-back didn't find the secret — surfaced under its own + * APIFY_TOKEN_NOT_STORED code so callers can tell "can't parse the response" + * apart from "parsed fine, but nothing was actually stored". */ export async function storeApifyToken(apiToken: string): Promise { const res = await ossFetch('/api/webscraper/apify/config', { @@ -157,15 +165,39 @@ export async function storeApifyToken(apiToken: string): Promise