Runner: usage of add-mask still echoes the value to the log

[ericsampson]

Describe the bug
According to https://github.com/actions/runner/issues/159, the issue where the add-mask workflow command echoes/leaks the secret was supposed to be fixed, but we still observe it.
This was also mentioned on the GitHub forum by a Partner

To Reproduce
Steps to reproduce the behavior:
echo "::add-mask::${{ steps.mystep.outputs.myvalue }}"

Expected behavior
raw output is not echoed to the log

Runner Version and Platform
Hosted
Ubuntu

[Nyholm]

I can confirm this issue.

[Briggsdf]

Have there been any updates(Or workarounds) on this? It seems like they recently pushed a fix for this, but it doesn't seem to have resolved the issue.

[ericsciple]

The mask needs to be added before the output is registered.

Otherwise at the time when the inputs are evaluated for the next step, the value is not yet registered as a secret. It doesn't get registered until the next step executes. When the step inputs are logged, it hasn't yet been registered as a secret.

[ericsciple]

We may need to detect this specific case and error early, rather than evaluate the expression.

[Briggsdf]

OK, at what point in the workflow would that be?

[ericsciple]

In the example from the description, the step mystep sets the output. It should register the mask value before setting the output.

The output is streaming, so too late if value is already printed.

[ZebraFlesh]

The mask needs to be added before the output is registered

Given the following example using AWS credentials and the setup-terraform action:

    - name: Terraform Output - Access Key ID
      id: terraform-output-1
      run: terraform output access_key_id
    - name: Terraform Output - Secret Key
      id: terraform-output-2
      run: terraform output secret_access_key
    - run: |
        echo "::add-mask::${{ steps.terraform-output-1.outputs.stdout }}"
        echo "::add-mask::${{ steps.terraform-output-2.outputs.stdout }}"
        echo "::set-env name=test_key_id::${{ steps.terraform-output-1.outputs.stdout }}"
        echo "::set-env name=test_secret::${{ steps.terraform-output-2.outputs.stdout }}"

I tried changing the masking to the following:

    - run: |
        echo "::add-mask::${{ steps.terraform-output-1.outputs.stdout }}"
        echo "::add-mask::${{ steps.terraform-output-2.outputs.stdout }}"
    - name: Capture Terraform Outputs
      run: |
        echo "::set-env name=test_key_id::${{ steps.terraform-output-1.outputs.stdout }}"
        echo "::set-env name=test_secret::${{ steps.terraform-output-2.outputs.stdout }}"

But my AWS secrets are still showing up in the logs. Those values are dynamic and not known until terraform outputs them, so I'm not sure what else I could do.

[ericsampson]

It doesn't get registered until the next step executes. When the step inputs are logged, it hasn't yet been registered as a secret.

Regardless of anything else, this should be added to the documentation for add-mask. There's no mention that it's only effective for the step after it's registered.

[ZebraFlesh]

I guess I’m back to my original post then: what is the point of add-mask if it inherently exposes secrets?

[ericsciple]

@ericsampson sorry bad phrasing on my part. I was referring to the specific example when I said that. The statement on it's own is not true.

The add-mask command takes effect from the moment it is processed. The step writes the command over stdout to the runner. From the moment the runner processes the line, all future lines will be scrubbed.

@ZebraFlesh @ericsampson here is an example how add-mask should be used with secret outputs:

on: push
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - id: sets-a-secret
        run: |
          the_secret=$((RANDOM))
          echo "::add-mask::$the_secret"
          echo "::set-output name=secret-number::$the_secret"
      - run: |
          echo "the secret number is ${{ steps.sets-a-secret.outputs.secret-number }}"

If you set ACTIONS_STEP_DEBUG you will see lots of additional output.

The secret value will be masked everywhere. If you remove the add-mask command, you will see the secret value printed in many places - even when the set-output command is processed. So it is critical the add-mask command comes first.

Hope that explanation helps. Let me know.

[ericsampson]

Thanks @ericsciple!

I was able to do a little playing around, and have a modified version of your example that shows what I'm experiencing. Pretend that the the_secret output below in init is set by something like an Terraform action as in the more realistic examples above. With the following code, the local_secret line does print the secret to the log:

on:
  pull_request:
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - id: init
        run: |
          echo "::set-output name=the_secret::$((RANDOM))"
      - id: sets-a-secret
        run: |
          local_secret="${{ steps.init.outputs.the_secret }}"
          echo "::add-mask::$local_secret"
          echo "::set-output name=secret-number::$local_secret"
      - run: |
          echo "the secret number is ${{ steps.sets-a-secret.outputs.secret-number }}"     

So it seems to me like the issue is in the context expansion? It seems like it's being expanded first and then printed to the log, whereas something like $MYVAR or $(EXPRESSION) is printed to the log as is and expand later (which is desirable in the case of secrets).
Is there a way to write the context expression to avoid this early expansion?
I also tried it without the local, and just inline in the add-mask, but that didn't help:

... 
        run: |
          echo "::add-mask::${{ steps.init.outputs.the_secret }}"
          echo "::set-output name=secret-number::$local_secret"

[imjohnbo]

Hi @ericsampson! Just taking a look from the Terraform perspective and wanting to confirm this behavior isn't only after setup-terraform – from your example here, it looks like it's general?

[ericsampson]

Hi @imjohnbo, I'm still coming up to speed on GH Actions, but AFAICT this is a platform thing and not specific to Terraform. It just tends to get exposed (and mentioned) by people because this scenario naturally happens often when using Terraform (or other IaC). My latest example uses no 3rd-party Actions, just the built in platform stuff, and still demonstrates it. And also the first time I came across this, I wasn't using setup-terraform. Unless I'm just out to lunch. Thanks for caring!

[ericsampson]

Yes, that latest example/repo steps is fully self-contained and runnable, I didn't run it with a step using setup-terraform or other 3rd-party actions that could have messed up some internal Action runner state/settings, and then trim that out of the code shown here. That code sample demonstrates what I'm seeing.

[ericsciple]

@imjohnbo what do you think about the terraform wrapper supporting a -secret argument?

Puts the burden on the caller to indicate whether they expect the output to be a secret.

For example:

  steps:
    - run: terraform -secret output secret_access_key

The terraform wrapper would:

• Intercept the -secret flag and not pass it to the underlying command (terraform output secret_accesskey)
• Echo ::add-mask::the secret value before echoing ::set-output name=stdout::the secret value

I skimmed the terraform CLI docs. Doesn't look like any commands accept a -secret flag today.

[ericsampson]

this isn't limited to Terraform though, that's adressing one symptom rather than the issue which could come from a multitude of sources. It seems like it's not possible to mask any arbitrary context expression? Unless I'm mistaken and there is a way for the user to achieve this (without requiring every Action author everywhere to support inbuilt masking, which seems unrealistic).

[Briggsdf]

this isn't limited to Terraform though, that's adressing one symptom rather than the issue which could come from a multitude of sources. It seems like it's not possible to mask any arbitrary context expression? Unless I'm mistaken and there is a way for the user to achieve this (without requiring every Action author everywhere to support inbuilt masking, which seems unrealistic).

Agreed, the only sense I can make of this is that they want you to lock in with Github Secrets. But trying to mask anything in the GitHub context seems undoable. In my case I'm trying to mask a value sent in a Webhooks payload ${{github.event.client_payload.some_value}}

[ericsciple]

Thanks all for the feedback!

There are two ways to register a secret today:

1. When you use a repository secret, it gets masked automatically
2. Otherwise need to echo add-mask _before_ outputting a secret. For example, if using set-output, need to echo add-mask first.

I would advise against adding secrets into the github.event.client_payload. That is a good feature request though.

/cc @chrispat fyi regarding ^ feature request

[ericsampson]

Thanks Eric, but how do you do point 2 for arbitrary context expressions? I'd love to understand, but either I'm missing something or we're talking by each other.

FWIW, regarding the Terraform Action, there is already a way in TF code to mark fields as containing sensitive/secret information, it just needs to get supported int the setup-terraform Action - no extra output argument required. @ZebraFlesh has already opened up this PR for that.

[ZebraFlesh]

@ZebraFlesh has already opened up this PR for that.

Issue, not PR 😛
edit: I also think it unlikely that it will be solved in that action. It seems to invalidate the entire design of the setup-terraform action (thin wrapper that just forwards things as outputs; now it needs to be smart and not so thin).

[ericsciple]

@ericsampson thank you for your patience

how do you do point 2 for arbitrary context expressions?

Sorry but it is not possible today. If the value is already in the context, it is too late.

Here are a few examples to illustrate. I'll summarize at the bottom.

Example 1: Debug output from set-output

This example illustrates why add-mask after set-output is too late. First, enable step debug logging. Then run:

on: push
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - id: sets-a-secret
        run: |
          echo "::set-output name=my-secret::$((RANDOM))"

Because debug logging is on, and because add-mask was not echoed first, the log contains:

::set-output name=my-secret::31487

Example 2: Action input with expression

Here is another example (debug logging not required):

on: push
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - id: sets-a-secret
        run: |
          echo "::set-output name=my-secret::$((RANDOM))"
      - uses: actions/checkout@v2
        with:
          not-a-real-input: ${{ steps.sets-a-secret.outputs.my-secret }}

Near the top of the logs for the checkout step, expand the folded line Run actions/checkout@v2 to see the inputs for the step. The inputs are printed to the log _before_ the step executes. Because add-mask was not echoed, the log contains the secret in plain text.

Run actions/checkout@v2
  with:
    not-a-real-input: 32467
    repository: ericsciple/testing
    token: ***
    ssh-strict: true
    persist-credentials: true
    clean: true
    fetch-depth: 1
    lfs: false
    submodules: false

Example 3: run script with expression

on: push
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - id: sets-a-secret
        run: |
          echo "::set-output name=my-secret::$((RANDOM))"
      - name: My script only contains bash comments
        run: |
          # comment 1
          # comment 2 ${{ steps.sets-a-secret.outputs.my-secret }}

Near the top of the logs for the second step, expand the folded line Run # comment 1 to see the inputs for the step. The inputs are printed to the log _before_ the script executes. Because add-mask was not echoed, the log contains the secret in plain text.

Run # comment 1
  # comment 1
  # comment 2 26441
  shell: /bin/bash -e {0}

Example 4: Default display name for a run step

on: push
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - id: sets-a-secret
        run: |
          echo "::set-output name=my-secret::$((RANDOM))"
      - run: |
          echo default step display name will the secret ${{ steps.sets-a-secret.outputs.my-secret }}

In the web UI you will see Run echo default step display name will the secret 31510. The default display name is calculated _before_ the script executes. Because add-mask was not echoed, the default display contains the secret in plain text.

Example 5: Debug output from expression evaluation

First, enable step debug logging. Then run:

on: push
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - id: sets-a-secret
        run: |
          echo "::set-output name=my-secret::$((RANDOM))"
      - run: |
          echo hello
        env:
          MY_SECRET: ${{ steps.sets-a-secret.outputs.my-secret }}

Because debug logging is on, and because add-mask was not echoed first, the log contains debug output from the expression evaluation:

##[debug]Evaluating: steps.sets-a-secret.outputs.my-secret
[...]
##[debug]Result: '16195'

Summary

The key factors why add-mask must be called first:

• Logs are streaming to the web console (the runner scrubs the lines before sending)
• Context values can be printed to the log for a variety of reasons (critical add-mask echoed prior):
  
  
  
  • When step debug logging is enabled, the set-output command is logged (example 1 above)
  
  
  • Action inputs (i.e. uses) are printed to the log (example 2 above)
  
  
  • Inline scripts (i.e. run) are printed to the log (example 3 above)
  
  
  • Default display name for inline scripts (i.e. run) is calculated before the script executes (example 4 above)
  
  
  • Debug output from expression evaluation (example 5 above)
  
  

Hope that helps. Feedback is welcome.

I need to look over the docs, and see whether the guidance can be improved. Open to ideas and I can pass along.

I also wonder whether we should add a secret parameter for the set-output command. It may guide folks down the correct path. For example:

echo "::set-output name=my-secret, secret=true::the value"

[ericsciple]

(edited the above comment, added 2 more examples)

[ericsampson]

Thanks very much for the detailed information @ericsciple.

1) I'm going to open up a new feature request issue to add a new add-mask bool to set-output, to match the syntax of the standalone add-mask command:
echo "::set-output name=my-secret, add-mask=true::the value"
This will help make it so that every Action author (like the TF action) isn't forced to reimplement this. They can if it makes sense for their application and they choose to, but if not there is always the fallback for the user.
1b) this will still not cover every case where a person might want to mask content in the wider non-output action context, like @Briggsdf's use case of webhooks. There are probably others.

2) I think it would be very valuable to add more information to the documentation somewhere to describe what gets executed before the script gets run, along the lines of your bullet points. Because this isn't obvious at all, without laboriously playing with scenarios : )

Along sort of similar lines, is there any mention in the documentation on the ability to run sub-shells in inline scripts? I only stumbled upon that in a Forum posting when trying to figure out how to do something.

[sshymko]

Accidentally discovered the following undocumented feature that can be used as a workaround for masking sensitive data.
GitHub Actions appears to automatically mask inputs / environment variables following certain naming conventions.
For instance, a plaintext variable named WEBHOOK_TOKEN holding a JWT is masked same way as encrypted secrets would.
It would be great to officially document this behavior along with the supported keywords to make it safe to rely upon.

GitHub Action configuration:

name: Test
on:
  workflow_dispatch:
    inputs:
      WEBHOOK_URL:
        description: 'Webhook URL'
        required: true
      WEBHOOK_METHOD:
        description: 'Webhook method'
        required: true
        default: 'GET'
      WEBHOOK_TOKEN:
        description: 'Webhook token'
        required: true
jobs:
  test:
    name: Test sensitive data masking
    runs-on: ubuntu-latest
    env:
      WEBHOOK_URL: ${{ github.event.inputs.WEBHOOK_URL }}
      WEBHOOK_METHOD: ${{ github.event.inputs.WEBHOOK_METHOD }}
      WEBHOOK_TOKEN: ${{ github.event.inputs.WEBHOOK_TOKEN }}
    steps:
      - name: Notify job start
        run: |
          curl -s -o /dev/null -w "%{http_code}\n" \
            -X "$WEBHOOK_METHOD" "$WEBHOOK_URL" \
            -H "Authorization: Bearer $WEBHOOK_TOKEN"

GitHub Action log: