[WRONG BRANCH] fix(cli): shell-quote hub invite arguments - #489
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Its title has been prefixed with |
github-actions
Bot
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85416af59b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ): string { | ||
| const clientsFlag = clients.length > 0 ? ` --clients ${clients.join(",")}` : ""; | ||
| return `echo '${code}' | ocx connect ${dataUrl} --management-url ${managementUrl}${clientsFlag} --pairing-code-stdin`; | ||
| const shellQuote = (value: string): string => `'${value.replaceAll("'", `'"'"'`)}'`; |
There was a problem hiding this comment.
When the joining machine uses cmd.exe, single quotes do not protect shell metacharacters, so an accepted origin such as https://x&whoami&x causes the pasted invite line to execute whoami as a separate command. This can occur even when the hub itself runs Unix because the generated command is explicitly intended for another machine; the repository's existing CMD escaper confirms that & and | require escaping in src/update/npm-invocation.mjs:4-8. Emit or label a POSIX-only command and provide safe PowerShell/CMD variants rather than treating this quoting as shell-independent.
AGENTS.md reference: src/AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| const clientsFlag = clients.length > 0 ? ` --clients ${shellQuote(clients.join(","))}` : ""; | ||
| return `echo ${shellQuote(code)} | ocx connect ${shellQuote(dataUrl)} --management-url ${shellQuote(managementUrl)}${clientsFlag} --pairing-code-stdin`; |
There was a problem hiding this comment.
The emitted and JSON-envelope command now quotes every URL and client argument, but docs-site/src/content/docs/guides/remote-hub.md:328-331 and :384-389 still claim to show the exact printed line while using the old unquoted form, and the Korean translation retains it as well. Update the English example and translated copies so users are not taught the unsafe command shape this change removes.
AGENTS.md reference: src/AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
1 participant
Motivation
ocx hub inviteprints a copy‑and‑run shell command that interpolates operator-supplied origins and client lists.hubInviteCommandreturned WHATWG‑canonicalized origins verbatim, which permitted shell expansion (command substitution, brace expansion, etc.) when a user pasted the line into a shell.Description
src/cli/hub.tsand apply it to every dynamic value inhubInviteCommandso the pairing code, data URL, management URL, and--clientspayload are emitted as safely quoted shell arguments.tests/cli/hub-invite.test.tsto expect quoted arguments and add a focused regression asserting quoting for embedded single quotes, command‑substitution characters, and environment-style expansions.Testing
bun test tests/cli/hub-invite.test.tsand all 23 tests passed.bun run typecheckandbun run privacy:scan, both succeeded.bun run test; the focused hub-invite coverage remained green but the full parallel suite encountered unrelated worker panics in this container (existing unrelated test failures), so only the focused tests and CI‑required checks were relied on for verification.Codex Task