-
Notifications
You must be signed in to change notification settings - Fork 935
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
usage of add-mask still echoes the value to the log #475
Comments
I can confirm this issue. |
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. |
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. |
We may need to detect this specific case and error early, rather than evaluate the expression. |
OK, at what point in the workflow would that be? |
In the example from the description, the step The output is streaming, so too late if value is already printed. |
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. |
Regardless of anything else, this should be added to the documentation for |
I guess I’m back to my original post then: what is the point of |
@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 Hope that explanation helps. Let me know. |
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
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
|
Hi @ericsampson! Just taking a look from the Terraform perspective and wanting to confirm this behavior isn't only after |
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! |
Yes, that latest example/repo steps is fully self-contained and runnable, I didn't run it with a step using |
@imjohnbo what do you think about the terraform wrapper supporting a 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:
I skimmed the terraform CLI docs. Doesn't look like any commands accept a |
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}} |
Thanks all for the feedback! There are two ways to register a secret today:
I would advise against adding secrets into the /cc @chrispat fyi regarding ^ feature request |
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. |
Issue, not PR 😛 |
@ericsampson thank you for your patience
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
|
(edited the above comment, added 2 more examples) |
Thanks very much for the detailed information @ericsciple.
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. |
I'm also trying to mask AWS credential variables but it fails authentication in the next step, despite the values are properly hidden in the workflow output: - name: Assume the 2nd role and overwrite the AWS env. vars
run: |
OUT=$(aws sts assume-role --role-arn arn:aws:iam::${{ env.AccountId }}:role/**** --role-session-name *** --external-id ***)
AWS_ACCESS_KEY_ID=$(echo $OUT | jq -r '.Credentials''.AccessKeyId')
echo "::add-mask::$AWS_ACCESS_KEY_ID"
echo "AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID" >> $GITHUB_ENV
AWS_SECRET_ACCESS_KEY=$(echo $OUT | jq -r '.Credentials''.SecretAccessKey')
echo "::add-mask::$AWS_SECRET_ACCESS_KEY"
echo "AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY" >> $GITHUB_ENV
AWS_SESSION_TOKEN=$(echo $OUT | jq -r '.Credentials''.SessionToken')
echo "::add-mask::$AWS_SESSION_TOKEN"
echo "AWS_SESSION_TOKEN=$AWS_SESSION_TOKEN" >> $GITHUB_ENV
- name: Login to Amazon ECR
id: login-ecr
uses: aws-actions/amazon-ecr-login@v1
with:
registries: ${{ env.registry }} if I omit the masking, ECR login step works just fine. Does the |
@gr0vity-dev Here's an update to #475 (comment), it is definitely possible to do this with workflowon: push
jobs:
my-job:
runs-on: ubuntu-latest
steps:
- id: sets-a-secret
run: |
the_secret=$((RANDOM))
echo "::add-mask::$the_secret"
echo "secret-number=$the_secret" >> "$GITHUB_OUTPUT"
- run: |
echo "the secret number is ${{ steps.sets-a-secret.outputs.secret-number }}"
raw logs
|
@Constantin07 you can test this theory by using something like (actual markup left as an exercise):
|
Thanks @jsoref, already sorted out by using |
I found out that this is done by MaskHint runner/src/Runner.Worker/Worker.cs Lines 167 to 183 in 5421fe3
and currently github send these MaskHints with the job \bv1\.[0-9A-Fa-f]{40}\b \bgh[pousr]{1}_[A-Za-z0-9]{36}\b \bgithub_pat_[0-9][A-Za-z0-9]{21}_[A-Za-z0-9]{59}\b \b(?:eyJ0eXAiOi|eyJhbGciOi|eyJ4NXQiOi|eyJraWQiOi)[^\s'";]+ \bBearer\s+[^\s'";]+ \b(?i:Password|Pwd)=(?:[^\s'";]+|"[^"]+") -(?i:Password|Pwd)\s+(?:[^\s'";]+|"[^"]+") (?:[a-zA-Z][a-zA-Z\d+-.]*):\/\/([a-zA-Z\d\-._~\!$&'()*+,;=%]+):([a-zA-Z\d\-._~\!$&'()*+,;=:%]*)@
|
I faced the same issue. Any solution for this? |
@Rahulkhinchi03 please do not use GitHub Issues to advertise your product. |
Because creds can't be kept out of the logs, even with [add-mask](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#masking-a-value-in-a-log). actions/runner#475
"It doesn't work on my machine"
I looks like it's only tested against a runtime variable generating case, i.e. $((RANDOM)). Solution would be that the output of the |
docker_creds is a multiline string. Masks are line oriented things. You'll want to do something like: docker_password=$(echo "$docker_creds" | jq -r .docker_password_SOMETHING )
echo "::add-mask::$docker_password" |
Thank you for your quick reply @jsoref , the problem is that the first line of your example is already in the logs, including the output of the jq command EDIT: actually my unmasked line is But how to mask it before sending it to $GITHUB_OUTPUT? |
don't do it like that. use: env:
login_ecr_outputs: ${{ steps.login-ecr.outputs }} or something ... you may need to cast a |
you can always base64 it, to get a single-line maskable string out of a multiline blob. |
@ericsampson I've put in a FR: aws-actions/amazon-ecr-login#483 |
@jsoref I've tried with env, but unfortunately
It's empty because I'm writing to env at the steps level. Can't assign it at job or workflow level since the variable is not existing at that moment, i.e.
I get the error that "steps" is not recognised |
No No No.
And note that you can't reach env from a different job. You could perhaps use |
I used GITHUB_OUTPUT but the value isn't masked |
But you put the string into the command instead of the |
@jsoref nevermind, the problem is in the used upstream action: |
Just want to share some code here for anyone that got stuck from this, taken from here google-github-actions/get-secretmanager-secrets#288
|
- I give up, ill figure it out later. actions/runner#475
@sourceful-karlson I'd say the secret value is going to be shown in the details of the run, in the env section, unless that google action does something I don't know |
I don't understand how this issue gains no traction. The existence of the function The function should be fixed ASAP with high priority or be deleted. |
@Croydon I don't know how to get more attention from someone at GitHub. Get a popular YTer like @ThePrimeagen to upload a video blasting this security issue that hasn't been addressed for four years? |
New to GH Actions, but having the same issue. Just trying to re-iterate and simply present it again for attention. jobs:
printing-secret:
runs-on: ubuntu-latest
steps:
- name: generate
id: generate
run: |
secret=`echo $RANDOM`
echo "::add-mask::$secret"
echo "secret=$secret" >> $GITHUB_OUTPUT
- name: print
run: |
echo "generated secret is : ${SECRET}"
env:
SECRET: ${{ steps.generate.outputs.secret }}
outputs:
secret: ${{ steps.generate.outputs.secret }}
fetch-secret:
runs-on: ubuntu-latest
needs: [printing-secret]
steps:
- name: fetch
id: fetch
run: |
echo "fetched secret is : ${FETCHED_SECRET}"
env:
FETCHED_SECRET: ${{ needs.printing-secret.outputs.secret }} It performs masking for steps under a job (and won't show-up when debugging is enabled) generated secret is : *** But doesn't fetch any value when passed between the jobs with fetched secret is : |
@p-pal, the first job triggers:
GitHub's documentation does include information about how to pass secrets between jobs: (I wrote that documentation a while ago...) It includes some hand waving, so you'll have to fill in the details using your preferred magic. |
Describe the bug
According to #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
The text was updated successfully, but these errors were encountered: