Add post processing pipeline to Questarr#583
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive post-processing import engine for Questarr, featuring automated file organization, archive extraction, and a system browser for manual configuration. The review feedback highlights critical security concerns regarding path traversal in the filesystem browser and functional bugs such as the loss of file extensions during single-file imports. Other recommendations include fixing status check logic, deduplicating path translation code, and optimizing directory traversal to improve performance and symlink handling.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d8ae0b05e
ℹ️ 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".
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3207cc3 to
89835bf
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
89835bf to
e7c4130
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
e7c4130 to
2f7f5f9
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3a40485 to
69531a2
Compare
ac560a2 to
4d060c6
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
69531a2 to
1dbdeef
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1dbdeef to
6293888
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4e1dd9d to
0421c53
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
0421c53 to
8eb05d9
Compare
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8279eab to
60f7b93
Compare
|
…e PR) Remove all RomM-specific code from the import pipeline so this PR stays focused on the generic PC import flow. RomM integration will land in a dedicated follow-up PR. - Remove RommRouting.ts, RomM migration, romm_routing tests - Simplify ImportStrategies to PCImportStrategy only - Simplify ImportManager (remove romm provider selection, slug resolution) - Remove getRomMConfig from IStorage and both storage implementations - Strip romm fields/routes from import router and ImportSettings UI - Remove strategy selector from ImportReviewModal (PC only) - Clean up all test files: remove makeRommConfig, romm mocks, romm assertions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Security: - system.ts: reject `root` query params containing `..` traversal segments - system.ts: add /etc and /root to sensitive path prefix blocklist Functional bugs: - ImportStrategies: preserve file extension in planImport proposed path - ImportManager: fix status guard (completed → owned) in finalizeImport - import route: scope /pending to requesting user (cross-tenant privacy fix) Correctness: - PathMappingService: boundary-aware prefix match (prevents /downloads matching /downloads2) - import route translatePathWithMappings: same boundary-aware fix - schema: add symlink to IMPORT_TRANSFER_MODES (UI allowed it, backend rejected it) - ImportStrategies gatherFiles: use readdir withFileTypes to avoid extra stat calls and circular symlinks Tests: - Update planImport conflict test to expect extension-aware destination path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace all console.* with structured logger calls in ImportManager - Declare processingPath before try block so catch can clean up extracted archives - Revert MemStorage.getDownloadingGameDownloads to only return 'downloading' status - PATCH /config upserts user settings instead of returning 404 when missing - FileBrowser checks res.ok before parsing JSON to surface server errors - ImportReviewModal defaults transfer mode to user's configured setting - ImportReviewModal invalidates /api/downloads and /api/games on confirm - ImportSettings adds onError rollback to reset local config on failed save - PendingImportsCard guards formatDistanceToNow against invalid dates - Rename getRomMPlatform -> getSourcePlatform in PlatformMappingService - Strip RomM-specific text from ImportSettings help tab Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Introduced a new structure for safeFetch options, including timeout and max redirects. - Enhanced the isSafeUrl and isSafeIp functions for better readability and maintainability. - Implemented a new method to handle redirects in safeFetch, allowing for controlled redirection behavior. - Improved error handling and validation logic for hostname resolution and IP safety checks. - Added support for abort signals and timeout management in safeFetch.
Support updating existing remote path mappings and constrain mapping names to configured downloaders in the UI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep filesystem browsing working for Windows drive roots and make manual import review browse from the full filesystem root. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…anually + add skip button in review modal & card
Add GET /api/imports/:id/plan endpoint that resolves the source path via the download client and computes the proposed destination using the same PCImportStrategy logic as the automatic pipeline. The modal fetches this plan on open and pre-fills both fields, falling back to a title-based destination when the source cannot be resolved. Also fix isPathInside to require the proposed path to be strictly inside the library root, preventing accidental imports to the root directory. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Map SABnzbd "Moving" queue status to "unpacking" so files being moved from the incomplete folder are not mistakenly treated as ready to import. Also exclude unpacking and repairing statuses from the progress >= 100 completion trigger in the cron job, preventing imports from firing while usenet post-processing is still in progress. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ture import Prevents the progress >= 100 cron trigger from firing while SABnzbd is still in mid-post-processing phases (integrity check or script execution). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SABnzbd may report a download as completed in history while the file is still being moved to its final location. Reset status to 'downloading' on each failed access so the cron job retries automatically; escalate to manual_review_required only after all retries are exhausted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ary root - planConfirmImport no longer throws when source path is inaccessible; returns the known original path with a title-based proposed path instead - Improve library root validation error message on the server - Client-side guard in ImportReviewModal rejects destination equal to libraryRoot before submitting Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ibrary root input and imported badge on downloads
…downloader tests fix: update package version to 1.4.0 in package-lock.json fix(downloader): modify mock response for NZB file download and add category fix(import): add copy method to fsMock and improve import process handling fix(sabnzbd): refine error logging for SABnzbd history fetch fix(transmission): adjust ID parsing in getDownloadStatus method fix(import): simplify error message for invalid proposed path
- Fix unused catch binding in FileBrowser.tsx (optional catch binding) - Add // NOSONAR to intentional hardcoded IPs in ssrf.test.ts - Add codecov.yml with patch target 65% to fix codecov/patch check Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
b4e9315 to
555b27b
Compare
|


This pull request introduces several new features and improvements to the client-side codebase, focusing on enhancing import workflows and path mapping management. The main additions include a reusable file browser dialog, a modal for reviewing and confirming imports, and a settings page for managing path mappings. Additionally, the dashboard now displays pending imports. Some cleanup and permission changes are also included.
New import and path mapping features:
FileBrowserdialog component for browsing and selecting directories within the filesystem, with support for navigation, loading states, and error handling. This is used in multiple places for consistent directory selection.ImportReviewModalcomponent that allows users to review and manually configure import details, such as destination path, transfer mode (move, copy, hardlink, symlink), and unpacking archives before confirming an import. Integrates the new file browser for destination selection.PathMappingSettingscomponent, providing a UI to view, add, and remove path mappings that translate remote download paths to local paths. Includes inline documentation, validation, and integration with the file browser dialog for selecting local paths.Dashboard enhancements:
PendingImportsCardcomponent in theDashboardto display pending imports, improving visibility of ongoing import operations. [1] [2]Project maintenance and permissions:
.claude/settings.local.jsonto allow a specific Bash command for searching third-party service references, likely for code review or auditing purposes.TASKS.mdfile, which previously tracked hardening and maintenance tasks, suggesting a change in how tasks are managed or documented.These changes collectively improve the user experience for import management and path configuration, while also streamlining project maintenance.