Page MenuHomePhabricator

Raise a setup warning when locked configuration has a configuration value stored in the database
ClosedPublic

Authored by epriestley on Feb 13 2019, 2:45 PM.
Tags
None
Referenced Files
F18764607: D20159.id48138.diff
Tue, Oct 7, 6:51 AM
F18749358: D20159.id.diff
Sat, Oct 4, 3:32 AM
F18709697: D20159.id48146.diff
Mon, Sep 29, 12:32 AM
F18703328: D20159.diff
Sun, Sep 28, 3:40 AM
F18634424: D20159.diff
Sep 16 2025, 10:17 PM
F18388562: D20159.id.diff
Aug 29 2025, 4:33 AM
F18366845: D20159.id48146.diff
Aug 27 2025, 10:50 PM
F18365923: D20159.id48146.diff
Aug 27 2025, 9:36 PM
Subscribers
None

Details

Summary

Ref T13249. See https://discourse.phabricator-community.org/t/configuring-the-number-of-taskmaster-daemons/2394/.

Today, when a configuration value is "locked", we prevent writes to the database. However, we still perform reads. When you upgrade, we generally don't want a bunch of your configuration to change by surprise.

Some day, I'd like to stop reading locked configuration from the database. This would defuse an escalation where an attacker finds a way to write to locked configuration despite safeguards, e.g. through SQL injection or policy bypass. Today, they could write to cluster.mailers or similar and substantially escalate access. A better behavior would be to ignore database values for cluster.mailers and other locked config, so that these impermissible writes have no effect.

Doing this today would break a lot of installs, but we can warn them about it now and then make the change at a later date.

Test Plan
  • Forced a phd.taskmasters config value into the database.
  • Saw setup warning.
  • Used bin/config delete --database phd.taskmasters to clear the warning.
  • Reviewed documentation changes.
  • Reviewed phd.taskmasters documentation adjustment.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
158

"configuration option"

This revision is now accepted and ready to land.Feb 13 2019, 6:45 PM
This revision was automatically updated to reflect the committed changes.
src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
158

🕶