admin/gbrain
1
0
Fork 0
mirror of https://github.com/garrytan/gbrain.git synced 2026-07-27 22:15:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "fix: honor explicit list_pages limit for local callers, warn on remote clamp, thread offset (#2591)"

Browse Source 
This reverts commit 70ffe4a2a2.
This commit is contained in:
Garry Tan
2026-07-23 11:01:29 -07:00
parent f02919c041
commit 5bee08c3c4
2 changed files with 2 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/operations.ts
+2 -32
View File
@@ -1388,11 +1388,7 @@ 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; 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.',
},
limit: { type: 'number', description: 'Max results (default 50)' },
// v0.29 — surface filter that already exists on PageFilters.
updated_after: {
type: 'string',
@@ -1419,36 +1415,10 @@ 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,
offset,
limit: clampSearchLimit(p.limit as number | undefined, 50, 100),
includeDeleted: (p.include_deleted as boolean) === true,
updated_after: typeof p.updated_after === 'string' ? p.updated_after : undefined,
sort,
test/list-clamp-local-trust.test.ts
-132
View File
@@ -1,132 +0,0 @@
/**
* 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<OperationContext> = {}): {
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));
});
});