woodpecker-ci / woodpecker

Woodpecker is a simple, yet powerful CI/CD engine with great extensibility.
https://woodpecker-ci.org
Apache License 2.0
4.29k stars 369 forks source link

Show secrets #1552

Closed simmstein closed 8 months ago

simmstein commented 1 year ago

Clear and concise description of the problem

Secrets cannot be displayed after they have been saved.

Suggested solution

Add a button to show/hide secrets.

Alternative

No response

Additional context

No response

Validations

lafriks commented 1 year ago

This is kind a tricky one as it could pose security threats (it's not shown for a reason). Correctly this would be implemented to ask for password but this is not really option for us...

simmstein commented 1 year ago

For now, we can't edit a secret wihout set the value: https://github.com/woodpecker-ci/woodpecker/blob/master/web/src/components/secrets/SecretEdit.vue#L14

The API removes the secret value: https://github.com/woodpecker-ci/woodpecker/blob/master/server/api/repo_secret.go#L124

simmstein commented 1 year ago

@lafriks some CI sytems like Gitlab allow to show secrets. That could be an option to allow it.

6543 commented 1 year ago

why would we weaken the security ?!?

if you have a secret saved that is not saved somewhere else, it's either a token that should be regenerated anyway if you change something, or a secret that you should have managed in some password-manager anyway ...

6543 commented 1 year ago

well that github does not follow best-practice in security from time to time and prioritize convenience is not something we should adapt - in my opinion.

If you really need to extract the secret well, just make an SQL call as admin ... that should be a fair enough burden to only be the last resort but still be possible

simmstein commented 1 year ago

if you have a secret saved that is not saved somewhere else, it's either a token that should be regenerated anyway if you change something, or a secret that you should have managed in some password-manager anyway ...

Show secrets is useful when you want to:

JakobDev commented 1 year ago

I suggest adding a a Checkbox "The value of this secret can be revealed" when creating a Secret. People who want to view the Content afterwards (like me), can check it, other who don't want it, don't check it. So both sides will be happy.

gapodo commented 1 year ago

IMHO allowing changes to secrets or revealing them is a horrible decision from a security standpoint.

For now, we can't edit a secret wihout set the value: https://github.com/woodpecker-ci/woodpecker/blob/master/web/src/components/secrets/SecretEdit.vue#L14

This alone opens the door for abuse, as you could remove a plugin requirement or permitted image and by doing this exfiltrate the secrets value...

Scenario

To show why it's a bad Idea, I'm painting a scenario here... (it's a case that could/would apply to a project I'm participating in)

The project has different people with different levels of access to things that go into secrets. Not all Admins on the repo have full access to those secrets (think stuff like signing keys).

If someone managing the signing key adds the secret, limits its usage to plugin and sets the image, as long as the image is secure, you can't exfiltrate the key.

Now lets assume (which sadly already happened) the secret is editable without the need to reenter the value: Another admin can go in, change the setting of that secret, and e.g. allow the use on pull_requests, remove the plugin only and / or image restriction. Now it's possible for that user to push (or just PR) a pipeline, that accesses that secret, but allows them to exfiltrate the value (base64 encode so the log doesn't masquarade it, upload to a http connection...)

Conclusion / thoughts

Edit: forgot that I wanted to comment on this as well...

I suggest adding a a Checkbox "The value of this secret can be revealed" when creating a Secret. People who want to view the Content afterwards (like me), can check it, other who don't want it, don't check it. So both sides will be happy.

This could be an option, though I severely like the base idea (it's a secret not a notepad), as long as it is possible to completely disable it on the instance level (should be the default, as that's the secure version) this could work.

Edit2: As I've just stumbled across this case in a completely unrelated setup (no woodpecker, not forge,...):

Even only allowing the user that created the secret to edit/display it without needing to provide it, may already cause problems. These may appear in a separation of concerns setup, where one person is allowed to edit the CI (is admin on the repo), but has no access to the secrets, while the second person has access to the secret, but not the CI. This setup is fairly common in more secured environments, and would mean the 2 persons meet up, admin unlocks the CI, secret owner enters the secret.

JakobDev commented 1 year ago

I use Woodpecker only for my personal projects, so I'm the only one who has access to the Project, which means there is not really a security risk. Having the Option to view the Secrets can help me debugging (e.g. check if I updated my Credentials for the project after changing it). I also think we should not bother with advanced permissions models at this time. Just a Checkbox "The value of this secret can be revealed" should be enough currently. So people who use Woodpecker only for personal projects (like me) or smaller teams where everybody know the Secrets can use this feature. If you use Woodpecker in a more complex environment, than it's the best to just keep the Secrets hidden in Woodpecker UI.

whysthatso commented 1 year ago

i would like to second the opposition to such a feature. the ci system is not a secret management tool, it is supposed to blackbox entered values. it is trivial to do so, hence debugging by adding different values in the editor (which shows the values initially) should be sufficient.

any option to retrospectively view secrets weakens the security model and raises all kinds of flags when used in regulated environments (think PCI-DSS, etc.).

please keep the system as simple as it currently is, it does not need to be easy.

6543 commented 1 year ago

-> #1556 we add a simple endpoint, that only is active if configured by the server admin on startup

qwerty287 commented 8 months ago

Closing as of https://github.com/woodpecker-ci/woodpecker/pull/1556#issuecomment-2009758558