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

Conversation

stackptr
Copy link

@stackptr stackptr commented Sep 5, 2024

manifest_paths filters listed vulnerabilities to specific manifests in a repository and slack_channel allows overriding the default channel specified when creating the slack_webhook. These additional inputs allow individual teams at Freckle to receive alerts for vulnerabilities affecting packages for which they are responsible.

Filtering alerts based on manifest_paths is tested working in a private repository. Documentation shows identical fields returned for org-level and enterprise alerts, but I do not have a good way to validate for sure.

Copy link
Member

@kunalnagar kunalnagar left a comment

Choose a reason for hiding this comment

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

@stackptr - thanks for putting up this PR! Have some minor comments.

Also, I've updated the PR title to conventional commit.

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.

@@ -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

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

@@ -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}

@@ -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,

@@ -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

@@ -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

Comment on lines +73 to +82
const alerts =
manifestPaths.length > 0
? fetchedAlerts.filter(
(alert) =>
alert.manifestPath &&
manifestPaths.some(
(manifestPath) => manifestPath === alert.manifestPath,
),
)
: fetchedAlerts
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


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

@kunalnagar kunalnagar changed the title Add inputs for manifest_paths and slack_channel feat: Add inputs for manifest_paths and slack_channel Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants