Skip to content
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- Fall back to default open rules with a warning when Storage emulator rules or targets are unconfigured.
- Added extensions replacement registry and scraper tool to track migrations for deprecated extensions ahead of the March 2027 decommission date.
- [Added] Loads existing `.env` files and passes environment variables to functions discovery in `runtimeDelegate`.
- Adds --immediate flag to ext:uninstall (#10921)
33 changes: 23 additions & 10 deletions src/emulator/storage/rules/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
describe("Storage Rules Config", () => {
const tmpDir = createTmpDir("storage-files");
const persistence = new Persistence(tmpDir);
const resolvePath = (fileName: string) => fileName;

Check warning on line 17 in src/emulator/storage/rules/config.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Missing return type on function

it("should parse rules config for single target", () => {
const rulesFile = "storage.rules";
Expand Down Expand Up @@ -92,20 +92,33 @@
expect(result[2].rules.content).to.contain("allow read, write: if request.auth!=null");
});

it("should throw FirebaseError when storage config is missing", () => {
it("should use default config when storage config is missing", () => {
const config = getOptions({ data: {}, path: resolvePath });
expect(() => getStorageRulesConfig(PROJECT_ID, config)).to.throw(
FirebaseError,
"Cannot start the Storage emulator without rules file specified in firebase.json: run 'firebase init' and set up your Storage configuration",
);
const result = getStorageRulesConfig(PROJECT_ID, config) as SourceFile;

expect(result.name).to.contain("templates/emulators/default_storage.rules");
expect(result.content).to.contain("allow read, write;");
});

it("should throw FirebaseError when rules file is missing", () => {
it("should use default config when rules file is missing", () => {
const config = getOptions({ data: { storage: {} }, path: resolvePath });
expect(() => getStorageRulesConfig(PROJECT_ID, config)).to.throw(
FirebaseError,
"Cannot start the Storage emulator without rules file specified in firebase.json: run 'firebase init' and set up your Storage configuration",
);
const result = getStorageRulesConfig(PROJECT_ID, config) as SourceFile;

expect(result.name).to.contain("templates/emulators/default_storage.rules");
expect(result.content).to.contain("allow read, write;");
});

it("should use default config when target is missing in .firebaserc", () => {
const config = getOptions({
data: {
storage: [{ target: "missing-target", rules: "main.rules" }],
},
path: resolvePath,
});
const result = getStorageRulesConfig(PROJECT_ID, config) as SourceFile;

expect(result.name).to.contain("templates/emulators/default_storage.rules");
expect(result.content).to.contain("allow read, write;");
});
Comment on lines +111 to 122

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Let's add a test case to verify that when multiple targets are configured, but only some are missing in .firebaserc, the emulator successfully parses the configured targets and skips the missing ones instead of discarding all rules.

  it("should use default config when target is missing in .firebaserc", () => {
    const config = getOptions({
      data: {
        storage: [{ target: "missing-target", rules: "main.rules" }],
      },
      path: resolvePath,
    });
    const result = getStorageRulesConfig(PROJECT_ID, config) as SourceFile;

    expect(result.name).to.contain("templates/emulators/default_storage.rules");
    expect(result.content).to.contain("allow read, write;");
  });

  it("should parse configured targets and skip missing targets in .firebaserc", () => {
    const mainRulesContent = Buffer.from(StorageRulesFiles.readWriteIfTrue.content);
    const mainRulesPath = persistence.appendBytes("storage_main.rules", mainRulesContent);

    const config = getOptions({
      data: {
        storage: [
          { target: "main", rules: mainRulesPath },
          { target: "missing-target", rules: "missing.rules" },
        ],
      },
      path: resolvePath,
    });
    config.rc.applyTarget(PROJECT_ID, "storage", "main", ["bucket_0"]);
    const result = getStorageRulesConfig(PROJECT_ID, config) as RulesConfig[];

    expect(result.length).to.equal(1);
    expect(result[0].resource).to.eql("bucket_0");
    expect(result[0].rules.name).to.equal(mainRulesPath);
  });


it("should throw FirebaseError when rules file is invalid", () => {
Expand All @@ -118,7 +131,7 @@
});
});

function getOptions(config: any): Options {

Check warning on line 134 in src/emulator/storage/rules/config.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
return {
cwd: "/",
configPath: "/",
Expand Down
46 changes: 28 additions & 18 deletions src/emulator/storage/rules/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
}

/**
* Parses rules file for each target specified in the storage config under {@link options}.

Check warning on line 17 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

The type 'options' is undefined
* @returns The rules file path if the storage config does not specify a target and an array

Check warning on line 18 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Invalid JSDoc tag (preference). Replace "returns" JSDoc tag with "return"
* of project resources and their corresponding rules files otherwise.
* @throws {FirebaseError} if storage config is missing or rules file is missing or invalid.
*/
Expand All @@ -23,52 +23,46 @@
projectId: string,
options: Options,
): SourceFile | RulesConfig[] {
const storageConfig = options.config.data.storage;

Check warning on line 26 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe member access .storage on an `any` value

Check warning on line 26 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe assignment of an `any` value
const storageLogger = EmulatorLogger.forEmulator(Emulators.STORAGE);
if (!storageConfig) {
if (Constants.isDemoProject(projectId)) {
storageLogger.logLabeled(
"BULLET",
"storage",
`Detected demo project ID "${projectId}", using a default (open) rules configuration.`,
);
return defaultStorageRules();
}
throw new FirebaseError(
"Cannot start the Storage emulator without rules file specified in firebase.json: run 'firebase init' and set up your Storage configuration",
);
logDefaultRulesWarning(projectId, storageLogger);
return defaultStorageRules();
}

// No target specified
if (!Array.isArray(storageConfig)) {
if (!storageConfig.rules) {

Check warning on line 35 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe member access .rules on an `any` value
throw new FirebaseError(
"Cannot start the Storage emulator without rules file specified in firebase.json: run 'firebase init' and set up your Storage configuration",
);
logDefaultRulesWarning(projectId, storageLogger);
return defaultStorageRules();
}

return getSourceFile(storageConfig.rules, options);

Check warning on line 40 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe member access .rules on an `any` value

Check warning on line 40 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe argument of type `any` assigned to a parameter of type `string`
}
// Multiple targets
const results: RulesConfig[] = [];
const { rc } = options;
for (const targetConfig of storageConfig) {
if (!targetConfig.target) {

Check warning on line 46 in src/emulator/storage/rules/config.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe member access .target on an `any` value
throw new FirebaseError("Must supply 'target' in Storage configuration");
}
const targets = rc.target(projectId, "storage", targetConfig.target);
if (targets.length === 0) {
// Fall back to open if this is a demo project
// Fall back to open if this is a demo project or targets are missing
if (Constants.isDemoProject(projectId)) {
storageLogger.logLabeled(
"BULLET",
"storage",
`Detected demo project ID "${projectId}", using a default (open) rules configuration. Storage targets in firebase.json will be ignored.`,
);
return defaultStorageRules();
} else {
storageLogger.logLabeled(
"WARN",
"storage",
`Storage target '${targetConfig.target}' in firebase.json is not configured in .firebaserc. The emulator will default to allowing all reads and writes.`,
);
}
// Otherwise, requireTarget will error out
rc.requireTarget(projectId, "storage", targetConfig.target);
return defaultStorageRules();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Returning defaultStorageRules() immediately here will discard any successfully parsed rules for other targets that were already processed and added to results. Instead, we should continue the loop to skip this unconfigured target, and only fall back to the default rules at the end of the function if no targets were successfully configured.

Suggested change
return defaultStorageRules();
continue;
References
  1. The style guide recommends using continue statements in loops to handle edge cases early and keep main logic flat. (link)

}
results.push(
...rc.target(projectId, "storage", targetConfig.target).map((resource: string) => {
Expand All @@ -79,6 +73,22 @@
return results;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If all targets were skipped because they were unconfigured (meaning results is empty), we should fall back to the default open rules. Otherwise, we should return the successfully parsed rules in results.

Suggested change
return results;
if (results.length === 0) {
return defaultStorageRules();
}
return results;

}

function logDefaultRulesWarning(projectId: string, storageLogger: EmulatorLogger): void {
if (Constants.isDemoProject(projectId)) {
storageLogger.logLabeled(
"BULLET",
"storage",
`Detected demo project ID "${projectId}", using a default (open) rules configuration.`,
);
} else {
storageLogger.logLabeled(
"WARN",
"storage",
"Did not find a Storage rules file specified in a firebase.json config file. The emulator will default to allowing all reads and writes. Learn more about this option: https://firebase.google.com/docs/emulator-suite/install_and_configure#security_rules_configuration.",
);
}
}

function defaultStorageRules(): SourceFile {
const defaultRulesPath = "emulators/default_storage.rules";
const name = absoluteTemplateFilePath(defaultRulesPath);
Expand Down
Loading