fix: 2 improvements across 2 files

[tomaioo]

Summary

fix: 2 improvements across 2 files

Problem

Severity: High | File: web/src/core/tools/generateRandomPassword.ts:L1

The generateRandomPassword function uses Math.random() for password generation, which is cryptographically insecure. Math.random() is not suitable for generating passwords or any security-sensitive tokens as it is predictable and not designed for cryptographic purposes. The function also has a flawed logic where it generates only 20 characters (2 * 10) but the approach of joining and replacing dots is fragile.

Solution

Replace Math.random() with crypto.getRandomValues() or crypto.randomBytes() for cryptographically secure random number generation. Consider using a well-tested library like crypto (Node.js) or Web Crypto API, or a dedicated password generation library. Also simplify the generation logic to be more readable and robust.

Changes

• web/src/core/tools/generateRandomPassword.ts (modified)
• web/src/core/usecases/launcher/decoupledLogic/computeRootForm/mergeRangeSliders/removeFormFieldGroupWithNoNodes.ts (modified)

Summary by CodeRabbit

• Bug Fixes
  • Improved password generation to use stronger random values and produce a consistent 20-character password.
  • Fixed handling of empty form field groups so they are removed more reliably when they can no longer be used.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR contains two unrelated changes: the random password generator now derives passwords from crypto.getRandomValues mapped to base-36 characters instead of Math.random(), and the form field group pruning logic switches from a forEach callback to a reverse-index for loop for safer in-place splicing.

Changes

Password Generation Update

Layer / File(s)	Summary
Crypto-based password generation web/src/core/tools/generateRandomPassword.ts	Replaces Math.random()-based password generation with crypto.getRandomValues filling a Uint8Array(20), converting each byte to base-36 and joining into the password.

Form Field Group Pruning

Layer / File(s)	Summary
Reverse-index removal loop web/src/core/usecases/launcher/decoupledLogic/computeRootForm/mergeRangeSliders/removeFormFieldGroupWithNoNodes.ts	Replaces forEach traversal with a reverse-index for loop that uses nodes.splice(i, 1) to remove empty, non-addable group nodes while preserving field/skip conditions.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is generic and does not clearly state the main changes in the PR.	Use a specific title like 'Harden random password generation and fix empty group removal logic'.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

web/src/core/tools/generateRandomPassword.ts (1)

2-4: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Modulo bias in byte-to-char mapping.

byte % 36 is biased since 256 isn't evenly divisible by 36 — values 0–3 ('0'-'3' in base-36) occur ~7.11/7.03 times more often than others across many draws, weakening the intended cryptographic uniformity this change was meant to achieve. Consider rejection sampling, or use a well-vetted library (e.g., nanoid) for generating secure random strings.

🔒 Proposed fix using rejection sampling

 export function generateRandomPassword() {
-    const array = new Uint8Array(20);
-    crypto.getRandomValues(array);
-    return Array.from(array, byte => (byte % 36).toString(36)).join("");
+    const chars = "0123456789abcdefghijklmnopqrstuvwxyz";
+    const maxValid = 256 - (256 % chars.length);
+    let password = "";
+    const buffer = new Uint8Array(1);
+    while (password.length < 20) {
+        crypto.getRandomValues(buffer);
+        if (buffer[0] < maxValid) {
+            password += chars[buffer[0] % chars.length];
+        }
+    }
+    return password;
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/core/tools/generateRandomPassword.ts` around lines 2 - 4, The
password generator in generateRandomPassword is using a biased byte-to-character
mapping via byte % 36. Update the random string generation logic in
generateRandomPassword to use rejection sampling (or another uniform approach)
so each base-36 character is selected with equal probability, and keep the
cryptographic randomness source via crypto.getRandomValues.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@web/src/core/tools/generateRandomPassword.ts`:
- Around line 2-4: The password generator in generateRandomPassword is using a
biased byte-to-character mapping via byte % 36. Update the random string
generation logic in generateRandomPassword to use rejection sampling (or another
uniform approach) so each base-36 character is selected with equal probability,
and keep the cryptographic randomness source via crypto.getRandomValues.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 811a0360-5bdf-40f6-9d3b-7cc29627dae7

📥 Commits

Reviewing files that changed from the base of the PR and between 6555d01 and 74fc78a.

📒 Files selected for processing (2)

• web/src/core/tools/generateRandomPassword.ts
• web/src/core/usecases/launcher/decoupledLogic/computeRootForm/mergeRangeSliders/removeFormFieldGroupWithNoNodes.ts