diff --git a/gitnexus-shared/src/index.ts b/gitnexus-shared/src/index.ts index 0c1f41766..faf136fe7 100644 --- a/gitnexus-shared/src/index.ts +++ b/gitnexus-shared/src/index.ts @@ -151,6 +151,7 @@ export { buildUqDispatchPayload, isValidOwnerRepo, parseOwnerRepoFromRemote, + stripGitSuffix, } from './integrations/understand-quickly.js'; export type { UqDispatchPayload } from './integrations/understand-quickly.js'; diff --git a/gitnexus-shared/src/integrations/understand-quickly.ts b/gitnexus-shared/src/integrations/understand-quickly.ts index de7fa9736..7fd36b527 100644 --- a/gitnexus-shared/src/integrations/understand-quickly.ts +++ b/gitnexus-shared/src/integrations/understand-quickly.ts @@ -64,9 +64,37 @@ export function buildUqDispatchPayload(id: string): UqDispatchPayload { * `owner/repo` validation. Conservative on purpose: GitHub's actual * naming rules are looser, but we want to catch local paths * (`/Users/...`), bare slugs (`my-repo`), and accidental whitespace. + * + * Tightened (LOW 8) to match GitHub's published slug rules: + * owner: starts with alnum, then alnum/hyphen only — no underscore, + * no dot. Length cap 39. + * repo: any of alnum/dot/hyphen/underscore. Length cap 100. */ export function isValidOwnerRepo(id: string): boolean { - return /^[A-Za-z0-9][A-Za-z0-9._-]*\/[A-Za-z0-9._-]+$/.test(id); + return /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})\/[A-Za-z0-9._-]{1,100}$/.test(id); +} + +/** + * Strip a single trailing `.git` (case-insensitive) and any trailing + * slashes from a URL-ish string. Bounded linear: each character is + * visited at most twice, no backtracking. + * + * Replaces `s.replace(/\.git\/*$/i, '').replace(/\/+$/, '')` which + * CodeQL's polynomial-regex check (codeql/js/polynomial-redos) flags as + * a worst-case O(n²) on adversarial input like "////.../x". + */ +export function stripGitSuffix(input: string): string { + let end = input.length; + // Trim trailing '/'. + while (end > 0 && input.charCodeAt(end - 1) === 0x2f) end--; + // Drop one trailing '.git' (case-insensitive). + if (end >= 4) { + const tail = input.slice(end - 4, end).toLowerCase(); + if (tail === '.git') end -= 4; + } + // Trim trailing '/' that may have sat between '.git' and the rest. + while (end > 0 && input.charCodeAt(end - 1) === 0x2f) end--; + return input.slice(0, end); } /** @@ -86,16 +114,32 @@ export function parseOwnerRepoFromRemote(url: string | null | undefined): string if (!trimmed) return null; // Strip a trailing `.git` (case-insensitive) and any trailing slashes // so https://h/o/r and https://h/o/r.git collapse to the same id. - const stripped = trimmed.replace(/\.git\/*$/i, '').replace(/\/+$/, ''); + // Bounded-linear helper avoids the polynomial-regex CodeQL alert. + const stripped = stripGitSuffix(trimmed); - // SCP-form SSH (`git@host:owner/repo`). - const ssh = stripped.match(/^[^@]+@[^:]+:([^/]+)\/([^/]+)$/); - if (ssh) return `${ssh[1]}/${ssh[2]}`; + // SCP-form SSH (`git@host:owner/repo`). Capture host so we can reject + // non-GitHub remotes — a GitLab origin like + // `https://gitlab.example.com/group/sub/project.git` would otherwise + // silently dispatch the wrong id (LOW 9). + const ssh = stripped.match(/^[^@]+@([^:]+):([^/]+)\/([^/]+)$/); + if (ssh) { + const host = ssh[1].toLowerCase(); + if (host !== 'github.com' && host !== 'www.github.com') return null; + return `${ssh[2]}/${ssh[3]}`; + } // URL forms (https://, ssh://, git://, file://) — last two path segments. - const url2 = stripped.match(/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\/[^/]+\/(.+)$/); + const url2 = stripped.match(/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\/([^/]+)\/(.+)$/); if (url2) { - const segments = url2[1].split('/').filter(Boolean); + // Strip optional `userinfo@` (e.g. `ssh://git@github.com/...`). + const authority = url2[1]; + const atIdx = authority.lastIndexOf('@'); + const hostAndPort = atIdx >= 0 ? authority.slice(atIdx + 1) : authority; + // Strip `:port` suffix if present. + const colonIdx = hostAndPort.indexOf(':'); + const host = (colonIdx >= 0 ? hostAndPort.slice(0, colonIdx) : hostAndPort).toLowerCase(); + if (host !== 'github.com' && host !== 'www.github.com') return null; + const segments = url2[2].split('/').filter(Boolean); if (segments.length >= 2) { const [owner, repo] = segments.slice(-2); return `${owner}/${repo}`; diff --git a/gitnexus/src/cli/publish.ts b/gitnexus/src/cli/publish.ts index 3281619a6..d025ec45b 100644 --- a/gitnexus/src/cli/publish.ts +++ b/gitnexus/src/cli/publish.ts @@ -44,10 +44,34 @@ const REGISTER_HINT = 'Register your repo once with: npx @understand-quickly/cli add\n' + 'Or use the wizard: https://looptech-ai.github.io/understand-quickly/add.html'; +/** + * Hard cap on the dispatch fetch to keep CI publish steps from stalling + * for the OS TCP timeout (~2 min) when api.github.com is unreachable. + * Matches the pattern used in `src/core/embeddings/http-client.ts`. + */ +const DISPATCH_TIMEOUT_MS = 15_000; + export const publishCommand = async ( inputPath?: string, options: PublishOptions = {}, ): Promise => { + // ── 0. Token gate FIRST — guarantees true no-op without the token. ── + // The README, CLI --help, and PR body all promise "exit 0 without + // UNDERSTAND_QUICKLY_TOKEN". Doing the index/repo-root checks before + // the token gate would make those promises false for users who haven't + // run `gitnexus analyze` yet but want to verify the command is wired. + const token = process.env[UNDERSTAND_QUICKLY_TOKEN_ENV]; + if (!token) { + cliInfo( + `[understand-quickly] ${UNDERSTAND_QUICKLY_TOKEN_ENV} is not set — skipping dispatch.\n` + + `Set it to a fine-grained PAT with "Repository dispatches: write" on ` + + `looptech-ai/understand-quickly to enable instant resync.\n` + + `(Without the token, the registry's nightly sync still picks up your entry.)`, + { skipped: 'no-token' }, + ); + return; + } + // ── 1. Resolve the repo root (same precedence as `analyze`) ────────── let repoPath: string; if (inputPath) { @@ -93,22 +117,8 @@ export const publishCommand = async ( return; } - // ── 4. Token gate: no token → informational no-op (exit 0) ─────────── - const token = process.env[UNDERSTAND_QUICKLY_TOKEN_ENV]; - if (!token) { - cliInfo( - `[understand-quickly] ${UNDERSTAND_QUICKLY_TOKEN_ENV} is not set — skipping dispatch.\n` + - `Set it to a fine-grained PAT with "Repository dispatches: write" on ` + - `looptech-ai/understand-quickly to enable instant resync.\n` + - `(Without the token, the registry's nightly sync still picks up ${id}.)`, - { id, skipped: 'no-token' }, - ); - return; - } - - // ── 5. Fire the dispatch ───────────────────────────────────────────── + // ── 4. Fire the dispatch ───────────────────────────────────────────── const payload = buildUqDispatchPayload(id); - const commit = getCurrentCommit(repoPath); let response: Response; try { response = await fetch(UNDERSTAND_QUICKLY_DISPATCH_URL, { @@ -118,43 +128,94 @@ export const publishCommand = async ( Authorization: `Bearer ${token}`, 'X-GitHub-Api-Version': '2022-11-28', 'Content-Type': 'application/json', + 'User-Agent': 'gitnexus-cli', }, body: JSON.stringify(payload), + signal: AbortSignal.timeout(DISPATCH_TIMEOUT_MS), }); } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - cliError(`[understand-quickly] dispatch network error: ${msg}`, { id }); + if (err instanceof Error && err.name === 'AbortError') { + cliError( + `[understand-quickly] dispatch timed out after ${DISPATCH_TIMEOUT_MS}ms. ` + + `Check network access to api.github.com and retry.`, + { id }, + ); + } else { + const msg = err instanceof Error ? err.message : String(err); + cliError(`[understand-quickly] dispatch network error: ${msg}`, { id }); + } process.exitCode = 1; return; } - // GitHub returns 204 on success, 404 when the token can't reach the - // registry repo, 401 when the token is invalid. Surface these - // distinctly so users debug without checking the docs. + // GitHub returns 204 on success. Distinct branches for 401/403/404/422 + // so users debug without checking the docs. if (response.status === 204) { + await response.body?.cancel().catch(() => {}); + // `getCurrentCommit` is only meaningful in the success path — moving + // it inside this branch removes a wasted child-process spawn on every + // error response (LOW 7). + const commit = getCurrentCommit(repoPath); cliInfo( `[understand-quickly] dispatched sync-entry for ${id}` + (commit ? ` @ ${commit.slice(0, 7)}` : '') + '.\n' + - `View the workflow run: ` + + `Note: a 204 only confirms GitHub accepted the dispatch. Whether the ` + + `registry workflow finds an entry for "${id}" is logged at ` + `https://github.com/looptech-ai/understand-quickly/actions/workflows/sync.yml`, { id, commit, status: response.status }, ); return; } - if (response.status === 404) { + if (response.status === 401) { cliError( - `[understand-quickly] dispatch returned 404 — the token cannot reach ` + - `looptech-ai/understand-quickly. Verify the PAT has Repository access ` + - `to that repo and the "Repository dispatches: write" permission.`, + `[understand-quickly] dispatch returned 401 — the ${UNDERSTAND_QUICKLY_TOKEN_ENV} value is invalid or expired.\n` + + `Regenerate a fine-grained PAT at https://github.com/settings/personal-access-tokens ` + + `with Repository access scoped to looptech-ai/understand-quickly and the ` + + `"Repository dispatches: write" permission, then retry.`, { id, status: response.status }, ); process.exitCode = 1; return; } - // 401, 403, 422, 5xx → bubble up the body so the user can act. + if (response.status === 403) { + cliError( + `[understand-quickly] dispatch returned 403 — the token authenticated but ` + + `lacks the "Repository dispatches: write" permission on ` + + `looptech-ai/understand-quickly. Edit the PAT scopes and retry.`, + { id, status: response.status }, + ); + process.exitCode = 1; + return; + } + + if (response.status === 404) { + cliError( + `[understand-quickly] dispatch returned 404 — the token cannot reach ` + + `looptech-ai/understand-quickly. Verify the PAT has Repository access to ` + + `that exact repo (not just your own org).`, + { id, status: response.status }, + ); + process.exitCode = 1; + return; + } + + if (response.status === 422) { + // Malformed event_type / client_payload — a code bug in this CLI, + // not a user mistake. Surface so we get bug reports. + const body422 = await response.text().catch(() => ''); + cliError( + `[understand-quickly] dispatch returned 422 (this is a CLI bug; please report).\n` + + `Body: ${body422 || '(empty)'}`, + { id, status: response.status }, + ); + process.exitCode = 1; + return; + } + + // 5xx and anything else → bubble the body so the user has something to act on. const body = await response.text().catch(() => ''); cliError( `[understand-quickly] dispatch failed with HTTP ${response.status}: ${body || '(empty body)'}`, diff --git a/gitnexus/test/unit/publish.test.ts b/gitnexus/test/unit/publish.test.ts index 9d8dc4a07..9dcef919b 100644 --- a/gitnexus/test/unit/publish.test.ts +++ b/gitnexus/test/unit/publish.test.ts @@ -1,11 +1,13 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest'; import fs from 'fs/promises'; import os from 'os'; import path from 'path'; +import { performance } from 'node:perf_hooks'; import { buildUqDispatchPayload, isValidOwnerRepo, parseOwnerRepoFromRemote, + stripGitSuffix, UNDERSTAND_QUICKLY_TOKEN_ENV, } from 'gitnexus-shared'; @@ -14,28 +16,77 @@ describe('understand-quickly helpers (gitnexus-shared)', () => { it.each([ ['looptech-ai/understand-quickly', true], ['abhigyanpatwari/GitNexus', true], - ['Some_Org/Some.Repo-2', true], + // LOW 8: GitHub user/org slugs are alnum/hyphen only — no underscore. + ['Some_Org/Some.Repo-2', false], ['', false], ['just-a-name', false], ['/Users/me/code/repo', false], ['org/with spaces', false], ['org//double', false], + // LOW 8 additions: + ['some_org/repo', false], // underscore in owner — invalid + ['-org/repo', false], // leading hyphen — invalid + ['org-/repo', true], // trailing hyphen — GitHub actually allows this + ['org/repo_with_underscore', true], + ['org/.dotfile', true], // repos may start with dot ])('returns %s for %j', (id, expected) => { expect(isValidOwnerRepo(id as string)).toBe(expected); }); }); + describe('stripGitSuffix (BLOCKER 1 — ReDoS-safe)', () => { + it.each([ + ['https://github.com/o/r.git', 'https://github.com/o/r'], + ['https://github.com/o/r.git/', 'https://github.com/o/r'], + ['https://github.com/o/r/', 'https://github.com/o/r'], + ['https://github.com/o/r', 'https://github.com/o/r'], + ['https://github.com/o/r.GIT', 'https://github.com/o/r'], + ['https://github.com/o/r//', 'https://github.com/o/r'], + ['', ''], + ['/', ''], + ])('strips %j -> %j', (input, expected) => { + expect(stripGitSuffix(input)).toBe(expected); + }); + + test('linear time on adversarial trailing slashes (regression for ReDoS)', () => { + const adversarial = 'https://github.com/o/r' + '/'.repeat(10_000); + const start = performance.now(); + const result = stripGitSuffix(adversarial); + const elapsed = performance.now() - start; + expect(result).toBe('https://github.com/o/r'); + expect(elapsed).toBeLessThan(50); // generous; should be sub-millisecond + }); + + test('parseOwnerRepoFromRemote terminates quickly on adversarial input', () => { + const adversarial = 'https://github.com/o/r.git' + '/'.repeat(10_000); + const start = performance.now(); + const result = parseOwnerRepoFromRemote(adversarial); + const elapsed = performance.now() - start; + expect(result).toBe('o/r'); + expect(elapsed).toBeLessThan(50); + }); + }); + describe('parseOwnerRepoFromRemote', () => { it.each([ ['git@github.com:looptech-ai/understand-quickly.git', 'looptech-ai/understand-quickly'], ['https://github.com/looptech-ai/understand-quickly', 'looptech-ai/understand-quickly'], ['https://github.com/looptech-ai/understand-quickly.git', 'looptech-ai/understand-quickly'], ['ssh://git@github.com/abhigyanpatwari/GitNexus.git', 'abhigyanpatwari/GitNexus'], - ['https://gitlab.example.com/group/sub/project.git', 'sub/project'], ])('parses %s -> %s', (url, expected) => { expect(parseOwnerRepoFromRemote(url)).toBe(expected); }); + // LOW 9: non-GitHub remotes must be rejected — a wrong id is worse + // than no id, since the user can always pass --id explicitly. + it.each([ + ['https://gitlab.example.com/group/sub/project.git'], + ['git@gitlab.example.com:group/sub/project.git'], + ['https://bitbucket.org/team/repo.git'], + ])('returns null for non-GitHub host %j', (input) => { + expect(parseOwnerRepoFromRemote(input)).toBeNull(); + }); + it.each([null, undefined, '', ' ', 'not-a-url', 'https://github.com/'])( 'returns null for %j', (input) => { @@ -102,4 +153,160 @@ describe('publishCommand (no-token no-op)', () => { expect(process.exitCode ?? 0).toBe(0); fetchSpy.mockRestore(); }); + + it('exits 0 with no token even when no index/repo exists (BLOCKER 2)', async () => { + // Per the README, CLI --help, and PR body: without a token, the + // command must be a no-op even if the repo lacks `.gitnexus/`. + const noIndexDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-publish-noidx-')); + try { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockImplementation(() => { + throw new Error('publishCommand should NOT call fetch when the token is missing'); + }); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(noIndexDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(process.exitCode ?? 0).toBe(0); + fetchSpy.mockRestore(); + } finally { + await fs.rm(noIndexDir, { recursive: true, force: true }); + } + }); +}); + +describe('publishCommand response branches (MEDIUM 5)', () => { + let tempDir: string; + let originalToken: string | undefined; + let exitCodeBefore: number | undefined; + let fetchSpy: ReturnType; + + beforeEach(async () => { + vi.resetModules(); + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-publish-resp-')); + await fs.mkdir(path.join(tempDir, '.gitnexus'), { recursive: true }); + await fs.writeFile( + path.join(tempDir, '.gitnexus', 'meta.json'), + JSON.stringify({ repoPath: tempDir, lastCommit: '', indexedAt: '' }), + 'utf-8', + ); + originalToken = process.env[UNDERSTAND_QUICKLY_TOKEN_ENV]; + process.env[UNDERSTAND_QUICKLY_TOKEN_ENV] = 'pat_test'; + exitCodeBefore = process.exitCode; + process.exitCode = 0; + fetchSpy = vi.spyOn(globalThis, 'fetch'); + }); + + afterEach(async () => { + if (originalToken !== undefined) { + process.env[UNDERSTAND_QUICKLY_TOKEN_ENV] = originalToken; + } else { + delete process.env[UNDERSTAND_QUICKLY_TOKEN_ENV]; + } + process.exitCode = exitCodeBefore; + vi.restoreAllMocks(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + function mockResponse(status: number, body = '') { + fetchSpy.mockResolvedValueOnce({ + status, + ok: status >= 200 && status < 300, + text: async () => body, + body: { cancel: async () => {} }, + headers: new Headers(), + } as unknown as Response); + } + + it('204 → exit 0 with success message', async () => { + mockResponse(204); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(process.exitCode ?? 0).toBe(0); + }); + + it('401 → exit 1 with PAT-invalid hint', async () => { + mockResponse(401, '{"message":"Bad credentials"}'); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(process.exitCode).toBe(1); + }); + + it('403 → exit 1 with scope-missing hint', async () => { + mockResponse(403, '{"message":"Resource not accessible"}'); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(process.exitCode).toBe(1); + }); + + it('404 → exit 1 with repo-access hint', async () => { + mockResponse(404, '{"message":"Not Found"}'); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(process.exitCode).toBe(1); + }); + + it('5xx → exit 1 with raw body', async () => { + mockResponse(503, 'gateway timeout'); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(process.exitCode).toBe(1); + }); + + it('network throw → exit 1', async () => { + fetchSpy.mockRejectedValueOnce(new Error('ECONNRESET')); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(process.exitCode).toBe(1); + }); + + it('AbortError (HIGH 4 — fetch timeout) → exit 1 with timed-out message', async () => { + const abort = new Error('aborted'); + abort.name = 'AbortError'; + fetchSpy.mockRejectedValueOnce(abort); + const errSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + expect(process.exitCode).toBe(1); + const written = errSpy.mock.calls.map((c) => String(c[0])).join(''); + expect(written).toMatch(/timed out/i); + errSpy.mockRestore(); + }); + + it('token never appears in any logged output', async () => { + process.env[UNDERSTAND_QUICKLY_TOKEN_ENV] = 'pat_secret_value'; + mockResponse(401, ''); + const errSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + const { publishCommand } = await import('../../src/cli/publish.js'); + await publishCommand(tempDir, { + id: 'looptech-ai/understand-quickly', + skipGit: true, + }); + const written = errSpy.mock.calls.map((c) => String(c[0])).join(''); + expect(written).not.toContain('pat_secret_value'); + errSpy.mockRestore(); + }); });