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

Injecting netrc credentials into non-trusted containers does not work #2583

Closed
5 tasks done
pat-s opened this issue Oct 14, 2023 · 4 comments
Closed
5 tasks done

Injecting netrc credentials into non-trusted containers does not work #2583

pat-s opened this issue Oct 14, 2023 · 4 comments
Labels
enhancement improve existing features

Comments

@pat-s
Copy link
Contributor

pat-s commented Oct 14, 2023

Component

agent

Describe the bug

By default netrc credentials (i.e. git credentials) are not injected into builds unless

  • the repo is set to trusted (which can only be done by admins)
  • "Only inject netrc credentials into trusted containers" is unchecked

However, the latter does not work as executing a "git push" back to the checked out repo does not work when the option is unchecked.
It only works if the repo is set to "trusted" - which cannot be enabled by a normal user.

Showcasing this in an example repo is hard as normal users cannot open the settings of a repo.
I've tested this multiple times in an example repo toggling different options on and off and running a simple git push at the end (after a dummy modification).

Also I think the current docs are wrong

Cloning pipeline step may need git credentials. They are injected via netrc. By default, they're only injected if this option is enabled, the repo is trusted (see above) or the image is a trusted clone image. If you uncheck the option, git credentials will be injected into any container in clone step.

I think it should read as:

"By default, they're only injected if this option is enabled and the repo is trusted - or if the "inject" option is unchecked."

Also "git credentials will be injected into any container in clone step" -> should probably be "into any container in addition to the 'clone' step".

System Info

`next-8629a418f8`

Additional context

No response

Validations

  • Read the Contributing Guidelines.
  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • Checked that the bug isn't fixed in the next version already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]
  • Check that this is a concrete bug. For Q&A join our Discord Chat Server or the Matrix room.
@pat-s pat-s added bug Something isn't working agent forge/gitea gitea forge related labels Oct 14, 2023
@pat-s
Copy link
Contributor Author

pat-s commented Oct 14, 2023

Diving deeper, I think the issue is that the "Only inject netrc credentials into trusted containers" option actually is limited to "trusted pipelines" or "trusted plugins" and "trusted clone images" but never applies to arbitrary steps and/or images..

// only inject netrc if it's a trusted repo or a trusted plugin
if !c.netrcOnlyTrusted || c.trustedPipeline || (container.IsPlugin() && container.IsTrustedCloneImage()) {
for k, v := range c.cloneEnv {
step.Environment[k] = v
}
}

Should the restriction to "trusted plugins" and "trusted clone images" be removed? I don't see why only these should be whitelisted if the repo owner actively deactivated the "Only inject netrc credentials into trusted containers" option.
Also if, then the option should read differently and or inform the user that there is a follow-p condition to be met.

@pat-s
Copy link
Contributor Author

pat-s commented Oct 14, 2023

It might be that there only a minor modification to the if-clause needed (see PR) which currently prevents the injection into untrusted containers.

@qwerty287 qwerty287 removed forge/gitea gitea forge related agent labels Nov 3, 2023
@6543 6543 added enhancement improve existing features and removed bug Something isn't working labels Nov 7, 2024
@6543
Copy link
Member

6543 commented Nov 7, 2024

As of now this is intentional and by default it should stay so, so enabling this behaviour a 'normal' repo admin should not be able to change only instance admins ...

@pat-s
Copy link
Contributor Author

pat-s commented Nov 10, 2024

Closing as not planned -> description in UI updated in #4342.

@pat-s pat-s closed this as not planned Won't fix, can't repro, duplicate, stale Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants