Skip to content

[WIP] Fix P2+ security bugs in zip-slip and unbounded requests#9

Closed
beautifulrem with Copilot wants to merge 1 commit into
copilot/audit-repository-for-p2-bugsfrom
copilot/sub-pr-8
Closed

[WIP] Fix P2+ security bugs in zip-slip and unbounded requests#9
beautifulrem with Copilot wants to merge 1 commit into
copilot/audit-repository-for-p2-bugsfrom
copilot/sub-pr-8

Conversation

Copilot AI commented Apr 27, 2026

Copy link
Copy Markdown

Thanks for the feedback on #8. I've created this new PR, which merges into #8, to address your comment. I will work on the changes and keep this PR's description up to date as I make progress.

Original PR: #8
Triggering comment (#8 (comment)):

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

@beautifulrem

Copy link
Copy Markdown
Owner

Closing this draft because it was only a follow-up Copilot placeholder after the agent hit its session limit. The additional audit fixes have been implemented directly on master in commit 03c4ff6.

@beautifulrem beautifulrem deleted the copilot/sub-pr-8 branch May 3, 2026 04:53
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.

2 participants