From e358a264c75e76c6b04d7478cc770c8430c0b864 Mon Sep 17 00:00:00 2001 From: lgelauff <32471480+lgelauff@users.noreply.github.com> Date: Sun, 4 Oct 2026 10:31:16 +0000 Subject: [PATCH] fix(meta): show resource names, not config values; logs/feel for maintainers only /meta and /meta/json/ are public. clastic's resource section shows the first 70 characters of repr() of each app resource, which for 'config' showed the start of the config dict to anyone. The resource section now lists each resource by name and type only (ResourceNamesPeripheral); the rest of /meta (start time, routes, host, process) is unchanged, so deploy.sh's smoke test keeps working. GET /v1/logs/feel (the front-end error log tail: usernames and error payloads) had no permission check; it now requires a maintainer, like /logs/api and /logs/api_exc. Co-Authored-By: Claude Opus 5.5 --- montage/app.py | 23 ++++++++++++++++++++++- montage/meta_endpoints.py | 6 ++++-- montage/tests/test_web_basic.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/montage/app.py b/montage/app.py index 3219be11..ff76d148 100644 --- a/montage/app.py +++ b/montage/app.py @@ -5,6 +5,7 @@ import logging from clastic import Application, StaticFileRoute, MetaApplication +from clastic.meta import DEFAULT_PERIPHERALS, ResourcePeripheral from clastic.static import StaticApplication from clastic.middleware import HTTPCacheMiddleware @@ -46,6 +47,26 @@ 'templates') +class ResourceNamesPeripheral(ResourcePeripheral): + """/meta's resource section with names and types only. clastic's + default shows the first 70 characters of each resource's repr(), + which for 'config' showed config values to anyone.""" + + def get_context(self, _application): + resources = [] + for key, val in _application.resources.items(): + desc = type(val).__name__ + if isinstance(val, (dict, list, tuple, set)): + desc += ' (%s items)' % len(val) + resources.append({'key': key, 'value': desc}) + return {'resources': resources} + + +META_PERIPHERALS = [ResourceNamesPeripheral() + if isinstance(peri, ResourcePeripheral) else peri + for peri in DEFAULT_PERIPHERALS] + + def set_mysql_session_charset_and_collation(connection, branch): # https://dev.mysql.com/doc/refman/8.0/en/set-names.html connection.execute("SET NAMES 'utf8mb4' COLLATE 'utf8mb4_unicode_ci'") @@ -197,7 +218,7 @@ def get_engine(): ('/', static_app), ('/', ui_app), ('/v1/', api_app), - ('/meta', MetaApplication())], + ('/meta', MetaApplication(base_peripherals=META_PERIPHERALS))], resources={'config': config}, middlewares=root_mws) diff --git a/montage/meta_endpoints.py b/montage/meta_endpoints.py index 73880f98..7d510274 100644 --- a/montage/meta_endpoints.py +++ b/montage/meta_endpoints.py @@ -127,8 +127,10 @@ def post_frontend_error_log(user, config, request_dict): return -def get_frontend_error_log(config, request_dict): - # TODO +def get_frontend_error_log(config, user, request_dict): + # usernames and error payloads: maintainers only, like the other logs + if not user or not user.is_maintainer: + raise Forbidden() request_dict = request_dict or dict() count = int(request_dict.get('count', DEFAULT_LINE_COUNT)) feel_path = config.get('feel_log_path', None) diff --git a/montage/tests/test_web_basic.py b/montage/tests/test_web_basic.py index eb7bd913..11e4597b 100644 --- a/montage/tests/test_web_basic.py +++ b/montage/tests/test_web_basic.py @@ -822,6 +822,39 @@ def test_home_client(base_client, api_client, mock_external_apis): #resp = base_client.fetch('public: logout', '/logout') +def test_meta_shows_no_config_values(montage_app, base_client): + """/meta and /meta/json/ are public; they list the app's resources by + name, but must not show config values (clastic's default showed the + first 70 characters of repr(config)).""" + config = montage_app.resources['config'] + values = [config.get(key) for key in ('cookie_secret', 'db_url', + 'oauth_client_id', + 'oauth_client_secret')] + values = [v for v in values if isinstance(v, str) and len(v) >= 6] + assert values # the devtest config has some of these + + for url in ('/meta/', '/meta/json/'): + resp = base_client._test_client.get(url) + assert resp.status_code == 200 + body = resp.get_data(as_text=True) + assert 'config' in body + for value in values: + assert value not in body, '%s shows a config value' % url + assert "{'" not in body.replace(''', "'") + + +def test_frontend_error_log_maintainers_only(montage_app, api_client, + monkeypatch): + from montage import rdb + # every test user is a maintainer; make LilyOfTheWest a plain coordinator + monkeypatch.setattr(rdb, 'MAINTAINERS', + [m for m in rdb.MAINTAINERS if m != 'LilyOfTheWest']) + + api_client.fetch('coordinator: read frontend error log', '/logs/feel', + as_user='LilyOfTheWest', error_code=403) + api_client.fetch('maintainer: read frontend error log', '/logs/feel') + + def test_multiple_jurors(api_client, mock_external_apis): # This is copied from above. What's the best way to break up the tests into # various stages? Should I use a pytest.fixture?