fix(security): enforce canonical path containment on renderer-supplied meeting paths#329
Open
Optic00 wants to merge 1 commit into
Open
fix(security): enforce canonical path containment on renderer-supplied meeting paths#329Optic00 wants to merge 1 commit into
Optic00 wants to merge 1 commit into
Conversation
…d meeting paths The Electron renderer is an untrusted trust boundary (XSS / compromised renderer / DevTools). Several IPC handlers passed renderer-supplied file paths straight to the Python backend or to fs without symlink-safe containment, enabling arbitrary local file read/write/delete: - query-transcript / query-transcript-stream: arbitrary file READ (and exfiltration via cloud/org providers) - the backend's query command reads any UTF-8 file, with no extension gate on the plain-text path. - add-/remove-meeting-to-folder: arbitrary file WRITE via the folders backend. - update-meeting: symlink escape - used the lexical validateSafeFilePath then read/wrote the original (symlink-followed) path, over the whole project root. - delete-meeting / reveal / system-audio: validateSafeFilePath was lexical-only, so a symlink inside an allowed dir escaped it. Route the four meeting-file handlers through validateMeetingFilePath() (output/-only, realpath-canonicalized) and pass the canonical realPath downstream. Harden validateSafeFilePath() with realpathSync canonicalization of both target and base dirs, covering delete/reveal/system-audio with no call-site change. The Python CLI intentionally still accepts arbitrary paths (the documented `query transcript.txt` debug command); the boundary is enforced in Electron. Also harden the stream handler that this change makes async: guard against a query-cancel or sender-destroy landing during the pre-spawn validation (cancel-race / orphaned backend), type-guard non-string input at the validateMeetingFilePath source (avoids an unhandled rejection), and make query-done delivery exception-safe. Adds a model-free T2 e2e spec asserting rejection of out-of-output/ paths, non-string input, and symlink escapes on both validator seams.
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
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.
Summary
Closes several path-traversal issues where the untrusted Electron renderer (XSS / compromised renderer / DevTools) could drive the backend or the main process into reading, writing, or deleting arbitrary local files. Enforcement is at the Electron trust boundary; the Python CLI stays intentionally permissive so the documented
query transcript.txtdebug path keeps working.Issues fixed
query-transcript/query-transcript-streampassed the renderer'ssummaryFilestraight to the backendquerycommand, which reads any UTF-8 file (no extension gate on the plain-text path). With a cloud/org provider the contents are also sent to the external AI service.add-/remove-meeting-to-folderpassedsummaryFilestraight to the folders backend, which rewrites the target file.update-meetingused a lexical-only path check, then read/wrote the original (symlink-followed) path across the whole project/storage root.delete-meeting/reveal/ system-audio shared the same lexical check.Fix
validateMeetingFilePath()(output/-only, realpath-canonicalized) and pass the canonicalrealPathdownstream — never the original renderer string.validateSafeFilePath()withrealpathSynccanonicalization of both target and base dirs, covering delete/reveal/system-audio with no call-site change.query-cancelor sender-destroy landing during pre-spawn validation (cancel-race / orphaned backend), type-guard non-string input at thevalidateMeetingFilePathsource (avoids an unhandled rejection that could crash main), and makequery-donedelivery exception-safe. Thechat_message_sentanalytics from feat: instrument product analytics for activation, engagement and usage #323 is preserved.Tests
New model-free T2 e2e spec
path-validation-security.t2.spec.ts: asserts rejection of out-of-output/paths, non-string input, and symlink escapes on both validator seams, and that the out-of-bounds file is never read / modified / deleted.meetings-crud/folders-crud/chat-sessionsT2 regressions green.Summary by cubic
Enforces symlink-safe, canonical path containment for renderer-supplied meeting files to block arbitrary local file read/write/delete from the Electron renderer. Validation now happens at the Electron trust boundary; the Python CLI remains permissive for documented debug flows.
query-transcript,query-transcript-stream,update-meeting, andadd/remove-meeting-to-folderwithvalidateMeetingFilePath(output/ only) and pass the canonicalrealPathdownstream.validateSafeFilePathusingfs.realpathSync, securing delete/reveal/system-audio without call-site changes.query-transcript-stream, add pending-cancel handling to avoid orphaned backends, and makequery-donedelivery exception-safe.output/paths, non-string input, and symlink escapes; asserts no unauthorized disk reads/writes/deletes.Written for commit 541654f. Summary will update on new commits.