Harden artifact serving: root-chain link validation, deterministic stream disposal

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<string * string, ConcurrentDictionary<string, string * int64>>()
-
-    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<string, string * int64>()
-                    )
-
-                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<string * string, ConcurrentDictionary<string, string * int64>>()
+
+    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<string, string * int64>()
+                        )
+
+                    digests[normalized] <- digest
+            | _ -> ()
+        with _ ->
+            ()
+
     let openValidated
-        (runDirectory: string)
         (fullPath: string)
         (normalized: string)
         (expectedSha256: string)
         (expectedBytes: int64)
         : Result<ArtifactFile, ArtifactLookupError> =
-        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<string, string * int64>()
@@ -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<ArtifactFile, ArtifactLookupError> =
         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
+
 [<Fact>]
 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))
+
+[<Fact>]
+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()
+
+[<Fact>]
+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)
+
+[<Fact>]
+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)
+
+[<Fact>]
+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()