From 273883f657d11063f095a4025b49308c5dfab7c9 Mon Sep 17 00:00:00 2001 From: Denozordec Date: Fri, 6 Nov 2026 22:32:50 +0700 Subject: [PATCH] =?UTF-8?q?fix(api):=20=D0=B7=D0=B0=D0=BA=D1=80=D1=8B?= =?UTF-8?q?=D1=82=D1=8C=20SSRF=20=D0=B2=20rates-proxy,=20=D0=BF=D1=80?= =?UTF-8?q?=D0=BE=D0=B2=D0=B5=D1=80=D1=8F=D1=82=D1=8C=20=D1=80=D0=BE=D0=BB?= =?UTF-8?q?=D1=8C=20=D0=B2=20space,=20=D1=83=D0=B1=D1=80=D0=B0=D1=82=D1=8C?= =?UTF-8?q?=20=D0=B3=D0=BE=D0=BD=D0=BA=D0=B8=20=D1=81=D0=B8=D0=BD=D0=BA?= =?UTF-8?q?=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - rates-proxy запрашивает только сохранённый settings.ratesUrl - viewer в space не может писать в платежи, баланс, провайдеров, аккаунты, проекты и запускать синк - /api/sync/status показывает журнал только текущего space - повторный синк аккаунта во время текущего отвечает 409 - тики планировщика не накладываются, каждый space идёт по своему интервалу Co-Authored-By: Claude Opus 5.5 --- apps/api/src/plugins/space.ts | 30 +++++ apps/api/src/routes/rates-proxy.ts | 12 +- apps/api/src/routes/review-guards.test.ts | 137 ++++++++++++++++++++ apps/api/src/routes/sync.ts | 8 +- apps/api/src/services/providers/sync-job.ts | 26 +++- apps/api/src/services/scheduler.test.ts | 40 ++++++ apps/api/src/services/scheduler.ts | 76 ++++++++--- 7 files changed, 307 insertions(+), 22 deletions(-) create mode 100644 apps/api/src/routes/review-guards.test.ts create mode 100644 apps/api/src/services/scheduler.test.ts diff --git a/apps/api/src/plugins/space.ts b/apps/api/src/plugins/space.ts index 4f5f09c..ee50796 100644 --- a/apps/api/src/plugins/space.ts +++ b/apps/api/src/plugins/space.ts @@ -62,6 +62,26 @@ export async function ensureUserSpaces(request: FastifyRequest): Promise { */ import { runWithSpace } from '@cfdm/db' +const WRITE_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']) +/** Routes without their own requireSpaceRole: a space viewer must not write here. */ +const MEMBER_WRITE_PREFIXES = [ + '/api/payments', + '/api/balance-ledger', + '/api/providers', + '/api/provider-accounts', + '/api/projects', +] + +export function needsMemberRole(method: string, url: string): boolean { + const path = url.split('?')[0] ?? url + // Every /api/sync call hits the hoster and writes (GET .../balance too), except the log. + if (path === '/api/sync' || path.startsWith('/api/sync/')) return path !== '/api/sync/status' + return ( + WRITE_METHODS.has(method.toUpperCase()) && + MEMBER_WRITE_PREFIXES.some((p) => path === p || path.startsWith(`${p}/`)) + ) +} + export const spacePlugin = fp(async (app) => { app.addHook('onRequest', (request, reply, done) => { void (async () => { @@ -123,6 +143,16 @@ export const spacePlugin = fp(async (app) => { } })() }) + + app.addHook('preHandler', async (request, reply) => { + if (!app.authConfig?.required || !request.authUser) return + if (!needsMemberRole(request.method, request.url)) return + if (!canWriteInSpace(request)) { + return reply.code(403).send({ + error: { code: 'FORBIDDEN', message: 'Недостаточно прав в пространстве (нужно: member)' }, + }) + } + }) }) export function requireSpaceRole( diff --git a/apps/api/src/routes/rates-proxy.ts b/apps/api/src/routes/rates-proxy.ts index 010c4ca..d322632 100644 --- a/apps/api/src/routes/rates-proxy.ts +++ b/apps/api/src/routes/rates-proxy.ts @@ -1,13 +1,19 @@ import type { FastifyPluginAsync } from 'fastify' +import { settingsRepository } from '@cfdm/db/repositories/settings' export const ratesProxyRoutes: FastifyPluginAsync = async (app) => { app.get('/api/rates-proxy', async (req, reply) => { + const ratesUrl = settingsRepository.getBySpace()?.ratesUrl?.trim() + if (!ratesUrl) { + return reply.code(400).send({ error: { code: 'VALIDATION', message: 'ratesUrl не задан в настройках' } }) + } + // Only the saved ratesUrl is fetched — an arbitrary ?url= would be an open SSRF proxy. const url = (req.query as { url?: string })?.url - if (!url) { - return reply.code(400).send({ error: { code: 'VALIDATION', message: 'Missing url parameter' } }) + if (url && url.trim() !== ratesUrl) { + return reply.code(400).send({ error: { code: 'VALIDATION', message: 'url должен совпадать с ratesUrl из настроек' } }) } try { - const response = await fetch(url, { signal: AbortSignal.timeout(10_000) }) + const response = await fetch(ratesUrl, { signal: AbortSignal.timeout(10_000) }) if (!response.ok) { return reply.code(502).send({ error: { code: 'UPSTREAM', message: `Upstream returned ${response.status}` } }) } diff --git a/apps/api/src/routes/review-guards.test.ts b/apps/api/src/routes/review-guards.test.ts new file mode 100644 index 0000000..910143e --- /dev/null +++ b/apps/api/src/routes/review-guards.test.ts @@ -0,0 +1,137 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { closeDb, getSqlite } from '@cfdm/db' +import { resetTestDb, seedTestProvider } from '@cfdm/db/test-setup' +import { settingsRepository } from '@cfdm/db/repositories/settings' +import { spacesRepository } from '@cfdm/db/repositories/spaces' +import { buildApp } from '../index.js' +import { needsMemberRole } from '../plugins/space.js' +import { runAccountSync, SyncInProgressError } from '../services/providers/sync-job.js' +import type { ProviderAdapter } from '../services/providers/types.js' + +describe('rates-proxy', () => { + let app: Awaited> + beforeEach(async () => { + resetTestDb() + settingsRepository.upsertForSpace('space-main', { ratesUrl: 'https://rates.test/daily.json' }) + app = await buildApp() + }) + afterEach(async () => { + await app.close() + vi.unstubAllGlobals() + closeDb() + }) + + it('refuses a url other than the saved ratesUrl', async () => { + const fetchMock = vi.fn() + vi.stubGlobal('fetch', fetchMock) + const res = await app.inject({ url: '/api/rates-proxy?url=http://169.254.169.254/latest' }) + expect(res.statusCode).toBe(400) + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('fetches the saved ratesUrl', async () => { + const fetchMock = vi.fn(async () => Response.json({ USD: 1 })) + vi.stubGlobal('fetch', fetchMock) + const res = await app.inject({ + url: `/api/rates-proxy?url=${encodeURIComponent('https://rates.test/daily.json')}`, + }) + expect(res.statusCode).toBe(200) + expect(fetchMock.mock.calls[0]?.[0]).toBe('https://rates.test/daily.json') + }) +}) + +describe('GET /api/sync/status', () => { + beforeEach(() => { + resetTestDb() + seedTestProvider() + }) + afterEach(() => closeDb()) + + it('returns only the current space log', async () => { + const sqlite = getSqlite() + sqlite.exec(`INSERT INTO spaces (id, name, slug, kind, createdAt) VALUES ('space-x', 'X', 'x', 'personal', '2026-01-01')`) + sqlite.exec(`INSERT INTO provider_accounts (id, spaceId, providerId, name) VALUES ('acc-x', 'space-x', 'prov-1', 'X')`) + sqlite.exec(`INSERT INTO sync_log (id, spaceId, accountId, startedAt, status) VALUES ('l-x', 'space-x', 'acc-x', '2026-01-01', 'ok')`) + const app = await buildApp() + const main = await app.inject({ url: '/api/sync/status' }) + expect((main.json() as unknown[]).length).toBe(0) + const x = await app.inject({ url: '/api/sync/status', headers: { 'x-space-id': 'space-x' } }) + expect((x.json() as { id: string }[]).map((r) => r.id)).toEqual(['l-x']) + await app.close() + }) +}) + +describe('runAccountSync lock', () => { + beforeEach(() => { + resetTestDb() + seedTestProvider() + }) + afterEach(() => closeDb()) + + it('rejects a second sync of the same account while one is running', async () => { + getSqlite().exec(`INSERT INTO provider_accounts (id, spaceId, providerId, name) VALUES ('acc-l', 'space-main', 'prov-1', 'L')`) + let finish!: () => void + const adapter = { + syncAccount: () => + new Promise((resolve) => { + finish = () => resolve({ vpsCount: 0, paymentsCount: 0, syncSummary: {} }) + }), + } as unknown as ProviderAdapter + const first = runAccountSync(adapter, { id: 'acc-l' }) + await expect(runAccountSync(adapter, { id: 'acc-l' })).rejects.toBeInstanceOf(SyncInProgressError) + finish() + await first + finish = undefined as never + const again = runAccountSync(adapter, { id: 'acc-l' }) + finish() + await expect(again).resolves.toMatchObject({ ok: true }) + }) +}) + +describe('space viewer cannot write', () => { + const secret = 'test-secret-at-least-8' + beforeEach(() => { + process.env.AUTH_REQUIRED = 'true' + process.env.AUTH_JWT_SECRET = secret + resetTestDb() + }) + afterEach(() => { + delete process.env.AUTH_REQUIRED + delete process.env.AUTH_JWT_SECRET + closeDb() + }) + + it('needsMemberRole covers money/provider writes and sync', () => { + expect(needsMemberRole('POST', '/api/payments')).toBe(true) + expect(needsMemberRole('GET', '/api/payments')).toBe(false) + expect(needsMemberRole('GET', '/api/sync/acc-1/balance')).toBe(true) + expect(needsMemberRole('GET', '/api/sync/status')).toBe(false) + expect(needsMemberRole('DELETE', '/api/provider-accounts/a')).toBe(true) + }) + + it('403 for viewer, 201 for member on POST /api/payments', async () => { + spacesRepository.create({ id: 'space-team', name: 'Team', slug: 'team', kind: 'personal', ownerUserId: 'owner' }) + spacesRepository.addMember('space-team', 'viewer-1', 'viewer') + spacesRepository.addMember('space-team', 'member-1', 'member') + const app = await buildApp() + const token = (sub: string) => + app.jwt.sign({ + sub, + email: `${sub}@t`, + name: sub, + apps: ['vps'], + permissions: ['vps:payments:write'], + iss: app.authConfig.issuer, + }) + const post = (sub: string) => + app.inject({ + method: 'POST', + url: '/api/payments', + headers: { authorization: `Bearer ${token(sub)}`, 'x-space-id': 'space-team' }, + payload: { type: 'topup', date: '2026-01-01', amount: 1 }, + }) + expect((await post('viewer-1')).statusCode).toBe(403) + expect((await post('member-1')).statusCode).toBe(201) + await app.close() + }) +}) diff --git a/apps/api/src/routes/sync.ts b/apps/api/src/routes/sync.ts index 89b0e3c..905f680 100644 --- a/apps/api/src/routes/sync.ts +++ b/apps/api/src/routes/sync.ts @@ -1,6 +1,6 @@ import type { FastifyPluginAsync } from 'fastify' import { desc, eq } from 'drizzle-orm' -import { getDb, schema } from '@cfdm/db' +import { getCurrentSpaceId, getDb, schema } from '@cfdm/db' import { providerAccountsRepository } from '@cfdm/db/repositories/provider-accounts' import { providersRepository } from '@cfdm/db/repositories/providers' @@ -9,7 +9,7 @@ import { resolveSyncAccount, SYNC_SETUP_ERRORS, } from '../services/providers/index.js' -import { runAccountSync } from '../services/providers/sync-job.js' +import { runAccountSync, SyncInProgressError } from '../services/providers/sync-job.js' interface SyncLogRow { id: string @@ -54,6 +54,7 @@ export const syncRoutes: FastifyPluginAsync = async (app) => { const rows = getDb() .select() .from(schema.syncLog) + .where(eq(schema.syncLog.spaceId, getCurrentSpaceId())) .orderBy(desc(schema.syncLog.startedAt)) .limit(50) .all() @@ -89,6 +90,9 @@ export const syncRoutes: FastifyPluginAsync = async (app) => { }, } } catch (err) { + if (err instanceof SyncInProgressError) { + return reply.code(409).send({ ok: false, error: err.message }) + } req.log.error(err) const message = err instanceof Error ? err.message : 'Sync failed' return reply.code(500).send({ ok: false, error: message }) diff --git a/apps/api/src/services/providers/sync-job.ts b/apps/api/src/services/providers/sync-job.ts index 47681fe..7e17150 100644 --- a/apps/api/src/services/providers/sync-job.ts +++ b/apps/api/src/services/providers/sync-job.ts @@ -8,13 +8,37 @@ export interface RunAccountSyncResult extends SyncResult { logId: string } +/** Manual and scheduled syncs of one account must not interleave (duplicate inserts, tariff wipe). */ +const inFlightAccounts = new Set() + +export class SyncInProgressError extends Error { + constructor() { + super('Синхронизация этого аккаунта уже выполняется') + } +} + export async function runAccountSync( adapter: ProviderAdapter, account: unknown, opts: { skipTariffs?: boolean; skipVpsPayments?: boolean } = {}, ): Promise { - const db = getDb() const accountRow = account as { id: string; spaceId?: string } + if (inFlightAccounts.has(accountRow.id)) throw new SyncInProgressError() + inFlightAccounts.add(accountRow.id) + try { + return await runAccountSyncLocked(adapter, account, accountRow, opts) + } finally { + inFlightAccounts.delete(accountRow.id) + } +} + +async function runAccountSyncLocked( + adapter: ProviderAdapter, + account: unknown, + accountRow: { id: string; spaceId?: string }, + opts: { skipTariffs?: boolean; skipVpsPayments?: boolean }, +): Promise { + const db = getDb() const logId = `sync-${accountRow.id}-${Date.now()}` const spaceId = accountRow.spaceId ?? getCurrentSpaceId() diff --git a/apps/api/src/services/scheduler.test.ts b/apps/api/src/services/scheduler.test.ts new file mode 100644 index 0000000..a614ec6 --- /dev/null +++ b/apps/api/src/services/scheduler.test.ts @@ -0,0 +1,40 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { closeDb } from '@cfdm/db' +import { resetTestDb } from '@cfdm/db/test-setup' + +vi.mock('./uptime-check.js', () => ({ runVpsUptimeChecks: vi.fn() })) + +import { runVpsUptimeChecks } from './uptime-check.js' +import { resetSchedulerState, runScheduledUptimeChecks } from './scheduler.js' + +const uptimeMock = vi.mocked(runVpsUptimeChecks) + +describe('scheduler guards', () => { + beforeEach(() => { + resetTestDb() + resetSchedulerState() + uptimeMock.mockReset() + }) + afterEach(() => closeDb()) + + it('skips a tick while the previous one runs and respects the space interval', async () => { + let finish!: () => void + uptimeMock.mockImplementation( + () => new Promise((resolve) => (finish = () => resolve({ checked: 0, down: 0, newlyDown: [], newlyUp: [] }))), + ) + const first = runScheduledUptimeChecks() + await runScheduledUptimeChecks() // overlapping tick → skipped + await vi.waitFor(() => expect(finish).toBeTypeOf('function')) + finish() + await first + expect(uptimeMock).toHaveBeenCalledTimes(1) + + uptimeMock.mockResolvedValue({ checked: 0, down: 0, newlyDown: [], newlyUp: [] }) + await runScheduledUptimeChecks() // space interval (5 min) not elapsed + expect(uptimeMock).toHaveBeenCalledTimes(1) + + resetSchedulerState() + await runScheduledUptimeChecks() + expect(uptimeMock).toHaveBeenCalledTimes(2) + }) +}) diff --git a/apps/api/src/services/scheduler.ts b/apps/api/src/services/scheduler.ts index 32679c3..58f719a 100644 --- a/apps/api/src/services/scheduler.ts +++ b/apps/api/src/services/scheduler.ts @@ -63,9 +63,57 @@ function allSettings(): SettingsRow[] { return settingsRepository.listAllSpaces() } -export async function runNotificationTick(): Promise { +function spaceIntervals(s: SettingsRow) { + return { + notify: Math.max(15, Number(s.notifyIntervalMinutes) || 60), + uptime: Math.max(1, Number(s.uptimeCheckIntervalMinutes) || 5), + sync: Math.max(15, Number(s.syncIntervalMinutes) || 60), + tariffs: Math.max(60, Number(s.syncTariffsIntervalMinutes) || 1440), + } +} + +const runningJobs = new Set() + +/** A tick that finds the previous run of the same job still going is skipped, not stacked. */ +async function exclusive(job: string, fn: () => Promise): Promise { + if (runningJobs.has(job)) { + console.warn(`Scheduler: ${job} ещё выполняется — тик пропущен`) + return + } + runningJobs.add(job) + try { + await fn() + } finally { + runningJobs.delete(job) + } +} + +const lastRunAt = new Map() + +/** Timers tick at the smallest interval of all spaces; each space still runs at its own. */ +function dueForSpace(job: string, spaceId: string, minutes: number): boolean { + const key = `${job}:${spaceId}` + const now = Date.now() + const last = lastRunAt.get(key) + if (last != null && now - last < minutes * 60_000 - 30_000) return false + lastRunAt.set(key, now) + return true +} + +export function resetSchedulerState(): void { + runningJobs.clear() + lastRunAt.clear() +} + +export const runNotificationTick = () => exclusive('notify', notificationTick) +export const runScheduledSync = () => exclusive('sync', scheduledSync) +export const runScheduledSyncTariffs = () => exclusive('tariffs', scheduledSyncTariffs) +export const runScheduledUptimeChecks = () => exclusive('uptime', scheduledUptimeChecks) + +async function notificationTick(): Promise { for (const settings of allSettings()) { const spaceId = settings.spaceId || MAIN_SPACE_ID + if (!dueForSpace('notify', spaceId, spaceIntervals(settings).notify)) continue try { await runWithSpaceAsync(spaceId, async () => { const payload = buildPaymentExpiryNotification() @@ -80,10 +128,11 @@ export async function runNotificationTick(): Promise { } } -export async function runScheduledSync(): Promise { +async function scheduledSync(): Promise { for (const settings of allSettings()) { if (!settings.syncEnabled) continue const spaceId = settings.spaceId || MAIN_SPACE_ID + if (!dueForSpace('sync', spaceId, spaceIntervals(settings).sync)) continue try { await runWithSpaceAsync(spaceId, async () => { const entries = getSyncableAccounts(spaceId) @@ -134,10 +183,11 @@ export async function runScheduledSync(): Promise { } } -export async function runScheduledSyncTariffs(): Promise { +async function scheduledSyncTariffs(): Promise { for (const settings of allSettings()) { if (!settings.syncEnabled) continue const spaceId = settings.spaceId || MAIN_SPACE_ID + if (!dueForSpace('tariffs', spaceId, spaceIntervals(settings).tariffs)) continue try { await runWithSpaceAsync(spaceId, async () => { const entries = getSyncableAccounts(spaceId) @@ -178,9 +228,10 @@ export async function runScheduledSyncTariffs(): Promise { } } -export async function runScheduledUptimeChecks(): Promise { +async function scheduledUptimeChecks(): Promise { for (const settings of allSettings()) { const spaceId = settings.spaceId || MAIN_SPACE_ID + if (!dueForSpace('uptime', spaceId, spaceIntervals(settings).uptime)) continue try { await runWithSpaceAsync(spaceId, async () => { const { newlyDown, newlyUp } = await runVpsUptimeChecks() @@ -261,19 +312,12 @@ function pickSchedulerIntervals(rows: SettingsRow[]): { let tariffsInterval: number | null = null for (const s of rows) { - notifyInterval = Math.min( - notifyInterval, - Math.max(15, Number(s.notifyIntervalMinutes) || 60), - ) - uptimeInterval = Math.min( - uptimeInterval, - Math.max(1, Number(s.uptimeCheckIntervalMinutes) || 5), - ) + const i = spaceIntervals(s) + notifyInterval = Math.min(notifyInterval, i.notify) + uptimeInterval = Math.min(uptimeInterval, i.uptime) if (s.syncEnabled) { - const si = Math.max(15, Number(s.syncIntervalMinutes) || 60) - const ti = Math.max(60, Number(s.syncTariffsIntervalMinutes) || 1440) - syncInterval = syncInterval == null ? si : Math.min(syncInterval, si) - tariffsInterval = tariffsInterval == null ? ti : Math.min(tariffsInterval, ti) + syncInterval = syncInterval == null ? i.sync : Math.min(syncInterval, i.sync) + tariffsInterval = tariffsInterval == null ? i.tariffs : Math.min(tariffsInterval, i.tariffs) } } return { notifyInterval, uptimeInterval, syncInterval, tariffsInterval }