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: 4 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,10 @@
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
</dependency>
<dependency>
<groupId>tools.jackson.dataformat</groupId>
<artifactId>jackson-dataformat-csv</artifactId>
</dependency>
<!-- Needed for Joda fields in de.rwth.idsg.steve.web.api -->
<dependency>
<groupId>tools.jackson.datatype</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package de.rwth.idsg.steve.repository.dto;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import jooq.steve.db.tables.records.TransactionStartRecord;
import lombok.Builder;
import lombok.Getter;
Expand Down Expand Up @@ -46,6 +47,16 @@ public class TransactionDetails {
@JsonIgnore
private final TransactionStartRecord nextTransactionStart;

@JsonPropertyOrder(value = {
"valueTimestamp",
"value",
"readingContext",
"format",
"measurand",
"location",
"unit",
"phase"
})
@Getter
@Builder
public static class MeterValues {
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/de/rwth/idsg/steve/service/TransactionService.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import org.joda.time.DateTime;
import org.springframework.stereotype.Service;
import org.springframework.util.CollectionUtils;
import tools.jackson.dataformat.csv.CsvMapper;
import tools.jackson.dataformat.csv.CsvSchema;

import java.io.Writer;
import java.util.Comparator;
Expand All @@ -56,6 +58,11 @@ public class TransactionService {
private final TransactionRepository transactionRepository;
private final OcppServerRepository ocppServerRepository;

private final CsvMapper csvMapper = CsvMapper.builder().findAndAddModules().build();
private final CsvSchema schema = csvMapper.schemaFor(TransactionDetails.MeterValues.class)
.withHeader()
.withNullValue("\"\""); // to be consistent with JOOQ's CSV behavior

public List<Transaction> getTransactions(TransactionQueryForm form) {
return transactionRepository.getTransactions(form);
}
Expand All @@ -64,6 +71,12 @@ public void writeTransactionsCSV(TransactionQueryForm form, Writer writer) {
transactionRepository.writeTransactionsCSV(form, writer);
}

public void writeTransactionMeterValuesCSV(int transactionPk, Writer writer) {
try (var seqWriter = csvMapper.writer(schema).writeValues(writer)) {
seqWriter.writeAll(getDetails(transactionPk).getValues());
}
Comment on lines +74 to +77
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Csv formula injection 🐞 Bug ⛨ Security

writeTransactionMeterValuesCSV exports raw string fields into a downloadable CSV without
neutralizing spreadsheet-formula prefixes, so values starting with '=', '+', '-' or '@' can execute
as formulas when opened in Excel/Sheets. This enables CSV injection from device/user-controlled
meter-value fields.
Agent Prompt
### Issue description
The meter-values CSV export writes untrusted strings directly to CSV. Spreadsheet apps may interpret fields starting with `=`, `+`, `-`, or `@` as formulas, enabling CSV injection when users open the downloaded file.

### Issue Context
Meter-value fields (e.g., `readingContext`, `format`, `measurand`, `location`, `unit`, `phase`, and potentially `value`) originate from DB columns and are rendered as untrusted in the JSP via HTML encoding, but the CSV export path performs no equivalent neutralization.

### Fix
Before writing rows, sanitize all string fields that may be opened in spreadsheets:
- For any cell value whose first non-empty character is one of `= + - @`, prefix with a single quote `'` (or another agreed neutralization like leading tab) so Excel/Sheets treat it as literal text.
- Apply this to every exported string column, ideally via a small helper (`sanitizeForSpreadsheet(String)`), and map `MeterValues` into a sanitized DTO/copy prior to `seqWriter.writeAll(...)`.

### Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/service/TransactionService.java[74-77]
- src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java[230-245]
- src/main/webapp/WEB-INF/views/data-man/transactionDetails.jsp[64-74]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

public List<Integer> getActiveTransactionIds(String chargeBoxId) {
return transactionRepository.getActiveTransactionIds(chargeBoxId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
private static final String TRANSACTIONS_PATH = "/transactions";
private static final String TRANSACTION_STOP_PATH = "/transactions/stop/{transactionPk}";
private static final String TRANSACTIONS_DETAILS_PATH = "/transactions/details/{transactionPk}";
private static final String TRANSACTIONS_DETAILS_METER_VALUES_CSV_PATH = TRANSACTIONS_DETAILS_PATH + "/meterValues.csv";

Check failure on line 67 in src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java

View workflow job for this annotation

GitHub Actions / checkstyle

[checkstyle] src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java#L67 <com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck>

Line is longer than 120 characters (found 124).
Raw output
/github/workspace/./src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java:67:0: error: Line is longer than 120 characters (found 124). (com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck)
private static final String TRANSACTIONS_QUERY_PATH = "/transactions/query";
private static final String RESERVATIONS_PATH = "/reservations";
private static final String RESERVATIONS_QUERY_PATH = "/reservations/query";
Expand Down Expand Up @@ -94,6 +95,17 @@
return "data-man/transactionDetails";
}

@RequestMapping(value = TRANSACTIONS_DETAILS_METER_VALUES_CSV_PATH)
public void getTransactionDetailsMeterValuesCsv(@PathVariable("transactionPk") int transactionPk,
HttpServletResponse response) throws IOException {
String fileName = "transaction_%s_meter_values.csv".formatted(transactionPk);
String headerKey = "Content-Disposition";
String headerValue = "attachment; filename=\"%s\"".formatted(fileName);
response.setContentType("text/csv");
response.setHeader(headerKey, headerValue);
transactionService.writeTransactionMeterValuesCSV(transactionPk, response.getWriter());
}
Comment on lines +98 to +107
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Meter csv leaks exception details 📘 Rule violation ⛨ Security

The new CSV download endpoint does not handle not-found/serialization failures and will fall back to
the global MVC error page, which renders ${exception}/${exception.cause} to the client
(information leakage) instead of a client-safe error response.
Agent Prompt
## Issue description
The new `meterValues.csv` endpoint does not catch/translate expected errors (e.g., transaction not found) into client-safe responses, and the MVC global error view displays exception details to the user.

## Issue Context
For download endpoints, not-found should map to a 404-style response and error messages must be client-safe (no internal exception details).

## Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java[98-107]
- src/main/java/de/rwth/idsg/steve/web/GlobalControllerAdvice.java[62-69]
- src/main/webapp/WEB-INF/views/00-error.jsp[22-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


@RequestMapping(value = TRANSACTIONS_QUERY_PATH)
public String getTransactionsQuery(@Valid @ModelAttribute(PARAMS) TransactionQueryForm params,
BindingResult result, Model model,
Expand Down
4 changes: 4 additions & 0 deletions src/main/webapp/WEB-INF/views/data-man/transactionDetails.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
</center>
<br>
<section><span>Intermediate Meter Values</span></section>
<form action="${ctxPath}/manager/transactions/details/${details.transaction.id}/meterValues.csv" method="get">
<input type="submit" value="Download CSV">
</form>
<br>
<table class="res">
<thead>
<tr>
Expand Down