You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
hardens OpenCode HTTP client URLs to loopback-only before health, request, and event calls, including bracket-safe IPv6 loopback handling
removes direct POSIX shell usage from spawn call sites via a shared platform option helper and resolves opencode before spawning server/version commands
replaces weak random IDs/temp patch suffixes with crypto.randomBytes
removes dynamic object dispatch for companion subcommands and places dispatch after handler declarations
rewrites safe-command stdin/flag handling through async stdin reads and allowlist sets
narrows Codacy Opengrep-only exclusions to reviewed noisy companion files and one isolated process test fixture while leaving ESLint, Biome, SonarCloud, and tests active
adds regression coverage for Windows shell command-path quoting and Git Bash path-only opencode resolution
Verification
npm test passed locally: 222 tests, 0 failures
node --check passed for the touched process module and test file
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR focuses on critical security hardening, including loopback-only constraints for HTTP traffic, shell injection prevention, and cryptographic randomness. While the PR addresses 39 existing issues, Codacy reports that the PR is currently not up to standards due to 17 new issues, including high-severity linting violations.
A primary concern is the lack of unit tests for the newly introduced security-critical validation logic (e.g., hostname checks, allowlists). Additionally, the absence of a coverage report makes it impossible to verify if these paths are exercised. Several implementation gaps were identified, such as incorrect IPv6 URL construction and potential information leaks through filesystem metadata access.
About this PR
The PR lacks new unit tests to verify critical security validation logic introduced, such as rejecting non-loopback hosts or invalid command flags. Furthermore, the empty coverage report prevents verification that these new security paths are covered by existing tests.
1 comment outside of the diffplugins/opencode/scripts/opencode-companion.mjs
line 143 🔴 HIGH RISK
Initialize reviewGateMaxPerSessionOverride with a default value to avoid it evaluating to undefined. Since the project's linting rules discourage both null and undefined, consider using a numeric sentinel (like 0 or -1).
Test suggestions
Missing: Verify that non-loopback hosts are rejected by normalizeLoopbackHost and assertLoopbackUrl in opencode-server.mjs.
Missing: Verify that safe-command.mjs rejects unknown flags and invalid values for --agent and --model (e.g., path traversal attempts).
Missing: Ensure withPlatformShell correctly configures the shell and windowsHide options across Windows and POSIX platforms.
Missing: Confirm that generateJobId and makeWorktreeId produce unique, random identifiers using crypto.randomBytes.
Missing: Automated test coverage for new security-critical code paths.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing: Verify that non-loopback hosts are rejected by normalizeLoopbackHost and assertLoopbackUrl in opencode-server.mjs.
2. Missing: Verify that safe-command.mjs rejects unknown flags and invalid values for --agent and --model (e.g., path traversal attempts).
3. Missing: Ensure withPlatformShell correctly configures the shell and windowsHide options across Windows and POSIX platforms.
4. Missing: Confirm that generateJobId and makeWorktreeId produce unique, random identifiers using crypto.randomBytes.
5. Missing: Automated test coverage for new security-critical code paths.
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
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.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
JohnnyVicious
force-pushed
the
fix/codacy-critical-72
branch
2 times, most recently
from
Summary
Closes #72.
opencodebefore spawning server/version commandscrypto.randomBytesopencoderesolutionVerification
npm testpassed locally: 222 tests, 0 failuresnode --checkpassed for the touched process module and test filegit diff --checkpassed0new issues on head3e970ed0new issues and21fixed issues on head3e970ed