Skip to content

Commit

Permalink
Add runner-level config to enforce PR security
Browse files Browse the repository at this point in the history
This allows self-hosted runners to limit who can run jobs to give "just
enough protection" to allow using self-hosted runners with public repos.

By default, the current behaviour is unchanged -- all jobs passed to the
runner are executed.

If the `.runner` config file has this block added to it:

```
  "pullRequestSecurity": {}
```

Then by only PRs from "CONTRIBUTORS" and "OWNERS" can run (as defined by
the field in
https://docs.github.com/en/free-pro-team@latest/graphql/reference/objects#pullrequest
-- nothing for us to have to work out ourselves.)

It is also possible to explicitly list users that are allowed to run
jobs on this worker:

```
  "pullRequestSecurity": {
    "allowedAuthors": ["ashb"]
  }
```

Or to _only_ allow the given users, but not all contributors:

```
  "pullRequestSecurity": {
    "allowContributors": false,
    "allowedAuthors": ["ashb"]
  }
```

Owners of the repo are always allowed to run jobs.

If an allowed user pushes a commit to a PR from a non-allowed user, than
_that_ build will be allowed to run on the self-hosted runner.
  • Loading branch information
ashb committed Sep 10, 2023
1 parent 3f5b813 commit 1448196
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 0 deletions.
16 changes: 16 additions & 0 deletions src/Runner.Common/ConfigurationStore.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using GitHub.Runner.Sdk;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.Serialization;
Expand Down Expand Up @@ -56,6 +57,9 @@ public sealed class RunnerSettings
[DataMember(EmitDefaultValue = false)]
public string ServerUrlV2 { get; set; }

[DataMember(Name = "PullRequestSecurity", EmitDefaultValue = false)]
public PullRequestSecuritySettings PullRequestSecuritySettings { get; set; }

[IgnoreDataMember]
public bool IsHostedServer
{
Expand Down Expand Up @@ -110,6 +114,18 @@ private void OnSerializing(StreamingContext context)
}
}

[DataContract]
public sealed class PullRequestSecuritySettings
{
// pullRequestSecurity is optional in the config -- if the key is
// defined, assume that we only want collaborators to run PRs.
[DataMember(EmitDefaultValue = false)]
public HashSet<string> AllowedAuthors = new HashSet<string>();

[DataMember(EmitDefaultValue = false)]
public bool AllowContributors = true;
}

[ServiceLocator(Default = typeof(ConfigurationStore))]
public interface IConfigurationStore : IRunnerService
{
Expand Down
11 changes: 11 additions & 0 deletions src/Runner.Worker/GitHubContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,16 @@ public GitHubContext ShallowCopy()

return copy;
}

public bool IsPullRequest()
{
PipelineContextData data;
if (TryGetValue("event_name", out data))
{
var eventName = data as StringContextData;
return eventName == "pull_request";
}
return false;
}
}
}
104 changes: 104 additions & 0 deletions src/Runner.Worker/JobRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading.Tasks;
using GitHub.DistributedTask.ObjectTemplating.Tokens;
using GitHub.DistributedTask.Pipelines;
using GitHub.DistributedTask.Pipelines.ContextData;
using GitHub.DistributedTask.WebApi;
using GitHub.Runner.Common;
using GitHub.Runner.Common.Util;
Expand Down Expand Up @@ -110,6 +111,21 @@ public async Task<TaskResult> RunAsync(AgentJobRequestMessage message, Cancellat
jobContext.InitializeJob(message, jobRequestCancellationToken);
Trace.Info("Starting the job execution context.");
jobContext.Start();

if (!JobPassesSecurityRestrictions(jobContext))
{
var configurationStore = HostContext.GetService<IConfigurationStore>();
RunnerSettings settings = configurationStore.GetSettings();

var issue = new Issue()
{
Type = IssueType.Error,
Message = $"Running job on worker {settings.AgentName} disallowed by security policy"
};
jobContext.AddIssue(issue, ExecutionContextLogOptions.Default);
return await CompleteJobAsync(server, jobContext, message, TaskResult.Failed);
}

jobContext.Debug($"Starting: {message.JobDisplayName}");

runnerShutdownRegistration = HostContext.RunnerShutdownToken.Register(() =>
Expand Down Expand Up @@ -505,5 +521,93 @@ private async Task ShutdownQueue(bool throwOnFailure)
}
}
}

private bool JobPassesSecurityRestrictions(IExecutionContext jobContext)
{
var gitHubContext = jobContext.ExpressionValues["github"] as GitHubContext;

try {
if (gitHubContext.IsPullRequest())
{
return OkayToRunPullRequest(gitHubContext);
}

return true;
}
catch (Exception ex)
{
Trace.Error("Caught exception in JobPassesSecurityRestrictions");
Trace.Error("As a safety precaution we are not allowing this job to run");
Trace.Error(ex);
return false;
}
}

private bool OkayToRunPullRequest(GitHubContext gitHubContext)
{
var configStore = HostContext.GetService<IConfigurationStore>();
var settings = configStore.GetSettings();
var prSecuritySettings = settings.PullRequestSecuritySettings;

if (prSecuritySettings is null) {
Trace.Info("No pullRequestSecurity defined in settings, allowing this build");
return true;
}

var githubEvent = gitHubContext["event"] as DictionaryContextData;
var prData = githubEvent["pull_request"] as DictionaryContextData;

var authorAssociation = prData.TryGetValue("author_association", out var value)
? value as StringContextData : null;


// TODO: Allow COLLABORATOR, MEMBER too -- possibly by a config setting
if (authorAssociation == "OWNER")
{
Trace.Info("PR is opened by a repo owner, always allowed");
return true;
}
else if (prSecuritySettings.AllowContributors && authorAssociation == "COLLABORATOR") {
Trace.Info("PR is from the repo collaborator, allowing");
return true;
}

// pull_request.user.login is the user who opened the PR. Always fixed
// Actor is the user who performed the push/open/close. For
// predictability we validate based on the PR opener, so we don't
// get surprised when re-opening an issue causes it to run on
// self-hosted runners.
var prUser = prData["user"] as DictionaryContextData;

if (prUser == null)
{
Trace.Info("Unable to get PR user, not allowing PR to run");
return false;
}

var login = prUser.TryGetValue("login", out value)
? value as StringContextData : null;

Trace.Info($"GitHub PR.user.login is {login as StringContextData}");

if (login == null)
{
Trace.Info("Unable to get PR login, not allowing PR to run");
return false;
}

if (prSecuritySettings.AllowedAuthors.Contains(login))
{
Trace.Info("Author in PR allowed list");
return true;
}
else
{
Trace.Info($"Not running job as actor ({login}) is not in {{{string.Join(", ", prSecuritySettings.AllowedAuthors)}}}");

return false;
}
}

}
}

0 comments on commit 1448196

Please sign in to comment.