mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-19 01:19:07 +00:00
* fix(calendar): implement the OAuth and calendar-listing endpoints The plugin's config advertised a three-step setup and only step 1 existed. Step 3's picker fetched /api/v3/plugins/calendar/list-calendars, which was never registered, so Flask fell through to the global 404 handler and the user saw "Resource not found" -- a message that names nothing and points nowhere. Step 2 had no endpoint at all, so even a working picker would have found no token to list with. Two routes, following the pattern the spotify and ytm plugins already use for their own auth scripts: POST /plugins/calendar/authenticate two-step Google OAuth GET /plugins/calendar/list-calendars calendars for the picker The authenticate route drives calendar_registration.py, which the plugin already ships and which was written expressly for this -- it reads a redirect URL on stdin and prints one JSON object. It takes two calls because a human has to visit Google in between; the script persists the PKCE verifier from the first call for the second, without which the exchange fails with "Missing code verifier". The listing route reads the token directly rather than shelling out again: the picker is interactive and a subprocess per click is slower than the API call it would wrap. It refreshes an expired token in place, sorts the primary calendar first, and drops entries with no id, which could not be selected anyway. Both name the plugin when it is not installed, rather than reproducing the anonymous 404 that started this. Verified against the live Google API on the dev rig: HTTP 200 with the account's real calendars. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(calendar): one input for the auth code, and a louder warning Two things reported after testing the flow. There were two boxes and no way to tell which to use. The config template's string branch dispatches widgets from an allow-list of names, and anything missing from it falls through to a plain input type=text -- so the field rendered both the widget's own box and a stray one for the same key. google-oauth is now on that list, which is all the widget ever needed to render in place of the fallback rather than beside it. And the warning that the redirect page fails to load was small grey text under a link, which is where it is least likely to be read. It is now an amber callout that leads with "The next page will fail to load. That is expected." The failure lands at exactly the moment the user has to act on it, and it looks precisely like the flow breaking rather than working. The paste box is labelled too, rather than relying on a placeholder that vanishes on focus. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(calendar): redact script diagnostics, and page the calendar list Three findings from CodeRabbit, all valid. Raw subprocess output was being returned to the client -- the script's stderr on one path, and its own error payload on another. CodeQL flagged the same line. That script handles OAuth client secrets and interpolates exceptions into its messages, so either could carry a secret or a path. Both now go to the log unredacted, where they are worth having in full, and reach the client through a redactor. That redactor already existed inside describe_exception, which only takes exceptions. Split out as redact_text: an exception is not the only thing worth returning, and a subprocess's stderr is just as capable of quoting a token. calendarList.list returns 100 entries per page by default, caps at 250, and hands back a nextPageToken when there are more. Reading one page would have hidden calendars from the picker with nothing to say the list was cut short. It now pages, asking for 250 at a time, bounded at ten pages so a malformed token cannot spin. And a test helper was a lambda where ruff wants a def. The five new tests fail against the previous commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(calendar): redact the last two raw exception interpolations CodeQL flagged four exposure paths. Two were mine and genuinely raw: the OSError from failing to spawn calendar_registration.py, which carries the interpreter path and whatever the OS chose to say, and the ImportError for the Google libraries, whose message named the missing module by interpolating the exception directly. Both now go through describe_exception, and the unredacted text goes to the log. The other two are the repo-wide pattern from PR #448 -- 67 handlers on main already return details=describe_exception(e), and these two new handlers follow it. That function is the sanitizer: it strips URL userinfo, auth headers and credential-shaped key=value pairs, collapses to one line and caps the length. CodeQL's taint tracking cannot see a sanitizer it has no model for, so it reports the flow regardless. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(calendar): announce status changes, name the paste box, drop a no-op Three more from the review, all valid. The status line is written after every async call -- the consent link is ready, the exchange failed -- and was a plain paragraph, so a screen reader was told none of it. It is a live region now. The paste box had a visible label that was never associated with it, so its only accessible name was the placeholder, which disappears on focus: precisely when the value is being pasted. The label now points at the input by id. And a conditional in the test helper returned the same value from both branches, which Ruff flags as RUF034. It was left over from making the fake page; one page is all those cases need, and TestPagination builds its own sequences. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * test(calendar): assert the accessibility relationships, not their parts The previous assertions searched for role="status", aria-live, a label `for` and an input `id` independently, so they passed whether or not those belonged together. Two attributes on different elements announce nothing, and a `for` that names something other than the input leaves it just as anonymous. Both attributes are now asserted on the status element itself, and the label and input are checked to go through the same identifier rather than merely both existing. Verified by mutation: a mismatched pair and a displaced aria-live are both caught. Reported by CodeRabbit, against tests I had written two commits earlier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
175 lines
5.7 KiB
Python
175 lines
5.7 KiB
Python
"""
|
|
Centralized error handling for web interface.
|
|
|
|
Provides helpers for consistent error responses across API endpoints.
|
|
"""
|
|
|
|
import re
|
|
from typing import Any, Optional
|
|
from flask import jsonify
|
|
|
|
from src.web_interface.errors import (
|
|
WebInterfaceError, ErrorCode, ErrorCategory
|
|
)
|
|
from src.logging_config import get_logger
|
|
|
|
|
|
logger = get_logger(__name__)
|
|
|
|
|
|
# Credentials that turn up inside exception text. A requests error quotes the
|
|
# URL it failed on, and plugins that authenticate by query string put their key
|
|
# there, so echoing an exception verbatim can hand out an API key. Redact the
|
|
# value, keep the parameter name -- knowing *which* credential was involved is
|
|
# part of the diagnosis.
|
|
_REDACT_CREDENTIAL = re.compile(
|
|
r'((?:api[_-]?key|access[_-]?token|auth|apikey|key|passwd|password|pwd|'
|
|
r'secret|sig|signature|token)["\']?\s*[=:]\s*["\']?)([^\s&"\'<>,}]+)',
|
|
re.IGNORECASE,
|
|
)
|
|
|
|
# `Authorization: <scheme> <credential>`. The scheme name is kept because it
|
|
# says which kind of credential failed; the credential goes. Any scheme
|
|
# matches, not a fixed list: ApiKey, Negotiate, NTLM, AWS4-HMAC-SHA256 and
|
|
# whatever a plugin's API invents next are all credentials, and a list would
|
|
# silently leak the ones nobody thought of. Not covered by the generic pattern
|
|
# above, whose value part stops at whitespace and so would keep the credential
|
|
# once a space follows the scheme.
|
|
_REDACT_AUTH_HEADER = re.compile(
|
|
r'((?:proxy-)?authorization["\']?\s*[=:]\s*["\']?\s*'
|
|
r'(?:[A-Za-z][\w.+-]*[ \t]+)?)' # optional scheme name, kept
|
|
r'([^\s,"\'<>}]+)', # the credential, redacted
|
|
re.IGNORECASE,
|
|
)
|
|
|
|
# Credentials embedded in a URL: https://user:password@host. requests quotes
|
|
# the full URL in its exceptions, so this is a realistic leak. The username is
|
|
# kept -- it identifies which account failed without being the secret.
|
|
_REDACT_URL_USERINFO = re.compile(r'([a-z][a-z0-9+.-]*://[^/\s:@]+:)([^/\s@]+)(@)',
|
|
re.IGNORECASE)
|
|
|
|
# Long enough for an errno string with a path, short enough not to dump a
|
|
# parser's worth of context into a JSON field.
|
|
_MAX_DETAIL_LENGTH = 400
|
|
|
|
|
|
def describe_exception(exc: BaseException,
|
|
max_length: int = _MAX_DETAIL_LENGTH) -> str:
|
|
"""
|
|
One-line, safe-to-return description of an exception.
|
|
|
|
The generic "an error occurred; see logs for details" tells a user nothing
|
|
and, when the failure is bad enough, the logs are unreachable too: a device
|
|
whose storage was failing returned that message from every endpoint
|
|
*including* the log viewer, because journalctl could not be executed. The
|
|
underlying `[Errno 5] Input/output error` named the fault immediately.
|
|
|
|
Returns "TypeName: message", credentials redacted and length capped. The
|
|
type alone is worth carrying -- a bare PermissionError says more than any
|
|
generic sentence.
|
|
|
|
Args:
|
|
exc: The exception to describe
|
|
max_length: Truncate beyond this many characters
|
|
|
|
Returns:
|
|
A single-line description, never empty
|
|
"""
|
|
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<redacted>\3', text)
|
|
text = _REDACT_AUTH_HEADER.sub(r'\1<redacted>', text)
|
|
text = _REDACT_CREDENTIAL.sub(r'\1<redacted>', text)
|
|
# Collapse newlines/tabs so the detail stays one line in a JSON field.
|
|
text = ' '.join(text.split())
|
|
if len(text) > max_length:
|
|
text = text[:max_length - 1].rstrip() + '…'
|
|
return text
|
|
|
|
|
|
def create_error_response(
|
|
error_code: ErrorCode,
|
|
message: str,
|
|
details: Optional[str] = None,
|
|
context: Optional[dict] = None,
|
|
suggested_fixes: Optional[list] = None,
|
|
status_code: int = 500
|
|
) -> tuple:
|
|
"""
|
|
Create a standardized error response.
|
|
|
|
Args:
|
|
error_code: Error code
|
|
message: Error message
|
|
details: Optional detailed error information
|
|
context: Optional context dictionary
|
|
suggested_fixes: Optional list of suggested fixes
|
|
status_code: HTTP status code
|
|
|
|
Returns:
|
|
Tuple of (jsonify response, status_code)
|
|
"""
|
|
error = WebInterfaceError(
|
|
error_code=error_code,
|
|
message=message,
|
|
details=details,
|
|
context=context or {},
|
|
suggested_fixes=suggested_fixes
|
|
)
|
|
|
|
return jsonify(error.to_dict()), status_code
|
|
|
|
|
|
def create_success_response(
|
|
data: Any = None,
|
|
message: Optional[str] = None,
|
|
metadata: Optional[dict] = None
|
|
) -> dict:
|
|
"""
|
|
Create a standardized success response.
|
|
|
|
Args:
|
|
data: Response data
|
|
message: Optional success message
|
|
metadata: Optional metadata (timing, version, etc.)
|
|
|
|
Returns:
|
|
Dictionary for jsonify
|
|
"""
|
|
response = {
|
|
"status": "success"
|
|
}
|
|
|
|
if data is not None:
|
|
response["data"] = data
|
|
|
|
if message:
|
|
response["message"] = message
|
|
|
|
if metadata:
|
|
response["metadata"] = metadata
|
|
|
|
return response
|
|
|