feat: restore QA automation and streamline docs on TS branch#23
Conversation
Reviewer's GuideRestores and extends local QA/security automation (lint, tests, gitleaks, SBOM, Renovate, Mend) via script/Make targets, hardens CI workflows, and simplifies developer/user docs while adding guidance for future auto-updates and signing. Sequence diagram for qa command executing lint, tests, and security checkssequenceDiagram
actor Developer
participant MakeOrNpm as make_or_npm
participant NodeScript as scripts_index_js
participant Utils as utils_module
participant Security as security_module
participant Gitleaks as gitleaks_binary
participant SbomTool as sbom_tool
Developer->>MakeOrNpm: make qa or npm run qa
MakeOrNpm->>NodeScript: node scripts/index.js qa
activate NodeScript
NodeScript->>Utils: runNpmScript("lint")
activate Utils
Utils-->>NodeScript: linter completed
deactivate Utils
NodeScript->>Utils: runNpmScript("test")
activate Utils
Utils-->>NodeScript: test suite completed
deactivate Utils
NodeScript->>Security: runSecurity()
activate Security
Security->>Security: ensureSecurityDirs()
Security->>Gitleaks: runCommand(gitleaks detect ...)
activate Gitleaks
Gitleaks-->>Security: write gitleaks_report.json
deactivate Gitleaks
Security->>SbomTool: runCommand(syft or cyclonedx_npm)
activate SbomTool
SbomTool-->>Security: write sbom.cyclonedx.json
deactivate SbomTool
Security-->>NodeScript: security checks completed
deactivate Security
NodeScript-->>MakeOrNpm: QA checks completed successfully
deactivate NodeScript
MakeOrNpm-->>Developer: QA run summary
Class diagram for new security automation module and script entrypointsclassDiagram
class Utils {
<<module>>
+string ROOT_DIR
+void ensureDir(dirPath)
+boolean fileExists(filePath)
+void runNpm(command)
+void runNpmScript(scriptName)
+void printHelp()
+Promise setupProject()
}
class Security {
<<module>>
+Promise runGitleaks()
+Promise runSbom()
+Promise runRenovate(extraArgs)
+Promise runRenovateLocal(extraArgs)
+Promise runMendScan()
+Promise runSecurity()
-void ensureSecurityDirs()
-boolean hasCommand(command)
-string[] getCommandCandidates(command)
-string resolveCommand(command, localCandidates)
-boolean runCommand(command, args, options)
-string getNpxCommand()
-object readPackageMetadata()
-string resolveTokenFromFile()
-string resolveTokenFromGhCli()
-object resolveRenovateToken()
-string detectRepoSlugFromGit()
-object addTokenToRenovateEnv(token)
-string[] splitEnvArgs(value)
}
class ScriptsIndex {
<<module>>
+Promise executeCommand()
-string command
-string[] args
}
class MakefileModule {
<<makefile>>
+target qa
+target security
+target gitleaks
+target sbom
+target renovate
+target renovate_local
+target mend_scan
+target validate
}
class PackageJsonScripts {
<<npm_scripts>>
+script qa
+script security
+script gitleaks
+script sbom
+script renovate
+script renovate_local
+script mend_scan
+script lint
+script lint_tests
}
class ExternalTools {
<<tools>>
+binary gitleaks
+binary syft
+npm @cyclonedx_cyclonedx_npm
+npm renovate
+binary mend_scan
+binary gh
}
ScriptsIndex ..> Utils : uses
ScriptsIndex ..> Security : delegates security commands
MakefileModule ..> ScriptsIndex : calls node scripts_index
PackageJsonScripts ..> ScriptsIndex : runs node scripts_index
Security ..> Utils : uses ROOT_DIR, ensureDir
Security ..> ExternalTools : invokes via runCommand
Utils <.. Security : shared directory and helpers
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
Review Summary by QodoRestore QA automation and security tooling with streamlined documentation
WalkthroughsDescription• Add comprehensive security automation module with gitleaks, SBOM, Renovate, and Mend scanning • Introduce qa command combining lint, test, and security checks • Streamline documentation with concise README and updated development guide • Update GitHub Actions workflows with latest action SHAs and simplified SBOM generation • Add PowerShell entry point for WSL UNC path support on Windows Diagramflowchart LR
A["scripts/index.js"] -->|imports| B["scripts/lib/security.js"]
B -->|runs| C["gitleaks"]
B -->|runs| D["SBOM generation"]
B -->|runs| E["Renovate"]
B -->|runs| F["Mend Scan"]
A -->|new commands| G["qa, security, gitleaks, sbom, renovate, renovate-local, mend-scan"]
H["Makefile"] -->|targets| G
I["package.json"] -->|scripts| G
J["make.ps1"] -->|PowerShell entry| A
K["GitHub Actions"] -->|updated| L["dependency-review, release, sbom workflows"]
File Changes1. scripts/index.js
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request consolidates development workflows and security tooling infrastructure. It updates GitHub Actions workflow versions, simplifies SBOM generation, adds new Makefile targets for automation tasks (qa, security, gitleaks, sbom, renovate, mend-scan), introduces a new security module for orchestrating security tools, creates a Windows PowerShell compatibility script, updates build prerequisites from Node.js v14+ to v20+, and modernizes documentation with Makefile-centric workflows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello @Mehdi-Bl, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the project's development and release infrastructure by re-establishing and expanding automated quality assurance and security checks. It streamlines documentation for better clarity and introduces a detailed plan for future release signing, ensuring a more robust and maintainable codebase. The changes aim to improve developer experience and enhance the security posture of the application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Hey - I've found 2 security issues, 2 other issues, and left some high level feedback:
Security issues:
- Detected calls to child_process from a function argument
command. This could lead to a command injection if the input is user controllable. Try to avoid calls to child_process, and if it is needed ensure user input is correctly sanitized or sandboxed. (link) - Detected calls to child_process from a function argument
command. This could lead to a command injection if the input is user controllable. Try to avoid calls to child_process, and if it is needed ensure user input is correctly sanitized or sandboxed. (link)
General comments:
- The
splitEnvArgshelper inscripts/lib/security.jsnaively splitsRENOVATE_EXTRA_ARGSon spaces, so any arguments requiring quoting (e.g., values containing spaces or=) will be broken; consider using a more robust parser or expecting a JSON/array format instead. - In
scripts/lib/security.js,resolveCommandonly checks a few hard-coded local candidates; if you intend to support tools installed undernode_modules/.bin(e.g., a locally installedgitleaksorsyft), add that directory to the search paths so the commands work without global/binary installs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `splitEnvArgs` helper in `scripts/lib/security.js` naively splits `RENOVATE_EXTRA_ARGS` on spaces, so any arguments requiring quoting (e.g., values containing spaces or `=`) will be broken; consider using a more robust parser or expecting a JSON/array format instead.
- In `scripts/lib/security.js`, `resolveCommand` only checks a few hard-coded local candidates; if you intend to support tools installed under `node_modules/.bin` (e.g., a locally installed `gitleaks` or `syft`), add that directory to the search paths so the commands work without global/binary installs.
## Individual Comments
### Comment 1
<location> `scripts/lib/security.js:150-155` </location>
<code_context>
+ throw new Error('npx is required for SBOM fallback but was not found');
+ }
+
+ runCommand(npxCommand, [
+ '--yes',
+ '@cyclonedx/cyclonedx-npm',
+ '--ignore-npm-errors',
+ '--package-lock-only',
+ '--output-format',
+ 'JSON',
+ '--output-file',
</code_context>
<issue_to_address>
**issue (bug_risk):** CycloneDX `--output-format` value casing may be invalid and cause SBOM generation to fail.
The CycloneDX npm CLI expects lowercase values for `--output-format` (`json`, `xml`, `bom`), but this fallback uses `'JSON'`. If the flag is case-sensitive, SBOM generation (and thus `npm run sbom` in CI) may fail. Consider changing this to the documented lowercase value, e.g. `'json'`.
</issue_to_address>
### Comment 2
<location> `docs/UPDATES_AND_SIGNING.md:44` </location>
<code_context>
+
+### 2) macOS signing + notarization
+
+Use Apple Developer ID Application certificate and notarization credentials:
+
+- `MACOS_CSC_LINK`
</code_context>
<issue_to_address>
**nitpick (typo):** Add an article before “Apple Developer ID Application certificate”.
Consider updating this line to: `Use an Apple Developer ID Application certificate and notarization credentials:`
```suggestion
Use an Apple Developer ID Application certificate and notarization credentials:
```
</issue_to_address>
### Comment 3
<location> `scripts/lib/security.js:27` </location>
<code_context>
execSync(checkCommand, { stdio: 'ignore' });
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-child-process):** Detected calls to child_process from a function argument `command`. This could lead to a command injection if the input is user controllable. Try to avoid calls to child_process, and if it is needed ensure user input is correctly sanitized or sandboxed.
*Source: opengrep*
</issue_to_address>
### Comment 4
<location> `scripts/lib/security.js:70` </location>
<code_context>
const result = spawnSync(command, args, {
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-child-process):** Detected calls to child_process from a function argument `command`. This could lead to a command injection if the input is user controllable. Try to avoid calls to child_process, and if it is needed ensure user input is correctly sanitized or sandboxed.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| ### 2) macOS signing + notarization | ||
|
|
||
| Use Apple Developer ID Application certificate and notarization credentials: |
There was a problem hiding this comment.
nitpick (typo): Add an article before “Apple Developer ID Application certificate”.
Consider updating this line to: Use an Apple Developer ID Application certificate and notarization credentials:
| Use Apple Developer ID Application certificate and notarization credentials: | |
| Use an Apple Developer ID Application certificate and notarization credentials: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71753c0b4e
ℹ️ 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".
| const explicitRepo = extraArgs.find((arg) => !arg.startsWith('-')); | ||
| const repoSlug = explicitRepo || process.env.RENOVATE_REPOSITORY || detectRepoSlugFromGit(); |
There was a problem hiding this comment.
Avoid treating flag values as renovate repo slug
The repo slug detection treats the first arg that does not start with - as an explicit repo. If a user passes a flag that takes a value (e.g., node scripts/index.js renovate --endpoint https://example.com owner/repo), the URL becomes explicitRepo, so repoSlug is set to the endpoint and the real repo is never appended. Renovate then runs against an invalid slug. This breaks renovate whenever a flag with a separate value appears before the repo argument; consider parsing flags/values or only treating a non-flag arg as the repo when it matches owner/repo.
Useful? React with 👍 / 👎.
Code Review by Qodo
1. Renovate token exposed in PR
|
| function resolveRenovateToken() { | ||
| if (process.env.RENOVATE_TOKEN) { | ||
| return { token: process.env.RENOVATE_TOKEN, source: 'RENOVATE_TOKEN' }; | ||
| } | ||
|
|
||
| if (process.env.GITHUB_TOKEN) { | ||
| return { token: process.env.GITHUB_TOKEN, source: 'GITHUB_TOKEN' }; | ||
| } | ||
|
|
||
| if (process.env.GH_TOKEN) { | ||
| return { token: process.env.GH_TOKEN, source: 'GH_TOKEN' }; | ||
| } | ||
|
|
||
| if (process.env.GITHUB_COM_TOKEN) { | ||
| return { token: process.env.GITHUB_COM_TOKEN, source: 'GITHUB_COM_TOKEN' }; | ||
| } | ||
|
|
||
| const fileToken = resolveTokenFromFile(); | ||
| if (fileToken) { | ||
| return { token: fileToken, source: 'RENOVATE_TOKEN_FILE' }; | ||
| } | ||
|
|
||
| const ghToken = resolveTokenFromGhCli(); | ||
| if (ghToken) { | ||
| return { token: ghToken, source: 'gh auth token' }; | ||
| } | ||
|
|
||
| return { token: '', source: '' }; | ||
| } |
There was a problem hiding this comment.
1. Renovate token exposed in pr 📘 Rule violation ⛨ Security
• The PR description includes what appears to be a GitHub access token (gho_...), which is sensitive credential material. • This constitutes an exposure of secrets outside of protected secret stores and can allow unauthorized access until revoked/rotated. • The token should be removed from the PR text and immediately revoked/rotated, then replaced with redacted placeholders in documentation/examples.
Agent Prompt
## Issue description
A token-like GitHub credential (`gho_...`) is present in the PR description. This is a secret exposure risk and must be removed and rotated.
## Issue Context
The compliance checklist prohibits exposing sensitive data (tokens/keys). PR text is not a secure secret store and is commonly indexed/copied.
## Fix Focus Areas
- scripts/lib/security.js[203-231]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| function runCommand(command, args = [], options = {}) { | ||
| const sanitizedArgs = args.map((arg) => { | ||
| if (/^--token=/.test(arg) || /^--\w*token=/.test(arg)) { | ||
| const [key] = arg.split('='); | ||
| return `${key}=***`; | ||
| } | ||
| return arg; | ||
| }); | ||
| const commandLine = [command, ...sanitizedArgs].join(' '); | ||
| console.log(`Running: ${commandLine}`); | ||
|
|
||
| const result = spawnSync(command, args, { | ||
| cwd: utils.ROOT_DIR, | ||
| stdio: 'inherit', | ||
| env: { ...process.env, ...(options.env || {}) }, | ||
| shell: false, | ||
| }); | ||
|
|
||
| if (result.error) { | ||
| throw new Error(`Failed to start command '${command}': ${result.error.message}`); | ||
| } | ||
|
|
||
| if (result.status !== 0) { | ||
| throw new Error(`Command failed (${result.status}): ${commandLine}`); | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| function getNpxCommand() { | ||
| return process.platform === 'win32' ? 'npx.cmd' : 'npx'; | ||
| } |
There was a problem hiding this comment.
2. Windows cmd execution 🐞 Bug ⛯ Reliability
• New security automation executes commands with spawnSync(..., shell:false) while selecting Windows batch shims like npx.cmd. • On Windows, invoking .cmd/.bat without a shell commonly fails, which can break npm run sbom, npm run renovate, npm run renovate:local, and other new automation commands. • This undermines cross-platform support claimed by the Makefile/docs and will surprise Windows contributors.
Agent Prompt
### Issue description
`scripts/lib/security.js` runs external tools using `spawnSync(..., shell: false)` but on Windows it resolves commands like `npx.cmd`. Executing `.cmd`/`.bat` typically requires a shell, so these security/dependency automation commands may fail on Windows.
### Issue Context
This affects newly added automation entry points (sbom/renovate/renovate-local/etc.) and will break Windows workflows.
### Fix Focus Areas
- scripts/lib/security.js[59-90]
### Suggested approach
- Update `runCommand()` to detect Windows batch shims (`.cmd`, `.bat`) and run them via `cmd.exe` (or set `shell: true` on Windows).
- Keep `shell: false` for real executables to avoid changing semantics elsewhere.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const explicitRepo = extraArgs.find((arg) => !arg.startsWith('-')); | ||
| const repoSlug = explicitRepo || process.env.RENOVATE_REPOSITORY || detectRepoSlugFromGit(); | ||
| if (!repoSlug) { | ||
| throw new Error( | ||
| 'RENOVATE_REPOSITORY is required when git remote origin cannot be resolved (expected owner/repo)' | ||
| ); | ||
| } | ||
|
|
||
| const platform = process.env.RENOVATE_PLATFORM || 'github'; | ||
| const args = ['--yes', 'renovate', `--platform=${platform}`]; | ||
|
|
||
| if (process.env.RENOVATE_ENDPOINT) { | ||
| args.push(`--endpoint=${process.env.RENOVATE_ENDPOINT}`); | ||
| } | ||
|
|
||
| if (process.env.RENOVATE_EXTRA_ARGS) { | ||
| args.push(...splitEnvArgs(process.env.RENOVATE_EXTRA_ARGS)); | ||
| } | ||
|
|
||
| if (extraArgs.length > 0) { | ||
| args.push(...extraArgs); | ||
| } | ||
|
|
||
| if (!explicitRepo) { | ||
| args.push(repoSlug); | ||
| } |
There was a problem hiding this comment.
3. Renovate repo arg parsing 🐞 Bug ✓ Correctness
• runRenovate() treats the first argument not starting with - as the repo slug. • Any common CLI usage that passes an option value as a separate token (e.g., --log-level debug, --config-file path) will mis-identify that value as the repo and omit the real repo slug, causing incorrect/failed Renovate runs. • This makes the new renovate entry point fragile and hard to use safely.
Agent Prompt
### Issue description
`runRenovate()` incorrectly detects the repo slug by taking the first arg that doesn’t start with `-`. This breaks when a flag uses a separate value token (the value is mistaken for the repo).
### Issue Context
Users may pass renovate flags as `--flag value` (two args). With the current detection, the value token becomes `explicitRepo`, and the real repo slug may never be appended.
### Fix Focus Areas
- scripts/lib/security.js[284-309]
### Suggested approach
- Replace `extraArgs.find((arg) => !arg.startsWith('-'))` with a stricter matcher for `owner/repo`.
- If no valid repo slug is found in args, always append `repoSlug` derived from env/git.
- Optionally: support an explicit `--repo=<owner/repo>` flag and document it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Code Review
This pull request is a great enhancement, adding comprehensive local QA and security automation, which significantly improves the development workflow. The documentation updates in README.md and DEVELOPMENT.md are clear and concise, and the new UPDATES_AND_SIGNING.md provides valuable planning for future releases. The addition of make.ps1 for better Windows and WSL support is a thoughtful touch. The new scripts, especially scripts/lib/security.js, are well-structured and robust. I've added a couple of suggestions to further improve the new security script's performance and robustness.
|
|
||
| const result = spawnSync(checker, [command], { | ||
| stdio: 'ignore', | ||
| cwd: utils.ROOT_DIR, |
There was a problem hiding this comment.
The regular expression for sanitizing tokens is a bit restrictive and contains a redundant check. The expression /^--token=/ is already covered by /^--\w*token=/. This can be simplified and made more robust to also sanitize other sensitive arguments like --api-key=... or --secret-token=....
| cwd: utils.ROOT_DIR, | |
| if (/^--[\w-]*?(?:token|key|secret)=/i.test(arg)) { |
| `--report-path=${reportPath}`, | ||
| ]; | ||
|
|
||
| if (token) { | ||
| console.log(`Using token source: ${source}`); | ||
| } else { | ||
| console.warn( | ||
| 'No GitHub token found (RENOVATE_TOKEN/GITHUB_TOKEN/GH_TOKEN/RENOVATE_TOKEN_FILE/gh auth token); GitHub-hosted dependencies may be skipped' | ||
| ); | ||
| } | ||
|
|
||
| if (process.env.RENOVATE_EXTRA_ARGS) { | ||
| args.push(...splitEnvArgs(process.env.RENOVATE_EXTRA_ARGS)); | ||
| } | ||
|
|
||
| if (extraArgs.length > 0) { | ||
| args.push(...extraArgs); | ||
| } | ||
|
|
||
| runCommand(npxCommand, args, { env }); | ||
|
|
||
| console.log(`Renovate local report written to: ${reportPath}`); | ||
| return reportPath; |
There was a problem hiding this comment.
The runSecurity function currently runs gitleaks and sbom checks sequentially. Since these are independent asynchronous operations, they can be executed in parallel using Promise.allSettled. This will improve the performance of the qa and security commands and make the error handling logic more concise.
async function runSecurity() {
console.log('Running security checks: gitleaks + sbom');
const results = await Promise.allSettled([runGitleaks(), runSbom()]);
const failures = results
.map((result, i) => {
if (result.status === 'rejected') {
const tool = i === 0 ? 'gitleaks' : 'sbom';
return `${tool} failed: ${result.reason.message}`;
}
return null;
})
.filter(Boolean);
if (failures.length > 0) {
throw new Error(failures.join('\n'));
}
console.log('Security checks completed successfully');
}There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/UPDATES_AND_SIGNING.md`:
- Around line 46-51: Update the macOS notarization credential name and clarify
GitHub secret→electron-builder env var mappings: replace references to
APPLE_ID_PASSWORD with APPLE_APP_SPECIFIC_PASSWORD (electron-builder expects an
app-specific password), and either rename the documented GitHub secrets
WINDOWS_CSC_LINK, WINDOWS_CSC_KEY_PASSWORD, MACOS_CSC_LINK,
MACOS_CSC_KEY_PASSWORD to the electron-builder conventions (e.g., WIN_CSC_LINK /
CSC_LINK and CSC_KEY_PASSWORD) or add a short note mapping each documented
secret to the exact env var names electron-builder expects (mentioning
APPLE_APP_SPECIFIC_PASSWORD, WIN_CSC_LINK / CSC_LINK, and CSC_KEY_PASSWORD) so
readers know how to set their GitHub secrets for the workflow.
In `@package.json`:
- Around line 20-21: The lint-staged pre-commit hook is missing the same
environment flag used by the npm scripts, causing inconsistent ESLint behavior;
update the "lint-staged" entry to invoke eslint with the same prefix used by the
"lint" and "lint:tests" scripts (i.e., prepend cross-env
ESLINT_USE_FLAT_CONFIG=false) so the pre-commit command matches those scripts
(refer to the "lint", "lint:tests", and "lint-staged" script names in
package.json to locate and modify the entry).
In `@README.md`:
- Around line 39-42: Remove the redundant npm run build:webpack step from the
README build instructions: keep the sequence as npm ci followed by npm run build
because the npm run build script already runs build:ts, build:css and the
webpack build internally (via lib/build.js), so delete the explicit npm run
build:webpack line to avoid running webpack twice.
In `@scripts/lib/security.js`:
- Around line 59-86: In runCommand, the token-masking regex for sanitizedArgs
uses /\^--\w*token=/ which doesn't match hyphenated flag names like
--api-token=; update the pattern to allow hyphens (e.g., match
/--[A-Za-z0-9-]*token=/ or /--[^\s=]*token=/) so flags such as
--api-token=secret are masked; modify the regex used when building sanitizedArgs
in the runCommand function accordingly and keep the same split-by-'=' logic that
returns `${key}=***` for masked flags.
- Around line 261-267: The current splitEnvArgs function naively splits on
spaces and breaks quoted arguments (e.g., RENOVATE_EXTRA_ARGS="--labels=\"foo
bar\" --dry-run"); update splitEnvArgs to perform shell-aware tokenization so it
preserves quoted strings and handles escaped quotes/whitespace, or explicitly
document that values cannot contain spaces. Specifically, replace the simple
space-split in splitEnvArgs with a minimal tokenizer that recognizes double and
single quotes and escaped characters (or import a tiny, well-tested
parser/tokenizer), and ensure the function returns tokens with surrounding
quotes removed and escapes processed.
🧹 Nitpick comments (7)
.github/workflows/release.yml (1)
202-202: Add a version comment to the pinnedsoftprops/action-gh-releaseSHA for maintainability.The commit SHA
a06a81a03ee405af7f2048a818ed3f03bbf83c7bis legitimate and corresponds to versionv2.5.0. Pinning to a full commit SHA is good supply-chain practice; adding an inline version comment makes it easy for future readers to identify which release is pinned:Suggested change
- uses: softprops/action-gh-release@a06a81a03ee405af7f2048a818ed3f03bbf83c7b + uses: softprops/action-gh-release@a06a81a03ee405af7f2048a818ed3f03bbf83c7b # v2.5.0make.ps1 (1)
20-28: Nit:Quote-BashArguses an unapproved PowerShell verb.PowerShell convention recommends approved verbs (e.g.,
ConvertTo-BashQuotedArg).Quoteisn't in the approved verb list and will triggerPSScriptAnalyzerwarnings. This is cosmetic for a dev-only script..github/workflows/dependency-review.yml (1)
27-27: Add version tag comment for auditability.The pinned SHA
3c4e3dcb1aa7874d2c16be7d79418e9b7efd6261corresponds to release v4.8.2 and is properly signed. Consider adding a trailing comment with the version tag for clarity (e.g.,# v4.8.2) to improve auditability of the pinned version.scripts/index.js (1)
129-135:qaduplicatesvalidatelogic — consider composing.The
qacase repeats the same lint + test sequence asvalidate(lines 122–127). Consider calling the validate path or extracting a shared helper to keep them in sync.♻️ Suggested refactor
case 'qa': console.log('Running QA checks (lint + test + security)...'); - await utils.runNpmScript('lint'); - await utils.runNpmScript('test'); + // reuse the same validation steps + await utils.runNpmScript('lint'); + await utils.runNpmScript('test'); + // then add security on top await security.runSecurity(); console.log('QA checks completed successfully'); break;A cleaner option would be to extract the lint+test sequence into a helper in
utilsand call it from bothvalidateandqa.scripts/lib/security.js (3)
23-32: Shell meta-character injection risk inhasCommand.The
commandparameter is interpolated directly into a shell string. Current callers only pass safe literals, but if this utility is ever called with user-derived input, it would allow arbitrary command execution via characters like;,|, or$().Consider validating or quoting the input:
🛡️ Defensive fix
function hasCommand(command) { + if (!/^[\w.+-]+$/.test(command)) { + return false; + } const checkCommand = process.platform === 'win32' ? `where ${command}` : `command -v ${command}`;
101-127: Async functions contain only synchronous code.
runGitleaks,runSbom,runRenovate,runRenovateLocal, andrunMendScanare declaredasyncbut only call synchronous functions (spawnSyncviarunCommand). Theawaitcalls inrunSecurity(lines 388, 394) andscripts/index.jsresolve immediately. This works but is misleading — readers may assume I/O is non-blocking when it actually blocks the event loop.Not a bug since the callers
awaitthem and the behavior is correct, but worth a note if you plan to refactor toward truly async execution later (e.g., usingspawnwith promises).Also applies to: 129-163, 269-312, 314-359, 361-380
248-259:addTokenToRenovateEnvoverwrites user's ambientGITHUB_TOKEN.The resolved token is broadcast to
GITHUB_TOKEN,GITHUB_COM_TOKEN, andRENOVATE_GITHUB_COM_TOKENin addition toRENOVATE_TOKEN. SincerunCommandmerges this withprocess.env(line 73), a user who sets separate tokens for GitHub API access vs. Renovate would have theirGITHUB_TOKENsilently overridden for the child process.This may be intentional for simplicity, but consider only setting the Renovate-specific variables and letting the user's existing
GITHUB_TOKENpass through unchanged:♻️ Suggested approach
function addTokenToRenovateEnv(token) { if (!token) { return {}; } return { RENOVATE_TOKEN: token, - GITHUB_TOKEN: token, GITHUB_COM_TOKEN: token, RENOVATE_GITHUB_COM_TOKEN: token, }; }
| - `MACOS_CSC_LINK` | ||
| - `MACOS_CSC_KEY_PASSWORD` | ||
| - `APPLE_ID` | ||
| - `APPLE_ID_PASSWORD` | ||
| - `APPLE_TEAM_ID` | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
electron-builder macOS notarization APPLE_ID_PASSWORD APPLE_APP_SPECIFIC_PASSWORD environment variables
💡 Result:
For electron-builder macOS notarization, the supported environment-variable sets are:
- App Store Connect API key (recommended):
APPLE_API_KEY,APPLE_API_KEY_ID,APPLE_API_ISSUER[1] - Apple ID + app-specific password:
APPLE_ID,APPLE_APP_SPECIFIC_PASSWORD,APPLE_TEAM_ID[1] - Keychain profile:
APPLE_KEYCHAIN,APPLE_KEYCHAIN_PROFILE[1]
So, if you’re currently using APPLE_ID_PASSWORD, that’s not the env var electron-builder documents for its notarization integration (it expects APPLE_APP_SPECIFIC_PASSWORD when using Apple ID auth) [1]. Some older guides/blog posts and custom @electron/notarize scripts still refer to an appleIdPassword/APPLE_ID_PASSWORD naming convention, but that’s separate from electron-builder’s documented env vars [2].
Example (Apple ID auth):
export APPLE_ID="you@example.com"
export APPLE_APP_SPECIFIC_PASSWORD="xxxx-xxxx-xxxx-xxxx"
export APPLE_TEAM_ID="ABCD123456"If these are set, electron-builder’s notarization step can run (unless you’ve disabled it) [1].
Sources:
[1] electron-builder docs (MacConfiguration/MasConfiguration notarize env var requirements) (electron.build)
[2] @electron/notarize docs (uses an appleIdPassword parameter for app-specific password auth) (packages.electronjs.org)
Citations:
- 1: https://www.electron.build/electron-builder.Interface.MacConfiguration.html?utm_source=openai
- 2: https://packages.electronjs.org/notarize/main/index.html?utm_source=openai
🌐 Web query:
electron-builder Windows code signing WIN_CSC_LINK WINDOWS_CSC_LINK environment variables
💡 Result:
electron-builder does not use WINDOWS_CSC_LINK / WINDOWS_CSC_KEY_PASSWORD. The supported environment variables are:
-
Cross-platform (Windows signing included):
CSC_LINK— your.p12/.pfxcert (https/file/base64/file path)CSC_KEY_PASSWORD— cert password
[1]
-
Windows-only override (primarily when building Windows on macOS and you need a different cert than
CSC_*):WIN_CSC_LINKWIN_CSC_KEY_PASSWORD
[1][2]
If you can’t/won’t use env vars, you can instead set build.win.certificateFile and build.win.certificatePassword in config, but the docs recommend env vars first. [3]
So: use CSC_LINK/CSC_KEY_PASSWORD for normal Windows builds; only use WIN_CSC_* when you specifically need a Windows-specific override; ignore WINDOWS_CSC_LINK because it’s not a recognized variable. [1][3]
Sources: [1] [2] [3]
🏁 Script executed:
cat -n docs/UPDATES_AND_SIGNING.md | head -80Repository: codingworkflow/ai-code-fusion
Length of output: 3021
Correct macOS notarization credential name and clarify GitHub secret mappings to electron-builder env vars.
Lines 46–50 reference APPLE_ID_PASSWORD, but electron-builder's notarization expects APPLE_APP_SPECIFIC_PASSWORD (an app-specific password, not the account password). Similarly, the GitHub secret names WINDOWS_CSC_LINK, WINDOWS_CSC_KEY_PASSWORD, MACOS_CSC_LINK, and MACOS_CSC_KEY_PASSWORD do not match electron-builder's recognized env vars (WIN_CSC_LINK/CSC_LINK, CSC_KEY_PASSWORD, etc.). Either update the secret names to align with electron-builder conventions or add a note explaining how these secrets map to the required env vars in the workflow.
🤖 Prompt for AI Agents
In `@docs/UPDATES_AND_SIGNING.md` around lines 46 - 51, Update the macOS
notarization credential name and clarify GitHub secret→electron-builder env var
mappings: replace references to APPLE_ID_PASSWORD with
APPLE_APP_SPECIFIC_PASSWORD (electron-builder expects an app-specific password),
and either rename the documented GitHub secrets WINDOWS_CSC_LINK,
WINDOWS_CSC_KEY_PASSWORD, MACOS_CSC_LINK, MACOS_CSC_KEY_PASSWORD to the
electron-builder conventions (e.g., WIN_CSC_LINK / CSC_LINK and
CSC_KEY_PASSWORD) or add a short note mapping each documented secret to the
exact env var names electron-builder expects (mentioning
APPLE_APP_SPECIFIC_PASSWORD, WIN_CSC_LINK / CSC_LINK, and CSC_KEY_PASSWORD) so
readers know how to set their GitHub secrets for the workflow.
| "lint": "cross-env ESLINT_USE_FLAT_CONFIG=false eslint src tests --ext .js,.jsx,.ts,.tsx --cache", | ||
| "lint:tests": "cross-env ESLINT_USE_FLAT_CONFIG=false eslint tests --ext .js,.jsx,.ts,.tsx --cache", |
There was a problem hiding this comment.
lint-staged config doesn't use the same ESLINT_USE_FLAT_CONFIG=false flag.
Lines 56-58 define the lint-staged rule as "eslint --fix" without the cross-env ESLINT_USE_FLAT_CONFIG=false prefix. This means pre-commit hooks may behave differently from the lint / lint:tests scripts if ESLint flat-config detection ever kicks in. Consider wrapping the lint-staged command consistently:
Proposed fix
"lint-staged": {
"{src,tests}/**/*.{js,jsx,ts,tsx}": [
- "eslint --fix"
+ "cross-env ESLINT_USE_FLAT_CONFIG=false eslint --fix"
],🤖 Prompt for AI Agents
In `@package.json` around lines 20 - 21, The lint-staged pre-commit hook is
missing the same environment flag used by the npm scripts, causing inconsistent
ESLint behavior; update the "lint-staged" entry to invoke eslint with the same
prefix used by the "lint" and "lint:tests" scripts (i.e., prepend cross-env
ESLINT_USE_FLAT_CONFIG=false) so the pre-commit command matches those scripts
(refer to the "lint", "lint:tests", and "lint-staged" script names in
package.json to locate and modify the entry).
| npm ci | ||
| npm run build:webpack | ||
| npm run build | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and read the build script
fd -t f 'index.js' scripts/ --max-depth 1 -x cat {} \;
# Also check package.json for build script definitions
echo "=== package.json build scripts ==="
cat package.json | jq '.scripts | select(. != null) | with_entries(select(.key | contains("build")))'Repository: codingworkflow/ai-code-fusion
Length of output: 7613
🏁 Script executed:
#!/bin/bash
# Read the build module
cat scripts/lib/build.jsRepository: codingworkflow/ai-code-fusion
Length of output: 4823
Remove the redundant npm run build:webpack call from the build instructions.
The npm run build command already includes TypeScript compilation (build:ts), CSS building (build:css), and webpack building internally via lib/build.js. Running npm run build:webpack explicitly before npm run build causes webpack to execute twice, wasting build time. The instructions should be simplified to just npm ci followed by npm run build.
🤖 Prompt for AI Agents
In `@README.md` around lines 39 - 42, Remove the redundant npm run build:webpack
step from the README build instructions: keep the sequence as npm ci followed by
npm run build because the npm run build script already runs build:ts, build:css
and the webpack build internally (via lib/build.js), so delete the explicit npm
run build:webpack line to avoid running webpack twice.
| function runCommand(command, args = [], options = {}) { | ||
| const sanitizedArgs = args.map((arg) => { | ||
| if (/^--token=/.test(arg) || /^--\w*token=/.test(arg)) { | ||
| const [key] = arg.split('='); | ||
| return `${key}=***`; | ||
| } | ||
| return arg; | ||
| }); | ||
| const commandLine = [command, ...sanitizedArgs].join(' '); | ||
| console.log(`Running: ${commandLine}`); | ||
|
|
||
| const result = spawnSync(command, args, { | ||
| cwd: utils.ROOT_DIR, | ||
| stdio: 'inherit', | ||
| env: { ...process.env, ...(options.env || {}) }, | ||
| shell: false, | ||
| }); | ||
|
|
||
| if (result.error) { | ||
| throw new Error(`Failed to start command '${command}': ${result.error.message}`); | ||
| } | ||
|
|
||
| if (result.status !== 0) { | ||
| throw new Error(`Command failed (${result.status}): ${commandLine}`); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Token masking regex misses hyphenated flag names.
The pattern --\w*token= uses \w* which matches word characters (letters, digits, underscore) but not hyphens. A flag like --api-token=secret would not be masked.
🛡️ Proposed fix
- if (/^--token=/.test(arg) || /^--\w*token=/.test(arg)) {
+ if (/^--[\w-]*token=/i.test(arg)) {🤖 Prompt for AI Agents
In `@scripts/lib/security.js` around lines 59 - 86, In runCommand, the
token-masking regex for sanitizedArgs uses /\^--\w*token=/ which doesn't match
hyphenated flag names like --api-token=; update the pattern to allow hyphens
(e.g., match /--[A-Za-z0-9-]*token=/ or /--[^\s=]*token=/) so flags such as
--api-token=secret are masked; modify the regex used when building sanitizedArgs
in the runCommand function accordingly and keep the same split-by-'=' logic that
returns `${key}=***` for masked flags.
| function splitEnvArgs(value) { | ||
| if (!value) { | ||
| return []; | ||
| } | ||
|
|
||
| return value.split(' ').filter(Boolean); | ||
| } |
There was a problem hiding this comment.
Naive space-splitting will break quoted arguments.
RENOVATE_EXTRA_ARGS="--labels=\"foo bar\" --dry-run" would be split into four tokens instead of two. This is a common pitfall with environment-based argument passing.
Consider using a minimal shell-aware tokenizer or documenting that values must not contain spaces:
♻️ Quick documentation fix (minimal approach)
function splitEnvArgs(value) {
if (!value) {
return [];
}
-
+ // NOTE: does not handle quoted strings; each arg must be a single
+ // space-delimited token (e.g. "--dry-run=full --automerge=false").
return value.split(' ').filter(Boolean);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function splitEnvArgs(value) { | |
| if (!value) { | |
| return []; | |
| } | |
| return value.split(' ').filter(Boolean); | |
| } | |
| function splitEnvArgs(value) { | |
| if (!value) { | |
| return []; | |
| } | |
| // NOTE: does not handle quoted strings; each arg must be a single | |
| // space-delimited token (e.g. "--dry-run=full --automerge=false"). | |
| return value.split(' ').filter(Boolean); | |
| } |
🤖 Prompt for AI Agents
In `@scripts/lib/security.js` around lines 261 - 267, The current splitEnvArgs
function naively splits on spaces and breaks quoted arguments (e.g.,
RENOVATE_EXTRA_ARGS="--labels=\"foo bar\" --dry-run"); update splitEnvArgs to
perform shell-aware tokenization so it preserves quoted strings and handles
escaped quotes/whitespace, or explicitly document that values cannot contain
spaces. Specifically, replace the simple space-split in splitEnvArgs with a
minimal tokenizer that recognizes double and single quotes and escaped
characters (or import a tiny, well-tested parser/tokenizer), and ensure the
function returns tokens with surrounding quotes removed and escapes processed.
Summary
• electron-builder version=24.13.3
• loaded configuration file=package.json ("build" field)
husky - Git hooks installed
added 1425 packages, and audited 1426 packages in 51s
262 packages are looking for funding
run
npm fundfor details26 vulnerabilities (11 moderate, 10 high, 5 critical)
To address issues that do not require attention, run:
npm audit fix
To address all issues possible (including breaking changes), run:
npm audit fix --force
Some issues need review, and may require choosing
a different dependency.
Run
npm auditfor details. +Running: npx --yes @cyclonedx/cyclonedx-npm --ignore-npm-errors --package-lock-only --output-format JSON --output-file /workspaces/poc/ai-code-fusion/dist/security/sbom/sbom.cyclonedx.json
SBOM generated with cyclonedx-npm: /workspaces/poc/ai-code-fusion/dist/security/sbom/sbom.cyclonedx.json
Validation
Running: npm run lint
Running: npm run test
Running security checks: gitleaks + sbom
Running: /workspaces/poc/ai-code-fusion/bin/gitleaks detect --source . --report-format json --report-path /workspaces/poc/ai-code-fusion/dist/security/gitleaks/gitleaks-report.json
Gitleaks report written to: /workspaces/poc/ai-code-fusion/dist/security/gitleaks/gitleaks-report.json
Running: npx --yes @cyclonedx/cyclonedx-npm --ignore-npm-errors --package-lock-only --output-format JSON --output-file /workspaces/poc/ai-code-fusion/dist/security/sbom/sbom.cyclonedx.json
SBOM generated with cyclonedx-npm: /workspaces/poc/ai-code-fusion/dist/security/sbom/sbom.cyclonedx.json
Security checks completed successfully
QA checks completed successfully
Running: npx --yes renovate --platform=local --dry-run=lookup --onboarding=false --require-config=optional --detect-host-rules-from-env=true --github-token-warn=false --report-type=file --report-path=/workspaces/poc/ai-code-fusion/dist/security/renovate/renovate-local-report.json
INFO: Repository started (repository=local)
"renovateVersion": "37.440.7"
INFO: Dependency extraction complete (repository=local)
"stats": {
"managers": {
"github-actions": {"fileCount": 4, "depCount": 31},
"npm": {"fileCount": 1, "depCount": 59}
},
"total": {"fileCount": 5, "depCount": 90}
}
INFO: Repository finished (repository=local)
"cloned": undefined,
"durationMs": 5012
WARN: Error cleaning up expired file cache
"err": {
"errno": -2,
"code": "ENOENT",
"syscall": "stat",
"path": "/tmp/renovate/cache/renovate/renovate-cache-v1/content-v2/sha512/85/fe/39f8ccf092d4daa565a0c55c3ac836b24eaf837183ee190df552ca30635e6a62782e5acd8a9e661259c2ec775238d8594dffd4ce735a755ee406cf507dd9",
"message": "ENOENT: no such file or directory, stat '/tmp/renovate/cache/renovate/renovate-cache-v1/content-v2/sha512/85/fe/39f8ccf092d4daa565a0c55c3ac836b24eaf837183ee190df552ca30635e6a62782e5acd8a9e661259c2ec775238d8594dffd4ce735a755ee406cf507dd9'",
"stack": "Error: ENOENT: no such file or directory, stat '/tmp/renovate/cache/renovate/renovate-cache-v1/content-v2/sha512/85/fe/39f8ccf092d4daa565a0c55c3ac836b24eaf837183ee190df552ca30635e6a62782e5acd8a9e661259c2ec775238d8594dffd4ce735a755ee406cf507dd9'\n at Object.stat (node:internal/fs/promises:1032:18)\n at /workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/cacache/lib/content/read.js:16:36\n at read (/workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/cacache/lib/content/read.js:14:32)\n at Object.getData [as get] (/workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/cacache/lib/get.js:27:16)\n at Object.cleanup (/workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/renovate/lib/util/cache/package/file.ts:86:19)\n at Object.cleanup (/workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/renovate/lib/util/cache/package/index.ts:86:5)\n at globalFinalize (/workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/renovate/lib/workers/global/initialize.ts:98:3)\n at start (/workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/renovate/lib/workers/global/index.ts:246:5)\n at /workspaces/config/.codex/codex4/.npm/_npx/05eeecd92f4e18e0/node_modules/renovate/lib/renovate.ts:18:22"
}
INFO: Renovate was run at log level "info". Set LOG_LEVEL=debug in environment variables to see extended debug logs.
Renovate local report written to: /workspaces/poc/ai-code-fusion/dist/security/renovate/renovate-local-report.json
Summary by Sourcery
Add local QA and security automation commands, simplify contributor and user documentation, and harden CI workflows for dependency and release management.
New Features:
Bug Fixes:
Enhancements:
CI:
Summary by CodeRabbit
Release Notes
New Features
Chores