-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
where: { shop: undefined }
: The entire table data was accidentally deleted
#20169
Comments
Hey, thanks for the report, we are aware of this issue and I fully agree with you that it can be dangerous. We should improve it. |
Thank you @millsp, maybe updateMany operation has a similar issue that I haven't tested, and if so, I'd like to see it solved together |
From our point of view this is not surprising and a "bug we have to fix", but how Prisma is currently designed to work: https://www.prisma.io/docs/concepts/components/prisma-client/null-and-undefined We know this can be dangerous, but it has been like this for quite some while and switching would also have other deficits. So as soon as we find the proper issue that requests us to change this, we will close this issue here as a duplicate. |
where: { shop: undefined }
: The entire table data was accidentally deleted
This is extremely dangerous, I do not think it's being treated as such. You are offering an abstraction that does the opposite of what the SQL standard does (and the majority of your users understand SQL before they understand Prisma). When filtering by emptiness ( I fear this is one famous case of mass-deletion away from a PR disaster for Prisma. I am a happy user of the library, mind you, but I'd recommend more urgency to this issue. I spent 2 minutes googling, and I already found a case of someone who had to suffer from this issue: https://www.farmin.dev/posts/35ad668d-29ae-4d86-8b4a-126b6d561aaa. |
Dude I just shot myself on the foot right now. As a Prisma advocate, my spirit is in SHAMBLES right this moment. |
I don't care if breaking change were to be introduced, this evil shall be corrected. |
Apparently if the data we are querying is expected to be // this will return null back to us
const user = await prisma.user.findFirst({
where: {
// let's use an empty string instead and prisma will use it in the SQL query!
id: somethingNull?.id ?? '',
}
}) OP's issue can be avoided by doing the same thing |
We unfortunately ran into this issue (luckily had snapshots we could restore), and I had to write some middleware because this was super dangerous. I hope this can help somebody! import { isEmpty } from "lodash";
import { Prisma } from "prisma/generated";
/**
* Middleware featured below
*/
// This naming convention specifically matches the
// safe word to the exported constant to support the
// middleware error handling
export const safeWordDelete = "FORCE_DELETE_ALL";
export const FORCE_DELETE_ALL = { id: safeWordDelete };
export const safeWordFind = "FIND_ALL";
export const FIND_ALL = { id: safeWordFind };
export const safeWordUpdate = "UPDATE_ALL";
export const UPDATE_ALL = { id: safeWordUpdate };
/**
* If you have found yourself here after a hard lesson:
*
* These middleware specifically do NOT guard in the case of a nested find, update, delete.
* These middleware also don't protect in the case of an undefined variable being passed in alongside a specific condition.
*
* Ex: {
* id, // undefined
* otherAttr: null
* }
*
* The above where clause will delete/modify/find all regardless of undefined status of variable appId.
*
* The !! best !! guard here is proper type adherence - do not bang, do not coerce.
*/
const _guardEnMasse = ({
params,
actions,
safeWord,
}: {
params: Prisma.MiddlewareParams;
actions: Prisma.MiddlewareParams["action"][];
safeWord: string;
}) => {
// Check if empty, if not and safeWord is provided, then set where = {} and proceed to execute
if (actions.includes(params.action)) {
// Don't allow delete all queries
if (isEmpty(params.args.where)) {
throw new Error(
`It looks like you just tried to perform a ${params.action} on all of the ${params.model}s. If this was intentional, pass 'where: ${safeWord}'`,
);
}
}
if (params.args?.where?.id === safeWord) {
params.args.where = {};
}
};
export const guardEnMasse: PrismaMiddleware = async (params, next) => {
/* DELETION PROTECTION MIDDLEWARE */
_guardEnMasse({
actions: ["delete", "deleteMany"],
params,
safeWord: safeWordDelete,
});
/* MASS FIND PROTECTION MIDDLEWARE */
_guardEnMasse({
actions: ["findMany"],
params,
safeWord: safeWordFind,
});
/* MASS UPDATE PROTECTION MIDDLEWARE */
_guardEnMasse({
actions: ["updateMany"],
params,
safeWord: safeWordUpdate,
});
return next(params);
}; and then const dbClient = new PrismaClient();
dbClient.$use(guardEnMasse);
const whoops = undefined;
// Throws an error
dbClient.sensitiveData.deleteMany({ where: { id: whoops } });
// Deletes everything
dbClient.insensitiveData.deleteMany({ where: FORCE_DELETE_ALL }); Best guard here is strict type safety and attribute validation ahead of db writes. Also, always always always backup your production database! |
Thank you @jakezegil for sharing how to add runtime safety to these queries, but I wondered if more could be done to prevent this sort of issue at compile time since all our infrastructure would be happy shipping Heavily influenced by @kz-d's https://github.com/kz-d/prisma-where-required generator, I wrote the following generator which 1) makes the argument to all the bulk operations required and then 2) makes the Combining the buildtime and runtime checks provides a degree of safety I feel more comfortable with. Setup is the same as in generator.ts #!/usr/bin/env node
import { generatorHandler } from "@prisma/generator-helper";
import { logger } from "@prisma/internals";
import { convert } from "./convertor";
generatorHandler({
onManifest: () => ({
prettyName: "where-required",
requiresGenerators: ["prisma-client-js"],
defaultOutput: ".",
}),
onGenerate: async (options) => {
const nodeModulePath = options.generator.config.nodeModulePath;
if (!nodeModulePath) {
logger.error("No nodeModulePath config.");
return;
}
const debug = options.generator.config.debug === "true";
const models = options.dmmf.datamodel.models;
convert({
path: `${nodeModulePath}/.prisma/client/index.d.ts`,
models: models.map((model) => model.name),
debug,
});
},
}); convertor.ts import { Project, SourceFile, ts } from "ts-morph";
import isPropertySignature = ts.isPropertySignature;
function removeQuestionTokenFromWhereFieldProperty(
sourceFile: SourceFile,
models: string[]
) {
const prismaNamespace = sourceFile.getModuleOrThrow("Prisma");
let isInTypeAlias = false;
prismaNamespace.transform((tr) => {
const currentNode = tr.currentNode;
if (
ts.isTypeAliasDeclaration(currentNode) &&
models.some(
(targetModel) =>
currentNode.name.getText().startsWith(targetModel) &&
!currentNode.name.getText().startsWith(`${targetModel}$`) &&
(currentNode.name.getText().endsWith("FindManyArgs") ||
currentNode.name.getText().endsWith("DeleteManyArgs") ||
currentNode.name.getText().endsWith("DeleteArgs") ||
currentNode.name.getText().endsWith("UpdateManyArgs"))
)
) {
isInTypeAlias = true;
const newNode = tr.visitChildren();
isInTypeAlias = false;
return newNode;
}
if (
isInTypeAlias &&
isPropertySignature(currentNode) &&
currentNode.name.getText() === "where"
) {
isInTypeAlias = false;
return tr.factory.updatePropertySignature(
currentNode,
currentNode.modifiers,
currentNode.name,
undefined,
currentNode.type
);
}
return tr.visitChildren();
});
}
function removeQuestionTokenFromActionFunctionProperty(
sourceFile: SourceFile,
models: string[]
) {
const prismaNamespace = sourceFile.getModuleOrThrow("Prisma");
let isInDelegate = false;
let isInFindMany = false;
prismaNamespace.transform((tr) => {
const currentNode = tr.currentNode;
if (
ts.isInterfaceDeclaration(currentNode) &&
models.some(
(targetModel) => currentNode.name.getText() === `${targetModel}Delegate`
)
) {
isInDelegate = true;
const newNode = tr.visitChildren();
isInDelegate = false;
return newNode;
}
if (
isInDelegate &&
ts.isMethodSignature(currentNode) &&
(currentNode.name.getText() === "findMany" ||
currentNode.name.getText() === "deleteMany" ||
currentNode.name.getText() === "delete" ||
currentNode.name.getText() === "updateMany")
) {
isInFindMany = true;
const newNode = tr.visitChildren();
isInFindMany = false;
return newNode;
}
if (isInFindMany && ts.isParameter(currentNode)) {
isInDelegate = false;
isInFindMany = false;
return tr.factory.updateParameterDeclaration(
currentNode,
currentNode.modifiers,
currentNode.dotDotDotToken,
currentNode.name,
undefined,
currentNode.type,
currentNode.initializer
);
}
return tr.visitChildren();
});
}
export function convert(arguments_: {
path: string;
models: string[];
debug: boolean;
}) {
const project = new Project();
const sourceFile = project.addSourceFileAtPath(arguments_.path);
removeQuestionTokenFromWhereFieldProperty(sourceFile, arguments_.models);
removeQuestionTokenFromActionFunctionProperty(sourceFile, arguments_.models);
sourceFile.saveSync();
} (I'll confess I found the ts-morph transform visitor difficult to reason about, there might be a more efficient way to write that loop) this added <5 seconds to the so then // Compile error (missing an arg)
dbClient.sensitiveData.deleteMany()
// Compile error (missing a where)
dbClient.sensitiveData.findMany({select: {...}})
// Runtime error (as above)
dbClient.sensitiveData.findMany({ where: {id: undefined}})
// Success via runtime override with maximal clarity on what's happening
dbClient.sensitiveData.findMany({ where: FIND_ALL }) |
Bug description
I left out the shop parameter check, causing the shop to be undefined in some cases, triggering the deletion of the entire table. This operation of Prisma is very risky, I hope to improve it.
The text was updated successfully, but these errors were encountered: