Skip to content

fix(storage): block path traversal in filters API on localfs#1727

Open
ygndotgg wants to merge 1 commit into
parseablehq:mainfrom
ygndotgg:fix-path-traversal
Open

fix(storage): block path traversal in filters API on localfs#1727
ygndotgg wants to merge 1 commit into
parseablehq:mainfrom
ygndotgg:fix-path-traversal

Conversation

@ygndotgg

@ygndotgg ygndotgg commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #XXXX.

Description

This PR fixes a path traversal issue in the Filters API that allowed an authenticated low-
privilege user to write or delete files outside the configured data directory when the local
filesystem storage backend was enabled.

The chosen fix is intentionally minimal and defensive:

  • validate stream_name at the Filters API handler boundary before create/update operations
  • reject invalid or path-like stream names with a 400 Bad Request
  • add a storage-layer guard in LocalFS::delete_object to block .. traversal components
  • add tests covering valid stream names and path-like inputs

Summary by CodeRabbit

  • Bug Fixes
    • User-defined filters now reject invalid stream names with a clear 400 Bad Request response.
    • Filter names using unsupported path-like formats are blocked before saving.
    • Local file storage now prevents directory traversal attempts, improving data protection.
    • Directory traversal errors now provide a dedicated, descriptive error response.

  Validate filter stream names before creating or updating filters, and reject
  path-traversal components in LocalFS delete operations so authenticated users
  cannot escape the configured data directory via /api/v1/filters.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

User-defined filter stream names are validated during creation and modification, with invalid names returning HTTP 400. Local filesystem deletion rejects relative paths containing parent-directory components and reports a traversal error.

Changes

User filter stream validation

Layer / File(s) Summary
Validate filter stream names
src/handlers/http/users/filters.rs
Filter creation and modification validate user-defined stream names, map validation failures to HTTP 400, and test flat and path-like names.

Local filesystem path validation

Layer / File(s) Summary
Reject traversal during deletion
src/storage/mod.rs, src/storage/localfs.rs
Adds ObjectStorageError::PathTraversal and rejects deletion paths containing parent-directory components before filesystem resolution.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: nikhilsinhaparseable, parmesant

Poem

A rabbit checks each stream by name,
And guards the paths from sneaky game.
No slash may slip, no parent flee,
Safe little burrows, tidy as can be.
Hop, hop—validation wins! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, specific, and accurately summarizes the main path-traversal fix in the Filters API.
Description check ✅ Passed The description includes the issue reference and a clear summary of the fix, but omits the template's testing/comment/documentation checklist.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/storage/localfs.rs`:
- Around line 431-440: Centralize native path validation by adding an
ensure_safe_path helper and updating path_in_root to return Result<PathBuf,
ObjectStorageError>, validate its resolved path there, and propagate errors at
every caller. Apply ensure_safe_path unconditionally to get_object’s final
file_path before fs::read, including the snapshot-resolution branch; update
put_object, delete_prefix, and delete_object to use the validated path and
remove delete_object’s old relative_path-only check.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f2d028c4-a98d-4fb9-babc-92f30ef23a1b

📥 Commits

Reviewing files that changed from the base of the PR and between dc8ff16 and 32ba4fd.

📒 Files selected for processing (3)
  • src/handlers/http/users/filters.rs
  • src/storage/localfs.rs
  • src/storage/mod.rs

Comment thread src/storage/localfs.rs
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