Skip to content

Commit a39e42e

Browse files
ekanshulclaude
andcommitted
Make explicitly-set REDASH_DISABLE_PUBLIC_URLS win over stored org settings
disable_public_urls is not editable in the admin UI, but the General Settings form posts back every value it received from GET /api/settings/organization, so the env-derived value gets persisted into organizations.settings. From then on the stored copy shadowed the environment variable, and flipping REDASH_DISABLE_PUBLIC_URLS had no effect. Give the environment variable precedence in Organization.get_setting(), but only when it is explicitly present in the environment. When the variable is unset, stored values keep working exactly as before, so API consumers that toggle the setting at runtime (e.g. the Cypress sharing specs) are unaffected, and the settings endpoint's persistence and audit behavior is unchanged. Fixes #7630 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ca79fe9 commit a39e42e

3 files changed

Lines changed: 38 additions & 1 deletion

File tree

redash/models/organizations.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from sqlalchemy.orm.attributes import flag_modified
33
from sqlalchemy_utils.models import generic_repr
44

5+
from redash.settings.organization import env_overrides as org_settings_env_overrides
56
from redash.settings.organization import settings as org_settings
67

78
from .base import Column, db, primary_key
@@ -66,7 +67,7 @@ def set_setting(self, key, value):
6667
flag_modified(self, "settings")
6768

6869
def get_setting(self, key, raise_on_missing=True):
69-
if key in self.settings.get("settings", {}):
70+
if key in self.settings.get("settings", {}) and key not in org_settings_env_overrides:
7071
return self.settings["settings"][key]
7172

7273
if key in org_settings:

redash/settings/organization.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,13 @@
8181
"hide_plotly_mode_bar": HIDE_PLOTLY_MODE_BAR,
8282
"disable_public_urls": DISABLE_PUBLIC_URLS,
8383
}
84+
85+
# Settings whose environment variable, when explicitly set, takes precedence over
86+
# a value stored in the organizations table. disable_public_urls is not editable
87+
# from the admin UI, but the admin settings form posts back every setting it
88+
# receives, so an env-derived value can end up persisted in the database; without
89+
# this override, that stored copy would silently shadow the environment variable
90+
# forever (#7630). When the environment variable is not set, the stored value
91+
# keeps working as before.
92+
_ENV_OVERRIDABLE = {"disable_public_urls": "REDASH_DISABLE_PUBLIC_URLS"}
93+
env_overrides = frozenset(key for key, var in _ENV_OVERRIDABLE.items() if var in os.environ)

tests/handlers/test_settings.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,34 @@
1+
from unittest import mock
2+
13
from redash.models import Organization
24
from tests import BaseTestCase
35

46

57
class TestOrganizationSettings(BaseTestCase):
8+
def test_env_override_wins_over_stored_value(self):
9+
# When REDASH_DISABLE_PUBLIC_URLS is explicitly set, a value persisted
10+
# into the organizations table by an older version must not shadow it (#7630).
11+
self.factory.org.settings["settings"] = {"disable_public_urls": True}
12+
13+
with mock.patch(
14+
"redash.models.organizations.org_settings_env_overrides",
15+
frozenset(["disable_public_urls"]),
16+
):
17+
self.assertEqual(self.factory.org.get_setting("disable_public_urls"), False)
18+
19+
def test_stored_value_applies_without_env_override(self):
20+
# Without the environment variable set, the stored value keeps working
21+
# (e.g. the Cypress sharing specs toggle it through the API).
22+
admin = self.factory.create_admin()
23+
rv = self.make_request(
24+
"post",
25+
"/api/settings/organization",
26+
data={"disable_public_urls": True},
27+
user=admin,
28+
)
29+
self.assertEqual(rv.status_code, 200)
30+
self.assertEqual(self.factory.org.get_setting("disable_public_urls"), True)
31+
632
def test_post(self):
733
admin = self.factory.create_admin()
834
rv = self.make_request(

0 commit comments

Comments
 (0)