Iteration 2 2 1

[JuergGood]

UX / UI Change PR

Scope

• Dark mode
• Mobile layout
• Tables/logs/admin views
• Tasks/list layouts
• Other:

Acceptance criteria (must be explicit)

What changed (systemic first)

• Tokens/theme:
• Shared components/styles:
• Component-specific changes (only if necessary):

Screens / viewports verified

• Mobile 375×667 light
• Mobile 375×667 dark
• Desktop 1440×900 light
• Desktop 1440×900 dark

UX Regression Checklist (must all be ✅)

Theme & contrast

• Action icons visible in dark mode (incl. mat-icon-button + destructive actions)
• Menus/dialogs/selects have readable contrast in dark mode
• Focus ring visible (keyboard navigation)

Mobile space & layout

• Header density acceptable (content above the fold on 375×667)
• No unintended horizontal scrolling on mobile

Responsive patterns

• Tables on mobile: cards or usable scroll (no clipped headers)
• Toolbars/filters don’t create orphan controls / accidental wraps
• Titles don’t break mid-word; truncation/wrapping intentional

Maintainability

• No new hardcoded hex colors (unless added to tokens)
• No new !important (or justified below)

Justifications (required if any)

• New !important added because:
• Component-specific workaround added because:

To fix the problem, sanitize the user-controlled path value before including it in log messages. The main goal is to prevent newlines and other control characters from breaking the log format or creating the impression of multiple entries, while still preserving enough information for debugging.

The best low-impact fix here is to create a sanitized version of path that strips or replaces newline (\n) and carriage-return (\r) characters (and optionally trims the result), and then log that sanitized value instead of the raw input. We can do this locally in the method without introducing new dependencies or changing behavior elsewhere.

Concretely, in DocController.getRawDoc at the path-traversal check, change:

if (!requestedPath.startsWith(basePath)) {
    log.warn("Potential path traversal attempt: {}", path);
    return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
}

to compute a safe sanitizedPath and use that in the log:

if (!requestedPath.startsWith(basePath)) {
    String sanitizedPath = path.replace('\n', '_')
                               .replace('\r', '_');
    log.warn("Potential path traversal attempt: {}", sanitizedPath);
    return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
}

This keeps the logging behavior (including the user-provided content) but prevents an attacker from injecting line breaks into the log. No new imports or methods are strictly required; the sanitization can be done inline where the log call occurs.

In general terms, to fix log injection you must ensure that any user-provided data is sanitized before being written to logs. For plain-text logs, that usually means removing or replacing newline and carriage-return characters and potentially other control characters, then logging the sanitized value instead of the raw input.

In this specific case, the fix is to avoid logging the raw path parameter and instead log a sanitized variant that cannot break log lines. The simplest, non-functional-change option is to create a sanitized version of path just for logging. For example, we can define a local variable such as String safePathForLog = path.replace('\n', '_').replace('\r', '_'); and then use safePathForLog in the log statements. This keeps all program behavior (file resolution, permission checks, responses) identical, while ensuring that the log output cannot contain raw line breaks from user input. To avoid duplicating logic, we can define this once near the top of getRawDoc (after the method signature) and reuse it in all log calls that include path (lines 97, 127, and 131). No new imports are needed, as String.replace is part of the standard library and String is already in scope.

Concretely:

• In getRawDoc, add a local variable right at the start of the try block (or just after the method begins) computing a log-safe version of the request parameter: String safePathForLog = path.replace('\n', '_').replace('\r', '_');.
• Change the log statements that currently interpolate path (log.warn on line 97, log.warn on line 127, and log.error on line 131) to use safePathForLog instead.
• The rest of the logic that relies on the original path should remain unchanged.

In general, to fix log injection issues, ensure that any user-controlled string is sanitized before being written to logs. For plain-text logs, this typically means removing or replacing newline (\n), carriage return (\r), and other control characters so that one log call cannot visually become multiple entries or alter log formatting.

For this specific code, the minimal, non-functional change is to sanitize the path parameter only for logging, leaving the original path value untouched for functional use. We can create a local sanitized variable that strips \r and \n characters from path right before the log.info call, and use that sanitized variable in the log message. This keeps the business logic (file resolution, etc.) unchanged and only affects what is written into logs.

Concretely:

• In DocController.getRawDoc, locate the log.info("Resolved partial task path {} to {}", path, requestedPath); line (around line 114).
• Immediately before this log line, introduce a new local variable, e.g. String safePathForLog = path.replace('\n', '_').replace('\r', '_');.
• Update the log.info call to use safePathForLog instead of the raw path.
• No new imports are needed, as String.replace is part of the standard library.

To fix this, user-controlled data should be sanitized or constrained before being written to the log. For log injection, the primary concern is stripping or neutralizing newline (\n, \r) and similar control characters so that a single log entry cannot be split into multiple lines. A typical pattern is to define a small helper method that takes a String (or Object) and returns a sanitized version with problematic characters removed or replaced.

The best minimal-impact fix here is:

• Introduce a private helper method in DocController, for example sanitizeForLogging(String input), that removes carriage returns and newlines (and optionally other non-printable control characters).
• Use that helper when logging both the original path and any derived values (like requestedPath) that originate from user input. In the specific flagged line, change requestedPath to sanitizeForLogging(requestedPath.toString()). Similarly, for the other logging lines that directly log path, wrap path with the same sanitizer.
• This keeps the existing functionality (log messages and control flow) intact while ensuring logged values cannot introduce injected newlines.

Concretely in backend/src/main/java/ch/goodone/backend/controller/DocController.java:

• Add the helper method near the top of the class (e.g. after the baseDir field).
• Update:
  • line 65: "Potential path traversal attempt: {}", path → use sanitizeForLogging(path)
  • line 114: "Resolved partial task path {} to {}", path, requestedPath → use sanitizeForLogging(path) and sanitizeForLogging(requestedPath.toString())
  • line 127: "File not found or not readable: {}", path → use sanitizeForLogging(path)
  • line 131: "Error reading doc file: {}", path, e → use sanitizeForLogging(path)

No new external dependencies are needed; the sanitizer can be implemented with basic String operations.

In general, to fix log injection, any user-controlled values logged should be sanitized before logging: at minimum, remove or neutralize newline and carriage-return characters and optionally other non-printable control characters. This avoids a situation where user input can break the log entry format and appear as separate entries or inject misleading content.

The best targeted fix here is to sanitize the path value once when it is received and then use the sanitized version both for logging and (optionally) for further processing if we want consistency. Since we must not change behavior beyond security, the safest minimal change is to introduce a small helper method in DocController that strips \r and \n (and optionally other control characters) from strings before they are logged, then wrap uses of path in the log statements with this helper, while leaving all other logic unchanged. No new external dependencies are needed; standard Java is sufficient.

Concretely, in backend/src/main/java/ch/goodone/backend/controller/DocController.java:

• Add a private static helper method, e.g. sanitizeForLog(String input), that:
  • Returns null if input is null.
  • Replaces \r and \n with a safe placeholder (e.g. a space) or removes them.
  • Optionally removes any other ISO control characters using StringBuilder and Character.isISOControl.
• Update log.warn("File not found or not readable: {}", path); to log sanitizeForLog(path) instead.
• Update log.error("Error reading doc file: {}", path, e); similarly.

This keeps the functional meaning of the logs (the path value) while preventing multi-line or control-character injection.

In general, to fix log injection, any user-controlled value that is written to logs should be sanitized or validated beforehand. Commonly, this means either (a) validating the value against an allowlist of acceptable characters (for example, only alphanumerics, some punctuation, and path separators) or (b) normalizing/removing dangerous characters such as line breaks and other control characters before logging. The main goal is to prevent an attacker from injecting additional log lines or manipulating the structure/semantics of log entries.

For this specific case, the best minimal fix that does not change existing behavior is to sanitize path only for logging, while leaving the original path untouched for application logic. We can do this by creating a sanitized copy (e.g., String safePathForLog = sanitizeForLogging(path);) right before logging and passing that to log.warn and log.error. The sanitization should at least strip carriage returns and newlines, and ideally other control characters, while keeping regular visible characters unchanged. Since we must keep the change localized to this file and code, we can implement a small private helper method sanitizeForLogging(String value) inside DocController that removes \r, \n, and other ISO control characters from the string, and use it in the two log statements that currently log raw path. No new external dependencies are needed; we can implement this with core Java (StringBuilder, Character.isISOControl). Concretely, in getRawDoc, update line 127 and 131 to log sanitizeForLogging(path) instead of path, and add the helper method inside DocController (e.g., near the bottom of the class).

[github-actions]

Qodana Community for JVM

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked

View the detailed Qodana report

To be able to view the detailed Qodana report, you can either:

• Register at Qodana Cloud and configure the action
• Use GitHub Code Scanning with Qodana
• Host Qodana report at GitHub Pages
• Inspect and use qodana.sarif.json (see the Qodana SARIF format for details)

To get *.log files or any other Qodana artifacts, run the action with upload-result option set to true,
so that the action will upload the files as the job artifacts:

      - name: 'Qodana Scan'
        uses: JetBrains/qodana-action@v2024.3.4
        with:
          upload-result: true

Contact Qodana team

Contact us at qodana-support@jetbrains.com

• Or via our issue tracker: https://jb.gg/qodana-issue
• Or share your feedback: https://jb.gg/qodana-discussions

To fix the problem, the unused Router import should be removed so that only the actually used symbol(s) are imported from @angular/router. This keeps the test file cleaner and avoids confusion about dependencies.

Concretely, in frontend/src/app/components/help/help-content.component.spec.ts, adjust the import on line 3 to remove Router, leaving only ActivatedRoute. No other code changes are required because Router is not referenced anywhere else in the snippet. No additional methods, imports, or definitions are needed.

In general, unused imports should be removed from the file to avoid confusion and keep the code clean. When multiple symbols are imported from the same module, remove only those that are not referenced anywhere in the file, keeping the ones that are used.

In this case, the best fix is to adjust the rxjs import on line 7 in frontend/src/app/components/help/help-content.component.spec.ts so that it only imports Subject, which is used to create paramsSubject and fragmentSubject. The symbol of should be removed from the curly-brace import list. No additional methods, imports, or definitions are required.

To fix the problem, sanitize the user-controlled string before logging it. For plain-text logs, the minimum is to remove or normalize line breaks and other control characters so a user cannot inject extra log entries or manipulate log structure. We can do this locally, just before logging, without changing any functional behavior.

The best targeted fix here is:

• Introduce a small private helper method in DocController that takes a String and returns a sanitized version with \r and \n removed (and optionally other control characters).
• Use this helper when logging searchPath and path so that any CR/LF characters supplied by the user cannot create forged log lines.
• Keep the original values for all non-logging logic (path traversal checks, file resolution, etc.) so behavior of the endpoint is unchanged except for log safety.

Concretely, in backend/src/main/java/ch/goodone/backend/controller/DocController.java:

1. Add a private method, e.g. sanitizeForLog, near the top of the class (just after the fields), which:
   • Returns null if the input is null.
   • Replaces \r and \n with spaces (or removes them).
2. Update line 89 to log sanitizeForLog(searchPath) instead of searchPath.
3. Optionally (but recommended for consistency and defense-in-depth), also update lines 65 and 97 to log sanitizeForLog(path).

No new imports are needed; String.replace is in java.lang.

---

In general, to avoid exposing internal mutable state, getters for mutable fields should not return the underlying collection directly. Instead, they should either return an immutable/read-only view, or a defensive copy. Setters/constructors should likewise defensively copy incoming mutable arguments. For RiskItem.evidencePaths, we only need to adjust the getter so external callers cannot directly mutate the internal map.

The best minimal fix here is to make getEvidencePaths return a new HashMap containing the same entries as the internal evidencePaths field. This preserves current behavior for read access (callers still see the same key-value pairs), while ensuring that any modifications to the returned map do not affect the internal representation. We should keep the lazy-initialization behavior so existing code that expects a non-null map continues to work. No changes are needed to the field declaration or other methods, and no extra imports are required because the code already uses fully-qualified java.util classes.

Concretely, in backend/src/main/java/ch/goodone/backend/ai/dto/RiskRadarResponse.java, within the RiskItem class, update the getEvidencePaths method (lines 66–71) to:

• Ensure evidencePaths is initialized as before if it is null.
• Return new java.util.HashMap<>(evidencePaths) instead of returning evidencePaths directly.