Skip to content

Commit 7728cde

Browse files
themr0cclaude
andcommitted
fix: resolve SonarCloud security hotspots and reduce code duplication
Replace execSync shell commands with Node.js native APIs (renameSync, copyFileSync, https.get) to eliminate command injection risks (S4721). Extract shared spawnCapture helper with explicit SAFE_PATH to fix PATH security warnings (S4036) and reduce duplication between buildTitle and runHtmltest from 4.6% to under 3%. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a22a5b0 commit 7728cde

1 file changed

Lines changed: 99 additions & 93 deletions

File tree

build/scripts/build-orchestrator.js

Lines changed: 99 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@
1111
* node build/scripts/build-orchestrator.js -b main --jobs 4
1212
*/
1313

14-
import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync, readdirSync } from 'node:fs';
14+
import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync, readdirSync, renameSync, copyFileSync } from 'node:fs';
1515
import { resolve, dirname, join } from 'node:path';
16-
import { execSync, spawn } from 'node:child_process';
16+
import { spawn } from 'node:child_process';
1717
import { cpus } from 'node:os';
1818
import { createRequire } from 'node:module';
1919
import { fileURLToPath } from 'node:url';
20+
import { get as httpsGet } from 'node:https';
2021
const __filename = fileURLToPath(import.meta.url);
2122
const __dirname = dirname(__filename);
2223

@@ -30,6 +31,7 @@ const EXCLUDED_TITLES = /rhdh-plugins-reference/;
3031
const CCUTIL_IMAGE = 'quay.io/ivanhorvath/ccutil:amazing';
3132
const HTMLTEST_IMAGE = 'docker.io/wjdp/htmltest:latest';
3233
const PAGES_BASE = 'https://redhat-developer.github.io/red-hat-developers-documentation-rhdh';
34+
const SAFE_PATH = '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin';
3335

3436
// ── Argument parsing ─────────────────────────────────────────────────────────
3537

@@ -127,36 +129,22 @@ class Semaphore {
127129
}
128130
}
129131

130-
// ── Single title build ───────────────────────────────────────────────────────
132+
// ── Spawn helper (shared by buildTitle and runHtmltest) ─────────────────────
131133

132-
function buildTitle(title, branch, repoRoot, verbose) {
134+
function spawnCapture(command, args, { cwd, verbose, groupName }) {
133135
return new Promise((resolve) => {
134136
const start = Date.now();
135-
const dest = join('titles-generated', branch, title.name);
136137
let output = '';
137138

138-
// Clean previous build artifacts
139-
const buildDir = join(repoRoot, title.dir, 'build');
140-
try { rmSync(buildDir, { recursive: true, force: true }); } catch {}
141-
142-
// Build with ccutil via Podman
143-
const podmanArgs = [
144-
'run', '--rm',
145-
'--volume', `${repoRoot}:/docs:z`,
146-
'--workdir', `/docs/${title.dir}`,
147-
CCUTIL_IMAGE,
148-
'ccutil', 'compile',
149-
'--format', 'html-single',
150-
'--lang', 'en-US',
151-
'--doctype', 'article',
152-
];
153-
154-
if (verbose) {
155-
console.log(`::group::${title.name}`);
156-
console.log(`podman ${podmanArgs.join(' ')}`);
139+
if (verbose && groupName) {
140+
console.log(`::group::${groupName}`);
141+
console.log(`${command} ${args.join(' ')}`);
157142
}
158143

159-
const proc = spawn('podman', podmanArgs, { cwd: repoRoot });
144+
const proc = spawn(command, args, {
145+
cwd,
146+
env: { ...process.env, PATH: SAFE_PATH },
147+
});
160148

161149
proc.stdout.on('data', (data) => {
162150
output += data.toString();
@@ -169,45 +157,68 @@ function buildTitle(title, branch, repoRoot, verbose) {
169157
});
170158

171159
proc.on('close', (code) => {
172-
if (verbose) console.log('::endgroup::');
173-
160+
if (verbose && groupName) console.log('::endgroup::');
174161
const duration = Math.round((Date.now() - start) / 1000);
175-
176-
if (code !== 0) {
177-
resolve({ title: title.name, status: 'failed', duration, output, errors: [] });
178-
return;
179-
}
180-
181-
// Move compiled output to destination
182-
const srcDir = join(repoRoot, title.dir, 'build', 'tmp', 'en-US', 'html-single');
183-
const destDir = join(repoRoot, dest);
184-
try {
185-
rmSync(destDir, { recursive: true, force: true });
186-
execSync(`mv -f "${srcDir}" "${destDir}"`, { cwd: repoRoot });
187-
} catch (err) {
188-
resolve({ title: title.name, status: 'failed', duration, output: output + '\n' + err.message, errors: [] });
189-
return;
190-
}
191-
192-
// Copy referenced images
193-
try {
194-
copyImages(destDir, repoRoot);
195-
} catch (err) {
196-
// Non-fatal: image copy failures shouldn't fail the build
197-
output += `\nWarning: image copy issue: ${err.message}`;
198-
}
199-
200-
resolve({ title: title.name, status: 'passed', duration, output, errors: [] });
162+
resolve({ code, duration, output });
201163
});
202164

203165
proc.on('error', (err) => {
204-
if (verbose) console.log('::endgroup::');
166+
if (verbose && groupName) console.log('::endgroup::');
205167
const duration = Math.round((Date.now() - start) / 1000);
206-
resolve({ title: title.name, status: 'failed', duration, output: err.message, errors: [] });
168+
resolve({ code: 1, duration, output: err.message });
207169
});
208170
});
209171
}
210172

173+
// ── Single title build ───────────────────────────────────────────────────────
174+
175+
async function buildTitle(title, branch, repoRoot, verbose) {
176+
const dest = join('titles-generated', branch, title.name);
177+
178+
// Clean previous build artifacts
179+
const buildDir = join(repoRoot, title.dir, 'build');
180+
try { rmSync(buildDir, { recursive: true, force: true }); } catch {}
181+
182+
const podmanArgs = [
183+
'run', '--rm',
184+
'--volume', `${repoRoot}:/docs:z`,
185+
'--workdir', `/docs/${title.dir}`,
186+
CCUTIL_IMAGE,
187+
'ccutil', 'compile',
188+
'--format', 'html-single',
189+
'--lang', 'en-US',
190+
'--doctype', 'article',
191+
];
192+
193+
const { code, duration, output } = await spawnCapture('podman', podmanArgs, {
194+
cwd: repoRoot, verbose, groupName: title.name,
195+
});
196+
197+
if (code !== 0) {
198+
return { title: title.name, status: 'failed', duration, output, errors: [] };
199+
}
200+
201+
// Move compiled output to destination
202+
const srcDir = join(repoRoot, title.dir, 'build', 'tmp', 'en-US', 'html-single');
203+
const destDir = join(repoRoot, dest);
204+
try {
205+
rmSync(destDir, { recursive: true, force: true });
206+
renameSync(srcDir, destDir);
207+
} catch (err) {
208+
return { title: title.name, status: 'failed', duration, output: output + '\n' + err.message, errors: [] };
209+
}
210+
211+
// Copy referenced images
212+
let finalOutput = output;
213+
try {
214+
copyImages(destDir, repoRoot);
215+
} catch (err) {
216+
finalOutput += `\nWarning: image copy issue: ${err.message}`;
217+
}
218+
219+
return { title: title.name, status: 'passed', duration, output: finalOutput, errors: [] };
220+
}
221+
211222
function copyImages(destDir, repoRoot) {
212223
const indexPath = join(destDir, 'index.html');
213224
if (!existsSync(indexPath)) return;
@@ -227,49 +238,23 @@ function copyImages(destDir, repoRoot) {
227238
const srcImage = resolve(repoRoot, im);
228239
if (existsSync(srcImage)) {
229240
try {
230-
execSync(`rsync -q "${srcImage}" "${imDir}/"`, { cwd: repoRoot });
241+
copyFileSync(srcImage, join(imDir, im.split('/').pop()));
231242
} catch {}
232243
}
233244
}
234245
}
235246

236247
// ── htmltest ─────────────────────────────────────────────────────────────────
237248

238-
function runHtmltest(repoRoot, verbose) {
239-
return new Promise((resolve) => {
240-
const start = Date.now();
241-
let output = '';
242-
243-
if (verbose) console.log('::group::htmltest');
244-
245-
const proc = spawn('podman', [
246-
'run', '--rm',
247-
'--volume', `${repoRoot}:/test:z`,
248-
HTMLTEST_IMAGE,
249-
'-c', '.htmltest.yml',
250-
], { cwd: repoRoot });
251-
252-
proc.stdout.on('data', (data) => {
253-
output += data.toString();
254-
if (verbose) process.stdout.write(data);
255-
});
256-
257-
proc.stderr.on('data', (data) => {
258-
output += data.toString();
259-
if (verbose) process.stderr.write(data);
260-
});
249+
async function runHtmltest(repoRoot, verbose) {
250+
const { code, duration, output } = await spawnCapture('podman', [
251+
'run', '--rm',
252+
'--volume', `${repoRoot}:/test:z`,
253+
HTMLTEST_IMAGE,
254+
'-c', '.htmltest.yml',
255+
], { cwd: repoRoot, verbose, groupName: 'htmltest' });
261256

262-
proc.on('close', (code) => {
263-
if (verbose) console.log('::endgroup::');
264-
const duration = Math.round((Date.now() - start) / 1000);
265-
resolve({ status: code === 0 ? 'passed' : 'failed', duration, output });
266-
});
267-
268-
proc.on('error', (err) => {
269-
if (verbose) console.log('::endgroup::');
270-
resolve({ status: 'failed', duration: 0, output: err.message });
271-
});
272-
});
257+
return { status: code === 0 ? 'passed' : 'failed', duration, output };
273258
}
274259

275260
// ── Index HTML generation ────────────────────────────────────────────────────
@@ -287,15 +272,36 @@ function generateBranchIndex(branch, results, repoRoot) {
287272
writeFileSync(join(indexDir, 'index.html'), html);
288273
}
289274

290-
function updateRootIndex(branch, repoRoot) {
275+
function fetchUrl(url) {
276+
return new Promise((resolve, reject) => {
277+
httpsGet(url, (res) => {
278+
if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) {
279+
fetchUrl(res.headers.location).then(resolve, reject);
280+
return;
281+
}
282+
if (res.statusCode !== 200) {
283+
res.resume();
284+
reject(new Error(`HTTP ${res.statusCode}`));
285+
return;
286+
}
287+
let data = '';
288+
res.on('data', (chunk) => { data += chunk; });
289+
res.on('end', () => resolve(data));
290+
res.on('error', reject);
291+
}).on('error', reject);
292+
});
293+
}
294+
295+
async function updateRootIndex(branch, repoRoot) {
291296
const isPR = branch.startsWith('pr-');
292297
const indexFile = isPR ? 'pulls.html' : 'index.html';
293298
const indexPath = join(repoRoot, 'titles-generated', indexFile);
294299
const url = `${PAGES_BASE}/${indexFile}`;
295300

296301
// Fetch existing index from GitHub Pages
297302
try {
298-
execSync(`curl -sSL "${url}" -o "${indexPath}"`, { cwd: repoRoot, timeout: 30000 });
303+
const data = await fetchUrl(url);
304+
writeFileSync(indexPath, data);
299305
} catch {
300306
// If fetch fails, create a minimal file
301307
writeFileSync(indexPath, '<html><body><ul>\n</ul></body></html>');
@@ -447,7 +453,7 @@ async function main() {
447453
generateBranchIndex(args.branch, buildResults, repoRoot);
448454

449455
// Update root index
450-
updateRootIndex(args.branch, repoRoot);
456+
await updateRootIndex(args.branch, repoRoot);
451457

452458
// Run htmltest
453459
console.log('\nRunning link validation (htmltest)...');

0 commit comments

Comments
 (0)