fix(security): nightwatch.conf.js — env-var creds + own-keys merge (SDK-6070/6072)

[AakashHotchandani]

⚠️ Logic-vuln changes — please human-review the behavioral impact

Two source-level security fixes in nightwatch.conf.js. Unlike the dependency/CI PRs in this audit, these change runtime behavior, so they're flagged for human review.

Ticket	CWE	Fix
SDK-6070	321	Read credentials from process.env instead of non-interpolating string literals
SDK-6072	915	Iterate own keys only (Object.keys) instead of prototype-walking for..in

SDK-6070 — credentials as JS literals

- userName: '${BROWSERSTACK_USERNAME}' || 'YOUR_USERNAME',
- accessKey: '${BROWSERSTACK_ACCESS_KEY}' || 'YOUR_ACCESS_KEY',
+ userName: process.env.BROWSERSTACK_USERNAME || 'YOUR_USERNAME',
+ accessKey: process.env.BROWSERSTACK_ACCESS_KEY || 'YOUR_ACCESS_KEY',

The single-quoted '${BROWSERSTACK_USERNAME}' does not interpolate — it's a literal string, always truthy, so the env var was never read and the || 'YOUR_...' fallback was dead code. Behavioral change to confirm: the sample now actually reads BROWSERSTACK_USERNAME / BROWSERSTACK_ACCESS_KEY from the environment (falling back to the placeholder when unset), which is the intended behavior.

SDK-6072 — prototype-chain merge

- for(let key in additonalEnvironments.test_settings) {
+ for (const key of Object.keys(additonalEnvironments.test_settings)) {

for..in enumerates inherited (prototype-chain) properties; if test_settings's prototype is polluted, attacker-controlled keys get merged into the nightwatch config. Object.keys() restricts to own enumerable properties. Functionally identical for plain objects.

Verification

• node --check nightwatch.conf.js passes.
• require('./nightwatch.conf.js') loads cleanly; test_settings resolves the same 8 environment keys (default, browserstack, browserstack.chrome/firefox/edge, env1/2/3).
• userName/accessKey fall back to placeholders when env vars are unset.
• Diff: nightwatch.conf.js only, 3 ins / 3 del.

Jira: SDK-6070, SDK-6072

🤖 Generated with Claude Code

[AakashHotchandani]

🤖 Automated review (pr-review agent)

Verdict: ✅ Approve (good to go) — one non-blocking defense-in-depth note.

Focus-area findings:

• SDK-6070 (correctness + behavior): Correct, and a strict improvement. The old '${BROWSERSTACK_USERNAME}' single-quoted literal never interpolated — the env var was never read and the || 'YOUR_...' fallback was dead code. process.env.BROWSERSTACK_USERNAME || 'YOUR_USERNAME' now reads the env var (verified against the plugin source) and falls back to the placeholder when unset. No repo code (tests, environments.js) depended on the old literal behavior, so no regression.
• SDK-6072 (Object.keys completeness): Switching for..in → for (const key of Object.keys(...)) resolves the prototype-chain enumeration (CWE-915) as the ticket prescribes. The subsequent object spread ...test_settings[key] only copies own-enumerable properties, so it does not reintroduce prototype-walk.
• Regression risk: None — node --check passes, require() loads cleanly, test_settings resolves the same 8 environment keys.

Non-blocking note (defense-in-depth): a residual prototype-pollution surface exists only if an upstream caller pollutes the nested test_settings[key] objects themselves; out of scope for this ticket and not a defect in this change.

Per-file confidence: nightwatch.conf.js → 🟢 All Clear. These are logic-vuln (behavioral) changes — human review still recommended per the PR body.