Refactor to reduce test code duplication

[PRiewe]

No description provided.

At a high level, we want to ensure that any filesystem paths derived from untrusted or semi‑trusted data (jar entry names and logical path segments) cannot be used to access files outside an intended base directory. The standard fix is to resolve the candidate path against the base directory, normalize/canonicalize it, and verify that it is still under that base directory before using it in any file operation.

Concretely, in this codebase the risky places are:

1. addArchive in FileSystem:

   • It stores entry.getName() (jar entry path) as the value in the PathTree. If an entry name contains .. or absolute elements, files.get(path) later returns that unsanitized value and it is passed to FileInputStream as a filesystem path. We should validate the entry name when mounting the archive and either reject or normalize dangerous entries.

2. getFile in FileSystem:

   • Uses temp.getPath() + toString(path) to create a File in the temp directory, without verifying that the resulting path is inside temp.
   • Uses new FileInputStream(files.get(path)) on a string that may be derived (indirectly) from jar entries.

3. saveToTemp in FileSystem:

   • Also uses temp.getPath() + toString(path) to create files for writing, with the same lack of boundary checks.

The safest, minimal‑behavior‑change fix is:

• Introduce a small helper in FileSystem to safely resolve a relative path (string constructed by toString(path) or stored in files) under a base directory, using Path.of(base, relative).normalize() and checking that the resulting path starts with the base directory path. If not, throw an IOException or log and abort.

• Use this helper:

  • In getFile, before using temp paths: instead of new File(temp.getPath() + toString(path)), compute a Path under temp with normalization and bounds check, then use that Path to open the file.
  • In saveToTemp, do the same when creating/writing files in temp.
  • In getFile’s non‑jar branch (new FileInputStream(files.get(path))), if files.get(path) is intended to be relative to some root, we should at least normalize it and optionally restrict it to not be absolute and not contain ... Since we’re not shown the full semantics of paths and directory roots, we’ll implement a conservative check: disallow absolute paths and .. segments in files.get(path) before using it.

• In addArchive, prevent storing clearly dangerous entry names. The code only uses the entry name as the value in the tree (not to open files inside the jar except via jar.getEntry(value), which is safe inside the jar), but that same value is later used to open an external file in the else‑branch of getFile. To avoid poisoning the mapping with dangerous values, we can:

  • Skip jar entries whose entry.getName() is absolute or contains .. path elements, and log a warning.
  • This doesn’t affect normal mods but prevents crafted jars from introducing unsafe paths.

Implementation details and locations:

• File: src/main/java/neon/systems/files/FileSystem.java

  1. Add an import for java.nio.file.Paths or just use Path.of; we already import java.nio.file.Path, so we can use Path.of(...) without new imports.

  2. Add a private helper method, e.g. private Path resolveUnderBase(File baseDir, String relativePath) throws IOException, which:

     • Builds a Path base = baseDir.toPath().toAbsolutePath().normalize();
     • Builds Path candidate = base.resolve(relativePath).normalize();
     • Checks if (!candidate.startsWith(base)) throw new IOException("Path traversal attempt: " + relativePath);
     • Returns candidate.

  3. Update getFile:

     • Replace new File(temp.getPath() + toString(path)) with logic that uses resolveUnderBase(temp, toString(path)).
     • Replace new FileInputStream(temp.getPath() + toString(path)) accordingly.
     • Before new FileInputStream(files.get(path)), validate that files.get(path) is not null, not absolute (!new File(name).isAbsolute()), and does not contain ".." as a segment (simple contains("..") or better, Path.of(name).normalize() and ensure it has no ..). Since we must avoid changing semantics too much and not know the root, we’ll implement a simple guard: if the normalized Path is absolute or contains .. segments (detected by comparing normalize() to Path.of(name) and/or scanning elements), we throw an IOException.

     To keep changes small, we can restrict ourselves to rejecting any files.get(path) that is absolute or whose normalized path starts with ".." or contains "/../" or File.separator + ".." + File.separator. That’s enough to avoid obvious path traversal into the host filesystem.

  4. Update saveToTemp:

     • Use resolveUnderBase(temp, toString(path)) to obtain a Path and then a File for writing (filePath.toFile()), instead of concatenating strings.

  5. Update addArchive:

     • After String name = entry.getName(); (or before adding to files), add a simple validation to skip dangerous names:
       • If name.startsWith("/") or name.contains(".."), log a warning and continue;.
       • This prevents storing entries with traversal patterns.

These changes keep functionality for legitimate paths but prevent traversal both when writing to temp and when using potentially poisoned values from files. They also address all CodeQL variants because they sanitize the data at the point where untrusted archive entry names flow into filesystem sinks.

---