fix(memory): scope InsightsRepo operations to userId — M2-001/002
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful

Security audit findings and fixes:

M2-001 — searchByEmbedding: confirmed already user-scoped via WHERE user_id
M2-002 — findByUser: confirmed already user-scoped
M2-002 — decayOldInsights: was global (no userId filter); now requires userId
  param and scopes UPDATE to eq(insights.userId, userId). Added decayAllInsights
  as a separate system-only method for cron tier management.

Additional unscoped operations fixed:
- findById: added userId param + AND eq(userId) to prevent cross-user read
- update: added userId param + AND eq(userId) to prevent cross-user write
- remove: added userId param + AND eq(userId) to prevent cross-user delete
- memory.controller getInsight/removeInsight: now pass user.id for ownership
- summarization.service: switched tier-management cron to decayAllInsights

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-03-21 15:16:24 -05:00
parent 36095ad80f
commit b5d600e39b
3 changed files with 47 additions and 12 deletions

View File

@@ -137,7 +137,7 @@ export class SummarizationService {
const promoted = await this.logService.logs.promoteToCold(warmCutoff); const promoted = await this.logService.logs.promoteToCold(warmCutoff);
const purged = await this.logService.logs.purge(coldCutoff); const purged = await this.logService.logs.purge(coldCutoff);
const decayed = await this.memory.insights.decayOldInsights(decayCutoff); const decayed = await this.memory.insights.decayAllInsights(decayCutoff);
this.logger.log( this.logger.log(
`Tier management: ${promoted} logs→cold, ${purged} purged, ${decayed} insights decayed`, `Tier management: ${promoted} logs→cold, ${purged} purged, ${decayed} insights decayed`,

View File

@@ -73,8 +73,8 @@ export class MemoryController {
} }
@Get('insights/:id') @Get('insights/:id')
async getInsight(@Param('id') id: string) { async getInsight(@CurrentUser() user: { id: string }, @Param('id') id: string) {
const insight = await this.memory.insights.findById(id); const insight = await this.memory.insights.findById(id, user.id);
if (!insight) throw new NotFoundException('Insight not found'); if (!insight) throw new NotFoundException('Insight not found');
return insight; return insight;
} }
@@ -97,8 +97,8 @@ export class MemoryController {
@Delete('insights/:id') @Delete('insights/:id')
@HttpCode(HttpStatus.NO_CONTENT) @HttpCode(HttpStatus.NO_CONTENT)
async removeInsight(@Param('id') id: string) { async removeInsight(@CurrentUser() user: { id: string }, @Param('id') id: string) {
const deleted = await this.memory.insights.remove(id); const deleted = await this.memory.insights.remove(id, user.id);
if (!deleted) throw new NotFoundException('Insight not found'); if (!deleted) throw new NotFoundException('Insight not found');
} }

View File

@@ -19,8 +19,11 @@ export function createInsightsRepo(db: Db) {
.limit(limit); .limit(limit);
}, },
async findById(id: string): Promise<Insight | undefined> { async findById(id: string, userId: string): Promise<Insight | undefined> {
const rows = await db.select().from(insights).where(eq(insights.id, id)); const rows = await db
.select()
.from(insights)
.where(and(eq(insights.id, id), eq(insights.userId, userId)));
return rows[0]; return rows[0];
}, },
@@ -29,17 +32,24 @@ export function createInsightsRepo(db: Db) {
return rows[0]!; return rows[0]!;
}, },
async update(id: string, data: Partial<NewInsight>): Promise<Insight | undefined> { async update(
id: string,
userId: string,
data: Partial<NewInsight>,
): Promise<Insight | undefined> {
const rows = await db const rows = await db
.update(insights) .update(insights)
.set({ ...data, updatedAt: new Date() }) .set({ ...data, updatedAt: new Date() })
.where(eq(insights.id, id)) .where(and(eq(insights.id, id), eq(insights.userId, userId)))
.returning(); .returning();
return rows[0]; return rows[0];
}, },
async remove(id: string): Promise<boolean> { async remove(id: string, userId: string): Promise<boolean> {
const rows = await db.delete(insights).where(eq(insights.id, id)).returning(); const rows = await db
.delete(insights)
.where(and(eq(insights.id, id), eq(insights.userId, userId)))
.returning();
return rows.length > 0; return rows.length > 0;
}, },
@@ -70,8 +80,33 @@ export function createInsightsRepo(db: Db) {
/** /**
* Decay relevance scores for old insights that haven't been accessed recently. * Decay relevance scores for old insights that haven't been accessed recently.
* Scoped to a specific user to prevent cross-user data mutation.
*/ */
async decayOldInsights(olderThan: Date, decayFactor = 0.95): Promise<number> { async decayOldInsights(userId: string, olderThan: Date, decayFactor = 0.95): Promise<number> {
const result = await db
.update(insights)
.set({
relevanceScore: sql`${insights.relevanceScore} * ${decayFactor}`,
decayedAt: new Date(),
updatedAt: new Date(),
})
.where(
and(
eq(insights.userId, userId),
lt(insights.updatedAt, olderThan),
sql`${insights.relevanceScore} > 0.1`,
),
)
.returning();
return result.length;
},
/**
* Decay relevance scores for all users' old insights.
* This is a system-level maintenance operation intended for scheduled cron jobs only.
* Do NOT expose this through user-facing API endpoints.
*/
async decayAllInsights(olderThan: Date, decayFactor = 0.95): Promise<number> {
const result = await db const result = await db
.update(insights) .update(insights)
.set({ .set({