Merge pull request #767 from actions/max-comment-length

Fix the max comment length issue
This commit is contained in:
Eli Reisman
2024-06-04 13:03:31 -07:00
committed by GitHub
8 changed files with 201 additions and 71 deletions

View File

@@ -1,4 +1,4 @@
fail_on_severity: critical
allow_licenses:
- "BSD"
- "GPL 2"
- 'BSD'
- 'GPL 2'

View File

@@ -1 +1 @@
allow-licenses: "MIT, GPL-2.0-only"
allow-licenses: 'MIT, GPL-2.0-only'

View File

@@ -1,5 +1,5 @@
import {expect, jest, test} from '@jest/globals'
import {Changes, ConfigurationOptions, Scorecard} from '../src/schemas'
import {Change, Changes, ConfigurationOptions, Scorecard} from '../src/schemas'
import * as summary from '../src/summary'
import * as core from '@actions/core'
import {createTestChange} from './fixtures/create-test-change'
@@ -109,6 +109,80 @@ test('prints headline as h1', () => {
expect(text).toContain('<h1>Dependency Review</h1>')
})
test('returns minimal summary in case the core.summary is too large for a PR comment', () => {
let changes: Changes = [
createTestChange({name: 'lodash', version: '1.2.3'}),
createTestChange({name: 'colors', version: '2.3.4'}),
createTestChange({name: '@foo/bar', version: '*'})
]
let minSummary: string = summary.addSummaryToSummary(
changes,
emptyInvalidLicenseChanges,
emptyChanges,
scorecard,
defaultConfig
)
// side effect DR report into core.summary as happens in main.ts
summary.addScannedDependencies(changes)
const text = core.summary.stringify()
expect(text).toContain('<h1>Dependency Review</h1>')
expect(minSummary).toContain('# Dependency Review')
expect(text).toContain('❌ 3 vulnerable package(s)')
expect(text).not.toContain('* ❌ 3 vulnerable package(s)')
expect(text).toContain('lodash')
expect(text).toContain('colors')
expect(text).toContain('@foo/bar')
expect(minSummary).toContain('* ❌ 3 vulnerable package(s)')
expect(minSummary).not.toContain('lodash')
expect(minSummary).not.toContain('colors')
expect(minSummary).not.toContain('@foo/bar')
expect(text.length).toBeGreaterThan(minSummary.length)
})
test('returns minimal summary formatted for posting as a PR comment', () => {
const OLD_ENV = process.env
let changes: Changes = [
createTestChange({name: 'lodash', version: '1.2.3'}),
createTestChange({name: 'colors', version: '2.3.4'}),
createTestChange({name: '@foo/bar', version: '*'})
]
process.env.GITHUB_SERVER_URL = 'https://github.com'
process.env.GITHUB_REPOSITORY = 'owner/repo'
process.env.GITHUB_RUN_ID = 'abc-123-xyz'
let minSummary: string = summary.addSummaryToSummary(
changes,
emptyInvalidLicenseChanges,
emptyChanges,
scorecard,
defaultConfig
)
process.env = OLD_ENV
// note: no Actions context values in unit test env
const expected = `
# Dependency Review
The following issues were found:
* ❌ 3 vulnerable package(s)
* ✅ 0 package(s) with incompatible licenses
* ✅ 0 package(s) with invalid SPDX license definitions
* ✅ 0 package(s) with unknown licenses.
[View full job summary](https://github.com/owner/repo/actions/runs/abc-123-xyz)
`.trim()
expect(minSummary).toEqual(expected)
})
test('only includes "No vulnerabilities or license issues found"-message if both are configured and nothing was found', () => {
summary.addSummaryToSummary(
emptyChanges,

53
dist/index.js generated vendored
View File

@@ -46,20 +46,19 @@ var __asyncValues = (this && this.__asyncValues) || function (o) {
function settle(resolve, reject, d, v) { Promise.resolve(v).then(function(v) { resolve({ value: v, done: d }); }, reject); }
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.commentPr = void 0;
exports.commentPr = exports.MAX_COMMENT_LENGTH = void 0;
const github = __importStar(__nccwpck_require__(5438));
const core = __importStar(__nccwpck_require__(2186));
const githubUtils = __importStar(__nccwpck_require__(3030));
const retry = __importStar(__nccwpck_require__(6298));
const request_error_1 = __nccwpck_require__(537);
exports.MAX_COMMENT_LENGTH = 65536;
const retryingOctokit = githubUtils.GitHub.plugin(retry.retry);
const octo = new retryingOctokit(githubUtils.getOctokitOptions(core.getInput('repo-token', { required: true })));
// Comment Marker to identify an existing comment to update, so we don't spam the PR with comments
const COMMENT_MARKER = '<!-- dependency-review-pr-comment-marker -->';
function commentPr(summary, config) {
function commentPr(commentContent, config) {
return __awaiter(this, void 0, void 0, function* () {
const commentContent = summary.stringify();
core.setOutput('comment-content', commentContent);
if (!(config.comment_summary_in_pr === 'always' ||
(config.comment_summary_in_pr === 'on-failure' &&
process.exitCode === core.ExitCode.Failure))) {
@@ -648,7 +647,7 @@ function run() {
core.debug(`Config Deny Packages: ${JSON.stringify(config)}`);
const deniedChanges = yield (0, deny_1.getDeniedChanges)(filteredChanges, config.deny_packages, config.deny_groups);
const scorecard = yield (0, scorecard_1.getScorecardLevels)(filteredChanges);
summary.addSummaryToSummary(vulnerableChanges, invalidLicenseChanges, deniedChanges, scorecard, config);
const minSummary = summary.addSummaryToSummary(vulnerableChanges, invalidLicenseChanges, deniedChanges, scorecard, config);
if (snapshot_warnings) {
summary.addSnapshotWarnings(config, snapshot_warnings);
}
@@ -675,7 +674,16 @@ function run() {
core.setOutput('dependency-changes', JSON.stringify(changes));
summary.addScannedDependencies(changes);
printScannedDependencies(changes);
yield (0, comment_pr_1.commentPr)(core.summary, config);
// include full summary in output; Actions will truncate if oversized
let rendered = core.summary.stringify();
core.setOutput('comment-content', rendered);
// if the summary is oversized, replace with minimal version
if (rendered.length >= comment_pr_1.MAX_COMMENT_LENGTH) {
core.debug('The comment was too big for the GitHub API. Falling back on a minimum comment');
rendered = minSummary;
}
// update the PR comment if needed with the right-sized summary
yield (0, comment_pr_1.commentPr)(rendered, config);
}
catch (error) {
if (error instanceof request_error_1.RequestError && error.status === 404) {
@@ -1313,10 +1321,15 @@ const icons = {
cross: '❌',
warning: '⚠️'
};
// generates the DR report summmary and caches it to the Action's core.summary.
// returns the DR summary string, ready to be posted as a PR comment if the
// final DR report is too large
function addSummaryToSummary(vulnerableChanges, invalidLicenseChanges, deniedChanges, scorecard, config) {
const out = [];
const scorecardWarnings = countScorecardWarnings(scorecard, config);
const licenseIssues = countLicenseIssues(invalidLicenseChanges);
core.summary.addHeading('Dependency Review', 1);
out.push('# Dependency Review');
if (vulnerableChanges.length === 0 &&
licenseIssues === 0 &&
deniedChanges.length === 0 &&
@@ -1326,17 +1339,21 @@ function addSummaryToSummary(vulnerableChanges, invalidLicenseChanges, deniedCha
config.license_check ? 'license issues' : '',
config.show_openssf_scorecard ? 'OpenSSF Scorecard issues' : ''
];
let msg = '';
if (issueTypes.filter(Boolean).length === 0) {
core.summary.addRaw(`${icons.check} No issues found.`);
msg = `${icons.check} No issues found.`;
}
else {
core.summary.addRaw(`${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`);
msg = `${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`;
}
return;
core.summary.addRaw(msg);
out.push(msg);
return out.join('\n');
}
core.summary
.addRaw('The following issues were found:')
.addList([
const foundIssuesHeader = 'The following issues were found:';
core.summary.addRaw(foundIssuesHeader);
out.push(foundIssuesHeader);
const summaryList = [
...(config.vulnerability_check
? [
`${checkOrFailIcon(vulnerableChanges.length)} ${vulnerableChanges.length} vulnerable package(s)`
@@ -1351,7 +1368,7 @@ function addSummaryToSummary(vulnerableChanges, invalidLicenseChanges, deniedCha
: []),
...(deniedChanges.length > 0
? [
`${checkOrFailIcon(deniedChanges.length)} ${deniedChanges.length} package(s) denied.`
`${checkOrWarnIcon(deniedChanges.length)} ${deniedChanges.length} package(s) denied.`
]
: []),
...(config.show_openssf_scorecard && scorecardWarnings > 0
@@ -1359,8 +1376,14 @@ function addSummaryToSummary(vulnerableChanges, invalidLicenseChanges, deniedCha
`${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.`
]
: [])
])
.addRaw('See the Details below.');
];
core.summary.addList(summaryList);
for (const line of summaryList) {
out.push(`* ${line}`);
}
core.summary.addRaw('See the Details below.');
out.push(`\n[View full job summary](${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID})`);
return out.join('\n');
}
exports.addSummaryToSummary = addSummaryToSummary;
function countScorecardWarnings(scorecard, config) {

2
dist/index.js.map generated vendored

File diff suppressed because one or more lines are too long

View File

@@ -5,6 +5,8 @@ import * as retry from '@octokit/plugin-retry'
import {RequestError} from '@octokit/request-error'
import {ConfigurationOptions} from './schemas'
export const MAX_COMMENT_LENGTH = 65536
const retryingOctokit = githubUtils.GitHub.plugin(retry.retry)
const octo = new retryingOctokit(
githubUtils.getOctokitOptions(core.getInput('repo-token', {required: true}))
@@ -14,13 +16,9 @@ const octo = new retryingOctokit(
const COMMENT_MARKER = '<!-- dependency-review-pr-comment-marker -->'
export async function commentPr(
summary: typeof core.summary,
commentContent: string,
config: ConfigurationOptions
): Promise<void> {
const commentContent = summary.stringify()
core.setOutput('comment-content', commentContent)
if (
!(
config.comment_summary_in_pr === 'always' ||

View File

@@ -22,7 +22,7 @@ import * as summary from './summary'
import {getRefs} from './git-refs'
import {groupDependenciesByManifest} from './utils'
import {commentPr} from './comment-pr'
import {commentPr, MAX_COMMENT_LENGTH} from './comment-pr'
import {getDeniedChanges} from './deny'
async function delay(ms: number): Promise<void> {
@@ -127,7 +127,7 @@ async function run(): Promise<void> {
const scorecard = await getScorecardLevels(filteredChanges)
summary.addSummaryToSummary(
const minSummary = summary.addSummaryToSummary(
vulnerableChanges,
invalidLicenseChanges,
deniedChanges,
@@ -166,7 +166,21 @@ async function run(): Promise<void> {
core.setOutput('dependency-changes', JSON.stringify(changes))
summary.addScannedDependencies(changes)
printScannedDependencies(changes)
await commentPr(core.summary, config)
// include full summary in output; Actions will truncate if oversized
let rendered = core.summary.stringify()
core.setOutput('comment-content', rendered)
// if the summary is oversized, replace with minimal version
if (rendered.length >= MAX_COMMENT_LENGTH) {
core.debug(
'The comment was too big for the GitHub API. Falling back on a minimum comment'
)
rendered = minSummary
}
// update the PR comment if needed with the right-sized summary
await commentPr(rendered, config)
} catch (error) {
if (error instanceof RequestError && error.status === 404) {
core.setFailed(

View File

@@ -10,17 +10,23 @@ const icons = {
warning: '⚠️'
}
// generates the DR report summmary and caches it to the Action's core.summary.
// returns the DR summary string, ready to be posted as a PR comment if the
// final DR report is too large
export function addSummaryToSummary(
vulnerableChanges: Changes,
invalidLicenseChanges: InvalidLicenseChanges,
deniedChanges: Changes,
scorecard: Scorecard,
config: ConfigurationOptions
): void {
): string {
const out: string[] = []
const scorecardWarnings = countScorecardWarnings(scorecard, config)
const licenseIssues = countLicenseIssues(invalidLicenseChanges)
core.summary.addHeading('Dependency Review', 1)
out.push('# Dependency Review')
if (
vulnerableChanges.length === 0 &&
@@ -33,54 +39,69 @@ export function addSummaryToSummary(
config.license_check ? 'license issues' : '',
config.show_openssf_scorecard ? 'OpenSSF Scorecard issues' : ''
]
let msg = ''
if (issueTypes.filter(Boolean).length === 0) {
core.summary.addRaw(`${icons.check} No issues found.`)
msg = `${icons.check} No issues found.`
} else {
core.summary.addRaw(
`${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`
)
msg = `${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`
}
return
core.summary.addRaw(msg)
out.push(msg)
return out.join('\n')
}
core.summary
.addRaw('The following issues were found:')
.addList([
...(config.vulnerability_check
? [
`${checkOrFailIcon(vulnerableChanges.length)} ${
vulnerableChanges.length
} vulnerable package(s)`
]
: []),
...(config.license_check
? [
`${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${
invalidLicenseChanges.forbidden.length
} package(s) with incompatible licenses`,
`${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${
invalidLicenseChanges.unresolved.length
} package(s) with invalid SPDX license definitions`,
`${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${
invalidLicenseChanges.unlicensed.length
} package(s) with unknown licenses.`
]
: []),
...(deniedChanges.length > 0
? [
`${checkOrFailIcon(deniedChanges.length)} ${
deniedChanges.length
} package(s) denied.`
]
: []),
...(config.show_openssf_scorecard && scorecardWarnings > 0
? [
`${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.`
]
: [])
])
.addRaw('See the Details below.')
const foundIssuesHeader = 'The following issues were found:'
core.summary.addRaw(foundIssuesHeader)
out.push(foundIssuesHeader)
const summaryList: string[] = [
...(config.vulnerability_check
? [
`${checkOrFailIcon(vulnerableChanges.length)} ${
vulnerableChanges.length
} vulnerable package(s)`
]
: []),
...(config.license_check
? [
`${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${
invalidLicenseChanges.forbidden.length
} package(s) with incompatible licenses`,
`${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${
invalidLicenseChanges.unresolved.length
} package(s) with invalid SPDX license definitions`,
`${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${
invalidLicenseChanges.unlicensed.length
} package(s) with unknown licenses.`
]
: []),
...(deniedChanges.length > 0
? [
`${checkOrWarnIcon(deniedChanges.length)} ${
deniedChanges.length
} package(s) denied.`
]
: []),
...(config.show_openssf_scorecard && scorecardWarnings > 0
? [
`${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.`
]
: [])
]
core.summary.addList(summaryList)
for (const line of summaryList) {
out.push(`* ${line}`)
}
core.summary.addRaw('See the Details below.')
out.push(
`\n[View full job summary](${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID})`
)
return out.join('\n')
}
function countScorecardWarnings(