Stop a model's words being a program - #63
Merged
Merged
Conversation
"Close Spotify" ends in `pkill <app>`, and app is whatever the language model
produced from the sentence. pkill's argument is an extended regular
expression, not a literal, and nothing validated it.
Measured with pgrep, which matches the same way without killing anything:
pgrep '.' 479 of 480 processes — the whole session
pgrep 'pylo.' the running pylon daemon
pgrep -x '.*' 478 — so -x alone is not the fix
The self-kill guard made it worse than it looked: isSelf compares literals
while pkill matches patterns, so `pylo.` is not equal to "pylon", passes the
guard, and then matches it.
Three things together, because none of them is enough alone:
- isProcessName: letters, digits, dot, dash, underscore, at least one
alphanumeric, 64 characters. Removes the metacharacters that make the
argument a program rather than a name.
- -x, so the pattern must match the whole name.
- dots escaped, because the dot has to stay — mount.ntfs-3g and python3.11
are on an ordinary desktop — and it is the one metacharacter that survives
the name check. `pkill -x 'pylo\.'` matches nothing.
Also from the same review, all smaller:
- exchange put the model's currency and coin names into URLs unescaped. The
host is fixed so this reached no other server, but a stray ? or & would
quietly request a different endpoint of that one. url.PathEscape and
url.Values now.
- pylon-ui's OpenURL is bound to the frontend and opened any scheme it was
handed. file:// hands a path to the desktop's default handler for whatever
it turns out to be. http and https only, and the check is split out so it
is testable without a window.
- the PID file is 0600, not 0644. On Unix it sits in /tmp beside every other
user's files.
The socket itself — /tmp, no authentication, no ownership check by clients —
is the larger finding from that review and is deliberately not in here.
YCistak
force-pushed
the
fix/harden-inputs
branch
from
August 13, 2026 20:24
1a4a038 to
adc77f6
Compare
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.
"Close Spotify" ends in
pkill <app>, andappis whatever the language model produced from the sentence. pkill's argument is an extended regular expression, not a literal, and nothing validated it.Measured with
pgrep, which matches the same way without killing anything:.pylo.pylondaemon-xwith.*-xalone is not the fixThe self-kill guard made it worse than it looked:
isSelfcompares literals while pkill matches patterns, sopylo.is not equal to"pylon", passes the guard, and then matches it.The fix is three things, because none is enough alone
isProcessName— letters, digits, dot, dash, underscore; at least one alphanumeric; 64 characters. Removes the metacharacters that make the argument a program rather than a name.-x— the pattern must match the whole name.mount.ntfs-3gandpython3.11are on an ordinary desktop), and it is the one metacharacter that survives the name check.pkill -x 'pylo\.'matches nothing.Verified against a real process table:
pgrep -x 'pylo\.'→ 0,pgrep -x 'pylon'→ 1.Tests drive the refusals (
.,..,.*,^.*$,chrome|pylon,a b,code;rm,$(id),[a-z]+, over-long), the escaped-dot path, the self-guard evasions, and that real names still work — a guard that breaks the feature is not a fix.Also from the same review, all smaller
?or&would quietly request a different endpoint of that one.url.PathEscapeandurl.Valuesnow.OpenURLis bound to the frontend and opened any scheme it was handed.file://hands a path to the desktop's default handler for whatever it turns out to be. http and https only; the check is split out so it is testable without a window./tmpbeside every other user's files.Deliberately not in here
The socket:
/tmp/pylon.sock, no authentication, and no client ever checks who owns it. On a shared machine another local user can bind that name first and receivepylon secret set <name> <api-key>in plaintext. It is the larger finding and needs its own change — the socket path is duplicated inpylon-uiby design, and moving it touches both modules.🤖 Generated with Claude Code