fix(paragraph-cli): prevent windows command injection and map base url configuration - #4
Open
magqqgq wants to merge 1 commit into
Open
fix(paragraph-cli): prevent windows command injection and map base url configuration#4magqqgq wants to merge 1 commit into
magqqgq wants to merge 1 commit into
Conversation
…l configuration ### Description This PR addresses a high-severity command injection vulnerability in the CLI's browser authentication flow and resolves a medium-severity configuration mismatch[cite: 40]. The update ensures that server-provided login URLs are handled securely on Windows and that custom API endpoint overrides are correctly mapped to the underlying SDK[cite: 40]. ### Key Changes * **Command Injection Prevention (`src/services/browser-auth.ts`):** - Removed the unsafe `cmd.exe /c start` shell dispatch, which previously allowed shell metacharacters and environment expansion in untrusted URLs to cross the CLI's process-launch boundary[cite: 40]. - The `openBrowser` function now explicitly validates `http:` and `https:` URL schemes and passes the normalized URL directly to `explorer.exe` using `execFile`, safely avoiding the Windows shell[cite: 40]. * **Configuration Alignment (`src/services/client.ts`):** - Mapped the documented `PARAGRAPH_API_URL` environment variable directly to the SDK's newly exposed `baseURL` option, ensuring the CLI reliably overrides the hard-coded API endpoint[cite: 40]. * **Regression Testing (`test/browser-auth.test.ts`):** - Added dedicated regression tests to verify non-HTTP URL scheme rejection and validate the secure `execFile` launcher invocation[cite: 40]. ### Validation & Testing * **Static Verification:** TypeScript syntax parsing (`node --experimental-strip-types --check`) passed successfully for the modified source and test files[cite: 40]. * **Reviewer Action Required:** The native CLI build (`npm run build`) and regression suite (`npm test`) were blocked during the offline audit because `tsup` and `vitest` dependencies were absent in the workspace[cite: 40]. A maintainer must install dependencies and execute the full test suite locally before merging[cite: 40].
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
1 participant
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.
Description
This PR addresses a high-severity command injection vulnerability in the CLI's browser authentication flow and resolves a medium-severity configuration mismatch[cite: 40]. The update ensures that server-provided login URLs are handled securely on Windows and that custom API endpoint overrides are correctly mapped to the underlying SDK[cite: 40].
Key Changes
src/services/browser-auth.ts):cmd.exe /c startshell dispatch, which previously allowed shell metacharacters and environment expansion in untrusted URLs to cross the CLI's process-launch boundary[cite: 40].openBrowserfunction now explicitly validateshttp:andhttps:URL schemes and passes the normalized URL directly toexplorer.exeusingexecFile, safely avoiding the Windows shell[cite: 40].src/services/client.ts):PARAGRAPH_API_URLenvironment variable directly to the SDK's newly exposedbaseURLoption, ensuring the CLI reliably overrides the hard-coded API endpoint[cite: 40].test/browser-auth.test.ts):execFilelauncher invocation[cite: 40].Validation & Testing
node --experimental-strip-types --check) passed successfully for the modified source and test files[cite: 40].npm run build) and regression suite (npm test) were blocked during the offline audit becausetsupandvitestdependencies were absent in the workspace[cite: 40]. A maintainer must install dependencies and execute the full test suite locally before merging[cite: 40].