mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-03 02:21:44 +00:00
* feat(storage): add configurable index storage and content retention tiers Rebase #3060 onto current origin/main. Keep GITNEXUS_STORAGE_PATH, GITNEXUS_STORAGE_ROOT, and GITNEXUS_CONTENT_RETENTION, and fold in main's FTS skip, embed-session, and help-text updates. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Keep legacy registry rows on the local storage fallback, resolve symlinks before the destructive-path guard, and align hook lookup with CLI branch slugs, branch-slot metadata, and longest-path match. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Only list swept upload directories after a successful removal so callers cannot treat a permission or transient rm failure as gone. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Document that getStoragePath may consult registered storage while this module still does not mutate the global registry. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(storage): close review findings for external indexes and retention Re-inspect ownership under the analyze lock, fail-closed when the registry file is missing, and keep skip-git hook discovery plus retention fields on HTTP/MCP list surfaces. /api/file stays 410 unless contentRetention is full. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * Address PR review feedback (#3060) Treat lock-only index dirs as empty, honor HTTP --force storage policy, and prefer registered plus branch-aware slots in hooks and augment. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Keep hook fallbacks inside the current worktree, compare foreign-local slots canonically, and make storage fixtures survive ownership validation. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix macOS hook test expecting realpath'd registry paths. resolveHookRepo returns the written registry path, not a filesystem realpath, so the assertion must match that. * Address gitnexus-check warnings on hook install docs and slot tests. The Cursor troubleshooting list omitted registry-query.cjs, and the writable-slot test only checked that isDirectory exists instead of that the path is a directory. * Align the HTTP catalog source-scan with skippable resolveRepo validation. resolveRepo lists fresh repos with validate: options.validateStorage !== false so DELETE can skip prune; the test still required a literal validate: true. * Harden storage path sinks so CodeQL path-injection and ReDoS alerts clear. Contain every filesystem probe inside the resolved storage slot with the inline path.relative idiom, reject filesystem-root slots, and trim slot basenames in linear time. * Settle bridge stamps before writing so CI size/mtime matches stay stable. LadybugDB can still flush into bridge.lbug after close+rename; persist whole-millisecond mtimes and wait for consecutive stats to agree so a freshly written pair matches. * Type the settled bridge stat as fs.Stats so tsc does not see bigint. Awaited<ReturnType<typeof fsp.stat>> collapsed the bigint overload and broke prepare/typecheck on CI. * Keep the bridge mtime stamp exact so same-size swaps still fail the pair check. Co-authored-by: Cursor <cursoragent@cursor.com> * Wrap the bridge stamp predicate so prettier --check stays green. Co-authored-by: Cursor <cursoragent@cursor.com> * Require a quiet interval before stamping a settled bridge file. Co-authored-by: Cursor <cursoragent@cursor.com> * Reuse shared storage and settle helpers instead of local copies. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
142 lines
5.7 KiB
TypeScript
142 lines
5.7 KiB
TypeScript
/**
|
|
* Unit tests for the /api/file handler — handleFileRequest.
|
|
*
|
|
* Calls the handler directly with mock req/res rather than mounting it on
|
|
* an Express app and binding a port. This is an intentional design choice:
|
|
* - Mounting the handler via app.get(...) inside a test triggers CodeQL's
|
|
* js/missing-rate-limiting query, which is correct for production
|
|
* route handlers but a false positive on tests of the handler logic.
|
|
* - Direct invocation also runs faster (no port allocation, no listen)
|
|
* and exercises the same code path used in production via createServer.
|
|
*
|
|
* Covers the gaps the PR #1322 review identified:
|
|
* - `?path=a&path=b` (array form) returns 400, not 500 — proves the catch
|
|
* block correctly routes BadRequestError via statusFromError.
|
|
* - `?path=../../../etc/passwd` returns 403 — traversal rejection.
|
|
* - `?path=%2e%2e%2fsecret` (encoded traversal) returns 403 — Express
|
|
* decodes the query string before the handler sees it.
|
|
* - Valid relative path returns 200 with file content.
|
|
* - Missing path returns 400.
|
|
* - Common-prefix sibling escape returns 403 (the path.relative idiom
|
|
* catches what startsWith(root + sep) would have missed).
|
|
*/
|
|
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
|
import path from 'node:path';
|
|
import fs from 'node:fs/promises';
|
|
import os from 'node:os';
|
|
import { handleFileRequest, type SourceAvailability } from '../../src/server/api.js';
|
|
|
|
let tmpRoot: string;
|
|
|
|
beforeAll(async () => {
|
|
tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-api-file-test-'));
|
|
await fs.writeFile(path.join(tmpRoot, 'hello.txt'), 'hello world\n', 'utf-8');
|
|
await fs.mkdir(path.join(tmpRoot, 'sub'), { recursive: true });
|
|
await fs.writeFile(path.join(tmpRoot, 'sub', 'nested.txt'), 'nested\n', 'utf-8');
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await fs.rm(tmpRoot, { recursive: true, force: true });
|
|
});
|
|
|
|
// Minimal express-shaped mock that captures status() / json() calls in a
|
|
// shape compatible with the handler's expected interface. Returns the
|
|
// final status (default 200 for naked res.json) and JSON body.
|
|
const invoke = async (
|
|
query: Record<string, unknown>,
|
|
availability?: SourceAvailability,
|
|
): Promise<{ status: number; body: any }> => {
|
|
let capturedStatus = 200;
|
|
let capturedBody: any = undefined;
|
|
const res = {
|
|
status(code: number) {
|
|
capturedStatus = code;
|
|
return this;
|
|
},
|
|
json(body: any) {
|
|
capturedBody = body;
|
|
},
|
|
};
|
|
await handleFileRequest({ query }, res, tmpRoot, availability);
|
|
return { status: capturedStatus, body: capturedBody };
|
|
};
|
|
|
|
describe('handleFileRequest — security wiring', () => {
|
|
it('returns 200 with content for a valid relative path', async () => {
|
|
const { status, body } = await invoke({ path: 'hello.txt' });
|
|
expect(status).toBe(200);
|
|
expect(body.content).toBe('hello world\n');
|
|
});
|
|
|
|
it('returns 200 for a nested valid path', async () => {
|
|
const { status, body } = await invoke({ path: 'sub/nested.txt' });
|
|
expect(status).toBe(200);
|
|
expect(body.content).toBe('nested\n');
|
|
});
|
|
|
|
it('returns 410 when the selected retention profile cannot provide full source', async () => {
|
|
const { status, body } = await invoke(
|
|
{ path: 'hello.txt' },
|
|
{ available: false, reason: 'content-retention' },
|
|
);
|
|
expect(status).toBe(410);
|
|
expect(body).toMatchObject({ code: 'source-unavailable', reason: 'content-retention' });
|
|
});
|
|
|
|
it('returns 400 when path is missing', async () => {
|
|
const { status, body } = await invoke({});
|
|
expect(status).toBe(400);
|
|
expect(body.error).toBe('Missing path');
|
|
});
|
|
|
|
it('returns 400 when path is an empty string', async () => {
|
|
const { status, body } = await invoke({ path: '' });
|
|
expect(status).toBe(400);
|
|
expect(body.error).toBe('Missing path');
|
|
});
|
|
|
|
// Reproducer for the PR #1322 review's HIGH finding #1.
|
|
// Before the catch-block fix this returned 500.
|
|
it('returns 400 when path is an array (?path=a&path=b)', async () => {
|
|
const { status, body } = await invoke({ path: ['a', 'b'] });
|
|
expect(status).toBe(400);
|
|
expect(body.error).toContain('path');
|
|
expect(body.error).toContain('array');
|
|
});
|
|
|
|
it('returns 403 for parent-directory traversal', async () => {
|
|
const { status, body } = await invoke({ path: '../../../etc/passwd' });
|
|
expect(status).toBe(403);
|
|
expect(body.error).toBe('Path traversal denied');
|
|
});
|
|
|
|
it('returns 403 for already-decoded traversal segments', async () => {
|
|
// Express decodes the query string before the handler sees it, so the
|
|
// analogue of a percent-encoded `%2e%2e%2f` arrives at the handler as
|
|
// '../'. Confirm the barrier still rejects.
|
|
const { status, body } = await invoke({ path: '../etc/passwd' });
|
|
expect(status).toBe(403);
|
|
expect(body.error).toBe('Path traversal denied');
|
|
});
|
|
|
|
it('returns 403 for an absolute path that escapes the root', async () => {
|
|
const { status, body } = await invoke({ path: '/etc/passwd' });
|
|
expect(status).toBe(403);
|
|
expect(body.error).toBe('Path traversal denied');
|
|
});
|
|
|
|
it('returns 404 for a path that resolves inside root but does not exist', async () => {
|
|
const { status, body } = await invoke({ path: 'does-not-exist.txt' });
|
|
expect(status).toBe(404);
|
|
expect(body.error).toBe('File not found');
|
|
});
|
|
|
|
it('rejects a common-prefix sibling directory escape (path.relative idiom)', async () => {
|
|
// The classic pitfall of `startsWith(root + sep)` is that '/tmp/repo' does
|
|
// not catch '/tmp/repo-evil/x'. The path.relative idiom does.
|
|
const sibling = path.basename(tmpRoot) + '-evil/secret';
|
|
const { status, body } = await invoke({ path: `../${sibling}` });
|
|
expect(status).toBe(403);
|
|
expect(body.error).toBe('Path traversal denied');
|
|
});
|
|
});
|