Harden open-file: protocol allowlist + safe spawn for URLs#72
Merged
Conversation
Split openFile into two exports: openFile (local paths, unchanged) and openUrl (external URLs, http(s)-only, spawned with no shell). openUrl parses via new URL() and rejects anything non-http(s) so a model-emitted file:///, data:, or javascript: URL can never reach a local handler. URLs are forwarded via child_process.spawn with an argv array, so shell metacharacters (; & ` $ | < >) in query strings cannot be interpreted as commands. Platform openers: open on darwin, explorer.exe on win32 (not cmd /c start), xdg-open on Linux/BSDs, no-op everywhere else. A no-op 'error' listener is attached before unref() so ENOENT from a missing xdg-open doesn't crash the TUI as an unhandled EventEmitter event.
Merged
Contributor
|
🎉 This PR is included in version 1.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
What
Splits
src/utils/open-file.tsinto two entry points:openFile(path)— local file paths via theopenpackage. Unchanged; all existing consumers (FileLink, ChafaImage, MediaChip, profile dialog) keep using it.openUrl(raw)— new. External URLs only. Parses vianew URL()and rejects anything whose protocol is nothttp:/https:, so a model-emittedfile:///,data:, orjavascript:URL can never reach a local handler. Spawns the platform opener with an argv array (no shell), so metacharacters in query strings cannot be interpreted as commands.openon darwin,explorer.exeon win32 (notcmd /c start),xdg-openon Linux/BSDs, no-op elsewhere. A no-operrorlistener is attached beforeunref()so ENOENT from a missing opener doesn't crash the TUI.Why now
This is the sink for the upcoming clickable-URL tokenizer in chat messages.
openUrlhas no consumers yet in this PR — the tokenizer lands separately and depends on this being merged first.Tests
test/open-file.test.ts— 19 cases: protocol allowlist (accepted + every rejected scheme), per-platform argv dispatch with spawn mocked, sync-throw and async-ENOENT error paths, hostname edge cases.tsc clean, 1001/1001 pass.