From afb692a06e6667efa735353e6607bcbf41d96cbb Mon Sep 17 00:00:00 2001 From: "tova.becker" Date: Tue, 13 Oct 2026 11:08:23 +0200 Subject: [PATCH 1/3] fix: correct label color padding and report invalid colors padStart's result was never assigned, so colors YAML read as numbers (like 000001) were sent unpadded. Unquoted colors like 5319e7 become 53190000000 and were sent anyway; they are now reported as errors pointing to quoting the value. --- lib/plugins/labels.js | 27 +++++++++++--- test/unit/lib/plugins/labels.test.js | 53 ++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/lib/plugins/labels.js b/lib/plugins/labels.js index 6e48d8dcb..395be30ef 100644 --- a/lib/plugins/labels.js +++ b/lib/plugins/labels.js @@ -22,11 +22,8 @@ module.exports = class Labels extends Diffable { if (this.entries) { this.entries.forEach(label => { // Force color to string since some hex colors can be numerical (e.g. 999999) - if (label.color) { - label.color = String(label.color).replace(/^#/, '') - if (label.color.length < 6) { - label.color.padStart(6, '0') - } + if (label.color !== undefined && label.color !== null) { + label.color = String(label.color).replace(/^#/, '').padStart(6, '0') } }) } @@ -61,6 +58,9 @@ module.exports = class Labels extends Diffable { this.log.debug(`Updating labels for ${JSON.stringify(attrs, null, 4)} ${JSON.stringify(this.repo, null, 4)}`) attrs.current_name = attrs.oldname || attrs.name delete attrs.oldname + if (this.hasInvalidColor(attrs)) { + return this.invalidColorError(attrs) + } if (this.nop) { return Promise.resolve([ new NopCommand(this.constructor.name, this.repo, this.github.rest.issues.updateLabel.endpoint(this.wrapAttrs(attrs)), 'Update label') @@ -70,6 +70,9 @@ module.exports = class Labels extends Diffable { } add (attrs) { + if (this.hasInvalidColor(attrs)) { + return this.invalidColorError(attrs) + } if (this.nop) { return Promise.resolve([ new NopCommand(this.constructor.name, this.repo, this.github.rest.issues.createLabel.endpoint(this.wrapAttrs(attrs)), 'Create label') @@ -98,4 +101,18 @@ module.exports = class Labels extends Diffable { isExcluded (label) { return this.exclude.some(rx => rx.test(label.name)) } + + hasInvalidColor (label) { + return label.color !== undefined && !/^[0-9a-f]{6}$/i.test(label.color) + } + + // YAML reads unquoted colors like 5319e7 as numbers (5319 × 10^7), which can't be recovered here. + invalidColorError (label) { + const msg = `Label '${label.name}' has invalid color '${label.color}'. Quote hex colors in YAML, e.g. color: '5319e7'.` + if (this.nop) { + return Promise.resolve([new NopCommand(this.constructor.name, this.repo, null, msg, 'ERROR')]) + } + this.logError(msg) + return Promise.resolve([]) + } } diff --git a/test/unit/lib/plugins/labels.test.js b/test/unit/lib/plugins/labels.test.js index 3f6ebe9d6..fa3679cc0 100644 --- a/test/unit/lib/plugins/labels.test.js +++ b/test/unit/lib/plugins/labels.test.js @@ -148,5 +148,58 @@ describe('Labels', () => { expect(github.rest.issues.createLabel).toHaveBeenCalledTimes(1) }) }) + + it('pads colors that YAML read as numbers', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + + const plugin = configure([ + { name: 'black', color: 0, description: '' }, // YAML treats `color: 000000` as 0 + { name: 'almost-black', color: 1, description: '' } // YAML treats `color: 000001` as 1 + ]) + + return plugin.sync().then(() => { + const createdColors = github.rest.issues.createLabel.mock.calls.map(([params]) => params.color) + expect(createdColors).toEqual(['000000', '000001']) + }) + }) + + describe('invalid colors', () => { + // YAML treats `color: 5319e7` as 5319 × 10^7 + const config = [{ name: 'jenkins-sync', color: 5319e7, description: '' }] + const expectedMessage = "Label 'jenkins-sync' has invalid color '53190000000'. Quote hex colors in YAML, e.g. color: '5319e7'." + + it('reports an error instead of creating the label', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + log.error = jest.fn() + const errors = [] + const plugin = new Labels(false, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) + + return plugin.sync().then(() => { + expect(github.rest.issues.createLabel).not.toHaveBeenCalled() + expect(errors).toEqual([expect.objectContaining({ msg: expectedMessage, plugin: 'Labels' })]) + }) + }) + + it('reports an error instead of updating the label', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([{ name: 'jenkins-sync', color: '5319e7', description: '' }])) + log.error = jest.fn() + const errors = [] + const plugin = new Labels(false, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) + + return plugin.sync().then(() => { + expect(github.rest.issues.updateLabel).not.toHaveBeenCalled() + expect(errors).toEqual([expect.objectContaining({ msg: expectedMessage })]) + }) + }) + + it('shows the error in the dry-run', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + const plugin = new Labels(true, github, { owner: 'bkeepers', repo: 'test' }, config, log, []) + + return plugin.sync().then(results => { + expect(results.flat()).toContainEqual(expect.objectContaining({ type: 'ERROR', action: expect.objectContaining({ msg: expectedMessage }) })) + }) + }) + }) }) }) From fd54ce30675f1c24df234243880edc903a15b520 Mon Sep 17 00:00:00 2001 From: "tova.becker" Date: Tue, 13 Oct 2026 11:23:53 +0200 Subject: [PATCH 2/3] test: use a generic label name in invalid color tests --- test/unit/lib/plugins/labels.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/lib/plugins/labels.test.js b/test/unit/lib/plugins/labels.test.js index fa3679cc0..30368384d 100644 --- a/test/unit/lib/plugins/labels.test.js +++ b/test/unit/lib/plugins/labels.test.js @@ -165,8 +165,8 @@ describe('Labels', () => { describe('invalid colors', () => { // YAML treats `color: 5319e7` as 5319 × 10^7 - const config = [{ name: 'jenkins-sync', color: 5319e7, description: '' }] - const expectedMessage = "Label 'jenkins-sync' has invalid color '53190000000'. Quote hex colors in YAML, e.g. color: '5319e7'." + const config = [{ name: 'hello-world', color: 5319e7, description: '' }] + const expectedMessage = "Label 'hello-world' has invalid color '53190000000'. Quote hex colors in YAML, e.g. color: '5319e7'." it('reports an error instead of creating the label', () => { github.paginate.mockReturnValueOnce(Promise.resolve([])) @@ -181,7 +181,7 @@ describe('Labels', () => { }) it('reports an error instead of updating the label', () => { - github.paginate.mockReturnValueOnce(Promise.resolve([{ name: 'jenkins-sync', color: '5319e7', description: '' }])) + github.paginate.mockReturnValueOnce(Promise.resolve([{ name: 'hello-world', color: '5319e7', description: '' }])) log.error = jest.fn() const errors = [] const plugin = new Labels(false, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) From 0fcdb6d65bd79a33f8e64a157439447926884883 Mon Sep 17 00:00:00 2001 From: "tova.becker" Date: Tue, 13 Oct 2026 11:53:55 +0200 Subject: [PATCH 3/3] fix: clarify invalid label color message and pad only numeric colors --- lib/plugins/labels.js | 11 +++++++---- test/unit/lib/plugins/labels.test.js | 14 +++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/plugins/labels.js b/lib/plugins/labels.js index 395be30ef..50b8822d1 100644 --- a/lib/plugins/labels.js +++ b/lib/plugins/labels.js @@ -21,9 +21,12 @@ module.exports = class Labels extends Diffable { if (this.entries) { this.entries.forEach(label => { - // Force color to string since some hex colors can be numerical (e.g. 999999) - if (label.color !== undefined && label.color !== null) { - label.color = String(label.color).replace(/^#/, '').padStart(6, '0') + // Force color to string since some hex colors can be numerical (e.g. 999999). + // Numbers lose leading zeros in YAML (000001 becomes 1), so only they are padded. + if (typeof label.color === 'number') { + label.color = String(label.color).padStart(6, '0') + } else if (label.color !== undefined && label.color !== null) { + label.color = String(label.color).replace(/^#/, '') } }) } @@ -108,7 +111,7 @@ module.exports = class Labels extends Diffable { // YAML reads unquoted colors like 5319e7 as numbers (5319 × 10^7), which can't be recovered here. invalidColorError (label) { - const msg = `Label '${label.name}' has invalid color '${label.color}'. Quote hex colors in YAML, e.g. color: '5319e7'.` + const msg = `Label '${label.name}' has invalid color '${label.color}'. Colors must be 6 hex characters, e.g. '5319e7'. Quote the value in YAML so it isn't read as a number.` if (this.nop) { return Promise.resolve([new NopCommand(this.constructor.name, this.repo, null, msg, 'ERROR')]) } diff --git a/test/unit/lib/plugins/labels.test.js b/test/unit/lib/plugins/labels.test.js index 30368384d..a7ccebe98 100644 --- a/test/unit/lib/plugins/labels.test.js +++ b/test/unit/lib/plugins/labels.test.js @@ -166,7 +166,7 @@ describe('Labels', () => { describe('invalid colors', () => { // YAML treats `color: 5319e7` as 5319 × 10^7 const config = [{ name: 'hello-world', color: 5319e7, description: '' }] - const expectedMessage = "Label 'hello-world' has invalid color '53190000000'. Quote hex colors in YAML, e.g. color: '5319e7'." + const expectedMessage = "Label 'hello-world' has invalid color '53190000000'. Colors must be 6 hex characters, e.g. '5319e7'. Quote the value in YAML so it isn't read as a number." it('reports an error instead of creating the label', () => { github.paginate.mockReturnValueOnce(Promise.resolve([])) @@ -200,6 +200,18 @@ describe('Labels', () => { expect(results.flat()).toContainEqual(expect.objectContaining({ type: 'ERROR', action: expect.objectContaining({ msg: expectedMessage }) })) }) }) + + it('reports a quoted non-hex color as written, without padding it', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + log.error = jest.fn() + const errors = [] + const plugin = new Labels(false, github, { owner: 'bkeepers', repo: 'test' }, [{ name: 'hello-world', color: 'red', description: '' }], log, errors) + + return plugin.sync().then(() => { + expect(github.rest.issues.createLabel).not.toHaveBeenCalled() + expect(errors).toEqual([expect.objectContaining({ msg: expect.stringContaining("invalid color 'red'. Colors must be 6 hex characters") })]) + }) + }) }) }) })