Skip to content

Commit 51d3ca0

Browse files
FelixTJDietrichclaude
FelixTJDietrich
and
claude
authored
fix(server): add size limit to directory injection in SandboxWorkspaceManager (#917)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e5a15b2 commit 51d3ca0

12 files changed

Lines changed: 268 additions & 25 deletions

File tree

‎server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/sandbox/SandboxProperties.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
* @param llmProxyPort port on which the LLM proxy listens (inside the app-server container)
4242
* @param appServerContainerId Docker container ID of the app-server (null=auto-detect from
4343
* HOSTNAME)
44+
* @param maxDirectoryBytes maximum total bytes for directory injection via tar (default 1 GB)
45+
* @param maxDirectoryEntries maximum entry count for directory injection (default 500,000)
4446
* @param defaultResourceLimits default resource constraints for containers
4547
*/
4648
@Validated
@@ -56,6 +58,8 @@ public record SandboxProperties(
5658
@Nullable String containerRuntime,
5759
@DefaultValue("8080") @Min(1) int llmProxyPort,
5860
@Nullable String appServerContainerId,
61+
@DefaultValue("1073741824") @Min(1) long maxDirectoryBytes, // 1 GB
62+
@DefaultValue("500000") @Min(1) int maxDirectoryEntries,
5963
@Valid DefaultResourceLimits defaultResourceLimits
6064
) {
6165
public SandboxProperties {

‎server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/DockerSandboxConfiguration.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,14 @@ public SandboxNetworkManager sandboxNetworkManager(DockerClientOperations ops, S
9696
}
9797

9898
@Bean
99-
public SandboxWorkspaceManager sandboxWorkspaceManager(DockerClientOperations ops) {
100-
return new SandboxWorkspaceManager(ops);
99+
public SandboxWorkspaceManager sandboxWorkspaceManager(DockerClientOperations ops, SandboxProperties properties) {
100+
return new SandboxWorkspaceManager(
101+
ops,
102+
SandboxWorkspaceManager.MAX_OUTPUT_BYTES,
103+
SandboxWorkspaceManager.MAX_SINGLE_FILE_BYTES,
104+
properties.maxDirectoryBytes(),
105+
properties.maxDirectoryEntries()
106+
);
101107
}
102108

103109
/**

‎server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/SandboxWorkspaceManager.java‎

Lines changed: 99 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
package de.tum.in.www1.hephaestus.agent.sandbox.docker;
22

33
import de.tum.in.www1.hephaestus.agent.sandbox.spi.SandboxException;
4+
import java.io.BufferedInputStream;
5+
import java.io.BufferedOutputStream;
46
import java.io.ByteArrayInputStream;
57
import java.io.ByteArrayOutputStream;
68
import java.io.IOException;
79
import java.io.InputStream;
10+
import java.io.OutputStream;
811
import java.nio.file.Files;
912
import java.nio.file.Path;
1013
import java.util.HashMap;
1114
import java.util.Map;
15+
import java.util.stream.Stream;
1216
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
1317
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
1418
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
@@ -34,19 +38,38 @@ public class SandboxWorkspaceManager {
3438
/** Maximum total size of injected input files (50 MB). */
3539
static final long MAX_INPUT_BYTES = 50L * 1024 * 1024;
3640

41+
/** Maximum total size of a directory injected via tar (1 GB). */
42+
static final long MAX_DIRECTORY_BYTES = 1024L * 1024 * 1024;
43+
44+
/** Maximum number of entries (files + directories) in a directory injection. */
45+
static final int MAX_DIRECTORY_ENTRIES = 500_000;
46+
47+
/** Maximum directory tree depth for walk operations. */
48+
static final int MAX_WALK_DEPTH = 50;
49+
3750
private final DockerFileOperations fileOps;
3851
private final long maxOutputBytes;
3952
private final long maxSingleFileBytes;
53+
private final long maxDirectoryBytes;
54+
private final int maxDirectoryEntries;
4055

4156
public SandboxWorkspaceManager(DockerFileOperations fileOps) {
42-
this(fileOps, MAX_OUTPUT_BYTES, MAX_SINGLE_FILE_BYTES);
57+
this(fileOps, MAX_OUTPUT_BYTES, MAX_SINGLE_FILE_BYTES, MAX_DIRECTORY_BYTES, MAX_DIRECTORY_ENTRIES);
4358
}
4459

45-
/** Package-private constructor for testing with smaller limits. */
46-
SandboxWorkspaceManager(DockerFileOperations fileOps, long maxOutputBytes, long maxSingleFileBytes) {
60+
/** Package-private constructor for testing with custom limits. */
61+
SandboxWorkspaceManager(
62+
DockerFileOperations fileOps,
63+
long maxOutputBytes,
64+
long maxSingleFileBytes,
65+
long maxDirectoryBytes,
66+
int maxDirectoryEntries
67+
) {
4768
this.fileOps = fileOps;
4869
this.maxOutputBytes = maxOutputBytes;
4970
this.maxSingleFileBytes = maxSingleFileBytes;
71+
this.maxDirectoryBytes = maxDirectoryBytes;
72+
this.maxDirectoryEntries = maxDirectoryEntries;
5073
}
5174

5275
/**
@@ -93,29 +116,78 @@ public void injectDirectories(String containerId, Map<String, String> directoryM
93116
}
94117

95118
/**
96-
* Walk a host directory, create a tar archive, and copy it into the container.
97-
* The tar entries are prefixed with the final path component so that extracting at
98-
* the parent of containerPath produces the correct layout.
119+
* Walk a host directory, create a tar archive on a temp file, and stream it into the container.
120+
*
121+
* <p>Uses a temporary file instead of {@link ByteArrayOutputStream} to avoid loading the entire
122+
* archive into JVM heap. Memory usage is O(buffer_size) regardless of directory size, since each
123+
* file is streamed through a fixed buffer. The docker-java transport streams the tar lazily via
124+
* chunked transfer encoding — no additional buffering occurs downstream.
125+
*
126+
* <p>The tar entries are prefixed with the final path component so that extracting at the parent
127+
* of containerPath produces the correct layout.
99128
*/
100129
private void injectDirectoryViaTar(String containerId, String hostPath, String containerPath) {
101130
Path hostDir = Path.of(hostPath);
102-
// Container path parent is where we extract; the tar has the dir name as prefix
103131
Path containerParent = Path.of(containerPath).getParent();
104132
String dirName = Path.of(containerPath).getFileName().toString();
105133
if (containerParent == null) {
106134
containerParent = Path.of("/");
107135
}
108136

137+
Path tempTar = null;
138+
try {
139+
tempTar = Files.createTempFile("hephaestus-inject-", ".tar");
140+
141+
// Phase 1: Walk directory and write tar to temp file.
142+
// Memory: O(COPY_BUFFER_SIZE) — each file is streamed, never loaded whole.
143+
writeTarToFile(tempTar, hostDir, dirName, hostPath);
144+
145+
// Phase 2: Stream tar from disk to Docker daemon.
146+
// docker-java wraps this in InputStreamEntity (chunked transfer) — no heap copy.
147+
try (InputStream tarStream = new BufferedInputStream(Files.newInputStream(tempTar))) {
148+
fileOps.copyArchiveToContainer(containerId, containerParent.toString(), tarStream);
149+
}
150+
} catch (IOException e) {
151+
throw new SandboxException("Failed to inject directory " + hostPath + " into container " + containerId, e);
152+
} finally {
153+
if (tempTar != null) {
154+
try {
155+
Files.deleteIfExists(tempTar);
156+
} catch (IOException e) {
157+
log.warn("Failed to delete temp tar file {}: {}", tempTar, e.getMessage());
158+
}
159+
}
160+
}
161+
}
162+
163+
/** Buffer size for streaming file contents into the tar archive (64 KB). */
164+
private static final int COPY_BUFFER_SIZE = 64 * 1024;
165+
166+
/**
167+
* Write a tar archive of the given directory to a file on disk. Files are streamed through a
168+
* fixed-size buffer rather than loaded entirely into memory.
169+
*/
170+
private void writeTarToFile(Path tarFile, Path hostDir, String dirName, String hostPath) throws IOException {
171+
long[] totalBytes = { 0 };
172+
int[] entryCount = { 0 };
173+
109174
try (
110-
ByteArrayOutputStream baos = new ByteArrayOutputStream();
111-
TarArchiveOutputStream tar = new TarArchiveOutputStream(baos)
175+
OutputStream fileOut = new BufferedOutputStream(Files.newOutputStream(tarFile), COPY_BUFFER_SIZE);
176+
TarArchiveOutputStream tar = new TarArchiveOutputStream(fileOut);
177+
Stream<Path> paths = Files.walk(hostDir, MAX_WALK_DEPTH)
112178
) {
113179
tar.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
114180
tar.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_POSIX);
115181

116-
// Walk the directory tree and add each file/directory
117-
Files.walk(hostDir).forEach(path -> {
182+
paths.forEach(path -> {
118183
try {
184+
entryCount[0]++;
185+
if (entryCount[0] > maxDirectoryEntries) {
186+
throw new SandboxException(
187+
"Directory injection exceeds entry count limit (" + maxDirectoryEntries + "): " + hostPath
188+
);
189+
}
190+
119191
String relativePath = hostDir.relativize(path).toString();
120192
String entryName = relativePath.isEmpty() ? dirName : dirName + "/" + relativePath;
121193

@@ -125,27 +197,33 @@ private void injectDirectoryViaTar(String containerId, String hostPath, String c
125197
tar.putArchiveEntry(dirEntry);
126198
tar.closeArchiveEntry();
127199
} else if (Files.isRegularFile(path)) {
128-
byte[] content = Files.readAllBytes(path);
200+
long fileSize = Files.size(path);
201+
totalBytes[0] += fileSize;
202+
if (totalBytes[0] > maxDirectoryBytes) {
203+
throw new SandboxException(
204+
"Directory injection exceeds size limit (" + maxDirectoryBytes + " bytes): " + hostPath
205+
);
206+
}
207+
129208
TarArchiveEntry fileEntry = new TarArchiveEntry(entryName);
130-
fileEntry.setSize(content.length);
209+
fileEntry.setSize(fileSize);
131210
fileEntry.setModTime(Files.getLastModifiedTime(path).toMillis());
132211
tar.putArchiveEntry(fileEntry);
133-
tar.write(content);
212+
213+
// Stream file through fixed buffer — not Files.readAllBytes()
214+
try (InputStream fileIn = Files.newInputStream(path)) {
215+
fileIn.transferTo(tar);
216+
}
134217
tar.closeArchiveEntry();
135218
}
136-
// Skip symlinks for security (already validated above)
219+
// Symlinks are silently skipped: Files.walk() does not follow them by default,
220+
// and Files.isRegularFile/isDirectory return false for unresolved symlinks.
137221
} catch (IOException e) {
138222
throw new SandboxException("Failed to add file to tar: " + path, e);
139223
}
140224
});
141225

142226
tar.finish();
143-
144-
try (InputStream tarStream = new ByteArrayInputStream(baos.toByteArray())) {
145-
fileOps.copyArchiveToContainer(containerId, containerParent.toString(), tarStream);
146-
}
147-
} catch (IOException e) {
148-
throw new SandboxException("Failed to inject directory " + hostPath + " into container " + containerId, e);
149227
}
150228
}
151229

‎server/application-server/src/main/resources/application.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,8 @@ hephaestus:
424424
# OCI runtime: null=runc (default), "runsc"=gVisor (recommended for production)
425425
container-runtime: ${SANDBOX_CONTAINER_RUNTIME:}
426426
llm-proxy-port: ${SANDBOX_LLM_PROXY_PORT:8080}
427+
max-directory-bytes: ${SANDBOX_MAX_DIRECTORY_BYTES:1073741824} # 1 GB
428+
max-directory-entries: ${SANDBOX_MAX_DIRECTORY_ENTRIES:500000}
427429
default-resource-limits:
428430
memory-bytes: ${SANDBOX_MEMORY_BYTES:4294967296} # 4 GB (includes tmpfs)
429431
cpus: ${SANDBOX_CPUS:2.0}

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/ContainerSecurityPolicyTest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ void setUp() {
3434
null,
3535
8080,
3636
null,
37+
209_715_200L,
38+
500_000,
3739
null
3840
);
3941
securityPolicy = new ContainerSecurityPolicy(properties, null);
@@ -205,6 +207,8 @@ void shouldUseGlobalRuntime() {
205207
"runsc",
206208
8080,
207209
null,
210+
209_715_200L,
211+
500_000,
208212
null
209213
);
210214
ContainerSecurityPolicy policyWithRuntime = new ContainerSecurityPolicy(propsWithRuntime, null);
@@ -265,6 +269,8 @@ void shouldIncludeSeccompWhenProvided() {
265269
null,
266270
8080,
267271
null,
272+
209_715_200L,
273+
500_000,
268274
null
269275
),
270276
"{\"defaultAction\":\"SCMP_ACT_ERRNO\"}"
@@ -497,6 +503,8 @@ void shouldPreventRuntimeDowngrade() {
497503
"runsc",
498504
8080,
499505
null,
506+
209_715_200L,
507+
500_000,
500508
null
501509
);
502510
ContainerSecurityPolicy policyWithRuntime = new ContainerSecurityPolicy(propsWithRuntime, null);

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/DockerHealthIndicatorTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ void setUp() {
3434
null,
3535
8080,
3636
null,
37+
209_715_200L,
38+
500_000,
3739
null
3840
);
3941
indicator = new DockerHealthIndicator(containerManager, properties);

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/DockerSandboxAdapterTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ void setUp() {
9191
null,
9292
8080,
9393
null,
94+
209_715_200L,
95+
500_000,
9496
null
9597
);
9698
meterRegistry = new SimpleMeterRegistry();

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/DockerSandboxLiveTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ void setUp() {
6969
null,
7070
8080,
7171
null,
72+
209_715_200L,
73+
500_000,
7274
null
7375
);
7476

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/SandboxContainerManagerTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ void setUp() {
4747
null,
4848
8080,
4949
null,
50+
209_715_200L,
51+
500_000,
5052
null
5153
);
5254
executor = Executors.newSingleThreadExecutor();

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/SandboxNetworkManagerTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ void setUp() {
4242
null,
4343
8080,
4444
"app-server-id",
45+
209_715_200L,
46+
500_000,
4547
null
4648
);
4749
manager = new SandboxNetworkManager(networkOps, properties);
@@ -102,6 +104,8 @@ void shouldFallBackToHostname() {
102104
null,
103105
8080,
104106
null,
107+
209_715_200L,
108+
500_000,
105109
null
106110
);
107111
SandboxNetworkManager mgr = new SandboxNetworkManager(networkOps, propsNoId, () -> "hostname-container-id");
@@ -128,6 +132,8 @@ void shouldReturnNullWhenNoContainerId() {
128132
null,
129133
8080,
130134
null,
135+
209_715_200L,
136+
500_000,
131137
null
132138
);
133139
SandboxNetworkManager mgr = new SandboxNetworkManager(networkOps, propsNoId, () -> null);
@@ -151,6 +157,8 @@ void shouldReturnNullWhenHostnameBlank() {
151157
null,
152158
8080,
153159
null,
160+
209_715_200L,
161+
500_000,
154162
null
155163
);
156164
SandboxNetworkManager mgr = new SandboxNetworkManager(networkOps, propsNoId, () -> " ");
@@ -175,6 +183,8 @@ void shouldCacheContainerId() {
175183
null,
176184
8080,
177185
null,
186+
209_715_200L,
187+
500_000,
178188
null
179189
);
180190
SandboxNetworkManager mgr = new SandboxNetworkManager(networkOps, propsNoId, () -> {
@@ -217,6 +227,8 @@ void shouldNoOpWhenNoContainerId() {
217227
null,
218228
8080,
219229
null,
230+
209_715_200L,
231+
500_000,
220232
null
221233
);
222234
SandboxNetworkManager mgr = new SandboxNetworkManager(networkOps, propsNoId, () -> null);

0 commit comments

Comments
 (0)