steid
Sign in

@jamesgill /

steid

feat: decide who may reach the git protocol

serve_git resolves the repository, applies view_repo's visibility rule, and
refuses writes — all before the backend exists. Once pack data is moving there
is no way to take a refusal back, so the ordering is the feature.

Three choices worth keeping:

The endpoint is named by the route rather than parsed from the path, and the
use case builds path_info itself from the validated handle and name. The string
that decides authorization and the string handed to git are therefore the same
string, so no URL can be coaxed into meaning a different operation than the one
that was checked.

Existence is settled before permission. A push to a repository the actor cannot
see is None, not Forbidden — answering "forbidden" would confirm that a private
repository by that name exists.

Push is refused explicitly rather than by leaning on http-backend disabling
receive-pack by default. That default is configuration, and a default that
helpfully changes is not a permission check. It becomes a real check in 4b.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JZwc7URWKVhkAuRTWiDmjA
JamesPatrickGill authored 8 days agoparent47db238Browse files8ac00bd701f6bb734b2adb3eaf70b3532144c948

4 files changed+561 −1

plans/current.md+9 −1View file
@@ -23,7 +23,7 @@ statistic the clone path could tempt us into computing.
2323 `GitOperation` (Read/Write) as the thing authorization is decided on
2424 - [x] Infrastructure: `GitHttpBackend` adapter, spawning through the existing `run_git`
2525 invoker; streams stdin in and stdout out, parsing CGI headers off the front
26−- [ ] Application: `serve_git` use case — resolves the repository, enforces visibility,
26+- [x] Application: `serve_git` use case — resolves the repository, enforces visibility,
2727 refuses writes outright, and only then delegates
2828 - [ ] Web: the three git routes under `/{handle}/repos/{name}.git/`
2929 - [ ] Verify with a real `git clone` of a repo with enough refs to trigger a gzipped
@@ -72,6 +72,14 @@ the origin. A private repository is not clonable by anyone yet — not even its
7272 the handler via `to_bytes(body, limit)` with a caller-chosen limit, so there is no
7373 layer to raise — and the git routes take the body unbuffered anyway. The warning
7474 carried from [0001](decisions/0001-git-over-http-not-ssh.md) is stale.
75+- **The endpoint is named by the route, not parsed from the path.** Three routes, three
76+ literal `GitEndpoint` values. The use case then builds `path_info` itself from the
77+ validated handle and name, so the string that decides authorization and the string
78+ handed to git are the same string, and a URL cannot be coaxed into meaning a different
79+ operation than the one that was checked.
80+- **Existence is settled before permission.** A push to a repository the actor cannot
81+ see answers `None` (404), not `Forbidden` — a 403 would confirm a private repository
82+ by that name exists.
7583 - **Milestone 4 was split.** See [ROADMAP.md](ROADMAP.md#why-this-order).
7684
7785 ### Open
src/application/git.rs+525 −0View file
@@ -0,0 +1,525 @@
1+//! Serving the git smart-HTTP protocol.
2+//!
3+//! The protocol itself belongs to `git http-backend`. What belongs here is the part git
4+//! cannot decide: whether this actor may do this to this repository, settled **before**
5+//! the backend is spawned. Once pack data is moving, refusing is no longer an option.
6+
7+use crate::domain::{
8+ Actor, DomainError, OrgName, RepoName,
9+ repository::{MembershipRepository, OrgRepository, RepoRepository},
10+};
11+
12+use super::{
13+ authz::is_org_member,
14+ error::Result,
15+ port::{ByteStream, GitMethod, GitProtocolServer, GitRequest, GitResponse},
16+};
17+
18+/// A git service, as named in the protocol.
19+///
20+/// The wire spelling and the enum are kept together because the two must not drift: the
21+/// same value decides authorization and is handed to the backend.
22+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
23+pub enum GitService {
24+ /// Sends objects to a client. What `git clone` and `git fetch` use.
25+ UploadPack,
26+ /// Receives objects from a client. What `git push` uses.
27+ ReceivePack,
28+}
29+
30+impl GitService {
31+ pub fn as_str(self) -> &'static str {
32+ match self {
33+ Self::UploadPack => "git-upload-pack",
34+ Self::ReceivePack => "git-receive-pack",
35+ }
36+ }
37+
38+ /// What this service asks permission to do.
39+ ///
40+ /// The whole authorization surface of the git transport is this function. Adding a
41+ /// service without deciding its operation is a compile error, which is the point of
42+ /// matching exhaustively rather than defaulting.
43+ pub fn operation(self) -> GitOperation {
44+ match self {
45+ Self::UploadPack => GitOperation::Read,
46+ Self::ReceivePack => GitOperation::Write,
47+ }
48+ }
49+}
50+
51+/// Parsing returns `Result`, never `Option`: an unrecognised service that silently
52+/// defaulted to read would eventually default the wrong way.
53+impl std::str::FromStr for GitService {
54+ type Err = DomainError;
55+
56+ fn from_str(value: &str) -> std::result::Result<Self, Self::Err> {
57+ match value {
58+ "git-upload-pack" => Ok(Self::UploadPack),
59+ "git-receive-pack" => Ok(Self::ReceivePack),
60+ other => Err(DomainError::validation(
61+ "service",
62+ format!("unknown git service {other:?}"),
63+ )),
64+ }
65+ }
66+}
67+
68+/// What a git request wants to do to a repository.
69+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
70+pub enum GitOperation {
71+ Read,
72+ Write,
73+}
74+
75+/// Which endpoint of the smart protocol is being asked for.
76+///
77+/// Named by the caller from the route it matched, never parsed out of a URL. Three
78+/// routes, three literal values — so a path cannot be coaxed into meaning a different
79+/// operation than the one that was authorized.
80+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
81+pub enum GitEndpoint {
82+ /// `GET {repo}.git/info/refs?service=…` — the ref advertisement that opens an
83+ /// exchange. Carries the service of the operation it is opening, so a push
84+ /// advertisement is a write before a single object moves.
85+ Advertisement(GitService),
86+ /// `POST {repo}.git/{service}` — the negotiation and the pack itself.
87+ Rpc(GitService),
88+}
89+
90+impl GitEndpoint {
91+ fn service(self) -> GitService {
92+ match self {
93+ Self::Advertisement(service) | Self::Rpc(service) => service,
94+ }
95+ }
96+
97+ fn method(self) -> GitMethod {
98+ match self {
99+ Self::Advertisement(_) => GitMethod::Get,
100+ Self::Rpc(_) => GitMethod::Post,
101+ }
102+ }
103+}
104+
105+/// The request headers the backend needs to see.
106+///
107+/// Not every header — only the four that change what git does. Forwarding the rest
108+/// would be handing a subprocess arbitrary client input for no benefit.
109+#[derive(Debug, Default, Clone, PartialEq, Eq)]
110+pub struct GitClientHeaders {
111+ pub content_type: Option<String>,
112+ /// Real clients gzip a request body once a repository has more than a handful of
113+ /// refs. Dropping this makes clones of real repositories fail while a one-ref test
114+ /// repository keeps passing.
115+ pub content_encoding: Option<String>,
116+ pub content_length: Option<String>,
117+ /// `version=2` for any modern client. Dropping it downgrades to v0 silently.
118+ pub git_protocol: Option<String>,
119+}
120+
121+/// Serves one git protocol request against a repository the actor is allowed to reach.
122+///
123+/// `Ok(None)` covers **both** "no such repository" and "not allowed to see it", exactly
124+/// as [`view_repo`](super::repo::view_repo) does, and the caller must render them
125+/// identically — a private repository has to be absent, not merely unclonable.
126+///
127+/// Writes are refused outright: push arrives with personal access tokens in Milestone
128+/// 4b. The refusal is a real authorization decision made here, not a reliance on `git
129+/// http-backend` disabling `receive-pack` by default — a default that helpfully changes
130+/// is not a permission check.
131+#[allow(clippy::too_many_arguments)]
132+pub async fn serve_git(
133+ handle: &OrgName,
134+ name: &RepoName,
135+ endpoint: GitEndpoint,
136+ headers: GitClientHeaders,
137+ body: ByteStream,
138+ actor: &Actor,
139+ orgs: &impl OrgRepository,
140+ memberships: &impl MembershipRepository,
141+ repos: &impl RepoRepository,
142+ protocol: &impl GitProtocolServer,
143+) -> Result<Option<GitResponse>> {
144+ let Some(org) = orgs.find_by_name(handle).await? else {
145+ return Ok(None);
146+ };
147+
148+ let Some(repo) = repos.find_by_org_and_name(&org.id, name).await? else {
149+ return Ok(None);
150+ };
151+
152+ // Existence is settled before permission, so a push to a repository the actor may
153+ // not even see answers "no such repository" rather than "not allowed" — the latter
154+ // would confirm it exists.
155+ if !repo.visibility.is_public() && !is_org_member(&org, actor, memberships).await? {
156+ return Ok(None);
157+ }
158+
159+ if endpoint.service().operation() == GitOperation::Write {
160+ return Err(DomainError::Forbidden.into());
161+ }
162+
163+ let service = endpoint.service();
164+ let path_info = format!("/{}/{}.git", org.name, repo.name);
165+
166+ // Built here from validated values rather than forwarded from the URL. `OrgName` and
167+ // `RepoName` already made traversal impossible, so the backend cannot be pointed at
168+ // a path this function did not agree to.
169+ let (path_info, query) = match endpoint {
170+ GitEndpoint::Advertisement(_) => (
171+ format!("{path_info}/info/refs"),
172+ format!("service={}", service.as_str()),
173+ ),
174+ GitEndpoint::Rpc(_) => (format!("{path_info}/{}", service.as_str()), String::new()),
175+ };
176+
177+ let response = protocol
178+ .serve(GitRequest {
179+ method: endpoint.method(),
180+ path_info,
181+ query,
182+ content_type: headers.content_type,
183+ content_encoding: headers.content_encoding,
184+ content_length: headers.content_length,
185+ git_protocol: headers.git_protocol,
186+ body,
187+ })
188+ .await?;
189+
190+ Ok(Some(response))
191+}
192+
193+#[cfg(test)]
194+mod tests {
195+ use super::*;
196+ use crate::{
197+ domain::{
198+ Membership, MembershipId, OrgId, Organization, RepoId, Repository, Role, UserId,
199+ Visibility,
200+ },
201+ infrastructure::{
202+ git::InMemoryGitProtocol,
203+ repository::{InMemoryMembershipRepo, InMemoryOrgRepo, InMemoryRepoRepo},
204+ },
205+ };
206+
207+ struct Fixture {
208+ orgs: InMemoryOrgRepo,
209+ memberships: InMemoryMembershipRepo,
210+ repos: InMemoryRepoRepo,
211+ protocol: InMemoryGitProtocol,
212+ owner: Actor,
213+ member: Actor,
214+ stranger: Actor,
215+ handle: OrgName,
216+ }
217+
218+ async fn fixture() -> Fixture {
219+ let orgs = InMemoryOrgRepo::new();
220+ let memberships = InMemoryMembershipRepo::new();
221+
222+ let org = Organization::new(OrgId::generate(), "acme", None).expect("valid org");
223+ orgs.save(&org).await.expect("save org");
224+
225+ let owner = UserId::generate();
226+ let member = UserId::generate();
227+
228+ for (user, role) in [(&owner, Role::Owner), (&member, Role::Member)] {
229+ memberships
230+ .save(&Membership::new(
231+ MembershipId::generate(),
232+ org.id.clone(),
233+ user.clone(),
234+ role,
235+ ))
236+ .await
237+ .expect("save membership");
238+ }
239+
240+ let repos = InMemoryRepoRepo::new();
241+ for (name, visibility) in [
242+ ("steid", Visibility::Public),
243+ ("secret", Visibility::Private),
244+ ] {
245+ repos
246+ .save(
247+ &Repository::new(RepoId::generate(), org.id.clone(), name, None, visibility)
248+ .expect("valid repo"),
249+ )
250+ .await
251+ .expect("save repo");
252+ }
253+
254+ Fixture {
255+ orgs,
256+ memberships,
257+ repos,
258+ protocol: InMemoryGitProtocol::new(),
259+ owner: Actor::User(owner),
260+ member: Actor::User(member),
261+ stranger: Actor::User(UserId::generate()),
262+ handle: org.name,
263+ }
264+ }
265+
266+ impl Fixture {
267+ async fn serve(
268+ &self,
269+ actor: &Actor,
270+ name: &str,
271+ endpoint: GitEndpoint,
272+ ) -> Result<Option<GitResponse>> {
273+ self.serve_with(actor, name, endpoint, GitClientHeaders::default())
274+ .await
275+ }
276+
277+ async fn serve_with(
278+ &self,
279+ actor: &Actor,
280+ name: &str,
281+ endpoint: GitEndpoint,
282+ headers: GitClientHeaders,
283+ ) -> Result<Option<GitResponse>> {
284+ serve_git(
285+ &self.handle,
286+ &RepoName::new(name).expect("valid name"),
287+ endpoint,
288+ headers,
289+ Box::pin(tokio::io::empty()),
290+ actor,
291+ &self.orgs,
292+ &self.memberships,
293+ &self.repos,
294+ &self.protocol,
295+ )
296+ .await
297+ }
298+ }
299+
300+ fn advertise_clone() -> GitEndpoint {
301+ GitEndpoint::Advertisement(GitService::UploadPack)
302+ }
303+
304+ fn advertise_push() -> GitEndpoint {
305+ GitEndpoint::Advertisement(GitService::ReceivePack)
306+ }
307+
308+ // --- what reaches the backend ----------------------------------------------
309+
310+ #[tokio::test]
311+ async fn an_advertisement_is_addressed_to_the_repository_on_disk() {
312+ // The URL says `/acme/repos/steid.git`; the disk says `acme/steid.git`. The use
313+ // case builds the second from validated values rather than trusting the first.
314+ let f = fixture().await;
315+
316+ f.serve(&Actor::Anonymous, "steid", advertise_clone())
317+ .await
318+ .expect("should serve")
319+ .expect("should be visible");
320+
321+ let request = f.protocol.requests().pop().expect("backend was reached");
322+ assert_eq!(request.path_info, "/acme/steid.git/info/refs");
323+ assert_eq!(request.query, "service=git-upload-pack");
324+ assert_eq!(request.method, GitMethod::Get);
325+ }
326+
327+ #[tokio::test]
328+ async fn an_rpc_posts_to_the_service_with_no_query() {
329+ let f = fixture().await;
330+
331+ f.serve(
332+ &Actor::Anonymous,
333+ "steid",
334+ GitEndpoint::Rpc(GitService::UploadPack),
335+ )
336+ .await
337+ .expect("should serve")
338+ .expect("should be visible");
339+
340+ let request = f.protocol.requests().pop().expect("backend was reached");
341+ assert_eq!(request.path_info, "/acme/steid.git/git-upload-pack");
342+ assert_eq!(request.query, "");
343+ assert_eq!(request.method, GitMethod::Post);
344+ }
345+
346+ #[tokio::test]
347+ async fn the_headers_that_change_gits_behaviour_are_forwarded() {
348+ // Both of these fail silently when dropped: one downgrades the protocol, the
349+ // other hands git a compressed body it will not recognise.
350+ let f = fixture().await;
351+
352+ f.serve_with(
353+ &Actor::Anonymous,
354+ "steid",
355+ GitEndpoint::Rpc(GitService::UploadPack),
356+ GitClientHeaders {
357+ content_encoding: Some("gzip".to_owned()),
358+ git_protocol: Some("version=2".to_owned()),
359+ ..GitClientHeaders::default()
360+ },
361+ )
362+ .await
363+ .expect("should serve")
364+ .expect("should be visible");
365+
366+ let request = f.protocol.requests().pop().expect("backend was reached");
367+ assert_eq!(request.content_encoding.as_deref(), Some("gzip"));
368+ assert_eq!(request.git_protocol.as_deref(), Some("version=2"));
369+ }
370+
371+ // --- visibility --------------------------------------------------------------
372+
373+ #[tokio::test]
374+ async fn a_public_repository_is_clonable_by_anyone() {
375+ let f = fixture().await;
376+
377+ for actor in [&Actor::Anonymous, &f.stranger, &f.member, &f.owner] {
378+ assert!(
379+ f.serve(actor, "steid", advertise_clone())
380+ .await
381+ .expect("should serve")
382+ .is_some(),
383+ "{actor:?} should be able to clone a public repository"
384+ );
385+ }
386+ }
387+
388+ #[tokio::test]
389+ async fn a_private_repository_is_absent_for_outsiders_and_never_reaches_git() {
390+ // `None`, not an error, and — the part worth testing — no bytes flow. A refusal
391+ // that arrives after the backend is spawned is not a refusal.
392+ let f = fixture().await;
393+
394+ for actor in [&Actor::Anonymous, &f.stranger] {
395+ assert!(
396+ f.serve(actor, "secret", advertise_clone())
397+ .await
398+ .expect("should serve")
399+ .is_none(),
400+ "{actor:?} should not see a private repository"
401+ );
402+ }
403+
404+ assert!(!f.protocol.was_called());
405+ }
406+
407+ #[tokio::test]
408+ async fn a_private_repository_is_clonable_by_any_member() {
409+ let f = fixture().await;
410+
411+ for actor in [&f.member, &f.owner] {
412+ assert!(
413+ f.serve(actor, "secret", advertise_clone())
414+ .await
415+ .expect("should serve")
416+ .is_some(),
417+ "{actor:?} should be able to clone a private repository"
418+ );
419+ }
420+ }
421+
422+ #[tokio::test]
423+ async fn an_unknown_repository_is_absent() {
424+ let f = fixture().await;
425+
426+ assert!(
427+ f.serve(&f.owner, "nothing-here", advertise_clone())
428+ .await
429+ .expect("should serve")
430+ .is_none()
431+ );
432+ assert!(!f.protocol.was_called());
433+ }
434+
435+ #[tokio::test]
436+ async fn an_unknown_handle_is_absent() {
437+ let f = fixture().await;
438+ let missing = OrgName::new("nobody").expect("valid handle");
439+
440+ let served = serve_git(
441+ &missing,
442+ &RepoName::new("steid").expect("valid"),
443+ advertise_clone(),
444+ GitClientHeaders::default(),
445+ Box::pin(tokio::io::empty()),
446+ &f.owner,
447+ &f.orgs,
448+ &f.memberships,
449+ &f.repos,
450+ &f.protocol,
451+ )
452+ .await
453+ .expect("should serve");
454+
455+ assert!(served.is_none());
456+ assert!(!f.protocol.was_called());
457+ }
458+
459+ // --- writes ------------------------------------------------------------------
460+
461+ #[tokio::test]
462+ async fn pushing_is_refused_for_everyone_including_the_owner() {
463+ // Milestone 4a has no way to authenticate a push, so nobody may make one. This
464+ // is an explicit refusal rather than a reliance on git's own default, which is
465+ // configuration and could change under us.
466+ let f = fixture().await;
467+
468+ for endpoint in [advertise_push(), GitEndpoint::Rpc(GitService::ReceivePack)] {
469+ for actor in [&Actor::Anonymous, &f.stranger, &f.member, &f.owner] {
470+ let error = f
471+ .serve(actor, "steid", endpoint)
472+ .await
473+ .expect_err("push should be refused");
474+
475+ assert!(
476+ matches!(error, super::super::Error::Domain(DomainError::Forbidden)),
477+ "{actor:?} on {endpoint:?} should be forbidden, got {error:?}"
478+ );
479+ }
480+ }
481+
482+ assert!(!f.protocol.was_called());
483+ }
484+
485+ #[tokio::test]
486+ async fn a_push_to_an_invisible_repository_is_absent_rather_than_forbidden() {
487+ // Order matters: existence is settled before permission. Answering "forbidden"
488+ // here would confirm that a private repository by that name exists.
489+ let f = fixture().await;
490+
491+ let served = f
492+ .serve(&Actor::Anonymous, "secret", advertise_push())
493+ .await
494+ .expect("should serve");
495+
496+ assert!(served.is_none());
497+ }
498+
499+ // --- service parsing ---------------------------------------------------------
500+
501+ #[tokio::test]
502+ async fn a_service_round_trips_through_its_wire_name() {
503+ for service in [GitService::UploadPack, GitService::ReceivePack] {
504+ assert_eq!(
505+ service
506+ .as_str()
507+ .parse::<GitService>()
508+ .expect("known service"),
509+ service
510+ );
511+ }
512+ }
513+
514+ #[tokio::test]
515+ async fn an_unknown_service_is_an_error_not_a_default() {
516+ assert!("git-do-whatever".parse::<GitService>().is_err());
517+ assert!("".parse::<GitService>().is_err());
518+ }
519+
520+ #[tokio::test]
521+ async fn only_upload_pack_reads() {
522+ assert_eq!(GitService::UploadPack.operation(), GitOperation::Read);
523+ assert_eq!(GitService::ReceivePack.operation(), GitOperation::Write);
524+ }
525+}
src/application/mod.rs+2 −0View file
@@ -7,6 +7,7 @@ pub(crate) mod authz;
77 pub mod claim;
88 pub mod config;
99 pub mod error;
10+pub mod git;
1011 pub mod identity;
1112 pub mod login;
1213 pub mod port;
@@ -17,6 +18,7 @@ pub mod session;
1718 pub use claim::{OwnerSpec, claim_instance, is_claimed};
1819 pub use config::AppConfig;
1920 pub use error::{Error, Result};
21+pub use git::{GitClientHeaders, GitEndpoint, GitOperation, GitService, serve_git};
2022 pub use identity::{Identity, describe_identity};
2123 pub use login::login;
2224 pub use profile::{PublicProfile, update_profile, view_profile};
src/application/port.rs+25 −0View file
@@ -188,6 +188,31 @@ pub struct GitResponse {
188188 pub body: ByteStream,
189189 }
190190
191+/// Written by hand because a body is a stream: it cannot be formatted without being
192+/// consumed, and a `Debug` that drains the response would be a trap.
193+impl std::fmt::Debug for GitRequest {
194+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
195+ f.debug_struct("GitRequest")
196+ .field("method", &self.method)
197+ .field("path_info", &self.path_info)
198+ .field("query", &self.query)
199+ .field("content_type", &self.content_type)
200+ .field("content_encoding", &self.content_encoding)
201+ .field("content_length", &self.content_length)
202+ .field("git_protocol", &self.git_protocol)
203+ .finish_non_exhaustive()
204+ }
205+}
206+
207+impl std::fmt::Debug for GitResponse {
208+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
209+ f.debug_struct("GitResponse")
210+ .field("status", &self.status)
211+ .field("headers", &self.headers)
212+ .finish_non_exhaustive()
213+ }
214+}
215+
191216 /// Serves the git smart-HTTP protocol for one repository.
192217 ///
193218 /// Says nothing about who may do this. Authorization is settled before a request ever