From 5bbed27e46c61744b1ad0d08b0abea4e50fb06fc Mon Sep 17 00:00:00 2001
From: Yggdrasil <yggdrasilworld@users.noreply.github.com>
Date: Wed, 14 Oct 2026 02:33:49 -0400
Subject: [PATCH] fix(guard): case-insensitive repo lookup + no silent
 ownership-grab
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- #564: the fullName lookup was case-sensitive while isAllowlisted
  lowercases — GitHub names are case-insensitive, so Owner/Repo vs
  owner/repo resolved differently. Match with ILike.
- #565: an unclaimed tracked repo was silently assigned to the first
  maintainer to sync it ('lets the first claimant take it'). Ownership
  shouldn't be a sync side-effect — allow the sync, keep it unassigned.

Resolves #564, #565
---
 src/auth/guards/repo-scope.guard.spec.ts | 22 +++++++++++-----------
 src/auth/guards/repo-scope.guard.ts      | 24 ++++++++++++++++--------
 2 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/auth/guards/repo-scope.guard.spec.ts b/src/auth/guards/repo-scope.guard.spec.ts
index fc2d8c45..d2a14a19 100644
--- a/src/auth/guards/repo-scope.guard.spec.ts
+++ b/src/auth/guards/repo-scope.guard.spec.ts
@@ -102,7 +102,10 @@ describe('RepoScopeGuard', () => {
     );
   });
 
-  it('claims an unassigned tracked repository for the first maintainer to sync it', async () => {
+  // Ownership is no longer a sync side-effect (#565) — an unclaimed repo is
+  // syncable but NOT auto-assigned to whoever happens to sync it first;
+  // maintainerId stays null until it's set explicitly.
+  it('allows an unassigned tracked repository without silently claiming it', async () => {
     userRepo.findOne.mockResolvedValue({
       id: 'user-1',
       roles: [UserRole.MAINTAINER],
@@ -113,10 +116,7 @@ describe('RepoScopeGuard', () => {
     });
 
     await expect(guard.canActivate(maintainerContext())).resolves.toBe(true);
-    expect(repositoryRepo.update).toHaveBeenCalledWith(
-      { id: 'repo-uuid-1' },
-      { maintainerId: 'user-1' },
-    );
+    expect(repositoryRepo.update).not.toHaveBeenCalled();
   });
 
   it('refuses a first-time sync of an untracked repository', async () => {
@@ -168,9 +168,9 @@ describe('RepoScopeGuard', () => {
   });
 
   it('refuses an unauthenticated request', async () => {
-    await expect(guard.canActivate(context({ owner: 'acme', repo: 'repo' }))).rejects.toThrow(
-      /Authentication is required/,
-    );
+    await expect(
+      guard.canActivate(context({ owner: 'acme', repo: 'repo' })),
+    ).rejects.toThrow(/Authentication is required/);
   });
 
   it('refuses a request whose authenticated user no longer exists', async () => {
@@ -182,8 +182,8 @@ describe('RepoScopeGuard', () => {
   });
 
   it('refuses a request with no repository parameters', async () => {
-    await expect(guard.canActivate(context({ user: { userId: 'user-1' } }))).rejects.toThrow(
-      NotFoundException,
-    );
+    await expect(
+      guard.canActivate(context({ user: { userId: 'user-1' } })),
+    ).rejects.toThrow(NotFoundException);
   });
 });
diff --git a/src/auth/guards/repo-scope.guard.ts b/src/auth/guards/repo-scope.guard.ts
index 0e70ef2c..8e8648d8 100644
--- a/src/auth/guards/repo-scope.guard.ts
+++ b/src/auth/guards/repo-scope.guard.ts
@@ -9,7 +9,7 @@ import { Reflector } from '@nestjs/core';
 import { InjectRepository } from '@nestjs/typeorm';
 import { ConfigService } from '@nestjs/config';
 import { Request } from 'express';
-import { Repository } from 'typeorm';
+import { ILike, Repository } from 'typeorm';
 import { Repository as RepositoryEntity } from '../../common/entities';
 import { User } from '../../common/entities';
 import { UserRole } from '../../common/enums';
@@ -75,13 +75,17 @@ export class RepoScopeGuard implements CanActivate {
       where: { id: request.user.userId },
       select: { id: true, roles: true },
     });
-    if (!user) throw new ForbiddenException('Authenticated user no longer exists');
+    if (!user)
+      throw new ForbiddenException('Authenticated user no longer exists');
     if (!user.roles.includes(UserRole.MAINTAINER)) {
       throw new ForbiddenException('Maintainer role required');
     }
 
+    // GitHub owner/repo names are case-insensitive — the isAllowlisted check
+    // lowercases both sides but this lookup didn't, so `Owner/Repo` and
+    // `owner/repo` resolved differently (#564). Match case-insensitively.
     const tracked = await this.repositoryRepo.findOne({
-      where: { fullName },
+      where: { fullName: ILike(fullName) },
       select: { id: true, maintainerId: true },
     });
 
@@ -102,16 +106,20 @@ export class RepoScopeGuard implements CanActivate {
       );
     }
 
-    // Tracked with no maintainer yet: allow, and claim it for this user so the
-    // scope is explicit from here on.
-    await this.repositoryRepo.update({ id: tracked.id }, { maintainerId: user.id });
+    // Tracked with no maintainer yet: the old code silently assigned it to
+    // the first requester ("lets the first claimant take it", #565) — any
+    // maintainer could claim ownership of any unclaimed repo just by syncing
+    // it. Don't self-assign; a repo's owner must be set explicitly. Allow the
+    // sync but leave maintainerId null — ownership assignment is a separate,
+    // deliberate action.
     return true;
   }
 
   /** Exact `owner/repo` entries from `GITHUB_SYNC_ALLOWED_REPOS`. */
   private isAllowlisted(fullName: string): boolean {
-    const raw = this.configService.get('github', { infer: true })
-      .syncAllowedRepos;
+    const raw = this.configService.get('github', {
+      infer: true,
+    }).syncAllowedRepos;
     if (!raw) return false;
     return raw
       .split(',')