From e96f25c9acf0806edcb85919dd3d6da0f8960fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20Daba=C5=A1inskas?= Date: Sat, 11 Jul 2026 12:55:01 +0300 Subject: [PATCH 1/4] fix(settings): handle nop mode results when check run is missing In nop mode, when triggered outside the webhook flow (e.g., via the full-sync entrypoint), the payload lacks a check run or repository. Previously, this would cause errors when attempting to update a non-existent check run. Now logs dry-run results to the console instead of attempting to update a check run when `payload.check_run` is undefined. This allows full-sync operations to complete successfully in nop mode while still providing visibility into the results. --- lib/settings.js | 8 +++++++ test/unit/lib/settings.test.js | 44 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/lib/settings.js b/lib/settings.js index fd7ca2693..8cea606c1 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -203,6 +203,14 @@ class Settings { }) }) + // A nop run triggered outside the webhook flow (e.g. the full-sync entrypoint) + // has no check run or repository in its payload, so there is nowhere to post + // the results other than the log. + if (!payload?.check_run) { + this.log.info(`Dry-run results:\n${JSON.stringify(this.results, null, 2)}`) + return + } + let error = false // Different logic const stats = { diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index e102379a6..5e479f4bc 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -549,4 +549,48 @@ repository: expect(mockRepoSync).toHaveBeenCalledTimes(1) }) }) // updateRepos - archived repo skipping + + describe('handleResults', () => { + let settings + + beforeEach(() => { + stubContext.octokit.rest.checks = { update: jest.fn().mockResolvedValue({}) } + stubContext.octokit.rest.issues = { createComment: jest.fn().mockResolvedValue({}) } + settings = new Settings(true, stubContext, mockRepo, {}, mockRef) + settings.results = [ + { + type: 'INFO', + plugin: 'Branches', + repo: 'test/test-repo', + endpoint: '', + body: '', + action: { msg: 'Changes found', additions: {}, modifications: { branch: {} }, deletions: {} } + } + ] + }) + + describe('in nop mode without a check run in the payload (full sync)', () => { + it('logs the results instead of updating a check run', async () => { + // stubContext.payload only contains `installation`, like a full-sync context + await settings.handleResults() + + expect(stubContext.log.info).toHaveBeenCalledWith(expect.stringContaining('Changes found')) + expect(stubContext.octokit.rest.checks.update).not.toHaveBeenCalled() + }) + }) + + describe('in nop mode with a check run in the payload (webhook flow)', () => { + it('completes the check run', async () => { + stubContext.payload.check_run = { id: 42, check_suite: { pull_requests: [{ number: 1 }] } } + stubContext.payload.repository = { owner: { login: 'test' }, name: 'test-repo' } + + await settings.handleResults() + + expect(stubContext.octokit.rest.checks.update).toHaveBeenCalledWith(expect.objectContaining({ + check_run_id: 42, + status: 'completed' + })) + }) + }) + }) // handleResults }) // Settings Tests From 427e1c68e2121538910ed493a93b3dde22a72201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20Daba=C5=A1inskas?= Date: Sat, 11 Jul 2026 13:01:14 +0300 Subject: [PATCH 2/4] fix(settings): handle nop mode when repository is missing from payload The nop mode check was only verifying the presence of `check_run` in the payload, but the full-sync entrypoint can have a check run without a repository object. This caused the code to attempt posting results when it should have logged them instead. Add an additional check for `payload.repository` to ensure both fields are present before attempting to update the check run. --- lib/settings.js | 2 +- test/unit/lib/settings.test.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/settings.js b/lib/settings.js index 8cea606c1..b670d89d7 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -206,7 +206,7 @@ class Settings { // A nop run triggered outside the webhook flow (e.g. the full-sync entrypoint) // has no check run or repository in its payload, so there is nowhere to post // the results other than the log. - if (!payload?.check_run) { + if (!payload?.check_run || !payload?.repository) { this.log.info(`Dry-run results:\n${JSON.stringify(this.results, null, 2)}`) return } diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index 5e479f4bc..dd55eee67 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -579,6 +579,17 @@ repository: }) }) + describe('in nop mode with a check run but no repository in the payload', () => { + it('logs the results instead of updating a check run', async () => { + stubContext.payload.check_run = { id: 42, check_suite: { pull_requests: [{ number: 1 }] } } + + await settings.handleResults() + + expect(stubContext.log.info).toHaveBeenCalledWith(expect.stringContaining('Changes found')) + expect(stubContext.octokit.rest.checks.update).not.toHaveBeenCalled() + }) + }) + describe('in nop mode with a check run in the payload (webhook flow)', () => { it('completes the check run', async () => { stubContext.payload.check_run = { id: 42, check_suite: { pull_requests: [{ number: 1 }] } } From e482b3d01012d8e859cf7e97e4b410eb2aa53cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20Daba=C5=A1inskas?= Date: Sat, 11 Jul 2026 13:09:43 +0300 Subject: [PATCH 3/4] 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 --- lib/settings.js | 10 +++++++--- test/unit/lib/settings.test.js | 6 ++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index b670d89d7..e20912caf 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -204,10 +204,14 @@ class Settings { }) // A nop run triggered outside the webhook flow (e.g. the full-sync entrypoint) - // has no check run or repository in its payload, so there is nowhere to post - // the results other than the log. + // can be missing the check run and repository webhook fields in its payload. + // If either is absent there is no check run to report to, so put the plan in + // the log instead. The full diff can contain config values (e.g. Actions + // variables), so it goes to debug; info gets a summary without them. if (!payload?.check_run || !payload?.repository) { - this.log.info(`Dry-run results:\n${JSON.stringify(this.results, null, 2)}`) + this.log.debug(`Dry-run results:\n${JSON.stringify(this.results, null, 2)}`) + const summary = this.results.map(res => `${res.type} ${res.plugin} ${res.repo}: ${res.action?.msg ?? ''}`).join('\n') + this.log.info(`Dry-run finished with ${this.results.length} planned change(s); the full diff is logged at debug level.\n${summary}`) return } diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index dd55eee67..1ecaa6e56 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -564,17 +564,19 @@ repository: repo: 'test/test-repo', endpoint: '', body: '', - action: { msg: 'Changes found', additions: {}, modifications: { branch: {} }, deletions: {} } + action: { msg: 'Changes found', additions: {}, modifications: { MY_VAR: { value: 'plain-value' } }, deletions: {} } } ] }) describe('in nop mode without a check run in the payload (full sync)', () => { - it('logs the results instead of updating a check run', async () => { + it('logs a summary instead of updating a check run, keeping config values out of info', async () => { // stubContext.payload only contains `installation`, like a full-sync context await settings.handleResults() expect(stubContext.log.info).toHaveBeenCalledWith(expect.stringContaining('Changes found')) + expect(stubContext.log.info).not.toHaveBeenCalledWith(expect.stringContaining('plain-value')) + expect(stubContext.log.debug).toHaveBeenCalledWith(expect.stringContaining('plain-value')) expect(stubContext.octokit.rest.checks.update).not.toHaveBeenCalled() }) }) From 7eb9e28a3aacc3350d355f69747bea04e12248d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20Daba=C5=A1inskas?= Date: Sat, 11 Jul 2026 13:20:27 +0300 Subject: [PATCH 4/4] fix(settings): pass nop mode results as object to protect config values Change the debug log call to pass results as an object instead of a serialized string. This ensures the logger only serializes the results when debug level is actually emitted, preventing unnecessary exposure of sensitive config values in logs. The previous implementation always serialized results to JSON before passing to the logger, which could expose sensitive values even when debug logging was disabled. --- lib/settings.js | 3 ++- test/unit/lib/settings.test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index e20912caf..18f92d21a 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -209,7 +209,8 @@ class Settings { // the log instead. The full diff can contain config values (e.g. Actions // variables), so it goes to debug; info gets a summary without them. if (!payload?.check_run || !payload?.repository) { - this.log.debug(`Dry-run results:\n${JSON.stringify(this.results, null, 2)}`) + // Passed as an object so the logger only serializes it when debug is emitted + this.log.debug({ results: this.results }, 'Dry-run results') const summary = this.results.map(res => `${res.type} ${res.plugin} ${res.repo}: ${res.action?.msg ?? ''}`).join('\n') this.log.info(`Dry-run finished with ${this.results.length} planned change(s); the full diff is logged at debug level.\n${summary}`) return diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index 1ecaa6e56..f47270b82 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -576,7 +576,7 @@ repository: expect(stubContext.log.info).toHaveBeenCalledWith(expect.stringContaining('Changes found')) expect(stubContext.log.info).not.toHaveBeenCalledWith(expect.stringContaining('plain-value')) - expect(stubContext.log.debug).toHaveBeenCalledWith(expect.stringContaining('plain-value')) + expect(stubContext.log.debug).toHaveBeenCalledWith({ results: settings.results }, 'Dry-run results') expect(stubContext.octokit.rest.checks.update).not.toHaveBeenCalled() }) })