fix: sanitise GITHUB_REF to be a valid tag value

[dgholz]

Fixes #247

Description of changes:
Applies the same sanitization to Branch as is applied to Workflow.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[dgholz]

@Mergifyio refresh

[mergify]

Command refresh: success

Pull request refreshed

[paragbhingre]

@dgholz Thanks for your contribution, could you please attach your test results/screenshots of showing how your branch name was outside of the [\p{L}\p{Z}\p{N}_.:/=+\-@]* regular expression?
Also I would like to know the steps that you took for testing on your side so that I can also do the testing on my side.

[dgholz]

Thanks for your contribution, could you please attach your test results/screenshots of showing how your branch name was outside of the [\p{L}\p{Z}\p{N}_.:/=+\-@]* regular expression?

I've added a test that verifies the AssumeRole call is made with a sanitized branch name, where the branch name from the environment had a lot of characters that were not suitable for a tag value: https://github.com/aws-actions/configure-aws-credentials/pull/259/files#diff-59e25b254be93038f106111be580b6fb54c6963b6c4e7ef744e58fb8f2b3e3a2R618. If you approve the pending run in https://github.com/aws-actions/configure-aws-credentials/actions/runs/1233568010 you'll see the test result.

Also I would like to know the steps that you took for testing on your side so that I can also do the testing on my side.

Ran our workflow that uses this action with a branch with a # in its name.

[dgholz]

@paragbhingre hi, is there anything else I can do to make this easy to merge? Happy to share a workflow that now works, screenshots, etc.

[dgholz]

@paragbhingre hello again, is there anything I can do to help?

[dgholz]

@paragbhingre hello, what can I do to make it easy to review this?

[dgholz]

@paragbhingre hello again, I've rebased onto latest master and I see the checks waiting for a maintainer to approve them for running in https://github.com/aws-actions/configure-aws-credentials/actions/runs/1653379542. Could you please approve it?

could you please attach your test results/screenshots of showing how your branch name was outside of the [\p{L}\p{Z}\p{N}_.:/=+\-@]* regular expression?

Just to be thorough: the branch that originally caused me to make this PR was something like git branch testing_for_aws-actions/configure-aws-credentials#259, with the mentioned issue being in one of our private repos.

[dgholz]

@paragbhingre hi, anything you'd like me to do to make reviewing this PR a piece of cake?

[dgholz]

@paragbhingre hello, is there someone else available to review this PR? Should I ask them?

[paragbhingre]

@dgholz so sorry for the late reply, I am no longer working on github actions but I will surely find someone to look at this or find some time to take a look at it. apologies again.

[peterwoodworth]

Thanks for your submission and patience @dgholz, however I'm having trouble reproducing this issue to begin with.

Ran our workflow that uses this action with a branch with a # in its name.

This isn't enough for me to reproduce the issue, I'm running this action fine on a branch called #@#%&001-Branch (and many more attempts with various branch names).

It makes sense to me why this issue is occurring, the character requirements are specified here. However I am not sure why I cannot reproduce. This works for me:

jobs:
  deploy:
    name: Upload to Amazon S3
    runs-on: ubuntu-latest
    # These permissions are needed to interact with GitHub's OIDC Token endpoint.
    permissions:
      id-token: write
      contents: read
    steps:
    - name: Checkout
      uses: actions/checkout@v2
    - name: Configure AWS credentials from Test account
      uses: aws-actions/configure-aws-credentials@v1
      with:
        role-to-assume: arn:aws:iam::123456789012:role/MyRole
        role-duration-seconds: 6000
        aws-region: us-east-1
        aws-access-key-id: xxx
        aws-secret-access-key: xxx

Your PR looks great and I'd love to approve it, however I do need to confirm the current behavior before I approve this. Is there any advice you can give to help reproduce this issue?

[peterwoodworth]

Since I'm not able to reproduce this, in other words: This means that either there are a stricter set of conditions where this requirement is enforced (meaning this would be a breaking change), or that the restriction with these characters has been lifted in the time since this issue and PR has been active and the documentation doesn't reflect that. We need more investigation into the root cause of this issue, and a consistent way to reproduce it before we can fix this issue. Going to close this PR and request that discussion continue on the original issue, thanks! 🙂

[dgholz]

hi @peterwoodworth , thanks for finding time to take a look at this. Try reproducing without using the OIDC token and instead with explicit aws-access-key-id and aws-secret-access-key, as using the OIDC token deletes the session tags (see

configure-aws-credentials/index.js

Line 92 in 5820660

delete assumeRoleRequest.Tags;

)