From bba99bea934fe397b6aed59d3f33a5315799fa28 Mon Sep 17 00:00:00 2001 From: "Somhairle H. Marisol" Date: Mon, 21 Sep 2026 07:54:39 +0800 Subject: Harden artifact serving: root-chain link validation, deterministic stream disposal --- README.md | 15 +- src/SomhairlesDream.Server/ArtifactRunApi.fs | 2 +- .../ArtifactRunCoordinator.fs | 116 ++++++------ tests/SomhairlesDream.Server.Tests/ApiTests.fs | 201 ++++++++++++++++++++- 4 files changed, 276 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 0f126db..4c23116 100644 --- a/README.md +++ b/README.md @@ -127,10 +127,17 @@ serves only paths published in the verified run manifest (logs, the manifest itself, and stray files are never exposed), `steps` serves only checkpointed GLBs (manifest members for completed runs), and every served stream is hashed against its recorded SHA-256 and byte count at request -time. Filesystem links inside a run directory (file or intermediate -directory symlinks/junctions) are rejected with `400`; hash or byte-count -deviations return `409`, and manifest failures surface as `500`. The -server binds to the ASP.NET default (add +time. Filesystem links anywhere in the chain from the configured artifact +root through the project and run directories down to the requested file +are rejected with `400` — for manifest reads, checkpoint digest recording, +and every served stream — and a failed open disposes its stream +deterministically. Hash or byte-count deviations return `409`, and +manifest verification failures surface as `500`. Link checks are not +atomic with the open: a concurrent local writer could substitute a path +component between the check and the open (TOCTOU); served content remains +bound to the recorded SHA-256 digest, so a substituted file is served only +if byte-identical. Operators must treat the artifact root as trusted +against local writers. The server binds to the ASP.NET default (add `--urls http://127.0.0.1:8099` to match the legacy port). ## CLI diff --git a/src/SomhairlesDream.Server/ArtifactRunApi.fs b/src/SomhairlesDream.Server/ArtifactRunApi.fs index 48ecc08..6c00d66 100644 --- a/src/SomhairlesDream.Server/ArtifactRunApi.fs +++ b/src/SomhairlesDream.Server/ArtifactRunApi.fs @@ -91,7 +91,7 @@ module ArtifactRunApi = | Error RunNotFound -> error options 404 "run not found" | Error(RunNotComplete snapshot) -> jsonWithStatus options 409 snapshot | Error(InvalidManifest message) -> error options 500 message - | Error(InvalidArtifactPath message) -> error options 500 message + | Error(InvalidArtifactPath message) -> error options 400 message | Error ArtifactNotFound -> error options 500 "manifest not found" | Error(ArtifactMismatch message) -> error options 500 message diff --git a/src/SomhairlesDream.Server/ArtifactRunCoordinator.fs b/src/SomhairlesDream.Server/ArtifactRunCoordinator.fs index ae222d5..022ac7d 100644 --- a/src/SomhairlesDream.Server/ArtifactRunCoordinator.fs +++ b/src/SomhairlesDream.Server/ArtifactRunCoordinator.fs @@ -75,34 +75,10 @@ type ArtifactRunCoordinator( else Error "artifact path escapes run directory" - let checkpointDigests = - ConcurrentDictionary>() - - let recordCheckpointDigest (ids: RunIds) (relativePath: string) = - try - match safeArtifactPath (runDirectory ids.ProjectId ids.RunId) relativePath with - | Ok(fullPath, normalized) when File.Exists(fullPath) -> - use stream = File.OpenRead(fullPath) - use sha = SHA256.Create() - - let digest = - (Convert.ToHexString(sha.ComputeHash(stream)).ToLowerInvariant(), stream.Length) - - let digests = - checkpointDigests.GetOrAdd( - (ids.ProjectId, ids.RunId), - fun _ -> ConcurrentDictionary() - ) - - digests[normalized] <- digest - | _ -> () - with _ -> - () - - let rejectLinks (runDirectory: string) (fullPath: string) = - let root = Path.GetFullPath(runDirectory) - let relative = fullPath.Substring(root.Length + 1) - let mutable current = root + let rejectLinks (fullPath: string) = + let rootPath = Path.GetFullPath(root) + let relative = fullPath.Substring(rootPath.Length + 1) + let mutable current = rootPath let mutable outcome = Ok () for segment in @@ -126,14 +102,40 @@ type ArtifactRunCoordinator( outcome + let checkpointDigests = + ConcurrentDictionary>() + + let recordCheckpointDigest (ids: RunIds) (relativePath: string) = + try + match safeArtifactPath (runDirectory ids.ProjectId ids.RunId) relativePath with + | Ok(fullPath, normalized) when File.Exists(fullPath) -> + match rejectLinks fullPath with + | Error _ -> () + | Ok () -> + use stream = File.OpenRead(fullPath) + use sha = SHA256.Create() + + let digest = + (Convert.ToHexString(sha.ComputeHash(stream)).ToLowerInvariant(), stream.Length) + + let digests = + checkpointDigests.GetOrAdd( + (ids.ProjectId, ids.RunId), + fun _ -> ConcurrentDictionary() + ) + + digests[normalized] <- digest + | _ -> () + with _ -> + () + let openValidated - (runDirectory: string) (fullPath: string) (normalized: string) (expectedSha256: string) (expectedBytes: int64) : Result = - match rejectLinks runDirectory fullPath with + match rejectLinks fullPath with | Error message -> Error(InvalidArtifactPath message) | Ok () -> try @@ -143,23 +145,30 @@ type ArtifactRunCoordinator( let stream = File.Open(fullPath, FileMode.Open, FileAccess.Read, FileShare.Read) - try - if stream.Length <> expectedBytes then - Error(ArtifactMismatch $"artifact byte count mismatch: {normalized}") - else - use sha = SHA256.Create() - - let sha256 = - Convert.ToHexString(sha.ComputeHash(stream)).ToLowerInvariant() - - if sha256 <> expectedSha256 then - Error(ArtifactMismatch $"artifact hash mismatch: {normalized}") + let outcome = + try + if stream.Length <> expectedBytes then + Error(ArtifactMismatch $"artifact byte count mismatch: {normalized}") else - stream.Seek(0L, SeekOrigin.Begin) |> ignore - Ok { Stream = stream :> Stream; RelativePath = normalized } - with _ -> + use sha = SHA256.Create() + + let sha256 = + Convert.ToHexString(sha.ComputeHash(stream)).ToLowerInvariant() + + if sha256 <> expectedSha256 then + Error(ArtifactMismatch $"artifact hash mismatch: {normalized}") + else + stream.Seek(0L, SeekOrigin.Begin) |> ignore + Ok { Stream = stream :> Stream; RelativePath = normalized } + with _ -> + stream.Dispose() + reraise () + + match outcome with + | Ok file -> Ok file + | Error lookupError -> stream.Dispose() - reraise () + Error lookupError with | :? FileNotFoundException | :? DirectoryNotFoundException -> Error ArtifactNotFound @@ -168,9 +177,12 @@ type ArtifactRunCoordinator( let verifiedManifest (projectId: string) (runId: string) = let path = Path.Combine(runDirectory projectId runId, "manifest.json") - match ArtifactVerifier.verify path with - | Ok manifest -> Ok manifest - | Error message -> Error(InvalidManifest message) + match rejectLinks path with + | Error message -> Error(InvalidArtifactPath message) + | Ok () -> + match ArtifactVerifier.verify path with + | Ok manifest -> Ok manifest + | Error message -> Error(InvalidManifest message) let memberDigests (manifest: ArtifactManifest) = let map = Dictionary() @@ -271,12 +283,12 @@ type ArtifactRunCoordinator( match safeArtifactPath directory relativePath with | Error message -> Error(InvalidArtifactPath message) | Ok(fullPath, normalized) -> - match rejectLinks directory fullPath with + match rejectLinks fullPath with | Error message -> Error(InvalidArtifactPath message) | Ok () -> match completedDigest projectId runId normalized with | Error lookupError -> Error lookupError - | Ok(sha256, bytes) -> openValidated directory fullPath normalized sha256 bytes + | Ok(sha256, bytes) -> openValidated fullPath normalized sha256 bytes member _.StepArtifact(projectId: string, runId: string, relativePath: string) : Result = match registry.TryFind(projectId, runId) with @@ -294,7 +306,7 @@ type ArtifactRunCoordinator( let snapshot = store.Observe(clock ()) let digest = - match rejectLinks directory fullPath with + match rejectLinks fullPath with | Error message -> Error(InvalidArtifactPath message) | Ok () -> if snapshot.Status = "complete" then @@ -306,4 +318,4 @@ type ArtifactRunCoordinator( match digest with | Error lookupError -> Error lookupError - | Ok(sha256, bytes) -> openValidated directory fullPath normalized sha256 bytes + | Ok(sha256, bytes) -> openValidated fullPath normalized sha256 bytes diff --git a/tests/SomhairlesDream.Server.Tests/ApiTests.fs b/tests/SomhairlesDream.Server.Tests/ApiTests.fs index be6a6cb..93ec849 100644 --- a/tests/SomhairlesDream.Server.Tests/ApiTests.fs +++ b/tests/SomhairlesDream.Server.Tests/ApiTests.fs @@ -67,9 +67,12 @@ let private requestBody runId = let private withAppUsing (bridgeFactory: unit -> IArtifactBridge) action = withTempDirectory (fun root -> let clock () = DateTimeOffset.Parse("2026-09-20T12:00:00Z") + let artifactRoot = Path.Combine(root, "artifacts") + Directory.CreateDirectory(artifactRoot) |> ignore + let coordinator = ArtifactRunCoordinator( - root, + artifactRoot, bridgeFactory, clock, TimeSpan.FromSeconds 15. @@ -287,6 +290,62 @@ let private waitForTerminal (coordinator: ArtifactRunCoordinator) runId = status = "complete" || status = "failed")) ) +let private fdCount () = + if Directory.Exists("/proc/self/fd") then + Directory.GetFiles("/proc/self/fd").Length + else + -1 + +let private copyDirectory (source: string) (destination: string) = + Directory.CreateDirectory(destination) |> ignore + + for directory in Directory.GetDirectories(source, "*", SearchOption.AllDirectories) do + Directory.CreateDirectory(Path.Combine(destination, directory.Substring(source.Length + 1))) + |> ignore + + for file in Directory.GetFiles(source, "*", SearchOption.AllDirectories) do + let relative = file.Substring(source.Length + 1) + let target = Path.Combine(destination, relative) + Directory.CreateDirectory(Path.GetDirectoryName(target)) |> ignore + File.Copy(file, target) + +let private getManifest (client: HttpClient) runId = + client + .GetAsync($"/api/artifacts/manifest?projectId={ids.ProjectId}&runId={runId}") + .GetAwaiter() + .GetResult() + +let private assertArtifactServed (client: HttpClient) runId = + let glbResponse = getArtifact client "file" runId "steps/01-foundation.glb" + Assert.Equal(HttpStatusCode.OK, glbResponse.StatusCode) + + let stepResponse = getArtifact client "steps" runId "steps/01-foundation.glb" + Assert.Equal(HttpStatusCode.OK, stepResponse.StatusCode) + + let manifestResponse = getManifest client runId + Assert.Equal(HttpStatusCode.OK, manifestResponse.StatusCode) + +let private assertArtifactRejected (client: HttpClient) runId = + let glbResponse = getArtifact client "file" runId "steps/01-foundation.glb" + Assert.Equal(HttpStatusCode.BadRequest, glbResponse.StatusCode) + + let stepResponse = getArtifact client "steps" runId "steps/01-foundation.glb" + Assert.Equal(HttpStatusCode.BadRequest, stepResponse.StatusCode) + + let manifestResponse = getManifest client runId + Assert.Equal(HttpStatusCode.BadRequest, manifestResponse.StatusCode) + +let private outsideDirectory (coordinator: ArtifactRunCoordinator) name = + Path.Combine(Directory.GetParent(coordinator.ArtifactRoot).FullName, name) + +let private replaceWithSymlink (source: string) (target: string) = + if File.Exists(source) then + File.Delete(source) + else + Directory.Delete(source, true) + + Directory.CreateSymbolicLink(source, target) |> ignore + [] let ``file endpoint serves only published manifest members`` () = withApp (fun client coordinator -> @@ -532,3 +591,143 @@ let ``events endpoint streams only the selected run`` () = .GetResult() Assert.Equal(HttpStatusCode.NotFound, unknown.StatusCode)) + +[] +let ``artifact streams dispose deterministically on repeated mismatches`` () = + let gate = new ManualResetEventSlim(false) + + try + withAppUsing + (fun () -> StepGatedBridge(gate) :> IArtifactBridge) + (fun client coordinator -> + let runId = "run-api-dispose" + use content = new StringContent(requestBody runId, Encoding.UTF8, "application/json") + let start = client.PostAsync("/api/runs/start", content).GetAwaiter().GetResult() + Assert.Equal(HttpStatusCode.Accepted, start.StatusCode) + + Assert.True( + waitFor (fun () -> + coordinator.TryFind(ids.ProjectId, runId) + |> Option.exists (fun store -> + let snapshot = store.Snapshot() + snapshot.Status = "running" && snapshot.CompletedSteps.Length = 1)) + ) + + let glbPath = Path.Combine(runDirectory coordinator runId, "steps", "01-foundation.glb") + File.WriteAllBytes(glbPath, glbBytes 0x09uy) + + let baseline = fdCount () + let assertNoLeak () = + if baseline >= 0 then + Assert.True( + fdCount () <= baseline + 2, + "failed artifact opens should not leak file handles" + ) + + for _ in 1..20 do + match coordinator.StepArtifact(ids.ProjectId, runId, "steps/01-foundation.glb") with + | Error(ArtifactMismatch _) -> () + | other -> failwith $"expected ArtifactMismatch, got {other}" + + assertNoLeak () + + for _ in 1..10 do + let response = getArtifact client "steps" runId "steps/01-foundation.glb" + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode) + + File.WriteAllBytes(glbPath, [| 0x01uy; 0x02uy; 0x03uy; 0x04uy; 0x05uy |]) + + for _ in 1..20 do + match coordinator.StepArtifact(ids.ProjectId, runId, "steps/01-foundation.glb") with + | Error(ArtifactMismatch _) -> () + | other -> failwith $"expected ArtifactMismatch, got {other}" + + assertNoLeak () + + for _ in 1..10 do + let response = getArtifact client "steps" runId "steps/01-foundation.glb" + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode) + + gate.Set() + waitForTerminal coordinator runId) + finally + gate.Set() + gate.Dispose() + +[] +let ``artifact serving rejects symlinked project directories`` () = + withApp (fun client coordinator -> + let runId = "run-api-projlink" + use content = new StringContent(requestBody runId, Encoding.UTF8, "application/json") + let start = client.PostAsync("/api/runs/start", content).GetAwaiter().GetResult() + Assert.Equal(HttpStatusCode.Accepted, start.StatusCode) + + Assert.True( + waitFor (fun () -> + coordinator.TryFind(ids.ProjectId, runId) + |> Option.exists (fun store -> store.Snapshot().Status = "complete")) + ) + + assertArtifactServed client runId + + let projectDirectory = Path.Combine(coordinator.ArtifactRoot, ids.ProjectId) + let escapeDirectory = outsideDirectory coordinator "escape-project" + copyDirectory projectDirectory escapeDirectory + replaceWithSymlink projectDirectory escapeDirectory + + assertArtifactRejected client runId) + +[] +let ``artifact serving rejects symlinked run directories`` () = + withApp (fun client coordinator -> + let runId = "run-api-runlink" + use content = new StringContent(requestBody runId, Encoding.UTF8, "application/json") + let start = client.PostAsync("/api/runs/start", content).GetAwaiter().GetResult() + Assert.Equal(HttpStatusCode.Accepted, start.StatusCode) + + Assert.True( + waitFor (fun () -> + coordinator.TryFind(ids.ProjectId, runId) + |> Option.exists (fun store -> store.Snapshot().Status = "complete")) + ) + + assertArtifactServed client runId + + let escapeDirectory = outsideDirectory coordinator "escape-run" + copyDirectory (runDirectory coordinator runId) escapeDirectory + replaceWithSymlink (runDirectory coordinator runId) escapeDirectory + + assertArtifactRejected client runId) + +[] +let ``artifact serving rejects links introduced during an active run`` () = + let gate = new ManualResetEventSlim(false) + + try + withAppUsing + (fun () -> StepGatedBridge(gate) :> IArtifactBridge) + (fun client coordinator -> + let runId = "run-api-midlink" + use content = new StringContent(requestBody runId, Encoding.UTF8, "application/json") + let start = client.PostAsync("/api/runs/start", content).GetAwaiter().GetResult() + Assert.Equal(HttpStatusCode.Accepted, start.StatusCode) + + Assert.True( + waitFor (fun () -> + coordinator.TryFind(ids.ProjectId, runId) + |> Option.exists (fun store -> + let snapshot = store.Snapshot() + snapshot.Status = "running" && snapshot.CompletedSteps.Length = 1)) + ) + + let escapeDirectory = outsideDirectory coordinator "escape-midrun" + copyDirectory (runDirectory coordinator runId) escapeDirectory + replaceWithSymlink (runDirectory coordinator runId) escapeDirectory + + gate.Set() + waitForTerminal coordinator runId + + assertArtifactRejected client runId) + finally + gate.Set() + gate.Dispose() -- cgit v1.2.3