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

2 files changed, 65 insertions, 53 deletions

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