fix(external): prevent task ID URL path injection#3324
Merged
danielaskdd merged 3 commits intoJun 24, 2026
Conversation
VectorPeak
force-pushed
the
fix/external-task-id-url-encoding
branch
from
June 24, 2026 11:01
c0801e5 to
6a5239b
Compare
VectorPeak
force-pushed
the
fix/external-task-id-url-encoding
branch
from
June 24, 2026 11:12
6a5239b to
6aea62e
Compare
VectorPeak
marked this pull request as ready for review
June 24, 2026 11:20
Collaborator
|
@codex review |
Contributor
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- encode local poll and result task ids as a single path segment
- prevent crafted ids from escaping `/tasks/{id}` or adding queries
- add regression tests for official and local mineru flows
- update docling test fakes to match encoded task id routing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This fixes a request path injection risk (CWE-116: Improper Encoding or Escaping of Output) in the external parser clients, where task identifiers returned by Docling/MinerU services are interpolated directly into URL path segments before polling or result-download requests.
The affected paths are built from a configured parser endpoint plus a service-returned task identifier:
task_id/batch_idare opaque values returned by the external parser service. If a compromised, malicious, or incompatible parser service returns a value containing URL-reserved characters such as/,?,#, or dot-segment-like path components, the HTTP client can interpret that value as request structure rather than a single path segment.The concrete impact is that LightRAG may send follow-up poll/result requests to an unintended path on the configured parser service origin. This does not change the configured host by itself, but it weakens the URL path boundary between LightRAG and the external parser service.
The parser client is the correct place to enforce this boundary because it is the component that consumes service-returned task identifiers and turns them into follow-up request paths. Relying on external services to always return URL-safe segment text leaves the final request construction too permissive.
Changes Made
Checklist
Additional Notes
Evidence:
The issue can be reproduced at the URL-construction layer with a parser-service-controlled identifier. This local reproduction uses fake parser clients only; it does not contact a real parser service.
A local mock parser service also shows the same issue at the HTTP request-target level. The client constructs a vulnerable MinerU official polling URL with a crafted
batch_id; the mock server records the request path it actually receives.Run locally:
.\.venv\Scripts\python.exe .\poc_task_id_path_injection.pyObserved output:
In the vulnerable case,
../changes the request path and?x=1becomes a query string. In the fixed case,quote(..., safe="")keeps the crafted identifier inside the intended path segment.The fragment is included only to show that the unencoded value is parsed as URL syntax. It is not relied on as server-side evidence because URL fragments are normally not sent in HTTP requests.
Possible call chain / impact:
This PR only changes URL path-segment encoding for parser-service identifiers. It does not change parser endpoint selection, upload payloads, polling semantics, zip extraction, caching, chunking, embedding, retrieval, or normal behavior for already URL-safe task IDs.
Validation:
.\.venv\Scripts\python.exe -m ruff check lightrag\parser\external\docling\client.py lightrag\parser\external\mineru\client.py- passed.\.venv\Scripts\python.exe -m pre_commit run --files lightrag\parser\external\docling\client.py lightrag\parser\external\mineru\client.py- passed.\.venv\Scripts\python.exe -m pytest tests\parser\external\docling\test_client.py tests\parser\external\mineru\test_client.py- passed, 30 tests.\.venv\Scripts\python.exe -m pytest tests\parser\external- passed, 174 testsgit diff --check- passedLinting and Formatting,Offline Tests (3.12), andOffline Tests (3.14)- passed