Integrated code lifecycle: Add auxiliary repositories in exercise export and import
#9612
Conversation
github-actions
bot
added
server
WalkthroughThe pull request introduces modifications to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (2)
Line range hint 182-188: Consider adding null check for programming exercise task service.
While the type check is good, we should ensure the programmingExerciseTaskService is not null before invoking methods on it.
@Override
protected void exportProblemStatementAndEmbeddedFilesAndExerciseDetails(Exercise exercise, List<String> exportErrors, Path exportDir, List<Path> pathsToBeZipped)
throws IOException {
if (exercise instanceof ProgrammingExercise programmingExercise) {
- // Used for a save typecast, this should always be true since this class only works with programming exercises.
- programmingExerciseTaskService.replaceTestIdsWithNames(programmingExercise);
+ // Used for a safe typecast, this should always be true since this class only works with programming exercises.
+ if (programmingExerciseTaskService != null) {
+ programmingExerciseTaskService.replaceTestIdsWithNames(programmingExercise);
+ }
}
super.exportProblemStatementAndEmbeddedFilesAndExerciseDetails(exercise, exportErrors, exportDir, pathsToBeZipped);
}Line range hint 182-188: Fix typo in comment.
The comment has a typo: "save" should be "safe".
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (2)
181-182: Consolidate calls to replaceImportedExerciseShortName
To improve readability and reduce duplication, consider combining the repositories into a single list before calling replaceImportedExerciseShortName.
Apply this diff to consolidate the calls:
- replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), List.of(solutionRepo, templateRepo, testRepo));
- replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), auxiliaryRepositories);
+ List<Repository> allRepositories = new ArrayList<>();
+ allRepositories.addAll(List.of(templateRepo, solutionRepo, testRepo));
+ allRepositories.addAll(auxiliaryRepositories);
+ replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), allRepositories);187-189: Refactor staging and committing of repository changes
There is repeated code for staging and committing changes for both main and auxiliary repositories. Consider refactoring this into a loop to handle all repositories uniformly.
Apply this diff to refactor the code:
gitService.stageAllChanges(templateRepo);
gitService.stageAllChanges(solutionRepo);
gitService.stageAllChanges(testRepo);
- for (Repository auxRepo : auxiliaryRepositories) {
- gitService.stageAllChanges(auxRepo);
- }
+ List<Repository> allRepositories = new ArrayList<>();
+ allRepositories.addAll(List.of(templateRepo, solutionRepo, testRepo));
+ allRepositories.addAll(auxiliaryRepositories);
+ for (Repository repo : allRepositories) {
+ gitService.stageAllChanges(repo);
+ }
gitService.commitAndPush(templateRepo, "Import template from file", true, user);
gitService.commitAndPush(solutionRepo, "Import solution from file", true, user);
gitService.commitAndPush(testRepo, "Import tests from file", true, user);
- for (Repository auxRepo : auxiliaryRepositories) {
- gitService.commitAndPush(auxRepo, "Import auxiliary repo from file", true, user);
- }
+ for (Repository repo : auxiliaryRepositories) {
+ gitService.commitAndPush(repo, "Import auxiliary repo from file", true, user);
+ }Also applies to: 194-196
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (4 hunks)
- src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (1)
Pattern 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/service/ProgrammingExerciseImportFromFileService.java (1)
Pattern 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/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts (1)
🪛 ast-grep
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
[warning] 176-176: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (new VcsRepositoryUri(auxiliaryRepository.getRepositoryUri()), false)
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 232-236: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (
retrieveRepositoryDirectoryPath(basePath, repoName).toFile(),
repository.getLocalPath().toFile(),
new NotFileFilter(new NameFileFilter(".git"))
)
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 235-235: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (new NameFileFilter(".git"))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 176-176: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (new VcsRepositoryUri(auxiliaryRepository.getRepositoryUri()), false)
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 232-236: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (
retrieveRepositoryDirectoryPath(basePath, repoName).toFile(),
repository.getLocalPath().toFile(),
new NotFileFilter(new NameFileFilter(".git"))
)
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 235-235: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (new NameFileFilter(".git"))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
🪛 Biome
src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts
[error] 53-53: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (9)
src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts (3)
55-57: LGTM! Good backward compatibility handling.
The code properly handles legacy exercise imports by checking and copying build config when needed. The comment clearly explains the purpose.
63-63: LGTM! Proper reassignment after modifications.
The code correctly assigns the modified programming exercise back to the base exercise property.
58-62: Verify integration with export functionality.
The changes handle auxiliary repositories during import, but we should verify that this integrates correctly with the export functionality mentioned in the PR objectives.
✅ Verification successful
The previous search might have been too specific. Let's broaden our search to understand both the auxiliary repositories handling and export functionality separately, which will help us verify their integration.
Based on the search results, I can now generate a final response as we have sufficient information about both the auxiliary repositories handling and export functionality.
The auxiliary repository handling during import is correctly implemented.
The code changes are consistent with both import and export functionality:
- Import: The code correctly handles auxiliary repositories by resetting their IDs during import (
exercise-import-from-file.component.ts) - Export: The system supports auxiliary repositories in exports as evidenced by:
- The model definition in
programming-exercise.model.ts - Export UI components like
programming-assessment-repo-export - Proper handling in both import and export JSON structures
- The model definition in
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for corresponding export handling of auxiliary repositories
rg -l "auxiliaryRepositories.*export" "src/main/webapp"
Length of output: 55
Script:
#!/bin/bash
# Let's try a broader search for auxiliary repositories handling
rg -A 5 "auxiliaryRepositories" "src/main/webapp"
# Also search for export-related functionality
rg -A 5 "export.*programming.*exercise" "src/main/webapp"
Length of output: 36500
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (2)
177-178: LGTM: Auxiliary repositories are correctly fetched and set.
The addition of auxiliary repositories aligns with the PR objectives and is placed at the appropriate point in the export flow.
177-178: Verify auxiliary repository handling across the codebase.
Let's ensure the auxiliary repositories are consistently handled throughout the export process.
✅ Verification successful
Let me analyze the results and generate one more verification script to complete our analysis.
Based on the verification results, I can now provide the final response:
Auxiliary repository handling is properly implemented in the export-import flow
The auxiliary repositories are:
- Correctly fetched during export using
auxiliaryRepositoryRepository.findByExerciseId() - Properly saved during import via
auxiliaryRepositoryRepository.save() - Consistently used across the codebase in related services (export, import, and repository services)
No null pointer or consistency issues were found in the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent handling of auxiliary repositories in export-related code
# Test 1: Check for auxiliary repository usage in export-related files
echo "Checking for auxiliary repository handling in export services..."
rg -l "auxiliaryRepository" --type java
# Test 2: Check for potential NPE with programmingExerciseTaskService
echo "Checking for null checks on programmingExerciseTaskService..."
ast-grep --pattern 'programmingExerciseTaskService.$_($_)'
Length of output: 9363
Script:
#!/bin/bash
# Check how auxiliary repositories are handled in the export/import process
# Check the export service implementation
echo "Checking auxiliary repository handling in export service..."
rg -A 10 "setAuxiliaryRepositories" src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java
# Check the import service implementation
echo "Checking auxiliary repository handling in import service..."
rg -A 10 "auxiliaryRepository" src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java
# Check for potential NPE scenarios
echo "Checking for null checks on auxiliary repositories..."
rg "auxiliaryRepositories.*null" --type java
Length of output: 3804
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (4)
175-178: Initialization of auxiliary repositories is correctly implemented
The auxiliary repositories are properly initialized and populated.
🧰 Tools
🪛 ast-grep
[warning] 176-176: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (new VcsRepositoryUri(auxiliaryRepository.getRepositoryUri()), false)
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 176-176: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (new VcsRepositoryUri(auxiliaryRepository.getRepositoryUri()), false)
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
277-277: Ensure stream is properly closed after use
While using streams, it's important to ensure they are properly closed after use to prevent resource leaks.
231-238: LGTM!
The method copyExerciseContentToRepository correctly updates to handle repository names as strings, enhancing flexibility for auxiliary repositories.
🧰 Tools
🪛 ast-grep
[warning] 232-236: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (
retrieveRepositoryDirectoryPath(basePath, repoName).toFile(),
repository.getLocalPath().toFile(),
new NotFileFilter(new NameFileFilter(".git"))
)
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 235-235: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (new NameFileFilter(".git"))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 232-236: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (
retrieveRepositoryDirectoryPath(basePath, repoName).toFile(),
repository.getLocalPath().toFile(),
new NotFileFilter(new NameFileFilter(".git"))
)
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 235-235: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (new NameFileFilter(".git"))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
274-284:
Fix incorrect method call getFirst() on List
The use of getFirst() on a List will result in a compilation error because List does not have this method.
Apply this diff to correct the method call:
- return result.getFirst();
+ return result.get(0);Likely invalid or redundant comment.
src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts
Show resolved
Hide resolved
src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts
Show resolved
Hide resolved
...ava/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java
Outdated
Show resolved
Hide resolved
...ava/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
Outdated
Show resolved
Hide resolved
...ava/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (1)
276-286: Enhance error handling and path filtering efficiency.The repository path retrieval could be more robust and efficient.
private Path retrieveRepositoryDirectoryPath(Path dirPath, String repoName) { - List<Path> result; - try (Stream<Path> walk = Files.walk(dirPath)) { - result = walk.filter(Files::isDirectory).filter(file -> file.getFileName().toString().endsWith("-" + repoName)).toList(); - } - catch (IOException e) { - throw new BadRequestAlertException("Could not read the directory", "programmingExercise", "couldnotreaddirectory"); - } - if (result.size() != 1) { - throw new IllegalArgumentException( - "There are either no or more than one sub-directories containing " + repoName + " in their name. Please make sure that there is exactly one."); - } - return result.getFirst(); + try (Stream<Path> walk = Files.walk(dirPath, 1)) { // Limit depth to immediate children + List<Path> matches = walk + .filter(Files::isDirectory) + .filter(file -> !file.equals(dirPath)) + .filter(file -> { + String fileName = file.getFileName().toString(); + return fileName.endsWith("-" + repoName); + }) + .toList(); + + if (matches.isEmpty()) { + throw new IllegalArgumentException( + String.format("No directory found for repository '%s'. Expected pattern: '*-%s'", repoName, repoName) + ); + } + if (matches.size() > 1) { + throw new IllegalArgumentException( + String.format("Multiple directories found for repository '%s': %s", repoName, + matches.stream().map(p -> p.getFileName().toString()).toList()) + ); + } + return matches.getFirst(); + } catch (IOException e) { + throw new BadRequestAlertException( + String.format("Failed to read directory while searching for repository '%s': %s", repoName, e.getMessage()), + "programmingExercise", + "directoryReadError" + ); + } }The improvements include:
- Limited directory traversal depth
- More specific error messages
- Explicit filtering of the root directory
- Better exception handling
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (1)
189-189: Consider batch loading auxiliary repositories for better performance.While the current implementation is correct, consider optimizing the auxiliary repository loading when exporting multiple exercises in batch. Instead of fetching repositories individually for each exercise, you could:
- Collect all exercise IDs
- Fetch auxiliary repositories in bulk
- Create a map of exerciseId -> repositories
- Look up from the map during export
This would reduce database round trips in batch scenarios.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (1)
Pattern 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/service/ProgrammingExerciseImportFromFileService.java (1)
Pattern 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
🪛 ast-grep
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
[warning] 234-238: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (
retrieveRepositoryDirectoryPath(basePath, repoName).toFile(),
repository.getLocalPath().toFile(),
new NotFileFilter(new NameFileFilter(".git"))
)
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 237-237: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (new NameFileFilter(".git"))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 234-238: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (
retrieveRepositoryDirectoryPath(basePath, repoName).toFile(),
repository.getLocalPath().toFile(),
new NotFileFilter(new NameFileFilter(".git"))
)
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 237-237: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (new NameFileFilter(".git"))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
🔇 Additional comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (2)
189-189: LGTM! The change correctly sets auxiliary repositories during export.
The addition ensures that auxiliary repositories are properly included when exporting programming exercises, which aligns with the PR objectives. The implementation correctly uses the repository to fetch auxiliary repositories by exercise ID.
189-189: Verify handling of auxiliary repositories during import.
Let's ensure that the import functionality correctly processes the auxiliary repositories that are now being exported.
✅ Verification successful
Auxiliary repositories are properly handled during import
The import functionality correctly processes auxiliary repositories that are being exported. The code shows comprehensive handling in both import services:
-
ProgrammingExerciseImportBasicServiceproperly clones and saves auxiliary repositories:- Creates new auxiliary repository objects
- Saves them to the database
- Associates them with the imported exercise
-
ProgrammingExerciseImportFromFileServicehandles the repository content:- Checks out each auxiliary repository
- Copies the imported content from the zip file
- Replaces exercise short names in files
- Commits and pushes changes
The implementation ensures end-to-end consistency between export and import of auxiliary repositories.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the import service handles auxiliary repositories.
# Test: Search for auxiliary repository handling in the import service.
# Expect: Code that processes auxiliary repositories during import.
rg -A 5 "auxiliaryRepositor" "src/main/java/**/ProgrammingExerciseImportFromFileService.java"
Length of output: 196
Script:
#!/bin/bash
# Let's first find the correct import service file
fd "ProgrammingExercise.*Import.*Service" src/
# Then search for auxiliary repository handling in all programming exercise import related files
rg -A 5 "auxiliaryRepositor" src/
Length of output: 331256
Script:
#!/bin/bash
# Let's search for import handling of auxiliary repositories
ast-grep --pattern 'class ProgrammingExerciseImportFromFileService {
$$$
importRepositories($$$) {
$$$
}
$$$
}'
# Also search for any auxiliary repository handling in import services
rg -A 5 "auxiliaryReposit.*import" src/
Length of output: 291
Script:
#!/bin/bash
# Let's try a different approach to find import handling
rg -A 10 "import.*exercise" "src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java"
# Also check the basic import service
rg -A 10 "import.*exercise" "src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportBasicService.java"
# And check for auxiliary repository handling in these files
rg "auxiliaryReposit" "src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImport*Service.java"
Length of output: 6702
Script:
#!/bin/bash
# Let's search for auxiliary repository handling in the import services with exact paths
rg -A 10 "auxiliaryReposit" "src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportBasicService.java"
# Also check the import from file service
rg -A 10 "auxiliaryReposit" "src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java"
# And check for repository import handling
rg -A 10 "importRepositories" "src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java"
Length of output: 8087
...ava/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
Show resolved
Hide resolved
...ava/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
Show resolved
Hide resolved
...ava/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
Show resolved
Hide resolved
|
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. |
SimonEntholzer
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
az108
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
|
github-actions
bot
added
the
deployment-error
HawKhiem
added
deploy:artemis-test4
and removed
deployment-error
HawKhiem
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
krusche
deleted the
feature/auxiliary-repositories/allow-import-and-export
branch
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Instructors can export and import exercises. We want exported programming exercises to contain the auxiliary repositories, and we want to be able to then import programming exercises with auxiliary repositories.
Fixes #9523
Description
Add the capability to export and import programming exercises with the auxiliary repositories.
Steps for Testing
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Client
Server
(last 25% of ProgrammingExerciseExportService.java are unrelated to this PR, see here. I will add tests for this in a follow up)
Screenshots
Downloading an exercise:
Select exported exercise:
Import after renaming:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation