Skip to content

Commit e6b618e

Browse files
elireismanelireisman
authoredJun 4, 2024··
Merge pull request #767 from actions/max-comment-length
Fix the max comment length issue
·
v4.6.0v4
2 parents 8d625cd + 3c42649 commit e6b618e

8 files changed

+201
-71
lines changed
 

‎__tests__/fixtures/config-allow-sample.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
fail_on_severity: critical
22
allow_licenses:
3-
- "BSD"
4-
- "GPL 2"
3+
- 'BSD'
4+
- 'GPL 2'

‎__tests__/fixtures/inline-license-config-sample.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
allow-licenses: "MIT, GPL-2.0-only"
1+
allow-licenses: 'MIT, GPL-2.0-only'

‎__tests__/summary.test.ts

+75-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {expect, jest, test} from '@jest/globals'
2-
import {Changes, ConfigurationOptions, Scorecard} from '../src/schemas'
2+
import {Change, Changes, ConfigurationOptions, Scorecard} from '../src/schemas'
33
import * as summary from '../src/summary'
44
import * as core from '@actions/core'
55
import {createTestChange} from './fixtures/create-test-change'
@@ -109,6 +109,80 @@ test('prints headline as h1', () => {
109109
expect(text).toContain('<h1>Dependency Review</h1>')
110110
})
111111

112+
test('returns minimal summary in case the core.summary is too large for a PR comment', () => {
113+
let changes: Changes = [
114+
createTestChange({name: 'lodash', version: '1.2.3'}),
115+
createTestChange({name: 'colors', version: '2.3.4'}),
116+
createTestChange({name: '@foo/bar', version: '*'})
117+
]
118+
119+
let minSummary: string = summary.addSummaryToSummary(
120+
changes,
121+
emptyInvalidLicenseChanges,
122+
emptyChanges,
123+
scorecard,
124+
defaultConfig
125+
)
126+
127+
// side effect DR report into core.summary as happens in main.ts
128+
summary.addScannedDependencies(changes)
129+
const text = core.summary.stringify()
130+
131+
expect(text).toContain('<h1>Dependency Review</h1>')
132+
expect(minSummary).toContain('# Dependency Review')
133+
134+
expect(text).toContain('❌ 3 vulnerable package(s)')
135+
expect(text).not.toContain('* ❌ 3 vulnerable package(s)')
136+
expect(text).toContain('lodash')
137+
expect(text).toContain('colors')
138+
expect(text).toContain('@foo/bar')
139+
140+
expect(minSummary).toContain('* ❌ 3 vulnerable package(s)')
141+
expect(minSummary).not.toContain('lodash')
142+
expect(minSummary).not.toContain('colors')
143+
expect(minSummary).not.toContain('@foo/bar')
144+
145+
expect(text.length).toBeGreaterThan(minSummary.length)
146+
})
147+
148+
test('returns minimal summary formatted for posting as a PR comment', () => {
149+
const OLD_ENV = process.env
150+
151+
let changes: Changes = [
152+
createTestChange({name: 'lodash', version: '1.2.3'}),
153+
createTestChange({name: 'colors', version: '2.3.4'}),
154+
createTestChange({name: '@foo/bar', version: '*'})
155+
]
156+
157+
process.env.GITHUB_SERVER_URL = 'https://github.com'
158+
process.env.GITHUB_REPOSITORY = 'owner/repo'
159+
process.env.GITHUB_RUN_ID = 'abc-123-xyz'
160+
161+
let minSummary: string = summary.addSummaryToSummary(
162+
changes,
163+
emptyInvalidLicenseChanges,
164+
emptyChanges,
165+
scorecard,
166+
defaultConfig
167+
)
168+
169+
process.env = OLD_ENV
170+
171+
// note: no Actions context values in unit test env
172+
const expected = `
173+
# Dependency Review
174+
The following issues were found:
175+
* ❌ 3 vulnerable package(s)
176+
* ✅ 0 package(s) with incompatible licenses
177+
* ✅ 0 package(s) with invalid SPDX license definitions
178+
* ✅ 0 package(s) with unknown licenses.
179+
180+
[View full job summary](https://github.com/owner/repo/actions/runs/abc-123-xyz)
181+
`.trim()
182+
183+
expect(minSummary).toEqual(expected)
184+
})
185+
112186
test('only includes "No vulnerabilities or license issues found"-message if both are configured and nothing was found', () => {
113187
summary.addSummaryToSummary(
114188
emptyChanges,

‎dist/index.js

+38-15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/index.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/comment-pr.ts

+3-5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import * as retry from '@octokit/plugin-retry'
55
import {RequestError} from '@octokit/request-error'
66
import {ConfigurationOptions} from './schemas'
77

8+
export const MAX_COMMENT_LENGTH = 65536
9+
810
const retryingOctokit = githubUtils.GitHub.plugin(retry.retry)
911
const octo = new retryingOctokit(
1012
githubUtils.getOctokitOptions(core.getInput('repo-token', {required: true}))
@@ -14,13 +16,9 @@ const octo = new retryingOctokit(
1416
const COMMENT_MARKER = '<!-- dependency-review-pr-comment-marker -->'
1517

1618
export async function commentPr(
17-
summary: typeof core.summary,
19+
commentContent: string,
1820
config: ConfigurationOptions
1921
): Promise<void> {
20-
const commentContent = summary.stringify()
21-
22-
core.setOutput('comment-content', commentContent)
23-
2422
if (
2523
!(
2624
config.comment_summary_in_pr === 'always' ||

‎src/main.ts

+17-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import * as summary from './summary'
2222
import {getRefs} from './git-refs'
2323

2424
import {groupDependenciesByManifest} from './utils'
25-
import {commentPr} from './comment-pr'
25+
import {commentPr, MAX_COMMENT_LENGTH} from './comment-pr'
2626
import {getDeniedChanges} from './deny'
2727

2828
async function delay(ms: number): Promise<void> {
@@ -127,7 +127,7 @@ async function run(): Promise<void> {
127127

128128
const scorecard = await getScorecardLevels(filteredChanges)
129129

130-
summary.addSummaryToSummary(
130+
const minSummary = summary.addSummaryToSummary(
131131
vulnerableChanges,
132132
invalidLicenseChanges,
133133
deniedChanges,
@@ -166,7 +166,21 @@ async function run(): Promise<void> {
166166
core.setOutput('dependency-changes', JSON.stringify(changes))
167167
summary.addScannedDependencies(changes)
168168
printScannedDependencies(changes)
169-
await commentPr(core.summary, config)
169+
170+
// include full summary in output; Actions will truncate if oversized
171+
let rendered = core.summary.stringify()
172+
core.setOutput('comment-content', rendered)
173+
174+
// if the summary is oversized, replace with minimal version
175+
if (rendered.length >= MAX_COMMENT_LENGTH) {
176+
core.debug(
177+
'The comment was too big for the GitHub API. Falling back on a minimum comment'
178+
)
179+
rendered = minSummary
180+
}
181+
182+
// update the PR comment if needed with the right-sized summary
183+
await commentPr(rendered, config)
170184
} catch (error) {
171185
if (error instanceof RequestError && error.status === 404) {
172186
core.setFailed(

‎src/summary.ts

+64-43
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,23 @@ const icons = {
1010
warning: '⚠️'
1111
}
1212

13+
// generates the DR report summmary and caches it to the Action's core.summary.
14+
// returns the DR summary string, ready to be posted as a PR comment if the
15+
// final DR report is too large
1316
export function addSummaryToSummary(
1417
vulnerableChanges: Changes,
1518
invalidLicenseChanges: InvalidLicenseChanges,
1619
deniedChanges: Changes,
1720
scorecard: Scorecard,
1821
config: ConfigurationOptions
19-
): void {
22+
): string {
23+
const out: string[] = []
24+
2025
const scorecardWarnings = countScorecardWarnings(scorecard, config)
2126
const licenseIssues = countLicenseIssues(invalidLicenseChanges)
2227

2328
core.summary.addHeading('Dependency Review', 1)
29+
out.push('# Dependency Review')
2430

2531
if (
2632
vulnerableChanges.length === 0 &&
@@ -33,54 +39,69 @@ export function addSummaryToSummary(
3339
config.license_check ? 'license issues' : '',
3440
config.show_openssf_scorecard ? 'OpenSSF Scorecard issues' : ''
3541
]
42+
43+
let msg = ''
3644
if (issueTypes.filter(Boolean).length === 0) {
37-
core.summary.addRaw(`${icons.check} No issues found.`)
45+
msg = `${icons.check} No issues found.`
3846
} else {
39-
core.summary.addRaw(
40-
`${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`
41-
)
47+
msg = `${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`
4248
}
4349

44-
return
50+
core.summary.addRaw(msg)
51+
out.push(msg)
52+
return out.join('\n')
4553
}
4654

47-
core.summary
48-
.addRaw('The following issues were found:')
49-
.addList([
50-
...(config.vulnerability_check
51-
? [
52-
`${checkOrFailIcon(vulnerableChanges.length)} ${
53-
vulnerableChanges.length
54-
} vulnerable package(s)`
55-
]
56-
: []),
57-
...(config.license_check
58-
? [
59-
`${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${
60-
invalidLicenseChanges.forbidden.length
61-
} package(s) with incompatible licenses`,
62-
`${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${
63-
invalidLicenseChanges.unresolved.length
64-
} package(s) with invalid SPDX license definitions`,
65-
`${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${
66-
invalidLicenseChanges.unlicensed.length
67-
} package(s) with unknown licenses.`
68-
]
69-
: []),
70-
...(deniedChanges.length > 0
71-
? [
72-
`${checkOrFailIcon(deniedChanges.length)} ${
73-
deniedChanges.length
74-
} package(s) denied.`
75-
]
76-
: []),
77-
...(config.show_openssf_scorecard && scorecardWarnings > 0
78-
? [
79-
`${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.`
80-
]
81-
: [])
82-
])
83-
.addRaw('See the Details below.')
55+
const foundIssuesHeader = 'The following issues were found:'
56+
core.summary.addRaw(foundIssuesHeader)
57+
out.push(foundIssuesHeader)
58+
59+
const summaryList: string[] = [
60+
...(config.vulnerability_check
61+
? [
62+
`${checkOrFailIcon(vulnerableChanges.length)} ${
63+
vulnerableChanges.length
64+
} vulnerable package(s)`
65+
]
66+
: []),
67+
...(config.license_check
68+
? [
69+
`${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${
70+
invalidLicenseChanges.forbidden.length
71+
} package(s) with incompatible licenses`,
72+
`${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${
73+
invalidLicenseChanges.unresolved.length
74+
} package(s) with invalid SPDX license definitions`,
75+
`${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${
76+
invalidLicenseChanges.unlicensed.length
77+
} package(s) with unknown licenses.`
78+
]
79+
: []),
80+
...(deniedChanges.length > 0
81+
? [
82+
`${checkOrWarnIcon(deniedChanges.length)} ${
83+
deniedChanges.length
84+
} package(s) denied.`
85+
]
86+
: []),
87+
...(config.show_openssf_scorecard && scorecardWarnings > 0
88+
? [
89+
`${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.`
90+
]
91+
: [])
92+
]
93+
94+
core.summary.addList(summaryList)
95+
for (const line of summaryList) {
96+
out.push(`* ${line}`)
97+
}
98+
99+
core.summary.addRaw('See the Details below.')
100+
out.push(
101+
`\n[View full job summary](${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID})`
102+
)
103+
104+
return out.join('\n')
84105
}
85106

86107
function countScorecardWarnings(

0 commit comments

Comments
 (0)
Please sign in to comment.