Merge pull request #3903 from OpenNeuroOrg/getFiles-refs-fix

refactor(server): Resolve refs for getFiles to prevent cache pollution/duplication

Author: nellh

packages/openneuro-server/src/cache/types.ts

@@ -16,4 +16,5 @@ export enum CacheType {
   brainInitiative = "brainInitiative",
   validation = "validation",
   dataciteYml = "dataciteYml",
+  gitRef = "ref",
 }

packages/openneuro-server/src/datalad/files.ts

@@ -15,7 +15,9 @@ import {
   setTree,
   type TreeEntry,
 } from "../cache/tree"
+import CacheItem, { CacheType } from "../cache/item"
 import { join } from "node:path"
+import { captureException } from "@sentry/node"
 
 /**
  * Convert to URL compatible path
@@ -271,6 +273,37 @@ async function cacheWorkerTrees(
   return result
 }
 
+/**
+ * Resolve a git reference (branch, tag, or short commit hash) to a full commit hash.
+ * @param datasetId The dataset ID.
+ * @param treeish The git reference to resolve.
+ * @returns The full commit hash.
+ */
+export const resolveGitRef = async (
+  datasetId: string,
+  treeish: string,
+): Promise<string> => {
+  const cache = new CacheItem(redis, CacheType.gitRef, [datasetId, treeish])
+  return cache.get(async () => {
+    const url = `http://${
+      getDatasetWorker(datasetId)
+    }/datasets/${datasetId}/refs/${treeish}`
+    const response = await fetch(url)
+    if (!response.ok) {
+      throw new Error(
+        `Failed to resolve git reference ${treeish}: ${response.statusText}`,
+      )
+    }
+    const data = await response.json()
+    if (!data.hash) {
+      throw new Error(
+        `Invalid response from datalad worker for git reference ${treeish}`,
+      )
+    }
+    return data.hash
+  })
+}
+
 /**
  * Get files for a specific revision (tree hash or commit hash).
  * Uses content-addressed caching keyed by full git hash.
@@ -279,6 +312,17 @@ export const getFiles = async (
   datasetId: string,
   treeish: string,
 ): Promise<DatasetFile[]> => {
+  // Guard against requests without a full git hash (40 = SHA-1, 64 = SHA-256)
+  if (treeish.length !== 40 && treeish.length !== 64) {
+    try {
+      treeish = await resolveGitRef(datasetId, treeish)
+    } catch (error) {
+      captureException(error, {
+        tags: { datasetId, treeish, source: "resolveGitRef" },
+      })
+      throw new Error(`Invalid git reference: ${treeish}`)
+    }
+  }
   // Try cache first
   const cached = await getTree(redis, treeish)
   if (cached) {

services/datalad/datalad_service/app.py

@@ -35,6 +35,7 @@
 from datalad_service.handlers.remote_import import RemoteImportResource
 from datalad_service.handlers.info import InfoResource
 from datalad_service.handlers.fsck import FsckResource
+from datalad_service.handlers.refs import RefsResource
 from datalad_service.middleware.auth import AuthenticateMiddleware
 from datalad_service.middleware.error import CustomErrorHandlerMiddleware
 
@@ -118,6 +119,7 @@ def create_app():
     dataset_remote_import_resource = RemoteImportResource(store)
     dataset_info_resource = InfoResource(store)
     dataset_fsck_resource = FsckResource(store)
+    dataset_refs_resource = RefsResource(store)
 
     app.add_route('/heartbeat', heartbeat)
 
@@ -181,6 +183,7 @@ def create_app():
     app.add_route(
         '/datasets/{dataset:dataset}/import/{import_id}', dataset_remote_import_resource
     )
+    app.add_route('/datasets/{dataset:dataset}/refs/{treeish}', dataset_refs_resource)
 
     if 'pytest' in sys.modules:
         return app  # Do not wrap in Sentry middleware during tests

services/datalad/datalad_service/handlers/refs.py

@@ -0,0 +1,33 @@
+import falcon
+import pygit2
+import os
+
+
+class RefsResource:
+    """
+    Resolve a git reference (branch, tag, or short commit hash) to a full commit hash.
+    """
+
+    def __init__(self, store):
+        self.store = store
+
+    async def on_get(self, req, resp, dataset, treeish):
+        dataset_path = self.store.get_dataset_path(dataset)
+        if not os.path.exists(dataset_path):
+            resp.media = {'error': f'Dataset "{dataset}" not found.'}
+            resp.status = falcon.HTTP_NOT_FOUND
+            return
+
+        try:
+            repo = pygit2.Repository(dataset_path)
+            commit = repo.revparse_single(treeish)
+            resp.media = {'hash': str(commit.id)}
+            resp.status = falcon.HTTP_OK
+        except pygit2.GitError:
+            resp.media = {
+                'error': f'Git reference "{treeish}" not found or is ambiguous in dataset "{dataset}".'
+            }
+            resp.status = falcon.HTTP_NOT_FOUND
+        except Exception as e:
+            resp.media = {'error': f'An unexpected error occurred: {e}'}
+            resp.status = falcon.HTTP_INTERNAL_SERVER_ERROR