fix: avoid erroring if conflicting techniques are set from environmen…

…t variables (#374)
slackapi · Dec 5, 2024 · 4879c43 · 4879c43
1 parent c77439f
commit 4879c43
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 20 deletions.
diff --git a/src/config.js b/src/config.js
@@ -110,16 +110,32 @@ export default class Config {
         core.getInput("webhook") || process.env.SLACK_WEBHOOK_URL || null,
       webhookType: core.getInput("webhook-type"),
     };
-    this.validate();
+    this.mask();
+    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)}`);
   }
 
+  /**
+   * 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.
+   * @param {core} core - GitHub Actions core utilities.
    */
-  validate() {
+  validate(core) {
     switch (this.inputs.retries.trim().toUpperCase()) {
       case this.Retries.ZERO:
       case this.Retries.FIVE:
@@ -133,29 +149,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 !!core.getInput("token") && !!core.getInput("webhook"):
         throw new SlackError(
           core,
           "Invalid input! Either the token 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 +182,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
@@ -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
@@ -32,12 +32,40 @@ 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("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);
+      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("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");
@@ -57,6 +85,23 @@ describe("config", () => {
       }
     });
 
+    it("errors if the method is provided without a token", 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 +110,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 +154,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/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
@@ -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");
@@ -56,6 +57,7 @@ describe("send", () => {
     });
 
     it("incoming webhook", async () => {
+      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");