diff --git a/CLAUDE.md b/CLAUDE.md index 64f7b52..18ad5a8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -354,6 +354,36 @@ last eighty French names, which left **0 differences over 7 734 captured strings of the diff proves nothing; the DOM does. And the DOM does not prove everything either — it never opens the save path, which is why the two defects above needed a test apiece. +## What wins between the file and the command line + +**The command line, on every setting — and for a long time that was true of nine out of +twenty-two.** `merge` was a hand-written list of `if`s, and it stopped being complete around +the fifth option added after it. Everything absent from the list was dropped without a word +the moment a `--config` sat on the line beside it: `--level coverage` measured everything, +`--jacoco-reports data` wrote its hundred and eighty files. Nothing failed — the run simply +did something other than what it had been asked, and the only way to notice was to count the +files afterwards. Found on 29 August 2026 while adding a key, not while reading the code. + +The rule is now **applied rather than enumerated**: a setting that differs from a fresh +`Config` is one the command line set, and it overrides the file. A list has to be remembered +at every new option; a comparison does not. `SettingsPrecedenceTest` walks `Config`'s fields +by reflection and fails the build on any that does not come through — which is the guard a +longer list could never be. + +**`SERVE_HOST` says where to listen, and never that one should.** The interface a machine +exposes is a property of that machine, so it belongs in the configuration; the decision to +serve is a gesture, so it stays on the command line. That split is the whole point: a +configuration file travels — into a repository, a ticket, another machine — and one that +could open a port by travelling would be unreadable safely. `--serve` remains the only thing +that puts the tool into listening, and a test holds it: exactly one `serve = true` in the +options switch. + +Past the loopback, the report becomes readable — and annotatable — by whoever reaches the +port, and it carries the argument values captured from a real application. So the default +stays `127.0.0.1`, the key is shipped commented out with its price beside it, the tool warns +at start-up when it listens wider without a secret, and the documentation carries the +deployment that does it properly: the proxy terminates TLS and the tool answers only to it. + ## Conventions - **The tool speaks English, and so does the code now.** The switch happened in stages — the diff --git a/bin/acceptance-local.sh b/bin/acceptance-local.sh index 5dddcbe..3c69c42 100755 --- a/bin/acceptance-local.sh +++ b/bin/acceptance-local.sh @@ -207,6 +207,53 @@ grep -q '"sourcesDisponibles":{"' reassemble/index.html step $? "the annotated code is back in the report" echo +# A configuration file and a command line say different things: the line wins. It used to +# win on nine settings out of twenty-two, and to lose in silence on the others. +echo "4 sexies. A --config beside the options does not swallow them" +cat > both.conf < both.log 2>&1 +step $? "a measurement with a --config AND options ends" +grep -q "no stack sampling" both.log +step $? " --level coverage was obeyed, not the file's \"full\"" +grep -q "JACOCO_REPORTS=data" both.log +step $? " and --jacoco-reports data too" +[ "$(count both)" -lt 20 ] +step $? " which shows on the disk: $(count both) files, not the file's hundreds" + +# The same, with the file found by its name instead of named on the line: it is still a +# file, and the options typed beside it were still typed. +mkdir -p implicit +( cd implicit && cat > runtime-xray.conf < implicit.log 2>&1 ) +step $? "a measurement with the implicit runtime-xray.conf ends" +grep -q "Configuration read from" implicit/implicit.log +step $? " the file was indeed found by its name" +grep -q "no stack sampling" implicit/implicit.log +step $? " and --level coverage was obeyed all the same" + +# The interface is a setting; putting the tool into listening is a gesture. A file that +# travels — into a repository, a ticket, another machine — must not be able to open a port. +java -jar "$JAR" --config both.conf --report-only > serves-not.log 2>&1 +step $? "reading with SERVE_HOST set and no --serve ends instead of listening" +! grep -q "Report served at" serves-not.log +step $? " nothing was put into listening" +echo + echo "5. The server writes the annotations beside the measurements" PORT="$(python3 -c 'import socket;s=socket.socket();s.bind(("127.0.0.1",0));print(s.getsockname()[1]);s.close()')" java -jar "$JAR" --report-only --out out \ diff --git a/docs/outil/annotations.md b/docs/outil/annotations.md index a72862b..0258742 100644 --- a/docs/outil/annotations.md +++ b/docs/outil/annotations.md @@ -159,6 +159,75 @@ diagnostic tool, not a service: the only write it accepts is a run's annotation, whose name it chooses itself, and the file serving refuses any path that would leave the served directory. +### Deploying it, so nobody has to remember the options + +Repeating `--serve-host` on every launch is how one ends up not repeating it. Two things +carry it instead — and neither of them ever *starts* a server, which is the point: + +**In the project's configuration**, because which interface a machine exposes is a property +of that machine: + +```conf +SERVE_HOST="0.0.0.0" +``` + +> ⚠️ **This key widens what is readable, and it is off by default.** Past the loopback, the +> report — with the argument values captured from a real application — becomes readable by +> whoever reaches the port, and the annotation writes become theirs too. The key sets the +> interface and nothing else: `--serve` remains the only gesture that puts the tool into +> listening, so a configuration file copied into a repository, a ticket or another machine +> cannot open a port by travelling. + +**In a service**, for a machine that serves results permanently. The secret goes through the +environment rather than the command line, where `ps` would show it: + +```ini +# /etc/systemd/system/runtime-xray.service +[Unit] +Description=Runtime X-Ray — shared report +After=network-online.target + +[Service] +User=xray +WorkingDirectory=/srv/xray +EnvironmentFile=/etc/runtime-xray.env # XRAY_SERVE_TOKEN=…, chmod 600 +ExecStart=/usr/bin/java -jar /opt/runtime-xray/runtime-xray.jar \ + --report-only --out /srv/xray/campaigns \ + --serve 8787 --serve-host 127.0.0.1 +Restart=on-failure + +[Install] +WantedBy=multi-user.target +``` + +Note the `127.0.0.1` in a unit meant to serve a whole team: **the TLS proxy is what listens +outside**, and the tool answers only to it. That way the plain HTTP never leaves the machine, +which is what the first caveat above asks for. + +```nginx +server { + listen 443 ssl; + server_name xray.internal.example.com; + ssl_certificate /etc/ssl/certs/xray.pem; + ssl_certificate_key /etc/ssl/private/xray.key; + + location / { + proxy_pass http://127.0.0.1:8787; + proxy_set_header Host $host; + # The report is a lot of small files, and some of them are large enough to matter. + proxy_buffering off; + } +} +``` + +Two things to check before opening it, and they are the ones people skip: + +- **The output directory is the perimeter.** Everything under `--out` is served, and a + campaign carries the observed application's log and its captured values. Serve a directory + that holds only what you meant to hand over. +- **`--serve-host 0.0.0.0` without a proxy is a decision**, not a shortcut. It works, the tool + warns about it at start-up, and the warning is the whole of the protection you then have. + ## Where the file is written Seen as files, **a run is a directory**. Its annotation can live in three places, and the diff --git a/docs/outil/mode-emploi.md b/docs/outil/mode-emploi.md index 6b26869..90bf727 100644 --- a/docs/outil/mode-emploi.md +++ b/docs/outil/mode-emploi.md @@ -200,7 +200,7 @@ the first go — **[Reducing the footprint on a large codebase](empreinte.md)** | `--follow [port]` | Serves **a page showing the run in progress** (default: 8788, local loopback): activity band, busy cores, output produced, tail of the log. The `progression.jsonl` file, for its part, is **always** written: `tail -f /progression.jsonl` follows the run with no browser and no open port | | `--context ["question"]` | Writes on standard output **a bounded extract of the report, ready to hand to a language model**: the facts that answer the question, their vocabulary, and at the top what was *not* measured. Sends nothing anywhere. The families picked up are **announced on standard error**; `--help` gives the table of recognised words, **in English** — French works too, undocumented — see [Having a report read by an AI](integration-ia.md) | | `--families a,b` | Names the fact families to attach **instead of deducing them from the question**. This is the path for scripts: the result no longer depends on the words used. An unknown family stops, with the list of those that exist | -| `--serve-host ` | Listening interface (default: `127.0.0.1`). `0.0.0.0` for a shared server | +| `--serve-host ` / `SERVE_HOST` | Listening interface (default: `127.0.0.1`), or one interface in particular. **Neither ever starts a server** — only `--serve` does — so a configuration file that travels cannot open a port. Past the loopback, the report and the values captured in it become readable and annotatable by whoever reaches the port: see [deploying it](annotations.md#deploying-it-so-nobody-has-to-remember-the-options) | | `--serve-token [secret]` | Guards the served report with a **shared secret**, asked once then remembered for twelve hours. With no value, a secret is drawn at random and shown. `XRAY_SERVE_TOKEN` does the same without exposing it in `ps`. Without the option, nothing is asked: to be kept for the local loopback or an already filtered network — see [what that secret is worth](annotations.md#what-that-secret-is-worth-and-what-it-is-not) | These options combine: `--report-only --serve` serves measurements already taken, without diff --git a/orchestrator/src/main/java/lab/xray/Config.java b/orchestrator/src/main/java/lab/xray/Config.java index 24723f2..4b41d08 100644 --- a/orchestrator/src/main/java/lab/xray/Config.java +++ b/orchestrator/src/main/java/lab/xray/Config.java @@ -59,6 +59,23 @@ public final class Config { */ public String archive = ""; public String runName = ""; + /** + * The interface the served report listens on — {@code 127.0.0.1} unless said otherwise. + * + *

It is here because which interface a machine exposes is a property of that + * machine, and repeating it on every launch is how one ends up not repeating it. + * What is not here, and must not be, is the decision to serve at all: this key + * says where to listen when {@code --serve} is given, and never puts anything into + * listening by itself. A configuration file travels — into a repository, into a ticket, + * onto another machine — and a file that can open a port by travelling is a file nobody + * can read safely. + * + *

Widening it beyond the loopback publishes the captured argument values of a real + * application to whoever reaches the port, and opens the annotation writes to them too. + * The tool says so at start-up when there is no shared secret; the documentation says + * what that secret is worth. + */ + public String serveHost = "127.0.0.1"; public int attachAfterSeconds = 8; public int maxSeconds = 600; public int watchCount = 10; @@ -276,6 +293,7 @@ void set(String key, String value) { case "LEVEL", "NIVEAU" -> level = value; case "COVER_INCLUDES" -> coverIncludes = value; case "JACOCO_REPORTS" -> jacocoReports = value; + case "SERVE_HOST" -> serveHost = value; case "ARCHIVE" -> archive = value; case "SAMPLE_INTERVAL_MS" -> sampleIntervalMs = parse(value, sampleIntervalMs); case "FOLLOW_PORT", "SUIVI_PORT" -> followPort = parse(value, followPort); @@ -560,12 +578,18 @@ public static void writeTemplate(Path file) throws IOException { # or "all". The files go into /exports/. #EXPORT="cpuprofile,lcov" - # Serving the report is not set here: it is a way of launching, not a property - # of the project. "--serve" serves the output directory and lets the page write - # its annotations beside the runs; "--serve-host 0.0.0.0" makes it a shared - # server, where several people annotate in parallel, which "--serve-token" - # closes with a secret (XRAY_SERVE_TOKEN so as not to expose it in "ps"). A - # secret does not belong in a file under version control. + # WHERE the report is served, when it is. "--serve" alone starts the server and + # lets the page write its annotations beside the runs; this key only decides the + # interface it listens on, and never puts anything into listening by itself — + # a configuration file travels, and one that could open a port by travelling + # would be unreadable safely. + # + # Beyond the loopback, the report becomes readable — and annotatable — by whoever + # reaches the port: it carries the captured argument values of a real + # application. Guard it with "--serve-token" (XRAY_SERVE_TOKEN so as not to + # expose the secret in "ps"), and put TLS in front, since this speaks plain HTTP. + # A secret does not belong in a file under version control. + #SERVE_HOST="0.0.0.0" # The repository to fetch the analysis components from, once. On a closed # network, name the internal mirror: it is the only setting that matters for diff --git a/orchestrator/src/main/java/lab/xray/Main.java b/orchestrator/src/main/java/lab/xray/Main.java index c9a4fc2..7974063 100644 --- a/orchestrator/src/main/java/lab/xray/Main.java +++ b/orchestrator/src/main/java/lab/xray/Main.java @@ -76,7 +76,6 @@ private static int run(String[] args) throws Exception { boolean reportOnly = false; boolean serve = false; int servePort = 8787; - String serveHost = "127.0.0.1"; String serveToken = null; boolean randomToken = false; @@ -135,7 +134,7 @@ private static int run(String[] args) throws Exception { config.followPort = Integer.parseInt(args[++i]); } } - case "--serve-host" -> serveHost = args[++i]; + case "--serve-host" -> config.serveHost = args[++i]; case "--serve-token" -> { // The secret attaches to the option, or stays silent: "--serve-token" // alone draws one at random and shows it. That is the most frequent @@ -200,7 +199,12 @@ private static int run(String[] args) throws Exception { Path def = Path.of(DEFAULT_CONFIG); if (Files.isRegularFile(def)) { System.out.println("▶ Configuration read from: " + DEFAULT_CONFIG); - config = Config.load(def); + // Merged, exactly as an explicit --config is: a file found by its name + // rather than named on the line is still a file, and the options typed + // beside it were still typed. + Config fromFile = Config.load(def); + merge(fromFile, config); + config = fromFile; } else { Config.writeTemplate(def); announceTemplate(def, ""); @@ -283,7 +287,7 @@ private static int run(String[] args) throws Exception { System.out.println(" Pass it to whoever needs access to the " + "report."); } - LocalServer.serve(outDir, serveHost, servePort, () -> { + LocalServer.serve(outDir, config.serveHost, servePort, () -> { // After a write the page is rebuilt: the annotation becomes the report's, // and not merely this browser's. Dashboard.build(outDir, sourceRoots(served), served.watchCount, served.hidden(), @@ -880,17 +884,34 @@ private static void announceTemplate(Path file, String relaunch) { System.out.println(" java -jar runtime-xray.jar " + relaunch); } - /** The command line's options win over the file. */ - private static void merge(Config base, Config overrides) { - if (!overrides.javaCommand.isBlank()) base.javaCommand = overrides.javaCommand; - if (!overrides.rootMethod.isBlank()) base.rootMethod = overrides.rootMethod; - if (!overrides.classesDir.isBlank()) base.classesDir = overrides.classesDir; - if (!overrides.hiddenPackages.isBlank()) base.hiddenPackages = overrides.hiddenPackages; - if (!overrides.sourceDirs.isBlank()) base.sourceDirs = overrides.sourceDirs; - if (!overrides.classFilter.isBlank()) base.classFilter = overrides.classFilter; - if (!overrides.runName.isBlank()) base.runName = overrides.runName; - if (!"runtime-xray-out".equals(overrides.outDir)) base.outDir = overrides.outDir; - if (!overrides.captureValues) base.captureValues = false; + /** + * The command line's options win over the file — all of them. + * + *

This used to be a hand-written list, and it covered nine settings out of + * twenty-two. Everything else given on the command line beside a {@code --config} was + * dropped without a word: {@code --level coverage} measured everything, and + * {@code --jacoco-reports data} wrote its hundred and eighty files. Nothing failed — + * the run simply did something other than what it had been asked, and the only way to + * notice was to count the files afterwards. + * + *

So the rule is applied rather than enumerated: a setting that differs from a + * fresh {@link Config} is one the command line set, and it overrides the file. + * A list has to be remembered at every new option; this does not. + */ + static void merge(Config base, Config overrides) { + Config pristine = new Config(); + for (java.lang.reflect.Field f : Config.class.getFields()) { + if (java.lang.reflect.Modifier.isStatic(f.getModifiers())) continue; + try { + Object given = f.get(overrides); + if (java.util.Objects.equals(given, f.get(pristine))) continue; + f.set(base, given); + } catch (IllegalAccessException e) { + // A public instance field of a public class: unreachable. And a setting + // silently not carried over is exactly what this method exists to stop. + throw new IllegalStateException("setting not carried over: " + f.getName(), e); + } + } } /** @@ -1295,8 +1316,16 @@ private static void usage() { --serve [port] Serve the report (default: 8787) and let the page write its annotations next to the runs, then rebuild it. Several people can annotate at once. - --serve-host Listening interface (default: 127.0.0.1). Use 0.0.0.0 - for a shared server. + --serve-host Listening interface (default: 127.0.0.1), or a single + interface such as 10.0.0.5. SERVE_HOST says the same + in the configuration file — where a machine exposes + itself is a property of that machine. Neither ever + STARTS a server: only --serve does, so a file that + travels cannot open a port. Past the loopback the + report, and the captured values in it, are readable + and annotatable by whoever reaches the port: guard it + with --serve-token and put TLS in front — this speaks + plain HTTP. The tool says so at start-up. --serve-token [s] Guard the served report with a shared secret, asked once then remembered for 12 h. With no value, a secret is drawn at random and printed. XRAY_SERVE_TOKEN does the diff --git a/orchestrator/src/test/java/lab/xray/ConfigTest.java b/orchestrator/src/test/java/lab/xray/ConfigTest.java index 61563b2..d6d275d 100644 --- a/orchestrator/src/test/java/lab/xray/ConfigTest.java +++ b/orchestrator/src/test/java/lab/xray/ConfigTest.java @@ -181,6 +181,27 @@ void jacocoReportsIsReadFromTheFile(@TempDir Path dir) throws IOException { assertEquals("data", Config.load(file).jacocoReports); } + @Test + @DisplayName("SERVE_HOST is read from the file, and its example carries the warning") + void serveHostIsReadAndItsPriceIsStated(@TempDir Path dir) throws IOException { + Path file = dir.resolve("c.conf"); + Files.writeString(file, "JAVA_CMD=\"java -jar a.jar\"\nSERVE_HOST=\"0.0.0.0\"\n", + StandardCharsets.UTF_8); + assertEquals("0.0.0.0", Config.load(file).serveHost); + assertEquals("127.0.0.1", new Config().serveHost, "the loopback stays the default"); + + Path template = dir.resolve("t.conf"); + Config.writeTemplate(template); + String text = Files.readString(template, StandardCharsets.UTF_8); + assertTrue(text.contains("#SERVE_HOST="), "the key must be shown, commented out"); + // Whoever uncomments it must read what it costs on the same screen, not in a + // paragraph somewhere else: past the loopback, the captured values of a real + // application become readable — and the annotations writable — by whoever reaches + // the port. + assertTrue(text.contains("XRAY_SERVE_TOKEN"), "and how to guard it: " + text); + assertTrue(text.contains("TLS"), "and that this speaks plain HTTP: " + text); + } + @Test @DisplayName("The level and its settings travel with the run") void levelIsRecorded() { diff --git a/orchestrator/src/test/java/lab/xray/SettingsPrecedenceTest.java b/orchestrator/src/test/java/lab/xray/SettingsPrecedenceTest.java new file mode 100644 index 0000000..6f50be8 --- /dev/null +++ b/orchestrator/src/test/java/lab/xray/SettingsPrecedenceTest.java @@ -0,0 +1,150 @@ +package lab.xray; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * What wins when the command line and the configuration file disagree. + * + *

The answer has always been "the command line", and for a long time it was true of nine + * settings out of twenty-two. The others — the observation level, what JaCoCo writes, the + * instrumented classes, the exports, the sampling interval — were dropped without a + * word the moment a {@code --config} was on the line beside them. A run asked for at + * {@code --level coverage} measured everything; one asked for at + * {@code --jacoco-reports data} wrote a hundred and eighty files instead of nine. Nothing + * failed: the tool simply did something other than what it had been told, and the only way + * to find out was to count afterwards. + * + *

The rule is now applied rather than listed — a setting that differs from a fresh + * {@link Config} is one the command line set — and this test holds the list that a list + * would have needed. + */ +class SettingsPrecedenceTest { + + /** The file's values: everything set, and all of them different from the defaults. */ + private static Config fromFile() { + Config file = new Config(); + file.javaCommand = "java -jar from-the-file.jar"; + file.level = "coverage"; + file.jacocoReports = Config.DATA; + file.coverIncludes = "file.pkg.*"; + file.exportFormats = "lcov"; + file.sampleIntervalMs = 50; + file.serveHost = "10.0.0.1"; + file.archive = Config.KEEP; + file.outDir = "from-the-file"; + return file; + } + + @Test + @DisplayName("Every setting given on the command line survives a --config beside it") + void theCommandLineWinsOnEverySetting() { + Config file = fromFile(); + Config line = new Config(); + line.level = "tree"; + line.jacocoReports = Config.MINIMAL; + line.coverIncludes = "line.pkg.*"; + line.exportFormats = "perf"; + line.sampleIntervalMs = 7; + line.serveHost = "0.0.0.0"; + line.archive = Config.REPLACE; + line.outDir = "from-the-line"; + + Main.merge(file, line); + + assertEquals("tree", file.level); + assertEquals(Config.MINIMAL, file.jacocoReports); + assertEquals("line.pkg.*", file.coverIncludes); + assertEquals("perf", file.exportFormats); + assertEquals(7, file.sampleIntervalMs); + assertEquals("0.0.0.0", file.serveHost); + assertEquals(Config.REPLACE, file.archive); + assertEquals("from-the-line", file.outDir); + // And what the line said nothing about stays what the file said. + assertEquals("java -jar from-the-file.jar", file.javaCommand); + } + + @Test + @DisplayName("A setting the command line did not touch never overwrites the file") + void anUntouchedSettingLeavesTheFileAlone() { + // This is the other half, and the reason the rule is "differs from a fresh Config" + // rather than "is not empty": a default is not an instruction. + Config file = fromFile(); + Main.merge(file, new Config()); + assertEquals("coverage", file.level); + assertEquals(Config.DATA, file.jacocoReports); + assertEquals(50, file.sampleIntervalMs); + assertEquals("10.0.0.1", file.serveHost); + assertEquals("from-the-file", file.outDir); + } + + @Test + @DisplayName("A setting added later is carried over without anyone remembering to") + void everySettingIsCoveredWithoutAList() throws Exception { + // The defect was a hand-written list that stopped being complete. The guard is not + // a longer list: it is that every field of Config, including the one added next + // week, goes through. A field left out here fails the build rather than a campaign. + List dropped = new ArrayList<>(); + for (Field f : Config.class.getFields()) { + if (Modifier.isStatic(f.getModifiers())) continue; + Config file = new Config(); + Config line = new Config(); + Object other = different(f.get(line)); + if (other == null) continue; + f.set(line, other); + Main.merge(file, line); + if (!other.equals(f.get(file))) dropped.add(f.getName()); + } + assertTrue(dropped.isEmpty(), + "these settings are lost when a --config accompanies them: " + dropped); + } + + /** Any value that is not the one given, whatever the field's type. */ + private static Object different(Object value) { + if (value instanceof String s) return s + "-changed"; + if (value instanceof Integer i) return i + 1; + if (value instanceof Boolean b) return !b; + return null; + } + + @Test + @DisplayName("SERVE_HOST says where to listen, and never that one should") + void theFileCannotOpenAPort() throws Exception { + // A configuration file travels: into a repository, a ticket, another machine. One + // that could put a port into listening by being copied would be unreadable safely. + // So the key sets an interface, and "--serve" alone decides to serve. + Config file = Config.load(java.nio.file.Files.writeString( + java.nio.file.Files.createTempDirectory("xray").resolve("x.conf"), + "SERVE_HOST=\"0.0.0.0\"\n")); + assertEquals("0.0.0.0", file.serveHost); + + String main = java.nio.file.Files.readString(java.nio.file.Path.of("") + .toAbsolutePath().getParent() + .resolve("orchestrator/src/main/java/lab/xray/Main.java").normalize(), + java.nio.charset.StandardCharsets.UTF_8); + int start = main.indexOf("switch (a) {"); + int end = main.indexOf("unknown option", start); + assertTrue(start > 0 && end > start, "the options switch must stay locatable"); + String options = main.substring(start, end); + assertTrue(options.contains("\"--serve\""), "the region really is the options switch"); + assertEquals(1, count(options, "serve = true"), + "exactly one thing puts the tool into listening, and it is --serve"); + assertFalse(main.contains("config.serveHost.isBlank()"), + "the interface never decides whether to serve"); + } + + private static int count(String haystack, String needle) { + int n = 0; + for (int i = haystack.indexOf(needle); i >= 0; i = haystack.indexOf(needle, i + 1)) n++; + return n; + } +}