WIP: full e2e tests

[trevor-e]

No description provided.

To fix this issue, the code should ensure that the file being accessed is strictly within the ARTIFACTS_DIR directory. The best-practice method is to:

1. Join the user-provided artifact_id (plus extension) with the base directory;
2. Normalize the resulting path using os.path.normpath or (for Path) .resolve();
3. Verify that the resulting path is still within (a child of) the base directory;
4. Only proceed to access the file if these checks pass.

In this snippet, in download_artifact, the lines constructing and checking artifact_path starting at line 71 must be changed. The check should:

• Use Path.resolve() to get the normalized absolute path for the requested file,
• Make sure the resolved path is within ARTIFACTS_DIR (using .relative_to() or path prefix checking).
  If the check fails, raise a 400 or 403 error.

No new custom helper methods are strictly required, but the code will need a proper check sequence as described above. The import for os already exists, and Path is already imported, so no new imports are required.

---

To properly fix this vulnerability, we must ensure that the resulting artifact_path—after joining the user input—remains within the intended artifacts root directory (ARTIFACTS_DIR). This is best done by:

• Composing the final path (with any extensions/postfixes) as before.
• Normalizing the resulting path with .resolve() (or os.path.normpath/os.path.realpath) to remove any traversal elements.
• Verifying that the normalized path is a descendant (or the same as) the allowed directory (ARTIFACTS_DIR). This can be checked by using Path.is_relative_to (Python 3.9+) or by comparing path components if using an older version.
• If the check fails, raise a 400 or 403 error.

All changes must be made only in the shown snippet, affecting the download_artifact function (lines 62–95 or so), specifically the calculation and security checking of artifact_path.

---

The best general fix is to ensure the constructed artifact_path cannot be manipulated to escape the intended ARTIFACTS_DIR. This is well-achieved by normalizing the resulting path (with .resolve()) and then checking that the resolved path is indeed inside the allowed directory. Specifically:

• After constructing artifact_path, call .resolve() on it.
• Check that the resolved path is a subpath (or the same path) as ARTIFACTS_DIR.resolve() (e.g., using is_relative_to in Python 3.9+, or a string startswith in older versions).
• Only proceed to open the file if this check passes; otherwise, return a 403 error.
• These changes should be applied to the download endpoint in server.py, lines 71–75 (right after constructing artifact_path).

Requirements:

• From Python 3.9+, Path.is_relative_to can be used (we'll assume Python 3.9+ since FastAPI is reasonably modern).
• No extra dependencies needed; use the standard library.

To fix this vulnerability, the server should ensure that any file access using a filename derived from user input (here, artifact_id) does not allow traversal outside of the intended directory (ARTIFACTS_DIR). The simplest and most robust approach is to:

1. Join and normalize the path (using os.path.normpath or Path.resolve) after combining the base directory and the constructed filename.
2. Check that the resolved path is within the intended directory (i.e., that the resolved path starts with or is a descendant of ARTIFACTS_DIR). If not, return a 400 or 403 error.
3. Perform existing file system operations only after this check passes.

This requires modifying the code at/near places where artifact_path is constructed and before it is used, in the download_artifact endpoint. No new methods are strictly needed—existing code can use the standard library pathlib and/or os.

---

General fix:
To prevent directory traversal or unintended file access, before accessing any file path that includes user input (like artifact_id), you must validate and constrain the resolved path to ensure it does not escape a designated root directory.

Best fix for this scenario:
Normalize and join the path using os.path.normpath (or PurePath if using pathlib), and then verify that the resulting path is within ARTIFACTS_DIR. If it is outside, abort with an error. You may also wish to further restrict artifact_id (e.g., only allow alphanumerics and -/_) using a regex, but the normalization+containment check is most important.

Implementation in code:

• After composing artifact_path = ARTIFACTS_DIR / f"{artifact_id}.zip", resolve artifact_path to its absolute/canonical form (using .resolve()).
• Check that the resolved path is a child of (or the same as) ARTIFACTS_DIR.resolve().
• If not, raise a 400 error or similar.

Required imports/methods:

• No new external dependencies are needed, Pathlib already supports .resolve() and path inclusion checks.

---

To fix the issue while preserving current behavior, the code should ensure user-provided artifact_id cannot escape the intended artifact storage directory (ARTIFACTS_DIR). The best general-purpose fix is to validate and sanitize the resulting file path. This can be done by joining and normalizing the path, then ensuring it resides within the intended directory via a prefix check, as recommended in the background section.

Specifically, before opening the file at artifact_path, do the following:

• Use os.path.normpath() or Path.resolve() to get the canonical final path.
• Ensure this normalized path is still within ARTIFACTS_DIR.
• If the check fails, return an error (HTTP 400 or 403).

This change is needed within the test_upload_artifact function, lines 295-299. No changes to imports are required since pathlib.Path is already imported.

---

The best way to fix this problem is to validate, before using, that the constructed file path is both safe and strictly contained within the intended parent directory (RESULTS_DIR). This can be accomplished by first joining and normalizing the constructed path with os.path.normpath (or using resolve() from pathlib), and then checking that the resulting path is still within RESULTS_DIR. If not, return an error. This solution avoids directory traversal as well as absolute-path targeting.

Concretely, in test_get_results, before any use of artifact_id to assemble a file path (in both lines 310 and 311), normalize the path and check containment:

• Build the full path using RESULTS_DIR / f"{artifact_id}_size_analysis.json" (and similarly for artifact_id_app).
• Call .resolve() on the result.
• Check if the resolved path is a child (descendant) of RESULTS_DIR.resolve(). If not, return an HTTP 400 or 403 error or set the flag as False (as appropriate).

Add an import for os or use Pathlib's facilities as the rest of the file does.

---

The best way to fix this issue is to ensure that any filename or file path constructed using untrusted input (artifact_id) cannot escape the intended "root" directory (RESULTS_DIR). To do this, we'll first construct the intended path, then normalize it using os.path.normpath or equivalent. We'll then ensure that the normalized path is still strictly within (i.e., starts with) the canonical path to RESULTS_DIR. If it does not, we raise an error.

Specifically, in the test_get_results endpoint, before using (RESULTS_DIR / f"{artifact_id}_size_analysis.json") and (RESULTS_DIR / f"{artifact_id}_app"), we will resolve and validate the filenames. This can be factored into a small helper function for reusability (since the same fix pattern will be used for both). No new functionality should be introduced; just extra checking and safe path construction. As minimum required, import os if not already, and provide the validation logic only using standard libraries.

---

To fix this, the code should ensure that any file path computed using user input remains inside the intended directory. The best-practice approach is to normalize the constructed path (with os.path.normpath or similar) and ensure it is still under the intended directory (in this case, RESULTS_DIR). This can be done by resolving the path and checking that it starts with the RESULTS_DIR. If the check fails, return a 400 or 403 error. Only then should the file access proceed.

The edit is required only around the usage of artifact_id for the file path, under test_get_size_analysis_raw in tests/e2e/mock-sentry-api/server.py.

You may need to import os or use methods from pathlib, but as os and pathlib are already imported, no new imports are needed.

---

To fix this path traversal vulnerability, sanitize or validate the user-supplied artifact_id before using it in a filesystem operation. The best approach is to construct the final file path, normalize it using os.path.normpath (or .resolve() from pathlib), and then check that the final path is indeed inside the intended RESULTS_DIR directory. If the check fails, return an HTTP 400 or 403 error. This prevents attackers from using crafted artifact_id values to escape the intended directory.

What to change:

• In test_get_size_analysis_raw, normalize the constructed path and ensure it is within the RESULTS_DIR.
• If the normalized path does not start with/contain the desired base directory, raise an HTTP error.
• To use os.path.normpath, ensure os is imported (which it is).
• The fix goes in the block beginning at line 316.

---