Skip to content

Commit 739ab56

Browse files
committed
security(operator): scope read-only views by repo permission
Per code review: writers (read/triage/write on the auth repo) could enumerate every repo the copier touches via the read-only operator endpoints, regardless of GitHub access to the underlying repos. Each endpoint leaks a different slice of topology — repo names, paths, commit SHAs, free-form Detail fields, full source→target workflow config — to anyone holding any valid PAT for the auth repo. Add a per-request repoFilter that memoises CanUserReadRepo lookups and post-filters rows. Operators (admin/maintain) bypass; writers only see rows whose referenced repos they can read on GitHub. - /audit/events: drop rows where source_repo or target_repo is unreadable; config_change rows (no repo) are operator-only. - /observability/webhook-traces: drop traces with no repo or with unreadable repos; total stays unfiltered so retention vs visibility stays distinguishable. - /logs (per-delivery): look up the delivery's repo via a new WebhookTraceBuffer.RepoForDelivery helper; deny writers when no trace exists or the repo is unreadable. - /workflows: drop workflows whose source or destination repo is unreadable; surface hidden_count so the UI can hint without naming the hidden rows. RoleWriter doc updated to describe the post-filter behaviour explicitly so the boundary is discoverable from the type.
1 parent 3612c14 commit 739ab56

5 files changed

Lines changed: 696 additions & 4 deletions

File tree

services/operator_auth.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,16 @@ var ghRepoNameRe = regexp.MustCompile(`^[a-zA-Z0-9_.-]{1,100}$`)
5858
type OperatorRole string
5959

6060
const (
61-
// RoleOperator has full access: view, replay, release.
61+
// RoleOperator has full access: view, replay, release. Operators are
62+
// trusted with the full repo topology and bypass per-repo scoping in
63+
// the read-only views (audit events, webhook traces, delivery logs,
64+
// workflow config).
6265
RoleOperator OperatorRole = "operator"
6366
// RoleWriter has read-only access: view workflows, audit, recent copies.
67+
// Read-only views are post-filtered by repoFilter so writers only see
68+
// rows that reference repos they can read on GitHub — without this they
69+
// could enumerate every source→target pairing and audit row in the
70+
// system, regardless of whether they have GitHub access to those repos.
6471
RoleWriter OperatorRole = "writer"
6572
// RoleDenied means the user has no access.
6673
RoleDenied OperatorRole = "denied"

services/operator_repo_filter.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package services
2+
3+
import (
4+
"context"
5+
"strings"
6+
)
7+
8+
// repoFilter post-filters operator UI rows so a writer only sees rows that
9+
// reference repos they can read on GitHub. Operators (admin/maintain on the
10+
// auth repo) bypass the check — they're trusted with full topology.
11+
//
12+
// Per-request only: results memoise inside one filter instance so we don't
13+
// hit the GitHub API more than once per distinct repo per request, even with
14+
// pages of audit/trace rows that share the same source/target.
15+
//
16+
// Construct via newRepoFilter and reuse for the duration of one HTTP handler.
17+
type repoFilter struct {
18+
ctx context.Context
19+
cache *ghAuthCache
20+
pat string
21+
user *OperatorUser
22+
// memo of repo → has-read for this request. nil entries are treated as
23+
// "deny" so a transient GitHub error fails closed for that row.
24+
memo map[string]bool
25+
}
26+
27+
func newRepoFilter(ctx context.Context, cache *ghAuthCache, pat string, user *OperatorUser) *repoFilter {
28+
return &repoFilter{ctx: ctx, cache: cache, pat: pat, user: user, memo: make(map[string]bool)}
29+
}
30+
31+
// bypass reports whether this filter should let every row through unmodified.
32+
// Operators see everything; writers go through per-repo checks.
33+
func (f *repoFilter) bypass() bool {
34+
return f == nil || f.user == nil || f.user.Role == RoleOperator
35+
}
36+
37+
// canRead returns true if the caller has read access to repo. Empty repo
38+
// returns true so a single missing field doesn't drop a row that's otherwise
39+
// scoped by a populated peer (e.g. a copy event with target_repo set but
40+
// source_repo empty). Callers that need both fields populated should compose
41+
// canRead checks themselves and treat empty as "no info" rather than "ok".
42+
func (f *repoFilter) canRead(repo string) bool {
43+
if f.bypass() {
44+
return true
45+
}
46+
repo = strings.TrimSpace(repo)
47+
if repo == "" {
48+
return true
49+
}
50+
if v, ok := f.memo[repo]; ok {
51+
return v
52+
}
53+
ok, err := f.cache.CanUserReadRepo(f.ctx, f.pat, f.user.Login, repo)
54+
// Fail closed on error: a transient GitHub 5xx shouldn't reveal a row
55+
// the writer wouldn't normally see. Cached deny entries naturally pass
56+
// through the same path.
57+
allowed := err == nil && ok
58+
f.memo[repo] = allowed
59+
return allowed
60+
}
61+
62+
// allowAuditEvent decides whether to surface one audit row to the caller.
63+
// A writer must be able to read every repo named on the row; otherwise the
64+
// row leaks the existence of a repo the writer has no business knowing about.
65+
// Rows with neither source_repo nor target_repo populated (config_change
66+
// events) are operator-only — writers don't get to see admin actions.
67+
func (f *repoFilter) allowAuditEvent(ev *AuditEvent) bool {
68+
if f.bypass() {
69+
return true
70+
}
71+
if strings.TrimSpace(ev.SourceRepo) == "" && strings.TrimSpace(ev.TargetRepo) == "" {
72+
return false
73+
}
74+
if ev.SourceRepo != "" && !f.canRead(ev.SourceRepo) {
75+
return false
76+
}
77+
if ev.TargetRepo != "" && !f.canRead(ev.TargetRepo) {
78+
return false
79+
}
80+
return true
81+
}
82+
83+
// filterAuditEvents returns the subset of events visible to the caller.
84+
// Returns the input slice unchanged when the filter is in bypass mode.
85+
func (f *repoFilter) filterAuditEvents(events []AuditEvent) []AuditEvent {
86+
if f.bypass() {
87+
return events
88+
}
89+
out := make([]AuditEvent, 0, len(events))
90+
for i := range events {
91+
if f.allowAuditEvent(&events[i]) {
92+
out = append(out, events[i])
93+
}
94+
}
95+
return out
96+
}
97+
98+
// filterWebhookTraces returns the subset of webhook traces visible to the
99+
// caller. Traces with no Repo populated are dropped for writers — a trace
100+
// without a repo can't be scoped, and the Detail field can carry sensitive
101+
// content (truncation is a length cap, not redaction).
102+
func (f *repoFilter) filterWebhookTraces(traces []WebhookTraceEntry) []WebhookTraceEntry {
103+
if f.bypass() {
104+
return traces
105+
}
106+
out := make([]WebhookTraceEntry, 0, len(traces))
107+
for _, t := range traces {
108+
if strings.TrimSpace(t.Repo) == "" {
109+
continue
110+
}
111+
if f.canRead(t.Repo) {
112+
out = append(out, t)
113+
}
114+
}
115+
return out
116+
}

0 commit comments

Comments
 (0)