-
Notifications
You must be signed in to change notification settings - Fork 294
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
Add an agent config for no-local-hooks #707
Conversation
// Turning off command eval will also turn off plugins. | ||
if cfg.NoCommandEval { | ||
// Turning off command eval or local hooks will also turn off plugins. | ||
if cfg.NoCommandEval || cfg.NoLocalHooks { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm...what's your thinking here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of disabling local hooks if you can use plugins? You'd just be able to modify pipeline.yml and achieve the same thing as local hooks otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local hooks by themselves aren't dangerous because you need source code access to change them. Editing a local hook is the same as editing a build script.
The point behind no-command-eval
is stopping Buildkite executing arbitrary commands on an agent. Since you can define plugins in the BK user interface, you could circumvent no-command-eval
(which is why we turn off plugins if no-command-eval
is on).
So I don't think we need to disable plugins if no-local-hooks
is on? Unless I'm missing another attack vector?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding was that the point behind no-local-hooks
is to prevent hooks in a repository being able to override the hooks set at the global level. They are for where you don't trust the source code, for instance third-party pull requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you have a different understanding of their purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, spoke about on video with @lox. If you consider plugins to be "3rd party local hooks" then this makes total sense...!
This adds a
no-local-hooks
config for the agent. Global hooks can still setBUILDKITE_NO_LOCAL_HOOKS
on a case-by-case basis.