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

Wrong possible id-token permissions types listed #33483

Closed
1 task done
cmaster11 opened this issue Jun 13, 2024 · 12 comments · Fixed by #34306
Closed
1 task done

Wrong possible id-token permissions types listed #33483

cmaster11 opened this issue Jun 13, 2024 · 12 comments · Fixed by #34306
Labels
actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team SME reviewed An SME has reviewed this issue/PR

Comments

@cmaster11
Copy link

cmaster11 commented Jun 13, 2024

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs

What part(s) of the article would you like to see updated?

In the Defining access for the GITHUB_TOKEN scopes section, the id-token permission is listed with read|write|none, but that permission cannot be set to read.

Trying to do so will produce:

image

Probably you want to list write|none?

I see in https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect#understanding-the-oidc-token that it also lists the read permission and a thread that mentions it. Is the read permission available ONLY in specific conditions?

Additional information

No response

@cmaster11 cmaster11 added the content This issue or pull request belongs to the Docs Content team label Jun 13, 2024
Copy link

welcome bot commented Jun 13, 2024

Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Jun 13, 2024
@nguyenalex836 nguyenalex836 added actions This issue or pull request should be reviewed by the docs actions team waiting for review Issue/PR is waiting for a writer's review and removed triage Do not begin working on this issue until triaged by the team labels Jun 13, 2024
@nguyenalex836
Copy link
Contributor

@cmaster11 Thank you for opening this issue! I'll get this triaged for review ✨

@TrickTurner

This comment was marked as spam.

@Sa-su121

This comment was marked as spam.

@janbrasna
Copy link
Contributor

janbrasna commented Jul 14, 2024

My guess is that it can become "read" implicitly in some scenarios, but can't be set to "read" by the authors deliberately. See #26481 (comment)


TL;DR: — Originally posted by @janbrasna in #32320 (comment):

I don't think this topic needs a new issue every single time a stale bot closes out the previous one;) I'm afraid it needs a better documentation explaining what the actual permission setting is and what it is for (not the whole OIDC workflow, that one clearly states when you need the permission, own-org vs. outside-org reusables etc. — but what the actual values for that permission mean, and if/when/how they can be set). Ex without the great writeup about OIDC use for deploy-pages I could have only guessed why such permission is necessary to grant, without any more explanation than to just blindly follow the boilerplate.

(In case of deploy-pages, the token here contains more info about the source event/repo protections wrt the environment to be deployed to, in a standardized way, that's not implicit on GITHUB_TOKEN, and GH apparently chose to consume their own OIDC mechanism instead of getting that info via GH API using the default token scope I guess…)

Connecting to cloud providers like AWS or GCP is pretty well documented here, but the issues cropping up repeatedly are about the lack of documentation for:

  1. why GH itself needs the permission (and OIDC flow to start with) for Pages deployment (only mentioned by the authors above),
  2. what does the "read" permission really mean, and why it can't be set, even though it's supposedly a valid value being used by default in some scenarios (and if that's a maximum permission for PRs from forks, what that means in real life, compared to "none").

I see it being rather unpopular topic with how often it ends up closed out by stale bot, but perhaps it's too obvious for engineers architecting the functionality, but without any visibility into how that values are then used for what in GHA code, we the end users are then left in the dark to just blindly trust the actions' instructions, or keep running in circles between unresolved issues trying to asses the potential impact this setting could have if not used correctly.

@nguyenalex836 nguyenalex836 added the needs SME This proposal needs review from a subject matter expert label Jul 18, 2024
Copy link
Contributor

Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert 👀

@joshmgross
Copy link
Member

👋 Hello from Actions Engineering,

I can confirm that read permissions are not valid for id-token. You can see this defined in our workflow schema where read is not a valid option:

          "id-token": {
            "type": "permission-level-write-or-no-access",
            "description": "Token to request an OpenID Connect token."
          },

https://github.com/actions/languageservices/blob/83bddd3332cb4dc988ded6784719527765619404/workflow-parser/src/workflow-v1.0.json#L1538-L1541

This permission is only used for creating OIDC tokens, there are no resources to read.

@nguyenalex836 nguyenalex836 added SME reviewed An SME has reviewed this issue/PR and removed waiting for review Issue/PR is waiting for a writer's review needs SME This proposal needs review from a subject matter expert labels Aug 14, 2024
@janbrasna
Copy link
Contributor

@joshmgross 🤙 Yo, thanks for confirming! May I ask what happens if one sets permissions: read-all? Does that skip over those that can't be set to read, or will still try setting the id-token too to "read" and fail?

@janbrasna
Copy link
Contributor

janbrasna commented Aug 14, 2024

Now it is clear it can't be set to "read", but there's still the confusion about why it is documented (and confirmed by staff on several occasions) to default to "read" in some cases, see: automatic-token-authentication#permissions-for-the-github_token ("Maximum access for pull requests from public forked repositories")

@joshmgross
Copy link
Member

👋 Hey @janbrasna, sorry for the confusion there.

permissions: read-all effectively means that id-token is set to none since read is not a valid permission.

That "Maximum access for pull requests from public forked repositories" section should list none for id-token.

@joshmgross
Copy link
Member

(and #32320 (comment) on #14626 (comment))

For reusable workflows outside the org and also for pull requests from public forked repos , default value issued to id-token is "read" to be safe and restrictive. Users could change it to "write" if they want to explicitly grant permissions for the workflow to request an OIDC JWT.

That should be "default value issued to id-token is "none""

There is no difference between a workflow having id-token: read and id-token: none.
#14626 (comment)

There's no functional difference between these two because there are no id-token resources to read, so the permission is just none. That being said, you can't explicitly set id-token to read.

@janbrasna
Copy link
Contributor

janbrasna commented Aug 14, 2024

Lovely. Thanks for confirming. That finally makes sense and I thought this should be the case, but didn't want to make the call (esp. since it's been around like this basically from its inception ~3yrs ago…;))

@joshmgross If I can be so bold, I'd love a fact-check from such an SME on PR #34306 reflecting this if you wouldn't mind someday… Appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team SME reviewed An SME has reviewed this issue/PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@janbrasna @cmaster11 @joshmgross @nguyenalex836 @TrickTurner @Sa-su121 and others