Fix CodeQL SSRF and path-traversal findings in form-detection model manager

This commit is contained in:
Anthony Stirling
2026-06-15 17:33:15 +01:00
parent 7bcaeb69c5
commit cfd6d19886
6 changed files with 23 additions and 37 deletions
@@ -52,7 +52,7 @@ public class FormDetectionModelController {
return ResponseEntity.badRequest().body(s);
}
try {
manager.startInstall(request.getModelId(), request.getOnnxUrl(), request.getSha256());
manager.startInstall(request.getModelId());
return ResponseEntity.accepted().body(manager.status());
} catch (IllegalStateException e) {
ModelStatusResponse s = manager.status();
@@ -115,11 +115,5 @@ public class FormDetectionModelController {
@Data
public static class InstallRequest {
private String modelId;
/** Optional override download URL (admin self-hosted export); falls back to the catalog. */
private String onnxUrl;
/** Optional override SHA-256; falls back to the catalog. */
private String sha256;
}
}
@@ -156,17 +156,18 @@ public class FormDetectionModelManager {
* @throws IllegalArgumentException unknown/invalid model id or bad checksum format
* @throws IllegalStateException no URL/checksum configured, or an install is already running
*/
public synchronized void startInstall(String modelId, String overrideUrl, String overrideSha) {
public synchronized void startInstall(String modelId) {
if (!SAFE_ID.matcher(modelId).matches()) {
throw new IllegalArgumentException("Invalid model id: " + modelId);
}
ModelCatalogEntry entry =
catalog.getById(modelId)
.orElseThrow(
() -> new IllegalArgumentException("Unknown model id: " + modelId));
if (!SAFE_ID.matcher(modelId).matches()) {
throw new IllegalArgumentException("Invalid model id: " + modelId);
}
String url = StringUtils.isNotBlank(overrideUrl) ? overrideUrl : entry.getOnnxUrl();
String rawSha = StringUtils.isNotBlank(overrideSha) ? overrideSha : entry.getSha256();
String sha = rawSha == null ? null : rawSha.toLowerCase(Locale.ROOT);
// URL + checksum come ONLY from the bundled catalog (trusted constants), never from the
// request, so an admin cannot point the download at an arbitrary host (avoids SSRF).
String url = entry.getOnnxUrl();
String sha = entry.getSha256() == null ? null : entry.getSha256().toLowerCase(Locale.ROOT);
if (StringUtils.isBlank(url) || StringUtils.isBlank(sha)) {
throw new IllegalStateException(
"Model '" + modelId + "' has no download URL/checksum configured yet");
@@ -339,7 +340,11 @@ public class FormDetectionModelManager {
if (StringUtils.isBlank(id) || !SAFE_ID.matcher(id).matches()) {
return;
}
Path file = modelDir().resolve(id + ".onnx");
Path base = modelDir().toAbsolutePath().normalize();
Path file = base.resolve(id + ".onnx").normalize();
if (!file.startsWith(base)) {
return; // path-traversal guard (SAFE_ID already blocks it; this also satisfies CodeQL)
}
try {
Files.deleteIfExists(file);
} catch (IOException e) {
@@ -72,7 +72,7 @@ class FormDetectionModelControllerTest {
Mockito.when(manager.status()).thenReturn(notInstalled());
Mockito.doThrow(new IllegalStateException("An install is already in progress"))
.when(manager)
.startInstall(Mockito.eq("ffdnet-s"), Mockito.any(), Mockito.any());
.startInstall(Mockito.eq("ffdnet-s"));
mvc(manager)
.perform(
@@ -100,7 +100,7 @@ class FormDetectionModelManagerTest {
FormDetectionModelManager m =
manager(dir, entry("http://127.0.0.1:" + port + "/model.onnx", modelSha), ep);
m.startInstall("test-model", null, null);
m.startInstall("test-model");
awaitState(m, "ready", 5000);
Path onnx = dir.resolve("test-model.onnx");
@@ -117,7 +117,7 @@ class FormDetectionModelManagerTest {
FormDetectionModelManager m =
manager(dir, entry("http://127.0.0.1:" + port + "/model.onnx", "0".repeat(64)), ep);
m.startInstall("test-model", null, null);
m.startInstall("test-model");
awaitState(m, "failed", 5000);
assertFalse(Files.exists(dir.resolve("test-model.onnx")), "no model on mismatch");
@@ -147,9 +147,9 @@ class FormDetectionModelManagerTest {
entry("http://127.0.0.1:" + port + "/gated.onnx", modelSha),
Mockito.mock(EndpointConfiguration.class));
m.startInstall("test-model", null, null); // begins, blocks in handler
m.startInstall("test-model"); // begins, blocks in handler
// installing flag is set synchronously before the worker thread spawns
assertThrows(IllegalStateException.class, () -> m.startInstall("test-model", null, null));
assertThrows(IllegalStateException.class, () -> m.startInstall("test-model"));
gate.countDown();
awaitState(m, "ready", 5000);
}
@@ -158,7 +158,7 @@ class FormDetectionModelManagerTest {
void rejectsBlankUrl(@TempDir Path dir) {
FormDetectionModelManager m =
manager(dir, entry("", ""), Mockito.mock(EndpointConfiguration.class));
assertThrows(IllegalStateException.class, () -> m.startInstall("test-model", null, null));
assertThrows(IllegalStateException.class, () -> m.startInstall("test-model"));
}
@Test
@@ -168,19 +168,6 @@ class FormDetectionModelManagerTest {
dir,
entry("http://127.0.0.1:" + port + "/model.onnx", modelSha),
Mockito.mock(EndpointConfiguration.class));
assertThrows(IllegalArgumentException.class, () -> m.startInstall("unknown", null, null));
}
@Test
void overrideUrlAndShaWork(@TempDir Path dir) throws Exception {
EndpointConfiguration ep = Mockito.mock(EndpointConfiguration.class);
// catalog entry has blank url/sha; admin supplies overrides at install time
FormDetectionModelManager m = manager(dir, entry("", ""), ep);
m.startInstall("test-model", "http://127.0.0.1:" + port + "/model.onnx", modelSha);
awaitState(m, "ready", 5000);
assertTrue(Files.exists(dir.resolve("test-model.onnx")));
assertTrue(m.isReady());
assertThrows(IllegalArgumentException.class, () -> m.startInstall("unknown"));
}
}
@@ -583,7 +583,7 @@ show = "Air-gapped / offline install instructions"
step1 = "1. On a machine with internet, download the model file:"
step2 = "2. Verify its SHA-256 checksum matches:"
step3 = "3. Copy it onto the Stirling-PDF server into the model directory:"
step4 = "4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). <configs> is the configs volume (e.g. /configs in Docker). Alternatively, point the Install button at an internal mirror using an override URL + checksum."
step4 = "4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). <configs> is the configs volume (e.g. /configs in Docker). The Docker image also pre-bundles this model, so it is ready with no download."
[admin.formDetection.engine]
auto = "Auto"
@@ -297,7 +297,7 @@ export default function AdminFormDetectionSection() {
<Text size="xs" c="dimmed">
{t(
"admin.formDetection.airgap.step4",
"4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). <configs> is the configs volume (e.g. /configs in Docker). Alternatively, point the Install button at an internal mirror using an override URL + checksum.",
"4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). <configs> is the configs volume (e.g. /configs in Docker). The Docker image also pre-bundles this model, so it is ready with no download.",
)}
</Text>
</Stack>