diff --git a/specs/notifications.md b/specs/notifications.md new file mode 100644 index 00000000..fee84b95 --- /dev/null +++ b/specs/notifications.md @@ -0,0 +1,132 @@ +# Notification System for Scheduled Triggers + +## Problem + +Warden's scheduled triggers analyze the full codebase on a cron. Today, findings go to a single tracking issue per skill that gets overwritten each run. There's no way to: +- Get notified in Slack when new findings appear +- Mark findings as false positives so they don't recur +- Track individual findings across runs + +## Goals + +1. Per-finding GitHub issues with semantic dedup across runs +2. Slack webhook notifications for new findings +3. Suppression file for false positives (rule-based pre-filtering) +4. Replace the single-tracking-issue approach with a provider-based notification layer + +## Configuration + +### `warden.toml` + +```toml +[[notifications]] +type = "github-issues" +labels = ["warden"] + +[[notifications]] +type = "slack" +webhookUrl = "$SLACK_WEBHOOK_URL" +``` + +Top-level `[[notifications]]` array. Each provider receives all non-suppressed findings independently. Environment variables expanded at runtime via `$VAR` syntax. + +### Migration + +The `[[notifications]]` section replaces the existing `schedule.issueTitle` tracking-issue approach. The `createOrUpdateIssue` code path and `schedule.issueTitle` config are removed. `schedule.createFixPR` and `schedule.fixBranchPrefix` remain (fix PRs are orthogonal to notifications). + +## Suppression File + +Located at `.agents/warden/suppressions.yaml`: + +```yaml +suppressions: + - skill: "security-audit" + paths: ["src/legacy/**"] + reason: "Legacy code, not worth fixing" + + - skill: "security-audit" + paths: ["src/admin/query.ts"] + title: "SQL injection" + reason: "Uses parameterized queries, false positive" +``` + +Rules match on: +- **skill** (required): Exact skill name +- **paths** (required): Glob patterns matched against finding location path +- **title** (optional): Substring match against finding title +- **reason** (required): Human-readable justification + +Loaded once per workflow run, applied before any provider receives findings. + +## Provider Interface + +```typescript +interface NotificationProvider { + readonly name: string; + notify(context: NotificationContext): Promise; +} + +interface NotificationContext { + findings: Finding[]; + reports: SkillReport[]; + repository: { owner: string; name: string }; + commitSha: string; +} + +interface NotificationResult { + provider: string; + sent: number; + skipped: number; + errors: string[]; +} +``` + +## Provider Flow + +``` +Skill Report -> Apply Suppressions -> All Providers (each gets same findings) + |-- github-issues (semantic dedup, creates/skips per finding) + |-- slack (sends all findings it receives) +``` + +## GitHub Issues Provider + +- Creates one issue per unique finding +- Dedup: two-tier + 1. Hash match via `` marker in issue body (cheap, catches identical text) + 2. Semantic match via Haiku for same-file findings with no hash match (handles LLM variation) +- Also checks closed issues with `warden:false-positive` label (treated as suppressed) +- Issue title: `[Warden] {finding.title}` +- Labels: configurable base labels + `warden:{skillName}` +- Body: severity, description, location with code link, suggested fix, hash marker +- Config: `labels` (string array, default `["warden"]`) + +## Slack Provider + +- Posts to incoming webhook URL using Block Kit formatting +- Sends all non-suppressed findings it receives +- Message: repo, commit, severity summary, up to 10 findings with details +- Skips notification if findings array is empty +- Config: `webhookUrl` (string, supports `$ENV_VAR` expansion) + +## Schedule Workflow Integration + +In `src/action/workflow/schedule.ts`, the `createOrUpdateIssue` call is replaced with: + +```typescript +const dispatcher = new NotificationDispatcher(providers, suppressions); +const result = await dispatcher.dispatch({ + findings: report.findings, + reports: [report], + repository: { owner, name: repo }, + commitSha: headSha, + skillName: resolved.name, +}); +``` + +## Future Work + +- Persistent storage for "previously seen" finding tracking across all providers +- `autoClose` config flag on GitHub Issues provider (close issues when findings disappear) +- CLI command for managing suppressions (`warden suppress add`) +- PR trigger integration (generic enough, but PR comments already have sophisticated dedup) diff --git a/src/action/workflow/__fixtures__/schedule-title/warden.toml b/src/action/workflow/__fixtures__/schedule-title/warden.toml index e8b9764d..6605fbfd 100644 --- a/src/action/workflow/__fixtures__/schedule-title/warden.toml +++ b/src/action/workflow/__fixtures__/schedule-title/warden.toml @@ -8,4 +8,4 @@ paths = ["src/**/*.ts"] type = "schedule" [skills.triggers.schedule] -issueTitle = "Custom Issue Title" +createFixPR = false diff --git a/src/action/workflow/schedule.test.ts b/src/action/workflow/schedule.test.ts index a5a4fbfb..f67cbcf4 100644 --- a/src/action/workflow/schedule.test.ts +++ b/src/action/workflow/schedule.test.ts @@ -52,12 +52,24 @@ vi.mock('../../event/schedule-context.js', () => ({ buildScheduleEventContext: vi.fn(), })); -// Mock GitHub issue/PR creation +// Mock GitHub PR creation (createOrUpdateIssue removed; notifications handle issues now) vi.mock('../../output/github-issues.js', () => ({ - createOrUpdateIssue: vi.fn(), createFixPR: vi.fn(), })); +// Mock suppressions loader +vi.mock('../../suppressions/loader.js', () => ({ + loadSuppressions: vi.fn(() => []), +})); + +// Mock notification system +vi.mock('../../notifications/index.js', () => ({ + NotificationDispatcher: vi.fn().mockImplementation(() => ({ + dispatch: vi.fn(() => Promise.resolve({ suppressed: 0, results: [] })), + })), + buildProviders: vi.fn(() => []), +})); + // Mock skill loader — filesystem reads; keep clearSkillsCache real vi.mock('../../skills/loader.js', async () => { const actual = await vi.importActual('../../skills/loader.js'); @@ -76,7 +88,7 @@ vi.mock('../../skills/loader.js', async () => { // Import after mocks import { runSkill } from '../../sdk/runner.js'; import { buildScheduleEventContext } from '../../event/schedule-context.js'; -import { createOrUpdateIssue, createFixPR } from '../../output/github-issues.js'; +import { createFixPR } from '../../output/github-issues.js'; import { resolveSkillAsync } from '../../skills/loader.js'; import { setFailed } from './base.js'; import { runScheduleWorkflow } from './schedule.js'; @@ -85,7 +97,6 @@ import { clearSkillsCache } from '../../skills/loader.js'; // Type the mocks const mockRunSkill = vi.mocked(runSkill); const mockBuildContext = vi.mocked(buildScheduleEventContext); -const mockCreateOrUpdateIssue = vi.mocked(createOrUpdateIssue); const mockCreateFixPR = vi.mocked(createFixPR); const mockResolveSkillAsync = vi.mocked(resolveSkillAsync); const mockSetFailed = vi.mocked(setFailed); @@ -195,11 +206,6 @@ describe('runScheduleWorkflow', () => { // Default mock: context with files, no findings mockBuildContext.mockResolvedValue(createScheduleContext()); mockRunSkill.mockResolvedValue(createSkillReport()); - mockCreateOrUpdateIssue.mockResolvedValue({ - issueNumber: 1, - issueUrl: 'https://github.com/test-owner/test-repo/issues/1', - created: true, - }); mockResolveSkillAsync.mockResolvedValue({ name: 'test-skill', description: 'Test skill', @@ -227,7 +233,6 @@ describe('runScheduleWorkflow', () => { await runScheduleWorkflow(mockOctokit, createDefaultInputs(), PR_ONLY_FIXTURES); expect(mockRunSkill).not.toHaveBeenCalled(); - expect(mockCreateOrUpdateIssue).not.toHaveBeenCalled(); }); it('fails when GITHUB_REPOSITORY is not set', async () => { @@ -270,7 +275,7 @@ describe('runScheduleWorkflow', () => { // --------------------------------------------------------------------------- describe('happy path', () => { - it('runs skill and creates issue when findings exist', async () => { + it('runs skill when findings exist', async () => { const finding = createFinding({ severity: 'high' }); const report = createSkillReport({ findings: [finding] }); mockRunSkill.mockResolvedValue(report); @@ -278,25 +283,14 @@ describe('runScheduleWorkflow', () => { await runScheduleWorkflow(mockOctokit, createDefaultInputs(), SCHEDULE_FIXTURES); expect(mockRunSkill).toHaveBeenCalledTimes(1); - expect(mockCreateOrUpdateIssue).toHaveBeenCalledWith( - mockOctokit, - 'test-owner', - 'test-repo', - [report], - expect.objectContaining({ - title: 'Warden: test-skill', - commitSha: 'abc123', - }) - ); }); - it('creates issue even when no findings', async () => { + it('runs skill even when no findings', async () => { mockRunSkill.mockResolvedValue(createSkillReport({ findings: [] })); await runScheduleWorkflow(mockOctokit, createDefaultInputs(), SCHEDULE_FIXTURES); expect(mockRunSkill).toHaveBeenCalledTimes(1); - expect(mockCreateOrUpdateIssue).toHaveBeenCalledTimes(1); }); it('skips skill run when no files match trigger', async () => { @@ -319,15 +313,14 @@ describe('runScheduleWorkflow', () => { await runScheduleWorkflow(mockOctokit, createDefaultInputs(), SCHEDULE_FIXTURES); expect(mockRunSkill).not.toHaveBeenCalled(); - expect(mockCreateOrUpdateIssue).not.toHaveBeenCalled(); }); }); // --------------------------------------------------------------------------- - // Issue & PR Creation + // Fix PR Creation // --------------------------------------------------------------------------- - describe('issue and PR creation', () => { + describe('fix PR creation', () => { it('creates fix PR when schedule.createFixPR is enabled', async () => { const finding = createFinding({ suggestedFix: { description: 'Fix it', diff: '--- a\n+++ b\n' }, @@ -369,9 +362,9 @@ describe('runScheduleWorkflow', () => { expect(mockCreateFixPR).not.toHaveBeenCalled(); }); - it('uses custom issue title from schedule config', async () => { - const report = createSkillReport({ findings: [] }); - mockRunSkill.mockResolvedValue(report); + it('does not create fix PR for schedule-title fixture', async () => { + const finding = createFinding(); + mockRunSkill.mockResolvedValue(createSkillReport({ findings: [finding] })); await runScheduleWorkflow( mockOctokit, @@ -379,15 +372,7 @@ describe('runScheduleWorkflow', () => { SCHEDULE_TITLE_FIXTURES ); - expect(mockCreateOrUpdateIssue).toHaveBeenCalledWith( - mockOctokit, - 'test-owner', - 'test-repo', - [report], - expect.objectContaining({ - title: 'Custom Issue Title', - }) - ); + expect(mockCreateFixPR).not.toHaveBeenCalled(); }); }); diff --git a/src/action/workflow/schedule.ts b/src/action/workflow/schedule.ts index c97753f5..d47e5141 100644 --- a/src/action/workflow/schedule.ts +++ b/src/action/workflow/schedule.ts @@ -7,12 +7,13 @@ import { dirname, join } from 'node:path'; import type { Octokit } from '@octokit/rest'; import { loadWardenConfig, resolveSkillConfigs } from '../../config/loader.js'; -import type { ScheduleConfig } from '../../config/schema.js'; import { buildScheduleEventContext } from '../../event/schedule-context.js'; import { runSkill } from '../../sdk/runner.js'; -import { createOrUpdateIssue, createFixPR } from '../../output/github-issues.js'; +import { createFixPR } from '../../output/github-issues.js'; import { shouldFail, countFindingsAtOrAbove, countSeverity } from '../../triggers/matcher.js'; import { resolveSkillAsync } from '../../skills/loader.js'; +import { loadSuppressions } from '../../suppressions/loader.js'; +import { NotificationDispatcher, buildProviders } from '../../notifications/index.js'; import type { SkillReport } from '../../types/index.js'; import type { ActionInputs } from '../inputs.js'; import { @@ -70,6 +71,13 @@ export async function runScheduleWorkflow( const defaultBranch = await getDefaultBranchFromAPI(octokit, owner, repo); + // Load suppressions and build notification providers + const suppressions = loadSuppressions(repoPath); + const providers = config.notifications + ? buildProviders({ configs: config.notifications, octokit, apiKey: inputs.anthropicApiKey }) + : []; + const dispatcher = new NotificationDispatcher(providers, suppressions); + logGroup('Processing schedule triggers'); for (const trigger of scheduleTriggers) { console.log(`- ${trigger.name}: ${trigger.skill}`); @@ -127,24 +135,21 @@ export async function runScheduleWorkflow( allReports.push(report); totalFindings += report.findings.length; - // Create/update issue with findings - const scheduleConfig: Partial = resolved.schedule ?? {}; - const issueTitle = scheduleConfig.issueTitle ?? `Warden: ${resolved.name}`; - - const issueResult = await createOrUpdateIssue(octokit, owner, repo, [report], { - title: issueTitle, - commitSha: headSha, - }); - - if (issueResult) { - console.log(`${issueResult.created ? 'Created' : 'Updated'} issue #${issueResult.issueNumber}`); - console.log(`Issue URL: ${issueResult.issueUrl}`); + // Dispatch notifications for findings + if (providers.length > 0) { + await dispatcher.dispatch({ + findings: report.findings, + reports: [report], + repository: { owner, name: repo }, + commitSha: headSha, + skillName: resolved.name, + }); } // Create fix PR if enabled and there are fixable findings - if (scheduleConfig.createFixPR) { + if (resolved.schedule?.createFixPR) { const fixResult = await createFixPR(octokit, owner, repo, report.findings, { - branchPrefix: scheduleConfig.fixBranchPrefix ?? 'warden-fix', + branchPrefix: resolved.schedule.fixBranchPrefix ?? 'warden-fix', baseBranch: defaultBranch, baseSha: headSha, repoPath, diff --git a/src/config/schema.ts b/src/config/schema.ts index f928f161..304184b4 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -34,8 +34,6 @@ export type SkillDefinition = z.infer; // Schedule-specific configuration export const ScheduleConfigSchema = z.object({ - /** Title for the tracking issue (default: "Warden: {skillName}") */ - issueTitle: z.string().optional(), /** Create PR with fixes when suggestedFix is available */ createFixPR: z.boolean().default(false), /** Branch prefix for fix PRs (default: "warden-fix") */ @@ -43,6 +41,27 @@ export const ScheduleConfigSchema = z.object({ }); export type ScheduleConfig = z.infer; +// Notification provider configurations +export const GitHubIssuesNotificationSchema = z.object({ + type: z.literal('github-issues'), + /** Labels to apply to created issues (default: ["warden"]) */ + labels: z.array(z.string()).default(['warden']), +}); +export type GitHubIssuesNotificationConfig = z.infer; + +export const SlackNotificationSchema = z.object({ + type: z.literal('slack'), + /** Slack incoming webhook URL. Supports $ENV_VAR expansion. */ + webhookUrl: z.string().min(1), +}); +export type SlackNotificationConfig = z.infer; + +export const NotificationConfigSchema = z.discriminatedUnion('type', [ + GitHubIssuesNotificationSchema, + SlackNotificationSchema, +]); +export type NotificationConfig = z.infer; + // Trigger type: where the trigger runs export const TriggerTypeSchema = z.enum(['pull_request', 'local', 'schedule']); export type TriggerType = z.infer; @@ -183,6 +202,8 @@ export const WardenConfigSchema = z defaults: DefaultsSchema.optional(), skills: z.array(SkillConfigSchema).default([]), runner: RunnerConfigSchema.optional(), + /** Notification providers for scheduled trigger findings */ + notifications: z.array(NotificationConfigSchema).optional(), }) .superRefine((config, ctx) => { const names = config.skills.map((s) => s.name); diff --git a/src/config/writer.test.ts b/src/config/writer.test.ts index 4b9030cf..476feb83 100644 --- a/src/config/writer.test.ts +++ b/src/config/writer.test.ts @@ -212,7 +212,6 @@ describe('generateSkillToml', () => { { type: 'schedule', schedule: { - issueTitle: 'Weekly Security Scan', createFixPR: true, fixBranchPrefix: 'security-fix', }, @@ -223,7 +222,6 @@ describe('generateSkillToml', () => { const result = generateSkillToml(skill); expect(result).toContain('[skills.triggers.schedule]'); - expect(result).toContain('issueTitle = "Weekly Security Scan"'); expect(result).toContain('createFixPR = true'); expect(result).toContain('fixBranchPrefix = "security-fix"'); }); diff --git a/src/config/writer.ts b/src/config/writer.ts index 2db7cd84..50300587 100644 --- a/src/config/writer.ts +++ b/src/config/writer.ts @@ -87,9 +87,6 @@ export function generateSkillToml(skill: SkillConfig): string { if (trigger.schedule) { lines.push(''); lines.push('[skills.triggers.schedule]'); - if (trigger.schedule.issueTitle) { - lines.push(`issueTitle = "${trigger.schedule.issueTitle}"`); - } if (trigger.schedule.createFixPR !== undefined) { lines.push(`createFixPR = ${trigger.schedule.createFixPR}`); } diff --git a/src/notifications/dispatcher.test.ts b/src/notifications/dispatcher.test.ts new file mode 100644 index 00000000..20f3db5b --- /dev/null +++ b/src/notifications/dispatcher.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { Finding } from '../types/index.js'; +import type { SuppressionRule } from '../suppressions/types.js'; +import type { NotificationProvider, NotificationResult } from './types.js'; +import { NotificationDispatcher } from './dispatcher.js'; + +function makeFinding(overrides: Partial = {}): Finding { + return { + id: 'f1', + severity: 'high', + title: 'SQL injection vulnerability', + description: 'Unsanitized user input', + location: { path: 'src/db/query.ts', startLine: 42 }, + ...overrides, + }; +} + +function makeProvider( + name: string, + result?: Partial +): NotificationProvider { + return { + name, + notify: vi.fn(() => + Promise.resolve({ + provider: name, + sent: 0, + skipped: 0, + errors: [], + ...result, + }) + ), + }; +} + +const baseContext = { + reports: [], + repository: { owner: 'test-owner', name: 'test-repo' }, + commitSha: 'abc123', + skillName: 'security-audit', +}; + +describe('NotificationDispatcher', () => { + it('passes findings to all providers', async () => { + const p1 = makeProvider('p1', { sent: 1 }); + const p2 = makeProvider('p2', { sent: 1 }); + const dispatcher = new NotificationDispatcher([p1, p2], []); + + const result = await dispatcher.dispatch({ + ...baseContext, + findings: [makeFinding()], + }); + + expect(result.suppressed).toBe(0); + expect(result.results).toHaveLength(2); + expect(p1.notify).toHaveBeenCalledTimes(1); + expect(p2.notify).toHaveBeenCalledTimes(1); + }); + + it('applies suppressions before sending to providers', async () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/db/**'], + reason: 'Known false positive', + }]; + + const provider = makeProvider('test'); + const dispatcher = new NotificationDispatcher([provider], rules); + + const result = await dispatcher.dispatch({ + ...baseContext, + findings: [makeFinding()], + }); + + expect(result.suppressed).toBe(1); + // Provider should receive empty findings + const notifyCall = vi.mocked(provider.notify).mock.calls[0]?.[0]; + expect(notifyCall?.findings).toEqual([]); + }); + + it('only suppresses matching findings', async () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/api/**'], + reason: 'test', + }]; + + const provider = makeProvider('test', { sent: 1 }); + const dispatcher = new NotificationDispatcher([provider], rules); + + const result = await dispatcher.dispatch({ + ...baseContext, + findings: [ + makeFinding({ id: 'f1', location: { path: 'src/db/query.ts', startLine: 1 } }), + makeFinding({ id: 'f2', location: { path: 'src/api/handler.ts', startLine: 1 } }), + ], + }); + + expect(result.suppressed).toBe(1); + const notifyCall = vi.mocked(provider.notify).mock.calls[0]?.[0]; + expect(notifyCall?.findings).toHaveLength(1); + expect(notifyCall?.findings[0]?.id).toBe('f1'); + }); + + it('handles provider errors gracefully', async () => { + const provider: NotificationProvider = { + name: 'failing', + notify: vi.fn(() => Promise.reject(new Error('Connection refused'))), + }; + const dispatcher = new NotificationDispatcher([provider], []); + + const result = await dispatcher.dispatch({ + ...baseContext, + findings: [makeFinding()], + }); + + expect(result.results).toHaveLength(1); + expect(result.results[0]?.errors).toContain('Connection refused'); + }); + + it('works with no providers', async () => { + const dispatcher = new NotificationDispatcher([], []); + + const result = await dispatcher.dispatch({ + ...baseContext, + findings: [makeFinding()], + }); + + expect(result.suppressed).toBe(0); + expect(result.results).toEqual([]); + }); + + it('skips suppression rules for different skill', async () => { + const rules: SuppressionRule[] = [{ + skill: 'other-skill', + paths: ['**/*'], + reason: 'test', + }]; + + const provider = makeProvider('test', { sent: 1 }); + const dispatcher = new NotificationDispatcher([provider], rules); + + const result = await dispatcher.dispatch({ + ...baseContext, + findings: [makeFinding()], + }); + + expect(result.suppressed).toBe(0); + const notifyCall = vi.mocked(provider.notify).mock.calls[0]?.[0]; + expect(notifyCall?.findings).toHaveLength(1); + }); +}); diff --git a/src/notifications/dispatcher.ts b/src/notifications/dispatcher.ts new file mode 100644 index 00000000..6cf762ba --- /dev/null +++ b/src/notifications/dispatcher.ts @@ -0,0 +1,80 @@ +import { applySuppression } from '../suppressions/matcher.js'; +import type { SuppressionRule } from '../suppressions/types.js'; +import type { NotificationProvider, NotificationContext, NotificationResult } from './types.js'; +import type { Finding, SkillReport } from '../types/index.js'; + +export interface DispatchContext { + findings: Finding[]; + reports: SkillReport[]; + repository: { owner: string; name: string }; + commitSha: string; + skillName: string; +} + +export interface DispatchResult { + suppressed: number; + results: NotificationResult[]; +} + +/** + * Orchestrates suppression filtering and sequential provider execution. + */ +export class NotificationDispatcher { + constructor( + private readonly providers: NotificationProvider[], + private readonly suppressions: SuppressionRule[] + ) {} + + async dispatch(context: DispatchContext): Promise { + // Apply suppressions + const filtered = applySuppression( + context.findings, + this.suppressions, + context.skillName + ); + const suppressed = context.findings.length - filtered.length; + + if (suppressed > 0) { + console.log(`Suppressed ${suppressed} finding${suppressed === 1 ? '' : 's'} via rules`); + } + + // Build the notification context with filtered findings + const notificationContext: NotificationContext = { + findings: filtered, + reports: context.reports, + repository: context.repository, + commitSha: context.commitSha, + skillName: context.skillName, + }; + + // Execute providers sequentially + const results: NotificationResult[] = []; + for (const provider of this.providers) { + try { + const result = await provider.notify(notificationContext); + results.push(result); + + if (result.sent > 0) { + console.log(`${provider.name}: sent ${result.sent} notification${result.sent === 1 ? '' : 's'}`); + } + if (result.skipped > 0) { + console.log(`${provider.name}: skipped ${result.skipped} (already tracked)`); + } + for (const error of result.errors) { + console.error(`${provider.name}: ${error}`); + } + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error(`${provider.name}: unexpected error: ${message}`); + results.push({ + provider: provider.name, + sent: 0, + skipped: 0, + errors: [message], + }); + } + } + + return { suppressed, results }; + } +} diff --git a/src/notifications/index.ts b/src/notifications/index.ts new file mode 100644 index 00000000..c96ff3f2 --- /dev/null +++ b/src/notifications/index.ts @@ -0,0 +1,50 @@ +import type { Octokit } from '@octokit/rest'; +import type { NotificationConfig } from '../config/schema.js'; +import type { NotificationProvider } from './types.js'; +import { GitHubIssuesProvider } from './providers/github-issues.js'; +import { SlackProvider } from './providers/slack.js'; + +export { NotificationDispatcher } from './dispatcher.js'; +export type { DispatchContext, DispatchResult } from './dispatcher.js'; +export type { NotificationProvider, NotificationContext, NotificationResult } from './types.js'; + +/** + * Expand environment variable references in a string. + * Replaces `$VAR_NAME` with the value of the environment variable. + */ +function expandEnvVars(value: string): string { + return value.replace(/\$([A-Z_][A-Z0-9_]*)/g, (_, name) => { + return process.env[name] ?? ''; + }); +} + +export interface BuildProvidersOptions { + configs: NotificationConfig[]; + octokit: Octokit; + apiKey: string; +} + +/** + * Build notification provider instances from configuration. + * Environment variable references in config values are expanded. + */ +export function buildProviders(options: BuildProvidersOptions): NotificationProvider[] { + const { configs, octokit, apiKey } = options; + const providers: NotificationProvider[] = []; + + for (const config of configs) { + switch (config.type) { + case 'github-issues': + providers.push(new GitHubIssuesProvider({ config, octokit, apiKey })); + break; + case 'slack': + providers.push(new SlackProvider({ + ...config, + webhookUrl: expandEnvVars(config.webhookUrl), + })); + break; + } + } + + return providers; +} diff --git a/src/notifications/providers/github-issues.test.ts b/src/notifications/providers/github-issues.test.ts new file mode 100644 index 00000000..4b508235 --- /dev/null +++ b/src/notifications/providers/github-issues.test.ts @@ -0,0 +1,234 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Octokit } from '@octokit/rest'; +import type { Finding } from '../../types/index.js'; +import type { NotificationContext } from '../types.js'; +import { + GitHubIssuesProvider, + generateIssueHash, + generateIssueMarker, + parseIssueMarker, +} from './github-issues.js'; + +// Mock Haiku to avoid real API calls +vi.mock('../../sdk/haiku.js', () => ({ + callHaiku: vi.fn(() => Promise.resolve({ success: true, data: [], usage: {} })), +})); + +import { callHaiku } from '../../sdk/haiku.js'; +const mockCallHaiku = vi.mocked(callHaiku); + +function makeFinding(overrides: Partial = {}): Finding { + return { + id: 'f1', + severity: 'high', + title: 'SQL injection vulnerability', + description: 'Unsanitized user input in query builder', + location: { path: 'src/db/query.ts', startLine: 42 }, + ...overrides, + }; +} + +function makeContext(overrides: Partial = {}): NotificationContext { + return { + findings: [makeFinding()], + reports: [], + repository: { owner: 'test-owner', name: 'test-repo' }, + commitSha: 'abc123def456', + skillName: 'security-audit', + ...overrides, + }; +} + +function createMockOctokit(existingIssues: unknown[] = []): Octokit { + return { + issues: { + listForRepo: vi.fn(() => Promise.resolve({ data: existingIssues })), + create: vi.fn(() => Promise.resolve({ data: { number: 1, html_url: 'https://example.com/1' } })), + getLabel: vi.fn(() => Promise.reject(new Error('Not found'))), + createLabel: vi.fn(() => Promise.resolve({ data: {} })), + }, + } as unknown as Octokit; +} + +describe('generateIssueHash', () => { + it('generates consistent hash for same input', () => { + const hash1 = generateIssueHash('title', 'desc'); + const hash2 = generateIssueHash('title', 'desc'); + expect(hash1).toBe(hash2); + }); + + it('generates different hash for different input', () => { + const hash1 = generateIssueHash('title1', 'desc'); + const hash2 = generateIssueHash('title2', 'desc'); + expect(hash1).not.toBe(hash2); + }); + + it('returns 16-char hex string', () => { + const hash = generateIssueHash('title', 'desc'); + expect(hash).toMatch(/^[a-f0-9]{16}$/); + }); +}); + +describe('generateIssueMarker / parseIssueMarker', () => { + it('round-trips marker', () => { + const hash = 'abcdef0123456789'; + const marker = generateIssueMarker(hash); + expect(parseIssueMarker(marker)).toBe(hash); + }); + + it('returns null for body without marker', () => { + expect(parseIssueMarker('some body text')).toBeNull(); + }); +}); + +describe('GitHubIssuesProvider', () => { + let mockOctokit: Octokit; + + beforeEach(() => { + vi.resetAllMocks(); + mockOctokit = createMockOctokit(); + mockCallHaiku.mockResolvedValue({ success: true, data: [], usage: {} as never }); + }); + + function createProvider(octokit?: Octokit) { + return new GitHubIssuesProvider({ + config: { type: 'github-issues', labels: ['warden'] }, + octokit: octokit ?? mockOctokit, + apiKey: 'test-key', + }); + } + + it('creates issue for new finding', async () => { + const provider = createProvider(); + const result = await provider.notify(makeContext()); + + expect(result.sent).toBe(1); + expect(result.skipped).toBe(0); + expect(result.errors).toEqual([]); + expect(vi.mocked(mockOctokit.issues.create)).toHaveBeenCalledWith( + expect.objectContaining({ + owner: 'test-owner', + repo: 'test-repo', + title: '[Warden] SQL injection vulnerability', + labels: ['warden', 'warden:security-audit'], + }) + ); + }); + + it('skips creating issue when hash matches existing open issue', async () => { + const finding = makeFinding(); + const hash = generateIssueHash(finding.title, finding.description); + const existingIssue = { + number: 42, + title: '[Warden] SQL injection vulnerability', + body: `Some body\n${generateIssueMarker(hash)}`, + state: 'open', + labels: [{ name: 'warden' }], + }; + + mockOctokit = createMockOctokit([existingIssue]); + const provider = createProvider(mockOctokit); + const result = await provider.notify(makeContext({ findings: [finding] })); + + expect(result.sent).toBe(0); + expect(result.skipped).toBe(1); + expect(vi.mocked(mockOctokit.issues.create)).not.toHaveBeenCalled(); + }); + + it('skips creating issue when hash matches closed false-positive', async () => { + const finding = makeFinding(); + const hash = generateIssueHash(finding.title, finding.description); + const closedIssue = { + number: 99, + title: '[Warden] SQL injection vulnerability', + body: `Some body\n${generateIssueMarker(hash)}`, + state: 'closed', + labels: [{ name: 'warden' }, { name: 'warden:false-positive' }], + }; + + // listForRepo is called twice: once for open, once for closed + vi.mocked(mockOctokit.issues.listForRepo) + .mockResolvedValueOnce({ data: [] } as never) + .mockResolvedValueOnce({ data: [closedIssue] } as never); + + const provider = createProvider(mockOctokit); + const result = await provider.notify(makeContext({ findings: [finding] })); + + expect(result.sent).toBe(0); + expect(result.skipped).toBe(1); + }); + + it('skips finding when semantic match found via Haiku', async () => { + const existingIssue = { + number: 10, + title: '[Warden] SQL injection risk', + body: `Body\n`, + state: 'open', + labels: [{ name: 'warden' }], + }; + + vi.mocked(mockOctokit.issues.listForRepo) + .mockResolvedValueOnce({ data: [existingIssue] } as never) + .mockResolvedValueOnce({ data: [] } as never); + + // Haiku says finding 1 matches issue 1 + mockCallHaiku.mockResolvedValue({ + success: true, + data: [{ findingIndex: 1, issueIndex: 1 }], + usage: {} as never, + }); + + const provider = createProvider(mockOctokit); + const result = await provider.notify(makeContext()); + + expect(result.sent).toBe(0); + expect(result.skipped).toBe(1); + }); + + it('returns empty result when no findings', async () => { + const provider = createProvider(); + const result = await provider.notify(makeContext({ findings: [] })); + + expect(result.sent).toBe(0); + expect(result.skipped).toBe(0); + expect(vi.mocked(mockOctokit.issues.listForRepo)).not.toHaveBeenCalled(); + }); + + it('reports errors when issue creation fails', async () => { + vi.mocked(mockOctokit.issues.create).mockRejectedValue(new Error('Rate limited')); + + const provider = createProvider(); + const result = await provider.notify(makeContext()); + + expect(result.sent).toBe(0); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toContain('Rate limited'); + }); + + it('creates issues for multiple new findings', async () => { + const findings = [ + makeFinding({ id: 'f1', title: 'Bug 1' }), + makeFinding({ id: 'f2', title: 'Bug 2' }), + ]; + + const provider = createProvider(); + const result = await provider.notify(makeContext({ findings })); + + expect(result.sent).toBe(2); + expect(vi.mocked(mockOctokit.issues.create)).toHaveBeenCalledTimes(2); + }); + + it('issue body contains severity, description, location link, and hash marker', async () => { + const provider = createProvider(); + await provider.notify(makeContext()); + + const createCall = vi.mocked(mockOctokit.issues.create).mock.calls[0]?.[0]; + const body = createCall?.body as string; + + expect(body).toContain('**Severity:** high'); + expect(body).toContain('`security-audit`'); + expect(body).toContain('src/db/query.ts'); + expect(body).toContain('`; +} + +/** + * Parse a warden issue marker from an issue body. + */ +export function parseIssueMarker(body: string): string | null { + const match = body.match(//); + return match?.[1] ?? null; +} + +interface ExistingIssue { + number: number; + title: string; + body: string; + state: 'open' | 'closed'; + labels: string[]; + hash: string | null; +} + +/** + * Fetch existing Warden issues (open + false-positive closed) from the repo. + */ +async function fetchWardenIssues( + octokit: Octokit, + owner: string, + repo: string, + labels: string[] +): Promise { + const primaryLabel = labels[0] ?? 'warden'; + const issues: ExistingIssue[] = []; + + // Fetch open issues with the primary label + const { data: openIssues } = await octokit.issues.listForRepo({ + owner, + repo, + state: 'open', + labels: primaryLabel, + per_page: 100, + }); + + for (const issue of openIssues) { + if (issue.pull_request) continue; + issues.push({ + number: issue.number, + title: issue.title, + body: issue.body ?? '', + state: 'open', + labels: issue.labels + .map((l) => (typeof l === 'string' ? l : l.name)) + .filter((n): n is string => !!n), + hash: parseIssueMarker(issue.body ?? ''), + }); + } + + // Fetch closed issues with false-positive label + const { data: closedIssues } = await octokit.issues.listForRepo({ + owner, + repo, + state: 'closed', + labels: `${primaryLabel},${FALSE_POSITIVE_LABEL}`, + per_page: 100, + }); + + for (const issue of closedIssues) { + if (issue.pull_request) continue; + issues.push({ + number: issue.number, + title: issue.title, + body: issue.body ?? '', + state: 'closed', + labels: issue.labels + .map((l) => (typeof l === 'string' ? l : l.name)) + .filter((n): n is string => !!n), + hash: parseIssueMarker(issue.body ?? ''), + }); + } + + return issues; +} + +/** Schema for LLM semantic matching response */ +const SemanticMatchSchema = z.array( + z.object({ + findingIndex: z.number().int(), + issueIndex: z.number().int(), + }) +); + +/** + * Use Haiku to find semantic matches between findings and existing issues. + */ +async function findSemanticMatches( + findings: Finding[], + existingIssues: ExistingIssue[], + apiKey: string +): Promise> { + if (findings.length === 0 || existingIssues.length === 0) { + return new Map(); + } + + const issueList = existingIssues + .map((issue, i) => `${i + 1}. "${issue.title}" (${issue.state})`) + .join('\n'); + + const findingList = findings + .map((f, i) => { + const loc = f.location ? `${f.location.path}:${f.location.startLine}` : 'general'; + return `${i + 1}. [${loc}] "${f.title}" - ${f.description}`; + }) + .join('\n'); + + const prompt = `Compare these code findings against existing tracked issues and identify matches. + +Existing issues: +${issueList} + +New findings: +${findingList} + +Return a JSON array of objects identifying which findings match which existing issues. +Only mark as a match if they describe the SAME issue (same bug, same location, same concern). + +Return ONLY the JSON array: +[{"findingIndex": 1, "issueIndex": 2}] +Return [] if none match.`; + + const result = await callHaiku({ + apiKey, + prompt, + schema: SemanticMatchSchema, + maxTokens: 512, + }); + + if (!result.success) { + console.warn(`Semantic issue matching failed: ${result.error}`); + return new Map(); + } + + const matches = new Map(); + for (const match of result.data) { + const issue = existingIssues[match.issueIndex - 1]; + if (issue) { + matches.set(match.findingIndex - 1, issue); + } + } + + return matches; +} + +/** + * Render a GitHub issue body for a finding. + */ +function renderIssueBody( + finding: Finding, + skillName: string, + commitSha: string, + owner: string, + repo: string, + hash: string +): string { + const lines: string[] = []; + const shortSha = commitSha.slice(0, 7); + + lines.push(`${SEVERITY_EMOJI[finding.severity]} **Severity:** ${finding.severity}`); + lines.push(`**Skill:** \`${skillName}\``); + lines.push(`**Commit:** \`${shortSha}\``); + lines.push(''); + + // Description + lines.push('### Description'); + lines.push(''); + lines.push(escapeHtml(finding.description)); + lines.push(''); + + // Location + if (finding.location) { + const lineRange = finding.location.endLine + ? `L${finding.location.startLine}-L${finding.location.endLine}` + : `L${finding.location.startLine}`; + const link = `https://github.com/${owner}/${repo}/blob/${commitSha}/${finding.location.path}#${lineRange}`; + + lines.push('### Location'); + lines.push(''); + lines.push(`[\`${finding.location.path}:${lineRange}\`](${link})`); + lines.push(''); + } + + // Suggested fix + if (finding.suggestedFix) { + lines.push('### Suggested Fix'); + lines.push(''); + lines.push(escapeHtml(finding.suggestedFix.description)); + if (finding.suggestedFix.diff) { + lines.push(''); + lines.push('```diff'); + lines.push(finding.suggestedFix.diff); + lines.push('```'); + } + lines.push(''); + } + + // Footer + lines.push('---'); + lines.push('*Generated by [Warden](https://github.com/getsentry/warden)*'); + lines.push(''); + lines.push(generateIssueMarker(hash)); + + return lines.join('\n'); +} + +export interface GitHubIssuesProviderOptions { + config: GitHubIssuesNotificationConfig; + octokit: Octokit; + apiKey: string; +} + +export class GitHubIssuesProvider implements NotificationProvider { + readonly name = 'github-issues'; + private readonly config: GitHubIssuesNotificationConfig; + private readonly octokit: Octokit; + private readonly apiKey: string; + + constructor(options: GitHubIssuesProviderOptions) { + this.config = options.config; + this.octokit = options.octokit; + this.apiKey = options.apiKey; + } + + async notify(context: NotificationContext): Promise { + const { findings, repository, commitSha, skillName } = context; + const { owner, name: repo } = repository; + const result: NotificationResult = { + provider: this.name, + sent: 0, + skipped: 0, + errors: [], + }; + + if (findings.length === 0) { + return result; + } + + // Fetch existing Warden issues for dedup + const existingIssues = await fetchWardenIssues( + this.octokit, + owner, + repo, + this.config.labels + ); + + // Phase 1: Hash-based dedup + const unmatchedFindings: { finding: Finding; index: number }[] = []; + + for (const [i, finding] of findings.entries()) { + const hash = generateIssueHash(finding.title, finding.description); + const hashMatch = existingIssues.find((issue) => issue.hash === hash); + + if (hashMatch) { + result.skipped++; + continue; + } + + unmatchedFindings.push({ finding, index: i }); + } + + if (unmatchedFindings.length === 0) { + return result; + } + + // Phase 2: Semantic dedup via Haiku (only for same-file findings) + const semanticMatches = await findSemanticMatches( + unmatchedFindings.map((f) => f.finding), + existingIssues, + this.apiKey + ); + + // Phase 3: Create issues for truly new findings + const labels = [ + ...this.config.labels, + `warden:${skillName}`, + ]; + + // Ensure labels exist before creating any issues + for (const label of labels) { + try { + await this.octokit.issues.getLabel({ owner, repo, name: label }); + } catch { + await this.octokit.issues.createLabel({ + owner, + repo, + name: label, + color: label === FALSE_POSITIVE_LABEL ? 'cccccc' : '7057ff', + }); + } + } + + for (const [i, { finding }] of unmatchedFindings.entries()) { + if (semanticMatches.has(i)) { + result.skipped++; + continue; + } + + const hash = generateIssueHash(finding.title, finding.description); + const title = `[Warden] ${finding.title}`; + const body = renderIssueBody(finding, skillName, commitSha, owner, repo, hash); + + try { + await this.octokit.issues.create({ + owner, + repo, + title, + body, + labels, + }); + + result.sent++; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + result.errors.push(`Failed to create issue for "${finding.title}": ${message}`); + } + } + + return result; + } +} diff --git a/src/notifications/providers/slack.test.ts b/src/notifications/providers/slack.test.ts new file mode 100644 index 00000000..e153c21b --- /dev/null +++ b/src/notifications/providers/slack.test.ts @@ -0,0 +1,143 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import type { Finding } from '../../types/index.js'; +import type { NotificationContext } from '../types.js'; +import { SlackProvider, buildSlackPayload } from './slack.js'; + +function makeFinding(overrides: Partial = {}): Finding { + return { + id: 'f1', + severity: 'high', + title: 'SQL injection vulnerability', + description: 'Unsanitized user input in query builder', + location: { path: 'src/db/query.ts', startLine: 42 }, + ...overrides, + }; +} + +function makeContext(overrides: Partial = {}): NotificationContext { + return { + findings: [makeFinding()], + reports: [], + repository: { owner: 'test-owner', name: 'test-repo' }, + commitSha: 'abc123def456', + skillName: 'security-audit', + ...overrides, + }; +} + +describe('buildSlackPayload', () => { + it('includes header with finding count', () => { + const payload = buildSlackPayload(makeContext()); + const blocks = payload['blocks'] as Record[]; + const header = blocks[0] as Record; + const text = header['text'] as Record; + expect(text['text']).toBe('Warden: 1 finding'); + }); + + it('pluralizes findings correctly', () => { + const ctx = makeContext({ findings: [makeFinding({ id: 'f1' }), makeFinding({ id: 'f2' })] }); + const payload = buildSlackPayload(ctx); + const blocks = payload['blocks'] as Record[]; + const header = blocks[0] as Record; + const text = header['text'] as Record; + expect(text['text']).toBe('Warden: 2 findings'); + }); + + it('includes repo, commit, and skill fields', () => { + const payload = buildSlackPayload(makeContext()); + const blocks = payload['blocks'] as Record[]; + const section = blocks[1] as Record; + const fields = section['fields'] as Record[]; + const texts = fields.map((f) => f['text']); + + expect(texts).toContainEqual(expect.stringContaining('test-owner/test-repo')); + expect(texts).toContainEqual(expect.stringContaining('abc123d')); + expect(texts).toContainEqual(expect.stringContaining('security-audit')); + }); + + it('caps displayed findings at 10', () => { + const findings = Array.from({ length: 15 }, (_, i) => + makeFinding({ id: `f${i}`, title: `Bug ${i}` }) + ); + const payload = buildSlackPayload(makeContext({ findings })); + const blocks = payload['blocks'] as Record[]; + + // header + section + divider + 10 findings + overflow context = 14 + expect(blocks).toHaveLength(14); + + // Last block should be context with overflow message + const last = blocks[blocks.length - 1]!; + expect(last['type']).toBe('context'); + }); +}); + +describe('SlackProvider', () => { + let originalFetch: typeof globalThis.fetch; + let mockFetch: ReturnType; + + beforeEach(() => { + originalFetch = globalThis.fetch; + mockFetch = vi.fn(); + globalThis.fetch = mockFetch as typeof globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + }); + + function createProvider(webhookUrl = 'https://hooks.slack.com/services/test') { + return new SlackProvider({ type: 'slack', webhookUrl }); + } + + it('posts to webhook and reports findings count', async () => { + mockFetch.mockResolvedValue({ ok: true, text: () => Promise.resolve('ok') }); + + const provider = createProvider(); + const result = await provider.notify(makeContext()); + + expect(result.sent).toBe(1); + expect(result.errors).toEqual([]); + expect(mockFetch).toHaveBeenCalledWith( + 'https://hooks.slack.com/services/test', + expect.objectContaining({ + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + }) + ); + }); + + it('skips when no findings', async () => { + const provider = createProvider(); + const result = await provider.notify(makeContext({ findings: [] })); + + expect(result.sent).toBe(0); + expect(result.skipped).toBe(0); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('reports error when webhook returns non-ok status', async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 403, + text: () => Promise.resolve('invalid_token'), + }); + + const provider = createProvider(); + const result = await provider.notify(makeContext()); + + expect(result.sent).toBe(0); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toContain('403'); + }); + + it('reports error when fetch throws', async () => { + mockFetch.mockRejectedValue(new Error('Network error')); + + const provider = createProvider(); + const result = await provider.notify(makeContext()); + + expect(result.sent).toBe(0); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toContain('Network error'); + }); +}); diff --git a/src/notifications/providers/slack.ts b/src/notifications/providers/slack.ts new file mode 100644 index 00000000..ae3615ae --- /dev/null +++ b/src/notifications/providers/slack.ts @@ -0,0 +1,129 @@ +import type { Severity } from '../../types/index.js'; +import { SEVERITY_ORDER } from '../../types/index.js'; +import type { SlackNotificationConfig } from '../../config/schema.js'; +import type { NotificationProvider, NotificationContext, NotificationResult } from '../types.js'; + +const MAX_FINDINGS_IN_MESSAGE = 10; + +const SEVERITY_EMOJI: Record = { + critical: ':rotating_light:', + high: ':warning:', + medium: ':large_orange_diamond:', + low: ':large_blue_diamond:', + info: ':information_source:', +}; + +/** + * Build Slack Block Kit payload for findings. + */ +export function buildSlackPayload(context: NotificationContext): Record { + const { findings, repository, commitSha, skillName } = context; + const shortSha = commitSha.slice(0, 7); + const repoName = `${repository.owner}/${repository.name}`; + + // Count by severity + const counts: Partial> = {}; + for (const f of findings) { + counts[f.severity] = (counts[f.severity] ?? 0) + 1; + } + + const severitySummary = (Object.keys(SEVERITY_ORDER) as Severity[]) + .filter((s) => (counts[s] ?? 0) > 0) + .map((s) => `${SEVERITY_EMOJI[s]} ${counts[s]} ${s}`) + .join(' '); + + const blocks: Record[] = [ + { + type: 'header', + text: { + type: 'plain_text', + text: `Warden: ${findings.length} finding${findings.length === 1 ? '' : 's'}`, + emoji: true, + }, + }, + { + type: 'section', + fields: [ + { type: 'mrkdwn', text: `*Repo:* ${repoName}` }, + { type: 'mrkdwn', text: `*Commit:* \`${shortSha}\`` }, + { type: 'mrkdwn', text: `*Skill:* ${skillName}` }, + { type: 'mrkdwn', text: `*Summary:* ${severitySummary}` }, + ], + }, + { type: 'divider' }, + ]; + + // Add finding details (capped) + const displayed = findings.slice(0, MAX_FINDINGS_IN_MESSAGE); + for (const finding of displayed) { + const loc = finding.location + ? `\`${finding.location.path}:${finding.location.startLine}\`` + : '_no location_'; + + blocks.push({ + type: 'section', + text: { + type: 'mrkdwn', + text: `${SEVERITY_EMOJI[finding.severity]} *${finding.title}*\n${loc}\n${finding.description.slice(0, 200)}`, + }, + }); + } + + if (findings.length > MAX_FINDINGS_IN_MESSAGE) { + blocks.push({ + type: 'context', + elements: [ + { + type: 'mrkdwn', + text: `_...and ${findings.length - MAX_FINDINGS_IN_MESSAGE} more finding${findings.length - MAX_FINDINGS_IN_MESSAGE === 1 ? '' : 's'}_`, + }, + ], + }); + } + + return { blocks }; +} + +export class SlackProvider implements NotificationProvider { + readonly name = 'slack'; + private readonly webhookUrl: string; + + constructor(config: SlackNotificationConfig) { + this.webhookUrl = config.webhookUrl; + } + + async notify(context: NotificationContext): Promise { + const result: NotificationResult = { + provider: this.name, + sent: 0, + skipped: 0, + errors: [], + }; + + if (context.findings.length === 0) { + return result; + } + + const payload = buildSlackPayload(context); + + try { + const response = await fetch(this.webhookUrl, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + }); + + if (!response.ok) { + const text = await response.text(); + result.errors.push(`Slack webhook returned ${response.status}: ${text}`); + } else { + result.sent = context.findings.length; + } + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + result.errors.push(`Slack webhook failed: ${message}`); + } + + return result; + } +} diff --git a/src/notifications/types.ts b/src/notifications/types.ts new file mode 100644 index 00000000..eb3c17bf --- /dev/null +++ b/src/notifications/types.ts @@ -0,0 +1,32 @@ +import type { Finding, SkillReport } from '../types/index.js'; + +/** + * Context passed to each notification provider. + */ +export interface NotificationContext { + findings: Finding[]; + reports: SkillReport[]; + repository: { owner: string; name: string }; + commitSha: string; + skillName: string; +} + +/** + * Result from a notification provider. + */ +export interface NotificationResult { + provider: string; + /** Number of notifications sent (e.g., issues created, messages posted) */ + sent: number; + /** Number of findings skipped (e.g., already tracked, deduped) */ + skipped: number; + errors: string[]; +} + +/** + * Interface that all notification providers implement. + */ +export interface NotificationProvider { + readonly name: string; + notify(context: NotificationContext): Promise; +} diff --git a/src/output/github-issues.ts b/src/output/github-issues.ts index 1fba7552..87661472 100644 --- a/src/output/github-issues.ts +++ b/src/output/github-issues.ts @@ -1,103 +1,9 @@ import { readFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import type { Octokit } from '@octokit/rest'; -import type { SkillReport, Finding } from '../types/index.js'; -import { renderIssueBody, renderNoFindingsUpdate } from './issue-renderer.js'; +import type { Finding } from '../types/index.js'; import { parsePatch } from '../diff/parser.js'; -export interface IssueResult { - issueNumber: number; - issueUrl: string; - created: boolean; // true if new, false if updated -} - -export interface CreateIssueOptions { - title: string; - commitSha: string; -} - -/** - * Create or update a GitHub issue with findings. - * Searches for existing open issue by title prefix, updates if found. - */ -export async function createOrUpdateIssue( - octokit: Octokit, - owner: string, - repo: string, - reports: SkillReport[], - options: CreateIssueOptions -): Promise { - const { title, commitSha } = options; - const allFindings = reports.flatMap((r) => r.findings); - const now = new Date(); - - // Search for existing open issue with matching title - const existingIssue = await findExistingIssue(octokit, owner, repo, title); - - // Render the issue body - const body = allFindings.length > 0 - ? renderIssueBody(reports, { - commitSha, - runTimestamp: now, - repoOwner: owner, - repoName: repo, - }) - : renderNoFindingsUpdate(commitSha, now); - - if (existingIssue) { - // Update existing issue - await octokit.issues.update({ - owner, - repo, - issue_number: existingIssue.number, - body, - }); - - return { - issueNumber: existingIssue.number, - issueUrl: existingIssue.html_url, - created: false, - }; - } - - // Skip creating new issue if no findings - if (allFindings.length === 0) { - return null; - } - - // Create new issue - const { data: newIssue } = await octokit.issues.create({ - owner, - repo, - title, - body, - }); - - return { - issueNumber: newIssue.number, - issueUrl: newIssue.html_url, - created: true, - }; -} - -async function findExistingIssue( - octokit: Octokit, - owner: string, - repo: string, - title: string -): Promise<{ number: number; html_url: string } | null> { - // Search for open issues with exact title match - const { data: issues } = await octokit.issues.listForRepo({ - owner, - repo, - state: 'open', - per_page: 100, - }); - - const matching = issues.find((issue) => issue.title === title); - return matching ? { number: matching.number, html_url: matching.html_url } : null; -} - export interface FixPRResult { prNumber: number; prUrl: string; diff --git a/src/output/index.ts b/src/output/index.ts index f66dd97f..8f5ce4d8 100644 --- a/src/output/index.ts +++ b/src/output/index.ts @@ -1,5 +1,4 @@ export * from './types.js'; export * from './renderer.js'; -export * from './issue-renderer.js'; export * from './github-issues.js'; export * from './github-checks.js'; diff --git a/src/output/issue-renderer.ts b/src/output/issue-renderer.ts deleted file mode 100644 index 8ec3ad05..00000000 --- a/src/output/issue-renderer.ts +++ /dev/null @@ -1,184 +0,0 @@ -import { SEVERITY_ORDER } from '../types/index.js'; -import type { SkillReport, Finding, Severity } from '../types/index.js'; -import { capitalize, countBySeverity } from '../cli/output/formatters.js'; -import { escapeHtml } from '../utils/index.js'; - -export interface IssueRenderOptions { - /** Commit SHA for linking to code */ - commitSha: string; - /** When the scan was run */ - runTimestamp: Date; - /** Repository owner for constructing file links */ - repoOwner?: string; - /** Repository name for constructing file links */ - repoName?: string; -} - -/** - * Render skill reports as a GitHub issue body. - */ -export function renderIssueBody( - reports: SkillReport[], - options: IssueRenderOptions -): string { - const { commitSha, runTimestamp, repoOwner, repoName } = options; - const lines: string[] = []; - - // Header with timestamp and commit - const shortSha = commitSha.slice(0, 7); - const timestamp = runTimestamp.toISOString(); - - lines.push('## Warden Scheduled Scan Results'); - lines.push(''); - lines.push(`**Run:** ${timestamp}`); - lines.push(`**Commit:** \`${shortSha}\``); - lines.push(''); - - // Collect all findings - const allFindings = reports.flatMap((r) => r.findings); - - if (allFindings.length === 0) { - lines.push('**No issues found.** The scheduled scan completed without finding any issues.'); - lines.push(''); - lines.push('---'); - lines.push('*Generated by [Warden](https://github.com/getsentry/warden)*'); - return lines.join('\n'); - } - - // Severity summary table - const counts = countBySeverity(allFindings); - lines.push('### Summary'); - lines.push(''); - lines.push('| Severity | Count |'); - lines.push('|----------|-------|'); - - for (const severity of ['critical', 'high', 'medium', 'low', 'info'] as Severity[]) { - if (counts[severity] > 0) { - lines.push(`| ${capitalize(severity)} | ${counts[severity]} |`); - } - } - lines.push(''); - - // Findings grouped by file - lines.push('### Findings'); - lines.push(''); - - // Sort findings by severity, then by file - const sortedFindings = [...allFindings].sort((a, b) => { - const severityDiff = SEVERITY_ORDER[a.severity] - SEVERITY_ORDER[b.severity]; - if (severityDiff !== 0) return severityDiff; - const aPath = a.location?.path ?? ''; - const bPath = b.location?.path ?? ''; - return aPath.localeCompare(bPath); - }); - - const byFile = groupFindingsByFile(sortedFindings); - const canLink = repoOwner && repoName; - - for (const [file, fileFindings] of Object.entries(byFile)) { - if (canLink) { - lines.push(`#### [\`${file}\`](https://github.com/${repoOwner}/${repoName}/blob/${commitSha}/${file})`); - } else { - lines.push(`#### \`${file}\``); - } - lines.push(''); - - for (const finding of fileFindings) { - lines.push(renderFindingItem(finding, { commitSha, repoOwner, repoName })); - } - lines.push(''); - } - - // General findings (no location) - const noLocation = sortedFindings.filter((f) => !f.location); - if (noLocation.length > 0) { - lines.push('#### General'); - lines.push(''); - for (const finding of noLocation) { - lines.push(renderFindingItem(finding, { commitSha, repoOwner, repoName })); - } - lines.push(''); - } - - // Per-skill summaries if multiple skills - if (reports.length > 1) { - lines.push('### Skill Summaries'); - lines.push(''); - for (const report of reports) { - lines.push(`**${report.skill}:** ${escapeHtml(report.summary)}`); - lines.push(''); - } - } - - // Footer - lines.push('---'); - lines.push('*Generated by [Warden](https://github.com/getsentry/warden)*'); - - return lines.join('\n'); -} - -function groupFindingsByFile(findings: Finding[]): Record { - const groups: Record = {}; - for (const finding of findings) { - if (finding.location) { - const path = finding.location.path; - groups[path] ??= []; - groups[path].push(finding); - } - } - return groups; -} - -function formatLineRange(loc: { startLine: number; endLine?: number }): string { - if (loc.endLine && loc.endLine !== loc.startLine) { - return `L${loc.startLine}-L${loc.endLine}`; - } - return `L${loc.startLine}`; -} - -interface LinkContext { - commitSha: string; - repoOwner?: string; - repoName?: string; -} - -function renderFindingItem(finding: Finding, ctx: LinkContext): string { - const { commitSha, repoOwner, repoName } = ctx; - const canLink = repoOwner && repoName && finding.location; - - let locationStr = ''; - if (finding.location) { - const lineRange = formatLineRange(finding.location); - if (canLink) { - locationStr = ` ([${lineRange}](https://github.com/${repoOwner}/${repoName}/blob/${commitSha}/${finding.location.path}#${lineRange}))`; - } else { - locationStr = ` (${lineRange})`; - } - } - - let line = `- \`${finding.id}\` **${escapeHtml(finding.title)}**${locationStr} · ${finding.severity}`; - line += `\n ${escapeHtml(finding.description)}`; - - if (finding.suggestedFix) { - line += `\n *Suggested fix:* ${escapeHtml(finding.suggestedFix.description)}`; - } - - return line; -} - -/** - * Render a brief status update for when no new findings are found. - */ -export function renderNoFindingsUpdate(commitSha: string, runTimestamp: Date): string { - const shortSha = commitSha.slice(0, 7); - const timestamp = runTimestamp.toISOString(); - - return [ - '## Latest Scan: No Issues Found', - '', - `Scan completed at ${timestamp} (commit \`${shortSha}\`) with no issues.`, - '', - '---', - '*Generated by [Warden](https://github.com/getsentry/warden)*', - ].join('\n'); -} diff --git a/src/suppressions/index.ts b/src/suppressions/index.ts new file mode 100644 index 00000000..bd28443e --- /dev/null +++ b/src/suppressions/index.ts @@ -0,0 +1,4 @@ +export { loadSuppressions } from './loader.js'; +export { applySuppression } from './matcher.js'; +export type { SuppressionRule, SuppressionFile } from './types.js'; +export { SuppressionRuleSchema, SuppressionFileSchema } from './types.js'; diff --git a/src/suppressions/loader.test.ts b/src/suppressions/loader.test.ts new file mode 100644 index 00000000..52d8624d --- /dev/null +++ b/src/suppressions/loader.test.ts @@ -0,0 +1,86 @@ +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { loadSuppressions } from './loader.js'; + +function makeTempDir(): string { + return mkdtempSync(join(tmpdir(), 'warden-test-')); +} + +function writeSuppressionsFile(repoPath: string, content: string): void { + const dir = join(repoPath, '.agents', 'warden'); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'suppressions.yaml'), content); +} + +describe('loadSuppressions', () => { + it('returns empty array when file does not exist', () => { + const repoPath = makeTempDir(); + expect(loadSuppressions(repoPath)).toEqual([]); + }); + + it('loads valid suppressions file', () => { + const repoPath = makeTempDir(); + writeSuppressionsFile(repoPath, ` +suppressions: + - skill: "security-audit" + paths: ["src/legacy/**"] + reason: "Legacy code" + - skill: "security-audit" + paths: ["src/admin/query.ts"] + title: "SQL injection" + reason: "Uses parameterized queries" +`); + + const rules = loadSuppressions(repoPath); + expect(rules).toHaveLength(2); + expect(rules[0]).toEqual({ + skill: 'security-audit', + paths: ['src/legacy/**'], + reason: 'Legacy code', + }); + expect(rules[1]).toEqual({ + skill: 'security-audit', + paths: ['src/admin/query.ts'], + title: 'SQL injection', + reason: 'Uses parameterized queries', + }); + }); + + it('returns empty array for file with empty suppressions list', () => { + const repoPath = makeTempDir(); + writeSuppressionsFile(repoPath, 'suppressions: []\n'); + + expect(loadSuppressions(repoPath)).toEqual([]); + }); + + it('throws on malformed YAML', () => { + const repoPath = makeTempDir(); + writeSuppressionsFile(repoPath, '{{not: yaml'); + + expect(() => loadSuppressions(repoPath)).toThrow(); + }); + + it('throws on invalid schema (missing required fields)', () => { + const repoPath = makeTempDir(); + writeSuppressionsFile(repoPath, ` +suppressions: + - skill: "test" +`); + + expect(() => loadSuppressions(repoPath)).toThrow(/Invalid suppressions file/); + }); + + it('throws when paths is empty', () => { + const repoPath = makeTempDir(); + writeSuppressionsFile(repoPath, ` +suppressions: + - skill: "test" + paths: [] + reason: "no paths" +`); + + expect(() => loadSuppressions(repoPath)).toThrow(/Invalid suppressions file/); + }); +}); diff --git a/src/suppressions/loader.ts b/src/suppressions/loader.ts new file mode 100644 index 00000000..ab3c2cdc --- /dev/null +++ b/src/suppressions/loader.ts @@ -0,0 +1,33 @@ +import { readFileSync, existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { parse as parseYaml } from 'yaml'; +import { SuppressionFileSchema } from './types.js'; +import type { SuppressionRule } from './types.js'; + +const SUPPRESSIONS_PATH = '.agents/warden/suppressions.yaml'; + +/** + * Load suppression rules from .agents/warden/suppressions.yaml. + * Returns an empty array if the file does not exist. + * Throws on malformed YAML or invalid schema. + */ +export function loadSuppressions(repoPath: string): SuppressionRule[] { + const filePath = join(repoPath, SUPPRESSIONS_PATH); + + if (!existsSync(filePath)) { + return []; + } + + const content = readFileSync(filePath, 'utf-8'); + const raw = parseYaml(content); + const result = SuppressionFileSchema.safeParse(raw); + + if (!result.success) { + const issues = result.error.issues + .map((i) => ` - ${i.path.join('.')}: ${i.message}`) + .join('\n'); + throw new Error(`Invalid suppressions file ${SUPPRESSIONS_PATH}:\n${issues}`); + } + + return result.data.suppressions; +} diff --git a/src/suppressions/matcher.test.ts b/src/suppressions/matcher.test.ts new file mode 100644 index 00000000..d3e03830 --- /dev/null +++ b/src/suppressions/matcher.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect } from 'vitest'; +import { applySuppression } from './matcher.js'; +import type { Finding } from '../types/index.js'; +import type { SuppressionRule } from './types.js'; + +function makeFinding(overrides: Partial = {}): Finding { + return { + id: 'f1', + severity: 'high', + title: 'SQL injection in query builder', + description: 'Unsanitized input', + location: { path: 'src/admin/query.ts', startLine: 10 }, + ...overrides, + }; +} + +describe('applySuppression', () => { + it('returns all findings when no rules', () => { + const findings = [makeFinding()]; + expect(applySuppression(findings, [], 'security-audit')).toEqual(findings); + }); + + it('returns all findings when rules are for different skill', () => { + const rules: SuppressionRule[] = [{ + skill: 'other-skill', + paths: ['**/*'], + reason: 'test', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual(findings); + }); + + it('suppresses finding matching skill and path glob', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/admin/**'], + reason: 'Legacy code', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual([]); + }); + + it('does not suppress finding when path does not match glob', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/api/**'], + reason: 'test', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual(findings); + }); + + it('suppresses finding matching skill, path, and title substring', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/admin/**'], + title: 'SQL injection', + reason: 'Uses parameterized queries', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual([]); + }); + + it('does not suppress when title does not match', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/admin/**'], + title: 'XSS', + reason: 'test', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual(findings); + }); + + it('title matching is case-insensitive', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/admin/**'], + title: 'sql INJECTION', + reason: 'test', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual([]); + }); + + it('does not suppress findings without location', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['**/*'], + reason: 'test', + }]; + const findings = [makeFinding({ location: undefined })]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual(findings); + }); + + it('suppresses only matching findings from mixed set', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/legacy/**'], + reason: 'Legacy code', + }]; + + const findings = [ + makeFinding({ id: 'f1', location: { path: 'src/legacy/old.ts', startLine: 5 } }), + makeFinding({ id: 'f2', location: { path: 'src/new/fresh.ts', startLine: 10 } }), + makeFinding({ id: 'f3', location: { path: 'src/legacy/ancient.ts', startLine: 1 } }), + ]; + + const result = applySuppression(findings, rules, 'security-audit'); + expect(result).toHaveLength(1); + expect(result[0]?.id).toBe('f2'); + }); + + it('matches exact file path pattern', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/admin/query.ts'], + reason: 'test', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual([]); + }); + + it('multiple path patterns are OR-matched', () => { + const rules: SuppressionRule[] = [{ + skill: 'security-audit', + paths: ['src/api/**', 'src/admin/**'], + reason: 'test', + }]; + const findings = [makeFinding()]; + + expect(applySuppression(findings, rules, 'security-audit')).toEqual([]); + }); +}); diff --git a/src/suppressions/matcher.ts b/src/suppressions/matcher.ts new file mode 100644 index 00000000..d8c61dac --- /dev/null +++ b/src/suppressions/matcher.ts @@ -0,0 +1,40 @@ +import { matchGlob } from '../triggers/matcher.js'; +import type { Finding } from '../types/index.js'; +import type { SuppressionRule } from './types.js'; + +/** + * Check if a finding matches a single suppression rule. + */ +function matchesRule(finding: Finding, rule: SuppressionRule): boolean { + const path = finding.location?.path; + if (!path) return false; + + // Check if any path pattern matches + const pathMatch = rule.paths.some((pattern) => matchGlob(pattern, path)); + if (!pathMatch) return false; + + // Check title substring if specified + if (rule.title && !finding.title.toLowerCase().includes(rule.title.toLowerCase())) { + return false; + } + + return true; +} + +/** + * Filter findings by applying suppression rules. + * Returns findings that are NOT suppressed. + */ +export function applySuppression( + findings: Finding[], + rules: SuppressionRule[], + skillName: string +): Finding[] { + if (rules.length === 0) return findings; + + // Pre-filter rules to only those matching this skill + const skillRules = rules.filter((r) => r.skill === skillName); + if (skillRules.length === 0) return findings; + + return findings.filter((finding) => !skillRules.some((rule) => matchesRule(finding, rule))); +} diff --git a/src/suppressions/types.ts b/src/suppressions/types.ts new file mode 100644 index 00000000..ad82dfa9 --- /dev/null +++ b/src/suppressions/types.ts @@ -0,0 +1,18 @@ +import { z } from 'zod'; + +export const SuppressionRuleSchema = z.object({ + /** Exact skill name to match */ + skill: z.string().min(1), + /** Glob patterns to match against finding location path */ + paths: z.array(z.string().min(1)).min(1), + /** Optional substring match against finding title */ + title: z.string().optional(), + /** Human-readable justification for the suppression */ + reason: z.string().min(1), +}); +export type SuppressionRule = z.infer; + +export const SuppressionFileSchema = z.object({ + suppressions: z.array(SuppressionRuleSchema).default([]), +}); +export type SuppressionFile = z.infer;