Skip to content

Commit e482b3d

Browse files
committed
fix(settings): improve nop mode logging to protect config values
When running in nop mode without a check run (e.g. full-sync), the results previously went to info level logs. Since the full diff can contain sensitive configuration values like Actions variables, this now: - Logs the full diff at debug level only - Logs a summary at info level that excludes config values - Shows the count of planned changes and references the debug log
1 parent 427e1c6 commit e482b3d

2 files changed

Lines changed: 11 additions & 5 deletions

File tree

lib/settings.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,10 +204,14 @@ class Settings {
204204
})
205205

206206
// A nop run triggered outside the webhook flow (e.g. the full-sync entrypoint)
207-
// has no check run or repository in its payload, so there is nowhere to post
208-
// the results other than the log.
207+
// can be missing the check run and repository webhook fields in its payload.
208+
// If either is absent there is no check run to report to, so put the plan in
209+
// the log instead. The full diff can contain config values (e.g. Actions
210+
// variables), so it goes to debug; info gets a summary without them.
209211
if (!payload?.check_run || !payload?.repository) {
210-
this.log.info(`Dry-run results:\n${JSON.stringify(this.results, null, 2)}`)
212+
this.log.debug(`Dry-run results:\n${JSON.stringify(this.results, null, 2)}`)
213+
const summary = this.results.map(res => `${res.type} ${res.plugin} ${res.repo}: ${res.action?.msg ?? ''}`).join('\n')
214+
this.log.info(`Dry-run finished with ${this.results.length} planned change(s); the full diff is logged at debug level.\n${summary}`)
211215
return
212216
}
213217

test/unit/lib/settings.test.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,17 +564,19 @@ repository:
564564
repo: 'test/test-repo',
565565
endpoint: '',
566566
body: '',
567-
action: { msg: 'Changes found', additions: {}, modifications: { branch: {} }, deletions: {} }
567+
action: { msg: 'Changes found', additions: {}, modifications: { MY_VAR: { value: 'plain-value' } }, deletions: {} }
568568
}
569569
]
570570
})
571571

572572
describe('in nop mode without a check run in the payload (full sync)', () => {
573-
it('logs the results instead of updating a check run', async () => {
573+
it('logs a summary instead of updating a check run, keeping config values out of info', async () => {
574574
// stubContext.payload only contains `installation`, like a full-sync context
575575
await settings.handleResults()
576576

577577
expect(stubContext.log.info).toHaveBeenCalledWith(expect.stringContaining('Changes found'))
578+
expect(stubContext.log.info).not.toHaveBeenCalledWith(expect.stringContaining('plain-value'))
579+
expect(stubContext.log.debug).toHaveBeenCalledWith(expect.stringContaining('plain-value'))
578580
expect(stubContext.octokit.rest.checks.update).not.toHaveBeenCalled()
579581
})
580582
})

0 commit comments

Comments
 (0)