fix: API 400 - absolute audio file paths are not allowed for temp folder

[goedzo]

This PR resolves the 400 Bad Request error triggered during file uploads where the server incorrectly rejected absolute paths for its own temporary files.

Key Changes:

• Fix: Updated _validate_audio_path to allow absolute paths strictly when they reside within the system's temporary directory.
  

• Hardening: Replaced string-based prefix checks with os.commonpath to prevent directory boundary bypasses (e.g., /tmp_evil vs /tmp).
  

• Refactor: Moved path validation to the API entry points (JSON/Multipart handlers) to distinguish between untrusted user-provided strings and trusted server-generated paths, ensuring _build_request no longer trips over internal temp files.
  

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Strengthened audio file path validation to enhance security and prevent directory traversal vulnerabilities
  • Improved consistency in path validation across different input methods and content types
  • Enhanced validation for both uploaded audio files and manually provided file paths
  • Better protection against unauthorized file access through improved security boundary checks

[coderabbitai]

📝 Walkthrough

Walkthrough

The API server's audio path validation has been tightened to restrict absolute paths to the system temporary directory only, while consolidating validation across JSON and form-based input handlers. Path normalization and boundary checks now apply consistently throughout request construction to prevent directory traversal.

Changes

Cohort / File(s)	Summary
Audio path validation refactoring acestep/api_server.py	Modified _validate_audio_path to accept absolute paths only within system temp directory, applied explicit validation to reference_audio_path and src_audio_path across JSON and multipart request handlers, added path normalization and traversal prevention checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix path traversal vulnerability in audio file path parameters #343: Modifies audio path validation in the same file by adding/centralizing _validate_audio_path and applying it to reference and source audio paths across request parsing, with additional boundary checks for absolute paths within system temp directory.

Suggested reviewers

• ChuxiJ

Poem

🐰 A rabbit hops through paths so tight,
Validating each absolute with might,
No traversal tricks shall slip on by,
Temp directory walls reach the sky,
Security bounces—our code flies right! 🛡️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing an issue where absolute audio file paths in the temporary folder were incorrectly rejected by the API.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@acestep/api_server.py`:
- Around line 905-921: The current temp-path acceptance logic in the code around
the path resolution (use of system_temp and requested_path and is_in_temp)
accepts any path under the system temp dir and thereby allows user-supplied
absolute paths; change it to only accept temp paths that the server actually
created for the current request/session by maintaining and checking a
per-request/per-session whitelist (e.g., store filenames/paths returned by
tempfile.mkstemp() when creating files and validate requested_path against that
whitelist) and reject any absolute temp paths not present in that whitelist;
also remove the redundant local import of tempfile (it’s already imported at
module level) and keep the os.path.commonpath check only as a secondary guard
after whitelist validation.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check module-level imports and the import at line 907
head -50 acestep/api_server.py | grep -n "import"

Repository: ace-step/ACE-Step-1.5

Length of output: 1025

---

🏁 Script executed:

# Check around line 26 for tempfile import and line 907
sed -n '20,30p' acestep/api_server.py
sed -n '900,925p' acestep/api_server.py

Repository: ace-step/ACE-Step-1.5

Length of output: 1322

---

🏁 Script executed:

# Search for how temp files are created and named in the codebase
rg -n "mkstemp|mkdtemp|NamedTemporaryFile|tempfile\." --type py -B2 -A2

Repository: ace-step/ACE-Step-1.5

Length of output: 2437

---

Security concern: User-controlled absolute paths in temp directory bypass per-session validation.

The updated logic now accepts any absolute path within the system temp directory, including paths that users supply directly. While tempfile.mkstemp() generates cryptographically random file names (making enumeration impractical), the function does not verify that a user-provided path actually corresponds to a legitimate server-generated file for the current request/session. This allows potential cross-request file access if an attacker successfully predicts or enumerates temp file names.

Recommended mitigations:

1. Maintain a whitelist of temp files created for the current request/session and validate against it before accepting user-provided temp paths
2. Only accept absolute temp paths that were generated by the server, not user-supplied strings
3. Document the security boundary clearly if accepting all temp paths is intentional

Additionally, remove the redundant import tempfile at line 907 (tempfile is already imported at module level on line 26).

🔧 Proposed fix for redundant import

    if not path:
        return None
-    
-    # Resolve requested path and system temp path to normalized absolute forms
-    import tempfile
+
+    # Resolve requested path and system temp path to normalized absolute forms
     system_temp = os.path.realpath(tempfile.gettempdir())

🤖 Prompt for AI Agents

In `@acestep/api_server.py` around lines 905 - 921, The current temp-path
acceptance logic in the code around the path resolution (use of system_temp and
requested_path and is_in_temp) accepts any path under the system temp dir and
thereby allows user-supplied absolute paths; change it to only accept temp paths
that the server actually created for the current request/session by maintaining
and checking a per-request/per-session whitelist (e.g., store filenames/paths
returned by tempfile.mkstemp() when creating files and validate requested_path
against that whitelist) and reject any absolute temp paths not present in that
whitelist; also remove the redundant local import of tempfile (it’s already
imported at module level) and keep the os.path.commonpath check only as a
secondary guard after whitelist validation.