From 3eb5ccf61c8c556aa105e9dc58f0d4ae17464f99 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 1 Feb 2026 17:32:07 +0300 Subject: [PATCH] treewide: implement authorized users for polling; cleanup Signed-off-by: NotAShelf Change-Id: I0c72309281e8c67e4ee4333c4c3bc1fe6a6a6964 --- config.example.ts | 12 +++++ src/github.ts | 71 +++++++++++++++++++------ src/index.ts | 54 +++++++++++++------ src/polling.ts | 131 +++++++++++++++++++++++++++++++++++++++++++--- src/server.ts | 47 ++++++++++++++++- src/types.ts | 4 ++ 6 files changed, 279 insertions(+), 40 deletions(-) diff --git a/config.example.ts b/config.example.ts index d922e4d..a195e45 100644 --- a/config.example.ts +++ b/config.example.ts @@ -116,6 +116,18 @@ const config: Config = { enabled: false, intervalMinutes: 5, // how often to check for new comments lookbackMinutes: 10, // how far back to look for comments on each poll + // Backfill: Catch up on missed pings after restart by persisting last processed timestamp + // backfill: true, + // stateFile: '.troutbot-polling-state.json', // where to store the state (optional, has default) + // Authorized users: Only these users can trigger on-demand analysis via @troutbot mentions + // Leave empty to allow all users (not recommended for public repos) + // authorizedUsers: ['trusted-user-1', 'trusted-user-2'], + // Polling-specific repositories: Override global repositories list for polling only + // If set, only these repos will be polled for @troutbot mentions + // Unauthorized repos will get a thumbsdown reaction and be ignored + // repositories: [ + // { owner: 'myorg', repo: 'myrepo' }, + // ], }, }; diff --git a/src/github.ts b/src/github.ts index 13fe784..40bdc95 100644 --- a/src/github.ts +++ b/src/github.ts @@ -69,6 +69,35 @@ export async function updateComment( getLogger().info(`Updated comment ${commentId} on ${owner}/${repo}`); } +export async function createReaction( + owner: string, + repo: string, + commentId: number, + reaction: + | 'thumbs_up' + | 'thumbs_down' + | 'laugh' + | 'confused' + | 'heart' + | 'hooray' + | 'eyes' + | 'rocket' +): Promise { + if (!octokit) { + getLogger().info(`[dry-run] Would add ${reaction} reaction to comment ${commentId}`); + return; + } + // Map thumbs_up/thumbs_down to GitHub API format (+1/-1) + const content = reaction === 'thumbs_up' ? '+1' : reaction === 'thumbs_down' ? '-1' : reaction; + await octokit.reactions.createForIssueComment({ + owner, + repo, + comment_id: commentId, + content: content as '+1' | '-1' | 'laugh' | 'confused' | 'heart' | 'hooray' | 'eyes' | 'rocket', + }); + getLogger().info(`Added ${reaction} reaction to comment ${commentId}`); +} + // Data fetching for engine backends export async function fetchCheckRuns( owner: string, @@ -133,15 +162,20 @@ export async function fetchPR( } | null> { if (!octokit) return null; - const { data } = await octokit.pulls.get({ owner, repo, pull_number: prNumber }); - return { - title: data.title, - body: data.body || '', - author: data.user?.login || '', - labels: (data.labels || []).map((l) => (typeof l === 'string' ? l : l.name || '')), - branch: data.head.ref, - sha: data.head.sha, - }; + try { + const { data } = await octokit.pulls.get({ owner, repo, pull_number: prNumber }); + return { + title: data.title, + body: data.body || '', + author: data.user?.login || '', + labels: (data.labels || []).map((l) => (typeof l === 'string' ? l : l.name || '')), + branch: data.head.ref, + sha: data.head.sha, + }; + } catch (err) { + getLogger().debug(`Failed to fetch PR ${owner}/${repo}#${prNumber}`, err); + return null; + } } export async function fetchIssue( @@ -156,13 +190,18 @@ export async function fetchIssue( } | null> { if (!octokit) return null; - const { data } = await octokit.issues.get({ owner, repo, issue_number: issueNumber }); - return { - title: data.title, - body: data.body || '', - author: data.user?.login || '', - labels: (data.labels || []).map((l) => (typeof l === 'string' ? l : l.name || '')), - }; + try { + const { data } = await octokit.issues.get({ owner, repo, issue_number: issueNumber }); + return { + title: data.title, + body: data.body || '', + author: data.user?.login || '', + labels: (data.labels || []).map((l) => (typeof l === 'string' ? l : l.name || '')), + }; + } catch (err) { + getLogger().debug(`Failed to fetch issue ${owner}/${repo}#${issueNumber}`, err); + return null; + } } export interface RecentComment { diff --git a/src/index.ts b/src/index.ts index 1545763..a59a62a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,6 +3,7 @@ import { initLogger, getLogger } from './logger.js'; import { initGitHub, fetchPR, + fetchIssue, hasExistingComment, postComment, updateComment, @@ -34,25 +35,44 @@ async function analyzeOne(target: string) { initGitHub(process.env.GITHUB_TOKEN); + // Try to fetch as PR first, then fall back to issue + let event: WebhookEvent; const prData = await fetchPR(owner, repo, prNumber); - if (!prData) { - logger.error(`Could not fetch PR ${owner}/${repo}#${prNumber}`); - process.exit(1); - } - const event: WebhookEvent = { - action: 'analyze', - type: 'pull_request', - number: prNumber, - title: prData.title, - body: prData.body, - owner, - repo, - author: prData.author, - labels: prData.labels, - branch: prData.branch, - sha: prData.sha, - }; + if (prData) { + event = { + action: 'analyze', + type: 'pull_request', + number: prNumber, + title: prData.title, + body: prData.body, + owner, + repo, + author: prData.author, + labels: prData.labels, + branch: prData.branch, + sha: prData.sha, + }; + } else { + // Try as issue + const issueData = await fetchIssue(owner, repo, prNumber); + if (!issueData) { + logger.error(`Could not fetch PR or issue ${owner}/${repo}#${prNumber}`); + process.exit(1); + } + + event = { + action: 'analyze', + type: 'issue', + number: prNumber, + title: issueData.title, + body: issueData.body, + owner, + repo, + author: issueData.author, + labels: issueData.labels, + }; + } const engine = createEngine(config.engine); const analysis = await engine.analyze(event); diff --git a/src/polling.ts b/src/polling.ts index 090dce6..bdeb33c 100644 --- a/src/polling.ts +++ b/src/polling.ts @@ -1,4 +1,4 @@ -import type { Config, WebhookEvent } from './types.js'; +import type { Config, WebhookEvent, RepoConfig } from './types.js'; import { listRecentComments, fetchPR, @@ -7,20 +7,75 @@ import { postComment, updateComment, formatComment, + createReaction, type RecentComment, } from './github.js'; import { createEngine } from './engine/index.js'; import { getLogger } from './logger.js'; import { recordEvent } from './events.js'; +import { readFileSync, writeFileSync, existsSync } from 'fs'; interface ProcessedComment { id: number; timestamp: number; } +interface PollingState { + lastProcessedAt: Record; +} + const processedComments: Map = new Map(); const MAX_PROCESSED_CACHE = 1000; +let pollingState: PollingState = { lastProcessedAt: {} }; + +function loadPollingState(stateFile: string): void { + if (existsSync(stateFile)) { + try { + const data = readFileSync(stateFile, 'utf-8'); + const parsed = JSON.parse(data); + // Validate that parsed data has expected structure + if ( + parsed && + typeof parsed === 'object' && + parsed.lastProcessedAt && + typeof parsed.lastProcessedAt === 'object' + ) { + pollingState = parsed; + } else { + getLogger().warn('Invalid polling state format, resetting to empty state'); + pollingState = { lastProcessedAt: {} }; + } + } catch { + // Ignore parse errors, use empty state + pollingState = { lastProcessedAt: {} }; + } + } +} + +function savePollingState(stateFile: string): void { + try { + writeFileSync(stateFile, JSON.stringify(pollingState, null, 2)); + } catch (err) { + getLogger().warn('Failed to save polling state', err); + } +} + +function getRepoKey(owner: string, repo: string): string { + return `${owner}/${repo}`; +} + +function getLastProcessedAt(owner: string, repo: string): Date | undefined { + const key = getRepoKey(owner, repo); + const timestamp = pollingState.lastProcessedAt[key]; + return timestamp ? new Date(timestamp) : undefined; +} + +function setLastProcessedAt(owner: string, repo: string, date: Date): void { + const key = getRepoKey(owner, repo); + pollingState.lastProcessedAt[key] = date.toISOString(); +} + function getCacheKey(owner: string, repo: string, commentId: number): string { return `${owner}/${repo}#${commentId}`; } @@ -92,6 +147,21 @@ async function analyzeAndComment( return result; } +function isAuthorized(username: string, authorizedUsers?: string[]): boolean { + if (!authorizedUsers || authorizedUsers.length === 0) { + return true; // No restrictions + } + const normalizedUsername = username.toLowerCase(); + return authorizedUsers.some((u) => u.toLowerCase() === normalizedUsername); +} + +function isRepoAuthorized(owner: string, repo: string, pollingRepos?: RepoConfig[]): boolean { + if (!pollingRepos || pollingRepos.length === 0) { + return true; // No restrictions, use global repos + } + return pollingRepos.some((r) => r.owner === owner && r.repo === repo); +} + async function processComment( comment: RecentComment, owner: string, @@ -109,6 +179,27 @@ async function processComment( return; } + // Check if repo is authorized for polling + const pollingRepos = config.polling?.repositories; + if (!isRepoAuthorized(owner, repo, pollingRepos)) { + logger.info( + `Unauthorized repo ${owner}/${repo} for polling, ignoring mention from ${comment.author}` + ); + await createReaction(owner, repo, comment.id, 'thumbs_down'); + markProcessed(owner, repo, comment.id); + return; + } + + // Check if user is authorized + const authorizedUsers = config.polling?.authorizedUsers; + if (!isAuthorized(comment.author, authorizedUsers)) { + logger.info( + `Unauthorized user ${comment.author} attempted on-demand analysis in ${owner}/${repo}#${comment.issueNumber}` + ); + markProcessed(owner, repo, comment.id); + return; + } + logger.info(`Found @troutbot mention in ${owner}/${repo}#${comment.issueNumber}`); try { @@ -168,7 +259,8 @@ async function pollRepository( owner: string, repo: string, config: Config, - since: Date + since: Date, + stateFile?: string ): Promise { const logger = getLogger(); @@ -176,8 +268,20 @@ async function pollRepository( const comments = await listRecentComments(owner, repo, since); logger.debug(`Fetched ${comments.length} recent comments from ${owner}/${repo}`); + let latestCommentDate = since; + for (const comment of comments) { await processComment(comment, owner, repo, config); + const commentDate = new Date(comment.createdAt); + if (commentDate > latestCommentDate) { + latestCommentDate = commentDate; + } + } + + // Update last processed timestamp + setLastProcessedAt(owner, repo, latestCommentDate); + if (stateFile) { + savePollingState(stateFile); } } catch (err) { logger.error(`Failed to poll ${owner}/${repo}`, err); @@ -206,10 +310,25 @@ export async function startPolling(config: Config): Promise { `Poll interval: ${pollingConfig.intervalMinutes} minutes, lookback: ${pollingConfig.lookbackMinutes} minutes` ); - // Do an initial poll - const initialSince = new Date(Date.now() - lookbackMs); + // Load persisted state if backfill is enabled + const stateFile = pollingConfig.backfill + ? pollingConfig.stateFile || '.troutbot-polling-state.json' + : undefined; + if (stateFile) { + loadPollingState(stateFile); + logger.info(`Polling state file: ${stateFile}`); + } + + // Do an initial poll - use persisted timestamp if available, otherwise use lookback for (const repo of config.repositories) { - await pollRepository(repo.owner, repo.repo, config, initialSince); + const lastProcessed = getLastProcessedAt(repo.owner, repo.repo); + const initialSince = lastProcessed || new Date(Date.now() - lookbackMs); + if (lastProcessed) { + logger.info( + `Resuming polling for ${repo.owner}/${repo.repo} from ${lastProcessed.toISOString()}` + ); + } + await pollRepository(repo.owner, repo.repo, config, initialSince, stateFile); } // Set up recurring polling @@ -217,7 +336,7 @@ export async function startPolling(config: Config): Promise { const since = new Date(Date.now() - lookbackMs); for (const repo of config.repositories) { - await pollRepository(repo.owner, repo.repo, config, since); + await pollRepository(repo.owner, repo.repo, config, since, stateFile); } }, intervalMs); } diff --git a/src/server.ts b/src/server.ts index b0cc53d..17ad3d3 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,7 +1,7 @@ import crypto from 'node:crypto'; import express from 'express'; import rateLimit from 'express-rate-limit'; -import type { Config, WebhookEvent } from './types.js'; +import type { Config, WebhookEvent, RepoConfig } from './types.js'; import { shouldProcess } from './filters.js'; import { createEngine } from './engine/index.js'; import { @@ -11,6 +11,7 @@ import { hasExistingComment, postComment, updateComment, + createReaction, } from './github.js'; import { getLogger } from './logger.js'; import { recordEvent } from './events.js'; @@ -257,6 +258,25 @@ async function handleCheckSuiteCompleted( } } +function isAuthorized(username: string, authorizedUsers?: string[]): boolean { + if (!authorizedUsers || authorizedUsers.length === 0) { + return true; // no restrictions + } + const normalizedUsername = username.toLowerCase(); + return authorizedUsers.some((u) => u.toLowerCase() === normalizedUsername); +} + +function isRepoAuthorizedForPolling( + owner: string, + repo: string, + pollingRepos?: RepoConfig[] +): boolean { + if (!pollingRepos || pollingRepos.length === 0) { + return true; // no restrictions, use global repos + } + return pollingRepos.some((r) => r.owner === owner && r.repo === repo); +} + async function handleOnDemandAnalysis( payload: Record, config: Config, @@ -270,6 +290,31 @@ async function handleOnDemandAnalysis( const issue = payload.issue as Record; const issueNumber = issue.number as number; const isPullRequest = issue.pull_request !== undefined; + const commentAuthor = (payload.comment as Record).user as Record< + string, + unknown + >; + const authorUsername = commentAuthor.login as string; + + // Check if repo is authorized for polling + const commentId = (payload.comment as Record).id as number; + const pollingRepos = config.polling?.repositories; + if (!isRepoAuthorizedForPolling(owner, repoName, pollingRepos)) { + logger.info( + `Unauthorized repo ${owner}/${repoName} for polling, ignoring mention from ${authorUsername}` + ); + await createReaction(owner, repoName, commentId, 'thumbs_down'); + return { skipped: true, reason: 'Unauthorized repository for polling' }; + } + + // Check if user is authorized + const authorizedUsers = config.polling?.authorizedUsers; + if (!isAuthorized(authorUsername, authorizedUsers)) { + logger.info( + `Unauthorized user ${authorUsername} attempted on-demand analysis in ${owner}/${repoName}#${issueNumber}` + ); + return { skipped: true, reason: 'Unauthorized user' }; + } logger.info( `On-demand analysis triggered for ${owner}/${repoName}#${issueNumber} (${isPullRequest ? 'PR' : 'issue'})` diff --git a/src/types.ts b/src/types.ts index 939a62e..2bbe471 100644 --- a/src/types.ts +++ b/src/types.ts @@ -13,6 +13,10 @@ export interface PollingConfig { enabled: boolean; intervalMinutes: number; lookbackMinutes: number; + backfill?: boolean; + stateFile?: string; + authorizedUsers?: string[]; + repositories?: RepoConfig[]; } export interface ServerConfig {