Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add inputs for manifest_paths and slack_channel #192

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ inputs:
description: 'Microsoft Teams Channel Webhook URL. More info: https://docs.microsoft.com/en-us/microsoftteams/platform/webhooks-and-connectors/how-to/add-incoming-webhook'
slack_webhook:
description: 'Slack Webhook URL. More info: https://api.slack.com/messaging/webhooks'
slack_channel:
description: 'Explicit channel for Slack webhook. If omitted, the channel configured in the webhook is used.'
required: false
default: ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove default? It will make checking easier as it will be undefined. Also, it will be inline with other optional fields which don't have any defaults.

pager_duty_integration_key:
description: 'Pager Duty Integration Key. More info: https://support.pagerduty.com/docs/services-and-integrations'
zenduty_api_key:
Expand Down Expand Up @@ -43,6 +47,8 @@ inputs:
description: 'Comma separated list of severities. E.g. low,medium,high,critical (NO SPACES BETWEEN COMMA AND SEVERITY)'
ecosystem:
description: 'A comma-separated list of ecosystems. If specified, only alerts for these ecosystems will be returned.'
manifest_paths:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Let's call it manifest
  • Let's make it a comma separated list, so it can be passed directly to the function - and let's also update the description to switch to Multi-line instead of comma-separated
    GitHub Docs

description: 'Multi-line list of manifest paths. If specified, only alerts for these manifests will be sent.'
branding:
icon: 'alert-octagon'
color: 'red'
Expand Down
26 changes: 19 additions & 7 deletions dist/index.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think any of these dist/ files should be checked in with PRs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion src/destinations/slack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const createAlertBlock = (alert: Alert): KnownBlock => ({
text: `
*Package:* ${alert.packageName}
*Repository:* ${getFullRepositoryNameFromAlert(alert)}
*Manifest path:* ${alert.manifestPath}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*Manifest path:* ${alert.manifestPath}
*Manifest:* ${alert.manifestPath}

*Vulnerability Version Range:* ${alert.vulnerability?.vulnerableVersionRange}
*Patched Version:* ${alert.vulnerability?.firstPatchedVersion}
*Severity:* ${alert.advisory?.severity}
Expand All @@ -61,9 +62,12 @@ export const validateSlackWebhookUrl = (url: string): boolean => {

export const sendAlertsToSlack = async (
webhookUrl: string,
channel: string,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Channel should be optional?

Suggested change
channel: string,
channel?: string,

alerts: Alert[],
): Promise<void> => {
const webhook = new IncomingWebhook(webhookUrl)
const webhook = new IncomingWebhook(webhookUrl, {
channel,
})
const alertBlocks: KnownBlock[] = []
alerts.forEach((alert) => {
alertBlocks.push(createAlertBlock(alert))
Expand Down
4 changes: 4 additions & 0 deletions src/entities/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export type DependabotAlert =
export interface Alert {
repository: Repository
packageName: string
manifestPath?: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
manifestPath?: string
manifest?: string

advisory?: Advisory
vulnerability?: Vulnerability
createdAt: string
Expand All @@ -25,6 +26,7 @@ export const toRepositoryAlert = (
owner: repositoryOwner,
},
packageName: dependabotAlert.security_vulnerability.package.name || '',
manifestPath: dependabotAlert.dependency.manifest_path,
advisory: dependabotAlert.security_advisory
? toAdvisory(dependabotAlert.security_advisory)
: undefined,
Expand All @@ -43,6 +45,7 @@ export const toOrgAlert = (dependabotOrgAlert: DependabotOrgAlert): Alert => ({
owner: dependabotOrgAlert.repository.owner.login,
},
packageName: dependabotOrgAlert.security_vulnerability.package.name || '',
manifestPath: dependabotOrgAlert.dependency.manifest_path,
advisory: dependabotOrgAlert.security_advisory
? toAdvisory(dependabotOrgAlert.security_advisory)
: undefined,
Expand All @@ -64,6 +67,7 @@ export const toEnterpriseAlert = (
},
packageName:
dependabotEnterpriseAlert.security_vulnerability.package.name || '',
manifestPath: dependabotEnterpriseAlert.dependency.manifest_path,
advisory: dependabotEnterpriseAlert.security_advisory
? toAdvisory(dependabotEnterpriseAlert.security_advisory)
: undefined,
Expand Down
30 changes: 24 additions & 6 deletions src/main.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getInput, setFailed } from '@actions/core'
import { getInput, getMultilineInput, setFailed } from '@actions/core'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use regular getInput for this

import { context } from '@actions/github'

import {
Expand All @@ -23,6 +23,7 @@ async function run(): Promise<void> {
const enterprise = getInput('enterprise')
const microsoftTeamsWebhookUrl = getInput('microsoft_teams_webhook')
const slackWebhookUrl = getInput('slack_webhook')
const slackChannel = getInput('slack_channel')
const pagerDutyIntegrationKey = getInput('pager_duty_integration_key')
const zenDutyApiKey = getInput('zenduty_api_key')
const zenDutyServiceId = getInput('zenduty_service_id')
Expand All @@ -39,12 +40,19 @@ async function run(): Promise<void> {
const count = parseInt(getInput('count'))
const severity = getInput('severity')
const ecosystem = getInput('ecosystem')
const manifestPaths = getMultilineInput('manifest_paths')

let alerts: Alert[] = []
let fetchedAlerts: Alert[] = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't need to change any of this

if (org) {
alerts = await fetchOrgAlerts(token, org, severity, ecosystem, count)
fetchedAlerts = await fetchOrgAlerts(
token,
org,
severity,
ecosystem,
count,
)
} else if (enterprise) {
alerts = await fetchEnterpriseAlerts(
fetchedAlerts = await fetchEnterpriseAlerts(
token,
org,
severity,
Expand All @@ -53,7 +61,7 @@ async function run(): Promise<void> {
)
} else {
const { owner, repo } = context.repo
alerts = await fetchRepositoryAlerts(
fetchedAlerts = await fetchRepositoryAlerts(
token,
repo,
owner,
Expand All @@ -62,6 +70,16 @@ async function run(): Promise<void> {
count,
)
}
const alerts =
manifestPaths.length > 0
? fetchedAlerts.filter(
(alert) =>
alert.manifestPath &&
manifestPaths.some(
(manifestPath) => manifestPath === alert.manifestPath,
),
)
: fetchedAlerts
Comment on lines +73 to +82
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't need this if you pass manifest to the function that fetches alerts - it will be passed to the octokit method and it will filter the alerts based on that criteria

if (alerts.length > 0) {
if (microsoftTeamsWebhookUrl) {
await sendAlertsToMicrosoftTeams(microsoftTeamsWebhookUrl, alerts)
Expand All @@ -70,7 +88,7 @@ async function run(): Promise<void> {
if (!validateSlackWebhookUrl(slackWebhookUrl)) {
setFailed(new Error('Invalid Slack Webhook URL'))
} else {
await sendAlertsToSlack(slackWebhookUrl, alerts)
await sendAlertsToSlack(slackWebhookUrl, slackChannel, alerts)
}
}
if (pagerDutyIntegrationKey) {
Expand Down