Skip to content

Commit

Permalink
Merge pull request #2471 from jakubm-canva/jkb-allowList
Browse files Browse the repository at this point in the history
Add allowed-plugin param to enable plugins allow-list
  • Loading branch information
moskyb committed Nov 2, 2023
2 parents 2646187 + 6f97940 commit a3c3ca8
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 28 deletions.
4 changes: 3 additions & 1 deletion agent/agent_configuration.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package agent

import (
"regexp"
"time"

"github.com/lestrrat-go/jwx/v2/jwk"
Expand All @@ -24,7 +25,8 @@ type AgentConfiguration struct {
GitCleanFlags string
GitFetchFlags string
GitSubmodules bool
AllowedRepositories []string
AllowedRepositories []*regexp.Regexp
AllowedPlugins []*regexp.Regexp
SSHKeyscan bool
CommandEval bool
PluginsEnabled bool
Expand Down
32 changes: 27 additions & 5 deletions agent/job_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package agent

import (
"context"
"encoding/json"
"fmt"
"io"
"os"
Expand All @@ -15,6 +16,7 @@ import (
"github.com/buildkite/agent/v3/api"
"github.com/buildkite/agent/v3/internal/experiments"
"github.com/buildkite/agent/v3/internal/job/shell"
"github.com/buildkite/agent/v3/internal/pipeline"
"github.com/buildkite/agent/v3/kubernetes"
"github.com/buildkite/agent/v3/logger"
"github.com/buildkite/agent/v3/metrics"
Expand Down Expand Up @@ -561,17 +563,37 @@ func (w LogWriter) Write(bytes []byte) (int, error) {
return len(bytes), nil
}

func validateRepository(allowedRepos []string, pipelineRepo string) error {
if len(allowedRepos) == 0 {
// validateJobValue returns an error if a job value doesn't match
// any allowed patterns.
func validateJobValue(allowedPatterns []*regexp.Regexp, jobValue string) error {
if len(allowedPatterns) == 0 {
return nil
}

for _, allowedRepo := range allowedRepos {
if match, _ := regexp.MatchString(allowedRepo, pipelineRepo); match {
for _, re := range allowedPatterns {
if match := re.MatchString(jobValue); match {
return nil
}
}
return fmt.Errorf("repository %s isn't allowed", pipelineRepo)
return fmt.Errorf("%s has no match in %s", jobValue, allowedPatterns)
}

// checkPlugins unmarshal and validates the plugins, if the list of allowed plugins is set.
// Disabled plugins or errors in json.Unmarshal will by-pass the plugin verification.
func (r *JobRunner) checkPlugins(ctx context.Context) error {
if !r.conf.AgentConfiguration.PluginsEnabled {
return nil
}
var ps pipeline.Plugins
pluginsVar := r.conf.Job.Env["BUILDKITE_PLUGINS"]
if err := json.Unmarshal([]byte(pluginsVar), &ps); len(pluginsVar) > 0 && err == nil {
for _, plugin := range ps {
if err := validateJobValue(r.conf.AgentConfiguration.AllowedPlugins, plugin.Source); err != nil {
return err
}
}
}
return nil
}

func (r *JobRunner) executePreBootstrapHook(ctx context.Context, hook string) (bool, error) {
Expand Down
45 changes: 30 additions & 15 deletions agent/job_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package agent

import (
"fmt"
"regexp"
"strings"
"testing"

Expand All @@ -19,21 +20,35 @@ func TestTruncateEnv(t *testing.T) {
assert.Equal(t, 64, len(fmt.Sprintf("FOO=%s\000", env["FOO"])))
}

func TestValidateRepository(t *testing.T) {
pipelineRepo := "github.com/buildkite/test"
allowedRepos := []string{}
func TestValidateJobValue(t *testing.T) {
bkTarget := "github.com/buildkite/test"
bkTargetRe := regexp.MustCompile("^github.com/buildkite/.*")
ghTargetRe := regexp.MustCompile("^github.com/nope/.*")

// empty allow list
err := validateRepository(allowedRepos, pipelineRepo)
require.NoError(t, err)

// not allowed
allowedRepos = append(allowedRepos, "^github.com/nope/.*")
err = validateRepository(allowedRepos, pipelineRepo)
require.Error(t, err)
tests := []struct {
name string
allowedTargets []*regexp.Regexp
pipelineTarget string
wantErr bool
}{{
name: "No error. Allowed targets no configured.",
allowedTargets: []*regexp.Regexp{},
pipelineTarget: bkTarget,
}, {
name: "No pipeline target match",
allowedTargets: []*regexp.Regexp{ghTargetRe},
pipelineTarget: bkTarget,
wantErr: true,
}, {
name: "Pipeline target match",
allowedTargets: []*regexp.Regexp{ghTargetRe, bkTargetRe},
pipelineTarget: bkTarget,
}}

// allowed
allowedRepos = append(allowedRepos, "^github.com/buildkite/.*")
err = validateRepository(allowedRepos, pipelineRepo)
require.NoError(t, err)
for _, tc := range tests {
err := validateJobValue(tc.allowedTargets, tc.pipelineTarget)
if (err != nil) != tc.wantErr {
t.Errorf("validateJobValue() error = %v, wantErr = %v", err, tc.wantErr)
}
}
}
17 changes: 12 additions & 5 deletions agent/run_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,20 @@ func (r *JobRunner) Run(ctx context.Context) error {
}
}

// Validate the repository if the list of allowed repositories is set
allowedRepos := r.conf.AgentConfiguration.AllowedRepositories
pipelineRepo := job.Env["BUILDKITE_REPO"]
err := validateRepository(allowedRepos, pipelineRepo)
// Validate the repository if the list of allowed repositories is set.
err := validateJobValue(r.conf.AgentConfiguration.AllowedRepositories, job.Env["BUILDKITE_REPO"])
if err != nil {
r.logStreamer.Process([]byte(fmt.Sprintf("%s", err)))
r.logger.Error("%s", err)
r.logger.Error("Repo %s", err)
exit.Status = -1
exit.SignalReason = SignalReasonAgentRefused
return nil
}
// Validate the plugins if the list of allowed plugins is set.
err = r.checkPlugins(ctx)
if err != nil {
r.logStreamer.Process([]byte(fmt.Sprintf("%s", err)))
r.logger.Error("Plugin %s", err)
exit.Status = -1
exit.SignalReason = SignalReasonAgentRefused
return nil
Expand Down
37 changes: 35 additions & 2 deletions clicommand/agent_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"os"
"os/signal"
"path/filepath"
"regexp"
"runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -134,6 +135,7 @@ type AgentStartConfig struct {
NoPluginValidation bool `cli:"no-plugin-validation"`
NoFeatureReporting bool `cli:"no-feature-reporting"`
AllowedRepositories []string `cli:"allowed-repositories" normalize:"list"`
AllowedPlugins []string `cli:"allowed-plugins" normalize:"list"`

HealthCheckAddr string `cli:"health-check-addr"`

Expand Down Expand Up @@ -211,6 +213,10 @@ func (asc AgentStartConfig) Features(ctx context.Context) []string {
features = append(features, "allowed-repositories")
}

if len(asc.AllowedPlugins) > 0 {
features = append(features, "allowed-plugins")
}

for _, exp := range experiments.Enabled(ctx) {
features = append(features, fmt.Sprintf("experiment-%s", exp))
}
Expand Down Expand Up @@ -542,9 +548,15 @@ var AgentStartCommand = cli.Command{
cli.StringSliceFlag{
Name: "allowed-repositories",
Value: &cli.StringSlice{},
Usage: "A comma-separated list of regular expressions representing repositories the agent is allowed to clone (for example, \"^git@github.com:buildkite/.*\" or \"^https://github.com/buildkite/.*\")",
Usage: `A comma-separated list of regular expressions representing repositories the agent is allowed to clone (for example, "^git@github.com:buildkite/.*" or "^https://github.com/buildkite/.*")`,
EnvVar: "BUILDKITE_ALLOWED_REPOSITORIES",
},
cli.StringSliceFlag{
Name: "allowed-plugins",
Value: &cli.StringSlice{},
Usage: `A comma-separated list of regular expressions representing plugins the agent is allowed to use (for example, "^buildkite-plugins/.*$" or "^/var/lib/buildkite-plugins/.*")`,
EnvVar: "BUILDKITE_PLUGINSS",
},
cli.BoolFlag{
Name: "metrics-datadog",
Usage: "Send metrics to DogStatsD for Datadog",
Expand Down Expand Up @@ -850,7 +862,6 @@ var AgentStartCommand = cli.Command{
PluginsEnabled: !cfg.NoPlugins,
PluginValidation: !cfg.NoPluginValidation,
LocalHooksEnabled: !cfg.NoLocalHooks,
AllowedRepositories: cfg.AllowedRepositories,
StrictSingleHooks: cfg.StrictSingleHooks,
RunInPty: !cfg.NoPTY,
ANSITimestamps: !cfg.NoANSITimestamps,
Expand Down Expand Up @@ -939,6 +950,28 @@ var AgentStartCommand = cli.Command{
l.Info("Agents will disconnect after %d seconds of inactivity", agentConf.DisconnectAfterIdleTimeout)
}

if len(cfg.AllowedRepositories) > 0 {
agentConf.AllowedRepositories = make([]*regexp.Regexp, 0, len(cfg.AllowedRepositories))
for _, v := range cfg.AllowedRepositories {
r, err := regexp.Compile(v)
if err != nil {
l.Fatal("Regex %s is allowed-repositories failed to compile: %v", v, err)
}
agentConf.AllowedRepositories = append(agentConf.AllowedRepositories, r)
}
}

if len(cfg.AllowedPlugins) > 0 {
agentConf.AllowedPlugins = make([]*regexp.Regexp, 0, len(cfg.AllowedPlugins))
for _, v := range cfg.AllowedPlugins {
r, err := regexp.Compile(v)
if err != nil {
l.Fatal("Regex %s in allowed-plugins failed to compile: %v", v, err)
}
agentConf.AllowedPlugins = append(agentConf.AllowedPlugins, r)
}
}

cancelSig, err := process.ParseSignal(cfg.CancelSignal)
if err != nil {
return fmt.Errorf("failed to parse cancel-signal: %w", err)
Expand Down

0 comments on commit a3c3ca8

Please sign in to comment.