From 3f6746065bf3dfd63be8db9b4f96b2d3d32d388f Mon Sep 17 00:00:00 2001 From: ayush-that Date: Fri, 23 Oct 2026 01:32:30 +0530 Subject: [PATCH 1/2] fix(ci): reject loopback targets in ci run --- DOCUMENTATION.md | 2 +- src/commands/ci-run.spec.ts | 139 ++++++++++++++++++++++++++++++++++++ src/commands/test.ts | 26 ++++++- 3 files changed, 164 insertions(+), 3 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index da69607..5c2063f 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -1091,7 +1091,7 @@ testsprite ci run --project proj_xxxxxxxx --filter checkout --fail-on-skipped testsprite ci run --project proj_xxxxxxxx --summary-file testsprite-summary.json ``` -`--project` defaults to `TESTSPRITE_PROJECT_ID`. All project tests are selected unless `--filter ` narrows them (case-insensitive substring); `--all` is optional. `--env ` names the environment whose sign-in the run uses; the name must be one of the project's environments (`testsprite project env list `), and an unknown name is refused **before** anything dispatches or charges, even with `--allow-empty`. That check fails closed: an environment list that cannot be read is that error (a bad key exits **3**, a backend that is down **10**), never a run that goes on to dispatch unchecked; only a backend answering that the project has no environment surface (a V2 project, or an older backend without the route) leaves the name to the batch's own validation. `--target-url ` is the address to test — a pull request's preview deployment, or a local app through an automatic tunnel (see [`test run`](#testsprite-test-run-test-id)); `--skip-preflight` skips its reachability probe when the preview is still warming up. Backend tests use the URL in their own code, not `--target-url`. +`--project` defaults to `TESTSPRITE_PROJECT_ID`. All project tests are selected unless `--filter ` narrows them (case-insensitive substring); `--all` is optional. `--env ` names the environment whose sign-in the run uses; the name must be one of the project's environments (`testsprite project env list `), and an unknown name is refused **before** anything dispatches or charges, even with `--allow-empty`. That check fails closed: an environment list that cannot be read is that error (a bad key exits **3**, a backend that is down **10**), never a run that goes on to dispatch unchecked; only a backend answering that the project has no environment surface (a V2 project, or an older backend without the route) leaves the name to the batch's own validation. `--target-url ` is a publicly reachable address to test, typically a pull request's preview deployment; `--skip-preflight` skips its reachability probe when the preview is still warming up. `ci run` refuses loopback targets, including a selected environment's stored loopback URL when no `--target-url` overrides it, with exit **5** and zero-valued step outputs. For a local app, use `testsprite test run --all --env --wait` (see [`test run`](#testsprite-test-run-test-id)). Backend tests use the URL in their own code, not `--target-url`. **One execution.** Every selected test is seeded into one execution and the server paces dispatch under the project's execution settings, exactly as for a GitHub App run — a CI run is one row in the Portal, never a first batch plus a rate-deferred retry. A backend that cannot honor this is refused with exit **7** (`admission-not-honored`) after the outputs are published; the batch is never silently cut to the per-key budget. diff --git a/src/commands/ci-run.spec.ts b/src/commands/ci-run.spec.ts index a51d176..2b61862 100644 --- a/src/commands/ci-run.spec.ts +++ b/src/commands/ci-run.spec.ts @@ -1,6 +1,8 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { Command } from 'commander'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createCiCommand, runCiRun, type CiRunOptions } from './ci.js'; @@ -268,6 +270,143 @@ function harness( const SHA = 'a'.repeat(40); const PREVIEW = 'https://gym-web-abc123-team.vercel.app'; +/* eslint-disable security/detect-non-literal-fs-filename -- All paths belong to this suite's isolated temporary directory. */ +describe('ci run loopback targets through the CLI entry', () => { + function run(args: string[]) { + const dir = mkdtempSync(join(tmpdir(), 'ci-run-entry-')); + dirs.push(dir); + const output = join(dir, 'outputs'); + writeFileSync(output, 'previous=value\n'); + const preload = ` +globalThis.fetch = async (input, init = {}) => { + const path = new URL(String(input)).pathname.replace('/api/cli/v1', ''); + const method = init.method ?? 'GET'; + const body = typeof init.body === 'string' ? JSON.parse(init.body) : undefined; + process.stderr.write('FETCH ' + JSON.stringify({ path, method, body }) + '\\n'); + if (path === '/telemetry') return new Response(null, { status: 204 }); + let response; + if (method === 'GET' && path === '/projects/project-1/env') { + response = { environments: [{ name: 'staging', url: 'http://localhost:3000', isDefault: true }] }; + } else if (method === 'POST' && path === '/tests/batch/run') { + response = { + accepted: [], conflicts: [], deferred: [], skippedFrontend: ['fe'], skippedIntegration: [], + admission: body.admission, targetUrl: body.targetUrl + }; + } else if (method === 'GET' && path === '/tests') { + response = { items: [{ id: 'fe', name: 'Checkout' }], nextToken: null }; + } else if (method === 'GET' && path === '/tests/fe') { + response = { id: 'fe', type: 'frontend', name: 'Checkout' }; + } else { + throw new Error('Unexpected request: ' + method + ' ' + path); + } + return new Response(JSON.stringify(response), { headers: { 'content-type': 'application/json' } }); +};`; + const result = spawnSync( + process.execPath, + [ + '--import', + `data:text/javascript,${encodeURIComponent(preload)}`, + fileURLToPath(new URL('../../dist/index.js', import.meta.url)), + 'ci', + 'run', + '--project', + 'project-1', + ...args, + ], + { + encoding: 'utf8', + timeout: 10_000, + env: { + ...process.env, + HOME: dir, + USERPROFILE: dir, + TESTSPRITE_API_KEY: 'sk-user-test', + TESTSPRITE_API_URL: 'https://api.example.com', + TESTSPRITE_NO_TELEMETRY: '0', + DO_NOT_TRACK: '0', + TESTSPRITE_NO_UPDATE_NOTIFIER: '1', + GITHUB_ACTIONS: 'true', + GITHUB_EVENT_NAME: undefined, + GITHUB_EVENT_PATH: undefined, + GITHUB_OUTPUT: output, + GITHUB_STEP_SUMMARY: undefined, + }, + }, + ); + expect(result.error).toBeUndefined(); + return { + ...result, + outputs: readFileSync(output, 'utf8'), + calls: result.stderr + .split('\n') + .filter(line => line.startsWith('FETCH ')) + .map( + line => + JSON.parse(line.slice(6)) as { + path: string; + method: string; + body?: Record; + }, + ), + }; + } + + it.each<[string, string[]]>([ + ['localhost', ['--target-url', 'http://localhost:3000']], + ['IPv4 loopback', ['--target-url', 'http://127.0.0.1:3000']], + ['IPv6 loopback', ['--target-url', 'http://[::1]:3000']], + ['named environment', ['--env', 'staging']], + ['default environment', []], + ['explicit dry run', ['--target-url', 'http://localhost:3000', '--dry-run']], + ['environment dry run', ['--env', 'staging', '--dry-run']], + ['explicit concurrency', ['--target-url', 'http://localhost:3000', '--max-concurrency', '5']], + ])('refuses %s before opening a tunnel or dispatching', (_label, args) => { + const result = run(args); + expect(result.status, result.stderr).toBe(5); + expect(result.stderr).toContain('test run --all --env --wait'); + expect(result.stderr).not.toMatch( + /--max-concurrency|--local|opening a tunnel|Would open a tunnel/, + ); + expect(result.outputs).toBe( + 'previous=value\ntotal=0\npassed=0\nfailed=0\nskipped=0\ntimedOut=0\n', + ); + expect( + result.calls.filter(call => call.path !== '/telemetry').every(call => call.method === 'GET'), + ).toBe(true); + if (!args.includes('--dry-run')) { + expect(result.calls.find(call => call.path === '/telemetry')?.body).toMatchObject({ + ciRunOutcome: 'refused_loopback', + }); + } + }); + + it.each([false, true])( + 'a public override of a loopback environment keeps the CI gate, strict=%s', + strict => { + const result = run([ + '--env', + 'staging', + '--target-url', + PREVIEW, + '--skip-preflight', + '--allow-empty', + ...(strict ? ['--fail-on-skipped'] : []), + ]); + expect(result.status, result.stderr).toBe(strict ? 1 : 0); + expect(result.outputs).toContain('total=1\npassed=0\nfailed=0\nskipped=1\ntimedOut=0\n'); + expect( + result.calls.filter(call => call.method === 'POST' && call.path !== '/telemetry'), + ).toMatchObject([ + { + path: '/tests/batch/run', + body: { admission: 'all', targetUrl: PREVIEW }, + }, + ]); + }, + ); +}); +/* eslint-enable security/detect-non-literal-fs-filename */ + function githubEnv( h: ReturnType, event: string, diff --git a/src/commands/test.ts b/src/commands/test.ts index 8bf5ce5..302e242 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -10025,6 +10025,22 @@ async function finishZeroDispatchBatch(params: { ); } +function refuseCiLoopbackTarget(deps: TestDeps): never { + recordTelemetryExtras({ ciRunOutcome: 'refused_loopback' }); + emitCiStepOutputs( + { total: 0, passed: 0, failed: 0, skipped: 0, timedOut: 0, runs: [] }, + { + env: deps.env ?? process.env, + stderr: deps.stderr ?? (line => process.stderr.write(`${line}\n`)), + }, + ); + throw localValidationError( + 'target-url', + 'ci run requires a publicly reachable target URL. For a local app, use ' + + 'testsprite test run --all --env --wait (with --project if needed)', + ); +} + /** * `test run --all --project ` — M4 piece-2. * @@ -10060,6 +10076,10 @@ export async function runTestRunAll( const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); const out = makeOutput(opts.output, deps); + if (opts.ci && opts.targetUrl !== undefined && parseLoopbackTargetUrl(opts.targetUrl)) { + refuseCiLoopbackTarget(deps); + } + if (opts.dryRun && opts.targetUrl === undefined && !opts.noWait) { let target: ResolvedRunTarget | undefined; try { @@ -10068,12 +10088,13 @@ export async function runTestRunAll( projectId, environment, dryRun: true, - stderr: stderrFn, + stderr: opts.ci ? () => undefined : stderrFn, }); } catch (err) { if (err instanceof InterruptError) throw err; } if (target) { + if (opts.ci) refuseCiLoopbackTarget(deps); await runTestRunLocalBatch( { ...opts, @@ -10150,9 +10171,10 @@ export async function runTestRunAll( environment, targetUrl: opts.targetUrl, noWait: opts.noWait, - stderr: stderrFn, + stderr: opts.ci ? () => undefined : stderrFn, }); if (automaticTarget) { + if (opts.ci) refuseCiLoopbackTarget(deps); await runTestRunLocalBatch( { ...opts, From 527ee8fc2f9583686713b284bc05d4d59a5b2c51 Mon Sep 17 00:00:00 2001 From: ayush-that Date: Fri, 23 Oct 2026 01:41:37 +0530 Subject: [PATCH 2/2] fix(ci): write step outputs for malformed loopback targets --- src/commands/ci-run.spec.ts | 9 +++++++++ src/commands/test.ts | 13 ++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/commands/ci-run.spec.ts b/src/commands/ci-run.spec.ts index 2b61862..1db0e9a 100644 --- a/src/commands/ci-run.spec.ts +++ b/src/commands/ci-run.spec.ts @@ -380,6 +380,15 @@ globalThis.fetch = async (input, init = {}) => { } }); + it('keeps the malformed loopback error and writes zero outputs', () => { + const result = run(['--target-url', 'http://localhost:3000/app']); + expect(result.status, result.stderr).toBe(5); + expect(result.stderr).toContain('no path'); + expect(result.outputs).toBe( + 'previous=value\ntotal=0\npassed=0\nfailed=0\nskipped=0\ntimedOut=0\n', + ); + }); + it.each([false, true])( 'a public override of a loopback environment keeps the CI gate, strict=%s', strict => { diff --git a/src/commands/test.ts b/src/commands/test.ts index 302e242..c35e053 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -10025,7 +10025,7 @@ async function finishZeroDispatchBatch(params: { ); } -function refuseCiLoopbackTarget(deps: TestDeps): never { +function refuseCiLoopbackTarget(deps: TestDeps, validationError?: unknown): never { recordTelemetryExtras({ ciRunOutcome: 'refused_loopback' }); emitCiStepOutputs( { total: 0, passed: 0, failed: 0, skipped: 0, timedOut: 0, runs: [] }, @@ -10034,6 +10034,7 @@ function refuseCiLoopbackTarget(deps: TestDeps): never { stderr: deps.stderr ?? (line => process.stderr.write(`${line}\n`)), }, ); + if (validationError !== undefined) throw validationError; throw localValidationError( 'target-url', 'ci run requires a publicly reachable target URL. For a local app, use ' + @@ -10076,8 +10077,14 @@ export async function runTestRunAll( const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); const out = makeOutput(opts.output, deps); - if (opts.ci && opts.targetUrl !== undefined && parseLoopbackTargetUrl(opts.targetUrl)) { - refuseCiLoopbackTarget(deps); + if (opts.ci && opts.targetUrl !== undefined) { + let loopback; + try { + loopback = parseLoopbackTargetUrl(opts.targetUrl); + } catch (err) { + refuseCiLoopbackTarget(deps, err); + } + if (loopback) refuseCiLoopbackTarget(deps); } if (opts.dryRun && opts.targetUrl === undefined && !opts.noWait) {