GH-46: Syncdb with #42 by zHelmet · Pull Request #50 · wunderio/ddev-wunderio-drupal

zHelmet · 2026-03-09T06:33:33Z

Overview

Use direct call for the docker to prevent In some rare occasions calling yq from ddev alias does something to params #42, and avoid using eval on any scripts to make it more secure
Add --skip-hooks option as used in ddev to provide way to manually enter deploy commands
Remove unnecessary confirmation prompts since the command is deliberately invoked by the user.
Ensure DDEV_APPROOT is set for any downstream consumers.

…arams, reduce container calls

Remove unnecessary confirmation prompts since the command is deliberately invoked by the user. Build the import-db command as an array instead of using eval, and pass --skip-hooks flag through to ddev import-db when requested. Remove post-run hints that are redundant when hooks handle deployment steps.

tormi

Thank you, Santeri! I think I'll add --deploy hook in #47 next.

tormi · 2026-03-09T07:10:48Z


 # Sanitize imported database (remove sensitive data).
 drush sqlsan -y || { display_error_message "Database sanitization failed"; exit 1; }
+drush deploy -y || { display_error_message "Drupal deploy failed"; exit 1; }


This shouldn't be on by default. Let's introduce --deploy flag instead with drush deploy and drush uli included.

With the --skip-hooks these are not executed so no need for flag on it, i rather would keep this since on a normal workflow i want to have dev at the latest configs

95% of time I also need to import configs and update db right after import so I would enable deploy by default too. In rare cases I need to debug production db locally, there's the --skip-hooks

Only thing against deploy is the the fact that it relies on drush version. It seems in rare cases running clear cache first can cause db errors and that's done in 12.x
https://www.drush.org/12.x/deploycommand/
In newer versions config import is first:
https://www.drush.org/14.x/deploycommand/

If we can't agree on this here, then maybe meeting is needed.

tormi · 2026-03-10T07:06:37Z

-# --- 6. Read remote alias details from Drush ---
-if ! alias_details=$(ddev drush sa "$SITE_ALIAS" 2>&1); then
+# --- 5. Read remote alias details from Drush ---
+if ! alias_details=$(ddev drush sa "$SITE_ALIAS" --format=yaml 2>&1); then


tormi · 2026-03-10T07:09:17Z

-fi
-
-# --- 4. Prepare dumps directory ---
+# --- 3. Prepare dumps directory ---


Dropping a database should always be accompanied by a warning, I would keep it and the --force flag.

Warning is okay but extra steps for just to import is kinda tedious. If that would be needed i suggest you promote this feature to be added upstream to ddev

Ok, let's remove --force. I'll commit the change.

We could still add warning right at the start of the command for people who are not familiar with this command or if somebody accidentally ran it. It could be in red. They still have time to cancel because db is removed after import is done.

@tormi @zHelmet

tormi · 2026-03-10T07:12:34Z

        PROJECT_ROOT="$PWD"
    fi
+    # Ensure DDEV_APPROOT is set for any downstream consumers.
+    DDEV_APPROOT="$PROJECT_ROOT"


tormi · 2026-03-10T07:12:51Z

  ddev syncdb <alias>                # e.g. ddev syncdb prod
  ddev syncdb prod --backup          # Back up local DB before overwriting
-  ddev syncdb prod --force            # Skip confirmation prompt
+  ddev syncdb prod --skip-hooks       # Skip ddev hooks


tormi · 2026-03-24T11:10:43Z

Thanks again for the work on this!

I still have concerns about the --skip-hooks flag, including from a GDPR/data safety perspective.

The issue

--skip-hooks bypasses both drush sqlsan AND drush deploy. This means developers can accidentally import production data with real PII (Personally Identifiable Information):

ddev syncdb prod --skip-hooks  # No sanitization!

The local database would then contain:

Real customer emails and names (users_field_data)
Session tokens (potential for session hijacking)
Form submissions with contact details
Any custom PII fields

If a developer's laptop is lost, stolen, or compromised - that's a GDPR violation.

Additionally, the current approach doesn't support a common use case: running sanitization WITHOUT deployment. With --skip-hooks, it's all-or-nothing - you either get both sqlsan + deploy, or neither.

Safer alternative

This PR is based on my GH-46 branch (PR #47), which intentionally takes a different approach:

Aspect	This PR #50 (`--skip-hooks`)	PR #47 GH-46 (`--deploy`)
Sanitization	Opt-out (risky)	Always runs via DDEV hooks
Deployment	Opt-out	Opt-in with `--deploy`
Sanitize without deploy	Not possible	Default behavior
GDPR safety	Developer must remember	Safe by default

# PR #47 (GH-46) approach
ddev syncdb prod           # imports + sanitizes (safe default, no deploy)
ddev syncdb prod --deploy  # imports + sanitizes + deploys

Suggestion

Could we consider merging the improvements from this PR (alias parsing, error handling) back into PR #47, but:

Remove --skip-hooks from syncdb
Keep --deploy as opt-in

Happy to discuss further!

zHelmet · 2026-03-24T12:10:09Z

Remove --skip-hooks from syncdb

Skip hooks has much higher value since there even might be some project specific hooks to run which are not always wanted. I would rather take this another way. Add sanitation into syncdb script and keep the --skip-hooks

tormi · 2026-03-24T13:57:37Z

So --skip-hooks would just be a ddev flag without any custom logic? 👍 then.

zHelmet added 5 commits February 25, 2026 09:08

In some rare occasions calling yq from ddev alias does something to p…

1479a5c

…arams, reduce container calls

Merge #47

30e3571

Merge #43

fa08d07

Merge branch GH-46 into bugfix/GH-46-syncdb

7ad3149

zHelmet changed the title ~~GH-46: Syncdb~~ GH-46: Syncdb with #42 Mar 9, 2026

zHelmet marked this pull request as ready for review March 9, 2026 06:44

zHelmet mentioned this pull request Mar 9, 2026

In some rare occasions calling yq from ddev alias does something to params #42

Closed

tormi reviewed Mar 10, 2026

View reviewed changes

zHelmet added 2 commits March 17, 2026 15:43

Merge branch GH-46 into bugfix/GH-46-syncdb

1748924

Restore syncdb

90c2eb8

zHelmet added 2 commits March 24, 2026 14:05

Make sure that sanitization is always ran

84d3a6d

Prefix drush command

45cfe3a

Remove private repo mention

d86be36

zHelmet merged commit 39f0e36 into GH-46 May 18, 2026

Conversation

zHelmet commented Mar 9, 2026

Overview

Uh oh!

tormi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zHelmet Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tormi commented Mar 24, 2026

The issue

Safer alternative

Suggestion

Uh oh!

zHelmet commented Mar 24, 2026

Uh oh!

tormi commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tormi left a comment •

edited

Loading

zHelmet Mar 10, 2026 •

edited

Loading