Sidebar adjustments

[JinethBosilu]

changed sidebars

In general, user-provided values should be sanitized or validated before being written to logs. For plain-text logs, at minimum this means stripping or replacing newline and other control characters that could break log structure or inject extra entries, or validating that the string matches an expected safe pattern and logging a sanitized placeholder if it does not.

For this specific case, the best fix with minimal behavior change is to sanitize dateOfBirth before logging it. The project already has com.crimeLink.analyzer.util.LogSanitizer used in CriminalController for name and nic. We should apply the same sanitization here by wrapping dateOfBirth with LogSanitizer.sanitize(...) at the log call site. This keeps the functional behavior (logging the problematic value) while ensuring any dangerous characters are neutralized in the logs.

Concretely:

• In src/main/java/com/crimeLink/analyzer/service/CriminalService.java, import LogSanitizer.
• Change the warning log in the catch block around LocalDate.parse(dateOfBirth) to log LogSanitizer.sanitize(dateOfBirth) instead of the raw dateOfBirth.

No other files or logic need to be modified.

In general, to fix log injection, ensure that any user-provided values included in log messages are sanitized to remove or neutralize characters that can break log structure (like \r, \n, and other control characters) or otherwise confuse log readers. In this codebase, a dedicated LogSanitizer utility already exists and is used in the controller when logging name and nic, so the best fix is to consistently reuse that utility wherever user input is logged.

Concretely, in CriminalService.createCriminal, line 73 logs the name parameter directly. We should import LogSanitizer in this service (it’s currently only imported in the controller) and apply it to name in the log statement. This preserves the existing business logic and persistence behavior while only changing how data is rendered in logs. The change is limited to:

• Adding import com.crimeLink.analyzer.util.LogSanitizer; at the top of CriminalService.java.
• Updating the logging call on line 73 to use LogSanitizer.sanitize(name) instead of name.

No new methods or external dependencies are needed; we simply reuse the existing sanitizer utility.

---

In general, to fix log injection issues, any user-provided value must be sanitized or validated before being written to logs. Typical mitigations are: stripping/normalizing newline and other control characters, whitelisting characters via a regex, or using a centralized sanitizer utility that performs this normalization and clearly marks user input.

For this specific case, the best fix with minimal functional change is to sanitize dateOfBirth before it is included in the log message. The project already has a LogSanitizer utility used in CriminalController for criminalId, so we should reuse that rather than creating ad‑hoc sanitization logic. Concretely, in CriminalService.updateCriminal, we will change the log.warn call inside the catch block to pass dateOfBirth through LogSanitizer.sanitize(...). To do this safely and consistently, we need to import com.crimeLink.analyzer.util.LogSanitizer at the top of CriminalService.java and adjust only the affected logging call:

• Add an import for LogSanitizer in CriminalService.java.
• Replace log.warn("Invalid date_of_birth format: {}", dateOfBirth); with log.warn("Invalid date_of_birth format: {}", LogSanitizer.sanitize(dateOfBirth));.

No method signatures or behavior of the application change other than the fact that logged dateOfBirth values are now sanitized. Persistence logic and parsing behavior remain identical.

---

In general, user-controlled data should be sanitized before being written to logs, especially when it can contain newline or control characters. This project already has com.crimeLink.analyzer.util.LogSanitizer used in CriminalController, so the best and least intrusive fix is to reuse that utility in CriminalService wherever criminalId (or any other user input) is logged. Core functionality (database lookups, storage operations) can keep using the raw criminalId; only the values passed to log.* calls need to be sanitized.

Concretely, in CriminalService.updateCriminal, we should import LogSanitizer at the top of the file, and wrap criminalId with LogSanitizer.sanitize(...) in the three log statements that currently interpolate the raw ID:

• log.info("Photo updated for criminal {}", criminalId);
• log.info("Embedding regenerated for criminal {}", criminalId);
• log.info("Criminal updated: {}", criminalId);

No behavior changes occur except for how the ID appears in logs; application logic remains identical. Only src/main/java/com/crimeLink/analyzer/service/CriminalService.java needs edits; CriminalController.java already uses LogSanitizer correctly.

In general, to fix log injection issues, ensure that any user-provided string passed into log messages is sanitized or validated before logging. In this codebase, there is already a LogSanitizer utility in com.crimeLink.analyzer.util used in CriminalController to sanitize criminalId for logging, so the most consistent and low-impact fix is to apply the same sanitizer in CriminalService wherever criminalId (or other tainted input) is logged.

Concretely for this issue, in CriminalService.updateCriminal we should avoid logging the raw criminalId. We can import com.crimeLink.analyzer.util.LogSanitizer and then replace each occurrence of criminalId in log statements with LogSanitizer.sanitize(criminalId). This should be done for:

• log.error("Photo upload failed for criminal {}: {}", criminalId, e.getMessage()); (the one flagged),
• as well as the related log lines in this method: log.info("Photo updated for criminal {}", criminalId);, log.info("Embedding regenerated for criminal {}", criminalId);, log.warn("Embedding regeneration failed for criminal {} (non-fatal): {}", criminalId, e.getMessage());, and log.info("Criminal updated: {}", criminalId); to keep behavior consistent and safe.

To implement this, we need to:

• Add an import for com.crimeLink.analyzer.util.LogSanitizer; at the top of CriminalService.java (without altering existing imports).
• Update the five logging calls in updateCriminal to call LogSanitizer.sanitize(criminalId) instead of passing criminalId directly. No functional behavior beyond log content changes; all database and service logic remains the same.

---

To generally fix log injection issues, any user-controlled data should be sanitized or validated before being written to logs. The simplest and least invasive approach here is to reuse the existing LogSanitizer utility already used in CriminalController, rather than reinventing sanitization logic. This keeps behavior consistent across the codebase and avoids changing functional logic—only the log messages are modified to use sanitized variants of criminalId.

Concretely, in SupabaseStorageService.deleteFolder, we should:

• Import com.crimeLink.analyzer.util.LogSanitizer.
• Compute a sanitized version of criminalId, e.g. String safeCriminalId = LogSanitizer.sanitize(criminalId); at the start of the method.
• Use safeCriminalId in all logging calls that currently interpolate the raw criminalId (lines 107, 115, 147, 149, 153).
• Keep using the raw criminalId for functional behavior (building prefixes, request bodies, etc.) so existing logic is unchanged.

All changes are confined to src/main/java/com/crimeLink/analyzer/service/SupabaseStorageService.java in the deleteFolder method and the imports section.

---

In general, to fix log injection issues you should sanitize or validate any user-controlled value before logging it. Common mitigations include stripping or replacing newline and carriage-return characters, optionally restricting to a safe character set, and centralizing this logic in a reusable utility method so it is consistently applied.

In this codebase, there is already a LogSanitizer utility used in CriminalController (e.g., LogSanitizer.sanitize(criminalId)), so the best fix is to apply the same sanitization in the downstream services where criminalId is logged. Specifically:

• In CriminalService.deleteCriminal, sanitize criminalId before including it in log messages (both the warning in the catch block and the final info log).
• In SupabaseStorageService.deleteFolder, sanitize criminalId in all log statements (log.warn and log.info) so that any malicious characters in the path variable cannot alter the log structure.

We will:

• Add an import for com.crimeLink.analyzer.util.LogSanitizer to CriminalService and SupabaseStorageService.
• Wrap criminalId with LogSanitizer.sanitize(...) in all relevant log calls within the shown snippets:
  • CriminalService: lines 197 and 202.
  • SupabaseStorageService: lines 107, 115, 147, 149, and 153.

No behavioral change occurs other than making the logged representation of criminalId safe; the value used for business logic (DB calls, HTTP requests, etc.) remains unchanged.

In general, to fix this kind of issue, any user-controlled data that is written to logs should be sanitized first, typically by stripping or escaping newline and other control characters and clearly delimiting user input. This ensures an attacker cannot forge extra log lines or otherwise confuse log parsers.

The best fix here, consistent with the rest of the codebase, is to apply the existing LogSanitizer.sanitize(...) helper to the criminalId before logging it in SupabaseStorageService.deleteFolder. This keeps behavior aligned with other logging in CriminalController and CriminalService, without changing any functional logic or external behavior beyond the log content. Specifically:

• Import com.crimeLink.analyzer.util.LogSanitizer into SupabaseStorageService.
• Update the log statements in deleteFolder that currently log criminalId directly (lines 107, 115, 147, 149, 153) to instead log LogSanitizer.sanitize(criminalId).

No new methods or complex changes are needed; we just ensure that the same sanitization used at the controller level is applied where the tainted value is actually logged inside this service.

---

In general, any user-controlled value written to logs should be sanitized or strictly validated before logging. In this codebase, the established pattern is to use LogSanitizer.sanitize(...) wherever request parameters or path variables are logged. We should apply the same pattern to SupabaseStorageService.deleteFolder (and ideally to other service logs using criminalId), ensuring the logged value cannot contain line breaks or other problematic characters, while still passing the raw value to business logic (e.g., Supabase API and repository calls) so functionality is unchanged.

The best targeted fix is:

• Import com.crimeLink.analyzer.util.LogSanitizer into SupabaseStorageService.
• In deleteFolder, compute a local String safeCriminalId = LogSanitizer.sanitize(criminalId);.
• Use safeCriminalId in all log statements instead of criminalId.
• Optionally, mirror this in CriminalService.deleteCriminal where criminalId is also logged unsanitized (this is on the same taint path and within the provided snippet).

These changes:

• Affect only logging, not the values sent to Supabase or the database.
• Reuse an existing utility, so no new dependencies are required.
• Maintain current behavior aside from making logs robust against injection.

In general, to fix log injection, any user-controlled or otherwise untrusted data should be sanitized (e.g., stripping newlines and control characters) before being written to logs. This can be done either inline at each log call or via a reusable helper/utility.

In this codebase, the CriminalController already uses LogSanitizer.sanitize(criminalId) when logging criminalId. The best fix that preserves existing functionality is to apply the same sanitization in the service layer where criminalId is logged: in CriminalService and SupabaseStorageService. Specifically:

• Import com.crimeLink.analyzer.util.LogSanitizer into CriminalService and SupabaseStorageService.
• Wrap every occurrence of criminalId (and similar user-controlled values) passed to log.* calls in these classes with LogSanitizer.sanitize(...).
• Do not change the behavior of non-logging code (e.g., values used for repository lookups or Supabase requests).

Concretely:

• In CriminalService.deleteCriminal, change:
  • log.warn("Storage cleanup failed for criminal {} (non-fatal): {}", criminalId, e.getMessage());
  • log.info("Criminal deleted: {}", criminalId);
    to use LogSanitizer.sanitize(criminalId).
• In SupabaseStorageService.uploadPhoto, change:
  • log.error("Failed to upload photo for criminal {}: {}", criminalId, e.getMessage());
• In SupabaseStorageService.deleteFolder, change all log calls that include criminalId:
  • log.warn("Failed to list storage files for criminal {}", criminalId);
  • log.info("No storage files found for criminal {}", criminalId);
  • log.info("Storage files deleted for criminal {}: {} file(s)", criminalId, prefixes.size());
  • log.warn("Storage deletion returned status {} for criminal {}", deleteResponse.getStatusCode(), criminalId);
  • log.warn("Storage cleanup failed for criminal {}: {}", criminalId, e.getMessage());
    to use LogSanitizer.sanitize(criminalId).

This requires only adding the import and wrapping arguments, without modifying method signatures or business logic.

---

[Copilot]

Pull request overview

This PR adds backend support for criminal record CRUD operations, integrates Supabase Storage for photo uploads, and extends the facial-recognition ML integration to handle additional criminal profile fields and embedding generation.

Changes:

• Add Supabase Storage configuration and a service to upload/delete criminal photos.
• Introduce direct DB-backed criminal CRUD service/controller with optional photo upload + embedding generation.
• Expand ML registration payload fields and add an ML embedding-generation call; update security rules for new/updated endpoints.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/main/resources/application.properties	Adds Supabase Storage configuration properties.
src/main/java/com/crimeLink/analyzer/service/SupabaseStorageService.java	New service to upload photos to Supabase and attempt folder cleanup.
src/main/java/com/crimeLink/analyzer/service/FacialRecognitionService.java	Extends registration payload and adds generateEmbedding() ML call.
src/main/java/com/crimeLink/analyzer/service/CriminalService.java	New DB CRUD service coordinating photo storage + embedding generation.
src/main/java/com/crimeLink/analyzer/repository/CriminalRepository.java	Adds query to fetch IDs with non-null face embeddings.
src/main/java/com/crimeLink/analyzer/entity/Criminal.java	Adds fields for risk level, history, photo URL, DOB, gender, alias.
src/main/java/com/crimeLink/analyzer/controller/FacialRecognitionController.java	Extends request parameters forwarded to the ML register endpoint.
src/main/java/com/crimeLink/analyzer/controller/CriminalController.java	New REST controller exposing /api/criminals CRUD endpoints.
src/main/java/com/crimeLink/analyzer/config/SecurityConfig.java	Adjusts role access rules for facial endpoints and adds rules for /api/criminals.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

createCriminal() returns the identifier under criminal_id, but the list/detail mappings use id. This makes the /api/criminals API inconsistent for clients. Consider standardizing the key name across all responses (e.g., always id or always criminal_id).

[Copilot]

/api/criminals/** already matches /api/criminals, so the additional matcher for /api/criminals is redundant and never reached. Removing the duplicate reduces confusion about which rule applies.

Suggested change

.requestMatchers("/api/criminals").hasAnyRole("Investigator", "OIC")

[Copilot]

SupabaseStorageService instantiates a new RestTemplate directly, which bypasses the configured RestTemplate bean (timeouts, shared config) used elsewhere (see RestTemplateConfig). Inject RestTemplate via constructor/@requiredargsconstructor instead of new RestTemplate() so calls don’t hang indefinitely and configuration stays consistent.

[Copilot]

The bulk-delete call doesn’t match the stated endpoint: the comment says "POST /storage/v1/object/remove" but the code sends an HTTP DELETE to /storage/v1/object/{bucket} with a JSON body containing prefixes. Supabase Storage’s bulk remove API is a POST to the /object/remove endpoint (and typically expects a list of paths), while DELETE is for single-object deletion. As written, this is likely to 404/405 and leave files undeleted; align the URL/method/payload with the bulk remove endpoint.

[Copilot]

listBody is built via string concatenation with criminalId, which can break JSON encoding if the id ever contains quotes/backslashes and makes the request harder to maintain. Build the JSON payload via ObjectMapper/Map so values are properly escaped, and consider pagination/looping since limit is fixed to 100 (folders with >100 objects won’t be fully deleted).

[Copilot]

deleteBody is constructed using double-brace initialization (new LinkedHashMap<>() {{ ... }}), which creates an extra anonymous class and can retain references unexpectedly. Prefer creating a normal Map and putting values explicitly before serializing.

Suggested change

	String deleteBody = mapper.writeValueAsString(new java.util.LinkedHashMap<String, Object>() {{
	put("prefixes", prefixes);
	}});
	java.util.Map<String, Object> deleteBodyMap = new java.util.LinkedHashMap<>();
	deleteBodyMap.put("prefixes", prefixes);
	String deleteBody = mapper.writeValueAsString(deleteBodyMap);