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
43 changes: 43 additions & 0 deletions packages/seed/src/__test__/validateFixtureOutputFolders.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { FernSeedConfig } from "../config/index.js";
import { validateFixtureOutputFolders } from "../validateFixtureOutputFolders.js";

function workspaceConfig(
fixtures: Record<string, FernSeedConfig.FixtureConfigurations[]>
): FernSeedConfig.SeedWorkspaceConfiguration {
return { fixtures } as FernSeedConfig.SeedWorkspaceConfiguration;
}

describe("validateFixtureOutputFolders", () => {
it("allows a single configuration writing to the fixture root", () => {
const errors = validateFixtureOutputFolders({
workspaceName: "go-sdk",
workspaceConfig: workspaceConfig({ streaming: [{ outputFolder: "." }] })
});
expect(errors).toEqual([]);
});

it("allows multiple configurations in distinct output folders", () => {
const errors = validateFixtureOutputFolders({
workspaceName: "go-sdk",
workspaceConfig: workspaceConfig({
"idempotency-headers": [
{ outputFolder: "no-custom-config" },
{ outputFolder: "auto-generate-idempotency-key" }
]
})
});
expect(errors).toEqual([]);
});

it("rejects a configuration writing to the fixture root alongside a nested configuration", () => {
const errors = validateFixtureOutputFolders({
workspaceName: "go-sdk",
workspaceConfig: workspaceConfig({
"idempotency-headers": [{ outputFolder: "." }, { outputFolder: "auto-generate-idempotency-key" }]
})
});
expect(errors).toHaveLength(1);
expect(errors[0]).toContain('fixture "idempotency-headers"');
expect(errors[0]).toContain('"auto-generate-idempotency-key"');
});
});
5 changes: 5 additions & 0 deletions packages/seed/src/loadGeneratorWorkspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { access, readdir, readFile } from "fs/promises";
import yaml from "js-yaml";

import { FernSeedConfig } from "./config/index.js";
import { validateFixtureOutputFolders } from "./validateFixtureOutputFolders.js";

export interface GeneratorWorkspace {
workspaceName: string;
Expand Down Expand Up @@ -59,6 +60,10 @@ export async function loadGeneratorWorkspaces(): Promise<GeneratorWorkspace[]> {
CONSOLE_LOGGER.warn(`Skipping ${workspace}: disabled in ${SEED_CONFIG_FILENAME}`);
continue;
}
const errors = validateFixtureOutputFolders({ workspaceName: workspace, workspaceConfig });
if (errors.length > 0) {
throw new Error(`Invalid ${SEED_CONFIG_FILENAME}:\n${errors.join("\n")}`);
}
Comment on lines +63 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 suggestion

loadGeneratorWorkspaces is called by every seed command, so one malformed seed.yml in an unrelated generator now hard-fails all of them. Consider collecting errors across workspaces and throwing once at the end (so users see every problem), or warning + skipping like the disabled branch above.

workspaces.push({
absolutePathToWorkspace,
workspaceConfig,
Expand Down
29 changes: 29 additions & 0 deletions packages/seed/src/validateFixtureOutputFolders.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { FernSeedConfig } from "./config/index.js";

/**
* Generation wipes a fixture's output folder before writing files, so a configuration
* that writes to the fixture root ("." ) destroys the output of any sibling configuration
* nested underneath it.
*/
export function validateFixtureOutputFolders({
workspaceName,
workspaceConfig
}: {
workspaceName: string;
workspaceConfig: FernSeedConfig.SeedWorkspaceConfiguration;
}): string[] {
const errors: string[] = [];
for (const [fixture, configurations] of Object.entries(workspaceConfig.fixtures ?? {})) {
const outputFolders = configurations.map((configuration) => configuration.outputFolder);
if (outputFolders.length > 1 && outputFolders.includes(".")) {
Comment on lines +17 to +18

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 suggestion

The check only catches the literal ".". It misses "./", "", and the more general nesting case (outputFolder: a + outputFolder: a/b), which has exactly the same rm -rf hazard. Consider normalizing with path.normalize and comparing prefixes:

Suggested change
const outputFolders = configurations.map((configuration) => configuration.outputFolder);
if (outputFolders.length > 1 && outputFolders.includes(".")) {
const normalized = outputFolders.map((f) => path.normalize(f ?? ".").replace(/\/+$/, ""));
const conflicts = normalized.filter((folder, i) =>
normalized.some((other, j) => i !== j && (other === folder || other.startsWith(`${folder}/`)))
);
if (conflicts.length > 0) {

(and adjust the message body accordingly). At minimum, normalize "./""." so the guard can't be trivially bypassed.

errors.push(
`${workspaceName}: fixture "${fixture}" has an outputFolder of "." alongside ${outputFolders
.filter((outputFolder) => outputFolder !== ".")
.map((outputFolder) => `"${outputFolder}"`)
.join(", ")}. ` +
`Give every configuration its own output folder so they do not overwrite each other.`
);
}
}
return errors;
}
2 changes: 1 addition & 1 deletion seed/go-sdk/seed.yml
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ fixtures:
- outputFolder: no-custom-config
customConfig: null
idempotency-headers:
- outputFolder: .
- outputFolder: no-custom-config
outputVersion: 0.0.1
customConfig:
packageName: fern
Expand Down
Loading