diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1a7212d758..262cd00f6d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,6 +29,7 @@ Every PR must include: 2. **Environment/Infrastructure updates** (config, environment variables) 3. **Service updates** (if DTOs/interfaces changed) 4. **Frontend synchronization** (if API contracts changed) +5. **Cron job inventory** (if a `@DfxCron` job was added, removed or re-scheduled) — [docs/cron-jobs.md](docs/cron-jobs.md) Missing any of these = changes requested. @@ -530,6 +531,12 @@ async processPayments(): Promise { } ``` +Declare a `process` flag unless the job maintains the disabled set itself. Without one the job +runs unconditionally and cannot be switched off without a deploy. + +[docs/cron-jobs.md](docs/cron-jobs.md) lists every scheduled job with its interval and flag. +**Adding, removing or re-scheduling a job must be reflected there in the same PR.** + Prefer longer intervals (15min) over aggressive polling (1min). Only use short intervals when truly needed. ### Await Discipline diff --git a/docs/cron-jobs.md b/docs/cron-jobs.md new file mode 100644 index 0000000000..ccb451d1ed --- /dev/null +++ b/docs/cron-jobs.md @@ -0,0 +1,249 @@ +# Cron jobs + +Every scheduled job this service runs: **131 `@DfxCron` declarations** across 92 files and 33 areas. + +## Columns + +| Column | Meaning | +| ------ | ------- | +| **Interval** | The `CronExpression` / `CustomCronExpression` the job is registered with | +| **Flag** | The `process:` kill switch that disables the job at runtime. `—` means the job has none and always runs | +| **Job** | Class and method | +| **File** | Path below `src/` | + +## Flags + +110 of the 131 jobs carry a `process` flag, 21 do not. A job with a flag can be switched off +without a deploy — `DfxCronService` skips it when the process appears in the disabled set, which +`ProcessService` refreshes from the `disabledProcesses` setting and the `DISABLED_PROCESSES` +environment variable every 30 seconds. + +A job **without** a flag runs unconditionally. That is deliberate for three of them — the +`ProcessService::resync*` jobs maintain the disabled set and the JWT denylists themselves, so +making them switchable would let a configuration change disable the mechanism that reads +configuration changes. For the remaining 18 it is simply an omission: + +| Job | Interval | +| --- | --- | +| `DexService::finalizePurchaseOrders` | 30 seconds | +| `ExchangeController::checkTrades` | 30 seconds | +| `AuthService::checkLists` | minute | +| `JwtRevocationSyncService::syncDeniedJwtAccounts` | minute | +| `TransactionController::checkLists` | minute | +| `UserDataService::processCleanupMailSecretCache` | minute | +| `TransactionHelper::updateCache` | 5 minutes | +| `RefService::checkRefs` | hour | +| `BuyService` / `SellService` / `SwapService` / `UserService` / `UserDataService` `::resetMonthlyVolumes` | 1st of month | +| `BuyService` / `SellService` / `SwapService` / `UserService` / `UserDataService` `::resetAnnualVolumes` | year | + +New jobs should declare a flag unless there is a reason like the one above. + +## Distribution + +| Interval | Jobs | +| -------- | ---: | +| second | 5 | +| 10 seconds | 3 | +| 30 seconds | 8 | +| minute | 49 | +| 5 minutes | 17 | +| 10 minutes | 15 | +| hour | 16 | +| day at 4am | 3 | +| day at 5am | 1 | +| day at 6am | 1 | +| day at 11pm | 1 | +| week | 1 | +| weekend | 1 | +| 1st day of month at midnight | 5 | +| year | 5 | + +Jobs by area: + +| Area | Jobs | Without flag | +| ---- | ---: | -----------: | +| `subdomains/generic/user` | 15 | 7 | +| `subdomains/core/monitoring` | 14 | — | +| `subdomains/core/accounting` | 13 | — | +| `subdomains/supporting/payin` | 12 | — | +| `integration/blockchain` | 6 | — | +| `subdomains/core/buy-crypto` | 6 | 4 | +| `subdomains/core/sell-crypto` | 5 | 2 | +| `subdomains/core/payment-link` | 4 | — | +| `subdomains/generic/kyc` | 4 | — | +| `subdomains/supporting/bank-tx` | 4 | — | +| `subdomains/supporting/bank` | 4 | — | +| `subdomains/supporting/fiat-output` | 4 | — | +| `subdomains/supporting/support-issue` | 4 | — | +| `shared` | 3 | 3 | +| `subdomains/core/liquidity-management` | 3 | — | +| `subdomains/core/referral` | 3 | 1 | +| `subdomains/core/trading` | 3 | — | +| `subdomains/supporting/payment` | 3 | 1 | +| `subdomains/supporting/pricing` | 3 | — | +| `integration/exchange` | 2 | 1 | +| `subdomains/core/custody` | 2 | — | +| `subdomains/supporting/log` | 2 | — | +| `subdomains/supporting/realunit` | 2 | — | +| `integration/binance-pay` | 1 | — | +| `subdomains/core/aml` | 1 | — | +| `subdomains/core/faucet-request` | 1 | — | +| `subdomains/core/history` | 1 | 1 | +| `subdomains/core/statistic` | 1 | — | +| `subdomains/generic/admin` | 1 | — | +| `subdomains/supporting/dex` | 1 | 1 | +| `subdomains/supporting/fiat-payin` | 1 | — | +| `subdomains/supporting/notification` | 1 | — | +| `subdomains/supporting/payout` | 1 | — | + +## How this list is produced + +Every `@DfxCron(` occurrence in `src/**/*.ts`. Decorator arguments are read by a balanced-paren +scan, so multi-line declarations are included — a line-based match misses four of them. The parsed +count is asserted against a raw text count of the decorator: **131 = 131**, no gap. Class and +method come from the enclosing `export class` (including `export abstract class`) and the +identifier following the decorator. + +## Known discrepancy + +`CitreaBaseStrategy::checkPayInEntries` is declared on an **abstract** class. NestJS discovers +providers rather than classes, so such a job is registered once per concrete subclass, not once per +declaration. There is currently one subclass, so the runtime count equals the declaration count — +but a second subclass would silently add another registration of the same job, sharing the flag and +the interval while running as an independent timer with its own lock. + +## Jobs + +| Interval | Flag | Job | File | +| -------- | ---- | --- | ---- | +| second | `PAY_IN` | `BitcoinStrategy::checkPayInEntries` | `subdomains/supporting/payin/strategies/register/impl/bitcoin.strategy.ts` | +| second | `PAY_IN` | `FiroStrategy::checkPayInEntries` | `subdomains/supporting/payin/strategies/register/impl/firo.strategy.ts` | +| second | `PAY_IN` | `MoneroStrategy::checkPayInEntries` | `subdomains/supporting/payin/strategies/register/impl/monero.strategy.ts` | +| second | `MONITOR_CONNECTION_POOL` | `MonitorConnectionPoolService::monitorConnectionPool` | `subdomains/core/monitoring/monitor-connection-pool.service.ts` | +| second | `PAY_IN` | `ZanoStrategy::checkPayInEntries` | `subdomains/supporting/payin/strategies/register/impl/zano.strategy.ts` | +| 10 seconds | `LIQUIDITY_MANAGEMENT` | `LiquidityManagementPipelineService::processPipelines` | `subdomains/core/liquidity-management/services/liquidity-management-pipeline.service.ts` | +| 10 seconds | `MONITOR_CONNECTION_POOL` | `MonitorConnectionPoolService::monitorConnectionPoolStatic` | `subdomains/core/monitoring/monitor-connection-pool.service.ts` | +| 10 seconds | `MONITOR_EVENT_LOOP` | `MonitorEventLoopService::monitorEventLoop` | `subdomains/core/monitoring/monitor-event-loop.service.ts` | +| 30 seconds | `LNURL_AUTH_CACHE` | `AuthLnUrlService::processCleanupAccessToken` | `subdomains/generic/user/models/auth/auth-lnurl.service.ts` | +| 30 seconds | `BANK_TX` | `BankTxService::checkBankTx` | `subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts` | +| 30 seconds | — | `DexService::finalizePurchaseOrders` | `subdomains/supporting/dex/services/dex.service.ts` | +| 30 seconds | — | `ExchangeController::checkTrades` | `integration/exchange/controllers/exchange.controller.ts` | +| 30 seconds | `PAY_OUT` | `PayoutService::processOrders` | `subdomains/supporting/payout/services/payout.service.ts` | +| 30 seconds | — | `ProcessService::resyncDeniedJwtAccounts` | `shared/services/process.service.ts` | +| 30 seconds | — | `ProcessService::resyncDeniedJwtAddresses` | `shared/services/process.service.ts` | +| 30 seconds | — | `ProcessService::resyncDisabledProcesses` | `shared/services/process.service.ts` | +| minute | `PAY_OUT` | `AdminService::completeLiquidityOrders` | `subdomains/generic/admin/admin.service.ts` | +| minute | `MONITORING` | `AmlObserver::fetch` | `subdomains/core/monitoring/observers/aml.observer.ts` | +| minute | — | `AuthService::checkLists` | `subdomains/generic/user/models/auth/auth.service.ts` | +| minute | `BANK_DATA_VERIFICATION` | `BankDataService::checkAndSetActive` | `subdomains/generic/user/models/bank-data/bank-data.service.ts` | +| minute | `MONITORING` | `BankObserver::fetch` | `subdomains/core/monitoring/observers/bank.observer.ts` | +| minute | `BANK_TX_RETURN_MAIL` | `BankTxReturnNotificationService::sendBankTxReturnMail` | `subdomains/supporting/bank-tx/bank-tx-return/bank-tx-return-notification.service.ts` | +| minute | `BUY_CRYPTO` | `BuyCryptoJobService::process` | `subdomains/core/buy-crypto/process/services/buy-crypto-job.service.ts` | +| minute | `BUY_FIAT` | `BuyFiatJobService::addFiatOutputs` | `subdomains/core/sell-crypto/process/services/buy-fiat-job.service.ts` | +| minute | `BUY_FIAT` | `BuyFiatJobService::checkCryptoPayIn` | `subdomains/core/sell-crypto/process/services/buy-fiat-job.service.ts` | +| minute | `BUY_FIAT_MAIL` | `BuyFiatNotificationService::sendNotificationMails` | `subdomains/core/sell-crypto/process/services/buy-fiat-notification.service.ts` | +| minute | `PAY_IN` | `CardanoStrategy::checkPayInEntries` | `subdomains/supporting/payin/strategies/register/impl/cardano.strategy.ts` | +| minute | `MONITORING` | `CheckoutObserver::fetch` | `subdomains/core/monitoring/observers/checkout.observer.ts` | +| minute | `PAY_IN` | `CitreaBaseStrategy::checkPayInEntries` | `subdomains/supporting/payin/strategies/register/impl/base/citrea.strategy.ts` | +| minute | `CUSTODY` | `CustodyJobService::handleOrders` | `subdomains/core/custody/services/custody-job.service.ts` | +| minute | `FIAT_OUTPUT` | `FiatOutputJobService::fillFiatOutput` | `subdomains/supporting/fiat-output/fiat-output-job.service.ts` | +| minute | `FIAT_PAY_IN` | `FiatPayInSyncService::syncCheckout` | `subdomains/supporting/fiat-payin/services/fiat-payin-sync.service.ts` | +| minute | `PAY_IN` | `InternetComputerStrategy::checkPayInEntries` | `subdomains/supporting/payin/strategies/register/impl/icp.strategy.ts` | +| minute | — | `JwtRevocationSyncService::syncDeniedJwtAccounts` | `subdomains/generic/user/models/user-data/jwt-revocation-sync.service.ts` | +| minute | `KYC` | `KycService::reviewKycSteps` | `subdomains/generic/kyc/services/kyc.service.ts` | +| minute | `LEDGER_BOOKING_BANK_TX` | `LedgerBookingJobService::runBankTx` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_BUY_CRYPTO` | `LedgerBookingJobService::runBuyCrypto` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_BUY_FIAT` | `LedgerBookingJobService::runBuyFiat` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_CRYPTO_INPUT` | `LedgerBookingJobService::runCryptoInput` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_EXCHANGE_TX` | `LedgerBookingJobService::runExchangeTx` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_LIQUIDITY_MANAGEMENT` | `LedgerBookingJobService::runLiquidityMgmt` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_LIQUIDITY_ORDER` | `LedgerBookingJobService::runLiquidityOrderDex` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_PAYOUT` | `LedgerBookingJobService::runPayoutOrder` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LEDGER_BOOKING_TRADING_ORDER` | `LedgerBookingJobService::runTradingOrder` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| minute | `LIQUIDITY_MANAGEMENT_CHECK_BALANCES` | `LiquidityManagementService::checkLiquidityBalances` | `subdomains/core/liquidity-management/services/liquidity-management.service.ts` | +| minute | `MONITORING` | `LiquidityObserver::fetch` | `subdomains/core/monitoring/observers/liquidity.observer.ts` | +| minute | `TRADING_LOG` | `LogJobService::saveTradingLog` | `subdomains/supporting/log/log-job.service.ts` | +| minute | `MONITORING` | `NodeHealthObserver::fetch` | `subdomains/core/monitoring/observers/node-health.observer.ts` | +| minute | `ORGANIZATION_SYNC` | `OrganizationService::syncOrganization` | `subdomains/generic/user/models/organization/organization.service.ts` | +| minute | `PAY_IN` | `PayInService::checkConfirmations` | `subdomains/supporting/payin/services/payin.service.ts` | +| minute | `PAY_IN` | `PayInService::forwardPayInEntries` | `subdomains/supporting/payin/services/payin.service.ts` | +| minute | `PAY_IN` | `PayInService::returnPayInEntries` | `subdomains/supporting/payin/services/payin.service.ts` | +| minute | `PAYMENT_CONFIRMATIONS` | `PaymentCronService::checkTxConfirmations` | `subdomains/core/payment-link/services/payment-cron.service.ts` | +| minute | `PAYMENT_EXPIRATION` | `PaymentCronService::processExpiredPayments` | `subdomains/core/payment-link/services/payment-cron.service.ts` | +| minute | `UPDATE_BLOCKCHAIN_FEE` | `PaymentLinkFeeService::updateFees` | `subdomains/core/payment-link/services/payment-link-fee.service.ts` | +| minute | `REALUNIT_QUOTE_COMPLETION` | `RealUnitJobService::completeSettledQuotes` | `subdomains/supporting/realunit/realunit-job.service.ts` | +| minute | `SUPPORT_BOT` | `SupportIssueJobService::sendAutoResponses` | `subdomains/supporting/support-issue/services/support-issue-job.service.ts` | +| minute | `TFA_CACHE` | `TfaService::processCleanupSecretCache` | `subdomains/generic/kyc/services/tfa.service.ts` | +| minute | `TRADING` | `TradingJobService::processOrders` | `subdomains/core/trading/services/trading-job.service.ts` | +| minute | `TRADING` | `TradingJobService::processRules` | `subdomains/core/trading/services/trading-job.service.ts` | +| minute | — | `TransactionController::checkLists` | `subdomains/core/history/controllers/transaction.controller.ts` | +| minute | `TX_MAIL` | `TransactionNotificationService::sendNotificationMails` | `subdomains/supporting/payment/services/transaction-notification.service.ts` | +| minute | `USER_DATA` | `UserDataJobService::fillUserData` | `subdomains/generic/user/models/user-data/user-data-job.service.ts` | +| minute | — | `UserDataService::processCleanupMailSecretCache` | `subdomains/generic/user/models/user-data/user-data.service.ts` | +| minute | `USER` | `UserJobService::fillUser` | `subdomains/generic/user/models/user/user-job.service.ts` | +| 5 minutes | `PRICING` | `AssetPricesJobService::updatePaymentPrices` | `subdomains/supporting/pricing/services/asset-prices-job.service.ts` | +| 5 minutes | `LNURL_AUTH_CACHE` | `AuthLnUrlService::processCleanupAuthCache` | `subdomains/generic/user/models/auth/auth-lnurl.service.ts` | +| 5 minutes | `BANK_TX_RETURN` | `BankTxReturnService::fillBankTxReturn` | `subdomains/supporting/bank-tx/bank-tx-return/bank-tx-return.service.ts` | +| 5 minutes | `BANK_TX` | `BankTxService::enrichYapealTransactions` | `subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts` | +| 5 minutes | `BLOCKCHAIN_CONFIG_CHECK` | `BlockchainConfigCheckService::logUnconfiguredClients` | `integration/blockchain/shared/services/blockchain-config-check.service.ts` | +| 5 minutes | `EXCHANGE_TX_SYNC` | `ExchangeTxService::syncExchangeJob` | `integration/exchange/services/exchange-tx.service.ts` | +| 5 minutes | `CRYPTO_PAYOUT` | `FaucetRequestService::checkFaucetRequests` | `subdomains/core/faucet-request/services/faucet-request.service.ts` | +| 5 minutes | `LEDGER_COA_BOOTSTRAP` | `LedgerBookingJobService::runCoaBootstrap` | `subdomains/core/accounting/services/ledger-booking-job.service.ts` | +| 5 minutes | `LEDGER_CUTOVER` | `LedgerCutoverService::run` | `subdomains/core/accounting/services/ledger-cutover.service.ts` | +| 5 minutes | `LIMIT_REQUEST_MAIL` | `LimitRequestNotificationService::sendNotificationMails` | `subdomains/supporting/support-issue/services/limit-request-notification.service.ts` | +| 5 minutes | `LIQUIDITY_MANAGEMENT` | `LiquidityManagementRuleService::reactivateRules` | `subdomains/core/liquidity-management/services/liquidity-management-rule.service.ts` | +| 5 minutes | `PAY_IN_MAIL` | `PayInNotificationService::sendNotificationMails` | `subdomains/supporting/payin/services/payin-notification.service.ts` | +| 5 minutes | `REALUNIT_TRANSFER_RECONCILIATION` | `RealUnitJobService::reconcilePendingTransfers` | `subdomains/supporting/realunit/realunit-job.service.ts` | +| 5 minutes | `SUPPORT_BOT` | `SupportEscalationService::checkEscalations` | `subdomains/supporting/support-issue/services/support-escalation.service.ts` | +| 5 minutes | `TRADING` | `TradingJobService::reactivateRules` | `subdomains/core/trading/services/trading-job.service.ts` | +| 5 minutes | — | `TransactionHelper::updateCache` | `subdomains/supporting/payment/services/transaction-helper.ts` | +| 5 minutes | `WEBHOOK` | `WebhookNotificationService::sendWebhooks` | `subdomains/generic/user/services/webhook/webhook-notification.service.ts` | +| 10 minutes | `BANK_ACCOUNT` | `BankAccountService::reloadUncheckedBankAccounts` | `subdomains/supporting/bank/bank-account/bank-account.service.ts` | +| 10 minutes | `DEURO_LOG_INFO` | `DEuroService::processLogInfo` | `integration/blockchain/deuro/deuro.service.ts` | +| 10 minutes | `MONITORING` | `ExchangeObserver::fetch` | `subdomains/core/monitoring/observers/exchange.observer.ts` | +| 10 minutes | `MONITORING` | `ExternalServicesObserver::fetch` | `subdomains/core/monitoring/observers/external-services.observer.ts` | +| 10 minutes | `BLOCKCHAIN_FEE_UPDATE` | `FeeService::updateBlockchainFees` | `subdomains/supporting/payment/services/fee.service.ts` | +| 10 minutes | `FRANKENCOIN_LOG_INFO` | `FrankencoinService::processLogInfo` | `integration/blockchain/frankencoin/frankencoin.service.ts` | +| 10 minutes | `JUICE_LOG_INFO` | `JuiceService::processLogInfo` | `integration/blockchain/juice/juice.service.ts` | +| 10 minutes | `MONITORING` | `NodeBalanceObserver::fetch` | `subdomains/core/monitoring/observers/node-balance.observer.ts` | +| 10 minutes | `MAIL_RETRY` | `NotificationJobService::resendUncompletedMails` | `subdomains/supporting/notification/services/notification-job.service.ts` | +| 10 minutes | `PAY_IN` | `PayInService::updateFailedPayments` | `subdomains/supporting/payin/services/payin.service.ts` | +| 10 minutes | `MONITORING` | `PaymentObserver::fetch` | `subdomains/core/monitoring/observers/payment.observer.ts` | +| 10 minutes | `MONITORING` | `RealUnitW2wGasObserver::fetch` | `subdomains/core/monitoring/observers/realunit-w2w-gas.observer.ts` | +| 10 minutes | `REF_PAYOUT` | `RefRewardJobService::processPendingRefRewards` | `subdomains/core/referral/reward/services/ref-reward-job.service.ts` | +| 10 minutes | `MONITORING` | `UserObserver::fetch` | `subdomains/core/monitoring/observers/user.observer.ts` | +| 10 minutes | `ZANO_ASSET_WHITELIST` | `ZanoService::setupAssetWhitelist` | `integration/blockchain/zano/services/zano.service.ts` | +| hour | `PRICING` | `AssetPricesJobService::updatePrices` | `subdomains/supporting/pricing/services/asset-prices-job.service.ts` | +| hour | `BANK_ACCOUNT` | `BankAccountService::reloadErrorBankAccounts` | `subdomains/supporting/bank/bank-account/bank-account.service.ts` | +| hour | `BINANCE_PAY_CERTIFICATES_UPDATE` | `BinancePayService::updateCertificates` | `integration/binance-pay/services/binance-pay.service.ts` | +| hour | `BUY_CRYPTO_AGGREGATION` | `BuyCryptoJobService::checkAggregatingTransactions` | `subdomains/core/buy-crypto/process/services/buy-crypto-job.service.ts` | +| hour | `ASSET_DECIMALS` | `EvmDecimalsService::setDecimals` | `integration/blockchain/shared/evm/evm-decimals.service.ts` | +| hour | `FIAT_OUTPUT` | `FiatOutputFrickService::checkFrickOrderStatus` | `subdomains/supporting/fiat-output/fiat-output-frick.service.ts` | +| hour | `FIAT_OUTPUT` | `FiatOutputJobService::checkOlkypayOrderStatus` | `subdomains/supporting/fiat-output/fiat-output-job.service.ts` | +| hour | `FIAT_OUTPUT` | `FiatOutputJobService::generateReports` | `subdomains/supporting/fiat-output/fiat-output-job.service.ts` | +| hour | `PRICING` | `FiatPricesService::updatePrices` | `subdomains/supporting/pricing/services/fiat-prices.service.ts` | +| hour | `KYC_MAIL` | `KycNotificationService::sendNotificationMails` | `subdomains/generic/kyc/services/kyc-notification.service.ts` | +| hour | `PAYMENT_FORWARDING` | `PaymentCronService::forwardDeposits` | `subdomains/core/payment-link/services/payment-cron.service.ts` | +| hour | — | `RefService::checkRefs` | `subdomains/core/referral/process/ref.service.ts` | +| hour | `UPDATE_STATISTIC` | `StatisticService::doUpdate` | `subdomains/core/statistic/statistic.service.ts` | +| hour | `SUPPORT_BOT` | `SupportIssueJobService::autoOnHold` | `subdomains/supporting/support-issue/services/support-issue-job.service.ts` | +| hour | `BLACK_SQUAD_MAIL` | `UserDataNotificationService::sendNotificationMails` | `subdomains/generic/user/models/user-data/user-data-notification.service.ts` | +| hour | `VIRTUAL_IBAN_FRICK_ISSUANCE_RECONCILIATION` | `VirtualIbanFrickIssuanceReconciliationService::reconcileRetiredIssuanceReferences` | `subdomains/supporting/bank/virtual-iban/virtual-iban-frick-issuance-reconciliation.service.ts` | +| day at 4am | `CUSTODY` | `CustodyJobService::resetExpiredConfirmedOrders` | `subdomains/core/custody/services/custody-job.service.ts` | +| day at 4am | `KYC` | `KycService::checkIdentSteps` | `subdomains/generic/kyc/services/kyc.service.ts` | +| day at 4am | `LEDGER_MARK_TO_MARKET` | `LedgerMarkToMarketService::run` | `subdomains/core/accounting/services/ledger-mark-to-market.service.ts` | +| day at 5am | `LEDGER_RECONCILIATION` | `LedgerReconciliationService::run` | `subdomains/core/accounting/services/ledger-reconciliation.service.ts` | +| day at 6am | `REF_PAYOUT` | `RefRewardJobService::createPendingRefRewards` | `subdomains/core/referral/reward/services/ref-reward-job.service.ts` | +| day at 11pm | `LOG_CLEANUP` | `LogService::cleanup` | `subdomains/supporting/log/log.service.ts` | +| week | `BANK_ACCOUNT` | `BankAccountService::checkFailedBankAccounts` | `subdomains/supporting/bank/bank-account/bank-account.service.ts` | +| weekend | `SANCTION_SYNC` | `SanctionService::syncList` | `subdomains/core/aml/services/sanction.service.ts` | +| 1st day of month at midnight | — | `BuyService::resetMonthlyVolumes` | `subdomains/core/buy-crypto/routes/buy/buy.service.ts` | +| 1st day of month at midnight | — | `SellService::resetMonthlyVolumes` | `subdomains/core/sell-crypto/route/sell.service.ts` | +| 1st day of month at midnight | — | `SwapService::resetMonthlyVolumes` | `subdomains/core/buy-crypto/routes/swap/swap.service.ts` | +| 1st day of month at midnight | — | `UserDataService::resetMonthlyVolumes` | `subdomains/generic/user/models/user-data/user-data.service.ts` | +| 1st day of month at midnight | — | `UserService::resetMonthlyVolumes` | `subdomains/generic/user/models/user/user.service.ts` | +| year | — | `BuyService::resetAnnualVolumes` | `subdomains/core/buy-crypto/routes/buy/buy.service.ts` | +| year | — | `SellService::resetAnnualVolumes` | `subdomains/core/sell-crypto/route/sell.service.ts` | +| year | — | `SwapService::resetAnnualVolumes` | `subdomains/core/buy-crypto/routes/swap/swap.service.ts` | +| year | — | `UserDataService::resetAnnualVolumes` | `subdomains/generic/user/models/user-data/user-data.service.ts` | +| year | — | `UserService::resetAnnualVolumes` | `subdomains/generic/user/models/user/user.service.ts` | diff --git a/src/subdomains/supporting/log/__tests__/client-error.controller.spec.ts b/src/subdomains/supporting/log/__tests__/client-error.controller.spec.ts new file mode 100644 index 0000000000..49597cffb3 --- /dev/null +++ b/src/subdomains/supporting/log/__tests__/client-error.controller.spec.ts @@ -0,0 +1,54 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { GUARDS_METADATA, METHOD_METADATA, PATH_METADATA } from '@nestjs/common/constants'; +import { RequestMethod } from '@nestjs/common/enums'; +import { THROTTLER_LIMIT, THROTTLER_TTL } from '@nestjs/throttler/dist/throttler.constants'; +import { RateLimitGuard } from 'src/shared/auth/rate-limit.guard'; +import { ClientErrorController } from '../client-error.controller'; +import { ClientErrorService } from '../client-error.service'; +import { CreateClientErrorDto } from '../dto/create-client-error.dto'; + +// This endpoint is deliberately unauthenticated, so the rate limit is the only thing standing +// between a public route and unbounded log writes. That wiring is worth pinning. +describe('ClientErrorController', () => { + let controller: ClientErrorController; + let service: DeepMocked; + + const dto: CreateClientErrorDto = Object.assign(new CreateClientErrorDto(), { message: 'boom' }); + + beforeEach(() => { + service = createMock(); + controller = new ClientErrorController(service); + }); + + it('passes the reported error and the request context to the service', () => { + controller.logError(dto, 'dfx-services', 'Mozilla/5.0'); + + expect(service.logError).toHaveBeenCalledWith(dto, 'dfx-services', 'Mozilla/5.0'); + }); + + it('accepts a report without client or user agent', () => { + controller.logError(dto); + + expect(service.logError).toHaveBeenCalledWith(dto, undefined, undefined); + }); + + // --- ROUTING & SECURITY METADATA --- // + + const handler = ClientErrorController.prototype.logError; + + it('is mounted as POST log/clientError', () => { + expect(Reflect.getMetadata(PATH_METADATA, ClientErrorController)).toBe('log/clientError'); + expect(Reflect.getMetadata(METHOD_METADATA, handler)).toBe(RequestMethod.POST); + }); + + it('guards the route with RateLimitGuard', () => { + expect(Reflect.getMetadata(GUARDS_METADATA, handler)).toEqual([RateLimitGuard]); + }); + + it('carries a route-level throttle, which is what gives the guard a limit at all', () => { + // RateLimitGuard resolves `routeOrClassLimit || this.options.limit`, and ThrottlerModule.forRoot() + // is registered without options - so without this decorator nothing would be throttled. + expect(Reflect.getMetadata(THROTTLER_LIMIT, handler)).toBe(20); + expect(Reflect.getMetadata(THROTTLER_TTL, handler)).toBe(60); + }); +}); diff --git a/src/subdomains/supporting/log/__tests__/client-error.service.spec.ts b/src/subdomains/supporting/log/__tests__/client-error.service.spec.ts new file mode 100644 index 0000000000..435306d9de --- /dev/null +++ b/src/subdomains/supporting/log/__tests__/client-error.service.spec.ts @@ -0,0 +1,322 @@ +import { DfxLogger } from 'src/shared/services/dfx-logger'; +import { ClientErrorService } from '../client-error.service'; +import { CreateClientErrorDto } from '../dto/create-client-error.dto'; + +const TOKEN = 'eyJhbGciOiJIUzI1NiJ9.abc'; + +describe('ClientErrorService', () => { + let service: ClientErrorService; + let error: jest.SpyInstance; + + function dto(values: Partial = {}): CreateClientErrorDto { + return Object.assign(new CreateClientErrorDto(), { message: 'Loading chunk 42 failed', ...values }); + } + + function loggedLine(): string { + return error.mock.calls[0][0]; + } + + beforeEach(() => { + service = new ClientErrorService(); + error = jest.spyOn(DfxLogger.prototype, 'error').mockImplementation(); + }); + + afterEach(() => jest.restoreAllMocks()); + + it('logs the error at ERROR level', () => { + service.logError(dto({ type: 'ChunkLoadError' })); + + expect(error).toHaveBeenCalledTimes(1); + expect(loggedLine()).toContain('message="Loading chunk 42 failed"'); + expect(loggedLine()).toContain('type="ChunkLoadError"'); + }); + + it('logs client, route, version and user agent', () => { + service.logError(dto({ route: '/buy', version: '1.2.3' }), 'dfx-services', 'Mozilla/5.0'); + + expect(loggedLine()).toContain('client="dfx-services"'); + expect(loggedLine()).toContain('route="/buy"'); + expect(loggedLine()).toContain('version="1.2.3"'); + expect(loggedLine()).toContain('userAgent="Mozilla/5.0"'); + }); + + it('logs absent context as an empty value', () => { + service.logError(dto()); + + expect(loggedLine()).toContain('client=""'); + expect(loggedLine()).toContain('route=""'); + expect(loggedLine()).toContain('version=""'); + expect(loggedLine()).toContain('userAgent=""'); + }); + + it('appends the stack when given', () => { + service.logError(dto({ stack: 'at BuyScreen (main.js:1:2)' })); + + expect(loggedLine()).toContain('stack="at BuyScreen (main.js:1:2)"'); + }); + + it('omits the stack field when no stack is given', () => { + service.logError(dto()); + + expect(loggedLine()).not.toContain('stack='); + }); + + // --- REDACTING --- // + // The endpoint is unauthenticated, so every field is attacker-controlled. A URL is cut to its + // path, which is what makes the encoding of a parameter irrelevant; matching parameter names + // alone loses a round to every new encoding. + + it.each([ + ['a query string', `GET https://app.example.com/buy?session=${TOKEN}&asset=BTC failed`], + ['a fragment', `https://app.example.com/cb#session=${TOKEN}`], + ['a matrix parameter', `https://app.example.com/buy;session=${TOKEN}`], + ['a percent-encoded parameter', `https://app.example.com/buy?session%3D${TOKEN}`], + ['a double-encoded parameter', `https://app.example.com/buy?session%253D${TOKEN}`], + ['a percent-encoded parameter name', `https://app.example.com/buy?sess%69on=${TOKEN}`], + ['a parameter with no recognisable name', `https://app.example.com/cb?x=${TOKEN}`], + ])('drops %s from a URL', (_case, message) => { + service.logError(dto({ message })); + + expect(loggedLine()).not.toContain(TOKEN); + }); + + // A URL does not need a scheme to carry a credential. A relative path is the normal shape for an + // app calling its own API, which makes this the common case rather than the exotic one. + it.each([ + ['a relative path', `GET /buy?address=${TOKEN} failed`], + ['a relative path with no recognisable name', `GET /cb?x=${TOKEN}`], + ['a protocol-relative URL', `//app.example.com/buy?walletAddress=${TOKEN}`], + ['a fully percent-encoded URL', `https%3A%2F%2Fapp.example.com%2Fbuy%3Faddress%3D${TOKEN}`], + ])('drops parameters from %s', (_case, message) => { + service.logError(dto({ message })); + + expect(loggedLine()).not.toContain(TOKEN); + }); + + // Credentials in the authority sit in front of every separator, so cutting at the query misses + // them entirely. + it('drops a credential embedded in the authority', () => { + service.logError(dto({ message: `https://admin:${TOKEN}@app.example.com/buy` })); + + expect(loggedLine()).not.toContain(TOKEN); + }); + + it.each([ + ['at the start of the value', `session=${TOKEN} lookup failed`], + ['behind a colon', `session: ${TOKEN}`], + ['in JSON', `{"token":"${TOKEN}"}`], + ['in a compound name', `accessToken=${TOKEN}`], + ['in a compound name behind a colon', `accessToken: ${TOKEN}`], + ['in a compound name with the secret part first', `sessionId: ${TOKEN}`], + ['in a snake_case name', `access_token=${TOKEN}`], + ['in an all-caps name', `SESSIONID=${TOKEN}`], + ['behind an authorization scheme', `authorization: Bearer ${TOKEN}`], + // A harmless assignment must not swallow the one behind it: these are the shapes a serialised + // form, a referrer or a debug dump arrives in, and a global replace never re-reads what a + // match already consumed. + ['behind an unrelated assignment', `debug: a=1;password=${TOKEN}`], + ['behind an unrelated query parameter', `Referrer: page=checkout&sessionToken=${TOKEN}`], + ['between unrelated form fields', `formData: name=John&password=${TOKEN}&city=Zurich`], + ['behind a comma-joined pair', `state: step=2,token=${TOKEN}`], + ['in a quoted value', `session="${TOKEN} more"`], + ['regardless of case', `Signature=${TOKEN}`], + ])('masks a bare secret assignment %s', (_case, message) => { + service.logError(dto({ message })); + + expect(loggedLine()).toContain(''); + expect(loggedLine()).not.toContain(TOKEN); + }); + + it.each(['session', 'signature', 'password', 'secret', 'token', 'otp', 'jwt', 'authorization', 'mail', 'apikey'])( + 'masks the %s assignment', + (name) => { + service.logError(dto({ message: `${name}=${TOKEN}` })); + + expect(loggedLine()).not.toContain(TOKEN); + }, + ); + + // `auth` on its own names authentication, not a credential. Listing it would take authMethod and + // OAuthProvider with it, so `authorization` is spelled out instead. + it('does not treat auth as a secret name on its own', () => { + service.logError(dto({ message: 'authMethod=MetaMask authProvider=walletconnect' })); + + expect(loggedLine()).toContain('authMethod=MetaMask'); + expect(loggedLine()).toContain('authProvider=walletconnect'); + }); + + // The documented limit, pinned so it is not "fixed" back into the swallowing behaviour above: + // a value ends at a joiner, so a secret that contains one keeps its tail. Masked in part beats + // the alternative, where an unrelated assignment in front of it hides it entirely. + it('masks only up to the joiner when a secret value contains one', () => { + service.logError(dto({ message: `password=hunter2,${TOKEN}` })); + + expect(loggedLine()).toContain('password='); + expect(loggedLine()).toContain(TOKEN); + }); + + it('redacts a credential carried in a stack', () => { + service.logError(dto({ stack: `at load (https://app.example.com/buy?signature=${TOKEN})` })); + + expect(loggedLine()).not.toContain(TOKEN); + }); + + // The counterpart failure: a name list broad enough to catch everything also redacts the fields + // that make a report worth reading. These are the ones a frontend error actually carries. + it.each([ + 'statusCode=502', + 'errorCode=E_TIMEOUT', + 'countryCode=CH', + 'currencyCode=CHF', + 'zipCode=8000', + 'keyboardLayout=qwerty', + 'monkey=banana', + 'asset=BTC', + 'amount=300', + 'chunkId=738', + ])('keeps the diagnostic value %s', (value) => { + service.logError(dto({ message: `failed with ${value}` })); + + expect(loggedLine()).toContain(value); + }); + + // Ordinary prose puts a colon after words that contain a secret name as a substring. Matching + // those would eat the half of the sentence that says what actually happened — and these are the + // sentences a frontend error most often consists of. + it.each([ + '401 Unauthorized: invalid credentials', + 'Error: Unauthorized: Session expired', + 'Failed to open in Gmail: no app found', + 'authMethod: MetaMask', + 'OAuthProvider: google', + 'TypeError: x is not a function', + ])('keeps the prose "%s"', (message) => { + service.logError(dto({ message })); + + expect(loggedLine()).toContain(message); + }); + + // Cutting a URL at its parameters also cuts a regex literal at a `?` or `#` inside it. The + // sentence around it stays readable, which is the trade-off taken here — pinned so a later + // change to the pattern does not widen the loss unnoticed. + it('keeps the sentence around a regex literal, even though the pattern itself is cut', () => { + service.logError(dto({ message: "Invalid email, expected /^\\S+?@\\S+$/ but got 'foo'" })); + + expect(loggedLine()).toContain('Invalid email, expected'); + expect(loggedLine()).toContain("but got 'foo'"); + }); + + // The endpoint takes free text from anyone, and Node runs it on the one thread that serves every + // other request. A pattern that backtracks over long words turns a single post into a stall. + it('sanitizes a full-length field without measurable cost', () => { + const worstCase = 'token-'.repeat(666); // 3996 chars, just inside the stack field limit + + const start = process.hrtime.bigint(); + service.logError(dto({ stack: worstCase })); + const elapsedMs = Number(process.hrtime.bigint() - start) / 1e6; + + expect(elapsedMs).toBeLessThan(100); + }); + + it('keeps the asset path of a failed chunk, which is the point of the report', () => { + service.logError(dto({ message: 'Loading chunk 738 failed (missing: https://app.example.com/static/js/738.js)' })); + + expect(loggedLine()).toContain('https://app.example.com/static/js/738.js'); + }); + + it('discards the query string, fragment and matrix parameters of a route', () => { + service.logError(dto({ route: `/buy;session=${TOKEN}?asset=BTC#top` })); + + expect(loggedLine()).toContain('route="/buy"'); + expect(loggedLine()).not.toContain(TOKEN); + }); + + // --- LOG INTEGRITY --- // + + it('quotes the free text so it cannot forge a log line', () => { + service.logError(dto({ message: 'real\n[Nest] 7 - ERROR [PayoutService] forged' })); + + expect(loggedLine()).not.toContain('\n'); + expect(loggedLine()).toContain('message="real\\n[Nest] 7 - ERROR [PayoutService] forged"'); + }); + + // Without quoting, this reads as context fields of its own and a log query cannot tell the + // forged ones from the genuine ones that follow. + it('quotes the free text so it cannot forge context fields', () => { + service.logError(dto({ message: 'boom route=/internal client=support trace_id=deadbeef' })); + + expect(loggedLine()).toContain('message="boom route=/internal client=support trace_id=deadbeef"'); + expect(loggedLine()).toContain('route=""'); + }); + + it('escapes control characters so a payload cannot repaint a terminal tailing the log', () => { + service.logError(dto({ stack: '\u001b[2J\u001b[31mFAKE CRITICAL\u001b[0m' })); + + expect(loggedLine()).not.toContain('\u001b'); + expect(loggedLine()).toContain('\\u001b'); + }); + + it.each([ + ['line separator', '\u2028'], + ['paragraph separator', '\u2029'], + ['next line', '\u0085'], + ['delete', '\u007f'], + ])('replaces the %s character, which survives string escaping', (_name, char) => { + service.logError(dto({ message: `a${char}b` })); + + expect(loggedLine()).toContain('message="a b"'); + }); + + it('escapes a quote so it cannot close the field early', () => { + service.logError(dto({ message: 'boom" route="/internal' })); + + expect(loggedLine()).toContain('\\"'); + expect(loggedLine()).toContain('route=""'); + }); + + // --- BUDGET --- // + // Per-IP throttling bounds one client. Without a ceiling across all of them, a distributed flood + // would bury genuine incidents in the same ERROR stream that alerting reads. + + it('logs every report while within the budget', () => { + for (let i = 0; i < 120; i++) service.logError(dto()); + + expect(error).toHaveBeenCalledTimes(120); + }); + + it('drops reports beyond the budget instead of flooding the stream', () => { + for (let i = 0; i < 130; i++) service.logError(dto()); + + expect(error).toHaveBeenCalledTimes(120); + }); + + it('reports how many were dropped once the window rolls over', () => { + jest.useFakeTimers(); + try { + for (let i = 0; i < 130; i++) service.logError(dto()); + jest.advanceTimersByTime(60001); + + service.logError(dto()); + + const lines = error.mock.calls.map((c) => c[0] as string); + expect(lines).toContain('Client error reporting over budget: 10 reports dropped'); + } finally { + jest.useRealTimers(); + } + }); + + it('logs again after the window rolls over', () => { + jest.useFakeTimers(); + try { + for (let i = 0; i < 130; i++) service.logError(dto()); + error.mockClear(); + jest.advanceTimersByTime(60001); + + service.logError(dto({ message: 'after the window' })); + + expect(error.mock.calls.some((c) => (c[0] as string).includes('after the window'))).toBe(true); + } finally { + jest.useRealTimers(); + } + }); +}); diff --git a/src/subdomains/supporting/log/client-error.controller.ts b/src/subdomains/supporting/log/client-error.controller.ts new file mode 100644 index 0000000000..aa8a9e34c0 --- /dev/null +++ b/src/subdomains/supporting/log/client-error.controller.ts @@ -0,0 +1,36 @@ +import { Body, Controller, Headers, HttpCode, HttpStatus, Post, UseGuards } from '@nestjs/common'; +import { ApiOperation, ApiTags } from '@nestjs/swagger'; +import { Throttle } from '@nestjs/throttler'; +import { RateLimitGuard } from 'src/shared/auth/rate-limit.guard'; +import { CLIENT_HEADER } from 'src/shared/utils/request-client'; +import { ClientErrorService } from './client-error.service'; +import { CreateClientErrorDto } from './dto/create-client-error.dto'; + +@ApiTags('log') +@Controller('log/clientError') +export class ClientErrorController { + constructor(private readonly clientErrorService: ClientErrorService) {} + + @Post() + @ApiOperation({ + summary: 'Report a frontend error', + description: + 'Records a client-side error as an ERROR log line so it becomes visible in log monitoring. ' + + 'Unauthenticated on purpose: the errors worth catching happen before or without a session.', + }) + // RateLimitGuard buckets by /24 (IPv4) or /64 (IPv6), so customers sharing a NAT share this + // budget and can crowd each other out. Accepted: an error report is diagnostic, not a customer + // action, and the alternative — no per-client limit at all — is worse. The service holds a + // second, process-wide budget, because a per-client limit alone does not bound a distributed + // flood of the ERROR stream. + @UseGuards(RateLimitGuard) + @Throttle(20, 60) + @HttpCode(HttpStatus.NO_CONTENT) + logError( + @Body() dto: CreateClientErrorDto, + @Headers(CLIENT_HEADER) client?: string, + @Headers('user-agent') userAgent?: string, + ): void { + this.clientErrorService.logError(dto, client, userAgent); + } +} diff --git a/src/subdomains/supporting/log/client-error.service.ts b/src/subdomains/supporting/log/client-error.service.ts new file mode 100644 index 0000000000..7750470f80 --- /dev/null +++ b/src/subdomains/supporting/log/client-error.service.ts @@ -0,0 +1,183 @@ +import { Injectable } from '@nestjs/common'; +import { DfxLogger } from 'src/shared/services/dfx-logger'; +import { CreateClientErrorDto } from './dto/create-client-error.dto'; + +// Anything shaped like a URL or a path is cut down to the part before its parameters. This is what +// carries the redaction: the query and the fragment are dropped whole, so it makes no difference +// how a parameter is named, spelled or encoded. Matching parameter names instead loses a round to +// every new encoding — percent-encoded, double-encoded, an encoded character inside the name. +// +// Anchored on the slash rather than on a scheme, which also covers the shapes that carry no +// scheme: a relative path — the normal shape for an app calling its own API — and a +// protocol-relative one. Anchoring matters for cost too: a scheme pattern has to be retried at +// every position of a long word, and this endpoint takes input from anyone. +const URL_LIKE_REGEX = /(?:\/|%2f)[^\s"'<>]*/gi; + +// Where the meaningful part of a URL ends, percent-encoded or not: ? # ; +const PARAMETER_START = /[?#;]|%3[fb]|%23/i; + +// A credential in the authority (//user:secret@host) has no separator in front of it and would +// survive the cut above. +const USERINFO_REGEX = /^(\/\/)[^/\s@]*@/; + +// A bare assignment outside a URL is still worth masking — but only for names that are +// unambiguously secret. A broad list fails the other way round: `code` would redact statusCode and +// countryCode, `key` would redact keyboardLayout, and the diagnostic value this endpoint exists +// for would go with them. +// `auth` is deliberately absent and spelled out instead: as a name part it means authentication, +// not a credential, and would take `authMethod` with it — while `Unauthorized` is not a name part +// at all. +const SECRET_NAMES = [ + 'session', + 'signature', + 'password', + 'secret', + 'token', + 'otp', + 'jwt', + 'authorization', + 'mail', + 'email', + 'apikey', +]; + +// Anchored on the separator, with the name read from the text in front of it. A name pattern +// placed before the separator has to be retried at every position of a long word, which is +// quadratic on input this endpoint accepts from anyone. +// The scheme of an `authorization: Bearer ` is matched separately, so that the credential +// after it is redacted rather than just the word naming the scheme. +// +// A value ends at any joiner, not only at whitespace. Letting it run to the next space would make +// a harmless assignment swallow the one behind it — `a=1;password=secret` matches at `a=`, the +// value eats the rest, and because a global replace never re-reads what a match consumed, the +// password is never examined. Stopping early costs the tail of a value that legitimately contains +// a comma; that leaves part of it masked instead of none of it. +const ASSIGNMENT_REGEX = /(=|%3D|:|%3A)(\s*)(["']?)((?:bearer|basic|digest|token)\s+)?([^\s"'=:,&;]*)/gi; +const NAME_BEFORE_REGEX = /([\w-]+)(["']?)\s*$/; + +// Splits a name into its parts: accessToken, session_id and api-key all yield their components. +const NAME_SEGMENT_REGEX = /[A-Z]?[a-z]+|[A-Z]+(?![a-z])|\d+/g; + +// How far back to look for the name of an assignment. Longer than any realistic parameter name. +const NAME_LOOKBEHIND = 64; + +// JSON string escaping covers everything below U+0020, where the ordinary line breaks and the +// ANSI escape live. These sit above it, survive the escaping, and still break a line or move a +// cursor in terminals and log viewers. +const EXOTIC_LINE_BREAKS = /[\u0085\u007f\u2028\u2029]/g; + +// Budget for one process and one minute. The endpoint is unauthenticated, and per-IP throttling +// only bounds a single client: without a ceiling across all of them, a distributed flood would +// bury genuine incidents in the same ERROR stream that alerting reads. +const LOG_BUDGET_PER_MINUTE = 120; +const BUDGET_WINDOW = 60000; + +@Injectable() +export class ClientErrorService { + private readonly logger = new DfxLogger(ClientErrorService); + + private windowStart = 0; + private windowCount = 0; + private suppressedCount = 0; + + logError(dto: CreateClientErrorDto, client?: string, userAgent?: string): void { + // Checked before the fields are built, so a flood cannot buy sanitizing work it never uses. + if (!this.isWithinBudget()) return; + + const { message, type, stack, route, version } = dto; + + // Context first, free text last and quoted: message, type and stack are attacker-controlled + // and would otherwise be indistinguishable from the key=value context a log query parses. + const fields = [ + `client=${ClientErrorService.quote(client)}`, + `route=${ClientErrorService.quote(ClientErrorService.toPath(route))}`, + `version=${ClientErrorService.quote(version)}`, + `userAgent=${ClientErrorService.quote(userAgent)}`, + `type=${ClientErrorService.quote(type)}`, + `message=${ClientErrorService.quote(message)}`, + ]; + if (stack) fields.push(`stack=${ClientErrorService.quote(stack)}`); + + this.logger.error(`Client error: ${fields.join(' ')}`); + } + + // --- BUDGET --- // + + private isWithinBudget(): boolean { + const now = Date.now(); + + if (now - this.windowStart >= BUDGET_WINDOW) { + const suppressed = this.suppressedCount; + + this.windowStart = now; + this.windowCount = 0; + this.suppressedCount = 0; + + // Report the gap rather than leaving a silent hole in the record. + if (suppressed) this.logger.error(`Client error reporting over budget: ${suppressed} reports dropped`); + } + + if (this.windowCount >= LOG_BUDGET_PER_MINUTE) { + this.suppressedCount++; + return false; + } + + this.windowCount++; + return true; + } + + // --- SANITIZING --- // + + // Reduces a URL or path to the part in front of its parameters, and drops any credential embedded + // in the authority. The query carries the session and the signature the frontend authenticates + // with; a matrix parameter (;key=value) and a percent-encoded separator carry them just as well. + private static toPath(value?: string): string | undefined { + return value?.split(PARAMETER_START)[0].replace(USERINFO_REGEX, '$1'); + } + + // A colon appears all over ordinary prose, so there a secret name only counts as a part of the + // name in its own right: `accessToken:` and `sessionId:` are matched, while `Unauthorized:` and + // `Gmail:` are not — and those sentences are what a frontend error usually consists of. In front + // of an equals sign, or quoted as a JSON key, no sentence collides, so a plain substring is the + // safer choice there: it also catches spellings that have no parts to split, such as SESSIONID. + private static isSecretName(name: string, quoted: boolean, separator: string): boolean { + const isProseSeparator = !quoted && (separator === ':' || separator.toLowerCase() === '%3a'); + if (!isProseSeparator) return SECRET_NAMES.some((secret) => name.toLowerCase().includes(secret)); + + const segments: string[] = name.match(NAME_SEGMENT_REGEX) ?? []; + + return segments.some((segment) => SECRET_NAMES.includes(segment.toLowerCase())); + } + + // Strips URL parameters, masks bare secret assignments, then embeds the value as a JSON string. + // The quoting is what makes the line unforgeable: line breaks, control characters and ANSI + // escapes come out as escape sequences, so a payload can neither open a log line of its own nor + // repaint a terminal that is tailing the log. + // + // What is guaranteed: a credential carried the way this app carries one — as a parameter of a URL + // or a path — cannot reach the log, whatever it is called and however it is encoded, because the + // parameters are dropped rather than inspected. + // + // What is not: outside a URL, masking depends on recognising the name in front of the value, so + // a secret under an unknown name, or under no name at all, is indistinguishable from an ordinary + // diagnostic string and is logged. Even under a known name the masking ends where the value + // ends — a value that itself contains a comma or an equals sign keeps its tail. + private static quote(value?: string): string { + if (value == null) return '""'; + + const redacted = value + .replace(URL_LIKE_REGEX, (match) => ClientErrorService.toPath(match) ?? match) + .replace(ASSIGNMENT_REGEX, (match, separator, space, quote, scheme, assigned, offset: number, whole: string) => { + if (!assigned) return match; + + const name = NAME_BEFORE_REGEX.exec(whole.slice(Math.max(0, offset - NAME_LOOKBEHIND), offset)); + + return name && ClientErrorService.isSecretName(name[1], Boolean(name[2]), separator) + ? `${separator}${space}${quote}${scheme ?? ''}` + : match; + }) + .replace(EXOTIC_LINE_BREAKS, ' '); + + return JSON.stringify(redacted); + } +} diff --git a/src/subdomains/supporting/log/dto/create-client-error.dto.ts b/src/subdomains/supporting/log/dto/create-client-error.dto.ts new file mode 100644 index 0000000000..189f8fc61f --- /dev/null +++ b/src/subdomains/supporting/log/dto/create-client-error.dto.ts @@ -0,0 +1,43 @@ +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; +import { IsNotEmpty, IsOptional, IsString, MaxLength } from 'class-validator'; +import { Util } from 'src/shared/utils/util'; + +export class CreateClientErrorDto { + @ApiProperty({ description: 'Error message' }) + @IsNotEmpty() + @IsString() + @Transform(Util.trim) + @MaxLength(500) + message: string; + + @ApiPropertyOptional({ description: 'Error type (e.g. ChunkLoadError, TypeError)' }) + @IsOptional() + @IsString() + @Transform(Util.trim) + @MaxLength(100) + type?: string; + + @ApiPropertyOptional({ description: 'Stack trace' }) + @IsOptional() + @IsString() + @Transform(Util.trim) + @MaxLength(4000) + stack?: string; + + @ApiPropertyOptional({ + description: 'Route the error occurred on (query string, fragment and matrix parameters are discarded server-side)', + }) + @IsOptional() + @IsString() + @Transform(Util.trim) + @MaxLength(500) + route?: string; + + @ApiPropertyOptional({ description: 'Frontend build version' }) + @IsOptional() + @IsString() + @Transform(Util.trim) + @MaxLength(50) + version?: string; +} diff --git a/src/subdomains/supporting/log/log.module.ts b/src/subdomains/supporting/log/log.module.ts index ea4fb08b6f..271dde52b7 100644 --- a/src/subdomains/supporting/log/log.module.ts +++ b/src/subdomains/supporting/log/log.module.ts @@ -1,6 +1,8 @@ import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; import { SharedModule } from 'src/shared/shared.module'; +import { ClientErrorController } from './client-error.controller'; +import { ClientErrorService } from './client-error.service'; import { LogController } from './log.controller'; import { Log } from './log.entity'; import { LogRepository } from './log.repository'; @@ -8,8 +10,8 @@ import { LogService } from './log.service'; @Module({ imports: [TypeOrmModule.forFeature([Log]), SharedModule], - controllers: [LogController], - providers: [LogRepository, LogService], + controllers: [LogController, ClientErrorController], + providers: [LogRepository, LogService, ClientErrorService], exports: [LogService], }) export class LogModule {}