fix: bound knowledge-base ingestion - #95
Closed
GautamSharma99 wants to merge 3 commits into
Closed
Conversation
Contributor
Author
|
Hey @mldangelo-oai @ianw-oai , could someone take a look at the knowledge-base resource limits and cancellation changes? I’d especially appreciate feedback on whether the chosen limits feel reasonable for real-world usage. Thanks! |
Collaborator
|
Thanks for putting this together. I don’t think this addresses a real problem for us, and the added complexity isn’t worth it, so I’m going to close this. |
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.
Summary
Add resource limits and cancellation support to knowledge-base discovery and document extraction.
Previously, recursive discovery accumulated every supported path without bounding directory depth, entry count, or document count. Documents
were then read completely into memory without enforcing their reported size, aggregate input size, or extracted-output size. PDF extraction
also processed every page without a page limit.
A large or adversarial knowledge base could therefore consume excessive memory, CPU, and temporary disk space. Cancellation was only checked
outside directory traversal and could not interrupt PDF processing.
Changes
Bounded discovery
Directory traversal now uses an asynchronous directory iterator instead of loading each directory's complete entry list.
The following limits are enforced:
The abort signal is checked:
Duplicate source paths are canonicalized and skipped before repeated discovery.
Bounded input
Knowledge-base input is limited to:
File metadata is checked during discovery so oversized inputs are rejected before temporary output is created or document contents are read.
Documents are opened with the no-follow boundary where supported and read in bounded 64 KiB chunks. Limits are rechecked against the opened
file and during reading, preventing a file-growth race from bypassing the initial metadata check.
Abort signals are checked between reads.
Bounded extraction
Extracted text is limited to:
The aggregate limit also bounds the amount of extracted text written to the temporary knowledge-base directory.
DOCX extraction checks the declared uncompressed size of
word/document.xmlbefore decompression and observes cancellation before and afterextraction.
Bounded and cancellable PDFs
PDF documents are limited to 512 pages.
PDF extraction now:
Documentation
The README now documents all knowledge-base limits so CLI behavior and user expectations match the implementation.
Security impact
This prevents untrusted or unexpectedly large knowledge bases from causing unbounded:
It also makes scan cancellation effective during knowledge-base discovery and PDF extraction.
Tests
Added regression coverage for:
Verification performed:
pnpm run typespnpm run formatpnpm run buildpnpm run testFull test result: