Skip to content

fix: avoid mutating caller/up-return objects with version key before validation #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ig-imanish wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: avoid mutating caller/up-return objects with version key before validation #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 63 additions & 0 deletions src/migrate-async.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,66 @@ test("migrateAsync: null state handling", async (t) => {

t.deepEqual(result, { _version: 1, initialized: true });
});

test("migrateAsync: schema validation error does not mutate up() return value or input state", async (t) => {
const upReturned = { name: "ab" }; // will fail .min(5)

const migrations = [
{
version: 1,
schema: z.object({ name: z.string().min(5) }),
up: async () => upReturned,
},
] as const;

const originalInput = { name: "start" };

t.false("_version" in upReturned);
t.false("_version" in originalInput);

const error = await t.throwsAsync(
() =>
migrateAsync({
state: originalInput,
migrations,
}),
{ instanceOf: ValidationError },
);

t.is(error?.version, 1);

t.false(
"_version" in upReturned,
"up() return value must not be mutated with version key on validation failure (async)",
);
t.false(
"_version" in originalInput,
"input state must not be mutated when a migration fails validation (async)",
);
});

test("migrateAsync: schema validation error does not mutate when up() returns same ref", async (t) => {
const originalState = { name: "start" };

const migrations = [
{
version: 1,
schema: z.object({ name: z.string().min(5), extra: z.boolean() }),
up: async (state: Record<string, unknown>) => {
(state as Record<string, unknown>).mutatedField = true;
return state;
},
},
] as const;

t.false("_version" in originalState);

await t.throwsAsync(() => migrateAsync({ state: originalState, migrations }), {
instanceOf: ValidationError,
});

t.false(
"_version" in originalState,
"original state must not receive version key on validation failure even if up returned same ref (async)",
);
});
69 changes: 69 additions & 0 deletions src/migrate.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,72 @@ test("complex schema transformations", (t) => {
users: [{ id: 1, name: "Alice", createdAt: "2024-01-01" }],
});
});

test("schema validation error does not mutate up() return value or input state", (t) => {
const upReturned = { name: "ab" }; // will fail .min(5)

const migrations = [
{
version: 1,
schema: z.object({ name: z.string().min(5) }),
up: () => upReturned,
},
] as const;

const originalInput = { name: "start" };

t.false("_version" in upReturned);
t.false("_version" in originalInput);

const error = t.throws(
() =>
migrate({
state: originalInput,
migrations,
}),
{ instanceOf: ValidationError },
);

t.is(error?.version, 1);

// Critical: no version key should have been written before/during failed validation
t.false(
"_version" in upReturned,
"up() return value must not be mutated with version key on validation failure",
);
t.false(
"_version" in originalInput,
"input state must not be mutated when a later migration fails validation",
);
t.is((upReturned as Record<string, unknown>)._version, undefined);
t.is((originalInput as Record<string, unknown>)._version, undefined);
});

test("schema validation error does not mutate when up() returns same ref as currentState", (t) => {
const originalState = { name: "start" };

const migrations = [
{
version: 1,
schema: z.object({ name: z.string().min(5), extra: z.boolean() }), // missing extra -> fail
up: (state: Record<string, unknown>) => {
// simulate a mutating up that returns same reference
(state as Record<string, unknown>).mutatedField = true;
return state;
},
},
] as const;

t.false("_version" in originalState);

t.throws(() => migrate({ state: originalState, migrations }), {
instanceOf: ValidationError,
});

t.false(
"_version" in originalState,
"original state must not be dirtied with version key when up() returned same ref and validation failed",
);
t.is((originalState as Record<string, unknown>)._version, undefined);
t.true((originalState as Record<string, unknown>).mutatedField); // other mutations from up() itself are user's responsibility
});
40 changes: 34 additions & 6 deletions src/migrate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,31 @@ export function migrate<
throw new MigrationError(migration.version, error);
}

// Add version to result
// Prepare a value for validation by injecting the version key *without*
// mutating the object returned by `up()` (or any shared reference such as
// a previous currentState). The schemaWithVersion requires the literal
// version, so we must provide it for validation. Using a shallow copy
// ensures that on ValidationError the caller's objects remain untouched,
// and the version key is only present on successfully validated outputs.
let toValidate: unknown = result;
if (result !== null && typeof result === "object") {
(result as Record<string, unknown>)[key] = migration.version;
if (Array.isArray(result)) {
toValidate = [...result];
(toValidate as Record<string, unknown>)[key] = migration.version;
} else {
toValidate = {
...(result as Record<string, unknown>),
[key]: migration.version,
};
}
}

// Validate against schema with version field
const schemaWithVersion = migration.schema.and(
z.object({ [key]: z.literal(migration.version) }),
);

const parseResult = schemaWithVersion.safeParse(result);
const parseResult = schemaWithVersion.safeParse(toValidate);
if (!parseResult.success) {
throw new ValidationError(migration.version, parseResult.error.issues);
}
Expand Down Expand Up @@ -195,17 +209,31 @@ export async function migrateAsync<
throw new MigrationError(migration.version, error);
}

// Add version to result
// Prepare a value for validation by injecting the version key *without*
// mutating the object returned by `up()` (or any shared reference such as
// a previous currentState). The schemaWithVersion requires the literal
// version, so we must provide it for validation. Using a shallow copy
// ensures that on ValidationError the caller's objects remain untouched,
// and the version key is only present on successfully validated outputs.
let toValidate: unknown = result;
if (result !== null && typeof result === "object") {
(result as Record<string, unknown>)[key] = migration.version;
if (Array.isArray(result)) {
toValidate = [...result];
(toValidate as Record<string, unknown>)[key] = migration.version;
} else {
toValidate = {
...(result as Record<string, unknown>),
[key]: migration.version,
};
}
}

// Validate against schema with version field
const schemaWithVersion = migration.schema.and(
z.object({ [key]: z.literal(migration.version) }),
);

const parseResult = schemaWithVersion.safeParse(result);
const parseResult = schemaWithVersion.safeParse(toValidate);
if (!parseResult.success) {
throw new ValidationError(migration.version, parseResult.error.issues);
}
Expand Down