mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 08:53:22 +00:00
v0.42.73.2 fix(security): fence dedup-resolved writes to the caller's own write scope (#3809)
* fix(security): fence the dedup-resolved slug under the caller's own confinement put_page's resolved-slug re-check tested `ctx.auth.boundSlugPrefixes` only. The delegated submit_agent -> subagent context carries `viaSubagent` + `allowedSlugPrefixes` but no `auth`, so a slug-bound client holding `agent` scope could delegate a write and have importFromContent's dedup pre-check redirect it onto a page outside its grant — where the disk write-through then re-rendered the victim's file with the caller's provenance. The re-check now applies whichever confinement the caller is actually under (OAuth binding and/or subagent allow-list / legacy namespace) via `slugOutsideCallerFence`, which composes the existing match rules rather than re-deriving them. Dedup returns status 'skipped' before any DB write, so the throw still rolls nothing back. The denial does not name the resolved slug (slug-enumeration oracle) and reads "your write scope", since either confinement can trigger it. Reported privately by Aleksei Razsadin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: coverage for the OAuth in-fence redirect and the missing-subagentId guard * v0.42.73.2 fix(security): fence dedup-resolved writes to the caller's own write scope VERSION + package.json + CHANGELOG for 0.42.73.2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: state that the write fence follows a delegated write --------- Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
Garry Tan
parent
aecb33e795
commit
15b9863d13
@@ -2,6 +2,26 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.42.73.2] - 2026-08-05
|
||||
|
||||
**A write that deduplication redirects onto an existing page is now checked against the write scope of whoever asked for it.** When the same content arrives under a new slug, gbrain recognises it and points the write at the page that already holds it. That redirected target is now tested against the caller's own scope — under whichever mechanism confines that caller. One of the two mechanisms was consulted at that point; both are now.
|
||||
|
||||
Nothing changes for local CLI use, or for clients that hold unrestricted write access — neither was ever scope-confined. A confined caller whose write dedups onto a page **inside** its own scope keeps working exactly as before; that redirect is a feature and it is preserved, with a regression test to keep it that way. A confined caller whose write dedups onto a page **outside** its scope now gets `permission_denied`, with the remedy in the message: drop the `id:` frontmatter field, or change the content, to write a new page under your own prefix. The denial does not name the page the write resolved to.
|
||||
|
||||
Recommended for any brain served over HTTP to scope-restricted clients.
|
||||
|
||||
### To take advantage of v0.42.73.2
|
||||
|
||||
```bash
|
||||
gbrain upgrade
|
||||
```
|
||||
|
||||
Nothing to configure. Existing clients keep their scopes unchanged, and no re-registration is needed.
|
||||
|
||||
### For contributors
|
||||
|
||||
Reported privately by an external security researcher, who supplied a fix and a regression test with it. The version that shipped composes the two existing scope-matching rules into a single predicate rather than restating either one, so the check at the door and the check after a redirect cannot drift apart; the audit the report prompted closed the same gap on one further caller path.
|
||||
|
||||
## [0.42.73.1] - 2026-08-05
|
||||
|
||||
**Removes the PR gate that v0.42.73.0 added, and reverts the v0.42.72.1 contribution-policy change it enforced.** The gate cannot function on this repository, and it caused a real incident before that was understood.
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -60,7 +60,15 @@ Isolation model:
|
||||
|
||||
The write fence is a **write** boundary within a source. It is not a privacy
|
||||
boundary, and it does not make every side effect prefix-clean. As of
|
||||
v0.42.72.0:
|
||||
v0.42.73.2:
|
||||
|
||||
- **The fence follows a delegated write.** When a client with `agent` scope
|
||||
hands work to a subagent via `submit_agent`, that subagent runs under its own
|
||||
slug confinement rather than the parent's OAuth binding. Both confinements are
|
||||
enforced, including on the path where deduplication redirects a write onto an
|
||||
existing page: the redirected target is checked against whichever confinement
|
||||
the calling context actually carries, so delegation does not widen what a
|
||||
client can write.
|
||||
|
||||
- **`add_link`/`remove_link` fence the `from` endpoint only.** A bound client
|
||||
can create an edge pointing AT a page it cannot write; the edge's `context`
|
||||
|
||||
+1
-1
@@ -148,7 +148,7 @@
|
||||
"bun": ">=1.3.10"
|
||||
},
|
||||
"license": "MIT",
|
||||
"version": "0.42.73.1",
|
||||
"version": "0.42.73.2",
|
||||
"overrides": {
|
||||
"@hono/node-server": "^2.0.5",
|
||||
"fast-uri": "^3.1.5",
|
||||
|
||||
+46
-18
@@ -213,20 +213,49 @@ function enforceSubagentSlugFence(ctx: OperationContext, slug: string, opName: s
|
||||
if (typeof ctx.subagentId !== 'number' || Number.isNaN(ctx.subagentId)) {
|
||||
throw new OperationError('permission_denied', `${opName} via subagent requires ctx.subagentId`);
|
||||
}
|
||||
if (slugUnderSubagentFence(ctx, slug)) return;
|
||||
const allowList = ctx.allowedSlugPrefixes;
|
||||
if (allowList && allowList.length > 0) {
|
||||
if (!matchesSlugAllowList(slug, allowList)) {
|
||||
throw new OperationError(
|
||||
'permission_denied',
|
||||
`${opName} slug '${slug}' is not within the trusted-workspace allow-list (${allowList.join(', ')})`
|
||||
);
|
||||
}
|
||||
} else {
|
||||
const prefix = `wiki/agents/${ctx.subagentId}/`;
|
||||
if (!slug.startsWith(prefix) || slug.length === prefix.length) {
|
||||
throw new OperationError('permission_denied', `${opName} via subagent must write under '${prefix}...'`);
|
||||
}
|
||||
}
|
||||
throw new OperationError(
|
||||
'permission_denied',
|
||||
allowList && allowList.length > 0
|
||||
? `${opName} slug '${slug}' is not within the trusted-workspace allow-list (${allowList.join(', ')})`
|
||||
: `${opName} via subagent must write under 'wiki/agents/${ctx.subagentId}/...'`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The subagent fence's MATCH RULE, without the throwing. Split out so the
|
||||
* resolved-slug re-check in put_page can ask the same question the entry
|
||||
* fence asks, instead of re-deriving the namespace literal and drifting.
|
||||
* Callers must have already established `ctx.viaSubagent === true`.
|
||||
*/
|
||||
function slugUnderSubagentFence(ctx: OperationContext, slug: string): boolean {
|
||||
const allowList = ctx.allowedSlugPrefixes;
|
||||
if (allowList && allowList.length > 0) return matchesSlugAllowList(slug, allowList);
|
||||
const prefix = `wiki/agents/${ctx.subagentId}/`;
|
||||
return slug.startsWith(prefix) && slug.length > prefix.length;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is `slug` outside whatever slug confinement THIS caller is under?
|
||||
*
|
||||
* A caller can be confined by EITHER mechanism, and the two arrive on
|
||||
* different context fields: an OAuth binding lands on `ctx.auth
|
||||
* .boundSlugPrefixes` (plain-prefix grammar), while a delegated subagent
|
||||
* lands on `ctx.viaSubagent` + `ctx.allowedSlugPrefixes` (glob grammar) and
|
||||
* carries NO `ctx.auth` at all. Testing only the OAuth field therefore lets
|
||||
* a bound client that also holds `agent` scope re-open the path it is fenced
|
||||
* out of simply by delegating the write through submit_agent — the same
|
||||
* bypass shape the facts-backstop gate below is keyed against.
|
||||
*
|
||||
* Unconfined callers (local CLI, unbound client) match neither arm and are
|
||||
* never fenced.
|
||||
*/
|
||||
function slugOutsideCallerFence(ctx: OperationContext, slug: string): boolean {
|
||||
const bound = ctx.auth?.boundSlugPrefixes;
|
||||
if (bound && !slugUnderBoundPrefixes(bound, slug)) return true;
|
||||
if (ctx.viaSubagent === true && !slugUnderSubagentFence(ctx, slug)) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1156,14 +1185,13 @@ const put_page: Operation = {
|
||||
// touching the DB, so throwing here leaves nothing to roll back.
|
||||
if (result.slug && result.slug !== slug) {
|
||||
// Deliberately does NOT name the resolved slug: it belongs to a page
|
||||
// outside the binding, and echoing it would turn frontmatter-id guessing
|
||||
// outside the fence, and echoing it would turn frontmatter-id guessing
|
||||
// into a slug-enumeration oracle.
|
||||
if (!slugUnderBoundPrefixes(ctx.auth?.boundSlugPrefixes ?? [], result.slug)
|
||||
&& ctx.auth?.boundSlugPrefixes) {
|
||||
ctx.logger.warn(`[put_page] dedup resolved '${slug}' to an out-of-fence page; refusing (client ${ctx.auth.clientId ?? 'unknown'})`);
|
||||
if (slugOutsideCallerFence(ctx, result.slug)) {
|
||||
ctx.logger.warn(`[put_page] dedup resolved '${slug}' to an out-of-fence page; refusing (client ${ctx.auth?.clientId ?? 'unknown'}, subagent ${ctx.subagentId ?? 'none'})`);
|
||||
throw new OperationError(
|
||||
'permission_denied',
|
||||
`put_page: this content already exists on a page outside your bound_slug_prefixes, so the write would have modified that page instead.`,
|
||||
`put_page: this content already exists on a page outside your write scope, so the write would have modified that page instead.`,
|
||||
'Remove the `id:` frontmatter field (or change the content) to write a new page under your own prefix.',
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
/**
|
||||
* put_page dedup resolved-slug fence (v0.42.73.1).
|
||||
*
|
||||
* importFromContent's dedup pre-check can resolve a write to a DIFFERENT page
|
||||
* than the caller named (same `frontmatter.id`), and the disk write-through
|
||||
* runs against that RESOLVED slug. The re-check that fences it shipped in
|
||||
* v0.42.72.0 testing `ctx.auth.boundSlugPrefixes` only — so a slug-bound
|
||||
* OAuth client holding `agent` scope could delegate the write through
|
||||
* submit_agent, whose subagent context carries `viaSubagent` +
|
||||
* `allowedSlugPrefixes` but NO `auth`, and land the rewrite on a page outside
|
||||
* its grant.
|
||||
*
|
||||
* Pins: every confinement a caller can be under fences the RESOLVED slug
|
||||
* (OAuth binding, trusted-workspace allow-list, legacy subagent namespace),
|
||||
* unconfined callers keep the dedup redirect, an in-fence redirect still
|
||||
* works, and the denial never names the resolved slug (it would be a
|
||||
* slug-enumeration oracle).
|
||||
*
|
||||
* PGLite hermetic. Every case resolves at the dedup pre-check, which returns
|
||||
* before any chunk/embed work.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { importFromContent } from '../src/core/import-file.ts';
|
||||
import { operations, OperationError } from '../src/core/operations.ts';
|
||||
import type { OperationContext, Operation, AuthInfo } from '../src/core/operations.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
const VICTIM_SLUG = 'people/alice-example';
|
||||
const VICTIM_ID = 'external-uuid-victim';
|
||||
|
||||
function put(): Operation {
|
||||
const found = operations.find(o => o.name === 'put_page');
|
||||
if (!found) throw new Error('put_page op missing');
|
||||
return found;
|
||||
}
|
||||
|
||||
function page(id: string, body: string): string {
|
||||
return ['---', 'type: concept', 'title: Notes', `id: ${id}`, '---', '', body].join('\n');
|
||||
}
|
||||
|
||||
function makeCtx(overrides: Partial<OperationContext> = {}): OperationContext {
|
||||
return {
|
||||
engine,
|
||||
config: { engine: 'pglite' } as any,
|
||||
logger: { info: () => {}, warn: () => {}, error: () => {} },
|
||||
dryRun: false,
|
||||
remote: true,
|
||||
sourceId: 'default',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function boundAuth(prefixes: string[]): AuthInfo {
|
||||
return {
|
||||
token: 'test-token',
|
||||
clientId: 'gbrain_cl_dedup_fence',
|
||||
scopes: ['read', 'write', 'agent'],
|
||||
sourceId: 'default',
|
||||
boundSlugPrefixes: prefixes,
|
||||
};
|
||||
}
|
||||
|
||||
/** The attacker's move: echo the victim's frontmatter id under an in-fence slug. */
|
||||
async function putEchoingVictimId(ctx: OperationContext, slug: string, id = VICTIM_ID) {
|
||||
return put().handler(ctx, { slug, content: page(id, 'Attacker body, different text.') });
|
||||
}
|
||||
|
||||
async function expectFenced(p: Promise<unknown>): Promise<void> {
|
||||
try {
|
||||
await p;
|
||||
throw new Error('should have thrown');
|
||||
} catch (e) {
|
||||
expect(e).toBeInstanceOf(OperationError);
|
||||
expect((e as OperationError).code).toBe('permission_denied');
|
||||
expect((e as Error).message).toContain('write scope');
|
||||
// The oracle guard: the resolved slug belongs to a page the caller may
|
||||
// not see, so it must never appear in the denial.
|
||||
expect((e as Error).message).not.toContain('alice-example');
|
||||
}
|
||||
}
|
||||
|
||||
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);
|
||||
const victim = await importFromContent(engine, VICTIM_SLUG, page(VICTIM_ID, 'Confidential.'), {
|
||||
noEmbed: true,
|
||||
sourceId: 'default',
|
||||
});
|
||||
expect(victim.status).toBe('imported');
|
||||
});
|
||||
|
||||
describe('put_page: dedup-resolved slug is fenced by the caller\'s own confinement', () => {
|
||||
test('delegated subagent (allow-list, NO auth) cannot rewrite an out-of-fence page', async () => {
|
||||
// The bypass: submit_agent's subagent context carries allowedSlugPrefixes
|
||||
// but no auth, so an auth-only re-check skipped exactly this caller.
|
||||
const ctx = makeCtx({
|
||||
viaSubagent: true,
|
||||
subagentId: 7,
|
||||
allowedSlugPrefixes: ['wiki/agents/7/*'],
|
||||
});
|
||||
await expectFenced(putEchoingVictimId(ctx, 'wiki/agents/7/notes'));
|
||||
});
|
||||
|
||||
test('legacy sandbox subagent (namespace fence, no allow-list) cannot either', async () => {
|
||||
const ctx = makeCtx({ viaSubagent: true, subagentId: 7 });
|
||||
await expectFenced(putEchoingVictimId(ctx, 'wiki/agents/7/notes'));
|
||||
});
|
||||
|
||||
test('slug-bound OAuth client cannot (the v0.42.72.0 case, still fenced)', async () => {
|
||||
const ctx = makeCtx({ auth: boundAuth(['emp-bob/']) });
|
||||
await expectFenced(putEchoingVictimId(ctx, 'emp-bob/notes'));
|
||||
});
|
||||
|
||||
test('a caller under BOTH confinements is fenced (requested slug satisfies both)', async () => {
|
||||
const ctx = makeCtx({
|
||||
auth: boundAuth(['emp-bob/']),
|
||||
viaSubagent: true,
|
||||
subagentId: 7,
|
||||
allowedSlugPrefixes: ['emp-bob/*'],
|
||||
});
|
||||
await expectFenced(putEchoingVictimId(ctx, 'emp-bob/notes'));
|
||||
});
|
||||
|
||||
test('feature preserved: a redirect INSIDE the fence still dedups', async () => {
|
||||
const inFence = await importFromContent(engine, 'wiki/agents/7/first', page('in-fence-id', 'Body.'), {
|
||||
noEmbed: true,
|
||||
sourceId: 'default',
|
||||
});
|
||||
expect(inFence.status).toBe('imported');
|
||||
|
||||
const ctx = makeCtx({
|
||||
viaSubagent: true,
|
||||
subagentId: 7,
|
||||
allowedSlugPrefixes: ['wiki/agents/7/*'],
|
||||
});
|
||||
const r = await putEchoingVictimId(ctx, 'wiki/agents/7/second', 'in-fence-id') as {
|
||||
slug: string; status: string;
|
||||
};
|
||||
expect(r.status).toBe('skipped');
|
||||
expect(r.slug).toBe('wiki/agents/7/first');
|
||||
});
|
||||
|
||||
test('feature preserved: a bound client\'s in-fence redirect still dedups', async () => {
|
||||
// The OAuth mirror of the case above — the fence must not break the happy
|
||||
// path it was already allowing before this change.
|
||||
const inFence = await importFromContent(engine, 'emp-bob/first', page('bob-id', 'Body.'), {
|
||||
noEmbed: true,
|
||||
sourceId: 'default',
|
||||
});
|
||||
expect(inFence.status).toBe('imported');
|
||||
|
||||
const ctx = makeCtx({ auth: boundAuth(['emp-bob/']) });
|
||||
const r = await putEchoingVictimId(ctx, 'emp-bob/second', 'bob-id') as {
|
||||
slug: string; status: string;
|
||||
};
|
||||
expect(r.status).toBe('skipped');
|
||||
expect(r.slug).toBe('emp-bob/first');
|
||||
});
|
||||
|
||||
test('fail-closed: viaSubagent without a subagentId is denied before any write', async () => {
|
||||
// enforceSubagentSlugFence refuses rather than trusting a dispatcher that
|
||||
// set viaSubagent but forgot the id — the branch slugUnderSubagentFence
|
||||
// would otherwise evaluate against 'wiki/agents/undefined/'.
|
||||
const ctx = makeCtx({ viaSubagent: true });
|
||||
const p = putEchoingVictimId(ctx, 'wiki/agents/7/notes');
|
||||
await expect(p).rejects.toBeInstanceOf(OperationError);
|
||||
await expect(p).rejects.toThrow(/requires ctx\.subagentId/);
|
||||
});
|
||||
|
||||
test('regression: an unconfined caller keeps the dedup redirect', async () => {
|
||||
const r = await putEchoingVictimId(makeCtx(), 'anywhere/notes') as { slug: string; status: string };
|
||||
expect(r.status).toBe('skipped');
|
||||
expect(r.slug).toBe(VICTIM_SLUG);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user