From 9baa80404895c8bdd4bc80c3b86e87ea7aceee55 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sat, 10 Oct 2026 19:02:09 -0400 Subject: [PATCH] refactor(stores): shared escapeLike and bumpGeneration helpers escapeLike replaces the LIKE escaping copied into PermissionStore (twice), ShareStore and FSEntryStore, and now also guards SubdomainStore's prefix filters, which passed caller prefixes into LIKE unescaped. bumpGeneration sends INCR and EXPIRE in one pipeline instead of two awaited commands in PermissionStore, EventSubscriptionStore and PresenceStore. --- .../stores/events/EventSubscriptionStore.ts | 17 +++-- src/backend/stores/events/PresenceStore.ts | 12 ++-- src/backend/stores/fs/FSEntryStore.ts | 7 +- .../stores/permission/PermissionStore.ts | 30 +++----- src/backend/stores/share/ShareStore.js | 4 +- .../stores/subdomain/SubdomainStore.test.ts | 22 ++++++ .../stores/subdomain/SubdomainStore.ts | 17 ++--- src/backend/util/redisGeneration.test.ts | 68 +++++++++++++++++++ src/backend/util/redisGeneration.ts | 41 +++++++++++ src/backend/util/sqlLike.test.ts | 32 +++++++++ src/backend/util/sqlLike.ts | 26 +++++++ 11 files changed, 227 insertions(+), 49 deletions(-) create mode 100644 src/backend/util/redisGeneration.test.ts create mode 100644 src/backend/util/redisGeneration.ts create mode 100644 src/backend/util/sqlLike.test.ts create mode 100644 src/backend/util/sqlLike.ts diff --git a/src/backend/stores/events/EventSubscriptionStore.ts b/src/backend/stores/events/EventSubscriptionStore.ts index 50c8dd8bf..546380dab 100644 --- a/src/backend/stores/events/EventSubscriptionStore.ts +++ b/src/backend/stores/events/EventSubscriptionStore.ts @@ -19,6 +19,7 @@ import { EVENTS_SESSION_SUBSCRIPTIONS_PER_SOCKET } from '../../controllers/events/limits.js'; import { HttpError } from '../../core/http/HttpError.js'; +import { bumpGeneration } from '../../util/redisGeneration.js'; import { PuterStore } from '../types.js'; import type { DispatchSubscription, @@ -1135,15 +1136,17 @@ export class EventSubscriptionStore extends PuterStore { // -- Generation -------------------------------------------------- /** - * Advance the user's subscription-set generation. A single-key `INCR`, so - * it is cluster-safe and costs one command; the broadcast that carries it - * is what actually invalidates other processes. + * Advance the user's subscription-set generation. Single-key, so it is + * cluster-safe; the broadcast that carries it is what actually invalidates + * other processes. */ async bumpGeneration(userId: number | string): Promise { - const key = generationKey(userId); - const next = await this.clients.redis.incr(key); - await this.clients.redis.expire(key, GENERATION_TTL_SECONDS); - return typeof next === 'number' ? next : 0; + const next = await bumpGeneration( + this.clients.redis, + generationKey(userId), + GENERATION_TTL_SECONDS, + ); + return Number.isFinite(next) ? next : 0; } async getGeneration(userId: number | string): Promise { diff --git a/src/backend/stores/events/PresenceStore.ts b/src/backend/stores/events/PresenceStore.ts index 1dec4f92e..b212ae7e6 100644 --- a/src/backend/stores/events/PresenceStore.ts +++ b/src/backend/stores/events/PresenceStore.ts @@ -18,6 +18,7 @@ */ import { CONCURRENT_SLOT_TTL_MS } from '../../core/http/middleware/rateLimit.js'; +import { bumpGeneration } from '../../util/redisGeneration.js'; import { PuterStore } from '../types.js'; import { KV_GLOBAL_APP_KEY } from '../systemKv/SystemKVStore.js'; @@ -458,11 +459,12 @@ export class PresenceStore extends PuterStore { // -- Generation -------------------------------------------------- - /** Advance the user's presence generation. One key, so one command. */ + /** Advance the user's presence generation. */ async bumpGeneration(userId: number): Promise { - const key = generationKey(userId); - const next = await this.clients.redis.incr(key); - await this.clients.redis.expire(key, GENERATION_TTL_SECONDS); - return typeof next === 'number' ? next : Number(next); + return bumpGeneration( + this.clients.redis, + generationKey(userId), + GENERATION_TTL_SECONDS, + ); } } diff --git a/src/backend/stores/fs/FSEntryStore.ts b/src/backend/stores/fs/FSEntryStore.ts index b648008a0..1e510f58b 100644 --- a/src/backend/stores/fs/FSEntryStore.ts +++ b/src/backend/stores/fs/FSEntryStore.ts @@ -24,6 +24,7 @@ import { v4 as uuidv4 } from 'uuid'; import { HttpError } from '../../core/http/HttpError.js'; import type { LayerInstances } from '../../types.js'; import { runWithConcurrencyLimit } from '../../util/concurrency.js'; +import { escapeLike } from '../../util/sqlLike.js'; import { keysetPage, normalizeLimit, @@ -2715,12 +2716,8 @@ export class FSEntryStore extends PuterStore { return entries; } - // Escape a string for safe use inside a LIKE pattern. We use backslash as - // the LIKE escape char so `%` and `_` in user paths aren't treated as wildcards. - // Uses `!` as the LIKE escape character — both MySQL and SQLite treat `!` as - // a plain character inside string literals, so no dialect-specific quoting. #escapeLikePattern(value: string): string { - return value.replace(/([!%_])/g, '!$1'); + return escapeLike(value); } // All descendants of a directory path (recursive). Paths in fsentries are diff --git a/src/backend/stores/permission/PermissionStore.ts b/src/backend/stores/permission/PermissionStore.ts index d4f6bc561..cc4f23a59 100644 --- a/src/backend/stores/permission/PermissionStore.ts +++ b/src/backend/stores/permission/PermissionStore.ts @@ -29,6 +29,8 @@ import { } from '../../services/permission/consts'; import { kv } from '../../util/kvSingleton'; import { keysetPage, openIdCursor } from '../../util/pagination'; +import { bumpGeneration } from '../../util/redisGeneration'; +import { escapeLike } from '../../util/sqlLike'; import type { UserRow } from '../user/UserStore'; // Short TTLs: FK CASCADE on user/app delete + PermissionService rewriters @@ -124,17 +126,7 @@ export interface FlatPermRef { const subtreeRoot = (prefix: string): string => prefix.endsWith(':') ? prefix.slice(0, -1) : prefix; -/** - * Match a permission and everything beneath it. - * - * `_` and `%` are LIKE wildcards, so an unescaped one would widen the match - * beyond the intended subtree. `!` as the escape character, matching - * FSEntryStore: a backslash one would have to be written `ESCAPE '\\'` in the - * SQL text, and MySQL processes backslash escapes inside string literals, so - * the `'\'` a JS `'\\'` produces reads as an escaped quote and leaves the - * literal unterminated. SQLite and Postgres accept it, which is why only MySQL - * would have seen the parse error. - */ +/** Match a permission and everything beneath it. */ const subtreeClause = ( permissions: string[], ): { where: string; params: string[] } => ({ @@ -143,7 +135,7 @@ const subtreeClause = ( .join(' OR '), params: permissions.flatMap((permission) => [ permission, - `${permission.replace(/([!%_])/g, '!$1')}:%`, + `${escapeLike(permission)}:%`, ]), }); @@ -498,13 +490,7 @@ export class PermissionStore extends PuterStore { permissions: string[], ): Promise> { if (permissions.length === 0) return []; - const where = permissions - .map(() => "(`permission` = ? OR `permission` LIKE ? ESCAPE '!')") - .join(' OR '); - const params = permissions.flatMap((permission) => [ - permission, - `${permission.replace(/([!%_])/g, '!$1')}:%`, - ]); + const { where, params } = subtreeClause(permissions); const rows = (await this.clients.db.read( 'SELECT `group_id`, `permission` FROM `user_to_group_permissions` ' + @@ -1344,16 +1330,16 @@ export class PermissionStore extends PuterStore { async #applyCacheGenerationBump(actorUid: string): Promise { const key = this.#cacheGenerationKey(actorUid); try { - const next = await this.clients.redis.incr(key); // Keep the counter alive well past the cache TTL so it can't // reset to 0 and revive same-generation stale entries. - await this.clients.redis.expire( + const next = await bumpGeneration( + this.clients.redis, key, PERMISSION_CACHE_GENERATION_TTL_SECONDS, ); // Make this node consistent immediately; other nodes pick up the // new value when their local copy expires (≤ local TTL). - if (typeof next === 'number') { + if (Number.isFinite(next)) { kv.set(this.#localCacheGenerationKey(actorUid), next, { EX: PERMISSION_CACHE_GENERATION_LOCAL_TTL_SECONDS, }); diff --git a/src/backend/stores/share/ShareStore.js b/src/backend/stores/share/ShareStore.js index 7e28568c7..779fa70d2 100644 --- a/src/backend/stores/share/ShareStore.js +++ b/src/backend/stores/share/ShareStore.js @@ -20,6 +20,7 @@ import { v4 as uuidv4 } from 'uuid'; import { HttpError } from '../../core/http/HttpError.js'; import { decodeCursor, keysetPage, openIdCursor } from '../../util/pagination'; +import { escapeLike } from '../../util/sqlLike'; import { PuterStore } from '../types'; /** Default page size for the keyset listings. */ @@ -881,12 +882,11 @@ export class ShareStore extends PuterStore { * @param {number} newOwnerId @param {string} path */ async reassignEntryOwnerUnder(newOwnerId, path) { - const escaped = path.replace(/([!%_])/g, '!$1'); await this.clients.db.write( 'UPDATE `share` SET `entry_owner_user_id` = ? WHERE `fsentry_id` ' + 'IN (SELECT `id` FROM `fsentries` WHERE `path` = ? OR ' + "`path` LIKE ? ESCAPE '!')", - [newOwnerId, path, `${escaped}/%`], + [newOwnerId, path, `${escapeLike(path)}/%`], ); } diff --git a/src/backend/stores/subdomain/SubdomainStore.test.ts b/src/backend/stores/subdomain/SubdomainStore.test.ts index 0f776cb65..f0eb31fe0 100644 --- a/src/backend/stores/subdomain/SubdomainStore.test.ts +++ b/src/backend/stores/subdomain/SubdomainStore.test.ts @@ -406,6 +406,28 @@ describe('SubdomainStore listing and counting', () => { ).toBe(0); }); + it('matches a prefix literally, not as a LIKE pattern', async () => { + const userId = await makeUser(); + const p = prefix(); + await createFor(userId, `${p}a_b`); + await createFor(userId, `${p}axb`); + + expect( + (await store.listByUserIdAndPrefix(userId, `${p}a_`)).map( + (r) => r.subdomain, + ), + ).toEqual([`${p}a_b`]); + expect(await store.countByUserIdAndPrefix(userId, `${p}a_`)).toBe(1); + expect( + ( + await store.listByUserId(userId, { excludePrefix: `${p}a_` }) + ).map((r) => r.subdomain), + ).toEqual([`${p}axb`]); + expect( + await store.count({ userId, excludePrefix: `${p}a_` }), + ).toBe(1); + }); + it('narrows a prefix listing by a single app id and paginates it', async () => { const userId = await makeUser(); const p = prefix(); diff --git a/src/backend/stores/subdomain/SubdomainStore.ts b/src/backend/stores/subdomain/SubdomainStore.ts index eafc3e1fe..7bed6d290 100644 --- a/src/backend/stores/subdomain/SubdomainStore.ts +++ b/src/backend/stores/subdomain/SubdomainStore.ts @@ -18,6 +18,7 @@ */ import { v4 as uuidv4 } from 'uuid'; +import { escapeLike } from '../../util/sqlLike'; import { PuterStore } from '../types'; /** @@ -358,8 +359,8 @@ export class SubdomainStore extends PuterStore { values.push(afterId); } if (excludePrefix) { - conditions.push('`subdomain` NOT LIKE ?'); - values.push(`${excludePrefix}%`); + conditions.push("`subdomain` NOT LIKE ? ESCAPE '!'"); + values.push(`${escapeLike(excludePrefix)}%`); } const whereClause = conditions.length ? `WHERE ${conditions.join(' AND ')}` @@ -393,8 +394,8 @@ export class SubdomainStore extends PuterStore { values.push(appOwner); } if (excludePrefix) { - conditions.push('`subdomain` NOT LIKE ?'); - values.push(`${excludePrefix}%`); + conditions.push("`subdomain` NOT LIKE ? ESCAPE '!'"); + values.push(`${escapeLike(excludePrefix)}%`); } const whereClause = conditions.length ? `WHERE ${conditions.join(' AND ')}` @@ -435,8 +436,8 @@ export class SubdomainStore extends PuterStore { ): Promise { if (!userId || prefix == null) return []; - const conditions = ['`user_id` = ?', '`subdomain` LIKE ?']; - const values: unknown[] = [userId, `${prefix}%`]; + const conditions = ['`user_id` = ?', "`subdomain` LIKE ? ESCAPE '!'"]; + const values: unknown[] = [userId, `${escapeLike(prefix)}%`]; const appOwnerFilter = appOwnerCondition(extra); if (appOwnerFilter) { conditions.push(appOwnerFilter.condition); @@ -478,8 +479,8 @@ export class SubdomainStore extends PuterStore { ): Promise { if (!userId || prefix == null) return 0; - const conditions = ['`user_id` = ?', '`subdomain` LIKE ?']; - const values: unknown[] = [userId, `${prefix}%`]; + const conditions = ['`user_id` = ?', "`subdomain` LIKE ? ESCAPE '!'"]; + const values: unknown[] = [userId, `${escapeLike(prefix)}%`]; const appOwnerFilter = appOwnerCondition(extra); if (appOwnerFilter) { conditions.push(appOwnerFilter.condition); diff --git a/src/backend/util/redisGeneration.test.ts b/src/backend/util/redisGeneration.test.ts new file mode 100644 index 000000000..c5bcce0a8 --- /dev/null +++ b/src/backend/util/redisGeneration.test.ts @@ -0,0 +1,68 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import MockRedis from 'ioredis-mock'; +import type { Cluster } from 'ioredis'; +import { describe, expect, it, vi } from 'vitest'; +import { bumpGeneration } from './redisGeneration'; + +const redis = new MockRedis() as unknown as Cluster; +const freshKey = () => `gen-test:${Math.random().toString(36).slice(2)}`; + +describe('bumpGeneration', () => { + it('increments from zero and renews the TTL each time', async () => { + const key = freshKey(); + expect(await bumpGeneration(redis, key, 60)).toBe(1); + await redis.expire(key, 5); + expect(await bumpGeneration(redis, key, 60)).toBe(2); + expect(await redis.ttl(key)).toBeGreaterThan(5); + }); + + it('sends the increment and the expiry in one round trip', async () => { + const pipeline = vi.spyOn(redis, 'pipeline'); + const incr = vi.spyOn(redis, 'incr'); + try { + await bumpGeneration(redis, freshKey(), 60); + expect(pipeline).toHaveBeenCalledTimes(1); + expect(incr).not.toHaveBeenCalled(); + } finally { + pipeline.mockRestore(); + incr.mockRestore(); + } + }); + + it('throws the error redis reports for either command', async () => { + const failing = { + pipeline: () => { + const chain = { + incr: () => chain, + expire: () => chain, + exec: async () => [ + [new Error('WRONGTYPE not an integer'), null], + [null, 1], + ], + }; + return chain; + }, + } as unknown as Cluster; + await expect(bumpGeneration(failing, freshKey(), 60)).rejects.toThrow( + 'WRONGTYPE', + ); + }); +}); diff --git a/src/backend/util/redisGeneration.ts b/src/backend/util/redisGeneration.ts new file mode 100644 index 000000000..295f5ee64 --- /dev/null +++ b/src/backend/util/redisGeneration.ts @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import type { Cluster } from 'ioredis'; + +/** + * Increment a generation counter and renew its TTL in one round trip; the two + * commands travel together, so the counter can't be left without an expiry. + * Returns the new value. + */ +export const bumpGeneration = async ( + redis: Pick, + key: string, + ttlSeconds: number, +): Promise => { + const results = await redis + .pipeline() + .incr(key) + .expire(key, ttlSeconds) + .exec(); + for (const [error] of results ?? []) { + if (error) throw error; + } + return Number(results?.[0]?.[1]); +}; diff --git a/src/backend/util/sqlLike.test.ts b/src/backend/util/sqlLike.test.ts new file mode 100644 index 000000000..2b4a0637e --- /dev/null +++ b/src/backend/util/sqlLike.test.ts @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { describe, expect, it } from 'vitest'; +import { escapeLike } from './sqlLike'; + +describe('escapeLike', () => { + it('escapes LIKE wildcards and the escape character itself', () => { + expect(escapeLike('50%_off!')).toBe('50!%!_off!!'); + }); + + it('leaves everything else alone', () => { + expect(escapeLike('workers.puter.')).toBe('workers.puter.'); + expect(escapeLike('/user/a b\\c')).toBe('/user/a b\\c'); + }); +}); diff --git a/src/backend/util/sqlLike.ts b/src/backend/util/sqlLike.ts new file mode 100644 index 000000000..6029299ca --- /dev/null +++ b/src/backend/util/sqlLike.ts @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +/** + * `value` as literal text inside a LIKE pattern, for use with `ESCAPE '!'`: `%` + * and `_` would otherwise widen the match. `!` rather than a backslash, which + * MySQL also reads as an escape inside the string literal itself. + */ +export const escapeLike = (value: string): string => + value.replace(/[!%_]/g, '!$&');