Skip to content

Commit b64f6ff

Browse files
aksOpsclaude
andcommitted
fix(security): scrub IOException details from /api/file responses (CodeQL CWE-209)
Three IOException handlers in GraphController#readFile concatenated the JDK's e.getMessage() into the response body. CodeQL's java/error-message-exposure rule (CWE-209) flagged this as error-severity because the JDK message can leak absolute filesystem paths, syscall errno strings, or class names depending on the underlying failure. Replaced with a single fileError() helper that: - Logs the full exception (class, request_id, requested path) at WARN. - Returns a generic public message + request_id only. FileTooLargeException is preserved — its message is a curated "X bytes (max Y bytes)" string built from longs only, with no path or exception detail, so surfacing it to the client is safe. Unblocks PR 1 (#106) CodeQL gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 331cf0c commit b64f6ff

1 file changed

Lines changed: 40 additions & 11 deletions

File tree

src/main/java/io/github/randomcodespace/iq/api/GraphController.java

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
import io.github.randomcodespace.iq.config.CodeIqConfig;
44
import io.github.randomcodespace.iq.query.QueryService;
5+
import org.slf4j.Logger;
6+
import org.slf4j.LoggerFactory;
7+
import org.slf4j.MDC;
58
import org.springframework.http.HttpStatus;
69
import org.springframework.http.MediaType;
710
import org.springframework.http.ResponseEntity;
@@ -33,6 +36,8 @@
3336
@Profile("serving")
3437
public class GraphController {
3538

39+
private static final Logger log = LoggerFactory.getLogger(GraphController.class);
40+
3641
private final QueryService queryService;
3742
private final CodeIqConfig config;
3843

@@ -257,17 +262,22 @@ public ResponseEntity<String> readFile(
257262
@RequestParam String path,
258263
@RequestParam(required = false) Integer startLine,
259264
@RequestParam(required = false) Integer endLine) {
265+
// Per-error rationale: response bodies must NEVER carry the underlying
266+
// exception message (CodeQL java/error-message-exposure / CWE-209). The
267+
// exception class + caller-supplied path are logged at WARN with the
268+
// request_id; clients receive a generic envelope and the request_id so
269+
// operators can correlate without a stack frame leaking class names,
270+
// absolute filesystem paths, or syscall errno strings.
260271
Path codebaseReal;
261272
try {
262273
codebaseReal = Path.of(config.getRootPath()).toRealPath();
263274
} catch (IOException e) {
264-
return ResponseEntity.status(500)
265-
.contentType(MediaType.TEXT_PLAIN)
266-
.body("Failed to resolve codebase root: " + e.getMessage());
275+
return fileError(HttpStatus.INTERNAL_SERVER_ERROR, "codebase_root_unavailable",
276+
"Failed to resolve codebase root.", path, e);
267277
}
268278
Path candidate = codebaseReal.resolve(path).normalize();
269279
if (!candidate.startsWith(codebaseReal)) {
270-
return ResponseEntity.status(403)
280+
return ResponseEntity.status(HttpStatus.FORBIDDEN)
271281
.contentType(MediaType.TEXT_PLAIN)
272282
.body("Path traversal blocked");
273283
}
@@ -277,12 +287,11 @@ public ResponseEntity<String> readFile(
277287
} catch (NoSuchFileException e) {
278288
return ResponseEntity.notFound().build();
279289
} catch (IOException e) {
280-
return ResponseEntity.status(500)
281-
.contentType(MediaType.TEXT_PLAIN)
282-
.body("Failed to resolve file: " + e.getMessage());
290+
return fileError(HttpStatus.INTERNAL_SERVER_ERROR, "file_resolve_failed",
291+
"Failed to resolve file.", path, e);
283292
}
284293
if (!resolvedReal.startsWith(codebaseReal)) {
285-
return ResponseEntity.status(403)
294+
return ResponseEntity.status(HttpStatus.FORBIDDEN)
286295
.contentType(MediaType.TEXT_PLAIN)
287296
.body("Path traversal blocked");
288297
}
@@ -295,16 +304,36 @@ public ResponseEntity<String> readFile(
295304
.contentType(MediaType.TEXT_PLAIN)
296305
.body(content);
297306
} catch (SafeFileReader.FileTooLargeException tooLarge) {
307+
// FileTooLargeException is a curated, sanitized message produced by
308+
// SafeFileReader (size cap context only, no path/exception details);
309+
// safe to surface to the client.
298310
return ResponseEntity.status(HttpStatus.CONTENT_TOO_LARGE)
299311
.contentType(MediaType.TEXT_PLAIN)
300312
.body(tooLarge.getMessage());
301313
} catch (IOException e) {
302-
return ResponseEntity.status(500)
303-
.contentType(MediaType.TEXT_PLAIN)
304-
.body("Failed to read file: " + e.getMessage());
314+
return fileError(HttpStatus.INTERNAL_SERVER_ERROR, "file_read_failed",
315+
"Failed to read file.", path, e);
305316
}
306317
}
307318

319+
/**
320+
* Build a sanitized error response for {@code /api/file}. Logs the full
321+
* exception (so operators can debug) but never echoes the JDK's IOException
322+
* detail back to the client — see CodeQL {@code java/error-message-exposure}
323+
* (CWE-209). The response body carries a generic message + request_id;
324+
* operators correlate via the WARN log line.
325+
*/
326+
private ResponseEntity<String> fileError(HttpStatus status, String code, String publicMessage,
327+
String requestedPath, IOException cause) {
328+
String requestId = MDC.get("request_id");
329+
log.warn("readFile {} (code={}, request_id={}, path={})",
330+
cause.getClass().getSimpleName(), code, requestId, requestedPath, cause);
331+
String body = publicMessage + (requestId != null ? " (request_id=" + requestId + ")" : "");
332+
return ResponseEntity.status(status)
333+
.contentType(MediaType.TEXT_PLAIN)
334+
.body(body);
335+
}
336+
308337
// POST /api/analyze removed — API/MCP server is read-only.
309338
// Analysis is done locally via CLI: codeiq analyze / codeiq index
310339
// Data is loaded into Neo4j on serve startup (auto-enrich).

0 commit comments

Comments
 (0)