Skip to content

fix: keep tenant config out of config.php - #3

Open
YvesCesar wants to merge 15 commits into
mainfrom
fix/prevent-tenant-config-leak
Open

fix: keep tenant config out of config.php#3
YvesCesar wants to merge 15 commits into
mainfrom
fix/prevent-tenant-config-leak

Conversation

@YvesCesar

@YvesCesar YvesCesar commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Problem

On a multi-tenant instance, any system config write flattened the current tenant's values into the instance-wide config.php, permanently.

OC\Config::readData() merges every config/*.config.php into $this->cache without recording where each key came from, and writeData() dumps that whole cache with var_export(). So a single occ config:system:set foo bar served
under a tenant host also rewrote mail_smtphost, trusted_domains and every other key the tenant defined — as global config.

Writing config is not something an admin has to ask for: an upgrade alone writes config.php about six times (Updater sets maintenance, loglevel, installed, version), and maintenance:repair, the admin mail form and background jobs such as updatenotification/ResetToken do the same. A cron running under one tenant was enough to overwrite the base config for everyone.

What changes

The loader body moves to src/loader.php, included from config/multitenancy.config.php inside OC\Config::readData(), so it runs in the config class scope ($this is the OC\Config instance).

Tenant values are injected into $this->envCache — the channel behind the NC_* environment variables. Nextcloud reads it with priority and writeData() never persists it, so tenant config cannot reach config.php at all. Unlike real NC_* variables, which getenv() only delivers as strings, this channel accepts arrays and booleans.

Array values are merged over the base with array_replace_recursive, the same rule Nextcloud applies to every *.config.php. Lists merge by index, so tenant trusted_domains must be written in full.

@YvesCesar
YvesCesar marked this pull request as draft August 20, 2026 12:18
Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
…channel

Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
It inferred admin intent by diffing files after the fact, but OC\Config::set()
and delete() decide against writing by comparing to the instance-wide cache,
so a tenant write can leave no trace on disk for the reconciler to find.
Reconciliation also used the merged cache as its baseline, which corrupted the
matrix on a plain read when another *.config.php defined the same key.

Tenant config stays read-only from the module: writes are matrix edits.

Signed-off-by: Yves César <yvesamorim73@gmail.com>
Reading straight from envCache returned the tenant value whole, so a tenant
overriding redis.port lost redis.host and the connection broke. Restores the
array_replace_recursive semantics tenants had before the channel switch.

Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
Signed-off-by: Yves César <yvesamorim73@gmail.com>
@YvesCesar
YvesCesar force-pushed the fix/prevent-tenant-config-leak branch from 0e858d2 to b5f9356 Compare August 20, 2026 13:01
Signed-off-by: Yves César <yvesamorim73@gmail.com>
@YvesCesar
YvesCesar force-pushed the fix/prevent-tenant-config-leak branch from da01c62 to f140a75 Compare August 21, 2026 15:55
Comment thread examples/multitenancy.config.php Outdated
@YvesCesar YvesCesar changed the title Fix/prevent tenant config leak fix: keep tenant config out of config.php Aug 25, 2026
…ixed path

Signed-off-by: Yves César <yvesamorim73@gmail.com>
@YvesCesar
YvesCesar marked this pull request as ready for review August 25, 2026 21:58
@YvesCesar
YvesCesar requested a review from vitormattos August 25, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants