diff --git a/.env.example b/.env.example index 1e1c1b0..0f7badb 100644 --- a/.env.example +++ b/.env.example @@ -27,6 +27,11 @@ APP_OKTA_GITHUB_USER_FIELD=githubUsername APP_OKTA_SYNC_RULES=[{"name":"sync-eng","enabled":true,"okta_group_pattern":"^github-eng-.*","github_team_prefix":"eng-","strip_prefix":"github-eng-","sync_members":true,"create_team_if_missing":true}] # APP_OKTA_SYNC_SAFETY_THRESHOLD=0.5 # Prevent mass removal if more than 50% would be removed (default: 0.5) +# security alerts monitoring (optional) +# APP_SECURITY_ALERTS_ENABLED=true +# APP_SECURITY_ALERTS_MIN_AGE_DAYS=30 # only report alerts older than N days (default: 30) +# APP_SECURITY_ALERTS_MIN_SEVERITY=high # minimum severity: critical, high, medium, low (default: high) + # slack configuration (optional) APP_SLACK_TOKEN=xoxb-xxxxxxxxxxxxxxxxxxxxxxxxxxxxx APP_SLACK_CHANNEL=C01234ABCDE @@ -34,6 +39,7 @@ APP_SLACK_CHANNEL=C01234ABCDE # APP_SLACK_CHANNEL_PR_BYPASS=C01234ABCDE # APP_SLACK_CHANNEL_OKTA_SYNC=C01234ABCDE # APP_SLACK_CHANNEL_ORPHANED_USERS=C01234ABCDE +# APP_SLACK_CHANNEL_SECURITY_ALERTS=C01234ABCDE # optional: custom footer note for PR bypass notifications (supports Slack mrkdwn) # APP_SLACK_FOOTER_NOTE_PR_BYPASS=_Please review the ._ diff --git a/README.md b/README.md index 8996877..413a683 100644 --- a/README.md +++ b/README.md @@ -5,24 +5,25 @@ notifications. Deploy as AWS Lambda, standard HTTP server, or container. ## Features -* **Okta group sync** - Automatically sync Okta groups to GitHub teams -* **Orphaned user detection** - Identify org members not in any synced teams -* **PR compliance monitoring** - Detect and notify when PRs bypass branch +- **Okta group sync** - Automatically sync Okta groups to GitHub teams +- **Orphaned user detection** - Identify org members not in any synced teams +- **PR compliance monitoring** - Detect and notify when PRs bypass branch protection -* **Automatic reconciliation** - Detects external team changes and triggers - sync -* **Flexible configuration** - Enable only what you need via environment +- **Security alerts monitoring** - Report stale Dependabot, code scanning, and + secret scanning alerts across the org +- **Automatic reconciliation** - Detects external team changes and triggers sync +- **Flexible configuration** - Enable only what you need via environment variables -* **Slack notifications** - Rich messages for violations and sync reports +- **Slack notifications** - Rich messages for violations and sync reports ## Quick Start ### Prerequisites -* Go ≥ 1.24 -* GitHub App ([setup guide](docs/github-app-setup.md)) -* **Optional**: Okta API Service app ([setup guide](docs/okta-setup.md)) -* **Optional**: Slack app ([setup guide](docs/slack-setup.md)) +- Go ≥ 1.24 +- GitHub App ([setup guide](docs/github-app-setup.md)) +- **Optional**: Okta API Service app ([setup guide](docs/okta-setup.md)) +- **Optional**: Slack app ([setup guide](docs/slack-setup.md)) ### Deployment Options @@ -42,8 +43,9 @@ make build-server # server listens on APP_PORT (default: 8080) # endpoints: # POST /webhooks - GitHub webhook receiver -# POST /scheduled/okta-sync - Trigger Okta sync (call via cron) -# POST /scheduled/slack-test - Send test notification to Slack +# POST /scheduled/okta-sync - Trigger Okta sync (call via cron) +# POST /scheduled/security-alerts - Trigger security alerts check +# POST /scheduled/slack-test - Send test notification to Slack # GET /server/status - Health check # GET /server/config - Config (secrets redacted) ``` @@ -80,6 +82,7 @@ APP_GITHUB_WEBHOOK_SECRET=arn:aws:ssm:us-east-1:123456789012:parameter/github-bo ``` **Requirements for SSM parameters**: + - Valid AWS credentials with `ssm:GetParameter` permission - Full SSM parameter ARN in format: `arn:aws:ssm:REGION:ACCOUNT:parameter/path/to/param` @@ -87,19 +90,19 @@ APP_GITHUB_WEBHOOK_SECRET=arn:aws:ssm:us-east-1:123456789012:parameter/github-bo ### Required: GitHub -| Variable | Description | -|-------------------------------------|---------------------------------| -| `APP_GITHUB_APP_ID` | GitHub App ID | -| `APP_GITHUB_APP_PRIVATE_KEY` | Private key (PEM) | -| `APP_GITHUB_APP_PRIVATE_KEY_PATH` | Path to private key file | -| `APP_GITHUB_INSTALLATION_ID` | Installation ID | -| `APP_GITHUB_ORG` | Organization name | -| `APP_GITHUB_WEBHOOK_SECRET` | Webhook signature secret | +| Variable | Description | +| --------------------------------- | ------------------------ | +| `APP_GITHUB_APP_ID` | GitHub App ID | +| `APP_GITHUB_APP_PRIVATE_KEY` | Private key (PEM) | +| `APP_GITHUB_APP_PRIVATE_KEY_PATH` | Path to private key file | +| `APP_GITHUB_INSTALLATION_ID` | Installation ID | +| `APP_GITHUB_ORG` | Organization name | +| `APP_GITHUB_WEBHOOK_SECRET` | Webhook signature secret | ### Optional: Okta Sync | Variable | Description | -|----------------------------------------|-----------------------------------------------| +| -------------------------------------- | --------------------------------------------- | | `APP_OKTA_DOMAIN` | Okta domain | | `APP_OKTA_CLIENT_ID` | OAuth 2.0 client ID | | `APP_OKTA_PRIVATE_KEY` | Private key (PEM) or use | @@ -111,28 +114,39 @@ APP_GITHUB_WEBHOOK_SECRET=arn:aws:ssm:us-east-1:123456789012:parameter/github-bo ### Optional: PR Compliance -| Variable | Description | -|----------------------------------|-------------------------------------------| -| `APP_PR_COMPLIANCE_ENABLED` | Enable monitoring (`true`) | -| `APP_PR_MONITORED_BRANCHES` | Branches to monitor (e.g., `main,master`) | +| Variable | Description | +| --------------------------- | ----------------------------------------- | +| `APP_PR_COMPLIANCE_ENABLED` | Enable monitoring (`true`) | +| `APP_PR_MONITORED_BRANCHES` | Branches to monitor (e.g., `main,master`) | + +### Optional: Security Alerts + +| Variable | Description | +| ---------------------------------- | ---------------------------------------- | +| `APP_SECURITY_ALERTS_ENABLED` | Enable monitoring (`true`) | +| `APP_SECURITY_ALERTS_MIN_AGE_DAYS` | Min alert age in days (default: `30`) | +| `APP_SECURITY_ALERTS_MIN_SEVERITY` | Min severity threshold (default: `high`) | + +Severity options: `critical`, `high`, `medium`, `low`. ### Optional: Slack -| Variable | Description | -|-----------------------------------|------------------------------------------| -| `APP_SLACK_TOKEN` | Bot token (`xoxb-...`) | -| `APP_SLACK_CHANNEL` | Default channel ID | -| `APP_SLACK_CHANNEL_PR_BYPASS` | Channel for PR bypass alerts (optional) | -| `APP_SLACK_CHANNEL_OKTA_SYNC` | Channel for sync reports (optional) | -| `APP_SLACK_CHANNEL_ORPHANED_USERS`| Channel for orphan alerts (optional) | +| Variable | Description | +| ----------------------------------- | --------------------------------------- | +| `APP_SLACK_TOKEN` | Bot token (`xoxb-...`) | +| `APP_SLACK_CHANNEL` | Default channel ID | +| `APP_SLACK_CHANNEL_PR_BYPASS` | Channel for PR bypass alerts (optional) | +| `APP_SLACK_CHANNEL_OKTA_SYNC` | Channel for sync reports (optional) | +| `APP_SLACK_CHANNEL_ORPHANED_USERS` | Channel for orphan alerts (optional) | +| `APP_SLACK_CHANNEL_SECURITY_ALERTS` | Channel for security alerts (optional) | ### Other -| Variable | Description | -|--------------------------|------------------------------------------------| -| `APP_PORT` | Server port (default: `8080`) | -| `APP_DEBUG_ENABLED` | Verbose logging (default: `false`) | -| `APP_BASE_PATH` | URL prefix to strip (e.g., `/api/v1`) | +| Variable | Description | +| ------------------- | ------------------------------------- | +| `APP_PORT` | Server port (default: `8080`) | +| `APP_DEBUG_ENABLED` | Verbose logging (default: `false`) | +| `APP_BASE_PATH` | URL prefix to strip (e.g., `/api/v1`) | ### Okta Sync Rules @@ -164,6 +178,7 @@ See [Okta Setup - Sync Rules](docs/okta-setup.md#step-10-configure-sync-rules) for detailed rule field documentation. **Sync Safety Features**: + - Only syncs `ACTIVE` Okta users; never removes outside collaborators - Safety threshold (default 50%) aborts sync if too many removals detected - Orphaned user detection alerts when org members aren't in any synced teams @@ -228,6 +243,13 @@ CMD ["/server"] | | - map groups to teams | | | | - sync membership | | | +------------------------------+ | + | | + | +------------------------------+ | + | | Security Alerts Monitor | | + | | - dependabot alerts | | + | | - code scanning alerts | | + | | - secret scanning alerts | | + | +------------------------------+ | +------------------------------------+ ``` @@ -245,12 +267,23 @@ CMD ["/server"] 1. **Receive**: GitHub webhook on PR merge to monitored branch 2. **Verify**: Validate webhook signature (HMAC-SHA256) 3. **Check**: Query branch protection rules and required status checks -4. **Detect**: Identify bypasses (admin override, missing reviews, failed checks) +4. **Detect**: Identify bypasses (admin override, missing reviews, failed + checks) 5. **Notify**: Send Slack alert with violation details +### Security Alerts Flow + +1. **Trigger**: Scheduled cron/EventBridge (`POST /scheduled/security-alerts`) +2. **Fetch**: Query Dependabot, code scanning, and secret scanning alerts across + the org +3. **Filter**: Keep open alerts older than threshold (default 30 days) at or + above minimum severity (default high) +4. **Report**: Send Slack notification summarizing stale alerts by repo + ## Troubleshooting **Common issues**: + - Unauthorized from GitHub: Check app installation and permissions - Group not found from Okta: Verify domain and scopes - Webhook signature fails: Verify `APP_GITHUB_WEBHOOK_SECRET` matches @@ -259,4 +292,3 @@ CMD ["/server"] ## License MIT - diff --git a/assets/github/manifest.json b/assets/github/manifest.json index d15687e..ac993e2 100644 --- a/assets/github/manifest.json +++ b/assets/github/manifest.json @@ -12,7 +12,11 @@ "organization_administration": "read", "members": "write", "contents": "read", - "pull_requests": "read" + "pull_requests": "read", + "checks": "read", + "dependabot_alerts": "read", + "code_scanning_alerts": "read", + "secret_scanning_alerts": "read" }, "default_events": [ "pull_request", diff --git a/cmd/lambda/README.md b/cmd/lambda/README.md index 0c5123f..d8a6d3b 100644 --- a/cmd/lambda/README.md +++ b/cmd/lambda/README.md @@ -8,6 +8,7 @@ alternatives like standard HTTP servers. The Lambda adapter translates AWS-specific events into the unified `app.HandleRequest()` interface: + - **API Gateway** (webhooks, status, config) → `app.Request{Type: HTTP}` - **EventBridge** (scheduled sync) → `app.Request{Type: Scheduled}` @@ -24,19 +25,20 @@ make build-lambda ### 1. Create Function -* **Runtime**: `provided.al2023` -* **Handler**: `bootstrap` -* **Architecture**: `x86_64` -* **Memory**: 256 MB -* **Timeout**: 30 seconds -* **IAM Role**: `AWSLambdaBasicExecutionRole` (no additional permissions needed) +- **Runtime**: `provided.al2023` +- **Handler**: `bootstrap` +- **Architecture**: `x86_64` +- **Memory**: 256 MB +- **Timeout**: 30 seconds +- **IAM Role**: `AWSLambdaBasicExecutionRole` (no additional permissions needed) ### 2. Upload Code Upload `dist/bootstrap` to your Lambda function via: + - AWS Console (function code upload) -- AWS CLI: `aws lambda update-function-code --function-name github-ops-app - --zip-file fileb://dist/bootstrap.zip` +- AWS CLI: + `aws lambda update-function-code --function-name github-ops-app --zip-file fileb://dist/bootstrap.zip` - Infrastructure as Code (Terraform, CDK, CloudFormation) ### 3. Configure Environment Variables @@ -59,16 +61,17 @@ Create an HTTP API Gateway: **Headers**: API Gateway automatically forwards all headers including `X-GitHub-Event` and `X-Hub-Signature-256`. -#### EventBridge (for Scheduled Okta Sync) +#### EventBridge (for Scheduled Tasks) -Create an EventBridge rule: +Create EventBridge rules for each scheduled feature you use: + +**Okta Sync** (sync Okta groups to GitHub teams): 1. **Rule Type**: Schedule -2. **Schedule**: - - Rate: `rate(1 hour)` or `rate(6 hours)` - - Cron: `cron(0 */6 * * ? *)` (every 6 hours) +2. **Schedule**: `rate(1 hour)` or `rate(6 hours)` 3. **Target**: Lambda function 4. **Input**: Configure constant (JSON): + ```json { "source": "aws.events", @@ -79,6 +82,23 @@ Create an EventBridge rule: } ``` +**Security Alerts** (report stale security alerts): + +1. **Rule Type**: Schedule +2. **Schedule**: `rate(1 day)` or `cron(0 9 ? * MON *)` (weekly Monday 9 AM) +3. **Target**: Lambda function +4. **Input**: Configure constant (JSON): + +```json +{ + "source": "aws.events", + "detail-type": "Scheduled Event", + "detail": { + "action": "security-alerts" + } +} +``` + ## Architecture ### Universal Handler @@ -91,6 +111,7 @@ func UniversalHandler(ctx context.Context, event json.RawMessage) (any, error) ``` **Supported Events**: + - `APIGatewayV2HTTPRequest` → Converts to `app.Request{Type: HTTP}` - `CloudWatchEvent` (EventBridge) → Converts to `app.Request{Type: Scheduled}` @@ -101,19 +122,21 @@ request type and path. When invoked via API Gateway: -| Method | Path | Description | -|--------|------------------------|-----------------------------------| -| POST | `/webhooks` | GitHub webhook receiver | -| POST | `/scheduled/okta-sync` | Trigger Okta sync | -| POST | `/scheduled/slack-test`| Send test notification to Slack | -| GET | `/server/status` | Health check and feature flags | -| GET | `/server/config` | Config inspection (secrets hidden)| +| Method | Path | Description | +| ------ | ---------------------------- | ---------------------------------- | +| POST | `/webhooks` | GitHub webhook receiver | +| POST | `/scheduled/okta-sync` | Trigger Okta sync | +| POST | `/scheduled/security-alerts` | Trigger security alerts check | +| POST | `/scheduled/slack-test` | Send test notification to Slack | +| GET | `/server/status` | Health check and feature flags | +| GET | `/server/config` | Config inspection (secrets hidden) | ## Monitoring ### CloudWatch Logs View logs: + ```bash aws logs tail /aws/lambda/your-function-name --follow ``` @@ -121,6 +144,7 @@ aws logs tail /aws/lambda/your-function-name --follow ### Metrics Lambda automatically tracks: + - Invocations - Duration - Errors @@ -129,6 +153,7 @@ Lambda automatically tracks: ### Debug Mode Enable verbose logging: + ```bash aws lambda update-function-configuration \ --function-name github-ops-app \ @@ -140,9 +165,10 @@ aws lambda update-function-configuration \ ### Memory Sizing Start with 256 MB and adjust based on CloudWatch metrics: + - **Under-provisioned**: Slow response times, timeouts - **Over-provisioned**: Wasted cost -- **Right-sized**: <100ms execution time for webhooks, <5s for sync +- **Right-sized**: \<100ms execution time for webhooks, \<5s for sync ### Timeout @@ -154,12 +180,14 @@ Start with 256 MB and adjust based on CloudWatch metrics: **Webhooks**: Pay-per-use (only when events occur) **Scheduled Sync**: Runs on schedule regardless of changes + - Hourly sync: ~730 invocations/month - 6-hour sync: ~120 invocations/month **Cost Example** (us-east-1, 256MB, 5s avg): + - Free tier: 1M requests, 400,000 GB-seconds/month -- Typical usage: <1000 invocations/month → **free** +- Typical usage: \<1000 invocations/month → **free** ## Troubleshooting @@ -169,6 +197,7 @@ Start with 256 MB and adjust based on CloudWatch metrics: validation failed" **Solutions**: + - Verify `APP_GITHUB_WEBHOOK_SECRET` matches GitHub App settings - Check API Gateway forwards `X-Hub-Signature-256` header - Ensure payload isn't modified by API Gateway (use proxy integration) @@ -178,6 +207,7 @@ validation failed" **Symptom**: No sync activity in logs **Solutions**: + - Verify EventBridge rule is **enabled** - Check rule target is configured correctly - Verify input payload has correct structure @@ -188,6 +218,7 @@ validation failed" **Symptom**: Function execution exceeds configured timeout **Solutions**: + - Increase timeout (max 15 minutes) - Reduce Okta sync scope (fewer rules/groups) - Check for slow API responses from GitHub/Okta @@ -198,6 +229,7 @@ validation failed" **Symptom**: First webhook after idle period is slow **Solutions**: + - Accept it (typically 100-500ms, GitHub webhooks tolerate this) - Use Lambda provisioned concurrency (increases cost) - Consider standard HTTP server for consistently low latency @@ -217,7 +249,8 @@ No code changes needed - the core app is deployment-agnostic. ## Alternatives Consider non-Lambda deployments if you: -- Need consistently low latency (<50ms) + +- Need consistently low latency (\<50ms) - Want to avoid AWS vendor lock-in - Prefer traditional server management - Have existing container infrastructure diff --git a/docs/github-app-setup.md b/docs/github-app-setup.md index feac482..a2c3b41 100644 --- a/docs/github-app-setup.md +++ b/docs/github-app-setup.md @@ -37,10 +37,14 @@ Under **Permissions**, set the following: ### Repository Permissions -| Permission | Access | Purpose | -| ------------- | ------ | -------------------------------- | -| Contents | Read | Read branch protection rules | -| Pull requests | Read | Access PR details for compliance | +| Permission | Access | Purpose | +| ---------------------- | ------ | --------------------------------- | +| Checks | Read | Read status checks on PRs | +| Code scanning alerts | Read | Fetch open code scanning alerts | +| Contents | Read | Read branch protection rules | +| Dependabot alerts | Read | Fetch open Dependabot alerts | +| Pull requests | Read | Access PR details for compliance | +| Secret scanning alerts | Read | Fetch open secret scanning alerts | #### Organization Permissions @@ -49,6 +53,10 @@ Under **Permissions**, set the following: | Members | Read/Write | Manage team membership | | Administration | Read | Read organization settings | +> **Note**: The three security alert permissions are only required if you enable +> security alerts monitoring (`APP_SECURITY_ALERTS_ENABLED=true`). You can omit +> them if you don't use that feature. + 4. Set installation scope: | Setting | Value | diff --git a/docs/slack-setup.md b/docs/slack-setup.md index 1b4b4ee..f7dc94b 100644 --- a/docs/slack-setup.md +++ b/docs/slack-setup.md @@ -117,6 +117,7 @@ APP_SLACK_CHANNEL=C01ABC2DEFG APP_SLACK_CHANNEL_PR_BYPASS=C01234ABCDE APP_SLACK_CHANNEL_OKTA_SYNC=C01234ABCDE APP_SLACK_CHANNEL_ORPHANED_USERS=C01234ABCDE +APP_SLACK_CHANNEL_SECURITY_ALERTS=C01234ABCDE ``` For AWS deployments, use SSM parameters: @@ -167,12 +168,13 @@ Expected response: The bot sends these notification types: -| Event | Description | -| -------------------- | ------------------------------------- | -| PR Compliance Alert | PR merged bypassing branch protection | -| Okta Sync Report | Summary of team membership changes | -| Orphaned Users Alert | Org members not in any synced teams | -| Sync Error | Errors during Okta sync process | +| Event | Description | +| ---------------------- | ------------------------------------- | +| PR Compliance Alert | PR merged bypassing branch protection | +| Okta Sync Report | Summary of team membership changes | +| Orphaned Users Alert | Org members not in any synced teams | +| Security Alerts Report | Stale security alerts across the org | +| Sync Error | Errors during Okta sync process | ## Troubleshooting diff --git a/internal/app/app.go b/internal/app/app.go index d71ca38..012b311 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -43,6 +43,8 @@ func (a *App) processScheduledEvent(ctx context.Context, evt ScheduledEvent) err switch evt.Action { case "okta-sync": return a.handleOktaSync(ctx) + case "security-alerts": + return a.handleSecurityAlerts(ctx) case "slack-test": return a.handleSlackTest(ctx) default: @@ -71,20 +73,22 @@ func (a *App) processWebhook(ctx context.Context, payload []byte, eventType stri // StatusResponse contains application status and feature flags. type StatusResponse struct { - Status string `json:"status"` - GitHubConfigured bool `json:"github_configured"` - OktaSyncEnabled bool `json:"okta_sync_enabled"` - PRComplianceCheck bool `json:"pr_compliance_check"` - SlackEnabled bool `json:"slack_enabled"` + Status string `json:"status"` + GitHubConfigured bool `json:"github_configured"` + OktaSyncEnabled bool `json:"okta_sync_enabled"` + PRComplianceCheck bool `json:"pr_compliance_check"` + SecurityAlertsEnabled bool `json:"security_alerts_enabled"` + SlackEnabled bool `json:"slack_enabled"` } // GetStatus returns current application status and enabled features. func (a *App) GetStatus() StatusResponse { return StatusResponse{ - Status: "ok", - GitHubConfigured: a.Config.IsGitHubConfigured(), - OktaSyncEnabled: a.Config.IsOktaSyncEnabled(), - PRComplianceCheck: a.Config.IsPRComplianceEnabled(), - SlackEnabled: a.Config.SlackEnabled, + Status: "ok", + GitHubConfigured: a.Config.IsGitHubConfigured(), + OktaSyncEnabled: a.Config.IsOktaSyncEnabled(), + PRComplianceCheck: a.Config.IsPRComplianceEnabled(), + SecurityAlertsEnabled: a.Config.IsSecurityAlertsEnabled(), + SlackEnabled: a.Config.SlackEnabled, } } diff --git a/internal/app/factory.go b/internal/app/factory.go index ac2beb4..dd91e5f 100644 --- a/internal/app/factory.go +++ b/internal/app/factory.go @@ -52,10 +52,11 @@ func NewApp(ctx context.Context, cfg *config.Config, logger *slog.Logger) (*App, if cfg.SlackEnabled { channels := notifiers.SlackChannels{ - Default: cfg.SlackChannel, - PRBypass: cfg.SlackChannelPRBypass, - OktaSync: cfg.SlackChannelOktaSync, - OrphanedUsers: cfg.SlackChannelOrphanedUsers, + Default: cfg.SlackChannel, + PRBypass: cfg.SlackChannelPRBypass, + OktaSync: cfg.SlackChannelOktaSync, + OrphanedUsers: cfg.SlackChannelOrphanedUsers, + SecurityAlerts: cfg.SlackChannelSecurityAlerts, } messages := notifiers.SlackMessages{ PRBypassFooterNote: cfg.SlackPRBypassFooterNote, diff --git a/internal/app/handlers.go b/internal/app/handlers.go index 82b5a5e..2ae3b54 100644 --- a/internal/app/handlers.go +++ b/internal/app/handlers.go @@ -211,6 +211,41 @@ func (a *App) shouldIgnoreWebhookChange(ctx context.Context, event webhookSender return false } +// handleSecurityAlerts checks for stale security alerts across the org +// and sends a Slack notification if any are found. +func (a *App) handleSecurityAlerts(ctx context.Context) error { + if !a.Config.IsSecurityAlertsEnabled() { + a.Logger.Info("security alerts monitoring is not enabled, skipping") + return nil + } + + if a.GitHubClient == nil { + return errors.Wrap(domain.ErrClientNotInit, "github client") + } + + report, err := a.GitHubClient.ListSecurityAlerts( + ctx, + a.Config.SecurityAlertsMinAgeDays, + a.Config.SecurityAlertsMinSeverity, + ) + if err != nil { + return errors.Wrap(err, "failed to fetch stale security alerts") + } + + a.Logger.Info("security alerts check completed", + slog.Int("total_alerts", report.TotalAlerts), + slog.Int("repos", report.RepoCount())) + + if report.HasAlerts() && a.Notifier != nil { + if err := a.Notifier.NotifySecurityAlerts(ctx, report, a.Config.GitHubOrg); err != nil { + a.Logger.Warn("failed to send security alerts notification", + slog.String("error", err.Error())) + } + } + + return nil +} + // handleSlackTest sends test notifications to Slack with sample data. // useful for verifying Slack connectivity and previewing message formats. func (a *App) handleSlackTest(ctx context.Context) error { @@ -236,5 +271,11 @@ func (a *App) handleSlackTest(ctx context.Context) error { } a.Logger.Info("sent test orphaned users notification") + // test 4: Security alerts notification + if err := a.Notifier.NotifySecurityAlerts(ctx, fakeSecurityAlertsReport(), "acme-corp"); err != nil { + return errors.Wrap(err, "failed to send test security alerts notification") + } + a.Logger.Info("sent test security alerts notification") + return nil } diff --git a/internal/app/handlers_test.go b/internal/app/handlers_test.go index 3a58fb3..1c7c694 100644 --- a/internal/app/handlers_test.go +++ b/internal/app/handlers_test.go @@ -25,6 +25,7 @@ type mockGitHubClient struct { listOrgMembersFn func(ctx context.Context) ([]string, error) isExternalCollaboratorFn func(ctx context.Context, username string) (bool, error) getAppSlugFn func(ctx context.Context) (string, error) + listSecurityAlertsFn func(ctx context.Context, minAgeDays int, minSeverity string) (*domain.SecurityAlertsReport, error) } func (m *mockGitHubClient) CheckPRCompliance(ctx context.Context, owner, repo string, prNumber int) (*domain.PRComplianceResult, error) { @@ -71,6 +72,12 @@ func (m *mockGitHubClient) GetAppSlug(ctx context.Context) (string, error) { return "test-app", nil } func (m *mockGitHubClient) GetOrg() string { return "test-org" } +func (m *mockGitHubClient) ListSecurityAlerts(ctx context.Context, minAgeDays int, minSeverity string) (*domain.SecurityAlertsReport, error) { + if m.listSecurityAlertsFn != nil { + return m.listSecurityAlertsFn(ctx, minAgeDays, minSeverity) + } + return &domain.SecurityAlertsReport{AlertsByRepo: map[string][]domain.SecurityAlert{}}, nil +} type mockOktaClient struct { getGroupsByPatternFn func(ctx context.Context, pattern string) ([]*domain.GroupInfo, error) @@ -91,9 +98,10 @@ func (m *mockOktaClient) GetGroupInfo(ctx context.Context, groupName string) (*d } type mockNotifier struct { - notifyPRBypassFn func(ctx context.Context, result *domain.PRComplianceResult, repoFullName string) error - notifyOktaSyncFn func(ctx context.Context, reports []*domain.SyncReport, githubOrg string) error - notifyOrphanedUsersFn func(ctx context.Context, report *domain.OrphanedUsersReport) error + notifyPRBypassFn func(ctx context.Context, result *domain.PRComplianceResult, repoFullName string) error + notifyOktaSyncFn func(ctx context.Context, reports []*domain.SyncReport, githubOrg string) error + notifyOrphanedUsersFn func(ctx context.Context, report *domain.OrphanedUsersReport) error + notifySecurityAlertsFn func(ctx context.Context, report *domain.SecurityAlertsReport, githubOrg string) error } func (m *mockNotifier) NotifyPRBypass(ctx context.Context, result *domain.PRComplianceResult, repoFullName string) error { @@ -114,6 +122,12 @@ func (m *mockNotifier) NotifyOrphanedUsers(ctx context.Context, report *domain.O } return nil } +func (m *mockNotifier) NotifySecurityAlerts(ctx context.Context, report *domain.SecurityAlertsReport, githubOrg string) error { + if m.notifySecurityAlertsFn != nil { + return m.notifySecurityAlertsFn(ctx, report, githubOrg) + } + return nil +} func discardLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) @@ -586,6 +600,176 @@ func TestRouter_BasePathStripping(t *testing.T) { } } +func TestHandleSecurityAlerts_Disabled(t *testing.T) { + a := &App{ + Config: &config.Config{}, + Logger: discardLogger(), + } + + err := a.handleSecurityAlerts(context.Background()) + if err != nil { + t.Fatalf("expected nil when disabled, got: %v", err) + } +} + +func TestHandleSecurityAlerts_ClientNil(t *testing.T) { + a := &App{ + Config: &config.Config{ + SecurityAlertsEnabled: true, + GitHubOrg: "org", + GitHubAppID: 1, + GitHubAppPrivateKey: []byte("k"), + GitHubInstallationID: 1, + }, + Logger: discardLogger(), + } + + err := a.handleSecurityAlerts(context.Background()) + if err == nil { + t.Fatal("expected error when client is nil") + } + if !errors.Is(err, domain.ErrClientNotInit) { + t.Errorf("expected ErrClientNotInit, got: %v", err) + } +} + +func TestHandleSecurityAlerts_Success(t *testing.T) { + notified := false + a := &App{ + Config: &config.Config{ + SecurityAlertsEnabled: true, + SecurityAlertsMinAgeDays: 30, + SecurityAlertsMinSeverity: "high", + GitHubOrg: "org", + GitHubAppID: 1, + GitHubAppPrivateKey: []byte("k"), + GitHubInstallationID: 1, + }, + Logger: discardLogger(), + GitHubClient: &mockGitHubClient{ + listSecurityAlertsFn: func(_ context.Context, minAge int, minSev string) (*domain.SecurityAlertsReport, error) { + if minAge != 30 { + t.Errorf("expected minAge 30, got %d", minAge) + } + if minSev != "high" { + t.Errorf("expected minSev high, got %s", minSev) + } + return &domain.SecurityAlertsReport{ + TotalAlerts: 2, + AlertsByRepo: map[string][]domain.SecurityAlert{ + "org/repo": {{Type: "dependabot", Severity: "critical"}}, + }, + }, nil + }, + }, + Notifier: &mockNotifier{ + notifySecurityAlertsFn: func(_ context.Context, report *domain.SecurityAlertsReport, org string) error { + notified = true + if report.TotalAlerts != 2 { + t.Errorf("expected 2 alerts, got %d", report.TotalAlerts) + } + if org != "org" { + t.Errorf("expected org, got %s", org) + } + return nil + }, + }, + } + + err := a.handleSecurityAlerts(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !notified { + t.Error("expected notification to be sent") + } +} + +func TestHandleSecurityAlerts_NoAlertsNoNotification(t *testing.T) { + notified := false + a := &App{ + Config: &config.Config{ + SecurityAlertsEnabled: true, + SecurityAlertsMinAgeDays: 30, + SecurityAlertsMinSeverity: "high", + GitHubOrg: "org", + GitHubAppID: 1, + GitHubAppPrivateKey: []byte("k"), + GitHubInstallationID: 1, + }, + Logger: discardLogger(), + GitHubClient: &mockGitHubClient{ + listSecurityAlertsFn: func(_ context.Context, _ int, _ string) (*domain.SecurityAlertsReport, error) { + return &domain.SecurityAlertsReport{ + TotalAlerts: 0, + AlertsByRepo: map[string][]domain.SecurityAlert{}, + }, nil + }, + }, + Notifier: &mockNotifier{ + notifySecurityAlertsFn: func(_ context.Context, _ *domain.SecurityAlertsReport, _ string) error { + notified = true + return nil + }, + }, + } + + err := a.handleSecurityAlerts(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if notified { + t.Error("should not notify when no alerts") + } +} + +func TestHandleSecurityAlerts_NotifierFailureDoesNotFail(t *testing.T) { + a := &App{ + Config: &config.Config{ + SecurityAlertsEnabled: true, + SecurityAlertsMinAgeDays: 30, + SecurityAlertsMinSeverity: "high", + GitHubOrg: "org", + GitHubAppID: 1, + GitHubAppPrivateKey: []byte("k"), + GitHubInstallationID: 1, + }, + Logger: discardLogger(), + GitHubClient: &mockGitHubClient{ + listSecurityAlertsFn: func(_ context.Context, _ int, _ string) (*domain.SecurityAlertsReport, error) { + return &domain.SecurityAlertsReport{ + TotalAlerts: 1, + AlertsByRepo: map[string][]domain.SecurityAlert{ + "org/repo": {{Type: "dependabot"}}, + }, + }, nil + }, + }, + Notifier: &mockNotifier{ + notifySecurityAlertsFn: func(_ context.Context, _ *domain.SecurityAlertsReport, _ string) error { + return errors.New("slack api error") + }, + }, + } + + err := a.handleSecurityAlerts(context.Background()) + if err != nil { + t.Fatalf("notifier failure should not propagate, got: %v", err) + } +} + +func TestProcessScheduledEvent_SecurityAlerts(t *testing.T) { + a := &App{ + Config: &config.Config{}, + Logger: discardLogger(), + } + + err := a.processScheduledEvent(context.Background(), ScheduledEvent{Action: "security-alerts"}) + if err != nil { + t.Fatalf("expected nil when feature disabled, got: %v", err) + } +} + // stubSender is a simple webhookSender for testing shouldIgnoreWebhookChange type stubSender struct { senderType string diff --git a/internal/app/testdata.go b/internal/app/testdata.go index bae49a2..0e0c07e 100644 --- a/internal/app/testdata.go +++ b/internal/app/testdata.go @@ -1,6 +1,8 @@ package app import ( + "time" + "github.com/cruxstack/github-ops-app/internal/domain" gh "github.com/google/go-github/v79/github" ) @@ -65,3 +67,70 @@ func fakeOrphanedUsersReport() *domain.OrphanedUsersReport { OrphanedUsers: []string{"orphan-user-1", "orphan-user-2", "legacy-bot"}, } } + +// fakeSecurityAlertsReport returns sample security alerts data for +// testing. +func fakeSecurityAlertsReport() *domain.SecurityAlertsReport { + now := time.Now() + return &domain.SecurityAlertsReport{ + MinAgeDays: 30, + MinSeverity: "high", + TotalAlerts: 5, + AlertsByRepo: map[string][]domain.SecurityAlert{ + "acme-corp/api-service": { + { + Type: "dependabot", + Repo: "acme-corp/api-service", + Number: 12, + Severity: "critical", + Summary: "Remote code execution in example-lib", + HTMLURL: "https://github.com/acme-corp/api-service/security/dependabot/12", + CreatedAt: now.AddDate(0, 0, -45), + AgeDays: 45, + }, + { + Type: "code_scanning", + Repo: "acme-corp/api-service", + Number: 7, + Severity: "high", + Summary: "SQL injection vulnerability", + HTMLURL: "https://github.com/acme-corp/api-service/security/code-scanning/7", + CreatedAt: now.AddDate(0, 0, -60), + AgeDays: 60, + }, + }, + "acme-corp/web-app": { + { + Type: "secret_scanning", + Repo: "acme-corp/web-app", + Number: 3, + Severity: "high", + Summary: "GitHub Personal Access Token", + HTMLURL: "https://github.com/acme-corp/web-app/security/secret-scanning/3", + CreatedAt: now.AddDate(0, 0, -90), + AgeDays: 90, + }, + { + Type: "dependabot", + Repo: "acme-corp/web-app", + Number: 25, + Severity: "high", + Summary: "Prototype pollution in lodash", + HTMLURL: "https://github.com/acme-corp/web-app/security/dependabot/25", + CreatedAt: now.AddDate(0, 0, -35), + AgeDays: 35, + }, + { + Type: "code_scanning", + Repo: "acme-corp/web-app", + Number: 14, + Severity: "critical", + Summary: "Cross-site scripting vulnerability", + HTMLURL: "https://github.com/acme-corp/web-app/security/code-scanning/14", + CreatedAt: now.AddDate(0, 0, -42), + AgeDays: 42, + }, + }, + }, + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 2895a98..f631502 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -50,15 +50,21 @@ type Config struct { OktaSyncSafetyThreshold float64 OktaOrphanedUserNotifications bool + // Security Alerts + SecurityAlertsEnabled bool + SecurityAlertsMinAgeDays int + SecurityAlertsMinSeverity string + // Slack - SlackEnabled bool - SlackToken string - SlackChannel string - SlackChannelPRBypass string - SlackChannelOktaSync string - SlackChannelOrphanedUsers string - SlackPRBypassFooterNote string - SlackAPIURL string + SlackEnabled bool + SlackToken string + SlackChannel string + SlackChannelPRBypass string + SlackChannelOktaSync string + SlackChannelOrphanedUsers string + SlackChannelSecurityAlerts string + SlackPRBypassFooterNote string + SlackAPIURL string } var ( @@ -172,23 +178,24 @@ func NewConfigWithContext(ctx context.Context) (*Config, error) { } cfg := Config{ - DebugEnabled: debugEnabled, - AdminToken: adminToken, - GitHubOrg: os.Getenv("APP_GITHUB_ORG"), - GitHubWebhookSecret: githubWebhookSecret, - GitHubBaseURL: os.Getenv("APP_GITHUB_BASE_URL"), - OktaDomain: os.Getenv("APP_OKTA_DOMAIN"), - OktaClientID: os.Getenv("APP_OKTA_CLIENT_ID"), - OktaBaseURL: os.Getenv("APP_OKTA_BASE_URL"), - OktaGitHubUserField: oktaGitHubUserField, - OktaSyncSafetyThreshold: oktaSyncSafetyThreshold, - SlackToken: slackToken, - SlackChannel: os.Getenv("APP_SLACK_CHANNEL"), - SlackChannelPRBypass: os.Getenv("APP_SLACK_CHANNEL_PR_BYPASS"), - SlackChannelOktaSync: os.Getenv("APP_SLACK_CHANNEL_OKTA_SYNC"), - SlackChannelOrphanedUsers: os.Getenv("APP_SLACK_CHANNEL_ORPHANED_USERS"), - SlackPRBypassFooterNote: os.Getenv("APP_SLACK_FOOTER_NOTE_PR_BYPASS"), - SlackAPIURL: os.Getenv("APP_SLACK_API_URL"), + DebugEnabled: debugEnabled, + AdminToken: adminToken, + GitHubOrg: os.Getenv("APP_GITHUB_ORG"), + GitHubWebhookSecret: githubWebhookSecret, + GitHubBaseURL: os.Getenv("APP_GITHUB_BASE_URL"), + OktaDomain: os.Getenv("APP_OKTA_DOMAIN"), + OktaClientID: os.Getenv("APP_OKTA_CLIENT_ID"), + OktaBaseURL: os.Getenv("APP_OKTA_BASE_URL"), + OktaGitHubUserField: oktaGitHubUserField, + OktaSyncSafetyThreshold: oktaSyncSafetyThreshold, + SlackToken: slackToken, + SlackChannel: os.Getenv("APP_SLACK_CHANNEL"), + SlackChannelPRBypass: os.Getenv("APP_SLACK_CHANNEL_PR_BYPASS"), + SlackChannelOktaSync: os.Getenv("APP_SLACK_CHANNEL_OKTA_SYNC"), + SlackChannelOrphanedUsers: os.Getenv("APP_SLACK_CHANNEL_ORPHANED_USERS"), + SlackChannelSecurityAlerts: os.Getenv("APP_SLACK_CHANNEL_SECURITY_ALERTS"), + SlackPRBypassFooterNote: os.Getenv("APP_SLACK_FOOTER_NOTE_PR_BYPASS"), + SlackAPIURL: os.Getenv("APP_SLACK_API_URL"), } if appIDStr := os.Getenv("APP_GITHUB_APP_ID"); appIDStr != "" { @@ -280,6 +287,27 @@ func NewConfigWithContext(ctx context.Context) (*Config, error) { } cfg.OktaOrphanedUserNotifications = orphanedUserNotifications + securityAlertsEnabled, _ := strconv.ParseBool( + os.Getenv("APP_SECURITY_ALERTS_ENABLED"), + ) + cfg.SecurityAlertsEnabled = securityAlertsEnabled + + cfg.SecurityAlertsMinAgeDays = 30 + if ageDaysStr := os.Getenv("APP_SECURITY_ALERTS_MIN_AGE_DAYS"); ageDaysStr != "" { + if ageDays, err := strconv.Atoi(ageDaysStr); err == nil && ageDays > 0 { + cfg.SecurityAlertsMinAgeDays = ageDays + } + } + + cfg.SecurityAlertsMinSeverity = "high" + if sev := os.Getenv("APP_SECURITY_ALERTS_MIN_SEVERITY"); sev != "" { + sev = strings.ToLower(strings.TrimSpace(sev)) + switch sev { + case "critical", "high", "medium", "low": + cfg.SecurityAlertsMinSeverity = sev + } + } + return &cfg, nil } @@ -327,6 +355,12 @@ func (c *Config) IsGitHubConfigured() bool { c.GitHubInstallationID != 0 } +// IsSecurityAlertsEnabled returns true if security alerts monitoring is +// enabled and GitHub is configured. +func (c *Config) IsSecurityAlertsEnabled() bool { + return c.SecurityAlertsEnabled && c.IsGitHubConfigured() +} + // ShouldMonitorBranch returns true if the given branch should be monitored // for PR compliance. func (c *Config) ShouldMonitorBranch(branch string) bool { @@ -374,15 +408,21 @@ type RedactedConfig struct { OktaSyncSafetyThreshold float64 `json:"okta_sync_safety_threshold"` OktaOrphanedUserNotifications bool `json:"okta_orphaned_user_notifications"` + // Security Alerts + SecurityAlertsEnabled bool `json:"security_alerts_enabled"` + SecurityAlertsMinAgeDays int `json:"security_alerts_min_age_days"` + SecurityAlertsMinSeverity string `json:"security_alerts_min_severity"` + // Slack - SlackEnabled bool `json:"slack_enabled"` - SlackToken string `json:"slack_token"` - SlackChannel string `json:"slack_channel"` - SlackChannelPRBypass string `json:"slack_channel_pr_bypass"` - SlackChannelOktaSync string `json:"slack_channel_okta_sync"` - SlackChannelOrphanedUsers string `json:"slack_channel_orphaned_users"` - SlackPRBypassFooterNote string `json:"slack_pr_bypass_footer_note"` - SlackAPIURL string `json:"slack_api_url"` + SlackEnabled bool `json:"slack_enabled"` + SlackToken string `json:"slack_token"` + SlackChannel string `json:"slack_channel"` + SlackChannelPRBypass string `json:"slack_channel_pr_bypass"` + SlackChannelOktaSync string `json:"slack_channel_okta_sync"` + SlackChannelOrphanedUsers string `json:"slack_channel_orphaned_users"` + SlackChannelSecurityAlerts string `json:"slack_channel_security_alerts"` + SlackPRBypassFooterNote string `json:"slack_pr_bypass_footer_note"` + SlackAPIURL string `json:"slack_api_url"` } // Redacted returns a copy of the config with secrets redacted. @@ -431,14 +471,20 @@ func (c *Config) Redacted() RedactedConfig { OktaSyncSafetyThreshold: c.OktaSyncSafetyThreshold, OktaOrphanedUserNotifications: c.OktaOrphanedUserNotifications, + // Security Alerts + SecurityAlertsEnabled: c.SecurityAlertsEnabled, + SecurityAlertsMinAgeDays: c.SecurityAlertsMinAgeDays, + SecurityAlertsMinSeverity: c.SecurityAlertsMinSeverity, + // Slack - SlackEnabled: c.SlackEnabled, - SlackToken: redact(c.SlackToken), - SlackChannel: c.SlackChannel, - SlackChannelPRBypass: c.SlackChannelPRBypass, - SlackChannelOktaSync: c.SlackChannelOktaSync, - SlackChannelOrphanedUsers: c.SlackChannelOrphanedUsers, - SlackPRBypassFooterNote: c.SlackPRBypassFooterNote, - SlackAPIURL: c.SlackAPIURL, + SlackEnabled: c.SlackEnabled, + SlackToken: redact(c.SlackToken), + SlackChannel: c.SlackChannel, + SlackChannelPRBypass: c.SlackChannelPRBypass, + SlackChannelOktaSync: c.SlackChannelOktaSync, + SlackChannelOrphanedUsers: c.SlackChannelOrphanedUsers, + SlackChannelSecurityAlerts: c.SlackChannelSecurityAlerts, + SlackPRBypassFooterNote: c.SlackPRBypassFooterNote, + SlackAPIURL: c.SlackAPIURL, } } diff --git a/internal/config/config_helpers_test.go b/internal/config/config_helpers_test.go index ed234a2..033cf95 100644 --- a/internal/config/config_helpers_test.go +++ b/internal/config/config_helpers_test.go @@ -125,6 +125,38 @@ func TestShouldMonitorBranch(t *testing.T) { } } +func TestIsSecurityAlertsEnabled(t *testing.T) { + tests := []struct { + name string + cfg Config + want bool + }{ + {name: "disabled flag", cfg: Config{SecurityAlertsEnabled: false}, want: false}, + { + name: "enabled but no github", + cfg: Config{SecurityAlertsEnabled: true}, + want: false, + }, + { + name: "enabled with github", + cfg: Config{ + SecurityAlertsEnabled: true, + GitHubOrg: "org", GitHubAppID: 1, + GitHubAppPrivateKey: []byte("k"), GitHubInstallationID: 1, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.cfg.IsSecurityAlertsEnabled(); got != tt.want { + t.Errorf("IsSecurityAlertsEnabled() = %v, want %v", got, tt.want) + } + }) + } +} + func TestRedacted(t *testing.T) { cfg := Config{ GitHubOrg: "my-org", diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 957c425..423efab 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -5,6 +5,91 @@ import ( "testing" ) +func TestSecurityAlertsConfigDefaults(t *testing.T) { + t.Setenv("APP_SECURITY_ALERTS_ENABLED", "") + t.Setenv("APP_SECURITY_ALERTS_MIN_AGE_DAYS", "") + t.Setenv("APP_SECURITY_ALERTS_MIN_SEVERITY", "") + t.Setenv("APP_SLACK_CHANNEL_SECURITY_ALERTS", "") + // clear keys that could interfere + t.Setenv("APP_GITHUB_APP_ID", "") + t.Setenv("APP_GITHUB_WEBHOOK_SECRET", "") + t.Setenv("APP_SLACK_TOKEN", "") + t.Setenv("APP_ADMIN_TOKEN", "") + t.Setenv("APP_OKTA_SYNC_RULES", "") + + cfg, err := NewConfig() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.SecurityAlertsEnabled { + t.Error("expected security alerts disabled by default") + } + if cfg.SecurityAlertsMinAgeDays != 30 { + t.Errorf("expected default min age 30, got %d", + cfg.SecurityAlertsMinAgeDays) + } + if cfg.SecurityAlertsMinSeverity != "high" { + t.Errorf("expected default severity high, got %q", + cfg.SecurityAlertsMinSeverity) + } +} + +func TestSecurityAlertsConfigParsing(t *testing.T) { + t.Setenv("APP_SECURITY_ALERTS_ENABLED", "true") + t.Setenv("APP_SECURITY_ALERTS_MIN_AGE_DAYS", "14") + t.Setenv("APP_SECURITY_ALERTS_MIN_SEVERITY", "critical") + t.Setenv("APP_SLACK_CHANNEL_SECURITY_ALERTS", "C_SEC_ALERTS") + // clear keys that could interfere + t.Setenv("APP_GITHUB_APP_ID", "") + t.Setenv("APP_GITHUB_WEBHOOK_SECRET", "") + t.Setenv("APP_SLACK_TOKEN", "") + t.Setenv("APP_ADMIN_TOKEN", "") + t.Setenv("APP_OKTA_SYNC_RULES", "") + + cfg, err := NewConfig() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !cfg.SecurityAlertsEnabled { + t.Error("expected security alerts enabled") + } + if cfg.SecurityAlertsMinAgeDays != 14 { + t.Errorf("expected min age 14, got %d", + cfg.SecurityAlertsMinAgeDays) + } + if cfg.SecurityAlertsMinSeverity != "critical" { + t.Errorf("expected severity critical, got %q", + cfg.SecurityAlertsMinSeverity) + } + if cfg.SlackChannelSecurityAlerts != "C_SEC_ALERTS" { + t.Errorf("expected channel C_SEC_ALERTS, got %q", + cfg.SlackChannelSecurityAlerts) + } +} + +func TestSecurityAlertsConfigInvalidSeverity(t *testing.T) { + t.Setenv("APP_SECURITY_ALERTS_MIN_SEVERITY", "INVALID") + t.Setenv("APP_SECURITY_ALERTS_ENABLED", "") + t.Setenv("APP_SECURITY_ALERTS_MIN_AGE_DAYS", "") + t.Setenv("APP_GITHUB_APP_ID", "") + t.Setenv("APP_GITHUB_WEBHOOK_SECRET", "") + t.Setenv("APP_SLACK_TOKEN", "") + t.Setenv("APP_ADMIN_TOKEN", "") + t.Setenv("APP_OKTA_SYNC_RULES", "") + + cfg, err := NewConfig() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.SecurityAlertsMinSeverity != "high" { + t.Errorf("expected default high for invalid severity, got %q", + cfg.SecurityAlertsMinSeverity) + } +} + func TestResolveEnvValue(t *testing.T) { ctx := context.Background() diff --git a/internal/domain/interfaces.go b/internal/domain/interfaces.go index c40da02..eb81cb5 100644 --- a/internal/domain/interfaces.go +++ b/internal/domain/interfaces.go @@ -36,6 +36,11 @@ type GitHubClient interface { // GetOrg returns the GitHub organization name. GetOrg() string + + // ListSecurityAlerts fetches open security alerts across the org + // that are older than minAgeDays and at or above minSeverity. + // covers dependabot, code scanning, and secret scanning alerts. + ListSecurityAlerts(ctx context.Context, minAgeDays int, minSeverity string) (*SecurityAlertsReport, error) } // OktaClient defines the interface for Okta API operations. @@ -61,4 +66,8 @@ type Notifier interface { // NotifyOrphanedUsers sends a notification about organization members // not in any synced teams. NotifyOrphanedUsers(ctx context.Context, report *OrphanedUsersReport) error + + // NotifySecurityAlerts sends a notification about stale security + // alerts across the organization. + NotifySecurityAlerts(ctx context.Context, report *SecurityAlertsReport, githubOrg string) error } diff --git a/internal/domain/types.go b/internal/domain/types.go index f08b05f..169c413 100644 --- a/internal/domain/types.go +++ b/internal/domain/types.go @@ -1,6 +1,10 @@ package domain -import "github.com/google/go-github/v79/github" +import ( + "time" + + "github.com/google/go-github/v79/github" +) // SyncRule defines how to sync Okta groups to GitHub teams. type SyncRule struct { @@ -124,3 +128,40 @@ type SyncResult struct { Reports []*SyncReport OrphanedUsers *OrphanedUsersReport } + +// SecurityAlert represents a normalized security alert from any source +// (dependabot, code scanning, or secret scanning). +type SecurityAlert struct { + Type string + Repo string + Number int + Severity string + Summary string + HTMLURL string + CreatedAt time.Time + AgeDays int +} + +// SecurityAlertsReport contains stale security alerts grouped by repo. +type SecurityAlertsReport struct { + MinAgeDays int + MinSeverity string + TotalAlerts int + AlertsByRepo map[string][]SecurityAlert + Errors []string +} + +// HasAlerts returns true if any stale alerts were found. +func (r *SecurityAlertsReport) HasAlerts() bool { + return r.TotalAlerts > 0 +} + +// HasErrors returns true if any errors occurred during alert fetching. +func (r *SecurityAlertsReport) HasErrors() bool { + return len(r.Errors) > 0 +} + +// RepoCount returns the number of repos with stale alerts. +func (r *SecurityAlertsReport) RepoCount() int { + return len(r.AlertsByRepo) +} diff --git a/internal/github/client/security_alerts.go b/internal/github/client/security_alerts.go new file mode 100644 index 0000000..fdeca32 --- /dev/null +++ b/internal/github/client/security_alerts.go @@ -0,0 +1,291 @@ +package client + +import ( + "context" + "math" + "time" + "unicode/utf8" + + "github.com/cockroachdb/errors" + "github.com/cruxstack/github-ops-app/internal/domain" + "github.com/google/go-github/v79/github" +) + +// severityRank maps severity strings to numeric values for comparison. +// higher values indicate greater severity. +var severityRank = map[string]int{ + "critical": 4, + "high": 3, + "medium": 2, + "low": 1, +} + +// meetsMinSeverity returns true if the alert severity is at or above +// the minimum threshold. +func meetsMinSeverity(alertSeverity, minSeverity string) bool { + return severityRank[alertSeverity] >= severityRank[minSeverity] +} + +// ListSecurityAlerts fetches open security alerts across the org that +// are older than minAgeDays and at or above minSeverity. covers +// dependabot, code scanning, and secret scanning alerts. +func (c *Client) ListSecurityAlerts(ctx context.Context, minAgeDays int, minSeverity string) (*domain.SecurityAlertsReport, error) { + if err := c.ensureValidToken(ctx); err != nil { + return nil, err + } + + cutoff := time.Now().AddDate(0, 0, -minAgeDays) + + report := &domain.SecurityAlertsReport{ + MinAgeDays: minAgeDays, + MinSeverity: minSeverity, + AlertsByRepo: make(map[string][]domain.SecurityAlert), + } + + c.fetchDependabotAlerts(ctx, cutoff, minSeverity, report) + c.fetchCodeScanningAlerts(ctx, cutoff, minSeverity, report) + c.fetchSecretScanningAlerts(ctx, cutoff, report) + + total := 0 + for _, alerts := range report.AlertsByRepo { + total += len(alerts) + } + report.TotalAlerts = total + + return report, nil +} + +// fetchDependabotAlerts fetches open dependabot alerts for the org, +// filtering by severity and age. results are sorted by creation date +// ascending so the pagination early-exit works correctly: once a page +// contains only alerts newer than the cutoff, all subsequent pages +// will also be newer. +func (c *Client) fetchDependabotAlerts(ctx context.Context, cutoff time.Time, minSeverity string, report *domain.SecurityAlertsReport) { + state := "open" + opts := &github.ListAlertsOptions{ + State: &state, + Sort: github.Ptr("created"), + Direction: github.Ptr("asc"), + ListCursorOptions: github.ListCursorOptions{ + PerPage: 100, + }, + } + + for { + alerts, resp, err := c.client.Dependabot.ListOrgAlerts(ctx, c.org, opts) + if err != nil { + report.Errors = append(report.Errors, + errors.Wrapf(err, "failed to fetch dependabot alerts for org '%s'", c.org).Error()) + return + } + + pastCutoff := false + for _, alert := range alerts { + created := alert.GetCreatedAt().Time + if !created.Before(cutoff) { + pastCutoff = true + break + } + + sev := "" + if alert.SecurityAdvisory != nil { + sev = alert.SecurityAdvisory.GetSeverity() + } + if sev == "" && alert.SecurityVulnerability != nil && + alert.SecurityVulnerability.Severity != nil { + sev = *alert.SecurityVulnerability.Severity + } + + if !meetsMinSeverity(sev, minSeverity) { + continue + } + + repo := "" + if alert.Repository != nil { + repo = alert.Repository.GetFullName() + } + + ageDays := int(math.Floor( + time.Since(created).Hours() / 24, + )) + + report.AlertsByRepo[repo] = append( + report.AlertsByRepo[repo], + domain.SecurityAlert{ + Type: "dependabot", + Repo: repo, + Number: alert.GetNumber(), + Severity: sev, + Summary: truncate(alert.SecurityAdvisory.GetSummary(), 120), + HTMLURL: alert.GetHTMLURL(), + CreatedAt: created, + AgeDays: ageDays, + }, + ) + } + + if resp.NextPageToken == "" || pastCutoff { + break + } + opts.ListCursorOptions.After = resp.NextPageToken + } +} + +// fetchCodeScanningAlerts fetches open code scanning alerts for the +// org, filtering by severity and age. results are sorted ascending +// by creation date so the early-exit optimization works correctly. +// the code scanning API only accepts a single severity value, so we +// fetch all severities and filter client-side via meetsMinSeverity. +func (c *Client) fetchCodeScanningAlerts(ctx context.Context, cutoff time.Time, minSeverity string, report *domain.SecurityAlertsReport) { + opts := &github.AlertListOptions{ + State: "open", + Sort: "created", + Direction: "asc", + ListCursorOptions: github.ListCursorOptions{ + PerPage: 100, + }, + } + + for { + alerts, resp, err := c.client.CodeScanning.ListAlertsForOrg(ctx, c.org, opts) + if err != nil { + report.Errors = append(report.Errors, + errors.Wrapf(err, "failed to fetch code scanning alerts for org '%s'", c.org).Error()) + return + } + + pastCutoff := false + for _, alert := range alerts { + created := alert.GetCreatedAt().Time + if !created.Before(cutoff) { + pastCutoff = true + break + } + + sev := "" + if alert.Rule != nil { + sev = alert.Rule.GetSecuritySeverityLevel() + if sev == "" { + sev = alert.Rule.GetSeverity() + } + } + + // alerts with unknown severity are included + // conservatively; we only exclude alerts that have a + // known severity below the threshold. + if sev != "" && !meetsMinSeverity(sev, minSeverity) { + continue + } + + repo := "" + if alert.Repository != nil { + repo = alert.Repository.GetFullName() + } + + summary := "" + if alert.Rule != nil { + summary = alert.Rule.GetDescription() + } + + ageDays := int(math.Floor( + time.Since(created).Hours() / 24, + )) + + report.AlertsByRepo[repo] = append( + report.AlertsByRepo[repo], + domain.SecurityAlert{ + Type: "code_scanning", + Repo: repo, + Number: int(alert.GetNumber()), + Severity: sev, + Summary: truncate(summary, 120), + HTMLURL: alert.GetHTMLURL(), + CreatedAt: created, + AgeDays: ageDays, + }, + ) + } + + if resp.NextPageToken == "" || pastCutoff { + break + } + opts.ListCursorOptions.After = resp.NextPageToken + } +} + +// fetchSecretScanningAlerts fetches open secret scanning alerts for +// the org, filtering by age only. results are sorted ascending by +// creation date so the early-exit optimization works correctly. +// secret scanning alerts have no severity in the GitHub API, so we +// assign "high" to ensure they are included when minSeverity is +// "high" or below but excluded when minSeverity is "critical". +func (c *Client) fetchSecretScanningAlerts(ctx context.Context, cutoff time.Time, report *domain.SecurityAlertsReport) { + opts := &github.SecretScanningAlertListOptions{ + State: "open", + Sort: "created", + Direction: "asc", + ListCursorOptions: github.ListCursorOptions{ + PerPage: 100, + }, + } + + for { + alerts, resp, err := c.client.SecretScanning.ListAlertsForOrg(ctx, c.org, opts) + if err != nil { + report.Errors = append(report.Errors, + errors.Wrapf(err, "failed to fetch secret scanning alerts for org '%s'", c.org).Error()) + return + } + + pastCutoff := false + for _, alert := range alerts { + created := alert.GetCreatedAt().Time + if !created.Before(cutoff) { + pastCutoff = true + break + } + + repo := "" + if alert.Repository != nil { + repo = alert.Repository.GetFullName() + } + + ageDays := int(math.Floor( + time.Since(created).Hours() / 24, + )) + + report.AlertsByRepo[repo] = append( + report.AlertsByRepo[repo], + domain.SecurityAlert{ + Type: "secret_scanning", + Repo: repo, + Number: alert.GetNumber(), + Severity: "high", + Summary: truncate(alert.GetSecretTypeDisplayName(), 120), + HTMLURL: alert.GetHTMLURL(), + CreatedAt: created, + AgeDays: ageDays, + }, + ) + } + + if resp.NextPageToken == "" || pastCutoff { + break + } + opts.ListCursorOptions.After = resp.NextPageToken + } +} + +// truncate shortens a string to max runes, appending "..." if +// needed. operates on rune count so multi-byte characters are not +// split mid-codepoint. +func truncate(s string, maxLen int) string { + if utf8.RuneCountInString(s) <= maxLen { + return s + } + runes := []rune(s) + if maxLen <= 3 { + return string(runes[:maxLen]) + } + return string(runes[:maxLen-3]) + "..." +} diff --git a/internal/github/client/security_alerts_test.go b/internal/github/client/security_alerts_test.go new file mode 100644 index 0000000..fba2e80 --- /dev/null +++ b/internal/github/client/security_alerts_test.go @@ -0,0 +1,70 @@ +package client + +import "testing" + +func TestMeetsMinSeverity(t *testing.T) { + tests := []struct { + alertSev string + minSev string + want bool + }{ + {"critical", "critical", true}, + {"critical", "high", true}, + {"critical", "medium", true}, + {"critical", "low", true}, + {"high", "critical", false}, + {"high", "high", true}, + {"high", "medium", true}, + {"high", "low", true}, + {"medium", "critical", false}, + {"medium", "high", false}, + {"medium", "medium", true}, + {"medium", "low", true}, + {"low", "critical", false}, + {"low", "high", false}, + {"low", "medium", false}, + {"low", "low", true}, + {"", "high", false}, + {"high", "", true}, + {"unknown", "high", false}, + } + + for _, tt := range tests { + t.Run(tt.alertSev+"_vs_"+tt.minSev, func(t *testing.T) { + got := meetsMinSeverity(tt.alertSev, tt.minSev) + if got != tt.want { + t.Errorf("meetsMinSeverity(%q, %q) = %v, want %v", + tt.alertSev, tt.minSev, got, tt.want) + } + }) + } +} + +func TestTruncate(t *testing.T) { + tests := []struct { + name string + input string + maxLen int + want string + }{ + {"short", "short", 10, "short"}, + {"exact", "exactly10!", 10, "exactly10!"}, + {"long", "this is too long", 10, "this is..."}, + {"exact_boundary", "abc", 3, "abc"}, + {"over_boundary", "abcd", 3, "abc"}, + {"empty", "", 5, ""}, + {"unicode_under", "héllo", 10, "héllo"}, + {"unicode_truncate", "日本語のテキスト", 5, "日本..."}, + {"emoji", "🔥🔥🔥🔥🔥", 4, "🔥..."}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := truncate(tt.input, tt.maxLen) + if got != tt.want { + t.Errorf("truncate(%q, %d) = %q, want %q", + tt.input, tt.maxLen, got, tt.want) + } + }) + } +} diff --git a/internal/notifiers/slack.go b/internal/notifiers/slack.go index 788e610..2d895f4 100644 --- a/internal/notifiers/slack.go +++ b/internal/notifiers/slack.go @@ -8,10 +8,11 @@ import ( // SlackChannels holds channel IDs for different notification types. // empty values fall back to the default channel. type SlackChannels struct { - Default string - PRBypass string - OktaSync string - OrphanedUsers string + Default string + PRBypass string + OktaSync string + OrphanedUsers string + SecurityAlerts string } // SlackMessages holds optional custom messages for different notification diff --git a/internal/notifiers/slack_messages.go b/internal/notifiers/slack_messages.go index 2500268..99a221f 100644 --- a/internal/notifiers/slack_messages.go +++ b/internal/notifiers/slack_messages.go @@ -3,6 +3,7 @@ package notifiers import ( "context" "fmt" + "sort" "github.com/cockroachdb/errors" "github.com/cruxstack/github-ops-app/internal/domain" @@ -283,3 +284,139 @@ func (s *SlackNotifier) NotifyOrphanedUsers(ctx context.Context, report *domain. return nil } + +// maxReposInNotification limits the number of repositories shown in a +// single Slack message to avoid exceeding block limits. +const maxReposInNotification = 15 + +// NotifySecurityAlerts sends a Slack notification summarizing open +// security alerts across the organization. +func (s *SlackNotifier) NotifySecurityAlerts(ctx context.Context, report *domain.SecurityAlertsReport, githubOrg string) error { + if report == nil || !report.HasAlerts() { + return nil + } + + blocks := []slack.Block{ + slack.NewHeaderBlock( + slack.NewTextBlockObject("plain_text", + "Security Alerts Report", false, false), + ), + slack.NewSectionBlock( + slack.NewTextBlockObject("mrkdwn", + fmt.Sprintf( + "*%d* open alert(s) across *%d* repository(s) in `%s`", + report.TotalAlerts, + report.RepoCount(), + githubOrg, + ), + false, false), + nil, nil, + ), + } + + // repo list sorted by most alerts first + repos := sortedReposByAlertCount(report.AlertsByRepo) + + repoLines := "" + for i, repo := range repos { + if i >= maxReposInNotification { + repoLines += fmt.Sprintf( + "\n_…and %d more_", len(repos)-i) + break + } + + alerts := report.AlertsByRepo[repo] + highest := highestSeverity(alerts) + secURL := fmt.Sprintf( + "https://github.com/%s/security", repo) + + repoLines += fmt.Sprintf("• <%s|%s> — %d alert(s), %s severity\n", + secURL, repo, len(alerts), highest) + } + + blocks = append(blocks, slack.NewSectionBlock( + slack.NewTextBlockObject("mrkdwn", repoLines, false, false), + nil, nil, + )) + + if report.HasErrors() { + errText := "*Errors:*\n" + for _, e := range report.Errors { + errText += fmt.Sprintf("• %s\n", e) + } + blocks = append(blocks, slack.NewSectionBlock( + slack.NewTextBlockObject("mrkdwn", errText, + false, false), + nil, nil, + )) + } + + blocks = append(blocks, slack.NewContextBlock("", + slack.NewTextBlockObject("mrkdwn", + fmt.Sprintf( + "Filtered to open alerts older than %d days · severity %s or above", + report.MinAgeDays, + report.MinSeverity, + ), + false, false), + )) + + channel := s.channelFor(s.channels.SecurityAlerts) + _, _, err := s.client.PostMessageContext( + ctx, + channel, + slack.MsgOptionBlocks(blocks...), + slack.MsgOptionText( + fmt.Sprintf("security alerts: %d open alerts across %d repos in %s", + report.TotalAlerts, report.RepoCount(), githubOrg), + false), + ) + + if err != nil { + return errors.Wrap(err, + "failed to post security alerts notification to slack") + } + + return nil +} + +// sortedReposByAlertCount returns repo names sorted by descending +// alert count, with alphabetical tiebreak. +func sortedReposByAlertCount(alertsByRepo map[string][]domain.SecurityAlert) []string { + repos := make([]string, 0, len(alertsByRepo)) + for repo := range alertsByRepo { + repos = append(repos, repo) + } + sort.Slice(repos, func(i, j int) bool { + ci := len(alertsByRepo[repos[i]]) + cj := len(alertsByRepo[repos[j]]) + if ci != cj { + return ci > cj + } + return repos[i] < repos[j] + }) + return repos +} + +// highestSeverity returns the highest severity label among alerts. +func highestSeverity(alerts []domain.SecurityAlert) string { + best := 0 + for _, a := range alerts { + if r := severityRank[a.Severity]; r > best { + best = r + } + } + for _, sev := range []string{"critical", "high", "medium", "low"} { + if severityRank[sev] == best { + return sev + } + } + return "unknown" +} + +var severityRank = map[string]int{ + "critical": 4, + "high": 3, + "medium": 2, + "low": 1, +} diff --git a/internal/notifiers/slack_messages_test.go b/internal/notifiers/slack_messages_test.go index 18fc39d..48421c5 100644 --- a/internal/notifiers/slack_messages_test.go +++ b/internal/notifiers/slack_messages_test.go @@ -213,6 +213,124 @@ func TestNotifyOktaSync_WithErrors(t *testing.T) { } } +func TestNotifySecurityAlerts_NilReport(t *testing.T) { + notifier := NewSlackNotifier("xoxb-test", SlackChannels{Default: "C"}, SlackMessages{}) + + err := notifier.NotifySecurityAlerts(context.Background(), nil, "org") + if err != nil { + t.Fatalf("expected nil for nil report, got: %v", err) + } +} + +func TestNotifySecurityAlerts_NoAlerts(t *testing.T) { + notifier := NewSlackNotifier("xoxb-test", SlackChannels{Default: "C"}, SlackMessages{}) + + report := &domain.SecurityAlertsReport{ + TotalAlerts: 0, + AlertsByRepo: map[string][]domain.SecurityAlert{}, + } + err := notifier.NotifySecurityAlerts(context.Background(), report, "org") + if err != nil { + t.Fatalf("expected nil for empty report, got: %v", err) + } +} + +func TestNotifySecurityAlerts_Success(t *testing.T) { + srv, messages := slackTestServer(t) + defer srv.Close() + + notifier := NewSlackNotifierWithAPIURL( + "xoxb-test", + SlackChannels{Default: "C_DEFAULT", SecurityAlerts: "C_SEC"}, + SlackMessages{}, + srv.URL+"/", + ) + + report := &domain.SecurityAlertsReport{ + MinAgeDays: 30, + MinSeverity: "high", + TotalAlerts: 3, + AlertsByRepo: map[string][]domain.SecurityAlert{ + "org/api": { + {Type: "dependabot", Severity: "critical", Number: 1}, + {Type: "code_scanning", Severity: "high", Number: 5}, + }, + "org/web": { + {Type: "secret_scanning", Severity: "high", Number: 3}, + }, + }, + } + + err := notifier.NotifySecurityAlerts(context.Background(), report, "my-org") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + msg := <-messages + if msg == nil { + t.Fatal("expected a message to be posted") + } +} + +func TestNotifySecurityAlerts_WithErrors(t *testing.T) { + srv, messages := slackTestServer(t) + defer srv.Close() + + notifier := NewSlackNotifierWithAPIURL( + "xoxb-test", + SlackChannels{Default: "C"}, + SlackMessages{}, + srv.URL+"/", + ) + + report := &domain.SecurityAlertsReport{ + MinAgeDays: 14, + MinSeverity: "medium", + TotalAlerts: 1, + AlertsByRepo: map[string][]domain.SecurityAlert{ + "org/repo": { + {Type: "dependabot", Severity: "high", Number: 10}, + }, + }, + Errors: []string{"failed to fetch code scanning alerts"}, + } + + err := notifier.NotifySecurityAlerts(context.Background(), report, "org") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + msg := <-messages + if msg == nil { + t.Fatal("expected a message") + } +} + +func TestChannelFor_SecurityAlerts(t *testing.T) { + n := &SlackNotifier{ + channels: SlackChannels{ + Default: "C_DEFAULT", + SecurityAlerts: "C_SEC", + }, + } + + if got := n.channelFor(n.channels.SecurityAlerts); got != "C_SEC" { + t.Errorf("expected C_SEC, got %s", got) + } +} + +func TestChannelFor_SecurityAlertsFallback(t *testing.T) { + n := &SlackNotifier{ + channels: SlackChannels{ + Default: "C_DEFAULT", + }, + } + + if got := n.channelFor(n.channels.SecurityAlerts); got != "C_DEFAULT" { + t.Errorf("expected C_DEFAULT fallback, got %s", got) + } +} + func TestChannelFor_CustomChannels(t *testing.T) { n := &SlackNotifier{ channels: SlackChannels{ diff --git a/internal/sync/sync_test.go b/internal/sync/sync_test.go index 59c2576..6e68a8d 100644 --- a/internal/sync/sync_test.go +++ b/internal/sync/sync_test.go @@ -76,6 +76,10 @@ func (m *mockGitHubClient) GetAppSlug(ctx context.Context) (string, error) { return "test-app", nil } +func (m *mockGitHubClient) ListSecurityAlerts(ctx context.Context, minAgeDays int, minSeverity string) (*domain.SecurityAlertsReport, error) { + return &domain.SecurityAlertsReport{AlertsByRepo: map[string][]domain.SecurityAlert{}}, nil +} + func (m *mockGitHubClient) GetOrg() string { return "test-org" }