From c27b2e4b0fbf1eb6737e267d99720ae54ac98330 Mon Sep 17 00:00:00 2001 From: symmetric-matthew <167941066+symmetric-matthew@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:24:40 -0700 Subject: [PATCH] fix(put): refuse to overwrite a non-empty page with empty content (#2708) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An empty --content (most commonly a non-interactive caller that meant file input — put has no --file flag — so the missing --content fell back to reading empty stdin) silently blanked existing pages. put_page now rejects an empty/whitespace-only body over an existing non-empty page with invalid_params, pointing at `gbrain capture --file PATH --slug SLUG` for file input; allow_empty: true (CLI: --allow-empty) opts into an intentional blank. The guard read is scoped to the exact (source_id, slug) row the write targets; new-slug creates and soft-deleted-page overwrites stay allowed. Co-authored-by: Matthew Thompson Co-authored-by: Claude Fable 5 --- src/core/operations.ts | 25 +++++ test/put-page-empty-guard.test.ts | 150 ++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 test/put-page-empty-guard.test.ts diff --git a/src/core/operations.ts b/src/core/operations.ts index 68f5a56e1..81008e074 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -773,6 +773,7 @@ const put_page: Operation = { params: { slug: { type: 'string', required: true, description: 'Page slug' }, content: { type: 'string', required: true, description: 'Full markdown content with YAML frontmatter' }, + allow_empty: { type: 'boolean', required: false, description: 'Allow overwriting an existing non-empty page with empty/whitespace-only content (default: false). Without it, put_page rejects the empty overwrite — the empty-stdin failure class.' }, // v0.39.3.0 provenance write-through (WARN-8 + A1 + CV6). Optional fields // for trusted local callers (capture CLI, autopilot, dream cycle). Remote // MCP callers (ctx.remote !== false) have their values OVERRIDDEN with @@ -822,6 +823,30 @@ const put_page: Operation = { enforceSubagentSlugFence(ctx, slug, 'put_page'); if (ctx.dryRun) return { dry_run: true, action: 'put_page', slug: p.slug }; + + // Empty-overwrite guard: empty/whitespace-only content over an existing + // non-empty page is almost always an input-plumbing failure (e.g. a + // caller that meant file input — put has no --file flag — so the missing + // --content fell back to reading an empty non-interactive stdin), not an + // intentional write. Refuse loudly unless the caller opts in with + // allow_empty. The read is scoped to the exact (source_id, slug) row the + // write below targets (engine.putPage defaults to 'default' when + // sourceId is unset). New-slug creates and soft-deleted-page overwrites + // stay allowed — nothing recoverable is lost there. + if ((p.content as string).trim() === '' && p.allow_empty !== true) { + const existing = await ctx.engine.getPage(slug, { sourceId: ctx.sourceId ?? 'default' }); + const existingBody = existing + ? `${existing.compiled_truth ?? ''}\n${existing.timeline ?? ''}`.trim() + : ''; + if (existingBody !== '') { + throw new OperationError( + 'invalid_params', + `Refusing to overwrite existing non-empty page '${slug}' with empty content.`, + 'For file input use `gbrain capture --file PATH --slug SLUG` (put has no --file flag). To intentionally blank the page, pass allow_empty: true (CLI: --allow-empty).', + ); + } + } + // Skip embedding when the AI gateway has no embedding provider configured. // Checks all auth env vars for the resolved provider, not just OPENAI_API_KEY, // so Gemini / Ollama / Voyage brains don't silently drop embeddings (Codex C2). diff --git a/test/put-page-empty-guard.test.ts b/test/put-page-empty-guard.test.ts new file mode 100644 index 000000000..42ba48d68 --- /dev/null +++ b/test/put-page-empty-guard.test.ts @@ -0,0 +1,150 @@ +/** + * put_page empty-overwrite guard tests. + * + * Class guard: empty/whitespace-only content over an existing non-empty page + * is an input-plumbing failure (e.g. a caller that meant file input — put has + * no --file flag — so the missing --content fell back to reading an empty + * non-interactive stdin), not an intentional write. put_page must refuse it + * loudly unless allow_empty is passed. New-slug creates, same-source scoping, + * and normal non-empty overwrites are unaffected. + */ + +import { afterAll, beforeAll, beforeEach, describe, expect, test } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; +import { operations, OperationError } from '../src/core/operations.ts'; +import type { OperationContext } from '../src/core/operations.ts'; +import { resetGateway } from '../src/core/ai/gateway.ts'; + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); + resetGateway(); +}); + +beforeEach(async () => { + await resetPgliteState(engine); + // No embedding provider in tests: isAvailable('embedding') must be false so + // put_page sets noEmbed and never makes a network call. + resetGateway(); +}); + +function makeCtx(overrides: Partial = {}): OperationContext { + return { + engine, + config: { engine: 'pglite' as const }, + logger: { info: () => {}, warn: () => {}, error: () => {} }, + dryRun: false, + remote: false, + sourceId: 'default', + ...overrides, + }; +} + +const putPage = operations.find((o) => o.name === 'put_page')!; + +const PAGE_CONTENT = '---\ntitle: Guarded\n---\n\n# Real body\n\nContent that must survive.'; + +async function seedPage(slug: string): Promise { + const result = (await putPage.handler(makeCtx(), { slug, content: PAGE_CONTENT })) as { + status: string; + }; + expect(result.status).toBe('created_or_updated'); +} + +async function expectRejected(params: Record, ctx = makeCtx()): Promise { + try { + await putPage.handler(ctx, params); + } catch (e) { + expect(e).toBeInstanceOf(OperationError); + return e as OperationError; + } + throw new Error('expected put_page to reject the empty overwrite, but it succeeded'); +} + +describe('put_page empty-overwrite guard — rejection', () => { + test('empty content over an existing non-empty page is rejected; page survives', async () => { + await seedPage('inbox/guarded'); + const err = await expectRejected({ slug: 'inbox/guarded', content: '' }); + expect(err.code).toBe('invalid_params'); + expect(err.message).toContain('inbox/guarded'); + expect(err.suggestion).toContain('capture --file PATH --slug SLUG'); + expect(err.suggestion).toContain('allow_empty'); + + const page = await engine.getPage('inbox/guarded', { sourceId: 'default' }); + expect(page).not.toBeNull(); + expect(page!.compiled_truth).toContain('Content that must survive.'); + }); + + test('whitespace-only content is rejected the same way', async () => { + await seedPage('inbox/guarded-ws'); + const err = await expectRejected({ slug: 'inbox/guarded-ws', content: ' \n\t \n' }); + expect(err.code).toBe('invalid_params'); + + const page = await engine.getPage('inbox/guarded-ws', { sourceId: 'default' }); + expect(page!.compiled_truth).toContain('Content that must survive.'); + }); + + test('remote (MCP) callers are guarded too', async () => { + await seedPage('inbox/guarded-remote'); + const err = await expectRejected( + { slug: 'inbox/guarded-remote', content: '' }, + makeCtx({ remote: true }), + ); + expect(err.code).toBe('invalid_params'); + }); +}); + +describe('put_page empty-overwrite guard — allowed paths', () => { + test('allow_empty: true blanks the page intentionally', async () => { + await seedPage('inbox/blank-me'); + const result = (await putPage.handler(makeCtx(), { + slug: 'inbox/blank-me', + content: '', + allow_empty: true, + })) as { status: string }; + expect(result.status).toBe('created_or_updated'); + const page = await engine.getPage('inbox/blank-me', { sourceId: 'default' }); + expect((page!.compiled_truth ?? '').trim()).toBe(''); + }); + + test('empty content on a new slug still creates the page', async () => { + const result = (await putPage.handler(makeCtx(), { + slug: 'inbox/new-empty', + content: '', + })) as { status: string }; + expect(result.status).toBe('created_or_updated'); + expect(await engine.getPage('inbox/new-empty', { sourceId: 'default' })).not.toBeNull(); + }); + + test('non-empty overwrite of an existing page is unaffected', async () => { + await seedPage('inbox/normal-update'); + const result = (await putPage.handler(makeCtx(), { + slug: 'inbox/normal-update', + content: '---\ntitle: Guarded\n---\n\n# Real body\n\nUpdated content.', + })) as { status: string }; + expect(result.status).toBe('created_or_updated'); + const page = await engine.getPage('inbox/normal-update', { sourceId: 'default' }); + expect(page!.compiled_truth).toContain('Updated content.'); + }); + + test('guard is scoped to the write-target source — a non-empty page in another source does not block', async () => { + await seedPage('shared/per-source'); // lands in 'default' + await engine.executeRaw("INSERT INTO sources (id, name) VALUES ('team-x', 'team-x')"); + const result = (await putPage.handler(makeCtx({ sourceId: 'team-x' }), { + slug: 'shared/per-source', + content: '', + })) as { status: string }; + expect(result.status).toBe('created_or_updated'); + // The default-source page is untouched. + const page = await engine.getPage('shared/per-source', { sourceId: 'default' }); + expect(page!.compiled_truth).toContain('Content that must survive.'); + }); +});