Skip to content

security: confine request-derived filesystem paths (43 js/path-injection alerts) - #77

Merged
alloevil merged 1 commit into
masterfrom
security/path-injection
Oct 18, 2026
Merged

alloevil merged 1 commit into
masterfrom
security/path-injection

Conversation

@alloevil

Copy link
Copy Markdown
Owner

Fixes all 43 open js/path-injection CodeQL alerts on master (5e48017) with a central path guard instead of suppressions.

Triage (from the alert data flows)

Source Sinks Verdict
req.query.dir / dirCodex… → resolveDir() (routes/sessions, insights, prompts, watch, export, search) readdir/readFile/stat in platforms/*, search, export Real: any absolute path was accepted, so the API could list/read any directory the process can see.
req.params.name / :sessionId → sanitizeAgentName / sanitizeSessionId openclaw/claude/omp/session routes joins Real bug: the regexes allowed .., so /api/agents/%2E%2E/sessions walked up one level.
library prompt names (LIBRARY_NAME_RE), prompts analysis key ([a-z0-9-]) lib/library.js, lib/prompts.js Effectively safe already (strict allow-lists), but routed through the same helper so the guard is explicit. No dismissals.

Fix

  • lib/safe-path.js: resolveInside(base, ...parts) (path.resolve + must stay inside base; rejects .. escape, absolute segments, NUL) and resolveAllowedDir(input) (absolute, no .., realpath must sit inside HOME / the configured platform & storage dirs / AGENTXRAY_ALLOWED_DIRS). Errors are PathEscapeError (status 403).
  • resolveDir() uses resolveAllowedDir; every request-derived join in platforms/openclaw, claude, omp, hermes, routes/sessions, search, library, prompts uses resolveInside.
  • /api middleware returns 403 for any bad dir* query param before routes run.
  • Agent names / session ids made only of dots → 400.
  • docs/usage.md documents the allowed roots.

Tests (added to existing files; test-file count stays 22)

api.test.js: ?dir=/etc, parent of HOME, ../ escape, relative, NUL, dirCodex=/etc → 403; symlink in HOME pointing outside → 403; explicit ?dir= inside HOME returns the same sessions as default; %2E%2E agent/session → 400. unit.test.js: helper unit tests. 349 → 356 tests; claims.json + ROADMAP updated (gate unchanged).

Local evidence (Node 22.23.3)

  • npm test: # tests 356 # pass 356 # fail 0
  • verify-claims v0.1.1: 16 machine-checked · 16 ok · 0 failed · 0 error · 3 manual
  • biome: no errors
  • CodeQL CLI 2.27.2, javascript-code-scanning suite: master 47 results (path-injection 43, redos 2, incomplete-sanitization 2) → this branch 4 (path-injection 0). The remaining 4 are handled in a follow-up PR.

…jection alerts)

Add lib/safe-path.js: resolveInside(base, ...parts) resolves under a trusted
base and rejects '..', absolute segments and NUL bytes; resolveAllowedDir()
validates ?dir= overrides (absolute, no '..', realpath inside HOME / configured
dirs / AGENTXRAY_ALLOWED_DIRS). Every path join built from request input now
goes through these. An /api middleware answers 403 for a bad dir* param.
sanitizeAgentName/sanitizeSessionId no longer accept dot-only names ('..').

Tests: 7 new cases (traversal 403/400, symlink escape, normal dir still served,
helper unit tests). Test count 349 -> 356 in claims.json and ROADMAP.
@alloevil
alloevil merged commit 152dc67 into master Oct 18, 2026
5 checks passed
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.

1 participant