diff --git a/actions/azurestorage.js b/actions/azurestorage.js index c4fe4db9..341db6fb 100644 --- a/actions/azurestorage.js +++ b/actions/azurestorage.js @@ -525,6 +525,31 @@ export const deleteBlob = async (containerName, blobName) => { return { "deleted": blobName }; }; +const isSafeDeletionPathSegment = (value) => + typeof value === "string" && + value.length > 0 && + value.length <= 255 && + value === value.trim() && + value !== "." && + value !== ".." && + !/[\\/\u0000-\u001f\u007f]/.test(value); + +const buildDeletionFolderPrefix = (resourcePath, expectedSegmentCount) => { + if (typeof resourcePath !== "string") { + throw new Error("Invalid deletion resource path"); + } + + const segments = resourcePath.split("/"); + if ( + segments.length !== expectedSegmentCount || + !segments.every(isSafeDeletionPathSegment) + ) { + throw new Error("Invalid deletion resource path"); + } + + return segments.join("/") + "/"; +}; + export const deleteBlobCase = async (containerName, blobName) => { const creds = new DefaultAzureCredential(); @@ -538,10 +563,12 @@ export const deleteBlobCase = async (containerName, blobName) => { //conLog"deleteBlobCase "); - console.log("blob to delete:", blobName); + const folderPrefix = buildDeletionFolderPrefix(blobName, 1); + + console.log("blob to delete:", folderPrefix); for await (const blob of containerClient.listBlobsFlat({ - prefix: blobName + prefix: folderPrefix })) { console.log(" ------ :", blob.name); containerClient.deleteBlob(blob.name); @@ -566,10 +593,12 @@ export const deleteBlobRep = async (containerName, blobName) => { const sasUrl = `${STORAGE_PATH}/${containerName}?${containerToken}`; const containerClient = new ContainerClient(sasUrl); - console.log("blob to delete:", blobName); + const folderPrefix = buildDeletionFolderPrefix(blobName, 2); + + console.log("blob to delete:", folderPrefix); for await (const blob of containerClient.listBlobsFlat({ - prefix: blobName + prefix: folderPrefix })) { console.log("------ :", blob.name); const blockBlobClient = containerClient.getBlockBlobClient(blob.name); diff --git a/pages/api/file/deleteblobcase.js b/pages/api/file/deleteblobcase.js index 002068a4..8fbf9408 100644 --- a/pages/api/file/deleteblobcase.js +++ b/pages/api/file/deleteblobcase.js @@ -17,6 +17,15 @@ const forbidden = (res) => message: "Request not permitted" }); +const isValidPathSegment = (value) => + typeof value === "string" && + value.length > 0 && + value.length <= 255 && + value === value.trim() && + value !== "." && + value !== ".." && + !/[\\/\u0000-\u001f\u007f]/.test(value); + ApiProxy.get(async (req, res) => { const containerName = req.query.container; const casefolderID = req.query.casefolderID; @@ -39,6 +48,14 @@ ApiProxy.get(async (req, res) => { const casefolderIDTrimmed = casefolderID.trim(); + if (!isValidPathSegment(casefolderIDTrimmed)) { + return respondError(res, { + status: 400, + code: "INVALID_REQUEST", + message: "Invalid request" + }); + } + const session = await getServerSession(req, res, authOptions(req, res)); if ( typeof session?.user?.id !== "string" || diff --git a/pages/api/file/deleteblobrep.js b/pages/api/file/deleteblobrep.js index d6c7f3f0..c129444b 100644 --- a/pages/api/file/deleteblobrep.js +++ b/pages/api/file/deleteblobrep.js @@ -17,6 +17,15 @@ const forbidden = (res) => message: "Request not permitted" }); +const isValidPathSegment = (value) => + typeof value === "string" && + value.length > 0 && + value.length <= 255 && + value === value.trim() && + value !== "." && + value !== ".." && + !/[\\/\u0000-\u001f\u007f]/.test(value); + ApiProxy.get(async (req, res) => { const containerName = req.query.container; const casefolderID = req.query.casefolderID; @@ -43,6 +52,17 @@ ApiProxy.get(async (req, res) => { const casefolderIDTrimmed = casefolderID.trim(); const repfileTrimmed = repfile.trim(); + if ( + !isValidPathSegment(casefolderIDTrimmed) || + !isValidPathSegment(repfileTrimmed) + ) { + return respondError(res, { + status: 400, + code: "INVALID_REQUEST", + message: "Invalid request" + }); + } + const session = await getServerSession(req, res, authOptions(req, res)); if ( typeof session?.user?.id !== "string" || diff --git a/tests/phase21/file-handler-contract.test.cjs b/tests/phase21/file-handler-contract.test.cjs index 53fcc5cc..66caa1f7 100644 --- a/tests/phase21/file-handler-contract.test.cjs +++ b/tests/phase21/file-handler-contract.test.cjs @@ -338,7 +338,7 @@ test("deleteblob handler dependency failure returns DELETE_BLOB_FAILED", async ( assert.strictEqual(res.state.jsonBody.error.code, "DELETE_BLOB_FAILED"); }); -test("deleteblobcase handler accepts encoded hash variant", async () => { +test("deleteblobcase handler accepts a valid case folder", async () => { let deletedCaseFolder = null; const mod = loadModule("pages/api/file/deleteblobcase.js", { nextConnect: createNextConnectMock(), @@ -346,7 +346,7 @@ test("deleteblobcase handler accepts encoded hash variant", async () => { getServerSession: async () => ({ user: { id: "c1" } }), authOptions: () => ({}), hashAPIPath: (queryPath) => - queryPath.includes("casefolderID=CASE%2FSUB") + queryPath.includes("casefolderID=CASE-SUB") ? "&hash=expected" : "&hash=other", respondError: respondErrorMock, @@ -360,7 +360,7 @@ test("deleteblobcase handler accepts encoded hash variant", async () => { const req = { query: { container: "c1", - casefolderID: "CASE/SUB", + casefolderID: "CASE-SUB", hash: "expected" } }; @@ -368,7 +368,41 @@ test("deleteblobcase handler accepts encoded hash variant", async () => { await mod.default.handler(req, res); assert.strictEqual(res.state.statusCode, 200); - assert.strictEqual(deletedCaseFolder, "CASE/SUB"); + assert.strictEqual(deletedCaseFolder, "CASE-SUB"); +}); + +test("deleteblobcase rejects unsafe normalized folder identifiers before deletion", async () => { + for (const casefolderID of [" ", "CASE-1/REP-1", "..", "CASE\\REP"]) { + let deletionCalls = 0; + const mod = loadModule("pages/api/file/deleteblobcase.js", { + nextConnect: createNextConnectMock(), + middleware: () => {}, + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), + hashAPIPath: () => "&hash=expected", + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + deleteBlobCase: async () => { + deletionCalls += 1; + } + }); + const res = createRes(); + + await mod.default.handler( + { + query: { + container: "c1", + casefolderID, + hash: "expected" + } + }, + res + ); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual(res.state.jsonBody.error.code, "INVALID_REQUEST"); + assert.strictEqual(deletionCalls, 0); + } }); test("deleteblobcase handler dependency failure returns DELETE_BLOB_CASE_FAILED", async () => { @@ -436,6 +470,46 @@ test("deleteblobrep handler accepts encoded hash variant", async () => { assert.strictEqual(deletedRepPath, "CASE-1/Rep Name"); }); +test("deleteblobrep rejects missing or unsafe folder segments before deletion", async () => { + for (const invalidQuery of [ + { casefolderID: " ", repfile: "REP-1" }, + { casefolderID: "CASE-1", repfile: " " }, + { casefolderID: "CASE-1/REP-1", repfile: "REP-1" }, + { casefolderID: "CASE-1", repfile: "REP-1/FILES" }, + { casefolderID: "CASE-1", repfile: ".." } + ]) { + let deletionCalls = 0; + const mod = loadModule("pages/api/file/deleteblobrep.js", { + nextConnect: createNextConnectMock(), + middleware: () => {}, + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), + hashAPIPath: () => "&hash=expected", + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + deleteBlobRep: async () => { + deletionCalls += 1; + } + }); + const res = createRes(); + + await mod.default.handler( + { + query: { + container: "c1", + hash: "expected", + ...invalidQuery + } + }, + res + ); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual(res.state.jsonBody.error.code, "INVALID_REQUEST"); + assert.strictEqual(deletionCalls, 0); + } +}); + test("deleteblobrep handler dependency failure returns DELETE_BLOB_REP_FAILED", async () => { const mod = loadModule("pages/api/file/deleteblobrep.js", { nextConnect: createNextConnectMock(), diff --git a/tests/phase22/azurestorage-helper-behaviour.test.cjs b/tests/phase22/azurestorage-helper-behaviour.test.cjs index e9f68049..3e32cf73 100644 --- a/tests/phase22/azurestorage-helper-behaviour.test.cjs +++ b/tests/phase22/azurestorage-helper-behaviour.test.cjs @@ -33,6 +33,65 @@ const loadAzureStorageHelperModule = (injected = {}) => { return context.module.exports; }; +const loadRecursiveDeletionHelpers = ({ blobNames = [] } = {}) => { + const filePath = path.join(rootDir, "actions", "azurestorage.js"); + const source = fs.readFileSync(filePath, "utf8"); + const start = source.indexOf("const isSafeDeletionPathSegment ="); + const end = source.indexOf("export const uploadFile ="); + + if (start < 0 || end < 0 || end <= start) { + throw new Error("Unable to isolate recursive deletion helper block"); + } + + const listedPrefixes = []; + const deletedBlobNames = []; + const containerClient = { + async *listBlobsFlat({ prefix }) { + listedPrefixes.push(prefix); + for (const name of blobNames.filter((blobName) => + blobName.startsWith(prefix) + )) { + yield { name }; + } + }, + deleteBlob(name) { + deletedBlobNames.push(name); + return Promise.resolve(); + }, + getBlockBlobClient() { + return { exists: async () => true }; + } + }; + + let helperSource = source + .slice(start, end) + .replace(/export const /g, "const "); + helperSource += + "\nmodule.exports = { deleteBlobCase, deleteBlobRep };\n"; + + const context = { + module: { exports: {} }, + exports: {}, + console: { log: () => {} }, + DefaultAzureCredential: class {}, + createContainerSas: async () => "sas-token", + STORAGE_PATH: "https://storage.example.test", + ContainerClient: class { + constructor() { + return containerClient; + } + } + }; + + vm.runInNewContext(helperSource, context, { filename: filePath }); + + return { + ...context.module.exports, + listedPrefixes, + deletedBlobNames + }; +}; + const tests = []; const test = (name, fn) => tests.push({ name, fn }); @@ -103,6 +162,63 @@ test("azurestorage helper builds hash metadata map with stable keys", () => { }); }); +test("case deletion uses a complete folder prefix and excludes lexical siblings", async () => { + const loaded = loadRecursiveDeletionHelpers({ + blobNames: [ + "TMP-ABC/TMP-ABC_appeal.json", + "TMP-ABC/files/evidence.pdf", + "TMP-ABC1/TMP-ABC1_appeal.json", + "TMP-ABCD/files/sibling.pdf" + ] + }); + + await loaded.deleteBlobCase("container-1", "TMP-ABC"); + + assert.deepStrictEqual(loaded.listedPrefixes, ["TMP-ABC/"]); + assert.deepStrictEqual(loaded.deletedBlobNames, [ + "TMP-ABC/TMP-ABC_appeal.json", + "TMP-ABC/files/evidence.pdf" + ]); +}); + +test("representation deletion uses a complete folder prefix and preserves parent and siblings", async () => { + const loaded = loadRecursiveDeletionHelpers({ + blobNames: [ + "CASE-1/CASE-1_case.json", + "CASE-1/REP-1/REP-1_rep.json", + "CASE-1/REP-1/files/evidence.pdf", + "CASE-1/REP-10/REP-10_rep.json", + "CASE-1/REP-2/REP-2_rep.json" + ] + }); + + await loaded.deleteBlobRep("container-1", "CASE-1/REP-1"); + + assert.deepStrictEqual(loaded.listedPrefixes, ["CASE-1/REP-1/"]); + assert.deepStrictEqual(loaded.deletedBlobNames, [ + "CASE-1/REP-1/REP-1_rep.json", + "CASE-1/REP-1/files/evidence.pdf" + ]); +}); + +test("recursive deletion helpers reject empty and unsafe resource paths before listing", async () => { + for (const [helperName, unsafePath] of [ + ["deleteBlobCase", ""], + ["deleteBlobCase", "CASE-1/REP-1"], + ["deleteBlobRep", "CASE-1"], + ["deleteBlobRep", "CASE-1/REP-1/FILES"], + ["deleteBlobRep", "CASE-1/../REP-1"] + ]) { + const loaded = loadRecursiveDeletionHelpers(); + + await assert.rejects( + () => loaded[helperName]("container-1", unsafePath), + /Invalid deletion resource path/ + ); + assert.deepStrictEqual(loaded.listedPrefixes, []); + } +}); + const run = async () => { let passed = 0; for (const currentTest of tests) {