Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions helper-apps/cortex-file-handler/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion helper-apps/cortex-file-handler/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@aj-archipelago/cortex-file-handler",
"version": "2.8.0",
"version": "2.8.1",
"description": "File handling service for Cortex - handles file uploads, media chunking, and document processing",
"type": "module",
"main": "src/index.js",
Expand Down
26 changes: 24 additions & 2 deletions helper-apps/cortex-file-handler/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,13 +501,34 @@ async function CortexFileHandler(context, req) {
return;
}

// Reconstruct missing filename from URL if needed (before creating response)
if (!hashResult.filename && hashResult.url) {
try {
const urlObj = new URL(hashResult.url);
const pathSegments = urlObj.pathname.split('/').filter(segment => segment.length > 0);
if (pathSegments.length > 0) {
// Extract filename from URL path (last segment)
const blobName = pathSegments[pathSegments.length - 1];
// Remove query params if any got included
hashResult.filename = blobName.split('?')[0];
}
} catch (error) {
context.log(`Error extracting filename from URL: ${error.message}`);
}
}

// Ensure hash is set if missing
if (!hashResult.hash) {
hashResult.hash = hash;
}

// Create the response object
const response = {
message: `File '${hashResult.filename}' uploaded successfully.`,
message: `File '${hashResult.filename || 'unknown'}' uploaded successfully.`,
filename: hashResult.filename,
url: hashResult.url,
gcs: hashResult.gcs,
hash: hashResult.hash,
hash: hashResult.hash || hash,
timestamp: new Date().toISOString(),
};

Expand Down Expand Up @@ -625,6 +646,7 @@ async function CortexFileHandler(context, req) {

// Update redis timestamp with current time
// Note: setFileStoreMap will remove shortLivedUrl fields before storing
// hashResult has already been enriched with filename/hash above if missing
await setFileStoreMap(hash, hashResult, resolvedContextId);

context.res = {
Expand Down
84 changes: 20 additions & 64 deletions helper-apps/cortex-file-handler/src/redis.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,6 @@ import { getDefaultContainerName } from "./constants.js";

const connectionString = process.env["REDIS_CONNECTION_STRING"];

/**
* Get key for Redis storage.
*
* IMPORTANT:
* - We **never** write hash+container scoped keys anymore (legacy only).
* - We *do* support (optional) hash+contextId scoping for per-user/per-context storage.
* - For reads, we can fall back to legacy hash+container keys if they still exist in Redis.
*
* Key format:
* - No context: "<hash>"
* - With contextId: "<hash>:ctx:<contextId>"
*
* @param {string} hash - The file hash
* @param {string|null} contextId - Optional context id
* @returns {string} The redis key for this hash/context
*/
export const getScopedHashKey = (hash, contextId = null) => {
if (!hash) return hash;
if (!contextId) return hash;
return `${hash}:ctx:${contextId}`;
};

const legacyContainerKey = (hash, containerName) => {
if (!hash || !containerName) return null;
Expand Down Expand Up @@ -304,32 +283,33 @@ const getFileStoreMap = async (hash, skipLazyCleanup = false, contextId = null)
if (contextId) {
const contextMapKey = `FileStoreMap:ctx:${contextId}`;
value = await client.hget(contextMapKey, hash);
}

// Fall back to unscoped map if not found
if (!value) {
// If contextId is provided, do NOT fall back to unscoped map
// This ensures proper context isolation and forces re-upload to current storage account
if (!value) {
return null;
}
} else {
// No contextId - check unscoped map
value = await client.hget("FileStoreMap", hash);
}

// Backwards compatibility for unscoped keys only:
// If unscoped hash doesn't exist, fall back to legacy hash+container key (if still present).
// SECURITY: Context-scoped lookups NEVER fall back - they must match exactly.
if (!value && !contextId) {
const baseHash = hash;

// Only allow fallback for unscoped keys (not context-scoped)
// Context-scoped keys are security-isolated and must match exactly
if (baseHash && !String(baseHash).includes(":")) {
if (hash && !String(hash).includes(":")) {
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This use of variable 'hash' always evaluates to true.

Suggested change
if (hash && !String(hash).includes(":")) {
if (!String(hash).includes(":")) {

Copilot uses AI. Check for mistakes.
const defaultContainerName = getDefaultContainerName();
const legacyKey = legacyContainerKey(baseHash, defaultContainerName);
const legacyKey = legacyContainerKey(hash, defaultContainerName);
if (legacyKey) {
value = await client.hget("FileStoreMap", legacyKey);
if (value) {
console.log(
`Found legacy container-scoped key ${legacyKey} for hash ${baseHash}; migrating to unscoped key`,
`Found legacy container-scoped key ${legacyKey} for hash ${hash}; migrating to unscoped key`,
);
// Migrate to unscoped key (we do NOT write legacy container-scoped keys)
await client.hset("FileStoreMap", baseHash, value);
await client.hset("FileStoreMap", hash, value);
// Delete the legacy key after migration
await client.hdel("FileStoreMap", legacyKey);
console.log(`Deleted legacy key ${legacyKey} after migration`);
Expand Down Expand Up @@ -415,62 +395,38 @@ const getFileStoreMap = async (hash, skipLazyCleanup = false, contextId = null)
// Function to remove key from "FileStoreMap" hash map
// If contextId is provided, removes from context-scoped map
// Otherwise removes from unscoped map
// Hash can be either raw hash or scoped key format (hash:ctx:contextId)
// If scoped format is provided, extracts base hash and removes both scoped and legacy keys
const removeFromFileStoreMap = async (hash, contextId = null) => {
try {
if (!hash) {
return;
}

// Extract base hash if hash is in scoped format (hash:ctx:contextId)
let baseHash = hash;
let extractedContextId = contextId;
if (String(hash).includes(":ctx:")) {
const parts = String(hash).split(":ctx:");
baseHash = parts[0];
if (parts.length > 1 && !extractedContextId) {
extractedContextId = parts[1];
}
}

let result = 0;

// First, try to delete from unscoped map (in case scoped key was stored there)
// First, try to delete from unscoped map
if (!contextId) {
// Remove from unscoped map (including scoped key format if present)
result = await client.hdel("FileStoreMap", hash);
// Also try removing with base hash if hash was scoped
if (hash !== baseHash) {
const baseResult = await client.hdel("FileStoreMap", baseHash);
if (baseResult > 0) {
result = baseResult;
}
}
}

// Also try to delete from context-scoped map if we extracted a contextId
if (extractedContextId) {
const contextMapKey = `FileStoreMap:ctx:${extractedContextId}`;
const contextResult = await client.hdel(contextMapKey, baseHash);
// Also try to delete from context-scoped map if contextId is provided
if (contextId) {
const contextMapKey = `FileStoreMap:ctx:${contextId}`;
const contextResult = await client.hdel(contextMapKey, hash);
if (contextResult > 0) {
result = contextResult;
}
} else if (contextId) {
// If contextId was provided explicitly, delete from context-scoped map
const contextMapKey = `FileStoreMap:ctx:${contextId}`;
result = await client.hdel(contextMapKey, hash);
}

if (result > 0) {
console.log(`The hash ${hash} was removed successfully`);
}

// Always try to clean up legacy container-scoped entry as well.
// This ensures we don't leave orphaned legacy keys behind.
// Only attempt legacy cleanup if baseHash doesn't contain a colon (not already scoped)
if (!String(baseHash).includes(":")) {
// Only attempt legacy cleanup if hash doesn't contain a colon
if (!String(hash).includes(":")) {
const defaultContainerName = getDefaultContainerName();
const legacyKey = legacyContainerKey(baseHash, defaultContainerName);
const legacyKey = legacyContainerKey(hash, defaultContainerName);
if (legacyKey) {
const legacyResult = await client.hdel("FileStoreMap", legacyKey);
if (legacyResult > 0) {
Expand Down
15 changes: 5 additions & 10 deletions helper-apps/cortex-file-handler/tests/deleteOperations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,28 +533,23 @@ test.serial("should handle backwards compatibility key removal correctly", async
t.is(uploadResponse.status, 200, "Upload should succeed");

// Manually create a legacy unscoped key to test backwards compatibility
const { setFileStoreMap, getFileStoreMap, getScopedHashKey } = await import("../src/redis.js");
const scopedHash = getScopedHashKey(testHash);
const hashResult = await getFileStoreMap(scopedHash);
const { setFileStoreMap, getFileStoreMap } = await import("../src/redis.js");
const hashResult = await getFileStoreMap(testHash);

if (hashResult) {
// Create legacy unscoped key
// Create legacy unscoped key (already exists from upload, but verify)
await setFileStoreMap(testHash, hashResult);

// Verify both keys exist
const scopedExists = await getFileStoreMap(scopedHash);
// Verify key exists
const legacyExists = await getFileStoreMap(testHash);
t.truthy(scopedExists, "Scoped key should exist");
t.truthy(legacyExists, "Legacy key should exist");

// Delete file - should remove both keys
const deleteResponse = await deleteFileByHash(testHash);
t.is(deleteResponse.status, 200, "Delete should succeed");

// Verify both keys are removed
const scopedAfter = await getFileStoreMap(scopedHash);
// Verify key is removed
const legacyAfter = await getFileStoreMap(testHash);
t.falsy(scopedAfter, "Scoped key should be removed");
t.falsy(legacyAfter, "Legacy key should be removed");
}

Expand Down
Loading