fix: P2+ security bugs — zip-slip, unbounded request body, query length limit

[Copilot]

Full-repo audit surfaced three P2+ issues: path traversal via malicious zip archives, OOM/hang from unbounded HTTP request bodies, and denial-of-service via arbitrarily large query inputs to the NLU pipeline.

Changes

ZIP slip — query_intelligence/external_data/sync.py

• _extract_zip_files called archive.extractall() with no path validation; a crafted entry like ../../etc/cron.d/evil writes outside the target dir
• Added _safe_zip_extractall() that resolves every member path and raises ValueError (blocking extraction) if any entry escapes the target directory via Path.is_relative_to()

# before
with zipfile.ZipFile(path) as archive:
    archive.extractall(extract_dir)

# after
with zipfile.ZipFile(path) as archive:
    _safe_zip_extractall(archive, extract_dir)  # raises ValueError on traversal

Unbounded request body — scripts/llm_response.py

• _read_json_body read exactly Content-Length bytes with no ceiling; an unauthenticated caller declaring Content-Length: 1073741824 causes OOM
• Added _MAX_REQUEST_BODY_BYTES = 10 MB constant; guard raises ValueError → HTTP 400 before any read when the declared length exceeds the cap

Query length DoS — query_intelligence/contracts.py

• AnalyzeRequest.query and PipelineRequest.query were unbounded str; a megabyte-scale input fans out through CRF, TF-IDF, entity resolution, and PostgreSQL FTS
• Added MAX_QUERY_LENGTH = 2000; both fields now carry Field(min_length=1, max_length=MAX_QUERY_LENGTH), producing HTTP 422 before the pipeline is entered

Tests

Added tests/test_security_fixes.py (12 tests): zip traversal blocked, normal extraction unaffected, real ThreadingHTTPServer returns 400 on oversized Content-Length, Pydantic rejects over-limit and empty queries, API endpoints return 422/200 as expected.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 075203c198

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

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".

[chatgpt-codex-connector]

Validate all ZIP members before extracting any file

Because extraction happens inside the same loop that performs the path check, an archive containing both safe entries and a later traversal entry will extract the early files and only then raise ValueError. In this failure mode, _extract_zip_files leaves a partially populated extract_dir, and future sync runs skip that archive entirely due the extract_dir.exists() guard, which can silently preserve incomplete/corrupted data. Pre-validating all members (or extracting to a temp dir and committing atomically) avoids this partial-write state.

Useful? React with 👍 / 👎.

[beautifulrem]

@copilot Please continue the full-repository audit from issue #7 and this PR. Do not treat the three current fixes as the end of the audit. Please look specifically for additional confirmed P2+ bugs or security/reliability risks, and either add focused commits to this PR or open a follow-up PR if a separate patch is cleaner.

Please avoid speculative/style findings. For each confirmed issue, include the vulnerable path, trigger condition, impact, and test coverage.

Follow-up areas to verify:

1. Query and payload limits across every entry point, not just FastAPI contracts.

   • Does MAX_QUERY_LENGTH also cover scripts/llm_response.py /respond and CLI paths that use coerce_query()?
   • Are whitespace-only queries rejected consistently?
   • Are user_profile, dialog_context, and nested JSON payloads bounded, or can a small number of requests still cause CPU/memory amplification?

2. Direct retrieval endpoint abuse.

   • /retrieval/search accepts raw nlu_result: dict. Check whether oversized keywords, symbols, entity_names, source_plan, or other fields can bypass NLU validation and fan out into document search, PostgreSQL FTS, SQL/live providers, or ranking.
   • If confirmed, add schema validation and list/string length caps for direct retrieval payloads.

3. Archive extraction beyond ZIP slip.

   • _extract_7z_files() shells out to unar; verify whether 7z path traversal, symlink entries, absolute paths, or archive bombs can escape/fill disk.
   • For ZIP, check whether _safe_zip_extractall() should pre-validate every member before extracting anything, and whether total uncompressed size / file count / compression ratio caps are needed.
   • Ensure failed extraction does not leave a partially trusted extract_dir that causes later runs to skip extraction.

4. External downloads and subprocesses.

   • download_http_file() streams without a total byte cap. Verify disk-fill risk from compromised or unexpectedly large upstream files.
   • Check whether HTTP download, HuggingFace snapshot, and git clone flows need total size limits, stronger allow-lists, temp-file atomicity, or subprocess timeouts.

5. Model loading and deserialization boundary.

   • The repo loads *.joblib models from QI_MODELS_DIR/models. Confirm the trust boundary: can any user-controlled API/CLI/env path cause unsafe deserialization of attacker-controlled files?
   • Check LLM model loading in scripts/llm_response.py: user-supplied model IDs/paths, remote downloads, and trust_remote_code behavior.

6. Request concurrency and slow-client behavior.

   • The HTTP server caps Content-Length, but verify slow clients, many idle connections, missing socket/read timeouts, invalid Content-Length, and many concurrent requests cannot tie up ThreadingHTTPServer threads.

7. Path writes and artifact output.

   • Re-check all path writes under outputs/, reports/, models/, data/external/, and manual_test/output/.
   • Confirm no user-provided identifier can influence path segments or overwrite unexpected files.

Please prioritize exploitable P2+ issues only. If a checked area is safe, briefly state why and what invariant/test proves it.

[Copilot]

@beautifulrem I've opened a new pull request, #9, to work on those changes. Once the pull request is ready, I'll request review from you.

[beautifulrem]

Closing because this Copilot PR has been superseded by the direct master commit 03c4ff6, which includes these fixes and additional hardening for direct retrieval payloads, external downloads, archive extraction, and llm_response request limits.