From 668b3bf830883fa342766b1dc84973b45f00395d Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Wed, 4 Dec 2024 19:27:43 -0800 Subject: [PATCH 1/3] fix: accept "token" but not "method" with "webhook" inputs --- src/config.js | 36 +++++++++++++++++++++--------------- src/send.js | 2 +- test/config.spec.js | 41 ++++++++++++++++++++++++++++++++++++----- test/send.spec.js | 1 + 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/src/config.js b/src/config.js index 43581aa7..f84573d4 100644 --- a/src/config.js +++ b/src/config.js @@ -110,12 +110,27 @@ export default class Config { core.getInput("webhook") || process.env.SLACK_WEBHOOK_URL || null, webhookType: core.getInput("webhook-type"), }; + this.mask(); this.validate(); core.debug(`Gathered action inputs: ${JSON.stringify(this.inputs)}`); this.content = new Content().get(this); core.debug(`Parsed request content: ${JSON.stringify(this.content)}`); } + /** + * Hide secret values provided in the inputs from appearing. + */ + mask() { + if (this.inputs.token) { + core.debug("Setting the provided token as a secret variable."); + core.setSecret(this.inputs.token); + } + if (this.inputs.webhook) { + core.debug("Setting the provided webhook as a secret variable."); + core.setSecret(this.inputs.webhook); + } + } + /** * Confirm the configurations are correct enough to continue. */ @@ -133,29 +148,20 @@ export default class Config { ); } switch (true) { - case !!this.inputs.token && !!this.inputs.webhook: - core.debug( - "Setting the provided token and webhook as secret variables.", - ); - core.setSecret(this.inputs.token); - core.setSecret(this.inputs.webhook); + case !!this.inputs.method && !!this.inputs.webhook: throw new SlackError( core, - "Invalid input! Either the token or webhook is required - not both.", + "Invalid input! Either the method or webhook is required - not both.", ); - case !!this.inputs.token: - core.debug("Setting the provided token as a secret variable."); - core.setSecret(this.inputs.token); - if (!this.inputs.method) { + case !!this.inputs.method: + if (!this.inputs.token) { throw new SlackError( core, - "Missing input! A method must be decided to use the token provided.", + "Missing input! A token must be provided to use the method decided.", ); } break; case !!this.inputs.webhook: - core.debug("Setting the provided webhook as a secret variable."); - core.setSecret(this.inputs.webhook); if (!this.inputs.webhookType) { throw new SlackError( core, @@ -175,7 +181,7 @@ export default class Config { default: throw new SlackError( core, - "Missing input! Either a token or webhook is required to take action.", + "Missing input! Either a method or webhook is required to take action.", ); } } diff --git a/src/send.js b/src/send.js index e54e84a1..e742991f 100644 --- a/src/send.js +++ b/src/send.js @@ -29,7 +29,7 @@ export default async function send(core) { */ async function post(config) { switch (true) { - case !!config.inputs.token: + case !!config.inputs.method: return await new Client().post(config); case !!config.inputs.webhook: return await new Webhook().post(config); diff --git a/test/config.spec.js b/test/config.spec.js index 633d015f..6e37bcf8 100644 --- a/test/config.spec.js +++ b/test/config.spec.js @@ -32,12 +32,27 @@ describe("config", () => { assert.equal(config.inputs.proxy, "https://example.com"); assert.equal(config.inputs.retries, config.Retries.ZERO); assert.equal(config.inputs.token, "xoxb-example"); + assert.isTrue(mocks.core.setSecret.withArgs("xoxb-example").called); }); - it("errors when both the token and webhook is provided", async () => { + it("allows a token and webhook if no method is provided", async () => { mocks.core.getInput.withArgs("token").returns("xoxb-example"); mocks.core.getInput.withArgs("webhook").returns("https://example.com"); mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); + const config = new Config(mocks.core); + assert.equal(config.inputs.token, "xoxb-example"); + assert.equal(config.inputs.webhook, "https://example.com"); + assert.equal(config.inputs.webhookType, "incoming-webhook"); + assert.isTrue(mocks.core.setSecret.withArgs("xoxb-example").called); + assert.isTrue( + mocks.core.setSecret.withArgs("https://example.com").called, + ); + }); + + it("errors when both the method and webhook is provided", async () => { + mocks.core.getInput.withArgs("method").returns("chat.postMessage"); + mocks.core.getInput.withArgs("webhook").returns("https://example.com"); + mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); try { new Config(mocks.core); assert.fail("Failed to error when invalid inputs are provided"); @@ -45,9 +60,8 @@ describe("config", () => { if (err instanceof SlackError) { assert.include( err.message, - "Invalid input! Either the token or webhook is required - not both.", + "Invalid input! Either the method or webhook is required - not both.", ); - assert.isTrue(mocks.core.setSecret.withArgs("xoxb-example").called); assert.isTrue( mocks.core.setSecret.withArgs("https://example.com").called, ); @@ -57,6 +71,23 @@ describe("config", () => { } }); + it("errors if the method is provided without a token provided", async () => { + mocks.core.getInput.withArgs("method").returns("chat.postMessage"); + try { + new Config(mocks.core); + assert.fail("Failed to error when invalid inputs are provided"); + } catch (err) { + if (err instanceof SlackError) { + assert.include( + err.message, + "Missing input! A token must be provided to use the method decided.", + ); + } else { + assert.fail("Failed to throw a SlackError", err); + } + } + }); + it("errors if neither the token or webhook is provided", async () => { try { new Config(mocks.core); @@ -65,7 +96,7 @@ describe("config", () => { if (err instanceof SlackError) { assert.include( err.message, - "Missing input! Either a token or webhook is required to take action.", + "Missing input! Either a method or webhook is required to take action.", ); } else { assert.fail("Failed to throw a SlackError", err); @@ -109,7 +140,7 @@ describe("config", () => { }); }); - describe("secrets", async () => { + describe("mask", async () => { it("treats the provided token as a secret", async () => { mocks.core.getInput.withArgs("token").returns("xoxb-example"); try { diff --git a/test/send.spec.js b/test/send.spec.js index a675d001..dc6ba7e7 100644 --- a/test/send.spec.js +++ b/test/send.spec.js @@ -56,6 +56,7 @@ describe("send", () => { }); it("incoming webhook", async () => { + mocks.core.getInput.withArgs("token").returns("xoxb-example"); // https://github.com/slackapi/slack-github-action/issues/373 mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); From ccebf00fc73f7987c0bd42faed57ed3f03712532 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Wed, 4 Dec 2024 20:06:28 -0800 Subject: [PATCH 2/3] fix: prevent conflicting inputs from the actual step with --- src/config.js | 9 +++++---- test/config.spec.js | 26 ++++++++++++++++++++------ test/index.spec.js | 2 ++ test/send.spec.js | 2 +- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/config.js b/src/config.js index f84573d4..52d57577 100644 --- a/src/config.js +++ b/src/config.js @@ -111,7 +111,7 @@ export default class Config { webhookType: core.getInput("webhook-type"), }; this.mask(); - this.validate(); + this.validate(core); core.debug(`Gathered action inputs: ${JSON.stringify(this.inputs)}`); this.content = new Content().get(this); core.debug(`Parsed request content: ${JSON.stringify(this.content)}`); @@ -133,8 +133,9 @@ export default class Config { /** * Confirm the configurations are correct enough to continue. + * @param {core} core - GitHub Actions core utilities. */ - validate() { + validate(core) { switch (this.inputs.retries.trim().toUpperCase()) { case this.Retries.ZERO: case this.Retries.FIVE: @@ -148,10 +149,10 @@ export default class Config { ); } switch (true) { - case !!this.inputs.method && !!this.inputs.webhook: + case !!core.getInput("token") && !!core.getInput("webhook"): throw new SlackError( core, - "Invalid input! Either the method or webhook is required - not both.", + "Invalid input! Either the token or webhook is required - not both.", ); case !!this.inputs.method: if (!this.inputs.token) { diff --git a/test/config.spec.js b/test/config.spec.js index 6e37bcf8..17e914f5 100644 --- a/test/config.spec.js +++ b/test/config.spec.js @@ -35,8 +35,8 @@ describe("config", () => { assert.isTrue(mocks.core.setSecret.withArgs("xoxb-example").called); }); - it("allows a token and webhook if no method is provided", async () => { - mocks.core.getInput.withArgs("token").returns("xoxb-example"); + it("allows token environment variables with a webhook", async () => { + process.env.SLACK_TOKEN = "xoxb-example"; mocks.core.getInput.withArgs("webhook").returns("https://example.com"); mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); const config = new Config(mocks.core); @@ -49,10 +49,23 @@ describe("config", () => { ); }); - it("errors when both the method and webhook is provided", async () => { + it("allows webhook environment variables with a token", async () => { + process.env.SLACK_WEBHOOK_URL = "https://example.com"; mocks.core.getInput.withArgs("method").returns("chat.postMessage"); + mocks.core.getInput.withArgs("token").returns("xoxb-example"); + const config = new Config(mocks.core); + assert.equal(config.inputs.method, "chat.postMessage"); + assert.equal(config.inputs.token, "xoxb-example"); + assert.equal(config.inputs.webhook, "https://example.com"); + assert.isTrue(mocks.core.setSecret.withArgs("xoxb-example").called); + assert.isTrue( + mocks.core.setSecret.withArgs("https://example.com").called, + ); + }); + + it("errors when both the token and webhook is provided", async () => { + mocks.core.getInput.withArgs("token").returns("xoxb-example"); mocks.core.getInput.withArgs("webhook").returns("https://example.com"); - mocks.core.getInput.withArgs("webhook-type").returns("incoming-webhook"); try { new Config(mocks.core); assert.fail("Failed to error when invalid inputs are provided"); @@ -60,8 +73,9 @@ describe("config", () => { if (err instanceof SlackError) { assert.include( err.message, - "Invalid input! Either the method or webhook is required - not both.", + "Invalid input! Either the token or webhook is required - not both.", ); + assert.isTrue(mocks.core.setSecret.withArgs("xoxb-example").called); assert.isTrue( mocks.core.setSecret.withArgs("https://example.com").called, ); @@ -71,7 +85,7 @@ describe("config", () => { } }); - it("errors if the method is provided without a token provided", async () => { + it("errors if the method is provided without a token", async () => { mocks.core.getInput.withArgs("method").returns("chat.postMessage"); try { new Config(mocks.core); diff --git a/test/index.spec.js b/test/index.spec.js index 3138c4a4..4e21eae1 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -61,6 +61,8 @@ export class Mock { this.core.getInput.reset(); this.core.getInput.withArgs("errors").returns("false"); this.core.getInput.withArgs("retries").returns("5"); + process.env.SLACK_TOKEN = ""; + process.env.SLACK_WEBHOOK_URL = ""; } } diff --git a/test/send.spec.js b/test/send.spec.js index dc6ba7e7..3db6b1e8 100644 --- a/test/send.spec.js +++ b/test/send.spec.js @@ -56,7 +56,7 @@ describe("send", () => { }); it("incoming webhook", async () => { - mocks.core.getInput.withArgs("token").returns("xoxb-example"); // https://github.com/slackapi/slack-github-action/issues/373 + process.env.SLACK_TOKEN = "xoxb-example"; // https://github.com/slackapi/slack-github-action/issues/373 mocks.core.getInput .withArgs("webhook") .returns("https://hooks.slack.com"); From eb4ebb944895c5e1036524cec02b44474425a941 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Wed, 4 Dec 2024 20:08:44 -0800 Subject: [PATCH 3/3] test: prevent regressions from a flipped send switch statement --- test/send.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/send.spec.js b/test/send.spec.js index 3db6b1e8..b35398b2 100644 --- a/test/send.spec.js +++ b/test/send.spec.js @@ -39,6 +39,7 @@ describe("send", () => { }); it("token", async () => { + process.env.SLACK_WEBHOOK_URL = "https://example.com"; // https://github.com/slackapi/slack-github-action/issues/373 mocks.api.resolves({ ok: true }); mocks.core.getInput.withArgs("method").returns("chat.postMessage"); mocks.core.getInput.withArgs("token").returns("xoxb-example");