From 03b0415b2c4e927a03f91fb5d84a7dccfa87995d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Wieczorek?= Date: Fri, 22 May 2026 02:37:01 +0200 Subject: [PATCH] Fix ESLint warnings for `server` (#331) * server: Remove stale eslint-disable and simplify nullish operations This patch fixes 5 ESLint warnings. The max-lines disable in actionStatistics.ts no longer suppresses any warning and only adds noise. In apiKey.ts, "||" is replaced with "??" for description and lastUsedAt fallbacks. "||" coerces any falsy value (including empty string: "") to the default, whereas "??" only fires on null/undefined - which may be the actual intent here. An empty-string description would be silently discarded with "||". In integrationRegistry/index.ts and RetryFailedNcmecDecisionsJob.ts, null-guarded assignment blocks (if (x == null) { x = ... }) are collapsed to "??=" assignments, which is the idiomatic shorthand for exactly that pattern and makes the intent immediately readable. * server: Type apiKey resolvers with generated GQLQueryResolvers/GQLMutationResolvers The Query and Mutation objects in apiKey.ts are typed as any, which removes all type-checking on resolver arguments and the context object. The codegen pipeline already produces GQLQueryResolvers and GQLMutationResolvers with the correct signatures. Using those types means wrong argument names, missing fields, or context misuse will be caught at compile time rather than at runtime. Error names must match a GraphQL union member because gqlErrorResult uses them as type name. 'InternalServerError' is a valid CoopErrorName but not a member of the RotateApiKeyResponse or RotateWebhookSigningKeyResponse unions, so Apollo can't resolve the type. * server: Cast GET route arrays to ControllerRouteList to remove per-route any route.get returns Route which is invariant with Route in ControllerRouteList, so individual routes needed as Route casts. Move the cast to the routes array level using as ControllerRouteList, which is already covered by the centralized eslint-disable on the type definition. * server: Fix passport typing without unsafe casts The SAML strategy callbacks passed Sequelize model instances to done(), which expects "Record". The original code used "as any" to fix the mismatch; simply removing those casts broke the build because Sequelize models carry no string index signature. Use "user.toJSON()" instead - it returns a plain object that genuinely satisfies "Record" at runtime. For serializeUser, augment "Express.User" in decs.d.ts with the single field it actually accesses ("id: string"). * server: Parameterise Kysely instances with concrete schema types PostgresAnalyticsAdapter and ClickhouseKyselyAdapter declare their Kysely fields as Kysely, which disables query-builder type- checking for every table access and column reference in those adapters. PostgresAnalyticsAdapter is parameterised with Kysely and ClickhouseKyselyAdapter with Kysely>, so Kysely can enforce table and column existence at compile time rather than at runtime. Stub method signatures using ...args: any[] are replaced with unknown[] so that the interface contract is actually enforced on the concrete classes. * server: Remove as-any casts in analytics loggers by completing AnalyticsSchema RuleExecutionLogger, ReportingRuleExecutionLogger, RoutingRuleExecutionLogger, and ContentApiLogger cast table-name strings and row arrays to any when calling bulkWrite because those table names were not yet keys of AnalyticsSchema. The casts silenced the errors but also prevented the compiler from catching a misspelled or removed table name. Adding the missing tables (REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS, MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS) to AnalyticsSchema and widening CONTENT_API_REQUESTS to {[key:string]:unknown} makes the constraint real and allows all call-site casts to be deleted or narrowed to their correct types. PostgresAnalyticsAdapter.flushTable used a cast that typed rows as a union of all schema table types. Kysely requires an intersection of required fields across those types, which the open-ended {[key:string]:unknown} entries (introduced in this commit) cannot satisfy. Instead, bulkWrite now stores a pre-built, fully-typed insert closure alongside each row buffer while it still knows the concrete table type (flushTable just calls that closure). * server: Replace deprecated SEMATTRS_EXCEPTION_* otel constants @opentelemetry/semantic-conventions deprecated the SEMATTRS_EXCEPTION_* identifiers in favor of ATTR_EXCEPTION_*. The two names refer to the same attribute keys, so this is a pure rename. Clears three @typescript-eslint/no-deprecated warnings. * server: Drop unnecessary conditionals and type assertions Small, mechanical cleanups flagged by @typescript-eslint/no-unnecessary-condition and @typescript-eslint/no-unnecessary-type-assertion. Each change either removes a guard the type system has already proven redundant, or removes an assertion that doesn't change the inferred type. * routes/integration_logos/serveIntegrationLogo*.ts: typed the express sendFile callback as "(err?: Error)" and simplified "err != null" to a truthiness check. Matches express's actual contract. * routes/reporting/submitReport.ts: dropped an always-truthy "hashes &&" guard. * rule_engine/RuleEngine.ts: dropped "?? []" on a non-nullable return. * utils/sql.ts: removed two "SelectQueryBuilder<...>" casts the inference now handles cleanly. * server: lower NCMEC file-annotation mapping complexity from 21 to 3 up a FileAnnotations object one case at a time. That tripped the complexity rule (max 20) and made the mapping itself hard to audit. Replaced with a single "Record" lookup table plus a small for-loop. The table is the single source of truth for the NCMEC-enum -> field-name correspondence, and TypeScript's exhaustiveness checking on the Record key forces every enum variant to be mapped. * server: Replace "any" with real types in production code Each case below previously used "any" (or "as any") to paper over a typing problem. Where a cast is still needed, it goes through the narrowest meaningful type. - condition_evaluator/conditionSet.ts: typed the working array as "Array>" and replaced two "as any" casts with a single documented narrowing cast at the return site. The public type is a discriminated union over arrays-of-leaves vs arrays-of-sets, which can't be pushed into during the loop; runtime contents stay uniform with the input conditions. - condition_evaluator/leafCondition.ts: typed the signal-result generic as "SignalResult", removing both an "as any" and an unnecessary "as SignalOutputType" assertion at the call time. - graphql/datasources/UserApi.ts: typed the login/signUp/logout passport glue using the generated "GQLMutation-Args" types and a small structural context type (instead of "any"). - services/apiKeyService/apiKeyService.ts: derived "ApiKeyRow" from "Selectable" (Kysely) and used it as the param type of "mapDbRecordToApiKeyRecord". - services/manualReviewToolService/modules/CommentOperations.ts: replaced "as any" / "as any[]" with "as JobId" / "as JobId[]" using the existing branded type. - services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts: typed warehouse rows as "Record" with explicit field-level casts, and the binding accumulator as "string[]". - services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts: preserved the runtime guard against missing org rows by indexing through "Partial". keyBy's "Record" return type lies about lookups always succeeding. - bin/run-worker-or-job.ts: replaced "(container as any)[name]" with a typed "container[name as keyof Dependencies] as WorkerOrJob" lookup. * server: Scope "any" suppressions to documented TODOs Three locations have "any" casts that can't be replaced today without upstream typing work that's already tracked by TODOs. - graphql/modules/insights.ts: resolver maps five times "getRulePassingContentSamples" results onto types that require "tags" and "policies" fields the datasource doesn't yet populate. The fix belongs in the datasource's typing, not in each resolver. Kept the localized "as any" cast and the existing TODO comments - graphql/resolvers.ts: same shape of pre-existing TODO on "getAllRuleInsights" return type - services/analyticsQueries/ItemHistoryQueries.ts: the data warehouse schema isn't modeled in TS, so the Kysely instance is intentionally "Kysely" (so the string-based column selections pass type-check). * server: Tighten test mocks and clean up test-file "any" usage Tests that mock private methods or attach properties to typed services need escape hatches that production code shouldn't. This patch replaces "any" with real types where possible. - condition_evaluator/conditionSet.test.ts: wrap RuleEvaluationContext / SafeTracer mock stubs with "eslint-disable" / "eslint-enable" and a comment. - services/derivedFieldsService/helpers.test.ts: the test intentionally passes invalid inputs to exercise error paths. Wrapped that block in a paired "eslint-disable" / "eslint-enable" for "no-explicit-any" with a comment. - services/manualReviewToolService/modules/CommentOperations.test.ts: replaced "as any" on job ids with "as JobId"; switched the "(commentOps as any).getRelatedJobIds(...)" private-method probe to bracket access ("commentOps['getRelatedJobIds'](...)"). Same access pattern at runtime without a need for casting. - services/orgAwareSignalExecutionService/signalExecutionService.test.ts: extended two existing narrow "eslint-disable" directives to also cover "no-explicit-any" for jest mock assignments. Added a paired "eslint-disable" / "eslint-enable" block where the original next-line directive couldn't reach the multi-line cast. - services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts: typed the mock query's "tracer" parameter as "unknown". Kept the unavoidable "jest.fn() as any" for the typed mock fixture and "{} as any" for the unused tracer arg, both with narrow per-line disables that name the rule and the reason. - test/extendExpect.ts: replaced "(toMatchSnapshot as any).call" with "toMatchSnapshot.call(this as SnapshotContext, ...)" using jest-snapshot's exported "Context" type. Added an explicit return type to satisfy "promise-function-async" (the inferred sync|async union was tripping the rule). * server: Keep single-case exhaustiveness switches, scope the warning Three switches dispatch on a discriminated union that today has only one variant: AggregationsService.evaluateAggregation, serializeAggregation, and the derivedFieldsService recipe-operation reducer. With one variant each case label is trivially equal to the discriminant, which trips @typescript-eslint/no-unnecessary-condition. The switches stay because they're load-bearing: when a new variant is added to the union, the existing case stops being exhaustive and the "default: assertUnreachable(...)" branch surfaces the gap at compile time. * Fix merge conflicts * server: Drop residual Sequelize toJSON() calls The two SAML strategy callbacks still called `user.toJSON()` left over from the Sequelize User instance. `kyselyUserFindByEmail` now returns a plain `GraphQLUserParent` object. * server: Fix unreachable null check in MRT job The "fresh deploy" guard checked "lastTimestamp.last_insert == null", which triggered @typescript-eslint/no-unnecessary-condition: the Kysely schema types "last_insert" as a non-null "Date", matching the SQL column ("NOT NULL DEFAULT '-infinity'"). This patch replaces "== null" with "Number.isFinite(last_insert.valueOf())". This removes the dead branch (and the lint warning) and makes the fresh-deploy path actually fall back to "new Date(0)" as intended. * address PR review feedback --------- Co-authored-by: Juan S. Mrad --- server/api.ts | 14 ++--- server/bin/run-worker-or-job.ts | 6 +- .../condition_evaluator/conditionSet.test.ts | 4 ++ server/condition_evaluator/conditionSet.ts | 39 +++++++----- server/condition_evaluator/leafCondition.ts | 4 +- server/decs.d.ts | 8 +++ server/graphql/datasources/UserApi.ts | 11 +++- server/graphql/modules/apiKey.ts | 34 ++++------- server/graphql/modules/insights.ts | 42 ++++++------- server/graphql/resolvers.ts | 5 +- .../integration_logos/serveIntegrationLogo.ts | 4 +- .../serveIntegrationLogoWithBackground.ts | 4 +- server/routes/policies/PoliciesRoutes.ts | 14 ++--- server/routes/reporting/submitReport.ts | 1 - server/routes/user_scores/UserScoresRoutes.ts | 14 ++--- server/rule_engine/RuleEngine.ts | 17 ++---- .../AggregationsService.ts | 14 +++++ .../analyticsLoggers/ContentApiLogger.ts | 2 +- .../ReportingRuleExecutionLogger.ts | 2 +- .../RoutingRuleExecutionLogger.ts | 2 +- .../analyticsLoggers/RuleExecutionLogger.ts | 3 +- .../analyticsQueries/ItemHistoryQueries.ts | 7 ++- .../services/apiKeyService/apiKeyService.ts | 7 ++- .../derivedFieldsService/helpers.test.ts | 5 ++ .../services/derivedFieldsService/helpers.ts | 8 +++ server/services/integrationRegistry/index.ts | 4 +- .../modules/CommentOperations.test.ts | 10 ++-- .../modules/CommentOperations.ts | 7 ++- .../services/ncmecService/ncmecReporting.ts | 60 +++++++++---------- .../signalExecutionService.test.ts | 9 ++- .../detectRulePassRateAnomaliesJob.ts | 6 +- .../getRuleAnomalyDetectionStatistics.test.ts | 4 +- .../getRuleAnomalyDetectionStatistics.ts | 33 +++++----- .../dataWarehouse/ClickhouseAdapter.ts | 11 ++-- .../dataWarehouse/IDataWarehouseAnalytics.ts | 39 ++++++++---- .../dataWarehouse/PostgresAnalyticsAdapter.ts | 51 ++++++++++------ server/test/extendExpect.ts | 12 ++-- server/utils/errors.ts | 3 + server/utils/sql.ts | 6 +- .../RefreshMRTDecisionsMaterializedViewJob.ts | 13 ++-- .../RetryFailedNcmecDecisionsJob.ts | 5 +- 41 files changed, 312 insertions(+), 232 deletions(-) diff --git a/server/api.ts b/server/api.ts index b9fcada..7d3e66f 100644 --- a/server/api.ts +++ b/server/api.ts @@ -10,9 +10,9 @@ import { MapperKind, mapSchema } from '@graphql-tools/utils'; import { MultiSamlStrategy } from '@node-saml/passport-saml'; import { SpanStatusCode } from '@opentelemetry/api'; import { - SEMATTRS_EXCEPTION_MESSAGE, - SEMATTRS_EXCEPTION_STACKTRACE, - SEMATTRS_EXCEPTION_TYPE, + ATTR_EXCEPTION_MESSAGE, + ATTR_EXCEPTION_STACKTRACE, + ATTR_EXCEPTION_TYPE, } from '@opentelemetry/semantic-conventions'; import connectPgSimple from 'connect-pg-simple'; import cors from 'cors'; @@ -276,7 +276,7 @@ export default async function makeApiServer(deps: Dependencies) { }, ); - passport.serializeUser((user: any, done) => { + passport.serializeUser((user, done) => { done(null, user.id); }); @@ -422,11 +422,11 @@ export default async function makeApiServer(deps: Dependencies) { span.setStatus({ code: SpanStatusCode.ERROR, message: err.message }); // I don't know if these attributes are necessary, with recordException - span.setAttribute(SEMATTRS_EXCEPTION_MESSAGE, err.message); + span.setAttribute(ATTR_EXCEPTION_MESSAGE, err.message); if (err.stack) { - span.setAttribute(SEMATTRS_EXCEPTION_STACKTRACE, err.stack); + span.setAttribute(ATTR_EXCEPTION_STACKTRACE, err.stack); } - span.setAttribute(SEMATTRS_EXCEPTION_TYPE, err.name); + span.setAttribute(ATTR_EXCEPTION_TYPE, err.name); const errors = (() => { if (err instanceof AggregateError) { diff --git a/server/bin/run-worker-or-job.ts b/server/bin/run-worker-or-job.ts index d0b6dae..a07617d 100644 --- a/server/bin/run-worker-or-job.ts +++ b/server/bin/run-worker-or-job.ts @@ -1,14 +1,16 @@ #!/usr/bin/env node import _ from 'lodash'; -import getBottle from '../iocContainer/index.js'; +import getBottle, { type Dependencies } from '../iocContainer/index.js'; import { logErrorJson } from '../utils/logging.js'; import { type WorkerOrJob } from '../workers_jobs/index.js'; const { container } = await getBottle(); const workerOrJobName = process.argv[2]; -const workerOrJob = (container as any)[workerOrJobName] as WorkerOrJob; +const workerOrJob = container[ + workerOrJobName as keyof Dependencies +] as WorkerOrJob; const controller = new AbortController(); // When the worker/job finishes naturally (which only applies to jobs, as diff --git a/server/condition_evaluator/conditionSet.test.ts b/server/condition_evaluator/conditionSet.test.ts index 77b710c..7784ee4 100644 --- a/server/condition_evaluator/conditionSet.test.ts +++ b/server/condition_evaluator/conditionSet.test.ts @@ -85,8 +85,12 @@ describe('Condition Evaluation', () => { await getConditionSetResults( { conditions, conjunction: ConditionConjunction.OR }, + // Tests only exercise getSignalCost / tracer.getActiveSpan; + // a full RuleEvaluationContext / SafeTracer is unnecessary. + /* eslint-disable @typescript-eslint/no-explicit-any */ { getSignalCost } as any, jest.fn() as any, + /* eslint-enable @typescript-eslint/no-explicit-any */ stubRunLeafCondition, ); diff --git a/server/condition_evaluator/conditionSet.ts b/server/condition_evaluator/conditionSet.ts index 8ff93fa..a827ade 100644 --- a/server/condition_evaluator/conditionSet.ts +++ b/server/condition_evaluator/conditionSet.ts @@ -69,12 +69,19 @@ export async function getConditionSetResults( runLeafCondition = defaultRunLeafCondition, ): Promise> { const { conjunction } = conditionSet; - const result: ConditionSetWithResult = { - conjunction, - conditions: [] as unknown as - | NonEmptyArray - | NonEmptyArray, - }; + // We collect mixed condition results during the loop, then narrow back to + // the public "ConditionSetWithResult.conditions" shape at return time. The + // public type is a discriminated union of "all leaves" or "all sets", which + // can't be safely pushed into; the array's runtime contents stay uniform + // because the input "conditionSet.conditions" is uniform. + // + // The element type is "ReadonlyDeep<...>" here because the inputs we push + // come from "ReadonlyDeep". We cast back to the mutable + // shape at the return site below; the cast is sound because the data is + // freshly built and never mutated by callers. + const conditionsWithResult: Array< + ReadonlyDeep + > = []; // The conditions, sorted with lowest cost ones first. const getSignalCost = evaluationContext.getSignalCost.bind(evaluationContext); @@ -99,17 +106,17 @@ export async function getConditionSetResults( // if we can determine it before evaluating all conditions. let finalOutcome: ConditionOutcome | undefined; - // This is basically mapping `conditions` to ConditionWithResult, and putting - // the mapped array in result.conditions. But we don't use `map` because we - // want to run the conditions in sequence (i.e., with an `await` on each loop + // This is basically mapping "conditions" to ConditionWithResult, and pushing + // them onto "conditionsWithResult". But we don't use "map" because we want to + // run the conditions in sequence (i.e., with an "await" on each loop // iteration), and map would run them in parallel. for (const condition of sortedConditions) { // If we already know the final outcome for the condition set, then we can // skip all subsequent conditions, and a condition that's skipped has an // identical representation for its ConditionWithResult, so we just push - // the skipped condition into result.conditions. + // the skipped condition through. if (finalOutcome !== undefined) { - result.conditions.push(condition as any); + conditionsWithResult.push(condition); continue; } @@ -133,14 +140,14 @@ export async function getConditionSetResults( ), }; - result.conditions.push(conditionWithResult as any); + conditionsWithResult.push(conditionWithResult); // console.log('RESULT ' + conditionWithResult.result.outcome); // Attempt to determine the result for the whole condition set from the // outcomes so far. If we can, save that result to skip running each // condition for the rest of the loop const conditionSetOutcome = tryGetOutcomeFromPartialOutcomes( - result.conditions.map((c) => c.result!.outcome), + conditionsWithResult.map((c) => c.result!.outcome), conjunction, ); @@ -150,12 +157,14 @@ export async function getConditionSetResults( } return { - ...result, + conjunction, + conditions: + conditionsWithResult as ConditionSetWithResult['conditions'], result: { outcome: finalOutcome ?? getConditionSetOutcome( - result.conditions.map((c) => c.result!.outcome), + conditionsWithResult.map((c) => c.result!.outcome), conjunction, ), }, diff --git a/server/condition_evaluator/leafCondition.ts b/server/condition_evaluator/leafCondition.ts index f7bbc1d..c879222 100644 --- a/server/condition_evaluator/leafCondition.ts +++ b/server/condition_evaluator/leafCondition.ts @@ -203,7 +203,7 @@ export async function runLeafCondition( }); }), ) - : signalInputValues.map((it): SignalResult => { + : signalInputValues.map((it): SignalResult => { // If the condition specified no signal, then we should act as though // the "identity" signal was used; i.e., the value that the condition // picked out (with `condition.input`) should be returned as-is. @@ -259,7 +259,7 @@ export async function runLeafCondition( it.score, condition.threshold, condition.comparator, - it.outputType as SignalOutputType, + it.outputType, ) : (it as SignalResult<{ scalarType: ScalarTypes['BOOLEAN'] }>).score, ), diff --git a/server/decs.d.ts b/server/decs.d.ts index c180375..3922c84 100644 --- a/server/decs.d.ts +++ b/server/decs.d.ts @@ -1,3 +1,11 @@ +declare namespace Express { + // Extend Express.User so passport callbacks (serializeUser, etc.) see the + // fields we actually use without per-call `as any` casts. + interface User { + id: string; + } +} + declare module 'homoglyph-search'; declare module 'nilsimsa'; diff --git a/server/graphql/datasources/UserApi.ts b/server/graphql/datasources/UserApi.ts index 5c88f8a..38329dd 100644 --- a/server/graphql/datasources/UserApi.ts +++ b/server/graphql/datasources/UserApi.ts @@ -14,6 +14,10 @@ import { } from '../../utils/errors.js'; import { safePick } from '../../utils/misc.js'; import { WEEK_MS } from '../../utils/time.js'; +import { + type GQLMutationLoginArgs, + type GQLMutationSignUpArgs, +} from '../generated.js'; import { type PassportGqlContext } from '../utils/passportContext.js'; import { buildGraphqlRuleParent } from './buildGraphqlRuleParent.js'; import { type GraphQLRuleParent } from './ruleKyselyPersistence.js'; @@ -74,7 +78,7 @@ class UserAPI { return kyselyUserFindByIds(this.kyselyPg, ids); } - async login(params: any, context: PassportGqlContext) { + async login(params: GQLMutationLoginArgs, context: PassportGqlContext) { const { email, password } = safePick(params.input, ['email', 'password']); // Reject missing/empty credentials as a bad request before verifying. @@ -118,7 +122,10 @@ class UserAPI { } } - async signUp(params: any, _: any): Promise { + async signUp( + params: GQLMutationSignUpArgs, + _: unknown, + ): Promise { const { role } = params.input; const { email, diff --git a/server/graphql/modules/apiKey.ts b/server/graphql/modules/apiKey.ts index 394c827..c1a370a 100644 --- a/server/graphql/modules/apiKey.ts +++ b/server/graphql/modules/apiKey.ts @@ -2,17 +2,7 @@ import { ErrorType, CoopError } from '../../utils/errors.js'; import { logErrorJson } from '../../utils/logging.js'; import { gqlErrorResult, gqlSuccessResult } from '../utils/gqlResult.js'; import { forbiddenError } from '../utils/errors.js'; - -/** Context shape required by rotateWebhookSigningKey (avoids importing resolvers). */ -type RotateWebhookSigningKeyContext = { - getUser: () => { - orgId: string; - getPermissions: () => readonly string[]; - } | null | undefined; - dataSources: { - orgAPI: { rotateWebhookSigningKey: (orgId: string) => Promise }; - }; -}; +import { type GQLMutationResolvers, type GQLQueryResolvers } from '../generated.js'; const typeDefs = /* GraphQL */ ` type ApiKey { @@ -73,8 +63,8 @@ const typeDefs = /* GraphQL */ ` } `; -const Query: any = { - async apiKey(_: any, __: any, context: any) { +const Query: GQLQueryResolvers = { + async apiKey(_, __, context) { const user = context.getUser(); if (!user || !user.orgId) { throw forbiddenError('User does not have permission to check if key exists'); @@ -89,8 +79,8 @@ const Query: any = { }, }; -const Mutation: any = { - async rotateApiKey(_: any, { input }: any, context: any) { +const Mutation: GQLMutationResolvers = { + async rotateApiKey(_, { input }, context) { const user = context.getUser(); if (!user || !user.orgId) { throw forbiddenError('User does not have permission to rotate the API key'); @@ -103,7 +93,7 @@ const Mutation: any = { const { apiKey, record } = await context.services.ApiKeyService.rotateApiKey( user.orgId, input.name, - input.description || null, + input.description ?? null, user.id ); @@ -116,7 +106,7 @@ const Mutation: any = { description: record.description, isActive: record.isActive, createdAt: record.createdAt.toISOString(), - lastUsedAt: record.lastUsedAt?.toISOString() || null, + lastUsedAt: record.lastUsedAt?.toISOString() ?? null, createdBy: record.createdBy, }, }, @@ -132,17 +122,13 @@ const Mutation: any = { type: [ErrorType.InternalServerError], title: 'Failed to rotate API key', detail: 'An error occurred while rotating the API key', - name: 'InternalServerError', + name: 'RotateApiKeyError', shouldErrorSpan: true, }), ); } }, - async rotateWebhookSigningKey( - _: unknown, - __: Record, - context: RotateWebhookSigningKeyContext, - ) { + async rotateWebhookSigningKey(_, __, context) { const user = context.getUser(); if (!user || !user.orgId) { throw forbiddenError('User does not have permission to rotate the webhook signing key'); @@ -171,7 +157,7 @@ const Mutation: any = { type: [ErrorType.InternalServerError], title: 'Failed to rotate webhook signing key', detail: 'An error occurred while rotating the webhook signing key', - name: 'InternalServerError', + name: 'RotateWebhookSigningKeyError', shouldErrorSpan: true, }), ); diff --git a/server/graphql/modules/insights.ts b/server/graphql/modules/insights.ts index 89739f1..952b194 100644 --- a/server/graphql/modules/insights.ts +++ b/server/graphql/modules/insights.ts @@ -180,10 +180,10 @@ const RuleInsights: GQLRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. graphql queries would throw an exception if + // asking for fields that aren't populated here. To provide them, we'll + // have to update (and add better typings for) getRulePassingContentSamples. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -193,6 +193,7 @@ const RuleInsights: GQLRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, async priorVersionSamples(rule, _args, context) { const user = context.getUser(); @@ -210,10 +211,8 @@ const RuleInsights: GQLRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. See latestVersionSamples for context. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -223,6 +222,7 @@ const RuleInsights: GQLRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, }; @@ -255,10 +255,10 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. graphql queries would throw an exception if + // asking for fields that aren't populated here. To provide them, we'll + // have to update (and add better typings for) getRulePassingContentSamples. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -268,6 +268,7 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, async priorVersionSamples(rule, _args, context) { const user = context.getUser(); @@ -285,10 +286,8 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. See latestVersionSamples for context. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -298,6 +297,7 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, }; @@ -346,12 +346,11 @@ const Query: GQLQueryResolvers = { const result = samples[0]; + // TODO: type this properly. graphql queries would throw an exception if + // asking for fields that aren't populated here. To provide them, we'll + // have to update (and add better typings for) getRulePassingContentSamples. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return gqlSuccessResult( - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. - { ...result, itemId: result.contentId, @@ -363,6 +362,7 @@ const Query: GQLQueryResolvers = { } as any, 'RuleExecutionResult', ); + /* eslint-enable @typescript-eslint/no-explicit-any */ } catch (e) { if (isCoopErrorOfType(e, 'NotFoundError')) { return gqlErrorResult(e); diff --git a/server/graphql/resolvers.ts b/server/graphql/resolvers.ts index fdb600d..db9cd69 100644 --- a/server/graphql/resolvers.ts +++ b/server/graphql/resolvers.ts @@ -97,10 +97,13 @@ const Query: GQLQueryResolvers = { } try { - // TODO: this response type actually isn't right; remove cast and fix errors. + // TODO: this response type actually isn't right; remove the cast below + // and fix the underlying mismatch in getAllRuleInsights' return shape. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return (await context.dataSources.ruleAPI.getAllRuleInsights( user.orgId, )) as any; + /* eslint-enable @typescript-eslint/no-explicit-any */ } catch (e) { // eslint-disable-next-line no-console console.error( diff --git a/server/routes/integration_logos/serveIntegrationLogo.ts b/server/routes/integration_logos/serveIntegrationLogo.ts index b0e485e..df49802 100644 --- a/server/routes/integration_logos/serveIntegrationLogo.ts +++ b/server/routes/integration_logos/serveIntegrationLogo.ts @@ -35,8 +35,8 @@ export default function serveIntegrationLogo( // so the SPA can load it via when deployed on a different origin // than the API. res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin'); - res.sendFile(filePath, (err) => { - if (err != null && !res.headersSent) { + res.sendFile(filePath, (err?: Error) => { + if (err && !res.headersSent) { next(err); } diff --git a/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts b/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts index 0ccbeae..9b8381a 100644 --- a/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts +++ b/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts @@ -35,8 +35,8 @@ export default function serveIntegrationLogoWithBackground( // so the SPA can load it via when deployed on a different origin // than the API. res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin'); - res.sendFile(filePath, (err) => { - if (err != null && !res.headersSent) { + res.sendFile(filePath, (err?: Error) => { + if (err && !res.headersSent) { next(err); } diff --git a/server/routes/policies/PoliciesRoutes.ts b/server/routes/policies/PoliciesRoutes.ts index 1886e27..52e031a 100644 --- a/server/routes/policies/PoliciesRoutes.ts +++ b/server/routes/policies/PoliciesRoutes.ts @@ -1,19 +1,15 @@ -import { route, type Route } from '../../utils/route-helpers.js'; +import { route } from '../../utils/route-helpers.js'; import { createApiKeyMiddleware } from '../../utils/apiKeyMiddleware.js'; -import { type Controller } from '../index.js'; +import { type Controller, type ControllerRouteList } from '../index.js'; import getPolicies from './getPolicies.js'; export type GetPoliciesOutput = { policies: { id: string; name: string; parentId: string | null }[]; }; -// eslint-disable-next-line @typescript-eslint/consistent-type-assertions export default { pathPrefix: '/policies', routes: [ - route.get('/', (deps) => [createApiKeyMiddleware(deps), getPolicies(deps)]) as Route< - any, - GetPoliciesOutput - >, - ], -} as Controller; + route.get('/', (deps) => [createApiKeyMiddleware(deps), getPolicies(deps)]), + ] as ControllerRouteList, +} satisfies Controller; diff --git a/server/routes/reporting/submitReport.ts b/server/routes/reporting/submitReport.ts index e147938..af6cf33 100644 --- a/server/routes/reporting/submitReport.ts +++ b/server/routes/reporting/submitReport.ts @@ -128,7 +128,6 @@ export default function submitReport({ const matchedBankNames: string[] = []; if ( - hashes && Object.keys(hashes).length > 0 && allBankNames.length > 0 ) { diff --git a/server/routes/user_scores/UserScoresRoutes.ts b/server/routes/user_scores/UserScoresRoutes.ts index 1cfd6a6..fc8148e 100644 --- a/server/routes/user_scores/UserScoresRoutes.ts +++ b/server/routes/user_scores/UserScoresRoutes.ts @@ -1,17 +1,13 @@ -import { route, type Route } from '../../utils/route-helpers.js'; +import { route } from '../../utils/route-helpers.js'; import { createApiKeyMiddleware } from '../../utils/apiKeyMiddleware.js'; -import { type Controller } from '../index.js'; +import { type Controller, type ControllerRouteList } from '../index.js'; import getUserScores from './getUserScores.js'; export type GetUserScoresOutput = number; -// eslint-disable-next-line @typescript-eslint/consistent-type-assertions export default { pathPrefix: '/user_scores', routes: [ - route.get('/', (deps) => [createApiKeyMiddleware(deps), getUserScores(deps)]) as Route< - any, - GetUserScoresOutput - >, - ], -} as Controller; + route.get('/', (deps) => [createApiKeyMiddleware(deps), getUserScores(deps)]), + ] as ControllerRouteList, +} satisfies Controller; diff --git a/server/rule_engine/RuleEngine.ts b/server/rule_engine/RuleEngine.ts index 3e01657..2dad841 100644 --- a/server/rule_engine/RuleEngine.ts +++ b/server/rule_engine/RuleEngine.ts @@ -101,19 +101,14 @@ class RuleEngine { executionsCorrelationId: RuleExecutionCorrelationId, sync: boolean = false, ) { - // enabledRules can be null when the contentType can't be found. - // getEnabledRulesForContentTypeEventuallyConsistent has `null` in its - // return type primarily in case the contentTypeId points to a content type - // that doesn't exist. However, even though we know that the contentTypeId - // is for a content type that does exist (because we have the full - // ContentType model object), we still must handle `null` b/c it could be - // that contentType was _just_ created and can't be seen yet by - // getEnabledRulesForContentTypeEventuallyConsistent, which, as the name - // implies, is eventually consistent. + // enabledRules will be an empty array when the contentType can't be found + // or has no rules attached. getEnabledRulesForItemTypeEventuallyConsistent + // is, as the name implies, eventually consistent, so a content type that + // was just created may not yet be visible. const enabledRules = - (await this.getEnabledRulesForItemTypeEventuallyConsistent( + await this.getEnabledRulesForItemTypeEventuallyConsistent( itemSubmission.itemType.id, - )) ?? []; + ); const [liveRules, backgroundRules] = partition( enabledRules, diff --git a/server/services/aggregationsService/AggregationsService.ts b/server/services/aggregationsService/AggregationsService.ts index 59c3594..f558bb7 100644 --- a/server/services/aggregationsService/AggregationsService.ts +++ b/server/services/aggregationsService/AggregationsService.ts @@ -66,7 +66,14 @@ export class AggregationsService { const keys = getStoreKeysForAggregation(aggregation, runtimeArgs); const kvs = await this.keyValueStore.getAll(keys); + // The Aggregation union has only one variant ('COUNT') today, so the + // case label and the discriminant are trivially equal and trip + // no-unnecessary-condition. The switch is intentional: when a new + // variant is added, the existing case stops being exhaustive and the + // "default: assertUnreachable(...)" branch surfaces the gap at compile + // time. Disabling the condition rule (only) preserves that safety net. switch (aggregation.aggregation.type) { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- see comment above case 'COUNT': return Array.from(kvs.values()).reduce((acc, count) => acc + count, 0); default: @@ -212,7 +219,14 @@ function getStoreKeyPrefixForAggregation( } function serializeAggregation(aggregation: Aggregation): string { + // The Aggregation union has only one variant ('COUNT') today, so the + // case label and the discriminant are trivially equal and trip + // no-unnecessary-condition. The switch is intentional: when a new + // variant is added, the existing case stops being exhaustive and the + // `default: assertUnreachable(...)` branch surfaces the gap at compile + // time. Disabling the condition rule (only) preserves that safety net. switch (aggregation.type) { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- see comment above case 'COUNT': return 'COUNT'; default: diff --git a/server/services/analyticsLoggers/ContentApiLogger.ts b/server/services/analyticsLoggers/ContentApiLogger.ts index e05508b..94bd046 100644 --- a/server/services/analyticsLoggers/ContentApiLogger.ts +++ b/server/services/analyticsLoggers/ContentApiLogger.ts @@ -52,7 +52,7 @@ class ContentApiLogger { const { itemType } = itemSubmission; const now = new Date(); await this.analytics.bulkWrite( - 'CONTENT_API_REQUESTS' as any, + 'CONTENT_API_REQUESTS', [ { ds: getUtcDateOnlyString(now), diff --git a/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts b/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts index 652053d..2c4c9c6 100644 --- a/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts +++ b/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts @@ -37,7 +37,7 @@ class ReportingRuleExecutionLogger { ) { const now = new Date(); await this.analytics.bulkWrite( - 'REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS' as any, + 'REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS', executions.map((data) => ({ ds: getUtcDateOnlyString(now), ts: now.valueOf(), diff --git a/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts b/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts index 3f52e68..00d3695 100644 --- a/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts +++ b/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts @@ -43,7 +43,7 @@ class RoutingRuleExecutionLogger { ) { const now = new Date(); await this.analytics.bulkWrite( - 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS' as any, + 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', executions.map((data) => ({ ds: getUtcDateOnlyString(now), ts: now.valueOf(), diff --git a/server/services/analyticsLoggers/RuleExecutionLogger.ts b/server/services/analyticsLoggers/RuleExecutionLogger.ts index d6fa029..ca871d1 100644 --- a/server/services/analyticsLoggers/RuleExecutionLogger.ts +++ b/server/services/analyticsLoggers/RuleExecutionLogger.ts @@ -3,6 +3,7 @@ import { type ReadonlyDeep } from 'type-fest'; import { type Dependencies } from '../../iocContainer/index.js'; import { inject } from '../../iocContainer/utils.js'; +import { type AnalyticsSchema } from '../../storage/dataWarehouse/IDataWarehouseAnalytics.js'; import { type RuleEnvironment } from '../../rule_engine/RuleEngine.js'; import { type ConditionSetWithResult } from '../../services/moderationConfigService/index.js'; import { fromCorrelationId } from '../../utils/correlationIds.js'; @@ -80,7 +81,7 @@ class RuleExecutionLogger { correlation_id: fromCorrelationId(data.correlationId), result: jsonStringifyUnstable(pickConditionPropsToLog(data.result)), passed: data.passed, - })) as any, + })) as AnalyticsSchema['RULE_EXECUTIONS'][], { batchTimeout: sync ? 0 : undefined }, ); } diff --git a/server/services/analyticsQueries/ItemHistoryQueries.ts b/server/services/analyticsQueries/ItemHistoryQueries.ts index 6b639ef..8e4f705 100644 --- a/server/services/analyticsQueries/ItemHistoryQueries.ts +++ b/server/services/analyticsQueries/ItemHistoryQueries.ts @@ -19,6 +19,11 @@ type ItemHistoryQueryFilter = { class ItemHistoryQueries { constructor(private readonly dialect: Dependencies['DataWarehouseDialect']) {} + // The data warehouse schema isn't statically modeled in TS, so we use + // "any" here to opt out of Kysely's column-name checking. Kysely's stricter + // alternatives ("Record") reject the string-based column + // selections this query relies on. + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- see comment above private get kysely(): Kysely { return this.dialect.getKyselyInstance(); } @@ -76,7 +81,7 @@ class ItemHistoryQueries { try { const results = await query.execute(); - return results.map((it: any) => ({ + return results.map((it) => ({ itemTypeName: it.itemTypeName, itemTypeId: it.itemTypeId, userId: it.userId, diff --git a/server/services/apiKeyService/apiKeyService.ts b/server/services/apiKeyService/apiKeyService.ts index c701752..fe51346 100644 --- a/server/services/apiKeyService/apiKeyService.ts +++ b/server/services/apiKeyService/apiKeyService.ts @@ -1,7 +1,10 @@ import crypto from 'node:crypto'; -import type { Kysely } from 'kysely'; +import type { Kysely, Selectable } from 'kysely'; import { inject } from '../../iocContainer/index.js'; import { type CombinedPg } from '../combinedDbTypes.js'; +import { type ApiKeyServicePg } from './dbTypes.js'; + +type ApiKeyRow = Selectable; export interface ApiKeyMetadata { name: string; @@ -196,7 +199,7 @@ class ApiKeyService { /** * Maps database record to ApiKeyRecord */ - private mapDbRecordToApiKeyRecord(record: any): ApiKeyRecord { + private mapDbRecordToApiKeyRecord(record: ApiKeyRow): ApiKeyRecord { return { id: record.id, orgId: record.org_id, diff --git a/server/services/derivedFieldsService/helpers.test.ts b/server/services/derivedFieldsService/helpers.test.ts index f526067..745d920 100644 --- a/server/services/derivedFieldsService/helpers.test.ts +++ b/server/services/derivedFieldsService/helpers.test.ts @@ -261,6 +261,10 @@ describe('Item type schemas', () => { }); test('should reject invalid specs', () => { + // The casts to "any" below intentionally bypass TS so that we can + // construct invalid inputs (the very thing parseDerivedFieldSpec is + // expected to reject at runtime). + /* eslint-disable @typescript-eslint/no-explicit-any */ expect(() => { parseDerivedFieldSpec( serializeDerivedFieldSpec({ @@ -309,6 +313,7 @@ describe('Item type schemas', () => { }), ); }).toThrowErrorMatchingInlineSnapshot(`"Invalid derived field spec"`); + /* eslint-enable @typescript-eslint/no-explicit-any */ }); }); }); diff --git a/server/services/derivedFieldsService/helpers.ts b/server/services/derivedFieldsService/helpers.ts index c54b2b4..c28678c 100644 --- a/server/services/derivedFieldsService/helpers.ts +++ b/server/services/derivedFieldsService/helpers.ts @@ -339,7 +339,15 @@ export async function getDerivedFieldValue( .reduce(async (derivedResPromise, recipeOperation) => { // get value(s) as they've been generated so far. const valueOrValues = await derivedResPromise; + // The DerivedFieldOperationType enum has only one variant + // (RUN_SIGNAL) today, so the case label and the discriminant are + // trivially equal and trip no-unnecessary-condition. The switch is + // intentional: when a new variant is added, the existing case stops + // being exhaustive and the "default: assertUnreachable(...)" branch + // surfaces the gap at compile time. Disabling the condition rule + // (only) preserves that safety net. switch (recipeOperation.type) { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- see comment above case DerivedFieldOperationType.RUN_SIGNAL: { const transformValue = transformWithSignal.bind( null, diff --git a/server/services/integrationRegistry/index.ts b/server/services/integrationRegistry/index.ts index 066e81d..c602d33 100644 --- a/server/services/integrationRegistry/index.ts +++ b/server/services/integrationRegistry/index.ts @@ -91,9 +91,7 @@ let cachedRegistry: IntegrationRegistry | null = null; * Returns the integration registry (built once on first call). */ export function getIntegrationRegistry(): IntegrationRegistry { - if (cachedRegistry == null) { - cachedRegistry = buildRegistry(); - } + cachedRegistry ??= buildRegistry(); return cachedRegistry; } diff --git a/server/services/manualReviewToolService/modules/CommentOperations.test.ts b/server/services/manualReviewToolService/modules/CommentOperations.test.ts index c87edc8..ad94eb5 100644 --- a/server/services/manualReviewToolService/modules/CommentOperations.test.ts +++ b/server/services/manualReviewToolService/modules/CommentOperations.test.ts @@ -6,6 +6,7 @@ import createUser from '../../../test/fixtureHelpers/createUser.js'; import { makeTestWithFixture } from '../../../test/utils.js'; import { UserPermission } from '../../userManagementService/index.js'; import CommentOperations from './CommentOperations.js'; +import { type JobId } from '../manualReviewToolService.js'; describe('CommentOperations', () => { const testWithFixtures = makeTestWithFixture(async () => { @@ -55,7 +56,7 @@ describe('CommentOperations', () => { .insertInto('manual_review_tool.job_creations') .values([ { - id: jobId1 as any, + id: jobId1 as JobId, org_id: orgId, item_id: itemId, item_type_id: itemTypeId, @@ -64,7 +65,7 @@ describe('CommentOperations', () => { enqueue_source_info: {}, }, { - id: jobId2 as any, + id: jobId2 as JobId, org_id: orgId, item_id: itemId, item_type_id: itemTypeId, @@ -121,8 +122,7 @@ describe('CommentOperations', () => { async ({ commentOps, orgId }) => { const nonExistentJobId = uuidv1(); - // Access private method for testing - const result = await (commentOps as any).getRelatedJobIds({ + const result = await commentOps['getRelatedJobIds']({ orgId, jobId: nonExistentJobId, }); @@ -134,7 +134,7 @@ describe('CommentOperations', () => { testWithFixtures( 'should return all related job IDs when job found in job_creations', async ({ commentOps, orgId, jobId1, jobId2 }) => { - const result = await (commentOps as any).getRelatedJobIds({ + const result = await commentOps['getRelatedJobIds']({ orgId, jobId: jobId1, }); diff --git a/server/services/manualReviewToolService/modules/CommentOperations.ts b/server/services/manualReviewToolService/modules/CommentOperations.ts index 88edd2b..6d43b3e 100644 --- a/server/services/manualReviewToolService/modules/CommentOperations.ts +++ b/server/services/manualReviewToolService/modules/CommentOperations.ts @@ -4,6 +4,7 @@ import { v1 as uuidv1 } from 'uuid'; import { makeNotFoundError } from '../../../utils/errors.js'; import { isForeignKeyViolationError } from '../../../utils/kysely.js'; import { type ManualReviewToolServicePg } from '../dbTypes.js'; +import { type JobId } from '../manualReviewToolService.js'; export type ManualReviewJobComment = { id: string; @@ -30,7 +31,7 @@ export default class CommentOperations { .selectFrom('manual_review_tool.job_creations') .select(['item_id', 'item_type_id']) .where('org_id', '=', orgId) - .where('id', '=', jobId as any) + .where('id', '=', jobId as JobId) .executeTakeFirst(); if (!currentJob) { @@ -58,7 +59,7 @@ export default class CommentOperations { .selectFrom('manual_review_tool.job_comments') .select(manualReviewCommentDbSelection) .where('org_id', '=', orgId) - .where('job_id', 'in', jobIds as any[]) + .where('job_id', 'in', jobIds as JobId[]) .orderBy('created_at', 'asc') .execute(); @@ -73,7 +74,7 @@ export default class CommentOperations { .selectFrom('manual_review_tool.job_comments') .select((eb) => eb.fn.count('id').as('count')) .where('org_id', '=', orgId) - .where('job_id', 'in', jobIds as any[]) + .where('job_id', 'in', jobIds as JobId[]) .executeTakeFirst(); return result?.count ? Number(result.count) : 0; diff --git a/server/services/ncmecService/ncmecReporting.ts b/server/services/ncmecService/ncmecReporting.ts index 3b88867..d3b1a50 100644 --- a/server/services/ncmecService/ncmecReporting.ts +++ b/server/services/ncmecService/ncmecReporting.ts @@ -1919,40 +1919,34 @@ export default class NcmecReporting { if (!fileAnnotations || fileAnnotations.length === 0) { return undefined; } - return { - ...(fileAnnotations.includes( - NCMECFileAnnotation.ANIME_DRAWING_VIRTUAL_HENTAI, - ) - ? { animeDrawingVirtualHentai: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.POTENTIAL_MEME) - ? { potentialMeme: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.VIRAL) - ? { viral: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.POSSIBLE_SELF_PRODUCTION) - ? { possibleSelfProduction: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.PHYSICAL_HARM) - ? { physicalHarm: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.VIOLENCE_GORE) - ? { violenceGore: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.BESTIALITY) - ? { bestiality: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.LIVE_STREAMING) - ? { liveStreaming: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.INFANT) - ? { infant: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.GENERATIVE_AI) - ? { generativeAi: undefined } - : {}), + // Map each NCMEC annotation enum value to the corresponding XML field + // name. Iterating through the table avoids one logical branch per + // annotation, which previously pushed cyclomatic complexity over the + // configured ESLint limit and made the function hard to extend when new + // annotation types are added. + const annotationFieldByType: Record< + NCMECFileAnnotationType, + keyof FileAnnotations + > = { + [NCMECFileAnnotation.ANIME_DRAWING_VIRTUAL_HENTAI]: + 'animeDrawingVirtualHentai', + [NCMECFileAnnotation.POTENTIAL_MEME]: 'potentialMeme', + [NCMECFileAnnotation.VIRAL]: 'viral', + [NCMECFileAnnotation.POSSIBLE_SELF_PRODUCTION]: 'possibleSelfProduction', + [NCMECFileAnnotation.PHYSICAL_HARM]: 'physicalHarm', + [NCMECFileAnnotation.VIOLENCE_GORE]: 'violenceGore', + [NCMECFileAnnotation.BESTIALITY]: 'bestiality', + [NCMECFileAnnotation.LIVE_STREAMING]: 'liveStreaming', + [NCMECFileAnnotation.INFANT]: 'infant', + [NCMECFileAnnotation.GENERATIVE_AI]: 'generativeAi', }; + + const result: FileAnnotations = {}; + for (const annotation of fileAnnotations) { + const field = annotationFieldByType[annotation]; + result[field] = undefined; + } + return result; } async #uploadFileDetails( diff --git a/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts b/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts index 539a876..9e5e183 100644 --- a/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts +++ b/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts @@ -30,10 +30,10 @@ describe('Signal Execution Service', () => { // eslint-disable-next-line functional/immutable-data mockLocationsLoader.close = jest.fn(); - // eslint-disable-next-line functional/immutable-data + /* eslint-disable functional/immutable-data, @typescript-eslint/no-explicit-any */ (mockTextBankStringsLoader as any).close = jest.fn(); - // eslint-disable-next-line functional/immutable-data, @typescript-eslint/no-explicit-any (mockGetImageBank as any).close = jest.fn(); + /* eslint-enable functional/immutable-data, @typescript-eslint/no-explicit-any */ describe('getTransientRunSignalWithCache', () => { beforeAll(async () => { @@ -133,10 +133,13 @@ describe('Signal Execution Service', () => { // signals don't hit the network. (The point of the mock is just so we can // spy on how many times runSignal was called.) const signalsServiceSpy = (await getBottle()).container.SignalsService; - // eslint-disable-next-line functional/immutable-data + /* eslint-disable functional/immutable-data, @typescript-eslint/no-explicit-any -- + jest.fn doesn't preserve the original method's overloads, so we need + a cast to assign back onto the typed "runSignal" property. */ signalsServiceSpy.runSignal = jest.fn( signalsServiceSpy.runSignal.bind(signalsServiceSpy), ) as any; + /* eslint-enable functional/immutable-data, @typescript-eslint/no-explicit-any */ const runSignal = makeGetTransientRunSignalWithCache( mockLocationsLoader, diff --git a/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts b/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts index 9afbdda..dd3340e 100644 --- a/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts +++ b/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts @@ -73,7 +73,11 @@ export default inject( const ruleNowInAlarm = newAlarmStatusByRule[rule.id].status === RuleAlarmStatus.ALARM; - const orgRow = orgsForChangedRules[rule.org_id]; + // Use a "Partial" view to make the index access return the honest + // "OrgAlertRow | undefined" so the runtime guard stays meaningful. + const orgRow = (orgsForChangedRules as Partial)[ + rule.org_id + ]; if (!orgRow) { return []; } diff --git a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts index 25b89d5..d221794 100644 --- a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts +++ b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts @@ -13,7 +13,7 @@ describe('getRuleAnomalyDetectionStatistics', () => { let queryMock: MockedFn< ( query: string, - tracer: any, + tracer: unknown, binds?: readonly unknown[], ) => Promise >; @@ -33,6 +33,7 @@ describe('getRuleAnomalyDetectionStatistics', () => { // Scope of this is just the test suite, so mutation should be ok. + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- jest.fn() defaults to a generic call signature; cast lets us use the typed "queryMock" declared above. queryMock = jest.fn() as any; queryMock.mockResolvedValue(queryResult); @@ -48,6 +49,7 @@ describe('getRuleAnomalyDetectionStatistics', () => { getRulePassStatistics = makeGetRuleAnomalyDetectionStatistics( dataWarehouseMock, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- the second injected arg (tracer) is unused by this code path; passing an empty object stub. {} as any, ); }); diff --git a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts index 198d434..c9dbe07 100644 --- a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts +++ b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts @@ -44,7 +44,7 @@ const makeGetRuleAnomalyDetectionaStatistics = // UTC time, which is what we need (current_timestamp() is server-local time). const [conditions, conditionBindValues] = unzip2([ ...(!includePeriodsInProgress - ? [['ts_end_exclusive <= SYSDATE()', [] as any] as const] + ? [['ts_end_exclusive <= SYSDATE()', [] as string[]] as const] : []), ...(startTime ? [['ts_start_inclusive >= ?', startTime] as const] : []), ...(ruleIds @@ -77,20 +77,23 @@ const makeGetRuleAnomalyDetectionaStatistics = bindValues, ); - return results.map((result: any) => ({ - ruleId: result.RULE_ID, - // name is a reminder that JS may trim the precision on the Date here, - // but that should be ok for our purposes. - approxRuleVersion: new Date(result.RULE_VERSION), - // nb: the warehouse returned value for a timestamp is a JS Date, but with - // some extra methods attached to it. These methods include toString, so - // we cast back to a proper Date to avoid the string representation - // changing (e.g., when serializing to JSON). - windowStart: new Date(result.TS_START_INCLUSIVE), - passCount: result.NUM_PASSES, - passingUsersCount: result.NUM_DISTINCT_USERS, - runsCount: result.NUM_RUNS, - })); + return results.map((result) => { + const row = result as Record; + return { + ruleId: row.RULE_ID as string, + // name is a reminder that JS may trim the precision on the Date here, + // but that should be ok for our purposes. + approxRuleVersion: new Date(row.RULE_VERSION as string | number | Date), + // nb: the warehouse returned value for a timestamp is a JS Date, but with + // some extra methods attached to it. These methods include toString, so + // we cast back to a proper Date to avoid the string representation + // changing (e.g., when serializing to JSON). + windowStart: new Date(row.TS_START_INCLUSIVE as string | number | Date), + passCount: row.NUM_PASSES as number, + passingUsersCount: row.NUM_DISTINCT_USERS as number, + runsCount: row.NUM_RUNS as number, + }; + }); }; export default inject( diff --git a/server/storage/dataWarehouse/ClickhouseAdapter.ts b/server/storage/dataWarehouse/ClickhouseAdapter.ts index 1f1615a..70b8a74 100644 --- a/server/storage/dataWarehouse/ClickhouseAdapter.ts +++ b/server/storage/dataWarehouse/ClickhouseAdapter.ts @@ -104,6 +104,7 @@ function createDialect(client: ClickHouseClient): Dialect { createAdapter(): DialectAdapter { return new PostgresAdapter(); }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Kysely's Dialect interface hardcodes Kysely for createIntrospector createIntrospector(db: Kysely) { return new PostgresIntrospector(db); }, @@ -112,8 +113,7 @@ function createDialect(client: ClickHouseClient): Dialect { export class ClickhouseKyselyAdapter implements IDataWarehouseDialect { private readonly client: ClickHouseClient; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private readonly kysely: Kysely; + private readonly kysely: Kysely>; constructor( connectionSettings: ClickhouseConnectionSettings, @@ -136,13 +136,14 @@ export class ClickhouseKyselyAdapter implements IDataWarehouseDialect { }, }); - this.kysely = new Kysely({ + this.kysely = new Kysely>({ dialect: createDialect(this.client), }); } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getKyselyInstance(): Kysely { + // The interface declares Kysely but we implement with a concrete schema type; + // callers that need a specific schema cast the result themselves. + getKyselyInstance(): Kysely> { return this.kysely; } diff --git a/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts b/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts index 9744907..520a064 100644 --- a/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts +++ b/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts @@ -91,38 +91,54 @@ export type AnalyticsSchema = { error_message?: string; }; + // Written by ContentApiLogger. Keep this shape in sync with the + // `bulkWrite('CONTENT_API_REQUESTS', ...)` call there so the analytics + // adapter can type-check the row contents at compile time. CONTENT_API_REQUESTS: { ds: string; ts: number; org_id: string; item_id: string; + item_data: string; item_type_id: string; - item_submission_id?: string; + item_type_kind: string; + item_type_name: string; + item_type_version: string; + item_type_schema: string; + item_type_schema_variant: string; + item_type_schema_field_roles?: Record; item_creator_id?: string; - endpoint: string; - method: string; - correlation_id: string; - duration_ms: number; - failed: boolean; - error_message?: string; + item_creator_type_id?: string; + request_id: string; + submission_id: string; + event: 'REQUEST_SUCCEEDED' | 'REQUEST_FAILED'; + failure_reason?: string; }; // Operational tables (using warehouse for operational data) 'REPORTING_SERVICE.REPORTS': { [key: string]: unknown; // Dynamic schema }; - + 'REPORTING_SERVICE.APPEALS': { [key: string]: unknown; // Dynamic schema }; - + 'USER_STATISTICS_SERVICE.USER_SCORES': { [key: string]: unknown; // Dynamic schema }; - + 'USER_STATISTICS_SERVICE.SUBMISSION_STATS': { [key: string]: unknown; // Dynamic schema }; + + 'REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS': { + [key: string]: unknown; // Logged by ReportingRuleExecutionLogger + }; + + 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS': { + [key: string]: unknown; // Logged by RoutingRuleExecutionLogger + }; }; /** @@ -222,7 +238,7 @@ export const ANALYTICS_SCHEMA_DOCS = { }, ACTION_EXECUTIONS: { description: 'Logs every action execution (moderation actions taken)', - partitionKey: 'ds', + partitionKey: 'ds', sortKey: 'ts', indexes: ['org_id', 'action_id', 'item_id'], }, @@ -272,4 +288,3 @@ CREATE TABLE rule_executions ( PARTITION BY ds ORDER BY (ds, ts, org_id); `; - diff --git a/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts b/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts index 74dce5a..674073f 100644 --- a/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts +++ b/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts @@ -11,7 +11,7 @@ * - See ../README.md for implementation guide */ -import { sql, type Kysely } from 'kysely'; +import { sql, type InsertObject, type Kysely } from 'kysely'; import type SafeTracer from '../../utils/SafeTracer.js'; import { type IDataWarehouseAnalytics, @@ -26,9 +26,14 @@ import { * Uses batch inserts and logical replication for CDC */ export class PostgresAnalyticsAdapter implements IDataWarehouseAnalytics { - private pendingWrites: Map = new Map(); + // Each entry pairs a row buffer with a typed flush closure captured while + // TableName is in scope in bulkWrite, so flushTable never needs a cast. + private pendingWrites = new Map< + string, + { rows: unknown[]; flush: (rows: unknown[]) => Promise } + >(); - constructor(private readonly kysely: Kysely) {} + constructor(private readonly kysely: Kysely) {} async bulkWrite( tableName: TableName, @@ -38,14 +43,24 @@ export class PostgresAnalyticsAdapter implements IDataWarehouseAnalytics { const tableKey = tableName as string; if (!this.pendingWrites.has(tableKey)) { - this.pendingWrites.set(tableKey, []); + this.pendingWrites.set(tableKey, { + rows: [], + // Closure captures the concrete TableName so Kysely can type-check + // the insert at the call site where the generic is still in scope. + flush: async (r) => { + await this.kysely + .insertInto(tableName) + .values(r as ReadonlyArray>) + .execute(); + }, + }); } - this.pendingWrites.get(tableKey)!.push(...rows); + this.pendingWrites.get(tableKey)!.rows.push(...rows); const batchSize = config?.batchSize ?? 500; const pending = this.pendingWrites.get(tableKey)!; - if (config?.batchTimeout === 0 || pending.length >= batchSize) { + if (config?.batchTimeout === 0 || pending.rows.length >= batchSize) { await this.flushTable(tableKey); } } @@ -87,36 +102,36 @@ export class PostgresAnalyticsAdapter implements IDataWarehouseAnalytics { } private async flushTable(tableName: string): Promise { - const rows = this.pendingWrites.get(tableName); - if (!rows || rows.length === 0) return; + const pending = this.pendingWrites.get(tableName); + if (!pending || pending.rows.length === 0) return; - await this.kysely.insertInto(tableName as any).values(rows).execute(); - this.pendingWrites.set(tableName, []); + await pending.flush(pending.rows); + pending.rows = []; } // Stub implementations - integrators must implement these - logActionExecutions = async (..._args: any[]): Promise => { + logActionExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logRuleExecutions = async (..._args: any[]): Promise => { + logRuleExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logItemModelScore = async (..._args: any[]): Promise => { + logItemModelScore = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logReportingRuleExecutions = async (..._args: any[]): Promise => { + logReportingRuleExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logContentApiRequest = async (..._args: any[]): Promise => { + logContentApiRequest = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logContentDetailsApiRequest = async (..._args: any[]): Promise => { + logContentDetailsApiRequest = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logRoutingRuleExecutions = async (..._args: any[]): Promise => { + logRoutingRuleExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logOrgCreation = async (..._args: any[]): Promise => { + logOrgCreation = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; } diff --git a/server/test/extendExpect.ts b/server/test/extendExpect.ts index 65a4afb..9717581 100644 --- a/server/test/extendExpect.ts +++ b/server/test/extendExpect.ts @@ -1,5 +1,5 @@ // eslint-disable-next-line import/no-extraneous-dependencies -import jestSnapshot from 'jest-snapshot'; +import jestSnapshot, { type Context as SnapshotContext } from 'jest-snapshot'; import { JSONPath } from 'jsonpath-plus'; import lodash from 'lodash'; @@ -41,7 +41,11 @@ declare global { } expect.extend({ - toMatchDynamicSnapshot(received, propertyMatchers: object, hint?: string) { + toMatchDynamicSnapshot( + received, + propertyMatchers: object, + hint?: string, + ): jest.CustomMatcherResult | Promise { // Treat property matcher keys as jsonpath queries // if they start with a $ and contain a dot. const isJsonPath = (it: string) => it[0] === '$' && it.includes('.'); @@ -63,8 +67,8 @@ expect.extend({ } }); - return (toMatchSnapshot as any).call( - this, + return toMatchSnapshot.call( + this as SnapshotContext, received, generatedPropertyMatchers, hint, diff --git a/server/utils/errors.ts b/server/utils/errors.ts index 08bdb0d..9185fcd 100644 --- a/server/utils/errors.ts +++ b/server/utils/errors.ts @@ -261,6 +261,9 @@ export type CoopErrorName = | 'BadRequestError' | 'UnauthenticatedError' | 'UnauthorizedError' + // apiKey errors + | 'RotateApiKeyError' + | 'RotateWebhookSigningKeyError' // gql mutation errors | UserErrorType | IntegrationErrorType diff --git a/server/utils/sql.ts b/server/utils/sql.ts index 3b66f1b..0144087 100644 --- a/server/utils/sql.ts +++ b/server/utils/sql.ts @@ -65,9 +65,5 @@ export function takeLast< for (const it of sortCriteria) { outer = outer.orderBy(it.column, it.order); } - return outer as SelectQueryBuilder< - DB & { [K in typeof SUBQUERY_ALIAS]: O }, - typeof SUBQUERY_ALIAS, - O - >; + return outer; } diff --git a/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts b/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts index c03b76e..d3e3c86 100644 --- a/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts +++ b/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts @@ -25,11 +25,14 @@ export default inject( throw new Error('No last_insert timestamp found for the table'); } - // When last_insert is null (fresh deploy), do a full backfill - const isInitialBackfill = lastTimestamp.last_insert == null; - const oneMinutePrevious = isInitialBackfill - ? new Date(0) - : new Date(lastTimestamp.last_insert.valueOf() - MINUTE_MS); + // On a fresh deploy the row is seeded with Postgres `-infinity`, + // which `pg-types` parses to the JS number `-Infinity`. In that case + // `new Date(-Infinity - MINUTE_MS)` would be an Invalid Date, so fall + // back to epoch and do a full backfill. + const lastInsertMs = lastTimestamp.last_insert.valueOf(); + const oneMinutePrevious = Number.isFinite(lastInsertMs) + ? new Date(lastInsertMs - MINUTE_MS) + : new Date(0); const insertedRows = await trx .insertInto('manual_review_tool.dim_mrt_decisions_materialized') diff --git a/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts b/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts index a554b72..18d9df6 100644 --- a/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts +++ b/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts @@ -53,10 +53,7 @@ export default inject( const orgId = row.org_id; const itemId = row.job_payload.payload.item.itemId; const itemTypeId = row.job_payload.payload.item.itemTypeIdentifier.id; - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - if (usersByOrg[orgId] === undefined) { - usersByOrg[orgId] = await userManagementService.getUsersForOrg(orgId); - } + usersByOrg[orgId] ??= await userManagementService.getUsersForOrg(orgId); const user = usersByOrg[orgId].find((it) => it.id === itemId); if (itemType === undefined || itemType.kind !== 'USER') { await ncmecService.insertOrUpdateNcmecReportError({ -- 2.51.2