Programming exercises: Make .git(.*) files removable and creatable in online editor

[theblobinthesky]

Checklist

General

• This is a small issue that I tested locally and was confirmed by another developer on a test server.
• I chose a title conforming to the naming conventions for pull requests.

Server

• Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
• I strictly followed the server coding and design guidelines.

Motivation and Context

This PR enables users to delete all .git(.*) files, which was incorrectly prohibited previously.
Fixes issue #9309.

Description

To delete a file e.g. in the 'Test' repository of a programming exercise, a file must be 'valid' according to the Repository::isValidFile method. To protect the '.git' folder from deletion, all files with '.git' in their path could not be deleted. Now, deletion only fails, iff the file itself is the '.git' directory or a if the a parent directory is the '.git' folder. This ensures harmless files like '.gitignore' and '.gitkeep' can be deleted, but the '.git' folder itself is still protected.

Steps to reproduce:
-) Create a new programming exercise with the default contents.
-) Click on "Edit in editor" to open the File Viewer.
-) Select the "TESTS" repository
-) Create the file ".gitkeep" in the root directory. Notice how it works.
-) Create the directory ".git" in the root directory. Notice how it errors out.
-) Delete the file ".gitkeep" in the root directory. Notice how it works.

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Summary by CodeRabbit

• Bug Fixes
  • Improved file validation to more accurately detect invalid files and directories, reducing false positives related to ".git" in file paths.

[coderabbitai]

Walkthrough

The isValidFile method in the Repository.java file was refactored to improve the logic for validating files. The check for null files and path traversal using "../" remains as an initial filter. The subsequent check for the presence of ".git" was refined: instead of rejecting any file with ".git" in its path, the method now only returns false if the file is a directory named exactly ".git". The method signature and recursive logic remain unchanged.

Changes

File Path	Change Summary
src/main/java/de/tum/cit/aet/artemis/programming/domain/Repository.java	Refactored isValidFile to separate null/path traversal checks from directory name check; narrowed ".git" check to directories named exactly ".git".

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Repository
    participant File

    Caller->>Repository: isValidFile(File)
    alt file is null or path contains "../"
        Repository-->>Caller: return false
    else file is directory named ".git"
        Repository-->>Caller: return false
    else
        Repository-->>Caller: (proceed with other logic)
    end

Loading

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)

• We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.

---

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91466eb and 1215452.

📒 Files selected for processing (1)

• src/main/java/de/tum/cit/aet/artemis/programming/domain/Repository.java (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...

src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

• src/main/java/de/tum/cit/aet/artemis/programming/domain/Repository.java

⏰ Context from checks skipped due to timeout of 90000ms (1)

• GitHub Check: Codacy Static Code Analysis

🔇 Additional comments (3)

src/main/java/de/tum/cit/aet/artemis/programming/domain/Repository.java (3)

52-54: Well-structured null and path traversal check

The separation of the null check and path traversal check from the ".git" check improves code clarity and follows the single responsibility principle.

---

56-58: Improved .git directory validation

This change correctly refines the validation to only block the ".git" directory itself rather than any file with ".git" in its path. The recursive call on line 64 ensures files inside the ".git" directory are also protected, while allowing legitimate files like ".gitignore" to be deleted.

---

51-65:

Details

✅ Verification successful

Verify edge case handling

The implementation correctly addresses the PR objectives by allowing deletion of files like ".gitignore" while protecting the ".git" directory.

---

🏁 Script executed:

#!/bin/bash
# Verify the implementation with some sample paths
echo "Test cases for isValidFile:"
echo "1. Case: .gitignore file (should be valid)"
echo "2. Case: .git directory (should be invalid)"
echo "3. Case: file inside .git directory like .git/config (should be invalid)"
echo "4. Case: directory with .git in name like my.github (should be valid)"
echo "5. Case: nested .git directory like project/.git/hooks (should be invalid)"

# Look for other usages of isValidFile to ensure compatibility
echo "Usages of isValidFile method:"
rg --no-filename "isValidFile\(" -A 2 -B 2

Length of output: 5607

---

Edge Case Handling Verified

The recursive approach correctly ensures that:

• Files with parent directories being a ".git" directory (or nested under one) are invalid.
• Files like ".gitignore" are allowed since only a directory exactly named ".git" is rejected.
• The check for "../" in the file path prevents potential directory traversal issues.
• The usage of the repository’s local path as a base case reinforces correct behavior.

All sample test cases and usage instances in the code confirm that the implementation meets the PR objectives.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[github-actions]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[marlonnienaber]

Code looks fine but the branch can't be deployed on Helios. I guess thats because the branch is coming from a fork. How should be handle this @krusche? I tested locally: Unfortunately I can not create .gitignore/.gitkeep files as requested in the issue. Is there any other steps necessary to allow this, besides creating a programming exercise and allowing the online editor? If so please add precise testing instructions. Thx :)

[theblobinthesky]

Oh yeah. Sorry about the fork, this branch was created when i didn't have push rights yet.

[theblobinthesky]

I've added detailed steps to reproduce in the description.
Both creating files and deleting files works for me.

This is how it looks on my machine:
Before this modification
After this modification

[b-fein]

Did you check that the issue only occurs for filenames starting with .git? The original issue report mentioned any files starting with a . but only gave the .gitkeep files as an example. If other hidden files are indeed not affected, then the solution makes sense and looks simple enough apart from the question in the inline comment.

[marlonnienaber]

I opened the wrong online editor (for working on exercises) initially. Tested again locally. Works as expected. Code looks good to me.

[b-fein]

Code looks good after inline questions were clarified.

[HawKhiem]

Code 👍

[jonas-de]

can't test on Helios - code looks good