Open
Conversation
Contributor
|
At a glance, I think we need some alignment on the files part in KB. Now KB relied on its own file uploading ability. But I think it should leverage the ability from file management. Benefit is pretty clear that in the future, file will only expose file id and meta, reading file can come from internal utility function. the path can not be the local file system only but also for some external storage. We should stop relying on the local file systems any more. |
Contributor
|
To be clear, we need to wait for #15 before we introduce any file related modification. |
Collaborator
Author
…support physical cleanup
- Add sanitize_path_component() for path traversal protection
- Add collection parameter to get_upload_path() and get_file_url()
- KB uploads now stored in user_{id}/{collection}/{filename} structure
- Delete collection physically removes directory before DB deletion
- Rename collection physically renames directory before DB update
- Add validate_file_path() and extract_user_id_from_prefix() to files API
- Enhanced security validation in upload, download, preview endpoints
Ported from FenixAOS feat/upload_dir branch (without LanceDB upgrade)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add filelock for collection dir operations to avoid concurrent conflicts - Replace rmtree with move_collection_dir_to_trash for atomicity and recovery - Add kb_physical_sync module (lock + trash helpers, cleanup_trash) - Update delete_collection_api and rename_collection_api to use lock and trash - Add filelock dependency via pyproject.toml - Fix tests: mock move_collection_dir_to_trash and assert new behavior - .gitignore: ignore filelock test artifacts (*MagicMock*.lock) Made-with: Cursor
- kb: register UploadedFile on ingest, sync DB on collection delete/rename with path sanitization; add type ignore for SQLAlchemy assignment - workspace_file_tool: use workspace_dir.resolve() for relative_to to fix macOS /var vs /private/var path mismatch in tests - tests: override get_db in test_kb_ingest_separators (mock); pass mock_db in test_multitenancy delete_collection_api; fix test_file_upload for file_id (task_id for folder validation, use /upload, KB ingest for list) - tests: remove test_files_security (tested helpers removed with file_id) Made-with: Cursor
sqhyz55
force-pushed
the
feat/upload_dir
branch
from
7616674 to
c90b7eb
Compare
CI uses langfuse 4.x stubs which omit start_span; local may run 3.x. Cast to Any in v3 branch to keep mypy clean without type: ignore. Made-with: Cursor
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
2 participants
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.
No description provided.