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

fix: add summary comment on failure when warn-only: true #827

Merged
merged 4 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
38 changes: 22 additions & 16 deletions dist/index.js

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: 3 additions & 3 deletions src/comment-pr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ const COMMENT_MARKER = '<!-- dependency-review-pr-comment-marker -->'

export async function commentPr(
commentContent: string,
config: ConfigurationOptions
config: ConfigurationOptions,
failureCount: number
): Promise<void> {
if (
!(
config.comment_summary_in_pr === 'always' ||
(config.comment_summary_in_pr === 'on-failure' &&
process.exitCode === core.ExitCode.Failure)
(config.comment_summary_in_pr === 'on-failure' && failureCount > 0)
)
) {
return
Expand Down
6 changes: 0 additions & 6 deletions src/deny.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ export async function getDeniedChanges(
}
}

if (hasDeniedPackage) {
core.setFailed('Dependency review detected denied packages.')
} else {
core.info('Dependency review did not detect any denied packages')
}

return changesDenied
}

Expand Down
34 changes: 24 additions & 10 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,23 +141,25 @@ async function run(): Promise<void> {
summary.addSnapshotWarnings(config, snapshot_warnings)
}

let failureCount = 0;

if (config.vulnerability_check) {
core.setOutput('vulnerable-changes', JSON.stringify(vulnerableChanges))
summary.addChangeVulnerabilitiesToSummary(vulnerableChanges, minSeverity)
printVulnerabilitiesBlock(vulnerableChanges, minSeverity, warnOnly)
failureCount += printVulnerabilitiesBlock(vulnerableChanges, minSeverity, warnOnly)
}
if (config.license_check) {
core.setOutput(
'invalid-license-changes',
JSON.stringify(invalidLicenseChanges)
)
summary.addLicensesToSummary(invalidLicenseChanges, config)
printLicensesBlock(invalidLicenseChanges, warnOnly)
failureCount += printLicensesBlock(invalidLicenseChanges, warnOnly)
}
if (config.deny_packages || config.deny_groups) {
core.setOutput('denied-changes', JSON.stringify(deniedChanges))
summary.addDeniedToSummary(deniedChanges)
printDeniedDependencies(deniedChanges, config)
failureCount += printDeniedDependencies(deniedChanges, config)
}
if (config.show_openssf_scorecard) {
summary.addScorecardToSummary(scorecard, config)
Expand All @@ -182,7 +184,7 @@ async function run(): Promise<void> {
}

// update the PR comment if needed with the right-sized summary
await commentPr(rendered, config)
await commentPr(rendered, config, failureCount)
} catch (error) {
if (error instanceof RequestError && error.status === 404) {
core.setFailed(
Expand All @@ -208,17 +210,17 @@ function printVulnerabilitiesBlock(
addedChanges: Changes,
minSeverity: Severity,
warnOnly: boolean
): void {
let vulFound = false
): number {
let vulCount = 0
core.group('Vulnerabilities', async () => {
if (addedChanges.length > 0) {
for (const change of addedChanges) {
printChangeVulnerabilities(change)
vulCount += change.vulnerabilities.length;
}
vulFound = true
}

if (vulFound) {
if (vulCount > 0) {
const msg = 'Dependency review detected vulnerable packages.'
if (warnOnly) {
core.warning(msg)
Expand All @@ -231,6 +233,7 @@ function printVulnerabilitiesBlock(
)
}
})
return vulCount
}

function printChangeVulnerabilities(change: Change): void {
Expand All @@ -249,9 +252,11 @@ function printChangeVulnerabilities(change: Change): void {
function printLicensesBlock(
invalidLicenseChanges: Record<string, Changes>,
warnOnly: boolean
): void {
): number {
let failureCount = 0;
core.group('Licenses', async () => {
if (invalidLicenseChanges.forbidden.length > 0) {
failureCount += invalidLicenseChanges.forbidden.length;
core.info('\nThe following dependencies have incompatible licenses:')
printLicensesError(invalidLicenseChanges.forbidden)
const msg = 'Dependency review detected incompatible licenses.'
Expand All @@ -262,6 +267,7 @@ function printLicensesBlock(
}
}
if (invalidLicenseChanges.unresolved.length > 0) {
failureCount += invalidLicenseChanges.unresolved.length;
core.warning(
'\nThe validity of the licenses of the dependencies below could not be determined. Ensure that they are valid SPDX licenses:'
)
Expand All @@ -272,6 +278,7 @@ function printLicensesBlock(
}
printNullLicenses(invalidLicenseChanges.unlicensed)
})
return failureCount;
}

function printLicensesError(changes: Changes): void {
Expand Down Expand Up @@ -373,7 +380,7 @@ function printScannedDependencies(changes: Changes): void {
function printDeniedDependencies(
changes: Changes,
config: ConfigurationOptions
): void {
): number {
core.group('Denied', async () => {
for (const denied of config.deny_packages) {
core.info(`Config: ${denied}`)
Expand All @@ -383,7 +390,14 @@ function printDeniedDependencies(
core.info(`Change: ${change.name}@${change.version} is denied`)
core.info(`Change: ${change.package_url} is denied`)
}

if (changes.length > 0) {
core.setFailed('Dependency review detected denied packages.')
} else {
core.info('Dependency review did not detect any denied packages')
}
})
return changes.length
}

function getScorecardChanges(changes: Changes): Changes {
Expand Down