This is an automated email from the ASF dual-hosted git repository. luigidemasi pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel-kit.git
commit 2863e915679e4b748b68e2bd24b8eace19e63bbf Author: Jinal Sarvaiya <[email protected]> AuthorDate: Mon Nov 9 13:14:05 2026 +0000 fix #99: invalidate Ship worker on any rejected project config, never through symlinks Signed-off-by: Jinal Sarvaiya <[email protected]> --- CHANGELOG.md | 17 +++++-- docs/modules/ROOT/pages/ship.adoc | 7 ++- .../camel/kit/generator/OpenCodeGenerator.java | 58 ++++++++++++++-------- .../camel/kit/generator/OpenCodeGeneratorTest.java | 16 ++++++ .../apache/camel/kit/service/InitServiceTest.java | 48 ++++++++++++++++++ 5 files changed, 119 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ead53c64..c5c4f5d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,10 +32,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 contract while preserving Qwen's named-agent dispatch model. Dispatch never uses `fork` or `fork_turns`. Fresh and regenerated workspaces receive the native skill and worker; runs retain their recorded host. - **OpenCode native Ship (#99)** — eligible sessions relay controller-issued stages through a dedicated - `camel-ship-worker` agent that denies all tools by default and allows only read, glob and grep; inherited MCP - servers are blocked by the deny-by-default policy. `--backend opencode-native` reuses the native task/result, recovery, oversight and deterministic - publication contract while preserving OpenCode's `task` tool dispatch model. Fresh and regenerated workspaces - receive the native skill, worker and command stub; runs retain their recorded host. + `camel-ship-worker` agent via OpenCode's `subagent` tool (`agent` argument). The worker denies `edit`, `write`, + `bash`, `notebook`, `subagent` and `mcp`, and inherits read access, per-path restrictions and external-directory + authorization from the parent session. Because `mcp: deny` does not match individual MCP actions, init derives + `<server>_*: deny` rules for every MCP server in the effective configuration (project layers, ancestor + directories, global, `OPENCODE_CONFIG`, `OPENCODE_CONFIG_DIR`, `OPENCODE_CONFIG_CONTENT` and the platform managed + directory), using OpenCode's ASCII name normalization. When that inventory cannot be established — an + `{env:}`/`{file:}` substitution or unparsable source, a server named like a built-in permission (`external`), or + macOS managed preferences — the worker is not generated, any previous worker is removed (including when + initialization is rejected because the project config is unparsable or has duplicate keys, which OpenCode may still + load), and the Ship skill falls back to CLI mode. Cleanup never follows a symlinked `.opencode/agents` directory. + `--backend opencode-native` reuses the native task/result, recovery, oversight and deterministic publication + contract. Fresh and regenerated workspaces receive the native skill, worker and command stub; runs retain their + recorded host. - **OpenAI Codex CLI native Ship (#225)** — eligible sessions relay controller-issued stages through the dedicated `camel_ship_worker` custom agent with `sandbox_mode = "read-only"` and instruction-level restrictions. `--backend codex-native` reuses the native task/result, recovery, oversight and deterministic publication contract diff --git a/docs/modules/ROOT/pages/ship.adoc b/docs/modules/ROOT/pages/ship.adoc index 55a6ca76..2ddcb974 100644 --- a/docs/modules/ROOT/pages/ship.adoc +++ b/docs/modules/ROOT/pages/ship.adoc @@ -216,10 +216,13 @@ certification. time (project layers, ancestor dirs, global XDG config, `OPENCODE_CONFIG`, `OPENCODE_CONFIG_CONTENT`, `OPENCODE_CONFIG_DIR`, and platform managed config — `/etc/opencode` on Linux, `/Library/Application Support/opencode` on macOS, `%ProgramData%\opencode` on Windows). The worker is not generated — and any existing one is removed — when - any config source contains an unresolved `{env:…}` or `{file:…}` substitution, cannot be parsed, a server name + any config source contains an unresolved `{env:…}` or `{file:…}` substitution, cannot be parsed (a project config + that init rejects, for example for duplicate keys, also removes the worker and is left untouched), a server name collides with a built-in permission prefix, or macOS MDM managed preferences (`ai.opencode.managed.plist`) are present. The worker inherits read access from the parent. If `--backend opencode-native` is requested and dispatch - is unavailable the skill stops without starting a run. Verified on OpenCode 2.2.5 with 26 real host cases. + is unavailable the skill stops without starting a run. Cleanup never follows a symlinked `.opencode/agents` + directory. Native pipeline acceptance on OpenCode 2.2.5 was run on Linux; the Windows and macOS managed-config + handling is covered by generator tests that simulate the OS selection, not by live Windows or macOS runs. |=== A fresh context is not an OS sandbox: read access follows the parent's permitted paths. Do not broaden permissions, diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/OpenCodeGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/OpenCodeGenerator.java index 7c41b2c0..9274a6bc 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/OpenCodeGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/OpenCodeGenerator.java @@ -2,6 +2,7 @@ package org.apache.camel.kit.generator; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.util.HashMap; import java.util.LinkedHashSet; @@ -38,38 +39,25 @@ public class OpenCodeGenerator extends DefaultGenerator { @Override public void preflight(InitContext ctx) throws Exception { + // Refuse to operate through a symlinked agents path before anything can be deleted or written. + requireRealShipWorkerPath(ctx); try { new OpenCodeConfigMerger().validateExisting(ctx.projectDir().resolve(ctx.agent().mcpConfigPath())); } catch (Exception e) { - // OpenCode substitutes {env:}/{file:} before parsing, so a project config that is invalid as raw - // JSON may still be valid for the host. Initialization aborts here, so remove the installed worker: - // its denial list can no longer be shown to match the effective MCP server set. Other validation - // failures (e.g. duplicate keys) keep the "fail before any project writes" contract. - if (projectConfigMayContainSubstitution(ctx)) { - Files.deleteIfExists(ctx.projectDir().resolve(".opencode/agents/camel-ship-worker.md")); - } + // OpenCode may still load a project config that Kit rejects (duplicate keys, or {env:}/{file:} + // substitutions that only become valid JSON after host substitution). Initialization aborts here, so + // the installed worker's denial list can no longer be shown to match the effective MCP server set: + // remove it. The user's configuration is left untouched. + removeShipWorker(ctx); throw e; } } - private static boolean projectConfigMayContainSubstitution(InitContext ctx) { - try { - for (Path configFile : OpenCodeProjectConfig.existingFiles(ctx.projectDir())) { - if (SUBSTITUTION_PATTERN.matcher(Files.readString(configFile)).find()) { - return true; - } - } - return false; - } catch (IOException e) { - return true; // unreadable: fail closed - } - } - @Override public void generate(InitContext ctx) throws Exception { // Invalidate the worker before project-config merging can abort; it is regenerated below only if the // effective MCP inventory is bounded. - Files.deleteIfExists(ctx.projectDir().resolve(".opencode/agents/camel-ship-worker.md")); + removeShipWorker(ctx); // Run default generation (commands, skills, MCP config) super.generate(ctx); @@ -101,6 +89,34 @@ public class OpenCodeGenerator extends DefaultGenerator { generateAgents(ctx, data, !collisionDetected && !unbounded); } + private static final String[] SHIP_WORKER_PATH = {".opencode", "agents", "camel-ship-worker.md"}; + + /** + * Fails when any component of the Ship worker path below the project is a symbolic link, so that cleanup and + * generation never operate outside the selected workspace. + */ + private static void requireRealShipWorkerPath(InitContext ctx) throws IOException { + Path current = ctx.projectDir(); + for (String component : SHIP_WORKER_PATH) { + current = current.resolve(component); + if (Files.isSymbolicLink(current)) { + throw new IOException( + "Initialization aborted: " + current + " is a symbolic link. Camel-Kit generates " + + "and removes files under the agent directories and refuses to operate " + + "through symbolic links; replace the link with a real directory and " + + "re-run init."); + } + if (!Files.exists(current, LinkOption.NOFOLLOW_LINKS)) { + return; + } + } + } + + private static void removeShipWorker(InitContext ctx) throws IOException { + requireRealShipWorkerPath(ctx); + Files.deleteIfExists(ctx.projectDir().resolve(".opencode/agents/camel-ship-worker.md")); + } + static String normalizeServerName(String name) { StringBuilder sb = new StringBuilder(name.length()); for (int i = 0; i < name.length(); i++) { diff --git a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/generator/OpenCodeGeneratorTest.java b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/generator/OpenCodeGeneratorTest.java index 9b57439f..7308dd00 100644 --- a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/generator/OpenCodeGeneratorTest.java +++ b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/generator/OpenCodeGeneratorTest.java @@ -1572,6 +1572,22 @@ class OpenCodeGeneratorTest { "Ship worker must not be generated when a config source cannot be parsed"); } + @Test + @EnabledOnOs(OS.LINUX) + void shipWorkerCleanupDoesNotFollowSymlinkedAgentsDirectory() throws Exception { + Path outside = Files.createDirectory(tempDir.resolve("outside-agents")); + Path sentinel = outside.resolve("camel-ship-worker.md"); + Files.writeString(sentinel, "outside"); + Files.createDirectories(tempDir.resolve(".opencode")); + Files.createSymbolicLink(tempDir.resolve(".opencode/agents"), outside); + + assertThrows(IOException.class, () -> new OpenCodeGenerator().preflight(createContext())); + assertThrows(IOException.class, () -> new OpenCodeGenerator().generate(createContext())); + + assertEquals("outside", Files.readString(sentinel), "cleanup must not follow the agents symlink"); + assertTrue(Files.isSymbolicLink(tempDir.resolve(".opencode/agents"))); + } + @Test void shipWorkerRemovedOnPreflightWhenRootProjectConfigHasStructuralSubstitution() throws Exception { InitContext ctx = createContext(); diff --git a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/InitServiceTest.java b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/InitServiceTest.java index 141e40c4..4dc903cc 100644 --- a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/InitServiceTest.java +++ b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/InitServiceTest.java @@ -182,7 +182,13 @@ class InitServiceTest { Files.writeString(tempDir.resolve(".opencode/opencode.jsonc"), original); Files.writeString(tempDir.resolve(file), original.replaceFirst("\"" + key + "\"\\s*:", "\"" + key + "\": {}, \"" + key + "\":")); + // The only permitted change on a failed preflight is the safety cleanup of the Ship worker, whose denial + // list can no longer be established while the project configuration is rejected. + Path worker = tempDir.resolve(".opencode/agents/camel-ship-worker.md"); + assertTrue(Files.exists(worker), "precondition: worker generated by the first init"); + Files.delete(worker); List<String> before = snapshot(tempDir); + Files.writeString(worker, "stale worker"); InvalidAgentConfigurationException failure = assertThrows(InvalidAgentConfigurationException.class, () -> new InitService() @@ -191,9 +197,51 @@ class InitServiceTest { assertTrue(failure.getMessage().contains(file)); assertTrue(failure.getMessage().contains("Duplicate field '" + key + "'")); assertTrue(failure.getMessage().contains("remove duplicate keys")); + assertFalse(Files.exists(worker), "stale Ship worker must be invalidated when preflight rejects the config"); assertEquals(before, snapshot(tempDir)); } + @ParameterizedTest + @ValueSource(strings = {"opencode.json", ".opencode/opencode.json"}) + void duplicateKeyConfigIntroducingNewServerInvalidatesShipWorkerOnReinit(String file) throws Exception { + new InitService().initialize(request(tempDir, "opencode", InitProgress.noop(), InitReporter.noop())); + Path worker = tempDir.resolve(".opencode/agents/camel-ship-worker.md"); + assertTrue(Files.exists(worker), "precondition: worker generated by the first init"); + + // OpenCode loads the last "mcp" value (introducing "probe"); Kit rejects the duplicate key + Path config = tempDir.resolve(file); + Files.createDirectories(config.getParent()); + Files.writeString(config, "{\"mcp\":{},\"mcp\":{\"probe\":{\"type\":\"local\",\"command\":[\"echo\"]}}}"); + String configBefore = Files.readString(config); + + assertThrows(InvalidAgentConfigurationException.class, + () -> new InitService() + .initialize(request(tempDir, "opencode", InitProgress.noop(), InitReporter.noop()))); + + assertFalse(Files.exists(worker), "worker without probe_*: deny must not survive a rejected reinit"); + assertEquals(configBefore, Files.readString(config), "user configuration must be left untouched"); + } + + @Test + @EnabledOnOs(OS.LINUX) + void failedPreflightDoesNotDeleteShipWorkerThroughSymlinkedAgentsDirectory() throws Exception { + Path outside = Files.createDirectory(tempDir.resolve("outside")); + Path sentinel = outside.resolve("camel-ship-worker.md"); + Files.writeString(sentinel, "outside"); + Files.createDirectories(tempDir.resolve(".opencode")); + Files.createSymbolicLink(tempDir.resolve(".opencode/agents"), outside); + Files.writeString(tempDir.resolve("opencode.json"), "{\"mcp\":{env:MCP_SERVERS}}"); + List<String> outsideBefore = snapshot(outside); + + IOException failure = assertThrows(IOException.class, + () -> new InitService() + .initialize(request(tempDir, "opencode", InitProgress.noop(), InitReporter.noop()))); + + assertTrue(failure.getMessage().contains("symbolic link")); + assertEquals(outsideBefore, snapshot(outside), "cleanup must not follow the agents symlink"); + assertTrue(Files.isSymbolicLink(tempDir.resolve(".opencode/agents"))); + } + @Test @EnabledOnOs(OS.LINUX) void rejectsSymlinkedAgentRootsBeforeWritingOutsideTheProject() throws Exception {
