ardalis / ardalis/CleanArchitecture
⚠️ SeedData may cause unintended deletion of production data if ToDoList is not used
- Dominant language
- C#
- Stars
- 18.5k
- Forks
- 3.1k
- PR merge metrics
- No merged PRs in 30d
Description
Hi and thanks for maintaining this great template, it's very useful to explore clean architecture in .NET.
I’d like to raise awareness of a potential data loss risk in SeedData.cs that could arise in some specific conditions, especially when developers use the sample as a base for real applications.
🔍 **Context**
In [SeedData.cs](https://github.com/ardalis/CleanArchitecture/blob/ec2d7fc4a62bb7e8e42a946f633644d9cf583a7b/sample/src/NimblePros.SampleToDo.Web/SeedData.cs#L38), the following logic is used:
``` csharp
public static async Task InitializeAsync(AppDbContext dbContext)
{
if (await dbContext.ToDoItems.AnyAsync()) // DANGEROUS CHECK !!!
{
return;
}
// if dbContext.ToDoItems is emptied or removed, potential risk to loose all data !!!
await PopulateTestDataAsync(dbContext);
}
public static async Task PopulateTestDataAsync(AppDbContext dbContext)
{
foreach (var item in dbContext.Projects)
{
dbContext.Remove(item);
}
foreach (var item in dbContext.ToDoItems)
{
dbContext.Remove(item);
}
foreach (var item in dbContext.Contributors)
{
dbContext.Remove(item);
}
// etc etc other production tables could be added here
await dbContext.SaveChangesAsync();
```
⚠️ Potential Risk
This behavior may cause accidental data loss in production in the following (not unrealistic) situation:
- A production application uses the template but do not remove ToDoList, while not using it, and keeping the seeding mechanism in place (for seeding other data).
- A DBA or ops team **removes or cleans** the unused ToDoLists table in production to clean up the schema.
- A local/**dev** version of the application connects to the **production** DB (by mistake or for diagnostics using prod connection string).
To note that table removal in the DB will not generate exception, as we have in the MiddlewareConfig the `EnsureCreatedAsync` so the eventually removed table is _recreated_.
```
await context.Database.EnsureCreatedAsync();
await SeedData.InitializeAsync(context);
```
So, while removing or truncating ToDoItems, the condition `!context.ToDoLists.Any()` would be true, and the deletion logic before that would wipe all data from ToDoItems, ToDoLists etc etc, even if other parts of the application used them or the model was extended.
✅ **Suggestion**
It might be safer to:
- Do not delete data.
- Or provide a warning/comment in the template that this logic is only meant for sample/dev environments and should never be deployed as-is, however, there is always a risk the developers don't real the full code or all comments.
Again, this isn’t a bug per se, and certainly not a mistake from your side, but a small design choice in the sample code that might have big consequences in derived applications if developers aren’t careful.
Thanks again for your work on this project!
Contributor guide
Research direction
Start with sample/src/NimblePros.SampleToDo.Web/SeedData.cs, especially InitializeAsync and PopulateTestDataAsync, then read MiddlewareConfig’s EnsureCreatedAsync call. Decide on safe seeding behavior when ToDoItems is empty or recreated, and verify the startup path no longer risks deleting existing application data when the sample table is absent or unused.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100