docs: add read:user to required token scopes

The read:user scope is needed for the bot to self-request as a
reviewer on PRs. Without it, the bot still functions but cannot
add itself to the reviewer list.

Closes #66

README.md

@@ -329,11 +329,12 @@ All flags have environment variable equivalents:
 ### Token Scopes Required
 
 | Scope | Purpose |
-|-------|---------|
+|-------|--------|
 | `write:issue` | Post and delete reviews |
 | `write:repository` | Read PR diffs, file content, commit statuses |
+| `read:user` | Self-request as reviewer (optional but recommended) |
 
-No `read:user` scope needed — the bot identifies itself from the review response.
+Without `read:user`, the bot still works but cannot add itself to the PR's reviewer list.
 
 ## Development
 

cmd/review-bot/main.go

@@ -79,7 +79,6 @@ func main() {
 	aicoreAPIURL := flag.String("aicore-api-url", envOrDefault("AICORE_API_URL", ""), "SAP AI Core API URL (for provider=aicore)")
 	aicoreResourceGroup := flag.String("aicore-resource-group", envOrDefault("AICORE_RESOURCE_GROUP", "default"), "SAP AI Core resource group (for provider=aicore)")
 
-	flag.Parse()
 	flag.Parse()
 
 	if *versionFlag {
@@ -181,7 +180,9 @@ func main() {
 		repoPersonas, err := review.LoadRepoPersonas(ctx, newGiteaClientAdapter(giteaClient), owner, repoName)
 		if err != nil {
 			slog.Warn("could not load repo personas", "repo", owner+"/"+repoName, "error", err)
-			// Continue with built-in personas only
+			// Continue with built-in personas only.
+			// NOTE: repoPersonas is nil here, but map indexing on a nil map is safe in Go
+			// (returns the zero value), so the fallback to built-in below works correctly.
 		}
 		if p, ok := repoPersonas[*personaName]; ok {
 			persona = p

review/repo_persona.go

@@ -28,6 +28,7 @@ type ContentEntry struct {
 // Returns an empty map (not nil) if the directory doesn't exist or is empty.
 // Individual parse failures are logged and skipped; the remaining personas are still returned.
 // Auth errors and other non-404 errors are propagated.
+// Files exceeding MaxPersonaFileSize are rejected to prevent resource exhaustion.
 func LoadRepoPersonas(ctx context.Context, client GiteaClient, owner, repo string) (map[string]*Persona, error) {
 	result := make(map[string]*Persona)
 
@@ -65,6 +66,16 @@ func LoadRepoPersonas(ctx context.Context, client GiteaClient, owner, repo strin
 			continue
 		}
 
+		// Enforce size limit before parsing to prevent resource exhaustion
+		if len(content) > MaxPersonaFileSize {
+			slog.Warn("repo persona file exceeds maximum size",
+				"file", entry.Path,
+				"repo", owner+"/"+repo,
+				"size", len(content),
+				"max", MaxPersonaFileSize)
+			continue
+		}
+
 		persona, err := ParsePersonaBytes([]byte(content), entry.Path)
 		if err != nil {
 			slog.Warn("could not parse repo persona file",
@@ -128,11 +139,12 @@ func isYAMLFile(name string) bool {
 }
 
 // isNotFoundError checks if an error represents a 404 response.
-// This is a string-based heuristic since we don't have access to gitea.APIError here.
+// This uses a specific "HTTP 404" substring match rather than a generic "not found"
+// match to avoid masking authentication failures or transport errors that might
+// contain "not found" in their message.
 func isNotFoundError(err error) bool {
 	if err == nil {
 		return false
 	}
-	errStr := err.Error()
-	return strings.Contains(errStr, "HTTP 404") || strings.Contains(errStr, "not found")
+	return strings.Contains(err.Error(), "HTTP 404")
 }

review/repo_persona_test.go

@@ -288,6 +288,36 @@ identity: Good persona
 			t.Fatalf("expected 1 persona (skipped failed fetch), got %d", len(personas))
 		}
 	})
+
+	t.Run("skips oversized files", func(t *testing.T) {
+		// Create a content string that exceeds MaxPersonaFileSize (64KB)
+		oversizedContent := strings.Repeat("a", MaxPersonaFileSize+1)
+		client := &mockGiteaClient{
+			contents: map[string][]ContentEntry{
+				RepoPersonaPath: {
+					{Name: "normal.yaml", Path: ".review-bot/personas/normal.yaml", Type: "file"},
+					{Name: "huge.yaml", Path: ".review-bot/personas/huge.yaml", Type: "file"},
+				},
+			},
+			files: map[string]string{
+				".review-bot/personas/normal.yaml": `name: normal
+identity: Normal sized persona
+`,
+				".review-bot/personas/huge.yaml": oversizedContent,
+			},
+		}
+		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		// Should have the normal one, skip the oversized
+		if len(personas) != 1 {
+			t.Fatalf("expected 1 persona (skipped oversized), got %d", len(personas))
+		}
+		if personas["normal"] == nil {
+			t.Error("expected normal persona")
+		}
+	})
 }
 
 func TestMergePersonas(t *testing.T) {
@@ -391,7 +421,10 @@ func TestIsNotFoundError(t *testing.T) {
 	}{
 		{nil, false},
 		{errors.New("HTTP 404: not found"), true},
-		{errors.New("something not found"), true},
+		{errors.New("HTTP 404"), true},
+		// Intentionally false: generic "not found" could mask auth/transport errors.
+		// Only explicit HTTP 404 responses should be treated as "directory doesn't exist".
+		{errors.New("something not found"), false},
 		{errors.New("HTTP 401: unauthorized"), false},
 		{errors.New("connection refused"), false},
 	}