Fix forbidden file detection to check all commits

CLAUDE.md

@@ -79,8 +79,28 @@ labs:
         - cpplint
     files:                        # Required files in repo
       - lab2.cpp
+    forbidden-files:              # Files students can't modify (warning, not blocking)
+      - test_main.py              # Exact file match
+      - tests/                    # Directory prefix match
 ```
 
+### Forbidden Files Detection
+
+When `forbidden-files` is configured, the system checks commits from students only:
+- Commits by organization admins (instructors) are automatically excluded
+- Uses GitHub Organization Membership API to determine instructor status
+- Only modifications by non-admin users are checked against forbidden files
+- If violations found: grade is still written, but cell is highlighted yellow with a comment
+- Warning is shown to student on frontend
+- This helps detect students who modify test files to cheat
+
+**How it works:**
+1. System fetches all commits from student repository
+2. For each commit, checks if author is an org admin via GitHub API
+3. Instructor commits are skipped (they can update tests/templates)
+4. Student commits are checked for forbidden file modifications
+5. Works with GitHub Classroom - students are not org admins by default
+
 ## CI/CD
 
 - **Tests**: Run on every push via `.github/workflows/tests.yml`

frontend/courses-front/src/components/registration-form/index.jsx

@@ -47,11 +47,12 @@ const handleSubmit = async () => {
 
       if (gradeResponse.status === "updated") {
         setCheckResult({
-          type: "success",
+          type: gradeResponse.warning ? "warning" : "success",
           message: gradeResponse.message,
           result: gradeResponse.result,
           passed: gradeResponse.passed,
           checks: gradeResponse.checks,
+          warning: gradeResponse.warning,
         });
       } else if (gradeResponse.status === "rejected") {
         setCheckResult({
@@ -208,6 +209,36 @@ const handleSubmit = async () => {
               ))}
             </div>
           )}
+
+          {checkResult.warning && (
+            <div style={{
+              marginTop: "12px",
+              padding: "12px",
+              borderRadius: "6px",
+              backgroundColor: "#fff8e1",
+              border: `1px solid ${colors.edit}`,
+            }}>
+              <div style={{
+                fontSize: "14px",
+                fontWeight: "500",
+                marginBottom: "8px",
+                color: "#e65100",
+              }}>
+                {checkResult.warning.message}
+              </div>
+              {checkResult.warning.files && checkResult.warning.files.length > 0 && (
+                <div style={{
+                  fontSize: "13px",
+                  color: colors.textSecondary,
+                  fontFamily: "monospace",
+                }}>
+                  {checkResult.warning.files.map((file, index) => (
+                    <div key={index}>• {file}</div>
+                  ))}
+                </div>
+              )}
+            </div>
+          )}
         </div>
       )}
     </MainContainer>

grading/__init__.py

@@ -38,6 +38,7 @@
     GitHubClient,
     CommitInfo,
     check_forbidden_modifications,
+    check_forbidden_files_in_list,
     get_default_forbidden_patterns,
 )
 
@@ -49,6 +50,8 @@
     prepare_grade_update,
     get_deadline_from_sheet,
     get_student_order,
+    set_cell_warning,
+    format_forbidden_files_note,
     StudentLocation,
     LabColumn,
     GradeUpdate,
@@ -58,6 +61,7 @@
     LabGrader,
     GradeResult,
     GradeStatus,
+    ForbiddenFilesWarning,
 )
 
 __all__ = [
@@ -83,6 +87,7 @@
     "GitHubClient",
     "CommitInfo",
     "check_forbidden_modifications",
+    "check_forbidden_files_in_list",
     "get_default_forbidden_patterns",
     # sheets_client
     "find_student_row",
@@ -92,11 +97,14 @@
     "prepare_grade_update",
     "get_deadline_from_sheet",
     "get_student_order",
+    "set_cell_warning",
+    "format_forbidden_files_note",
     "StudentLocation",
     "LabColumn",
     "GradeUpdate",
     # grader
     "LabGrader",
     "GradeResult",
     "GradeStatus",
+    "ForbiddenFilesWarning",
 ]

grading/github_client.py

@@ -14,6 +14,7 @@ class CommitInfo:
     """Information about a commit."""
     sha: str
     files: list[dict[str, Any]]  # List of {filename, status, ...}
+    all_modified_files: list[str] | None = None  # All files modified across all commits
 
 
 @dataclass
@@ -196,6 +197,138 @@ def get_job_logs(self, org: str, repo: str, job_id: int) -> str | None:
 
         return resp.text
 
+    def is_org_admin(self, org: str, username: str) -> bool:
+        """
+        Check if a user is an admin of the organization.
+
+        This is used to distinguish instructor commits from student commits.
+        Only commits from org admins are considered "instructor commits" and
+        are excluded from forbidden file checks.
+
+        Args:
+            org: Organization name
+            username: GitHub username to check
+
+        Returns:
+            True if user is an admin, False otherwise
+
+        Note:
+            Returns False if user is not a member, or if API call fails.
+            Students should NOT be org admins in GitHub Classroom setup.
+        """
+        if not username:
+            return False
+
+        # Check membership and role
+        url = f"{self.BASE_URL}/orgs/{org}/memberships/{username}"
+        resp = requests.get(url, headers=self.headers)
+
+        if resp.status_code == 200:
+            data = resp.json()
+            role = data.get("role")
+            # Only admins are considered instructors
+            return role == "admin"
+
+        # Not a member or API error - assume student
+        return False
+
+    def get_commits_with_authors(self, org: str, repo: str) -> list[dict[str, Any]]:
+        """
+        Get all commits with author information and modified files.
+
+        This iterates through all commits and returns detailed information
+        including author GitHub login and modified files.
+
+        Args:
+            org: Organization or user name
+            repo: Repository name
+
+        Returns:
+            List of dicts with keys:
+                - sha: commit SHA
+                - author_login: GitHub username (or None if no GitHub author)
+                - files: list of modified file paths
+
+        Example:
+            [
+                {
+                    "sha": "abc123",
+                    "author_login": "student123",
+                    "files": ["main.cpp", "test.cpp"]
+                },
+                ...
+            ]
+        """
+        commits_info = []
+
+        # Get all commits (paginated)
+        page = 1
+        per_page = 100
+        while True:
+            url = f"{self.BASE_URL}/repos/{org}/{repo}/commits"
+            params = {"page": page, "per_page": per_page}
+            resp = requests.get(url, headers=self.headers, params=params)
+
+            if resp.status_code != 200:
+                break
+
+            commits = resp.json()
+            if not commits:
+                break
+
+            # Get details for each commit
+            for commit_summary in commits:
+                commit_sha = commit_summary["sha"]
+
+                # Extract author login (GitHub user)
+                author_login = None
+                if commit_summary.get("author"):
+                    author_login = commit_summary["author"].get("login")
+
+                # Get commit details with files
+                commit_url = f"{self.BASE_URL}/repos/{org}/{repo}/commits/{commit_sha}"
+                commit_resp = requests.get(commit_url, headers=self.headers)
+
+                if commit_resp.status_code == 200:
+                    commit_data = commit_resp.json()
+                    files = [f.get("filename") for f in commit_data.get("files", []) if f.get("filename")]
+
+                    commits_info.append({
+                        "sha": commit_sha,
+                        "author_login": author_login,
+                        "files": files,
+                    })
+
+            # Check if there are more pages
+            if len(commits) < per_page:
+                break
+            page += 1
+
+        return commits_info
+
+    def get_all_modified_files(self, org: str, repo: str) -> list[str]:
+        """
+        Get all files that were modified across all commits in the repository.
+
+        This iterates through all commits and collects unique file paths
+        that were added, modified, or removed.
+
+        Args:
+            org: Organization or user name
+            repo: Repository name
+
+        Returns:
+            List of unique file paths that were modified in any commit
+        """
+        commits = self.get_commits_with_authors(org, repo)
+        modified_files: set[str] = set()
+
+        for commit in commits:
+            for filename in commit["files"]:
+                modified_files.add(filename)
+
+        return list(modified_files)
+
 
 def check_forbidden_modifications(
     commit_files: list[dict[str, Any]],
@@ -235,6 +368,42 @@ def check_forbidden_modifications(
     return violations
 
 
+def check_forbidden_files_in_list(
+    all_files: list[str],
+    forbidden_patterns: list[str]
+) -> list[str]:
+    """
+    Check if any forbidden files are in the list of modified files.
+
+    Unlike check_forbidden_modifications, this works with a simple list of filenames
+    without status information. Use this when checking all files modified across
+    all commits in a repository.
+
+    Args:
+        all_files: List of file paths that were modified
+        forbidden_patterns: List of forbidden file paths or prefixes
+
+    Returns:
+        List of forbidden files that were found
+
+    Examples:
+        >>> check_forbidden_files_in_list(["test_main.py", "main.cpp"], ["test_main.py"])
+        ['test_main.py']
+        >>> check_forbidden_files_in_list(["tests/test1.py"], ["tests/"])
+        ['tests/test1.py']
+    """
+    violations = []
+
+    for filename in all_files:
+        for pattern in forbidden_patterns:
+            # Exact match or prefix match (for directories like "tests/")
+            if filename == pattern or filename.startswith(pattern):
+                violations.append(filename)
+                break
+
+    return violations
+
+
 def get_default_forbidden_patterns(required_files: list[str]) -> list[str]:
     """
     Get default forbidden file patterns based on required files.