fix: production-readiness hardening from full-codebase review
Apply six isolated, low-risk fixes surfaced by the parallel production-readiness review (backend, frontend, security, perf, UI/UX, bugs+features). Backend - Mask access_token in provider GET responses and drop it on edit when carrying the *** placeholder — fixes plaintext leak of HA long-lived tokens (security H-1). Centralized via PROVIDER_SECRET_FIELDS so all call sites stay in sync (C-5). - Hold HA status-change tasks in a module-level set with a done_callback — asyncio.create_task only keeps weak refs and the task could be GC'd before its row was written (C-1). - Roll back the request session in the Telegram-webhook catch-all so a handler exception cannot leak uncommitted writes into the next request (C-2). - Bail before reading the 1 MiB webhook body when the Gitea provider has no secret configured or the request has no signature header. For the generic webhook with bearer_token auth, verify the Authorization header before the body read. Closes the pre-auth resource-exhaustion amplifier (C-3). Frontend - Add supportsAutoOrganize capability to ProviderDescriptor and consume it from RuleEditor instead of `provider.type !== 'immich'`, bringing the last action-rule editor under CLAUDE.md rule 8 (no provider-type hardcoding in components). - Snackbar: add role="region" + per-toast role/aria-live/aria-atomic so screen readers announce success/error toasts. - Sidebar nav: add aria-current="page" on the active link so the active state has an accessible name. - New snackbar.region key in en + ru (locale parity preserved). Out of scope for this commit (tracked in .claude/reviews/README.md ship-blocker list): secret encryption at rest, JWT cookie move, Alembic adoption, webhook idempotency, deferred-dispatch crash window, persisted Telegram update watermark, bridge_self counter lock — each needs more than a mechanical edit.
This commit is contained in:
@@ -164,17 +164,23 @@ async def gitea_webhook(token: str, request: Request):
|
||||
|
||||
webhook_secret = (provider.config or {}).get("webhook_secret", "")
|
||||
|
||||
# Read raw body for HMAC check
|
||||
raw_body = await _read_bounded_body(request)
|
||||
|
||||
# Bail BEFORE reading the body if either the provider is missing a
|
||||
# secret (admin misconfiguration) or the inbound request has no
|
||||
# signature header. Either way the request can never authenticate,
|
||||
# so there's no reason to spend the 1 MiB body read first.
|
||||
if not webhook_secret:
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail="Webhook secret not configured on this provider",
|
||||
)
|
||||
|
||||
signature = request.headers.get("X-Gitea-Signature", "")
|
||||
if not signature or not _verify_gitea_signature(webhook_secret, raw_body, signature):
|
||||
if not signature:
|
||||
raise HTTPException(status_code=403, detail="Invalid signature")
|
||||
|
||||
# Body needed for the HMAC check — reads at most _MAX_WEBHOOK_BODY_BYTES.
|
||||
raw_body = await _read_bounded_body(request)
|
||||
|
||||
if not _verify_gitea_signature(webhook_secret, raw_body, signature):
|
||||
raise HTTPException(status_code=403, detail="Invalid signature")
|
||||
|
||||
# Parse event header + payload
|
||||
@@ -446,6 +452,18 @@ async def generic_webhook(token: str, request: Request):
|
||||
store_payloads = provider_config.get("store_payloads", True)
|
||||
max_stored = min(max(int(provider_config.get("max_stored_payloads", 20)), 1), 100)
|
||||
|
||||
# Reject misconfigured providers (auth_mode requires a secret but none
|
||||
# set) BEFORE the 1 MiB body read. For non-HMAC modes we can also
|
||||
# verify the credential header up front; HMAC needs the body.
|
||||
auth_mode = provider_config.get("auth_mode", "none")
|
||||
if auth_mode in {"hmac_sha256", "bearer_token"} and not provider_config.get("webhook_secret"):
|
||||
raise HTTPException(status_code=403, detail="Authentication failed")
|
||||
if auth_mode == "bearer_token":
|
||||
auth_header = request.headers.get("Authorization", "")
|
||||
secret = provider_config.get("webhook_secret", "")
|
||||
if not auth_header.startswith("Bearer ") or not hmac.compare_digest(auth_header[7:], secret):
|
||||
raise HTTPException(status_code=403, detail="Authentication failed")
|
||||
|
||||
raw_body = await _read_bounded_body(request)
|
||||
|
||||
# Bounded read above already enforces the size cap; no need to re-check.
|
||||
|
||||
Reference in New Issue
Block a user