fix: restore vault sonar env and TS tailwind scan for QA screenshots#26
Conversation
Reviewer's GuideThis PR hardens and restores the Sonar/Vault environment loading and scanning pipeline by introducing a robust env loader and Vault helper script, preferring a native sonar-scanner when available, expanding Sonar configuration/env handling, and fixing Tailwind’s content configuration to include TypeScript/TSX so QA screenshots render correctly. Sequence diagram for the updated SonarQube scan processsequenceDiagram
actor Developer
participant NpmScript as Npm_script_sonar
participant SonarScan as sonar_scan_js
participant EnvLoader as Env_loader
participant NativeScanner as Native_sonar_scanner
participant NpmScanner as Npm_sonarqube_scanner
participant SonarQube as SonarQube_server
Developer->>NpmScript: Run npm_run_sonar
NpmScript->>SonarScan: node scripts/sonar-scan.js
Note over SonarScan,EnvLoader: Load Sonar and Vault related env vars
SonarScan->>EnvLoader: Initialize_locked_env_keys
EnvLoader->>EnvLoader: loadEnvViaBash(projectRoot, lockedEnvKeys)
alt Env_vars_loaded_via_bash
EnvLoader-->>SonarScan: bashLoadedKeys
else No_env_from_bash
EnvLoader->>EnvLoader: loadEnvFile(.env, lockedEnvKeys)
EnvLoader-->>SonarScan: dotenv_keys
end
EnvLoader->>EnvLoader: loadEnvFile(.env.local, lockedEnvKeys)
EnvLoader-->>SonarScan: local_dotenv_keys
SonarScan->>SonarScan: Resolve sonarUrl and sonarToken
SonarScan->>SonarScan: Read sonar-project.properties
SonarScan->>SonarScan: Resolve projectKey, projectName, projectVersion
SonarScan->>SonarScan: Build scannerOptions
alt sonarToken_present
SonarScan->>SonarScan: Add sonar.token to scannerOptions
else sonarToken_missing
SonarScan->>Developer: Log_unauthenticated_scan_message
end
Note over SonarScan,NativeScanner: Prefer native sonar-scanner if available
SonarScan->>NativeScanner: runWithNativeScanner(scannerOptions)
alt Native_scanner_found_and_successful
NativeScanner->>SonarQube: Execute_scan_with_properties
SonarQube-->>NativeScanner: Analysis_success
NativeScanner-->>SonarScan: success
SonarScan->>Developer: Log_scan_completed_successfully
else Native_scanner_not_found
SonarScan->>SonarScan: Log_native_scanner_not_found
SonarScan->>NpmScanner: sonarqubeScanner(scannerConfig, callback)
NpmScanner->>SonarQube: Execute_scan_via_wrapper
SonarQube-->>NpmScanner: Analysis_result
alt Wrapper_scan_success
NpmScanner-->>SonarScan: null_error
SonarScan->>Developer: Log_scan_completed_successfully
else Wrapper_scan_failure
NpmScanner-->>SonarScan: error_object
SonarScan->>Developer: Log_failure_and_auth_hints
end
end
Flow diagram for vault_helper.sh secret loadinggraph TD
Start["Start_vault_helper_load_from_definitions"] --> CheckDefs["Check_VAULT_SECRET_PATHS_present"]
CheckDefs -->|No| ErrorNoDefs["Log_error_no_secret_paths_and_exit"]
CheckDefs -->|Yes| ParseDefs["Parse_secret_definitions_and_required_vars"]
ParseDefs --> RequireCli["Require_vault_and_jq_binaries"]
RequireCli -->|Missing_cli| ErrorCli["Log_error_missing_cli_and_exit"]
RequireCli -->|OK| LoadToken["Try_load_VAULT_TOKEN_from_VAULT_TOKEN_FILE"]
LoadToken --> ValidateToken["Validate_existing_or_loaded_token"]
ValidateToken -->|Valid| HaveToken["Use_existing_valid_token"]
ValidateToken -->|Invalid| AuthFlow["Prompt_user_and_authenticate_with_vault"]
AuthFlow -->|Failure| ErrorAuth["Log_authentication_failure_and_exit"]
AuthFlow -->|Success| HaveToken
HaveToken --> LoopPaths["For_each_secret_definition"]
LoopPaths --> ParseEntry["Parse_entry_into_path_and_optional_mappings"]
ParseEntry -->|Empty_path| WarnSkip["Log_warning_and_skip_entry"]
ParseEntry -->|Valid_path| FetchSecrets["vault_kv_get_JSON_for_path"]
FetchSecrets -->|Failure| ErrorFetch["Log_error_fetching_secrets_and_exit"]
FetchSecrets -->|Success| ExtractData["Extract_data_object_from_JSON"]
ExtractData -->|Empty_data| WarnNoSecrets["Log_warning_no_secrets_and_continue"]
ExtractData -->|Has_data| ExportEnv["Generate_export_statements_and_eval"]
ExportEnv --> ApplyMappings["Apply_optional_VAR_to_key_mappings"]
ApplyMappings --> NextPath["Next_secret_definition"]
WarnNoSecrets --> NextPath
WarnSkip --> NextPath
NextPath -->|More_paths| LoopPaths
NextPath -->|No_more_paths| CheckRequired["Validate_required_env_vars_if_any"]
CheckRequired -->|Missing_required| ErrorMissing["Log_missing_required_vars_and_exit"]
CheckRequired -->|All_present| Success["Log_success_Vault_secrets_loaded"]
ErrorNoDefs --> End["Exit_with_failure"]
ErrorCli --> End
ErrorAuth --> End
ErrorFetch --> End
ErrorMissing --> End
Success --> End
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
|
📝 WalkthroughWalkthroughAdds a tracked shell environment loader ( Changes
Sequence Diagram(s)sequenceDiagram
actor Dev
participant Env as .env Loader
participant VaultHelper as vault-helper.sh
participant Vault as Vault Service
participant Scanner as sonar-scan.js
participant Native as Native Scanner
Dev->>Env: source .env
Env->>VaultHelper: vault_helper::load_from_definitions(defs, required, token_file)
VaultHelper->>VaultHelper: discover/validate VAULT_TOKEN (env/file)
alt token missing
VaultHelper->>Dev: prompt for credentials
Dev->>VaultHelper: provide credentials
VaultHelper->>Vault: authenticate -> token
Vault->>VaultHelper: return token
VaultHelper->>VaultHelper: save token (file)
end
VaultHelper->>Vault: fetch secret paths
Vault->>VaultHelper: return secret payloads
VaultHelper->>Env: export secrets as env vars
Env->>Scanner: invoke sonar-scan.js (with env)
Scanner->>Scanner: merge env from shell/.env/.env.local
Scanner->>Native: resolveNativeScannerPath()
alt native found
Scanner->>Native: run native scanner with -D options
Native->>Scanner: exit status
else
Scanner->>Scanner: fallback to npm wrapper scanner
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
Review Summary by QodoRestore vault-backed sonar env and tailwind TS scanning
WalkthroughsDescription• Refactored environment variable loading with vault support and bash sourcing - Added loadEnvViaBash() to source .env.vault and .env files via bash - Implemented parseEnvValue() for proper quote and escape sequence handling - Added selective environment variable loading based on variable type (SONAR_, VAULT_, DTRACK_) • Implemented native sonar-scanner detection and execution - Added runWithNativeScanner() to use native binary instead of npm wrapper - Added resolveNativeScannerPath() to locate scanner in PATH or ~/.sonar directory - Falls back to npm wrapper if native scanner unavailable • Extended Tailwind CSS content scanning to include TypeScript files - Updated content pattern to include .ts and .tsx extensions • Created vault-helper.sh bash utility for secure secret management - Provides token validation, authentication, and secret fetching from Vault - Supports custom variable mappings and required variable validation • Restored committed .env file with vault loader wrapper - Loads SonarQube and DependencyTrack secrets from Vault - Provides sensible defaults for local development Diagramflowchart LR
A["sonar-scan.js"] -->|loads via bash| B["loadEnvViaBash()"]
B -->|sources| C[".env.vault & .env"]
C -->|provides tokens| D["SONAR_TOKEN<br/>DTRACK_API_KEY"]
A -->|attempts| E["runWithNativeScanner()"]
E -->|resolves path| F["resolveNativeScannerPath()"]
F -->|finds binary| G["~/.sonar/sonar-scanner"]
E -->|fallback| H["npm sonarqube-scanner"]
A -->|scans| I["src/renderer/**/*.{js,jsx,ts,tsx}"]
J[".env"] -->|sources| K["vault-helper.sh"]
K -->|authenticates| L["Vault"]
L -->|returns secrets| M["kv/Sonarqube<br/>kv/dependencytrack"]
File Changes1. scripts/sonar-scan.js
|
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 enhances the project's development and CI/CD environment by re-establishing a robust system for managing environment variables and secrets via Vault, improving the SonarQube scanning process, and ensuring correct styling for UI components. The changes streamline the setup for local development and automated quality checks, while also addressing a security alert. 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
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
|
![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:
- sonar-scan.js has grown quite large and multi-purpose (env loading, scanner resolution, execution); consider extracting the env-loading helpers and native-scanner resolution into separate modules to keep the script easier to reason about and maintain.
- vault-helper.sh’s authenticate flow is fully interactive (read username/password from tty), which makes it hard to use in CI or non-interactive environments; consider supporting non-interactive login via environment variables or existing Vault auth methods as a fallback.
- The SONAR_/VAULT_/DTRACK_ prefix logic in shouldPreferDotenvValue/shouldImportFromShell hardcodes which keys are overridden/imported; consider centralizing these prefixes in a single configuration structure so it’s clear and easier to extend if additional key families are needed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- sonar-scan.js has grown quite large and multi-purpose (env loading, scanner resolution, execution); consider extracting the env-loading helpers and native-scanner resolution into separate modules to keep the script easier to reason about and maintain.
- vault-helper.sh’s authenticate flow is fully interactive (read username/password from tty), which makes it hard to use in CI or non-interactive environments; consider supporting non-interactive login via environment variables or existing Vault auth methods as a fallback.
- The SONAR_/VAULT_/DTRACK_ prefix logic in shouldPreferDotenvValue/shouldImportFromShell hardcodes which keys are overridden/imported; consider centralizing these prefixes in a single configuration structure so it’s clear and easier to extend if additional key families are needed.
## Individual Comments
### Comment 1
<location> `scripts/vault-helper.sh:44` </location>
<code_context>
+ local token="$1"
+ local file="$2"
+ mkdir -p "$(dirname "$file")"
+ umask 077
+ printf '%s\n' "$token" >"$file"
+ chmod 600 "$file" 2>/dev/null || true
</code_context>
<issue_to_address>
**issue (bug_risk):** Changing umask in save_token has a global side effect on the caller shell.
`umask 077` here permanently changes the process umask, which can unexpectedly alter permissions of files created later by callers sourcing this helper. Consider saving and restoring the previous umask, for example:
```bash
vault_helper::save_token() {
local token="$1" file="$2" old_umask
old_umask=$(umask)
mkdir -p "$(dirname "$file")"
umask 077
printf '%s
' "$token" >"$file"
umask "$old_umask"
chmod 600 "$file" 2>/dev/null || true
vault_helper::log_info "Saved Vault token to $file"
}
```
</issue_to_address>
### Comment 2
<location> `scripts/vault-helper.sh:78-84` </location>
<code_context>
+
+ VAULT_TOKEN="$token"
+ export VAULT_TOKEN
+ vault_helper::save_token "$token" "$VAULT_TOKEN_FILE"
+
+ unset username password token
</code_context>
<issue_to_address>
**suggestion (bug_risk):** authenticate assumes VAULT_TOKEN_FILE is set, which may break if used outside load_from_definitions.
Here `vault_helper::authenticate` passes `$VAULT_TOKEN_FILE` to `vault_helper::save_token`, but that variable is only set in `vault_helper::load_from_definitions`. If `authenticate` is called directly (e.g. from another script), `$VAULT_TOKEN_FILE` may be empty and the token may be written to an unintended or invalid path. Consider defaulting to `$HOME/.vault-token` when it’s unset, or failing fast with a clear error instead.
```suggestion
VAULT_TOKEN="$token"
export VAULT_TOKEN
# Determine where to save the Vault token:
# - Prefer explicitly configured VAULT_TOKEN_FILE
# - Fallback to the standard ~/.vault-token location
local token_file="${VAULT_TOKEN_FILE:-${HOME:+$HOME/.vault-token}}"
if [[ -z "$token_file" ]]; then
vault_helper::log_error "VAULT_TOKEN_FILE is not set and HOME is unset; cannot determine token file path"
unset username password token
return 1
fi
vault_helper::save_token "$token" "$token_file"
unset username password token
return 0
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| local token="$1" | ||
| local file="$2" | ||
| mkdir -p "$(dirname "$file")" | ||
| umask 077 |
There was a problem hiding this comment.
issue (bug_risk): Changing umask in save_token has a global side effect on the caller shell.
umask 077 here permanently changes the process umask, which can unexpectedly alter permissions of files created later by callers sourcing this helper. Consider saving and restoring the previous umask, for example:
vault_helper::save_token() {
local token="$1" file="$2" old_umask
old_umask=$(umask)
mkdir -p "$(dirname "$file")"
umask 077
printf '%s
' "$token" >"$file"
umask "$old_umask"
chmod 600 "$file" 2>/dev/null || true
vault_helper::log_info "Saved Vault token to $file"
}| VAULT_TOKEN="$token" | ||
| export VAULT_TOKEN | ||
| vault_helper::save_token "$token" "$VAULT_TOKEN_FILE" | ||
|
|
||
| unset username password token | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): authenticate assumes VAULT_TOKEN_FILE is set, which may break if used outside load_from_definitions.
Here vault_helper::authenticate passes $VAULT_TOKEN_FILE to vault_helper::save_token, but that variable is only set in vault_helper::load_from_definitions. If authenticate is called directly (e.g. from another script), $VAULT_TOKEN_FILE may be empty and the token may be written to an unintended or invalid path. Consider defaulting to $HOME/.vault-token when it’s unset, or failing fast with a clear error instead.
| VAULT_TOKEN="$token" | |
| export VAULT_TOKEN | |
| vault_helper::save_token "$token" "$VAULT_TOKEN_FILE" | |
| unset username password token | |
| return 0 | |
| } | |
| VAULT_TOKEN="$token" | |
| export VAULT_TOKEN | |
| # Determine where to save the Vault token: | |
| # - Prefer explicitly configured VAULT_TOKEN_FILE | |
| # - Fallback to the standard ~/.vault-token location | |
| local token_file="${VAULT_TOKEN_FILE:-${HOME:+$HOME/.vault-token}}" | |
| if [[ -z "$token_file" ]]; then | |
| vault_helper::log_error "VAULT_TOKEN_FILE is not set and HOME is unset; cannot determine token file path" | |
| unset username password token | |
| return 1 | |
| fi | |
| vault_helper::save_token "$token" "$token_file" | |
| unset username password token | |
| return 0 | |
| } |
![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 aims to enhance the development and CI workflow by improving the SonarQube scanning script, adding a vault-helper.sh for secret management, and fixing the Tailwind CSS configuration. However, it introduces several critical security vulnerabilities, primarily Remote Code Execution (RCE) in scripts/sonar-scan.js due to sourcing environment files as bash scripts. Further command injection risks exist with shell: true on Windows and eval in the Vault helper script. While the changes are well-implemented and include positive enhancements like prioritizing a native sonar-scanner binary, there are minor suggestions to improve script robustness. It is highly recommended to address these security concerns by using safe environment variable parsers and avoiding the execution of untrusted content as shell scripts.
| const result = spawnSync('bash', ['-lc', shellScript], { | ||
| cwd: projectRoot, | ||
| encoding: 'utf8', | ||
| stdio: ['inherit', 'pipe', 'inherit'], | ||
| env: process.env, | ||
| }); |
There was a problem hiding this comment.
The loadEnvViaBash function executes spawnSync('bash', ['-lc', shellScript], ...) where shellScript sources ./.env and ./.env.vault. This creates a critical Remote Code Execution (RCE) vulnerability, as an attacker can modify .env in a pull request to execute malicious bash code on the CI server or a developer's machine when the sonar scan runs. Additionally, the -l (login) flag in bash -lc can introduce unpredictable environment variables from user-specific profile scripts, making the script less consistent. It is recommended to address the RCE by using safe environment variable parsers and avoiding the execution of untrusted content as shell scripts, and to remove the -l flag for more isolated and secure execution.
| return 1 | ||
| fi | ||
|
|
||
| eval "$exports" |
There was a problem hiding this comment.
The script uses eval to export environment variables fetched from Vault. While the values are escaped using jq's @sh filter, the keys are not. If an attacker can control the keys of the secrets stored in Vault, they can inject arbitrary shell commands that will be executed when eval "$exports" is called. For example, a secret key named VAR; touch /tmp/pwned would result in the execution of touch /tmp/pwned.
| read -r -s -p "Vault password: " password | ||
| echo >&2 | ||
|
|
||
| if ! login_json=$(vault login -format=json -method=userpass username="$username" password="$password" 2>/dev/null); then |
There was a problem hiding this comment.
In the vault_helper::authenticate function, stderr from the vault login command is redirected to /dev/null. This suppresses potentially useful error messages if authentication fails, making it difficult to debug issues related to incorrect credentials, network problems, or a misconfigured Vault address. By removing the redirection, Vault's native error messages will be displayed, improving debuggability.
| if ! login_json=$(vault login -format=json -method=userpass username="$username" password="$password" 2>/dev/null); then | |
| if ! login_json=$(vault login -format=json -method=userpass username="$username" password="$password"); then |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/sonar-scan.js (1)
279-281:⚠️ Potential issue | 🟡 Minor
sonar-project.propertiesread will throw if file is missing.
fs.readFileSyncon line 281 throws an uncaught exception if the file doesn't exist, crashing the script with an opaque error. Other file accesses in this script (coverage path, .env) useexistsSyncguards.🛡️ Proposed fix
const propertiesPath = path.join(__dirname, '..', 'sonar-project.properties'); -const propertiesContent = fs.readFileSync(propertiesPath, 'utf8'); const properties = {}; +if (fs.existsSync(propertiesPath)) { + const propertiesContent = fs.readFileSync(propertiesPath, 'utf8'); -// Simple parser for properties file -propertiesContent.split('\n').forEach((line) => { - line = line.trim(); - if (line && !line.startsWith('#')) { - const delimiterIndex = line.indexOf('='); - if (delimiterIndex === -1) { - return; - } - const key = line.slice(0, delimiterIndex).trim(); - const value = line.slice(delimiterIndex + 1).trim(); - if (key && value) { - properties[key] = value; + propertiesContent.split('\n').forEach((line) => { + line = line.trim(); + if (line && !line.startsWith('#')) { + const delimiterIndex = line.indexOf('='); + if (delimiterIndex === -1) { + return; + } + const key = line.slice(0, delimiterIndex).trim(); + const value = line.slice(delimiterIndex + 1).trim(); + if (key && value) { + properties[key] = value; + } } - } -}); + }); +}
🤖 Fix all issues with AI agents
In `@scripts/sonar-scan.js`:
- Around line 301-305: The default Sonar project key fallback is stale: update
the fallback value used when resolving projectKey (the expression assigning to
projectKey that checks process.env.SONAR_PROJECT_KEY, process.env.SONAR_PROJECT,
and properties['sonar.projectKey']) to use 'ai-code-fusion' instead of
'ai-code-prep' so the code, .env, and Sonar logs are consistent.
- Around line 92-147: The spawned bash in loadEnvViaBash is getting log lines
from vault-helper.sh on stdout which corrupt env -0 output; fix this by changing
vault-helper.sh's log_info and log_warn to write to stderr (use >&2 or redirect
their prints to stderr) so they no longer pollute stdout, and also make
loadEnvViaBash more robust by sanitizing result.stdout before parsing (e.g.,
drop any leading bytes up to the first NUL or filter out lines/entries that
don't look like valid KEY=VALUE pairs) in the loadEnvViaBash function so stray
helper output can't inject malformed entries.
In `@scripts/vault-helper.sh`:
- Around line 59-84: In vault_helper::authenticate change the vault login
invocation so the password is not passed as a CLI argument (avoid
password="$password"); instead omit the password arg and feed the password via
stdin (e.g. pipe printf '%s\n' "$password" into the vault login command or let
vault prompt for the password) so the secret never appears in the process list;
update the login_json assignment that currently calls vault login -format=json
-method=userpass username="$username" password="$password" to the stdin-based
variant and keep the rest of the function (token extraction, VAULT_TOKEN export,
vault_helper::save_token) unchanged.
- Around line 40-48: The vault_helper::save_token function sets umask 077
globally which leaks into the caller's shell; change the implementation to scope
the umask to the write operation only (e.g., run the mkdir/printf/chmod sequence
inside a subshell or use a grouped command with a locally-set umask) so the
restrictive umask does not persist after the function returns, and keep the
vault_helper::log_info call outside or after the subshell as needed.
- Around line 141-149: The current use of eval "$exports" after building export
lines from data_json is a command-injection risk because the jq template embeds
.key unescaped; change this to either (a) escape keys with `@sh` in the jq output
so both key and value are safely quoted before eval, or (b) eliminate eval
entirely by iterating over keys parsed from data_json (e.g., using jq -r
'keys[]' or to_entries[]), and for each key use a safe assignment pattern like
printf -v / export (the same approach used by set_secret_if_empty) to set
environment variables; ensure you update the error path that calls
vault_helper::log_error when parsing fails and keep the behavior of returning 1
on error.
🧹 Nitpick comments (4)
scripts/vault-helper.sh (1)
86-96: Shellcheck SC2163 is a false positive here, but consider a clearer idiom.
export "$var"where$varholds a variable name likeSONAR_TOKENis correct and intentional — it marks the already-assigned variable for export. ShellCheck flags it because it can't infer intent. You can silence it with a directive, or use the combineddeclare -gxidiom:♻️ Optional: silence SC2163
vault_helper::set_secret_if_empty() { local var="$1" local value="$2" local source="$3" if [[ -z "${!var:-}" && -n "$value" ]]; then printf -v "$var" '%s' "$value" + # shellcheck disable=SC2163 export "$var" vault_helper::log_info "Mapped ${source} -> ${var}" fi }.env (2)
1-3: Shebang on line 3 is ineffective; line 1 comment has a grammar nit.The shebang (
#!/usr/bin/env bash) must be the very first line of the file to be recognized by the kernel. Since this file is always sourced (never executed directly), it's cosmetic — but it's misleading where it sits. Also, line 1 reads like it's missing punctuation: "This file is committed to git no secrets" → "committed to git — no secrets."♻️ Suggested reorder
-# This file is committed to git no secrets. - -#!/usr/bin/env bash +#!/usr/bin/env bash +# This file is committed to git — no secrets.
1-58: All dotenv-linter warnings are false positives — this is a bash script, not a dotenv file.The
.envfilename causesdotenv-linterto parse this as a KEY=VALUE file, producing dozens of spurious warnings (IncorrectDelimiter, LowercaseKey, etc.). Consider either:
- Adding this file to a dotenv-linter exclusion/ignore list, or
- Renaming to e.g.
.env.shorscripts/load-env.shand updating references.Option 2 would also avoid confusing developers and other tools (IDEs,
docker-compose, Node'sdotenvlibrary) that expect.envto be a flat key-value file.scripts/sonar-scan.js (1)
193-240: Scanner directory sort assumes lexicographic ordering equals version ordering.Line 228-229:
.sort().reverse()picks the last directory alphabetically, which works for same-digit-count versions (e.g.,sonar-scanner-5.0.1vssonar-scanner-6.0.0) but breaks for mixed-length major versions (sonar-scanner-9.xsorts aftersonar-scanner-10.xlexicographically). This is an edge case that's unlikely today but worth noting.
| function loadEnvViaBash(projectRoot, lockedEnvKeys) { | ||
| if (!fs.existsSync(path.join(projectRoot, '.env'))) { | ||
| return []; | ||
| } | ||
|
|
||
| const shellScript = ` | ||
| if [ -f "./.env.vault" ]; then | ||
| . ./.env.vault || true | ||
| fi | ||
| if [ -f "./.env" ]; then | ||
| set -a | ||
| . ./.env || true | ||
| set +a | ||
| fi | ||
| env -0 | ||
| `; | ||
|
|
||
| const result = spawnSync('bash', ['-lc', shellScript], { | ||
| cwd: projectRoot, | ||
| encoding: 'utf8', | ||
| stdio: ['inherit', 'pipe', 'inherit'], | ||
| env: process.env, | ||
| }); | ||
|
|
||
| if (result.error && result.error.code === 'ENOENT') { | ||
| return []; | ||
| } | ||
|
|
||
| if (result.error || result.status !== 0 || !result.stdout) { | ||
| return []; | ||
| } | ||
|
|
||
| const loadedKeys = []; | ||
| const entries = result.stdout.split('\0').filter(Boolean); | ||
| for (const entry of entries) { | ||
| const delimiterIndex = entry.indexOf('='); | ||
| if (delimiterIndex <= 0) { | ||
| continue; | ||
| } | ||
|
|
||
| const key = entry.slice(0, delimiterIndex); | ||
| const value = entry.slice(delimiterIndex + 1); | ||
| if (!shouldImportFromShell(key)) { | ||
| continue; | ||
| } | ||
|
|
||
| if (!shouldPreferDotenvValue(key, value, lockedEnvKeys)) { | ||
| continue; | ||
| } | ||
|
|
||
| process.env[key] = value; | ||
| loadedKeys.push(key); | ||
| } | ||
|
|
||
| return loadedKeys; | ||
| } |
There was a problem hiding this comment.
Vault helper log_info/log_warn write to stdout, corrupting the captured env -0 output.
loadEnvViaBash captures stdout from the spawned bash process (line 112: 'pipe'). When .env is sourced inside that subprocess, it calls vault_helper::load_from_definitions, which invokes vault_helper::log_info — and that writes to stdout (no >&2 redirect in vault-helper.sh lines 3-5, 7-9). These log lines get interleaved with the env -0 null-separated output, corrupting the first parsed entry and potentially injecting garbage key-value pairs (e.g., log lines containing =).
Two fixes needed in combination:
- In
scripts/vault-helper.sh: redirectlog_infoandlog_warnto stderr:
🐛 Fix in vault-helper.sh
vault_helper::log_info() {
- printf '[INFO] %s\n' "$1"
+ printf '[INFO] %s\n' "$1" >&2
}
vault_helper::log_warn() {
- printf '[WARN] %s\n' "$1"
+ printf '[WARN] %s\n' "$1" >&2
}- Alternatively, in
sonar-scan.js: redirect the subprocess stdout through a filter, or simply accept that bash-sourcing may produce extraneous output and add a more resilient parser (though fixing the root cause in the helper is cleaner).
🤖 Prompt for AI Agents
In `@scripts/sonar-scan.js` around lines 92 - 147, The spawned bash in
loadEnvViaBash is getting log lines from vault-helper.sh on stdout which corrupt
env -0 output; fix this by changing vault-helper.sh's log_info and log_warn to
write to stderr (use >&2 or redirect their prints to stderr) so they no longer
pollute stdout, and also make loadEnvViaBash more robust by sanitizing
result.stdout before parsing (e.g., drop any leading bytes up to the first NUL
or filter out lines/entries that don't look like valid KEY=VALUE pairs) in the
loadEnvViaBash function so stray helper output can't inject malformed entries.
| const projectKey = | ||
| process.env.SONAR_PROJECT_KEY || properties['sonar.projectKey'] || 'ai-code-prep'; | ||
| process.env.SONAR_PROJECT_KEY || | ||
| process.env.SONAR_PROJECT || | ||
| properties['sonar.projectKey'] || | ||
| 'ai-code-prep'; |
There was a problem hiding this comment.
Default project key 'ai-code-prep' appears stale.
The PR description, .env file (line 57: SONAR_PROJECT_KEY=ai-code-fusion), and the SonarQube logs all reference ai-code-fusion as the project key. The fallback on line 305 still uses 'ai-code-prep'.
- properties['sonar.projectKey'] ||
- 'ai-code-prep';
+ properties['sonar.projectKey'] ||
+ 'ai-code-fusion';📝 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.
| const projectKey = | |
| process.env.SONAR_PROJECT_KEY || properties['sonar.projectKey'] || 'ai-code-prep'; | |
| process.env.SONAR_PROJECT_KEY || | |
| process.env.SONAR_PROJECT || | |
| properties['sonar.projectKey'] || | |
| 'ai-code-prep'; | |
| const projectKey = | |
| process.env.SONAR_PROJECT_KEY || | |
| process.env.SONAR_PROJECT || | |
| properties['sonar.projectKey'] || | |
| 'ai-code-fusion'; |
🤖 Prompt for AI Agents
In `@scripts/sonar-scan.js` around lines 301 - 305, The default Sonar project key
fallback is stale: update the fallback value used when resolving projectKey (the
expression assigning to projectKey that checks process.env.SONAR_PROJECT_KEY,
process.env.SONAR_PROJECT, and properties['sonar.projectKey']) to use
'ai-code-fusion' instead of 'ai-code-prep' so the code, .env, and Sonar logs are
consistent.
| vault_helper::save_token() { | ||
| local token="$1" | ||
| local file="$2" | ||
| mkdir -p "$(dirname "$file")" | ||
| umask 077 | ||
| printf '%s\n' "$token" >"$file" | ||
| chmod 600 "$file" 2>/dev/null || true | ||
| vault_helper::log_info "Saved Vault token to $file" | ||
| } |
There was a problem hiding this comment.
umask 077 leaks into the caller's session.
Since this script is designed to be sourced (via .env), calling umask 077 on line 44 permanently changes the umask for the entire shell session, not just for the printf on line 45. All subsequent file creation by the user's shell will use the restrictive 077 mask.
Use a subshell to scope the umask change:
🛡️ Proposed fix
vault_helper::save_token() {
local token="$1"
local file="$2"
mkdir -p "$(dirname "$file")"
- umask 077
- printf '%s\n' "$token" >"$file"
- chmod 600 "$file" 2>/dev/null || true
+ ( umask 077; printf '%s\n' "$token" >"$file" )
+ chmod 600 "$file" 2>/dev/null || true
vault_helper::log_info "Saved Vault token to $file"
}🤖 Prompt for AI Agents
In `@scripts/vault-helper.sh` around lines 40 - 48, The vault_helper::save_token
function sets umask 077 globally which leaks into the caller's shell; change the
implementation to scope the umask to the write operation only (e.g., run the
mkdir/printf/chmod sequence inside a subshell or use a grouped command with a
locally-set umask) so the restrictive umask does not persist after the function
returns, and keep the vault_helper::log_info call outside or after the subshell
as needed.
| vault_helper::authenticate() { | ||
| local username password login_json token | ||
| read -r -p "Vault username: " username >&2 | ||
| read -r -s -p "Vault password: " password | ||
| echo >&2 | ||
|
|
||
| if ! login_json=$(vault login -format=json -method=userpass username="$username" password="$password" 2>/dev/null); then | ||
| vault_helper::log_error "Vault authentication failed" | ||
| return 1 | ||
| fi | ||
|
|
||
| token=$(jq -r '.auth.client_token // empty' <<<"$login_json") | ||
| unset login_json | ||
|
|
||
| if [[ -z "$token" ]]; then | ||
| vault_helper::log_error "Vault login response did not include a token" | ||
| return 1 | ||
| fi | ||
|
|
||
| VAULT_TOKEN="$token" | ||
| export VAULT_TOKEN | ||
| vault_helper::save_token "$token" "$VAULT_TOKEN_FILE" | ||
|
|
||
| unset username password token | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
Password passed as a CLI argument is visible in the process list.
On line 65, password="$password" is passed as an argument to vault login. This makes the password visible to any user who can run ps on the same host. The vault CLI supports reading credentials from stdin or environment variables, which would be safer.
However, since this is an interactive local-dev flow (prompted credentials), the exposure window is brief and the risk is limited.
🤖 Prompt for AI Agents
In `@scripts/vault-helper.sh` around lines 59 - 84, In vault_helper::authenticate
change the vault login invocation so the password is not passed as a CLI
argument (avoid password="$password"); instead omit the password arg and feed
the password via stdin (e.g. pipe printf '%s\n' "$password" into the vault login
command or let vault prompt for the password) so the secret never appears in the
process list; update the login_json assignment that currently calls vault login
-format=json -method=userpass username="$username" password="$password" to the
stdin-based variant and keep the rest of the function (token extraction,
VAULT_TOKEN export, vault_helper::save_token) unchanged.
| if ! exports=$(printf '%s' "$data_json" | jq -r ' | ||
| to_entries[]? | | ||
| "export \(.key)=\(.value | @sh)" | ||
| '); then | ||
| vault_helper::log_error "Unable to parse secrets from ${path}" | ||
| return 1 | ||
| fi | ||
|
|
||
| eval "$exports" |
There was a problem hiding this comment.
eval "$exports" is a command injection vector via unsanitized vault secret keys.
The jq template on line 143 embeds .key directly into the shell string without escaping. While .value is safely quoted with @sh, a vault key containing shell metacharacters (e.g., $(cmd) or backticks) would be executed. Even if vault keys are admin-controlled today, defense in depth matters here.
Quote the key with @sh as well:
🛡️ Proposed fix
if ! exports=$(printf '%s' "$data_json" | jq -r '
to_entries[]? |
- "export \(.key)=\(.value | `@sh`)"
+ "export \(.key | `@sh`)=\(.value | `@sh`)"
'); thenAlternatively, avoid eval entirely by iterating keys with jq -r 'keys[]' and assigning via printf -v / export, similar to the set_secret_if_empty pattern already in this file.
🤖 Prompt for AI Agents
In `@scripts/vault-helper.sh` around lines 141 - 149, The current use of eval
"$exports" after building export lines from data_json is a command-injection
risk because the jq template embeds .key unescaped; change this to either (a)
escape keys with `@sh` in the jq output so both key and value are safely quoted
before eval, or (b) eliminate eval entirely by iterating over keys parsed from
data_json (e.g., using jq -r 'keys[]' or to_entries[]), and for each key use a
safe assignment pattern like printf -v / export (the same approach used by
set_secret_if_empty) to set environment variables; ensure you update the error
path that calls vault_helper::log_error when parsing fails and keep the behavior
of returning 1 on error.
Code Review by Qodo
1. sonar.token passed via CLI
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| console.log(`Using native sonar-scanner: ${scannerBinary}`); | ||
| const args = Object.entries(scannerOptions).map(([key, value]) => `-D${key}=${value}`); | ||
| const useWindowsShell = process.platform === 'win32' && scannerBinary.toLowerCase().endsWith('.bat'); | ||
| const result = spawnSync(scannerBinary, args, { | ||
| cwd: projectRoot, | ||
| stdio: 'inherit', | ||
| shell: useWindowsShell, | ||
| }); |
There was a problem hiding this comment.
1. sonar.token passed via cli 📘 Rule violation ⛨ Security
• The native scanner path uses spawnSync(scannerBinary, args, ...) where args are built as
-D${key}=${value}, which will include the Sonar token when scannerOptions['sonar.token'] is set.
• Passing secrets via command-line arguments can expose tokens via process listings and CI/job
telemetry, violating secure handling requirements for sensitive data.
Agent Prompt
## Issue description
The implementation passes the Sonar token via `-Dsonar.token=...` on the command line when using the native scanner. CLI arguments can be observable (e.g., process list, CI logs/telemetry), which can leak secrets.
## Issue Context
This occurs in the native scanner execution path where all scanner options are converted to `-Dkey=value` arguments, and `sonar.token` is added when `SONAR_TOKEN` is present.
## Fix Focus Areas
- scripts/sonar-scan.js[173-180]
- scripts/sonar-scan.js[332-334]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| #!/usr/bin/env bash | ||
|
|
||
| # Vault environment loader wrapper | ||
| # Usage: source .env (never run directly) | ||
|
|
||
| set -o pipefail | ||
|
|
||
| if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then | ||
| echo "This script must be sourced: source .env" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
2. Windows sonar env broken 🐞 Bug ✓ Correctness
• make.bat still assumes .env is a line-oriented KEY=VALUE file, but this PR converts .env into a bash script. • On Windows, make sonar will not load SONAR_* variables correctly, so npm run sonar may run with missing config (and now defaults to localhost). • This is a breaking change for Windows developers using make.bat sonar.
Agent Prompt
### Issue description
`make.bat sonar` expects `.env` to be a simple `KEY=VALUE` file, but `.env` is now a bash script. This breaks environment loading on Windows.
### Issue Context
Windows batch parsing cannot interpret bash constructs (`#!/usr/bin/env bash`, `[[ ... ]]`, `source`, function calls). As a result, SONAR-related env vars won't be loaded.
### Fix Focus Areas
- .env[1-58]
- make.bat[30-50]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const shellScript = ` | ||
| if [ -f "./.env.vault" ]; then | ||
| . ./.env.vault || true | ||
| fi | ||
| if [ -f "./.env" ]; then | ||
| set -a | ||
| . ./.env || true | ||
| set +a | ||
| fi | ||
| env -0 | ||
| `; | ||
|
|
||
| const result = spawnSync('bash', ['-lc', shellScript], { | ||
| cwd: projectRoot, | ||
| encoding: 'utf8', | ||
| stdio: ['inherit', 'pipe', 'inherit'], | ||
| env: process.env, | ||
| }); | ||
|
|
||
| if (result.error && result.error.code === 'ENOENT') { | ||
| return []; | ||
| } | ||
|
|
||
| if (result.error || result.status !== 0 || !result.stdout) { | ||
| return []; | ||
| } | ||
|
|
||
| const loadedKeys = []; | ||
| const entries = result.stdout.split('\0').filter(Boolean); | ||
| for (const entry of entries) { | ||
| const delimiterIndex = entry.indexOf('='); | ||
| if (delimiterIndex <= 0) { | ||
| continue; | ||
| } | ||
|
|
||
| const key = entry.slice(0, delimiterIndex); | ||
| const value = entry.slice(delimiterIndex + 1); | ||
| if (!shouldImportFromShell(key)) { | ||
| continue; | ||
| } | ||
|
|
||
| if (!shouldPreferDotenvValue(key, value, lockedEnvKeys)) { | ||
| continue; | ||
| } | ||
|
|
||
| process.env[key] = value; | ||
| loadedKeys.push(key); | ||
| } | ||
|
|
There was a problem hiding this comment.
3. Stdout corrupts env import 🐞 Bug ⛯ Reliability
• scripts/sonar-scan.js captures stdout from a bash subshell and assumes it contains only env -0 output. • The sourced .env/Vault helper prints informational logs to **stdout**, so those logs get prepended into the captured buffer and can corrupt the NUL-delimited parsing. • This can lead to missing/incorrect SONAR_*/VAULT_* imports even when Vault loading succeeds.
Agent Prompt
### Issue description
`loadEnvViaBash()` parses `env -0` from stdout, but `.env`/Vault helper emits logs to stdout, corrupting the NUL-delimited env stream.
### Issue Context
Any stdout before `env -0` becomes part of the captured buffer and breaks the parser’s assumptions.
### Fix Focus Areas
- scripts/sonar-scan.js[92-147]
- scripts/vault-helper.sh[3-13]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| SECRET_DEFS="${VAULT_SECRET_PATHS:-$DEFAULT_VAULT_SECRET_DEFS}" | ||
| REQUIRED_VARS="${VAULT_REQUIRED_VARS:-$DEFAULT_VAULT_REQUIRED_VARS}" | ||
|
|
||
| vault_helper::load_from_definitions "$SECRET_DEFS" "$REQUIRED_VARS" "$VAULT_TOKEN_FILE" |
There was a problem hiding this comment.
4. Vault prompts can hang 🐞 Bug ⛯ Reliability
• .env unconditionally calls vault_helper::load_from_definitions, which may invoke interactive read prompts when a token is missing/invalid. • scripts/sonar-scan.js runs .env inside a spawned bash with stdin inherited; in non-interactive contexts this can block until timeout. • This makes npm run sonar fragile on machines that have vault+jq installed but no valid token file/env configured.
Agent Prompt
### Issue description
Running `.env` can trigger interactive Vault login prompts, which can hang `npm run sonar` in non-interactive environments.
### Issue Context
`.env` always calls `vault_helper::load_from_definitions`, and the helper will prompt via `read` if a token is missing/invalid.
### Fix Focus Areas
- .env[29-47]
- scripts/vault-helper.sh[59-84]
- scripts/vault-helper.sh[205-220]
- scripts/sonar-scan.js[92-147]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/sonar-scan.js (2)
265-270:⚠️ Potential issue | 🟡 MinorRaw URL leaked in error log despite redaction elsewhere.
Line 268 logs the full
sonarUrlin the error message, but line 273 redacts it viaredactUrlForLogs. For consistency with the redaction policy, this error path should also redact.Proposed fix
- console.error(`Error: SONAR_URL is not a valid URL: ${sonarUrl}`); + console.error(`Error: SONAR_URL is not a valid URL: ${redactUrlForLogs(sonarUrl)}`);
289-308:⚠️ Potential issue | 🟠 MajorUnhandled crash if
sonar-project.propertiesis missing.
fs.readFileSyncon line 291 throwsENOENTif the file doesn't exist, and this code runs outside thetry/catchblock (line 323). The script will crash with an unhandled exception instead of falling back to defaults.Proposed fix
const propertiesPath = path.join(__dirname, '..', 'sonar-project.properties'); -const propertiesContent = fs.readFileSync(propertiesPath, 'utf8'); const properties = {}; -// Simple parser for properties file -propertiesContent.split('\n').forEach((line) => { - line = line.trim(); - if (line && !line.startsWith('#')) { - const delimiterIndex = line.indexOf('='); - if (delimiterIndex === -1) { - return; - } - const key = line.slice(0, delimiterIndex).trim(); - const value = line.slice(delimiterIndex + 1).trim(); - if (key && value) { - properties[key] = value; +if (fs.existsSync(propertiesPath)) { + const propertiesContent = fs.readFileSync(propertiesPath, 'utf8'); + propertiesContent.split('\n').forEach((line) => { + line = line.trim(); + if (line && !line.startsWith('#')) { + const delimiterIndex = line.indexOf('='); + if (delimiterIndex === -1) { + return; + } + const key = line.slice(0, delimiterIndex).trim(); + const value = line.slice(delimiterIndex + 1).trim(); + if (key && value) { + properties[key] = value; + } } - } -}); + }); +} else { + console.warn('sonar-project.properties not found; using defaults'); +}
🤖 Fix all issues with AI agents
In @.github/workflows/secrets-gate.yml:
- Around line 25-33: Add SHA256 checksum verification for the downloaded
gitleaks release: in the "Install gitleaks" step (where GITLEAKS_VERSION is used
and gitleaks.tar.gz is saved), also curl the corresponding checksums file from
the same release, extract the expected checksum for
gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz, compute the SHA256 of the
downloaded gitleaks.tar.gz (e.g., using sha256sum) and compare them, and exit
non‑zero if they do not match; alternatively replace this manual install with
the official gitleaks/gitleaks-action which performs verification automatically.
In `@scripts/sonar-scan.js`:
- Around line 184-190: The spawnSync call can break on Windows when
useWindowsShell is true because cmd.exe will interpret metacharacters in the
`args` built from `scannerOptions`; update the logic around `scannerBinary`,
`useWindowsShell`, `args`, and the `spawnSync` invocation to avoid cmd.exe
parsing: prefer the `.exe` scanner if available (check for an `.exe` sibling of
`scannerBinary` and use it instead), and if you must invoke the `.bat`,
wrap/escape each `-D${key}=${value}` arg for cmd.exe (quote each argument and
escape internal quotes/percent signs appropriately) before passing to
`spawnSync` with `shell: true`; alternatively, set `shell: false` and invoke the
`.bat` via `cmd.exe /c` with safely quoted arguments to ensure metacharacters
are not treated by the shell.
🧹 Nitpick comments (3)
scripts/capture-ui-screenshot.js (1)
379-388: Consider explicit timeouts onwaitForFunctioncalls for consistency.Throughout the file,
waitForSelectorandwaitForcalls use an explicit{ timeout: 10000 }, but the threewaitForFunctioncalls (lines 380, 421, 443) rely on Playwright's default 30 s timeout. If the intent is to fail fast on broken UI, adding an explicit timeout would keep behaviour consistent and make the expected wait budget obvious to future maintainers.Example for one of the call sites
- await page.waitForFunction((totalFiles) => { - const fileTreeRoot = document.querySelector('.file-tree'); - if (!fileTreeRoot) { - return false; - } - const summaryText = fileTreeRoot.textContent || ''; - return summaryText.includes(`of ${totalFiles} files selected`); - }, MOCK_TOTAL_FILE_COUNT); + await page.waitForFunction((totalFiles) => { + const fileTreeRoot = document.querySelector('.file-tree'); + if (!fileTreeRoot) { + return false; + } + const summaryText = fileTreeRoot.textContent || ''; + return summaryText.includes(`of ${totalFiles} files selected`); + }, MOCK_TOTAL_FILE_COUNT, { timeout: 10000 });Also applies to: 420-428, 442-446
.github/workflows/secrets-gate.yml (2)
35-46: Single-SHA fallback scans the entire history — confirm this is intentional.When
github.event.beforeis all zeros (new branch push) or onworkflow_dispatch, Line 45 emits a single SHA. Passing that togitleaks git --log-opts=<sha>is equivalent togit log <sha>, which walks the commit and all its ancestors — a full-history scan. This is a sensible default for a first-push or manual audit, but it's worth adding an inline comment so future maintainers don't mistake it for a single-commit scan.
19-23: Add version comments to pinned SHA references for maintainability.Pinned commit SHAs ensure supply-chain security, but become opaque over time. A trailing version comment clarifies which release is deployed. Both actions in this file should be updated with their corresponding version tags.
📝 Suggested changes
- name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0- name: Upload gitleaks report if: always() - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0
| - name: Install gitleaks | ||
| env: | ||
| GITLEAKS_VERSION: 8.24.3 | ||
| run: | | ||
| set -euo pipefail | ||
| curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" -o gitleaks.tar.gz | ||
| tar -xzf gitleaks.tar.gz gitleaks | ||
| chmod +x gitleaks | ||
| echo "${PWD}" >> "${GITHUB_PATH}" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
gitleaks releases GitHub checksums SHA256
💡 Result:
Yes—Gitleaks publishes SHA256 checksums on GitHub Releases:
- In each release, there is a dedicated asset named like
gitleaks_<version>_checksums.txt(e.g.,gitleaks_8.30.0_checksums.txt) that contains SHA256 hashes for the release artifacts. [1] - Additionally, GitHub itself displays an SHA256 digest next to each uploaded release asset (computed by GitHub at upload time), so you can verify without relying only on the project-provided checksum file. [2]
To verify locally, GitHub recommends using the CLI command gh release verify-asset <tag> <artifact-path>. [3]
Sources: [1] [2] [3]
🌐 Web query:
gitleaks-action GitHub official repository
💡 Result:
The official GitHub repository for gitleaks-action is gitleaks/gitleaks-action. [1]
https://github.com/gitleaks/gitleaks-action
Verify the downloaded gitleaks binary's checksum to mitigate supply-chain risk.
The gitleaks binary is fetched directly from GitHub Releases without verifying its SHA256 digest. If the release asset were ever tampered with, a malicious binary would be installed silently. Gitleaks publishes SHA256 checksums for each release in a dedicated checksum file, and GitHub also displays digests alongside each asset.
🔒 Proposed fix — add checksum verification
- name: Install gitleaks
env:
GITLEAKS_VERSION: 8.24.3
+ GITLEAKS_SHA256: "<expected-sha256-for-linux-x64-tarball>"
run: |
set -euo pipefail
curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" -o gitleaks.tar.gz
+ echo "${GITLEAKS_SHA256} gitleaks.tar.gz" | sha256sum -c -
tar -xzf gitleaks.tar.gz gitleaks
chmod +x gitleaks
echo "${PWD}" >> "${GITHUB_PATH}"Alternatively, use the official gitleaks/gitleaks-action, which handles installation and verification automatically.
🤖 Prompt for AI Agents
In @.github/workflows/secrets-gate.yml around lines 25 - 33, Add SHA256 checksum
verification for the downloaded gitleaks release: in the "Install gitleaks" step
(where GITLEAKS_VERSION is used and gitleaks.tar.gz is saved), also curl the
corresponding checksums file from the same release, extract the expected
checksum for gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz, compute the SHA256
of the downloaded gitleaks.tar.gz (e.g., using sha256sum) and compare them, and
exit non‑zero if they do not match; alternatively replace this manual install
with the official gitleaks/gitleaks-action which performs verification
automatically.
| const args = Object.entries(scannerOptions).map(([key, value]) => `-D${key}=${value}`); | ||
| const useWindowsShell = process.platform === 'win32' && scannerBinary.toLowerCase().endsWith('.bat'); | ||
| const result = spawnSync(scannerBinary, args, { | ||
| cwd: projectRoot, | ||
| stdio: 'inherit', | ||
| shell: useWindowsShell, | ||
| }); |
There was a problem hiding this comment.
On Windows with .bat scanner, shell: true can misinterpret special characters in -D arguments.
When useWindowsShell is true, spawnSync delegates to cmd.exe, which interprets shell metacharacters (&, |, ^, >, etc.) in the argument values. If any scanner option value contains these characters (e.g., from sonar.exclusions glob patterns), the command may break or behave unexpectedly.
Consider quoting each argument for cmd.exe or using the .exe variant of the scanner if available.
🤖 Prompt for AI Agents
In `@scripts/sonar-scan.js` around lines 184 - 190, The spawnSync call can break
on Windows when useWindowsShell is true because cmd.exe will interpret
metacharacters in the `args` built from `scannerOptions`; update the logic
around `scannerBinary`, `useWindowsShell`, `args`, and the `spawnSync`
invocation to avoid cmd.exe parsing: prefer the `.exe` scanner if available
(check for an `.exe` sibling of `scannerBinary` and use it instead), and if you
must invoke the `.bat`, wrap/escape each `-D${key}=${value}` arg for cmd.exe
(quote each argument and escape internal quotes/percent signs appropriately)
before passing to `spawnSync` with `shell: true`; alternatively, set `shell:
false` and invoke the `.bat` via `cmd.exe /c` with safely quoted arguments to
ensure metacharacters are not treated by the shell.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/components/SourceTab.tsx (1)
247-256:⚠️ Potential issue | 🟡 Minor
forEachcallbacks implicitly return a value (Biome lint error).
onFileSelect/onFolderSelect(typed asSelectionHandler) may return a value. Arrow shorthand(x) => fn(x)implicitly returns it, which is flagged by Biome forforEachcallbacks since the return value is discarded. Use a block body to silence this and make intent explicit.Proposed fix
- selectedFiles.forEach((filePath) => onFileSelect(filePath, false)); - selectedFolders.forEach((folderPath) => onFolderSelect(folderPath, false)); + selectedFiles.forEach((filePath) => { onFileSelect(filePath, false); }); + selectedFolders.forEach((folderPath) => { onFolderSelect(folderPath, false); });.gitignore (1)
64-67:⚠️ Potential issue | 🟡 MinorConsider using
.env.exampleinstead of committing.env.The
.envfile is currently tracked by git (it's not listed in.gitignore). While the current file contains only Vault loader scaffolding with no actual secrets, this pattern deviates from standard security practices and creates risk for future development:
- Even with good intentions, developers may accidentally add secrets to
.env- Shared secret reference paths (e.g.,
kv/Sonarqube/sonarqube,kv/dependencytrack) in a committed file can leak vault structure information- The wildcard pattern
.env.*protects environment-specific files, but the base.envremains exposedRecommended approach: Add
.envto.gitignore(line 65) and create.env.exampleto document the Vault loader structure for developers to copy locally. This maintains the scaffolding benefits while following security best practices.Current .gitignore section
# Environment variables and local config .env.local .env.* .scannerwork/
🤖 Fix all issues with AI agents
In `@src/renderer/components/ProcessedTab.tsx`:
- Line 214: The element in ProcessedTab.tsx uses conflicting Tailwind utilities
"min-h-48" and "min-h-0" in the className string; remove the one that does not
match the intended behavior (keep "min-h-48" only if you want a 12rem minimum
height, or keep "min-h-0" only if you need the element to fully shrink in a flex
container) and update the className on the element that currently reads
"min-h-48 min-h-0 flex-1 overflow-auto rounded-md border ..." accordingly.
🧹 Nitpick comments (1)
src/renderer/components/TabBar.tsx (1)
17-34: ARIA tab roles are well-applied; consider adding keyboard navigation for full compliance.The
role="tablist",role="tab",aria-selected, andaria-controlsattributes correctly implement the ARIA tabs pattern. However, the WAI-ARIA Tabs Pattern also expects:
tabIndex={activeTab === tab.id ? 0 : -1}so only the active tab is in the tab sequence.- Arrow-key navigation between tabs (
ArrowLeft/ArrowRight).This is not a blocker but would complete the accessibility story.
| <div | ||
| id='processed-content' | ||
| className='max-h-[45vh] min-h-48 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm' | ||
| className='min-h-48 min-h-0 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm' |
There was a problem hiding this comment.
Conflicting Tailwind min-h utilities: min-h-48 and min-h-0.
Both min-h-48 (12rem) and min-h-0 (0px) are applied to the same element. In Tailwind, the winner depends on CSS specificity/order in the generated stylesheet, which is non-deterministic from the class list order in HTML. If you want a minimum height with flex shrinking, use only min-h-48. If you need the element to shrink below that in a flex context, min-h-0 alone is correct.
Proposed fix (pick one intent)
If the intent is a 12rem minimum:
- className='min-h-48 min-h-0 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm'
+ className='min-h-48 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm'If the intent is to allow full flex shrinking:
- className='min-h-48 min-h-0 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm'
+ className='min-h-0 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm'📝 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.
| className='min-h-48 min-h-0 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm' | |
| className='min-h-48 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm' |
| className='min-h-48 min-h-0 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm' | |
| className='min-h-0 flex-1 overflow-auto rounded-md border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-700 p-4 shadow-sm' |
🤖 Prompt for AI Agents
In `@src/renderer/components/ProcessedTab.tsx` at line 214, The element in
ProcessedTab.tsx uses conflicting Tailwind utilities "min-h-48" and "min-h-0" in
the className string; remove the one that does not match the intended behavior
(keep "min-h-48" only if you want a 12rem minimum height, or keep "min-h-0" only
if you need the element to fully shrink in a flex container) and update the
className on the element that currently reads "min-h-48 min-h-0 flex-1
overflow-auto rounded-md border ..." accordingly.
1 participant
Summary
.env+.env.local) and fallback behaviorSecrets Gateworkflow)Security Incident Handling
Validation
make sonarnpm run gitleaksnpm run lintnpm test -- --runInBandnpm run build:css && npm run build:webpack && npm run qa:screenshotSummary by CodeRabbit
New Features
Improvements
Documentation