diff --git a/src/core/operations.ts b/src/core/operations.ts index 68f5a56e1..7e20012c2 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -1388,7 +1388,11 @@ const list_pages: Operation = { params: { type: { type: 'string', description: 'Filter by page type' }, tag: { type: 'string', description: 'Filter by tag' }, - limit: { type: 'number', description: 'Max results (default 50)' }, + limit: { type: 'number', description: 'Max results (default 50; remote callers are capped at 100)' }, + offset: { + type: 'number', + description: 'Skip first N rows (pagination). Engine-supported since PageFilters gained offset; previously accepted at the CLI and silently dropped.', + }, // v0.29 — surface filter that already exists on PageFilters. updated_after: { type: 'string', @@ -1415,10 +1419,36 @@ const list_pages: Operation = { // were ignored at this op handler and the engine returned every source's // pages indiscriminately. const scope = sourceScopeOpts(ctx); + // The 100-row cap exists to protect remote MCP/OAuth transports from + // unbounded result dumps. Local CLI callers (ctx.remote === false — the + // same trust boundary that already bypasses scope enforcement, see the + // Operation.scope doc above) own the machine, and a full enumeration is a + // legitimate local operation, so an explicit limit above 100 is honored. + // Anything that is not strictly `false` stays remote/untrusted (defense + // in depth, matching the ctx.remote contract). + const requestedLimit = p.limit as number | undefined; + const isLocal = ctx.remote === false; + const limit = isLocal + ? clampSearchLimit(requestedLimit, 50, Number.MAX_SAFE_INTEGER) + : clampSearchLimit(requestedLimit, 50, 100); + if (!isLocal && requestedLimit !== undefined && Number.isFinite(requestedLimit) && requestedLimit > limit) { + // Loud clamp, parity with the three search paths ("search limit clamped + // from N to 100"). logger.warn goes to stderr — `list` stdout is + // tab-separated and consumed by scripts, so it must stay clean. + ctx.logger.warn(`[gbrain] Warning: list limit clamped from ${requestedLimit} to ${limit}; use offset to paginate`); + } + // Thread offset through — PageFilters has supported it all along; the op + // layer just never passed it, so `--offset` was accepted and ignored. + const requestedOffset = p.offset as number | undefined; + const offset = + requestedOffset !== undefined && Number.isFinite(requestedOffset) && requestedOffset > 0 + ? Math.floor(requestedOffset) + : undefined; const pages = await ctx.engine.listPages({ type: p.type as any, tag: p.tag as string, - limit: clampSearchLimit(p.limit as number | undefined, 50, 100), + limit, + offset, includeDeleted: (p.include_deleted as boolean) === true, updated_after: typeof p.updated_after === 'string' ? p.updated_after : undefined, sort, diff --git a/test/list-clamp-local-trust.test.ts b/test/list-clamp-local-trust.test.ts new file mode 100644 index 000000000..04178023b --- /dev/null +++ b/test/list-clamp-local-trust.test.ts @@ -0,0 +1,132 @@ +/** + * list_pages clamp local-trust + offset threading — op-level coverage. + * + * Pins (upstream draft "gbrain list silently clamps --limit to 100"): + * - Local callers (ctx.remote === false) get an explicit limit above 100 + * honored — full enumeration is a legitimate local operation. + * - Remote callers keep the 100-row DoS cap, and the clamp is now LOUD: + * exactly one logger.warn (stderr, never stdout) naming both numbers. + * - Defaults unchanged: no limit → 50 rows for both local and remote. + * - `offset` threads through to the engine (PageFilters supported it all + * along; the op layer dropped it, so `--offset` was silently ignored). + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { operationsByName } from '../src/core/operations.ts'; +import type { OperationContext } from '../src/core/operations.ts'; + +const SEED_COUNT = 120; // must exceed the remote cap (100) and the default (50) + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + for (let i = 0; i < SEED_COUNT; i++) { + // Zero-padded slugs → sort:'slug' gives a deterministic order for the + // offset assertions regardless of insert timestamps. + await engine.putPage(`listclamp/page-${String(i).padStart(3, '0')}`, { + type: 'note', + title: `Page ${i}`, + compiled_truth: 'body', + }); + } +}); + +afterAll(async () => { + if (engine) await engine.disconnect(); +}); + +function mkCtx(overrides: Partial = {}): { + ctx: OperationContext; + warnings: string[]; +} { + const warnings: string[] = []; + const ctx = { + engine, + config: {} as any, + logger: { + info: () => {}, + warn: (msg: string) => warnings.push(msg), + error: () => {}, + } as any, + dryRun: false, + remote: false, + ...overrides, + } as OperationContext; + return { ctx, warnings }; +} + +const op = () => operationsByName['list_pages']; + +describe('list_pages — local callers escape the 100-row clamp', () => { + test('remote=false with limit 100000 returns every page', async () => { + const { ctx, warnings } = mkCtx({ remote: false }); + const rows = (await op().handler(ctx, { limit: 100000 })) as any[]; + expect(rows.length).toBe(SEED_COUNT); + expect(warnings.length).toBe(0); + }); + + test('remote=false default (no limit) is still 50 — default unchanged', async () => { + const { ctx } = mkCtx({ remote: false }); + const rows = (await op().handler(ctx, {})) as any[]; + expect(rows.length).toBe(50); + }); +}); + +describe('list_pages — remote callers keep the cap, loudly', () => { + test('remote=true with limit 100000 returns 100 and warns once with both numbers', async () => { + const { ctx, warnings } = mkCtx({ remote: true }); + const rows = (await op().handler(ctx, { limit: 100000 })) as any[]; + expect(rows.length).toBe(100); + expect(warnings.length).toBe(1); + expect(warnings[0]).toContain('list limit clamped from 100000 to 100'); + }); + + test('remote=true with limit <= 100 does not warn', async () => { + const { ctx, warnings } = mkCtx({ remote: true }); + const rows = (await op().handler(ctx, { limit: 60 })) as any[]; + expect(rows.length).toBe(60); + expect(warnings.length).toBe(0); + }); + + test('anything not strictly remote===false is treated as remote (defense in depth)', async () => { + // ctx.remote contract: consumers treat non-false as untrusted even if the + // type is bypassed via cast. + const { ctx, warnings } = mkCtx({ remote: undefined as any }); + const rows = (await op().handler(ctx, { limit: 100000 })) as any[]; + expect(rows.length).toBe(100); + expect(warnings.length).toBe(1); + }); +}); + +describe('list_pages — offset threads through (regression: was silently ignored)', () => { + test('offset shifts the window under sort=slug', async () => { + const { ctx } = mkCtx({ remote: false }); + const all = (await op().handler(ctx, { limit: 100000, sort: 'slug' })) as any[]; + const paged = (await op().handler(ctx, { limit: 10, offset: 5, sort: 'slug' })) as any[]; + expect(paged.length).toBe(10); + expect(paged.map(r => r.slug)).toEqual(all.slice(5, 15).map(r => r.slug)); + }); + + test('offset near the end truncates the page', async () => { + const { ctx } = mkCtx({ remote: false }); + const rows = (await op().handler(ctx, { + limit: 100000, + offset: SEED_COUNT - 7, + sort: 'slug', + })) as any[]; + expect(rows.length).toBe(7); + }); + + test('garbage offset (negative / NaN) is ignored, not fatal', async () => { + const { ctx } = mkCtx({ remote: false }); + const neg = (await op().handler(ctx, { limit: 10, offset: -5, sort: 'slug' })) as any[]; + const nan = (await op().handler(ctx, { limit: 10, offset: NaN, sort: 'slug' })) as any[]; + const base = (await op().handler(ctx, { limit: 10, sort: 'slug' })) as any[]; + expect(neg.map(r => r.slug)).toEqual(base.map(r => r.slug)); + expect(nan.map(r => r.slug)).toEqual(base.map(r => r.slug)); + }); +});