fix(init): Initialzer has to fail if migration is not done#13754
fix(init): Initialzer has to fail if migration is not done#13754kiblik wants to merge 1 commit intoDefectDojo:devfrom
Conversation
Signed-off-by: kiblik <[email protected]>
|
This pull request leaves the entrypoint script continuing startup even when migrations are missing — it sets a non‑zero return code but proceeds with initialization and only exits later, which can let the app run against an outdated schema and risk security or data integrity issues. The problematic logic is in docker/entrypoint-initializer.sh (lines ~135–138) where missing migrations are reported but startup is not halted immediately.
Improper Error Handling of Missing Migrations in
|
| Vulnerability | Improper Error Handling of Missing Migrations |
|---|---|
| Description | The script detects missing database migrations and sets a return code of 52. However, it explicitly states 'Continuing startup despite missing migrations...' and proceeds with further initialization steps before eventually exiting with the non-zero return code. This allows the application to attempt to start and potentially run in an inconsistent state with an outdated database schema. If recent migrations included security-critical changes (e.g., new access control mechanisms, validation constraints, or data integrity rules), their absence could lead to exploitable security vulnerabilities. |
django-DefectDojo/docker/entrypoint-initializer.sh
Lines 135 to 138 in defd3d4
All finding details can be found in the DryRun Security Dashboard.
valentijnscholten
left a comment
There was a problem hiding this comment.
I think it has always been a warning. Then I made it stricter at some point. Then during this PR I think I realized users are locked out if they make changes but can no longer start Defect Dojo anymore to run makemigrations. Now would probably be a good time to add to the DOCKER.md how users (developers) can make new migrations. I think we still need to do something like starting uwsgi and postgress with --no-deps.
I'm not sure about starting uwsgi. It easily create confusion, why service is started but failing with some unrelated error. User will see that UI is not able to access not existing field, not that he did not perform migration. What about extending |
|
We need to provide instructions to users/developers on how to create a migration. It's not clear currently. |
I have been bit by this before, and my solution was to back out my model changes, start the app, reapply changes, and then make the new migrations. I'm not quite sure how to capture that in documentation |
|
What I do is |
|
^ that is better advice! It should be included in the docs somewhere before we can move forward on this one |
|
Converting to draft for now |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Since #13169, we are avoiding failure of
docker/entrypoint-initializer.sh, which is understandable in the related context; however, it is not the best behavior. If migration is missing, we shouldexitwith some error code; otherwise, it is easy to miss bugs in implementation.