Deprecate secrets section

[lafriks]

There is imho not really much sense to have two ways to do the same thing and there has already been too much magic with secrets uppercase/lowercase etc, can't be used with plugins also, so imho it should be deprecated and removed in future in favor of from_secret syntax

[xoxys]

How would use cases like this https://woodpecker-ci.org/docs/next/usage/secrets#use-secrets-in-commands work then? Also, after we made it impossible to set environment on plugins anymore this is the only remaining way to pass additional secrets that must be set as env var to a plugin.

[xoxys]

We can not permanently leave things aside.

We should not deprecate features that are used and without an alternative solution. Right now, there is no other way to pass custom envs to plugins anymore. Envs don't work anymore with the latest release and people are already asking for guidance ... If we can't provide alternative solutions, stop deprecating stuff.

I think secrets should be treated the same as env anyway from security perspective

Why? Secrets can only be set by admins, no need to dictate admins what env vars they are allowed to set.

[zc-devs]

guidance

If I understand correctly, plugins are set up via settings only (now). So, guidance is simple: push the plugin's maintainers to add required settings/options :) But again it is just my perception.

[xoxys]

I don't see any value in this behavior to be honest. If someone has the rights to change env vars in a CI config, the one can also change the entire step or add a new one. In this new step, everything can be executed again. Why we try to prevent to set any env var manually in plugins is unclear to me...

[lafriks]

Because plugins can expose secrets that otherwise would be protected by plugin functionality

[zc-devs]

environment:
  docker_username:
    from_secret: docker_password
  docker_username: johnny

at least it looks similar to Kubernetes

containers:
- envFrom:
  - configMapRef:
        name: gitea
  - secretRef:
        name: gitea

containers:
- env:
  - name: BACKEND_USERNAME
    valueFrom:
      configMapKeyRef:
        name: backend-user
        key: backend-username
  - name: DB_PASSWORD
    valueFrom:
      secretKeyRef:
        name: db-pwd
        key: db-pwd

https://kubernetes.io/docs/tasks/inject-data-application/distribute-credentials-secure/
https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/

[qwerty287]

Not everything in this thread seems to be correct, so I'd like to clean up some things - if I understand something wrong, please ask again :)

In general I agree to @lafriks to drop this section.

First of all, I'd like to make clear what a plugin is: A plugin is basically just a docker container that has a certain, pinned entrypoint and the settings. In general, a plugin should only be allowed to be configured with options that were allowed by the authors, i.e. that were exposed via a setting.

However, it's actually not that important whether something is a plugin or not internally. Internally, this is only used to check whether a clone container gets credentials (and that's not what we're discussing about currently) and to check whether a plugin is escalated and will therefore get privileged=true. Everything else is not important. If you don't need these features, everything will work as always, even though it's not classified as plugin internally. Plugins are just docker containers.

If you can set any env var now, it no longer is a plugin because you're trying to access a feature that was not exposed by the author.

Secrets basically are env vars, and the only reason that they are still allowed on plugins is actually a security bug that should be fixed.

So: on plugins secrets should be disallowed anyways. For regular containers, we can then also drop that as you can just use environent. If you need to inject a secret as env var into a plugin, you should use a setting. Otherwise, it's no longer a plugin as explained above.

I don't see any value in this behavior to be honest. If someone has the rights to change env vars in a CI config, the one can also change the entire step or add a new one. In this new step, everything can be executed again. Why we try to prevent to set any env var manually in plugins is unclear to me...

You can change it, yes, but if you change it in a way it is no longer a plugin (e.g. custom env), the container does not get privileged=true as it gets without the changed configuration.
Any container that uses env vars (and secrets), commands or entrypoints is not a plugin. Privileged mode should only be enabled for them if explicitly requested which requires the repo to be trusted.

[qwerty287]

I actually didn't know that it's impossible to use environment and settings, sorry for my misunderstanding here. I opened #4027 please continue there for this discussion

[pat-s]

So to understand that correctly: in a normal container I would use environment: in the future to pass secrets? And they will also be masked accordingly in the logs?

I understand why plugins shouldn't allow for arbitrary secrets for security reasons. However, I think this will often lead to plugins adding a section that will allow for arbitrary "extra-params" or "extra-env" (https://codeberg.org/woodpecker-plugins/ansible/pulls/27). For some this is even needed, otherwise they can't operate, as users must have a way to pass arbitrary information (e.g. in ansible (vars) or docker (build_args)).
Isn't this then effectively the same as allowing arbitrary secrets in the end, as they can't be asserted within the plugin itself, at least not other than "is a list/map"?

[qwerty287]

So to understand that correctly: in a normal container I would use environment: in the future to pass secrets?

Yes.

And they will also be masked accordingly in the logs?

That's a good question, I have to check it out. It should be masked of course.

I understand why plugins shouldn't allow for arbitrary secrets for security reasons. However, I think this will often lead to plugins adding a section that will allow for arbitrary "extra-params" or "extra-env" (https://codeberg.org/woodpecker-plugins/ansible/pulls/27). For some this is even needed, otherwise they can't operate, as users must have a way to pass arbitrary information (e.g. in ansible (vars) or docker (build_args)). Isn't this then effectively the same as allowing arbitrary secrets in the end, as they can't be asserted within the plugin itself, at least not other than "is a list/map"?

In some cases, this can be the same (if these are set an env vars in the end). If the plugin is implemented properly and the software is used internally allows this, these extra params should not be set as env var, but rather be given directly and only for this purpose to it. So Ansible vars should only be used as Ansible vars directly and explicitly given to ansible therefore, but they shouldn't be used for anything else.

If the software doesn't allow this, you're right, it's indeed the same.

[6543]

And they will also be masked accordingly in the logs?

That's a good question, I have to check it out. It should be masked of course.

they will , currently the agent just gets all secrets of the repo whitch a workflow run in told and will mask them if they pop up.

this mechanism works, but i wana limit the secrets exposure so I'm currently working out a new way: #4384 (expect some time before it is ready)