Skip to content

Commit 9210916

Browse files
Merge pull request #376 from markmhendrickson/fix/365-list-relationships-tenant-isolation
fix(security): scope /list_relationships queries to authenticated user (#365)
2 parents bb4fc89 + 772d582 commit 9210916

5 files changed

Lines changed: 166 additions & 7 deletions

File tree

src/actions.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6096,17 +6096,28 @@ app.post("/list_relationships", async (req, res) => {
60966096
const { relationshipsService } = await import("./services/relationships.js");
60976097

60986098
try {
6099+
// Tenant isolation: resolve authenticated user and scope all queries to
6100+
// their records. See docs/security/advisories/2026-05-21-relationship-
6101+
// endpoint-tenant-isolation.md (GHSA-wrr4-782v-jhwh) for context.
6102+
const userId = await getAuthenticatedUserId(req, parsed.data.user_id);
6103+
60996104
let relationships;
61006105
if (relationship_type) {
6101-
relationships = await relationshipsService.getRelationshipsByType(relationship_type as any);
6106+
relationships = await relationshipsService.getRelationshipsByType(
6107+
relationship_type as any,
6108+
false,
6109+
userId
6110+
);
61026111
// Filter by entity_id
61036112
relationships = relationships.filter(
61046113
(rel) => rel.source_entity_id === entity_id || rel.target_entity_id === entity_id
61056114
);
61066115
} else {
61076116
relationships = await relationshipsService.getRelationshipsForEntity(
61086117
entity_id,
6109-
normalizedDirection
6118+
normalizedDirection,
6119+
false,
6120+
userId
61106121
);
61116122
}
61126123

src/server.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2259,6 +2259,12 @@ export class NeotomaServer {
22592259
args: unknown
22602260
): Promise<{ content: Array<{ type: string; text: string }> }> {
22612261
const parsed = ListRelationshipsRequestSchema.parse(args ?? {});
2262+
2263+
// Tenant isolation: scope all queries to the authenticated user.
2264+
// See docs/security/advisories/2026-05-21-relationship-endpoint-
2265+
// tenant-isolation.md (GHSA-wrr4-782v-jhwh) for context.
2266+
const userId = this.getAuthenticatedUserId(parsed.user_id);
2267+
22622268
const normalizedDirection =
22632269
parsed.direction === "incoming" || parsed.direction === "inbound"
22642270
? "inbound"
@@ -2273,7 +2279,8 @@ export class NeotomaServer {
22732279
let outboundQuery = db
22742280
.from("relationship_snapshots")
22752281
.select("*")
2276-
.eq("source_entity_id", parsed.entity_id);
2282+
.eq("source_entity_id", parsed.entity_id)
2283+
.eq("user_id", userId);
22772284

22782285
if (parsed.relationship_type) {
22792286
outboundQuery = outboundQuery.eq("relationship_type", parsed.relationship_type);
@@ -2306,7 +2313,8 @@ export class NeotomaServer {
23062313
let inboundQuery = db
23072314
.from("relationship_snapshots")
23082315
.select("*")
2309-
.eq("target_entity_id", parsed.entity_id);
2316+
.eq("target_entity_id", parsed.entity_id)
2317+
.eq("user_id", userId);
23102318

23112319
if (parsed.relationship_type) {
23122320
inboundQuery = inboundQuery.eq("relationship_type", parsed.relationship_type);

src/services/relationships.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ export class RelationshipsService {
244244
entityId: string,
245245
direction: "outgoing" | "incoming" | "both" = "both",
246246
includeDeleted: boolean = false,
247+
userId?: string,
247248
): Promise<RelationshipSnapshot[]> {
248249
let query;
249250

@@ -264,6 +265,13 @@ export class RelationshipsService {
264265
.or(`source_entity_id.eq.${entityId},target_entity_id.eq.${entityId}`);
265266
}
266267

268+
// Tenant isolation: scope to authenticated user when provided.
269+
// The `userId` argument is required by all production callers; legacy
270+
// internal callers (e.g. tests, system jobs) may omit it.
271+
if (userId) {
272+
query = query.eq("user_id", userId);
273+
}
274+
267275
const { data, error } = await query.order("last_observation_at", {
268276
ascending: false,
269277
});
@@ -327,12 +335,21 @@ export class RelationshipsService {
327335
async getRelationshipsByType(
328336
type: RelationshipType,
329337
includeDeleted: boolean = false,
338+
userId?: string,
330339
): Promise<RelationshipSnapshot[]> {
331-
const { data, error } = await db
340+
let query = db
332341
.from("relationship_snapshots")
333342
.select("*")
334-
.eq("relationship_type", type)
335-
.order("last_observation_at", { ascending: false });
343+
.eq("relationship_type", type);
344+
345+
// Tenant isolation: scope to authenticated user when provided.
346+
if (userId) {
347+
query = query.eq("user_id", userId);
348+
}
349+
350+
const { data, error } = await query.order("last_observation_at", {
351+
ascending: false,
352+
});
336353

337354
if (error) {
338355
throw new Error(`Failed to get relationships by type: ${error.message}`);

src/shared/action_schemas.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ export const ListRelationshipsRequestSchema = z.object({
108108
relationship_type: RelationshipTypeSchema.optional(),
109109
limit: z.number().int().positive().optional().default(100),
110110
offset: z.number().int().nonnegative().optional().default(0),
111+
// Optional override honored only for LOCAL_DEV_USER_ID (see getAuthenticatedUserId).
112+
user_id: z.string().optional(),
111113
});
112114

113115
export const TimelineEventsRequestSchema = z.object({
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* Regression test for GHSA-wrr4-782v-jhwh /
3+
* docs/security/advisories/2026-05-21-relationship-endpoint-tenant-isolation.md
4+
*
5+
* Invariant: getRelationshipsForEntity and getRelationshipsByType MUST apply
6+
* an .eq("user_id", userId) filter on the relationship_snapshots query when
7+
* a userId is provided. Without this filter, callers can read relationship
8+
* edges belonging to other users on the same instance.
9+
*
10+
* The test records the chained calls against a mock db and asserts the
11+
* expected filter clauses are present.
12+
*/
13+
14+
import { describe, it, expect, beforeEach, vi } from "vitest";
15+
16+
type Call = { method: string; args: unknown[] };
17+
const recordedCalls: Call[] = [];
18+
19+
function makeChainable() {
20+
const proxy: any = new Proxy(
21+
{},
22+
{
23+
get(_target, prop: string) {
24+
if (prop === "then" || typeof prop === "symbol") return undefined;
25+
return (...args: unknown[]) => {
26+
recordedCalls.push({ method: prop, args });
27+
if (prop === "order") {
28+
// terminal: resolve with empty data
29+
return Promise.resolve({ data: [], error: null });
30+
}
31+
return proxy;
32+
};
33+
},
34+
}
35+
);
36+
return proxy;
37+
}
38+
39+
vi.mock("../../src/db.js", () => ({
40+
db: {
41+
from: (table: string) => {
42+
recordedCalls.push({ method: "from", args: [table] });
43+
return makeChainable();
44+
},
45+
},
46+
}));
47+
48+
import { relationshipsService } from "../../src/services/relationships.js";
49+
50+
describe("relationships tenant isolation (GHSA-wrr4-782v-jhwh)", () => {
51+
beforeEach(() => {
52+
recordedCalls.length = 0;
53+
});
54+
55+
it("getRelationshipsForEntity outgoing applies user_id filter when userId provided", async () => {
56+
await relationshipsService.getRelationshipsForEntity(
57+
"ent_abc",
58+
"outgoing",
59+
true, // includeDeleted to skip the deletion-observation second query
60+
"user-1"
61+
);
62+
63+
const eqCalls = recordedCalls.filter((c) => c.method === "eq");
64+
const hasUserIdEq = eqCalls.some(
65+
(c) => c.args[0] === "user_id" && c.args[1] === "user-1"
66+
);
67+
expect(hasUserIdEq).toBe(true);
68+
});
69+
70+
it("getRelationshipsForEntity incoming applies user_id filter when userId provided", async () => {
71+
await relationshipsService.getRelationshipsForEntity(
72+
"ent_abc",
73+
"incoming",
74+
true,
75+
"user-1"
76+
);
77+
78+
const eqCalls = recordedCalls.filter((c) => c.method === "eq");
79+
const hasUserIdEq = eqCalls.some(
80+
(c) => c.args[0] === "user_id" && c.args[1] === "user-1"
81+
);
82+
expect(hasUserIdEq).toBe(true);
83+
});
84+
85+
it("getRelationshipsForEntity both directions applies user_id filter when userId provided", async () => {
86+
await relationshipsService.getRelationshipsForEntity(
87+
"ent_abc",
88+
"both",
89+
true,
90+
"user-1"
91+
);
92+
93+
const eqCalls = recordedCalls.filter((c) => c.method === "eq");
94+
const hasUserIdEq = eqCalls.some(
95+
(c) => c.args[0] === "user_id" && c.args[1] === "user-1"
96+
);
97+
expect(hasUserIdEq).toBe(true);
98+
});
99+
100+
it("getRelationshipsByType applies user_id filter when userId provided", async () => {
101+
await relationshipsService.getRelationshipsByType(
102+
"WORKS_AT" as any,
103+
true,
104+
"user-1"
105+
);
106+
107+
const eqCalls = recordedCalls.filter((c) => c.method === "eq");
108+
const hasUserIdEq = eqCalls.some(
109+
(c) => c.args[0] === "user_id" && c.args[1] === "user-1"
110+
);
111+
expect(hasUserIdEq).toBe(true);
112+
});
113+
114+
it("does NOT apply user_id filter when userId omitted (back-compat for internal callers)", async () => {
115+
await relationshipsService.getRelationshipsForEntity("ent_abc", "outgoing", true);
116+
117+
const eqCalls = recordedCalls.filter((c) => c.method === "eq");
118+
const hasUserIdEq = eqCalls.some((c) => c.args[0] === "user_id");
119+
expect(hasUserIdEq).toBe(false);
120+
});
121+
});

0 commit comments

Comments
 (0)