From 13b84b0551dfda81f9f95a32121de621a25bcb0c Mon Sep 17 00:00:00 2001 From: Levi van Noort <73097785+levivannoort@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:44:19 +0200 Subject: [PATCH 1/3] fix(orchestrator): refuse jobs whose run comes from a fork A pull request runs its own copy of the workflow, so a fork can rewrite runs-on to ask for any gcrunner label, and gcrunner provisioned every queued job with one. An expression in runs-on that sends fork pull requests to GitHub-hosted runners only steered well-behaved ones. Before resolving the runner, the orchestrator now looks up the job's run and refuses it unless its head repository is the repository itself (a deleted fork leaves none, and is refused too). The refusal goes through the configuration-error path: a failed gcrunner check saying why, and the run cancelled. A failed lookup is retried like any other GitHub error. Tests reach the lookup through runSource, which TestMain points at the job's own repository, so the tests that are not about it need no GitHub API. --- orchestrator/github.go | 24 +++++++++++ orchestrator/preemption_test.go | 32 +++++++++------ orchestrator/run_source_test.go | 70 +++++++++++++++++++++++++++++++++ orchestrator/webhook.go | 30 +++++++++++++- 4 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 orchestrator/run_source_test.go diff --git a/orchestrator/github.go b/orchestrator/github.go index cfaed81..4068a63 100644 --- a/orchestrator/github.go +++ b/orchestrator/github.go @@ -413,6 +413,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 +439,24 @@ func fetchRun(ctx context.Context, owner, repo string, runID int64, installation return &run, nil } +// lookupRunSource returns the full name of the repository the job's run +// comes from, or "" when GitHub no longer knows it. +func lookupRunSource(ctx context.Context, event WorkflowJobEvent) (string, error) { + owner, repo := event.Repository.Owner.Login, event.Repository.Name + installationToken, err := getInstallationToken(ctx, owner) + if err != nil { + return "", fmt.Errorf("get installation token: %w", err) + } + run, err := fetchRun(ctx, owner, repo, event.WorkflowJob.RunID, installationToken) + if err != nil { + return "", fmt.Errorf("look up run %d: %w", event.WorkflowJob.RunID, err) + } + if run.HeadRepository == nil { + return "", nil + } + return run.HeadRepository.FullName, nil +} + const ( runStatusCompleted = "completed" runConclusionCancelled = "cancelled" diff --git a/orchestrator/preemption_test.go b/orchestrator/preemption_test.go index c284082..573f460 100644 --- a/orchestrator/preemption_test.go +++ b/orchestrator/preemption_test.go @@ -25,19 +25,22 @@ 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, 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 + 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 +60,14 @@ 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)) + 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..eafd609 --- /dev/null +++ b/orchestrator/run_source_test.go @@ -0,0 +1,70 @@ +package function + +import ( + "context" + "net/http" + "os" + "strings" + "testing" +) + +// 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()) +} + +func withRunSourceLookup(t *testing.T) { + t.Helper() + original := runSource + runSource = lookupRunSource + t.Cleanup(func() { runSource = original }) +} + +// 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) + } +} 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) From d074cfe0aabfdbebfab7f93ea6b2c4d1c89b1557 Mon Sep 17 00:00:00 2001 From: Levi van Noort <73097785+levivannoort@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:44:19 +0200 Subject: [PATCH 2/3] docs: jobs from forks get no VM --- website/content/docs/security.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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. From 2b6d80b8b20ae7fa2f28b909080f8ee7cf347018 Mon Sep 17 00:00:00 2001 From: Levi van Noort <73097785+levivannoort@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:51:19 +0200 Subject: [PATCH 3/3] perf(orchestrator): look each run's source up once Production queues about 2,500 jobs an hour, many sharing a run. The head repository of a run never changes, so lookups are cached per run for six hours and concurrent jobs of one run share a single request. Failed lookups are not cached. --- orchestrator/github.go | 72 +++++++++++++++++++++++++++------ orchestrator/preemption_test.go | 4 +- orchestrator/run_source_test.go | 50 ++++++++++++++++++++++- 3 files changed, 112 insertions(+), 14 deletions(-) diff --git a/orchestrator/github.go b/orchestrator/github.go index 4068a63..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} @@ -439,22 +441,68 @@ 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. +// 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 := event.Repository.Owner.Login, event.Repository.Name - installationToken, err := getInstallationToken(ctx, owner) - if err != nil { - return "", fmt.Errorf("get installation token: %w", err) - } - run, err := fetchRun(ctx, owner, repo, event.WorkflowJob.RunID, installationToken) + 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 "", fmt.Errorf("look up run %d: %w", event.WorkflowJob.RunID, err) - } - if run.HeadRepository == nil { - return "", nil + return "", err } - return run.HeadRepository.FullName, nil + return source.(string), nil } const ( diff --git a/orchestrator/preemption_test.go b/orchestrator/preemption_test.go index 573f460..e2da0f8 100644 --- a/orchestrator/preemption_test.go +++ b/orchestrator/preemption_test.go @@ -26,7 +26,7 @@ var ( // github stands in for api.github.com: it reports the run as runStatus // (completed when empty) with runConclusion (failure when empty and // completed) from headRepository (null when empty), or answers the run with -// runLookupStatus when set, reports job 42 as jobStatus, answers the rerun request +// 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 { @@ -34,6 +34,7 @@ type github struct { runConclusion string headRepository string runLookupStatus int + runLookups int jobStatus string rerunStatus int checkStatus int @@ -60,6 +61,7 @@ func (g *github) RoundTrip(req *http.Request) (*http.Response, error) { if status == "completed" && conclusion == "" { conclusion = "failure" } + g.runLookups++ if g.runLookupStatus != 0 { return answer(g.runLookupStatus, `{"message": "Server Error"}`) } diff --git a/orchestrator/run_source_test.go b/orchestrator/run_source_test.go index eafd609..1fb1a78 100644 --- a/orchestrator/run_source_test.go +++ b/orchestrator/run_source_test.go @@ -6,6 +6,7 @@ import ( "os" "strings" "testing" + "time" ) // TestMain points runSource at the job's own repository, so tests that are @@ -17,11 +18,17 @@ func TestMain(m *testing.M) { 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 - t.Cleanup(func() { runSource = original }) + 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 @@ -68,3 +75,44 @@ func TestAFailedRunLookupIsRetried(t *testing.T) { 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) + } +}