Skip to content

fix: prevent path traversal in config file handlers#1388

Open
ankita10119 wants to merge 2 commits into
masterfrom
DXCDT-1616
Open

fix: prevent path traversal in config file handlers#1388
ankita10119 wants to merge 2 commits into
masterfrom
DXCDT-1616

Conversation

@ankita10119
Copy link
Copy Markdown
Contributor

🔧 Changes

Fixes a path traversal vulnerability in config file handlers where user-supplied file paths in configuration (e.g. code, customScripts) could reference files outside the config directory using sequences like ../../.

Changes:

  • DirectoryContext.loadFile: Resolves the final path and validates it stays within the config root before loading. Removes the previous fallback that silently tried the raw path if the relative path didn't exist, that fallback was the entry point for traversal
  • yaml/index.ts: Same boundary check applied to the YAML context's loadFile, throwing an explicit error if the resolved path escapes the config directory
  • databases.ts: Added config root boundary check for customScripts file paths defined in database.json
  • actions.ts / actionModules.ts: Removed the fs.existsSync branch that allowed absolute or root-relative paths to bypass the relative-path resolution, paths are now always resolved relative to the handler folder

📚 References

🔬 Testing

  • Updated existing action tests to use relative paths (./actions/code.js) instead of absolute paths, reflecting the new enforcement that file references must stay within the config directory
  • Updated YAML pages test to use relative page.html instead of an absolute path, and fixed file write ordering to match the new resolution logic
  • Path traversal attempts (e.g. ../../etc/passwd as a script path) will now throw an explicit error rather than silently loading the file

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@ankita10119 ankita10119 requested a review from a team as a code owner May 15, 2026 07:07
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.81%. Comparing base (eb215ab) to head (5d3d020).

Files with missing lines Patch % Lines
src/context/directory/handlers/databases.ts 66.66% 1 Missing and 1 partial ⚠️
src/context/directory/index.ts 66.66% 1 Missing and 1 partial ⚠️
src/context/yaml/index.ts 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1388      +/-   ##
==========================================
- Coverage   79.84%   79.81%   -0.04%     
==========================================
  Files         153      153              
  Lines        7052     7055       +3     
  Branches     1547     1547              
==========================================
  Hits         5631     5631              
- Misses        781      783       +2     
- Partials      640      641       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants