Skip to content
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

Action give warning about long term credentials when using InstanceRole permissions on self-hosted runners #885

Closed
bplessis-swi opened this issue Oct 11, 2023 · 7 comments · Fixed by #926
Labels
bug Something isn't working effort/medium This issue will take a few days of effort to fix p2

Comments

@bplessis-swi
Copy link

Describe the bug

Hi,

We are using self-hosted runners within our AWS account, with InstanceRole level permissions that allow for AssumeRole to different deploy roles. There is no long-term AWS credentials, or at least not in the common sense.

Warning: To avoid using long-term AWS credentials, please update your workflows to authenticate using OpenID Connect. See https://s12d.com/gha-oidc-aws for more information.

Expected Behavior

No warning should show up

Current Behavior

A warning pop-up for each call to configure-aws-credentials in our workflows

Warning: To avoid using long-term AWS credentials, please update your workflows to authenticate using OpenID Connect. See https://s12d.com/gha-oidc-aws for more information.

Reproduction Steps

Simply using configure-aws-credentials without any credentials

    - name: Configure AWS Credentials
      uses: aws-actions/configure-aws-credentials@v4
      with:
        aws-region: ${{ env.AWS_REGION }}
        role-to-assume: arn:aws:iam::${{ env.AWS_ACCOUNT }}:role/${{ inputs.role-name }}
        role-duration-seconds: ${{ inputs.aws-credential-timeout }}

Possible Solution

No response

Additional Information/Context

No response

@bplessis-swi bplessis-swi added bug Something isn't working needs-triage This issue still needs to be triaged labels Oct 11, 2023
@peterwoodworth peterwoodworth added p2 and removed needs-triage This issue still needs to be triaged labels Oct 16, 2023
@peterwoodworth
Copy link
Contributor

Thanks, i hadn't considered this case. I'm also not sure at all how we could properly detect this. I suppose we could add an "acknowledge-warning" prop, but that requires the user to take a step they shouldn't have to. Will need to look into this

@peterwoodworth peterwoodworth added the effort/medium This issue will take a few days of effort to fix label Oct 16, 2023
@manumbs
Copy link

manumbs commented Oct 17, 2023

@peterwoodworth I'm not sure about this, but perhaps you could validate the presence of the AWS_ROLE_ARN and/or AWS_WEB_IDENTITY_TOKEN_FILE environment variables

@bplessis-swi
Copy link
Author

bplessis-swi commented Oct 19, 2023

Why not triggering the warning only when aws-access-key-id and/or aws-secret-access-key are in use/have been provided to the action ?

@asinbow
Copy link

asinbow commented Oct 31, 2023

The corresponding code:

if (!process.env['AWS_SESSION_TOKEN']) {
core.warning(
'To avoid using long-term AWS credentials, please update your workflows to authenticate using OpenID Connect.' +
' See https://s12d.com/gha-oidc-aws for more information.'
);
}

And the pull request: #871
It checks the environment variable: "AWS_SESSION_TOKEN".

@ponkio-o
Copy link

I am facing a similar problem. I agree with @bplessis-swi

I think the case where it can be determined that long-term credentials are being used is when only aws-access-key-id and aws-secret-access-key are configured.

@peterwoodworth
Copy link
Contributor

I don't work with this team anymore so I cannot provide a review/merge, @tim-finnigan could you look into this please? I think I had assumed these variables would always be filled if it gets to that point, but that might not be the case based on the above comments.

Copy link

github-actions bot commented Dec 6, 2023

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working effort/medium This issue will take a few days of effort to fix p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants