v1.0.10 - #40
Conversation
axios adjustment and folder option for wf update command
Reviewer's GuideAdds a new --folder/-d mode to Sequence diagram for wf update --folder feature resolution and processingsequenceDiagram
actor User
participant workflow_cli
participant updateCommand
participant resolveFeatureFolders
participant listFeatureFolders
participant glob
participant publishComponent
User->>workflow_cli: wf update --folder name
workflow_cli->>updateCommand: updateCommand(options)
updateCommand->>resolveFeatureFolders: resolveFeatureFolders(projectRoot, options.folder)
resolveFeatureFolders-->>updateCommand: folderDirs
alt folderDirs is empty
updateCommand->>listFeatureFolders: listFeatureFolders(projectRoot)
listFeatureFolders-->>updateCommand: availableNames
updateCommand->>User: print error and available folders
else folderDirs found
updateCommand->>glob: glob(toGlobPattern(dir, **/*.csx))
glob-->>updateCommand: csxFiles
updateCommand->>glob: glob(toGlobPattern(dir, **/*.json))
glob-->>updateCommand: jsonFiles
loop for each jsonFile
updateCommand->>publishComponent: publishComponent(baseUrl, componentData)
publishComponent-->>updateCommand: result
end
updateCommand-->>User: summary of updates
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new --folder (-d) option to the update command, allowing users to update all components under a specific feature folder across all component types, independent of Git status. It also refactors the API client in src/lib/api.js to use a custom Axios instance with persistent connections. However, several issues were identified in the review: a critical ReferenceError was introduced in src/lib/api.js due to the removal of the USER_AGENT variable while it is still in use, and there is a security concern regarding the hardcoded disabling of TLS verification. Additionally, it is recommended to add error handling around the new globbing operations in src/commands/update.js and defensive parameter validation in resolveFeatureFolders to prevent runtime crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const https = require('node:https'); | ||
| const http = require('node:http'); | ||
|
|
||
| // Identifies requests as coming from the CLI (e.g. "vnext-workflow-cli/1.0.0") | ||
| const USER_AGENT = `vnext-workflow-cli/${pkg.version}`; | ||
| // Create axios instance with custom agents for both HTTP and HTTPS | ||
| const apiClient = axios.create({ | ||
| httpAgent: new http.Agent({ keepAlive: true }), | ||
| httpsAgent: new https.Agent({ | ||
| rejectUnauthorized: false // Allow self-signed certificates | ||
| }) | ||
| }); |
There was a problem hiding this comment.
This block introduces two important issues:
- Critical Bug (ReferenceError): The
USER_AGENTconstant andpackage.jsonimport were removed, butUSER_AGENTis still referenced on line 22 intestApiConnection. This will cause aReferenceError: USER_AGENT is not definedwhenevertestApiConnectionis called, which silently fails the API health check and always reports the connection as down. - Security Vulnerability (Insecure TLS): Hardcoding
rejectUnauthorized: falsedisables SSL/TLS certificate validation for all HTTPS requests, making the CLI vulnerable to Man-in-the-Middle (MitM) attacks.
Recommendation:
- Restore the
USER_AGENTdefinition and configure it globally on theapiClientinstance. - Consider making
rejectUnauthorizedconfigurable (e.g., via a CLI configuration option or environment variable) rather than hardcoded tofalse.
| const https = require('node:https'); | |
| const http = require('node:http'); | |
| // Identifies requests as coming from the CLI (e.g. "vnext-workflow-cli/1.0.0") | |
| const USER_AGENT = `vnext-workflow-cli/${pkg.version}`; | |
| // Create axios instance with custom agents for both HTTP and HTTPS | |
| const apiClient = axios.create({ | |
| httpAgent: new http.Agent({ keepAlive: true }), | |
| httpsAgent: new https.Agent({ | |
| rejectUnauthorized: false // Allow self-signed certificates | |
| }) | |
| }); | |
| const https = require('node:https'); | |
| const http = require('node:http'); | |
| const pkg = require('../../package.json'); | |
| const USER_AGENT = 'vnext-workflow-cli/' + pkg.version; | |
| // Create axios instance with custom agents for both HTTP and HTTPS | |
| const apiClient = axios.create({ | |
| httpAgent: new http.Agent({ keepAlive: true }), | |
| httpsAgent: new https.Agent({ | |
| rejectUnauthorized: false // Allow self-signed certificates (consider making this configurable) | |
| }), | |
| headers: { | |
| 'User-Agent': USER_AGENT | |
| } | |
| }); |
| for (const dir of folderDirs) { | ||
| const files = await glob(toGlobPattern(dir, '**/*.json'), { ignore: ignorePatterns }); | ||
| jsonFiles.push(...files.map(f => ({ | ||
| path: f, | ||
| type: detectComponentType(f, projectRoot), | ||
| fileName: path.basename(f) | ||
| }))); | ||
| } |
There was a problem hiding this comment.
The globbing operation for finding JSON files in the feature folders is not wrapped in a try-catch block. If glob throws an error (e.g., due to permission issues), the CLI will crash with an unhandled promise rejection, leaving the spinner hanging. Wrapping this in a try-catch block and calling spinner.fail() ensures graceful error handling.
try {
for (const dir of folderDirs) {
const files = await glob(toGlobPattern(dir, '**/*.json'), { ignore: ignorePatterns });
jsonFiles.push(...files.map(f => ({
path: f,
type: detectComponentType(f, projectRoot),
fileName: path.basename(f)
})));
}
} catch (error) {
spinner.fail(chalk.red('Error finding JSON files: ' + error.message));
return;
}| async function resolveFeatureFolders(projectRoot, name) { | ||
| const isDir = (p) => fs.existsSync(p) && fs.statSync(p).isDirectory(); |
There was a problem hiding this comment.
To prevent potential runtime crashes (e.g., TypeError: Path must be a string), we should add a defensive check at the beginning of resolveFeatureFolders to ensure name is a valid non-empty string before passing it to path utilities.
| async function resolveFeatureFolders(projectRoot, name) { | |
| const isDir = (p) => fs.existsSync(p) && fs.statSync(p).isDirectory(); | |
| async function resolveFeatureFolders(projectRoot, name) { | |
| if (typeof name !== 'string' || !name.trim()) { | |
| return []; | |
| } | |
| const isDir = (p) => fs.existsSync(p) && fs.statSync(p).isDirectory(); |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
src/lib/api.js,USER_AGENTis still referenced intestApiConnectionheaders but the constant andpkgimport were removed, which will cause a runtime reference error—either reintroduce the constant or remove its usage. - The new
https.Agentis created withrejectUnauthorized: false, which disables TLS certificate validation; consider making this behavior configurable or scoped to explicit dev/test modes instead of the default client configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `src/lib/api.js`, `USER_AGENT` is still referenced in `testApiConnection` headers but the constant and `pkg` import were removed, which will cause a runtime reference error—either reintroduce the constant or remove its usage.
- The new `https.Agent` is created with `rejectUnauthorized: false`, which disables TLS certificate validation; consider making this behavior configurable or scoped to explicit dev/test modes instead of the default client configuration.
## Individual Comments
### Comment 1
<location path="src/lib/api.js" line_range="18-22" />
<code_context>
*/
async function testApiConnection(baseUrl) {
try {
- const response = await axios.get(`${baseUrl}/health`, {
+ const response = await apiClient.get(`${baseUrl}/health`, {
timeout: 5000,
headers: { 'User-Agent': USER_AGENT }
});
</code_context>
<issue_to_address>
**issue (bug_risk):** USER_AGENT is no longer defined, which will throw at runtime before the request is made.
This reference remains from before the package.json import was removed, so testApiConnection will now throw a ReferenceError before the health check runs. Please either restore a USER_AGENT constant (ideally shared with other API calls) or drop this header to align with existing apiClient usage.
</issue_to_address>
### Comment 2
<location path="src/lib/api.js" line_range="6-9" />
<code_context>
-// Identifies requests as coming from the CLI (e.g. "vnext-workflow-cli/1.0.0")
-const USER_AGENT = `vnext-workflow-cli/${pkg.version}`;
+// Create axios instance with custom agents for both HTTP and HTTPS
+const apiClient = axios.create({
+ httpAgent: new http.Agent({ keepAlive: true }),
+ httpsAgent: new https.Agent({
+ rejectUnauthorized: false // Allow self-signed certificates
+ })
+});
</code_context>
<issue_to_address>
**🚨 issue (security):** Disabling TLS verification (`rejectUnauthorized: false`) weakens security and may not be appropriate outside of a controlled environment.
This setting causes the client to trust any certificate, including invalid or malicious ones. If you only need this for local/self‑signed development, please gate it behind configuration and keep strict verification as the default. Alternatively, support a custom CA bundle for self‑signed certs instead of disabling verification entirely.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| async function testApiConnection(baseUrl) { | ||
| try { | ||
| const response = await axios.get(`${baseUrl}/health`, { | ||
| const response = await apiClient.get(`${baseUrl}/health`, { | ||
| timeout: 5000, | ||
| headers: { 'User-Agent': USER_AGENT } |
There was a problem hiding this comment.
issue (bug_risk): USER_AGENT is no longer defined, which will throw at runtime before the request is made.
This reference remains from before the package.json import was removed, so testApiConnection will now throw a ReferenceError before the health check runs. Please either restore a USER_AGENT constant (ideally shared with other API calls) or drop this header to align with existing apiClient usage.
| const apiClient = axios.create({ | ||
| httpAgent: new http.Agent({ keepAlive: true }), | ||
| httpsAgent: new https.Agent({ | ||
| rejectUnauthorized: false // Allow self-signed certificates |
There was a problem hiding this comment.
🚨 issue (security): Disabling TLS verification (rejectUnauthorized: false) weakens security and may not be appropriate outside of a controlled environment.
This setting causes the client to trust any certificate, including invalid or malicious ones. If you only need this for local/self‑signed development, please gate it behind configuration and keep strict verification as the default. Alternatively, support a custom CA bundle for self‑signed certs instead of disabling verification entirely.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
1 participant
Summary by Sourcery
Add feature-folder based update mode and adjust API client configuration for improved connectivity.
New Features:
Enhancements:
Documentation: