DEV Community

Cover image for PrestaShop's Configuration table is global, and uninstalling your module can delete someone else's settings

PrestaShop's Configuration table is global, and uninstalling your module can delete someone else's settings

Configuration::updateValue('width', 40) looks harmless. It is one row in ps_configuration, it holds your module's setting, and it works.

It works until a second module does the same thing. ps_configuration is a shop-wide key/value table with no namespace and no owner column. Two modules that pick the same name share one row: whichever saves last wins, and neither has any way to notice. Then one of them is uninstalled, runs its tidy-up, and deletes it:

public function uninstall()
{
    Configuration::deleteByName('width');   // whose width?
    return parent::uninstall();
}
Enter fullscreen mode Exit fullscreen mode

That is the bug we found in one of our own modules. It stored theme, background, width and effect as bare names. Fixing that one module was easy. The interesting part was the question it raised: how many of the others do this?

The sweep, and why the first one was wrong

We have 57 module repositories. Grepping them for Configuration:: calls produces a lot of hits, most of them fine, so the sweep needs a filter. Our first filter was:

A name that is UPPERCASE with underscores is prefixed. Anything else is suspect.

That is wrong, and it is wrong in the direction that hides bugs. It passed three names that are uppercase, underscored, and completely unprefixed:

  • API_DATE_FROM and PRODUCT_PAGE_FRONT_ENABLE — in a seller-dashboard module
  • VIDEO_PRODUCTS_NBR — in a product-video module

API_* is about as generic as a key can get. It looks disciplined. It is not.

The right test is not capitalisation, it is derivation: does the prefix come from the module's own name? CG_ for combinationgrid, CSF_ for customerservicefile, PCT_ for productcustomtab — initialisms are fine, because they derive. API_ derives from nothing.

We re-ran the sweep with that criterion. Total: five real cases across the catalogue, four found in the first pass and one found later, by accident, during unrelated work.

Not every cross-module read is a bug

The sweep surfaced plenty of modules reading keys with another module's prefix. Almost all of those were deliberate — two modules that integrate on purpose, one reading the other's setting to decide whether to render something. That is a dependency, and it should be documented, but it is not this bug.

The distinction that matters is write and delete, not read. Reading a foreign key is a coupling decision. Writing one is a collision. Deleting one on uninstall is data loss in someone else's module.

One finding was a false positive worth mentioning because of what it turned out to be. A module appeared to touch a bare key named theme. The hit was in tests/ReviewNudgeTest.php, in a fixture that plants a foreign unprefixed key and then asserts that uninstall left it alone. It was not the bug; it was the test that proves the bug is absent.

Where the risk actually sits

In all five cases, the modules' current settings were properly prefixed. Nobody was actively writing bare names. The damage was concentrated in one place: legacy cleanup in uninstall().

The pattern goes like this. Version 1.x stored show_notavailable and colorshape. Version 2.x migrated to COLORSONPRODUCTLIST_* and, being tidy, kept deleting the old names on uninstall so a reinstall would start clean. The intent is right. The effect is that uninstalling this module deletes a row that, on some other shop, belongs to a different module entirely.

The same lines had also been copied into the upgrade-2.0.0.php scripts — same bug class, second location, and easy to miss if you only audit uninstall().

The fix

Deleting your own historical leftovers is not worth risking another module's data. Two options, in order of preference:

1. Stop deleting them. The residue costs a few bytes in ps_configuration and harms nothing. This is what we did in every case.

public function uninstall()
{
    // Only this module's own prefixed keys. The bare 1.x names
    // ('show_notavailable', 'colorshape', ...) are left alone: configuration
    // is shop-wide and the module cannot prove another module does not own
    // a row by one of those names.
    foreach (array_keys(self::defaultSettings()) as $key) {
        Configuration::deleteByName($key);
    }

    return parent::uninstall();
}
Enter fullscreen mode Exit fullscreen mode

The comment is load-bearing. Without it, the next person to read this file sees dead cleanup code and helpfully restores it.

2. If you must delete, prove ownership first. Check that the stored value is a shape only your module writes. This is fragile, and it is only worth it when the leftover actually causes a problem.

Full prefix migration — read the bare key, write the prefixed one, keep the old row — is a bigger change and it is only warranted when the module is still reading bare names. If your current keys are already prefixed, the uninstall step is the whole bug.

Making it stay fixed

A grep sweep finds this once. A test keeps it found. Ours plants foreign keys and asserts uninstall left them alone:

Configuration::$store = [
    'MYMODULE_SETTING'  => '1',
    'MYMODULE_OTHER'    => 'tok',
    'theme'             => 'another-modules-value',
    'OTHERMODULE_THING' => 'another-modules-value',
];

$module->uninstall();

ok(Configuration::get('theme') === 'another-modules-value',
   'a bare foreign key was not touched');
ok(Configuration::get('OTHERMODULE_THING') === 'another-modules-value',
   'another module prefixed key was not touched');
Enter fullscreen mode Exit fullscreen mode

Configuration is a small enough surface to stub — get, updateValue, deleteByName over an array — so this runs as plain php tests/ConfigurationKeyTest.php with no PrestaShop and no database. Cheap enough that every module can carry one.

If you are running the sweep yourself, a per-module negative match is more reliable than a global positive one:

deleteByName\('(?!MYPREFIX_)
Enter fullscreen mode Exit fullscreen mode

Run it once per module with that module's own prefix substituted in. Semi-generic names like API_* slip past hand-written positive patterns; they do not slip past "anything that is not my prefix".

Why this is worth an hour of your time

The failure mode is invisible from inside either module. Module A's setting resets and its author blames a caching layer. Module B's uninstall is the cause and its author never finds out, because it happened on someone else's shop, three months later, after an unrelated support ticket.

Nothing errors. Nothing is logged. The support conversation goes nowhere, because both authors are looking at code that is correct in isolation.

Grep your own modules for deleteByName and read every line of what follows. It is a short list.


We maintain around sixty PrestaShop modules at MEG Venture. The audit above took an afternoon and closed five bugs that no customer had reported and no test could have caught, because the damage happens in someone else's code.

Top comments (0)