From 326348d89630d52a0bb4c6a0852d60c5169770c7 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 03:29:51 -0700 Subject: [PATCH 1/6] feat(selfhost): add a modular moderation-rules engine with a cross-repo violation ledger Adds a single, config-driven moderation layer over the three existing per-PR anti-abuse mechanisms (contributor cap, blacklist, review-nag): every time one of them fires against a non-exempt contributor, it now counts toward that login's install-wide violation tally. At >=1 lifetime violation the contributor gets a configurable "warning" label; at the configured ban threshold (default 5) they get a "banned" label and, when enabled, are auto-added to the existing global contributor blacklist. - global_moderation_config: a new singleton table holding the whole-layer on/off switch, which of the three rules participate, label text, ban threshold, an optional violation-decay window (permanent by default), and whether a ban auto-enforces. - Per-repo overrides in repository_settings (moderationGateMode, moderationRules, moderationWarningLabel, moderationBannedLabel), wired through the full settings pipeline: migration, Drizzle schema, RepositorySettings, .gittensory.yml parsing, OpenAPI. - The violation ledger reuses the existing audit_events table (install-wide by construction, same shape review-nag's own cooldown counter already uses) rather than a new table. - A single convergence point in the PR/issue action executors: every anti-abuse close already tags itself with a closeKind (blacklist / contributor_cap / review_nag), so escalation hooks in there instead of duplicating wiring at each mechanism's own several call sites. - Fully generic: no repo-specific values are hardcoded into the engine. A self-hoster's own deployment config chooses everything; the whole layer defaults OFF until an operator opts in. --- apps/gittensory-ui/public/openapi.json | 79 +++------ migrations/0102_global_moderation_config.sql | 21 +++ .../0103_repository_moderation_settings.sql | 10 ++ scripts/check-schema-drift.mjs | 1 + src/db/repositories.ts | 131 ++++++++++++++ src/db/schema.ts | 7 + src/openapi/schemas.ts | 4 + src/queue/processors.ts | 30 +++- src/services/agent-action-executor.ts | 100 ++++++++++- src/settings/moderation-rules.ts | 126 ++++++++++++++ src/signals/focus-manifest.ts | 22 +++ src/types.ts | 15 ++ test/unit/agent-action-executor.test.ts | 140 ++++++++++++++- test/unit/focus-manifest.test.ts | 27 +++ test/unit/moderation-config-db.test.ts | 162 ++++++++++++++++++ test/unit/moderation-rules.test.ts | 136 +++++++++++++++ 16 files changed, 949 insertions(+), 62 deletions(-) create mode 100644 migrations/0102_global_moderation_config.sql create mode 100644 migrations/0103_repository_moderation_settings.sql create mode 100644 src/settings/moderation-rules.ts create mode 100644 test/unit/moderation-config-db.test.ts create mode 100644 test/unit/moderation-rules.test.ts diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 8ffbdae6fd..fbc0106837 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8970,65 +8970,30 @@ "typeLabelsEnabled": { "type": "boolean" }, - "typeLabels": { - "type": "object", - "properties": { - "bug": { - "type": "string" - }, - "feature": { - "type": "string" - }, - "priority": { - "type": "string" - } - }, - "required": [ - "bug", - "feature", - "priority" + "moderationGateMode": { + "type": "string", + "enum": [ + "inherit", + "off", + "enabled" ] }, - "linkedIssueLabelPropagation": { - "type": "object", - "properties": { - "enabled": { - "type": "boolean" - }, - "mode": { - "type": "string", - "enum": [ - "exclusive_type_label" - ] - }, - "mappings": { - "type": "array", - "items": { - "type": "object", - "properties": { - "issueLabel": { - "type": "string" - }, - "prLabel": { - "type": "string" - }, - "removeOtherTypeLabels": { - "type": "boolean" - } - }, - "required": [ - "issueLabel", - "prLabel", - "removeOtherTypeLabels" - ] - } - } - }, - "required": [ - "enabled", - "mode", - "mappings" - ] + "moderationRules": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "contributor_cap", + "blacklist", + "review_nag" + ] + } + }, + "moderationWarningLabel": { + "type": "string" + }, + "moderationBannedLabel": { + "type": "string" } }, "required": [ diff --git a/migrations/0102_global_moderation_config.sql b/migrations/0102_global_moderation_config.sql new file mode 100644 index 0000000000..3f9563ab92 --- /dev/null +++ b/migrations/0102_global_moderation_config.sql @@ -0,0 +1,21 @@ +-- Global moderation-rules engine config (#selfhost-mod-engine): singleton row, mirroring +-- global_contributor_blacklist and global_agent_controls's shape (one row, id = 'singleton'). Off by default +-- (enabled = 0) -- zero behavior change for an install that hasn't opted in. rules_json is the DEFAULT set of +-- the three existing anti-abuse mechanisms (contributor cap, blacklist, review-nag) that count toward a +-- contributor's shared, cross-repo violation tally; a repo can override its own participating rules via +-- repository_settings.moderation_rules_json. violation_decay_days is nullable: null = a permanent, never- +-- decaying lifetime tally (the default, matching the existing global-blacklist's permanent-ban philosophy); +-- a positive integer = only violations within that many days count toward ban_threshold. +CREATE TABLE IF NOT EXISTS global_moderation_config ( + id TEXT PRIMARY KEY, + enabled INTEGER NOT NULL DEFAULT 0, + rules_json TEXT NOT NULL DEFAULT '["contributor_cap","blacklist","review_nag"]', + warning_label TEXT NOT NULL DEFAULT 'mod:warning', + banned_label TEXT NOT NULL DEFAULT 'mod:banned', + ban_threshold INTEGER NOT NULL DEFAULT 5, + violation_decay_days INTEGER, + auto_blacklist_on_ban INTEGER NOT NULL DEFAULT 1, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_by TEXT +); +INSERT OR IGNORE INTO global_moderation_config (id) VALUES ('singleton'); diff --git a/migrations/0103_repository_moderation_settings.sql b/migrations/0103_repository_moderation_settings.sql new file mode 100644 index 0000000000..f1c9dc1883 --- /dev/null +++ b/migrations/0103_repository_moderation_settings.sql @@ -0,0 +1,10 @@ +-- Per-repo overrides for the moderation-rules engine (#selfhost-mod-engine), layered over +-- global_moderation_config (0101). moderation_gate_mode defaults to 'inherit' (defers to the global master +-- switch) -- 'off'/'enabled' let one repo opt out of or into the whole layer regardless of the global default, +-- e.g. an operator piloting the feature on a single repo before flipping the global default on. The three +-- override columns are nullable: NULL means "inherit the global value", never "unset to empty/off" -- an +-- explicit repo-level empty rules list would be indistinguishable from "not configured" otherwise. +ALTER TABLE repository_settings ADD COLUMN moderation_gate_mode TEXT NOT NULL DEFAULT 'inherit'; +ALTER TABLE repository_settings ADD COLUMN moderation_rules_json TEXT; +ALTER TABLE repository_settings ADD COLUMN moderation_warning_label TEXT; +ALTER TABLE repository_settings ADD COLUMN moderation_banned_label TEXT; diff --git a/scripts/check-schema-drift.mjs b/scripts/check-schema-drift.mjs index 119524becf..e686f6d596 100755 --- a/scripts/check-schema-drift.mjs +++ b/scripts/check-schema-drift.mjs @@ -39,6 +39,7 @@ const MIGRATIONS_DIR = process.env.CHECK_SCHEMA_DRIFT_DIR || "migrations"; export const RAW_SQL_ONLY_TABLES = new Set([ "global_agent_controls", "global_contributor_blacklist", + "global_moderation_config", "orb_enrollments", "orb_export_cursor", "orb_github_installations", diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 6f4b54d7d7..c723bd744c 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -163,6 +163,7 @@ import { classifyMcpClientVersion, LATEST_RECOMMENDED_MCP_VERSION, MINIMUM_SUPPO import { DEFAULT_COMMAND_AUTHORIZATION_POLICY, normalizeCommandAuthorizationPolicy } from "../settings/command-authorization"; import { normalizeContributorBlacklist } from "../settings/contributor-blacklist"; import { normalizeAutoCloseExemptLogins } from "../settings/auto-close-exempt"; +import { DEFAULT_GLOBAL_MODERATION_CONFIG, normalizeModerationLabel, normalizeModerationRules, type GlobalModerationConfig, type ModerationRuleType } from "../settings/moderation-rules"; import { normalizeAutonomyPolicy, normalizeAutoMaintainPolicy, DEFAULT_AUTO_MAINTAIN_POLICY } from "../settings/autonomy"; import { DEFAULT_TYPE_LABELS, normalizeTypeLabelSet } from "../settings/pr-type-label"; import { DEFAULT_LINKED_ISSUE_LABEL_PROPAGATION, normalizeLinkedIssueLabelPropagationConfig } from "../review/linked-issue-label-propagation"; @@ -525,6 +526,10 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise commandRateLimitMaxPerWindow: 20, commandRateLimitAiMaxPerWindow: 5, commandRateLimitWindowHours: 24, + moderationGateMode: "inherit", + moderationRules: undefined, + moderationWarningLabel: undefined, + moderationBannedLabel: undefined, }; } return { @@ -589,6 +594,10 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise commandRateLimitMaxPerWindow: normalizePositiveIntWithDefault(row.commandRateLimitMaxPerWindow, 20), commandRateLimitAiMaxPerWindow: normalizePositiveIntWithDefault(row.commandRateLimitAiMaxPerWindow, 5), commandRateLimitWindowHours: normalizePositiveIntWithDefault(row.commandRateLimitWindowHours, 24), + moderationGateMode: normalizeModerationGateMode(row.moderationGateMode), + moderationRules: parseModerationRulesColumn(row.moderationRulesJson), + moderationWarningLabel: normalizeModerationLabel(row.moderationWarningLabel), + moderationBannedLabel: normalizeModerationLabel(row.moderationBannedLabel), createdAt: row.createdAt, updatedAt: row.updatedAt, }; @@ -689,6 +698,10 @@ export async function upsertRepositorySettings(env: Env, settings: Partial { + const db = getDb(env.DB); + const conditions = [eq(auditEvents.actor, actor), inArray(auditEvents.eventType, eventTypes)]; + if (sinceIso !== undefined) conditions.push(gte(auditEvents.createdAt, sinceIso)); + const [row] = await db + .select({ count: sql`count(*)` }) + .from(auditEvents) + .where(and(...conditions)); + /* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */ + if (!row) return 0; + return row.count; +} + +/** Moderation-rules engine: record one violation for `actor` under the given rule's `eventType` (see + * `MODERATION_VIOLATION_EVENT_TYPE` in settings/moderation-rules.ts). `targetKey` carries the repo#number for + * audit-trail/evidence purposes only -- the COUNT query above deliberately does not scope by it. */ +export async function recordModerationViolation(env: Env, args: { eventType: string; actor: string; targetKey: string; repoFullName: string; ruleReason: string }): Promise { + await recordAuditEvent(env, { + eventType: args.eventType, + actor: args.actor, + targetKey: args.targetKey, + outcome: "completed", + detail: args.ruleReason, + metadata: { repoFullName: args.repoFullName }, + }); +} + +/** Read the singleton global moderation-rules engine config (#selfhost-mod-engine). Missing table or malformed + * JSON fail open to {@link DEFAULT_GLOBAL_MODERATION_CONFIG} (`enabled: false`) -- a DB hiccup on this path + * must never accidentally turn ON a layer capable of auto-banning a contributor across every gated repo. */ +export async function getGlobalModerationConfig(env: Env): Promise { + try { + const row = await env.DB.prepare( + "SELECT enabled, rules_json, warning_label, banned_label, ban_threshold, violation_decay_days, auto_blacklist_on_ban FROM global_moderation_config WHERE id = 'singleton'", + ).first<{ + enabled: number; + rules_json: string; + warning_label: string; + banned_label: string; + ban_threshold: number; + violation_decay_days: number | null; + auto_blacklist_on_ban: number; + }>(); + if (!row) return DEFAULT_GLOBAL_MODERATION_CONFIG; + return { + enabled: row.enabled === 1, + rules: normalizeModerationRules(parseJson(row.rules_json, null)).rules, + warningLabel: normalizeModerationLabel(row.warning_label) ?? DEFAULT_GLOBAL_MODERATION_CONFIG.warningLabel, + bannedLabel: normalizeModerationLabel(row.banned_label) ?? DEFAULT_GLOBAL_MODERATION_CONFIG.bannedLabel, + banThreshold: normalizePositiveIntWithDefault(row.ban_threshold, DEFAULT_GLOBAL_MODERATION_CONFIG.banThreshold), + violationDecayDays: normalizeOpenItemCap(row.violation_decay_days), + autoBlacklistOnBan: row.auto_blacklist_on_ban === 1, + }; + } catch { + return DEFAULT_GLOBAL_MODERATION_CONFIG; + } +} + +/** Upsert the singleton global moderation-rules engine config. Input is normalized/validated once so malformed + * stored data never reaches enforcement. Returns the normalized persisted config for convenience/tests. */ +export async function upsertGlobalModerationConfig( + env: Env, + input: Partial & { updatedBy?: string | null }, +): Promise { + const current = await getGlobalModerationConfig(env); + const resolved: GlobalModerationConfig = { + enabled: input.enabled ?? current.enabled, + rules: input.rules ? normalizeModerationRules(input.rules as unknown).rules : current.rules, + warningLabel: normalizeModerationLabel(input.warningLabel) ?? current.warningLabel, + bannedLabel: normalizeModerationLabel(input.bannedLabel) ?? current.bannedLabel, + banThreshold: input.banThreshold !== undefined ? normalizePositiveIntWithDefault(input.banThreshold, current.banThreshold) : current.banThreshold, + violationDecayDays: input.violationDecayDays !== undefined ? normalizeOpenItemCap(input.violationDecayDays) : current.violationDecayDays, + autoBlacklistOnBan: input.autoBlacklistOnBan ?? current.autoBlacklistOnBan, + }; + await env.DB.prepare( + "INSERT INTO global_moderation_config (id, enabled, rules_json, warning_label, banned_label, ban_threshold, violation_decay_days, auto_blacklist_on_ban, updated_at, updated_by) VALUES ('singleton', ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, ?) ON CONFLICT(id) DO UPDATE SET enabled = excluded.enabled, rules_json = excluded.rules_json, warning_label = excluded.warning_label, banned_label = excluded.banned_label, ban_threshold = excluded.ban_threshold, violation_decay_days = excluded.violation_decay_days, auto_blacklist_on_ban = excluded.auto_blacklist_on_ban, updated_at = excluded.updated_at, updated_by = excluded.updated_by", + ) + .bind( + resolved.enabled ? 1 : 0, + jsonString(resolved.rules), + resolved.warningLabel, + resolved.bannedLabel, + resolved.banThreshold, + resolved.violationDecayDays, + resolved.autoBlacklistOnBan ? 1 : 0, + input.updatedBy ?? null, + ) + .run(); + return resolved; +} + /** Whether `deliveryId` has ALREADY been recorded for this (actor, eventType, targetKey) within `sinceIso` -- * makes a counting/rate-limit check idempotent against a REDELIVERED or retried webhook event (GitHub can * and does redeliver the same issue_comment event), which would otherwise increment the counter twice for @@ -6005,6 +6123,19 @@ function normalizeCommandRateLimitPolicy(value: string | null | undefined): "off return value === "hold" ? value : "off"; } +function normalizeModerationGateMode(value: string | null | undefined): "inherit" | "off" | "enabled" { + return value === "off" || value === "enabled" ? value : "inherit"; +} + +// NULL means "inherit the global rule set" (undefined), distinct from a normalized-but-empty list -- a repo +// that explicitly configured an empty moderationRules override (opting every rule out) must stay empty, not +// be coerced back to "inherit". Mirrors parseContributorBlacklist/parseAutoCloseExemptLogins's JSON-parse +// shape, except the column itself (not just malformed JSON) can be genuinely absent. +function parseModerationRulesColumn(value: string | null | undefined): RepositorySettings["moderationRules"] { + if (value === null || value === undefined) return undefined; + return normalizeModerationRules(parseJson(value, null)).rules; +} + // A review-nag threshold/window is a discrete positive count, not a score — reuses the same non-clamping, // non-rounding shape as contributorOpenPrCap's normalizeOpenItemCap (#2270): an invalid value (fractional, // non-positive, non-finite) falls back to the given default rather than being silently coerced. diff --git a/src/db/schema.ts b/src/db/schema.ts index 9c1b334e7a..b775040dda 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -137,6 +137,13 @@ export const repositorySettings = sqliteTable("repository_settings", { commandRateLimitMaxPerWindow: integer("command_rate_limit_max_per_window").notNull().default(20), commandRateLimitAiMaxPerWindow: integer("command_rate_limit_ai_max_per_window").notNull().default(5), commandRateLimitWindowHours: integer("command_rate_limit_window_hours").notNull().default(24), + // Moderation-rules engine (#selfhost-mod-engine): per-repo overrides layered over global_moderation_config. + // 'inherit' (default) defers to the global master switch; 'off'/'enabled' force this repo regardless of it. + moderationGateMode: text("moderation_gate_mode").notNull().default("inherit"), + // Nullable: null = inherit the global rule set / label text, never "unset to empty". + moderationRulesJson: text("moderation_rules_json"), + moderationWarningLabel: text("moderation_warning_label"), + moderationBannedLabel: text("moderation_banned_label"), createdAt: text("created_at").notNull().$defaultFn(() => nowIso()), updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()), }); diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index e96f06288e..0c2a7d1ab9 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -678,6 +678,10 @@ export const RepositorySettingsSchema = z commandRateLimitMaxPerWindow: z.number().int().positive().optional(), commandRateLimitAiMaxPerWindow: z.number().int().positive().optional(), commandRateLimitWindowHours: z.number().int().positive().optional(), + moderationGateMode: z.enum(["inherit", "off", "enabled"]).optional(), + moderationRules: z.array(z.enum(["contributor_cap", "blacklist", "review_nag"])).optional(), + moderationWarningLabel: z.string().optional(), + moderationBannedLabel: z.string().optional(), createdAt: z.string().nullable().optional(), updatedAt: z.string().nullable().optional(), }) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 4b034dc796..021b0e5484 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2220,6 +2220,12 @@ async function runAgentMaintenancePlanAndExecute( // CI-run cancellation on a contributor_cap close (#2462): the repo's own explicit setting always wins; // null/undefined (unset) falls back to the install-wide CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT env var. contributorCapCancelCi: settings.contributorCapCancelCi ?? env.CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT === "true", + moderationSettings: { + moderationGateMode: settings.moderationGateMode, + moderationRules: settings.moderationRules, + moderationWarningLabel: settings.moderationWarningLabel, + moderationBannedLabel: settings.moderationBannedLabel, + }, }, breakerOnPlan, ); @@ -4095,7 +4101,16 @@ async function maybeCloseIssueOverContributorCap( if (planned.length > 0) { await executeIssueMaintenanceActions( env, - { installationId, repoFullName, issueNumber: issue.number, autonomy: settings.autonomy, agentPaused: settings.agentPaused, agentDryRun: settings.agentDryRun }, + { + installationId, + repoFullName, + issueNumber: issue.number, + autonomy: settings.autonomy, + agentPaused: settings.agentPaused, + agentDryRun: settings.agentDryRun, + authorLogin, + moderationSettings: { moderationGateMode: settings.moderationGateMode, moderationRules: settings.moderationRules, moderationWarningLabel: settings.moderationWarningLabel, moderationBannedLabel: settings.moderationBannedLabel }, + }, planned, ); } @@ -4164,7 +4179,16 @@ async function maybeCloseIssueOverContributorCap( for (const overCapNumber of overCapNumbers) { await executeIssueMaintenanceActions( env, - { installationId, repoFullName, issueNumber: overCapNumber, autonomy: settings.autonomy, agentPaused: settings.agentPaused, agentDryRun: settings.agentDryRun }, + { + installationId, + repoFullName, + issueNumber: overCapNumber, + autonomy: settings.autonomy, + agentPaused: settings.agentPaused, + agentDryRun: settings.agentDryRun, + authorLogin, + moderationSettings: { moderationGateMode: settings.moderationGateMode, moderationRules: settings.moderationRules, moderationWarningLabel: settings.moderationWarningLabel, moderationBannedLabel: settings.moderationBannedLabel }, + }, planned, ); } @@ -9468,6 +9492,7 @@ async function maybeThrottleReviewNagPing( agentDryRun: settings.agentDryRun, installationPermissions: installation?.permissions ?? null, authorLogin: pr.authorLogin, + moderationSettings: { moderationGateMode: settings.moderationGateMode, moderationRules: settings.moderationRules, moderationWarningLabel: settings.moderationWarningLabel, moderationBannedLabel: settings.moderationBannedLabel }, }, planned, ); @@ -9632,6 +9657,7 @@ async function maybeThrottleMonitoredMentions( agentDryRun: settings.agentDryRun, installationPermissions: installation?.permissions ?? null, authorLogin: pr.authorLogin, + moderationSettings: { moderationGateMode: settings.moderationGateMode, moderationRules: settings.moderationRules, moderationWarningLabel: settings.moderationWarningLabel, moderationBannedLabel: settings.moderationBannedLabel }, }, planned, ); diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index f3cf3d5bde..772b6bf879 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -1,4 +1,18 @@ -import { bumpPullRequestMergeAttempt, createPendingAgentActionIfAbsent, insertNotificationDeliveryIfAbsent, isGlobalAgentFrozen, markPullRequestApproved, markPullRequestMergeBlocked, recordAuditEvent } from "../db/repositories"; +import { + bumpPullRequestMergeAttempt, + countModerationViolationsForActor, + createPendingAgentActionIfAbsent, + getGlobalContributorBlacklist, + getGlobalModerationConfig, + insertNotificationDeliveryIfAbsent, + isGlobalAgentFrozen, + markPullRequestApproved, + markPullRequestMergeBlocked, + recordAuditEvent, + recordModerationViolation, + upsertGlobalContributorBlacklist, +} from "../db/repositories"; +import { isAuthorBlacklisted } from "../settings/contributor-blacklist"; import { classifyMergeFailure, MERGE_RETRY_CAP } from "./merge-failure"; import { notifyActionToDiscord, notifyActionToSlack, type NotifyOutcome } from "./notify-discord"; import { cancelInFlightWorkflowRunsForHeadSha, createInstallationToken, githubErrorStatus, isGitHubRateLimitedError } from "../github/app"; @@ -8,11 +22,18 @@ import { ensurePullRequestLabel, removePullRequestLabel } from "../github/labels import { closeIssue, closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../github/pr-actions"; import { fetchPullRequestFreshness, pullRequestFreshnessDetail } from "../github/pr-freshness"; import { isActingAutonomyLevel, resolveAutonomy } from "../settings/autonomy"; -import { buildAgentActionAudit, isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness } from "../settings/agent-execution"; +import { buildAgentActionAudit, isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness, type AgentActionMode } from "../settings/agent-execution"; import type { PlannedAgentAction } from "../settings/agent-actions"; import type { AgentActionClass, AgentPendingActionParams, AutonomyLevel, AutonomyPolicy } from "../types"; import { errorMessage } from "../utils/json"; import { AGENT_LABEL_PENDING_CLOSURE } from "../review/linked-issue-hard-rules"; +import { + MODERATION_VIOLATION_EVENT_TYPE, + moderationTierForViolationCount, + resolveEffectiveModerationRules, + resolveModerationGateEnabled, + type ModerationRuleType, +} from "../settings/moderation-rules"; // The agent actor name on every audit record — the App acts on the maintainer's behalf per their configured // autonomy (the config IS the authorization; there is no human commenter to authorize, unlike #824). @@ -51,6 +72,20 @@ export type AgentActionExecutionContext = { // ?? the CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT env var) before building the context — the executor itself has no // settings access, only whatever ctx carries, mirroring how agentPaused/agentDryRun are already threaded in. contributorCapCancelCi?: boolean | undefined; + // Moderation-rules engine (#selfhost-mod-engine): the repo's PER-REPO override fields, resolved by the + // CALLER from RepositorySettings before building the context (same "the executor has no settings access" + // shape as contributorCapCancelCi above). Absent/undefined ⇒ inherit the global config's own defaults. The + // GLOBAL config itself (whole-layer enabled, threshold, decay, auto-blacklist) is read directly by the + // executor via getGlobalModerationConfig -- a single extra DB read only on the rare path where a + // moderation-tracked close actually completed, not threaded through every caller. + moderationSettings?: ModerationContextSettings | undefined; +}; + +export type ModerationContextSettings = { + moderationGateMode?: "inherit" | "off" | "enabled" | undefined; + moderationRules?: ModerationRuleType[] | undefined; + moderationWarningLabel?: string | undefined; + moderationBannedLabel?: string | undefined; }; export type AgentActionOutcome = { @@ -255,9 +290,66 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE } } + await maybeEscalateModeration(env, { installationId: ctx.installationId, repoFullName: ctx.repoFullName, number: ctx.pullNumber, authorLogin: ctx.authorLogin, mode, moderationSettings: ctx.moderationSettings }, planned, outcomes); return outcomes; } +const MODERATION_RULE_TYPES = new Set(Object.keys(MODERATION_VIOLATION_EVENT_TYPE)); + +/** + * Moderation-rules engine (#selfhost-mod-engine): a SINGLE convergence point for all three anti-abuse + * mechanisms (blacklist, contributor cap, review-nag) that already tag their `close` action with a matching + * `closeKind` -- rather than duplicating this wiring at every one of their several call sites in + * `queue/processors.ts`, this scans the JUST-EXECUTED plan for a moderation-tracked close that actually + * COMPLETED (not denied/queued/dry-run -- an action that didn't really happen must not count as a violation) + * and, if so, records one violation + escalates. Never throws: every write here is best-effort, matching how + * the rest of this file treats CI-cancellation/notification side effects as non-critical to the close itself. + * A no-op in `dry_run`/`paused` mode (no label/ban side effects for a mutation that didn't really happen). + */ +async function maybeEscalateModeration( + env: Env, + args: { installationId: number; repoFullName: string; number: number; authorLogin?: string | null | undefined; mode: AgentActionMode; moderationSettings: ModerationContextSettings | undefined }, + planned: PlannedAgentAction[], + outcomes: AgentActionOutcome[], +): Promise { + if (!args.authorLogin || args.mode !== "live") return; + const index = planned.findIndex((action, i) => action.actionClass === "close" && action.closeKind !== undefined && MODERATION_RULE_TYPES.has(action.closeKind) && outcomes[i]?.outcome === "completed"); + const closeKind = index === -1 ? undefined : planned[index]?.closeKind; + if (closeKind === undefined) return; + const rule = closeKind as ModerationRuleType; + + const globalConfig = await getGlobalModerationConfig(env); + if (!resolveModerationGateEnabled(globalConfig.enabled, args.moderationSettings?.moderationGateMode ?? "inherit")) return; + const effectiveRules = resolveEffectiveModerationRules(globalConfig.rules, args.moderationSettings?.moderationRules); + if (!effectiveRules.includes(rule)) return; + + const targetKey = `${args.repoFullName}#${args.number}`; + await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE[rule], actor: args.authorLogin, targetKey, repoFullName: args.repoFullName, ruleReason: `${rule} violation` }).catch(() => undefined); + + const allEventTypes = Object.values(MODERATION_VIOLATION_EVENT_TYPE); + const sinceIso = globalConfig.violationDecayDays !== null ? new Date(Date.now() - globalConfig.violationDecayDays * 24 * 60 * 60 * 1000).toISOString() : undefined; + const totalCount = await countModerationViolationsForActor(env, args.authorLogin, allEventTypes, sinceIso); + const tier = moderationTierForViolationCount(totalCount, globalConfig.banThreshold); + /* v8 ignore next -- defensive: the violation just recorded above always makes totalCount >= 1 by the time + execution reaches here (the only way to see "none" is the record write itself silently failing, which + moderationTierForViolationCount's own unit tests already cover directly for count=0). */ + if (tier === "none") return; + + const label = tier === "banned" ? (args.moderationSettings?.moderationBannedLabel ?? globalConfig.bannedLabel) : (args.moderationSettings?.moderationWarningLabel ?? globalConfig.warningLabel); + await ensurePullRequestLabel(env, args.installationId, args.repoFullName, args.number, label, { createMissingLabel: true }).catch(() => undefined); + + if (tier === "banned" && globalConfig.autoBlacklistOnBan) { + /* v8 ignore next -- getGlobalContributorBlacklist never actually resolves undefined (it fails open to + `[]`); the `?? []` only satisfies RepositorySettings["contributorBlacklist"]'s optional TS type. */ + const current = (await getGlobalContributorBlacklist(env)) ?? []; + if (!isAuthorBlacklisted(args.authorLogin, current)) { + const banReason = `moderation-engine auto-ban: ${totalCount} lifetime violations reached the configured threshold`; + const nextBlacklist = [...current, { login: args.authorLogin, reason: banReason, evidence: [targetKey] }]; + await upsertGlobalContributorBlacklist(env, { contributorBlacklist: nextBlacklist }).catch(() => undefined); + } + } +} + /** CI-run cancellation on a contributor_cap close (#2462): runs cancelInFlightWorkflowRunsForHeadSha and * records exactly one of two audit outcomes, mirroring the established `github_app.*_permission_missing` * convention (processors.ts's check-run/gate-check permission-missing audits) so a fleet-wide actions:write @@ -314,6 +406,9 @@ export type IssueActionExecutionContext = { autonomy: AutonomyPolicy | null | undefined; agentPaused?: boolean | undefined; agentDryRun?: boolean | undefined; + // Issue author login -- needed for the moderation-rules engine's violation ledger (#selfhost-mod-engine). + authorLogin?: string | null | undefined; + moderationSettings?: ModerationContextSettings | undefined; }; /** @@ -387,6 +482,7 @@ export async function executeIssueMaintenanceActions(env: Env, ctx: IssueActionE } } + await maybeEscalateModeration(env, { installationId: ctx.installationId, repoFullName: ctx.repoFullName, number: ctx.issueNumber, authorLogin: ctx.authorLogin, mode, moderationSettings: ctx.moderationSettings }, planned, outcomes); return outcomes; } diff --git a/src/settings/moderation-rules.ts b/src/settings/moderation-rules.ts new file mode 100644 index 0000000000..4dc6a1822b --- /dev/null +++ b/src/settings/moderation-rules.ts @@ -0,0 +1,126 @@ +// Centralized moderation-rules engine (generic self-host feature, #selfhost-mod-engine). A single modular +// layer over the three EXISTING anti-abuse mechanisms (contributor cap, blacklist, review-nag) that already +// short-circuit a PR's disposition: every time one of them fires against a non-exempt contributor, it counts +// toward that login's install-wide violation tally (the shared `audit_events` ledger, keyed by actor). At +// >=1 lifetime violation the contributor is labeled with `warningLabel`; at >=`banThreshold` they are labeled +// `bannedLabel` and (when `autoBlacklistOnBan`) auto-added to the existing global contributor blacklist -- +// the SAME "permanent two-strikes" enforcement an already-banned login gets. +// +// Config-as-code, layered the same as every other setting: a global default (the whole layer can be off, +// which rules count, the label text, the threshold, whether a ban auto-enforces) with a PER-REPO override +// that can turn the layer off/on for just that repo and override which rules feed IT specifically. NEVER +// hard-coded for any one repo -- a self-hoster's own `.gittensory.yml`/dashboard settings choose everything. + +/** The three EXISTING anti-abuse mechanisms this engine can count violations from. Kept as a closed union + * (not an open string) so an unrecognized value is always a normalization error, never silently accepted. */ +export type ModerationRuleType = "contributor_cap" | "blacklist" | "review_nag"; + +const ALL_MODERATION_RULE_TYPES: readonly ModerationRuleType[] = ["contributor_cap", "blacklist", "review_nag"]; + +/** The `audit_events.event_type` recorded for each rule's violation -- namespaced under `moderation.violation.*` + * so a cross-eventType, cross-repo count query (see `db/repositories.ts`) can scope to exactly this family. */ +export const MODERATION_VIOLATION_EVENT_TYPE: Record = { + contributor_cap: "moderation.violation.contributor_cap", + blacklist: "moderation.violation.blacklist", + review_nag: "moderation.violation.review_nag", +}; + +export const DEFAULT_MODERATION_WARNING_LABEL = "mod:warning"; +export const DEFAULT_MODERATION_BANNED_LABEL = "mod:banned"; +export const DEFAULT_MODERATION_BAN_THRESHOLD = 5; +// Keep the decay lookback operationally bounded, mirroring MAX_REVIEW_NAG_COOLDOWN_DAYS -- repo-controlled +// config cannot overflow Date arithmetic. +export const MAX_MODERATION_VIOLATION_DECAY_DAYS = 3650; + +const MAX_LABEL_CHARS = 100; + +export type GlobalModerationConfig = { + enabled: boolean; + rules: ModerationRuleType[]; + warningLabel: string; + bannedLabel: string; + banThreshold: number; + // null = permanent/lifetime tally (never decays), matching the existing global-blacklist's permanent-ban + // philosophy. A positive integer = only violations within that many days count toward the threshold. + violationDecayDays: number | null; + autoBlacklistOnBan: boolean; +}; + +export const DEFAULT_GLOBAL_MODERATION_CONFIG: GlobalModerationConfig = { + enabled: false, + rules: [...ALL_MODERATION_RULE_TYPES], + warningLabel: DEFAULT_MODERATION_WARNING_LABEL, + bannedLabel: DEFAULT_MODERATION_BANNED_LABEL, + banThreshold: DEFAULT_MODERATION_BAN_THRESHOLD, + violationDecayDays: null, + autoBlacklistOnBan: true, +}; + +/** Normalize a raw moderation-rules list (DB JSON or `.gittensory.yml`) into a validated, de-duplicated list + * of known rule types. Never throws: an unknown/malformed entry is dropped with a warning, matching the + * normalize-with-warnings shape every other settings list in this codebase already uses. */ +export function normalizeModerationRules(input: unknown): { rules: ModerationRuleType[]; warnings: string[] } { + const warnings: string[] = []; + if (input === undefined || input === null) return { rules: [], warnings }; + if (!Array.isArray(input)) { + warnings.push("moderationRules must be a list of rule type strings; ignoring it."); + return { rules: [], warnings }; + } + const rules: ModerationRuleType[] = []; + const seen = new Set(); + for (const [index, raw] of input.entries()) { + if (typeof raw !== "string" || !(ALL_MODERATION_RULE_TYPES as readonly string[]).includes(raw)) { + warnings.push(`moderationRules[${index}] is not a recognized rule type (expected one of ${ALL_MODERATION_RULE_TYPES.join(", ")}); ignoring it.`); + continue; + } + const rule = raw as ModerationRuleType; + if (seen.has(rule)) continue; + seen.add(rule); + rules.push(rule); + } + return { rules, warnings }; +} + +/** Normalize a raw moderation label value: empty/whitespace-only collapses to undefined (falls back to the + * caller's default), overlong is truncated. Never throws. Mirrors blacklistLabel/contributorCapLabel's + * shape, minus the explicit-null-means-"no label" case those close-coupled labels use -- a moderation label + * is always applied when the tier is reached, never suppressible to "no label at all". */ +export function normalizeModerationLabel(input: unknown): string | undefined { + if (typeof input !== "string") return undefined; + const trimmed = input.trim(); + if (trimmed.length === 0) return undefined; + return trimmed.slice(0, MAX_LABEL_CHARS); +} + +/** Effective rule set for one repo: an explicit per-repo override REPLACES the global list entirely (not a + * union) -- a repo opting out of counting review-nag toward the shared tally, for example, must be able to + * do so without also losing the ability to opt out of the others. Absent/undefined override ⇒ inherit the + * global list unchanged. */ +export function resolveEffectiveModerationRules(globalRules: readonly ModerationRuleType[], perRepoOverride: readonly ModerationRuleType[] | null | undefined): ModerationRuleType[] { + return perRepoOverride ? [...perRepoOverride] : [...globalRules]; +} + +export type ModerationGateMode = "inherit" | "off" | "enabled"; + +/** Whether the WHOLE moderation layer runs for one repo: `off` force-disables regardless of the global + * default (an operator piloting the feature on some repos only), `enabled` force-enables regardless of the + * global default (a repo that wants it before the operator flips the global default on), `inherit` (the + * default) defers to the global master switch. */ +export function resolveModerationGateEnabled(globalEnabled: boolean, gateMode: ModerationGateMode): boolean { + if (gateMode === "off") return false; + if (gateMode === "enabled") return true; + return globalEnabled; +} + +export type ModerationTier = "none" | "warning" | "banned"; + +/** Pure escalation decision: given the actor's TOTAL violation count (including the one that just fired, + * already recorded by the caller) and the configured ban threshold, which tier applies. A non-positive + * threshold (malformed config) can never be reached by a real count, so it degrades to "always banned once + * any violation exists" rather than throwing -- still a safe, non-silent failure mode for a misconfigured + * threshold, not a crash. */ +export function moderationTierForViolationCount(count: number, banThreshold: number): ModerationTier { + if (count <= 0) return "none"; + if (count >= banThreshold) return "banned"; + return "warning"; +} diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index aeeaf960dd..a6d1d5ee61 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -6,6 +6,7 @@ import { mergeContributorBlacklists, normalizeContributorBlacklist } from "../se import { normalizeAutoCloseExemptLogins } from "../settings/auto-close-exempt"; import { DEFAULT_TYPE_LABELS, normalizeTypeLabelSet } from "../settings/pr-type-label"; import { DEFAULT_LINKED_ISSUE_LABEL_PROPAGATION, normalizeLinkedIssueLabelPropagationConfig, VALID_LINKED_ISSUE_LABEL_PROPAGATION_MODES } from "../review/linked-issue-label-propagation"; +import { normalizeModerationLabel, normalizeModerationRules } from "../settings/moderation-rules"; import { hasUnsafeWildcardCount } from "./change-guardrail"; import { PUBLIC_LOCAL_PATH_INLINE } from "./redaction"; @@ -192,6 +193,10 @@ export type FocusManifestSettings = Partial< | "commandRateLimitMaxPerWindow" | "commandRateLimitAiMaxPerWindow" | "commandRateLimitWindowHours" + | "moderationGateMode" + | "moderationRules" + | "moderationWarningLabel" + | "moderationBannedLabel" > > & { // `typeLabels`/`linkedIssueLabelPropagation` are declared PARTIAL here (not via the `Pick 0") so an intentional empty list still applies. + if (r.moderationRules !== undefined) { + const { rules, warnings: moderationRuleWarnings } = normalizeModerationRules(r.moderationRules); + warnings.push(...moderationRuleWarnings); + out.moderationRules = rules; + } + const moderationWarningLabel = normalizeModerationLabel(r.moderationWarningLabel); + if (moderationWarningLabel !== undefined) out.moderationWarningLabel = moderationWarningLabel; + const moderationBannedLabel = normalizeModerationLabel(r.moderationBannedLabel); + if (moderationBannedLabel !== undefined) out.moderationBannedLabel = moderationBannedLabel; return out; } diff --git a/src/types.ts b/src/types.ts index c084b39d6a..1ad27c5e3f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -825,6 +825,21 @@ export type RepositorySettings = { /** Per-repo dry-run/shadow mode (#776): when true, the action layer records what it WOULD do without * performing any GitHub mutation. Default false. */ agentDryRun?: boolean | undefined; + /** Moderation-rules engine (#selfhost-mod-engine): whether the whole layer runs on THIS repo. `"inherit"` + * (the DB default) defers to `global_moderation_config.enabled`; `"off"`/`"enabled"` force this repo + * regardless of the global default. Always populated by the DB layer; optional so existing settings + * fixtures/callers need not be touched. */ + moderationGateMode?: "inherit" | "off" | "enabled" | undefined; + /** Moderation-rules engine: a per-repo override of WHICH of the three existing anti-abuse mechanisms + * (contributor cap, blacklist, review-nag) feed a contributor's shared, cross-repo violation tally. + * `undefined`/absent ⇒ inherit the global rule set (`resolveEffectiveModerationRules`'s default shape). */ + moderationRules?: ("contributor_cap" | "blacklist" | "review_nag")[] | undefined; + /** Moderation-rules engine: per-repo override of the label applied at >=1 lifetime violation. `undefined` ⇒ + * the global config's `warningLabel` (itself defaulting to `"mod:warning"`). */ + moderationWarningLabel?: string | undefined; + /** Moderation-rules engine: per-repo override of the label applied at >= the ban threshold. `undefined` ⇒ + * the global config's `bannedLabel` (itself defaulting to `"mod:banned"`). */ + moderationBannedLabel?: string | undefined; createdAt?: string | null | undefined; updatedAt?: string | null | undefined; }; diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index ca4ace9bda..d4a224353d 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -53,7 +53,7 @@ import { } from "../../src/services/agent-action-executor"; import type { PlannedAgentAction } from "../../src/settings/agent-actions"; import { AGENT_LABEL_PENDING_CLOSURE } from "../../src/review/linked-issue-hard-rules"; -import { isGlobalAgentFrozen, setGlobalAgentFrozen, upsertPullRequestFromGitHub } from "../../src/db/repositories"; +import { getGlobalContributorBlacklist, isGlobalAgentFrozen, setGlobalAgentFrozen, upsertGlobalModerationConfig, upsertPullRequestFromGitHub } from "../../src/db/repositories"; import { createTestEnv } from "../helpers/d1"; function ctx(over: Partial = {}): AgentActionExecutionContext { @@ -681,6 +681,144 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { }); }); +describe("moderation-rules engine escalation (#selfhost-mod-engine)", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(fetchPullRequestFreshness).mockImplementation(async (_env, args) => ({ + status: "current", + liveHeadSha: args.expectedHeadSha ?? null, + liveState: "open", + })); + // clearAllMocks() resets call history but does NOT drain a queued mockRejectedValueOnce/mockResolvedValueOnce + // left over from an earlier test elsewhere in this file (e.g. the installation-health-refresh tests above + // queue a one-time closePullRequest rejection) -- re-pin the base implementation explicitly so this describe + // block's "the close actually completed" assumption is never at the mercy of file-level test order. + vi.mocked(closePullRequest).mockResolvedValue({ state: "closed" }); + }); + + const coupledClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "over the per-contributor open-item cap", closeComment: "closing", closeKind: "contributor_cap" }; + const coupledLabel: PlannedAgentAction = { actionClass: "label", autonomyClass: "close", requiresApproval: false, reason: "over the per-contributor open-item cap", label: "over-contributor-limit", labelOp: "add", closeKind: "contributor_cap" }; + + it("OFF by default: a completed contributor_cap close applies NO mod label when the global moderation config is disabled (the DB default)", async () => { + const env = createTestEnv({}); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", expect.anything()); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", expect.anything()); + }); + + it("applies the default mod:warning label at the 1st lifetime violation once the global config is enabled", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); + }); + + it("4 violations -> warning only; the 5th (default threshold) escalates to mod:banned + auto-blacklists the login", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + for (let i = 0; i < 4; i++) { + vi.clearAllMocks(); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", expect.anything()); + } + vi.clearAllMocks(); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", { createMissingLabel: true }); + const blacklist = await getGlobalContributorBlacklist(env); + expect(blacklist?.map((entry) => entry.login)).toContain("farmer99"); + }); + + it("does NOT auto-blacklist when autoBlacklistOnBan is off, even at the ban threshold", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true, banThreshold: 1, autoBlacklistOnBan: false }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", { createMissingLabel: true }); + const blacklist = await getGlobalContributorBlacklist(env); + expect(blacklist?.map((entry) => entry.login)).not.toContain("farmer99"); + }); + + it("does not double-add an actor who is already on the global blacklist", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true, banThreshold: 1 }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [{ ...coupledClose }, { ...coupledLabel }]); + const blacklist = await getGlobalContributorBlacklist(env); + expect(blacklist?.filter((entry) => entry.login === "farmer99")).toHaveLength(1); + }); + + it("per-repo moderationGateMode 'off' force-disables the layer even when the global config is enabled", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", moderationSettings: { moderationGateMode: "off" } }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", expect.anything()); + }); + + it("per-repo moderationGateMode 'enabled' force-enables the layer even when the global config is disabled (the default)", async () => { + const env = createTestEnv({}); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", moderationSettings: { moderationGateMode: "enabled" } }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); + }); + + it("per-repo moderationRules override EXCLUDING contributor_cap means a contributor_cap close on THIS repo does not count as a violation", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", moderationSettings: { moderationRules: ["blacklist"] } }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", expect.anything()); + }); + + it("per-repo custom label overrides win over the global config's label", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true, warningLabel: "global:warn" }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", moderationSettings: { moderationWarningLabel: "repo:warn" } }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "repo:warn", { createMissingLabel: true }); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "global:warn", expect.anything()); + }); + + it("no escalation in dry-run mode -- a mutation that didn't really happen must not count as a violation", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", agentDryRun: true }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).not.toHaveBeenCalled(); + }); + + it("no escalation when the close is denied (not completed) -- e.g. the label-close split-brain guard's own denial path", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", installationPermissions: { pull_requests: "read", issues: "write" } }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", expect.anything()); + }); + + it("no escalation for a close with no author login (defensive -- should not happen for a real PR/issue)", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: undefined }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", expect.anything()); + }); + + it("no escalation for an UNRELATED heuristic close (not one of the three moderation-tracked rule types)", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true }); + const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "gate failed", closeComment: "closing", closeKind: "heuristic" }; + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [heuristicClose]); + expect(ensurePullRequestLabel).not.toHaveBeenCalled(); + }); + + it("violationDecayDays (rolling window) excludes an old violation from the ban threshold, unlike the permanent-tally default", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true, banThreshold: 2, violationDecayDays: 1 }); + // A violation from 10 days ago -- outside the 1-day decay window -- must NOT count toward the threshold. + const tenDaysAgo = new Date(Date.now() - 10 * 24 * 60 * 60 * 1000).toISOString(); + await env.DB.prepare("INSERT INTO audit_events (id, event_type, actor, target_key, outcome, detail, metadata_json, created_at) VALUES (?, ?, ?, ?, 'completed', 'old', '{}', ?)") + .bind(crypto.randomUUID(), "moderation.violation.contributor_cap", "farmer99", "owner/repo#1", tenDaysAgo) + .run(); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + // Only the JUST-recorded violation counts (1 < banThreshold 2) -> warning, not banned. + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", expect.anything()); + }); +}); + function issueCtx(over: Partial = {}): IssueActionExecutionContext { return { installationId: 123, diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index e167b7b0ee..e8475e515d 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -1578,6 +1578,33 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(noOverride.autoCloseExemptLogins).toEqual(["keep-me"]); }); + it("parses + resolves the moderation-rules engine settings from the settings: block, overlaying the DB (#selfhost-mod-engine)", () => { + const manifest = parseFocusManifest({ settings: { moderationGateMode: "enabled", moderationRules: ["blacklist", "not-a-rule" as never], moderationWarningLabel: "repo:warn", moderationBannedLabel: "repo:ban" } }); + expect(manifest.settings.moderationGateMode).toBe("enabled"); + expect(manifest.settings.moderationRules).toEqual(["blacklist"]); // invalid entry dropped + expect(manifest.settings.moderationWarningLabel).toBe("repo:warn"); + expect(manifest.settings.moderationBannedLabel).toBe("repo:ban"); + // yml overlays (replaces) the DB-configured values. + const eff = resolveEffectiveSettings({ moderationGateMode: "off", moderationRules: ["review_nag"], moderationWarningLabel: "db:warn", moderationBannedLabel: "db:ban" } as unknown as RepositorySettings, manifest); + expect(eff.moderationGateMode).toBe("enabled"); + expect(eff.moderationRules).toEqual(["blacklist"]); + expect(eff.moderationWarningLabel).toBe("repo:warn"); + expect(eff.moderationBannedLabel).toBe("repo:ban"); + // Omitted in yml ⇒ the DB-configured values survive untouched. + const noOverride = resolveEffectiveSettings({ moderationGateMode: "off", moderationWarningLabel: "db:warn" } as unknown as RepositorySettings, parseFocusManifest({})); + expect(noOverride.moderationGateMode).toBe("off"); + expect(noOverride.moderationWarningLabel).toBe("db:warn"); + // An intentional EMPTY moderationRules override (opting every rule out for this repo) still applies -- + // distinct from an all-invalid block, which is dropped instead (see autoCloseExemptLogins above). + const emptyOverride = resolveEffectiveSettings({ moderationRules: ["blacklist"] } as unknown as RepositorySettings, parseFocusManifest({ settings: { moderationRules: [] } })); + expect(emptyOverride.moderationRules).toEqual([]); + // An invalid enum / blank label is dropped with a warning rather than silently coerced. + const invalid = parseFocusManifest({ settings: { moderationGateMode: "sometimes" as never, moderationWarningLabel: " " } }); + expect(invalid.settings.moderationGateMode).toBeUndefined(); + expect(invalid.settings.moderationWarningLabel).toBeUndefined(); + expect(invalid.warnings.some((w) => /settings\.moderationGateMode/.test(w))).toBe(true); + }); + it("an EXPLICIT yml null force-clears a DB-configured cap, distinct from an omitted key (regression, gate finding on #2467)", () => { // Omitted key preserves the DB value (already covered above); an explicit `null` must ALSO be able to // override a DB-configured cap back to "no cap" — the documented `yml > DB > null` precedence otherwise diff --git a/test/unit/moderation-config-db.test.ts b/test/unit/moderation-config-db.test.ts new file mode 100644 index 0000000000..1ad792bd8b --- /dev/null +++ b/test/unit/moderation-config-db.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from "vitest"; +import { + countModerationViolationsForActor, + getGlobalModerationConfig, + getRepositorySettings, + recordModerationViolation, + upsertGlobalModerationConfig, + upsertRepositorySettings, +} from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; +import { DEFAULT_GLOBAL_MODERATION_CONFIG, MODERATION_VIOLATION_EVENT_TYPE } from "../../src/settings/moderation-rules"; + +describe("global moderation config DB round-trip (#selfhost-mod-engine)", () => { + it("defaults to DEFAULT_GLOBAL_MODERATION_CONFIG (off) for a fresh install", async () => { + const env = createTestEnv(); + expect(await getGlobalModerationConfig(env)).toEqual(DEFAULT_GLOBAL_MODERATION_CONFIG); + }); + + it("returns the default when the singleton row is missing", async () => { + const env = createTestEnv(); + await env.DB.prepare("DELETE FROM global_moderation_config WHERE id = 'singleton'").run(); + expect(await getGlobalModerationConfig(env)).toEqual(DEFAULT_GLOBAL_MODERATION_CONFIG); + }); + + it("fails open to the default when the table is unavailable", async () => { + const env = createTestEnv(); + await env.DB.prepare("DROP TABLE global_moderation_config").run(); + expect(await getGlobalModerationConfig(env)).toEqual(DEFAULT_GLOBAL_MODERATION_CONFIG); + }); + + it("falls back to the default warning/banned label when the stored row has an empty/whitespace label (e.g. written directly via raw SQL, bypassing app-level upsert validation)", async () => { + const env = createTestEnv(); + await env.DB.prepare("UPDATE global_moderation_config SET warning_label = '', banned_label = ' ' WHERE id = 'singleton'").run(); + const resolved = await getGlobalModerationConfig(env); + expect(resolved.warningLabel).toBe(DEFAULT_GLOBAL_MODERATION_CONFIG.warningLabel); + expect(resolved.bannedLabel).toBe(DEFAULT_GLOBAL_MODERATION_CONFIG.bannedLabel); + }); + + it("persists a full upsert and reads it back", async () => { + const env = createTestEnv(); + const resolved = await upsertGlobalModerationConfig(env, { + enabled: true, + rules: ["blacklist", "review_nag"], + warningLabel: "custom:warning", + bannedLabel: "custom:banned", + banThreshold: 3, + violationDecayDays: 90, + autoBlacklistOnBan: false, + updatedBy: "JSONbored", + }); + expect(resolved).toEqual({ + enabled: true, + rules: ["blacklist", "review_nag"], + warningLabel: "custom:warning", + bannedLabel: "custom:banned", + banThreshold: 3, + violationDecayDays: 90, + autoBlacklistOnBan: false, + }); + expect(await getGlobalModerationConfig(env)).toEqual(resolved); + }); + + it("a PARTIAL upsert only changes the given fields, preserving the rest from the current row", async () => { + const env = createTestEnv(); + await upsertGlobalModerationConfig(env, { enabled: true, banThreshold: 3 }); + const resolved = await upsertGlobalModerationConfig(env, { warningLabel: "mod:caution" }); + expect(resolved.enabled).toBe(true); + expect(resolved.banThreshold).toBe(3); + expect(resolved.warningLabel).toBe("mod:caution"); + }); + + it("drops an invalid rule type on upsert with a fallback to a valid subset, and coerces a malformed threshold/label back to the current value", async () => { + const env = createTestEnv(); + const invalidRules = ["blacklist", "not-a-rule"] as unknown as ("contributor_cap" | "blacklist" | "review_nag")[]; + const resolved = await upsertGlobalModerationConfig(env, { rules: invalidRules, banThreshold: -1, warningLabel: " " }); + expect(resolved.rules).toEqual(["blacklist"]); + // Malformed values fall back to the CURRENT row's value (this is the first write, so that's still the + // module default), not silently to 0/empty. + expect(resolved.banThreshold).toBe(DEFAULT_GLOBAL_MODERATION_CONFIG.banThreshold); + expect(resolved.warningLabel).toBe(DEFAULT_GLOBAL_MODERATION_CONFIG.warningLabel); + }); +}); + +describe("moderation violation ledger (#selfhost-mod-engine)", () => { + it("records a violation and counts it back for the actor", async () => { + const env = createTestEnv(); + await recordModerationViolation(env, { + eventType: MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, + actor: "farmer99", + targetKey: "owner/repo#42", + repoFullName: "owner/repo", + ruleReason: "contributor_cap violation", + }); + const count = await countModerationViolationsForActor(env, "farmer99", [MODERATION_VIOLATION_EVENT_TYPE.contributor_cap]); + expect(count).toBe(1); + }); + + it("counts across MULTIPLE rule types and MULTIPLE repos for the same actor (install-wide, not per-repo)", async () => { + const env = createTestEnv(); + await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, actor: "farmer99", targetKey: "owner/repo-a#1", repoFullName: "owner/repo-a", ruleReason: "cap" }); + await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.blacklist, actor: "farmer99", targetKey: "owner/repo-b#2", repoFullName: "owner/repo-b", ruleReason: "blacklist" }); + await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.review_nag, actor: "someone-else", targetKey: "owner/repo-a#3", repoFullName: "owner/repo-a", ruleReason: "nag" }); + const count = await countModerationViolationsForActor(env, "farmer99", Object.values(MODERATION_VIOLATION_EVENT_TYPE)); + expect(count).toBe(2); // only farmer99's two, not someone-else's + }); + + it("respects an optional sinceIso rolling-window bound (violation-decay support)", async () => { + const env = createTestEnv(); + await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.blacklist, actor: "farmer99", targetKey: "owner/repo#1", repoFullName: "owner/repo", ruleReason: "old" }); + const futureIso = new Date(Date.now() + 60_000).toISOString(); // strictly after the just-recorded violation + const count = await countModerationViolationsForActor(env, "farmer99", [MODERATION_VIOLATION_EVENT_TYPE.blacklist], futureIso); + expect(count).toBe(0); // outside the (future-dated, deliberately empty) window + }); + + it("returns 0 for an actor with no recorded violations", async () => { + const env = createTestEnv(); + const count = await countModerationViolationsForActor(env, "nobody", Object.values(MODERATION_VIOLATION_EVENT_TYPE)); + expect(count).toBe(0); + }); +}); + +describe("per-repo moderation settings DB round-trip (#selfhost-mod-engine)", () => { + it("defaults to 'inherit' gate mode and undefined overrides for an unconfigured repo", async () => { + const settings = await getRepositorySettings(createTestEnv(), "owner/none"); + expect(settings.moderationGateMode).toBe("inherit"); + expect(settings.moderationRules).toBeUndefined(); + expect(settings.moderationWarningLabel).toBeUndefined(); + expect(settings.moderationBannedLabel).toBeUndefined(); + }); + + it("persists an explicit gate mode + rule override + custom labels", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { + repoFullName: "owner/repo", + moderationGateMode: "enabled", + moderationRules: ["blacklist"], + moderationWarningLabel: "repo:warn", + moderationBannedLabel: "repo:ban", + }); + const settings = await getRepositorySettings(env, "owner/repo"); + expect(settings.moderationGateMode).toBe("enabled"); + expect(settings.moderationRules).toEqual(["blacklist"]); + expect(settings.moderationWarningLabel).toBe("repo:warn"); + expect(settings.moderationBannedLabel).toBe("repo:ban"); + }); + + it("persists an explicit EMPTY moderationRules override distinctly from 'not configured' (undefined)", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", moderationRules: [] }); + const settings = await getRepositorySettings(env, "owner/repo"); + expect(settings.moderationRules).toEqual([]); + }); + + it("round-trips through an UPDATE (not just the initial INSERT)", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", moderationGateMode: "off" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", moderationGateMode: "enabled", moderationWarningLabel: "updated:warn" }); + const settings = await getRepositorySettings(env, "owner/repo"); + expect(settings.moderationGateMode).toBe("enabled"); + expect(settings.moderationWarningLabel).toBe("updated:warn"); + }); +}); diff --git a/test/unit/moderation-rules.test.ts b/test/unit/moderation-rules.test.ts new file mode 100644 index 0000000000..9703642392 --- /dev/null +++ b/test/unit/moderation-rules.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from "vitest"; +import { + DEFAULT_GLOBAL_MODERATION_CONFIG, + DEFAULT_MODERATION_BANNED_LABEL, + DEFAULT_MODERATION_BAN_THRESHOLD, + DEFAULT_MODERATION_WARNING_LABEL, + MODERATION_VIOLATION_EVENT_TYPE, + moderationTierForViolationCount, + normalizeModerationLabel, + normalizeModerationRules, + resolveEffectiveModerationRules, + resolveModerationGateEnabled, +} from "../../src/settings/moderation-rules"; + +describe("normalizeModerationRules (#selfhost-mod-engine)", () => { + it("returns [] for null/undefined and a non-array (with a warning)", () => { + expect(normalizeModerationRules(undefined).rules).toEqual([]); + expect(normalizeModerationRules(null).rules).toEqual([]); + const notArray = normalizeModerationRules("contributor_cap"); + expect(notArray.rules).toEqual([]); + expect(notArray.warnings[0]).toMatch(/must be a list/); + }); + + it("accepts every known rule type", () => { + const { rules, warnings } = normalizeModerationRules(["contributor_cap", "blacklist", "review_nag"]); + expect(rules).toEqual(["contributor_cap", "blacklist", "review_nag"]); + expect(warnings).toEqual([]); + }); + + it("drops unrecognized entries with a warning, keeping the valid ones", () => { + const { rules, warnings } = normalizeModerationRules(["contributor_cap", "not-a-rule", 42, null]); + expect(rules).toEqual(["contributor_cap"]); + expect(warnings.length).toBe(3); + }); + + it("de-duplicates repeated rule types", () => { + const { rules } = normalizeModerationRules(["blacklist", "blacklist", "review_nag"]); + expect(rules).toEqual(["blacklist", "review_nag"]); + }); + + it("returns [] (not the default rule set) for an intentional empty array — an explicit opt-out-of-everything must survive, not be coerced back to a default", () => { + expect(normalizeModerationRules([]).rules).toEqual([]); + }); +}); + +describe("normalizeModerationLabel (#selfhost-mod-engine)", () => { + it("returns undefined for a non-string, empty, or whitespace-only value", () => { + expect(normalizeModerationLabel(undefined)).toBeUndefined(); + expect(normalizeModerationLabel(null)).toBeUndefined(); + expect(normalizeModerationLabel(42)).toBeUndefined(); + expect(normalizeModerationLabel("")).toBeUndefined(); + expect(normalizeModerationLabel(" ")).toBeUndefined(); + }); + + it("trims and returns a valid label", () => { + expect(normalizeModerationLabel(" mod:custom ")).toBe("mod:custom"); + }); + + it("truncates an overlong label", () => { + const long = "x".repeat(200); + expect(normalizeModerationLabel(long)?.length).toBe(100); + }); +}); + +describe("resolveEffectiveModerationRules (#selfhost-mod-engine)", () => { + const globalRules = ["contributor_cap", "blacklist", "review_nag"] as const; + + it("inherits the global list when no per-repo override is given", () => { + expect(resolveEffectiveModerationRules(globalRules, undefined)).toEqual([...globalRules]); + expect(resolveEffectiveModerationRules(globalRules, null)).toEqual([...globalRules]); + }); + + it("REPLACES (not unions) the global list with an explicit per-repo override", () => { + expect(resolveEffectiveModerationRules(globalRules, ["blacklist"])).toEqual(["blacklist"]); + }); + + it("an explicit EMPTY per-repo override opts this repo out of every rule, distinct from 'inherit'", () => { + expect(resolveEffectiveModerationRules(globalRules, [])).toEqual([]); + }); +}); + +describe("resolveModerationGateEnabled (#selfhost-mod-engine)", () => { + it("'off' force-disables regardless of the global default", () => { + expect(resolveModerationGateEnabled(true, "off")).toBe(false); + expect(resolveModerationGateEnabled(false, "off")).toBe(false); + }); + + it("'enabled' force-enables regardless of the global default", () => { + expect(resolveModerationGateEnabled(true, "enabled")).toBe(true); + expect(resolveModerationGateEnabled(false, "enabled")).toBe(true); + }); + + it("'inherit' defers to the global default", () => { + expect(resolveModerationGateEnabled(true, "inherit")).toBe(true); + expect(resolveModerationGateEnabled(false, "inherit")).toBe(false); + }); +}); + +describe("moderationTierForViolationCount (#selfhost-mod-engine)", () => { + it("returns 'none' for a non-positive count", () => { + expect(moderationTierForViolationCount(0, 5)).toBe("none"); + expect(moderationTierForViolationCount(-1, 5)).toBe("none"); + }); + + it("returns 'warning' for 1..threshold-1", () => { + expect(moderationTierForViolationCount(1, 5)).toBe("warning"); + expect(moderationTierForViolationCount(4, 5)).toBe("warning"); + }); + + it("returns 'banned' at and above the threshold", () => { + expect(moderationTierForViolationCount(5, 5)).toBe("banned"); + expect(moderationTierForViolationCount(6, 5)).toBe("banned"); + }); + + it("degrades a malformed non-positive threshold to 'always banned once any violation exists' rather than throwing", () => { + expect(moderationTierForViolationCount(1, 0)).toBe("banned"); + expect(moderationTierForViolationCount(1, -1)).toBe("banned"); + }); +}); + +describe("constants + event-type map (#selfhost-mod-engine)", () => { + it("default labels/threshold match the documented defaults", () => { + expect(DEFAULT_MODERATION_WARNING_LABEL).toBe("mod:warning"); + expect(DEFAULT_MODERATION_BANNED_LABEL).toBe("mod:banned"); + expect(DEFAULT_MODERATION_BAN_THRESHOLD).toBe(5); + expect(DEFAULT_GLOBAL_MODERATION_CONFIG.enabled).toBe(false); + expect(DEFAULT_GLOBAL_MODERATION_CONFIG.violationDecayDays).toBeNull(); + expect(DEFAULT_GLOBAL_MODERATION_CONFIG.autoBlacklistOnBan).toBe(true); + }); + + it("every rule type has a distinct, namespaced event type", () => { + const values = Object.values(MODERATION_VIOLATION_EVENT_TYPE); + expect(new Set(values).size).toBe(values.length); + for (const eventType of values) expect(eventType).toMatch(/^moderation\.violation\./); + }); +}); From e847af9e1f5de9952451a8b56a0af26efdf3078f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 04:00:31 -0700 Subject: [PATCH 2/6] fix(selfhost): preserve DB-configured moderationRules on a malformed .gittensory.yml override normalizeModerationRules degrades BOTH a genuinely empty yml list and a malformed one (non-array, or every entry invalid) to the same empty array, but only the former is an intentional "opt every rule out for this repo" -- the latter is bad config that should leave the DB value untouched, not silently disable moderation for the repo. Distinguish them by the raw input's own shape (a literal empty array) rather than the normalized result alone. --- src/signals/focus-manifest.ts | 15 +++++++++------ test/unit/focus-manifest.test.ts | 9 +++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index a6d1d5ee61..dc0470e8d7 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -1138,15 +1138,18 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) // Moderation-rules engine (#selfhost-mod-engine): per-repo override of the global moderation config. const moderationGateMode = normalizeOptionalEnum(r.moderationGateMode, "settings.moderationGateMode", ["inherit", "off", "enabled"] as const, warnings); if (moderationGateMode !== null) out.moderationGateMode = moderationGateMode; - // Only set when at least one VALID rule survives normalization, same "never blank the DB-configured value - // via a malformed block" reasoning as autoCloseExemptLogins above -- an explicit EMPTY override (opting - // every rule out for this repo) is expressed by setting an empty array, which normalizeModerationRules - // itself returns unchanged, so this guard would also silently drop that intentional case. Guard on - // `r.moderationRules !== undefined` alone (not "length > 0") so an intentional empty list still applies. + // #gate-flagged: normalizeModerationRules returns an EMPTY rules array for two semantically different + // inputs -- a genuinely empty yml list (`moderationRules: []`, an intentional "opt every rule out for this + // repo") and a MALFORMED one (a non-array, or an array where every entry fails validation) that degrades to + // empty as its safe fallback. Applying the malformed case as an override would silently disable every rule + // for this repo instead of leaving the DB-configured value intact, so the two must be told apart by the RAW + // input's own shape -- not just the normalized result -- before assigning. A PARTIAL list (some valid, some + // invalid entries) still applies the surviving valid subset, mirroring autoCloseExemptLogins' behavior. if (r.moderationRules !== undefined) { const { rules, warnings: moderationRuleWarnings } = normalizeModerationRules(r.moderationRules); warnings.push(...moderationRuleWarnings); - out.moderationRules = rules; + const intentionalEmptyList = Array.isArray(r.moderationRules) && r.moderationRules.length === 0; + if (rules.length > 0 || intentionalEmptyList) out.moderationRules = rules; } const moderationWarningLabel = normalizeModerationLabel(r.moderationWarningLabel); if (moderationWarningLabel !== undefined) out.moderationWarningLabel = moderationWarningLabel; diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index e8475e515d..463fbed1e0 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -1598,6 +1598,15 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = // distinct from an all-invalid block, which is dropped instead (see autoCloseExemptLogins above). const emptyOverride = resolveEffectiveSettings({ moderationRules: ["blacklist"] } as unknown as RepositorySettings, parseFocusManifest({ settings: { moderationRules: [] } })); expect(emptyOverride.moderationRules).toEqual([]); + // REGRESSION (gate-flagged): an ALL-INVALID moderationRules block (every entry fails validation, so + // normalizeModerationRules ALSO degrades it to an empty array) must NOT be treated as the intentional + // empty-list case above -- it is malformed input, not a real opt-out, so the DB-configured value survives. + const allInvalidPreserved = resolveEffectiveSettings({ moderationRules: ["blacklist"] } as unknown as RepositorySettings, parseFocusManifest({ settings: { moderationRules: ["not-a-rule", "also-not-a-rule"] as never } })); + expect(allInvalidPreserved.moderationRules).toEqual(["blacklist"]); + // REGRESSION (gate-flagged): a non-array moderationRules value (e.g. a typo'd bare string) is malformed + // the same way -- must not silently disable every rule for this repo either. + const nonArrayPreserved = resolveEffectiveSettings({ moderationRules: ["review_nag"] } as unknown as RepositorySettings, parseFocusManifest({ settings: { moderationRules: "blacklist" as never } })); + expect(nonArrayPreserved.moderationRules).toEqual(["review_nag"]); // An invalid enum / blank label is dropped with a warning rather than silently coerced. const invalid = parseFocusManifest({ settings: { moderationGateMode: "sometimes" as never, moderationWarningLabel: " " } }); expect(invalid.settings.moderationGateMode).toBeUndefined(); From cfff87f18f4ba989f0fa88a7602273f1ff0fba1e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 04:03:25 -0700 Subject: [PATCH 3/6] chore(selfhost): renumber moderation-engine migrations past a colliding 0102 merged upstream --- ...al_moderation_config.sql => 0103_global_moderation_config.sql} | 0 ...ation_settings.sql => 0104_repository_moderation_settings.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename migrations/{0102_global_moderation_config.sql => 0103_global_moderation_config.sql} (100%) rename migrations/{0103_repository_moderation_settings.sql => 0104_repository_moderation_settings.sql} (100%) diff --git a/migrations/0102_global_moderation_config.sql b/migrations/0103_global_moderation_config.sql similarity index 100% rename from migrations/0102_global_moderation_config.sql rename to migrations/0103_global_moderation_config.sql diff --git a/migrations/0103_repository_moderation_settings.sql b/migrations/0104_repository_moderation_settings.sql similarity index 100% rename from migrations/0103_repository_moderation_settings.sql rename to migrations/0104_repository_moderation_settings.sql From 40b994858b93923e308224d16c50322bc5f32c92 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 04:26:05 -0700 Subject: [PATCH 4/6] fix(selfhost): scope the moderation escalation count to the currently-effective rules maybeEscalateModeration counted every rule type ever recorded (Object.values(MODERATION_VIOLATION_EVENT_TYPE)) toward the ban threshold, regardless of which rules the global/per-repo config currently has enabled. A rule an operator has excluded still influenced the ban decision as long as a violation of that kind was recorded at some point (globally, or on a repo that still counts it). Scope the count to effectiveRules (already computed for the record-time gate just above) instead, so excluding a rule is an ongoing policy stance about what a contributor's standing is judged on, consistently applied at both record time and count time. --- src/services/agent-action-executor.ts | 9 +++++++-- test/unit/agent-action-executor.test.ts | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 772b6bf879..8a755212c7 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -326,9 +326,14 @@ async function maybeEscalateModeration( const targetKey = `${args.repoFullName}#${args.number}`; await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE[rule], actor: args.authorLogin, targetKey, repoFullName: args.repoFullName, ruleReason: `${rule} violation` }).catch(() => undefined); - const allEventTypes = Object.values(MODERATION_VIOLATION_EVENT_TYPE); + // #gate-flagged: count only the CURRENTLY-effective rule types, not every rule type ever recorded. A rule + // an operator has excluded (globally or for this repo) must not go on influencing the ban decision just + // because a violation of that kind happened to get recorded before the exclusion, or on a repo that still + // counts it -- "we don't count reviewNag violations" is an ongoing policy stance about what this contributor's + // standing should be judged on, not a per-recording footnote that only applies to where it happened. + const countedEventTypes = effectiveRules.map((r) => MODERATION_VIOLATION_EVENT_TYPE[r]); const sinceIso = globalConfig.violationDecayDays !== null ? new Date(Date.now() - globalConfig.violationDecayDays * 24 * 60 * 60 * 1000).toISOString() : undefined; - const totalCount = await countModerationViolationsForActor(env, args.authorLogin, allEventTypes, sinceIso); + const totalCount = await countModerationViolationsForActor(env, args.authorLogin, countedEventTypes, sinceIso); const tier = moderationTierForViolationCount(totalCount, globalConfig.banThreshold); /* v8 ignore next -- defensive: the violation just recorded above always makes totalCount >= 1 by the time execution reaches here (the only way to see "none" is the record write itself silently failing, which diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index d4a224353d..d949fb0da1 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -55,6 +55,7 @@ import type { PlannedAgentAction } from "../../src/settings/agent-actions"; import { AGENT_LABEL_PENDING_CLOSURE } from "../../src/review/linked-issue-hard-rules"; import { getGlobalContributorBlacklist, isGlobalAgentFrozen, setGlobalAgentFrozen, upsertGlobalModerationConfig, upsertPullRequestFromGitHub } from "../../src/db/repositories"; import { createTestEnv } from "../helpers/d1"; +import { MODERATION_VIOLATION_EVENT_TYPE } from "../../src/settings/moderation-rules"; function ctx(over: Partial = {}): AgentActionExecutionContext { return { @@ -767,6 +768,22 @@ describe("moderation-rules engine escalation (#selfhost-mod-engine)", () => { expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", expect.anything()); }); + it("REGRESSION (gate-flagged): the escalation count is scoped to the CURRENTLY-effective rule types, not every rule type ever recorded -- an excluded rule's historical violations must not push the count toward the ban threshold", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true, banThreshold: 2 }); + // A contributor_cap violation recorded earlier (e.g. from a repo/period where cap DID count). + await env.DB.prepare("INSERT INTO audit_events (id, event_type, actor, target_key, outcome, detail, metadata_json, created_at) VALUES (?, ?, ?, ?, 'completed', 'old', '{}', ?)") + .bind(crypto.randomUUID(), MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, "farmer99", "owner/repo#1", new Date().toISOString()) + .run(); + // THIS repo only cares about blacklist -- a blacklist close here should count ONLY the blacklist history, + // not the pre-existing contributor_cap violation, so the total stays at 1 (< threshold 2) -> warning. + const blacklistClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "blacklisted contributor", closeComment: "closing", closeKind: "blacklist" }; + const blacklistLabel: PlannedAgentAction = { actionClass: "label", autonomyClass: "close", requiresApproval: false, reason: "blacklisted contributor", label: "slop", labelOp: "add", closeKind: "blacklist" }; + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", moderationSettings: { moderationRules: ["blacklist"] } }), [blacklistClose, blacklistLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", expect.anything()); + }); + it("per-repo custom label overrides win over the global config's label", async () => { const env = createTestEnv({}); await upsertGlobalModerationConfig(env, { enabled: true, warningLabel: "global:warn" }); From d880732b275ab837013f480ce61a6cf6fe6ad143 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:28:12 -0700 Subject: [PATCH 5/6] fix(selfhost): make the moderation violation ledger idempotent and clamp the decay window Two gate-flagged defects in the moderation-rules engine: - recordModerationViolation recorded a fresh violation on every completed tracked close with no idempotency check, so a webhook redelivery or queue retry re-executing an already-recorded close could double-count it and falsely push a contributor toward the ban threshold. Now idempotent per (actor, eventType, targetKey); a duplicate returns false and the caller skips the rest of escalation. - violationDecayDays was normalized with the generic, unbounded normalizeOpenItemCap, but it feeds Date arithmetic on the live close path (Date.now() - days * 86400000).toISOString(); an unbounded value could overflow into an Invalid Date and throw a RangeError, crashing the close. Clamped to MAX_MODERATION_VIOLATION_DECAY_DAYS on both read and write, mirroring reviewNagCooldownDays' own clamping shape for the same family of day-count settings. Also fixes two non-blocking doc-comment nits the same review pass raised (a stale migration-number cross-reference, and an over-broad description of what "fails open" on malformed JSON). --- .../0104_repository_moderation_settings.sql | 2 +- src/db/repositories.ts | 54 ++++++++++++--- src/services/agent-action-executor.ts | 8 ++- test/unit/agent-action-executor.test.ts | 39 +++++++++-- test/unit/moderation-config-db.test.ts | 66 ++++++++++++++++++- 5 files changed, 150 insertions(+), 19 deletions(-) diff --git a/migrations/0104_repository_moderation_settings.sql b/migrations/0104_repository_moderation_settings.sql index f1c9dc1883..96ad6bac0d 100644 --- a/migrations/0104_repository_moderation_settings.sql +++ b/migrations/0104_repository_moderation_settings.sql @@ -1,5 +1,5 @@ -- Per-repo overrides for the moderation-rules engine (#selfhost-mod-engine), layered over --- global_moderation_config (0101). moderation_gate_mode defaults to 'inherit' (defers to the global master +-- global_moderation_config (0103). moderation_gate_mode defaults to 'inherit' (defers to the global master -- switch) -- 'off'/'enabled' let one repo opt out of or into the whole layer regardless of the global default, -- e.g. an operator piloting the feature on a single repo before flipping the global default on. The three -- override columns are nullable: NULL means "inherit the global value", never "unset to empty/off" -- an diff --git a/src/db/repositories.ts b/src/db/repositories.ts index c723bd744c..171cb50a21 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -163,7 +163,7 @@ import { classifyMcpClientVersion, LATEST_RECOMMENDED_MCP_VERSION, MINIMUM_SUPPO import { DEFAULT_COMMAND_AUTHORIZATION_POLICY, normalizeCommandAuthorizationPolicy } from "../settings/command-authorization"; import { normalizeContributorBlacklist } from "../settings/contributor-blacklist"; import { normalizeAutoCloseExemptLogins } from "../settings/auto-close-exempt"; -import { DEFAULT_GLOBAL_MODERATION_CONFIG, normalizeModerationLabel, normalizeModerationRules, type GlobalModerationConfig, type ModerationRuleType } from "../settings/moderation-rules"; +import { DEFAULT_GLOBAL_MODERATION_CONFIG, MAX_MODERATION_VIOLATION_DECAY_DAYS, normalizeModerationLabel, normalizeModerationRules, type GlobalModerationConfig, type ModerationRuleType } from "../settings/moderation-rules"; import { normalizeAutonomyPolicy, normalizeAutoMaintainPolicy, DEFAULT_AUTO_MAINTAIN_POLICY } from "../settings/autonomy"; import { DEFAULT_TYPE_LABELS, normalizeTypeLabelSet } from "../settings/pr-type-label"; import { DEFAULT_LINKED_ISSUE_LABEL_PROPAGATION, normalizeLinkedIssueLabelPropagationConfig } from "../review/linked-issue-label-propagation"; @@ -2385,10 +2385,32 @@ export async function countModerationViolationsForActor(env: Env, actor: string, return row.count; } +/** Moderation-rules engine: whether a violation has ALREADY been recorded for this EXACT (actor, eventType, + * targetKey) tuple. Deliberately NO time window (unlike hasRecentAuditEvent's sinceIso) -- "this PR/issue + * already contributed a violation of this kind to the tally" is permanently true once recorded, not + * something that should re-count on a later replay just because time has passed. */ +export async function hasModerationViolationForTarget(env: Env, actor: string, eventType: string, targetKey: string): Promise { + const db = getDb(env.DB); + const rows = await db + .select({ id: auditEvents.id }) + .from(auditEvents) + .where(and(eq(auditEvents.actor, actor), eq(auditEvents.eventType, eventType), eq(auditEvents.targetKey, targetKey))) + .limit(1); + return rows.length > 0; +} + /** Moderation-rules engine: record one violation for `actor` under the given rule's `eventType` (see - * `MODERATION_VIOLATION_EVENT_TYPE` in settings/moderation-rules.ts). `targetKey` carries the repo#number for - * audit-trail/evidence purposes only -- the COUNT query above deliberately does not scope by it. */ -export async function recordModerationViolation(env: Env, args: { eventType: string; actor: string; targetKey: string; repoFullName: string; ruleReason: string }): Promise { + * `MODERATION_VIOLATION_EVENT_TYPE` in settings/moderation-rules.ts). `targetKey` carries the repo#number, + * and -- unlike the COUNT query above, which deliberately does not scope by it -- IS the idempotency key here + * (#gate-flagged): a webhook redelivery or queue retry that re-executes an already-recorded close must not + * double-count the SAME enforcement action toward the ban threshold. Returns whether a NEW row was actually + * inserted (false for an already-recorded duplicate), so the caller can skip redundant escalation work + * (re-labeling, re-checking the ban threshold) when nothing new actually happened. Best-effort, not a hard + * guarantee under true concurrency (no unique constraint on audit_events for this) -- matches this + * codebase's other check-then-act coalescing helpers, and is more than sufficient for the sequential + * redelivery/retry pattern it defends against. */ +export async function recordModerationViolation(env: Env, args: { eventType: string; actor: string; targetKey: string; repoFullName: string; ruleReason: string }): Promise { + if (await hasModerationViolationForTarget(env, args.actor, args.eventType, args.targetKey)) return false; await recordAuditEvent(env, { eventType: args.eventType, actor: args.actor, @@ -2397,11 +2419,25 @@ export async function recordModerationViolation(env: Env, args: { eventType: str detail: args.ruleReason, metadata: { repoFullName: args.repoFullName }, }); + return true; +} + +// #gate-flagged: same non-clamping, non-rounding shape as normalizeOpenItemCap, PLUS an upper bound -- +// unlike an ordinary open-item cap, this value feeds Date arithmetic on the LIVE close path +// (`Date.now() - violationDecayDays * 86400000`); an unbounded value (e.g. a typo adding extra zeros) can +// overflow into an Invalid Date, and calling .toISOString() on an Invalid Date THROWS, crashing the close. +// Clamped (Math.min), not dropped to null, mirroring normalizeReviewNagCooldownDays' own clamping shape for +// the same "still meaningful, just bounded" family of day-count settings. +function normalizeModerationDecayDays(value: number | null | undefined): number | null { + const parsed = normalizeOpenItemCap(value); + return parsed === null ? null : Math.min(parsed, MAX_MODERATION_VIOLATION_DECAY_DAYS); } -/** Read the singleton global moderation-rules engine config (#selfhost-mod-engine). Missing table or malformed - * JSON fail open to {@link DEFAULT_GLOBAL_MODERATION_CONFIG} (`enabled: false`) -- a DB hiccup on this path - * must never accidentally turn ON a layer capable of auto-banning a contributor across every gated repo. */ +/** Read the singleton global moderation-rules engine config (#selfhost-mod-engine). A missing table/row fails + * open to the FULL {@link DEFAULT_GLOBAL_MODERATION_CONFIG} (`enabled: false`) -- a DB hiccup on this path + * must never accidentally turn ON a layer capable of auto-banning a contributor across every gated repo. + * Malformed JSON in an otherwise-present row is narrower: only `rules_json` degrades (to an empty rules + * list, via `normalizeModerationRules`), while every other column is still read from the row as normal. */ export async function getGlobalModerationConfig(env: Env): Promise { try { const row = await env.DB.prepare( @@ -2422,7 +2458,7 @@ export async function getGlobalModerationConfig(env: Env): Promise undefined); + // #gate-flagged: idempotent per (actor, eventType, targetKey) -- a webhook redelivery or queue retry that + // re-executes an ALREADY-recorded close is not a new violation, so skip the rest of escalation entirely + // (re-labeling/re-checking the ban threshold off a stale "nothing new happened" pass is redundant, not just + // harmless). A write failure fails OPEN (treated as "new"), matching this function's existing best-effort + // philosophy elsewhere -- a lost write should not also silently suppress the escalation it was recording for. + const isNewViolation = await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE[rule], actor: args.authorLogin, targetKey, repoFullName: args.repoFullName, ruleReason: `${rule} violation` }).catch(() => true); + if (!isNewViolation) return; // #gate-flagged: count only the CURRENTLY-effective rule types, not every rule type ever recorded. A rule // an operator has excluded (globally or for this repo) must not go on influencing the ban decision just diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index d949fb0da1..5ec2d49c2e 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -717,15 +717,17 @@ describe("moderation-rules engine escalation (#selfhost-mod-engine)", () => { it("4 violations -> warning only; the 5th (default threshold) escalates to mod:banned + auto-blacklists the login", async () => { const env = createTestEnv({}); await upsertGlobalModerationConfig(env, { enabled: true }); + // Each iteration is a DIFFERENT PR (distinct pullNumber) -- 4 separate enforcement actions, not the same + // one replayed 4 times (the idempotency fix above correctly collapses same-target replays to ONE violation). for (let i = 0; i < 4; i++) { vi.clearAllMocks(); - await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); - expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); - expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", expect.anything()); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", pullNumber: 100 + i }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 100 + i, "mod:warning", { createMissingLabel: true }); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 100 + i, "mod:banned", expect.anything()); } vi.clearAllMocks(); - await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); - expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", { createMissingLabel: true }); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", pullNumber: 104 }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 104, "mod:banned", { createMissingLabel: true }); const blacklist = await getGlobalContributorBlacklist(env); expect(blacklist?.map((entry) => entry.login)).toContain("farmer99"); }); @@ -742,8 +744,10 @@ describe("moderation-rules engine escalation (#selfhost-mod-engine)", () => { it("does not double-add an actor who is already on the global blacklist", async () => { const env = createTestEnv({}); await upsertGlobalModerationConfig(env, { enabled: true, banThreshold: 1 }); - await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); - await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [{ ...coupledClose }, { ...coupledLabel }]); + // Two DISTINCT PRs (different pullNumber) -- two genuinely separate violations, not a same-target replay + // (which the idempotency fix would correctly no-op before ever reaching the blacklist-membership check). + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", pullNumber: 7 }), [coupledClose, coupledLabel]); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", pullNumber: 8 }), [{ ...coupledClose }, { ...coupledLabel }]); const blacklist = await getGlobalContributorBlacklist(env); expect(blacklist?.filter((entry) => entry.login === "farmer99")).toHaveLength(1); }); @@ -834,6 +838,27 @@ describe("moderation-rules engine escalation (#selfhost-mod-engine)", () => { expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", expect.anything()); }); + + it("REGRESSION (gate-flagged): a webhook redelivery / queue retry that re-executes the SAME close (same repo+number) does not double-count the violation or escalate past what the single real enforcement action warrants", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true, banThreshold: 2 }); + // First pass: this contributor's ONLY violation -> warning (1 < threshold 2). + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); + vi.clearAllMocks(); + vi.mocked(closePullRequest).mockResolvedValue({ state: "closed" }); + // A REPLAY of the exact same close (same pullNumber, same repo) -- e.g. GitHub redelivers the webhook, or + // the queue job retries after the mutation already succeeded. Must NOT count as a 2nd violation. + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel]); + expect(ensurePullRequestLabel).not.toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:banned", expect.anything()); + }); + + it("REGRESSION (gate-flagged): an absurdly large violationDecayDays does not throw on the live close path (clamped before it ever reaches Date arithmetic)", async () => { + const env = createTestEnv({}); + await upsertGlobalModerationConfig(env, { enabled: true, violationDecayDays: Number.MAX_SAFE_INTEGER }); + await expect(executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [coupledClose, coupledLabel])).resolves.not.toThrow(); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "mod:warning", { createMissingLabel: true }); + }); }); function issueCtx(over: Partial = {}): IssueActionExecutionContext { diff --git a/test/unit/moderation-config-db.test.ts b/test/unit/moderation-config-db.test.ts index 1ad792bd8b..22762c356e 100644 --- a/test/unit/moderation-config-db.test.ts +++ b/test/unit/moderation-config-db.test.ts @@ -3,12 +3,13 @@ import { countModerationViolationsForActor, getGlobalModerationConfig, getRepositorySettings, + hasModerationViolationForTarget, recordModerationViolation, upsertGlobalModerationConfig, upsertRepositorySettings, } from "../../src/db/repositories"; import { createTestEnv } from "../helpers/d1"; -import { DEFAULT_GLOBAL_MODERATION_CONFIG, MODERATION_VIOLATION_EVENT_TYPE } from "../../src/settings/moderation-rules"; +import { DEFAULT_GLOBAL_MODERATION_CONFIG, MAX_MODERATION_VIOLATION_DECAY_DAYS, MODERATION_VIOLATION_EVENT_TYPE } from "../../src/settings/moderation-rules"; describe("global moderation config DB round-trip (#selfhost-mod-engine)", () => { it("defaults to DEFAULT_GLOBAL_MODERATION_CONFIG (off) for a fresh install", async () => { @@ -79,6 +80,29 @@ describe("global moderation config DB round-trip (#selfhost-mod-engine)", () => expect(resolved.banThreshold).toBe(DEFAULT_GLOBAL_MODERATION_CONFIG.banThreshold); expect(resolved.warningLabel).toBe(DEFAULT_GLOBAL_MODERATION_CONFIG.warningLabel); }); + + it("REGRESSION (gate-flagged): violationDecayDays above MAX_MODERATION_VIOLATION_DECAY_DAYS is CLAMPED, not passed through raw -- an unbounded value overflows Date arithmetic on the live close path", async () => { + const env = createTestEnv(); + const resolved = await upsertGlobalModerationConfig(env, { violationDecayDays: MAX_MODERATION_VIOLATION_DECAY_DAYS + 1_000_000 }); + expect(resolved.violationDecayDays).toBe(MAX_MODERATION_VIOLATION_DECAY_DAYS); + expect(await getGlobalModerationConfig(env)).toEqual(resolved); + // Confirms the clamp actually keeps Date arithmetic sane -- this would throw (RangeError: Invalid time + // value) if the raw unclamped input were used instead. + expect(() => new Date(Date.now() - resolved.violationDecayDays! * 24 * 60 * 60 * 1000).toISOString()).not.toThrow(); + }); + + it("a violationDecayDays AT the max is preserved unclamped (boundary, not just strictly-under)", async () => { + const env = createTestEnv(); + const resolved = await upsertGlobalModerationConfig(env, { violationDecayDays: MAX_MODERATION_VIOLATION_DECAY_DAYS }); + expect(resolved.violationDecayDays).toBe(MAX_MODERATION_VIOLATION_DECAY_DAYS); + }); + + it("a raw DB row with an over-max violation_decay_days is also clamped on READ (not just on write)", async () => { + const env = createTestEnv(); + await env.DB.prepare("UPDATE global_moderation_config SET violation_decay_days = ? WHERE id = 'singleton'").bind(MAX_MODERATION_VIOLATION_DECAY_DAYS * 10).run(); + const resolved = await getGlobalModerationConfig(env); + expect(resolved.violationDecayDays).toBe(MAX_MODERATION_VIOLATION_DECAY_DAYS); + }); }); describe("moderation violation ledger (#selfhost-mod-engine)", () => { @@ -117,6 +141,46 @@ describe("moderation violation ledger (#selfhost-mod-engine)", () => { const count = await countModerationViolationsForActor(env, "nobody", Object.values(MODERATION_VIOLATION_EVENT_TYPE)); expect(count).toBe(0); }); + + it("REGRESSION (gate-flagged): recordModerationViolation is idempotent per (actor, eventType, targetKey) -- a webhook replay/queue retry re-recording the SAME close must not double-count it", async () => { + const env = createTestEnv(); + const args = { eventType: MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, actor: "farmer99", targetKey: "owner/repo#42", repoFullName: "owner/repo", ruleReason: "contributor_cap violation" }; + const firstInsert = await recordModerationViolation(env, args); + const secondInsert = await recordModerationViolation(env, args); // simulates a redelivered webhook / retried queue job + const thirdInsert = await recordModerationViolation(env, args); + expect(firstInsert).toBe(true); // a genuinely new violation + expect(secondInsert).toBe(false); // already recorded -- no-op + expect(thirdInsert).toBe(false); + const count = await countModerationViolationsForActor(env, "farmer99", [MODERATION_VIOLATION_EVENT_TYPE.contributor_cap]); + expect(count).toBe(1); // NOT 3 + }); + + it("a DIFFERENT targetKey (a different PR/issue) for the SAME actor+eventType is a genuinely new violation, not deduped", async () => { + const env = createTestEnv(); + const first = await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, actor: "farmer99", targetKey: "owner/repo#42", repoFullName: "owner/repo", ruleReason: "cap" }); + const second = await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, actor: "farmer99", targetKey: "owner/repo#43", repoFullName: "owner/repo", ruleReason: "cap" }); + expect(first).toBe(true); + expect(second).toBe(true); + const count = await countModerationViolationsForActor(env, "farmer99", [MODERATION_VIOLATION_EVENT_TYPE.contributor_cap]); + expect(count).toBe(2); + }); + + it("a DIFFERENT eventType on the SAME targetKey (e.g. a PR that trips both cap and blacklist) is a genuinely new violation, not deduped", async () => { + const env = createTestEnv(); + const cap = await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, actor: "farmer99", targetKey: "owner/repo#42", repoFullName: "owner/repo", ruleReason: "cap" }); + const blacklist = await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.blacklist, actor: "farmer99", targetKey: "owner/repo#42", repoFullName: "owner/repo", ruleReason: "blacklist" }); + expect(cap).toBe(true); + expect(blacklist).toBe(true); + }); + + describe("hasModerationViolationForTarget", () => { + it("returns false before any violation is recorded, true after, with NO time window (unlike hasRecentAuditEvent)", async () => { + const env = createTestEnv(); + expect(await hasModerationViolationForTarget(env, "farmer99", MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, "owner/repo#42")).toBe(false); + await recordModerationViolation(env, { eventType: MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, actor: "farmer99", targetKey: "owner/repo#42", repoFullName: "owner/repo", ruleReason: "cap" }); + expect(await hasModerationViolationForTarget(env, "farmer99", MODERATION_VIOLATION_EVENT_TYPE.contributor_cap, "owner/repo#42")).toBe(true); + }); + }); }); describe("per-repo moderation settings DB round-trip (#selfhost-mod-engine)", () => { From 14356e315c8e13d42df33590a7cc9c701be0854e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:34:05 -0700 Subject: [PATCH 6/6] chore(selfhost): renumber moderation-engine migrations past another colliding 0103 merged upstream --- apps/gittensory-ui/public/openapi.json | 60 +++++++++++++++++++ ....sql => 0104_global_moderation_config.sql} | 0 ...> 0105_repository_moderation_settings.sql} | 2 +- 3 files changed, 61 insertions(+), 1 deletion(-) rename migrations/{0103_global_moderation_config.sql => 0104_global_moderation_config.sql} (100%) rename migrations/{0104_repository_moderation_settings.sql => 0105_repository_moderation_settings.sql} (93%) diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index fbc0106837..956a0198cf 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8994,6 +8994,66 @@ }, "moderationBannedLabel": { "type": "string" + }, + "typeLabels": { + "type": "object", + "properties": { + "bug": { + "type": "string" + }, + "feature": { + "type": "string" + }, + "priority": { + "type": "string" + } + }, + "required": [ + "bug", + "feature", + "priority" + ] + }, + "linkedIssueLabelPropagation": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "mode": { + "type": "string", + "enum": [ + "exclusive_type_label" + ] + }, + "mappings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "issueLabel": { + "type": "string" + }, + "prLabel": { + "type": "string" + }, + "removeOtherTypeLabels": { + "type": "boolean" + } + }, + "required": [ + "issueLabel", + "prLabel", + "removeOtherTypeLabels" + ] + } + } + }, + "required": [ + "enabled", + "mode", + "mappings" + ] } }, "required": [ diff --git a/migrations/0103_global_moderation_config.sql b/migrations/0104_global_moderation_config.sql similarity index 100% rename from migrations/0103_global_moderation_config.sql rename to migrations/0104_global_moderation_config.sql diff --git a/migrations/0104_repository_moderation_settings.sql b/migrations/0105_repository_moderation_settings.sql similarity index 93% rename from migrations/0104_repository_moderation_settings.sql rename to migrations/0105_repository_moderation_settings.sql index 96ad6bac0d..029de5f908 100644 --- a/migrations/0104_repository_moderation_settings.sql +++ b/migrations/0105_repository_moderation_settings.sql @@ -1,5 +1,5 @@ -- Per-repo overrides for the moderation-rules engine (#selfhost-mod-engine), layered over --- global_moderation_config (0103). moderation_gate_mode defaults to 'inherit' (defers to the global master +-- global_moderation_config (0104). moderation_gate_mode defaults to 'inherit' (defers to the global master -- switch) -- 'off'/'enabled' let one repo opt out of or into the whole layer regardless of the global default, -- e.g. an operator piloting the feature on a single repo before flipping the global default on. The three -- override columns are nullable: NULL means "inherit the global value", never "unset to empty/off" -- an