feat: Add LibPDF integration for PDF form filling and signing #189
Conversation
- Add tools/pdf/ directory with overview.md and libpdf.md subagents - Create pdf-helper.sh script for common PDF operations (info, fields, fill, merge, text) - Update subagent-index.toon with new PDF tools entry - LibPDF supports: form filling, digital signatures (PAdES), encryption, merge/split - Architecture designed for future PDF tool additions
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the system's capabilities by introducing comprehensive PDF processing features. It establishes a dedicated Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR adds PDF processing capabilities with a new helper script and documentation, introduces background execution support for full-loop workflows with monitoring commands, and bumps the version to 2.84.0 across all version files. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant full-loop-helper.sh as full-loop-helper
participant nohup as Background Process
participant PID File as State
participant Logs as Log Output
User->>full-loop-helper: start --background
full-loop-helper->>full-loop-helper: Detect --bg flag
full-loop-helper->>nohup: Launch with nohup
nohup->>Logs: Redirect stdout/stderr
full-loop-helper->>PID File: Write background PID
nohup->>nohup: Execute _run_foreground task
full-loop-helper-->>User: Return immediately<br/>(show logs guidance)
User->>full-loop-helper: status
full-loop-helper->>PID File: Read PID
full-loop-helper-->>User: Report background process state
User->>full-loop-helper: logs
full-loop-helper->>Logs: Tail log file
full-loop-helper-->>User: Display recent log lines
User->>full-loop-helper: cancel
full-loop-helper->>PID File: Read PID
full-loop-helper->>nohup: Terminate process
full-loop-helper->>PID File: Remove file
full-loop-helper-->>User: Confirm cancellation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a powerful PDF helper script and associated documentation using LibPDF. The implementation is well-structured, but I've identified a critical command injection vulnerability in the shell script that needs to be addressed. Additionally, there are a few correctness issues in the script and some inaccuracies in the documentation examples. My review provides specific suggestions to resolve these issues.
.agent/scripts/pdf-helper.sh
Outdated
| run_script " | ||
| import { PDF } from '@libpdf/core'; | ||
| import { readFileSync } from 'fs'; | ||
|
|
||
| const bytes = readFileSync('$file'); | ||
| const pdf = await PDF.load(bytes); | ||
| const pages = await pdf.getPages(); | ||
| const form = await pdf.getForm(); | ||
| const fields = form.getFieldNames(); | ||
|
|
||
| console.log('File:', '$file'); | ||
| console.log('Pages:', pages.length); | ||
| console.log('Form fields:', fields.length); | ||
|
|
||
| if (pages.length > 0) { | ||
| const { width, height } = pages[0].getSize(); | ||
| console.log('Page size:', Math.round(width), 'x', Math.round(height), 'points'); | ||
| } | ||
| " |
There was a problem hiding this comment.
This block is vulnerable to command injection. The $file variable is directly interpolated into the JavaScript string. A malicious filename containing special characters (e.g., a single quote) could break out of the string and execute arbitrary code. This is a critical security risk.
To fix this, you should pass shell variables to the script via environment variables and use single quotes for the script string to prevent shell expansion.
This same vulnerability pattern exists in cmd_fields, cmd_fill, cmd_merge, and cmd_text and must be fixed in all of them.
Here is the corrected version for cmd_info:
| run_script " | |
| import { PDF } from '@libpdf/core'; | |
| import { readFileSync } from 'fs'; | |
| const bytes = readFileSync('$file'); | |
| const pdf = await PDF.load(bytes); | |
| const pages = await pdf.getPages(); | |
| const form = await pdf.getForm(); | |
| const fields = form.getFieldNames(); | |
| console.log('File:', '$file'); | |
| console.log('Pages:', pages.length); | |
| console.log('Form fields:', fields.length); | |
| if (pages.length > 0) { | |
| const { width, height } = pages[0].getSize(); | |
| console.log('Page size:', Math.round(width), 'x', Math.round(height), 'points'); | |
| } | |
| " | |
| FILE="$file" run_script ' | |
| import { PDF } from "@libpdf/core"; | |
| import { readFileSync } from "fs"; | |
| const file = process.env.FILE; | |
| const bytes = readFileSync(file); | |
| const pdf = await PDF.load(bytes); | |
| const pages = await pdf.getPages(); | |
| const form = await pdf.getForm(); | |
| const fields = form.getFieldNames(); | |
| console.log("File:", file); | |
| console.log("Pages:", pages.length); | |
| console.log("Form fields:", fields.length); | |
| if (pages.length > 0) { | |
| const { width, height } = pages[0].getSize(); | |
| console.log("Page size:", Math.round(width), "x", Math.round(height), "points"); | |
| } | |
| ' |
.agent/scripts/pdf-helper.sh
Outdated
| if $runtime -e "require('@libpdf/core')" 2>/dev/null; then | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
The check for @libpdf/core uses require(), which will fail in a Node.js ESM context. Since run_script uses import with --input-type=module, this check should also use import to be consistent and correct.
| if $runtime -e "require('@libpdf/core')" 2>/dev/null; then | |
| return 0 | |
| fi | |
| if [[ "$runtime" == "bun" ]] && bun -e "import('@libpdf/core')" &>/dev/null; then | |
| return 0 | |
| elif [[ "$runtime" == "node" ]] && node --input-type=module -e "import('@libpdf/core')" &>/dev/null; then | |
| return 0 | |
| fi |
|
|
||
| # Merge PDFs | ||
| cmd_merge() { | ||
| local output="$1" |
There was a problem hiding this comment.
The merge command depends on jq to construct the JSON array of files, but the script doesn't check if jq is installed. This will cause the script to fail abruptly if jq is missing. You should add a check and provide a user-friendly error message.
| local output="$1" | |
| if ! command -v jq &>/dev/null; then | |
| echo -e "${RED}Error:${NC} 'jq' is not installed. Please install it to use the merge command." | |
| return 1 | |
| fi | |
| local output="$1" |
.agent/tools/pdf/libpdf.md
Outdated
| import { PDF, P12Signer } from '@libpdf/core'; | ||
|
|
||
| // Load certificate from P12/PFX file | ||
| const p12Bytes = await fs.readFile('certificate.p12'); |
There was a problem hiding this comment.
The fs.readFile function is used here without importing the fs module. This will cause a runtime error. Several other code examples in this file have the same issue (e.g., lines 300, 382, 458). Please add import * as fs from 'fs/promises'; to the relevant code blocks where file system operations are performed.
.agent/tools/pdf/libpdf.md
Outdated
|
|
||
| // Copy pages to new document | ||
| const newPdf = PDF.create(); | ||
| const [page1, page2] = await pdf.copyPages(pdf, [0, 1]); |
There was a problem hiding this comment.
The usage of copyPages appears incorrect. Based on similar APIs like pdf-lib, you would typically call copyPages on the destination document, passing the source document as an argument. The line should likely be await newPdf.copyPages(pdf, [0, 1]); to copy pages from pdf into newPdf.
| const [page1, page2] = await pdf.copyPages(pdf, [0, 1]); | |
| const [page1, page2] = await newPdf.copyPages(pdf, [0, 1]); |
.agent/tools/pdf/overview.md
Outdated
| const pdf = await PDF.load(bytes); | ||
| const signer = await P12Signer.create(p12Bytes, 'password'); | ||
|
|
||
| const signed = await pdf.sign({ | ||
| signer, | ||
| reason: 'I approve this document', | ||
| }); |
There was a problem hiding this comment.
The 'Sign a Document' example is incomplete and potentially incorrect.
- The
p12Bytesvariable is used without being defined. It should be loaded, for example, from a file usingfs.readFile. - The
pdf.sign()method likely returns an object containing the signed bytes (e.g.,{ bytes }), which needs to be destructured. The current codeconst signed = await pdf.sign(...)may not capture the output correctly.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 00:52:30 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 00:53:58 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
🤖 Augment PR SummarySummary: Introduces a new PDF tooling area for the aidevops agent framework, centered on LibPDF integration for form filling and digital signatures. Changes:
Technical Notes: The helper script shells out to inline ESM JavaScript ( 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
.agent/scripts/pdf-helper.sh
Outdated
| # fields <file> - List form field names and types | ||
| # fill <file> <json> - Fill form fields from JSON | ||
| # merge <output> <files..> - Merge multiple PDFs | ||
| # extract <file> <pages> - Extract pages (e.g., "1-3,5,7-9") |
There was a problem hiding this comment.
The header comment advertises an extract command, but there’s no extract implementation (and it’s not listed in cmd_help / main()), so users will hit “Unknown command”. Consider removing it from the header or adding the command handler.
🤖 Was this useful? React with 👍 or 👎
.agent/scripts/pdf-helper.sh
Outdated
| fi | ||
|
|
||
| # Try to import | ||
| if $runtime -e "require('@libpdf/core')" 2>/dev/null; then |
There was a problem hiding this comment.
check_libpdf uses require('@libpdf/core') to test availability; since @libpdf/core is ESM (type: "module"), this will fail under Node and can falsely report “not found” (especially for non-local installs). Consider checking via ESM import() (matching run_script’s module mode).
🤖 Was this useful? React with 👍 or 👎
.agent/scripts/pdf-helper.sh
Outdated
| const bytes = readFileSync('$file'); | ||
| const pdf = await PDF.load(bytes); | ||
| const pages = await pdf.getPages(); | ||
| const form = await pdf.getForm(); |
There was a problem hiding this comment.
LibPDF pdf.getForm() returns PDFForm | null; form.getFieldNames() will throw for PDFs without an AcroForm. Consider guarding for null/empty forms here (and in other commands that call getForm()).
Other Locations
.agent/scripts/pdf-helper.sh:122.agent/scripts/pdf-helper.sh:157
🤖 Was this useful? React with 👍 or 👎
.agent/scripts/pdf-helper.sh
Outdated
| console.log('Form fields:', fields.length); | ||
|
|
||
| if (pages.length > 0) { | ||
| const { width, height } = pages[0].getSize(); |
There was a problem hiding this comment.
pages[0].getSize() doesn’t match the LibPDF PDFPage API (which exposes page.width/page.height), so info may crash on valid PDFs. Consider switching to the supported page dimension properties.
🤖 Was this useful? React with 👍 or 👎
| check_libpdf || return 1 | ||
|
|
||
| local files_json | ||
| files_json=$(printf '%s\n' "${files[@]}" | jq -R . | jq -s .) |
There was a problem hiding this comment.
cmd_merge depends on jq, but the script doesn’t check for it; with set -e, a missing jq will abort with a confusing error. Consider adding a preflight check with a clearer message.
🤖 Was this useful? React with 👍 or 👎
.agent/scripts/pdf-helper.sh
Outdated
| const pages = await pdf.getPages(); | ||
|
|
||
| for (let i = 0; i < pages.length; i++) { | ||
| const text = await pages[i].getTextContent(); |
There was a problem hiding this comment.
LibPDF PDFPage exposes extractText(); getTextContent() is likely not a method, so text may fail at runtime. Consider aligning this with the LibPDF API used elsewhere.
Other Locations
.agent/tools/pdf/libpdf.md:346
🤖 Was this useful? React with 👍 or 👎
.agent/tools/pdf/libpdf.md
Outdated
|
|
||
| // Copy pages to new document | ||
| const newPdf = PDF.create(); | ||
| const [page1, page2] = await pdf.copyPages(pdf, [0, 1]); |
There was a problem hiding this comment.
This example calls pdf.copyPages(...), but LibPDF’s API is copyPagesFrom(source, indices, options?) (per the PDF API docs). As written, the snippet is likely to mislead readers or fail if copied verbatim.
🤖 Was this useful? React with 👍 or 👎
| ### Encrypt on Save | ||
|
|
||
| ```typescript | ||
| const output = await pdf.save({ |
There was a problem hiding this comment.
The “Encrypt on Save” snippet uses pdf.save({ userPassword, ownerPassword, permissions, ... }), but LibPDF applies encryption via pdf.setProtection(...) and then save(). This section appears to document a non-existent save option.
🤖 Was this useful? React with 👍 or 👎
- Fix command injection vulnerability by using environment variables - Fix ESM import check for @libpdf/core detection - Add jq dependency check for merge command - Fix fs import statements in documentation examples - Fix copyPages API usage in extract pages example
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 00:58:15 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
- Add --background/--bg flag for async execution (avoids MCP timeout) - Add 'logs' command to view background loop output - Fix tool detection: check OPENCODE env var before CLAUDE_CODE - Update cancel to kill background process if running - Update documentation with background mode examples Fixes timeout issue when running full-loop via MCP Bash tool.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 01:15:04 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Use page.width/height instead of page.getSize() - Use page.extractText().text instead of page.getTextContent() - Use pdf.getPages() (sync) instead of await pdf.getPages() - Use copyPagesFrom() instead of copyPages() - Handle null form in info/fields/fill commands - Show filled/skipped fields in fill command output - Remove undocumented 'extract' command from header
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 01:22:02 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
1 participant
Summary
@libpdf/core) for form filling and digital signaturestools/pdf/architecture for future PDF tool additionspdf-helper.shscript for common CLI operationsChanges
New Files
.agent/tools/pdf/overview.md- PDF tools selection guide and quick reference.agent/tools/pdf/libpdf.md- Comprehensive LibPDF usage documentation.agent/scripts/pdf-helper.sh- CLI helper for PDF operationsModified Files
.agent/subagent-index.toon- Added PDF tools entry (32 subagents, 21 scripts)Features
LibPDF Capabilities:
Helper Script Commands:
Testing
Related
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.