Templated payload variables are replaced before the payload is parsed

[flupzor]

runs:
  using: "composite"
  steps:
    - name: Post a message in a channel
      uses: slackapi/slack-github-action@v2.0.0
      with:
        webhook: "${{ env.SLACK_WEBHOOK_URL }}"
        webhook-type: incoming-webhook
        # See: https://api.slack.com/reference/messaging/attachments
        payload: |
          attachments:
            - color: "${{ inputs.status == 'success' && 'good' || inputs.status == 'failure' && 'danger' || '#808080' }}"
              pretext: "${{ inputs.text }}"
              title: "Build Status: ${{ inputs.status }}"
              title_link: "${{ github.event.pull_request.html_url || github.event.head_commit.url }}"
              fields:
                - title: "Author"
                  value: "${{ github.event.head_commit.author.name }}"
                  short: true
                - title: "Repository"
                  value: "<${{ github.event.repository.html_url }}|${{ github.event.repository.full_name }}>"
                  short: true
                - title: "Head commit"
                  value: "<${{ github.event.head_commit.url }}|${{ github.event.head_commit.message }}>"
                  short: false

With the above call to slack-github-action I ran into a parsing issue. For certain commit messages, the followed error occured.

file:///home/runner/_work/_actions/slackapi/slack-github-action/v2.0.0/src/content.js:93
      throw new SlackError(
^
SlackError: Invalid input! Failed to parse contents of the provided payload
    at Content.getContentPayload (file:///home/runner/_work/_actions/slackapi/slack-github-action/v2.0.0/src/content.js:93:1)
    at Content.get (file:///home/runner/_work/_actions/slackapi/slack-github-action/v2.0.0/src/content.js:28:1)
    at new Config (file:///home/runner/_work/_actions/slackapi/slack-github-action/v2.0.0/src/config.js:115:1)
    at send (file:///home/runner/_work/_actions/slackapi/slack-github-action/v2.0.0/src/send.js:13:1)
    at file:///home/runner/_work/_actions/slackapi/slack-github-action/v2.0.0/src/index.js:9:1
    at ModuleJob.run (node:internal/modules/esm/module_job:263:25)
    at ModuleLoader.import (node:internal/modules/esm/loader:540:24)
    at asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5)

After trying a bunch of commit messages, I could simplify the string that would cause the above error to the following

\n---\n

This string would be inserted into ${{ github.event.head_commit.message }}>

This is a YAML-directive. Which is weird, because the variables do not exist at YAML-level.

Code

I then further tracked it down to the following code.

First, it translates the variables to their values. Here

slack-github-action/src/content.js

Line 121 in d07fadf

const content = this.templatize(config, input);

And

slack-github-action/src/content.js

Lines 155 to 166 in d07fadf

	templatize(config, input) {
	if (!config.inputs.payloadTemplated) {
	return input;
	}
	const template = input.replace(/\$\{\{/g, "{{"); // swap ${{ for {{
	const context = {
	env: process.env,
	github: github.context,
	};
	return markup.up(template, context);
	}
	}

It tries to parse the payload parameter as YAML

slack-github-action/src/content.js

Lines 64 to 77 in d07fadf

	try {
	const input = this.templatize(config, config.inputs.payload);
	const content = /** @type {Content} */ (
	yaml.load(input, {
	schema: yaml.JSON_SCHEMA,
	})
	);
	return /** @type {Content} */ (content);
	} catch (error) {
	if (error instanceof Error) {
	config.core.debug("Failed to parse input payload as YAML");
	config.core.debug(error.message);
	}
	}

If that fails, it tries to parse the payload parameter as JSON

slack-github-action/src/content.js

Lines 78 to 100 in d07fadf

	try {
	const trimmed = config.inputs.payload.trim();
	if (
	!config.inputs.payload.startsWith("{") &&
	!config.inputs.payload.endsWith("}")
	) {
	config.core.debug(
	"Wrapping input payload in braces to create valid JSON",
	);
	const comma = trimmed.replace(/,$/, ""); // remove trailing comma
	const wrap = `{${comma}}`;
	return JSON.parse(wrap);
	}
	return JSON.parse(trimmed);
	} catch (/** @type {any} */ error) {
	throw new SlackError(
	config.core,
	"Invalid input! Failed to parse contents of the provided payload",
	{
	cause: error,
	},
	);
	}

If that fails, it exits.

What I think is the problem

The problem I see with this however, is that the order is incorrect. What is being done is:

• Change variables to their output value.
• Parse Yaml

What should be done is:

• Parse Yaml
• Change variables to their output value

This might also have security implications, since the inserted variables could modify the yaml document. I know to little about YAML and use cases for slack-github-action to be sure.

[zimeg]

Hey @flupzor! 👋 Thanks so much for finding the cause of this error and investigations more 👾 ✨

The updates to parsing logic suggested is interesting to me and I'll leave a follow up comment around this since it might be adjacent to the error raised with inputs now, or at least a workaround might be possible for this error.

Right now the most interesting lines to me are the input:

\n---\n

And how these inputs are gathered in YAML:

runs:
  using: "composite"
  steps:
    - name: Example
      uses: slackapi/slack-github-action@v2.0.0
      with:
        payload: |
          attachments:
            - fields:
                - title: "Head commit"
                  value: "<${{ github.event.head_commit.url }}|${{ github.event.head_commit.message }}>"
                  short: false

As I understand, GitHub Actions parses the workflow.yml file and replaces variables before running steps. This causes the commit message with linebreaks to be substituted before the example step starts.

With the double quotes used this might cause the value to break in unexpected ways, and using single quotes might fix this case, but I'm not certain it covers all cases as expected...

Small note too that the templatize function requires the payload-templated input to be "true" but other ideas around that in the follow up comment 🏷

Using JSON for the payload value instead of YAML has been more resilient to unexpected inputs so far with a few additional helper functions, and I'm wondering if this might be possible for you to use?

The toJSON inline function escapes special characters and is often good for handling user inputs. This requires the entire payload is also JSON though, which can resemble this:

- name: Escape commit messages
  id: message
  run: |
    COMMIT_MSG=${{ toJSON(github.event.head_commit.message) }}
    COMMIT_MSG=$(echo "$COMMIT_MSG" | sed -e 's/^"//' -e 's/"$//' -e 's/"/\\"/g')
    echo "message=$COMMIT_MSG" >> "$GITHUB_OUTPUT"
- name: Attach a webhook
  uses: slackapi/slack-github-action@v2.0.0
  env:
    HEAD_COMMIT_MESSAGE: ${{ steps.message.outputs.message }}
  with:
    errors: true
    webhook: ${{ secrets.SLACK_INCOMING_WEBHOOK }}
    webhook-type: incoming-webhook
    payload: |
      {
        "attachments": [
          {
            "color": "${{ inputs.status == 'success' && 'good' || inputs.status == 'failure' && 'danger' || '#808080' }}",
            "pretext": "${{ inputs.text }}",
            "title": "Build Status: ${{ inputs.status }}",
            "title_link": "${{ github.event.pull_request.html_url || github.event.head_commit.url }}",
            "fields": [
              {
                "title": "Author",
                "value": "${{ github.event.head_commit.author.name }}",
                "short": true
              },
              {
                "title": "Repository",
                "value": "<${{ github.event.repository.html_url }}|${{ github.event.repository.full_name }}>",
                "short": true
              },
              {
                "title": "Head commit",
                "value": "<${{ github.event.head_commit.url }}|${{ env.HEAD_COMMIT_MESSAGE }}>",
                "short": false
              }
            ]
          }
        ]
      }

And I was finding some extra replacements were needed to remove surrounding quotes to use this variable as link text.

The error messages around this can be improved to and this is tracked in #359 but I'm hoping for now this might be helpful? Please let me know if errors continue though! 🙏

[zimeg]

📚 Adding the documentation label since this is an example we might want to highlight elsewhere!

[zimeg]

📝 Also following up on the ideas around parsing the input payload before replacing variables:

What should be done is:

• Parse Yaml
• Change variables to their output value

To me this seems ideal when using the payload-templated option, especially with the payload-file-path input! ¹

The current implementation replaces templated placeholders for the entire payload string all at once, then converts that from a YAML or JSON string into actual JSON. Replacing templated values within the actual JSON would fix these parsing errors I'd also hope!

IMO this would match documented inputs better too since we recommend using variables as values and not keys for the same worries noted.

Let's mark this as a bug with discussion as well. I haven't explored this yet, but I'm curious if this'd be useful in similar issues #82 #377 #357 and what others think?

Edit: Wrong issue!

Footnotes

1. Noting the payload-file-path input since parsing of the inline payload variable is done before the step runs, though the payload-templated option can still be used with this 🏷 ↩

[mbektimirov]

I had to cut out multi line commits to leave only the first line (aka subject) to normally run slack action:
Continuing discussion from #359

- name: Get commit subject
  id: commit_subject
  env:
      COMMIT_MSG: ${{ github.event.head_commit.message }}
  run: |
      echo "subject=${COMMIT_MSG%%$'\n'*}" >>"$GITHUB_OUTPUT"

- name: Share ZIP on Slack
  uses: slackapi/slack-github-action@v2.1.0
  with:
      method: files.uploadV2
      token: ${{ secrets.SLACK_BOT_TOKEN }}
      payload: |
          channel_id: ${{ secrets.SLACK_CHANNEL_CANARY_BUILDS_ID }}
          initial_comment: |
            🚀 *New build*
            *Commit message:*
            ${{ steps.commit_subject.outputs.subject }}
          file: file.zip
          filename: filename-${{ github.sha }}.zip

[zimeg]

As I understand, GitHub Actions parses the workflow.yml file and replaces variables before running steps.

Quick correction related to env variables: These are passed to the action separate and can be used within code! 📚

It's subtle but that makes a big difference for what's possible in inputs!

Changes of #449 have a fix to replace templated variables after parsing inputs - this is a great call @flupzor 🙏 ✨

Edit: Oh perhaps these are still substituted beforehand - more testing is showing that to be true😓
Edit: Not to discount the improvements for payload-file-path when parsing before replacement!