Screenshot #1481
Screenshot #1481
Conversation
📝 WalkthroughWalkthroughAdds session-aware image handling across editor and Tauri plugins: Editor now optionally receives a sessionId and initializes extensions via createExtensions(sessionId). Tiptap extensions integrate @tiptap/extension-file-handler and @hypr/plugin-misc for image drop/paste upload and deletion. Tauri misc plugin exposes image_upload and image_delete commands and registers them. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Editor
participant TiptapExt as Tiptap FileHandler
participant MiscCmd as @hypr/plugin-misc (JS)
participant Tauri as Tauri Runtime
participant MiscRS as misc::commands (Rust)
participant FS as App Data FS
rect rgb(245,248,255)
note over Editor,TiptapExt: Initialization (session-aware)
User->>Editor: Open editor (with sessionId)
Editor->>TiptapExt: init createExtensions(sessionId)
end
rect rgb(240,255,245)
note over User,TiptapExt: Image paste/drag-drop
User-->>Editor: Paste/Drop image file
Editor->>TiptapExt: onPaste/onDrop(file, sessionId)
TiptapExt->>MiscCmd: imageUpload(sessionId, bytes, ext)
MiscCmd->>Tauri: invoke("image_upload", payload)
Tauri->>MiscRS: image_upload(session_id, data, ext)
MiscRS->>FS: mkdirs/write image
MiscRS-->>Tauri: tauri://localhost/.../images/<session>/<uuid>.<ext>
Tauri-->>MiscCmd: URL
MiscCmd-->>TiptapExt: URL
TiptapExt->>Editor: insert image node with src=URL
end
rect rgb(255,245,245)
note over Editor,MiscRS: Deletion (when invoked)
Editor->>MiscCmd: imageDelete(sessionId, filename)
MiscCmd->>Tauri: invoke("image_delete")
Tauri->>MiscRS: image_delete(session_id, filename)
MiscRS->>FS: remove file if exists
MiscRS-->>Editor: Ok / Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/tiptap/src/shared/extensions.ts (1)
19-23: Normalize 'jpg' to 'jpeg' to match common encoders.Safer for downstream when server expects “jpeg”.
-const getFileExtension = (filename: string): string => { - const ext = filename.split('.').pop()?.toLowerCase(); - return ext || 'png'; -}; +const getFileExtension = (filename: string): string => { + const ext = filename.split('.').pop()?.toLowerCase(); + return ext === 'jpg' ? 'jpeg' : (ext || 'png'); +};
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
Cargo.lockis excluded by!**/*.lockplugins/misc/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/misc/permissions/autogenerated/commands/image_delete.tomlis excluded by!plugins/**/permissions/**plugins/misc/permissions/autogenerated/commands/image_upload.tomlis excluded by!plugins/**/permissions/**plugins/misc/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/misc/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/misc/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/desktop/src/components/editor-area/index.tsx(2 hunks)packages/tiptap/package.json(2 hunks)packages/tiptap/src/editor/index.tsx(2 hunks)packages/tiptap/src/shared/extensions.ts(2 hunks)plugins/listener/Cargo.toml(1 hunks)plugins/misc/Cargo.toml(1 hunks)plugins/misc/build.rs(1 hunks)plugins/misc/src/commands.rs(1 hunks)plugins/misc/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/misc/src/lib.rsplugins/misc/src/commands.rspackages/tiptap/src/shared/extensions.tspackages/tiptap/src/editor/index.tsxapps/desktop/src/components/editor-area/index.tsxplugins/misc/build.rs
🧬 Code graph analysis (2)
plugins/misc/src/lib.rs (1)
plugins/misc/src/commands.rs (2)
image_upload(104-128)image_delete(132-145)
plugins/misc/src/commands.rs (1)
owhisper/owhisper-config/src/lib.rs (1)
data_dir(52-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (8)
plugins/misc/Cargo.toml (1)
30-30: UUID dep is appropriate for file naming.Matches usage in image uploads; minimal feature set. LGTM.
plugins/misc/build.rs (1)
10-11: Command IDs wired into build script correctly.Matches the new commands; consistent with the pattern used for other entries.
plugins/misc/src/lib.rs (1)
19-21: Commands registered with Specta as expected.Keeps plugin surface consistent with build.rs.
apps/desktop/src/components/editor-area/index.tsx (1)
308-309: PassingsessionIdto Editor is correct.Matches the new Editor prop and enables session‑scoped extensions.
plugins/listener/Cargo.toml (1)
21-21: Keep hypr-aec — it's used. Found usages in plugins/listener/src/actors/source.rs:.chunks(hypr_aec::BLOCK_SIZE)at lines 188, 232, 238.plugins/misc/src/commands.rs (1)
102-129: Guard against path traversal & unsafe extensions — plugins/misc/src/commands.rs:102-129session_id and extension are embedded directly into paths/filenames; validate both and limit uploads.
- Validate session_id server-side as a UUID (e.g., uuid::Uuid::parse_str(&session_id)) and reject anything that contains separators or parent‑dir components.
- Do not trust the client-provided extension: sniff file type from content (MIME/image magic) and map to a small whitelist {png,jpg,jpeg,webp,gif}; ensure the stored extension contains only [a‑z0‑9], no '.' or '/' characters.
- Prevent path traversal: construct images_dir from the canonical app data dir + validated session_id; after computing file_path, assert it is contained within images_dir (e.g., compare canonicalized prefixes or inspect Path components for ParentDir).
- Enforce upload size limits (e.g., reject > 5 MiB) and prefer streaming writes rather than holding large Vec in memory.
- Use restrictive file permissions and URL-encode filenames in the returned tauri:// URL.
I could not verify where session_id/extension originate in the repo — confirm their generator/source and ensure server-side checks are the enforcement point.
packages/tiptap/src/editor/index.tsx (1)
18-19: Session‑aware extensions — remount verified (no action required).Editor uses createExtensions(sessionId). EditorArea remounts via key including sessionId (apps/desktop/src/components/editor-area/index.tsx); chat input’s Editor does not pass sessionId (apps/desktop/src/components/right-panel/components/chat/chat-input.tsx) and is unaffected.
packages/tiptap/src/shared/extensions.ts (1)
170-171: Back-compat export does NOT exclude FileHandler — createExtensions('') registers it.createExtensions('') includes FileHandler.configure(...) which calls miscCommands.imageUpload(sessionId,...); exporting extensions = createExtensions('') means non-session flows will attempt uploads with an empty sessionId. Gate FileHandler when sessionId is falsy or ensure miscCommands.imageUpload handles an empty sessionId.
- Locations: packages/tiptap/src/shared/extensions.ts (FileHandler.configure), packages/tiptap/src/editor/index.tsx:35 (uses shared.extensions when no sessionId), packages/tiptap/src/renderer/index.tsx:16 (uses shared.extensions in renderer).
Likely an incorrect or invalid review comment.
| const handleImagePaste = useCallback(async (file: File) => { | ||
| // TODO: Handle pasted image file | ||
| // - Upload the file to storage | ||
| // - Get back image URL | ||
| // - Insert into editor at cursor position | ||
| console.log('Image pasted:', file.name, file.size); | ||
| }, []); | ||
|
|
||
| const handleImageCopy = useCallback(async (imageUrl: string) => { | ||
| // TODO: Handle copying image to clipboard | ||
| // - Fetch image as blob | ||
| // - Copy to clipboard | ||
| console.log('Image copy requested:', imageUrl); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
Remove unused callbacks to meet repo guidelines.
handleImagePaste and handleImageCopy are unused; violates “no unused functions.”
Apply this diff:
- const handleImagePaste = useCallback(async (file: File) => {
- // TODO: Handle pasted image file
- // - Upload the file to storage
- // - Get back image URL
- // - Insert into editor at cursor position
- console.log('Image pasted:', file.name, file.size);
- }, []);
-
- const handleImageCopy = useCallback(async (imageUrl: string) => {
- // TODO: Handle copying image to clipboard
- // - Fetch image as blob
- // - Copy to clipboard
- console.log('Image copy requested:', imageUrl);
- }, []);
+ // Intentionally omitted: image paste/copy handlers will be wired where used.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleImagePaste = useCallback(async (file: File) => { | |
| // TODO: Handle pasted image file | |
| // - Upload the file to storage | |
| // - Get back image URL | |
| // - Insert into editor at cursor position | |
| console.log('Image pasted:', file.name, file.size); | |
| }, []); | |
| const handleImageCopy = useCallback(async (imageUrl: string) => { | |
| // TODO: Handle copying image to clipboard | |
| // - Fetch image as blob | |
| // - Copy to clipboard | |
| console.log('Image copy requested:', imageUrl); | |
| }, []); | |
| // Intentionally omitted: image paste/copy handlers will be wired where used. |
🤖 Prompt for AI Agents
In apps/desktop/src/components/editor-area/index.tsx around lines 256 to 270,
the two unused callbacks handleImagePaste and handleImageCopy should be removed
to satisfy the “no unused functions” guideline; delete their entire useCallback
declarations (including TODO comments and console.log) and ensure there are no
remaining references to these identifiers elsewhere in the file, or replace any
future usages with a single exported/implemented utility if needed.
| "@hypr/plugin-misc": "workspace:^", | ||
| "@hypr/ui": "workspace:^", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
New deps OK; keep TipTap versions aligned.
Both additions make sense for image handling. Ensure the lockfile resolves all @tiptap/* to the same 3.4.x to avoid duplicate installs.
Run to inspect @tiptap versions across the workspace:
Also applies to: 28-29
🏁 Script executed:
#!/bin/bash
fd -a package.json -E node_modules -E dist -E build -E out \
| xargs -I{} sh -c 'jq -r "\"\(.name)//deps\" as \$p | (.dependencies//{} + .devDependencies//{} + .peerDependencies//{}) | to_entries[] | select(.key|startswith(\"@tiptap/\")) | "\$p:\(.key)@\(.value)\"" "{}"' \
| sort -uLength of output: 1187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find package.json files excluding common build dirs
mapfile -t files < <(find . -type f -name package.json -not -path "*/node_modules/*" -not -path "*/dist/*" -not -path "*/build/*" -not -path "*/out/*")
if [ ${#files[@]} -eq 0 ]; then
echo "NO_PACKAGE_JSON_FOUND"
exit 0
fi
for f in "${files[@]}"; do
name=$(jq -r '.name // empty' "$f" 2>/dev/null || true)
if [ -z "$name" ]; then name="$f"; fi
jq -r '(.dependencies // {} + .devDependencies // {} + .peerDependencies // {} + .optionalDependencies // {}) | to_entries[] | select(.key | startswith("@tiptap/")) | "\(.key)@\(.value)"' "$f" 2>/dev/null \
| awk -v p="$f" -v n="$name" '{print p " // " n ":" $0}'
done | sort -u || trueLength of output: 56
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if ! command -v jq >/dev/null 2>&1; then
echo "MISSING_JQ"
exit 0
fi
echo "---- PACKAGE.JSON SCAN ----"
find . -type f -name package.json -not -path "*/node_modules/*" -not -path "*/dist/*" -not -path "*/build/*" -not -path "*/out/*" -print0 \
| while IFS= read -r -d '' f; do
name=$(jq -r '.name // empty' "$f" 2>/dev/null || true)
if [ -z "$name" ]; then name="$f"; fi
jq -r '(.dependencies // {} + .devDependencies // {} + .peerDependencies // {} + .optionalDependencies // {})
| to_entries[]
| select(.key | test("^@tiptap/"))
| "\(.key)@\(.value)"' "$f" 2>/dev/null \
| awk -v f="$f" -v n="$name" '{print f " // " n ":" $0}'
done | sort -u || true
echo "---- LOCKFILE SCAN ----"
find . -type f \( -name "pnpm-lock.yaml" -o -name "yarn.lock" -o -name "package-lock.json" -o -name "npm-shrinkwrap.json" \) -not -path "*/node_modules/*" -print0 \
| while IFS= read -r -d '' lf; do
echo "LOCKFILE: $lf"
if grep -n --color=never -E "@tiptap/" "$lf" >/dev/null 2>&1; then
grep -n -E "@tiptap/" "$lf" | sed -n '1,200p'
else
echo "NO_TIPTAP_FOUND_IN_LOCKFILE"
fi
done || trueLength of output: 13537
New deps OK; align @tiptap versions in lockfile.
@hypr/plugin-misc and @hypr/ui additions are fine for image handling. packages/tiptap/package.json pulls @tiptap/extension-file-handler@^3.4.2, but pnpm-lock.yaml shows most @tiptap/* resolved to 3.4.1 while extension-file-handler (and extension-text-style transitively) resolved to 3.4.2. Align all @tiptap/* to a single 3.4.x (either upgrade everything to 3.4.2 or pin file-handler/text-style to 3.4.1) and regenerate the lockfile to avoid duplicate installs.
| import StarterKit from "@tiptap/starter-kit"; | ||
| import FileHandler from "@tiptap/extension-file-handler"; | ||
|
|
||
| import { commands as miscCommands } from "@hypr/plugin-misc"; |
There was a problem hiding this comment.
Gate FileHandler behind sessionId and avoid bundling Tauri in web/SSR; drop PII-y debug logs.
- Prevents calling imageUpload with empty sessionId.
- Prevents web/SSR breakage from statically importing @hypr/plugin-misc.
- Removes logs that can leak full note contents.
Apply this diff:
-import { commands as miscCommands } from "@hypr/plugin-misc";
@@
ClipboardTextSerializer,
- FileHandler.configure({
+ ...(sessionId ? [
+ FileHandler.configure({
allowedMimeTypes: ['image/png', 'image/jpeg', 'image/gif', 'image/webp'],
onDrop: async (currentEditor, files, pos) => {
- console.log("onDrop", files, pos)
for (const file of files) {
try {
// Convert file to bytes
const arrayBuffer = await file.arrayBuffer()
const bytes = new Uint8Array(arrayBuffer)
const extension = getFileExtension(file.name)
- // Upload via Tauri command
- const imageUrl = await miscCommands.imageUpload(sessionId, Array.from(bytes), extension)
- console.log("full note html: ", currentEditor.getHTML())
+ const { commands: miscCommands } = await import("@hypr/plugin-misc")
+ const imageUrl = await miscCommands.imageUpload(sessionId, Array.from(bytes), extension)
// Insert URL (not base64!)
currentEditor
.chain()
.insertContentAt(pos, {
type: 'image',
attrs: {
src: imageUrl,
},
})
.focus()
.run()
} catch (error) {
console.error('Failed to upload image:', error)
}
}
},
onPaste: async (currentEditor, files, htmlContent) => {
for (const file of files) {
- console.log("onPaste", files, htmlContent)
if (htmlContent) {
- // if there is htmlContent, stop manual insertion & let other extensions handle insertion via inputRule
- console.log(htmlContent) // eslint-disable-line no-console
return false
}
try {
// Convert file to bytes
const arrayBuffer = await file.arrayBuffer()
const bytes = new Uint8Array(arrayBuffer)
const extension = getFileExtension(file.name)
- // Upload via Tauri command
- const imageUrl = await miscCommands.imageUpload(sessionId, Array.from(bytes), extension)
- console.log("full note html: ", currentEditor.getHTML())
+ const { commands: miscCommands } = await import("@hypr/plugin-misc")
+ const imageUrl = await miscCommands.imageUpload(sessionId, Array.from(bytes), extension)
// Insert URL (not base64!)
currentEditor
.chain()
.insertContentAt(currentEditor.state.selection.anchor, {
type: 'image',
attrs: {
src: imageUrl,
},
})
.focus()
.run()
} catch (error) {
console.error('Failed to upload image:', error)
}
}
},
- }),
+ }),
+ ] : []),Also applies to: 101-167
🤖 Prompt for AI Agents
In packages/tiptap/src/shared/extensions.ts around lines 11 and 101-167, avoid
statically importing @hypr/plugin-misc and remove PII debug logging: change the
top-level import to a dynamic/conditional import used only when a valid
sessionId/environment indicates native (Tauri) context (i.e., do not import on
web/SSR), guard any FileHandler/imageUpload calls so they only run when
sessionId is non-empty/defined, and strip or replace any logs that print full
note contents or other PII with minimal, non-sensitive messages; ensure error
handling still reports errors (without leaking content) and adjust types/returns
accordingly.
| #[tauri::command] | ||
| #[specta::specta] | ||
| pub async fn image_delete<R: tauri::Runtime>( | ||
| app: tauri::AppHandle<R>, | ||
| session_id: String, | ||
| image_filename: String, | ||
| ) -> Result<(), String> { | ||
| let data_dir = app.path().app_data_dir().map_err(|e| e.to_string())?; | ||
| let image_path = data_dir.join(session_id).join("images").join(image_filename); | ||
|
|
||
| if image_path.exists() { | ||
| std::fs::remove_file(image_path).map_err(|e| e.to_string())?; | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Same traversal concern for deletions.
image_filename should be a bare filename from your own upload flow; if not, .. or separators could delete outside the images dir. Ensure upstream constraints.
🤖 Prompt for AI Agents
In plugins/misc/src/commands.rs around lines 130 to 145, the current
image_delete accepts image_filename directly and can be exploited with path
traversal (e.g. "../" or path separators) to delete files outside the images
directory; fix by validating/sanitizing the filename before joining: require a
bare filename (no path separators or ".."), use the
Path::new(image_filename).file_name() to get a single OS filename and reject if
None or differs from the input, and additionally reject any names containing '/'
or '\\' or starting with '.'; optionally compute the resulting path and
canonicalize it and assert it starts_with the expected images directory before
calling remove_file, returning an error string on invalid input.
|
Thanks for working on this! I dont have a perspective on how to handle the image media (i.e. pull into internal Hyprnote storage or just treat as a foreign Markdown link or something else) but am happy to help test for usability. Thanks! |
3 participants
DON'T MERGE THIS YET, INCOMPLETE