[SahinurDEV]

Description

Fixes #11301.

The Storage emulator evaluates rules against the object name from the request, but it stored and looked up objects under path.join(bucket, object). path.join collapses . and .. segments, so users/A/../B/photo.png was authorized as a path under users/A/ and then read, overwrote or deleted users/B/photo.png. Cloud Storage keeps object names literal, so those are two different objects there, and rules tests against the emulator could pass or fail for the wrong reason.

StorageLayer now keys objects by ${bucket}/${object} with the name unchanged. Bucket names can't contain /, so the key is still unique per bucket and object. The same key is used for uploads, reads, deletes, copies and export/import, so rules and storage agree on the name. (On Windows path.join also turned \ in a name into a separator, so a\b and a/b collided; that goes away too.)

The one place that relied on the normalized key was import: older exports made on Windows stored names with \ separators (see scripts/storage-emulator-integration/import/windows-emulator-data), and the key was rebuilt with the platform separator. Import now maps those \ separators to /, so those exports still load as before.

Also added a CHANGELOG entry.

Scenarios Tested

New unit tests in src/emulator/storage/files.spec.ts:

• uploading users/A/../B/photo.png doesn't overwrite users/B/photo.png, and both read back their own bytes
• reading users/A/../B/photo.png or users/B/./photo.png returns not found instead of users/B/photo.png
• deleting users/A/../B/photo.png doesn't delete users/B/photo.png
• copying to users/A/../B/photo.png doesn't overwrite users/B/photo.png
• both objects stay distinct through export and import
• importing the existing Windows export fixture still serves test-directory/test_nested_upload.jpg and test_upload.jpg

The five dot-segment tests fail on main and pass with this change; the Windows import test passes on both.

• npx mocha 'src/emulator/**/*.spec.ts': 601 passing, 1 pending
• npx mocha 'src/**/*.spec.{ts,js}': 5884 passing; the only failure is utils datetimeString, which fails the same way on main because my machine's timezone is UTC+6
• tsc --project tsconfig.compile.json, eslint (no new warnings) and prettier are clean

I didn't run the storage emulator integration suite (scripts/storage-emulator-integration) locally.

Sample Commands

N/A

[gemini-code-assist]

Code Review

This pull request fixes an issue in the Storage emulator where object names containing . or .. segments were stored under their normalized names, potentially leading to unauthorized reads, overwrites, or deletions of other objects. This is resolved by avoiding path.join (which collapses these segments) and instead using direct string interpolation for paths. Additionally, the import logic for older Windows-exported paths has been simplified. Feedback on the changes includes addressing a regex bug in the getPathSep helper function and ensuring that temporary directories created during the new export/import tests are properly cleaned up using rmSync in a try...finally block.

[gemini-code-assist]

The helper function getPathSep has a regex bug where the character class [\/|\\\\] includes a literal | character (which is treated as a literal character rather than a logical OR inside []). This means any filename containing a | character would have it incorrectly identified as a path separator. Since getPathSep is only used here, we can avoid using it entirely and match the first path separator correctly using a standard regex match.

Suggested change

	let fileName = metadata.name;
	const objectNameSep = getPathSep(fileName);
	// Replace all file separators with that of current platform for compatibility
	if (fileName !== path.sep) {
	fileName = fileName.split(objectNameSep).join(path.sep);
	if (getPathSep(fileName) === "\\") {
	fileName = fileName.split("\\").join("/");
	}
	let fileName = metadata.name;
	const firstSep = fileName.match(/[\/\\]/)?.[0];
	if (firstSep === "\\\\") {
	fileName = fileName.split("\\\\").join("/");
	}

[SahinurDEV]

Good catch, thanks. getPathSep was existing code, but this PR relies on its result, so a name like a|b\c.txt from an old Windows export was seen as using | and its backslashes were not mapped to /. In 78daee2 I removed getPathSep (this was its only caller) and check the first separator directly with /[/\\]/.exec(fileName)?.[0] === "\\". I compared against a single \ rather than the suggested "\\\\": the match is always one character, so a two-backslash string would never equal it and the Windows mapping would stop working. I also added a test that imports a|b\c.txt and reads it back as a|b/c.txt.

[gemini-code-assist]

Import rmSync from the fs module to allow cleaning up temporary directories created during tests.

Suggested change

	import { mkdtempSync } from "fs";
	import { mkdtempSync, rmSync } from "fs";

[SahinurDEV]

Thanks, done in 78daee2: the spec now imports rmSync and uses it to delete the temporary directories.

[gemini-code-assist]

The temporary directories created with mkdtempSync are never cleaned up, which can clutter the system's temporary folder over time. Wrapping the test logic in a try...finally block ensures that these directories are always deleted, even if the test assertions fail.

      it("should keep the objects apart through export and import", async () => {
        const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR);
        await uploadDecoded(storageLayer, "bucket", "users/B/photo.png", "from B");
        await uploadDecoded(storageLayer, "bucket", "users/A/../B/photo.png", "from A");
        const exportDir = mkdtempSync(path.join(tmpdir(), "storage-export-"));
        const importDir = mkdtempSync(path.join(tmpdir(), "storage-import-"));
        try {
          await storageLayer.export(exportDir, { initiatedBy: "test" });

          const imported = new StorageLayer(
            "project",
            new Map(),
            new Map(),
            ALWAYS_TRUE_RULES_VALIDATOR,
            ALWAYS_TRUE_ADMIN_CREDENTIAL_VALIDATOR,
            new Persistence(importDir),
            new StorageCloudFunctions("project"),
          );
          imported.import(exportDir, { initiatedBy: "test" });

          expect(await readData(imported, "users/B/photo.png")).to.equal("from B");
          expect(await readData(imported, "users/A/../B/photo.png")).to.equal("from A");
        } finally {
          rmSync(exportDir, { recursive: true, force: true });
          rmSync(importDir, { recursive: true, force: true });
        }
      });

[SahinurDEV]

Thanks, done in 78daee2: the export/import test now creates both temp directories up front and deletes them in a finally, so they get cleaned up even when an assertion fails. The new | import test cleans up its export directory the same way.