Merge branch 'worktree-fix-c3-ssh-authz'
zhenyi committed
Modified dvcs/gitssh/handler/command.rs dvcs/gitssh/handler/command.rs +13 −1 View dvcs/gitssh/handler/command.rs
@@ -44,8 +44,20 @@
4444 };
4545 service_label = service.as_subcommand();
4646
47 // Build the actor with no pre-granted repo scopes so that
48 // `repo::permission::effective_access` evaluates the real
49 // space_role / collaborator / is_public checks instead of
50 // short-circuiting on an unconditional RepoRead scope (C3:
51 // any SSH user could clone any private repo).
4752 let actor = match &self.operator {
48 Some(u) => ActorContext::for_user(u.uid),
53 Some(u) => ActorContext {
54 user_id: Some(u.uid),
55 scopes: Default::default(),
56 is_site_admin: u.is_admin,
57 credential_max: None,
58 channel_role: None,
59 space_role: None,
60 },
4961 None => ActorContext::anonymous(),
5062 };
5163 let required = if service.is_write() {
Modified lib/repo/actor.rs lib/repo/actor.rs +0 −13 View lib/repo/actor.rs
@@ -73,19 +73,6 @@
7373 }
7474 }
7575
76 pub fn for_user(user_id: Uuid) -> Self {
77 let mut scopes = HashSet::new();
78 scopes.insert(TokenScope::RepoRead);
79 Self {
80 user_id: Some(user_id),
81 scopes,
82 is_site_admin: false,
83 credential_max: None,
84 channel_role: None,
85 space_role: None,
86 }
87 }
88
8976 pub fn has_scope(&self, scope: TokenScope) -> bool {
9077 self.is_site_admin || self.scopes.iter().any(|s| s.implies(scope))
9178 }
Modified lib/repo/permission.rs lib/repo/permission.rs +61 −6 View lib/repo/permission.rs
@@ -209,10 +209,39 @@
209209 }
210210 }
211211
212 fn require_read(actor: &ActorContext, is_public: bool) -> Result<(), PermissionError> {
213 require(actor, RepoAccess::Read, None, None, None, is_public)
214 }
215
216 // C3 regression: the SSH clone gate (`require(Read)` on a private repo)
217 // must deny a plain authenticated outsider. Before the C3 fix the SSH
218 // actor carried an unconditional RepoRead scope that short-circuited
219 // `effective_access` to Read, so this returned Ok.
220 #[test]
221 fn ssh_plain_user_cannot_clone_private_repo() {
222 let a = actor(Some(Uuid::now_v7()), false);
223 assert_eq!(require_read(&a, false), Err(PermissionError::Insufficient));
224 }
225
226 #[test]
227 fn ssh_plain_user_can_clone_public_repo() {
228 let a = actor(Some(Uuid::now_v7()), false);
229 assert_eq!(require_read(&a, true), Ok(()));
230 }
231
232 #[test]
233 fn anonymous_cannot_read_private_repo() {
234 let anon = actor(None, false);
235 assert_eq!(require_read(&anon, false), Err(PermissionError::Anonymous));
236 }
237
212238 #[test]
213239 fn site_admin_is_admin() {
214240 let a = actor(Some(Uuid::now_v7()), true);
215 assert_eq!(effective_access(&a, None, None, None, false), RepoAccess::Admin);
241 assert_eq!(
242 effective_access(&a, None, None, None, false),
243 RepoAccess::Admin
244 );
216245 }
217246
218247 #[test]
@@ -258,13 +287,19 @@
258287 #[test]
259288 fn public_repo_readable_for_authenticated_outsider() {
260289 let a = actor(Some(Uuid::now_v7()), false);
261 assert_eq!(effective_access(&a, None, None, None, true), RepoAccess::Read);
290 assert_eq!(
291 effective_access(&a, None, None, None, true),
292 RepoAccess::Read
293 );
262294 }
263295
264296 #[test]
265297 fn private_repo_none_for_outsider() {
266298 let a = actor(Some(Uuid::now_v7()), false);
267 assert_eq!(effective_access(&a, None, None, None, false), RepoAccess::None);
299 assert_eq!(
300 effective_access(&a, None, None, None, false),
301 RepoAccess::None
302 );
268303 }
269304
270305 #[test]
@@ -283,10 +318,30 @@
283318 #[test]
284319 fn require_write_gating() {
285320 let owner = actor(Some(Uuid::now_v7()), false);
286 assert!(require(&owner, RepoAccess::Write, Some(ChannelRole::Owner), None, None, false).is_ok());
321 assert!(
322 require(
323 &owner,
324 RepoAccess::Write,
325 Some(ChannelRole::Owner),
326 None,
327 None,
328 false
329 )
330 .is_ok()
331 );
287332
288333 let moderator = actor(Some(Uuid::now_v7()), false);
289 assert!(require(&moderator, RepoAccess::Admin, Some(ChannelRole::Moderator), None, None, false).is_err());
334 assert!(
335 require(
336 &moderator,
337 RepoAccess::Admin,
338 Some(ChannelRole::Moderator),
339 None,
340 None,
341 false
342 )
343 .is_err()
344 );
290345
291346 let outsider = actor(Some(Uuid::now_v7()), false);
292347 assert!(require(&outsider, RepoAccess::Write, None, None, None, true).is_err());
@@ -294,4 +349,4 @@
294349 let anon = actor(None, false);
295350 assert!(require(&anon, RepoAccess::Read, None, None, None, true).is_ok());
296351 }
297}
No newline at end of file
352}