tenseleyflow/shithub / 4c2a504

Browse files

S15: refactor SSH dispatch, HTTP git, repo lookup, pre-receive hook to use policy.Can

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
4c2a5048bbfd9c0a4dc5bde9248782f8b67b27fe
Parents
0f2f494
Tree
c4e78c7

5 changed files

StatusFile+-
M cmd/shithubd/hook.go 26 16
M internal/git/protocol/ssh_dispatch.go 19 14
A internal/git/protocol/ssh_dispatch_collab_test.go 150 0
M internal/web/handlers/githttp/handler.go 25 12
M internal/web/handlers/repo/repo.go 11 1
cmd/shithubd/hook.gomodified
@@ -19,8 +19,10 @@ import (
1919
 	"github.com/jackc/pgx/v5/pgxpool"
2020
 	"github.com/spf13/cobra"
2121
22
+	"github.com/tenseleyFlow/shithub/internal/auth/policy"
2223
 	"github.com/tenseleyFlow/shithub/internal/infra/config"
2324
 	"github.com/tenseleyFlow/shithub/internal/infra/db"
25
+	reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc"
2426
 	usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc"
2527
 	"github.com/tenseleyFlow/shithub/internal/worker"
2628
 	workerdb "github.com/tenseleyFlow/shithub/internal/worker/sqlc"
@@ -167,10 +169,11 @@ type errHookGate struct{ kind string }
167169
 func (e errHookGate) Error() string { return "shithub-hook: " + e.kind }
168170
169171
 var (
170
-	errHookSuspended = errHookGate{"user suspended"}
171
-	errHookArchived  = errHookGate{"repo archived"}
172
-	errHookDeleted   = errHookGate{"repo deleted"}
173
-	errHookMissing   = errHookGate{"missing context"}
172
+	errHookSuspended  = errHookGate{"user suspended"}
173
+	errHookArchived   = errHookGate{"repo archived"}
174
+	errHookDeleted    = errHookGate{"repo deleted"}
175
+	errHookMissing    = errHookGate{"missing context"}
176
+	errHookPermDenied = errHookGate{"permission denied"}
174177
 )
175178
176179
 func friendlyHookErr(err error) string {
@@ -181,6 +184,8 @@ func friendlyHookErr(err error) string {
181184
 		return "shithub: this repository is archived; pushes are disabled."
182185
 	case errors.Is(err, errHookDeleted):
183186
 		return "shithub: this repository has been deleted."
187
+	case errors.Is(err, errHookPermDenied):
188
+		return "shithub: you do not have write access to this repository."
184189
 	case errors.Is(err, errHookMissing):
185190
 		return "shithub: server error: hook context missing. Contact the operator."
186191
 	default:
@@ -197,24 +202,29 @@ func preReceiveCheck(ctx context.Context, h *hookCtx) error {
197202
 	if err != nil {
198203
 		return fmt.Errorf("user lookup: %w", err)
199204
 	}
200
-	if user.SuspendedAt.Valid {
201
-		return errHookSuspended
202
-	}
203205
204
-	row := h.pool.QueryRow(ctx,
205
-		`SELECT is_archived, deleted_at FROM repos WHERE id = $1`, h.repoID)
206
-	var archived bool
207
-	var deletedAt pgtype.Timestamptz
208
-	if err := row.Scan(&archived, &deletedAt); err != nil {
206
+	rq := reposdb.New()
207
+	repo, err := rq.GetRepoByID(ctx, h.pool, h.repoID)
208
+	if err != nil {
209209
 		return fmt.Errorf("repo lookup: %w", err)
210210
 	}
211
-	if deletedAt.Valid {
212
-		return errHookDeleted
211
+
212
+	actor := policy.UserActor(user.ID, user.Username, user.SuspendedAt.Valid, false)
213
+	repoRef := policy.NewRepoRefFromRepo(repo)
214
+	decision := policy.Can(ctx, policy.Deps{Pool: h.pool}, actor, policy.ActionRepoWrite, repoRef)
215
+	if decision.Allow {
216
+		return nil
213217
 	}
214
-	if archived {
218
+	switch decision.Code {
219
+	case policy.DenyRepoDeleted:
220
+		return errHookDeleted
221
+	case policy.DenyActorSuspended:
222
+		return errHookSuspended
223
+	case policy.DenyArchived:
215224
 		return errHookArchived
225
+	default:
226
+		return errHookPermDenied
216227
 	}
217
-	return nil
218228
 }
219229
220230
 func postReceiveEnqueue(ctx context.Context, h *hookCtx, refs []refUpdate) error {
internal/git/protocol/ssh_dispatch.gomodified
@@ -16,6 +16,7 @@ import (
1616
 	"github.com/jackc/pgx/v5/pgtype"
1717
 	"github.com/jackc/pgx/v5/pgxpool"
1818
19
+	"github.com/tenseleyFlow/shithub/internal/auth/policy"
1920
 	"github.com/tenseleyFlow/shithub/internal/infra/storage"
2021
 	reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc"
2122
 	usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc"
@@ -111,22 +112,26 @@ func PrepareDispatch(ctx context.Context, deps SSHDispatchDeps, in SSHDispatchIn
111112
 		return nil, parsed, fmt.Errorf("%w: %v", ErrSSHInternal, err)
112113
 	}
113114
114
-	// Inline authz — same shape as the HTTP handler. Public repos can
115
-	// be pulled by anyone; everything else requires owner identity.
116
-	// (S15 lifts this into policy.Can.)
117
-	if parsed.Service == UploadPack {
118
-		if repo.Visibility == reposdb.RepoVisibilityPrivate && user.ID != owner.ID {
119
-			return nil, parsed, ErrSSHRepoNotFound
120
-		}
121
-	} else {
122
-		if user.ID != owner.ID {
123
-			return nil, parsed, ErrSSHPermDenied
124
-		}
125
-		if repo.IsArchived {
115
+	// Authz via policy.Can. Map the policy decision back to the typed
116
+	// SSH errors so the friendly-message catalogue keeps working.
117
+	repoRef := policy.NewRepoRefFromRepo(repo)
118
+	actor := policy.UserActor(user.ID, user.Username, user.SuspendedAt.Valid, false)
119
+	action := policy.ActionRepoRead
120
+	if parsed.Service == ReceivePack {
121
+		action = policy.ActionRepoWrite
122
+	}
123
+	decision := policy.Can(ctx, policy.Deps{Pool: deps.Pool}, actor, action, repoRef)
124
+	if !decision.Allow {
125
+		switch decision.Code {
126
+		case policy.DenyActorSuspended:
127
+			return nil, parsed, ErrSSHSuspended
128
+		case policy.DenyArchived:
126129
 			return nil, parsed, ErrSSHArchived
127
-		}
128
-		if repo.DeletedAt.Valid {
130
+		case policy.DenyVisibility, policy.DenyRepoDeleted:
131
+			// Existence-leak guard: pretend the repo doesn't exist.
129132
 			return nil, parsed, ErrSSHRepoNotFound
133
+		default:
134
+			return nil, parsed, ErrSSHPermDenied
130135
 		}
131136
 	}
132137
internal/git/protocol/ssh_dispatch_collab_test.goadded
@@ -0,0 +1,150 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package protocol_test
4
+
5
+import (
6
+	"context"
7
+	"errors"
8
+	"testing"
9
+
10
+	"github.com/tenseleyFlow/shithub/internal/auth/policy"
11
+	policydb "github.com/tenseleyFlow/shithub/internal/auth/policy/sqlc"
12
+	"github.com/tenseleyFlow/shithub/internal/git/protocol"
13
+)
14
+
15
+// TestDispatch_CollabWriteCanPushPrivate confirms that adding eve as a
16
+// 'write' collaborator on alice's private repo lets her push. Before
17
+// S15 this would have been ErrSSHPermDenied; after S15 the policy
18
+// package grants based on the role row.
19
+func TestDispatch_CollabWriteCanPushPrivate(t *testing.T) {
20
+	t.Parallel()
21
+	env := setupDispatch(t)
22
+	pq := policydb.New()
23
+	if err := pq.UpsertCollabRole(context.Background(), env.pool, policydb.UpsertCollabRoleParams{
24
+		RepoID: repoIDFor(t, env, "alice", "private"),
25
+		UserID: env.eve,
26
+		Role:   policydb.CollabRoleWrite,
27
+	}); err != nil {
28
+		t.Fatalf("UpsertCollabRole: %v", err)
29
+	}
30
+
31
+	_, _, err := protocol.PrepareDispatch(context.Background(), env.deps, protocol.SSHDispatchInput{
32
+		OriginalCommand: "git-receive-pack 'alice/private'",
33
+		UserID:          env.eve,
34
+	})
35
+	if err != nil {
36
+		t.Fatalf("collab-write push denied: %v", err)
37
+	}
38
+}
39
+
40
+// TestDispatch_CollabReadCannotPush confirms that a 'read' grant lets
41
+// eve clone but not push.
42
+func TestDispatch_CollabReadCannotPush(t *testing.T) {
43
+	t.Parallel()
44
+	env := setupDispatch(t)
45
+	pq := policydb.New()
46
+	if err := pq.UpsertCollabRole(context.Background(), env.pool, policydb.UpsertCollabRoleParams{
47
+		RepoID: repoIDFor(t, env, "alice", "private"),
48
+		UserID: env.eve,
49
+		Role:   policydb.CollabRoleRead,
50
+	}); err != nil {
51
+		t.Fatalf("UpsertCollabRole: %v", err)
52
+	}
53
+	// Pull: allowed.
54
+	if _, _, err := protocol.PrepareDispatch(context.Background(), env.deps, protocol.SSHDispatchInput{
55
+		OriginalCommand: "git-upload-pack 'alice/private'",
56
+		UserID:          env.eve,
57
+	}); err != nil {
58
+		t.Fatalf("collab-read pull denied: %v", err)
59
+	}
60
+	// Push: denied with permission-denied (not 404, eve has read so
61
+	// existence is no secret).
62
+	_, _, err := protocol.PrepareDispatch(context.Background(), env.deps, protocol.SSHDispatchInput{
63
+		OriginalCommand: "git-receive-pack 'alice/private'",
64
+		UserID:          env.eve,
65
+	})
66
+	if !errors.Is(err, protocol.ErrSSHPermDenied) {
67
+		t.Fatalf("collab-read push: err = %v, want ErrSSHPermDenied", err)
68
+	}
69
+}
70
+
71
+// TestDispatch_DemotedCollabLosesAccessNextRequest mirrors the spec's
72
+// definition-of-done bullet: dropping the role takes effect immediately
73
+// on the next request (no cross-request cache).
74
+func TestDispatch_DemotedCollabLosesAccessNextRequest(t *testing.T) {
75
+	t.Parallel()
76
+	env := setupDispatch(t)
77
+	pq := policydb.New()
78
+	repoID := repoIDFor(t, env, "alice", "private")
79
+	if err := pq.UpsertCollabRole(context.Background(), env.pool, policydb.UpsertCollabRoleParams{
80
+		RepoID: repoID, UserID: env.eve, Role: policydb.CollabRoleAdmin,
81
+	}); err != nil {
82
+		t.Fatalf("UpsertCollabRole admin: %v", err)
83
+	}
84
+	// Eve as admin: push allowed.
85
+	if _, _, err := protocol.PrepareDispatch(context.Background(), env.deps, protocol.SSHDispatchInput{
86
+		OriginalCommand: "git-receive-pack 'alice/private'",
87
+		UserID:          env.eve,
88
+	}); err != nil {
89
+		t.Fatalf("admin push denied: %v", err)
90
+	}
91
+	// Demote to read.
92
+	if err := pq.UpsertCollabRole(context.Background(), env.pool, policydb.UpsertCollabRoleParams{
93
+		RepoID: repoID, UserID: env.eve, Role: policydb.CollabRoleRead,
94
+	}); err != nil {
95
+		t.Fatalf("UpsertCollabRole read: %v", err)
96
+	}
97
+	// Next request: push denied.
98
+	_, _, err := protocol.PrepareDispatch(context.Background(), env.deps, protocol.SSHDispatchInput{
99
+		OriginalCommand: "git-receive-pack 'alice/private'",
100
+		UserID:          env.eve,
101
+	})
102
+	if !errors.Is(err, protocol.ErrSSHPermDenied) {
103
+		t.Fatalf("demoted push: err = %v, want ErrSSHPermDenied", err)
104
+	}
105
+}
106
+
107
+// TestDispatch_RemovedCollabFallsBack404 confirms that fully removing
108
+// the row turns eve back into a stranger on a private repo: the
109
+// rejection is "repo not found" (existence-leak guard).
110
+func TestDispatch_RemovedCollabFallsBack404(t *testing.T) {
111
+	t.Parallel()
112
+	env := setupDispatch(t)
113
+	pq := policydb.New()
114
+	repoID := repoIDFor(t, env, "alice", "private")
115
+	if err := pq.UpsertCollabRole(context.Background(), env.pool, policydb.UpsertCollabRoleParams{
116
+		RepoID: repoID, UserID: env.eve, Role: policydb.CollabRoleRead,
117
+	}); err != nil {
118
+		t.Fatalf("UpsertCollabRole: %v", err)
119
+	}
120
+	if err := pq.RemoveCollab(context.Background(), env.pool, policydb.RemoveCollabParams{
121
+		RepoID: repoID, UserID: env.eve,
122
+	}); err != nil {
123
+		t.Fatalf("RemoveCollab: %v", err)
124
+	}
125
+	_, _, err := protocol.PrepareDispatch(context.Background(), env.deps, protocol.SSHDispatchInput{
126
+		OriginalCommand: "git-upload-pack 'alice/private'",
127
+		UserID:          env.eve,
128
+	})
129
+	if !errors.Is(err, protocol.ErrSSHRepoNotFound) {
130
+		t.Fatalf("removed collab pull: err = %v, want ErrSSHRepoNotFound", err)
131
+	}
132
+	// Sanity-check the policy decision separately.
133
+	_ = policy.ActionRepoRead
134
+}
135
+
136
+// repoIDFor looks up the bigserial id for a repo so the collab insert
137
+// references a real row. setupDispatch creates "public" and "private";
138
+// callers ask by name.
139
+func repoIDFor(t *testing.T, env *dispatchEnv, owner, name string) int64 {
140
+	t.Helper()
141
+	var id int64
142
+	err := env.pool.QueryRow(context.Background(),
143
+		`SELECT r.id FROM repos r JOIN users u ON u.id = r.owner_user_id
144
+         WHERE u.username = $1 AND r.name = $2`,
145
+		owner, name).Scan(&id)
146
+	if err != nil {
147
+		t.Fatalf("repoIDFor %s/%s: %v", owner, name, err)
148
+	}
149
+	return id
150
+}
internal/web/handlers/githttp/handler.gomodified
@@ -12,6 +12,7 @@ import (
1212
 	"github.com/go-chi/chi/v5"
1313
 	"github.com/jackc/pgx/v5/pgtype"
1414
15
+	"github.com/tenseleyFlow/shithub/internal/auth/policy"
1516
 	"github.com/tenseleyFlow/shithub/internal/git/protocol"
1617
 	reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc"
1718
 	"github.com/tenseleyFlow/shithub/internal/web/middleware"
@@ -152,28 +153,40 @@ func (h *Handlers) authorizeForService(w http.ResponseWriter, r *http.Request, s
152153
 	}
153154
154155
 	auth, authErr := h.resolveBasicAuth(r.Context(), r.Header.Get("Authorization"))
155
-	requireAuth := svc == protocol.ReceivePack || row.Visibility == reposdb.RepoVisibilityPrivate
156
+	repoRef := policy.NewRepoRefFromRepo(row)
157
+	requireAuth := svc == protocol.ReceivePack || repoRef.IsPrivate()
156158
 	if authErr != nil || (requireAuth && auth.Anonymous) {
157159
 		writeChallenge(w)
158160
 		return reposdb.Repo{}, false
159161
 	}
160162
161
-	// Permission check — inline; S15 replaces this with the policy package.
162
-	// V1: only the owner can read a private repo or write to any repo.
163
-	if !auth.Anonymous && auth.UserID != owner.ID {
164
-		http.Error(w, "forbidden", http.StatusForbidden)
165
-		return reposdb.Repo{}, false
163
+	// Build the policy actor and ask Can(). Owner identity, collab role,
164
+	// archived/deleted gates all live in the policy package now.
165
+	var actor policy.Actor
166
+	if auth.Anonymous {
167
+		actor = policy.AnonymousActor()
168
+	} else {
169
+		actor = policy.UserActor(auth.UserID, auth.Username, false, false)
166170
 	}
171
+	action := policy.ActionRepoRead
167172
 	if svc == protocol.ReceivePack {
168
-		if row.IsArchived {
173
+		action = policy.ActionRepoWrite
174
+	}
175
+	decision := policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, action, repoRef)
176
+	if !decision.Allow {
177
+		switch decision.Code {
178
+		case policy.DenyRepoDeleted:
179
+			http.Error(w, "gone", http.StatusGone)
180
+		case policy.DenyArchived:
181
+			// User sees this directly in their git client.
169182
 			writeGitErrorMessage(w, http.StatusForbidden,
170183
 				"repository is archived; pushes are disabled")
171
-			return reposdb.Repo{}, false
172
-		}
173
-		if row.DeletedAt.Valid {
174
-			http.Error(w, "gone", http.StatusGone)
175
-			return reposdb.Repo{}, false
184
+		case policy.DenyVisibility:
185
+			http.Error(w, "not found", http.StatusNotFound)
186
+		default:
187
+			http.Error(w, "forbidden", http.StatusForbidden)
176188
 		}
189
+		return reposdb.Repo{}, false
177190
 	}
178191
 	return row, true
179192
 }
internal/web/handlers/repo/repo.gomodified
@@ -18,6 +18,7 @@ import (
1818
 	"github.com/jackc/pgx/v5/pgxpool"
1919
2020
 	"github.com/tenseleyFlow/shithub/internal/auth/audit"
21
+	"github.com/tenseleyFlow/shithub/internal/auth/policy"
2122
 	"github.com/tenseleyFlow/shithub/internal/auth/throttle"
2223
 	"github.com/tenseleyFlow/shithub/internal/infra/storage"
2324
 	"github.com/tenseleyFlow/shithub/internal/repos"
@@ -245,7 +246,16 @@ func (h *Handlers) lookupRepoForViewer(ctx context.Context, ownerName, repoName
245246
 	if err != nil {
246247
 		return reposdb.Repo{}, err
247248
 	}
248
-	if row.Visibility == reposdb.RepoVisibilityPrivate && (viewerID == 0 || viewerID != owner.ID) {
249
+	// Visibility decision delegated to policy.Can. We use ActionRepoRead;
250
+	// when the decision denies, return ErrNoRows so the caller can 404.
251
+	repoRef := policy.NewRepoRefFromRepo(row)
252
+	var actor policy.Actor
253
+	if viewerID == 0 {
254
+		actor = policy.AnonymousActor()
255
+	} else {
256
+		actor = policy.UserActor(viewerID, "", false, false)
257
+	}
258
+	if !policy.Can(ctx, policy.Deps{Pool: h.d.Pool}, actor, policy.ActionRepoRead, repoRef).Allow {
249259
 		return reposdb.Repo{}, pgx.ErrNoRows
250260
 	}
251261
 	return row, nil