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

Clarify permission level needed to access secrets #1087

Closed
muru opened this issue Nov 5, 2020 · 9 comments · Fixed by #2330
Closed

Clarify permission level needed to access secrets #1087

muru opened this issue Nov 5, 2020 · 9 comments · Fixed by #2330
Labels
actions This issue or pull request should be reviewed by the docs actions team ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team help wanted Anyone is welcome to open a pull request to fix this issue

Comments

@muru
Copy link
Contributor

muru commented Nov 5, 2020

What article on docs.github.com is affected?

https://docs.github.com/en/free-pro-team@latest/actions/reference/encrypted-secrets#creating-encrypted-secrets-for-a-repository

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

In "Creating encrypted secrets for a repository," it says:

To create secrets for a user account repository, you must be the repository owner. To create secrets for an organization repository, you must have admin access.

However, according to the API docs:

Authenticated users must have collaborator access to a repository to create, update, or read secrets.

So, only admins can use the web interface to access secrets, but users with write access or above can use the API (directly, or indirectly, e.g., via Terraform). This is in fact the actual behaviour.

Additional information

This feels like a security issue, since updating secrets via the API involves not much of a record or oversight AFAICT (as opposed to, say, creating a workflow that dumps all secrets, which could leave traces that a bad actor cannot easily remove). I did report it as one, and the response is that this is intentional to allow users with write access to create and maintain workflows (which is understandable and fine by me). However, the current behaviour can be a bit surprising and the docs a bit time-wasting (I would write Terraform code that my org's admin would then apply, when I could have just applied them myself).

@welcome
Copy link

welcome bot commented Nov 5, 2020

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 Nov 5, 2020
@janiceilene
Copy link
Contributor

👋 Thanks for opening an issue @muru! Can you clarify what change you'd like to see in the "Creating encrypted secrets for a repository"? Are you hoping a line is added to that article letting folks know that anyone with write+ permissions can use the API to access secrets?

@janiceilene janiceilene added actions This issue or pull request should be reviewed by the docs actions team ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team labels Nov 6, 2020
@muru
Copy link
Contributor Author

muru commented Nov 9, 2020

@janiceilene Hmmm, yes. Or a rephrasing of the whole thing to include the reason for this particular behaviour. Something like:

Like other repository settings, secrets can be managed from the web interface only by repository owners (for user repositories) or users with admin access (for organization repositories). However, to enable developers to create and maintain workflows, users with collaborator access are allowed to use the API to manage secrets.

(Or perhaps with the last sentence as a {% note %}.)

@janiceilene
Copy link
Contributor

👍 Thanks for the clarification @muru! I'll leave it to the team to respond further.

@runleonarun runleonarun added internal-fix-openapi help wanted Anyone is welcome to open a pull request to fix this issue and removed internal-fix-openapi triage Do not begin working on this issue until triaged by the team labels Dec 22, 2020
@runleonarun
Copy link
Contributor

This sounds great, @muru! I added the "Help wanted" so someone can go ahead and fix this!

muru added a commit to muru/docs that referenced this issue Dec 25, 2020
Closes github#1087

I considered changing the `permissions-statement-secrets-repository`
reusable to include a reference to the API, but then I noticed that the
other place using it (["Enabling debug logging"][1]) already mentioned
the API, so instead I added a note. Including a mention of "web
interface" lead to (IMO) too much duplication in text, so I rephrased it
to be more like the `permissions-statement-secrets-api` reusable.

[1]: https://docs.github.com/en/free-pro-team@latest/actions/managing-workflow-runs/enabling-debug-logging
@kbluck
Copy link

kbluck commented Feb 25, 2021

I would ask that it go further than updating the docs, but rather that the web behavior conform to the API behavior. It's a little obnoxious that users with suitable write access have to use the API merely to list secrets to find out their names, which are apparently hidden from them on the web interface.

@janiceilene
Copy link
Contributor

👋 @kbluck We only own the documentation on docs.github.com in this repo. The best place to send your feedback is to https://support.github.com/contact/feedback 💛

hubwriter added a commit that referenced this issue Mar 4, 2021
* Clarify access level needed for secrets in web interface

Closes #1087

I considered changing the `permissions-statement-secrets-repository`
reusable to include a reference to the API, but then I noticed that the
other place using it (["Enabling debug logging"][1]) already mentioned
the API, so instead I added a note. Including a mention of "web
interface" lead to (IMO) too much duplication in text, so I rephrased it
to be more like the `permissions-statement-secrets-api` reusable.

[1]: https://docs.github.com/en/free-pro-team@latest/actions/managing-workflow-runs/enabling-debug-logging

* Update data/reusables/github-actions/permissions-statement-secrets-repository.md

I'm going to go ahead and change this back.

Co-authored-by: hubwriter <hubwriter@github.com>
@GMNGeoffrey
Copy link

So the doc update here was actually wiped out in 95bd8c84f9, replaced with the much-less-clear "You can use the REST API to manage secrets." (followed by a link to those docs). The permission levels being different between the UI and the API is such an odd thing that it really deserves to be called out. Reading the current documentation, I would never think "oh I'm not an admin, so I can't do this from the UI, but since it says you can manage it from the API maybe I'll look and see if there are totally different access levels there". As @kbluck notes, the real issue here is that there are even different access levels (did you file feedback somewhere?), but given that there are, it would be nice if the docs actually reflected that and it's disappointing that the doc update to fix this bug was effectively reverted

jnidzwetzki pushed a commit to jnidzwetzki/docs that referenced this issue Oct 6, 2022
Co-authored-by: Lana Brindley <github@lanabrindley.com>
@nstein-gpjoule
Copy link

So the doc update here was actually wiped out in 95bd8c84f9, replaced with the much-less-clear "You can use the REST API to manage secrets." (followed by a link to those docs). The permission levels being different between the UI and the API is such an odd thing that it really deserves to be called out. Reading the current documentation, I would never think "oh I'm not an admin, so I can't do this from the UI, but since it says you can manage it from the API maybe I'll look and see if there are totally different access levels there". As @kbluck notes, the real issue here is that there are even different access levels (did you file feedback somewhere?), but given that there are, it would be nice if the docs actually reflected that and it's disappointing that the doc update to fix this bug was effectively reverted

A follow up would be great!
@kbluck is there any ticket / issue one can subscribe to?

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 ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team help wanted Anyone is welcome to open a pull request to fix this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants