diff --git a/src/cli/commands/model_method_run.ts b/src/cli/commands/model_method_run.ts index eacafa60..173c215a 100644 --- a/src/cli/commands/model_method_run.ts +++ b/src/cli/commands/model_method_run.ts @@ -167,7 +167,13 @@ export const modelMethodRunCommand = new Command() `${definition.id}-${timestamp}.log`, ); const runLogCategory: string[] = []; - await runFileSink.register(runLogCategory, logFilePath, redactor); + const logBoundary = swampPath(repoDir); + await runFileSink.register( + runLogCategory, + logFilePath, + redactor, + logBoundary, + ); runLogger.info("Found model {name} ({type})", { name: definition.name, diff --git a/src/domain/models/user_model_loader.ts b/src/domain/models/user_model_loader.ts index 3ac23289..78888148 100644 --- a/src/domain/models/user_model_loader.ts +++ b/src/domain/models/user_model_loader.ts @@ -39,6 +39,7 @@ import { SWAMP_DATA_DIR, SWAMP_SUBDIRS, } from "../../infrastructure/persistence/paths.ts"; +import { assertSafePath } from "../../infrastructure/persistence/safe_path.ts"; const logger = getLogger(["swamp", "models", "loader"]); @@ -367,6 +368,8 @@ export class UserModelLoader { // Bundle and write to cache const js = await bundleExtension(absolutePath, denoPath); + const bundleBoundary = join(this.repoDir, SWAMP_DATA_DIR); + await assertSafePath(bundlePath, bundleBoundary); await Deno.mkdir(dirname(bundlePath), { recursive: true }); await Deno.writeTextFile(bundlePath, js); logger.debug`Wrote bundle cache: ${bundlePath}`; diff --git a/src/domain/vaults/local_encryption_vault_provider.ts b/src/domain/vaults/local_encryption_vault_provider.ts index 36e6a0a3..b22dfd52 100644 --- a/src/domain/vaults/local_encryption_vault_provider.ts +++ b/src/domain/vaults/local_encryption_vault_provider.ts @@ -24,6 +24,7 @@ import { SWAMP_SUBDIRS, swampPath, } from "../../infrastructure/persistence/paths.ts"; +import { assertSafePath } from "../../infrastructure/persistence/safe_path.ts"; /** * Configuration options for local encryption vault. @@ -62,6 +63,7 @@ export class LocalEncryptionVaultProvider implements VaultProvider { private readonly name: string; private readonly config: LocalEncryptionConfig; private readonly vaultDir: string; + private readonly secretsBoundary: string; /** Cache for key material (not the derived key, since each secret has unique salt) */ private keyMaterialCache?: CryptoKey; @@ -71,6 +73,7 @@ export class LocalEncryptionVaultProvider implements VaultProvider { // Compute secrets directory from base_dir + vault name // Path: {base_dir}/.swamp/secrets/local_encryption/{vault_name} const baseDir = config.base_dir ?? Deno.cwd(); + this.secretsBoundary = swampPath(baseDir); this.vaultDir = swampPath( baseDir, SWAMP_SUBDIRS.secrets, @@ -114,6 +117,7 @@ export class LocalEncryptionVaultProvider implements VaultProvider { const encryptedData = await this.encrypt(secretValue, masterKey, salt); const encryptedFilePath = join(this.vaultDir, `${secretKey}.enc`); + await assertSafePath(encryptedFilePath, this.secretsBoundary); await atomicWriteTextFile( encryptedFilePath, JSON.stringify(encryptedData, null, 2), @@ -480,6 +484,7 @@ export class LocalEncryptionVaultProvider implements VaultProvider { * Ensures the vault directory exists. */ private async ensureVaultDirectory(): Promise { + await assertSafePath(this.vaultDir, this.secretsBoundary); try { await Deno.mkdir(this.vaultDir, { recursive: true, mode: 0o700 }); } catch (error) { diff --git a/src/domain/workflows/execution_service.ts b/src/domain/workflows/execution_service.ts index 97817113..2eacc062 100644 --- a/src/domain/workflows/execution_service.ts +++ b/src/domain/workflows/execution_service.ts @@ -803,10 +803,12 @@ export class WorkflowExecutionService { `workflow-run-${run.id}.log`, ); const workflowLogCategory: string[] = []; + const workflowLogBoundary = swampPath(this.repoDir); await runFileSink.register( workflowLogCategory, workflowLogPath, secretRedactor, + workflowLogBoundary, ); run.setLogFile(workflowLogPath); diff --git a/src/infrastructure/logging/run_file_sink.ts b/src/infrastructure/logging/run_file_sink.ts index 2fe55869..01f35b2b 100644 --- a/src/infrastructure/logging/run_file_sink.ts +++ b/src/infrastructure/logging/run_file_sink.ts @@ -19,6 +19,7 @@ import { getTextFormatter, type LogRecord, type Sink } from "@logtape/logtape"; import type { SecretRedactor } from "../../domain/secrets/mod.ts"; +import { assertSafePath } from "../persistence/safe_path.ts"; /** * Formats a LogRecord as a plain text line for file output. @@ -69,6 +70,7 @@ export class RunFileSink { categoryPrefix: string[], filePath: string, redactor?: SecretRedactor, + boundary?: string, ): Promise { const key = prefixKey(categoryPrefix); // Close existing writer if any @@ -77,6 +79,11 @@ export class RunFileSink { existing.fd.close(); } + // Validate the file path stays within the expected boundary + if (boundary) { + await assertSafePath(filePath, boundary); + } + // Ensure parent directory exists const dir = filePath.substring(0, filePath.lastIndexOf("/")); if (dir) { diff --git a/src/infrastructure/persistence/json_telemetry_repository.ts b/src/infrastructure/persistence/json_telemetry_repository.ts index 77836ccf..e35e358a 100644 --- a/src/infrastructure/persistence/json_telemetry_repository.ts +++ b/src/infrastructure/persistence/json_telemetry_repository.ts @@ -22,6 +22,7 @@ import { join } from "@std/path"; import { atomicWriteTextFile } from "./atomic_write.ts"; import type { TelemetryRepository } from "../../domain/telemetry/repositories.ts"; import { SWAMP_SUBDIRS, swampPath } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import { TelemetryEntry, type TelemetryEntryData, @@ -46,6 +47,7 @@ export class JsonTelemetryRepository implements TelemetryRepository { async save(entry: TelemetryEntry): Promise { try { const telemetryDir = this.getTelemetryDir(); + await assertSafePath(telemetryDir, swampPath(this.repoDir)); await ensureDir(telemetryDir); const date = entry.startedAt.toISOString().split("T")[0]; diff --git a/src/infrastructure/persistence/safe_path.ts b/src/infrastructure/persistence/safe_path.ts new file mode 100644 index 00000000..165fdf38 --- /dev/null +++ b/src/infrastructure/persistence/safe_path.ts @@ -0,0 +1,116 @@ +// Swamp, an Automation Framework +// Copyright (C) 2026 System Initiative, Inc. +// +// This file is part of Swamp. +// +// Swamp is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License version 3 +// as published by the Free Software Foundation, with the Swamp +// Extension and Definition Exception (found in the "COPYING-EXCEPTION" +// file). +// +// Swamp is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with Swamp. If not, see . + +import { dirname, join, resolve } from "@std/path"; + +/** + * Error thrown when a path resolves outside its expected boundary, + * typically due to a symlink pointing outside the repository. + */ +export class PathTraversalError extends Error { + readonly path: string; + readonly boundary: string; + readonly resolvedTarget: string; + + constructor(path: string, boundary: string, resolvedTarget: string) { + super( + `Path traversal detected: "${path}" resolves to "${resolvedTarget}" which is outside boundary "${boundary}"`, + ); + this.name = "PathTraversalError"; + this.path = path; + this.boundary = boundary; + this.resolvedTarget = resolvedTarget; + } +} + +/** + * Resolves a path to its real filesystem location, handling the case where + * the path (or parts of it) may not exist yet. + * + * If the path exists, uses `Deno.realPath()` directly. Otherwise, walks up + * to the nearest existing ancestor, resolves that, and appends the remaining + * non-existent segments. + */ +async function resolveRealPath(path: string): Promise { + const normalized = resolve(path); + + try { + return await Deno.realPath(normalized); + } catch (e) { + if (!(e instanceof Deno.errors.NotFound)) { + throw e; + } + } + + // Path doesn't exist — walk up to find the nearest existing ancestor + const segments: string[] = []; + let current = normalized; + + while (true) { + const parent = dirname(current); + if (parent === current) { + // Reached filesystem root without finding an existing path; + // fall back to the lexically resolved path + return normalized; + } + + segments.unshift(current.slice(parent.length + 1)); + current = parent; + + try { + const realParent = await Deno.realPath(current); + return join(realParent, ...segments); + } catch (e) { + if (!(e instanceof Deno.errors.NotFound)) { + throw e; + } + // Keep walking up + } + } +} + +/** + * Asserts that `path` resolves (following symlinks) to a location within + * `boundary`. + * + * This protects against symlink-based path traversal attacks where a + * directory like `.swamp/outputs` is replaced with a symlink pointing + * outside the repository. + * + * The check handles paths that don't yet exist by resolving the nearest + * existing ancestor and appending the remaining segments. + * + * @param path - The path to validate + * @param boundary - The directory the path must stay within + * @throws {PathTraversalError} if the resolved path escapes the boundary + */ +export async function assertSafePath( + path: string, + boundary: string, +): Promise { + const resolvedBoundary = await resolveRealPath(boundary); + const resolvedPath = await resolveRealPath(path); + + if ( + resolvedPath !== resolvedBoundary && + !resolvedPath.startsWith(resolvedBoundary + "/") + ) { + throw new PathTraversalError(path, boundary, resolvedPath); + } +} diff --git a/src/infrastructure/persistence/safe_path_test.ts b/src/infrastructure/persistence/safe_path_test.ts new file mode 100644 index 00000000..2b067d07 --- /dev/null +++ b/src/infrastructure/persistence/safe_path_test.ts @@ -0,0 +1,186 @@ +// Swamp, an Automation Framework +// Copyright (C) 2026 System Initiative, Inc. +// +// This file is part of Swamp. +// +// Swamp is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License version 3 +// as published by the Free Software Foundation, with the Swamp +// Extension and Definition Exception (found in the "COPYING-EXCEPTION" +// file). +// +// Swamp is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with Swamp. If not, see . + +import { assertEquals, assertRejects } from "@std/assert"; +import { join } from "@std/path"; +import { assertSafePath, PathTraversalError } from "./safe_path.ts"; + +Deno.test("assertSafePath", async (t) => { + const tmpDir = await Deno.makeTempDir({ prefix: "swamp-safe-path-" }); + + try { + // Set up test directory structure + const boundary = join(tmpDir, "repo", ".swamp", "outputs"); + await Deno.mkdir(boundary, { recursive: true }); + + const outsideDir = join(tmpDir, "outside"); + await Deno.mkdir(outsideDir, { recursive: true }); + + await t.step("path within boundary passes", async () => { + const subdir = join(boundary, "subdir"); + await Deno.mkdir(subdir, { recursive: true }); + + // Should not throw + await assertSafePath(subdir, boundary); + }); + + await t.step("path equal to boundary passes", async () => { + await assertSafePath(boundary, boundary); + }); + + await t.step( + "symlink escaping boundary throws PathTraversalError", + async () => { + const symlinkPath = join(boundary, "evil-link"); + await Deno.symlink(outsideDir, symlinkPath); + + try { + await assertRejects( + () => assertSafePath(join(symlinkPath, "file.txt"), boundary), + PathTraversalError, + "outside boundary", + ); + } finally { + await Deno.remove(symlinkPath); + } + }, + ); + + await t.step( + "non-existent path with no symlinks passes", + async () => { + const nonExistent = join(boundary, "does", "not", "exist", "yet"); + + // Should not throw — all ancestors are real directories within boundary + await assertSafePath(nonExistent, boundary); + }, + ); + + await t.step( + "non-existent path where ancestor is symlink escaping boundary throws", + async () => { + const symlinkDir = join(boundary, "sneaky-dir"); + await Deno.symlink(outsideDir, symlinkDir); + + try { + await assertRejects( + () => + assertSafePath( + join(symlinkDir, "subdir", "file.txt"), + boundary, + ), + PathTraversalError, + "outside boundary", + ); + } finally { + await Deno.remove(symlinkDir); + } + }, + ); + + await t.step( + "internal symlink staying within boundary passes", + async () => { + const realDir = join(boundary, "real-data"); + await Deno.mkdir(realDir, { recursive: true }); + const internalLink = join(boundary, "link-to-real"); + await Deno.symlink(realDir, internalLink); + + try { + // Should not throw — symlink stays within the boundary + await assertSafePath(join(internalLink, "file.txt"), boundary); + } finally { + await Deno.remove(internalLink); + } + }, + ); + + await t.step( + "boundary directory replaced with symlink throws", + async () => { + // This simulates the original attack: .swamp/outputs is a symlink + const attackBoundary = join(tmpDir, "repo2", ".swamp", "outputs"); + await Deno.mkdir(join(tmpDir, "repo2", ".swamp"), { recursive: true }); + await Deno.symlink(outsideDir, attackBoundary); + + try { + // The boundary itself is a symlink to outside — the path resolves + // outside, but the boundary also resolves outside, so the check + // passes. The real protection is at the parent level. + const parentBoundary = join(tmpDir, "repo2", ".swamp"); + await assertRejects( + () => + assertSafePath(join(attackBoundary, "file.txt"), parentBoundary), + PathTraversalError, + "outside boundary", + ); + } finally { + await Deno.remove(attackBoundary); + } + }, + ); + + await t.step( + "using symlinked directory as boundary does NOT catch the attack", + async () => { + // Demonstrates why the boundary must be the PARENT of the + // potentially-symlinked directory, not the directory itself. + const attackDir = join(tmpDir, "repo3", ".swamp", "outputs"); + await Deno.mkdir(join(tmpDir, "repo3", ".swamp"), { recursive: true }); + await Deno.symlink(outsideDir, attackDir); + + try { + // Using the symlinked dir itself as boundary — both sides resolve + // through the symlink, so the check passes (this is the bug). + await assertSafePath(join(attackDir, "file.txt"), attackDir); + // ^ Does NOT throw — demonstrating the incorrect boundary choice + + // Using the parent .swamp/ as boundary correctly catches it + const correctBoundary = join(tmpDir, "repo3", ".swamp"); + await assertRejects( + () => assertSafePath(join(attackDir, "file.txt"), correctBoundary), + PathTraversalError, + ); + } finally { + await Deno.remove(attackDir); + await Deno.remove(join(tmpDir, "repo3"), { recursive: true }); + } + }, + ); + + await t.step("PathTraversalError contains path details", async () => { + const symlinkPath = join(boundary, "error-test-link"); + await Deno.symlink(outsideDir, symlinkPath); + + try { + const filePath = join(symlinkPath, "secret.txt"); + const error = await assertRejects( + () => assertSafePath(filePath, boundary), + PathTraversalError, + ); + assertEquals(error.path, filePath); + assertEquals(error.boundary, boundary); + } finally { + await Deno.remove(symlinkPath); + } + }); + } finally { + await Deno.remove(tmpDir, { recursive: true }); + } +}); diff --git a/src/infrastructure/persistence/unified_data_repository.ts b/src/infrastructure/persistence/unified_data_repository.ts index 8fdf3813..2603ea4e 100644 --- a/src/infrastructure/persistence/unified_data_repository.ts +++ b/src/infrastructure/persistence/unified_data_repository.ts @@ -21,6 +21,7 @@ import { join, relative, resolve } from "@std/path"; import { parse as parseYaml, stringify as stringifyYaml } from "@std/yaml"; import { atomicWriteFile, atomicWriteTextFile } from "./atomic_write.ts"; import { SWAMP_SUBDIRS, swampPath } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import { Data, type DataId, @@ -640,6 +641,8 @@ export class FileSystemUnifiedDataRepository implements UnifiedDataRepository { data.name, newVersion, ); + const boundary = swampPath(this.repoDir); + await assertSafePath(metadataPath, boundary); const metadata = dataToSave.toData(); // Remove undefined values const cleanData = JSON.parse(JSON.stringify(metadata)); @@ -653,6 +656,7 @@ export class FileSystemUnifiedDataRepository implements UnifiedDataRepository { data.name, newVersion, ); + await assertSafePath(contentPath, boundary); await atomicWriteFile(contentPath, content); // Update latest symlink @@ -683,6 +687,7 @@ export class FileSystemUnifiedDataRepository implements UnifiedDataRepository { dataName, latestVersion, ); + await assertSafePath(contentPath, swampPath(this.repoDir)); const file = await Deno.open(contentPath, { append: true }); try { await file.write(content); @@ -1169,6 +1174,7 @@ export class FileSystemUnifiedDataRepository implements UnifiedDataRepository { dataName: string, ): Promise<{ version: number; versionDir: string }> { const dataNameDir = this.getDataNameDir(type, modelId, dataName); + await assertSafePath(dataNameDir, swampPath(this.repoDir)); await Deno.mkdir(dataNameDir, { recursive: true }); const versions = await this.listVersions(type, modelId, dataName); @@ -1253,6 +1259,7 @@ export class FileSystemUnifiedDataRepository implements UnifiedDataRepository { version: number, ): Promise { const dataNameDir = this.getDataNameDir(type, modelId, dataName); + await assertSafePath(dataNameDir, swampPath(this.repoDir)); const latestPath = join(dataNameDir, "latest"); const target = version.toString(); diff --git a/src/infrastructure/persistence/yaml_definition_repository.ts b/src/infrastructure/persistence/yaml_definition_repository.ts index 258001e9..f31a4c4d 100644 --- a/src/infrastructure/persistence/yaml_definition_repository.ts +++ b/src/infrastructure/persistence/yaml_definition_repository.ts @@ -24,6 +24,7 @@ import { atomicWriteTextFile } from "./atomic_write.ts"; import { cleanupEmptyParentDirs } from "./directory_cleanup.ts"; import { parse as parseYaml, stringify as stringifyYaml } from "@std/yaml"; import { SWAMP_SUBDIRS, swampPath } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import type { DefinitionRepository } from "../../domain/definitions/repositories.ts"; import { ModelType } from "../../domain/models/model_type.ts"; import { @@ -218,6 +219,7 @@ export class YamlDefinitionRepository implements DefinitionRepository { async save(type: ModelType, definition: Definition): Promise { const dir = this.getTypeDir(type); + await assertSafePath(dir, swampPath(this.repoDir)); await ensureDir(dir); const path = this.getPath(type, definition.id); diff --git a/src/infrastructure/persistence/yaml_evaluated_definition_repository.ts b/src/infrastructure/persistence/yaml_evaluated_definition_repository.ts index 77229655..3d94b80c 100644 --- a/src/infrastructure/persistence/yaml_evaluated_definition_repository.ts +++ b/src/infrastructure/persistence/yaml_evaluated_definition_repository.ts @@ -24,6 +24,7 @@ import { cleanupEmptyParentDirs } from "./directory_cleanup.ts"; import { parse as parseYaml, stringify as stringifyYaml } from "@std/yaml"; import { ModelType } from "../../domain/models/model_type.ts"; import { SWAMP_SUBDIRS, swampPath } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import { createDefinitionId, Definition, @@ -234,6 +235,7 @@ export class YamlEvaluatedDefinitionRepository { */ async save(type: ModelType, definition: Definition): Promise { const dir = this.getTypeDir(type); + await assertSafePath(dir, swampPath(this.repoDir)); await ensureDir(dir); const path = this.getPath(type, definition.id); diff --git a/src/infrastructure/persistence/yaml_evaluated_workflow_repository.ts b/src/infrastructure/persistence/yaml_evaluated_workflow_repository.ts index b4291568..a5dc67ab 100644 --- a/src/infrastructure/persistence/yaml_evaluated_workflow_repository.ts +++ b/src/infrastructure/persistence/yaml_evaluated_workflow_repository.ts @@ -23,6 +23,7 @@ import { parse as parseYaml, stringify as stringifyYaml } from "@std/yaml"; import { atomicWriteTextFile } from "./atomic_write.ts"; import type { WorkflowId } from "../../domain/workflows/workflow_id.ts"; import { SWAMP_SUBDIRS, swampPath } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import { Workflow, type WorkflowData, @@ -90,6 +91,7 @@ export class YamlEvaluatedWorkflowRepository { async save(workflow: Workflow): Promise { const dir = this.getWorkflowsDir(); + await assertSafePath(dir, swampPath(this.repoDir)); await ensureDir(dir); const path = this.getPath(workflow.id); diff --git a/src/infrastructure/persistence/yaml_output_repository.ts b/src/infrastructure/persistence/yaml_output_repository.ts index ae14070d..aeb8833e 100644 --- a/src/infrastructure/persistence/yaml_output_repository.ts +++ b/src/infrastructure/persistence/yaml_output_repository.ts @@ -28,6 +28,7 @@ import { toAbsolutePath, toRelativePath, } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import type { OutputRepository } from "../../domain/models/repositories.ts"; import type { DefinitionId } from "../../domain/definitions/definition.ts"; import type { ModelType } from "../../domain/models/model_type.ts"; @@ -162,6 +163,7 @@ export class YamlOutputRepository implements OutputRepository { output: ModelOutput, ): Promise { const dir = this.getMethodDir(type, method); + await assertSafePath(dir, swampPath(this.repoDir)); await ensureDir(dir); const path = this.getPath(type, method, output); diff --git a/src/infrastructure/persistence/yaml_vault_config_repository.ts b/src/infrastructure/persistence/yaml_vault_config_repository.ts index 5a0a9af2..cbc50c25 100644 --- a/src/infrastructure/persistence/yaml_vault_config_repository.ts +++ b/src/infrastructure/persistence/yaml_vault_config_repository.ts @@ -22,6 +22,7 @@ import { join, resolve } from "@std/path"; import { parse as parseYaml, stringify as stringifyYaml } from "@std/yaml"; import { atomicWriteTextFile } from "./atomic_write.ts"; import { SWAMP_SUBDIRS, swampPath } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import { VaultConfig, type VaultConfigData, @@ -160,6 +161,7 @@ export class YamlVaultConfigRepository { */ async save(config: VaultConfig): Promise { const dir = this.getTypeDir(config.type); + await assertSafePath(dir, swampPath(this.repoDir)); await ensureDir(dir); const path = this.getPath(config.type, config.id); diff --git a/src/infrastructure/persistence/yaml_workflow_repository.ts b/src/infrastructure/persistence/yaml_workflow_repository.ts index 6b319dd7..e4513832 100644 --- a/src/infrastructure/persistence/yaml_workflow_repository.ts +++ b/src/infrastructure/persistence/yaml_workflow_repository.ts @@ -24,6 +24,7 @@ import { atomicWriteTextFile } from "./atomic_write.ts"; import { parse as parseYaml, stringify as stringifyYaml } from "@std/yaml"; import type { WorkflowRepository } from "../../domain/workflows/repositories.ts"; import { SWAMP_SUBDIRS, swampPath } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import { createWorkflowId, type WorkflowId, @@ -115,6 +116,7 @@ export class YamlWorkflowRepository implements WorkflowRepository { async save(workflow: Workflow): Promise { const dir = this.getWorkflowsDir(); + await assertSafePath(dir, swampPath(this.repoDir)); await ensureDir(dir); const path = this.getPath(workflow.id); diff --git a/src/infrastructure/persistence/yaml_workflow_run_repository.ts b/src/infrastructure/persistence/yaml_workflow_run_repository.ts index 09be886f..2d0a03a2 100644 --- a/src/infrastructure/persistence/yaml_workflow_run_repository.ts +++ b/src/infrastructure/persistence/yaml_workflow_run_repository.ts @@ -28,6 +28,7 @@ import { toAbsolutePath, toRelativePath, } from "./paths.ts"; +import { assertSafePath } from "./safe_path.ts"; import { createWorkflowRunId, type WorkflowId, @@ -170,6 +171,7 @@ export class YamlWorkflowRunRepository implements WorkflowRunRepository { async save(workflowId: WorkflowId, run: WorkflowRun): Promise { const dir = this.getRunsDir(workflowId); + await assertSafePath(dir, swampPath(this.repoDir)); await ensureDir(dir); const path = this.getPath(workflowId, run.id); diff --git a/src/infrastructure/repo/symlink_repo_index_service.ts b/src/infrastructure/repo/symlink_repo_index_service.ts index 4e5db83b..e5cda4d6 100644 --- a/src/infrastructure/repo/symlink_repo_index_service.ts +++ b/src/infrastructure/repo/symlink_repo_index_service.ts @@ -25,13 +25,17 @@ */ import { ensureDir } from "@std/fs"; -import { join, relative, resolve } from "@std/path"; +import { join, relative } from "@std/path"; import { getLogger } from "@logtape/logtape"; import { SWAMP_DATA_DIR, SWAMP_SUBDIRS, swampPath, } from "../persistence/paths.ts"; +import { + assertSafePath, + PathTraversalError, +} from "../persistence/safe_path.ts"; import type { PruneResult, RebuildResult, @@ -128,11 +132,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { async handleModelDeleted(event: ModelDeleted): Promise { const modelsBaseDir = join(this.repoDir, "models"); const modelDir = join(modelsBaseDir, event.modelName); - this.assertPathContained( - modelDir, - modelsBaseDir, - `modelName "${event.modelName}"`, - ); + await assertSafePath(modelDir, this.repoDir); await this.removeDirectory(modelDir); } @@ -151,11 +151,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { async handleWorkflowDeleted(event: WorkflowDeleted): Promise { const workflowsBaseDir = join(this.repoDir, "workflows"); const workflowDir = join(workflowsBaseDir, event.workflowName); - this.assertPathContained( - workflowDir, - workflowsBaseDir, - `workflowName "${event.workflowName}"`, - ); + await assertSafePath(workflowDir, this.repoDir); await this.removeDirectory(workflowDir); } @@ -202,11 +198,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { async handleVaultDeleted(event: VaultDeleted): Promise { const vaultsBaseDir = join(this.repoDir, "vaults"); const vaultDir = join(vaultsBaseDir, event.vaultName); - this.assertPathContained( - vaultDir, - vaultsBaseDir, - `vaultName "${event.vaultName}"`, - ); + await assertSafePath(vaultDir, this.repoDir); await this.removeDirectory(vaultDir); } @@ -413,11 +405,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { ): Promise { const modelsBaseDir = join(this.repoDir, "models"); const modelDir = join(modelsBaseDir, modelName); - this.assertPathContained( - modelDir, - modelsBaseDir, - `modelName "${modelName}"`, - ); + await assertSafePath(modelDir, this.repoDir); await ensureDir(modelDir); // Create ModelType for path generation @@ -544,20 +532,12 @@ export class SymlinkRepoIndexService implements RepoIndexService { for (const [tagKey, valueMap] of customTags) { const modelDir = join(typeDir, ".."); const tagKeyDir = join(modelDir, tagKey); - this.assertPathContained( - tagKeyDir, - modelDir, - `tagKey "${tagKey}"`, - ); + await assertSafePath(tagKeyDir, modelDir); await ensureDir(tagKeyDir); for (const [tagValue, dataItems] of valueMap) { const tagValueDir = join(tagKeyDir, tagValue); - this.assertPathContained( - tagValueDir, - tagKeyDir, - `tagValue "${tagValue}"`, - ); + await assertSafePath(tagValueDir, tagKeyDir); await ensureDir(tagValueDir); for (const data of dataItems) { @@ -584,8 +564,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { } } catch (error) { if ( - error instanceof Error && - error.message.startsWith("Path traversal detected") + error instanceof PathTraversalError ) { throw error; } @@ -622,11 +601,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { ): Promise { const workflowsBaseDir = join(this.repoDir, "workflows"); const workflowDir = join(workflowsBaseDir, workflowName); - this.assertPathContained( - workflowDir, - workflowsBaseDir, - `workflowName "${workflowName}"`, - ); + await assertSafePath(workflowDir, this.repoDir); await ensureDir(workflowDir); // Symlink to workflow.yaml @@ -654,11 +629,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { ): Promise { const workflowsBaseDir = join(this.repoDir, "workflows"); const workflowDir = join(workflowsBaseDir, workflowName); - this.assertPathContained( - workflowDir, - workflowsBaseDir, - `workflowName "${workflowName}"`, - ); + await assertSafePath(workflowDir, this.repoDir); const runsDir = join(workflowDir, "runs"); await ensureDir(runsDir); @@ -700,11 +671,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { for (const job of run.jobs) { for (const step of job.steps) { const stepDir = join(stepsDir, step.stepName); - this.assertPathContained( - stepDir, - stepsDir, - `stepName "${step.stepName}"`, - ); + await assertSafePath(stepDir, stepsDir); await ensureDir(stepDir); // Note: Step output symlinks would require additional context @@ -728,11 +695,7 @@ export class SymlinkRepoIndexService implements RepoIndexService { ): Promise { const vaultsBaseDir = join(this.repoDir, "vaults"); const vaultDir = join(vaultsBaseDir, vaultName); - this.assertPathContained( - vaultDir, - vaultsBaseDir, - `vaultName "${vaultName}"`, - ); + await assertSafePath(vaultDir, this.repoDir); await ensureDir(vaultDir); // Symlink to vault.yaml @@ -976,27 +939,6 @@ export class SymlinkRepoIndexService implements RepoIndexService { } } - /** - * Asserts that a path is contained within an expected parent directory. - * Throws if the resolved path escapes the parent, preventing path traversal. - */ - private assertPathContained( - path: string, - expectedParent: string, - context: string, - ): void { - const resolvedPath = resolve(path); - const resolvedParent = resolve(expectedParent); - if ( - resolvedPath !== resolvedParent && - !resolvedPath.startsWith(resolvedParent + "/") - ) { - throw new Error( - `Path traversal detected: ${context} resolves outside expected directory`, - ); - } - } - /** * Recursively removes broken symlinks in a directory. */