diff --git a/orchestrator/github.go b/orchestrator/github.go index cfaed81..9c9dab2 100644 --- a/orchestrator/github.go +++ b/orchestrator/github.go @@ -15,9 +15,11 @@ import ( "net/url" "strconv" "strings" + "sync" "time" "github.com/golang-jwt/jwt/v5" + "golang.org/x/sync/singleflight" ) var githubClient = &http.Client{Timeout: 30 * time.Second} @@ -413,6 +415,12 @@ func rerunWorkflowJob(ctx context.Context, owner, repo string, job WorkflowJob) type workflowRun struct { Status string `json:"status"` Conclusion string `json:"conclusion"` + // HeadRepository is the repository the run's code and workflow come + // from: a fork for a pull request from one, and null once that fork is + // deleted. + HeadRepository *struct { + FullName string `json:"full_name"` + } `json:"head_repository"` } func fetchRun(ctx context.Context, owner, repo string, runID int64, installationToken string) (*workflowRun, error) { @@ -433,6 +441,70 @@ func fetchRun(ctx context.Context, owner, repo string, runID int64, installation return &run, nil } +// runSources remembers where each run comes from, which never changes for a +// run, so the jobs of one run (dozens, queued together) cost one lookup. +var runSources = &runSourceCache{sources: make(map[string]runSourceEntry), ttl: 6 * time.Hour, nowFunc: time.Now} + +type runSourceCache struct { + mu sync.Mutex + sources map[string]runSourceEntry + ttl time.Duration + nowFunc func() time.Time + group singleflight.Group +} + +type runSourceEntry struct { + source string + fetchedAt time.Time +} + +// lookupRunSource returns the full name of the repository the job's run +// comes from, or "" when GitHub no longer knows it. Failed lookups are not +// remembered. +func lookupRunSource(ctx context.Context, event WorkflowJobEvent) (string, error) { + owner, repo, runID := event.Repository.Owner.Login, event.Repository.Name, event.WorkflowJob.RunID + key := fmt.Sprintf("%s/%s#%d", owner, repo, runID) + c := runSources + c.mu.Lock() + entry, ok := c.sources[key] + c.mu.Unlock() + if ok && c.nowFunc().Sub(entry.fetchedAt) < c.ttl { + return entry.source, nil + } + + // Shared by every job of the run waiting on it, so it must outlive the + // first job's request. + fetchCtx := context.WithoutCancel(ctx) + source, err, _ := c.group.Do(key, func() (any, error) { + installationToken, err := getInstallationToken(fetchCtx, owner) + if err != nil { + return "", fmt.Errorf("get installation token: %w", err) + } + run, err := fetchRun(fetchCtx, owner, repo, runID, installationToken) + if err != nil { + return "", fmt.Errorf("look up run %d: %w", runID, err) + } + source := "" + if run.HeadRepository != nil { + source = run.HeadRepository.FullName + } + now := c.nowFunc() + c.mu.Lock() + for key, entry := range c.sources { + if now.Sub(entry.fetchedAt) >= c.ttl { + delete(c.sources, key) + } + } + c.sources[key] = runSourceEntry{source: source, fetchedAt: now} + c.mu.Unlock() + return source, nil + }) + if err != nil { + return "", err + } + return source.(string), nil +} + const ( runStatusCompleted = "completed" runConclusionCancelled = "cancelled" diff --git a/orchestrator/preemption_test.go b/orchestrator/preemption_test.go index c284082..e2da0f8 100644 --- a/orchestrator/preemption_test.go +++ b/orchestrator/preemption_test.go @@ -25,19 +25,23 @@ var ( // github stands in for api.github.com: it reports the run as runStatus // (completed when empty) with runConclusion (failure when empty and -// completed) and job 42 as jobStatus, answers the rerun request +// completed) from headRepository (null when empty), or answers the run with +// runLookupStatus when set, counting runLookups, reports job 42 as jobStatus, answers the rerun request // with rerunStatus and the check run with checkStatus, lists the run's latest // jobs from latest (name to attempts), and records what it was asked to do. type github struct { - runStatus string - runConclusion string - jobStatus string - rerunStatus int - checkStatus int - latest map[string][]int - reruns int - cancels int - checks []map[string]any + runStatus string + runConclusion string + headRepository string + runLookupStatus int + runLookups int + jobStatus string + rerunStatus int + checkStatus int + latest map[string][]int + reruns int + cancels int + checks []map[string]any } func (g *github) RoundTrip(req *http.Request) (*http.Response, error) { @@ -57,7 +61,15 @@ func (g *github) RoundTrip(req *http.Request) (*http.Response, error) { if status == "completed" && conclusion == "" { conclusion = "failure" } - return answer(http.StatusOK, fmt.Sprintf(`{"status": %q, "conclusion": %q}`, status, conclusion)) + g.runLookups++ + if g.runLookupStatus != 0 { + return answer(g.runLookupStatus, `{"message": "Server Error"}`) + } + head := "null" + if g.headRepository != "" { + head = fmt.Sprintf(`{"full_name": %q}`, g.headRepository) + } + return answer(http.StatusOK, fmt.Sprintf(`{"status": %q, "conclusion": %q, "head_repository": %s}`, status, conclusion, head)) case req.URL.Path == "/repos/appwrite-labs/cloud/actions/jobs/42": return answer(http.StatusOK, fmt.Sprintf(`{"status": %q}`, g.jobStatus)) case req.URL.Path == "/repos/appwrite-labs/cloud/actions/jobs/42/rerun": diff --git a/orchestrator/run_source_test.go b/orchestrator/run_source_test.go new file mode 100644 index 0000000..1fb1a78 --- /dev/null +++ b/orchestrator/run_source_test.go @@ -0,0 +1,118 @@ +package function + +import ( + "context" + "net/http" + "os" + "strings" + "testing" + "time" +) + +// TestMain points runSource at the job's own repository, so tests that are +// not about where a run comes from need no GitHub API for it. +func TestMain(m *testing.M) { + runSource = func(_ context.Context, event WorkflowJobEvent) (string, error) { + return event.Repository.FullName, nil + } + os.Exit(m.Run()) +} + +// withRunSourceLookup makes jobs look their run up through GitHub, starting +// from an empty cache. +func withRunSourceLookup(t *testing.T) { + t.Helper() + original := runSource + runSource = lookupRunSource + runSources.sources = map[string]runSourceEntry{} + t.Cleanup(func() { + runSource = original + runSources.sources = map[string]runSourceEntry{} + }) +} + +// A pull request from a fork runs the fork's copy of the workflow, which can +// ask for any gcrunner label, so its job gets a failed check and a cancelled +// run instead of a VM, even with a config and zones that would provision it. +// So does a run whose fork has been deleted since. +func TestARunFromAnotherRepositoryGetsNoVM(t *testing.T) { + config := "runners:\n deploy:\n family: c3d\n cpu: 4\n" + gce := engine{zones: []string{"europe-west3-a"}, catalogue: map[string][]*MachineTypeInfo{"europe-west3-a": fourVCPUs}} + for name, source := range map[string]string{"fork": "mallory/cloud", "deleted fork": ""} { + withRunSourceLookup(t) + gh := &github{checkStatus: http.StatusCreated, headRepository: source} + if code := queuedJob(t, gh, config, gce); code != http.StatusOK { + t.Errorf("%s: status %d, want 200 so Cloud Tasks does not retry", name, code) + } + if len(gh.checks) != 1 || gh.cancels != 1 { + t.Fatalf("%s: %d check runs and %d cancels, want one of each", name, len(gh.checks), gh.cancels) + } + output, _ := gh.checks[0]["output"].(map[string]any) + if summary, _ := output["summary"].(string); !strings.Contains(summary, "only runs jobs from appwrite-labs/cloud itself") { + t.Errorf("%s: check run summary %q does not say why", name, summary) + } + } +} + +func TestARunFromTheRepositoryItselfIsAllowed(t *testing.T) { + withRunSourceLookup(t) + useGitHub(t, &github{headRepository: "Appwrite-Labs/Cloud"}) + event := WorkflowJobEvent{WorkflowJob: WorkflowJob{ID: 42, RunID: 7}, Repository: cloudRepository} + if err := checkRunSource(context.Background(), event); err != nil { + t.Errorf("run from the repository itself refused: %v", err) + } +} + +// Not knowing where a run comes from is no reason to fail it: the task is +// retried. +func TestAFailedRunLookupIsRetried(t *testing.T) { + withRunSourceLookup(t) + gh := &github{checkStatus: http.StatusCreated, runLookupStatus: http.StatusBadGateway} + if code := queuedJob(t, gh, "runners: {}\n", engine{}); code != http.StatusInternalServerError { + t.Errorf("status %d, want 500 so Cloud Tasks retries", code) + } + if gh.cancels != 0 || len(gh.checks) != 0 { + t.Errorf("failed the run (%d checks, %d cancels) without knowing where it comes from", len(gh.checks), gh.cancels) + } +} + +// The jobs of one run share a lookup, until it expires. +func TestJobsOfOneRunLookItUpOnce(t *testing.T) { + withRunSourceLookup(t) + gh := &github{headRepository: "appwrite-labs/cloud"} + useGitHub(t, gh) + now := jobStart + runSources.nowFunc = func() time.Time { return now } + t.Cleanup(func() { runSources.nowFunc = time.Now }) + for id := int64(1); id <= 3; id++ { + event := WorkflowJobEvent{WorkflowJob: WorkflowJob{ID: id, RunID: 7}, Repository: cloudRepository} + if err := checkRunSource(context.Background(), event); err != nil { + t.Fatalf("job %d refused: %v", id, err) + } + } + if gh.runLookups != 1 { + t.Errorf("%d lookups for three jobs of one run, want 1", gh.runLookups) + } + now = now.Add(runSources.ttl) + if err := checkRunSource(context.Background(), WorkflowJobEvent{WorkflowJob: WorkflowJob{ID: 4, RunID: 7}, Repository: cloudRepository}); err != nil { + t.Fatal(err) + } + if gh.runLookups != 2 { + t.Errorf("%d lookups, want the expired entry looked up again", gh.runLookups) + } +} + +// A failed lookup is not remembered: the retry asks GitHub again. +func TestAFailedRunLookupIsNotRemembered(t *testing.T) { + withRunSourceLookup(t) + gh := &github{headRepository: "appwrite-labs/cloud", runLookupStatus: http.StatusBadGateway} + useGitHub(t, gh) + event := WorkflowJobEvent{WorkflowJob: WorkflowJob{ID: 42, RunID: 7}, Repository: cloudRepository} + if err := checkRunSource(context.Background(), event); err == nil { + t.Fatal("lookup failure not reported") + } + gh.runLookupStatus = 0 + if err := checkRunSource(context.Background(), event); err != nil { + t.Errorf("retry after GitHub recovered: %v", err) + } +} diff --git a/orchestrator/webhook.go b/orchestrator/webhook.go index f804e34..7bc4855 100644 --- a/orchestrator/webhook.go +++ b/orchestrator/webhook.go @@ -518,7 +518,11 @@ func provisionJob(ctx context.Context, event WorkflowJobEvent) (*RunnerLabels, e return nil, nil } - labels, err := resolveJob(ctx, event, job) + var labels *RunnerLabels + err := checkRunSource(ctx, event) + if err == nil { + labels, err = resolveJob(ctx, event, job) + } if err == nil { log.Printf("Job %d: creating VM with labels %+v", event.WorkflowJob.ID, labels) if err = provisionVM(ctx, event, labels); err == nil { @@ -612,6 +616,30 @@ var ( findZone = findInstanceZone ) +// Indirected so tests that don't look at where a run comes from need no +// GitHub API. +var runSource = lookupRunSource + +// checkRunSource refuses a job whose run comes from another repository, such +// as a pull request from a fork. That run uses the fork's copy of the +// workflow, which can ask for any gcrunner label whatever the repository's +// own workflow says, so a fork must never get a VM. A run GitHub no longer +// ties to a repository is refused too. +func checkRunSource(ctx context.Context, event WorkflowJobEvent) error { + source, err := runSource(ctx, event) + if err != nil { + return err + } + if !strings.EqualFold(source, event.Repository.FullName) { + if source == "" { + source = "a repository GitHub no longer knows" + } + return fmt.Errorf("%w: run %d comes from %s, and gcrunner only runs jobs from %s itself; give jobs from forks a GitHub-hosted runner", + errConfiguration, event.WorkflowJob.RunID, source, event.Repository.FullName) + } + return nil +} + // resolveJob turns the job's labels into a runner using the repository's config. func resolveJob(ctx context.Context, event WorkflowJobEvent, job *JobLabels) (*RunnerLabels, error) { config, err := loadRepositoryConfig(ctx, event) diff --git a/website/content/docs/security.md b/website/content/docs/security.md index c6bdf8c..630888d 100644 --- a/website/content/docs/security.md +++ b/website/content/docs/security.md @@ -15,6 +15,22 @@ Every incoming webhook from GitHub is authenticated using HMAC-SHA256. GitHub si The webhook secret is stored in Google Secret Manager and never appears in logs, environment variables, or Terraform state. +## Jobs From Forks + +**Attack vector:** A pull request from a fork runs arbitrary code on a VM in your project. + +A pull request runs its own copy of the workflow, so a fork can rewrite `runs-on` to ask for any gcrunner label, whatever the repository's own workflow says. An expression in `runs-on` that sends fork pull requests to GitHub-hosted runners only steers well-behaved ones. gcrunner itself therefore looks up the run of every queued job and refuses it when the run comes from another repository (`head_repository` differs from the repository, or is gone because the fork was deleted). The run gets a failed `gcrunner` check saying why and is cancelled; no VM is created. + +Give jobs from forks a GitHub-hosted runner, for example: + +```yaml +runs-on: >- + ${{ (github.event_name != 'pull_request' + || github.event.pull_request.head.repo.full_name == github.repository) + && format('gcrunner={0}', github.run_id) + || 'ubuntu-latest' }} +``` + ## Ephemeral, Isolated VMs **Attack vector:** A compromised job contaminates future jobs or pivots to other workloads.