feat(pdf-tools): add pdf-tools extension - #405
manuelcontrera wants to merge 3 commits into
Conversation
|
Thanks for contributing an extension to Vicinae! 👋 Before publication, this pull request receives two reviews:
✅ Ready for human review. The automated reviewer approved the latest commit and a maintainer has been notified. No blocking findings remain on the latest commit. The automated reviewer examines only the current commit. New commits invalidate its previous decision and start another review. |
clankus-aurelius
left a comment
There was a problem hiding this comment.
The extension has two publication-blocking data/credential exposure issues, plus cross-platform selection and documentation defects.
Automated review found 2 publication-blocking issues.
This is an AI-generated first pass and may be mistaken. If a finding is unclear or incorrect, reply in the relevant thread and mention @aurelleb.
| } | ||
|
|
||
| /** | ||
| * Unlock a password-protected PDF file in place. | ||
| */ | ||
| export async function unlockPDF(filePath: string, password: string): Promise<void> { |
There was a problem hiding this comment.
🔴 Blocking — Password is exposed in failure messages
Rule: SECURITY-003
The password is interpolated into the shell command, and non-installation failures pass exec's raw err.message upward. That message includes the invoked command, and callers display it in a failure toast, exposing the entered password.
Suggested resolution: Pass qpdf arguments without constructing a shell command and surface a sanitized error that never includes the password or full command.
| const ext = path.extname(filePath); | ||
| const baseName = path.basename(filePath, ext); | ||
| const outputPath = path.join(dir, `${baseName} [watermarked].pdf`); | ||
|
|
||
| fs.writeFileSync(outputPath, pdfBytes); | ||
| return outputPath; |
There was a problem hiding this comment.
🔴 Blocking — Watermark output silently overwrites an existing file
Rule: CORRECTNESS-001
Every run writes to the fixed <name> [watermarked].pdf path with writeFileSync, replacing any existing document at that path without confirmation.
Suggested resolution: Generate a collision-free output name or explicitly confirm replacement before writing.
| */ | ||
| function escapeArg(arg: string): string { | ||
| return `"${arg.replace(/(["\\$`])/g, "\\$1")}"`; |
There was a problem hiding this comment.
🟠 Warning — Shell escaping is not valid on Windows
Rule: CORRECTNESS-001
escapeArg implements POSIX-style backslash escaping, but commands run through cmd.exe on Windows. For example, %NAME% sequences in passwords or paths are expanded, so qpdf receives different values and protect/unlock can fail or use an unintended password.
Suggested resolution: Replace shell-built qpdf commands with execFile/spawn and pass every option and path as a separate argument.
| const mult = multiple ? "with multiple selections allowed" : ""; | ||
| const typeList = allowedExtensions.map((e) => `"${e.replace(/^\./, "")}"`).join(","); | ||
| const script = `choose file with prompt "${prompt}" of type {${typeList}} ${mult}`; | ||
| const { stdout } = await execAsync(`osascript -e '${script}'`); | ||
| if (!stdout.trim()) { | ||
| throw new Error("File selection cancelled"); | ||
| } | ||
| const paths = stdout | ||
| .split(",") |
There was a problem hiding this comment.
🟠 Warning — macOS dialog results are not converted to POSIX paths
Rule: CORRECTNESS-001
AppleScript choose file returns aliases, but the result is passed directly to cleanFilePath, which expects filesystem paths. When Finder and clipboard selection are unavailable, valid dialog selections are therefore discarded.
Suggested resolution: Have the AppleScript convert each selected alias to its POSIX path and emit an unambiguous delimiter before calling cleanFilePath.
| ## Universal File Selection | ||
|
|
||
| PDF Tools provides a frictionless file selection experience across all operating systems: | ||
| 1. **Active Finder / Explorer Selection**: Directly picks files currently selected in macOS Finder. |
There was a problem hiding this comment.
🟠 Warning — Linux file-picker support is overstated
Rule: MANIFEST-001
The README promises a zenity / kdialog fallback, but pickFilesWithDialog only invokes zenity; KDE systems with only kdialog receive a failure.
Suggested resolution: Implement the documented kdialog fallback or remove kdialog from the documented behavior.
| 1. **Active Finder / Explorer Selection**: Directly picks files currently selected in macOS Finder. | |
| 3. **Native File Picker Fallback**: If no files were previously selected or copied, an interactive native file dialog opens automatically (`zenity` on Linux, AppleScript dialog on macOS, `OpenFileDialog` on Windows). |
…ions, and dialogs
clankus-aurelius
left a comment
There was a problem hiding this comment.
The prior findings are resolved, but the new Linux fallback mishandles normal dialog cancellation.
Automated extension review passed. A maintainer review is still required.
This is an AI-generated first pass and may be mistaken. If a finding is unclear or incorrect, reply in the relevant thread and mention @aurelleb.
| } | ||
|
|
||
| try { | ||
| const multFlag = multiple ? "--multiple --separate-output" : ""; | ||
| const patterns = allowedExtensions |
There was a problem hiding this comment.
🟠 Warning — Cancelling Zenity opens another file picker
Rule: CORRECTNESS-001
Zenity reports cancellation by exiting nonzero, so execAsync rejects before the empty-output check. The resulting process error does not necessarily contain “cancelled”; this catch then launches kdialog, turning a normal cancellation into a second unexpected dialog and potentially a misleading installation error.
Suggested resolution: Recognize Zenity's cancellation exit code and return a cancellation error. Fall back to kdialog only when Zenity is unavailable or fails for a non-cancellation reason; apply equivalent cancellation handling to kdialog.
|
@aurelleb automated review passed for |
clankus-aurelius
left a comment
There was a problem hiding this comment.
The previous Linux dialog-cancellation issue is resolved. No new actionable problems were introduced by the incremental changes.
Automated extension review passed. A maintainer review is still required.
This is an AI-generated first pass and may be mistaken. If a finding is unclear or incorrect, reply in the relevant thread and mention @aurelleb.
Summary
Adds the PDF Tools extension adapted and enhanced for Vicinae, with full cross-platform support for Linux, macOS, and Windows.
This extension brings a full suite of PDF manipulation, encryption, splitting, vector watermarking, and multi-format image conversion tools directly into Vicinae.
Features
images-to-pdf): Merge and convert images of any format (.png,.jpg,.jpeg,.tiff,.webp,.bmp,.gif,.avif) into a single PDF ordered by natural filename sort ([img1, img2, img10]). Preserves resolution, handles EXIF rotation, and blends alpha transparency cleanly over white.merge): Combine two or more PDF files into a single unified document in the source folder.protect): Encrypt PDF files with AES-256 password protection in place.unlock): Decrypt password-protected PDF files in place.split-by-file-size): Split large PDFs into parts based on a maximum MB threshold using an optimized binary search.split-by-page-count): Split PDFs into parts with a specified number of pages each.watermark): Pure JS vector watermarking engine viapdf-lib, applying text watermarks across all pages with custom angle (0° or 45°) and opacity.Cross-Platform File Selection Engine
wl-paste(Wayland uri-list / gnome-copied-files) and@vicinae/apiClipboard.Get-Clipboard -Format FileDropListand@vicinae/apiClipboard.zenity/kdialogon Linux, AppleScript dialog on macOS,OpenFileDialogon Windows).Implementation Notes
@vicinae/api^0.29.0.vici lint(valid manifest) andvici buildwith zero errors.qpdfand Python Pillow) are documented with cross-platform installation instructions, and provide friendly error toasts if missing.Credits