fix(mcp): intersect per-call scope params with env-pinned tenant scoping (#108)

CHANGELOG.md

@@ -10,7 +10,7 @@ All notable changes to engram are documented here. The format follows
 ### Fixed
 - A stdio start no longer bridges its per-process scoping away (#87, P1). `engram mcp` / `dist/interfaces/mcp/server.js` with `ENGRAM_SCOPE` or `ENGRAM_READ_SCOPES` in the environment — every Hermes profile child, or any host following `docs/integrate-your-agent.md` — used to bridge to a healthy daemon, which ran each forwarded call under *its* env: every profile wrote `global` and read every scope, silently. Such a start now runs inline without probing, with the reason on stderr (`running inline (ENGRAM_SCOPE=hermes:career is per-process; the daemon would ignore it)`). The daemon's `/health` gained `dbPath`; with `ENGRAM_DB_PATH` set the bridge is used only when it matches the daemon's, otherwise inline (`ENGRAM_DB_PATH=… but the daemon serves …`). A daemon that does not report `dbPath` (older build) keeps bridging as before.
 - `forget` no longer honours `scope: "global"` as a scope override when the server's own env pins reads (`ENGRAM_SCOPE` / `ENGRAM_READ_SCOPES`): an env-pinned stdio child (the Hermes memory-provider transport) could pass it to delete, or search for and delete, another tenant's memories (#109). The override still works from an unrestricted server. The tool description says so; the `tool-schemas` snapshot is re-recorded for that sentence only.
-
+- Per-call `scope` / `read_scopes` could escape an env-pinned tenant (#108). When `ENGRAM_SCOPE` / `ENGRAM_READ_SCOPES` are set (a stdio child scoped by the Hermes plugin), `resolveCallScoping` now intersects `read_scopes` with the env read scopes (empty intersection is an error) and rejects a `scope` that is not one of them — a tenant may only write where it may read — instead of letting the params replace the env. The shared HTTP daemon, where the env is unset and the params are the tenant identity, is unchanged.
 
 ### Changed
 - Test scaffolding shared instead of copied (#126, part of the #114 over-engineering burn-down). No behaviour change and no test lost: `tests/mocks/embeddings.ts` replaces eleven copies of the hash-based embeddings `vi.mock` factory, `tests/mocks/llm-fetch.ts` the four `stubFetch` / `makeAnthropicMock` copies in the `*-factory` suites, `tests/mocks/fake-worker.ts` the two `FakeWorker` classes; `tests/helpers.ts` gains the raw row inserts (`insertEntity`, `insertRelationship`, `insertCluster`, `insertBridgeScore`, `insertConversation`, `insertExchange`, `insertMemory`), `createTestMemory`, `makeExchanges` and the built-CLI guard (`builtCli`, `itBuilt`), and loses its `cosineSimilarity` copy (tests import `src/_core/search/vector.ts`) and the one-caller `createTestFixture` / `createSyntheticToolCall`. `vitest.config.ts` sets `test.unstubEnvs`, so suites isolate env with `vi.stubEnv(key, undefined)` instead of hand-rolled save/restore loops.

docs/integrate-your-agent.md

@@ -191,17 +191,22 @@ the `scope` / `read_scopes` parameters instead.
 
 Per call, `recall` and `recall_session` accept `scope` (reads then default to
 `global` + that scope) and `read_scopes` (explicit list); `remember` and
-`remember_batch` accept `scope`; `ingest_turn` requires `scope`. A parameter
-overrides the environment for that call only, so one HTTP daemon can serve
-several tenants — the Hermes plugin sends `hermes:<profile>` on every write.
-With nothing set the server is single-tenant: writes land in `global`, reads
-see everything. Convention: name scopes `<host>:<profile>`, for example
-`codex:work` or `hermes:career`.
-
-Because the model can pass `scope` / `read_scopes` itself, the integration
-layer (your hook or plugin), not the tool schema, is what pins a tenant: set
-the params from your side and do not expose them to the model if it must not
-choose its own scope.
+`remember_batch` accept `scope`; `ingest_turn` requires `scope`. With the
+environment unset (the shared HTTP daemon) the params are the tenant identity
+for that call, so one daemon can serve several tenants — the Hermes plugin
+sends `hermes:<profile>` on every write. With `ENGRAM_SCOPE` /
+`ENGRAM_READ_SCOPES` set (a stdio child pinned to one tenant) the params can
+only narrow: `read_scopes` is intersected with the env read scopes and `scope`
+must be one of them; anything else is rejected, so a prompt-injected model
+cannot read or write another tenant. With nothing set the server is
+single-tenant: writes land in `global`, reads see everything. Convention: name
+scopes `<host>:<profile>`, for example `codex:work` or `hermes:career`.
+
+On the HTTP daemon the model can pass `scope` / `read_scopes` itself, so the
+integration layer (your hook or plugin), not the tool schema, is what pins a
+tenant there: set the params from your side and do not expose them to the
+model if it must not choose its own scope. Pin the env on a stdio child
+instead when the model must be sandboxed.
 
 **Caveat:** scope filtering applies to the **semantic** memory table only.
 Episodic conversation exchanges and graph entities are not scope-filtered.

src/interfaces/mcp/scoping.ts

@@ -62,28 +62,45 @@ function requireScope(value: string, param: string): string {
 }
 
 /**
- * Resolve the scoping for a single tool call: request params win over env,
- * env is the default when params are absent.
+ * Resolve the scoping for a single tool call.
  *
- * - `scope` overrides ENGRAM_SCOPE. When ENGRAM_READ_SCOPES is unset it also
- *   re-derives the read default to global + own, mirroring the env rule.
- * - `read_scopes` overrides ENGRAM_READ_SCOPES (and the derived default).
+ * When the env does not restrict (no ENGRAM_SCOPE / ENGRAM_READ_SCOPES — the
+ * shared HTTP daemon), params are the tenant identity: `scope` sets the write
+ * scope and re-derives reads to global + own; `read_scopes` is taken as given.
+ *
+ * When the env restricts (a stdio child pinned to one tenant, #108), params
+ * may only narrow it: `read_scopes` is intersected with the env read scopes
+ * (empty intersection throws) and `scope` must be one of them — a tenant may
+ * only write where it may read. The env read scopes stay the read default.
  */
 export function resolveCallScoping(
   env: Record<string, string | undefined>,
   params: CallScopeParams,
 ): TenantScoping {
   const base = getTenantScoping(env);
+  const allowed = base.readScopes;
 
-  const writeScope = params.scope !== undefined ? requireScope(params.scope, "scope") : base.writeScope;
+  let writeScope = base.writeScope;
+  if (params.scope !== undefined) {
+    writeScope = requireScope(params.scope, "scope");
+    if (allowed && !allowed.includes(writeScope)) {
+      throw new Error(`scope "${writeScope}" is outside this server's ENGRAM_READ_SCOPES (${allowed.join(",")})`);
+    }
+  }
 
-  let readScopes = base.readScopes;
+  let readScopes = allowed;
   if (params.read_scopes !== undefined) {
     if (!Array.isArray(params.read_scopes) || params.read_scopes.length === 0) {
       throw new Error("read_scopes must contain at least one scope");
     }
     readScopes = params.read_scopes.map((s) => requireScope(s, "read_scopes"));
-  } else if (params.scope !== undefined && !env.ENGRAM_READ_SCOPES?.trim()) {
+    if (allowed) {
+      readScopes = readScopes.filter((s) => allowed.includes(s));
+      if (readScopes.length === 0) {
+        throw new Error(`read_scopes has no scope inside this server's ENGRAM_READ_SCOPES (${allowed.join(",")})`);
+      }
+    }
+  } else if (params.scope !== undefined && !allowed) {
     readScopes = ["global", writeScope as string];
   }
 

tests/core/tenant-scoping.test.ts

@@ -57,20 +57,42 @@ describe("resolveCallScoping (per-request override)", () => {
     expect(s.readScopes).toEqual(["global", "hermes:career"]);
   });
 
-  it("scope param overrides ENGRAM_SCOPE but keeps an explicit ENGRAM_READ_SCOPES", () => {
-    const env = { ENGRAM_SCOPE: "hermes:pmp", ENGRAM_READ_SCOPES: "global,hermes:pmp" };
+  it("scope param may pick another env read scope and keeps ENGRAM_READ_SCOPES as the read default", () => {
+    const env = { ENGRAM_SCOPE: "hermes:pmp", ENGRAM_READ_SCOPES: "global,hermes:pmp,hermes:career" };
     const s = resolveCallScoping(env, { scope: "hermes:career" });
     expect(s.writeScope).toBe("hermes:career");
-    expect(s.readScopes).toEqual(["global", "hermes:pmp"]);
+    expect(s.readScopes).toEqual(["global", "hermes:pmp", "hermes:career"]);
   });
 
-  it("read_scopes param overrides env read scopes and leaves writeScope alone", () => {
-    const env = { ENGRAM_SCOPE: "hermes:pmp" };
-    const s = resolveCallScoping(env, { read_scopes: [" hermes:career "] });
+  // #108: an env-pinned child must not escape its tenant through params.
+  it("scope param outside the env read scopes throws", () => {
+    expect(() => resolveCallScoping({ ENGRAM_SCOPE: "hermes:career" }, { scope: "hermes:personal" })).toThrow(
+      /outside this server's ENGRAM_READ_SCOPES/,
+    );
+    expect(() =>
+      resolveCallScoping({ ENGRAM_READ_SCOPES: "global,hermes:career" }, { scope: "hermes:personal" }),
+    ).toThrow(/outside/);
+  });
+
+  it("read_scopes param is intersected with the env read scopes", () => {
+    const env = { ENGRAM_SCOPE: "hermes:pmp", ENGRAM_READ_SCOPES: "global,hermes:pmp,hermes:career" };
+    const s = resolveCallScoping(env, { read_scopes: [" hermes:career ", "hermes:personal"] });
     expect(s.writeScope).toBe("hermes:pmp");
     expect(s.readScopes).toEqual(["hermes:career"]);
   });
 
+  it("read_scopes param with no scope inside the env read scopes throws", () => {
+    expect(() => resolveCallScoping({ ENGRAM_SCOPE: "hermes:pmp" }, { read_scopes: ["hermes:personal"] })).toThrow(
+      /no scope inside/,
+    );
+  });
+
+  it("without env restriction read_scopes is taken as given", () => {
+    const s = resolveCallScoping({}, { read_scopes: [" hermes:career "] });
+    expect(s.writeScope).toBeUndefined();
+    expect(s.readScopes).toEqual(["hermes:career"]);
+  });
+
   it("rejects empty / whitespace scope strings and empty read_scopes", () => {
     expect(() => resolveCallScoping({}, { scope: "" })).toThrow(/scope/);
     expect(() => resolveCallScoping({}, { scope: "   " })).toThrow(/scope/);

tests/interfaces/mcp/per-request-scoping.test.ts

@@ -159,9 +159,13 @@ describe("per-request scope / read_scopes params", () => {
       // env read default = global + own, so the career memory is hidden…
       const hidden = await handleToolCall("recall", { query: "interview", sources: ["semantic"] });
       expect(hidden.content[0].text as string).not.toContain("Databricks");
-      // …unless the call overrides read_scopes
-      const shown = await handleToolCall("recall", { query: "interview", sources: ["semantic"], read_scopes: ["hermes:career"] });
-      expect(shown.content[0].text as string).toContain("Databricks");
+      // …and the call cannot widen past the env (#108): read_scopes intersects, scope must be readable
+      const escape = await handleToolCall("recall", { query: "interview", sources: ["semantic"], read_scopes: ["hermes:career"] });
+      expect(escape.isError).toBe(true);
+      expect(escape.content[0].text as string).toContain("no scope inside");
+      const write = await handleToolCall("remember", { content: "leak", scope: "hermes:career" });
+      expect(write.isError).toBe(true);
+      expect(write.content[0].text as string).toContain("outside");
     } finally {
       delete process.env.ENGRAM_SCOPE;
     }