Repository navigation
Conversation
On Windows, `emdash login` and `emdash-plugin login` opened the browser with `cmd /c start "" <url>`. cmd.exe re-parses its command line, so the server-supplied URL was read as cmd syntax: `emdash-plugin login` opened its authorization URL cut at the first `&` (its OAuth library puts `request_uri` after `client_id`), and `&`, `|` or `%NAME%` in a URL from the server could run further commands or put environment variables into the address. Each CLI now resolves the opener through a small helper of its own (the two packages do not depend on each other). The helper opens only http(s) URLs, passes the re-serialised URL, and on Windows calls `rundll32 url.dll,FileProtocolHandler` with the URL as one unquoted argument. macOS and Linux keep `open` and `xdg-open`. Unit tests cover the command chosen per platform, URLs that carry `&`, `|` and `%`, re-serialisation, and the refused inputs.
🦋 Changeset detectedLatest commit: f583619 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The approach is sound. Replacing cmd /c start with rundll32 url.dll,FileProtocolHandler removes cmd.exe from the path. That fixes the Windows truncation of OAuth URLs at the first & and closes the shell-injection and %VAR% expansion risk. Restricting opens to http(s) and passing the re-serialised URL add defence in depth. Both reviewers approved with no findings. They checked both CLI call sites, URL validation and serialisation, per-platform arguments, the tests and the changeset. Both reviews were static: neither ran the code on Windows. The unit tests cover command construction, not actual browser delivery.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
upgrade-emdash
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-ai-search
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
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 does this PR do?
On Windows,
emdash loginandemdash-plugin loginopened the login URL withcmd /c start "" <url>, andcmd.exere-parses that URL as command syntax:emdash-plugin loginopened the authorization page withoutrequest_uri, because the URL (…?client_id=…&request_uri=…) was cut at the first&. Login only worked by copying the printed URL by hand.emdash-plugin pds-conformanceuses the same flow.emdash login), so&,|or%NAME%in it could run further commands or put environment variables into the address.Both CLIs now use a small
browserOpenCommandhelper, duplicated because the two packages don't depend on each other. It opens onlyhttp:andhttps:URLs, passes the re-serialised URL, refuses a host containing"(Node would quote the argument), and on Windows runsrundll32 url.dll,FileProtocolHandler <url>, which passes the URL as one argument without a command interpreter. macOS and Linux keepopenandxdg-open.I found this while reviewing and testing the EmDash CLI login on Windows.
Type of change
Your connection to this change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been run (oxfmt on the changed files)messages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. (n/a: CLI only)AI assistance disclosure
Screenshots / test output
Not applicable, no UI change.
Tested on Windows 11 (Node 24.22.2, Chrome 157) with both CLIs built from this branch, plus the separate
build-ui-types.tsfix that the Windows build needs:emdash-plugin loginagainst a local stand-in for the account's PDS and authorization server: with the old opener the browser got the authorization URL withoutrequest_uri; with this change it got the complete URL, and the stand-in's redirect reached the CLI's loopback callback.emdash loginagainst a local stand-in instance whoseverification_uricontains&,|,^and%OS%: the browser got exactly that URL.%OS%, cut the URL at&, and cmd.exe tried to run the rest as a command.packages/plugin-cli, 5 inpackages/core) cover the command per platform, URLs with&,|and%, re-serialisation and refused inputs. They pass, and so do the core CLI tests. On Windows, 7 init and release-prepare tests in the full@emdash-cms/plugin-clisuite fail; they don't reach the browser opener.