feat(infra): new workspace infrastructure setup#746
Conversation
| if (result instanceof Success success) { | ||
| return ResponseEntity.ok(success.message()); | ||
| } else if (result instanceof Failure failure) { | ||
| return ResponseEntity.badRequest().body(failure.error()); |
Check warning
Code scanning / CodeQL
Information exposure through an error message Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix this kind of problem you should avoid returning raw exception messages or stack traces to clients. Instead, return a generic, user-friendly error message (for example, “Unable to start threads at this time”) and log the detailed exception on the server side where it is available for diagnostics but not exposed.
For this specific controller, the best fix with minimal functional change is:
- Change
tryStartThreadsandtryStopThreadsso that they no longer wrape.getMessage()into theFailurerecord. - Instead, log the caught
IllegalStateExceptionon the server (using a logger) and return aFailurewith a generic, non-revealing error message, such as “Could not start threads” or “Could not stop threads”. - Because we only see this file, we will add a logger in this controller using a standard logging utility that is likely already present in a Spring Boot app, such as SLF4J (
org.slf4j.Logger/LoggerFactory). - The public API (
startThreadsandstopThreads) will continue to behave similarly from the client perspective: on failure they still return400 Bad Requestwith a human-readable message, but the content will be generic and not derived from the exception message.
Concretely:
- Add imports for
org.slf4j.Loggerandorg.slf4j.LoggerFactoryat the top ofThreadManagementController.java. - Add a
private static final Logger logger = LoggerFactory.getLogger(ThreadManagementController.class);field. - Modify the
catch (IllegalStateException e)blocks intryStartThreadsandtryStopThreadsto calllogger.error("... context ...", e);and return aFailurewith a fixed message instead ofe.getMessage().
| @@ -3,11 +3,15 @@ | ||
| import com.unicorn.store.service.ThreadGeneratorService; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api/threads") | ||
| public class ThreadManagementController { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(ThreadManagementController.class); | ||
|
|
||
| private final ThreadGeneratorService threadGeneratorService; | ||
|
|
||
| public ThreadManagementController(ThreadGeneratorService threadGeneratorService) { | ||
| @@ -46,7 +45,8 @@ | ||
| threadGeneratorService.startThreads(count); | ||
| return new Success("Successfully started " + count + " threads"); | ||
| } catch (IllegalStateException e) { | ||
| return new Failure(e.getMessage()); | ||
| logger.error("Failed to start {} threads", count, e); | ||
| return new Failure("Could not start threads due to an internal error"); | ||
| } | ||
| } | ||
|
|
||
| @@ -55,7 +55,8 @@ | ||
| threadGeneratorService.stopThreads(); | ||
| return new Success("Successfully stopped all threads"); | ||
| } catch (IllegalStateException e) { | ||
| return new Failure(e.getMessage()); | ||
| logger.error("Failed to stop threads", e); | ||
| return new Failure("Could not stop threads due to an internal error"); | ||
| } | ||
| } | ||
|
|
| if (result instanceof Success success) { | ||
| return ResponseEntity.ok(success.message()); | ||
| } else if (result instanceof Failure failure) { | ||
| return ResponseEntity.badRequest().body(failure.error()); |
Check warning
Code scanning / CodeQL
Information exposure through an error message Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix information exposure via error messages in a REST controller, you should avoid returning raw exception messages to clients. Instead, return a generic, non-sensitive error description (e.g., “Unable to start threads”) and log the detailed exception message and stack trace on the server side for diagnostics.
For this file, the safest minimal change is:
- Introduce logging in
ThreadManagementController(e.g., usingorg.slf4j.LoggerandLoggerFactory). - In
tryStartThreadsandtryStopThreads, catchIllegalStateExceptionand:- Log the exception (including stack trace) with context.
- Return a
Failurewith a generic error message that does not includee.getMessage().
- Keep the public API behavior (HTTP status codes, basic success messages) the same, only changing the content of error messages to be generic.
Concretely:
- Add
import org.slf4j.Logger;andimport org.slf4j.LoggerFactory;at the top ofThreadManagementController.java. - Add a
private static final Logger logger = LoggerFactory.getLogger(ThreadManagementController.class);field. - In
tryStartThreads, changereturn new Failure(e.getMessage());to something like:logger.error("Failed to start threads", e);return new Failure("Unable to start threads at this time.");
- In
tryStopThreads, changereturn new Failure(e.getMessage());similarly:logger.error("Failed to stop threads", e);return new Failure("Unable to stop threads at this time.");
This preserves the structure and HTTP semantics but prevents exposure of exception details.
| @@ -3,11 +3,15 @@ | ||
| import com.unicorn.store.service.ThreadGeneratorService; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api/threads") | ||
| public class ThreadManagementController { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(ThreadManagementController.class); | ||
|
|
||
| private final ThreadGeneratorService threadGeneratorService; | ||
|
|
||
| public ThreadManagementController(ThreadGeneratorService threadGeneratorService) { | ||
| @@ -46,7 +45,8 @@ | ||
| threadGeneratorService.startThreads(count); | ||
| return new Success("Successfully started " + count + " threads"); | ||
| } catch (IllegalStateException e) { | ||
| return new Failure(e.getMessage()); | ||
| logger.error("Failed to start threads", e); | ||
| return new Failure("Unable to start threads at this time."); | ||
| } | ||
| } | ||
|
|
||
| @@ -55,7 +55,8 @@ | ||
| threadGeneratorService.stopThreads(); | ||
| return new Success("Successfully stopped all threads"); | ||
| } catch (IllegalStateException e) { | ||
| return new Failure(e.getMessage()); | ||
| logger.error("Failed to stop threads", e); | ||
| return new Failure("Unable to stop threads at this time."); | ||
| } | ||
| } | ||
|
|
- Remove sensitive data from Lambda function logs (database-setup.py, password-exporter.py) - Replace f-string logging with generic success messages for security compliance - Reorganize CDK nag suppression rules in base-stack.yaml for consistency - Reorder suppression rule format to use id-first structure across all CloudFormation templates - Consolidate and standardize metadata configuration in java-on-amazon-eks-stack.yaml - Update java-on-aws-immersion-day-stack.yaml metadata structure - Update java-spring-ai-agents-stack.yaml metadata structure - Improve generate.sh script for CloudFormation generation - Enhance security posture by preventing accidental exposure of ARNs and secret names in logs
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.