This is an automated email from the ASF dual-hosted git repository. luigidemasi pushed a commit to branch fix/security-model-verification-153-158 in repository https://gitbox.apache.org/repos/asf/camel-kit.git
commit 5befd5c5dc693d2f0ef2f888a15d447966219757 Author: Luigi De Masi <[email protected]> AuthorDate: Mon Nov 9 19:15:20 2026 +0100 Fix #157: confine generated writes and catalog extraction Fixes #157 Co-authored-by: OpenAI Codex <[email protected]> --- CHANGELOG.md | 2 + docs/modules/ROOT/pages/agents.adoc | 13 ++ docs/security-model.md | 17 +- docs/security-model.yaml | 7 +- docs/security-verification.md | 22 ++ .../camel/kit/catalog/CitrusSchemaDownloader.java | 20 +- .../camel/kit/catalog/CitrusSchemaProcessor.java | 6 +- .../camel/kit/generator/AgentsMdGenerator.java | 3 +- .../camel/kit/generator/AntigravityGenerator.java | 2 +- .../camel/kit/generator/AtomicFileWriter.java | 34 ++- .../camel/kit/generator/ClaudeGenerator.java | 2 +- .../apache/camel/kit/generator/CodexGenerator.java | 2 +- .../camel/kit/generator/CommandStubGenerator.java | 3 +- .../camel/kit/generator/CopilotGenerator.java | 2 +- .../camel/kit/generator/DispatchBlockAppender.java | 2 +- .../camel/kit/generator/McpConfigGenerator.java | 2 +- .../camel/kit/generator/OpenCodeGenerator.java | 2 +- .../kit/generator/PersonaResourceInstaller.java | 2 +- .../apache/camel/kit/generator/PiGenerator.java | 2 +- .../apache/camel/kit/generator/QwenGenerator.java | 2 +- .../kit/generator/SkillResourceInstaller.java | 6 +- .../kit/generator/TemplateResourceCopier.java | 2 +- .../camel/kit/generator/TraitApplicator.java | 2 +- .../kit/generator/VersionPlaceholderResolver.java | 2 +- .../camel/kit/service/ForageCatalogService.java | 11 + .../org/apache/camel/kit/service/InitService.java | 77 ++++--- .../org/apache/camel/kit/util/ConfinedPaths.java | 50 ++++ .../kit/catalog/CitrusSchemaDownloaderTest.java | 252 +++++++++++++++++++++ .../camel/kit/generator/AtomicFileWriterTest.java | 34 +++ .../kit/service/ForageCatalogConfinementTest.java | 96 ++++++++ .../kit/service/InitWriteConfinementTest.java | 114 ++++++++++ .../knowledge/mcp/IndexArchiveConfinementTest.java | 130 +++++++++++ 32 files changed, 843 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8951af1..be030dc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Generated and cached path confinement (#157)** — refuse descendant symlinks before init/catalog writes and validate catalog versions before extraction or replacement. + - **Claude MCP permissions (#155)** — replace server-wide wildcard approvals with explicit manifest-listed tools, with generation and actual-host permission checks. - **Graph input boundaries (#154)** — reject outside-root and symlink inputs, bound source bytes and discovery, and apply the parser timeout to POM analysis. diff --git a/docs/modules/ROOT/pages/agents.adoc b/docs/modules/ROOT/pages/agents.adoc index 895b3dd0..2b9ca2d4 100644 --- a/docs/modules/ROOT/pages/agents.adoc +++ b/docs/modules/ROOT/pages/agents.adoc @@ -194,3 +194,16 @@ v1 mode-rule files. == Claude MCP approvals Fresh and regenerated Claude workspaces pre-approve only the explicit MCP tools listed in the workflow manifest. Other server tools remain visible and require Claude Code permission. Regenerate older workspaces to replace server-wide wildcard grants; review user-level settings separately. The generated Read and Maven command grants retain their existing behavior. + +== Generated path checks + +Initialization checks shared generated outputs and the selected agent directories before +writing. Replace symbolic links inside those managed paths with real files or directories +before running init; this includes links at generated file destinations. Existing regular-file +permissions are preserved when generated outputs are replaced. The explicitly +selected project directory may itself be an alias. Existing unrelated project directories +are not treated as generated outputs. + +Citrus and Forage cache versions must be single safe path components. Their downloaders refuse +linked cache ancestors and output paths, so an existing workspace cannot redirect extraction +or generated quick references outside the chosen destination. diff --git a/docs/security-model.md b/docs/security-model.md index 0d3d7791..008ac56c 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -514,10 +514,13 @@ violation warrants coordinated disclosure and a CVE; **hardening** means it is f writes must remain within the operator-selected target. An untrusted tree or symlink that redirects a write outside it is in scope. An intentional operator-selected destination is authorized" *(maintainer, 2026-10-27 — Q9)*. Retired generated assets are deleted only when inside the project, through no symbolic link, and only regular files *(documented — - `GeneratedAssetCleaner`)*; generated files are written atomically *(documented — `AtomicFileWriter`)*; existing - `.codex/config.toml` / `opencode.json` are merged, not replaced *(documented — `docs/architecture.md`)*. The - "writes only inside the target directory" half, including through a symbolic link inside the target, is - *(inferred — V5)*. *Violation:* a file created, replaced or + `GeneratedAssetCleaner`)*; generated file replacements use atomic writes *(documented — `AtomicFileWriter`)*; existing + `.codex/config.toml` / `opencode.json` are merged, not replaced *(documented — `docs/architecture.md`)*. `InitService` preflights shared generated paths and the selected agent trees before writing; + `ConfinedPaths` refuses pre-existing descendant links (including generated file leaves). + Explicitly selected target aliases remain supported. Citrus and Forage validate version components + and cache ancestry; Citrus also checks extracted paths and derived reference destinations. + Knowledge extracts into fresh staging and refuses unsafe activation paths. + See [V5 evidence](security-verification.md#v5-generated-and-extracted-paths). *Violation:* a file created, replaced or deleted outside the target directory, or a deletion through a symlink. *Severity:* security-critical when reachable from a §4.6 attacker-controllable row (the analysed tree, registry data); otherwise correctness. 4. **P4 — Pinned, named-repository supply chain for everything Camel-Kit fetches or configures.** Catalog, schema and @@ -1045,10 +1048,8 @@ own issue, linked below. indexer — for listeners, child processes, network origins, filesystem writes, environment reads, telemetry and actual parser limits. *Interim triage:* inspected statements stay *(inferred)*; a breach of a claimed property is `VALID`. -- **V5 — Init and download path controls (Q7, Q9; #157) → §4.8 P3, P4.** Verify that `init` writes stay inside the - operator-selected target when a generated subtree is a symbolic link, and that every archive extraction (Citrus - schemas, Forage catalog, Knowledge index) stays inside its cache directory. *Interim triage:* a redirected write or - an extraction escape reachable from an untrusted tree or a downloaded artifact is `VALID`. +- **V5 — Init and download path controls (Q7, Q9; #157): implemented.** Regression tests cover shared init outputs, every agent, generated leaves, cache ancestors, malicious versions and archive paths for Citrus, Forage and Knowledge. [V5 evidence](security-verification.md#v5-generated-and-extracted-paths). Redirected writes remain `VALID`. + - **V6 — Knowledge section verification (Q11; #158) → §4.4a, §4.8 P12–P14.** Maintainer verification of the statements drafted from code and the README: no listener in the default configuration, no process execution and no network access other than index acquisition, tool-argument handling, cache permissions, the SSE posture, and the classpath diff --git a/docs/security-model.yaml b/docs/security-model.yaml index 8f960713..72acec8f 100644 --- a/docs/security-model.yaml +++ b/docs/security-model.yaml @@ -294,9 +294,9 @@ claimed_properties: property: init confines its writes to the operator-selected target directory and removes only what it generated, through no symlink. An untrusted tree or a symbolic link that redirects a write outside the target is in scope; an intentionally operator-selected destination is authorized. violation_symptom: File created, replaced or deleted outside the operator-selected target, or a write or deletion redirected through a symbolic link. severity: security-critical when reachable from an attacker-controllable input (analysed tree, registry data); otherwise correctness - provenance: contract - maintainer (2026-10-27, Q9); deletion guards documented (GeneratedAssetCleaner, AtomicFileWriter); write confinement through symlinked subtrees inferred (V5) + provenance: "Contract: maintainer (2026-10-27, Q9). InitService/ConfinedPaths preflight shared and selected-agent outputs; GeneratedAssetCleaner guards deletion; generated file replacements use AtomicFileWriter. InitWriteConfinementTest covers static hostile descendant links and selected-root aliases." - id: P4 - property: Pinned, named-repository supply chain for everything fetched or configured; Ship resolver uses fixed HTTPS Maven Central, checksum fail, no redirects, exact coordinates, size caps, SHA-256 content identity, isolated URLClassLoader payloads. Kit's own handling of what it downloads is in scope - catalog JSON is read from the JAR stream in memory, Citrus schema entries are written only inside the cache directory after a normalize/startsWith guard, Forage catalog entries are wr [...] + property: Pinned, named-repository supply chain for everything fetched or configured; Ship resolver uses fixed HTTPS Maven Central, checksum fail, no redirects, exact coordinates, size caps, SHA-256 content identity, isolated URLClassLoader payloads. Kit's own handling of what it downloads is in scope - catalog JSON is read from the JAR stream in memory, Citrus schema entries are written only inside the cache directory after lexical containment and descendant-link checks; Citrus/Fora [...] violation_symptom: Unpinned coordinate, repository outside the named lists, download accepted on checksum mismatch (Ship), artifact outside its exact path, payload class loaded from elsewhere, or a downloaded artifact that escapes the cache directory or executes code through Kit's parsing or extraction. severity: security-critical provenance: documented (mcp-setup.md, distribution.properties, ShipMavenResolver, ShipJvmPayloadBootstrap, CatalogDownloader, CitrusSchemaDownloader, ForageCatalogService); in-scope clause maintainer (2026-10-27, Q7) @@ -615,7 +615,8 @@ verification_items: sections: ["4.8 P3", "4.8 P4"] work: Verify that init writes stay inside the operator-selected target when a generated subtree is a symbolic link, and that every archive extraction path (Citrus schemas, Forage catalog, Knowledge index) stays inside its cache directory. interim_triage: A redirected write or an extraction escape reachable from an untrusted tree or a downloaded artifact is VALID. - status: open + status: "implemented" + evidence: "InitWriteConfinementTest, ForageCatalogConfinementTest, CitrusSchemaDownloaderTest, IndexArchiveConfinementTest plus existing IndexDownloader tests; security-verification.md V5" - id: V6 tracking: apache/camel-kit#158 title: Knowledge section verification diff --git a/docs/security-verification.md b/docs/security-verification.md index 636734fd..494fedc0 100644 --- a/docs/security-verification.md +++ b/docs/security-verification.md @@ -57,3 +57,25 @@ All 12 cases passed. The fixture drives the actual CLI permission protocol witho model account; it does not certify future Claude releases or unrelated user-level grants. The reproducible harness is `tools/verify_claude_mcp_permissions.py`; use `--help` for paths. Website impact: required; agent reference updated. Existing workspaces need regeneration. + + +## V5: generated and extracted paths + +`InitWriteConfinementTest` covers shared directory ancestors, generated file leaves, +all eight agents' nested skill outputs and the positive selected-root-alias case. +`ForageCatalogConfinementTest` covers linked cache ancestors, relative/absolute version escapes +and the exact two-entry archive allowlist. `CitrusSchemaDownloaderTest` covers traversal, +cache/schema links and both generated quick-reference leaves, with an in-memory HTTP archive. +`IndexArchiveConfinementTest` covers traversal, absolute entries and Unix ZIP symlink metadata +through actual hash-verified acquisition. Existing downloader tests cover unsafe activation, +staging, version names and fallback. + +Baseline probes demonstrated outside writes for init/Citrus/Forage, including deletion of an +outside directory selected by a malicious Forage version. All outside paths were test fixtures. +The fix rejects unsafe versions and pre-existing descendant links before writes/deletions. +Generated file leaves are replaced through AtomicFileWriter rather than truncated in place; +six hard-link regressions prove outside sentinels remain unchanged for init and Citrus outputs. +An explicitly chosen root alias is allowed. Extraction creates ordinary files and never +materializes ZIP symlink metadata. These checks do not provide descriptor-relative isolation +against concurrent same-account directory substitution; existing same-account exclusions apply. +Website impact: required; initialization behavior documented in the agent reference. diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaDownloader.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaDownloader.java index d5ec3cc1..8aa60082 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaDownloader.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaDownloader.java @@ -13,6 +13,9 @@ import java.util.function.Consumer; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; +import org.apache.camel.kit.generator.AtomicFileWriter; +import org.apache.camel.kit.util.ConfinedPaths; + import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -29,11 +32,15 @@ public class CitrusSchemaDownloader { private final ObjectMapper mapper; public CitrusSchemaDownloader(Path cacheDir) { - this.cacheDir = cacheDir; - this.httpClient = HttpClient.newBuilder() + this(cacheDir, HttpClient.newBuilder() .connectTimeout(Duration.ofSeconds(10)) .followRedirects(HttpClient.Redirect.NORMAL) - .build(); + .build()); + } + + CitrusSchemaDownloader(Path cacheDir, HttpClient httpClient) { + this.cacheDir = cacheDir; + this.httpClient = httpClient; this.mapper = new ObjectMapper(); } @@ -49,7 +56,8 @@ public class CitrusSchemaDownloader { } public JsonNode fetchCitrusSchemas(String version, boolean forceRefresh, Consumer<String> log) throws Exception { - Path citrusDir = cacheDir.resolve("citrus").resolve(version); + Path citrusDir = getCitrusSchemasDir(version); + ConfinedPaths.requireTree(cacheDir, citrusDir); Path indexFile = citrusDir.resolve("index.json"); if (!forceRefresh && Files.exists(indexFile)) { @@ -97,11 +105,12 @@ public class CitrusSchemaDownloader { + name); } + ConfinedPaths.requirePath(cacheDir, targetFile); // Create parent directories if needed Files.createDirectories(targetFile.getParent()); byte[] data = zis.readAllBytes(); - Files.write(targetFile, data); + AtomicFileWriter.write(targetFile, data); fileCount++; } } @@ -125,6 +134,7 @@ public class CitrusSchemaDownloader { * Get the path to the cached Citrus schemas directory. */ public Path getCitrusSchemasDir(String version) { + ConfinedPaths.requireVersion(version); return cacheDir.resolve("citrus").resolve(version); } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaProcessor.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaProcessor.java index e9414b12..2b7f2fd8 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaProcessor.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/catalog/CitrusSchemaProcessor.java @@ -5,6 +5,8 @@ import java.nio.file.Path; import java.util.Iterator; import java.util.Map; +import org.apache.camel.kit.generator.AtomicFileWriter; + import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; @@ -56,7 +58,7 @@ public class CitrusSchemaProcessor { // Write the quick reference Path outputFile = citrusCacheDir.resolve("citrus-quick-reference.json"); - Files.writeString(outputFile, mapper.writerWithDefaultPrettyPrinter().writeValueAsString(reference)); + AtomicFileWriter.write(outputFile, mapper.writerWithDefaultPrettyPrinter().writeValueAsString(reference)); // Also generate a markdown version for easier AI consumption generateMarkdownReference(citrusCacheDir, reference); @@ -275,6 +277,6 @@ public class CitrusSchemaProcessor { md.append("\n"); Path mdFile = citrusCacheDir.resolve("citrus-quick-reference.md"); - Files.writeString(mdFile, md.toString()); + AtomicFileWriter.write(mdFile, md.toString()); } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AgentsMdGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AgentsMdGenerator.java index 7fa1cfac..fbad641c 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AgentsMdGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AgentsMdGenerator.java @@ -1,6 +1,5 @@ package org.apache.camel.kit.generator; -import java.nio.file.Files; import java.util.Map; import org.apache.camel.kit.util.AnsiColors; @@ -16,7 +15,7 @@ class AgentsMdGenerator { "SKILLS_DIR", skillsDirFromProjectRoot(ctx), "BOB2", "bob2".equals(ctx.agentName()))); String content = qute.render("templates/shared/agents-md.md", data); - Files.writeString(ctx.projectDir().resolve("AGENTS.md"), content); + AtomicFileWriter.write(ctx.projectDir().resolve("AGENTS.md"), content); ctx.printer().println(AnsiColors.green("✓") + " Generated AGENTS.md with skill routing and iron laws"); } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AntigravityGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AntigravityGenerator.java index ade35dee..d884179e 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AntigravityGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AntigravityGenerator.java @@ -84,7 +84,7 @@ public class AntigravityGenerator extends DefaultGenerator { if (!PersonaResourceInstaller.isPersonaTemplate(template)) { Path target = ctx.projectDir().resolve(template.target()); Files.createDirectories(target.getParent()); - Files.writeString(target, templates.render(template.source(), + AtomicFileWriter.write(target, templates.render(template.source(), Map.of("COMMAND_PREFIX", ctx.commandPrefix()))); } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AtomicFileWriter.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AtomicFileWriter.java index 4a54438b..6a04f3ab 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AtomicFileWriter.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/AtomicFileWriter.java @@ -1,27 +1,45 @@ package org.apache.camel.kit.generator; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.PosixFileAttributeView; -final class AtomicFileWriter { +public final class AtomicFileWriter { private AtomicFileWriter() { } - static void write(Path target, String content) throws IOException { + public static void write(Path target, String content) throws IOException { + write(target, content.getBytes(StandardCharsets.UTF_8)); + } + + public static void write(Path target, byte[] content) throws IOException { Path temporary = Files.createTempFile(target.getParent(), target.getFileName().toString(), ".tmp"); try { - Files.writeString(temporary, content); - try { - Files.move(temporary, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); - } catch (AtomicMoveNotSupportedException e) { - Files.move(temporary, target, StandardCopyOption.REPLACE_EXISTING); - } + Files.write(temporary, content); + replace(temporary, target); } finally { Files.deleteIfExists(temporary); } } + + public static void replace(Path temporary, Path target) throws IOException { + if (Files.isRegularFile(target, LinkOption.NOFOLLOW_LINKS)) { + PosixFileAttributeView permissions = Files.getFileAttributeView( + target, PosixFileAttributeView.class, LinkOption.NOFOLLOW_LINKS); + if (permissions != null) { + Files.setPosixFilePermissions(temporary, permissions.readAttributes().permissions()); + } + } + try { + Files.move(temporary, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(temporary, target, StandardCopyOption.REPLACE_EXISTING); + } + } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ClaudeGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ClaudeGenerator.java index 965cd7c9..9f394a96 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ClaudeGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ClaudeGenerator.java @@ -81,7 +81,7 @@ public class ClaudeGenerator extends DefaultGenerator { Map.of( "COMMAND_PREFIX", ctx.commandPrefix())); String content = templateEngine.render("templates/claude/claude-md.md", data); - Files.writeString(ctx.projectDir().resolve("CLAUDE.md"), content); + AtomicFileWriter.write(ctx.projectDir().resolve("CLAUDE.md"), content); } private void generateSettings(InitContext ctx) throws Exception { diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CodexGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CodexGenerator.java index 22b2b749..b48f8178 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CodexGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CodexGenerator.java @@ -58,7 +58,7 @@ public class CodexGenerator extends DefaultGenerator { } Path target = ctx.projectDir().resolve(template.target()); if (AGENTS_TEMPLATE.equals(template.source())) { - Files.writeString(target, templateEngine.render( + AtomicFileWriter.write(target, templateEngine.render( template.source(), Map.of("COMMAND_PREFIX", ctx.commandPrefix()))); } else { copyTemplateResource(template.source(), target); diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CommandStubGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CommandStubGenerator.java index 344e1583..3bf0ff36 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CommandStubGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CommandStubGenerator.java @@ -1,6 +1,5 @@ package org.apache.camel.kit.generator; -import java.nio.file.Files; import java.util.List; import java.util.Locale; @@ -18,7 +17,7 @@ class CommandStubGenerator { for (WorkflowCommand command : commands) { String content = commandContent(ctx, command); String filename = command.name() + "." + ctx.agent().fileFormat(); - Files.writeString(ctx.commandsDir().resolve(filename), content); + AtomicFileWriter.write(ctx.commandsDir().resolve(filename), content); } ctx.printer().println(AnsiColors.green("✓") + " Created " + commands.size() + " command stubs"); diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CopilotGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CopilotGenerator.java index 5c80ceac..787bf583 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CopilotGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/CopilotGenerator.java @@ -46,7 +46,7 @@ public class CopilotGenerator extends DefaultGenerator { Files.createDirectories(parent); } if (RENDERED_TEMPLATES.contains(template.source())) { - Files.writeString(target, templateEngine.render(template.source(), data)); + AtomicFileWriter.write(target, templateEngine.render(template.source(), data)); } else { copyTemplateResource(template.source(), target); } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/DispatchBlockAppender.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/DispatchBlockAppender.java index b731a7d4..c0380667 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/DispatchBlockAppender.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/DispatchBlockAppender.java @@ -18,6 +18,6 @@ class DispatchBlockAppender { String dispatchTemplatePath = descriptor.dispatchTemplatePath(); String dispatchBlock = TemplateUtils.readTemplate(dispatchTemplatePath); String existing = Files.readString(skillMdFile); - Files.writeString(skillMdFile, existing + "\n---\n\n" + dispatchBlock); + AtomicFileWriter.write(skillMdFile, existing + "\n---\n\n" + dispatchBlock); } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/McpConfigGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/McpConfigGenerator.java index 975c4cde..f47861aa 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/McpConfigGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/McpConfigGenerator.java @@ -66,7 +66,7 @@ class McpConfigGenerator { } } } else { - Files.writeString(configFile, processed); + AtomicFileWriter.write(configFile, processed); } ctx.printer().println(AnsiColors.green("✓") + " MCP config created for " + ctx.agent().name()); 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 9274a6bc..ce86c1ba 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 @@ -366,7 +366,7 @@ public class OpenCodeGenerator extends DefaultGenerator { continue; } String content = templateEngine.render("templates/opencode/agents/" + agentName + ".md", data); - Files.writeString(agentsDir.resolve(agentName + ".md"), content); + AtomicFileWriter.write(agentsDir.resolve(agentName + ".md"), content); count++; } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PersonaResourceInstaller.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PersonaResourceInstaller.java index ab5a2831..d511d60d 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PersonaResourceInstaller.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PersonaResourceInstaller.java @@ -84,7 +84,7 @@ final class PersonaResourceInstaller { String content = Files.readString(markdown); String rewritten = rewriteReferences(content, targetDirectory); if (!rewritten.equals(content)) { - Files.writeString(markdown, rewritten); + AtomicFileWriter.write(markdown, rewritten); } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PiGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PiGenerator.java index 7432c79f..fca1c8fb 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PiGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/PiGenerator.java @@ -46,7 +46,7 @@ public class PiGenerator extends DefaultGenerator { Files.createDirectories(parent); } if (RENDERED_TEMPLATES.contains(template.source())) { - Files.writeString(target, templateEngine.render(template.source(), data)); + AtomicFileWriter.write(target, templateEngine.render(template.source(), data)); } else { copyTemplateResource(template.source(), target); } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/QwenGenerator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/QwenGenerator.java index db24b935..b437bf30 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/QwenGenerator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/QwenGenerator.java @@ -41,7 +41,7 @@ public class QwenGenerator extends DefaultGenerator { private void generateQwenMd(InitContext ctx, Map<String, Object> data) throws Exception { String content = templateEngine.render("templates/qwen/qwen-md.md", data); - Files.writeString(ctx.projectDir().resolve("QWEN.md"), content); + AtomicFileWriter.write(ctx.projectDir().resolve("QWEN.md"), content); } private void generateSubAgents(InitContext ctx) throws Exception { diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java index 65223fee..66bda972 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java @@ -209,7 +209,7 @@ class SkillResourceInstaller { .replaceAll(Matcher.quoteReplacement("$" + skill.name())); } if (!adapted.equals(content)) { - Files.writeString(markdown, adapted); + AtomicFileWriter.write(markdown, adapted); } } @@ -241,7 +241,7 @@ class SkillResourceInstaller { } } if (inserted) { - Files.writeString(skillFile, updated.toString()); + AtomicFileWriter.write(skillFile, updated.toString()); } } @@ -303,7 +303,7 @@ class SkillResourceInstaller { } } if (inserted) { - Files.writeString(skillFile, updated.toString()); + AtomicFileWriter.write(skillFile, updated.toString()); } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TemplateResourceCopier.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TemplateResourceCopier.java index ada6ab18..5107bffd 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TemplateResourceCopier.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TemplateResourceCopier.java @@ -18,7 +18,7 @@ class TemplateResourceCopier { if (parent != null) { Files.createDirectories(parent); } - Files.writeString(target, new String(is.readAllBytes(), StandardCharsets.UTF_8)); + AtomicFileWriter.write(target, new String(is.readAllBytes(), StandardCharsets.UTF_8)); } } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TraitApplicator.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TraitApplicator.java index 44e17a9e..a1f1d849 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TraitApplicator.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/TraitApplicator.java @@ -76,7 +76,7 @@ class TraitApplicator { } String closeSentinel = "<!-- /TRAIT:" + ctx.agentName() + " -->"; - Files.writeString(targetFile, + AtomicFileWriter.write(targetFile, existing + "\n---\n\n" + sentinel + "\n" + traitContent + "\n" + closeSentinel + "\n"); return true; } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/VersionPlaceholderResolver.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/VersionPlaceholderResolver.java index 7b032956..8c300991 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/VersionPlaceholderResolver.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/VersionPlaceholderResolver.java @@ -45,7 +45,7 @@ class VersionPlaceholderResolver { return; } matcher.appendTail(rendered); - Files.writeString(mdFile, rendered.toString()); + AtomicFileWriter.write(mdFile, rendered.toString()); } static Map<String, String> buildVersionTemplateData(DistributionConfig dist) { diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/ForageCatalogService.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/ForageCatalogService.java index 900d1a38..f752d0ad 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/ForageCatalogService.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/ForageCatalogService.java @@ -15,6 +15,8 @@ import java.util.Set; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; +import org.apache.camel.kit.util.ConfinedPaths; + /** * Downloads the published Forage catalog artifact and caches its two JSON files under * {@code .camel-kit/.cache/forage/<version>/}. Fail-soft by design: callers treat any IOException as "no Forage support @@ -29,6 +31,11 @@ public class ForageCatalogService { public boolean isCached(Path camelKitDir, String forageVersion) { Path dir = cacheDir(camelKitDir, forageVersion); + try { + ConfinedPaths.requireTree(camelKitDir, dir); + } catch (IOException e) { + return false; + } return CATALOG_ENTRIES.stream() .allMatch(entry -> Files.exists(dir.resolve(Path.of(entry).getFileName().toString()))); } @@ -36,6 +43,7 @@ public class ForageCatalogService { /** Resolves the artifact from Maven Central and caches the catalog files. Skips when already cached. */ public Path cache(Path camelKitDir, String catalogArtifact, String forageVersion) throws IOException, InterruptedException { + ConfinedPaths.requireTree(camelKitDir, cacheDir(camelKitDir, forageVersion)); if (isCached(camelKitDir, forageVersion)) { return cacheDir(camelKitDir, forageVersion); } @@ -53,6 +61,7 @@ public class ForageCatalogService { HttpRequest request = HttpRequest.newBuilder(URI.create(url)).timeout(Duration.ofSeconds(60)).build(); HttpResponse<InputStream> response = client.send(request, HttpResponse.BodyHandlers.ofInputStream()); if (response.statusCode() != 200) { + response.body().close(); throw new IOException("Forage catalog download failed: HTTP " + response.statusCode() + " for " + url); } try (InputStream body = response.body()) { @@ -63,6 +72,7 @@ public class ForageCatalogService { /** Extracts the catalog JSON entries from the jar stream into the versioned cache dir (atomic: temp dir + move). */ public Path cacheFromJar(InputStream jarStream, Path camelKitDir, String forageVersion) throws IOException { Path finalDir = cacheDir(camelKitDir, forageVersion); + ConfinedPaths.requireTree(camelKitDir, finalDir); Files.createDirectories(finalDir.getParent()); Path tempDir = Files.createTempDirectory(finalDir.getParent(), "forage-dl-"); try { @@ -88,6 +98,7 @@ public class ForageCatalogService { } private Path cacheDir(Path camelKitDir, String forageVersion) { + ConfinedPaths.requireVersion(forageVersion); return camelKitDir.resolve(".cache").resolve("forage").resolve(forageVersion); } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java index 2aed3052..1d67cebc 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java @@ -1,16 +1,18 @@ package org.apache.camel.kit.service; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.file.Files; -import java.nio.file.LinkOption; import java.nio.file.Path; import java.time.LocalDate; import java.time.ZoneId; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Properties; +import java.util.Set; import org.apache.camel.kit.catalog.CitrusSchemaDownloader; import org.apache.camel.kit.config.AgentConfig; @@ -18,6 +20,7 @@ import org.apache.camel.kit.config.AgentRegistry; import org.apache.camel.kit.config.DistributionConfig; import org.apache.camel.kit.generator.AgentGenerator; import org.apache.camel.kit.generator.AgentGeneratorFactory; +import org.apache.camel.kit.generator.AtomicFileWriter; import org.apache.camel.kit.generator.InitContext; import org.apache.camel.kit.generator.QuteTemplateEngine; import org.apache.camel.kit.graph.GraphBuildResult; @@ -27,6 +30,7 @@ import org.apache.camel.kit.graph.ParserDiagnostic; import org.apache.camel.kit.graph.ProjectGraph; import org.apache.camel.kit.graph.RuntimeDetector; import org.apache.camel.kit.graph.model.NodeType; +import org.apache.camel.kit.util.ConfinedPaths; import org.apache.camel.kit.util.TemplateUtils; /** @@ -56,7 +60,7 @@ public class InitService { skillsDir, targetDir, request.commandPrefix(), request.distribution(), request.withCitrusMcp(), request.printer()); AgentGenerator generator = AgentGeneratorFactory.create(request.agentName()); - requireRealAgentDirectories(targetDir, skillsDir, commandsDir); + requireConfinedOutputs(targetDir, request.agentName(), agent); generator.preflight(genCtx); List<Path> createdPaths = new ArrayList<>(); @@ -101,27 +105,26 @@ public class InitService { citrusVersion, citrusSchemaCount, MAVEN_WRAPPER_VERSION, graph, createdPaths, warnings); } - /** - * Fails initialization up front — before any project files are written — when a managed agent path component is a - * symbolic link. Generation and retired-asset cleanup refuse to operate through symbolic links, and failing late - * would leave a half-initialized workspace. - */ - private void requireRealAgentDirectories(Path targetDir, Path... managedDirs) throws IOException { - for (Path dir : managedDirs) { - Path current = targetDir; - for (Path component : targetDir.relativize(dir)) { - 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.isDirectory(current, LinkOption.NOFOLLOW_LINKS)) { - break; - } + private void requireConfinedOutputs(Path targetDir, String agentName, AgentConfig agent) throws IOException { + Set<Path> outputs = new LinkedHashSet<>(); + for (String relative : List.of(".camel-kit", ".mvn/wrapper", "mvnw", "mvnw.cmd", "AGENTS.md", + "docs/constitution.md", "docs/flows", "test/data", "schemas")) { + outputs.add(Path.of(relative)); + } + outputs.add(Path.of(agent.skillsDirectory()).getName(0)); + if (agent.generatesCommandStubs()) { + outputs.add(Path.of(agent.commandDirectory()).getName(0)); + } + outputs.add(Path.of(agent.mcpConfigPath()).getName(0)); + for (var template : AgentRegistry.descriptor(agentName).templates()) { + outputs.add(Path.of(template.target()).getName(0)); + } + try { + for (Path output : outputs) { + ConfinedPaths.requireTree(targetDir, targetDir.resolve(output)); } + } catch (IOException e) { + throw new IOException("Initialization aborted: " + e.getMessage(), e); } } @@ -168,9 +171,9 @@ public class InitService { } Path configFile = dir.resolve("config.properties"); - try (var out = Files.newOutputStream(configFile)) { - config.store(out, "Camel-Kit Project Configuration"); - } + var out = new ByteArrayOutputStream(); + config.store(out, "Camel-Kit Project Configuration"); + AtomicFileWriter.write(configFile, out.toByteArray()); createdPaths.add(configFile); } @@ -207,7 +210,7 @@ public class InitService { "DATE", LocalDate.now(ZoneId.systemDefault()).toString(), "CAMEL_VERSION", request.distribution().camelMainVersion())); Path constitution = dir.resolve("constitution.md"); - Files.writeString(constitution, content); + AtomicFileWriter.write(constitution, content); createdPaths.add(constitution); } @@ -229,7 +232,7 @@ public class InitService { for (String template : additionalTemplates) { String content = TemplateUtils.readTemplate("templates/" + template); Path target = templatesDir.resolve(template); - Files.writeString(target, content); + AtomicFileWriter.write(target, content); createdPaths.add(target); } } @@ -246,18 +249,18 @@ public class InitService { """, MAVEN_WRAPPER_VERSION, MAVEN_WRAPPER_VERSION); Path wrapperProperties = wrapperDir.resolve("maven-wrapper.properties"); - Files.writeString(wrapperProperties, wrapperProps); + AtomicFileWriter.write(wrapperProperties, wrapperProps); createdPaths.add(wrapperProperties); String mvnwScript = TemplateUtils.readTemplate("maven/mvnw"); Path mvnwPath = projectDir.resolve("mvnw"); - Files.writeString(mvnwPath, mvnwScript); + AtomicFileWriter.write(mvnwPath, mvnwScript); mvnwPath.toFile().setExecutable(true); createdPaths.add(mvnwPath); String mvnwCmdScript = TemplateUtils.readTemplate("maven/mvnw.cmd"); Path mvnwCmd = projectDir.resolve("mvnw.cmd"); - Files.writeString(mvnwCmd, mvnwCmdScript); + AtomicFileWriter.write(mvnwCmd, mvnwCmdScript); createdPaths.add(mvnwCmd); } @@ -279,7 +282,13 @@ public class InitService { } ProjectGraph projectGraph = result.graph(); if (projectGraph.nodeCount() > 0) { - GraphSerializer.write(projectGraph, graphFile, targetDir.toAbsolutePath().toString()); + Path temporary = Files.createTempFile(camelKitDir, "project-graph-", ".json"); + try { + GraphSerializer.write(projectGraph, temporary, targetDir.toAbsolutePath().toString()); + AtomicFileWriter.replace(temporary, graphFile); + } finally { + Files.deleteIfExists(temporary); + } createdPaths.add(graphFile); String detectedProjectType = detectProjectType(projectGraph); String detectedRuntime = updateConfigWithRuntime(camelKitDir, projectGraph, request.distribution()); @@ -340,9 +349,9 @@ public class InitService { String runtime = RuntimeDetector.detect(graph); writeVersionConfig(config, distribution, runtime); - try (var out = Files.newOutputStream(configFile)) { - config.store(out, "Camel-Kit Project Configuration"); - } + var out = new ByteArrayOutputStream(); + config.store(out, "Camel-Kit Project Configuration"); + AtomicFileWriter.write(configFile, out.toByteArray()); return runtime; } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/util/ConfinedPaths.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/util/ConfinedPaths.java new file mode 100644 index 00000000..9b073eca --- /dev/null +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/util/ConfinedPaths.java @@ -0,0 +1,50 @@ +package org.apache.camel.kit.util; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; + +/** Preflight for generated paths below an explicitly selected destination. */ +public final class ConfinedPaths { + private ConfinedPaths() { + } + + public static void requirePath(Path destination, Path output) throws IOException { + Path root = destination.toAbsolutePath().normalize(); + Path target = output.toAbsolutePath().normalize(); + if (!target.startsWith(root)) { + throw new IOException("Generated path is outside the destination: " + output); + } + Path current = root; + for (Path component : root.relativize(target)) { + current = current.resolve(component); + if (Files.isSymbolicLink(current)) { + throw new IOException("Refusing generated path through symbolic link: " + current); + } + if (Files.exists(current, LinkOption.NOFOLLOW_LINKS) + && !Files.isDirectory(current, LinkOption.NOFOLLOW_LINKS) + && !Files.isRegularFile(current, LinkOption.NOFOLLOW_LINKS)) { + throw new IOException("Generated path is not a regular file or directory: " + current); + } + } + } + + public static void requireTree(Path destination, Path output) throws IOException { + requirePath(destination, output); + if (Files.isDirectory(output, LinkOption.NOFOLLOW_LINKS)) { + try (var paths = Files.walk(output)) { + var iterator = paths.iterator(); + while (iterator.hasNext()) { + requirePath(destination, iterator.next()); + } + } + } + } + + public static void requireVersion(String version) { + if (version == null || !version.matches("[A-Za-z0-9][A-Za-z0-9._-]*")) { + throw new IllegalArgumentException("Invalid catalog version: " + version); + } + } +} diff --git a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/catalog/CitrusSchemaDownloaderTest.java b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/catalog/CitrusSchemaDownloaderTest.java new file mode 100644 index 00000000..7bf23383 --- /dev/null +++ b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/catalog/CitrusSchemaDownloaderTest.java @@ -0,0 +1,252 @@ +package org.apache.camel.kit.catalog; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.net.Authenticator; +import java.net.CookieHandler; +import java.net.ProxySelector; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpHeaders; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSession; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.*; + +class CitrusSchemaDownloaderTest { + + private static final String VERSION = "5.0.1"; + private static final String PREFIX = "org/citrusframework/schema/citrus/" + VERSION + "/"; + + @TempDir + Path tempDir; + + @ParameterizedTest + @ValueSource(strings = {"../outside", "/absolute", "..", "", "1/2", "1\\2"}) + void versionCannotSelectAnOutsideDestination(String version) throws Exception { + Path cache = Files.createDirectory(tempDir.resolve("cache")); + CitrusSchemaDownloader downloader + = new CitrusSchemaDownloader(cache, new ArchiveClient(archive("schema.json"))); + assertThrows(IllegalArgumentException.class, () -> downloader.fetchCitrusSchemas(version, true)); + try (var paths = Files.list(cache)) { + assertEquals(0, paths.count()); + } + } + + @Test + void validArchiveGeneratesSchemasAndReferenceInsideCache() throws Exception { + Path cache = tempDir.resolve("cache"); + fetch(cache, archive("index.json")); + assertEquals("{}", Files.readString(cache.resolve("citrus/" + VERSION + "/index.json"))); + assertTrue(Files.isRegularFile(cache.resolve("citrus/" + VERSION + "/citrus-quick-reference.json"))); + } + + @ParameterizedTest + @ValueSource(strings = {"../../../outside", "/absolute"}) + void archivePathsCannotEscapeCache(String entry) throws Exception { + Path cache = tempDir.resolve("cache"); + Path outside = Files.writeString(tempDir.resolve("outside"), "untouched"); + String name = entry.startsWith("/") ? outside.toString() : entry; + assertThrows(Exception.class, () -> fetch(cache, archive(name))); + assertEquals("untouched", Files.readString(outside)); + } + + @ParameterizedTest + @ValueSource(strings = {"citrus", "citrus/5.0.1", "citrus/5.0.1/nested"}) + void existingCacheDirectorySymlinksCannotRedirectExtraction(String relative) throws Exception { + Path cache = Files.createDirectory(tempDir.resolve("cache")); + Path outside = Files.createDirectory(tempDir.resolve("outside")); + Path sentinel = Files.writeString(outside.resolve("keep"), "untouched"); + Path link = cache.resolve(relative); + Files.createDirectories(link.getParent()); + Files.createSymbolicLink(link, outside); + + assertThrows(Exception.class, () -> fetch(cache, archive("nested/schema.json"))); + + assertEquals("untouched", Files.readString(sentinel)); + try (var paths = Files.list(outside)) { + assertEquals(List.of(sentinel), paths.toList()); + } + assertTrue(Files.isSymbolicLink(link)); + } + + @ParameterizedTest + @ValueSource(strings = {"schema.json", "citrus-quick-reference.json", "citrus-quick-reference.md"}) + void schemaAndGeneratedReferenceFilesCannotOverwriteSymlinkTargets(String name) throws Exception { + Path cache = Files.createDirectory(tempDir.resolve("cache")); + Path version = Files.createDirectories(cache.resolve("citrus/" + VERSION)); + Path sentinel = Files.writeString(tempDir.resolve("outside"), "untouched"); + Path link = Files.createSymbolicLink(version.resolve(name), sentinel); + + assertThrows(Exception.class, () -> fetch(cache, archive("schema.json"))); + + assertEquals("untouched", Files.readString(sentinel)); + assertTrue(Files.isSymbolicLink(link)); + } + + @ParameterizedTest + @ValueSource(strings = {"schema.json", "citrus-quick-reference.json", "citrus-quick-reference.md"}) + void schemasAndReferencesReplaceHardLinksWithoutChangingOutsideContent(String name) throws Exception { + Path cache = Files.createDirectory(tempDir.resolve("cache")); + Path version = Files.createDirectories(cache.resolve("citrus/" + VERSION)); + Path sentinel = Files.writeString(tempDir.resolve("outside"), "untouched"); + Path generated = Files.createLink(version.resolve(name), sentinel); + + fetch(cache, archive("schema.json")); + + assertEquals("untouched", Files.readString(sentinel)); + assertTrue(Files.isRegularFile(generated)); + assertNotEquals("untouched", Files.readString(generated)); + assertFalse(Files.isSameFile(generated, sentinel), "Extraction and processing must replace the selected entry"); + } + + private void fetch(Path cache, byte[] jar) throws Exception { + // Requires a package-private (Path, HttpClient) constructor; the public API remains unchanged. + new CitrusSchemaDownloader(cache, new ArchiveClient(jar)).fetchCitrusSchemas(VERSION, true, message -> { + }); + } + + private byte[] archive(String name) throws IOException { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ZipOutputStream zip = new ZipOutputStream(bytes)) { + zip.putNextEntry(new ZipEntry(PREFIX + name)); + zip.write("{}".getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + return bytes.toByteArray(); + } + + private static class ArchiveClient extends HttpClient { + private final byte[] archive; + + ArchiveClient(byte[] archive) { + this.archive = archive; + } + + @Override + public <T> HttpResponse<T> send(HttpRequest request, HttpResponse.BodyHandler<T> handler) { + return new HttpResponse<>() { + @Override + public int statusCode() { + return 200; + } + + @Override + public HttpRequest request() { + return request; + } + + @Override + public Optional<HttpResponse<T>> previousResponse() { + return Optional.empty(); + } + + @Override + public HttpHeaders headers() { + return HttpHeaders.of(Map.of(), (k, v) -> true); + } + + @SuppressWarnings("unchecked") + @Override + public T body() { + return (T) new ByteArrayInputStream(archive); + } + + @Override + public Optional<SSLSession> sslSession() { + return Optional.empty(); + } + + @Override + public URI uri() { + return request.uri(); + } + + @Override + public Version version() { + return Version.HTTP_1_1; + } + }; + } + + @Override + public Optional<CookieHandler> cookieHandler() { + return Optional.empty(); + } + + @Override + public Optional<Duration> connectTimeout() { + return Optional.empty(); + } + + @Override + public Redirect followRedirects() { + return Redirect.NEVER; + } + + @Override + public Optional<ProxySelector> proxy() { + return Optional.empty(); + } + + @Override + public SSLContext sslContext() { + throw new UnsupportedOperationException(); + } + + @Override + public SSLParameters sslParameters() { + return new SSLParameters(); + } + + @Override + public Optional<Authenticator> authenticator() { + return Optional.empty(); + } + + @Override + public Version version() { + return Version.HTTP_1_1; + } + + @Override + public Optional<Executor> executor() { + return Optional.empty(); + } + + @Override + public <T> CompletableFuture<HttpResponse<T>> sendAsync( + HttpRequest request, + HttpResponse.BodyHandler<T> handler) { + throw new UnsupportedOperationException(); + } + + @Override + public <T> CompletableFuture<HttpResponse<T>> sendAsync( + HttpRequest request, + HttpResponse.BodyHandler<T> handler, HttpResponse.PushPromiseHandler<T> push) { + throw new UnsupportedOperationException(); + } + } +} diff --git a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/generator/AtomicFileWriterTest.java b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/generator/AtomicFileWriterTest.java new file mode 100644 index 00000000..d69a9164 --- /dev/null +++ b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/generator/AtomicFileWriterTest.java @@ -0,0 +1,34 @@ +package org.apache.camel.kit.generator; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermissions; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +class AtomicFileWriterTest { + @TempDir + Path directory; + + @Test + void replacementPreservesModeWithoutModifyingTheLinkedInode() throws Exception { + assumeTrue(Files.getFileAttributeView(directory, PosixFileAttributeView.class) != null); + Path original = Files.writeString(directory.resolve("outside"), "original"); + var mode = PosixFilePermissions.fromString("rwxr-xr-x"); + Files.setPosixFilePermissions(original, mode); + Path output = Files.createLink(directory.resolve("generated"), original); + + AtomicFileWriter.write(output, "generated content"); + + assertEquals("original", Files.readString(original)); + assertEquals("generated content", Files.readString(output)); + assertEquals(mode, Files.getPosixFilePermissions(output)); + assertEquals(mode, Files.getPosixFilePermissions(original)); + assertFalse(Files.isSameFile(original, output)); + } +} diff --git a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/ForageCatalogConfinementTest.java b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/ForageCatalogConfinementTest.java new file mode 100644 index 00000000..47e746dd --- /dev/null +++ b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/ForageCatalogConfinementTest.java @@ -0,0 +1,96 @@ +package org.apache.camel.kit.service; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.*; + +class ForageCatalogConfinementTest { + + @TempDir + Path tempDir; + + @ParameterizedTest + @ValueSource(strings = {".cache", ".cache/forage"}) + void cacheParentsCannotRedirectExtraction(String relative) throws Exception { + Path project = Files.createDirectory(tempDir.resolve("project")); + Path outside = Files.createDirectory(tempDir.resolve("outside")); + Path sentinel = Files.writeString(outside.resolve("keep"), "untouched"); + Path link = project.resolve(relative); + Files.createDirectories(link.getParent()); + Files.createSymbolicLink(link, outside); + + assertThrows(IOException.class, + () -> new ForageCatalogService().cacheFromJar(new ByteArrayInputStream(archive()), project, "1.5.0")); + + assertEquals("untouched", Files.readString(sentinel)); + try (var paths = Files.list(outside)) { + assertEquals(List.of(sentinel), paths.toList()); + } + assertTrue(Files.isSymbolicLink(link)); + } + + @ParameterizedTest + @ValueSource(strings = {"../../../outside", "/absolute"}) + void versionCannotSelectAnOutsideDestination(String version) throws Exception { + Path project = Files.createDirectory(tempDir.resolve("project")); + Path outside = Files.createDirectory(tempDir.resolve("outside")); + Path sentinel = Files.writeString(outside.resolve("keep"), "untouched"); + Files.createDirectories(project.resolve(".cache/forage")); + String input = version.startsWith("/") ? outside.toString() : version; + + assertThrows(IllegalArgumentException.class, + () -> new ForageCatalogService().cacheFromJar(new ByteArrayInputStream(archive()), project, input)); + + assertEquals("untouched", Files.readString(sentinel)); + try (var paths = Files.list(outside)) { + assertEquals(List.of(sentinel), paths.toList()); + } + } + + @Test + void archiveEntryNamesCannotEscapeTheExactCatalogAllowlist() throws Exception { + Path project = Files.createDirectory(tempDir.resolve("project")); + Path sentinel = Files.writeString(tempDir.resolve("outside"), "untouched"); + byte[] bytes = archive("catalog/../../../../../outside", sentinel.toString(), + "catalog/..\\..\\outside", "catalog/forage-catalog.json/../../outside"); + + Path cache = new ForageCatalogService().cacheFromJar(new ByteArrayInputStream(bytes), project, "1.5.0"); + + assertEquals("untouched", Files.readString(sentinel)); + assertEquals("{}", Files.readString(cache.resolve("forage-catalog.json"))); + assertEquals("{}", Files.readString(cache.resolve("forage-configuration-catalog.json"))); + try (var paths = Files.list(cache)) { + assertEquals(2, paths.count()); + } + } + + private byte[] archive(String... hostileNames) throws IOException { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ZipOutputStream zip = new ZipOutputStream(bytes)) { + for (String name : hostileNames) { + zip.putNextEntry(new ZipEntry(name)); + zip.write("hostile".getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + for (String name : List.of("catalog/forage-catalog.json", "catalog/forage-configuration-catalog.json")) { + zip.putNextEntry(new ZipEntry(name)); + zip.write("{}".getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + } + return bytes.toByteArray(); + } +} diff --git a/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/InitWriteConfinementTest.java b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/InitWriteConfinementTest.java new file mode 100644 index 00000000..c2e2033b --- /dev/null +++ b/kit/camel-kit-core/src/test/java/org/apache/camel/kit/service/InitWriteConfinementTest.java @@ -0,0 +1,114 @@ +package org.apache.camel.kit.service; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import org.apache.camel.kit.CamelKitMain; +import org.apache.camel.kit.config.AgentRegistry; +import org.apache.camel.kit.output.Printer; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.*; + +class InitWriteConfinementTest { + + @TempDir + Path tempDir; + + @ParameterizedTest + @ValueSource(strings = {".camel-kit", ".camel-kit/templates", "docs", ".mvn", ".mvn/wrapper"}) + void generatedDirectoriesCannotRedirectWrites(String relative) throws Exception { + Path project = Files.createDirectory(tempDir.resolve("project")); + Path outside = Files.createDirectory(tempDir.resolve("outside")); + Path sentinel = Files.writeString(outside.resolve("keep"), "untouched"); + Path link = project.resolve(relative); + Files.createDirectories(link.getParent()); + Files.createSymbolicLink(link, outside); + + assertThrows(IOException.class, () -> new InitService().initialize(request(project, "bob2"))); + + assertEquals("untouched", Files.readString(sentinel)); + try (var paths = Files.list(outside)) { + assertEquals(List.of(sentinel), paths.toList(), "No generated files may be written through the link"); + } + assertTrue(Files.isSymbolicLink(link)); + } + + @ParameterizedTest + @ValueSource(strings = { + ".camel-kit/config.properties", ".camel-kit/templates/flow.md", ".camel-kit/project-graph.json", + "docs/constitution.md", + ".mvn/wrapper/maven-wrapper.properties", "mvnw", "mvnw.cmd", "AGENTS.md"}) + void generatedFilesCannotOverwriteOutsideSymlinkTargets(String relative) throws Exception { + Path project = Files.createDirectory(tempDir.resolve("project")); + Path sentinel = Files.writeString(tempDir.resolve("outside"), "untouched"); + Path link = project.resolve(relative); + Files.createDirectories(link.getParent()); + Files.createSymbolicLink(link, sentinel); + + assertThrows(IOException.class, () -> new InitService().initialize(request(project, "bob2"))); + + assertEquals("untouched", Files.readString(sentinel)); + assertTrue(Files.isSymbolicLink(link)); + } + + @ParameterizedTest + @ValueSource(strings = {".camel-kit/config.properties", "mvnw", "AGENTS.md"}) + void generatedFilesReplaceHardLinksWithoutChangingOutsideContent(String relative) throws Exception { + Path project = Files.createDirectory(tempDir.resolve("project")); + Path sentinel = Files.writeString(tempDir.resolve("outside"), "untouched"); + Path generated = project.resolve(relative); + Files.createDirectories(generated.getParent()); + Files.createLink(generated, sentinel); + + new InitService().initialize(request(project, "bob2")); + + assertEquals("untouched", Files.readString(sentinel)); + assertTrue(Files.isRegularFile(generated)); + assertNotEquals("untouched", Files.readString(generated)); + assertFalse(Files.isSameFile(generated, sentinel), "Generation must replace the selected directory entry"); + } + + @ParameterizedTest + @ValueSource(strings = {"antigravity", "bob2", "claude", "codex", "copilot", "opencode", "pi", "qwen"}) + void nestedSkillDirectoryCannotRedirectAnyAgentWrites(String agent) throws Exception { + Path project = Files.createDirectory(tempDir.resolve("project")); + Path outside = Files.createDirectory(tempDir.resolve("outside")); + Path sentinel = Files.writeString(outside.resolve("context-authority.md"), "untouched"); + Path link = project.resolve(AgentRegistry.get(agent).skillsDirectory()).resolve("shared"); + Files.createDirectories(link.getParent()); + Files.createSymbolicLink(link, outside); + + assertThrows(IOException.class, () -> new InitService().initialize(request(project, agent))); + + assertEquals("untouched", Files.readString(sentinel), agent); + try (var paths = Files.list(outside)) { + assertEquals(List.of(sentinel), paths.toList(), agent); + } + assertTrue(Files.isSymbolicLink(link)); + } + + @Test + void explicitlySelectedTargetMayBeASymlink() throws Exception { + Path actual = Files.createDirectory(tempDir.resolve("actual")); + Path selected = Files.createSymbolicLink(tempDir.resolve("selected"), actual); + + new InitService().initialize(request(selected, "bob2")); + + assertTrue(Files.isRegularFile(actual.resolve(".camel-kit/config.properties"))); + assertTrue(Files.isRegularFile(actual.resolve("AGENTS.md"))); + assertTrue(Files.isSymbolicLink(selected)); + } + + private InitRequest request(Path project, String agent) { + return new InitRequest( + "orders", agent, project, "default", true, false, "auto", "camel-kit", "5.0.1", + CamelKitMain.distribution(), Printer.noop(), InitProgress.noop(), InitReporter.noop()); + } +} diff --git a/knowledge/mcp/src/test/java/org/apache/camel/kit/knowledge/mcp/IndexArchiveConfinementTest.java b/knowledge/mcp/src/test/java/org/apache/camel/kit/knowledge/mcp/IndexArchiveConfinementTest.java new file mode 100644 index 00000000..323cf838 --- /dev/null +++ b/knowledge/mcp/src/test/java/org/apache/camel/kit/knowledge/mcp/IndexArchiveConfinementTest.java @@ -0,0 +1,130 @@ +package org.apache.camel.kit.knowledge.mcp; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.MessageDigest; +import java.util.HexFormat; +import java.util.Optional; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.*; + +class IndexArchiveConfinementTest { + + @TempDir + Path tempDir; + + @ParameterizedTest + @ValueSource(strings = { + "knowledge-index/../../outside", "knowledge-index//absolute", + "/absolute", "knowledge-index/..\\..\\outside", "knowledge-index/C:\\outside"}) + void archiveNamesCannotOverwriteOutsideFiles(String entry) throws Exception { + Path outside = Files.writeString(tempDir.resolve("outside"), "untouched"); + String name = entry.equals("/absolute") ? outside.toString() + : entry.equals("knowledge-index//absolute") ? "knowledge-index/" + outside : entry; + Path manifest = publish(name, "hostile", false); + IndexDownloader downloader = downloader(manifest); + + // Rejection or omission of unsafe entries are both acceptable confinement outcomes. + try { + Path index = downloader.resolve().dir(); + assertEquals("index", Files.readString(index.resolve("segments_1"))); + } catch (IOException expected) { + assertFalse(Files.exists(tempDir.resolve("cache/current"))); + } + assertEquals("untouched", Files.readString(outside)); + } + + @Test + void unixSymlinkEntryCannotCreateALinkUsedByTheFollowingEntry() throws Exception { + Path outside = Files.createDirectory(tempDir.resolve("outside")); + Path sentinel = Files.writeString(outside.resolve("keep"), "untouched"); + Path manifest = publish("knowledge-index/link", outside.toString(), true); + + // java.util.zip has no symlink extraction: a link entry must never become a filesystem symlink. + try { + Path index = downloader(manifest).resolve().dir(); + assertFalse(Files.isSymbolicLink(index.resolve("link"))); + } catch (IOException expected) { + assertFalse(Files.exists(tempDir.resolve("cache/current"))); + } + assertEquals("untouched", Files.readString(sentinel)); + assertFalse(Files.exists(outside.resolve("escape"))); + } + + private Path publish(String name, String content, boolean symlink) throws Exception { + Path publish = Files.createDirectory(tempDir.resolve("publish")); + Path archive = publish.resolve("index.zip"); + try (ZipOutputStream zip = new ZipOutputStream(Files.newOutputStream(archive))) { + write(zip, "knowledge-index/segments_1", "index"); + write(zip, name, content); + if (symlink) { + write(zip, "knowledge-index/link/escape", "hostile"); + } + } + if (symlink) { + byte[] bytes = Files.readAllBytes(archive); + byte[] filename = name.getBytes(StandardCharsets.UTF_8); + for (int i = 0; i + 46 + filename.length <= bytes.length; i++) { + if (bytes[i] == 0x50 && bytes[i + 1] == 0x4b && bytes[i + 2] == 1 && bytes[i + 3] == 2 + && java.util.Arrays.equals(filename, + java.util.Arrays.copyOfRange(bytes, i + 46, i + 46 + filename.length))) { + bytes[i + 5] = 3; // Unix creator: external attributes contain the POSIX mode. + int attributes = 0120777 << 16; + for (int j = 0; j < 4; j++) { + bytes[i + 38 + j] = (byte) (attributes >>> (8 * j)); + } + } + } + Files.write(archive, bytes); + } + String sha = HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256").digest(Files.readAllBytes(archive))); + return Files.writeString(publish.resolve("index.json"), new ObjectMapper().createObjectNode() + .put("version", "v1").put("sha256", sha).put("asset", "index.zip") + .put("embeddingModel", "granite-embedding-small-english-r2-q8").toString()); + } + + private void write(ZipOutputStream zip, String name, String content) throws IOException { + zip.putNextEntry(new ZipEntry(name)); + zip.write(content.getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + + private IndexDownloader downloader(Path manifest) { + return new IndexDownloader(new IndexResolverConfig() { + @Override + public Optional<String> path() { + return Optional.empty(); + } + + @Override + public Optional<String> url() { + return Optional.of(manifest.toUri().toString()); + } + + @Override + public String releasesUrl() { + return "http://unused.invalid/releases";; + } + + @Override + public Optional<String> version() { + return Optional.empty(); + } + + @Override + public String cacheDir() { + return tempDir.resolve("cache").toString(); + } + }); + } +}
