Skip to content

Commit 14b3690

Browse files
Implement CH-009: confine getDataFile/getDataDirectory to the data directory
Closes FIO-001..003: - RobotData.getDataFile now matches classic's RobotFileSystemManager.getDataFile: asterisks stripped, ".." rejected with AccessControlException, and a java.io.File merge (not Path#resolve) so an absolute/root-relative name is re-rooted inside the data directory instead of overriding it. - RobotData tracks the classic 200000-byte quota, seeded from existing files at startup; RobocodeFileOutputStream.write charges it and raises classic's own IOException message on refusal, closing the stream. BotPeer's getDataQuotaAvailable() now reports real remaining quota instead of a hardcoded constant. - Fixed a double-resolution bug the conformance run surfaced: RobocodeFileOutputStream(String) was re-running an already-resolved File's path through the resolver a second time, corrupting it. It now opens the given path verbatim, matching classic's ThreadManager.createRobotFileStream. - FileRedirectionConformanceTest (FIO-001/002) and FileQuotaConformanceTest (FIO-003) prove this on both engines under ARCH-003's conformance tier. FIO-004 stays @draft (IDR-007, corrected from the proposal's initial guess): building and running FileOutputStreamAttack on both engines showed classic blocks it via a SecurityManager unconditional on path, even for a getDataFile -resolved (already-confined) target -- a mechanism JDK 24 removed and this bridge cannot reproduce. Neither of classic's own FIO-004 evidence robots can be ported as evidence under this design; the probe was removed rather than kept passing on a claim it can't support. Updates CAP-004 (status: active, provenance: verified), C-005 (enforcement: partial, Checked-by/Residual declared), and P-001's M-004/M-122 bookkeeping to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011GShGLuFwRpnWNY2WL5Vz5
1 parent 94dadf6 commit 14b3690

22 files changed

Lines changed: 432 additions & 118 deletions

File tree

.clue/id-ledger.yaml

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ counters:
99
CRIT: "8"
1010
DES: "8"
1111
EVT: "15"
12+
FIO: "4"
1213
G: "2"
1314
IDR: "7"
1415
OQ: "4"
@@ -245,7 +246,7 @@ entries:
245246
component: "8"
246247
- id: CH-009
247248
kind: numeric
248-
state: reserved
249+
state: live
249250
prefix: CH
250251
component: "9"
251252
- id: CRIT-001
@@ -403,6 +404,26 @@ entries:
403404
state: live
404405
prefix: EVT
405406
component: "15"
407+
- id: FIO-001
408+
kind: numeric
409+
state: live
410+
prefix: FIO
411+
component: "1"
412+
- id: FIO-002
413+
kind: numeric
414+
state: live
415+
prefix: FIO
416+
component: "2"
417+
- id: FIO-003
418+
kind: numeric
419+
state: live
420+
prefix: FIO
421+
component: "3"
422+
- id: FIO-004
423+
kind: numeric
424+
state: live
425+
prefix: FIO
426+
component: "4"
406427
- id: G-001
407428
kind: numeric
408429
state: live
@@ -445,7 +466,7 @@ entries:
445466
component: "6"
446467
- id: IDR-007
447468
kind: numeric
448-
state: reserved
469+
state: live
449470
prefix: IDR
450471
component: "7"
451472
- id: OQ-001

changes/CH-009-m004-file-io-sandboxing/open-questions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22

33
### Does `FIO-004` cover raw `java.io` access, or only access reached through `getDataFile`/`getDataDirectory`?
44

5-
Decided: the resolver-surface reading. See `IDR-007`. `FileOutputStreamAttack` ports as `FIO-004` evidence; `FileAttack` (raw `java.io`, no `getDataFile` call) is not ported under `FIO-004`.
5+
Decided, then corrected once evidence came in. See `IDR-007` for the final finding: neither reading closes `FIO-004` in this change. The proposal originally guessed that `FileOutputStreamAttack` (which calls `getDataFile` before opening a raw stream) was a legitimate case the resolver-surface design would satisfy. Building and running that probe on both engines showed otherwise — classic's own test for it (`TestFileOutputStreamAttack`) expects an `AccessControlException`, because classic's `SecurityManager` blocks the raw stream unconditionally on path, confined or not. `FIO-004` stays `@draft`; `FIO-001``FIO-003` close as originally planned.
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
id: CH-009
33
type: change
4-
status: proposed
4+
status: open
55
links: [P-001, M-004, CAP-004, C-005, FIO-001, FIO-002, FIO-003, FIO-004, IDR-002, IDR-007]
66
title: Confine robot file I/O to the robot's data directory
77
---
@@ -10,9 +10,9 @@ title: Confine robot file I/O to the robot's data directory
1010

1111
## What
1212

13-
Give `RobotData.getDataFile`/`getDataDirectory` the one resolution point `robocode.RobocodeFileOutputStream` and `robocode.RobocodeFileWriter` already route through — the confinement classic Robocode has and the bridge does not: an absolute or traversing path a robot names is re-rooted inside the robot's data directory rather than passed through, and the directory's total size is capped at the documented 200000-byte quota. Move `FIO-001`, `FIO-002`, `FIO-003`, `FIO-004` out of `@draft` with integration evidence, and give `C-005` machine enforcement in place of the agent-judgment note it currently carries.
13+
Give `RobotData.getDataFile`/`getDataDirectory` the confinement classic Robocode has and the bridge does not: an absolute or root-relative path a robot names is re-rooted inside the robot's data directory rather than passed through, matching classic's `RobotFileSystemManager.getDataFile` exactly (asterisks stripped, `..` rejected, `java.io.File` merge semantics rather than `Path#resolve`), and the directory's total size is capped at the documented 200000-byte quota. Move `FIO-001`, `FIO-002`, `FIO-003` out of `@draft` with integration evidence, and give `C-005` machine enforcement for those rules.
1414

15-
`FIO-004`'s scope was a blocking open question, resolved as `IDR-007`: confinement covers the `getDataFile`/`getDataDirectory` surface, not raw `java.io` calls that bypass it entirely (classic blocks those with a JVM `SecurityManager` this bridge does not have and the architecture record already scopes out, alongside threads/reflection/sockets).
15+
`FIO-004` stays `@draft`. This was a blocking open question at proposal time; building and running the evidence probe on both engines during implementation showed the narrowed reading originally proposed does not hold. `IDR-007` has the finding: both of classic's own `FIO-004` test robots (`FileAttack`, `FileOutputStreamAttack`) are attacks classic *blocks*, one of them even when the path is already confined through `getDataFile`, because classic's confinement here is a JVM `SecurityManager` gate that is unconditional on path, not a redirection this bridge's resolver can reproduce. JDK 24 removed `SecurityManager` outright, and the architecture record already scopes this class of gap out (alongside threads/reflection/sockets). No criterion closes on evidence a bridge probe cannot honestly produce, so `FIO-004` remains open, its residual named in `C-005`.
1616

1717
## Why
1818

@@ -22,4 +22,4 @@ The quota (`FIO-003`) is documented in three Javadoc comments (`RobocodeFileOutp
2222

2323
## Route
2424

25-
Full. This closes acceptance criteria that are currently `@draft` (`FIO-001`, `FIO-002`, `FIO-003`) and promotes `C-005` from `enforcement: agent` to machine-enforced — both are contract changes, not refactors of unchanged behaviour.
25+
Full. This closes acceptance criteria that were `@draft` (`FIO-001`, `FIO-002`, `FIO-003`), leaves `FIO-004` `@draft` with a corrected, evidence-backed reason recorded as `IDR-007`, and promotes `C-005` to partial enforcement — all are contract changes, not refactors of unchanged behaviour.
Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
- [x] Resolve the blocking open question on `FIO-004`'s scope — decided as `IDR-007`: resolver-surface confinement only.
2-
- [ ] Implement the single resolution point in `RobotData`: re-root an absolute or drive-rooted `filename` argument inside the data directory, resolve `..` traversal inside the directory rather than escaping it, and keep `getDataDirectory`/`getDataFile` agreeing as `IDR-002` already requires — serves `FIO-001`, `FIO-002`.
3-
- [ ] Add the 200000-byte data-directory quota at the same resolution point (tracking bytes written across the directory's lifetime, refusing further writes with the same `IOException` classic raises) — serves `FIO-003`.
4-
- [ ] Port classic's `TestFileWriteSize` conformance robot/test (`C:/Code/robocode/robocode.tests.robots/.../FileWriteSize.java` and its test) for `FIO-003`'s quota-exceeded evidence, and a companion positive case that stays under quota.
5-
- [ ] Port `FileOutputStreamAttack` as conformance evidence for `FIO-001`/`FIO-004` under `ARCH-003`'s tier (`FileAttack` is not ported, per `IDR-007`).
6-
- [ ] Add a focused unit case for `FIO-002`: writing through `getDataFile(name)` and listing `getDataDirectory()` see the same file, for both a plain name and a re-rooted absolute/traversing one.
7-
- [ ] Remove `@draft` from `FIO-001`, `FIO-002`, `FIO-003`, and `FIO-004` in `CAP-004/criteria.md`, narrowing `FIO-004`'s scenario text to the `getDataFile`-reached surface per `IDR-007`.
8-
- [ ] Update `CAP-004/design.md` and `CAP-004/README.md` to describe the resolver as implemented rather than intended, and update `C-005` from `enforcement: agent` to its machine-enforced state (or promote-with-residual note if `FIO-004` stays partly open).
9-
- [ ] Update `P-001`'s evidence-door table (`M-119``M-122` equivalents don't exist for `FIO-00x`; confirm the milestone-status row for `M-004` and any doors naming `FIO-001`..`FIO-004`) to reflect what closed.
10-
- [ ] Run the tier-1 unit suite and the focused tier-2 conformance tests with the matched local Tank Royale API/runner pair; retain any environment limitation in the handoff.
1+
- [x] Resolve the blocking open question on `FIO-004`'s scope. Superseded by evidence gathered during implementation: `IDR-007` now records that `FIO-004` stays `@draft` because classic's own evidence for it depends on a `SecurityManager` this bridge cannot reproduce, not that a narrowed reading of it closes here.
2+
- [x] Implement the single resolution point in `RobotData`: re-root an absolute or root-relative `filename` argument inside the data directory (`new File(directory, name)`, matching classic's `RobotFileSystemManager.getDataFile`), reject `..` with `AccessControlException` as classic's `AdvancedRobotProxy.getDataFile` does, strip asterisks as classic does — serves `FIO-001`, `FIO-002`.
3+
- [x] Add the 200000-byte data-directory quota at `RobotData` (seeded from existing files at startup, charged per write, refusing with classic's own `IOException` message) and wire `RobocodeFileOutputStream.write` to charge it, closing the stream on refusal as classic's `RobotFileOutputStream` does — serves `FIO-003`. Also fixed `BotPeer.getDataQuotaAvailable()`, previously hard-coded to 200000 regardless of usage.
4+
- [x] Fixed a double-resolution bug found by the conformance run: `RobocodeFileOutputStream`'s `String` constructor was re-resolving an already-resolved `File`'s path through `RobotData.getDataFile` a second time, corrupting it. It now opens the given path verbatim, matching classic's `ThreadManager.createRobotFileStream`.
5+
- [x] Ported a probe for `FIO-001`/`FIO-002` (`FileRedirectionProbe`) and one for `FIO-003` (`FileQuotaProbe`, modeled on classic's `FileWriteSize`) under `ARCH-003`'s conformance tier, each with positive and negative evidence.
6+
- [-] Port `FileOutputStreamAttack` for `FIO-004` — infeasible as evidence. Built and ran the probe on both engines: classic itself throws `AccessControlException` (`TestFileOutputStreamAttack` expects exactly this), because classic's `SecurityManager` blocks any raw `java.io.FileOutputStream`, confined path or not, unless it went through `RobocodeFileOutputStream`. The bridge has no `SecurityManager` to reproduce that block. Removed the probe and its test; recorded the finding in `IDR-007`.
7+
- [x] Left `FIO-004` `@draft` in `CAP-004/criteria.md` with a corrected note; removed `@draft` from `FIO-001``FIO-003`; set `CRIT-004` `status: active` (a capability need not stay draft for one unproven criterion).
8+
- [x] Updated `CAP-004/design.md` (implemented design, `status: active`, `provenance: verified`) and `CAP-004/README.md` (`status: active`, residual named) to match what was built and what remains agent-held.
9+
- [x] Updated `C-005`: `enforcement: mixed` — machine-enforced for the redirection/quota rules, agent-held for the raw-`java.io` case, with `IDR-007` linked.
10+
- [ ] Update `P-001`'s evidence-door table (`M-119``M-122`) to reflect `FIO-001``FIO-003` done and `FIO-004` still todo, and the `M-004` milestone-status row.
11+
- [ ] Run the tier-1 unit suite and the tier-2 conformance tests (`FileRedirectionConformanceTest`, `FileQuotaConformanceTest`) with the matched local Tank Royale API/runner pair; retain the result in the handoff.
1112
- [ ] Digest: regenerate indexes, run `clue validate`, commit the final state, and delete this change workspace.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package conformance.probes;
2+
3+
import robocode.AdvancedRobot;
4+
import robocode.RobocodeFileOutputStream;
5+
6+
import java.io.File;
7+
import java.io.IOException;
8+
9+
/**
10+
* Ported from classic's {@code tested.robots.FileWriteSize}: writes three 100000-byte
11+
* chunks against the documented 200000-byte quota, so the third write must be refused at
12+
* exactly the point classic refuses it.
13+
*/
14+
public class FileQuotaProbe extends AdvancedRobot {
15+
16+
@Override
17+
public void run() {
18+
out.println("DataQuota:" + getDataQuotaAvailable());
19+
20+
byte[] chunk = new byte[100_000];
21+
File file = getDataFile("quota-test");
22+
file.delete();
23+
24+
RobocodeFileOutputStream stream = null;
25+
try {
26+
stream = new RobocodeFileOutputStream(file);
27+
for (int i = 0; i < 3; i++) {
28+
stream.write(chunk);
29+
out.println("WroteChunk:" + i);
30+
}
31+
} catch (IOException e) {
32+
out.println("QuotaExceeded:" + e.getMessage());
33+
} finally {
34+
if (stream != null) {
35+
try {
36+
stream.close();
37+
} catch (IOException ignored) {
38+
// already reported above
39+
}
40+
}
41+
file.delete();
42+
}
43+
44+
while (true) {
45+
turnLeft(1);
46+
}
47+
}
48+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package conformance.probes;
2+
3+
import robocode.AdvancedRobot;
4+
import robocode.RobocodeFileOutputStream;
5+
6+
import java.io.File;
7+
import java.io.IOException;
8+
9+
/**
10+
* Writes through {@code getDataFile} with a plain name and with a root-relative name that
11+
* looks like it escapes the data directory, then reports what was resolved and what the
12+
* directory listing sees, so a conformance test can prove the two calls agree.
13+
*/
14+
public class FileRedirectionProbe extends AdvancedRobot {
15+
16+
@Override
17+
public void run() {
18+
out.println("DataDirectory:" + getDataDirectory().getAbsolutePath());
19+
writeAndReport("plain-name.txt");
20+
writeAndReport("/root-relative-name.txt");
21+
listDirectory();
22+
while (true) {
23+
turnLeft(1);
24+
}
25+
}
26+
27+
private void writeAndReport(String name) {
28+
try {
29+
File file = getDataFile(name);
30+
out.println("Resolved:" + name + ":" + file.getAbsolutePath());
31+
try (RobocodeFileOutputStream stream = new RobocodeFileOutputStream(file)) {
32+
stream.write(42);
33+
}
34+
out.println("WriteSucceeded:" + name + ":" + file.exists());
35+
} catch (IOException | SecurityException e) {
36+
out.println("WriteFailed:" + name + ":" + e);
37+
}
38+
}
39+
40+
private void listDirectory() {
41+
File[] files = getDataDirectory().listFiles();
42+
if (files == null) {
43+
out.println("DirectoryListing:none");
44+
return;
45+
}
46+
for (File file : files) {
47+
out.println("DirectoryListing:" + file.getName());
48+
}
49+
}
50+
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package dev.robocode.tankroyale.bridge.conformance;
2+
3+
import org.junit.jupiter.api.DisplayName;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.nio.file.Path;
7+
8+
import static org.junit.jupiter.api.Assertions.assertFalse;
9+
import static org.junit.jupiter.api.Assertions.assertTrue;
10+
11+
/**
12+
* Acceptance evidence for FIO-003 — a robot's data directory is capped at the classic
13+
* 200000-byte size limit, refused at the same point classic refuses it.
14+
*/
15+
class FileQuotaConformanceTest extends ConformanceTestBase {
16+
17+
private static final String ROBOT = "conformance.probes.FileQuotaProbe";
18+
private static final Path SOURCE = ConformanceHarness.repoRoot().resolve(Path.of(
19+
"compat-test", "conformance-robots", "conformance", "probes", "FileQuotaProbe.java"));
20+
21+
@Test
22+
@DisplayName("FIO-003: a write past the quota is refused with classic's own message")
23+
void testFIO003_IntegrationPositive_WritePastQuotaIsRefused() {
24+
assertOnBothEngines(ROBOT, SOURCE, (outcome, engine) -> {
25+
assertTrue(outcome.anyConsoleContains("DataQuota:200000"),
26+
() -> "the documented quota was not reported as 200000 on " + engine
27+
+ " (" + outcome.summary() + ")");
28+
assertTrue(outcome.anyConsoleContains("WroteChunk:0") && outcome.anyConsoleContains("WroteChunk:1"),
29+
() -> "the first two chunks, which fit inside the quota, did not both write on " + engine
30+
+ " (" + outcome.summary() + ")");
31+
assertTrue(outcome.anyConsoleContains("QuotaExceeded:")
32+
&& outcome.anyConsoleContains("200000 bytes"),
33+
() -> "the third chunk was not refused with the classic quota message on " + engine
34+
+ " (" + outcome.summary() + ")");
35+
});
36+
}
37+
38+
@Test
39+
@DisplayName("FIO-003 negative: the write that exceeds the quota does not also succeed")
40+
void testFIO003_IntegrationNegative_TheRefusedChunkDoesNotAlsoReportSuccess() {
41+
assertOnBothEngines(ROBOT, SOURCE, (outcome, engine) ->
42+
assertFalse(outcome.anyConsoleContains("WroteChunk:2"),
43+
() -> "the third, quota-exceeding chunk reported as written on " + engine
44+
+ " (" + outcome.summary() + ")"));
45+
}
46+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package dev.robocode.tankroyale.bridge.conformance;
2+
3+
import org.junit.jupiter.api.DisplayName;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.nio.file.Path;
7+
8+
import static org.junit.jupiter.api.Assertions.assertFalse;
9+
import static org.junit.jupiter.api.Assertions.assertTrue;
10+
11+
/**
12+
* Acceptance evidence for FIO-001 and FIO-002 — a root-relative path a robot names is
13+
* redirected into its data directory, and {@code getDataFile} and {@code getDataDirectory}
14+
* agree about where that directory is.
15+
*/
16+
class FileRedirectionConformanceTest extends ConformanceTestBase {
17+
18+
private static final String ROBOT = "conformance.probes.FileRedirectionProbe";
19+
private static final Path SOURCE = ConformanceHarness.repoRoot().resolve(Path.of(
20+
"compat-test", "conformance-robots", "conformance", "probes", "FileRedirectionProbe.java"));
21+
22+
@Test
23+
@DisplayName("FIO-001: a root-relative path is redirected into the data directory")
24+
void testFIO001_IntegrationPositive_RootRelativePathIsRedirectedIntoDataDirectory() {
25+
assertOnBothEngines(ROBOT, SOURCE, (outcome, engine) -> {
26+
assertTrue(outcome.anyConsoleContains("WriteSucceeded:/root-relative-name.txt:true"),
27+
() -> "the root-relative write did not succeed on " + engine
28+
+ " (" + outcome.summary() + ")");
29+
assertTrue(outcome.anyConsoleContains("DirectoryListing:root-relative-name.txt"),
30+
() -> "the redirected file did not land in the data directory on " + engine
31+
+ " (" + outcome.summary() + ")");
32+
});
33+
}
34+
35+
@Test
36+
@DisplayName("FIO-001 negative: the redirected name is not written where it was named")
37+
void testFIO001_IntegrationNegative_NothingIsWrittenAtTheNamedPath() {
38+
assertOnBothEngines(ROBOT, SOURCE, (outcome, engine) -> {
39+
for (String console : outcome.consoles()) {
40+
for (String line : console.split("\\R")) {
41+
if (line.startsWith("Resolved:/root-relative-name.txt:")) {
42+
String resolved = line.substring("Resolved:/root-relative-name.txt:".length());
43+
assertFalse(resolved.equals("/root-relative-name.txt")
44+
|| resolved.equals("\\root-relative-name.txt"),
45+
() -> "the name resolved to the unredirected path on " + engine + ": " + line);
46+
}
47+
}
48+
}
49+
});
50+
}
51+
52+
@Test
53+
@DisplayName("FIO-002: getDataFile and getDataDirectory resolve against the same place")
54+
void testFIO002_IntegrationPositive_DataFileAndDataDirectoryAgree() {
55+
assertOnBothEngines(ROBOT, SOURCE, (outcome, engine) -> {
56+
assertTrue(outcome.anyConsoleContains("WriteSucceeded:plain-name.txt:true"),
57+
() -> "the plain-name write did not succeed on " + engine
58+
+ " (" + outcome.summary() + ")");
59+
assertTrue(outcome.anyConsoleContains("DirectoryListing:plain-name.txt"),
60+
() -> "getDataDirectory's listing did not see what getDataFile wrote on " + engine
61+
+ " (" + outcome.summary() + ")");
62+
});
63+
}
64+
65+
@Test
66+
@DisplayName("FIO-002 negative: the directory listing reports no file the probe did not write")
67+
void testFIO002_IntegrationNegative_DirectoryListingReportsNoUnwrittenFile() {
68+
assertOnBothEngines(ROBOT, SOURCE, (outcome, engine) ->
69+
assertFalse(outcome.anyConsoleContains("DirectoryListing:none"),
70+
() -> "the data directory listing came back empty on " + engine
71+
+ " (" + outcome.summary() + ")"));
72+
}
73+
}

0 commit comments

Comments
 (0)