diff --git a/src/web_interface/error_handler.py b/src/web_interface/error_handler.py index ea6423a4..706bd747 100644 --- a/src/web_interface/error_handler.py +++ b/src/web_interface/error_handler.py @@ -77,6 +77,25 @@ def describe_exception(exc: BaseException, """ message = str(exc).strip() text = f"{type(exc).__name__}: {message}" if message else type(exc).__name__ + return redact_text(text, max_length) + + +def redact_text(text: str, max_length: int = _MAX_DETAIL_LENGTH) -> str: + """Make arbitrary text safe to hand back over HTTP. + + Split out of describe_exception because exceptions are not the only thing + worth returning: a subprocess's stderr, or a message a helper script + printed, is just as useful to a user and just as capable of carrying a + token or a password in it. + + Args: + text: The text to redact + max_length: Truncate beyond this many characters + + Returns: + A single line, credentials replaced, length capped. + """ + text = text or '' # Order matters: the URL and header forms are more specific than the # generic key=value pattern, which would otherwise chew the scheme. text = _REDACT_URL_USERINFO.sub(r'\1\3', text) diff --git a/test/web_interface/test_calendar_oauth_endpoints.py b/test/web_interface/test_calendar_oauth_endpoints.py index c170fa8b..a5d17646 100644 --- a/test/web_interface/test_calendar_oauth_endpoints.py +++ b/test/web_interface/test_calendar_oauth_endpoints.py @@ -189,10 +189,21 @@ class TestListingShape: import types fake_pickle = types.SimpleNamespace(load=lambda f: creds, dump=lambda *a: None) - fake_build = lambda *a, **k: types.SimpleNamespace( - calendarList=lambda: types.SimpleNamespace( - list=lambda: types.SimpleNamespace( - execute=lambda: {'items': items}))) + pages = items if isinstance(items, list) and items and isinstance(items[0], dict) \ + else items + if isinstance(pages, list): + pages = [{'items': pages}] + + state = {'i': 0} + + def fake_list(**kwargs): + page = pages[min(state['i'], len(pages) - 1)] + state['i'] += 1 + return types.SimpleNamespace(execute=lambda: page) + + def fake_build(*args, **kwargs): + return types.SimpleNamespace( + calendarList=lambda: types.SimpleNamespace(list=fake_list)) real_import = __builtins__['__import__'] if isinstance(__builtins__, dict) \ else __builtins__.__import__ @@ -239,3 +250,95 @@ class TestListingShape: self._authenticate(client, monkeypatch, [{'summary': 'ghost'}, {'id': 'real@x'}]) body = client.get('/api/v3/plugins/calendar/list-calendars').get_json() assert [c['id'] for c in body['calendars']] == ['real@x'] + + +class TestPagination: + """calendarList.list pages at 250 and defaults to 100.""" + + def _paged(self, client, monkeypatch, pages): + import types + creds = type('C', (), {'expired': False, 'refresh_token': None, 'valid': True})() + (client.plugin_dir / 'token.pickle').write_bytes(b'x') + state = {'i': 0} + seen = [] + + def fake_list(**kwargs): + seen.append(kwargs) + page = pages[min(state['i'], len(pages) - 1)] + state['i'] += 1 + return types.SimpleNamespace(execute=lambda: page) + + def fake_build(*args, **kwargs): + return types.SimpleNamespace( + calendarList=lambda: types.SimpleNamespace(list=fake_list)) + + real_import = __builtins__['__import__'] if isinstance(__builtins__, dict) \ + else __builtins__.__import__ + + def fake_import(name, *args, **kwargs): + if name == 'pickle': + return types.SimpleNamespace(load=lambda f: creds, dump=lambda *a: None) + if name == 'google.auth.transport.requests': + return types.SimpleNamespace(Request=object) + if name == 'googleapiclient.discovery': + return types.SimpleNamespace(build=fake_build) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr('builtins.__import__', fake_import) + return seen + + def test_every_page_is_collected(self, client, monkeypatch): + # Taking only the first page would hide calendars from the picker with + # nothing to say the list was cut short. + self._paged(client, monkeypatch, [ + {'items': [{'id': 'a@x', 'summary': 'A'}], 'nextPageToken': 't1'}, + {'items': [{'id': 'b@x', 'summary': 'B'}], 'nextPageToken': 't2'}, + {'items': [{'id': 'c@x', 'summary': 'C'}]}, + ]) + body = client.get('/api/v3/plugins/calendar/list-calendars').get_json() + assert [c['id'] for c in body['calendars']] == ['a@x', 'b@x', 'c@x'] + + def test_the_page_token_is_passed_back(self, client, monkeypatch): + seen = self._paged(client, monkeypatch, [ + {'items': [{'id': 'a@x', 'summary': 'A'}], 'nextPageToken': 'tok'}, + {'items': [{'id': 'b@x', 'summary': 'B'}]}, + ]) + client.get('/api/v3/plugins/calendar/list-calendars') + assert seen[0]['pageToken'] is None + assert seen[1]['pageToken'] == 'tok' + assert all(k['maxResults'] == 250 for k in seen) + + def test_a_looping_token_cannot_spin_forever(self, client, monkeypatch): + # Every page claims another follows. + self._paged(client, monkeypatch, [ + {'items': [{'id': 'a@x', 'summary': 'A'}], 'nextPageToken': 'same'}, + ]) + body = client.get('/api/v3/plugins/calendar/list-calendars').get_json() + assert body['status'] == 'success' + assert len(body['calendars']) <= mod._CALENDAR_LIST_MAX_PAGES + + +class TestDiagnosticsAreRedacted: + def test_script_stderr_is_redacted_on_the_way_out(self, tmp_path): + script = tmp_path / 'calendar_registration.py' + script.write_text( + 'import sys\n' + 'sys.stderr.write("boom client_secret=hunter2 more\\n")\n', + encoding='utf-8') + payload, error = mod._run_calendar_registration(tmp_path, '') + assert payload is None + assert 'hunter2' not in error, error + assert '' in error, error + + def test_a_failing_script_payload_is_redacted(self, client): + (client.plugin_dir / 'credentials.json').write_text('{}', encoding='utf-8') + (client.plugin_dir / 'calendar_registration.py').write_text( + 'import json\n' + 'print(json.dumps({"status": "error", ' + '"message": "Failed: client_secret=topsecret"}))\n', + encoding='utf-8') + body = client.post('/api/v3/plugins/calendar/authenticate', + json={}).get_json() + assert body['status'] == 'error' + assert 'topsecret' not in json.dumps(body), body + assert '' in body['message'], body diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index 5043d840..b4375bf7 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -22,7 +22,7 @@ logger = logging.getLogger(__name__) from src.web_interface.api_helpers import success_response, error_response, validate_request_json from src.web_interface.errors import ErrorCode from src.web_interface.secret_helpers import find_secret_fields, separate_secrets -from src.web_interface.error_handler import describe_exception +from src.web_interface.error_handler import describe_exception, redact_text from src.plugin_system.operation_types import OperationType from src.web_interface.validators import ( validate_file_upload @@ -7312,6 +7312,11 @@ def upload_calendar_credentials(): logger.error('Error in upload_calendar_credentials', exc_info=True) return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 +# calendarList.list pages at 250 entries maximum. Ten pages is far past any +# real account and exists only so a malformed nextPageToken cannot spin here. +_CALENDAR_LIST_MAX_PAGES = 10 + + def _calendar_plugin_dir() -> Optional[Path]: """Where the calendar plugin is installed, or None if it is not.""" if api_v3.plugin_manager: @@ -7363,9 +7368,15 @@ def _run_calendar_registration(plugin_dir: Path, stdin_payload: str): if isinstance(payload, dict): return payload, None - detail = (result.stderr or result.stdout or '').strip()[:300] + raw = (result.stderr or result.stdout or '').strip() + # The unredacted text goes to the log, where it is worth having in full. + # What comes back over HTTP is redacted: this is a script that handles + # OAuth client secrets, and its stderr can quote them. + if raw: + logger.error('calendar_registration.py failed (exit %s): %s', + result.returncode, raw) return None, 'Authentication script produced no result%s' % ( - ': %s' % detail if detail else '') + ': %s' % redact_text(raw) if raw else '') @api_v3.route('/plugins/calendar/authenticate', methods=['POST']) @@ -7404,8 +7415,14 @@ def authenticate_calendar(): return jsonify({'status': 'error', 'message': error}), 500 if payload.get('status') != 'success': # The script's own diagnosis is more useful than anything that - # could be reconstructed here. - return jsonify(payload), 400 + # could be reconstructed here -- but it interpolates exceptions + # into its messages, so it reaches the client redacted and the + # original goes to the log. + logger.error('calendar authentication failed: %s', payload) + safe = dict(payload) + safe['message'] = redact_text(str(payload.get('message', '') + or 'Authentication failed')) + return jsonify(safe), 400 return jsonify(payload) except Exception as e: @@ -7469,7 +7486,26 @@ def list_calendar_calendars(): }), 400 service = build_google_service('calendar', 'v3', credentials=creds) - entries = service.calendarList().list().execute().get('items', []) + + # calendarList.list returns 100 entries per page by default and caps at + # 250, handing back a nextPageToken when there are more. Taking only + # the first page would silently hide calendars from the picker, and the + # user would have no way to tell the list was truncated. + entries = [] + page_token = None + for _ in range(_CALENDAR_LIST_MAX_PAGES): + response = service.calendarList().list( + maxResults=250, pageToken=page_token).execute() + entries.extend(response.get('items', [])) + page_token = response.get('nextPageToken') + if not page_token: + break + else: + # 2500 calendars in, something is wrong with the account or the + # token is looping; show what was collected rather than spin. + logger.warning( + 'calendarList paging stopped at %d pages with more remaining', + _CALENDAR_LIST_MAX_PAGES) calendars = [{ 'id': entry.get('id'),