Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
ARG spring_profile=""
ARG challenge59_webhook_url="YUhSMGNITTZMeTlvYjI5cmN5NXpiR0ZqYXk1amIyMHZjMlZ5ZG1salpYTXZWREEwVkRRd1RraFlMMEl3T1VSQlRrb3lUamRMTDJNeWFqYzFSVEUzVjFrd2NFeE5SRXRvU0RsbGQzZzBhdz09"
ENV SPRING_PROFILES_ACTIVE=$spring_profile
ENV ARG_BASED_PASSWORD=$argBasedPassword

Check warning on line 16 in Dockerfile

View workflow job for this annotation

GitHub Actions / build-preview

Sensitive data should not be used in the ARG or ENV commands

SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for sensitive data (ENV "ARG_BASED_PASSWORD") More info: https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/
ENV APP_VERSION=$argBasedVersion

Check warning on line 17 in Dockerfile

View workflow job for this annotation

GitHub Actions / build-preview

Variables should be defined before their use

UndefinedVar: Usage of undefined variable '$argBasedVersion' More info: https://docs.docker.com/go/dockerfile/rule/undefined-var/
ENV DOCKER_ENV_PASSWORD="This is it"

Check warning on line 18 in Dockerfile

View workflow job for this annotation

GitHub Actions / build-preview

Sensitive data should not be used in the ARG or ENV commands

SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for sensitive data (ENV "DOCKER_ENV_PASSWORD") More info: https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/
ENV AZURE_KEY_VAULT_ENABLED=false

Check warning on line 19 in Dockerfile

View workflow job for this annotation

GitHub Actions / build-preview

Sensitive data should not be used in the ARG or ENV commands

SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for sensitive data (ENV "AZURE_KEY_VAULT_ENABLED") More info: https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/
ENV CHALLENGE59_SLACK_WEBHOOK_URL=$challenge59_webhook_url
ENV SPRINGDOC_UI=false
ENV SPRINGDOC_DOC=false
Expand Down Expand Up @@ -58,7 +58,7 @@
chmod 600 /var/run/secrets/kubernetes.io/serviceaccount/token

# Create a dynamic archive
RUN java -XX:ArchiveClassesAtExit=application.jsa -Dspring.context.exit=onRefresh -jar application.jar
RUN java --add-modules=jdk.unsupported -XX:ArchiveClassesAtExit=application.jsa -Dspring.context.exit=onRefresh -jar application.jar

# Clean up the mocked token
RUN rm -rf /var/run/secrets/kubernetes.io
Expand All @@ -70,4 +70,4 @@
RUN adduser -u 2000 -D wrongsecrets
USER wrongsecrets

CMD java -jar -XX:SharedArchiveFile=application.jsa -Dspring.profiles.active=$(echo ${SPRING_PROFILES_ACTIVE}) -Dspringdoc.swagger-ui.enabled=${SPRINGDOC_UI} -Dspringdoc.api-docs.enabled=${SPRINGDOC_DOC} -D application.jar
CMD java --add-modules=jdk.unsupported -jar -XX:SharedArchiveFile=application.jsa -Dspring.profiles.active=$(echo ${SPRING_PROFILES_ACTIVE}) -Dspringdoc.swagger-ui.enabled=${SPRINGDOC_UI} -Dspringdoc.api-docs.enabled=${SPRINGDOC_DOC} -D application.jar

Check warning on line 73 in Dockerfile

View workflow job for this annotation

GitHub Actions / build-preview

JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals

JSONArgsRecommended: JSON arguments recommended for CMD to prevent unintended behavior related to OS signals More info: https://docs.docker.com/go/dockerfile/rule/json-args-recommended/
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ This repository contains **intentionally vulnerable code and configuration files

### 👨‍💻 Development & Contribution
- [Notes on development](#notes-on-development)
- [Spring Boot 4 adoption checklist](docs/SPRING_BOOT_4_ADOPTION_CHECKLIST.md)
- [Dependency management](#dependency-management)
- [Get the project started in IntelliJ IDEA](#get-the-project-started-in-intellij-idea)
- [Automatic reload during development](#automatic-reload-during-development)
Expand Down
4 changes: 4 additions & 0 deletions docs/DEVELOPMENT_PATTERNS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

This document outlines common code patterns, conventions, and best practices used throughout the WrongSecrets project.

## Related documentation

- [Spring Boot 4 Adoption Checklist](SPRING_BOOT_4_ADOPTION_CHECKLIST.md)

## Challenge Structure Patterns

### Challenge Interface vs FixedAnswerChallenge
Expand Down
146 changes: 146 additions & 0 deletions docs/SPRING_BOOT_4_ADOPTION_CHECKLIST.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
# Spring Boot 4 Adoption Checklist (WrongSecrets)

This checklist is tailored to the current `wrongsecrets` codebase (Spring Boot `4.0.3`, Java `25`).

## How to use this document

- Keep this as a living checklist in PRs.
- Mark items complete when merged.
- Prefer small, focused migrations (one concern per PR).

## Current baseline (already in place)

- [x] Spring Boot `4.0.3` is configured in `pom.xml`.
- [x] Spring Cloud line is aligned (`2025.1.1`).
- [x] `@ConfigurationProperties` is already used in multiple places.
- [x] Mockito inline-mock-maker warning addressed by passing Mockito as Java agent in Surefire.

---

## Priority 0 — Safety and consistency (start here)

### 1) Standardize HTTP error responses with `ProblemDetail`

- [ ] Add a global `@RestControllerAdvice` for API endpoints that returns `ProblemDetail`.
- [ ] Keep MVC HTML error handling as-is for Thymeleaf pages; only modernize JSON API errors.
- [ ] Add tests that assert RFC 9457-style payload fields (`type`, `title`, `status`, `detail`, `instance`).

**Why now:** Reduces custom exception payload drift and improves API consistency.

### 2) Replace new `RestTemplate` usage with `RestClient`

- [ ] Stop introducing any new `RestTemplate` usage.
- [ ] Migrate existing bean in `WrongSecretsApplication` from `RestTemplate` to `RestClient.Builder`.
- [ ] Migrate call sites incrementally (start with `SlackNotificationService`).
- [ ] Add timeout and retry policy explicitly for outbound calls.

**Current state:** `RestTemplate` bean and usage exist and can be migrated safely in phases.

### 3) Add/verify deprecation gate in CI

- [ ] Run compile with deprecation warnings enabled in CI (`-Xlint:deprecation`).
- [ ] Fail build on newly introduced deprecations (can be soft-fail initially).
- [ ] Track remaining suppressions/deprecations as explicit TODOs.

**Why now:** Boot 4/Spring 7 deprecations will accumulate quickly otherwise.

---

## Priority 1 — Observability and operability

### 4) Enable tracing + log correlation end-to-end

- [ ] Ensure tracing is enabled in all non-local profiles.
- [ ] Ensure logs include trace/span correlation IDs.
- [ ] Add dashboard/alerts for key challenge-flow operations.

### 5) Harden Actuator for production profiles

- [ ] Verify readiness/liveness probes are exposed and used by deployment manifests.
- [ ] Restrict sensitive actuator endpoints by profile.
- [ ] Add health contributors for external dependencies used in runtime profiles.

### 6) Structured logging profile

- [ ] Use JSON logs for cloud/container profiles.
- [ ] Keep developer-friendly text logs for local profile.
- [ ] Document expected log fields for incident response.

---

## Priority 2 — Runtime and performance

### 7) Evaluate virtual threads for I/O-heavy flows

- [ ] Add profile-based toggle (`spring.threads.virtual.enabled=true`) for evaluation.
- [ ] Run load comparison (latency, throughput, memory) before default-enabling.
- [ ] Keep a rollback toggle in case of third-party incompatibilities.

### 8) Validate graceful shutdown behavior

- [ ] Verify request drain behavior on shutdown in containerized environments.
- [ ] Confirm no challenge state corruption occurs during rolling updates.

### 9) AOT/native readiness checks

- [ ] Add optional CI job for AOT/native compatibility (not necessarily release artifact yet).
- [ ] Record blockers (reflection/dynamic proxies/resources) in this document.

---

## Priority 3 — Security and configuration posture

### 10) Expand typed config, reduce scattered `@Value`

- [ ] Introduce/extend `@ConfigurationProperties` classes for grouped settings.
- [ ] Limit direct `@Value` usage to simple one-off values.
- [ ] Validate config with bean validation annotations.

### 11) TLS/SSL bundles standardization

- [ ] Use SSL bundle config for outbound TLS trust/key material where applicable.
- [ ] Remove ad-hoc SSL setup code if present.

### 12) Secret handling consistency by profile

- [ ] Document expected secret source per profile (`docker`, `k8s`, `aws`, `gcp`, `azure`).
- [ ] Ensure no fallback path accidentally logs sensitive values.

---

## Priority 4 — Testing modernization

### 13) Keep Mockito java-agent setup stable

- [x] Surefire passes Mockito as `-javaagent`.
- [ ] Mirror same setup in Failsafe if/when integration tests use inline mocking.

### 14) Strengthen integration testing with Testcontainers service connection patterns

- [ ] Prefer service-connection style wiring for test dependencies.
- [ ] Reduce custom bootstrapping code in integration tests where possible.

### 15) Add contract tests for outbound HTTP clients

- [ ] Add tests for success, timeout, retry, and non-2xx mapping behavior.
- [ ] Ensure migrated `RestClient` paths are fully covered.

---

## Concrete first 5 PRs

1. **PR 1:** Add API `ProblemDetail` advice + tests.
2. **PR 2:** Introduce `RestClient` bean and migrate `SlackNotificationService`.
3. **PR 3:** Add deprecation checks to CI and document policy.
4. **PR 4:** Add tracing/log-correlation defaults for non-local profiles.
5. **PR 5:** Virtual thread evaluation profile + benchmark notes.

---

## Definition of done for Boot 4 adoption

- [ ] No new `RestTemplate` code introduced.
- [ ] API errors are standardized on `ProblemDetail`.
- [ ] Deprecation warnings are tracked and controlled in CI.
- [ ] Observability baseline (metrics, traces, log correlation) is active in non-local profiles.
- [ ] Migration choices and rollout decisions are documented in `docs/`.
48 changes: 34 additions & 14 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.5.11</version>
<version>4.0.3</version>
<!-- lookup parent from repository -->
</parent>

Expand Down Expand Up @@ -47,11 +47,11 @@
<archunit.version>1.4.1</archunit.version>
<asciidoctor.maven.plugin.version>3.2.0</asciidoctor.maven.plugin.version>
<asciidoctorj.version>3.0.1</asciidoctorj.version>
<aws.sdk.version>2.40.9</aws.sdk.version>
<aws.sdk.version>2.42.4</aws.sdk.version>
<bootstrap.version>5.3.8</bootstrap.version>
<checkstyle-maven.version>3.6.0</checkstyle-maven.version>
<checkstyle.version>13.2.0</checkstyle.version>
<com.azure.spring.version>6.0.0</com.azure.spring.version>
<com.azure.spring.version>7.0.0</com.azure.spring.version>
<cyclonedx-maven.version>2.9.1</cyclonedx-maven.version>
<cyclonedx.core.version>12.1.0</cyclonedx.core.version>
<datatables.version>2.3.7</datatables.version>
Expand All @@ -60,10 +60,12 @@
<findsecbugs.version>1.14.0</findsecbugs.version>
<frontend-maven-plugin.version>1.15.4</frontend-maven-plugin.version>
<gatling-maven-plugin.version>4.21.0</gatling-maven-plugin.version>
<gatling.version>3.14.9</gatling.version>
<gcp.sdk.version>7.4.5</gcp.sdk.version>
<gatling.version>3.15.0</gatling.version>
<github.button.version>2.14.1</github.button.version>
<io.netty.version>4.1.123.Final</io.netty.version>
<google.cloud.libraries-bom.version>26.76.0</google.cloud.libraries-bom.version>
<!-- Pin Groovy to 4.x for thymeleaf-layout-dialect compatibility (Spring Boot 4.0 manages 5.x) -->
<groovy.version>4.0.25</groovy.version>
<io.netty.version>4.1.130.Final</io.netty.version>
<java.version>25</java.version>
<jquery.version>3.7.1</jquery.version>
<jruby.version>10.0.3.0</jruby.version>
Expand All @@ -77,12 +79,12 @@
<spotbugs-maven.version>4.9.8.2</spotbugs-maven.version>
<spotbugs.version>4.9.8</spotbugs.version>
<spotless-maven-plugin.version>3.2.1</spotless-maven-plugin.version>
<spring-boot.version>3.5.11</spring-boot.version>
<spring-vault.version>3.2.0</spring-vault.version>
<spring.cloud-version>2025.0.0</spring.cloud-version>
<spring.security.version>6.5.8</spring.security.version>
<spring-boot.version>4.0.3</spring-boot.version>
<spring-vault.version>4.0.1</spring-vault.version>
<spring.cloud-version>2025.1.1</spring.cloud-version>
<spring.security.version>7.0.3</spring.security.version>
<springdoc-maven.version>1.5</springdoc-maven.version>
<springdoc.version>2.8.15</springdoc.version>
<springdoc.version>3.0.1</springdoc.version>
<system-stubs-jupiter.version>2.1.8</system-stubs-jupiter.version>
<testcontainers-cypress.version>1.10.0</testcontainers-cypress.version>
<testcontainers.version>1.21.4</testcontainers.version>
Expand All @@ -108,8 +110,8 @@

<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>spring-cloud-gcp-dependencies</artifactId>
<version>${gcp.sdk.version}</version>
<artifactId>libraries-bom</artifactId>
<version>${google.cloud.libraries-bom.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down Expand Up @@ -297,6 +299,12 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-webmvc-test</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>uk.org.webcompere</groupId>
<artifactId>system-stubs-jupiter</artifactId>
Expand Down Expand Up @@ -658,12 +666,24 @@
<artifactId>gatling-maven-plugin</artifactId>
<version>${gatling-maven-plugin.version}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>3.8.1</version>
<executions>
<execution>
<goals>
<goal>properties</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${maven-surefire-plugin.version}</version>
<configuration>
<argLine>-Dspring.profiles.active=test,maven-test</argLine>
<argLine>-Dspring.profiles.active=test,maven-test -javaagent:${org.mockito:mockito-core:jar}</argLine>
</configuration>
</plugin>
</plugins>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.owasp.wrongsecrets;

import io.swagger.v3.oas.annotations.Operation;
import org.springframework.boot.web.servlet.error.ErrorController;
import org.springframework.boot.webmvc.error.ErrorController;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package org.owasp.wrongsecrets.challenges;

import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import io.swagger.v3.oas.annotations.Operation;
import java.util.List;
import lombok.extern.slf4j.Slf4j;
import net.minidev.json.JSONArray;
import net.minidev.json.JSONObject;
import org.owasp.wrongsecrets.Challenges;
import org.owasp.wrongsecrets.RuntimeEnvironment;
import org.owasp.wrongsecrets.ScoreCard;
Expand Down Expand Up @@ -45,11 +45,11 @@ public ChallengesCtfController(
summary = "Gives all challenges back in a jsonArray, to be used with the Juiceshop CTF cli")
public String getChallenges() {
List<ChallengeDefinition> definitions = challenges.getDefinitions().challenges();
JSONObject json = new JSONObject();
JSONArray jsonArray = new JSONArray();
var json = JsonNodeFactory.instance.objectNode();
ArrayNode jsonArray = JsonNodeFactory.instance.arrayNode();
for (int i = 0; i < definitions.size(); i++) {
ChallengeDefinition definition = definitions.get(i);
JSONObject jsonChallenge = new JSONObject();
var jsonChallenge = JsonNodeFactory.instance.objectNode();
jsonChallenge.put("id", i + 1);
jsonChallenge.put("name", definition.name().name());
jsonChallenge.put("key", definition.name().shortName());
Expand All @@ -68,13 +68,18 @@ public String getChallenges() {
.map(s -> s.hint().contents().get())
.orElse(disabledChallenge));
jsonChallenge.put("solved", scoreCard.getChallengeCompleted(definition));
jsonChallenge.put("disabledEnv", getDisabledEnv(definition));
String disabledEnv = getDisabledEnv(definition);
if (disabledEnv != null) {
jsonChallenge.put("disabledEnv", disabledEnv);
} else {
jsonChallenge.putNull("disabledEnv");
}
jsonChallenge.put("difficulty", getDificulty(definition.difficulty()));
jsonArray.add(jsonChallenge);
}
json.put("status", "success");
json.put("data", jsonArray);
String result = json.toJSONString();
json.set("data", jsonArray);
String result = json.toString();
log.trace("returning {}", result);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public boolean answerCorrect(String answer) {
return false;
}
} catch (Exception e) {
log.warn("given answer is not an integer", e);
log.warn("given answer is not an integer. Exception: {}", e.getMessage());
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ private String getActualSecret() {
try {
return Files.readString(Paths.get(dockerMountsecret, "secret.txt"), StandardCharsets.UTF_8);
} catch (Exception e) {
log.warn("Exception during file reading, defaulting to default without cloud environment", e);
log.warn(
"Exception during file reading, defaulting to default without cloud environment."
+ " Exception message: {}",
e.getMessage());
return Challenges.ErrorResponses.OUTSIDE_DOCKER;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;

import java.util.Base64;
import lombok.extern.slf4j.Slf4j;
import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
Expand All @@ -12,6 +13,7 @@
* variables. Shows how an ex-employee could misuse the webhook if it's not rotated when they leave.
*/
@Component
@Slf4j
public class Challenge59 extends FixedAnswerChallenge {

private final String obfuscatedSlackWebhookUrl;
Expand All @@ -37,6 +39,7 @@ private String deobfuscateSlackWebhookUrl(String obfuscatedUrl) {
byte[] secondDecode = Base64.getDecoder().decode(firstDecode);
return new String(secondDecode, UTF_8);
} catch (Exception e) {
log.warn("Webhook URL not properly set for Slack in {}", this);
// Return a default value if the environment variable is not properly set
return "https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX";
}
Expand Down
Loading
Loading