diff --git a/scripts/run-mock-tests.sh b/scripts/run-mock-tests.sh index dba789635..5790e119a 100755 --- a/scripts/run-mock-tests.sh +++ b/scripts/run-mock-tests.sh @@ -6,13 +6,9 @@ EXCLUDE_OPTS=( ! -path 'test/clients/*' ! -name 'mock-gitlab-server.ts' ! -name 'server-launcher.ts' ! -name 'oauth-tests.ts' ! -name 'schema-tests.ts' ! -name 'test-json-schema.ts' - ! -name 'dynamic-api-url-test.ts' ! -name 'dynamic-routing-tests.ts' - ! -name 'callback-proxy-tests.ts' ! -name 'test-all-transport-server.ts' - ! -name 'multi-server-test.ts' ! -name 'client-pool-test.ts' - ! -name 'no-proxy-test.ts' ! -name 'no-proxy-integration-test.ts' - ! -name 'remote-auth-tests.ts' ! -name 'test-mr-diffs-filter.ts' - ! -name 'test-mr-file-diffs.ts' ! -name 'test-token-optimizations.ts' - ! -name 'test-merge-request-approvals.ts' ! -name 'config-allowed-groups.test.ts' + ! -name 'test-merge-request-approvals.ts' + # Need a live GitLab token (GITLAB_TOKEN_TEST / TEST_PROJECT_ID) + ! -name 'remote-auth-tests.ts' ! -name 'test-all-transport-server.ts' ) run_mock_tests() { @@ -40,6 +36,8 @@ run_mock_tests 4 \ -o -path 'test/oauth-scopes.test.ts' \ -o -path 'test/oauth-device-flow-tests.ts' \ -o -path 'test/personal-access-token-file.test.ts' \ + -o -path 'test/config-allowed-groups.test.ts' \ + -o -path 'test/no-proxy-test.ts' \ -o -path 'test/cli/*.test.ts' \) # Co-located unit tests for scripts/ helper modules — no MCP/mock server processes @@ -47,7 +45,7 @@ node --import tsx/esm --test --experimental-test-isolation=none scripts/tool-cov # Server-spawning suites — sequential to avoid port races and node:test IPC flakes run_mock_tests 1 \ - \( -name '*.test.ts' -o -name 'test-*.ts' -o -name '*-tests.ts' -o -name 'remote-auth-simple-test.ts' \) \ + \( -name '*.test.ts' -o -name 'test-*.ts' -o -name '*-tests.ts' -o -name '*-test.ts' \) \ ! -path 'test/utils/*' \ ! -path 'test/path-segment-encoding.test.ts' \ ! -path 'test/nullish-tool-arguments-schema.test.ts' \ @@ -63,6 +61,8 @@ run_mock_tests 1 \ ! -path 'test/oauth-scopes.test.ts' \ ! -path 'test/oauth-device-flow-tests.ts' \ ! -path 'test/personal-access-token-file.test.ts' \ + ! -path 'test/config-allowed-groups.test.ts' \ + ! -path 'test/no-proxy-test.ts' \ ! -path 'test/cli/*' tsx test/oauth-tests.ts diff --git a/test/README.md b/test/README.md index 4bf639c3c..aa8f30c3f 100644 --- a/test/README.md +++ b/test/README.md @@ -38,8 +38,9 @@ Pass 1 matches, under `test/`, and still subject to the exclude list below: - `test/nullish-tool-arguments-schema.test.ts` - `test/stateless/codec.test.ts`, `client-id.test.ts`, `config-ttl.test.ts`, `session-id.test.ts`, `callback-proxy.test.ts`, `consumed-proxy-code-cache.test.ts` - `test/oauth-startup.test.ts`, `test/deprecated-env.test.ts`, `test/oauth-scopes.test.ts`, `test/oauth-device-flow-tests.ts` +- `test/personal-access-token-file.test.ts`, `test/config-allowed-groups.test.ts`, `test/no-proxy-test.ts` -Pass 3 matches everything else under `test/` named `*.test.ts`, `test-*.ts`, `*-tests.ts`, or `remote-auth-simple-test.ts`, except pass 1 paths and the exclude list. +Pass 3 matches everything else under `test/` named `*.test.ts`, `test-*.ts`, `*-tests.ts`, or `*-test.ts`, except pass 1 paths and the exclude list. `test:oauth` overlaps `test:mock` (`oauth-tests.ts` is step 4; `oauth-device-flow-tests.ts` is pass 1). `test:schema`'s `*-schema.test.ts` files are also pass 1. `schema-tests.ts` and `test-json-schema.ts` are only `test:schema`. `test:stateless`, `test:mcp-oauth`, `test:remote-auth`, and `test:list-merge-requests` re-run files that `test:mock` already runs. @@ -79,23 +80,12 @@ No live GitLab token is required for `test:mock`. It also skips these names. `oauth-tests.ts` is still step 4 of `test:mock` (`tsx test/oauth-tests.ts`). The others are not run by `test:mock`: -| File | How to run it | -| -------------------------------------------------- | ---------------------------------------------------------------------------- | -| `test/schema-tests.ts`, `test/test-json-schema.ts` | `npm run test:schema` | -| `test/test-merge-request-approvals.ts` | `npm run test:approvals` | -| `test/callback-proxy-tests.ts` | No npm script. Run the file yourself. | -| `test/client-pool-test.ts` | No npm script. | -| `test/config-allowed-groups.test.ts` | No npm script. | -| `test/dynamic-api-url-test.ts` | No npm script. | -| `test/dynamic-routing-tests.ts` | No npm script. | -| `test/multi-server-test.ts` | No npm script. | -| `test/no-proxy-test.ts` | No npm script. | -| `test/no-proxy-integration-test.ts` | No npm script. | -| `test/remote-auth-tests.ts` | No npm script. `test:remote-auth` runs `remote-auth-simple-test.ts` instead. | -| `test/test-all-transport-server.ts` | No npm script. | -| `test/test-mr-diffs-filter.ts` | No npm script. | -| `test/test-mr-file-diffs.ts` | No npm script. | -| `test/test-token-optimizations.ts` | No npm script. | +| File | How to run it | +| -------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | +| `test/schema-tests.ts`, `test/test-json-schema.ts` | `npm run test:schema` | +| `test/test-merge-request-approvals.ts` | `npm run test:approvals` | +| `test/remote-auth-tests.ts` | Needs a live GitLab token and `TEST_PROJECT_ID`. No npm script. `test:remote-auth` runs `remote-auth-simple-test.ts` instead. | +| `test/test-all-transport-server.ts` | Needs a live GitLab token and `TEST_PROJECT_ID`. No npm script. | `test/validate-api.js` and `test/cli/load-config-snapshot.ts` do not match the `find` globs. @@ -135,6 +125,6 @@ Use a throwaway project. ## Adding a test -`scripts/run-mock-tests.sh` only runs files its `find` selects. Name the file `*.test.ts`, `test-*.ts`, or `*-tests.ts` under `test/`, and do not use a name in the exclude list above. A new pure unit test stays on the parallel pass if it is `test/utils/*.test.ts`, `test/cli/*.test.ts`, or `*-schema.test.ts`. Any other new match runs in the sequential pass. A name that matches none of those patterns is not run until the script's `find` is updated. +`scripts/run-mock-tests.sh` only runs files its `find` selects. Name the file `*.test.ts`, `test-*.ts`, `*-tests.ts`, or `*-test.ts` under `test/`, and do not use a name in the exclude list above. A new pure unit test stays on the parallel pass if it is `test/utils/*.test.ts`, `test/cli/*.test.ts`, or `*-schema.test.ts`. Any other new match runs in the sequential pass. A name that matches none of those patterns is not run until the script's `find` is updated. Tool counts in [tool invocation coverage](../docs/reference/tool-coverage.md) only when the test sends that tool through MCP. `scripts/tool-coverage/coverage.ts` (used by `scripts/generate-tool-coverage.ts`, which `npm run docs:tools` runs) counts a string-literal name in a `callTool*` call (`callTool`, `callToolJson`, `callToolAsync`, `client.callTool`), including `{ name }` only when that object is the first argument. It also counts `params.name` on an object whose `method` is `"tools/call"`, and `{ name }` on the argument right after a `"tools/call"` string. A bare name, a toolset filter, or `callTool(env, toolName)` does not count. `test/tool-description-quality.test.ts` is skipped by that scanner. `test:live` does not count. Regenerate with `npm run docs:tools` (`make tools-docs` runs that) and commit the generated docs. diff --git a/test/client-pool-test.ts b/test/client-pool-test.ts index 5632515e4..1c44e344c 100644 --- a/test/client-pool-test.ts +++ b/test/client-pool-test.ts @@ -32,6 +32,7 @@ describe('Client Pool Limits', () => { let mockGitLab: MockGitLabServer; let servers: ServerInstance[] = []; let mockGitLabUrl: string; + let mockGitLab2: MockGitLabServer; before(async () => { // Start mock GitLab server @@ -43,6 +44,13 @@ describe('Client Pool Limits', () => { await mockGitLab.start(); mockGitLabUrl = mockGitLab.getUrl(); + // Second mock GitLab server, used as the third distinct pool entry + mockGitLab2 = new MockGitLabServer({ + port: await findMockServerPort(), + validTokens: [MOCK_TOKEN] + }); + await mockGitLab2.start(); + // Start MCP server with pool limit const mcpPort = await findAvailablePort(MCP_SERVER_PORT_BASE); const server = await launchServer({ @@ -54,6 +62,7 @@ describe('Client Pool Limits', () => { REMOTE_AUTHORIZATION: 'true', ENABLE_DYNAMIC_API_URL: 'true', // Enable dynamic URLs to test pooling GITLAB_API_URL: `${mockGitLabUrl}/api/v4`, + GITLAB_ALLOWED_HOSTS: `${mockGitLabUrl.replace('127.0.0.1', 'localhost')}/api/v4,${mockGitLab2.getUrl()}/api/v4`, GITLAB_POOL_MAX_SIZE: String(POOL_MAX_SIZE), } }); @@ -70,6 +79,9 @@ describe('Client Pool Limits', () => { if (mockGitLab) { await mockGitLab.stop(); } + if (mockGitLab2) { + await mockGitLab2.stop(); + } }); test('should enforce pool size limit', async () => { @@ -108,11 +120,11 @@ describe('Client Pool Limits', () => { } await client2.disconnect(); - // URL 3: gitlab-3.example.com (Fake, distinct string) + // URL 3: second mock server (distinct origin) // This should trigger the pool limit check const client3 = new CustomHeaderClient({ 'authorization': `Bearer ${MOCK_TOKEN}`, - 'x-gitlab-api-url': 'https://gitlab-3.example.com/api/v4' + 'x-gitlab-api-url': `${mockGitLab2.getUrl()}/api/v4` }); await client3.connect(mcpUrl); diff --git a/test/dynamic-api-url-test.ts b/test/dynamic-api-url-test.ts index a946993b8..1cd4208f3 100644 --- a/test/dynamic-api-url-test.ts +++ b/test/dynamic-api-url-test.ts @@ -65,6 +65,7 @@ describe('Dynamic API URL - Multiple GitLab Instances', () => { STREAMABLE_HTTP: 'true', REMOTE_AUTHORIZATION: 'true', ENABLE_DYNAMIC_API_URL: 'true', + GITLAB_ALLOWED_HOSTS: `${mockGitLabUrl1}/api/v4,${mockGitLabUrl2}/api/v4`, GITLAB_READ_ONLY_MODE: 'true', } }); @@ -276,6 +277,7 @@ describe('Dynamic API URL - Connection Pool', () => { STREAMABLE_HTTP: 'true', REMOTE_AUTHORIZATION: 'true', ENABLE_DYNAMIC_API_URL: 'true', + GITLAB_ALLOWED_HOSTS: `${mockGitLabUrl1}/api/v4,${mockGitLabUrl2}/api/v4`, GITLAB_CLIENT_POOL_SIZE: '5', GITLAB_CLIENT_IDLE_TIMEOUT: '30000', } diff --git a/test/dynamic-routing-tests.ts b/test/dynamic-routing-tests.ts index d95bd8c48..d90ddc608 100644 --- a/test/dynamic-routing-tests.ts +++ b/test/dynamic-routing-tests.ts @@ -13,6 +13,7 @@ import { Request, Response } from "express"; const MOCK_TOKEN_DEFAULT = 'glpat-mock-token-default'; const MOCK_TOKEN_HEADER = 'glpat-mock-token-header'; +const MCP_AUTH_TOKEN = 'mcp-static-auth-token'; describe('Dynamic Routing and Authentication Scenarios', () => { const originalToken = process.env.GITLAB_TOKEN_TEST; @@ -55,6 +56,7 @@ describe('Dynamic Routing and Authentication Scenarios', () => { REMOTE_AUTHORIZATION: "false", ENABLE_DYNAMIC_API_URL: "false", GITLAB_TOKEN_TEST: MOCK_TOKEN_DEFAULT, + STREAMABLE_HTTP_AUTH_TOKEN: MCP_AUTH_TOKEN, }, }); mcpUrl = `http://${HOST}:${mcpPort}/mcp`; @@ -79,7 +81,8 @@ describe('Dynamic Routing and Authentication Scenarios', () => { const client = new CustomHeaderClient({ headers: { - 'authorization': `Bearer ${MOCK_TOKEN_HEADER}`, // This should be ignored + 'authorization': `Bearer ${MCP_AUTH_TOKEN}`, // MCP auth only, not sent to GitLab + 'private-token': MOCK_TOKEN_HEADER, // This should be ignored 'X-GitLab-API-URL': 'http://localhost:9999/api/v4', // This should be ignored } }); @@ -149,6 +152,7 @@ describe('Dynamic Routing and Authentication Scenarios', () => { REMOTE_AUTHORIZATION: "false", ENABLE_DYNAMIC_API_URL: "true", GITLAB_TOKEN_TEST: "mock-token", // Required to bypass launcher check + STREAMABLE_HTTP_AUTH_TOKEN: MCP_AUTH_TOKEN, }, }), (err: Error) => { @@ -186,6 +190,8 @@ describe('Dynamic Routing and Authentication Scenarios', () => { GITLAB_PERSONAL_ACCESS_TOKEN: MOCK_TOKEN_DEFAULT, REMOTE_AUTHORIZATION: "true", ENABLE_DYNAMIC_API_URL: "true", + GITLAB_ALLOWED_HOSTS: `${headerMockServer.getUrl()}/api/v4`, + GITLAB_TOOLSETS: "all", }, }); mcpUrl = `http://${HOST}:${mcpPort}/mcp`; diff --git a/test/multi-server-test.ts b/test/multi-server-test.ts index 0158b88c9..c103043c7 100644 --- a/test/multi-server-test.ts +++ b/test/multi-server-test.ts @@ -177,32 +177,19 @@ describe("Dynamic Client Mode (ENABLE_DYNAMIC_API_URL=true)", () => { await client.disconnect(); }); - test("should fail tool call when header contains a non-whitelisted URL", async () => { + test("should reject a header URL that is not configured", async () => { const client = new CustomHeaderClient({ headers: { 'authorization': `Bearer ${MOCK_TOKEN}`, 'X-GitLab-API-URL': 'http://localhost:9999/api/v4', } }); - // Connect succeeds (server accepts any valid URL format), - // but tool calls fail because the target server doesn't exist - await client.connect(mcpUrl); - try { - const result = await client.callTool('get_project', { project_id: "1" }); - // If callTool returns (doesn't throw), check for error in response - const content = result.content[0]; - assert.ok('text' in content, 'Should have text content'); - assert.ok( - content.text.includes('Error') || content.text.includes('error') || content.text.includes('ECONNREFUSED') || result.isError, - 'Should return an error for unreachable server' - ); - } catch (err: any) { - // MCP client may throw for server-side errors (-32603) - assert.ok( - err.message.includes('ECONNREFUSED') || err.message.includes('failed') || err.message.includes('error'), - `Should contain connection error info: ${err.message}` - ); - } + // Header URLs must match GITLAB_API_URL or GITLAB_ALLOWED_HOSTS, + // so the server refuses the request before creating a session + await assert.rejects( + client.connect(mcpUrl), + /Missing Private-Token, JOB-TOKEN, or Authorization header/ + ); await client.disconnect(); }); }); \ No newline at end of file diff --git a/test/no-proxy-integration-test.ts b/test/no-proxy-integration-test.ts index a69772a63..08c876f28 100644 --- a/test/no-proxy-integration-test.ts +++ b/test/no-proxy-integration-test.ts @@ -13,7 +13,7 @@ import { TransportMode, HOST } from './utils/server-launcher.js'; -import { MockGitLabServer, findMockServerPort } from './utils/mock-gitlab-server.js'; +import { MockGitLabServer, findMockServerPort, resetMockServerState } from './utils/mock-gitlab-server.js'; import { CustomHeaderClient } from './clients/custom-header-client.js'; // Test constants @@ -123,17 +123,18 @@ describe('NO_PROXY Integration Tests', () => { const client = new CustomHeaderClient({ 'authorization': `Bearer ${MOCK_TOKEN}`, }); - await client.connect(mcpUrl); - - // This should fail because it tries to use the nonexistent proxy - try { + + // This should fail because it tries to use the nonexistent proxy. + // The token is checked against GitLab when the session starts, so the + // proxy error can surface on connect or on the first tool call. + // A request that skipped the proxy would reach the mock server. + resetMockServerState(mockGitLab); + await assert.rejects(async () => { + await client.connect(mcpUrl); await client.callTool('list_projects', { per_page: 1 }); - assert.fail('Request should have failed due to proxy connection error'); - } catch (error: any) { - console.log(' ✓ Request failed as expected (proxy error)'); - // Expected to fail with connection/proxy error - assert.ok(error, 'Should throw an error when proxy fails'); - } + }, /Invalid GitLab authentication header|fetch failed|ECONNREFUSED|ENOTFOUND/); + assert.strictEqual(mockGitLab.getRequestCount(), 0, 'Request should have gone through the proxy'); + console.log(' ✓ Request failed as expected (proxy error)'); await client.disconnect(); }); diff --git a/test/utils/mock-gitlab-server.ts b/test/utils/mock-gitlab-server.ts index 8ab90d993..b5955304d 100644 --- a/test/utils/mock-gitlab-server.ts +++ b/test/utils/mock-gitlab-server.ts @@ -674,6 +674,10 @@ export class MockGitLabServer { getPort(): number { return this.config.port; } + + getRequestCount(): number { + return this.requestCount; + } } /**