diff --git a/src/backend/controllers/fs/FSController.test.ts b/src/backend/controllers/fs/FSController.test.ts index 89db117e1..236a54251 100644 --- a/src/backend/controllers/fs/FSController.test.ts +++ b/src/backend/controllers/fs/FSController.test.ts @@ -23,9 +23,11 @@ import { v4 as uuidv4 } from 'uuid'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; +import { consumeRouteRateLimit } from '../../core/http/middleware/rateLimit.js'; import { PuterServer } from '../../server.js'; import { setupTestServer } from '../../testUtil.js'; import { generateDefaultFsentries } from '../../util/userProvisioning.js'; +import { SHARE_LIST_LIMIT } from '../share/limits.js'; import type { FSController } from './FSController.js'; import type { ClientSignedWriteResponse, @@ -798,6 +800,40 @@ describe('FSController.statEntry', () => { statusCode: 401, }); }); + + it('draws return_shares from the share-listing budget', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const path = `/${username}/Documents/share-budget`; + await withActor(actor, () => + controller.mkdirEntry(makeReq({ body: { path }, actor }), makeRes().res), + ); + + // Spend the whole share:list bucket, as /share/shares' gate would. + const chargeReq = makeReq({ body: {}, actor }); + for (let i = 0; i < SHARE_LIST_LIMIT.limit; i++) { + await consumeRouteRateLimit(chargeReq, SHARE_LIST_LIMIT); + } + + await expect( + withActor(actor, () => + controller.statEntry( + makeReq({ body: { path, return_shares: true }, actor }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ + statusCode: 429, + legacyCode: 'too_many_requests', + }); + + // A plain stat spends only its own budget, so it still admits. + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.statEntry(makeReq({ body: { path }, actor }), res), + ); + expect((captured.body as { name: string }).name).toBe('share-budget'); + }); }); // ── /readdir (readdirEntries) ─────────────────────────────────────── diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index 9f20f1d52..59cac95af 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -47,6 +47,8 @@ import { } from '../../util/concurrency.js'; import { applyInlineContentSecurity } from '../../util/inlineContentSecurity.js'; import { listClientShares } from '../share/clientShare.js'; +import { SHARE_LIST_LIMIT } from '../share/limits.js'; +import { consumeRouteRateLimit } from '../../core/http/middleware/rateLimit.js'; import { PuterController } from '../types.js'; import { STORAGE_OP_COSTS } from '../../services/metering/costs.js'; import { @@ -991,6 +993,17 @@ export class FSController extends PuterController { await this.#assertAccess(actor, entry.path, 'see'); const wantsSize = this.#toBoolean(body.return_size); + const wantsShares = this.#toBoolean(body.return_shares); + // `return_shares` is the share-listing route's work, so it spends from + // the share-listing budget too, not just `fs:stat`'s. + if ( + wantsShares && + !(await consumeRouteRateLimit(req, SHARE_LIST_LIMIT)) + ) { + throw new HttpError(429, 'Too many requests.', { + legacyCode: 'too_many_requests', + }); + } const [subtreeSize, suggestedApps, shareFlags, shares] = await Promise.all([ entry.isDir && wantsSize @@ -998,7 +1011,7 @@ export class FSController extends PuterController { : undefined, this.services.suggestedApps.getSuggestedApps(entry), this.services.share.shareFlags(actor, [entry]), - this.#toBoolean(body.return_shares) + wantsShares ? listClientShares( this.services.share, this.clients.event, @@ -2234,8 +2247,7 @@ export class FSController extends PuterController { // the ActorUser type. Access via the escape hatch until a proper // storage-quota mechanism is in place. const actorUser = req.actor?.user as - | Record - | undefined; + Record | undefined; const candidates = [ this.#toStorageCapacityCandidate(actorUser?.free_storage), diff --git a/src/backend/controllers/fs/LegacyFSController.test.ts b/src/backend/controllers/fs/LegacyFSController.test.ts index bf0b652b8..63e397855 100644 --- a/src/backend/controllers/fs/LegacyFSController.test.ts +++ b/src/backend/controllers/fs/LegacyFSController.test.ts @@ -23,11 +23,13 @@ import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; +import { consumeRouteRateLimit } from '../../core/http/middleware/rateLimit.js'; import { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterServer } from '../../server.js'; import { setupTestServer } from '../../testUtil.js'; import { signFile } from '../../util/fileSigning.js'; import { generateDefaultFsentries } from '../../util/userProvisioning.js'; +import { SHARE_LIST_LIMIT } from '../share/limits.js'; import type { LegacyFSController } from './LegacyFSController.js'; // ── Test harness ──────────────────────────────────────────────────── @@ -426,6 +428,40 @@ describe('LegacyFSController.stat', () => { statusCode: 401, }); }); + + it('draws return_shares from the share-listing budget', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const path = `/${username}/Documents/share-budget`; + await withActor(actor, () => + controller.mkdir(makeReq({ body: { path }, actor }), makeRes().res), + ); + + // Spend the whole share:list bucket, as /share/shares' gate would. + const chargeReq = makeReq({ body: {}, actor }); + for (let i = 0; i < SHARE_LIST_LIMIT.limit; i++) { + await consumeRouteRateLimit(chargeReq, SHARE_LIST_LIMIT); + } + + await expect( + withActor(actor, () => + controller.stat( + makeReq({ body: { path, return_shares: true }, actor }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ + statusCode: 429, + legacyCode: 'too_many_requests', + }); + + // A plain stat spends only its own budget, so it still admits. + const plain = makeRes(); + await withActor(actor, () => + controller.stat(makeReq({ body: { path }, actor }), plain.res), + ); + expect(plain.captured.body).toMatchObject({ name: 'share-budget' }); + }); }); describe('LegacyFSController.delete', () => { diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index a745ab821..519692010 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -50,6 +50,8 @@ import { } from '../../util/hostedAppBacking.js'; import { applyInlineContentSecurity } from '../../util/inlineContentSecurity.js'; import { listClientShares } from '../share/clientShare.js'; +import { SHARE_LIST_LIMIT } from '../share/limits.js'; +import { consumeRouteRateLimit } from '../../core/http/middleware/rateLimit.js'; import { PuterController } from '../types.js'; import { FS_BATCH_CONCURRENT, @@ -487,6 +489,13 @@ export class LegacyFSController extends PuterController { // callers working. `return_owner` is a no-op — `owner` is always set. if (getBoolean(body, 'return_versions')) shaped.versions = []; if (getBoolean(body, 'return_shares')) { + // Share-listing work spends the share-listing budget, not just + // `fs:stat`'s. + if (!(await consumeRouteRateLimit(req, SHARE_LIST_LIMIT))) { + throw new HttpError(429, 'Too many requests.', { + legacyCode: 'too_many_requests', + }); + } shaped.shares = await listClientShares( this.services.share, this.clients.event, @@ -859,9 +868,7 @@ export class LegacyFSController extends PuterController { // Trash, and `null`/`{}` when restoring. See // `src/gui/src/helpers.js` → `window.move_items`. newMetadata: (body.new_metadata ?? undefined) as - | Record - | null - | undefined, + Record | null | undefined, }); const oldPath = source.path; await this.#emitGuiEvent('outer.gui.item.moved', moved, { @@ -1308,8 +1315,7 @@ export class LegacyFSController extends PuterController { } type SignedOrEmpty = - | (SignedFile & { path?: string }) - | Record; + (SignedFile & { path?: string }) | Record; const result: { signatures: SignedOrEmpty[]; token?: string } = { signatures: [], }; @@ -1930,10 +1936,7 @@ export class LegacyFSController extends PuterController { const subjectRef = body.subject; const appRef = body.app; const mode = (getString(body, 'mode') ?? 'read') as - | 'see' - | 'list' - | 'read' - | 'write'; + 'see' | 'list' | 'read' | 'write'; if (!subjectRef || !appRef) throw new HttpError(400, '`subject` and `app` are required', { legacyCode: 'bad_request', diff --git a/src/backend/controllers/share/ShareController.ts b/src/backend/controllers/share/ShareController.ts index e914f1bda..b079fbb43 100644 --- a/src/backend/controllers/share/ShareController.ts +++ b/src/backend/controllers/share/ShareController.ts @@ -31,28 +31,7 @@ import { toClientShare } from './clientShare.js'; import { runWithConcurrencyLimitSettled } from '../../util/concurrency.js'; import { normalizeLimit } from '../../util/pagination.js'; import { PuterController } from '../types.js'; - -/** - * Two windows: a burst ceiling, and a daily one so a slow drip can't add up to - * a mail-merge. Neither bounds _shares_ — one request carries many — which is - * what `ShareService`'s per-day quota is for. - */ -const SHARE_LIMIT = [ - { scope: 'share:mutate', limit: 60, window: 60_000, key: 'user' as const }, - { - scope: 'share:mutate-daily', - limit: 500, - window: 24 * 60 * 60_000, - key: 'user' as const, - }, -]; - -const SHARE_LIST_LIMIT = { - scope: 'share:list', - limit: 600, - window: 60_000, - key: 'user' as const, -}; +import { SHARE_LIMIT, SHARE_LIST_LIMIT } from './limits.js'; /** Distinct (holder, item) pairs run together; see the note on grouping below. */ const SHARE_CONCURRENCY = 8; diff --git a/src/backend/controllers/share/limits.ts b/src/backend/controllers/share/limits.ts new file mode 100644 index 000000000..0f60725d7 --- /dev/null +++ b/src/backend/controllers/share/limits.ts @@ -0,0 +1,51 @@ +/* + * 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 { RouteRateLimit } from '../../core/http/types'; + +// Shared like `fs/limits.ts` is shared: the scope only ties counters +// together if every caller passes the same spec. + +/** + * Two windows: a burst ceiling, and a daily one so a slow drip can't add up to + * a mail-merge. Neither bounds _shares_ — one request carries many — which is + * what `ShareService`'s per-day quota is for. + */ +export const SHARE_LIMIT: RouteRateLimit[] = [ + { scope: 'share:mutate', limit: 60, window: 60_000, key: 'user' }, + { + scope: 'share:mutate-daily', + limit: 500, + window: 24 * 60 * 60_000, + key: 'user', + }, +]; + +/** + * One bucket for every share-listing read. The `/share` routes gate on it, and + * `stat` charges it imperatively when `return_shares` asks for the same listing + * work — otherwise the flag runs share listings under `fs:stat`'s far more + * generous budget. + */ +export const SHARE_LIST_LIMIT: RouteRateLimit = { + scope: 'share:list', + limit: 600, + window: 60_000, + key: 'user', +}; diff --git a/src/backend/core/http/middleware/rateLimit.js b/src/backend/core/http/middleware/rateLimit.js index 94f5426f9..3447cd149 100644 --- a/src/backend/core/http/middleware/rateLimit.js +++ b/src/backend/core/http/middleware/rateLimit.js @@ -709,6 +709,34 @@ export async function checkRateLimit(key, limit, windowMs, backend) { } } +/** + * Charge a route-shaped rate-limit spec from inside a handler. + * + * `rateLimitGate` takes one static spec per route, which is wrong for a route + * whose cost depends on a request parameter: `stat` with `return_shares` does + * the same work as the share-listing route, and should spend from the same + * budget. This resolves the key and the per-subscription limit exactly as the + * gate does — same spec in, same bucket out — so a handler can charge a second + * scope conditionally. Returns true if allowed; fails open on backend error. + */ +export async function consumeRouteRateLimit(req, spec) { + const { + window: windowMs, + key: strategy = 'fingerprint', + scope, + backend, + } = spec; + const backendPair = resolveBackend(backend); + const key = resolveKey(req, scope ?? req.route?.path ?? 'route', strategy); + try { + const limit = await resolveSubscriptionLimit(req, spec); + return await backendPair.rate(key, limit, windowMs); + } catch (err) { + console.error('[rate-limit] handler charge failed, failing open:', err); + return true; + } +} + /** * Read whether `key` still has budget, without spending any. The twin to * `checkRateLimit` for gates whose budget is consumed by an outcome rather than diff --git a/src/backend/core/http/middleware/rateLimit.test.js b/src/backend/core/http/middleware/rateLimit.test.js index eba19f91a..fe2dbfa90 100644 --- a/src/backend/core/http/middleware/rateLimit.test.js +++ b/src/backend/core/http/middleware/rateLimit.test.js @@ -38,6 +38,7 @@ import { checkDriverRateLimit, checkRateLimit, concurrencyGate, + consumeRouteRateLimit, configureRateLimit, listConfiguredRateLimitBackends, peekRateLimit, @@ -1427,6 +1428,77 @@ describe('checkRateLimit', () => { }); }); +describe('consumeRouteRateLimit', () => { + beforeEach(() => configureRateLimit()); + afterEach(() => configureRateLimit()); + + const uniqueScope = () => `charge-${Math.random().toString(36).slice(2, 10)}`; + + it('spends from the same bucket the route gate does', async () => { + // The whole point of the helper: a handler charge and the gate must + // resolve the identical key for the same spec + request. + const opts = { limit: 2, window: 60_000, key: 'user', scope: uniqueScope() }; + const req = { actor: { user: { id: 7 } }, headers: {} }; + + expect(await consumeRouteRateLimit(req, opts)).toBe(true); + + const next = vi.fn(); + await rateLimitGate(opts)(req, {}, next); + expect(next.mock.calls[0][0]).toBeUndefined(); + + // Gate + helper together exhausted the bucket; either side now rejects. + expect(await consumeRouteRateLimit(req, opts)).toBe(false); + const rejectedNext = vi.fn(); + await rateLimitGate(opts)(req, {}, rejectedNext); + expect(isHttpError(rejectedNext.mock.calls[0][0])).toBe(true); + }); + + it("buckets per user under the 'user' strategy", async () => { + const opts = { limit: 1, window: 60_000, key: 'user', scope: uniqueScope() }; + const reqFor = (id) => ({ actor: { user: { id } }, headers: {} }); + expect(await consumeRouteRateLimit(reqFor(1), opts)).toBe(true); + expect(await consumeRouteRateLimit(reqFor(1), opts)).toBe(false); + expect(await consumeRouteRateLimit(reqFor(2), opts)).toBe(true); + }); + + it('applies bySubscription overrides via the wired metering service', async () => { + configureRateLimit({ + metering: { + getActorSubscription: async (actor) => ({ + id: actor.user.uuid === 'free' ? 'user_free' : 'other', + }), + }, + }); + const opts = { + limit: 5, + window: 60_000, + bySubscription: { user_free: 1 }, + key: 'user', + scope: uniqueScope(), + }; + const freeReq = { actor: { user: { id: 1, uuid: 'free' } }, headers: {} }; + expect(await consumeRouteRateLimit(freeReq, opts)).toBe(true); + expect(await consumeRouteRateLimit(freeReq, opts)).toBe(false); + }); + + it('fails open when the backend throws', async () => { + configureRateLimit({ + default: 'redis', + redis: { + multi: () => { + throw new Error('redis exploded'); + }, + }, + }); + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const opts = { limit: 1, window: 60_000, key: 'ip', scope: uniqueScope() }; + const req = { ip: '5.5.5.5', headers: {} }; + expect(await consumeRouteRateLimit(req, opts)).toBe(true); + expect(spy).toHaveBeenCalled(); + spy.mockRestore(); + }); +}); + describe('peekRateLimit', () => { beforeEach(() => configureRateLimit()); afterEach(() => configureRateLimit()); diff --git a/src/docs/src/FS/stat.md b/src/docs/src/FS/stat.md index 5f845c062..d37eb048f 100755 --- a/src/docs/src/FS/stat.md +++ b/src/docs/src/FS/stat.md @@ -39,7 +39,7 @@ A `Promise` that resolves to the [`FSItem`](/Objects/fsitem) object of the speci The item carries `is_shared`: `true` when it has been shared with someone, `false` when it has not, and `null` when the item is not yours — whether someone else's file has other recipients is not yours to see. It covers shares granted by anyone holding `manage` on the item, not only your own, the same way [`getShares()`](/FS/getShares/) does. Only shares **on the item itself** count. A file inside a folder you shared is reachable through that folder without being shared itself, so it reports `false`; `getShares()` is what reports inherited access. -With `returnShares: true`, the result also carries `shares` — an array of the same share objects [`getShares()`](/FS/getShares/) returns, including access inherited from a parent folder and unclaimed invitations. It is empty unless you own the item or hold `manage` on it, so asking for it never fails a `stat()` you were otherwise allowed to make. +With `returnShares: true`, the result also carries `shares` — an array of the same share objects [`getShares()`](/FS/getShares/) returns, including access inherited from a parent folder and unclaimed invitations. It is empty unless you own the item or hold `manage` on it, so asking for it never fails a `stat()` you were otherwise allowed to make. Because it does `getShares()`'s work, it also spends from the [share-read limit](/rate-limits-and-quotas/#sharing) on top of `stat()`'s own. ## Examples diff --git a/src/docs/src/rate-limits-and-quotas.md b/src/docs/src/rate-limits-and-quotas.md index 5ad859edb..db3a0512a 100644 --- a/src/docs/src/rate-limits-and-quotas.md +++ b/src/docs/src/rate-limits-and-quotas.md @@ -166,7 +166,7 @@ Sharing is bounded twice: on the calls, and on how many people one account can r | Recipients per request | 10 | | Items per request | 50 | -The read limit is one bucket shared by every share-listing call, so polling one of them spends budget the others need. +The read limit is one bucket shared by every share-listing call, so polling one of them spends budget the others need. `stat()` with `returnShares` does the same listing work, so it spends from this bucket too, on top of its own `stat` budget. A "new share" is one that gives someone access they didn't already have. Changing the mode on an existing share, or re-sharing an item the recipient already has, costs nothing. Over the daily limit, `share` fails with `share_daily_limit_reached`.