Skip to content

Commit d15379b

Browse files
cyl-authcyl-auth
committed
Refactor logging in Script class to simplify handling of sensitive arguments
1 parent 5bae182 commit d15379b

File tree

1 file changed

+51
-163
lines changed

1 file changed

+51
-163
lines changed

‎utils/src/main/java/com/cloud/utils/script/Script.java‎

Lines changed: 51 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -247,16 +247,12 @@ static String stackTraceAsString(Throwable throwable) {
247247
public String execute(OutputInterpreter interpreter) {
248248
String[] command = _command.toArray(new String[_command.size()]);
249249
String commandLine = buildCommandLine(command);
250-
if (_logger.isDebugEnabled() && !avoidLoggingCommand && sensitiveArgIndices.isEmpty()) {
250+
if (_logger.isDebugEnabled() && !avoidLoggingCommand) {
251251
_logger.debug(String.format("Executing command [%s].", commandLine.split(KeyStoreUtils.KS_FILENAME)[0]));
252252
}
253253

254254
try {
255-
if (sensitiveArgIndices.isEmpty()) {
256-
_logger.trace(String.format("Creating process for command [%s].", commandLine));
257-
} else {
258-
_logger.trace("Creating process for command with sensitive arguments.");
259-
}
255+
_logger.trace(String.format("Creating process for command [%s].", commandLine));
260256
ProcessBuilder pb = new ProcessBuilder(command);
261257
pb.redirectErrorStream(true);
262258
if (_workDir != null)
@@ -274,126 +270,62 @@ public String execute(OutputInterpreter interpreter) {
274270
_thread = Thread.currentThread();
275271
ScheduledFuture<String> future = null;
276272
if (_timeout > 0) {
277-
if (sensitiveArgIndices.isEmpty()) {
278-
_logger.trace(String.format(
279-
"Scheduling the execution of command [%s] with a timeout of [%s] milliseconds.",
280-
commandLine, _timeout));
281-
} else {
282-
_logger.trace(String.format(
283-
"Scheduling the execution of command with sensitive arguments with a timeout of [%s] milliseconds.",
284-
_timeout));
285-
}
273+
_logger.trace(String.format(
274+
"Scheduling the execution of command [%s] with a timeout of [%s] milliseconds.",
275+
commandLine, _timeout));
286276
future = s_executors.schedule(this, _timeout, TimeUnit.MILLISECONDS);
287277
}
288278

289279
long processPid = _process.pid();
290280
Task task = null;
291281
if (interpreter != null && interpreter.drain()) {
292-
if (sensitiveArgIndices.isEmpty()) {
293-
_logger.trace(String.format("Executing interpreting task of process [%s] for command [%s].",
294-
processPid, commandLine));
295-
} else {
296-
_logger.trace(String.format(
297-
"Executing interpreting task of process [%s] for command with sensitive arguments.",
298-
processPid));
299-
}
282+
_logger.trace(String.format("Executing interpreting task of process [%s] for command [%s].",
283+
processPid, commandLine));
300284
task = new Task(interpreter, ir);
301285
s_executors.execute(task);
302286
}
303287

304288
while (true) {
305-
if (sensitiveArgIndices.isEmpty()) {
306-
_logger.trace(String.format("Attempting process [%s] execution for command [%s] with timeout [%s].",
307-
processPid, commandLine, _timeout));
308-
} else {
309-
_logger.trace(String.format(
310-
"Attempting process [%s] execution for command with sensitive arguments with timeout [%s].",
311-
processPid, _timeout));
312-
}
289+
_logger.trace(String.format("Attempting process [%s] execution for command [%s] with timeout [%s].",
290+
processPid, commandLine, _timeout));
313291
try {
314292
if (_process.waitFor(_timeout, TimeUnit.MILLISECONDS)) {
315-
if (sensitiveArgIndices.isEmpty()) {
316-
_logger.trace(String.format(
317-
"Process [%s] execution for command [%s] completed within timeout period [%s].",
318-
processPid, commandLine,
319-
_timeout));
320-
} else {
321-
_logger.trace(String.format(
322-
"Process [%s] execution for command with sensitive arguments completed within timeout period [%s].",
323-
processPid,
324-
_timeout));
325-
}
293+
_logger.trace(String.format(
294+
"Process [%s] execution for command [%s] completed within timeout period [%s].",
295+
processPid, commandLine,
296+
_timeout));
326297
if (_process.exitValue() == 0) {
327-
if (sensitiveArgIndices.isEmpty()) {
328-
_logger.debug(String.format("Successfully executed process [%s] for command [%s].",
329-
processPid, commandLine));
330-
} else {
331-
_logger.debug(String.format(
332-
"Successfully executed process [%s] for command with sensitive arguments.",
333-
processPid));
334-
}
298+
_logger.debug(String.format("Successfully executed process [%s] for command [%s].",
299+
processPid, commandLine));
335300
if (interpreter != null) {
336301
if (interpreter.drain()) {
337-
if (sensitiveArgIndices.isEmpty()) {
338-
_logger.trace(
339-
String.format("Returning task result of process [%s] for command [%s].",
340-
processPid, commandLine));
341-
} else {
342-
_logger.trace(String.format(
343-
"Returning task result of process [%s] for command with sensitive arguments.",
344-
processPid));
345-
}
346-
return task.getResult();
347-
}
348-
if (sensitiveArgIndices.isEmpty()) {
349302
_logger.trace(
350-
String.format("Returning interpretation of process [%s] for command [%s].",
303+
String.format("Returning task result of process [%s] for command [%s].",
351304
processPid, commandLine));
352-
} else {
353-
_logger.trace(String.format(
354-
"Returning interpretation of process [%s] for command with sensitive arguments.",
355-
processPid));
305+
return task.getResult();
356306
}
307+
_logger.trace(
308+
String.format("Returning interpretation of process [%s] for command [%s].",
309+
processPid, commandLine));
357310
return interpreter.interpret(ir);
358311
} else {
359312
// null return exitValue apparently
360-
if (sensitiveArgIndices.isEmpty()) {
361-
_logger.trace(String.format("Process [%s] for command [%s] exited with value [%s].",
362-
processPid, commandLine,
363-
_process.exitValue()));
364-
} else {
365-
_logger.trace(String.format(
366-
"Process [%s] for command with sensitive arguments exited with value [%s].",
367-
processPid,
368-
_process.exitValue()));
369-
}
313+
_logger.trace(String.format("Process [%s] for command [%s] exited with value [%s].",
314+
processPid, commandLine,
315+
_process.exitValue()));
370316
return String.valueOf(_process.exitValue());
371317
}
372318
} else {
373-
if (sensitiveArgIndices.isEmpty()) {
374-
_logger.warn(String.format("Execution of process [%s] for command [%s] failed.",
375-
processPid, commandLine));
376-
} else {
377-
_logger.warn(String.format(
378-
"Execution of process [%s] for command with sensitive arguments failed.",
379-
processPid));
380-
}
319+
_logger.warn(String.format("Execution of process [%s] for command [%s] failed.",
320+
processPid, commandLine));
381321
break;
382322
}
383323
}
384324
} catch (InterruptedException e) {
385325
if (!_isTimeOut) {
386-
if (sensitiveArgIndices.isEmpty()) {
387-
_logger.debug(String.format(
388-
"Exception [%s] occurred; however, it was not a timeout. Therefore, proceeding with the execution of process [%s] for command "
389-
+ "[%s].",
390-
e.getMessage(), processPid, commandLine), e);
391-
} else {
392-
_logger.debug(String.format(
393-
"Exception [%s] occurred; however, it was not a timeout. Therefore, proceeding with the execution of process [%s] for command "
394-
+ "with sensitive arguments.",
395-
e.getMessage(), processPid), e);
396-
}
326+
_logger.debug(String.format(
327+
"Exception [%s] occurred; however, it was not a timeout. Therefore, proceeding with the execution of process [%s] for command [%s].",
328+
e.getMessage(), processPid, commandLine), e);
397329
continue;
398330
}
399331
} finally {
@@ -406,33 +338,21 @@ public String execute(OutputInterpreter interpreter) {
406338
TimedOutLogger log = new TimedOutLogger(_process);
407339
Task timedoutTask = new Task(log, ir);
408340

409-
if (sensitiveArgIndices.isEmpty()) {
410-
_logger.trace(String.format("Running timed out task of process [%s] for command [%s].", processPid,
411-
commandLine));
412-
} else {
413-
_logger.trace(String.format(
414-
"Running timed out task of process [%s] for command with sensitive arguments.",
415-
processPid));
416-
}
341+
_logger.trace(String.format("Running timed out task of process [%s] for command [%s].", processPid,
342+
commandLine));
417343
timedoutTask.run();
418-
if (!_passwordCommand && sensitiveArgIndices.isEmpty()) {
344+
if (_passwordCommand) {
345+
_logger.warn(String.format("Process [%s] for command [%s] timed out.", processPid, commandLine));
346+
} else {
419347
_logger.warn(String.format("Process [%s] for command [%s] timed out. Output is [%s].", processPid,
420348
commandLine, timedoutTask.getResult()));
421-
} else {
422-
_logger.warn(
423-
String.format("Process [%s] for command with sensitive arguments timed out.", processPid));
424349
}
425350

426351
return ERR_TIMEOUT;
427352
}
428353

429-
if (sensitiveArgIndices.isEmpty()) {
430-
_logger.debug(String.format("Exit value of process [%s] for command [%s] is [%s].", processPid,
431-
commandLine, _process.exitValue()));
432-
} else {
433-
_logger.debug(String.format("Exit value of process [%s] for command with sensitive arguments is [%s].",
434-
processPid, _process.exitValue()));
435-
}
354+
_logger.debug(String.format("Exit value of process [%s] for command [%s] is [%s].", processPid,
355+
commandLine, _process.exitValue()));
436356

437357
BufferedReader reader = new BufferedReader(new InputStreamReader(_process.getInputStream()), 128);
438358

@@ -443,49 +363,24 @@ public String execute(OutputInterpreter interpreter) {
443363
error = String.valueOf(_process.exitValue());
444364
}
445365

446-
if (sensitiveArgIndices.isEmpty()) {
447-
_logger.warn(String.format("Process [%s] for command [%s] encountered the error: [%s].", processPid,
448-
commandLine, error));
449-
} else {
450-
_logger.warn(
451-
String.format("Process [%s] for command with sensitive arguments encountered the error: [%s].",
452-
processPid, error));
453-
}
366+
_logger.warn(String.format("Process [%s] for command [%s] encountered the error: [%s].", processPid,
367+
commandLine, error));
454368

455369
return error;
456370
} catch (SecurityException ex) {
457-
if (sensitiveArgIndices.isEmpty()) {
458-
_logger.warn(String.format(
459-
"Exception [%s] occurred. This may be due to an attempt of executing command [%s] as non root.",
460-
ex.getMessage(), commandLine),
461-
ex);
462-
} else {
463-
_logger.warn(String.format(
464-
"Exception [%s] occurred. This may be due to an attempt of executing command with sensitive data as non root.",
465-
ex.getMessage()),
466-
ex);
467-
}
371+
_logger.warn(String.format(
372+
"Exception [%s] occurred. This may be due to an attempt of executing command [%s] as non root.",
373+
ex.getMessage(), commandLine),
374+
ex);
468375
return stackTraceAsString(ex);
469376
} catch (Exception ex) {
470-
if (sensitiveArgIndices.isEmpty()) {
471-
_logger.warn(String.format("Exception [%s] occurred when attempting to run command [%s].",
472-
ex.getMessage(), commandLine), ex);
473-
} else {
474-
_logger.warn(
475-
String.format("Exception [%s] occurred when attempting to run a command with sensitive data.",
476-
ex.getMessage()),
477-
ex);
478-
}
377+
_logger.warn(String.format("Exception [%s] occurred when attempting to run command [%s].",
378+
ex.getMessage(), commandLine), ex);
479379
return stackTraceAsString(ex);
480380
} finally {
481381
if (_process != null) {
482-
if (sensitiveArgIndices.isEmpty()) {
483-
_logger.trace(
484-
String.format("Destroying process [%s] for command [%s].", _process.pid(), commandLine));
485-
} else {
486-
_logger.trace(String.format("Destroying process [%s] for a command with sensitive data.",
487-
_process.pid()));
488-
}
382+
_logger.trace(
383+
String.format("Destroying process [%s] for command [%s].", _process.pid(), commandLine));
489384
IOUtils.closeQuietly(_process.getErrorStream());
490385
IOUtils.closeQuietly(_process.getOutputStream());
491386
IOUtils.closeQuietly(_process.getInputStream());
@@ -496,10 +391,11 @@ public String execute(OutputInterpreter interpreter) {
496391

497392
public String executeIgnoreExitValue(OutputInterpreter interpreter, int exitValue) {
498393
String[] command = _command.toArray(new String[_command.size()]);
394+
String commandLine = buildCommandLine(command);
499395

500-
if (_logger.isDebugEnabled() && sensitiveArgIndices.isEmpty()) {
396+
if (_logger.isDebugEnabled() && !avoidLoggingCommand) {
501397
_logger.debug(
502-
String.format("Executing: %s", buildCommandLine(command).split(KeyStoreUtils.KS_FILENAME)[0]));
398+
String.format("Executing: %s", commandLine.split(KeyStoreUtils.KS_FILENAME)[0]));
503399
}
504400

505401
try {
@@ -510,7 +406,7 @@ public String executeIgnoreExitValue(OutputInterpreter interpreter, int exitValu
510406

511407
_process = pb.start();
512408
if (_process == null) {
513-
_logger.warn(String.format("Unable to execute: %s", buildCommandLine(command)));
409+
_logger.warn(String.format("Unable to execute: %s", commandLine));
514410
return String.format("Unable to execute the command: %s", command[0]);
515411
}
516412

@@ -574,12 +470,8 @@ public String executeIgnoreExitValue(OutputInterpreter interpreter, int exitValu
574470
Task timedoutTask = new Task(log, ir);
575471

576472
timedoutTask.run();
577-
if (!_passwordCommand && sensitiveArgIndices.isEmpty()) {
578-
_logger.warn(String.format("Timed out: %s. Output is: %s", buildCommandLine(command),
473+
_logger.warn(String.format("Timed out: %s. Output is: %s", commandLine,
579474
timedoutTask.getResult()));
580-
} else {
581-
_logger.warn(String.format("Timed out: %s", "command with sensitive data"));
582-
}
583475

584476
return ERR_TIMEOUT;
585477
}
@@ -603,11 +495,7 @@ public String executeIgnoreExitValue(OutputInterpreter interpreter, int exitValu
603495
_logger.warn("Security Exception....not running as root?", ex);
604496
return stackTraceAsString(ex);
605497
} catch (Exception ex) {
606-
if (sensitiveArgIndices.isEmpty()) {
607-
_logger.warn(String.format("Exception: %s", buildCommandLine(command)), ex);
608-
} else {
609-
_logger.warn("Exception on command with sensitive data.", ex);
610-
}
498+
_logger.warn(String.format("Exception: %s", commandLine), ex);
611499
return stackTraceAsString(ex);
612500
} finally {
613501
if (_process != null) {

0 commit comments

Comments
 (0)