Skip to content

fix: address path traversal vulnerability and false warnings in config file handlers - #1449

Open
ankita10119 wants to merge 6 commits into
masterfrom
DXCDT-2091
Open

fix: address path traversal vulnerability and false warnings in config file handlers#1449
ankita10119 wants to merge 6 commits into
masterfrom
DXCDT-2091

Conversation

@ankita10119

Copy link
Copy Markdown
Contributor

🔧 Changes

Background and motivation

This PR addresses two related problems reported against the 8.x release line:

1. Security vulnerability (SEC-23449) - path traversal
Resource configurations (actions, actionModules, rules, hooks, databases) reference external code files by path. A malicious or misconfigured path such as ../../sensitive-file.js could cause the CLI to load a file from outside the intended config directory. This is a path traversal vulnerability reported by the security team.

2. False deprecation warnings on legitimate relative paths (issue #1432)
Users reported that the CLI was incorrectly emitting deprecation warnings for valid relative paths such as ./actions/action-one/code.js. Investigation confirmed the root cause: the actions and actionModules handlers were using a fragile regex to pre-process paths before passing them to loadFile(), and loadFile() itself had a silent isFile fallback (isFile(inSubfolder) ? inSubfolder : inRoot) that behaved unpredictably depending on whether the file existed at the expected location.

These two problems share the same root cause: inconsistent and fragile path resolution logic across handlers.


Root cause

databases.ts already had the correct approach, it used path.resolve + startsWith(configRoot) directly in the handler. However, actions.ts and actionModules.ts used a regex to strip path prefixes before delegating to loadFile():

 // Old approach — fragile regex stripping caused false warnings
 const unixPath = action.code.replace(/[\\/]+/g, '/').replace(/^([a-zA-Z]+:|\.\/)/, '');
 action.code = context.loadFile(unixPath, actionFolder);

The regex incorrectly stripped the ./ prefix, causing loadFile() to resolve against the wrong base path and trigger false warnings for valid paths.

rules.ts and hooks.ts passed paths directly to loadFile() without any pre-processing, relying on the isFile fallback which could silently load the wrong file.


Edge cases identified and addressed

  1. Windows backslash paths - action.code can contain actions\code.js on Windows. Fixed by normalizing backslashes with .replace(/\/g, '/') before calling path.resolve.
  2. Action code path relative to config root - Action JSON stores the code path relative to the config root (e.g. "./actions/action-one/code.js"). The old regex stripped ./, causing misresolution. Fixed by resolving directly from context.filePath using path.resolve.
  3. Rule and hook script paths relative to their subfolder - Rule JSON stores script paths relative to the rules/ subfolder (e.g. "somerule.js"), not the config root. Fixed by resolving from configRoot/rules/ in rules.ts and configRoot/hooks/ in hooks.ts.
  4. Absolute paths passed to loadFile() - On Unix, path.join(base, absolutePath) silently discards the base. Fixed by using path.resolve() throughout, which handles absolute paths correctly.
  5. Silent isFile fallback - The old loadFile() tried inSubfolder, and if the file didn't exist there, silently fell back to inRoot with no error or warning, potentially loading the wrong file. Removed entirely, path is now resolved deterministically with no fallback.
  6. Monorepo setups with shared code outside config root - Some teams store shared action code in a directory above the config root. A hard block would break these legitimate setups. Addressed with the AUTH0_ALLOW_EXTERNAL_CODE_PATHS escape hatch (see below).

What changed

  • actions.ts / actionModules.ts: Replaced regex pre-processing + loadFile() delegation with an explicit path.resolve + startsWith check, directly calling loadFileAndReplaceKeywords. Now consistent with databases.ts.
  • rules.ts / hooks.ts: Replaced loadFile() delegation with the same inline path.resolve + startsWith check.
  • directory/index.ts (loadFile()): Removed the isFile fallback. Emits a deprecation warning when path resolves outside config root. This will become a hard error in v9.0.0.
  • yaml/index.ts (loadFile()): Same fix - emits a deprecation warning when path resolves outside config root.
  • databases.ts: Detection logic was already correct; updated to support AUTH0_ALLOW_EXTERNAL_CODE_PATHS consistently with other handlers.
  • types.ts: Added AUTH0_ALLOW_EXTERNAL_CODE_PATHS?: boolean to the Config type.
  • docs/configuring-the-deploy-cli.md: Added documentation for AUTH0_ALLOW_EXTERNAL_CODE_PATHS including the monorepo use case, directory structure example, upgrade guidance, and security notice.

AUTH0_ALLOW_EXTERNAL_CODE_PATHS escape hatch

Introduces an explicit opt-in config flag for monorepo setups where code files legitimately reside outside the config root directory. When set to true, the deprecation warning is suppressed and the file loads successfully. When not set (default), a deprecation warning is emitted, users seeing this warning should move their files inside the config directory before upgrading to v9.0.0, where this will be enforced as a hard error.

⚠️ Reviewer attention required: Please specifically evaluate the AUTH0_ALLOW_EXTERNAL_CODE_PATHS flag.
This flag was introduced solely to support the monorepo use case described above. However, it bypasses the path traversal protection. The security team should confirm whether this escape hatch is acceptable and under what conditions it should be permitted. Full context is documented in docs/configuring-the-deploy-cli.md.

📚 References

Unit tests have been added or updated for all affected handlers:

  • test/context/directory/actions.test.js: Added tests for valid relative paths (no warning), path traversal warning, escape hatch (AUTH0_ALLOW_EXTERNAL_CODE_PATHS), and Windows-style backslash paths.
  • test/context/directory/rules.test.js: Added test asserting a deprecation warning is emitted when script path resolves outside config root.
  • test/context/directory/hooks.test.js: Added test asserting a deprecation warning is emitted when script path resolves outside config root.

All 25 tests in the above files pass.

🔬 Testing

Manual testing:

  • Point an action JSON at a path using ../../ traversal - verify the deprecation warning fires and the deploy continues (8.x behavior).
  • Point an action JSON at a valid relative path (./actions/action-one/code.js) - verify it deploys with no warning.
  • Set AUTH0_ALLOW_EXTERNAL_CODE_PATHS: true with an external path - verify it loads successfully with no warning.

📝 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 August 4, 2026 16:35
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.91228% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.22%. Comparing base (e245f16) to head (86355e5).

Files with missing lines Patch % Lines
src/context/directory/index.ts 12.50% 7 Missing ⚠️
src/context/directory/handlers/actionModules.ts 55.55% 3 Missing and 1 partial ⚠️
src/context/directory/handlers/databases.ts 40.00% 3 Missing ⚠️
src/context/directory/handlers/hooks.ts 77.77% 1 Missing and 1 partial ⚠️
src/context/directory/handlers/rules.ts 77.77% 1 Missing and 1 partial ⚠️
src/context/yaml/index.ts 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1449      +/-   ##
==========================================
- Coverage   80.36%   80.22%   -0.15%     
==========================================
  Files         163      163              
  Lines        7595     7619      +24     
  Branches     1677     1686       +9     
==========================================
+ Hits         6104     6112       +8     
- Misses        797      811      +14     
- Partials      694      696       +2     

☔ View full report in Codecov by Harness.
📢 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.

@harshithRai harshithRai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, provided v9 (#1448) lands the hard-error enforcement as I see it does there, since this only warns.

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.

3 participants