v0.42.37.0 fix(security,ingest): source-isolation grant enforcement + non-string frontmatter guard + papercuts (#1999)

* fix(security): scope cross-source reads to the caller grant; close get_page exact-path leak

One shared resolveRequestedScope() routes every source-scoped read op
(query, code_callers/callees, search_by_image, code_blast/flow, get_page)
through a single fail-closed trust+grant ladder: a remote caller's __all__
collapses to its granted sources (never the whole brain) and an explicit
out-of-grant source_id is rejected. get_page's exact-match path now honors a
federated grant via getPage(sourceIds[]) in both engines. Legacy bearer tokens
carry their stored permissions.source_id grant (bounded, never widened). Also
retries getConfig on transient connection loss.

Closes #1924, #1371, #1393, #1336, #1603.

* fix(ingest): non-string frontmatter no longer aborts lint/sync; embed/hook/catalog papercuts

Parser coerces a non-string title to a string and falls back to inference for
slug/type (never fabricating a "123" slug), with a lint NON_STRING_FIELD finding
surfacing the malformed frontmatter; a defensive guard in content-sanity stops a
non-string title from crashing the whole lint/sync run brain-wide. Plus: embed
--catch-up no longer arms the overflowed 32-bit budget timer (and surfaces
unembeddable chunks); the frontmatter pre-commit hook ships a correct .md/.mdx
regex; and the skill catalog parses YAML block-scalar descriptions.

Closes #1883, #1658, #1556, #1948, #1946, #1840, #1711.

* v0.42.37.0 fix(security,ingest): source-isolation grant enforcement + non-string frontmatter guard + papercuts

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs: add NON_STRING_FIELD frontmatter validation class to docs for v0.42.37.0

The v0.42.37.0 non-string-frontmatter fix added an eighth validation
class (NON_STRING_FIELD / lint code frontmatter-non-string-field). Update
the two current-state docs that enumerate the validation classes:
- skills/frontmatter-guard/SKILL.md (seven->eight + table row)
- docs/integrations/pre-commit.md (seven->eight + table row)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-08 21:19:25 -07:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 959af1068d
commit 1eb430a2df
22 changed files with 768 additions and 89 deletions
+19
View File
@@ -53,6 +53,25 @@ describe('frontmatter install-hook (B13)', () => {
}
});
test('#1840 — generated hook matches .md/.mdx (single-backslash regex, not over-escaped)', () => {
installHook(tmp, false);
const content = readFileSync(join(tmp, '.githooks', 'pre-commit'), 'utf8');
// The shell must see `grep -E '\.mdx?$'`. Pre-fix it emitted `'\\.mdx?$'`
// (literal backslash), so the hook matched nothing and silently no-opped.
expect(content).toContain("grep -E '\\.mdx?$'");
expect(content).not.toContain("grep -E '\\\\.mdx?$'");
// Prove the emitted pattern actually selects markdown files. Extract the
// exact pattern between the single quotes and run it through ripgrep-free
// JS regex parity (POSIX ERE `\.mdx?$` == JS `/\.mdx?$/`).
const m = content.match(/grep -E '([^']+)'/);
expect(m).not.toBeNull();
const re = new RegExp(m![1]);
expect(re.test('notes/thing.md')).toBe(true);
expect(re.test('notes/thing.mdx')).toBe(true);
expect(re.test('notes/thing.txt')).toBe(false);
});
test('installHook refuses to clobber existing hook without --force', () => {
const hooksDir = join(tmp, '.githooks');
mkdirSync(hooksDir, { recursive: true });
@@ -0,0 +1,83 @@
/**
* Non-string frontmatter title/type/slug (#1883, #1658, #1556, #1948).
*
* A YAML scalar like `title: 123` parses to a NUMBER. Pre-fix the parser cast it
* `as string`, so a non-string flowed downstream typed as string and crashed the
* first `.toLowerCase()` in content-sanity — aborting the whole lint/sync run
* brain-wide (root trigger behind the never-converging-sync reports #1794/#1939).
*
* Fix: coerce title to a string at the parser; for slug/type fall back to
* inference (never fabricate a "123" slug); guard content-sanity defensively;
* and surface the malformed frontmatter via a lint NON_STRING_FIELD finding.
*/
import { describe, test, expect } from 'bun:test';
import { parseMarkdown } from '../src/core/markdown.ts';
import { assessContentSanity } from '../src/core/content-sanity.ts';
const fm = (body: string) => `---\n${body}\n---\nbody text here\n`;
describe('parseMarkdown coerces/guards non-string frontmatter', () => {
test('numeric title is coerced to a string (intent preserved)', () => {
const p = parseMarkdown(fm('title: 123'), 'notes/thing.md');
expect(typeof p.title).toBe('string');
expect(p.title).toBe('123');
});
test('boolean title is coerced to a string', () => {
const p = parseMarkdown(fm('title: false'), 'notes/thing.md');
expect(p.title).toBe('false');
});
test('missing title falls back to inferred title (string)', () => {
const p = parseMarkdown(fm('type: note'), 'notes/My Thing.md');
expect(typeof p.title).toBe('string');
expect(p.title.length).toBeGreaterThan(0);
});
test('non-string slug is coerced to a usable string (date slugs are legitimate)', () => {
// YAML parses `2024-06-01` as a Date; coerceFrontmatterString → "2024-06-01".
const p = parseMarkdown(fm('slug: 2024-06-01'), 'notes/real-slug.md');
expect(typeof p.slug).toBe('string');
expect(p.slug).toBe('2024-06-01');
});
test('numeric type is coerced to a string (never crashes downstream)', () => {
const p = parseMarkdown(fm('type: 5\ntitle: ok'), 'notes/thing.md');
expect(typeof p.type).toBe('string');
});
test('valid string fields pass through unchanged', () => {
const p = parseMarkdown(fm('title: Real Title\ntype: concept\nslug: my-slug'), 'x.md');
expect(p.title).toBe('Real Title');
expect(p.type).toBe('concept');
expect(p.slug).toBe('my-slug');
});
});
describe('lint surfaces non-string frontmatter (NON_STRING_FIELD)', () => {
test('numeric title produces a NON_STRING_FIELD validation error', () => {
const p = parseMarkdown(fm('title: 123'), 'notes/thing.md', { validate: true });
const codes = (p.errors ?? []).map(e => e.code);
expect(codes).toContain('NON_STRING_FIELD');
});
test('all-string frontmatter produces no NON_STRING_FIELD error', () => {
const p = parseMarkdown(fm('title: Fine\ntype: note'), 'notes/thing.md', { validate: true });
const codes = (p.errors ?? []).map(e => e.code);
expect(codes).not.toContain('NON_STRING_FIELD');
});
});
describe('assessContentSanity never throws on a non-string title (belt-and-suspenders)', () => {
test('numeric title does not crash the sanity pass', () => {
expect(() =>
assessContentSanity({ compiled_truth: 'hello world', timeline: '', title: 123 as unknown as string }),
).not.toThrow();
});
test('undefined title does not crash the sanity pass', () => {
expect(() =>
assessContentSanity({ compiled_truth: 'hello world', timeline: '', title: undefined as unknown as string }),
).not.toThrow();
});
});
+91
View File
@@ -0,0 +1,91 @@
/**
* #1393 — get_page exact-match path honors the federated source grant.
*
* Pre-fix the exact path used scalar `ctx.sourceId` only:
* const sourceOpts = ctx.sourceId ? { sourceId: ctx.sourceId } : {};
* A remote OAuth client with a federated `allowedSources` grant (and no single
* ctx.sourceId) therefore got an UNSCOPED exact lookup — a cross-source read of
* any page by slug. The fuzzy path was already scoped (#1436); this closes the
* exact path by (a) routing it through sourceScopeOpts and (b) teaching
* engine.getPage to honor a `sourceIds[]` array (both engines).
*/
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { resetPgliteState } from './helpers/reset-pglite.ts';
import { operations, OperationError, type OperationContext } from '../src/core/operations.ts';
let engine: PGLiteEngine;
const get_page = operations.find(o => o.name === 'get_page')!;
function ctxOf(overrides: Partial<OperationContext> = {}): OperationContext {
return {
engine: engine as any,
config: {} as any,
logger: console as any,
dryRun: false,
remote: true,
sourceId: 'default',
...overrides,
};
}
beforeAll(async () => {
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
}, 60_000);
afterAll(async () => {
if (engine) await engine.disconnect();
}, 60_000);
beforeEach(async () => {
await resetPgliteState(engine);
await engine.executeRaw(`INSERT INTO sources (id, name, local_path) VALUES ('alpha', 'alpha', '/tmp/alpha') ON CONFLICT (id) DO NOTHING`);
await engine.executeRaw(`INSERT INTO sources (id, name, local_path) VALUES ('beta', 'beta', '/tmp/beta') ON CONFLICT (id) DO NOTHING`);
// Distinct slugs per source so an exact lookup can leak across the boundary.
await engine.putPage('secret/beta-doc', {
type: 'note', title: 'Beta secret', compiled_truth: 'beta-only content', frontmatter: {},
}, { sourceId: 'beta' });
await engine.putPage('shared/alpha-doc', {
type: 'note', title: 'Alpha doc', compiled_truth: 'alpha content', frontmatter: {},
}, { sourceId: 'alpha' });
});
describe('engine.getPage honors sourceIds[] (federated grant)', () => {
test('sourceIds[] matching the page returns it', async () => {
const page = await engine.getPage('secret/beta-doc', { sourceIds: ['alpha', 'beta'] });
expect(page?.title).toBe('Beta secret');
});
test('sourceIds[] NOT containing the page returns null', async () => {
const page = await engine.getPage('secret/beta-doc', { sourceIds: ['alpha'] });
expect(page).toBeNull();
});
test('sourceIds[] takes precedence over scalar sourceId', async () => {
// scalar says alpha, array says beta-only — array wins, page found.
const page = await engine.getPage('secret/beta-doc', { sourceId: 'alpha', sourceIds: ['beta'] });
expect(page?.title).toBe('Beta secret');
});
});
describe('get_page handler closes the cross-source exact-read leak', () => {
test('remote client granted only [alpha] CANNOT read a beta-only slug', async () => {
const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['alpha'] } as any });
// Pre-fix this returned the beta page (leak). Now it is scoped out → 404.
await expect(get_page.handler(ctx, { slug: 'secret/beta-doc' })).rejects.toBeInstanceOf(OperationError);
});
test('remote client granted [alpha, beta] CAN read the beta slug', async () => {
const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['alpha', 'beta'] } as any });
const page: any = await get_page.handler(ctx, { slug: 'secret/beta-doc' });
expect(page.title).toBe('Beta secret');
});
test('remote client granted only [alpha] CAN read its own alpha slug', async () => {
const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['alpha'] } as any });
const page: any = await get_page.handler(ctx, { slug: 'shared/alpha-doc' });
expect(page.title).toBe('Alpha doc');
});
});
+45
View File
@@ -0,0 +1,45 @@
/**
* #1336 — legacy bearer tokens honor their stored federated_read grant.
*
* Pre-fix the legacy access_tokens path hardcoded `sourceId: 'default'` and never
* populated `allowedSources`, so a token whose `permissions.source_id` granted
* multiple sources could not read across them (and MCP reads silently returned
* empty in non-default brains). The grant is now parsed and threaded.
*
* Bounded by design: never widened to "all" — an empty/garbage value keeps the
* 'default' floor and no federated grant.
*/
import { describe, test, expect } from 'bun:test';
import { parseLegacyTokenScope } from '../src/mcp/http-transport.ts';
describe('parseLegacyTokenScope', () => {
test('array grant → allowedSources (federated read) with first as scalar floor', () => {
expect(parseLegacyTokenScope(['dept-x', 'shared'])).toEqual({ sourceId: 'dept-x', allowedSources: ['dept-x', 'shared'] });
});
test('single-element array → that source as both floor and grant', () => {
expect(parseLegacyTokenScope(['only'])).toEqual({ sourceId: 'only', allowedSources: ['only'] });
});
test('string grant → scalar source, no federated array', () => {
expect(parseLegacyTokenScope('team-a')).toEqual({ sourceId: 'team-a' });
});
test('absent grant → default floor, never widened', () => {
expect(parseLegacyTokenScope(undefined)).toEqual({ sourceId: 'default' });
expect(parseLegacyTokenScope(null)).toEqual({ sourceId: 'default' });
});
test('empty array → default floor, no grant (NOT "all")', () => {
expect(parseLegacyTokenScope([])).toEqual({ sourceId: 'default' });
});
test('garbage (number / empty string) → default floor', () => {
expect(parseLegacyTokenScope(123)).toEqual({ sourceId: 'default' });
expect(parseLegacyTokenScope('')).toEqual({ sourceId: 'default' });
});
test('array with non-string junk is filtered to valid sources', () => {
expect(parseLegacyTokenScope(['a', 5, '', 'b'])).toEqual({ sourceId: 'a', allowedSources: ['a', 'b'] });
});
});
+49
View File
@@ -0,0 +1,49 @@
/**
* #1711 — skill catalog parses YAML block-scalar `description:` fields.
*
* Pre-fix `parseDescriptionField` matched `description: |` with a greedy regex
* and captured the bare block indicator (`|` / `>`) as the description, so a
* skill written with `description: |` showed a literal "|" in the catalog
* instead of its text.
*/
import { describe, test, expect } from 'bun:test';
import { oneLineDescription } from '../src/core/skill-catalog.ts';
describe('oneLineDescription — block scalars', () => {
test('literal block scalar (|) folds indented lines into the description', () => {
const raw = ['name: demo', 'description: |', ' First line of the description.', ' Second line continues it.'].join('\n');
const out = oneLineDescription(raw, 'body fallback');
expect(out).toBe('First line of the description. Second line continues it.');
expect(out).not.toContain('|');
});
test('folded block scalar (>) is parsed too', () => {
const raw = ['description: >', ' Folded description', ' across two lines.'].join('\n');
expect(oneLineDescription(raw, 'fallback')).toBe('Folded description across two lines.');
});
test('chomping/indent indicators (|-, >+, |2) are recognized', () => {
const raw = ['description: |-', ' Trimmed block scalar.'].join('\n');
expect(oneLineDescription(raw, 'fallback')).toBe('Trimmed block scalar.');
});
test('block scalar with no indented continuation falls back to body prose', () => {
const raw = ['description: |', 'name: next-key'].join('\n');
expect(oneLineDescription(raw, 'Body prose line')).toBe('Body prose line');
});
});
describe('oneLineDescription — inline scalars still work', () => {
test('plain inline description', () => {
expect(oneLineDescription('description: A plain one-liner', 'fallback')).toBe('A plain one-liner');
});
test('quoted inline description strips surrounding quotes', () => {
expect(oneLineDescription('description: "Quoted desc"', 'fallback')).toBe('Quoted desc');
expect(oneLineDescription("description: 'Single quoted'", 'fallback')).toBe('Single quoted');
});
test('absent description falls back to first prose line', () => {
expect(oneLineDescription('name: x', 'The first prose line.')).toBe('The first prose line.');
});
});
+123
View File
@@ -0,0 +1,123 @@
/**
* Source-isolation trust+grant resolver (#1924, #1371, #1393).
*
* The cross-source leak class: a remote OAuth client scoped to one source could
* pass `source_id: "__all__"` (or an explicit out-of-grant source_id) to read
* sources it was never granted. Every source-scoped read op now routes through
* ONE resolver. These tests pin the trust+grant matrix at the unit level so a
* future per-handler "optimization" that re-inlines the `__all__` branch fails
* loudly here.
*/
import { describe, test, expect } from 'bun:test';
import {
resolveRequestedScope,
resolveCodeIntelScope,
OperationError,
type OperationContext,
} from '../src/core/operations.ts';
function ctxOf(overrides: Partial<OperationContext> = {}): OperationContext {
return {
engine: {} as any,
config: {} as any,
logger: console as any,
dryRun: false,
remote: true,
sourceId: 'default',
...overrides,
};
}
describe('resolveRequestedScope — __all__ / all_sources', () => {
test('trusted local + __all__ spans every source (empty scope)', () => {
const scope = resolveRequestedScope(ctxOf({ remote: false, sourceId: 'a' }), '__all__');
expect(scope).toEqual({});
});
test('remote + __all__ collapses to the caller grant, NOT the whole brain', () => {
const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any });
const scope = resolveRequestedScope(ctx, '__all__');
expect(scope).toEqual({ sourceIds: ['a', 'b'] });
});
test('remote + __all__ with single-source grant scopes to that one source', () => {
const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a'] } as any });
expect(resolveRequestedScope(ctx, '__all__')).toEqual({ sourceIds: ['a'] });
});
test('remote + __all__ with no federated grant falls back to scalar sourceId (never empty)', () => {
const ctx = ctxOf({ remote: true, sourceId: 'a' });
expect(resolveRequestedScope(ctx, '__all__')).toEqual({ sourceId: 'a' });
});
test('all_sources=true is treated identically to __all__', () => {
const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['x'] } as any });
expect(resolveRequestedScope(ctx, undefined, true)).toEqual({ sourceIds: ['x'] });
});
});
describe('resolveRequestedScope — explicit source_id', () => {
test('remote + explicit source_id OUTSIDE the grant is rejected', () => {
const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a'] } as any });
expect(() => resolveRequestedScope(ctx, 'b')).toThrow(OperationError);
try {
resolveRequestedScope(ctx, 'b');
} catch (e) {
expect((e as OperationError).code).toBe('permission_denied');
}
});
test('remote + explicit source_id INSIDE the grant is allowed', () => {
const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any });
expect(resolveRequestedScope(ctx, 'b')).toEqual({ sourceId: 'b' });
});
test('trusted local + explicit source_id is allowed even with no grant', () => {
expect(resolveRequestedScope(ctxOf({ remote: false }), 'anything')).toEqual({ sourceId: 'anything' });
});
test('remote with no federated grant array can pass an explicit source_id (scalar-floor model)', () => {
// allowedSources undefined → no federated restriction to enforce; the scalar
// sourceId path governs. (Empty [] is treated the same as undefined.)
expect(resolveRequestedScope(ctxOf({ remote: true }), 'z')).toEqual({ sourceId: 'z' });
const emptyGrant = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: [] } as any });
expect(resolveRequestedScope(emptyGrant, 'z')).toEqual({ sourceId: 'z' });
});
});
describe('resolveRequestedScope — default (no param)', () => {
test('falls back to the canonical sourceScopeOpts ladder (federated array wins)', () => {
const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any });
expect(resolveRequestedScope(ctx, undefined)).toEqual({ sourceIds: ['a', 'b'] });
});
test('falls back to scalar sourceId when no federated grant', () => {
expect(resolveRequestedScope(ctxOf({ remote: true, sourceId: 'a' }), undefined)).toEqual({ sourceId: 'a' });
});
});
describe('resolveCodeIntelScope — single-source code traversal', () => {
test('scalar sourceId → that source, allSources false', () => {
expect(resolveCodeIntelScope(ctxOf({ remote: true, sourceId: 'a' }), undefined)).toEqual({ allSources: false, sourceId: 'a' });
});
test('single-element federated grant → that one source', () => {
const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['only'] } as any });
expect(resolveCodeIntelScope(ctx, '__all__')).toEqual({ allSources: false, sourceId: 'only' });
});
test('multi-source federated grant → rejected (must specify one)', () => {
const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any });
expect(() => resolveCodeIntelScope(ctx, '__all__')).toThrow(OperationError);
});
test('trusted local + all → allSources true (spans the brain)', () => {
// ctx.sourceId is empty so the resolver yields {} → trusted-local allSources.
expect(resolveCodeIntelScope(ctxOf({ remote: false, sourceId: '' }), '__all__')).toEqual({ allSources: true, sourceId: undefined });
});
test('remote with no source in scope is denied, never widened to all', () => {
const ctx = ctxOf({ remote: true, sourceId: '' });
expect(() => resolveCodeIntelScope(ctx, '__all__')).toThrow(OperationError);
});
});