W4.5 RCE Migration Pattern (Sonnet batch spec)

Plan: oom-cascade-recovery-2026-04-14 \u00b7 Section: logical-dancing-book.md \u00a758 W4.5 \u00b7 Governance: \u00a739 / \u00a743 / \u00a748 / \u00a749

Canonical first site (pattern established 2026-04-17 by Opus): F884 \u2014 workspace-atlas/scripts/atlas-auto-button-handler.js lines 31-34.

Verified: 3/3 regression tests pass; file is node --check clean; w3_findings row updated to status=FIXED.

Sonnet: apply this pattern uniformly to the ~50 remaining OPEN + CRITICAL RCE findings. Each site gets its own regression test following the 3-assertion template below.

The vulnerability class

Every site has the shape:

const { execSync } = require('child_process');
// ...
execSync(
  `node ${handlerScript} ${userControlledA} ${userControlledB}`,
  { cwd: WS, ... }
);

Where one or more of the interpolated values originate from an attacker-controllable input surface \u2014 Discord interaction payload, webhook body, URL parameter, SMS text, etc.

The architectural fix (no band-aid, per \u00a743)

Replace with:

const { execFileSync } = require('child_process');
// ...
// 1. Validate every user-controlled input before spawn (fail-closed)
const errors = validateInputs({ /* ... */ });
if (errors.length) return { success: false, error: errors.join('; ') };
 
// 2. Spawn via argv-array (no shell; metacharacters are literal)
execFileSync(
  'node',                                         // executable
  [handlerScript, valueA, valueB, valueC],        // argv array
  { cwd: WS, stdio: 'pipe', encoding: 'utf8', timeout: 60_000 }
);

Three mandatory changes per site:

require('child_process') import switches execSync \u2192 execFileSync (or exec \u2192 execFile)
Add an input validator block at the top of the file (or import from lib/safe-exec.js validators)
Convert the shell-string call to argv-array form

Input validators (copy from safe-exec.js where possible)

Input kind	Regex / validator	Fail-closed behavior
Discord snowflake (channelId, userId, messageId)	`/^\d{17,20}$/`	return `{ success: false, error: ... }`
HubSpot object ID	`/^\d{1,19}$/`	reject with 400 if in HTTP context
UUID v4	canonical RFC 4122	reject
E.164 phone	`/^\+[1-9]\d{10,14}$/`	reject
Free-text label (buttonLabel, SMS text)	max length 200 + no `/[\x00-\x08\x0e-\x1f`]/`	reject with message “forbidden control characters”
Path-like	no `..` segment, no `/[;	&`$<>]/`

Prefer: const { validators } = require('/home/opsadmin/.openclaw/workspace/scripts/lib/safe-exec');

For sites spawning fire-and-forget background work (not waiting for completion), use safeSpawnDetached from the same lib (log-file redirect + detached:true + unref() built in).

Regression test template (§49 mandatory)

Each migration ships a scripts/tests/test-<site-key>-injection.js with THREE assertions minimum:

const assert = require('assert');
const Module = require('module');
const realCP = require('child_process');
const calls = [];
 
// Stub child_process BEFORE requiring module under test
const origLoad = Module._load;
Module._load = function (req, parent, ...rest) {
  if (req === 'child_process') {
    return {
      ...realCP,
      execSync:      (cmd, opts)        => { calls.push({ fn: 'execSync', cmd, opts }); return ''; },
      execFileSync:  (file, args, opts) => { calls.push({ fn: 'execFileSync', file, args, opts }); return ''; },
      exec:          (cmd, opts, cb)    => { calls.push({ fn: 'exec', cmd, opts }); if (cb) cb(null, '', ''); },
      execFile:      (file, args, opts, cb) => { calls.push({ fn: 'execFile', file, args, opts }); if (cb) cb(null, '', ''); },
    };
  }
  return origLoad.call(this, req, parent, ...rest);
};
 
const mod = require('../<site-under-test>.js');
 
// Assertion 1 \u2014 SAFE PATH: valid inputs reach execFileSync/execFile with argv ARRAY
await test('valid inputs dispatch via argv-array (no execSync, no shell)', async () => {
  calls.length = 0;
  await mod.entrypoint({ /* valid Discord-shaped inputs */ });
  assert.strictEqual(calls.filter(c => /^execSync|^exec$/.test(c.fn)).length, 0);
  const goodCall = calls.find(c => c.fn === 'execFileSync' || c.fn === 'execFile');
  assert.ok(goodCall, 'expected execFileSync/execFile call');
  assert.ok(Array.isArray(goodCall.args), 'args must be array, not shell string');
});
 
// Assertion 2 \u2014 INJECTION NEUTRALIZED: adversarial payload does not reach shell
await test('adversarial input does not cause shell invocation', async () => {
  calls.length = 0;
  const adversarial = ADVERSARIAL_PAYLOAD_FOR_THIS_INPUT_SURFACE;   // see table below
  await mod.entrypoint({ /* with adversarial value substituted */ });
  assert.strictEqual(calls.filter(c => /^execSync|^exec$/.test(c.fn)).length, 0,
    'MUST NOT invoke shell on adversarial input');
  // sentinel file check: if injection worked, attacker would have touched a well-known path
  assert.ok(!require('fs').existsSync(SENTINEL_PATH),
    'shell injection sentinel must never be created');
});
 
// Assertion 3 \u2014 VALIDATOR REJECTS NON-CONFORMING INPUT
await test('malformed input type rejected by validator', async () => {
  calls.length = 0;
  const result = await mod.entrypoint({ /* non-snowflake channelId or similar */ });
  // Either rejected with error OR passed through as literal (never shell)
  assert.strictEqual(calls.filter(c => /^execSync|^exec$/.test(c.fn)).length, 0);
});

Adversarial payloads by input surface

Input surface	Payload to test in Assertion 2	Sentinel path
Discord buttonLabel	a string with embedded semicolon + touch command (construct programmatically, do NOT paste literals in notes fields \u2014 Cloudflare blocks them)	`/tmp/atlas-pwned-should-never-exist-$$` (actual + string-interp; if actual file appears, injection worked)
Discord customId	semicolon + shell cmd appended	same pattern
Webhook dealId	`'; DROP TABLE...'` style	sentinel file in /tmp
OpenPhone callId	Same	same
SMS message body	Same	same

Construct payloads in the test with String.fromCharCode(0x22, 0x3b, ...) or template pieces so the SQL notes field never contains the literal adversarial string (avoids Cloudflare WAF rejecting UPDATE w3_findings requests).

Per-site workflow (for Sonnet)

For each target F-number in the RCE batch queue:

Query: SELECT f_number, file_path, line_numbers, architectural_fix FROM w3_findings WHERE f_number='Fxxxx'
Read the file to understand the shape.
Write scripts/tests/test-<site-key>-injection.js with the 3-assertion template tailored to the specific user-controlled inputs for this site.
Run the test \u2014 verify it FAILS on current code (proves it catches the bug).
Apply the migration: execSync \u2192 execFileSync + add validator + argv-array form.
Run node --check on the file.
Run the regression test \u2014 verify PASSES.
Capture sha256sum of the modified source file and the test file.
UPDATE w3_findings SET status='FIXED', fixed_at=NOW(), fix_commit_hash='sha256:<hash>', notes='W4.5 batch fix 2026-04-17 via Sonnet against pattern docs/w4-5-rce-migration-pattern.md. <one-line summary>. Regression test <test-file-path> sha256:<hash> 3/3 passing.' WHERE f_number='Fxxxx'
Report per-site: { f_number, file, test_file, lines_changed, test_result: "3/3 pass" }.

Output format to return to Opus (§model-routing dispatch rule #8)

{
  "batch_id": "W4.5-<wave-number>",
  "sites_attempted": N,
  "sites_fixed": M,
  "sites_skipped": [{ "f_number": "...", "reason": "..." }],
  "new_test_files": [ "path1", "path2", ... ],
  "updated_source_files": [ "path1", ... ],
  "w3_findings_updated": [ "F884", "F196", ... ]
}

What is NOT in this pattern (intentionally)

Process pool caps (p-queue / ExecPool): some findings recommend concurrency caps. For this migration keep scope to the injection fix only \u2014 concurrency caps are a separate fix for a separate finding class and would bloat each PR.
Refactoring to in-process require: some findings recommend removing the child process entirely by calling the target module via require(). That’s architecturally correct but is a LARGER refactor per-site \u2014 out of scope for this batch, tracked as follow-up. The injection-fix is the gate for closing the CRITICAL severity.
Git commits: workspace-atlas is not currently git-tracked. Fixes land inline; the fix_commit_hash field in w3_findings stores sha256:<content-hash> as the traceable marker until git is initialized there.

Rollback per site

git restore <file> if under git, else restore from .planning/w4-5-rollback-snapshots/<f-number>.bak (Sonnet: capture cp file file.bak before each edit if not under git).

Known traps

Cloudflare WAF blocks SQL UPDATE requests whose notes field contains shell-injection-looking strings. Build adversarial payloads programmatically in the TEST, never store them in the notes column. Use the word “adversarial” or “injection payload” instead of the literal bytes when describing in notes.
Module stub before require: the Module._load hook MUST be installed BEFORE the module-under-test is required, else the real child_process.execSync runs and the test either shells out or errors.
process.exit in handlers: some scripts call process.exit(0) on success, which kills the test harness. Use if (require.main === module) guards and stub process.exit in test if needed.
test-message-id literal: atlas code has a test-mode snowflake bypass. Keep that bypass in the validator for backward compatibility with existing test invocations.

First site reference (canonical)

Source: /home/opsadmin/.openclaw/workspace-atlas/scripts/atlas-auto-button-handler.js
Test: /home/opsadmin/.openclaw/workspace-atlas/scripts/tests/test-atlas-auto-button-handler-injection.js
w3_findings row: f_number='F884' \u2014 status='FIXED', fix_commit_hash='sha256:f0f6db6e...'
Diff summary: 5 lines added (validator block + 1 line in handler), 1 line swapped (execSync \u2192 execFileSync + argv-array)

Quartz 4

Explorer

w4-5-rce-migration-pattern